Re: [PATCH] Input: introduce managed input devices (add devres support)

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Mon, Oct 29, 2012 at 09:02:26PM +0100, Henrik Rydberg wrote:
> > > > @@ -1766,8 +1830,14 @@ EXPORT_SYMBOL(input_allocate_device);
> > > >   */
> > > >  void input_free_device(struct input_dev *dev)
> > > >  {
> > > > -	if (dev)
> > > > +	if (dev) {
> > > > +		if (dev->devres_managed)
> > > > +			WARN_ON(devres_destroy(dev->dev.parent,
> > > > +						devm_input_device_release,
> > > > +						devm_input_device_match,
> > > > +						dev));
> > > >  		input_put_device(dev);
> > > 
> > > Device is put twice?
> > 
> > No, devres_destroy() does not actually run the release handler so we
> > need to call it explicitly.
> 
> Ok, I see it now - it merely uses the handler to qualify the matching object.
> 
> > > Why not add the resource to the input device instead? For one, it
> > > would make the order of unregister and release more apparent.
> > 
> > And what would that achieve? What would trigger unregistration?
> 
> As you say, it is a matter of view. We do not want to replay the whole
> "function with object argument or object with member function"
> debate. :-)
> 
> > > Right
> > > now, the code seems to rely on the reverse for-loop in the devres
> > > implementation.
> > 
> > That is the whole point of devres: it releases resources attached to
> > the parent device (either when ->probe() fails or after ->remove() is
> > called) in the opposed order of acquiring said resources. Think of it as
> > calling destructors in C++ code.
> 
> That's what I did, but I mapped register() to a member of the input
> resource, rather than to the parent device. If the parent device does
> not need to know how to unregister the input device, it makes sense to
> do so.
> 
> Either way, the code looks functional to me.

So is that "reviewed-by"?

Thanks.

-- 
Dmitry