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

[Vladimir Zapolskiy]

Hi Bryan,

On 9/12/24 18:11, Bryan O'Donoghue wrote:
> On 12/09/2024 13:44, Vladimir Zapolskiy wrote:
> > > csiphy0
> > >
> > > vdda-phy-supply = <&vreg_l2c_0p9>;
> > > vdda-pll-supply = <&vreg_l1c_1p2>;
> > >
> > > This is also the case for csiphy 1/2/4
> > >
> > > So, I _don't_ believe this is work we need to do, since its the same
> > > regulator for each PHY.
> >
> > This is board specific, and even if the separation is not needed on the
> > boards
> > you have just checked, still it may be needed on some boards, which are
> > not yet
> > checked/not yet known.
>
> There is a Power Grid Analysis document which specifies these rails @
> the SoC level and assumes you've used the Qcom PMIC to power, moreover
> the PGA re-uses the same regulator over and over again.
>
> You _could_ provide that power from your own PMIC which provides the
> same voltage range as the Qcom PMIC you haven't used. Even if you did
> provide that from your own PMIC you'd have to provide _separate_ rails
> for the various CSIPHYs before it would be required to have a per PHY
> rail requirement on this SoC.
>
> Are people really powering these SoCs with their own PMICs ?
> No probably not.
>
> Should we add the support for it anyway ?
> Maybe.

To have a set of regulators is a matter of proper IC/IP description, actually
here I see very little option for a divergence or disagreement.

> So to reiterate:
>
> 1. csiphyX-vdda-phy-supply
>      csiphyX-vdda-pll-supply
>
>      In the dts and yaml
>
>      => The names should be generic from the perspective of the driver

As for me I don't care about the particular names, somebody else can care.

> 2. camss.c::csiphy_res_sm8550
>      [0].regulators = { "csiphy0-vdda-phy-supply",
>                         "csiphy0-vdda-pll-supply" }
>      ...
>
>      [N].regulators = { "csiphyN-vdda-phy-supply",
>                         "csiphyN-vdda-pll-supply" }
>
>      => The regulators for the PHY should be defined in the
>         PHY resources description

This is obvious.

> 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.

Hence there shall be no requirement to describe any non-present supplies,
which is a legit case, if there is no connection and usage of the
correspondent non-supplied PHY.

--
Best wishes,
Vladimir