2022-12-28 10:37:06

by Icenowy Zheng

[permalink] [raw]
Subject: Re: [PATCH v1 01/11] dt-bindings: usb: Add device id for Genesys Logic hub controller

在 2022-12-28星期三的 10:03 +0000,Anand Moon写道:
> Add usb hub device id for Genesys Logic, Inc. GL852G-OHG Hub USB 2.0
> root hub and Genesys Logic, Inc. GL3523-QFN76 USB 3.1 root hub.
>
> Signed-off-by: Anand Moon <[email protected]>
> ---
>  Documentation/devicetree/bindings/usb/genesys,gl850g.yaml | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> index a9f831448cca..db009f3ef438 100644
> --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> @@ -16,6 +16,8 @@ properties:
>    compatible:
>      enum:
>        - usb5e3,608
> +      - genesys,usb5e3,610
> +      - genesys,usb5e3,620

I don't think genesys, is needed here because usb5e3 means USB VID
0x05e3, which is already linked to Genesys Logic.

In addition, the control logic of these two hubs are needed to be
verified.

And what's the status of the companion hub of the USB3 hub? Is it
really a USB3-only hub, or is its USB2 part just equal to another USB3
hub?

>  
>    reg: true
>  


2022-12-28 10:40:45

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH v1 01/11] dt-bindings: usb: Add device id for Genesys Logic hub controller

Hi Icenowy,

Thanks for the review comments

On Wed, 28 Dec 2022 at 15:38, Icenowy Zheng <[email protected]> wrote:
>
> 在 2022-12-28星期三的 10:03 +0000,Anand Moon写道:
> > Add usb hub device id for Genesys Logic, Inc. GL852G-OHG Hub USB 2.0
> > root hub and Genesys Logic, Inc. GL3523-QFN76 USB 3.1 root hub.
> >
> > Signed-off-by: Anand Moon <[email protected]>
> > ---
> > Documentation/devicetree/bindings/usb/genesys,gl850g.yaml | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > index a9f831448cca..db009f3ef438 100644
> > --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > @@ -16,6 +16,8 @@ properties:
> > compatible:
> > enum:
> > - usb5e3,608
> > + - genesys,usb5e3,610
> > + - genesys,usb5e3,620
>
> I don't think genesys, is needed here because usb5e3 means USB VID
> 0x05e3, which is already linked to Genesys Logic.
>

Ok, I added this as genesys, is it part of the manufacturer or vendor name
which is most commonly used for adding vendor-specific compatible strings.

> In addition, the control logic of these two hubs are needed to be
> verified.

We were using gpio-hog to reset the USB hub, so now switch to USB onboard hub

>
> And what's the status of the companion hub of the USB3 hub? Is it
> really a USB3-only hub, or is its USB2 part just equal to another USB3
> hub?
>
usb5e3,610 is USB 2.0 port hub present on Odroid C1 and Odroid C2 board

usb5e3,620 is USB 3.1 port hub present on Odroid N2.

> >
> > reg: true
> >
>

Thanks
-Anand

2023-01-04 23:31:12

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v1 01/11] dt-bindings: usb: Add device id for Genesys Logic hub controller

On Wed, Dec 28, 2022 at 03:59:17PM +0530, Anand Moon wrote:
> Hi Icenowy,
>
> Thanks for the review comments
>
> On Wed, 28 Dec 2022 at 15:38, Icenowy Zheng <[email protected]> wrote:
> >
> > 在 2022-12-28星期三的 10:03 +0000,Anand Moon写道:
> > > Add usb hub device id for Genesys Logic, Inc. GL852G-OHG Hub USB 2.0
> > > root hub and Genesys Logic, Inc. GL3523-QFN76 USB 3.1 root hub.

nit: QFN 76 is one of the package options of the GL3523, I expect this
binding to be applicable as well for other GL3523 variants, so I'd suggest
to drop the suffix. Not sure what 'OHG' stands for, the Genesys website
only lists a GL852G hub with different package types (none of them 'OHG'),
so I'd say drop the suffix unless it is known that 'OHG' variant uses
a different product id than other GL852G variants

