Quoting Douglas Anderson (2022-04-25 14:06:42)
> We're supposed to list the supplies in the dt bindings but there are
> none in the DP controller bindings. Looking at the Linux driver and
> existing device trees, we can see that two supplies are expected:
> - vdda-0p9-supply
> - vdda-1p2-supply
>
> Let's list them both in the bindings. Note that the datasheet for
> sc7280 doesn't describe these supplies very verbosely. For the 0p9
> supply, for instance, it says "Power for eDP 0.9 V circuits". This
> this is obvious from the property name, we don't bother cluttering the
> bindings with a description.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
>
> .../devicetree/bindings/display/msm/dp-controller.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> index cd05cfd76536..dba31108db51 100644
> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> @@ -76,6 +76,9 @@ properties:
> "#sound-dai-cells":
> const: 0
>
> + vdda-0p9-supply: true
> + vdda-1p2-supply: true
> +
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
> properties:
> @@ -137,6 +140,9 @@ examples:
>
> power-domains = <&rpmhpd SC7180_CX>;
>
> + vdda-0p9-supply = <&vdda_usb_ss_dp_core>;
Having 'a' in 'vdda' typically means 'analog' for 'analog' circuits, so
I'd expect this to only matter for the phy that contains the analog
circuitry. It would be great to remove the regulator code from
drivers/gpu/drm/msm/dp/dp_power.c and move the regulator_set_load() call
to the phy driver if possible. Hopefully qcom folks can help clarify
here.
> + vdda-1p2-supply = <&vdda_usb_ss_dp_1p2>;
Hi,
On Mon, Apr 25, 2022 at 2:14 PM Stephen Boyd <[email protected]> wrote:
>
> Quoting Douglas Anderson (2022-04-25 14:06:42)
> > We're supposed to list the supplies in the dt bindings but there are
> > none in the DP controller bindings. Looking at the Linux driver and
> > existing device trees, we can see that two supplies are expected:
> > - vdda-0p9-supply
> > - vdda-1p2-supply
> >
> > Let's list them both in the bindings. Note that the datasheet for
> > sc7280 doesn't describe these supplies very verbosely. For the 0p9
> > supply, for instance, it says "Power for eDP 0.9 V circuits". This
> > this is obvious from the property name, we don't bother cluttering the
> > bindings with a description.
> >
> > Signed-off-by: Douglas Anderson <[email protected]>
> > ---
> >
> > .../devicetree/bindings/display/msm/dp-controller.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > index cd05cfd76536..dba31108db51 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > @@ -76,6 +76,9 @@ properties:
> > "#sound-dai-cells":
> > const: 0
> >
> > + vdda-0p9-supply: true
> > + vdda-1p2-supply: true
> > +
> > ports:
> > $ref: /schemas/graph.yaml#/properties/ports
> > properties:
> > @@ -137,6 +140,9 @@ examples:
> >
> > power-domains = <&rpmhpd SC7180_CX>;
> >
> > + vdda-0p9-supply = <&vdda_usb_ss_dp_core>;
>
> Having 'a' in 'vdda' typically means 'analog' for 'analog' circuits, so
> I'd expect this to only matter for the phy that contains the analog
> circuitry. It would be great to remove the regulator code from
> drivers/gpu/drm/msm/dp/dp_power.c and move the regulator_set_load() call
> to the phy driver if possible. Hopefully qcom folks can help clarify
> here.
Interesting. Oddly enough, the sc7280 datasheet doesn't list the "_A".
It calls these "VDD_VREF_0P9" and "VDD_VREF_1P2". However, on the
schematic in front of me someone labeled these pins on the sc7280 with
the "A". ...and the driver looks for a supply with the "a". :-/
It would be good to get clarification from someone with better information.
-Doug
Hi Doug,
>>
>> Quoting Douglas Anderson (2022-04-25 14:06:42)
>> > We're supposed to list the supplies in the dt bindings but there are
>> > none in the DP controller bindings. Looking at the Linux driver and
>> > existing device trees, we can see that two supplies are expected:
>> > - vdda-0p9-supply
>> > - vdda-1p2-supply
>> >
>> > Let's list them both in the bindings. Note that the datasheet for
>> > sc7280 doesn't describe these supplies very verbosely. For the 0p9
>> > supply, for instance, it says "Power for eDP 0.9 V circuits". This
>> > this is obvious from the property name, we don't bother cluttering
>> > the bindings with a description.
>> >
>> > Signed-off-by: Douglas Anderson <[email protected]>
>> > ---
>> >
>> > .../devicetree/bindings/display/msm/dp-controller.yaml | 6 ++++++
>> > 1 file changed, 6 insertions(+)
>> >
>> > diff --git
>> > a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>> > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>> > index cd05cfd76536..dba31108db51 100644
>> > ---
>> > a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>> > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.ya
>> > +++ ml
>> > @@ -76,6 +76,9 @@ properties:
>> > "#sound-dai-cells":
>> > const: 0
>> >
>> > + vdda-0p9-supply: true
>> > + vdda-1p2-supply: true
>> > +
>> > ports:
>> > $ref: /schemas/graph.yaml#/properties/ports
>> > properties:
>> > @@ -137,6 +140,9 @@ examples:
>> >
>> > power-domains = <&rpmhpd SC7180_CX>;
>> >
>> > + vdda-0p9-supply = <&vdda_usb_ss_dp_core>;
>>
>> Having 'a' in 'vdda' typically means 'analog' for 'analog' circuits,
>> so I'd expect this to only matter for the phy that contains the analog
>> circuitry. It would be great to remove the regulator code from
>> drivers/gpu/drm/msm/dp/dp_power.c and move the regulator_set_load()
>> call to the phy driver if possible. Hopefully qcom folks can help
>> clarify here.
>
>Interesting. Oddly enough, the sc7280 datasheet doesn't list the "_A".
>It calls these "VDD_VREF_0P9" and "VDD_VREF_1P2". However, on the
>schematic in front of me someone labeled these pins on the sc7280 with the
>"A". ...and the driver looks for a supply with the "a". :-/
>
>It would be good to get clarification from someone with better information.
>
>-Doug
Our internal power grid documents list the regulators as VDD_A_*_1P2 and VDD_A_*_0P9
for all the platforms.
So, as a practice, we put the same name in the DT files. Hence,
Reviewed-by: Sankeerth Billakanti <[email protected]>
Thank you,
Sankeerth