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

[Matthias Kaehlcke]

On Wed, May 1, 2024 at 4:04 PM Fabio Estevam <festevam at gmail.com> wrote:
>
> Adding Matthias, the onboard usb hub driver maintainer.
>
> 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 :(

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

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.

> > b) Do the same thing but use Makefile syntax instead of Kconfig
> >    syntax:
> >
> > usb-onboard-dev-$(CONFIG_USB_ONBOARD_DEV) += onboard_usb_dev.o
> > obj-$(CONFIG_USB) += usb-onboard-dev.o
> >
> > (or something close to this, need to try).
> >
> >
> > However, I still feel there should be a better way to solve
> > the problem at the code level rather than the Kbuild level.
> >
> > I don't know exactly how a "usb_device_driver" (as opposed
> > to a "usb_driver") works here, but from what I can tell,
> > the idea here is that the USB bus probe finds a device, matches
> > the driver and then continues probing it. If the onboard-dev
> > driver gets loaded after the initial bus probe, that driver
> > won't exist yet and it then runs into some error that
> > prevents it from being retried when after the onboard-dev
> > driver is there. If you can pinpoint exactly what error it
> > runs into, try to make it retry the same thing after
> > -EPROBE_DEFER.
> >
> >     Arnd