Received-SPF: pass (google.com: domain of linux-kernel+bounces-4157-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Mon, 18 Dec 2023 14:22:31 -0300
From: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: Marcelo Schmitt <marcelo.schmitt@analog.com>, apw@canonical.com,
	joe@perches.com, dwaipayanray1@gmail.com, lukas.bulwahn@gmail.com,
	paul.cercueil@analog.com, Michael.Hennerich@analog.com,
	lars@metafoo.de, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	dan.carpenter@linaro.org, dlechner@baylibre.com,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 03/15] iio: adc: ad7091r: Set alert bit in config
 register
Message-ID: <ZYB_1ymQEhUbdSki@debian-BULLSEYE-live-builder-AMD64>
References: <cover.1702746240.git.marcelo.schmitt1@gmail.com>
 <e8da2ee98d6df88318b14baf3dc9630e20218418.1702746240.git.marcelo.schmitt1@gmail.com>
 <20231217145557.36d6a738@jic23-huawei>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231217145557.36d6a738@jic23-huawei>

On 12/17, Jonathan Cameron wrote:
> On Sat, 16 Dec 2023 14:46:37 -0300
> Marcelo Schmitt <marcelo.schmitt@analog.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@analog.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);
>