On Wed, 30 May 2018, Martin Liu wrote:
> SOC have internal I/O buses that can't be proved for devices. The
> devices on the buses can be accessed directly without additinal
> configuration required. This type of bus is represented as
> "simple-bus". In some platforms, we name "soc" with "simple-bus"
> attribute and many devices are hooked under it desribed in DT
> (device tree).
>
> In commit 'bf74ad5bc417 introduce ("[PATCH] Hold the device's
> parent's lock during probe and remove")' to solve USB subsystem
> lock sequence since usb device's characteristic. Thus "soc"
> needs to be locked whenever a device and driver's probing
> happen under "soc" bus. During this period, an async driver
> tries to probe a device which is under the "soc" bus would be
> blocked until previous driver finish the probing and release "soc"
> lock. And the next probing under the "soc" bus need to wait for
> async finish. Because of that, driver's async probe for init
> time improvement will be shadowed.
>
> Since many devices don't have USB devices' characteristic, they
> actually don't need parent's lock. Thus, we introduce a lock flag
> in bus_type struct and driver core would lock the parent lock base
> on the flag. For usbsystem, we set this flag in usb relatvie
> bus_type struct to keep original lock behavior in driver core.
>
> Async probe could have more benefit after this patch.
>
> Signed-off-by: Martin Liu <liumartin@xxxxxxxxxx>
> ---
> Changes in v3:
> -move lock flag to bus_type struct and set the flag in usb
> relative bus_type struct to keep original lock behavior.
> -fix sign name.
>
> [v2]: https://lkml.org/lkml/2018/5/29/108
> [v1]: https://lkml.org/lkml/2018/5/22/545
>
> Currently, I have the flag set in USB relatvie bus_type struct.
> Since I'm not familar with USB part, need some feedback to know
> if they cover all the cases that original case driver core
> protects. Thanks.
As far as I know, only usb_bus_type needs the flag. Not ulpi_bus or
usb_serial_bus_type. However, you should check with the maintainers to
make sure.
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 9a2ab6751a23..073954a1a1c5 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -94,6 +94,7 @@ static struct bus_type ulpi_bus = {
> .uevent = ulpi_uevent,
> .probe = ulpi_probe,
> .remove = ulpi_remove,
> + .need_parent_lock = 1,
> };
>
> /* -------------------------------------------------------------------------- */
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index 9792cedfc351..209ee5d8a92d 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -1922,4 +1922,5 @@ struct bus_type usb_bus_type = {
> .name = "usb",
> .match = usb_device_match,
> .uevent = usb_uevent,
> + .need_parent_lock = 1,
> };
> diff --git a/drivers/usb/serial/bus.c b/drivers/usb/serial/bus.c
> index 9e265eb92611..55b2636b2804 100644
> --- a/drivers/usb/serial/bus.c
> +++ b/drivers/usb/serial/bus.c
> @@ -166,6 +166,7 @@ struct bus_type usb_serial_bus_type = {
> .probe = usb_serial_device_probe,
> .remove = usb_serial_device_remove,
> .drv_groups = usb_serial_drv_groups,
> + .need_parent_lock = 1,
> };
>
> int usb_serial_bus_register(struct usb_serial_driver *driver)
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 477956990f5e..019b193aeb24 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -98,6 +98,8 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
> * @lock_key: Lock class key for use by the lock validator
> * @force_dma: Assume devices on this bus should be set up by dma_configure()
> * even if DMA capability is not explicitly described by firmware.
> + * @need_parent_lock: Assume devices on this bus should hold its' parent's
> + * lock during probe and remove. Currently, USB needs it.
This comment is not written very well. Suggested improvement:
* @need_parent_lock: When probing or removing a device on this bus, the
device core should lock the device's parent.
If you want, you can say that usb_bus_type sets the flag. I don't
think it's really necessary to mention this.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
