Re: [PATCH 07/13] dt-bindings: media: camss: Add qcom,sm8550-camss binding

[Depeng Shao]

Hi Vladimir, Bryan,

On 9/18/2024 7:16 AM, Vladimir Zapolskiy wrote:
> Hi Bryan,
>
> On 9/18/24 01:40, Bryan O'Donoghue wrote:
> > On 13/09/2024 06:06, Vladimir Zapolskiy wrote:
> > > On 9/13/24 01:41, Bryan O'Donoghue wrote:
> > > > On 12/09/2024 21:57, Vladimir Zapolskiy wrote:
> > > > > > 3. Required not optional in the yaml
> > > > > >
> > > > > >        => You can't use the PHY without its regulators
> > > > >
> > > > > No, the supplies shall be optional, since it's absolutely possible to
> > > > > have
> > > > > such a board, where supplies are merely not connected to the SoC.
> > > >
> > > > For any _used_ PHY both supplies are certainly required.
> > > >
> > > > That's what the yaml/dts check for this should achieve.
> > >
> > > I believe it is technically possible by writing an enormously complex
> > > scheme, when all possible "port" cases and combinations are listed.
> > >
> > > Do you see any simpler way? Do you insist that it is utterly needed?
> >
> > I asked Krzysztof about this offline.
> >
> > He said something like
> >
> > Quote:
> > This is possible, but I think not between child nodes.
> > https://elixir.bootlin.com/linux/v6.11-rc7/source/Documentation/ 
> > devicetree/bindings/example-schema.yaml#L194
> >
> > You could require something in children, but not in parent node. For
> > children something around:
> > https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/ 
> > devicetree/bindings/net/qcom,ipa.yaml#L174
> >
> > allOf:
> >     - if:
> >         required:
> >           - something-in-parent
> >       then:
> >         properties:
> >           child-node:
> >             required:
> >               - something-in-child
> >
> > I will see if I can turn that into a workable proposal/patch.
>
> thank you for pushing my review request forward.
>
> Overall I believe making supply properties as optional ones is sufficient,
> technically straightforward and merely good enough, thus please let me
> ask you to ponder on this particular variant one more time.

So, we are discussing two things.

1# Use separate supplies for each CSI block, looks like there is no 
doubt about it anymore. So, I will update it just like based on suggestion.

csiphyX-vdda-phy-supply
csiphyX-vdda-pll-supply

Then I will need below items in the required list if they are required.
required:
  - csiphy0-vdda-phy-supply
  - csiphy0-vdda-pll-supply
  - csiphy1-vdda-phy-supply
  - csiphy1-vdda-pll-supply
...
  - csiphy7-vdda-phy-supply
  - csiphy7-vdda-pll-supply

2# Regarding the CSI supplies, if they need to be making as optional?
Looks like there is no conclusion now.

@Bryan, do you agree with this?

Thanks,
Depeng