> > >
> > > Signed-off-by: Anand Moon <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/usb/genesys,gl850g.yaml | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > index a9f831448cca..db009f3ef438 100644
> > > --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > @@ -16,6 +16,8 @@ properties:
> > > compatible:
> > > enum:
> > > - usb5e3,608
> > > + - genesys,usb5e3,610
> > > + - genesys,usb5e3,620
> >
> > I don't think genesys, is needed here because usb5e3 means USB VID
> > 0x05e3, which is already linked to Genesys Logic.
> >
>
> Ok, I added this as genesys, is it part of the manufacturer or vendor name
> which is most commonly used for adding vendor-specific compatible strings.

That doesn't follow the generic USB binding, please drop 'genesys,'.

> > In addition, the control logic of these two hubs are needed to be
> > verified.
>
> We were using gpio-hog to reset the USB hub, so now switch to USB onboard hub
>
> >
> > And what's the status of the companion hub of the USB3 hub? Is it
> > really a USB3-only hub, or is its USB2 part just equal to another USB3
> > hub?
> >
> usb5e3,610 is USB 2.0 port hub present on Odroid C1 and Odroid C2 board
>
> usb5e3,620 is USB 3.1 port hub present on Odroid N2.

Like Icenowy I would expect the GL3523 to provide also a 2.0 hub.

What is the output of 'lsusb' on the Odroid N2?

2023-01-07 15:07:06

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH v1 01/11] dt-bindings: usb: Add device id for Genesys Logic hub controller

Hi Matthias,

Thanks for the review comments

On Thu, 5 Jan 2023 at 04:25, Matthias Kaehlcke <[email protected]> wrote:
>
> On Wed, Dec 28, 2022 at 03:59:17PM +0530, Anand Moon wrote:
> > Hi Icenowy,
> >
> > Thanks for the review comments
> >
> > On Wed, 28 Dec 2022 at 15:38, Icenowy Zheng <[email protected]> wrote:
> > >
> > > 在 2022-12-28星期三的 10:03 +0000,Anand Moon写道:
> > > > Add usb hub device id for Genesys Logic, Inc. GL852G-OHG Hub USB 2.0
> > > > root hub and Genesys Logic, Inc. GL3523-QFN76 USB 3.1 root hub.
>
> nit: QFN 76 is one of the package options of the GL3523, I expect this
> binding to be applicable as well for other GL3523 variants, so I'd suggest
> to drop the suffix. Not sure what 'OHG' stands for, the Genesys website
> only lists a GL852G hub with different package types (none of them 'OHG'),
> so I'd say drop the suffix unless it is known that 'OHG' variant uses
> a different product id than other GL852G variants
>

Yes, I will be sure to do this reference, I just picked these from the
board schematics.

> > > >
> > > > Signed-off-by: Anand Moon <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/usb/genesys,gl850g.yaml | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > index a9f831448cca..db009f3ef438 100644
> > > > --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > @@ -16,6 +16,8 @@ properties:
> > > > compatible:
> > > > enum:
> > > > - usb5e3,608
> > > > + - genesys,usb5e3,610
> > > > + - genesys,usb5e3,620
> > >
> > > I don't think genesys, is needed here because usb5e3 means USB VID
> > > 0x05e3, which is already linked to Genesys Logic.
> > >
> >
> > Ok, I added this as genesys, is it part of the manufacturer or vendor name
> > which is most commonly used for adding vendor-specific compatible strings.
>
> That doesn't follow the generic USB binding, please drop 'genesys,'.
>

Yes, I will drop this in the next version.

> > > In addition, the control logic of these two hubs are needed to be
> > > verified.
> >
> > We were using gpio-hog to reset the USB hub, so now switch to USB onboard hub
> >
> > >
> > > And what's the status of the companion hub of the USB3 hub? Is it
> > > really a USB3-only hub, or is its USB2 part just equal to another USB3
> > > hub?
> > >
> > usb5e3,610 is USB 2.0 port hub present on Odroid C1 and Odroid C2 board
> >
> > usb5e3,620 is USB 3.1 port hub present on Odroid N2.
>
> Like Icenowy I would expect the GL3523 to provide also a 2.0 hub.
>
> What is the output of 'lsusb' on the Odroid N2?

