On Fri, Mar 8, 2019 at 3:30 PM Tomasz Duszynski <tduszyns@xxxxxxxxx> wrote:
>
> > @@ -368,7 +376,6 @@ static int as3935_probe(struct spi_device *spi)
> >
> > spi_set_drvdata(spi, indio_dev);
> > mutex_init(&st->lock);
> > - INIT_DELAYED_WORK(&st->work, as3935_event_work);
>
> Any specific reason for moving this elsewhere?
Yes. On the remove path, cancel_delayed_work_sync() should execute after
free_irq(), but before triggered_buffer_cleanup(). So the devm_add_action()
must run right before devm_request_irq(). I figured it would make sense to
group the devm_add_action() and INIT_WORK() together, as they are
related. This also makes it easier to understand the probe/remove order
when reading the code.
> >
> > ret = of_property_read_u32(np,
> > "ams,tuning-capacitor-pf", &st->tune_cap);
> > @@ -414,22 +421,27 @@ static int as3935_probe(struct spi_device *spi)
> > iio_trigger_set_drvdata(trig, indio_dev);
> > trig->ops = &iio_interrupt_trigger_ops;
> >
> > - ret = iio_trigger_register(trig);
> > + ret = devm_iio_trigger_register(&spi->dev, trig);
> > if (ret) {
> > dev_err(&spi->dev, "failed to register trigger\n");
> > return ret;
> > }
> >
> > - ret = iio_triggered_buffer_setup(indio_dev, iio_pollfunc_store_time,
> > - &as3935_trigger_handler, NULL);
> > + ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> > + iio_pollfunc_store_time, as3935_trigger_handler, NULL);
>
> You can fix arguments alignment while you are at it.
>
What type of alignment would you prefer? This?
ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
iio_pollfunc_store_time,
as3935_trigger_handler, NULL);
