linux-kernel - Re: [PATCH v3 2/2] iio: adc: Add TI ADS1100 and ADS1000

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: <abe9224f-9c4d-f8f5-ad7c-5d034bd1056d@topic.nl>
Date:   Mon, 6 Mar 2023 13:56:15 +0100
From:   Mike Looijmans <mike.looijmans@...ic.nl>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Jonathan Cameron <jic23@...nel.org>
CC:     devicetree@...r.kernel.org, linux-iio@...r.kernel.org,
        Caleb Connolly <caleb.connolly@...aro.org>,
        ChiYuan Huang <cy_huang@...htek.com>,
        ChiaEn Wu <chiaen_wu@...htek.com>,
        Cosmin Tanislav <demonsingur@...il.com>,
        Ibrahim Tilki <Ibrahim.Tilki@...log.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Ramona Bolboaca <ramona.bolboaca@...log.com>,
        William Breathitt Gray <william.gray@...aro.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] iio: adc: Add TI ADS1100 and ADS1000


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@...icproducts.com
W: www.topic.nl

Please consider the environment before printing this e-mail
On 06-03-2023 13:11, Andy Shevchenko wrote:
> On Sat, Mar 04, 2023 at 05:57:51PM +0000, Jonathan Cameron wrote:
>> On Tue, 28 Feb 2023 07:31:51 +0100
>> Mike Looijmans <mike.looijmans@...ic.nl> wrote:
> ...
>
>>> +	for (i = 0; i < 4; i++) {
>>> +		if (BIT(i) == gain) {
>>> +			ads1100_set_config_bits(data, ADS1100_PGA_MASK, i);
>>> +			return 0;
>>> +		}
>>> +	}
>> Andy's suggestion of something like..
>> 	if (!gain)
>> 		return -EINVAL;
>> 	i = ffs(gain);
>> 	if (i >= 4 || BIT(i) != gain)
>> 		return -EINVAL;
>>
>> 	ads...
>>
>> Is perhaps nicer than the loop.
> Even better:
>
> 	if (!gain || !is_power_of_2(gain))
> 		return -EINVAL;
>
> 	i = ffs(gain);
> 	if (i >= 4)
> 		return -EINVAL;
>
I'd guess that "is_power_of_2" is about the same complexity as "ffs".

If we want smaller code, in retrospect, I'd vote for omitting that 
power-of-two check altogether.

The IIO device reports this for a 3v3 Vdd:
# cat /sys/bus/iio/devices/iio\:device1/scale_available
0.100708007 0.050354003 0.025177001 0.012588500
# echo 0.012588500 > /sys/bus/iio/devices/iio:device1/scale

That last statement results in val2=12588 in this call.
The whole point of this exercise is that the value '0.012588' 
corresponds to a gain of '8' here. There's already quite a bit of 
rounding going on, since writing to the scale turns the value into 
"micro" scale.

Also, looking at the "avail" table. the values for "7" and "8" are much 
closer together than "3" and "4".

And the correct formula for the inverse of "gain = BIT(i);" is "i = 
ffs(gain) - 1;" because ffs(1) == 1

So I propose this code:

     if (gain <= 0 || gain > 8)
         return -EINVAL;

     regval = ffs(gain) - 1;
     ads1100_set_config_bits(data, ADS1100_PGA_MASK, regval);


-- 
Mike Looijmans