| Message ID | 20201113212650.507680-8-alexandre.belloni@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: adc: at91_adc: cleanup DT bindings | expand |
On Fri, 13 Nov 2020 22:26:48 +0100 Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > Use devm_input_allocate_device to allocate the input device to simplify the > error and remove paths. > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> This one I'm less keen on. Whilst it's obviously not a problem in this particular case I'd ideally like to keep the remove order as the exact reverse of probe - that makes it easy to review changes quickly. Now, you could easily enough make this fine by using devm for the other items that happen before this (dev_add_action_or_reset needed in a few cases). Jonathan > --- > drivers/iio/adc/at91_adc.c | 25 +++---------------------- > 1 file changed, 3 insertions(+), 22 deletions(-) > > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c > index 76aeebce6f4d..d09ceb315c5a 100644 > --- a/drivers/iio/adc/at91_adc.c > +++ b/drivers/iio/adc/at91_adc.c > @@ -937,9 +937,8 @@ static int at91_ts_register(struct iio_dev *idev, > { > struct at91_adc_state *st = iio_priv(idev); > struct input_dev *input; > - int ret; > > - input = input_allocate_device(); > + input = devm_input_allocate_device(&pdev->dev); > if (!input) { > dev_err(&idev->dev, "Failed to allocate TS device!\n"); > return -ENOMEM; > @@ -964,8 +963,7 @@ static int at91_ts_register(struct iio_dev *idev, > if (st->touchscreen_type != ATMEL_ADC_TOUCHSCREEN_4WIRE) { > dev_err(&pdev->dev, > "This touchscreen controller only support 4 wires\n"); > - ret = -EINVAL; > - goto err; > + return -EINVAL; > } > > input_set_abs_params(input, ABS_X, 0, (1 << MAX_RLPOS_BITS) - 1, > @@ -977,20 +975,7 @@ static int at91_ts_register(struct iio_dev *idev, > st->ts_input = input; > input_set_drvdata(input, st); > > - ret = input_register_device(input); > - if (ret) > - goto err; > - > - return ret; > - > -err: > - input_free_device(st->ts_input); > - return ret; > -} > - > -static void at91_ts_unregister(struct at91_adc_state *st) > -{ > - input_unregister_device(st->ts_input); > + return input_register_device(input); > } > > static int at91_adc_probe(struct platform_device *pdev) > @@ -1203,8 +1188,6 @@ static int at91_adc_probe(struct platform_device *pdev) > if (!st->touchscreen_type) { > at91_adc_trigger_remove(idev); > at91_adc_buffer_remove(idev); > - } else { > - at91_ts_unregister(st); > } > error_disable_adc_clk: > clk_disable_unprepare(st->adc_clk); > @@ -1224,8 +1207,6 @@ static int at91_adc_remove(struct platform_device *pdev) > if (!st->touchscreen_type) { > at91_adc_trigger_remove(idev); > at91_adc_buffer_remove(idev); > - } else { > - at91_ts_unregister(st); > } > clk_disable_unprepare(st->adc_clk); > clk_disable_unprepare(st->clk);
On 14/11/2020 17:13:40+0000, Jonathan Cameron wrote: > On Fri, 13 Nov 2020 22:26:48 +0100 > Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > > Use devm_input_allocate_device to allocate the input device to simplify the > > error and remove paths. > > > > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > > This one I'm less keen on. Whilst it's obviously not a problem in > this particular case I'd ideally like to keep the remove order > as the exact reverse of probe - that makes it easy to review changes > quickly. > > Now, you could easily enough make this fine by using devm for the > other items that happen before this (dev_add_action_or_reset needed > in a few cases). > Right, I'll drop it for now.
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c index 76aeebce6f4d..d09ceb315c5a 100644 --- a/drivers/iio/adc/at91_adc.c +++ b/drivers/iio/adc/at91_adc.c @@ -937,9 +937,8 @@ static int at91_ts_register(struct iio_dev *idev, { struct at91_adc_state *st = iio_priv(idev); struct input_dev *input; - int ret; - input = input_allocate_device(); + input = devm_input_allocate_device(&pdev->dev); if (!input) { dev_err(&idev->dev, "Failed to allocate TS device!\n"); return -ENOMEM; @@ -964,8 +963,7 @@ static int at91_ts_register(struct iio_dev *idev, if (st->touchscreen_type != ATMEL_ADC_TOUCHSCREEN_4WIRE) { dev_err(&pdev->dev, "This touchscreen controller only support 4 wires\n"); - ret = -EINVAL; - goto err; + return -EINVAL; } input_set_abs_params(input, ABS_X, 0, (1 << MAX_RLPOS_BITS) - 1, @@ -977,20 +975,7 @@ static int at91_ts_register(struct iio_dev *idev, st->ts_input = input; input_set_drvdata(input, st); - ret = input_register_device(input); - if (ret) - goto err; - - return ret; - -err: - input_free_device(st->ts_input); - return ret; -} - -static void at91_ts_unregister(struct at91_adc_state *st) -{ - input_unregister_device(st->ts_input); + return input_register_device(input); } static int at91_adc_probe(struct platform_device *pdev) @@ -1203,8 +1188,6 @@ static int at91_adc_probe(struct platform_device *pdev) if (!st->touchscreen_type) { at91_adc_trigger_remove(idev); at91_adc_buffer_remove(idev); - } else { - at91_ts_unregister(st); } error_disable_adc_clk: clk_disable_unprepare(st->adc_clk); @@ -1224,8 +1207,6 @@ static int at91_adc_remove(struct platform_device *pdev) if (!st->touchscreen_type) { at91_adc_trigger_remove(idev); at91_adc_buffer_remove(idev); - } else { - at91_ts_unregister(st); } clk_disable_unprepare(st->adc_clk); clk_disable_unprepare(st->clk);
Use devm_input_allocate_device to allocate the input device to simplify the error and remove paths. Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> --- drivers/iio/adc/at91_adc.c | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-)