Message ID | 24a577e6e157e1199817ab36631cec51675ef3ca.1695380366.git.mazziesaccount@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp5540311vqi; Fri, 22 Sep 2023 05:42:34 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEsdct3oAr0PEU12B1m6afkR0nNCPi2Y76EVKGY5RiRKHumogA9epHY68a5rCxhPw3UTpfF X-Received: by 2002:a05:6a20:9381:b0:14c:de3:95d6 with SMTP id x1-20020a056a20938100b0014c0de395d6mr9312773pzh.45.1695386553813; Fri, 22 Sep 2023 05:42:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695386553; cv=none; d=google.com; s=arc-20160816; b=Rjw+kvfC6PlEvkMSGhCllBNPA5HPwunisDQ8um9/2lvBT/1AFvHZ6F3gHOA8Avc5S9 AjC3tYaY78qv/TUnWElSSnoJyg4pc0CIs3M4V8CP0EnB3t7zeRiJRXaqDFVmiifwTjzM ZMMM6KXwbMNCh/QB2GOzncQBSZdSn3ivrPZwfHwkjYl5Ywz1LRtmVN+ZIWtjaRR/68BN hCn1mJAVGmzkaBpG0yXGmb1QxocdZlWtqbqMR06YtwPcMInNZ7E+dDFILsPVmlVUxhWk bospQT9iB7DxtFNnxRuyFaj4z2ZCuP3GX5U+YzzYbVurORC0BHDRyOM6txAJDT+VDjWp eHbQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=cIAB6MpcjuuKqpe+1Lhuap6hKVsOivGLkNwO83oo9k0=; fh=+XkJcXOKJ4Dl6AYTln3UzvhZHjSiM5yCnDKdoCP8e7U=; b=xCtKzVzYFbZ3AlXiFxJbhzjXqmGaU6ESXGT72r9DjtT07zJilds/UyCo52Pik3Eb33 G6BTZaZ1QCXnbVuL9bGsImEOfuwCFI04wNSZ5IPtW53ozAoGJ3m5detFE8fkEjn9EtpG w94q4M+2+zegjjn8M3qiNukWPTjbQni17P4OPHH2CQhaxTGhrVaUtQiR9xu2iqHrz3zE KiX2pIi1Q3TeQdqGbGRhnSTxkLhCZnERiOtcwIFsGryXe9UnTVZgoKezMAf4orqsQrDS ggqPZhYYXj6QOzy4yPLR+VM0HYEaLg/TUaBCYhJVAdRh8/iYVTEEZmGQSVQaji3nH9Mf fhqg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=DdXYddpV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from pete.vger.email (pete.vger.email. [2620:137:e000::3:6]) by mx.google.com with ESMTPS id cp6-20020a170902e78600b001a6f0eab385si3747706plb.55.2023.09.22.05.42.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Sep 2023 05:42:33 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=DdXYddpV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by pete.vger.email (Postfix) with ESMTP id 7C4D081C4A9B; Fri, 22 Sep 2023 04:18:21 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at pete.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233575AbjIVLSG (ORCPT <rfc822;chrisfriedt@gmail.com> + 30 others); Fri, 22 Sep 2023 07:18:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60936 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230156AbjIVLSF (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 22 Sep 2023 07:18:05 -0400 Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [IPv6:2a00:1450:4864:20::12c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5A866AF; Fri, 22 Sep 2023 04:17:59 -0700 (PDT) Received: by mail-lf1-x12c.google.com with SMTP id 2adb3069b0e04-50437f39c9dso1036218e87.3; Fri, 22 Sep 2023 04:17:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695381477; x=1695986277; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=cIAB6MpcjuuKqpe+1Lhuap6hKVsOivGLkNwO83oo9k0=; b=DdXYddpVAz5TSDVu2S7BpvGzts8SN4rn2NVYUSpgm1KVA3cPLCCzb83ukZes6s0P9s Rje2sFAV7pmqKCg3vzlQ95AjtaGajZQVQYYpVxDzS/qgZ89usx9LBbjMQcUthrsDQEaF 9/3mup9+mZGXcWL1enfi2ZgksElk4prlBE3gBqMXBMUImcDcYIHjP9mKi9gdHP40OShs 5u80suFrU+AaZlipgLFB8Nui2cKoiiyIs3JwOBGe5gX6ntrBw9Qx/+4DHPKRudkjU/Jl LDAKLSbRhfpzO4DTsfBUA+mL/5lhq62eaDr0mmLqesOyK8nLkAZ3HX3Y/5qp/cXzeFWy GqtA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695381477; x=1695986277; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=cIAB6MpcjuuKqpe+1Lhuap6hKVsOivGLkNwO83oo9k0=; b=szRwBVhqiZvOcNR1FUge2ByEvQnkwzVvzJZ03Jj57nBl/XMGm0VOBbChskdRobCn8s Ed2CyCkgBu41UEP+OeWHrYRc/qbcz62r4U1WltuK4gQhLWNVe7v1F0Z1tW18mln1FEtT lRQ1IcWIsN5A1UcbUgX9zAF/00gBNKnWUKAP/JBmhTUrM2wqvGsBGnOqqo4f1kslXcpG Zuj0fwpE1qYpiU9nKLEwQv7mPlSe+f2pmAfk4bjEM6tX7yLss2eqX5uHgy/HjxMxwsQ3 H/2kOOKy7kEjUr2YNH6cIXy5WsEzC98xlmFpkAY9z0rwQeMEVaYWJOrV7amoLmqaF2yq jwBQ== X-Gm-Message-State: AOJu0Yw/aysW0+YcjzcElcYq8/1Wp5MAEu9X2sli3p32h0UVqBp6OUFj uWW/WOnpChJXIGayhsRdCmI= X-Received: by 2002:a05:6512:2356:b0:504:35a1:31ce with SMTP id p22-20020a056512235600b0050435a131cemr2576998lfu.30.1695381477520; Fri, 22 Sep 2023 04:17:57 -0700 (PDT) Received: from dc78bmyyyyyyyyyyyyyyt-3.rev.dnainternet.fi (dc78bmyyyyyyyyyyyyyyt-3.rev.dnainternet.fi. [2001:14ba:16f8:1500::1]) by smtp.gmail.com with ESMTPSA id u2-20020a056512040200b005030a35019dsm692099lfk.178.2023.09.22.04.17.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 22 Sep 2023 04:17:56 -0700 (PDT) Date: Fri, 22 Sep 2023 14:17:49 +0300 From: Matti Vaittinen <mazziesaccount@gmail.com> To: Matti Vaittinen <mazziesaccount@gmail.com>, Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> Cc: Jonathan Cameron <jic23@kernel.org>, Lars-Peter Clausen <lars@metafoo.de>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, Matti Vaittinen <mazziesaccount@gmail.com>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Angel Iglesias <ang.iglesiasg@gmail.com>, Andreas Klinger <ak@it-klinger.de>, Christophe JAILLET <christophe.jaillet@wanadoo.fr>, Benjamin Bara <bbara93@gmail.com>, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v3 3/6] iio: try searching for exact scan_mask Message-ID: <24a577e6e157e1199817ab36631cec51675ef3ca.1695380366.git.mazziesaccount@gmail.com> References: <cover.1695380366.git.mazziesaccount@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7R35CQ0ZLI6pqD5C" Content-Disposition: inline In-Reply-To: <cover.1695380366.git.mazziesaccount@gmail.com> X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Fri, 22 Sep 2023 04:18:21 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777741650725590753 X-GMAIL-MSGID: 1777741650725590753 |
Series |
Support ROHM BM1390 pressure sensor
|
|
Commit Message
Matti Vaittinen
Sept. 22, 2023, 11:17 a.m. UTC
When IIO goes through the available scan masks in order to select the
best suiting one, it will just accept the first listed subset of channels
which meets the user's requirements. This works great for most of the
drivers as they can sort the list of channels in the order where
the 'least costy' channel selections come first.
It may be that in some cases the ordering of the list of available scan
masks is not thoroughly considered. We can't really try outsmarting the
drivers by selecting the smallest supported subset - as this might not
be the 'least costy one' - but we can at least try searching through the
list to see if we have an exactly matching mask. It should be sane
assumption that if the device can support reading only the exact
channels user is interested in, then this should be also the least costy
selection - and if it is not and optimization is important, then the
driver could consider omitting setting the 'available_scan_mask' and
doing demuxing - or just omitting the 'costy exact match' and providing
only the more efficient broader selection of channels.
Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
drivers/iio/industrialio-buffer.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
Comments
On Fri, 22 Sep 2023 14:17:49 +0300 Matti Vaittinen <mazziesaccount@gmail.com> wrote: > When IIO goes through the available scan masks in order to select the > best suiting one, it will just accept the first listed subset of channels > which meets the user's requirements. This works great for most of the > drivers as they can sort the list of channels in the order where > the 'least costy' channel selections come first. > > It may be that in some cases the ordering of the list of available scan > masks is not thoroughly considered. We can't really try outsmarting the > drivers by selecting the smallest supported subset - as this might not > be the 'least costy one' - but we can at least try searching through the > list to see if we have an exactly matching mask. It should be sane > assumption that if the device can support reading only the exact > channels user is interested in, then this should be also the least costy > selection - and if it is not and optimization is important, then the > driver could consider omitting setting the 'available_scan_mask' and > doing demuxing - or just omitting the 'costy exact match' and providing > only the more efficient broader selection of channels. > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> Whilst I fully agree with the reasoning behind this, I'd rather we did an audit of drivers to find any that have a non logical order (one came up today in review) and fix them up. A quick and dirty grep didn't find it to be a common problem, at least partly as most users of this feature only provide one valid mask. The few complex corners I found appear to be fine with the expected shortest sequences first. Defending against driver bugs is losing game if it makes the core code more complex to follow by changing stuff in non debug paths. One option might be to add a trivial check at iio_device_register() that we don't have scan modes that are subsets of modes earlier in the list. These lists are fairly short so should be cheap to run. That would incorporate ensuring exact matches come earlier by default. Jonathan > --- > drivers/iio/industrialio-buffer.c | 25 +++++++++++++++++++------ > 1 file changed, 19 insertions(+), 6 deletions(-) > > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c > index 176d31d9f9d8..e97396623373 100644 > --- a/drivers/iio/industrialio-buffer.c > +++ b/drivers/iio/industrialio-buffer.c > @@ -411,19 +411,32 @@ static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks, > const unsigned long *mask, > bool strict) > { > + const unsigned long *first_subset = NULL; > + > if (bitmap_empty(mask, masklength)) > return NULL; > - while (*av_masks) { > - if (strict) { > + > + if (strict) { > + while (*av_masks) { > if (bitmap_equal(mask, av_masks, masklength)) > return av_masks; > - } else { > - if (bitmap_subset(mask, av_masks, masklength)) > - return av_masks; > + > + av_masks += BITS_TO_LONGS(masklength); > } > + > + return NULL; > + } > + while (*av_masks) { > + if (bitmap_equal(mask, av_masks, masklength)) > + return av_masks; > + > + if (!first_subset && bitmap_subset(mask, av_masks, masklength)) > + first_subset = av_masks; > + > av_masks += BITS_TO_LONGS(masklength); > } > - return NULL; > + > + return first_subset; > } > > static bool iio_validate_scan_mask(struct iio_dev *indio_dev,
On Sun, 24 Sep 2023 17:07:26 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 22 Sep 2023 14:17:49 +0300 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > > When IIO goes through the available scan masks in order to select the > > best suiting one, it will just accept the first listed subset of channels > > which meets the user's requirements. This works great for most of the > > drivers as they can sort the list of channels in the order where > > the 'least costy' channel selections come first. > > > > It may be that in some cases the ordering of the list of available scan > > masks is not thoroughly considered. We can't really try outsmarting the > > drivers by selecting the smallest supported subset - as this might not > > be the 'least costy one' - but we can at least try searching through the > > list to see if we have an exactly matching mask. It should be sane > > assumption that if the device can support reading only the exact > > channels user is interested in, then this should be also the least costy > > selection - and if it is not and optimization is important, then the > > driver could consider omitting setting the 'available_scan_mask' and > > doing demuxing - or just omitting the 'costy exact match' and providing > > only the more efficient broader selection of channels. > > > > Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > > Whilst I fully agree with the reasoning behind this, I'd rather we > did an audit of drivers to find any that have a non logical order > (one came up today in review) and fix them up. > > A quick and dirty grep didn't find it to be a common problem, at least > partly as most users of this feature only provide one valid mask. > The few complex corners I found appear to be fine with the expected > shortest sequences first. > > Defending against driver bugs is losing game if it makes the core > code more complex to follow by changing stuff in non debug paths. > One option might be to add a trivial check at iio_device_register() > that we don't have scan modes that are subsets of modes earlier in the list. > These lists are fairly short so should be cheap to run. > > That would incorporate ensuring exact matches come earlier by default. BTW I'd have sent these as a separate series as there is potential that this will distract from or slow down the driver + not all the CC list will care about this core cleanup. Jonathan > > Jonathan > > > > --- > > drivers/iio/industrialio-buffer.c | 25 +++++++++++++++++++------ > > 1 file changed, 19 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c > > index 176d31d9f9d8..e97396623373 100644 > > --- a/drivers/iio/industrialio-buffer.c > > +++ b/drivers/iio/industrialio-buffer.c > > @@ -411,19 +411,32 @@ static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks, > > const unsigned long *mask, > > bool strict) > > { > > + const unsigned long *first_subset = NULL; > > + > > if (bitmap_empty(mask, masklength)) > > return NULL; > > - while (*av_masks) { > > - if (strict) { > > + > > + if (strict) { > > + while (*av_masks) { > > if (bitmap_equal(mask, av_masks, masklength)) > > return av_masks; > > - } else { > > - if (bitmap_subset(mask, av_masks, masklength)) > > - return av_masks; > > + > > + av_masks += BITS_TO_LONGS(masklength); > > } > > + > > + return NULL; > > + } > > + while (*av_masks) { > > + if (bitmap_equal(mask, av_masks, masklength)) > > + return av_masks; > > + > > + if (!first_subset && bitmap_subset(mask, av_masks, masklength)) > > + first_subset = av_masks; > > + > > av_masks += BITS_TO_LONGS(masklength); > > } > > - return NULL; > > + > > + return first_subset; > > } > > > > static bool iio_validate_scan_mask(struct iio_dev *indio_dev, >
On 9/24/23 19:07, Jonathan Cameron wrote: > On Fri, 22 Sep 2023 14:17:49 +0300 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > >> When IIO goes through the available scan masks in order to select the >> best suiting one, it will just accept the first listed subset of channels >> which meets the user's requirements. This works great for most of the >> drivers as they can sort the list of channels in the order where >> the 'least costy' channel selections come first. >> >> It may be that in some cases the ordering of the list of available scan >> masks is not thoroughly considered. We can't really try outsmarting the >> drivers by selecting the smallest supported subset - as this might not >> be the 'least costy one' - but we can at least try searching through the >> list to see if we have an exactly matching mask. It should be sane >> assumption that if the device can support reading only the exact >> channels user is interested in, then this should be also the least costy >> selection - and if it is not and optimization is important, then the >> driver could consider omitting setting the 'available_scan_mask' and >> doing demuxing - or just omitting the 'costy exact match' and providing >> only the more efficient broader selection of channels. >> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> > > Whilst I fully agree with the reasoning behind this, I'd rather we > did an audit of drivers to find any that have a non logical order > (one came up today in review) and fix them up. > > A quick and dirty grep didn't find it to be a common problem, at least > partly as most users of this feature only provide one valid mask. It's always good to hear there is not many problems found :) This patch was not inspired by auditing the existing code - it was inspired by the fact that I would have wrongly ordered the available_scan_masks for bm1390 myself. I just happened to notice the oddity in active_scan_masks while I was trying to figure out if it was the driver, IIO or user-space code which messed my buffer when I disabled timestamps. > The few complex corners I found appear to be fine with the expected > shortest sequences first. > > Defending against driver bugs is losing game if it makes the core > code more complex to follow by changing stuff in non debug paths. I think I agree, although I could argue that it depends on the amount of added complexity. Still ... > One option might be to add a trivial check at iio_device_register() ... this suggestion is superior to the check added in this patch. > that we don't have scan modes that are subsets of modes earlier in the list. > These lists are fairly short so should be cheap to run. Yes. And running the check at the registration phase should not be a big problem. And, if it appears to be a problem, then we can add a registration variant which omits the checks for those rare drivers which would _really_ be hurt by the few extra cycles spent on registration. > That would incorporate ensuring exact matches come earlier by default. Yes. I like the idea, wish I had invented it myself ;) > > Jonathan > > >> --- >> drivers/iio/industrialio-buffer.c | 25 +++++++++++++++++++------ >> 1 file changed, 19 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c >> index 176d31d9f9d8..e97396623373 100644 >> --- a/drivers/iio/industrialio-buffer.c >> +++ b/drivers/iio/industrialio-buffer.c >> @@ -411,19 +411,32 @@ static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks, >> const unsigned long *mask, >> bool strict) >> { >> + const unsigned long *first_subset = NULL; >> + >> if (bitmap_empty(mask, masklength)) >> return NULL; >> - while (*av_masks) { >> - if (strict) { >> + >> + if (strict) { >> + while (*av_masks) { >> if (bitmap_equal(mask, av_masks, masklength)) >> return av_masks; >> - } else { >> - if (bitmap_subset(mask, av_masks, masklength)) >> - return av_masks; >> + >> + av_masks += BITS_TO_LONGS(masklength); >> } >> + >> + return NULL; >> + } >> + while (*av_masks) { >> + if (bitmap_equal(mask, av_masks, masklength)) >> + return av_masks; >> + >> + if (!first_subset && bitmap_subset(mask, av_masks, masklength)) >> + first_subset = av_masks; >> + >> av_masks += BITS_TO_LONGS(masklength); >> } >> - return NULL; >> + >> + return first_subset; >> } >> >> static bool iio_validate_scan_mask(struct iio_dev *indio_dev, >
On 9/24/23 19:10, Jonathan Cameron wrote: > On Sun, 24 Sep 2023 17:07:26 +0100 > Jonathan Cameron <jic23@kernel.org> wrote: > >> On Fri, 22 Sep 2023 14:17:49 +0300 >> Matti Vaittinen <mazziesaccount@gmail.com> wrote: >> >>> When IIO goes through the available scan masks in order to select the >>> best suiting one, it will just accept the first listed subset of channels >>> which meets the user's requirements. This works great for most of the >>> drivers as they can sort the list of channels in the order where >>> the 'least costy' channel selections come first. >>> >>> It may be that in some cases the ordering of the list of available scan >>> masks is not thoroughly considered. We can't really try outsmarting the >>> drivers by selecting the smallest supported subset - as this might not >>> be the 'least costy one' - but we can at least try searching through the >>> list to see if we have an exactly matching mask. It should be sane >>> assumption that if the device can support reading only the exact >>> channels user is interested in, then this should be also the least costy >>> selection - and if it is not and optimization is important, then the >>> driver could consider omitting setting the 'available_scan_mask' and >>> doing demuxing - or just omitting the 'costy exact match' and providing >>> only the more efficient broader selection of channels. >>> >>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> >> >> Whilst I fully agree with the reasoning behind this, I'd rather we >> did an audit of drivers to find any that have a non logical order >> (one came up today in review) and fix them up. >> >> A quick and dirty grep didn't find it to be a common problem, at least >> partly as most users of this feature only provide one valid mask. >> The few complex corners I found appear to be fine with the expected >> shortest sequences first. >> >> Defending against driver bugs is losing game if it makes the core >> code more complex to follow by changing stuff in non debug paths. >> One option might be to add a trivial check at iio_device_register() >> that we don't have scan modes that are subsets of modes earlier in the list. >> These lists are fairly short so should be cheap to run. >> >> That would incorporate ensuring exact matches come earlier by default. > > BTW I'd have sent these as a separate series as there is potential that > this will distract from or slow down the driver + not all the CC list > will care about this core cleanup. I was not so worried about the driver being postponed. I was prepared to suggest to merging a subset of the patches if need be - while I can continue work with the rest of the series ;) What comes to people being interested in the core-changes Vs. people being interested in the driver changes - I'd expect the core changes to concern much wider audience than the driver changes. But yes, knowing the amount of mails people go through, limiting the recipient to most relevant ones never hurts. Besides, I think there is no conflicts/dependencies as driver changes don't change core/tools, and core/tool changes don't touch the driver so splitting this to two series should be trivial. Will do that for next version. Yours, -- Matti
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c index 176d31d9f9d8..e97396623373 100644 --- a/drivers/iio/industrialio-buffer.c +++ b/drivers/iio/industrialio-buffer.c @@ -411,19 +411,32 @@ static const unsigned long *iio_scan_mask_match(const unsigned long *av_masks, const unsigned long *mask, bool strict) { + const unsigned long *first_subset = NULL; + if (bitmap_empty(mask, masklength)) return NULL; - while (*av_masks) { - if (strict) { + + if (strict) { + while (*av_masks) { if (bitmap_equal(mask, av_masks, masklength)) return av_masks; - } else { - if (bitmap_subset(mask, av_masks, masklength)) - return av_masks; + + av_masks += BITS_TO_LONGS(masklength); } + + return NULL; + } + while (*av_masks) { + if (bitmap_equal(mask, av_masks, masklength)) + return av_masks; + + if (!first_subset && bitmap_subset(mask, av_masks, masklength)) + first_subset = av_masks; + av_masks += BITS_TO_LONGS(masklength); } - return NULL; + + return first_subset; } static bool iio_validate_scan_mask(struct iio_dev *indio_dev,