| Message ID | 20191023082714.18681-1-alexandru.ardelean@analog.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: hdc100x: fix iio_triggered_buffer_{predisable,postenable} positions | expand |
On Wed, 23 Oct 2019 11:27:14 +0300 Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > The iio_triggered_buffer_postenable() hook should be called first to > attach the poll function and the iio_triggered_buffer_predisable() hook > should be called last in the predisable hook. > > This change updates the driver to attach/detach the poll func in the > correct order. > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> Seems fine, but should have cc'd Matt and I'd like to give him time to take a quick look. +CC Matt. Thanks, Jonathan > --- > drivers/iio/humidity/hdc100x.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c > index bfe1cdb16846..963ff043eecf 100644 > --- a/drivers/iio/humidity/hdc100x.c > +++ b/drivers/iio/humidity/hdc100x.c > @@ -278,31 +278,34 @@ static int hdc100x_buffer_postenable(struct iio_dev *indio_dev) > struct hdc100x_data *data = iio_priv(indio_dev); > int ret; > > + ret = iio_triggered_buffer_postenable(indio_dev); > + if (ret) > + return ret; > + > /* Buffer is enabled. First set ACQ Mode, then attach poll func */ > mutex_lock(&data->lock); > ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, > HDC100X_REG_CONFIG_ACQ_MODE); > mutex_unlock(&data->lock); > if (ret) > - return ret; > + iio_triggered_buffer_predisable(indio_dev); > > - return iio_triggered_buffer_postenable(indio_dev); > + return ret; > } > > static int hdc100x_buffer_predisable(struct iio_dev *indio_dev) > { > struct hdc100x_data *data = iio_priv(indio_dev); > - int ret; > - > - /* First detach poll func, then reset ACQ mode. OK to disable buffer */ > - ret = iio_triggered_buffer_predisable(indio_dev); > - if (ret) > - return ret; > + int ret, ret2; > > mutex_lock(&data->lock); > ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, 0); > mutex_unlock(&data->lock); > > + ret2 = iio_triggered_buffer_predisable(indio_dev); > + if (ret == 0) > + ret = ret2; > + > return ret; > } >
On Sun, Oct 27, 2019 at 6:58 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Wed, 23 Oct 2019 11:27:14 +0300 > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > > The iio_triggered_buffer_postenable() hook should be called first to > > attach the poll function and the iio_triggered_buffer_predisable() hook > > should be called last in the predisable hook. > > > > This change updates the driver to attach/detach the poll func in the > > correct order. > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > Seems fine, but should have cc'd Matt and I'd like to give him time > to take a quick look. > Reviewed-by: Matt Ranostay <matt.ranostay@konsulko.com> > +CC Matt. > > Thanks, > > Jonathan > > > --- > > drivers/iio/humidity/hdc100x.c | 19 +++++++++++-------- > > 1 file changed, 11 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c > > index bfe1cdb16846..963ff043eecf 100644 > > --- a/drivers/iio/humidity/hdc100x.c > > +++ b/drivers/iio/humidity/hdc100x.c > > @@ -278,31 +278,34 @@ static int hdc100x_buffer_postenable(struct iio_dev *indio_dev) > > struct hdc100x_data *data = iio_priv(indio_dev); > > int ret; > > > > + ret = iio_triggered_buffer_postenable(indio_dev); > > + if (ret) > > + return ret; > > + > > /* Buffer is enabled. First set ACQ Mode, then attach poll func */ > > mutex_lock(&data->lock); > > ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, > > HDC100X_REG_CONFIG_ACQ_MODE); > > mutex_unlock(&data->lock); > > if (ret) > > - return ret; > > + iio_triggered_buffer_predisable(indio_dev); > > > > - return iio_triggered_buffer_postenable(indio_dev); > > + return ret; > > } > > > > static int hdc100x_buffer_predisable(struct iio_dev *indio_dev) > > { > > struct hdc100x_data *data = iio_priv(indio_dev); > > - int ret; > > - > > - /* First detach poll func, then reset ACQ mode. OK to disable buffer */ > > - ret = iio_triggered_buffer_predisable(indio_dev); > > - if (ret) > > - return ret; > > + int ret, ret2; > > > > mutex_lock(&data->lock); > > ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, 0); > > mutex_unlock(&data->lock); > > > > + ret2 = iio_triggered_buffer_predisable(indio_dev); > > + if (ret == 0) > > + ret = ret2; > > + > > return ret; > > } > > >
On Sun, 2019-10-27 at 20:37 +0200, Matt Ranostay wrote: > [External] > > On Sun, Oct 27, 2019 at 6:58 PM Jonathan Cameron <jic23@kernel.org> > wrote: > > On Wed, 23 Oct 2019 11:27:14 +0300 > > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > > > > The iio_triggered_buffer_postenable() hook should be called first to > > > attach the poll function and the iio_triggered_buffer_predisable() > > > hook > > > should be called last in the predisable hook. > > > > > > This change updates the driver to attach/detach the poll func in the > > > correct order. > > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > Seems fine, but should have cc'd Matt and I'd like to give him time > > to take a quick look. > > > > Reviewed-by: Matt Ranostay <matt.ranostay@konsulko.com> Apologies for not CC-ing Matt. I guess I wasn't paying [close enough] attention to the output of get_maintainers.pl Thanks Alex > > > +CC Matt. > > > > Thanks, > > > > Jonathan > > > > > --- > > > drivers/iio/humidity/hdc100x.c | 19 +++++++++++-------- > > > 1 file changed, 11 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/iio/humidity/hdc100x.c > > > b/drivers/iio/humidity/hdc100x.c > > > index bfe1cdb16846..963ff043eecf 100644 > > > --- a/drivers/iio/humidity/hdc100x.c > > > +++ b/drivers/iio/humidity/hdc100x.c > > > @@ -278,31 +278,34 @@ static int hdc100x_buffer_postenable(struct > > > iio_dev *indio_dev) > > > struct hdc100x_data *data = iio_priv(indio_dev); > > > int ret; > > > > > > + ret = iio_triggered_buffer_postenable(indio_dev); > > > + if (ret) > > > + return ret; > > > + > > > /* Buffer is enabled. First set ACQ Mode, then attach poll func > > > */ > > > mutex_lock(&data->lock); > > > ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, > > > HDC100X_REG_CONFIG_ACQ_MODE); > > > mutex_unlock(&data->lock); > > > if (ret) > > > - return ret; > > > + iio_triggered_buffer_predisable(indio_dev); > > > > > > - return iio_triggered_buffer_postenable(indio_dev); > > > + return ret; > > > } > > > > > > static int hdc100x_buffer_predisable(struct iio_dev *indio_dev) > > > { > > > struct hdc100x_data *data = iio_priv(indio_dev); > > > - int ret; > > > - > > > - /* First detach poll func, then reset ACQ mode. OK to disable > > > buffer */ > > > - ret = iio_triggered_buffer_predisable(indio_dev); > > > - if (ret) > > > - return ret; > > > + int ret, ret2; > > > > > > mutex_lock(&data->lock); > > > ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, > > > 0); > > > mutex_unlock(&data->lock); > > > > > > + ret2 = iio_triggered_buffer_predisable(indio_dev); > > > + if (ret == 0) > > > + ret = ret2; > > > + > > > return ret; > > > } > > >
On Mon, 28 Oct 2019 07:11:51 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > On Sun, 2019-10-27 at 20:37 +0200, Matt Ranostay wrote: > > [External] > > > > On Sun, Oct 27, 2019 at 6:58 PM Jonathan Cameron <jic23@kernel.org> > > wrote: > > > On Wed, 23 Oct 2019 11:27:14 +0300 > > > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > > > > > > The iio_triggered_buffer_postenable() hook should be called first to > > > > attach the poll function and the iio_triggered_buffer_predisable() > > > > hook > > > > should be called last in the predisable hook. > > > > > > > > This change updates the driver to attach/detach the poll func in the > > > > correct order. > > > > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > Seems fine, but should have cc'd Matt and I'd like to give him time > > > to take a quick look. > > > > > > > Reviewed-by: Matt Ranostay <matt.ranostay@konsulko.com> > > Apologies for not CC-ing Matt. > I guess I wasn't paying [close enough] attention to the output of > get_maintainers.pl > > Thanks > Alex > Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > > > > > +CC Matt. > > > > > > Thanks, > > > > > > Jonathan > > > > > > > --- > > > > drivers/iio/humidity/hdc100x.c | 19 +++++++++++-------- > > > > 1 file changed, 11 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/drivers/iio/humidity/hdc100x.c > > > > b/drivers/iio/humidity/hdc100x.c > > > > index bfe1cdb16846..963ff043eecf 100644 > > > > --- a/drivers/iio/humidity/hdc100x.c > > > > +++ b/drivers/iio/humidity/hdc100x.c > > > > @@ -278,31 +278,34 @@ static int hdc100x_buffer_postenable(struct > > > > iio_dev *indio_dev) > > > > struct hdc100x_data *data = iio_priv(indio_dev); > > > > int ret; > > > > > > > > + ret = iio_triggered_buffer_postenable(indio_dev); > > > > + if (ret) > > > > + return ret; > > > > + > > > > /* Buffer is enabled. First set ACQ Mode, then attach poll func > > > > */ > > > > mutex_lock(&data->lock); > > > > ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, > > > > HDC100X_REG_CONFIG_ACQ_MODE); > > > > mutex_unlock(&data->lock); > > > > if (ret) > > > > - return ret; > > > > + iio_triggered_buffer_predisable(indio_dev); > > > > > > > > - return iio_triggered_buffer_postenable(indio_dev); > > > > + return ret; > > > > } > > > > > > > > static int hdc100x_buffer_predisable(struct iio_dev *indio_dev) > > > > { > > > > struct hdc100x_data *data = iio_priv(indio_dev); > > > > - int ret; > > > > - > > > > - /* First detach poll func, then reset ACQ mode. OK to disable > > > > buffer */ > > > > - ret = iio_triggered_buffer_predisable(indio_dev); > > > > - if (ret) > > > > - return ret; > > > > + int ret, ret2; > > > > > > > > mutex_lock(&data->lock); > > > > ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, > > > > 0); > > > > mutex_unlock(&data->lock); > > > > > > > > + ret2 = iio_triggered_buffer_predisable(indio_dev); > > > > + if (ret == 0) > > > > + ret = ret2; > > > > + > > > > return ret; > > > > } > > > >
diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c index bfe1cdb16846..963ff043eecf 100644 --- a/drivers/iio/humidity/hdc100x.c +++ b/drivers/iio/humidity/hdc100x.c @@ -278,31 +278,34 @@ static int hdc100x_buffer_postenable(struct iio_dev *indio_dev) struct hdc100x_data *data = iio_priv(indio_dev); int ret; + ret = iio_triggered_buffer_postenable(indio_dev); + if (ret) + return ret; + /* Buffer is enabled. First set ACQ Mode, then attach poll func */ mutex_lock(&data->lock); ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, HDC100X_REG_CONFIG_ACQ_MODE); mutex_unlock(&data->lock); if (ret) - return ret; + iio_triggered_buffer_predisable(indio_dev); - return iio_triggered_buffer_postenable(indio_dev); + return ret; } static int hdc100x_buffer_predisable(struct iio_dev *indio_dev) { struct hdc100x_data *data = iio_priv(indio_dev); - int ret; - - /* First detach poll func, then reset ACQ mode. OK to disable buffer */ - ret = iio_triggered_buffer_predisable(indio_dev); - if (ret) - return ret; + int ret, ret2; mutex_lock(&data->lock); ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, 0); mutex_unlock(&data->lock); + ret2 = iio_triggered_buffer_predisable(indio_dev); + if (ret == 0) + ret = ret2; + return ret; }
The iio_triggered_buffer_postenable() hook should be called first to attach the poll function and the iio_triggered_buffer_predisable() hook should be called last in the predisable hook. This change updates the driver to attach/detach the poll func in the correct order. Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- drivers/iio/humidity/hdc100x.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)