| Message ID | 20231204144429.45197-2-linux.amoon@gmail.com |
|---|---|
| State | Accepted |
| Commit | 3d9894e26e556cd83f1c0ef9f757e375e2cf52c7 |
| Headers | show |
| Series | [v6,1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub | expand |
On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote: > Add the binding example for the USB3.1 Genesys Logic GL3523 > integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed > hub. > > 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. > > GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves > power to the USB HUB, it uses 5V power regulator. > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > V6: fix the description of the regulators > Updated the commit message for regulator updates. > add reviewed by Conor Dooley > [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/ > v5: upgrade peer-hub description : Conor Dooley > [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] > v4: Fix the description of peer-hub and update the commit message. > Schematics of the Odroid N2+ > https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf > V3: fix the dt_binding_check error, added new example for Genesys GL3523 > v2: added Genesys GL3523 binding > v1: none > --- > .../bindings/usb/genesys,gl850g.yaml | 65 +++++++++++++++++-- > 1 file changed, 61 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > index ee08b9c3721f..c6f63a69396d 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@icenowy.me> > > -allOf: > - - $ref: usb-device.yaml# > - > properties: > compatible: > enum: > @@ -27,12 +24,46 @@ 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 > > +allOf: > + - $ref: usb-device.yaml# > + - if: > + properties: > + compatible: > + contains: > + enum: > + - usb5e3,608 > + then: > + properties: > + peer-hub: false > + vdd-supply: false > + reset-gpios: true > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - usb5e3,610 > + - usb5e3,620 > + then: > + properties: > + peer-hub: true > + vdd-supply: true > + reset-gpios: true No need for this if schema. The default is they are allowed. Rob
On 06/12/2023 18:14, Anand Moon wrote: > Hi Rob, > > On Wed, 6 Dec 2023 at 19:23, Rob Herring <robh@kernel.org> wrote: >> >> On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote: >>> Add the binding example for the USB3.1 Genesys Logic GL3523 >>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed >>> hub. >>> >>> 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. >>> >>> GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves >>> power to the USB HUB, it uses 5V power regulator. >>> >>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >>> --- >>> V6: fix the description of the regulators >>> Updated the commit message for regulator updates. >>> add reviewed by Conor Dooley >>> [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/ >>> v5: upgrade peer-hub description : Conor Dooley >>> [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] >>> v4: Fix the description of peer-hub and update the commit message. >>> Schematics of the Odroid N2+ >>> https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf >>> V3: fix the dt_binding_check error, added new example for Genesys GL3523 >>> v2: added Genesys GL3523 binding >>> v1: none >>> --- >>> .../bindings/usb/genesys,gl850g.yaml | 65 +++++++++++++++++-- >>> 1 file changed, 61 insertions(+), 4 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml >>> index ee08b9c3721f..c6f63a69396d 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@icenowy.me> >>> >>> -allOf: >>> - - $ref: usb-device.yaml# >>> - >>> properties: >>> compatible: >>> enum: >>> @@ -27,12 +24,46 @@ 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 >>> >>> +allOf: >>> + - $ref: usb-device.yaml# >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - usb5e3,608 >>> + then: >>> + properties: >>> + peer-hub: false >>> + vdd-supply: false >>> + reset-gpios: true >>> + >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - usb5e3,610 >>> + - usb5e3,620 >>> + then: >>> + properties: >>> + peer-hub: true >>> + vdd-supply: true >>> + reset-gpios: true >> >> No need for this if schema. The default is they are allowed. >> > > 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. Best regards, Krzysztof
Hi Krzysztof On Thu, 7 Dec 2023 at 14:00, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 06/12/2023 18:14, Anand Moon wrote: > > Hi Rob, > > > > On Wed, 6 Dec 2023 at 19:23, Rob Herring <robh@kernel.org> wrote: > >> > >> On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote: > >>> Add the binding example for the USB3.1 Genesys Logic GL3523 > >>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed > >>> hub. > >>> > >>> 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. > >>> > >>> GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves > >>> power to the USB HUB, it uses 5V power regulator. > >>> > >>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> > >>> --- > >>> V6: fix the description of the regulators > >>> Updated the commit message for regulator updates. > >>> add reviewed by Conor Dooley > >>> [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/ > >>> v5: upgrade peer-hub description : Conor Dooley > >>> [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] > >>> v4: Fix the description of peer-hub and update the commit message. > >>> Schematics of the Odroid N2+ > >>> https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf > >>> V3: fix the dt_binding_check error, added new example for Genesys GL3523 > >>> v2: added Genesys GL3523 binding > >>> v1: none > >>> --- > >>> .../bindings/usb/genesys,gl850g.yaml | 65 +++++++++++++++++-- > >>> 1 file changed, 61 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > >>> index ee08b9c3721f..c6f63a69396d 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@icenowy.me> > >>> > >>> -allOf: > >>> - - $ref: usb-device.yaml# > >>> - > >>> properties: > >>> compatible: > >>> enum: > >>> @@ -27,12 +24,46 @@ 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 > >>> > >>> +allOf: > >>> + - $ref: usb-device.yaml# > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + enum: > >>> + - usb5e3,608 > >>> + then: > >>> + properties: > >>> + peer-hub: false > >>> + vdd-supply: false > >>> + reset-gpios: true > >>> + > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + enum: > >>> + - usb5e3,610 > >>> + - usb5e3,620 > >>> + then: > >>> + properties: > >>> + peer-hub: true > >>> + vdd-supply: true > >>> + reset-gpios: true > >> > >> No need for this if schema. The default is they are allowed. > >> > > > > 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. required: - compatible - reg - reset-gpio 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 additionalProperties: false > > Best regards, > Krzysztof > Thanks -Anand
Hi Krzysztof, On Thu, 7 Dec 2023 at 18:11, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 07/12/2023 13:33, Anand Moon wrote: > > Hi Krzysztof > > > > On Thu, 7 Dec 2023 at 14:00, Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> wrote: > >> > >> On 06/12/2023 18:14, Anand Moon wrote: > >>> Hi Rob, > >>> > >>> On Wed, 6 Dec 2023 at 19:23, Rob Herring <robh@kernel.org> wrote: > >>>> > >>>> On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote: > >>>>> Add the binding example for the USB3.1 Genesys Logic GL3523 > >>>>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed > >>>>> hub. > >>>>> > >>>>> 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. > >>>>> > >>>>> GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves > >>>>> power to the USB HUB, it uses 5V power regulator. > >>>>> > >>>>> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > >>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> > >>>>> --- > >>>>> V6: fix the description of the regulators > >>>>> Updated the commit message for regulator updates. > >>>>> add reviewed by Conor Dooley > >>>>> [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@gmail.com/ > >>>>> v5: upgrade peer-hub description : Conor Dooley > >>>>> [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] > >>>>> v4: Fix the description of peer-hub and update the commit message. > >>>>> Schematics of the Odroid N2+ > >>>>> https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf > >>>>> V3: fix the dt_binding_check error, added new example for Genesys GL3523 > >>>>> v2: added Genesys GL3523 binding > >>>>> v1: none > >>>>> --- > >>>>> .../bindings/usb/genesys,gl850g.yaml | 65 +++++++++++++++++-- > >>>>> 1 file changed, 61 insertions(+), 4 deletions(-) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > >>>>> index ee08b9c3721f..c6f63a69396d 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@icenowy.me> > >>>>> > >>>>> -allOf: > >>>>> - - $ref: usb-device.yaml# > >>>>> - > >>>>> properties: > >>>>> compatible: > >>>>> enum: > >>>>> @@ -27,12 +24,46 @@ 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 > >>>>> > >>>>> +allOf: > >>>>> + - $ref: usb-device.yaml# > >>>>> + - if: > >>>>> + properties: > >>>>> + compatible: > >>>>> + contains: > >>>>> + enum: > >>>>> + - usb5e3,608 > >>>>> + then: > >>>>> + properties: > >>>>> + peer-hub: false > >>>>> + vdd-supply: false > >>>>> + reset-gpios: true > >>>>> + > >>>>> + - if: > >>>>> + properties: > >>>>> + compatible: > >>>>> + contains: > >>>>> + enum: > >>>>> + - usb5e3,610 > >>>>> + - usb5e3,620 > >>>>> + then: > >>>>> + properties: > >>>>> + peer-hub: true > >>>>> + vdd-supply: true > >>>>> + reset-gpios: true > >>>> > >>>> No need for this if schema. The default is they are allowed. > >>>> > >>> > >>> 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 I have verified that example Documentation/devicetree/bindings/usb/genesys,gl850g.example.dts generate is correct required: - compatible - reg - reset-gpio - peer-hub - vdd-supply 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 additionalProperties: false > Best regards, > Krzysztof > Thanks -Anand
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? Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml index ee08b9c3721f..c6f63a69396d 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@icenowy.me> -allOf: - - $ref: usb-device.yaml# - properties: compatible: enum: @@ -27,12 +24,46 @@ 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 +allOf: + - $ref: usb-device.yaml# + - if: + properties: + compatible: + contains: + enum: + - usb5e3,608 + then: + properties: + peer-hub: false + vdd-supply: false + reset-gpios: true + + - if: + properties: + compatible: + contains: + enum: + - usb5e3,610 + - usb5e3,620 + then: + properties: + peer-hub: true + vdd-supply: true + reset-gpios: true + additionalProperties: false examples: @@ -49,3 +80,29 @@ examples: reset-gpios = <&pio 7 2 GPIO_ACTIVE_LOW>; }; }; + + - | + #include <dt-bindings/gpio/gpio.h> + usb { + dr_mode = "host"; + #address-cells = <1>; + #size-cells = <0>; + + /* 2.0 hub on port 1 */ + hub_2_0: hub@1 { + compatible = "usb5e3,610"; + reg = <1>; + peer-hub = <&hub_3_0>; + reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>; + vdd-supply = <&vcc_5v>; + }; + + /* 3.1 hub on port 4 */ + hub_3_0: hub@2 { + compatible = "usb5e3,620"; + reg = <2>; + peer-hub = <&hub_2_0>; + reset-gpios = <&gpio 20 GPIO_ACTIVE_LOW>; + vdd-supply = <&vcc_5v>; + }; + };