alarm@odroid-n2:~$ lsusb -tv
/: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
ID 1d6b:0003 Linux Foundation 3.0 root hub
|__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 5000M
ID 05e3:0620 Genesys Logic, Inc. GL3523 Hub
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/2p, 480M
ID 1d6b:0002 Linux Foundation 2.0 root hub
|__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
ID 05e3:0610 Genesys Logic, Inc. Hub

Thanks

-Anand

2023-01-09 16:32:20

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH v1 01/11] dt-bindings: usb: Add device id for Genesys Logic hub controller

On Sat, Jan 07, 2023 at 08:31:23PM +0530, Anand Moon wrote:
> Hi Matthias,
>
> Thanks for the review comments
>
> On Thu, 5 Jan 2023 at 04:25, Matthias Kaehlcke <[email protected]> wrote:
> >
> > On Wed, Dec 28, 2022 at 03:59:17PM +0530, Anand Moon wrote:
> > > Hi Icenowy,
> > >
> > > Thanks for the review comments
> > >
> > > On Wed, 28 Dec 2022 at 15:38, Icenowy Zheng <[email protected]> wrote:
> > > >
> > > > 在 2022-12-28星期三的 10:03 +0000,Anand Moon写道:
> > > > > Add usb hub device id for Genesys Logic, Inc. GL852G-OHG Hub USB 2.0
> > > > > root hub and Genesys Logic, Inc. GL3523-QFN76 USB 3.1 root hub.
> >
> > nit: QFN 76 is one of the package options of the GL3523, I expect this
> > binding to be applicable as well for other GL3523 variants, so I'd suggest
> > to drop the suffix. Not sure what 'OHG' stands for, the Genesys website
> > only lists a GL852G hub with different package types (none of them 'OHG'),
> > so I'd say drop the suffix unless it is known that 'OHG' variant uses
> > a different product id than other GL852G variants
> >
>
> Yes, I will be sure to do this reference, I just picked these from the
> board schematics.
>
> > > > >
> > > > > Signed-off-by: Anand Moon <[email protected]>
> > > > > ---
> > > > > Documentation/devicetree/bindings/usb/genesys,gl850g.yaml | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > > b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > > index a9f831448cca..db009f3ef438 100644
> > > > > --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > > +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > > @@ -16,6 +16,8 @@ properties:
> > > > > compatible:
> > > > > enum:
> > > > > - usb5e3,608
> > > > > + - genesys,usb5e3,610
> > > > > + - genesys,usb5e3,620
> > > >
> > > > I don't think genesys, is needed here because usb5e3 means USB VID
> > > > 0x05e3, which is already linked to Genesys Logic.
> > > >
> > >
> > > Ok, I added this as genesys, is it part of the manufacturer or vendor name
> > > which is most commonly used for adding vendor-specific compatible strings.
> >
> > That doesn't follow the generic USB binding, please drop 'genesys,'.
> >
>
> Yes, I will drop this in the next version.
>
> > > > In addition, the control logic of these two hubs are needed to be
> > > > verified.
> > >
> > > We were using gpio-hog to reset the USB hub, so now switch to USB onboard hub
> > >
> > > >
> > > > And what's the status of the companion hub of the USB3 hub? Is it
> > > > really a USB3-only hub, or is its USB2 part just equal to another USB3
> > > > hub?
> > > >
> > > usb5e3,610 is USB 2.0 port hub present on Odroid C1 and Odroid C2 board
> > >
> > > usb5e3,620 is USB 3.1 port hub present on Odroid N2.
> >
> > Like Icenowy I would expect the GL3523 to provide also a 2.0 hub.
> >
> > What is the output of 'lsusb' on the Odroid N2?
>
> alarm@odroid-n2:~$ lsusb -tv
> /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
> ID 1d6b:0003 Linux Foundation 3.0 root hub
> |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 5000M
> ID 05e3:0620 Genesys Logic, Inc. GL3523 Hub
> /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/2p, 480M
> ID 1d6b:0002 Linux Foundation 2.0 root hub
> |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
> ID 05e3:0610 Genesys Logic, Inc. Hub

Thanks. So it looks like 0x0610 is the product id of the USB 2.0 portion
of the GL3523. From this patch it seems the GL852G uses the same product
id?

2023-01-09 18:35:00

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH v1 01/11] dt-bindings: usb: Add device id for Genesys Logic hub controller

Hi Matthias,

