Hi Dmitry,
Thanks for having a look!
On 5/6/20 2:05 AM, Dmitry Torokhov wrote:
> On Tue, May 05, 2020 at 10:37:01AM +0200, Marco Felsch wrote:
>> On 20-05-04 19:37, Daniel Mack wrote:
>>> @@ -1488,30 +1472,11 @@ static int ads7846_remove(struct spi_device *spi)
>>> struct ads7846 *ts = spi_get_drvdata(spi);
>>>
>>> sysfs_remove_group(&spi->dev.kobj, &ads784x_attr_group);
>>> -
>>> ads7846_disable(ts);
>>> - free_irq(ts->spi->irq, ts);
>>> -
>>> - input_unregister_device(ts->input);
>>> -
>>> - ads784x_hwmon_unregister(spi, ts);
>>> -
>>> - regulator_put(ts->reg);
>>> -
>>> - if (!ts->get_pendown_state) {
>>> - /*
>>> - * If we are not using specialized pendown method we must
>>> - * have been relying on gpio we set up ourselves.
>>> - */
>>> - gpio_free(ts->gpio_pendown);
>>> - }
>>>
>>> if (ts->filter_cleanup)
>>> ts->filter_cleanup(ts->filter_data);
>
> This makes filter_cleanup() be called much earlier now, before we free
> interrupt, unregister input device, etc.
>
> I am very concerned with mixing manual unwinding and devm and would
> very much prefer if everything would be converted to devm.
Yes, I see. These filter_init()/filter_cleanup() functions can
potentially be set by pdata users, hence I thought it's easiest if we
keep it as-is.
However, turns out this pdata logic has no active user in mainline, so I
can just add a patch to remove it completely and simplify the code further.
Will pick up yours and Marco's comments and put them in a v3.
Thanks!
Daniel
