Messages in this thread |  | | From | Andy Shevchenko <> | Date | Tue, 29 May 2018 20:08:08 +0300 | Subject | Re: [RFC PATCH v3] driver core: hold dev's parent lock when needed |
| |
On Tue, May 29, 2018 at 7:34 PM, Martin Liu <liumartin@google.com> wrote: > SOC have internal I/O buses that can't be proved for devices. The
Perhaps SoC as a common abbr for system-on-chip.
> 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).
described
> > In commit 'bf74ad5bc417 introduce ("[PATCH] Hold the device's > parent's lock during probe and remove")'
The formal commit reference doesn't include '' (surrounding quotes) and words in square brackets (like [PATCH] here).
> to solve USB subsystem > lock sequence since usb device's characteristic. Thus "soc"
usb or USB ?
> 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
USB system
USB
relative
> bus_type struct to keep original lock behavior in driver core. > > Async probe could have more benefit after this patch.
> - if (dev->parent) /* Needed for USB */ > + if (dev->parent && dev->bus->need_parent_lock)
So, why not to use bus directly like bus->...?
> device_lock(dev->parent); > device_release_driver(dev); > - if (dev->parent) > + if (dev->parent && dev->bus->need_parent_lock) > device_unlock(dev->parent);
Ditto here and everywhere else in the patch where applicable.
> + .need_parent_lock = 1, > + .need_parent_lock = 1, > + .need_parent_lock = 1,
It's boolean, you need to use true or false. Check and fix your code correspondingly.
> + bool need_parent_lock;
-- With Best Regards, Andy Shevchenko
|  |