Re: [PATCH v2 2/3] dt-bindings: phy: phy-stm32-usbphyc: add optional phy tuning properties

[Rob Herring <robh@kernel.org>]

On Wed, Oct 6, 2021 at 1:51 AM Amelie DELAUNAY
<amelie.delaunay@foss.st.com> wrote:
>
> Hi Rob,
>
> On 10/6/21 12:45 AM, Rob Herring wrote:
> > On Tue, 05 Oct 2021 17:24:52 +0200, Amelie Delaunay wrote:
> >> This patch adds the description of new optional phy tuning properties
> >> for usbphyc phy sub nodes.
> >>
> >> Signed-off-by: Amelie Delaunay <amelie.delaunay@foss.st.com>
> >> ---
> >> Changes in v2:
> >> - st,phy-tuning property removed
> >> - tuning properties are now put directly in each child node
> >> - tuning properties are no more free form text and their name reworked
> >> ---
> >>   .../bindings/phy/phy-stm32-usbphyc.yaml       | 126 ++++++++++++++++++
> >>   1 file changed, 126 insertions(+)
> >>
> >
> > Running 'make dtbs_check' with the schema in this patch gives the
> > following warnings. Consider if they are expected or the schema is
> > incorrect. These may not be new warnings.
> >
> > Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> > This will change in the future.
> >
> > Full log is available here: https://patchwork.ozlabs.org/patch/1536730
> >
> >
> > usbphyc@5a006000: usb-phy@0: 'phy-supply' is a required property
> >       arch/arm/boot/dts/stm32mp157a-icore-stm32mp1-ctouch2.dt.yaml
> >       arch/arm/boot/dts/stm32mp157a-icore-stm32mp1-edimm2.2.dt.yaml
> >       arch/arm/boot/dts/stm32mp157a-microgea-stm32mp1-microdev2.0.dt.yaml
> >       arch/arm/boot/dts/stm32mp157a-microgea-stm32mp1-microdev2.0-of7.dt.yaml
> >       arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml
> >       arch/arm/boot/dts/stm32mp157c-odyssey.dt.yaml
> >
> > usbphyc@5a006000: usb-phy@1: 'phy-supply' is a required property
> >       arch/arm/boot/dts/stm32mp157a-icore-stm32mp1-ctouch2.dt.yaml
> >       arch/arm/boot/dts/stm32mp157a-icore-stm32mp1-edimm2.2.dt.yaml
> >       arch/arm/boot/dts/stm32mp157a-microgea-stm32mp1-microdev2.0.dt.yaml
> >       arch/arm/boot/dts/stm32mp157a-microgea-stm32mp1-microdev2.0-of7.dt.yaml
> >       arch/arm/boot/dts/stm32mp157c-lxa-mc1.dt.yaml
> >       arch/arm/boot/dts/stm32mp157c-odyssey.dt.yaml
> >
>
> These warnings are due to the fact that usbphyc parent node is disabled
> in parent device tree (stm32mp151.dtsi) but not the "usb-phy" child
> nodes. These warnings are not introduced by this current patch "optional
> phy tuning properties" but anyway.

Yes, I said that may be the case.

> The device trees mentioned don't
> enable usbphyc, so they don't have to set phy-supply in usbphyc child nodes.
>
> $ git diff arch/arm/boot/dts/stm32mp151.dtsi
> diff --git a/arch/arm/boot/dts/stm32mp151.dtsi
> b/arch/arm/boot/dts/stm32mp151.dtsi
> index bd289bf5d269..abef8b6be40a 100644
> --- a/arch/arm/boot/dts/stm32mp151.dtsi
> +++ b/arch/arm/boot/dts/stm32mp151.dtsi
> @@ -1507,11 +1507,13 @@ usbphyc: usbphyc@5a006000 {
>                          usbphyc_port0: usb-phy@0 {
>                                  #phy-cells = <0>;
>                                  reg = <0>;
> +                               status = "disabled";
>                          };
>
>                          usbphyc_port1: usb-phy@1 {
>                                  #phy-cells = <1>;
>                                  reg = <1>;
> +                               status = "disabled";
>                          };
>                  };
>
> Disable child nodes while parent node is already disabled fixes the
> warning. But it means to add status = "okay"; in child nodes everywhere
> usbphyc is enabled.

Presumably, you already have to add phy-supply everywhere.

> Is it normal dtbs_check checks in child nodes when parent node is disabled?

I'll look into doing that.

Rob