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;
MIME-Version: 1.0
References: <20220609121838.v22.2.I7c9a1f1d6ced41dd8310e8a03da666a32364e790@changeid>
 <CAD=FV=W6erE8ByabmYSL_OWJPKYGqysDMGYQX6j7_PSEYGZ4YQ@mail.gmail.com>
 <YqpprpUHmlD62YzI@google.com> <CAD=FV=VNDamV4+j07TrnX3cUs2-D5ySbeQ-zfU=Eef8+WagGig@mail.gmail.com>
 <Yqub17iT4O7aqFMi@google.com> <CAD=FV=VEztPLhsrJecZUdyHCW7ZfFTVvxyqY5CqRVv2mWyrLog@mail.gmail.com>
 <YquoSMiQS+RG8rOM@google.com> <CAD=FV=W81pSEUbzw2ZQgs_TJ9MLnHQHiDopZXZ6bHdS7QMzAyA@mail.gmail.com>
 <YqvMffveCPiKQEUk@google.com> <CAD=FV=UJOStPfRR3Hq2DmRBSH-HCtZ16hAU9eVH5w6Hm=WSJRQ@mail.gmail.com>
 <YqytDNB2y4+qT8GD@google.com>
In-Reply-To: <YqytDNB2y4+qT8GD@google.com>
From:   Doug Anderson <dianders@chromium.org>
Date:   Fri, 24 Jun 2022 13:33:19 -0700
Message-ID: <CAD=FV=UT0XtMjZ9syQPGXeTEaUrwGTb_LgDow+cofgmx4D30VA@mail.gmail.com>
Subject: Re: [PATCH v22 2/3] usb: misc: Add onboard_usb_hub driver
To:     Matthias Kaehlcke <mka@chromium.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Alan Stern <stern@rowland.harvard.edu>,
        Rob Herring <robh+dt@kernel.org>,
        Frank Rowand <frowand.list@gmail.com>,
        Mathias Nyman <mathias.nyman@intel.com>,
        Felipe Balbi <balbi@kernel.org>,
        Michal Simek <michal.simek@xilinx.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Krzysztof Kozlowski <krzk@kernel.org>,
        Stephen Boyd <swboyd@chromium.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>, Bastien Nocera <hadess@hadess.net>,
        Peter Chen <peter.chen@kernel.org>,
        Ravi Chandra Sadineni <ravisadineni@chromium.org>,
        Roger Quadros <rogerq@kernel.org>,
        Linux USB List <linux-usb@vger.kernel.org>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Souradeep Chowdhury <quic_schowdhu@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi,

On Fri, Jun 17, 2022 at 9:34 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> > Looking at the "companion-hub" case with fresh eyes, too, I wonder if
> > that can be simpler. If we find a companion hub, do we need both the
> > check for usb_hcd_is_primary_hcd() and the check to see whether the
> > pdev was already created?
>
> I was also doubting about this and concluded that it is still needed.
>
> Let's use once more the trogdor config as example, which has one physical
> onboard hub chip with a USB 3.1 hub and a USB 2.1 companion hub, connected
> to the dwc3 controller:
>
> &usb_1_dwc3 {
>         dr_mode = "host";
>         #address-cells = <1>;
>         #size-cells = <0>;
>
>         /* 2.x hub on port 1 */
>         usb_hub_2_x: hub@1 {
>                 compatible = "usbbda,5411";
>                 reg = <1>;
>                 vdd-supply = <&pp3300_hub>;
>                 companion-hub = <&usb_hub_3_x>;
>         };
>
>         /* 3.x hub on port 2 */
>         usb_hub_3_x: hub@2 {
>                 compatible = "usbbda,411";
>                 reg = <2>;
>                 vdd-supply = <&pp3300_hub>;
>                 companion-hub = <&usb_hub_2_x>;
>         };
> };
>
> Let's assume we don't check for the pdev. With our change above for root hubs
> the loop is now only executed for the primary HCD. In the first iteration
> we encounter the 2.x hub, it has a companion hub, but that alone doesn't
> tell us much, so we create a pdev. In the next iteration we encouter the
> 3.x hub, it also has a companion hub, but we don't know/check that the
> companion already has a pdev, so we create another one for the same
> physical hub.

Ah, you are correct. You only run into that case for the root hub,
correct? For everything else it's impossible?

...and I guess things would be different if inside the loop you
actually set "hcd" to point to the "hcd" of the child device. I guess
that's where my confusion keeps stemming from. "hcd" is the parent's
host controller which is not always the same as the child's host
controller.

It would have been keen if we could somehow know the child's host
controller and get a pointer to that, but we can't because the child
device hasn't been enumerated yet.

OK, I'm convinced. I'll mention it in your v23 but maybe I'll have a
slightly better chance of figuring this out if/when I look at this
again if we rename "hcd" to "parent_hcd".


-Doug