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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230924131454.7cae02a9@jic23-huawei>
Date:   Sun, 24 Sep 2023 13:14:54 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Matti Vaittinen <mazziesaccount@...il.com>
Cc:     Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Angel Iglesias <ang.iglesiasg@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Andreas Klinger <ak@...klinger.de>,
        Benjamin Bara <bbara93@...il.com>, linux-iio@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] iio: pressure: Support ROHM BU1390

On Thu, 21 Sep 2023 12:00:39 +0300
Matti Vaittinen <mazziesaccount@...il.com> 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
>