Re: [PATCH v2] iio: adc: ad7173: prevent scan if too many setups requested

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 23 Jul 2025 17:34:42 +0300
Andy Shevchenko <andriy.shevchenko@intel.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