2022-12-28 10:37:42

by Anand Moon

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

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 <[email protected]>
---
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


2023-01-04 21:16:04

by Matthias Kaehlcke

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

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 <[email protected]>
> ---
> 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
>

2023-01-07 15:21:36

by Anand Moon

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

Hi Matthias,

Thanks for your review comments,

On Thu, 5 Jan 2023 at 02:13, Matthias Kaehlcke <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >

2023-01-09 16:45:29

by Matthias Kaehlcke

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

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 <[email protected]> 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 <[email protected]>
> > > ---
> > > 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/[email protected]/