On Thu, Dec 16, 2021 at 9:38 AM Alexandru Ardelean
<ardeleanalex@xxxxxxxxx> wrote:
>
> On Wed, Dec 15, 2021 at 10:04 PM Miquel Raynal
> <miquel.raynal@xxxxxxxxxxx> wrote:
> >
> > In order to later move this variable within the opaque structure, let's
> > create a helper for accessing it in read-only mode. This helper will be
> > exposed and kept accessible for the few drivers that could need it. The
> > write access to this variable however should be fully reserved to the
> > core so in a second step we will add another helper, not exported to the
> > device drivers.
>
> The naming needs a bit of discussion.
> I would have gone for iio_dev_get_current_mode() or something like that.
>
> And I would probably not use this helper inside the IIO core stuff
> (i.e. drivers/iio/industrialio-*.c files)
> Mostly because [if now used only in IIO core] it makes the
> "indio_dev->currentmode" assignment and access easier to trace.
Oh, I just saw the next patch with iio_set_internal_mode().
I guess having a set helper (like this) resolves the discussion about tracing.
Still not sure whether it makes sense to use these helpers inside IIO
core, but ¯\_(ツ)_/¯
>
> There's also the change that accessing "indio_dev->currentmode"
> becomes a function-symbol which has rules at link-time and may add
> some new jmp/ret instructions.
> But It doesn't look like this is used in any fast-paths, so it's not
> an issue as much.
>
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > ---
> > drivers/iio/accel/bmc150-accel-core.c | 4 ++--
> > drivers/iio/adc/at91-sama5d2_adc.c | 4 ++--
> > drivers/iio/industrialio-buffer.c | 6 +++---
> > drivers/iio/industrialio-core.c | 11 +++++++++++
> > drivers/iio/industrialio-trigger.c | 2 +-
> > include/linux/iio/iio.h | 9 ++++++---
> > 6 files changed, 25 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
> > index b0678c351e82..0a191463d462 100644
> > --- a/drivers/iio/accel/bmc150-accel-core.c
> > +++ b/drivers/iio/accel/bmc150-accel-core.c
> > @@ -1525,7 +1525,7 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
> > struct bmc150_accel_data *data = iio_priv(indio_dev);
> > int ret = 0;
> >
> > - if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
> > + if (iio_get_internal_mode(indio_dev) == INDIO_BUFFER_TRIGGERED)
> > return 0;
> >
> > mutex_lock(&data->mutex);
> > @@ -1557,7 +1557,7 @@ static int bmc150_accel_buffer_predisable(struct iio_dev *indio_dev)
> > {
> > struct bmc150_accel_data *data = iio_priv(indio_dev);
> >
> > - if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
> > + if (iio_get_internal_mode(indio_dev) == INDIO_BUFFER_TRIGGERED)
> > return 0;
> >
> > mutex_lock(&data->mutex);
> > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> > index 4c922ef634f8..2b7f6371950e 100644
> > --- a/drivers/iio/adc/at91-sama5d2_adc.c
> > +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> > @@ -1117,7 +1117,7 @@ static int at91_adc_buffer_prepare(struct iio_dev *indio_dev)
> > return at91_adc_configure_touch(st, true);
> >
> > /* if we are not in triggered mode, we cannot enable the buffer. */
> > - if (!(indio_dev->currentmode & INDIO_ALL_TRIGGERED_MODES))
> > + if (!(iio_get_internal_mode(indio_dev) & INDIO_ALL_TRIGGERED_MODES))
> > return -EINVAL;
> >
> > /* we continue with the triggered buffer */
> > @@ -1159,7 +1159,7 @@ static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
> > return at91_adc_configure_touch(st, false);
> >
> > /* if we are not in triggered mode, nothing to do here */
> > - if (!(indio_dev->currentmode & INDIO_ALL_TRIGGERED_MODES))
> > + if (!(iio_get_internal_mode(indio_dev) & INDIO_ALL_TRIGGERED_MODES))
> > return -EINVAL;
> >
> > /*
> > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
> > index e180728914c0..f4dbab7c44fa 100644
> > --- a/drivers/iio/industrialio-buffer.c
> > +++ b/drivers/iio/industrialio-buffer.c
> > @@ -1101,7 +1101,7 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
> > goto err_disable_buffers;
> > }
> >
> > - if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> > + if (iio_get_internal_mode(indio_dev) == INDIO_BUFFER_TRIGGERED) {
> > ret = iio_trigger_attach_poll_func(indio_dev->trig,
> > indio_dev->pollfunc);
> > if (ret)
> > @@ -1120,7 +1120,7 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
> > return 0;
> >
> > err_detach_pollfunc:
> > - if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> > + if (iio_get_internal_mode(indio_dev) == INDIO_BUFFER_TRIGGERED) {
> > iio_trigger_detach_poll_func(indio_dev->trig,
> > indio_dev->pollfunc);
> > }
> > @@ -1162,7 +1162,7 @@ static int iio_disable_buffers(struct iio_dev *indio_dev)
> > ret = ret2;
> > }
> >
> > - if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> > + if (iio_get_internal_mode(indio_dev) == INDIO_BUFFER_TRIGGERED) {
> > iio_trigger_detach_poll_func(indio_dev->trig,
> > indio_dev->pollfunc);
> > }
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index 463a63d5bf56..a1d6e30d034a 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -2057,6 +2057,17 @@ void iio_device_release_direct_mode(struct iio_dev *indio_dev)
> > }
> > EXPORT_SYMBOL_GPL(iio_device_release_direct_mode);
> >
> > +/**
> > + * iio_get_internal_mode() - helper function providing read-only access to the
> > + * opaque @currentmode variable
> > + * @indio_dev: IIO device structure for device
> > + **/
> > +int iio_get_internal_mode(struct iio_dev *indio_dev)
> > +{
> > + return indio_dev->currentmode;
> > +}
> > +EXPORT_SYMBOL_GPL(iio_get_internal_mode);
> > +
> > subsys_initcall(iio_init);
> > module_exit(iio_exit);
> >
> > diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> > index b23caa2f2aa1..71b07d6111d6 100644
> > --- a/drivers/iio/industrialio-trigger.c
> > +++ b/drivers/iio/industrialio-trigger.c
> > @@ -411,7 +411,7 @@ static ssize_t iio_trigger_write_current(struct device *dev,
> > int ret;
> >
> > mutex_lock(&indio_dev->mlock);
> > - if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
> > + if (iio_get_internal_mode(indio_dev) == INDIO_BUFFER_TRIGGERED) {
> > mutex_unlock(&indio_dev->mlock);
> > return -EBUSY;
> > }
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index 0312da2e83f8..dcab17f44552 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -677,15 +677,18 @@ struct iio_dev *devm_iio_device_alloc(struct device *parent, int sizeof_priv);
> > __printf(2, 3)
> > struct iio_trigger *devm_iio_trigger_alloc(struct device *parent,
> > const char *fmt, ...);
> > +
> > +int iio_get_internal_mode(struct iio_dev *indio_dev);
> > +
> > /**
> > * iio_buffer_enabled() - helper function to test if the buffer is enabled
> > * @indio_dev: IIO device structure for device
> > **/
> > static inline bool iio_buffer_enabled(struct iio_dev *indio_dev)
> > {
> > - return indio_dev->currentmode
> > - & (INDIO_BUFFER_TRIGGERED | INDIO_BUFFER_HARDWARE |
> > - INDIO_BUFFER_SOFTWARE);
> > + return iio_get_internal_mode(indio_dev) &
> > + (INDIO_BUFFER_TRIGGERED | INDIO_BUFFER_HARDWARE |
> > + INDIO_BUFFER_SOFTWARE);
> > }
> >
> > /**
> > --
> > 2.27.0
> >
