linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Cooper <alcooperx@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	": Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	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>
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile
Date: Wed, 13 May 2020 15:42:25 -0400	[thread overview]
Message-ID: <CAOGqxeU=LjAmAYR9wGeKZo_eLj+ehkrANTM+04E7xWNAp82EJA@mail.gmail.com> (raw)
In-Reply-To: <c0642cf4-1436-aec4-96fd-355a897f6418@gmail.com>

On Wed, May 13, 2020 at 1:46 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 5/13/2020 10:39 AM, Alan Stern wrote:
> > On Wed, May 13, 2020 at 07:05:05PM +0200, Greg Kroah-Hartman wrote:
> >> On Wed, May 13, 2020 at 09:31:11AM -0700, Florian Fainelli wrote:
> >>>
> >>>
> >>> On 5/13/2020 9:27 AM, Greg Kroah-Hartman wrote:
> >>>> On Wed, May 13, 2020 at 08:08:07AM -0700, Florian Fainelli wrote:
> >>>>>
> >>>>>
> >>>>> On 5/13/2020 5:26 AM, Greg Kroah-Hartman wrote:
> >>>>>> On Tue, May 12, 2020 at 11:00:15AM -0400, Al Cooper wrote:
> >>>>>>> Some BRCMSTB USB chips have an XHCI, EHCI and OHCI controller
> >>>>>>> on the same port where XHCI handles 3.0 devices, EHCI handles 2.0
> >>>>>>> devices and OHCI handles <2.0 devices. Currently the Makefile
> >>>>>>> has XHCI linking at the bottom which will result in the XHIC driver
> >>>>>>> initalizing after the EHCI and OHCI drivers and any installed 3.0
> >>>>>>> device will be seen as a 2.0 device. Moving the XHCI linking
> >>>>>>> above the EHCI and OHCI linking fixes the issue.
> >>>>>>
> >>>>>> What happens if all of these are modules and they are loaded in a
> >>>>>> different order?  This makefile change will not help with that, you need
> >>>>>> to have logic in the code in order to properly coordinate this type of
> >>>>>> mess, sorry.
> >>>>>
> >>>>> I believe we should be using module soft dependencies to instruct the
> >>>>> module loaders to load the modules in the correct order, so something
> >>>>> like this would do (not tested) for xhci-plat-hcd.c:
> >>>>>
> >>>>> MODULE_SOFTDEP("post: ehci-hcd ohci-hcd");
> >>>>>
> >>>>> and I am not sure whether we need to add the opposite for ehci-hcd and
> >>>>> ohci-hcd:
> >>>>>
> >>>>> MODULE_SOFTDEP("pre: xhci-plat-hcd");
> >>>>
> >>>> That's a nice start, but what happens if that isn't honored?  This
> >>>> really needs to work properly for any order as you never can guarantee
> >>>> module/driver loading order in a system of modules.
> >>>
> >>> I also suggested that device links may help, though I am not sure. What
> >>> do you suggest to be done?
> >>
> >> No idea.  device links will help if you defer the probe properly until
> >> you see the proper drivers binding correctly.
> >
> > I suspect that in general there is no way to do this properly.
> >
> > We can't modify ehci-hcd and ohci-hcd to make them wait.  In fact, for
> > all they know, xhci-hcd will _never_ be loaded.
> >
> > One thing that might be possible (although not all platforms may support
> > it) is if xhci-hcd could somehow disconnect all devices attached to a
> > peer port when it starts up.  But that would be disruptive to any
> > devices that aren't USB-3.
> >
> > We faced a very similar ordering problem between ehci-hcd and
> > [ou]hci-hcd many years ago, and we never found a good solution.
> > We did arrange the link order so that ehci-hcd precedes the others, and
> > we added a warning message to ehci-hcd which gets printed if the module
> > initialization routine runs after [ou]hci-hcd is loaded.  Also, there
> > are MODULE_SOFTDEP lines in ohci-pci.c and uhci-pci.c.
>
> Given that these modules are used on specific SoC platforms, where we
> usually provide a reference implementation of user space and kernel
> space and documentation, it seems to me that the MODULE_SOFTDEP(),
> despite being a hint and best effort from user space module loaders is
> probably acceptable.
> --
> Florian

What I found in the past is that things work. For example if the ehci
driver starts first, the USB device will come up as a 2.0 device and
when the XHCI driver comes up the device will switch to 3.0. I've see
the same thing happen if OHCI starts before EHCI. It's just that there
are some poorly behaved USB devices that have trouble with this.

Al

  reply	other threads:[~2020-05-13 19:42 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-12 15:00 [PATCH v10 0/5] Add XHCI, EHCI and OHCI support for Broadcom STB SoS's Al Cooper
2020-05-12 15:00 ` [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile Al Cooper
2020-05-13 12:26   ` Greg Kroah-Hartman
2020-05-13 15:08     ` Florian Fainelli
2020-05-13 15:26       ` Andy Shevchenko
2020-05-13 15:28         ` Florian Fainelli
2020-05-13 16:27       ` Greg Kroah-Hartman
2020-05-13 16:31         ` Florian Fainelli
2020-05-13 17:05           ` Greg Kroah-Hartman
2020-05-13 17:39             ` Alan Stern
2020-05-13 17:46               ` Florian Fainelli
2020-05-13 19:42                 ` Alan Cooper [this message]
2020-05-20 17:29                   ` Alan Cooper
2020-05-21  6:09                     ` Greg Kroah-Hartman
2020-05-21 15:37                       ` Florian Fainelli
2020-05-12 15:00 ` [PATCH v10 2/5] dt-bindings: Add Broadcom STB USB support Al Cooper
2020-05-12 15:00 ` [PATCH v10 3/5] usb: xhci: xhci-plat: Add support for Broadcom STB SoC's Al Cooper
2020-05-12 15:00 ` [PATCH v10 4/5] usb: ehci: Add new EHCI driver " Al Cooper
2020-05-12 15:00 ` [PATCH v10 5/5] usb: host: Add ability to build new Broadcom STB USB drivers Al Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOGqxeU=LjAmAYR9wGeKZo_eLj+ehkrANTM+04E7xWNAp82EJA@mail.gmail.com' \
    --to=alcooperx@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=robh+dt@kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=yoshihiro.shimoda.uh@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).