Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 9 Jan 2023 16:29:06 +0000
From:   Matthias Kaehlcke <mka@chromium.org>
To:     Anand Moon <linux.amoon@gmail.com>
Cc:     Icenowy Zheng <uwu@icenowy.me>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        linux-amlogic@lists.infradead.org, linux-usb@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 01/11] dt-bindings: usb: Add device id for Genesys
 Logic hub controller
Message-ID: <Y7xA0nO53cBRetz4@google.com>
References: <20221228100321.15949-1-linux.amoon@gmail.com>
 <20221228100321.15949-2-linux.amoon@gmail.com>
 <ef20a7dfa027f1a5a24a515e347af10c06a4da85.camel@icenowy.me>
 <CANAwSgSJ1SQXUovgM6FHUozr46C_CogLaAD4gf4ANNHXoav6ag@mail.gmail.com>
 <Y7YDwJX6aqa8vbQK@google.com>
 <CANAwSgTVnQXGoOfuxmwT7zCMJMY0Hw+uyQx126kNPL2WKGR8Hg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CANAwSgTVnQXGoOfuxmwT7zCMJMY0Hw+uyQx126kNPL2WKGR8Hg@mail.gmail.com>
Precedence: bulk

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?