* [PATCH v4 0/2] Use onboard usb hub reset on Odroid N2
@ 2023-11-22 18:23 Anand Moon
2023-11-22 18:23 ` Anand Moon
2023-11-22 18:23 ` Anand Moon
0 siblings, 2 replies; 14+ messages in thread
From: Anand Moon @ 2023-11-22 18:23 UTC (permalink / raw)
Cc: Anand Moon, Neil Armstrong, linux-amlogic
Add new binding for Genesys GL3523
Fix the dt_binding_check error
Fix commit message and peer-hub description
Previous:
v1: https://lore.kernel.org/r/20231010032446.3194-1-linux.amoon@gmail.com
v2: https://lore.kernel.org/all/169700589265.2911018.855796610271678067.robh@kernel.org/
v3: https://lore.kernel.org/all/20231119023454.1591-2-linux.amoon@gmail.com/
Thanks
-Anand
Anand Moon (2):
dt-bindings: usb: Add the binding example for the Genesys Logic GL3523
hub
arm64: dts: amlogic: Used onboard usb hub reset on odroid n2
.../bindings/usb/genesys,gl850g.yaml | 67 +++++++++++++++++--
.../dts/amlogic/meson-g12b-odroid-n2.dtsi | 36 ++++++----
2 files changed, 87 insertions(+), 16 deletions(-)
--
2.42.0
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub 2023-11-22 18:23 [PATCH v4 0/2] Use onboard usb hub reset on Odroid N2 Anand Moon @ 2023-11-22 18:23 ` Anand Moon 2023-11-22 18:23 ` Anand Moon 1 sibling, 0 replies; 14+ messages in thread From: Anand Moon @ 2023-11-22 18:23 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng Cc: Anand Moon, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel 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. Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. which has a common reset pin and power supply. peer-hub phandle each peer controller with proper gpio reset and help each peer power on during initialization and power off during suspend. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- 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 | 67 +++++++++++++++++-- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: vdd-supply: description: - the regulator that provides 3.3V core power to the hub. + phandle to the regulator that provides power to the hub. + + peer-hub: + $ref: /schemas/types.yaml#/definitions/phandle + description: + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. + which has a common reset pin and power supply. + peer-hub phandle each peer controller with proper gpio reset + and help each peer power on during initialization + and power off during suspend. 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 +82,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>; + }; + }; -- 2.42.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub @ 2023-11-22 18:23 ` Anand Moon 0 siblings, 0 replies; 14+ messages in thread From: Anand Moon @ 2023-11-22 18:23 UTC (permalink / raw) To: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng Cc: Anand Moon, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel 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. Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. which has a common reset pin and power supply. peer-hub phandle each peer controller with proper gpio reset and help each peer power on during initialization and power off during suspend. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- 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 | 67 +++++++++++++++++-- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: vdd-supply: description: - the regulator that provides 3.3V core power to the hub. + phandle to the regulator that provides power to the hub. + + peer-hub: + $ref: /schemas/types.yaml#/definitions/phandle + description: + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. + which has a common reset pin and power supply. + peer-hub phandle each peer controller with proper gpio reset + and help each peer power on during initialization + and power off during suspend. 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 +82,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>; + }; + }; -- 2.42.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub 2023-11-22 18:23 ` Anand Moon @ 2023-11-23 17:56 ` Conor Dooley -1 siblings, 0 replies; 14+ messages in thread From: Conor Dooley @ 2023-11-23 17:56 UTC (permalink / raw) To: Anand Moon Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 4446 bytes --] On Wed, Nov 22, 2023 at 11:53:46PM +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. > > Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > which has a common reset pin and power supply. > peer-hub phandle each peer controller with proper gpio reset > and help each peer power on during initialization > and power off during suspend. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > 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 | 67 +++++++++++++++++-- > 1 file changed, 63 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: > > vdd-supply: > description: > - the regulator that provides 3.3V core power to the hub. > + phandle to the regulator that provides power to the hub. > + > + peer-hub: Should the property not be "peer-controller"? Your description refers to them as such. > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > + which has a common reset pin and power supply. > + peer-hub phandle each peer controller with proper gpio reset > + and help each peer power on during initialization > + and power off during suspend. I generally hate to talk about non-native speakers grammar etc, but what you have here is in need of a lot of improvement. The below is my attempt to understand what you are trying to say: "For onboard hubs that support USB 3.x and USB 2.0 controllers with shared resets and power supplies, this property is used to identify the controllers with which these are shared." Also - this is one particular system, what prevents there being a hub that has more than 2 controllers? Also, as you insist that this is generic, and not just for genesys, should this not be defined in a common location? Cheers, Conor. > > 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 +82,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>; > + }; > + }; > -- > 2.42.0 > [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 167 bytes --] _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub @ 2023-11-23 17:56 ` Conor Dooley 0 siblings, 0 replies; 14+ messages in thread From: Conor Dooley @ 2023-11-23 17:56 UTC (permalink / raw) To: Anand Moon Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 4446 bytes --] On Wed, Nov 22, 2023 at 11:53:46PM +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. > > Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > which has a common reset pin and power supply. > peer-hub phandle each peer controller with proper gpio reset > and help each peer power on during initialization > and power off during suspend. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > 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 | 67 +++++++++++++++++-- > 1 file changed, 63 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: > > vdd-supply: > description: > - the regulator that provides 3.3V core power to the hub. > + phandle to the regulator that provides power to the hub. > + > + peer-hub: Should the property not be "peer-controller"? Your description refers to them as such. > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > + which has a common reset pin and power supply. > + peer-hub phandle each peer controller with proper gpio reset > + and help each peer power on during initialization > + and power off during suspend. I generally hate to talk about non-native speakers grammar etc, but what you have here is in need of a lot of improvement. The below is my attempt to understand what you are trying to say: "For onboard hubs that support USB 3.x and USB 2.0 controllers with shared resets and power supplies, this property is used to identify the controllers with which these are shared." Also - this is one particular system, what prevents there being a hub that has more than 2 controllers? Also, as you insist that this is generic, and not just for genesys, should this not be defined in a common location? Cheers, Conor. > > 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 +82,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>; > + }; > + }; > -- > 2.42.0 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub 2023-11-23 17:56 ` Conor Dooley @ 2023-11-24 10:48 ` Anand Moon -1 siblings, 0 replies; 14+ messages in thread From: Anand Moon @ 2023-11-24 10:48 UTC (permalink / raw) To: Conor Dooley Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel Hi Conor On Thu, 23 Nov 2023 at 23:26, Conor Dooley <conor@kernel.org> wrote: > > On Wed, Nov 22, 2023 at 11:53:46PM +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. > > > > Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > which has a common reset pin and power supply. > > peer-hub phandle each peer controller with proper gpio reset > > and help each peer power on during initialization > > and power off during suspend. > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > 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 | 67 +++++++++++++++++-- > > 1 file changed, 63 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > > index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: > > > > vdd-supply: > > description: > > - the regulator that provides 3.3V core power to the hub. > > + phandle to the regulator that provides power to the hub. > > + > > + peer-hub: > > Should the property not be "peer-controller"? Your description refers to > them as such. No, as per my understanding, peer-hub represents a complete USB hub. See the lock diagram in the below link. > > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: > > + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > > + which has a common reset pin and power supply. > > + peer-hub phandle each peer controller with proper gpio reset > > + and help each peer power on during initialization > > + and power off during suspend. > > I generally hate to talk about non-native speakers grammar etc, but what > you have here is in need of a lot of improvement. The below is my > attempt to understand what you are trying to say: > > "For onboard hubs that support USB 3.x and USB 2.0 controllers with > shared resets and power supplies, this property is used to identify > the controllers with which these are shared." > Sorry for the poor grammar, I will update this in the next v5. > Also - this is one particular system, what prevents there being a hub > that has more than 2 controllers? Also, as you insist that this is > generic, and not just for genesys, should this not be defined in a > common location? Here is the block diagram of the Genesys GL3523 hub. [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] It has two USB 2.0 and USB 3.1 controllers, so using peer-hub node the onboard hub module will bring up this hub. There are many examples that use similar properties hence it is generic. # Documentation/devicetree/bindings/usb/cypress,hx3.yaml # Documentation/devicetree/bindings/usb/microchip,usb5744.yaml # Documentation/devicetree/bindings/usb/realtek,rts5411.yaml # Documentation/devicetree/bindings/usb/ti,usb8041.yaml # Documentation/devicetree/bindings/usb/vialab,vl817.yaml > > Cheers, > Conor. > Thanks -Anand _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub @ 2023-11-24 10:48 ` Anand Moon 0 siblings, 0 replies; 14+ messages in thread From: Anand Moon @ 2023-11-24 10:48 UTC (permalink / raw) To: Conor Dooley Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel Hi Conor On Thu, 23 Nov 2023 at 23:26, Conor Dooley <conor@kernel.org> wrote: > > On Wed, Nov 22, 2023 at 11:53:46PM +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. > > > > Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > which has a common reset pin and power supply. > > peer-hub phandle each peer controller with proper gpio reset > > and help each peer power on during initialization > > and power off during suspend. > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > --- > > 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 | 67 +++++++++++++++++-- > > 1 file changed, 63 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > > index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: > > > > vdd-supply: > > description: > > - the regulator that provides 3.3V core power to the hub. > > + phandle to the regulator that provides power to the hub. > > + > > + peer-hub: > > Should the property not be "peer-controller"? Your description refers to > them as such. No, as per my understanding, peer-hub represents a complete USB hub. See the lock diagram in the below link. > > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: > > + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > > + which has a common reset pin and power supply. > > + peer-hub phandle each peer controller with proper gpio reset > > + and help each peer power on during initialization > > + and power off during suspend. > > I generally hate to talk about non-native speakers grammar etc, but what > you have here is in need of a lot of improvement. The below is my > attempt to understand what you are trying to say: > > "For onboard hubs that support USB 3.x and USB 2.0 controllers with > shared resets and power supplies, this property is used to identify > the controllers with which these are shared." > Sorry for the poor grammar, I will update this in the next v5. > Also - this is one particular system, what prevents there being a hub > that has more than 2 controllers? Also, as you insist that this is > generic, and not just for genesys, should this not be defined in a > common location? Here is the block diagram of the Genesys GL3523 hub. [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] It has two USB 2.0 and USB 3.1 controllers, so using peer-hub node the onboard hub module will bring up this hub. There are many examples that use similar properties hence it is generic. # Documentation/devicetree/bindings/usb/cypress,hx3.yaml # Documentation/devicetree/bindings/usb/microchip,usb5744.yaml # Documentation/devicetree/bindings/usb/realtek,rts5411.yaml # Documentation/devicetree/bindings/usb/ti,usb8041.yaml # Documentation/devicetree/bindings/usb/vialab,vl817.yaml > > Cheers, > Conor. > Thanks -Anand ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub 2023-11-24 10:48 ` Anand Moon @ 2023-11-24 12:25 ` Conor Dooley -1 siblings, 0 replies; 14+ messages in thread From: Conor Dooley @ 2023-11-24 12:25 UTC (permalink / raw) To: Anand Moon Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 5216 bytes --] On Fri, Nov 24, 2023 at 04:18:23PM +0530, Anand Moon wrote: > Hi Conor > > On Thu, 23 Nov 2023 at 23:26, Conor Dooley <conor@kernel.org> wrote: > > > > On Wed, Nov 22, 2023 at 11:53:46PM +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. > > > > > > Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > which has a common reset pin and power supply. > > > peer-hub phandle each peer controller with proper gpio reset > > > and help each peer power on during initialization > > > and power off during suspend. > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > --- > > > 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 | 67 +++++++++++++++++-- > > > 1 file changed, 63 insertions(+), 4 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > > > index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: > > > > > > vdd-supply: > > > description: > > > - the regulator that provides 3.3V core power to the hub. > > > + phandle to the regulator that provides power to the hub. > > > + > > > + peer-hub: > > > > Should the property not be "peer-controller"? Your description refers to > > them as such. > > No, as per my understanding, peer-hub represents a complete USB hub. > See the lock diagram in the below link. > > > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: > > > + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > > > > > + which has a common reset pin and power supply. > > > + peer-hub phandle each peer controller with proper gpio reset This is what I don't get. You say "peer-hub phandle each peer controller..". It is hard for me to understand that portion of the sentence, but the interchanging of "hub" and "controller" is confusing. The title of the binding says "hub controller", so maybe it is better to use that here. > > > + and help each peer power on during initialization > > > + and power off during suspend. > > > > I generally hate to talk about non-native speakers grammar etc, but what > > you have here is in need of a lot of improvement. The below is my > > attempt to understand what you are trying to say: > > > > "For onboard hubs that support USB 3.x and USB 2.0 controllers with > > shared resets and power supplies, this property is used to identify > > the controllers with which these are shared." "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." I re-worded this again to try and remove the use of "controller". Do you think that this still makes sense? > Sorry for the poor grammar, I will update this in the next v5. > > > Also - this is one particular system, what prevents there being a hub > > that has more than 2 controllers? Also, as you insist that this is > > generic, and not just for genesys, should this not be defined in a > > common location? > > Here is the block diagram of the Genesys GL3523 hub. > [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] > > It has two USB 2.0 and USB 3.1 controllers, so using peer-hub node > the onboard hub module will bring up this hub. > > There are many examples that use similar properties hence it is generic. > > # Documentation/devicetree/bindings/usb/cypress,hx3.yaml > # Documentation/devicetree/bindings/usb/microchip,usb5744.yaml > # Documentation/devicetree/bindings/usb/realtek,rts5411.yaml > # Documentation/devicetree/bindings/usb/ti,usb8041.yaml > # Documentation/devicetree/bindings/usb/vialab,vl817.yaml Which brings me back to the unanswered question, should this not be defined in a common location given there are several devices using it? I assume because it only applies to hub controllers and not other types of devices. Also, the descriptions that I saw when looking at some of those other bindings are similarly poor. I can't bring myself to care any more, just clean up the ambiguous wording here and I'll ack the next version, I don't expect you to sort out the wording in other bindings. Cheers, Conor. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 167 bytes --] _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub @ 2023-11-24 12:25 ` Conor Dooley 0 siblings, 0 replies; 14+ messages in thread From: Conor Dooley @ 2023-11-24 12:25 UTC (permalink / raw) To: Anand Moon Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 5216 bytes --] On Fri, Nov 24, 2023 at 04:18:23PM +0530, Anand Moon wrote: > Hi Conor > > On Thu, 23 Nov 2023 at 23:26, Conor Dooley <conor@kernel.org> wrote: > > > > On Wed, Nov 22, 2023 at 11:53:46PM +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. > > > > > > Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > which has a common reset pin and power supply. > > > peer-hub phandle each peer controller with proper gpio reset > > > and help each peer power on during initialization > > > and power off during suspend. > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > --- > > > 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 | 67 +++++++++++++++++-- > > > 1 file changed, 63 insertions(+), 4 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > > > index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: > > > > > > vdd-supply: > > > description: > > > - the regulator that provides 3.3V core power to the hub. > > > + phandle to the regulator that provides power to the hub. > > > + > > > + peer-hub: > > > > Should the property not be "peer-controller"? Your description refers to > > them as such. > > No, as per my understanding, peer-hub represents a complete USB hub. > See the lock diagram in the below link. > > > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: > > > + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > > > > > + which has a common reset pin and power supply. > > > + peer-hub phandle each peer controller with proper gpio reset This is what I don't get. You say "peer-hub phandle each peer controller..". It is hard for me to understand that portion of the sentence, but the interchanging of "hub" and "controller" is confusing. The title of the binding says "hub controller", so maybe it is better to use that here. > > > + and help each peer power on during initialization > > > + and power off during suspend. > > > > I generally hate to talk about non-native speakers grammar etc, but what > > you have here is in need of a lot of improvement. The below is my > > attempt to understand what you are trying to say: > > > > "For onboard hubs that support USB 3.x and USB 2.0 controllers with > > shared resets and power supplies, this property is used to identify > > the controllers with which these are shared." "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." I re-worded this again to try and remove the use of "controller". Do you think that this still makes sense? > Sorry for the poor grammar, I will update this in the next v5. > > > Also - this is one particular system, what prevents there being a hub > > that has more than 2 controllers? Also, as you insist that this is > > generic, and not just for genesys, should this not be defined in a > > common location? > > Here is the block diagram of the Genesys GL3523 hub. > [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] > > It has two USB 2.0 and USB 3.1 controllers, so using peer-hub node > the onboard hub module will bring up this hub. > > There are many examples that use similar properties hence it is generic. > > # Documentation/devicetree/bindings/usb/cypress,hx3.yaml > # Documentation/devicetree/bindings/usb/microchip,usb5744.yaml > # Documentation/devicetree/bindings/usb/realtek,rts5411.yaml > # Documentation/devicetree/bindings/usb/ti,usb8041.yaml > # Documentation/devicetree/bindings/usb/vialab,vl817.yaml Which brings me back to the unanswered question, should this not be defined in a common location given there are several devices using it? I assume because it only applies to hub controllers and not other types of devices. Also, the descriptions that I saw when looking at some of those other bindings are similarly poor. I can't bring myself to care any more, just clean up the ambiguous wording here and I'll ack the next version, I don't expect you to sort out the wording in other bindings. Cheers, Conor. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub 2023-11-24 12:25 ` Conor Dooley @ 2023-11-24 16:52 ` Anand Moon -1 siblings, 0 replies; 14+ messages in thread From: Anand Moon @ 2023-11-24 16:52 UTC (permalink / raw) To: Conor Dooley Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel Hi Conor, On Fri, 24 Nov 2023 at 17:55, Conor Dooley <conor@kernel.org> wrote: > > On Fri, Nov 24, 2023 at 04:18:23PM +0530, Anand Moon wrote: > > Hi Conor > > > > On Thu, 23 Nov 2023 at 23:26, Conor Dooley <conor@kernel.org> wrote: > > > > > > On Wed, Nov 22, 2023 at 11:53:46PM +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. > > > > > > > > Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > > which has a common reset pin and power supply. > > > > peer-hub phandle each peer controller with proper gpio reset > > > > and help each peer power on during initialization > > > > and power off during suspend. > > > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > 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 | 67 +++++++++++++++++-- > > > > 1 file changed, 63 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > > > > index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: > > > > > > > > vdd-supply: > > > > description: > > > > - the regulator that provides 3.3V core power to the hub. > > > > + phandle to the regulator that provides power to the hub. > > > > + > > > > + peer-hub: > > > > > > Should the property not be "peer-controller"? Your description refers to > > > them as such. > > > > No, as per my understanding, peer-hub represents a complete USB hub. > > See the lock diagram in the below link. > > > > > > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > > + description: > > > > + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > > > > > > > > + which has a common reset pin and power supply. > > > > + peer-hub phandle each peer controller with proper gpio reset > > This is what I don't get. You say "peer-hub phandle each peer > controller..". It is hard for me to understand that portion of the > sentence, but the interchanging of "hub" and "controller" is > confusing. The title of the binding says "hub controller", so maybe it > is better to use that here. > > > > > + and help each peer power on during initialization > > > > + and power off during suspend. > > > > > > I generally hate to talk about non-native speakers grammar etc, but what > > > you have here is in need of a lot of improvement. The below is my > > > attempt to understand what you are trying to say: > > > > > > "For onboard hubs that support USB 3.x and USB 2.0 controllers with > > > shared resets and power supplies, this property is used to identify > > > the controllers with which these are shared." > > "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." > Thanks for your review comments. Ok will update this in the next version. > I re-worded this again to try and remove the use of "controller". > Do you think that this still makes sense? > > > Sorry for the poor grammar, I will update this in the next v5. > > > > > Also - this is one particular system, what prevents there being a hub > > > that has more than 2 controllers? Also, as you insist that this is > > > generic, and not just for genesys, should this not be defined in a > > > common location? > > > > Here is the block diagram of the Genesys GL3523 hub. > > [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] > > > > It has two USB 2.0 and USB 3.1 controllers, so using peer-hub node > > the onboard hub module will bring up this hub. > > > > There are many examples that use similar properties hence it is generic. > > > > # Documentation/devicetree/bindings/usb/cypress,hx3.yaml > > # Documentation/devicetree/bindings/usb/microchip,usb5744.yaml > > # Documentation/devicetree/bindings/usb/realtek,rts5411.yaml > > # Documentation/devicetree/bindings/usb/ti,usb8041.yaml > > # Documentation/devicetree/bindings/usb/vialab,vl817.yaml > > Which brings me back to the unanswered question, should this not be > defined in a common location given there are several devices using it? > I assume because it only applies to hub controllers and not other types > of devices. > > Also, the descriptions that I saw when looking at some of those other > bindings are similarly poor. I can't bring myself to care any more, > just clean up the ambiguous wording here and I'll ack the next version, > I don't expect you to sort out the wording in other bindings. > Ok > Cheers, > Conor. Thanks -Anand _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub @ 2023-11-24 16:52 ` Anand Moon 0 siblings, 0 replies; 14+ messages in thread From: Anand Moon @ 2023-11-24 16:52 UTC (permalink / raw) To: Conor Dooley Cc: Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Icenowy Zheng, Neil Armstrong, linux-amlogic, linux-usb, devicetree, linux-kernel Hi Conor, On Fri, 24 Nov 2023 at 17:55, Conor Dooley <conor@kernel.org> wrote: > > On Fri, Nov 24, 2023 at 04:18:23PM +0530, Anand Moon wrote: > > Hi Conor > > > > On Thu, 23 Nov 2023 at 23:26, Conor Dooley <conor@kernel.org> wrote: > > > > > > On Wed, Nov 22, 2023 at 11:53:46PM +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. > > > > > > > > Onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > > which has a common reset pin and power supply. > > > > peer-hub phandle each peer controller with proper gpio reset > > > > and help each peer power on during initialization > > > > and power off during suspend. > > > > > > > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > > > > --- > > > > 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 | 67 +++++++++++++++++-- > > > > 1 file changed, 63 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml > > > > index ee08b9c3721f..bc3b3f4c8473 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,48 @@ properties: > > > > > > > > vdd-supply: > > > > description: > > > > - the regulator that provides 3.3V core power to the hub. > > > > + phandle to the regulator that provides power to the hub. > > > > + > > > > + peer-hub: > > > > > > Should the property not be "peer-controller"? Your description refers to > > > them as such. > > > > No, as per my understanding, peer-hub represents a complete USB hub. > > See the lock diagram in the below link. > > > > > > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > > + description: > > > > + onboard USB hub supports USB 3.x and USB 2.0 peer controllers. > > > > > > > > > > + which has a common reset pin and power supply. > > > > + peer-hub phandle each peer controller with proper gpio reset > > This is what I don't get. You say "peer-hub phandle each peer > controller..". It is hard for me to understand that portion of the > sentence, but the interchanging of "hub" and "controller" is > confusing. The title of the binding says "hub controller", so maybe it > is better to use that here. > > > > > + and help each peer power on during initialization > > > > + and power off during suspend. > > > > > > I generally hate to talk about non-native speakers grammar etc, but what > > > you have here is in need of a lot of improvement. The below is my > > > attempt to understand what you are trying to say: > > > > > > "For onboard hubs that support USB 3.x and USB 2.0 controllers with > > > shared resets and power supplies, this property is used to identify > > > the controllers with which these are shared." > > "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." > Thanks for your review comments. Ok will update this in the next version. > I re-worded this again to try and remove the use of "controller". > Do you think that this still makes sense? > > > Sorry for the poor grammar, I will update this in the next v5. > > > > > Also - this is one particular system, what prevents there being a hub > > > that has more than 2 controllers? Also, as you insist that this is > > > generic, and not just for genesys, should this not be defined in a > > > common location? > > > > Here is the block diagram of the Genesys GL3523 hub. > > [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] > > > > It has two USB 2.0 and USB 3.1 controllers, so using peer-hub node > > the onboard hub module will bring up this hub. > > > > There are many examples that use similar properties hence it is generic. > > > > # Documentation/devicetree/bindings/usb/cypress,hx3.yaml > > # Documentation/devicetree/bindings/usb/microchip,usb5744.yaml > > # Documentation/devicetree/bindings/usb/realtek,rts5411.yaml > > # Documentation/devicetree/bindings/usb/ti,usb8041.yaml > > # Documentation/devicetree/bindings/usb/vialab,vl817.yaml > > Which brings me back to the unanswered question, should this not be > defined in a common location given there are several devices using it? > I assume because it only applies to hub controllers and not other types > of devices. > > Also, the descriptions that I saw when looking at some of those other > bindings are similarly poor. I can't bring myself to care any more, > just clean up the ambiguous wording here and I'll ack the next version, > I don't expect you to sort out the wording in other bindings. > Ok > Cheers, > Conor. Thanks -Anand ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 2/2] arm64: dts: amlogic: Used onboard usb hub reset on odroid n2 2023-11-22 18:23 [PATCH v4 0/2] Use onboard usb hub reset on Odroid N2 Anand Moon 2023-11-22 18:23 ` Anand Moon @ 2023-11-22 18:23 ` Anand Moon 1 sibling, 0 replies; 14+ messages in thread From: Anand Moon @ 2023-11-22 18:23 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl Cc: Anand Moon, linux-amlogic, devicetree, linux-arm-kernel, linux-kernel On Odroid n2/n2+ previously use gpio-hog to reset the usb hub, switch to used on-board usb hub reset to enable the usb hub and enable power to hub. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- .../dts/amlogic/meson-g12b-odroid-n2.dtsi | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi index 91c9769fda20..9e671444eca6 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi @@ -31,6 +31,30 @@ hub_5v: regulator-hub_5v { enable-active-high; }; + /* USB hub supports both USB 2.0 and USB 3.0 root hub */ + usb-hub { + 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>; + vdd-supply = <&usb_pwr_en>; + }; + + /* 3.0 hub on port 4 */ + hub_3_0: hub@2 { + compatible = "usb5e3,620"; + reg = <2>; + peer-hub = <&hub_2_0>; + reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>; + vdd-supply = <&vcc_5v>; + }; + }; + sound { compatible = "amlogic,axg-sound-card"; model = "ODROID-N2"; @@ -234,18 +258,6 @@ &gpio { "PIN_3", /* GPIOX_17 */ "PIN_5", /* GPIOX_18 */ "PIN_36"; /* GPIOX_19 */ - /* - * WARNING: The USB Hub on the Odroid-N2 needs a reset signal - * to be turned high in order to be detected by the USB Controller - * This signal should be handled by a USB specific power sequence - * in order to reset the Hub when USB bus is powered down. - */ - usb-hub-hog { - gpio-hog; - gpios = <GPIOH_4 GPIO_ACTIVE_HIGH>; - output-high; - line-name = "usb-hub-reset"; - }; }; &i2c3 { -- 2.42.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/2] arm64: dts: amlogic: Used onboard usb hub reset on odroid n2 @ 2023-11-22 18:23 ` Anand Moon 0 siblings, 0 replies; 14+ messages in thread From: Anand Moon @ 2023-11-22 18:23 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl Cc: Anand Moon, linux-amlogic, devicetree, linux-arm-kernel, linux-kernel On Odroid n2/n2+ previously use gpio-hog to reset the usb hub, switch to used on-board usb hub reset to enable the usb hub and enable power to hub. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- .../dts/amlogic/meson-g12b-odroid-n2.dtsi | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi index 91c9769fda20..9e671444eca6 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi @@ -31,6 +31,30 @@ hub_5v: regulator-hub_5v { enable-active-high; }; + /* USB hub supports both USB 2.0 and USB 3.0 root hub */ + usb-hub { + 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>; + vdd-supply = <&usb_pwr_en>; + }; + + /* 3.0 hub on port 4 */ + hub_3_0: hub@2 { + compatible = "usb5e3,620"; + reg = <2>; + peer-hub = <&hub_2_0>; + reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>; + vdd-supply = <&vcc_5v>; + }; + }; + sound { compatible = "amlogic,axg-sound-card"; model = "ODROID-N2"; @@ -234,18 +258,6 @@ &gpio { "PIN_3", /* GPIOX_17 */ "PIN_5", /* GPIOX_18 */ "PIN_36"; /* GPIOX_19 */ - /* - * WARNING: The USB Hub on the Odroid-N2 needs a reset signal - * to be turned high in order to be detected by the USB Controller - * This signal should be handled by a USB specific power sequence - * in order to reset the Hub when USB bus is powered down. - */ - usb-hub-hog { - gpio-hog; - gpios = <GPIOH_4 GPIO_ACTIVE_HIGH>; - output-high; - line-name = "usb-hub-reset"; - }; }; &i2c3 { -- 2.42.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/2] arm64: dts: amlogic: Used onboard usb hub reset on odroid n2 @ 2023-11-22 18:23 ` Anand Moon 0 siblings, 0 replies; 14+ messages in thread From: Anand Moon @ 2023-11-22 18:23 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl Cc: Anand Moon, linux-amlogic, devicetree, linux-arm-kernel, linux-kernel On Odroid n2/n2+ previously use gpio-hog to reset the usb hub, switch to used on-board usb hub reset to enable the usb hub and enable power to hub. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- .../dts/amlogic/meson-g12b-odroid-n2.dtsi | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi index 91c9769fda20..9e671444eca6 100644 --- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi @@ -31,6 +31,30 @@ hub_5v: regulator-hub_5v { enable-active-high; }; + /* USB hub supports both USB 2.0 and USB 3.0 root hub */ + usb-hub { + 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>; + vdd-supply = <&usb_pwr_en>; + }; + + /* 3.0 hub on port 4 */ + hub_3_0: hub@2 { + compatible = "usb5e3,620"; + reg = <2>; + peer-hub = <&hub_2_0>; + reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>; + vdd-supply = <&vcc_5v>; + }; + }; + sound { compatible = "amlogic,axg-sound-card"; model = "ODROID-N2"; @@ -234,18 +258,6 @@ &gpio { "PIN_3", /* GPIOX_17 */ "PIN_5", /* GPIOX_18 */ "PIN_36"; /* GPIOX_19 */ - /* - * WARNING: The USB Hub on the Odroid-N2 needs a reset signal - * to be turned high in order to be detected by the USB Controller - * This signal should be handled by a USB specific power sequence - * in order to reset the Hub when USB bus is powered down. - */ - usb-hub-hog { - gpio-hog; - gpios = <GPIOH_4 GPIO_ACTIVE_HIGH>; - output-high; - line-name = "usb-hub-reset"; - }; }; &i2c3 { -- 2.42.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-11-24 16:53 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-22 18:23 [PATCH v4 0/2] Use onboard usb hub reset on Odroid N2 Anand Moon 2023-11-22 18:23 ` [PATCH v4 1/2] dt-bindings: usb: Add the binding example for the Genesys Logic GL3523 hub Anand Moon 2023-11-22 18:23 ` Anand Moon 2023-11-23 17:56 ` Conor Dooley 2023-11-23 17:56 ` Conor Dooley 2023-11-24 10:48 ` Anand Moon 2023-11-24 10:48 ` Anand Moon 2023-11-24 12:25 ` Conor Dooley 2023-11-24 12:25 ` Conor Dooley 2023-11-24 16:52 ` Anand Moon 2023-11-24 16:52 ` Anand Moon 2023-11-22 18:23 ` [PATCH v4 2/2] arm64: dts: amlogic: Used onboard usb hub reset on odroid n2 Anand Moon 2023-11-22 18:23 ` Anand Moon 2023-11-22 18:23 ` Anand Moon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.