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] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d929df3-ac51-7d1f-ec00-856849cc70c7@gmail.com>
Date:   Thu, 21 Sep 2023 12:00:39 +0300
From:   Matti Vaittinen <mazziesaccount@...il.com>
To:     Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        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 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! ~~