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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
