linux-kernel - Re: [PATCH v4 03/15] iio: adc: ad7091r: Set alert bit in config register

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: Mon, 18 Dec 2023 14:22:31 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>, 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,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 03/15] iio: adc: ad7091r: Set alert bit in config
 register

On 12/17, Jonathan Cameron wrote:
> On Sat, 16 Dec 2023 14:46:37 -0300
> Marcelo Schmitt <marcelo.schmitt@...log.com> wrote:
> 
> > The ad7091r-base driver sets up an interrupt handler for firing events
> > when inputs are either above or below a certain threshold.
> > However, for the interrupt signal to come from the device it must be
> > configured to enable the ALERT/BUSY/GPO pin to be used as ALERT, which
> > was not being done until now.
> > Enable interrupt signals on the ALERT/BUSY/GPO pin by setting the proper
> > bit in the configuration register.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> Hi Marcelo,
> 
> In V3 review I asked if this should have a fixes tag.  I've assumed for now
> the answer is no and applied it without.  If you let me know fast enough
> I can probably slip on in, but if not you may want to consider requesting
> a backport after this is upstream.

The events for these devices would not work both because of the broken
dereference fixed in patch 2 and the alert signal was not being enabled.
Patch 2 fixed a null pointer dereference that would lead to an error in the
kernel. This patch (on top of the previous one) makes the event generation
actually work although it's not fixing any errors.
I was hesitant in marking this one with a fixes tag too worrying I might be
adding too many fixes tags for a feature that never worked.
Anyway, looks like I should have added that so at least now I have learned something.
Will see how to ask backports for the fixing patches once they get in mainline.

Thanks

> 
> Applied to the togreg branch of iio.git and pushed out as testing for 0-day
> to take a look at it.
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/adc/ad7091r-base.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/iio/adc/ad7091r-base.c b/drivers/iio/adc/ad7091r-base.c
> > index 0e5d3d2e9c98..8aaa854f816f 100644
> > --- a/drivers/iio/adc/ad7091r-base.c
> > +++ b/drivers/iio/adc/ad7091r-base.c
> > @@ -28,6 +28,7 @@
> >  #define AD7091R_REG_RESULT_CONV_RESULT(x)   ((x) & 0xfff)
> >  
> >  /* AD7091R_REG_CONF */
> > +#define AD7091R_REG_CONF_ALERT_EN   BIT(4)
> >  #define AD7091R_REG_CONF_AUTO   BIT(8)
> >  #define AD7091R_REG_CONF_CMD    BIT(10)
> >  
> > @@ -232,6 +233,11 @@ int ad7091r_probe(struct device *dev, const char *name,
> >  	iio_dev->channels = chip_info->channels;
> >  
> >  	if (irq) {
> > +		ret = regmap_update_bits(st->map, AD7091R_REG_CONF,
> > +					 AD7091R_REG_CONF_ALERT_EN, BIT(4));
> > +		if (ret)
> > +			return ret;
> > +
> >  		ret = devm_request_threaded_irq(dev, irq, NULL,
> >  				ad7091r_event_handler,
> >  				IRQF_TRIGGER_FALLING | IRQF_ONESHOT, name, iio_dev);
>