Re: [PATCH v6 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub
From: Anand Moon
Date: Tue Dec 12 2023 - 06:38:15 EST
Hi Krzysztof,
On Tue, 12 Dec 2023 at 14:10, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 12/12/2023 07:22, Anand Moon wrote:
> > Hi Krzysztof,
> >
> > On Fri, 8 Dec 2023 at 17:47, Krzysztof Kozlowski
> > <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >>
> >> On 08/12/2023 12:19, Anand Moon wrote:
> >>> Hi Krzysztof,
> >>>
> >>> On Fri, 8 Dec 2023 at 13:14, Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >>>>
> >>>> On 08/12/2023 01:24, Anand Moon wrote:
> >>>>>>>>>
> >>>>>>>>> If I move reset-gpios to required, I observe the below warning.
> >>>>>>>>>
> >>>>>>>>> DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb
> >>>>>>>>> /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
> >>>>>>>>> hub@1: 'reset-gpio' is a required property
> >>>>>>>>> from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
> >>>>>>>>
> >>>>>>>> Where are the properties defined? If you open the binding you see:
> >>>>>>>> nowhere. You cannot define properties in some variant with "true".
> >>>>>>>> Please define all of them in top-level and only narrow/constrain when
> >>>>>>>> applicable.
> >>>>>>>>
> >>>>>>> What I meant is the example below, required meant applicable for both
> >>>>>>> the binding
> >>>>>>> But it shows me the above warning.
> >>>>>>
> >>>>>> My explanation stands... So again:
> >>>>>>
> >>>>>>>> Please define all of them in top-level and only narrow/constrain when
> >>>>>>>> applicable.
> >>>>>>
> >>>>> Apologies, But I have tried this multiple times but have not been able
> >>>>> to fix the device tree warning
> >>>>
> >>>> Did you document all properties in top-level "properties:" block?
> >>>>
> >>> Yes, I have,
> >>>
> >>> Can you suggest a couple of examples to follow?
> >>> I looked at some of the YAML files but could not fix my issue.
> >>
> >> 99% of bindings. Look also at example-schema.
> >>
> >> You can also attach here complete patch for fast look / short review.
> >>
> >
> > Please find the modified patch, I have tried a few things but none
> > resolve the binding warning.
> > I am not able to debug this.
>
> And where is the warning or the issue? Can you describe what problem do
> you have?
Here is the list of warnings I observed with this patch
DTC_CHK Documentation/devicetree/bindings/usb/nvidia,tegra186-xusb.example.dtb
/home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
hub@1: 'vdd-supply' is a required property
from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
hub@1: 'reset-gpios' is a required property
from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb:
hub@1: 'peer-hub' is a required property
from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
DTC_CHK Documentation/devicetree/bindings/usb/ti,tps6598x.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/xlnx,usb2.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/renesas,usb-xhci.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/renesas,usbhs.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/brcm,bcm7445-ehci.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/cypress,cypd4226.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/cdns,usb3.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/renesas,rzn1-usbf.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/ci-hdrc-usb2.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/brcm,usb-pinmap.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/renesas,usb3-peri.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/qcom,pmic-typec.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/microchip,usb5744.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/smsc,usb3503.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/mediatek,musb.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/vialab,vl817.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/marvell,pxau2o-ehci.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/richtek,rt1711h.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/usb.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/realtek,rts5411.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/nvidia,tegra210-xusb.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/realtek,rtd-dwc3.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/usb-drd.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/genesys,gl850g.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/ti,j721e-usb.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/ti,am62-usb.example.dtb
/home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/genesys,gl850g.example.dtb:
hub@1: 'vdd-supply' is a required property
from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/genesys,gl850g.example.dtb:
hub@1: 'peer-hub' is a required property
from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
DTC_CHK Documentation/devicetree/bindings/usb/renesas,rzv2m-usb3drd.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/usb-hcd.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/fsl,imx8mq-dwc3.example.dtb
DTC_CHK Documentation/devicetree/bindings/usb/mediatek,mtu3.example.dtb
/home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb:
hub@1: 'vdd-supply' is a required property
from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb:
hub@1: 'reset-gpios' is a required property
from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
/home/amoon/mainline/linux-amlogic-6.y-devel/Documentation/devicetree/bindings/usb/usb-hcd.example.dtb:
hub@1: 'peer-hub' is a required property
from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml#
DTC_CHK Documentation/devicetree/bindings/usb/nxp,isp1760.example.dtb
>
> >
> > -Thanks
> > Anand
> > -----8<----------8<----------8<----------8<----------8<----------8<-----
> > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > index ee08b9c3721f..7f75fa3c1945 100644
> > --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > @@ -9,9 +9,6 @@ title: Genesys Logic USB hub controller
> > maintainers:
> > - Icenowy Zheng <uwu@xxxxxxxxxx>
> >
> > -allOf:
> > - - $ref: usb-device.yaml#
> > -
> > properties:
> > compatible:
> > enum:
> > @@ -27,11 +24,47 @@ properties:
> >
> > vdd-supply:
> > description:
> > - the regulator that provides 3.3V core power to the hub.
> > + The regulator that provides 3.3V or 5.0V core power to the hub.
> > +
> > + peer-hub:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description:
> > + For onboard hub controllers that support USB 3.x and USB 2.0 hubs with
> > + shared resets and power supplies, this property is used to identify the
> > + hubs with which these are shared.
> >
> > required:
> > - compatible
> > - reg
> > + - vdd-supply
> > + - reset-gpios
> > + - peer-hub
> > +
> > +allOf:
> > + - $ref: usb-device.yaml#
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - usb5e3,608
> > + then:
> > + properties:
> > + peer-hub: false
> > + vdd-supply: false
> > +
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - usb5e3,610
> > + - usb5e3,620
> > + then:
> > + properties:
> > + peer-hub: true
> > + vdd-supply: true
>
> Drop this if:, redundant.
>
No, this does not resolve the above issue.
>
Thanks
-Anand