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

[Jonathan Cameron]

On Thu, 21 Sep 2023 12:00:39 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:

> 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.

I should have read on in the thread. Indeed - ordering of preferences needs
to be in driver control for exactly the reason you came up with!

Thanks,

Jonathan


> 
> Thanks for listening - and sorry for the noise :)
> 
> Yours,
> 	-- Matti
>