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

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

[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 <linux.amoon@gmail.com>
---
 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
 
   reg: true
 
-- 
2.38.1

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

[Icenowy Zheng]

在 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 <linux.amoon@gmail.com>
> ---
>  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
>  

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

[Anand Moon]

Hi Icenowy,

Thanks for the review comments

On Wed, 28 Dec 2022 at 15:38, Icenowy Zheng <uwu@icenowy.me> 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 <linux.amoon@gmail.com>
> > ---
> >  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

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

[Matthias Kaehlcke]

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 <uwu@icenowy.me> 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 <linux.amoon@gmail.com>
> > > ---
> > >  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?

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

[Anand Moon]

Hi Matthias,

Thanks for the review comments

On Thu, 5 Jan 2023 at 04:25, Matthias Kaehlcke <mka@chromium.org> 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 <uwu@icenowy.me> 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 <linux.amoon@gmail.com>
> > > > ---
> > > >  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

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

[Matthias Kaehlcke]

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 <mka@chromium.org> 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 <uwu@icenowy.me> 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 <linux.amoon@gmail.com>
> > > > > ---
> > > > >  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?

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

[Anand Moon]

Hi Matthias,

On Mon, 9 Jan 2023 at 21:59, Matthias Kaehlcke <mka@chromium.org> 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 <mka@chromium.org> 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 <uwu@icenowy.me> 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 <linux.amoon@gmail.com>
> > > > > > ---
> > > > > >  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

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

[Krzysztof Kozlowski]

On 28/12/2022 11:03, Anand Moon wrote:
> 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 <linux.amoon@gmail.com>
> ---
>  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

This does not look like correct compatible. Did you test the bindings
and DTS? This should fail.

Best regards,
Krzysztof

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

[Anand Moon]

Hi Krzysztof,

Thanks for your review comments.

On Wed, 28 Dec 2022 at 20:38, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 28/12/2022 11:03, Anand Moon wrote:
> > 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 <linux.amoon@gmail.com>
> > ---
> >  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
>
> This does not look like correct compatible. Did you test the bindings
> and DTS? This should fail.
>

Yes, I have done and it did not report any error.

make ARCH=arm64 mrproper
make ARCH=arm64 defconfig
make ARCH=arm64 -j$(nproc) dtbs
make CHECK_DTBS=y amlogic/meson-gxbb-odroidc2.dtb
make CHECK_DTBS=y amlogic/meson-g12b-odroid-n2.dtb
make CHECK_DTBS=y amlogic/meson-sm1-odroid-c4.dtb

Ok I will update this in the next version for all the patches.

> Best regards,
> Krzysztof
>

Thanks
-Anand

[PATCH v1 05/11] usb: misc: onboard_usb_hub: add Genesys Logic GL852G-OHG hub support

[Anand Moon]

Genesys Logic GL852G-OHG is a 4-port USB 2.0 STT hub that has a reset pin to
toggle and a 5.0V core supply exported though an integrated LDO is
available for powering it.

Add the support for this hub, for controlling the reset pin and the core
power supply.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/usb/misc/onboard_usb_hub.c | 1 +
 drivers/usb/misc/onboard_usb_hub.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
index 94e7966e199d..c0e8e6f4ec0a 100644
--- a/drivers/usb/misc/onboard_usb_hub.c
+++ b/drivers/usb/misc/onboard_usb_hub.c
@@ -409,6 +409,7 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev)
 
 static const struct usb_device_id onboard_hub_id_table[] = {
 	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 */
+	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G-OHG USB 2.0 */
 	{ USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 */
 	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */
 	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */
diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
index 62129a6a1ba5..2ee1b0032d23 100644
--- a/drivers/usb/misc/onboard_usb_hub.h
+++ b/drivers/usb/misc/onboard_usb_hub.h
@@ -31,6 +31,7 @@ static const struct of_device_id onboard_hub_match[] = {
 	{ .compatible = "usb451,8140", .data = &ti_tusb8041_data, },
 	{ .compatible = "usb451,8142", .data = &ti_tusb8041_data, },
 	{ .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
+	{ .compatible = "genesys,usb5e3,610", .data = &genesys_gl850g_data, },
 	{ .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
 	{ .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
 	{ .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
-- 
2.38.1

Re: [PATCH v1 05/11] usb: misc: onboard_usb_hub: add Genesys Logic GL852G-OHG hub support

[Matthias Kaehlcke]

On Wed, Dec 28, 2022 at 10:03:14AM +0000, Anand Moon wrote:
> Genesys Logic GL852G-OHG is a 4-port USB 2.0 STT hub that has a reset pin to
> toggle and a 5.0V core supply exported though an integrated LDO is
> available for powering it.
> 
> Add the support for this hub, for controlling the reset pin and the core
> power supply.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  drivers/usb/misc/onboard_usb_hub.c | 1 +
>  drivers/usb/misc/onboard_usb_hub.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> index 94e7966e199d..c0e8e6f4ec0a 100644
> --- a/drivers/usb/misc/onboard_usb_hub.c
> +++ b/drivers/usb/misc/onboard_usb_hub.c
> @@ -409,6 +409,7 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev)
>  
>  static const struct usb_device_id onboard_hub_id_table[] = {
>  	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 */
> +	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G-OHG USB 2.0 */
>  	{ USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 */
>  	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */
>  	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */
> diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
> index 62129a6a1ba5..2ee1b0032d23 100644
> --- a/drivers/usb/misc/onboard_usb_hub.h
> +++ b/drivers/usb/misc/onboard_usb_hub.h
> @@ -31,6 +31,7 @@ static const struct of_device_id onboard_hub_match[] = {
>  	{ .compatible = "usb451,8140", .data = &ti_tusb8041_data, },
>  	{ .compatible = "usb451,8142", .data = &ti_tusb8041_data, },
>  	{ .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
> +	{ .compatible = "genesys,usb5e3,610", .data = &genesys_gl850g_data, },

s/genesys,//

>  	{ .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
>  	{ .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
>  	{ .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
> -- 
> 2.38.1
> 

Re: [PATCH v1 05/11] usb: misc: onboard_usb_hub: add Genesys Logic GL852G-OHG hub support

[Matthias Kaehlcke]

On Wed, Jan 04, 2023 at 08:28:18PM +0000, Matthias Kaehlcke wrote:
> On Wed, Dec 28, 2022 at 10:03:14AM +0000, Anand Moon wrote:
> > Genesys Logic GL852G-OHG is a 4-port USB 2.0 STT hub that has a reset pin to
> > toggle and a 5.0V core supply exported though an integrated LDO is
> > available for powering it.
> > 
> > Add the support for this hub, for controlling the reset pin and the core
> > power supply.
> > 
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  drivers/usb/misc/onboard_usb_hub.c | 1 +
> >  drivers/usb/misc/onboard_usb_hub.h | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> > index 94e7966e199d..c0e8e6f4ec0a 100644
> > --- a/drivers/usb/misc/onboard_usb_hub.c
> > +++ b/drivers/usb/misc/onboard_usb_hub.c
> > @@ -409,6 +409,7 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev)
> >  
> >  static const struct usb_device_id onboard_hub_id_table[] = {
> >  	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 */
> > +	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G-OHG USB 2.0 */

Do you happen to know what '-OHG' stands for? The Genesys website only
mentions a GL852G with different package types (none of them 'OHG').
Unless there is an 'OHG' variant with a different product id I'd say
let's drop the suffix.

> >  	{ USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 */
> >  	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */
> >  	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */
> > diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
> > index 62129a6a1ba5..2ee1b0032d23 100644
> > --- a/drivers/usb/misc/onboard_usb_hub.h
> > +++ b/drivers/usb/misc/onboard_usb_hub.h
> > @@ -31,6 +31,7 @@ static const struct of_device_id onboard_hub_match[] = {
> >  	{ .compatible = "usb451,8140", .data = &ti_tusb8041_data, },
> >  	{ .compatible = "usb451,8142", .data = &ti_tusb8041_data, },
> >  	{ .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
> > +	{ .compatible = "genesys,usb5e3,610", .data = &genesys_gl850g_data, },
> 
> s/genesys,//
> 
> >  	{ .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
> >  	{ .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
> >  	{ .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
> > -- 
> > 2.38.1
> > 

Re: [PATCH v1 05/11] usb: misc: onboard_usb_hub: add Genesys Logic GL852G-OHG hub support

[Anand Moon]

Hi Matthias

On Thu, 5 Jan 2023 at 02:23, Matthias Kaehlcke <mka@chromium.org> wrote:
>
> On Wed, Jan 04, 2023 at 08:28:18PM +0000, Matthias Kaehlcke wrote:
> > On Wed, Dec 28, 2022 at 10:03:14AM +0000, Anand Moon wrote:
> > > Genesys Logic GL852G-OHG is a 4-port USB 2.0 STT hub that has a reset pin to
> > > toggle and a 5.0V core supply exported through an integrated LDO is
> > > available for powering it.
> > >
> > > Add the support for this hub, for controlling the reset pin and the core
> > > power supply.
> > >
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > ---
> > >  drivers/usb/misc/onboard_usb_hub.c | 1 +
> > >  drivers/usb/misc/onboard_usb_hub.h | 1 +
> > >  2 files changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> > > index 94e7966e199d..c0e8e6f4ec0a 100644
> > > --- a/drivers/usb/misc/onboard_usb_hub.c
> > > +++ b/drivers/usb/misc/onboard_usb_hub.c
> > > @@ -409,6 +409,7 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev)
> > >
> > >  static const struct usb_device_id onboard_hub_id_table[] = {
> > >     { USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 */
> > > +   { USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G-OHG USB 2.0 */
>
> Do you happen to know what '-OHG' stands for? The Genesys website only
> mentions a GL852G with different package types (none of them 'OHG').
> Unless there is an 'OHG' variant with a different product id I'd say
> let's drop the suffix.

Ok, done.


>
> > >     { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 */
> > >     { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */
> > >     { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */
> > > diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
> > > index 62129a6a1ba5..2ee1b0032d23 100644
> > > --- a/drivers/usb/misc/onboard_usb_hub.h
> > > +++ b/drivers/usb/misc/onboard_usb_hub.h
> > > @@ -31,6 +31,7 @@ static const struct of_device_id onboard_hub_match[] = {
> > >     { .compatible = "usb451,8140", .data = &ti_tusb8041_data, },
> > >     { .compatible = "usb451,8142", .data = &ti_tusb8041_data, },
> > >     { .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
> > > +   { .compatible = "genesys,usb5e3,610", .data = &genesys_gl850g_data, },
> >
> > s/genesys,//
> >
> > >     { .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
> > >     { .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
> > >     { .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
> > > --
> > > 2.38.1
> > >

[PATCH v1 06/11] usb: misc: onboard_usb_hub: add Genesys Logic GL3523-QFN76 hub support

[Anand Moon]

Genesys Logic GL3523-QFN76 is a 4-port USB 3.1 hub that has a reset pin to
toggle and a 5.0V core supply exported though an integrated LDO is
available for powering it.

Add the support for this hub, for controlling the reset pin and the core
power supply.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/usb/misc/onboard_usb_hub.c | 1 +
 drivers/usb/misc/onboard_usb_hub.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
index c0e8e6f4ec0a..699050eb3f17 100644
--- a/drivers/usb/misc/onboard_usb_hub.c
+++ b/drivers/usb/misc/onboard_usb_hub.c
@@ -410,6 +410,7 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev)
 static const struct usb_device_id onboard_hub_id_table[] = {
 	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 */
 	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G-OHG USB 2.0 */
+	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0620) }, /* Genesys Logic GL3523-QFN76 USB 3.1 */
 	{ USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 */
 	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */
 	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */
diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
index 2ee1b0032d23..b32fad3a70f9 100644
--- a/drivers/usb/misc/onboard_usb_hub.h
+++ b/drivers/usb/misc/onboard_usb_hub.h
@@ -32,6 +32,7 @@ static const struct of_device_id onboard_hub_match[] = {
 	{ .compatible = "usb451,8142", .data = &ti_tusb8041_data, },
 	{ .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
 	{ .compatible = "genesys,usb5e3,610", .data = &genesys_gl850g_data, },
+	{ .compatible = "genesys,usb5e3,620", .data = &genesys_gl850g_data, },
 	{ .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
 	{ .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
 	{ .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
-- 
2.38.1

Re: [PATCH v1 06/11] usb: misc: onboard_usb_hub: add Genesys Logic GL3523-QFN76 hub support

[Matthias Kaehlcke]

Hi Anand,

On Wed, Dec 28, 2022 at 10:03:15AM +0000, Anand Moon wrote:
> Genesys Logic GL3523-QFN76 is a 4-port USB 3.1 hub that has a reset pin to
> toggle and a 5.0V core supply exported though an integrated LDO is
> available for powering it.
> 
> Add the support for this hub, for controlling the reset pin and the core
> power supply.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  drivers/usb/misc/onboard_usb_hub.c | 1 +
>  drivers/usb/misc/onboard_usb_hub.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> index c0e8e6f4ec0a..699050eb3f17 100644
> --- a/drivers/usb/misc/onboard_usb_hub.c
> +++ b/drivers/usb/misc/onboard_usb_hub.c
> @@ -410,6 +410,7 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev)
>  static const struct usb_device_id onboard_hub_id_table[] = {
>  	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 */
>  	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G-OHG USB 2.0 */
> +	{ USB_DEVICE(VENDOR_ID_GENESYS, 0x0620) }, /* Genesys Logic GL3523-QFN76 USB 3.1 */

Please drop the '-QFN76' suffix. The GL3523 comes in different packages, 'QFN76'
is one of them, I'd expect the other packages to use the same product id.

The GL3523 is a single IC, however like the TI USB8041 or the RTS5414 it
provides both a USB 3.1 and a USB 2.0 hub. You should also add an entry for
the USB 2.0 hub here.

>  	{ USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 */
>  	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */
>  	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */
> diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
> index 2ee1b0032d23..b32fad3a70f9 100644
> --- a/drivers/usb/misc/onboard_usb_hub.h
> +++ b/drivers/usb/misc/onboard_usb_hub.h
> @@ -32,6 +32,7 @@ static const struct of_device_id onboard_hub_match[] = {
>  	{ .compatible = "usb451,8142", .data = &ti_tusb8041_data, },
>  	{ .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
>  	{ .compatible = "genesys,usb5e3,610", .data = &genesys_gl850g_data, },
> +	{ .compatible = "genesys,usb5e3,620", .data = &genesys_gl850g_data, },

s/genesys,//

This reuses the settings of the GL850G hub, which doesn't seem correct in
this case. For the GL850G a (minimum) reset time of 3us is configured. The
data sheet of the GL3523 says:

  "The (internal) reset will be released after approximately 40 μS after
   power good.

   To fully control the reset process of GL3523, we suggest the reset time
   applied in the external reset circuit should longer than that of the
   internal reset circuit."

Since it is 'approximately 40 μS' I'd say make the external reset 50 μS
to be on the safe side, it's a very short time in any case.

Please also add an entry for the USB 2.0 part of the IC.

>  	{ .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
>  	{ .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
>  	{ .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
> -- 
> 2.38.1
> 

Re: [PATCH v1 06/11] usb: misc: onboard_usb_hub: add Genesys Logic GL3523-QFN76 hub support

[Anand Moon]

Hi Matthias,

Thanks for your review comments,

On Thu, 5 Jan 2023 at 02:13, Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Hi Anand,
>
> On Wed, Dec 28, 2022 at 10:03:15AM +0000, Anand Moon wrote:
> > Genesys Logic GL3523-QFN76 is a 4-port USB 3.1 hub that has a reset pin to
> > toggle and a 5.0V core supply exported though an integrated LDO is
> > available for powering it.
> >
> > Add the support for this hub, for controlling the reset pin and the core
> > power supply.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  drivers/usb/misc/onboard_usb_hub.c | 1 +
> >  drivers/usb/misc/onboard_usb_hub.h | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> > index c0e8e6f4ec0a..699050eb3f17 100644
> > --- a/drivers/usb/misc/onboard_usb_hub.c
> > +++ b/drivers/usb/misc/onboard_usb_hub.c
> > @@ -410,6 +410,7 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev)
> >  static const struct usb_device_id onboard_hub_id_table[] = {
> >       { USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 */
> >       { USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G-OHG USB 2.0 */
> > +     { USB_DEVICE(VENDOR_ID_GENESYS, 0x0620) }, /* Genesys Logic GL3523-QFN76 USB 3.1 */
>
> Please drop the '-QFN76' suffix. The GL3523 comes in different packages, 'QFN76'
> is one of them, I'd expect the other packages to use the same product id.
>
> The GL3523 is a single IC, however like the TI USB8041 or the RTS5414 it
> provides both a USB 3.1 and a USB 2.0 hub. You should also add an entry for
> the USB 2.0 hub here.
>

Ok,

> >       { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 */
> >       { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */
> >       { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */
> > diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
> > index 2ee1b0032d23..b32fad3a70f9 100644
> > --- a/drivers/usb/misc/onboard_usb_hub.h
> > +++ b/drivers/usb/misc/onboard_usb_hub.h
> > @@ -32,6 +32,7 @@ static const struct of_device_id onboard_hub_match[] = {
> >       { .compatible = "usb451,8142", .data = &ti_tusb8041_data, },
> >       { .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
> >       { .compatible = "genesys,usb5e3,610", .data = &genesys_gl850g_data, },
> > +     { .compatible = "genesys,usb5e3,620", .data = &genesys_gl850g_data, },
>
> s/genesys,//
>
> This reuses the settings of the GL850G hub, which doesn't seem correct in
> this case. For the GL850G a (minimum) reset time of 3us is configured. The
> data sheet of the GL3523 says:
>
>   "The (internal) reset will be released after approximately 40 μS after
>    power good.
>
>    To fully control the reset process of GL3523, we suggest the reset time
>    applied in the external reset circuit should longer than that of the
>    internal reset circuit."
>
> Since it is 'approximately 40 μS' I'd say make the external reset 50 μS
> to be on the safe side, it's a very short time in any case.
>

Thanks for this input will update this in the next version.

> Please also add an entry for the USB 2.0 part of the IC.

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

So earlier patch adds support for this device ID.
>
> >       { .compatible = "usbbda,411", .data = &realtek_rts5411_data, },
> >       { .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
> >       { .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
> > --
> > 2.38.1
> >

Re: [PATCH v1 06/11] usb: misc: onboard_usb_hub: add Genesys Logic GL3523-QFN76 hub support

[Matthias Kaehlcke]

On Sat, Jan 07, 2023 at 08:28:11PM +0530, Anand Moon wrote:
> Hi Matthias,
> 
> Thanks for your review comments,
> 
> On Thu, 5 Jan 2023 at 02:13, Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > Hi Anand,
> >
> > On Wed, Dec 28, 2022 at 10:03:15AM +0000, Anand Moon wrote:
> > > Genesys Logic GL3523-QFN76 is a 4-port USB 3.1 hub that has a reset pin to
> > > toggle and a 5.0V core supply exported though an integrated LDO is
> > > available for powering it.
> > >
> > > Add the support for this hub, for controlling the reset pin and the core
> > > power supply.
> > >
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > ---
> > >  drivers/usb/misc/onboard_usb_hub.c | 1 +
> > >  drivers/usb/misc/onboard_usb_hub.h | 1 +
> > >  2 files changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> > > index c0e8e6f4ec0a..699050eb3f17 100644
> > > --- a/drivers/usb/misc/onboard_usb_hub.c
> > > +++ b/drivers/usb/misc/onboard_usb_hub.c
> > > @@ -410,6 +410,7 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev)
> > >  static const struct usb_device_id onboard_hub_id_table[] = {
> > >       { USB_DEVICE(VENDOR_ID_GENESYS, 0x0608) }, /* Genesys Logic GL850G USB 2.0 */
> > >       { USB_DEVICE(VENDOR_ID_GENESYS, 0x0610) }, /* Genesys Logic GL852G-OHG USB 2.0 */
> > > +     { USB_DEVICE(VENDOR_ID_GENESYS, 0x0620) }, /* Genesys Logic GL3523-QFN76 USB 3.1 */
> >
> > Please drop the '-QFN76' suffix. The GL3523 comes in different packages, 'QFN76'
> > is one of them, I'd expect the other packages to use the same product id.
> >
> > The GL3523 is a single IC, however like the TI USB8041 or the RTS5414 it
> > provides both a USB 3.1 and a USB 2.0 hub. You should also add an entry for
> > the USB 2.0 hub here.
> >
> 
> Ok,
> 
> > >       { USB_DEVICE(VENDOR_ID_MICROCHIP, 0x2514) }, /* USB2514B USB 2.0 */
> > >       { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */
> > >       { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */
> > > diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
> > > index 2ee1b0032d23..b32fad3a70f9 100644
> > > --- a/drivers/usb/misc/onboard_usb_hub.h
> > > +++ b/drivers/usb/misc/onboard_usb_hub.h
> > > @@ -32,6 +32,7 @@ static const struct of_device_id onboard_hub_match[] = {
> > >       { .compatible = "usb451,8142", .data = &ti_tusb8041_data, },
> > >       { .compatible = "usb5e3,608", .data = &genesys_gl850g_data, },
> > >       { .compatible = "genesys,usb5e3,610", .data = &genesys_gl850g_data, },
> > > +     { .compatible = "genesys,usb5e3,620", .data = &genesys_gl850g_data, },
> >
> > s/genesys,//
> >
> > This reuses the settings of the GL850G hub, which doesn't seem correct in
> > this case. For the GL850G a (minimum) reset time of 3us is configured. The
> > data sheet of the GL3523 says:
> >
> >   "The (internal) reset will be released after approximately 40 μS after
> >    power good.
> >
> >    To fully control the reset process of GL3523, we suggest the reset time
> >    applied in the external reset circuit should longer than that of the
> >    internal reset circuit."
> >
> > Since it is 'approximately 40 μS' I'd say make the external reset 50 μS
> > to be on the safe side, it's a very short time in any case.
> >
> 
> Thanks for this input will update this in the next version.
> 
> > Please also add an entry for the USB 2.0 part of the IC.
> 
> 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
> 
> So earlier patch adds support for this device ID.

Do I understand correctly that 0x0610 is the product id of both the
GL852G [1] and the USB 2 part of the GL3523 (the above 'lsusb'
output)?

[1] https://patchwork.kernel.org/project/linux-usb/patch/20221228100321.15949-2-linux.amoon@gmail.com/

[PATCH v1 08/11] dt-bindings: usb: Add binding for Via lab VL817Q7 hub controller

[Anand Moon]

The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that
features 4 downstream ports, an internal 5V regulator and has
external reset pin.

Add a device tree binding for its USB protocol part.
The internal LDO is not covered by this and can just be modelled
as a fixed regulator.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 .../bindings/usb/vialab,vl817q7.yaml          | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml

diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
new file mode 100644
index 000000000000..4ae995160fd5
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Via labs VL817Q7 USB 3.1 hub controller
+
+maintainers:
+  - Anand Moon <linux.amoon@gmail.com>
+
+allOf:
+  - $ref: usb-device.yaml#
+
+properties:
+  compatible:
+    enum:
+      - vialab,usb2109
+
+  reg: true
+
+  reset-gpios:
+    description: GPIO controlling the RESET# pin.
+
+  vdd-supply:
+    description:
+      the regulator that provides 5.0V core power to the hub.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    usb {
+        dr_mode = "host";
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        hub: hub@1 {
+            compatible = "vialab,usb2109"
+            reg = <1>;
+            reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
+        };
+    };
-- 
2.38.1

Re: [PATCH v1 08/11] dt-bindings: usb: Add binding for Via lab VL817Q7 hub controller

[Johan Hovold]

On Wed, Dec 28, 2022 at 10:03:17AM +0000, Anand Moon wrote:
> The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that
> features 4 downstream ports, an internal 5V regulator and has
> external reset pin.
> 
> Add a device tree binding for its USB protocol part.
> The internal LDO is not covered by this and can just be modelled
> as a fixed regulator.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  .../bindings/usb/vialab,vl817q7.yaml          | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> new file mode 100644
> index 000000000000..4ae995160fd5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Via labs VL817Q7 USB 3.1 hub controller
> +
> +maintainers:
> +  - Anand Moon <linux.amoon@gmail.com>
> +
> +allOf:
> +  - $ref: usb-device.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - vialab,usb2109

This isn't a valid compatible string for USB devices (should be
"usb<vid>,<pid>").

Same for the other binding.

Also the bindings should go before the driver changes in the series.

Johan

Re: [PATCH v1 08/11] dt-bindings: usb: Add binding for Via lab VL817Q7 hub controller

[Anand Moon]

Hi Johan

Thanks for your review comments.

On Wed, 28 Dec 2022 at 16:32, Johan Hovold <johan@kernel.org> wrote:
>
> On Wed, Dec 28, 2022 at 10:03:17AM +0000, Anand Moon wrote:
> > The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that
> > features 4 downstream ports, an internal 5V regulator and has
> > external reset pin.
> >
> > Add a device tree binding for its USB protocol part.
> > The internal LDO is not covered by this and can just be modelled
> > as a fixed regulator.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  .../bindings/usb/vialab,vl817q7.yaml          | 47 +++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> > new file mode 100644
> > index 000000000000..4ae995160fd5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> > @@ -0,0 +1,47 @@
> > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Via labs VL817Q7 USB 3.1 hub controller
> > +
> > +maintainers:
> > +  - Anand Moon <linux.amoon@gmail.com>
> > +
> > +allOf:
> > +  - $ref: usb-device.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - vialab,usb2109
>
> This isn't a valid compatible string for USB devices (should be
> "usb<vid>,<pid>").
>
> Same for the other binding.
>

ok, I will change this in the next version.

> Also the bindings should go before the driver changes in the series.
>
> Johan

Thanks
-Anand

Re: [PATCH v1 08/11] dt-bindings: usb: Add binding for Via lab VL817Q7 hub controller

[Rob Herring]

On Wed, 28 Dec 2022 10:03:17 +0000, Anand Moon wrote:
> The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that
> features 4 downstream ports, an internal 5V regulator and has
> external reset pin.
> 
> Add a device tree binding for its USB protocol part.
> The internal LDO is not covered by this and can just be modelled
> as a fixed regulator.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  .../bindings/usb/vialab,vl817q7.yaml          | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml: '$id' is a required property
	hint: Metaschema for devicetree binding documentation
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
Error: Documentation/devicetree/bindings/usb/vialab,vl817q7.example.dts:26.17-18 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/usb/vialab,vl817q7.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1508: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20221228100321.15949-9-linux.amoon@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v1 08/11] dt-bindings: usb: Add binding for Via lab VL817Q7 hub controller

[Matthias Kaehlcke]

On Wed, Dec 28, 2022 at 10:03:17AM +0000, Anand Moon wrote:
> The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that
> features 4 downstream ports, an internal 5V regulator and has
> external reset pin.
> 
> Add a device tree binding for its USB protocol part.
> The internal LDO is not covered by this and can just be modelled
> as a fixed regulator.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  .../bindings/usb/vialab,vl817q7.yaml          | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> new file mode 100644
> index 000000000000..4ae995160fd5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Via labs VL817Q7 USB 3.1 hub controller

nit: VIA Labs VL817-Q7

> +
> +maintainers:
> +  - Anand Moon <linux.amoon@gmail.com>
> +
> +allOf:
> +  - $ref: usb-device.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - vialab,usb2109

This is not a valid compatible string as Johan already noted.

Besides that the VL817-Q7 provides both a 3.1 and a 2.0 USB hub, which
are enumerated separately. Please also add a compatible string for the
2.0 hub (assuming 0x2109 is the 3.1 hub).

> +
> +  reg: true
> +
> +  reset-gpios:
> +    description: GPIO controlling the RESET# pin.
> +
> +  vdd-supply:
> +    description:
> +      the regulator that provides 5.0V core power to the hub.
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    usb {
> +        dr_mode = "host";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        hub: hub@1 {
> +            compatible = "vialab,usb2109"
> +            reg = <1>;
> +            reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
> +        };

Please also add a node for the other hub and link the two nodes with
each other through the 'peer-hub' property. See realtek,rts5411.yaml
for reference.

Re: [PATCH v1 08/11] dt-bindings: usb: Add binding for Via lab VL817Q7 hub controller

[Anand Moon]

Hi Matthias,

On Thu, 5 Jan 2023 at 04:07, Matthias Kaehlcke <mka@chromium.org> wrote:
>
> On Wed, Dec 28, 2022 at 10:03:17AM +0000, Anand Moon wrote:
> > The VIA Lab VL817-Q7 is a USB 3.1 Gen 1 4-Port hub controller that
> > features 4 downstream ports, an internal 5V regulator and has
> > external reset pin.
> >
> > Add a device tree binding for its USB protocol part.
> > The internal LDO is not covered by this and can just be modelled
> > as a fixed regulator.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  .../bindings/usb/vialab,vl817q7.yaml          | 47 +++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> > new file mode 100644
> > index 000000000000..4ae995160fd5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/vialab,vl817q7.yaml
> > @@ -0,0 +1,47 @@
> > +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Via labs VL817Q7 USB 3.1 hub controller
>
> nit: VIA Labs VL817-Q7
>
Ok
> > +
> > +maintainers:
> > +  - Anand Moon <linux.amoon@gmail.com>
> > +
> > +allOf:
> > +  - $ref: usb-device.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - vialab,usb2109
>
> This is not a valid compatible string as Johan already noted.
>
> Besides that the VL817-Q7 provides both a 3.1 and a 2.0 USB hub, which
> are enumerated separately. Please also add a compatible string for the
> 2.0 hub (assuming 0x2109 is the 3.1 hub).
>
Yes, correct,
actually, I would like to rename this file to vialab,vl817.yaml
since vialab,vl817-q7 is used for USB 3.1 hub and vialab,vl817-q5 is
used for USB 2.0 hub.

[0] https://datasheet.lcsc.com/lcsc/1808111624_VIA-Tech-VL817-Q7-B0_C209756.pdf

> > +
> > +  reg: true
> > +
> > +  reset-gpios:
> > +    description: GPIO controlling the RESET# pin.
> > +
> > +  vdd-supply:
> > +    description:
> > +      the regulator that provides 5.0V core power to the hub.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    usb {
> > +        dr_mode = "host";
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        hub: hub@1 {
> > +            compatible = "vialab,usb2109"
> > +            reg = <1>;
> > +            reset-gpios = <&gpio GPIOH_4 GPIO_ACTIVE_LOW>;
> > +        };
>
> Please also add a node for the other hub and link the two nodes with
> each other through the 'peer-hub' property. See realtek,rts5411.yaml
> for reference.

Ok, I will update the example according,

Thanks

-Anand.

[PATCH v1 10/11] usb: misc: onboard_usb_hub: add VIA LAB VL817Q7 hub support

[Anand Moon]

VIA LAB VL817Q7 is a 4-port USB 3.1 hub that has a reset pin to
toggle and a 5.0V core supply exported though an integrated LDO is
available for powering it.

Add the support for this hub, for controlling the reset pin and the core
power supply.

Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/usb/misc/onboard_usb_hub.c | 2 ++
 drivers/usb/misc/onboard_usb_hub.h | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
index 699050eb3f17..025572019d16 100644
--- a/drivers/usb/misc/onboard_usb_hub.c
+++ b/drivers/usb/misc/onboard_usb_hub.c
@@ -335,6 +335,7 @@ static struct platform_driver onboard_hub_driver = {
 #define VENDOR_ID_MICROCHIP	0x0424
 #define VENDOR_ID_REALTEK	0x0bda
 #define VENDOR_ID_TI		0x0451
+#define VENDOR_ID_VIA		0x2109
 
 /*
  * Returns the onboard_hub platform device that is associated with the USB
@@ -418,6 +419,7 @@ static const struct usb_device_id onboard_hub_id_table[] = {
 	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 */
 	{ USB_DEVICE(VENDOR_ID_TI, 0x8140) }, /* TI USB8041 3.0 */
 	{ USB_DEVICE(VENDOR_ID_TI, 0x8142) }, /* TI USB8041 2.0 */
+	{ USB_DEVICE(VENDOR_ID_VIA, 0x0817) }, /* VIA VL817Q7 3.1 */
 	{}
 };
 MODULE_DEVICE_TABLE(usb, onboard_hub_id_table);
diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
index b32fad3a70f9..1fb3371ebdae 100644
--- a/drivers/usb/misc/onboard_usb_hub.h
+++ b/drivers/usb/misc/onboard_usb_hub.h
@@ -26,6 +26,10 @@ static const struct onboard_hub_pdata genesys_gl850g_data = {
 	.reset_us = 3,
 };
 
+static const struct onboard_hub_pdata vialab_vl817q7_data = {
+	.reset_us = 3,
+};
+
 static const struct of_device_id onboard_hub_match[] = {
 	{ .compatible = "usb424,2514", .data = &microchip_usb424_data, },
 	{ .compatible = "usb451,8140", .data = &ti_tusb8041_data, },
@@ -37,6 +41,7 @@ static const struct of_device_id onboard_hub_match[] = {
 	{ .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
 	{ .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
 	{ .compatible = "usbbda,5414", .data = &realtek_rts5411_data, },
+	{ .compatible = "vialab,usb2109", .data = &vialab_vl817q7_data, },
 	{}
 };
 
-- 
2.38.1

Re: [PATCH v1 10/11] usb: misc: onboard_usb_hub: add VIA LAB VL817Q7 hub support

[Matthias Kaehlcke]

Hi Andand,

On Wed, Dec 28, 2022 at 10:03:19AM +0000, Anand Moon wrote:
> VIA LAB VL817Q7 is a 4-port USB 3.1 hub that has a reset pin to
> toggle and a 5.0V core supply exported though an integrated LDO is
> available for powering it.
> 
> Add the support for this hub, for controlling the reset pin and the core
> power supply.
> 
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
>  drivers/usb/misc/onboard_usb_hub.c | 2 ++
>  drivers/usb/misc/onboard_usb_hub.h | 5 +++++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> index 699050eb3f17..025572019d16 100644
> --- a/drivers/usb/misc/onboard_usb_hub.c
> +++ b/drivers/usb/misc/onboard_usb_hub.c
> @@ -335,6 +335,7 @@ static struct platform_driver onboard_hub_driver = {
>  #define VENDOR_ID_MICROCHIP	0x0424
>  #define VENDOR_ID_REALTEK	0x0bda
>  #define VENDOR_ID_TI		0x0451
> +#define VENDOR_ID_VIA		0x2109
>  
>  /*
>   * Returns the onboard_hub platform device that is associated with the USB
> @@ -418,6 +419,7 @@ static const struct usb_device_id onboard_hub_id_table[] = {
>  	{ USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 */
>  	{ USB_DEVICE(VENDOR_ID_TI, 0x8140) }, /* TI USB8041 3.0 */
>  	{ USB_DEVICE(VENDOR_ID_TI, 0x8142) }, /* TI USB8041 2.0 */
> +	{ USB_DEVICE(VENDOR_ID_VIA, 0x0817) }, /* VIA VL817Q7 3.1 */

The VL817Q7 is a single IC, however like the TI USB8041 or the RTS5414 it
provides both a USB 3.1 and a USB 2.0 hub. You should also add an entry for
the USB 2.0 hub here.


>  	{}
>  };
>  MODULE_DEVICE_TABLE(usb, onboard_hub_id_table);
> diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
> index b32fad3a70f9..1fb3371ebdae 100644
> --- a/drivers/usb/misc/onboard_usb_hub.h
> +++ b/drivers/usb/misc/onboard_usb_hub.h
> @@ -26,6 +26,10 @@ static const struct onboard_hub_pdata genesys_gl850g_data = {
>  	.reset_us = 3,
>  };
>  
> +static const struct onboard_hub_pdata vialab_vl817q7_data = {
> +	.reset_us = 3,
> +};
> +
>  static const struct of_device_id onboard_hub_match[] = {
>  	{ .compatible = "usb424,2514", .data = &microchip_usb424_data, },
>  	{ .compatible = "usb451,8140", .data = &ti_tusb8041_data, },
> @@ -37,6 +41,7 @@ static const struct of_device_id onboard_hub_match[] = {
>  	{ .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
>  	{ .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
>  	{ .compatible = "usbbda,5414", .data = &realtek_rts5411_data, },
> +	{ .compatible = "vialab,usb2109", .data = &vialab_vl817q7_data, },

ditto

Actually you added the device id entry for the 3.1 hub and a compatible string
of the 2.0 hub (or vice versa). Above the device id is 0x0817, here it is
0x2109. Please add both USB 3.1 and 2.0 and make sure the device id and the USB
version in the comment for the device id table match.

Re: [PATCH v1 10/11] usb: misc: onboard_usb_hub: add VIA LAB VL817Q7 hub support

[Anand Moon]

Hi Matthias,

Thanks for your review comments.

On Thu, 5 Jan 2023 at 01:52, Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Hi Andand,
>
> On Wed, Dec 28, 2022 at 10:03:19AM +0000, Anand Moon wrote:
> > VIA LAB VL817Q7 is a 4-port USB 3.1 hub that has a reset pin to
> > toggle and a 5.0V core supply exported though an integrated LDO is
> > available for powering it.
> >
> > Add the support for this hub, for controlling the reset pin and the core
> > power supply.
> >
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> >  drivers/usb/misc/onboard_usb_hub.c | 2 ++
> >  drivers/usb/misc/onboard_usb_hub.h | 5 +++++
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c
> > index 699050eb3f17..025572019d16 100644
> > --- a/drivers/usb/misc/onboard_usb_hub.c
> > +++ b/drivers/usb/misc/onboard_usb_hub.c
> > @@ -335,6 +335,7 @@ static struct platform_driver onboard_hub_driver = {
> >  #define VENDOR_ID_MICROCHIP  0x0424
> >  #define VENDOR_ID_REALTEK    0x0bda
> >  #define VENDOR_ID_TI         0x0451
> > +#define VENDOR_ID_VIA                0x2109
> >
> >  /*
> >   * Returns the onboard_hub platform device that is associated with the USB
> > @@ -418,6 +419,7 @@ static const struct usb_device_id onboard_hub_id_table[] = {
> >       { USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 */
> >       { USB_DEVICE(VENDOR_ID_TI, 0x8140) }, /* TI USB8041 3.0 */
> >       { USB_DEVICE(VENDOR_ID_TI, 0x8142) }, /* TI USB8041 2.0 */
> > +     { USB_DEVICE(VENDOR_ID_VIA, 0x0817) }, /* VIA VL817Q7 3.1 */
>
> The VL817Q7 is a single IC, however like the TI USB8041 or the RTS5414 it
> provides both a USB 3.1 and a USB 2.0 hub. You should also add an entry for
> the USB 2.0 hub here.
>
Ok,

>
> >       {}
> >  };
> >  MODULE_DEVICE_TABLE(usb, onboard_hub_id_table);
> > diff --git a/drivers/usb/misc/onboard_usb_hub.h b/drivers/usb/misc/onboard_usb_hub.h
> > index b32fad3a70f9..1fb3371ebdae 100644
> > --- a/drivers/usb/misc/onboard_usb_hub.h
> > +++ b/drivers/usb/misc/onboard_usb_hub.h
> > @@ -26,6 +26,10 @@ static const struct onboard_hub_pdata genesys_gl850g_data = {
> >       .reset_us = 3,
> >  };
> >
> > +static const struct onboard_hub_pdata vialab_vl817q7_data = {
> > +     .reset_us = 3,
> > +};
> > +
> >  static const struct of_device_id onboard_hub_match[] = {
> >       { .compatible = "usb424,2514", .data = &microchip_usb424_data, },
> >       { .compatible = "usb451,8140", .data = &ti_tusb8041_data, },
> > @@ -37,6 +41,7 @@ static const struct of_device_id onboard_hub_match[] = {
> >       { .compatible = "usbbda,5411", .data = &realtek_rts5411_data, },
> >       { .compatible = "usbbda,414", .data = &realtek_rts5411_data, },
> >       { .compatible = "usbbda,5414", .data = &realtek_rts5411_data, },
> > +     { .compatible = "vialab,usb2109", .data = &vialab_vl817q7_data, },
>
> ditto
>
> Actually you added the device id entry for the 3.1 hub and a compatible string
> of the 2.0 hub (or vice versa). Above the device id is 0x0817, here it is
> 0x2109. Please add both USB 3.1 and 2.0 and make sure the device id and the USB
> version in the comment for the device id table match.

Yes I messed up the compatible string
On Odrodi C4
alarm@odroid-c4:~$ 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 2109:0817 VIA Labs, Inc.
/:  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 2109:2817 VIA Labs, Inc.

vendor ID is 0x2109 and the device ID is 0817,
So I have fixed the compatible string as below.
      compatible = "usb2109,2817";  /* USB 2.0 hub */
      compatible = "usb2109,817";   /* USB 3.1 hub */

Thanks


-Anand