linux-kernel - Re: [PATCH v4 15/15] iio: adc: ad7091r: Allow users to configure device events

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]

Date: Sun, 17 Dec 2023 15:54:09 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: <apw@...onical.com>, <joe@...ches.com>, <dwaipayanray1@...il.com>,
 <lukas.bulwahn@...il.com>, <paul.cercueil@...log.com>,
 <Michael.Hennerich@...log.com>, <lars@...afoo.de>, <robh+dt@...nel.org>,
 <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
 <dan.carpenter@...aro.org>, <dlechner@...libre.com>,
 <marcelo.schmitt1@...il.com>, <linux-iio@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 15/15] iio: adc: ad7091r: Allow users to configure
 device events

On Sat, 16 Dec 2023 14:51:50 -0300
Marcelo Schmitt <marcelo.schmitt@...log.com> wrote:

> Implement event configuration callbacks allowing users to read/write
> event thresholds and enable/disable event generation.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> ---
> This is from a review suggestion David made on v3 [1].
> 
> Is this the case for a Suggested-by tag?
> 
> [1]: https://lore.kernel.org/linux-iio/CAMknhBFPbAqp4-AQdmbp+VRW-Ksk1PxaLCG+3n=Zk4gyStqhgw@mail.gmail.com/#t
> 
>  drivers/iio/adc/ad7091r-base.c | 117 +++++++++++++++++++++++++++++++--
>  1 file changed, 113 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> index 57355ca157a1..64e8baeff258 100644
> --- a/drivers/iio/adc/ad7091r-base.c
> +++ b/drivers/iio/adc/ad7091r-base.c
> @@ -20,19 +20,18 @@ const struct iio_event_spec ad7091r_events[] = {
>  	{
>  		.type = IIO_EV_TYPE_THRESH,
>  		.dir = IIO_EV_DIR_RISING,
> -		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> -				 BIT(IIO_EV_INFO_ENABLE),
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),

This is an ABI change.  So would need a really strong reason to make it...
mind you - it seems like this has been broken until now anyway so this change
may be fine.


>  	},
>  	{
>  		.type = IIO_EV_TYPE_THRESH,
>  		.dir = IIO_EV_DIR_FALLING,
> -		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
> -				 BIT(IIO_EV_INFO_ENABLE),
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE),
>  	},
>  	{
>  		.type = IIO_EV_TYPE_THRESH,
>  		.dir = IIO_EV_DIR_EITHER,
>  		.mask_separate = BIT(IIO_EV_INFO_HYSTERESIS),
> +		.mask_shared_by_all = BIT(IIO_EV_INFO_ENABLE),
It's relatively unusual that you can't separate the two event directions with careful
control of the thresholds.  So I think you can implement the existing ABI by
just setting the thresholds to the either 0 or 2^12 - 1 as appropriate.
The docs seem to say it must exceed the value, or fall below the min so these values
should ensure it can't do either.

You can then enable the event generate if one of them is set.