[PATCH] ARM: multi_v7_defconfig: Select CONFIG_USB_ONBOARD_DEV as built-in

[Arnd Bergmann]

On Thu, May 2, 2024, at 16:49, Matthias Kaehlcke wrote:
> On Wed, May 1, 2024 at 4:04 PM Fabio Estevam <festevam at gmail.com> wrote:
>> On Tue, Apr 30, 2024 at 5:25 PM Arnd Bergmann <arnd at arndb.de> wrote:
>> >
>> > On Tue, Apr 30, 2024, at 21:53, Fabio Estevam wrote:
>> > > On Tue, Apr 30, 2024 at 4:53 AM Arnd Bergmann <arnd at arndb.de> wrote:
>> > >
>> > >> It does sound like this is something the kernel should be able to
>> > >> get to work properly in some form, but I don't think making it
>> > >> a 'bool' symbol is the correct answer here: if CONFIG_USB is
>> > >> set to =m, it would be impossible to include USB_ONBOARD_DEV
>> > >> in this case.
>> > >>
>> > >> Fabio, can you explain how making it built-in addresses the
>> > >> problem here? I assume this is related to probe order, so I
>> > >> wonder if it's just a matter of making the usb hub driver
>> > >> properly handle -EPROBE_DEFER until the onboard dev has been
>> > >> initialized.
>> > >
>> > > From drivers/usb/misc/Kconfig:
>> > >
>> > > "config USB_ONBOARD_DEV
>> > > tristate "Onboard USB device support"
>> > > .....
>> > >   This driver can be used as a module but its state (module vs
>> > >   builtin) must match the state of the USB subsystem. Enabling
>> > >   this config will enable the driver and it will automatically
>> > >   match the state of the USB subsystem. If this driver is a
>> > >   module it will be called onboard_usb_dev."
>> >
>> > Ok, so there is some kind of design mistake here that this
>> > is trying to paper over. Kbuild should always be powerful
>> > enough to enforce these things without having a person read
>> > a comment though.
>
> Agreed that it would be preferable to enforce this at Kbuild level (or
> address it otherwise without having a person to read the comment).
>
> Kconfig *build* dependencies (mutual dependencies between the USB core
> and the onboard_hub/dev driver) were a major headache that stalled
> landing the driver for a long time, with at least one revert after
> landing.
>
> The change log of the final version that landed reflects some of that ordeal:
>
> https://lore.kernel.org/linux-usb/20220630123445.v24.3.I7c9a1f1d6ced41dd8310e8a03da666a32364e790@changeid/
>
> Eventually the build dependencies were fixed, but apparently there is
> still a runtime issue :(

I can see what the link time problem was and how it's currently
being worked around. It would be nice to have something better
than the

ifdef CONFIG_USB_ONBOARD_DEV
usbcore-y                       += ../misc/onboard_usb_dev_pdevs.o
endif

bit, but this bit is obviously not the problem here.

>> > > In multi_v7_defconfig:
>> > > CONFIG_USB=y and CONFIG_USB_ONBOARD_DEV=m, so there is a mismatch.
>> > >
>> > > My patch enforces CONFIG_USB_ONBOARD_DEV=y to guarantee the matching.
>> > >
>> > > Is there any other way to solve this?
>> >
>> > I can think of multiple ways to enforce this in Kbuild:
>> >
>> > a) make CONFIG_USB_ONBOARD_DEV a 'bool' symbol and then add
>> >    a hidden symbol that duplicates the state of CONFIG_USB when
>> >    USB_ONBOARD_DEV is enabled:
>> >
>> > config CONFIG_USB_ONBOARD_DEV_MODULE
>> >      def_tristate USB
>> >      depends on CONFIG_USB_ONBOARD_DEV
>> >
>
> An intermediate version of the driver had something similar:
>
> config USB_ONBOARD_HUB
>   bool "Onboard USB hub support"
>
>   if USB_ONBOARD_HUB
>   config USB_ONBOARD_HUB_ACTUAL
>   tristate
>   default m if USB=m
>   default y if USB=y
>   endif

I think this is exactly the same think I wrote above, just written
in a more verbose way.

> https://lore.kernel.org/linux-usb/20220609121838.v22.2.I7c9a1f1d6ced41dd8310e8a03da666a32364e790@changeid/
>
> But it was removed later since the *build* dependency issues were
> resolved otherwise.
>
> We could add a construct like that again if we can't come up with a
> better solution.

The bit I still don't understand is why the current code is
broken with CONFIG_USB=y CONFIG_USB_ONBOARD_DEV=m but not
with CONFIG_USB=m.

Most likely there is a runtime dependency that requires the
usb_onboard_dev module to be initialized before probing
any USB host controller that is a parent of an onboard hub.

This can work if you load the modules in the right order, but
I don't see this being actually enforced, so why doesn't
CONFIG_USB=m CONFIG_USB_ONBOARD_DEV=m fail if you load
xhci first. If the probe deferral for the hub works correctly,
why doesn't it work with USB=y?

     Arnd