On Wed, 15 Jul 2020 07:16:29 +0300 Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote: > The original patch was error-ed by the submitter (me) and not by the author > (Lars). > After looking through the discussion logs (on email), it seems that this > order was wrong for the start, even though the order implemented in the > drivers was correct. > > Discussions: > - first RFC: https://lore.kernel.org/linux-iio/20180622135322.3459-1-alexandru.ardelean@xxxxxxxxxx/ > - 2nd patch: https://lore.kernel.org/linux-iio/20181219140912.22582-1-alexandru.ardelean@xxxxxxxxxx/ > - final patch-sets: > https://lore.kernel.org/linux-iio/20200522104632.517470-1-alexandru.ardelean@xxxxxxxxxx/ > https://lore.kernel.org/linux-iio/20200525113855.178821-1-alexandru.ardelean@xxxxxxxxxx/ > > The last one was applied. > > The idea is that pollfunc should be attached before calling the > 'indio_dev->setup_ops->postenable' hook and should be detached after > calling the 'indio_dev->setup_ops->predisable' hook. > > While the drivers were updated to take this into account, the change to the > IIO core was somehow omitted and was made wrong. > > This change fixes the order to the proper form. > > Fixes f11d59d87b862: ("iio: Move attach/detach of the poll func to the core") > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> Great thanks. Applied to the togreg branch of iio.git. Thanks, Jonathan > --- > drivers/iio/industrialio-buffer.c | 31 ++++++++++++++++++------------- > 1 file changed, 18 insertions(+), 13 deletions(-) > > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c > index 2aec8b85f40d..a7d7e5143ed2 100644 > --- a/drivers/iio/industrialio-buffer.c > +++ b/drivers/iio/industrialio-buffer.c > @@ -971,24 +971,29 @@ static int iio_enable_buffers(struct iio_dev *indio_dev, > goto err_disable_buffers; > } > > + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) { > + ret = iio_trigger_attach_poll_func(indio_dev->trig, > + indio_dev->pollfunc); > + if (ret) > + goto err_disable_buffers; > + } > + > if (indio_dev->setup_ops->postenable) { > ret = indio_dev->setup_ops->postenable(indio_dev); > if (ret) { > dev_dbg(&indio_dev->dev, > "Buffer not started: postenable failed (%d)\n", ret); > - goto err_disable_buffers; > + goto err_detach_pollfunc; > } > } > > - if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) { > - ret = iio_trigger_attach_poll_func(indio_dev->trig, > - indio_dev->pollfunc); > - if (ret) > - goto err_disable_buffers; > - } > - > return 0; > > +err_detach_pollfunc: > + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) { > + iio_trigger_detach_poll_func(indio_dev->trig, > + indio_dev->pollfunc); > + } > err_disable_buffers: > list_for_each_entry_continue_reverse(buffer, &iio_dev_opaque->buffer_list, > buffer_list) > @@ -1014,11 +1019,6 @@ static int iio_disable_buffers(struct iio_dev *indio_dev) > if (list_empty(&iio_dev_opaque->buffer_list)) > return 0; > > - if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) { > - iio_trigger_detach_poll_func(indio_dev->trig, > - indio_dev->pollfunc); > - } > - > /* > * If things go wrong at some step in disable we still need to continue > * to perform the other steps, otherwise we leave the device in a > @@ -1032,6 +1032,11 @@ static int iio_disable_buffers(struct iio_dev *indio_dev) > ret = ret2; > } > > + if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) { > + iio_trigger_detach_poll_func(indio_dev->trig, > + indio_dev->pollfunc); > + } > + > list_for_each_entry(buffer, &iio_dev_opaque->buffer_list, buffer_list) { > ret2 = iio_buffer_disable(buffer, indio_dev); > if (ret2 && !ret)
