Hello,
On Tuesday 15 Aug 2017 19:09:33 Sakari Ailus wrote:
> On Mon, Jul 31, 2017 at 12:31:58AM +0200, Niklas Söderlund wrote:
> > The re-probing of subdevices when unregistering a notifier is tricky to
> > understand, and implemented somewhat as a hack. Add a comment trying to
> > explain why the re-probing is needed in the first place and why existing
> > helper functions can't be used in this situation.
> >
> > Signed-off-by: Niklas Söderlund <[email protected]>
> > ---
> >
> > drivers/media/v4l2-core/v4l2-async.c | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c
> > b/drivers/media/v4l2-core/v4l2-async.c index
> > d91ff0a33fd3eaff..a3c5a1f6d4d2ab03 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -234,6 +234,23 @@ void v4l2_async_notifier_unregister(struct
> > v4l2_async_notifier *notifier)>
> > mutex_unlock(&list_lock);
> >
> > + /*
> > + * Try to re-probe the subdevices which where part of the notifier.
> > + * This is done so subdevices which where part of the notifier will
> > + * be re-probed to a pristine state and put back on the global
> > + * list of subdevices so they can once more be found and associated
> > + * with a new notifier.
>
> Instead of tweaking the code trying to handle unhandleable error conditions
> in notifier unregistration and adding lengthy stories on why this is done
> the way it is, could we simply get rid of the driver re-probing?
>
> I can't see why drivers shouldn't simply cope with the current interfaces
> without re-probing to which I've never seen any reasoned cause. When a
> sub-device driver is unbound, simply return the sub-device node to the list
> of async sub-devices.
I agree, this is a hack that we should get rid of. Reprobing has been there
from the very beginning, it's now 4 years and a half old, let's allow it to
retire :-)
> Or can someone come up with a valid reason why the re-probing code should
> stay? :-)
>
> > + *
> > + * One might be tempted to use device_reprobe() to handle the re-
> > + * probing. Unfortunately this is not possible since some video
> > + * device drivers call v4l2_async_notifier_unregister() from
> > + * there remove function leading to a dead lock situation on
> > + * device_lock(dev->parent). This lock is held when video device
> > + * drivers remove function is called and device_reprobe() also
> > + * tries to take the same lock, so using it here could lead to a
> > + * dead lock situation.
> > + */
> > +
> > for (i = 0; i < count; i++) {
> >
> > /* If we handled USB devices, we'd have to lock the parent too
*/
> > device_release_driver(dev[i]);
--
Regards,
Laurent Pinchart
