Re: [PATCH v2 2/3] iio: pressure: Support ROHM BU1390

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 9/21/23 11:17, Matti Vaittinen wrote:

> Another thing to note is that, when we build the available_scan_mask 
> array - we should either pay attention to the order of masks - or change 
> the iio_scan_mask_match() to not accept first matching subset but to go 
> through all of the masks unless it finds and exactly matching one (and 
> in general prefer the smallest subset). Not sure this is worth the extra 
> cycles though.

Replying to myself and to those who I perhaps managed to confuse :)

As a result of above pondering I wrote this:

@@ -411,6 +418,8 @@ static const unsigned long 
*iio_scan_mask_match(const unsigned long *av_masks,
                                                 const unsigned long *mask,
                                                 bool strict)
  {
+       const unsigned long *smallest = NULL;
+
         if (bitmap_empty(mask, masklength))
                 return NULL;
         while (*av_masks) {
@@ -418,12 +427,16 @@ static const unsigned long 
*iio_scan_mask_match(const unsigned long *av_masks,
                         if (bitmap_equal(mask, av_masks, masklength))
                                 return av_masks;
                 } else {
-                       if (bitmap_subset(mask, av_masks, masklength))
-                               return av_masks;
+                       if (bitmap_subset(mask, av_masks, masklength)) {
+                               if (!smallest ||
+                                   bitmap_weight(av_masks, BITS_PER_LONG) <
+                                   bitmap_weight(smallest, BITS_PER_LONG))
+                                       smallest = av_masks;
+                       }
                 }
                 av_masks += BITS_TO_LONGS(masklength);
         }
-       return NULL;
+       return smallest;
  }

but ...
... I see a problem that some of the channels may be more costly to 
access than the other. It could be that reading some of the channels is 
just a matter of getting a cached value, while other could require a 
long measurement time and access to significant amount of registers. So, 
the knowledge of preferred scan masks should indeed be on the driver 
side. Hence, the ordering of the masks in the order of preference makes 
perfect sense. What we could do in the IIO core side is still go through 
all of the available masks to see if we find an exact match. I guess we 
could also document the fact that the order of masks matters.

Thanks for listening - and sorry for the noise :)

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~