Re: [PATCH v2 2/3] input: touch: ads7846: switch to devm initialization

[Daniel Mack <daniel@zonque.org>]

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