* [PATCH] iio: proximity: sx9500: Deduplicate buffer managing functions
@ 2025-04-29 9:39 Mikhael Abdallah de Oliveira Pinto
2025-04-29 15:07 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Mikhael Abdallah de Oliveira Pinto @ 2025-04-29 9:39 UTC (permalink / raw)
To: jic23; +Cc: Mikhael Abdallah de Oliveira Pinto, Augusto Bernarde, linux-iio
Refactor to share logic between buffer enable/disable handlers.
Signed-off-by: Mikhael Abdallah de Oliveira Pinto <mikhael_abdallah@usp.br>
Co-developed-by: Augusto Bernarde <augustomb@usp.br>
Signed-off-by: Augusto Bernarde <augustomb@usp.br>
---
drivers/iio/proximity/sx9500.c | 50 ++++++++++++++++------------------
1 file changed, 23 insertions(+), 27 deletions(-)
diff --git a/drivers/iio/proximity/sx9500.c b/drivers/iio/proximity/sx9500.c
index c4e94d0fb163..75459c85116b 100644
--- a/drivers/iio/proximity/sx9500.c
+++ b/drivers/iio/proximity/sx9500.c
@@ -674,52 +674,48 @@ static irqreturn_t sx9500_trigger_handler(int irq, void *private)
return IRQ_HANDLED;
}
-static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
+static int sx9500_buffer_manage_chan_users(struct iio_dev *indio_dev, bool enable_channels)
{
struct sx9500_data *data = iio_priv(indio_dev);
int ret = 0, i;
mutex_lock(&data->mutex);
- for (i = 0; i < SX9500_NUM_CHANNELS; i++)
+ for (i = 0; i < SX9500_NUM_CHANNELS; i++) {
if (test_bit(i, indio_dev->active_scan_mask)) {
- ret = sx9500_inc_chan_users(data, i);
+ if (enable_channels)
+ ret = sx9500_inc_chan_users(data, i);
+ else
+ ret = sx9500_dec_chan_users(data, i);
if (ret)
break;
}
+ }
- if (ret)
- for (i = i - 1; i >= 0; i--)
- if (test_bit(i, indio_dev->active_scan_mask))
- sx9500_dec_chan_users(data, i);
+ if (ret) {
+ for (i = i - 1; i >= 0; i--) {
+ if (test_bit(i, indio_dev->active_scan_mask)) {
+ if (enable_channels)
+ sx9500_dec_chan_users(data, i);
+ else
+ sx9500_inc_chan_users(data, i);
+ }
+ }
+ }
mutex_unlock(&data->mutex);
return ret;
}
-static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
+static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
{
- struct sx9500_data *data = iio_priv(indio_dev);
- int ret = 0, i;
-
- mutex_lock(&data->mutex);
-
- for (i = 0; i < SX9500_NUM_CHANNELS; i++)
- if (test_bit(i, indio_dev->active_scan_mask)) {
- ret = sx9500_dec_chan_users(data, i);
- if (ret)
- break;
- }
-
- if (ret)
- for (i = i - 1; i >= 0; i--)
- if (test_bit(i, indio_dev->active_scan_mask))
- sx9500_inc_chan_users(data, i);
-
- mutex_unlock(&data->mutex);
+ return sx9500_buffer_manage_chan_users(indio_dev, true);
+}
- return ret;
+static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
+{
+ return sx9500_buffer_manage_chan_users(indio_dev, false);
}
static const struct iio_buffer_setup_ops sx9500_buffer_setup_ops = {
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] iio: proximity: sx9500: Deduplicate buffer managing functions
2025-04-29 9:39 [PATCH] iio: proximity: sx9500: Deduplicate buffer managing functions Mikhael Abdallah de Oliveira Pinto
@ 2025-04-29 15:07 ` Jonathan Cameron
2025-05-01 18:23 ` Mikhael Abdallah de Oliveira Pinto
0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2025-04-29 15:07 UTC (permalink / raw)
To: Mikhael Abdallah de Oliveira Pinto; +Cc: jic23, Augusto Bernarde, linux-iio
On Tue, 29 Apr 2025 06:39:23 -0300
Mikhael Abdallah de Oliveira Pinto <mikhael_abdallah@usp.br> wrote:
> Refactor to share logic between buffer enable/disable handlers.
>
> Signed-off-by: Mikhael Abdallah de Oliveira Pinto <mikhael_abdallah@usp.br>
> Co-developed-by: Augusto Bernarde <augustomb@usp.br>
> Signed-off-by: Augusto Bernarde <augustomb@usp.br>
> ---
> drivers/iio/proximity/sx9500.c | 50 ++++++++++++++++------------------
In my view this isn't a significant enough reduction to justify the more complex code.
Particularly in the error paths.
Jonathan
> 1 file changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/iio/proximity/sx9500.c b/drivers/iio/proximity/sx9500.c
> index c4e94d0fb163..75459c85116b 100644
> --- a/drivers/iio/proximity/sx9500.c
> +++ b/drivers/iio/proximity/sx9500.c
> @@ -674,52 +674,48 @@ static irqreturn_t sx9500_trigger_handler(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> -static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
> +static int sx9500_buffer_manage_chan_users(struct iio_dev *indio_dev, bool enable_channels)
> {
> struct sx9500_data *data = iio_priv(indio_dev);
> int ret = 0, i;
>
> mutex_lock(&data->mutex);
>
> - for (i = 0; i < SX9500_NUM_CHANNELS; i++)
> + for (i = 0; i < SX9500_NUM_CHANNELS; i++) {
> if (test_bit(i, indio_dev->active_scan_mask)) {
> - ret = sx9500_inc_chan_users(data, i);
> + if (enable_channels)
> + ret = sx9500_inc_chan_users(data, i);
> + else
> + ret = sx9500_dec_chan_users(data, i);
> if (ret)
> break;
> }
> + }
>
> - if (ret)
> - for (i = i - 1; i >= 0; i--)
> - if (test_bit(i, indio_dev->active_scan_mask))
> - sx9500_dec_chan_users(data, i);
> + if (ret) {
> + for (i = i - 1; i >= 0; i--) {
> + if (test_bit(i, indio_dev->active_scan_mask)) {
> + if (enable_channels)
> + sx9500_dec_chan_users(data, i);
> + else
> + sx9500_inc_chan_users(data, i);
> + }
> + }
> + }
>
> mutex_unlock(&data->mutex);
>
> return ret;
> }
>
> -static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
> +static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
> {
> - struct sx9500_data *data = iio_priv(indio_dev);
> - int ret = 0, i;
> -
> - mutex_lock(&data->mutex);
> -
> - for (i = 0; i < SX9500_NUM_CHANNELS; i++)
> - if (test_bit(i, indio_dev->active_scan_mask)) {
> - ret = sx9500_dec_chan_users(data, i);
> - if (ret)
> - break;
> - }
> -
> - if (ret)
> - for (i = i - 1; i >= 0; i--)
> - if (test_bit(i, indio_dev->active_scan_mask))
> - sx9500_inc_chan_users(data, i);
> -
> - mutex_unlock(&data->mutex);
> + return sx9500_buffer_manage_chan_users(indio_dev, true);
> +}
>
> - return ret;
> +static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + return sx9500_buffer_manage_chan_users(indio_dev, false);
> }
>
> static const struct iio_buffer_setup_ops sx9500_buffer_setup_ops = {
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] iio: proximity: sx9500: Deduplicate buffer managing functions
2025-04-29 15:07 ` Jonathan Cameron
@ 2025-05-01 18:23 ` Mikhael Abdallah de Oliveira Pinto
0 siblings, 0 replies; 3+ messages in thread
From: Mikhael Abdallah de Oliveira Pinto @ 2025-05-01 18:23 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: jic23, Augusto Bernarde, linux-iio
Hi Jonathan,
Thanks for the feedback. I've decided not to continue with this change
and will abandon it.
Best, Mikhael
Em ter., 29 de abr. de 2025 às 12:07, Jonathan Cameron
<Jonathan.Cameron@huawei.com> escreveu:
>
> On Tue, 29 Apr 2025 06:39:23 -0300
> Mikhael Abdallah de Oliveira Pinto <mikhael_abdallah@usp.br> wrote:
>
> > Refactor to share logic between buffer enable/disable handlers.
> >
> > Signed-off-by: Mikhael Abdallah de Oliveira Pinto <mikhael_abdallah@usp.br>
> > Co-developed-by: Augusto Bernarde <augustomb@usp.br>
> > Signed-off-by: Augusto Bernarde <augustomb@usp.br>
> > ---
> > drivers/iio/proximity/sx9500.c | 50 ++++++++++++++++------------------
>
> In my view this isn't a significant enough reduction to justify the more complex code.
> Particularly in the error paths.
>
> Jonathan
>
>
> > 1 file changed, 23 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/iio/proximity/sx9500.c b/drivers/iio/proximity/sx9500.c
> > index c4e94d0fb163..75459c85116b 100644
> > --- a/drivers/iio/proximity/sx9500.c
> > +++ b/drivers/iio/proximity/sx9500.c
> > @@ -674,52 +674,48 @@ static irqreturn_t sx9500_trigger_handler(int irq, void *private)
> > return IRQ_HANDLED;
> > }
> >
> > -static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
> > +static int sx9500_buffer_manage_chan_users(struct iio_dev *indio_dev, bool enable_channels)
> > {
> > struct sx9500_data *data = iio_priv(indio_dev);
> > int ret = 0, i;
> >
> > mutex_lock(&data->mutex);
> >
> > - for (i = 0; i < SX9500_NUM_CHANNELS; i++)
> > + for (i = 0; i < SX9500_NUM_CHANNELS; i++) {
> > if (test_bit(i, indio_dev->active_scan_mask)) {
> > - ret = sx9500_inc_chan_users(data, i);
> > + if (enable_channels)
> > + ret = sx9500_inc_chan_users(data, i);
> > + else
> > + ret = sx9500_dec_chan_users(data, i);
> > if (ret)
> > break;
> > }
> > + }
> >
> > - if (ret)
> > - for (i = i - 1; i >= 0; i--)
> > - if (test_bit(i, indio_dev->active_scan_mask))
> > - sx9500_dec_chan_users(data, i);
> > + if (ret) {
> > + for (i = i - 1; i >= 0; i--) {
> > + if (test_bit(i, indio_dev->active_scan_mask)) {
> > + if (enable_channels)
> > + sx9500_dec_chan_users(data, i);
> > + else
> > + sx9500_inc_chan_users(data, i);
> > + }
> > + }
> > + }
> >
> > mutex_unlock(&data->mutex);
> >
> > return ret;
> > }
> >
> > -static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
> > +static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
> > {
> > - struct sx9500_data *data = iio_priv(indio_dev);
> > - int ret = 0, i;
> > -
> > - mutex_lock(&data->mutex);
> > -
> > - for (i = 0; i < SX9500_NUM_CHANNELS; i++)
> > - if (test_bit(i, indio_dev->active_scan_mask)) {
> > - ret = sx9500_dec_chan_users(data, i);
> > - if (ret)
> > - break;
> > - }
> > -
> > - if (ret)
> > - for (i = i - 1; i >= 0; i--)
> > - if (test_bit(i, indio_dev->active_scan_mask))
> > - sx9500_inc_chan_users(data, i);
> > -
> > - mutex_unlock(&data->mutex);
> > + return sx9500_buffer_manage_chan_users(indio_dev, true);
> > +}
> >
> > - return ret;
> > +static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
> > +{
> > + return sx9500_buffer_manage_chan_users(indio_dev, false);
> > }
> >
> > static const struct iio_buffer_setup_ops sx9500_buffer_setup_ops = {
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-05-01 18:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-29 9:39 [PATCH] iio: proximity: sx9500: Deduplicate buffer managing functions Mikhael Abdallah de Oliveira Pinto
2025-04-29 15:07 ` Jonathan Cameron
2025-05-01 18:23 ` Mikhael Abdallah de Oliveira Pinto
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.