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

[Henrik Rydberg]

> > > @@ -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.

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/