2020-05-12 15:03:08

by Al Cooper

[permalink] [raw]
Subject: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile

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.

Signed-off-by: Al Cooper <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Tested-by: Yoshihiro Shimoda <[email protected]>
---
drivers/usb/host/Makefile | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index b191361257cc..a7f0b8ff7179 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -37,6 +37,16 @@ endif

obj-$(CONFIG_USB_PCI) += pci-quirks.o

+# NOTE: BRCMSTB systems require that xhci driver be linked before the
+# ehci/ohci drivers because they share a port and need the XHCI driver
+# to come up first to properly enumerate 3.0 devices.
+obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
+obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
+obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
+obj-$(CONFIG_USB_XHCI_HISTB) += xhci-histb.o
+obj-$(CONFIG_USB_XHCI_MTK) += xhci-mtk.o
+obj-$(CONFIG_USB_XHCI_TEGRA) += xhci-tegra.o
+
obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o
obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o
obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o
@@ -69,12 +79,6 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o

obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
-obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
-obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
-obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
-obj-$(CONFIG_USB_XHCI_HISTB) += xhci-histb.o
-obj-$(CONFIG_USB_XHCI_MTK) += xhci-mtk.o
-obj-$(CONFIG_USB_XHCI_TEGRA) += xhci-tegra.o
obj-$(CONFIG_USB_SL811_HCD) += sl811-hcd.o
obj-$(CONFIG_USB_SL811_CS) += sl811_cs.o
obj-$(CONFIG_USB_U132_HCD) += u132-hcd.o
--
2.17.1


2020-05-13 15:10:29

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile



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");

Al, do you want to test that?
--
Florian

2020-05-13 15:29:01

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile

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");

JFYI: not all user space support this (alas, but that's current state of
affairs), OTOH I don't really care about those which do not support it
(Busybox).

--
With Best Regards,
Andy Shevchenko


2020-05-13 15:30:50

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile



On 5/13/2020 8:26 AM, Andy Shevchenko 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");
>
> JFYI: not all user space support this (alas, but that's current state of
> affairs), OTOH I don't really care about those which do not support it
> (Busybox).

I know that Gentoo's initramfs tool does not support it, however given
there are no symbols being cross referenced, I am not sure how to best
support this other than using these hints, and possibly making use of
device links too?
--
Florian

2020-05-13 16:33:05

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile



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?
--
Florian

2020-05-13 17:07:57

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile

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.

good luck!

greg k-h

2020-05-13 17:42:01

by Alan Stern

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile

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.

Alan Stern

2020-05-13 17:49:05

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile



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

2020-05-13 19:45:07

by Al Cooper

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile

On Wed, May 13, 2020 at 1:46 PM Florian Fainelli <[email protected]> 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

2020-05-13 20:40:48

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile

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.

thanks,

greg k-h

2020-05-13 20:52:05

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile

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.

thanks,

greg k-h

2020-05-20 17:31:45

by Al Cooper

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile

Greg, Alan,

The other 4 related patches were accepted into usb-next and I just
realized that this one didn't make it. This patch will not fix the
"insmod out of order" issue, but will help our controllers work with
some poorly behaved USB devices when the drivers are builtin.

Thanks
Al

On Wed, May 13, 2020 at 3:42 PM Alan Cooper <[email protected]> wrote:
>
> On Wed, May 13, 2020 at 1:46 PM Florian Fainelli <[email protected]> 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

2020-05-21 06:10:54

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Wed, May 20, 2020 at 01:29:45PM -0400, Alan Cooper wrote:
> Greg, Alan,
>
> The other 4 related patches were accepted into usb-next and I just
> realized that this one didn't make it. This patch will not fix the
> "insmod out of order" issue, but will help our controllers work with
> some poorly behaved USB devices when the drivers are builtin.

As it doesn't solve the real issue, I did not accept this so that you
all can continue to work on creating a real solution that works for both
situations (built in and as modules.)

I thought I said that already...

thanks,

greg k-h

2020-05-21 15:39:22

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH v10 1/5] usb: xhci: Change the XHCI link order in the Makefile



On 5/20/2020 11:09 PM, Greg Kroah-Hartman wrote:
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
>
> A: No.
> Q: Should I include quotations after my reply?
>
> http://daringfireball.net/2007/07/on_top
>
> On Wed, May 20, 2020 at 01:29:45PM -0400, Alan Cooper wrote:
>> Greg, Alan,
>>
>> The other 4 related patches were accepted into usb-next and I just
>> realized that this one didn't make it. This patch will not fix the
>> "insmod out of order" issue, but will help our controllers work with
>> some poorly behaved USB devices when the drivers are builtin.
>
> As it doesn't solve the real issue, I did not accept this so that you
> all can continue to work on creating a real solution that works for both
> situations (built in and as modules.)
>
> I thought I said that already...

Your message was not clear to me at least, I understood your message as:
I acknowledge the problem you are trying to solve and accept Al's
solution for the case where modules are built-in, and another solution
should be found for when the modules are built as loadable modules.

But okay, your message is clear now :).
--
Florian