Hi Conor On Thu, 23 Nov 2023 at 23:26, Conor Dooley <conor@xxxxxxxxxx> 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@xxxxxxxxx> > > --- > > 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@xxxxxxxxxx> > > > > -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