| Message ID | 20240726123609.57191-1-sjg@chromium.org |
|---|---|
| State | Accepted |
| Commit | 59ca3a2f20f37fd45e4ccf266ae2df9379c7af91 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | usb: bootm: Drop old USB-device-removal code | expand |
Hi Simon, Thank you for the patch. On ven., juil. 26, 2024 at 06:36, Simon Glass <sjg@chromium.org> wrote: > USB is stopped using driver model now, in dm_remove_devices_flags() in > announce_and_cleanup() at the top of this file. > > The usb_stop() call actually unbinds devices. > > When a USB device is unbound, it causes any bootflows attached to it to > be removed, via a call to bootdev_clear_bootflows() from > bootdev_pre_unbind(). This obviously makes it impossible to boot the > bootflow. > > However, when booting a bootflow that relies on USB, usb_stop() is > called, which unbinds the device. At that point any information > attached to the bootflow is dropped. > > This is quite risky since the contents of freed memory are not > guaranteed to remain unchanged. Depending on what other options are > done before boot, a hard-to-find bug may crop up. > > Drop the call to this old function. > > Leave the netconsole call there, since this needs conversion to > driver model. > > Signed-off-by: Simon Glass <sjg@chromium.org> > Suggested-by: Shantur Rathore <i@shantur.com> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > --- > The original patch was here: > https://patchwork.ozlabs.org/project/uboot/patch/ > 20231119121144.v5.5.If206027372f73ce32480223e5626f4b944e281b7@changeid/ > > boot/bootm.c | 12 ------------ > 1 file changed, 12 deletions(-) > > diff --git a/boot/bootm.c b/boot/bootm.c > index 376d63aafc9..480f8e6a0e6 100644 > --- a/boot/bootm.c > +++ b/boot/bootm.c > @@ -740,18 +740,6 @@ ulong bootm_disable_interrupts(void) > eth_halt(); > #endif > > -#if defined(CONFIG_CMD_USB) > - /* > - * turn off USB to prevent the host controller from writing to the > - * SDRAM while Linux is booting. This could happen (at least for OHCI > - * controller), because the HCCA (Host Controller Communication Area) > - * lies within the SDRAM and the host controller writes continously to > - * this area (as busmaster!). The HccaFrameNumber is for example > - * updated every 1 ms within the HCCA structure in SDRAM! For more > - * details see the OpenHCI specification. > - */ > - usb_stop(); > -#endif > return iflag; > } > > -- > 2.34.1
On Fri, 26 Jul 2024 06:36:09 -0600, Simon Glass wrote: > USB is stopped using driver model now, in dm_remove_devices_flags() in > announce_and_cleanup() at the top of this file. > > The usb_stop() call actually unbinds devices. > > When a USB device is unbound, it causes any bootflows attached to it to > be removed, via a call to bootdev_clear_bootflows() from > bootdev_pre_unbind(). This obviously makes it impossible to boot the > bootflow. > > [...] Applied to u-boot/master, thanks!
diff --git a/boot/bootm.c b/boot/bootm.c index 376d63aafc9..480f8e6a0e6 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -740,18 +740,6 @@ ulong bootm_disable_interrupts(void) eth_halt(); #endif -#if defined(CONFIG_CMD_USB) - /* - * turn off USB to prevent the host controller from writing to the - * SDRAM while Linux is booting. This could happen (at least for OHCI - * controller), because the HCCA (Host Controller Communication Area) - * lies within the SDRAM and the host controller writes continously to - * this area (as busmaster!). The HccaFrameNumber is for example - * updated every 1 ms within the HCCA structure in SDRAM! For more - * details see the OpenHCI specification. - */ - usb_stop(); -#endif return iflag; }
USB is stopped using driver model now, in dm_remove_devices_flags() in announce_and_cleanup() at the top of this file. The usb_stop() call actually unbinds devices. When a USB device is unbound, it causes any bootflows attached to it to be removed, via a call to bootdev_clear_bootflows() from bootdev_pre_unbind(). This obviously makes it impossible to boot the bootflow. However, when booting a bootflow that relies on USB, usb_stop() is called, which unbinds the device. At that point any information attached to the bootflow is dropped. This is quite risky since the contents of freed memory are not guaranteed to remain unchanged. Depending on what other options are done before boot, a hard-to-find bug may crop up. Drop the call to this old function. Leave the netconsole call there, since this needs conversion to driver model. Signed-off-by: Simon Glass <sjg@chromium.org> Suggested-by: Shantur Rathore <i@shantur.com> --- The original patch was here: https://patchwork.ozlabs.org/project/uboot/patch/ 20231119121144.v5.5.If206027372f73ce32480223e5626f4b944e281b7@changeid/ boot/bootm.c | 12 ------------ 1 file changed, 12 deletions(-)