On Mon, 9 Jan 2023 at 21:59, Matthias Kaehlcke <[email protected]> wrote:
>
> On Sat, Jan 07, 2023 at 08:31:23PM +0530, Anand Moon wrote:
> > Hi Matthias,
> >
> > Thanks for the review comments
> >
> > On Thu, 5 Jan 2023 at 04:25, Matthias Kaehlcke <[email protected]> wrote:
> > >
> > > On Wed, Dec 28, 2022 at 03:59:17PM +0530, Anand Moon wrote:
> > > > Hi Icenowy,
> > > >
> > > > Thanks for the review comments
> > > >
> > > > On Wed, 28 Dec 2022 at 15:38, Icenowy Zheng <[email protected]> wrote:
> > > > >
> > > > > 在 2022-12-28星期三的 10:03 +0000,Anand Moon写道:
> > > > > > Add usb hub device id for Genesys Logic, Inc. GL852G-OHG Hub USB 2.0
> > > > > > root hub and Genesys Logic, Inc. GL3523-QFN76 USB 3.1 root hub.
> > >
> > > nit: QFN 76 is one of the package options of the GL3523, I expect this
> > > binding to be applicable as well for other GL3523 variants, so I'd suggest
> > > to drop the suffix. Not sure what 'OHG' stands for, the Genesys website
> > > only lists a GL852G hub with different package types (none of them 'OHG'),
> > > so I'd say drop the suffix unless it is known that 'OHG' variant uses
> > > a different product id than other GL852G variants
> > >
> >
> > Yes, I will be sure to do this reference, I just picked these from the
> > board schematics.
> >
> > > > > >
> > > > > > Signed-off-by: Anand Moon <[email protected]>
> > > > > > ---
> > > > > > Documentation/devicetree/bindings/usb/genesys,gl850g.yaml | 2 ++
> > > > > > 1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > > > b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > > > index a9f831448cca..db009f3ef438 100644
> > > > > > --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
> > > > > > @@ -16,6 +16,8 @@ properties:
> > > > > > compatible:
> > > > > > enum:
> > > > > > - usb5e3,608
> > > > > > + - genesys,usb5e3,610
> > > > > > + - genesys,usb5e3,620
> > > > >
> > > > > I don't think genesys, is needed here because usb5e3 means USB VID
> > > > > 0x05e3, which is already linked to Genesys Logic.
> > > > >
> > > >
> > > > Ok, I added this as genesys, is it part of the manufacturer or vendor name
> > > > which is most commonly used for adding vendor-specific compatible strings.
> > >
> > > That doesn't follow the generic USB binding, please drop 'genesys,'.
> > >
> >
> > Yes, I will drop this in the next version.
> >
> > > > > In addition, the control logic of these two hubs are needed to be
> > > > > verified.
> > > >
> > > > We were using gpio-hog to reset the USB hub, so now switch to USB onboard hub
> > > >
> > > > >
> > > > > And what's the status of the companion hub of the USB3 hub? Is it
> > > > > really a USB3-only hub, or is its USB2 part just equal to another USB3
> > > > > hub?
> > > > >
> > > > usb5e3,610 is USB 2.0 port hub present on Odroid C1 and Odroid C2 board
> > > >
> > > > usb5e3,620 is USB 3.1 port hub present on Odroid N2.
> > >
> > > Like Icenowy I would expect the GL3523 to provide also a 2.0 hub.
> > >
> > > What is the output of 'lsusb' on the Odroid N2?
> >
> > alarm@odroid-n2:~$ lsusb -tv
> > /: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/1p, 5000M
> > ID 1d6b:0003 Linux Foundation 3.0 root hub
> > |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 5000M
> > ID 05e3:0620 Genesys Logic, Inc. GL3523 Hub
> > /: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci-hcd/2p, 480M
> > ID 1d6b:0002 Linux Foundation 2.0 root hub
> > |__ Port 1: Dev 2, If 0, Class=Hub, Driver=hub/4p, 480M
> > ID 05e3:0610 Genesys Logic, Inc. Hub
>
> Thanks. So it looks like 0x0610 is the product id of the USB 2.0 portion
> of the GL3523. From this patch it seems the GL852G uses the same product
> id?

Yes, it looks correct as per the lsusb output

Thanks
-Anand