On Tue, Dec 20, 2022 at 05:19:34PM +0100, Stefan Wahren wrote:
> Hi Matthias,
>
> Am 20.12.22 um 01:30 schrieb Matthias Kaehlcke:
> > On Mon, Dec 19, 2022 at 11:32:58PM +0100, Stefan Wahren wrote:
> > > Hi Matthias,
> > >
> > > Am 19.12.22 um 18:44 schrieb Matthias Kaehlcke:
> > > > Hi Stefan,
> > > >
> > > > Sorry for the regression.
> > > >
> > > > What seems to happen is this:
> > > >
> > > > arch/arm/boot/dts/bcm283x-rpi-lan7515.dtsi specifies device nodes for the
> > > > two (nested) USB hubs (which is done rarely since USB devices (including
> > > > hubs) are autodetected). The DT nodes were most likely only added to be
> > > > able to configure the LED modes of the USB to Ethernet adapter. With
> > > > 43993626de00 ("usb: misc: onboard-hub: add support for Microchip USB2514B
> > > > USB 2.0 hub") the onboard_usb_hub driver gained support for the hubs on
> > > > the RPi3. The onboard_usb_hub driver expects a regulator ("vdd") in the DT
> > > > node of the USB hub, which isn't present for the RPi3 (this isn't an error
> > > > per se). Without the regulator the onboard_hub platform driver fails to
> > > > probe, when the USB driver of the hub is probed (onboard_hub_usbdev_probe())
> > > > it doesn't find the corresponding platform driver instance
> > > > (_find_onboard_hub()) and defers probing. When the deferred probe runs it
> > > > encounters the same situation, rinse and repeat.
> > > I forgot to mention that in error case /sys/kernel/debug/devices_deferred
> > > was empty.
> > > > One possible fix would be to specify the 'missing' "vdd" property, however
> > > > that wouln't fix the issue for other boards with a similar configurations.
> > > > Instead the driver could check if "vdd" exists in the DT node of the hub,
> > > > and not defer probing if it doesn't.
> > > >
> > > > Could you please try if the below patch fixes the issue on the Rpi 3?
> > > Yes, this prevents probing of onboard-usb-hub and the issue.
> > Thanks for the confirmation, I'll send out a proper patch to get this fixed
> > upstream.
>
> sorry, i accidentally disabled this driver during testing of the patch. So i
> erroneously assumed the patch is working, but it's not. I seems that the
> change is never reached (add dev_info around your change).
Ah, good you caught it before a 'fix' was landed :)
> The following worked on my Raspberry Pi 3 B+
>
> diff --git a/drivers/usb/misc/onboard_usb_hub.c
> b/drivers/usb/misc/onboard_usb_hub.c
> index de3627af3c84..570e9f3d2d89 100644
> --- a/drivers/usb/misc/onboard_usb_hub.c
> +++ b/drivers/usb/misc/onboard_usb_hub.c
> @@ -227,6 +227,9 @@ static int onboard_hub_probe(struct platform_device
> *pdev)
> if (!hub)
> return -ENOMEM;
>
> + if (!of_get_property(dev->of_node, "vdd", NULL))
> + return -ENODEV;
> +
> hub->vdd = devm_regulator_get(dev, "vdd");
> if (IS_ERR(hub->vdd))
> return PTR_ERR(hub->vdd);
Today I learned that regulator_get() doesn't return an error when the regulator
isn't specified, but the 'dummy regulator'. With that the platform driver is
instantiated, which is not intended. The proper fix is probably to skip the
creation of the 'raw' platform device in onboard_hub_create_pdevs() when the
'vdd-supply' does not exist.
I tried to repro the Rpi 3 case by adjusting sc7280-herobrine.dtsi to look
somewhat similar to the Rpi 3 hub config, but so far I wasn't 'successful'
with breaking USB by deleting 'vdd-supply' (and 'peer-hub'). So I don't fully
understand your scenario, but I'm relatively confident that not creating the
platform devices should fix it.
I'll try to send out a v2 of the fix before disappearing over the holidays
after tomorrow.
