[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250724120706.222da8e5@jic23-huawei>
Date: Thu, 24 Jul 2025 12:07:06 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: David Lechner <dlechner@...libre.com>, Lars-Peter Clausen
<lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, Nuno
Sá <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>, linux-iio@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: adc: ad7173: prevent scan if too many setups
requested
On Wed, 23 Jul 2025 17:34:42 +0300
Andy Shevchenko <andriy.shevchenko@...el.com> wrote:
> On Tue, Jul 22, 2025 at 02:20:07PM -0500, David Lechner wrote:
> > Add a check to ad7173_update_scan_mode() to ensure that we didn't exceed
> > the maximum number of unique channel configurations.
> >
> > In the AD7173 family of chips, there are some chips that have 16
> > CHANNELx registers but only 8 setups (combination of CONFIGx, FILTERx,
> > GAINx and OFFSETx registers). Since commit 92c247216918 ("iio: adc:
> > ad7173: fix num_slots"), it is possible to have more than 8 channels
> > enabled in a scan at the same time, so it is possible to get a bad
> > configuration when more than 8 channels are using unique configurations.
> > This happens because the algorithm to allocate the setup slots only
> > takes into account which slot has been least recently used and doesn't
> > know about the maximum number of slots available.
> >
> > Since the algorithm to allocate the setup slots is quite complex, it is
> > simpler to check after the fact if the current state is valid or not.
> > So this patch adds a check in ad7173_update_scan_mode() after setting up
> > all of the configurations to make sure that the actual setup still
> > matches the requested setup for each enabled channel. If not, we prevent
> > the scan from being enabled and return an error.
> >
> > The setup comparison in ad7173_setup_equal() is refactored to a separate
> > function since we need to call it in two places now.
>
> ...
>
> > + * ad7173_setup_equal - Compare two channel setups
>
> Better naming is
> ad7173_is_setup_equal().
>
Agree. Tweaked with following and applied to the fixes-togreg-for-6.17
branch of iio.git + marked for stable.
diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 131cd1cf8a23..683146e83ab2 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -200,7 +200,7 @@ struct ad7173_channel_config {
/*
* Following fields are used to compare equality. If you
* make adaptations in it, you most likely also have to adapt
- * ad7173_setup_equal(), too.
+ * ad7173_is_setup_equal(), too.
*/
struct_group(config_props,
bool bipolar;
@@ -562,7 +562,7 @@ static void ad7173_reset_usage_cnts(struct ad7173_state *st)
}
/**
- * ad7173_setup_equal - Compare two channel setups
+ * ad7173_is_setup_equal - Compare two channel setups
* @cfg1: First channel configuration
* @cfg2: Second channel configuration
*
@@ -571,8 +571,8 @@ static void ad7173_reset_usage_cnts(struct ad7173_state *st)
*
* Returns: true if the setups are identical, false otherwise
*/
-static bool ad7173_setup_equal(const struct ad7173_channel_config *cfg1,
- const struct ad7173_channel_config *cfg2)
+static bool ad7173_is_setup_equal(const struct ad7173_channel_config *cfg1,
+ const struct ad7173_channel_config *cfg2)
{
/*
* This is just to make sure that the comparison is adapted after
@@ -601,7 +601,7 @@ ad7173_find_live_config(struct ad7173_state *st, struct ad7173_channel_config *c
for (i = 0; i < st->num_channels; i++) {
cfg_aux = &st->channels[i].cfg;
- if (cfg_aux->live && ad7173_setup_equal(cfg, cfg_aux))
+ if (cfg_aux->live && ad7173_is_setup_equal(cfg, cfg_aux))
return cfg_aux;
}
return NULL;
@@ -1283,7 +1283,7 @@ static int ad7173_update_scan_mode(struct iio_dev *indio_dev,
* have too many unique configurations requested for
* the available slots and at least one was overwritten.
*/
- if (!ad7173_setup_equal(cfg1, cfg2)) {
+ if (!ad7173_is_setup_equal(cfg1, cfg2)) {
/*
* At this point, there isn't a way to tell
* which setups are actually programmed in the
Powered by blists - more mailing lists