* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox