Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200430111258.6091-1-alcooperx@gmail.com> <20200430111258.6091-5-alcooperx@gmail.com>
 <20200505105413.GA93160@kroah.com>
In-Reply-To: <20200505105413.GA93160@kroah.com>
From:   Alan Cooper <alcooperx@gmail.com>
Date:   Wed, 6 May 2020 14:59:10 -0400
Message-ID: <CAOGqxeUJsYmQS8N3yvb8U76J-afFa9qKiu+S0=SK1OYmu+qYeQ@mail.gmail.com>
Subject: Re: [PATCH v6 4/4] usb: host: Add ability to build new Broadcom STB
 USB drivers
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     ": Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
        Alan Stern <stern@rowland.harvard.edu>,
        Andy Shevchenko <andy.shevchenko@gmail.com>,
        BCM Kernel Feedback <bcm-kernel-feedback-list@broadcom.com>,
        DTML <devicetree@vger.kernel.org>,
        Krzysztof Kozlowski <krzk@kernel.org>,
        USB list <linux-usb@vger.kernel.org>,
        Mathias Nyman <mathias.nyman@intel.com>,
        Rob Herring <robh+dt@kernel.org>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, May 5, 2020 at 6:54 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Apr 30, 2020 at 07:12:58AM -0400, Al Cooper wrote:
> > Add the build system changes needed to get the Broadcom STB XHCI,
> > EHCI and OHCI functionality working. The OHCI support does not
> > require anything unique to Broadcom so the standard ohci-platform
> > driver is being used. The link order for XHCI was changed in the
> > Makefile because of the way STB XHCI, EHCI and OHCI controllers
> > share a port which requires that the XHCI driver be initialized
> > first. Also update MAINTAINERS.
> >
> > Signed-off-by: Al Cooper <alcooperx@gmail.com>
> > ---
> >  MAINTAINERS               |  8 ++++++++
> >  drivers/usb/host/Kconfig  | 16 ++++++++++++++++
> >  drivers/usb/host/Makefile | 16 ++++++++++------
> >  3 files changed, 34 insertions(+), 6 deletions(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 26f281d9f32a..6147ed78d212 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3481,6 +3481,14 @@ S:     Supported
> >  F:   Documentation/devicetree/bindings/i2c/brcm,brcmstb-i2c.yaml
> >  F:   drivers/i2c/busses/i2c-brcmstb.c
> >
> > +BROADCOM BRCMSTB USB EHCI DRIVER
> > +M:   Al Cooper <alcooperx@gmail.com>
> > +L:   linux-usb@vger.kernel.org
> > +L:   bcm-kernel-feedback-list@broadcom.com
> > +S:   Maintained
> > +F:   Documentation/devicetree/bindings/usb/brcm,bcm7445-ehci.yaml
> > +F:   drivers/usb/host/ehci-brcm.*
> > +
> >  BROADCOM BRCMSTB USB2 and USB3 PHY DRIVER
> >  M:   Al Cooper <alcooperx@gmail.com>
> >  L:   linux-kernel@vger.kernel.org
> > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> > index 55bdfdf11e4c..7d58fd66e412 100644
> > --- a/drivers/usb/host/Kconfig
> > +++ b/drivers/usb/host/Kconfig
> > @@ -97,6 +97,22 @@ config USB_XHCI_TEGRA
> >
> >  endif # USB_XHCI_HCD
> >
> > +config USB_EHCI_BRCMSTB
> > +       tristate
> > +
> > +config USB_BRCMSTB
> > +     tristate "Broadcom STB USB support"
> > +     depends on (ARCH_BRCMSTB && PHY_BRCM_USB) || COMPILE_TEST
> > +     select USB_OHCI_HCD_PLATFORM if USB_OHCI_HCD
> > +     select USB_EHCI_BRCMSTB if USB_EHCI_HCD
> > +     select USB_XHCI_PLATFORM if USB_XHCI_HCD
> > +     help
> > +       Say Y to enable support for XHCI, EHCI and OHCI host controllers
> > +       found in Broadcom STB SoC's.
> > +
> > +       Disabling this will keep the controllers and corresponding
> > +       PHYs powered down.
>
> Whhat are the module names?

I'll add the module names.

>
> And why 2 config options here?

I'd like the option to build an XHCI only system by not enabling
USB_EHCI_HCD and USB_OHCI_HCD.

>
> > +
> >  config USB_EHCI_HCD
> >       tristate "EHCI HCD (USB 2.0) support"
> >       depends on HAS_DMA && HAS_IOMEM
> > diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
> > index b191361257cc..85fa6ace552a 100644
> > --- a/drivers/usb/host/Makefile
> > +++ b/drivers/usb/host/Makefile
> > @@ -37,6 +37,15 @@ endif
> >
> >  obj-$(CONFIG_USB_PCI)        += pci-quirks.o
> >
> > +# NOTE: BRCMSTB systems require that xhci driver be linked before the
> > +# ehci/ohci drivers.
>
> Why?  Please do this as a separate change before your makefile changes.

Okay

>
> And what else will this break?  We have relied on this link order for a
> very long time, changing it could cause issues...
>
> I don't see how your driver needs this, please explain in great detail.

For this explaination, I'm going to call our EHCI and OHCI controllers EOHCI.
We have some SoC's that have an XHCI controller and an EOHCI
controller on the same  port, where the XHCI controller handles the
3.0 devices and the EOHCI handles the <= 2.0 devices. If the EOHCI
controller comes up before the XHCI controller, any 3.0 devices
installed on the port will be seen as a 2.0 device. Once the 3.0
controller comes up, most 3.0 USB devices will switch to 3.0, but this
seems to cause problems on some USB devices.
In the past, this wasn't a problem because we had custom XHCI, EHCI
and OHCI drivers that I could order any way necessary. Now that the
standard platform XHCI and OHCI drivers are being used this becomes a
problem.

Thanks for the review
Al

>
> thanks,
>
> greg k-h