Re: [PATCH v3 7/7] iio: pressure: bmp280: Move bmp085 interrupt to new configuration

[Vasileios Amoiridis <vassilisamir@gmail.com>]

On Sat, Aug 24, 2024 at 11:02:42AM +0100, Jonathan Cameron wrote:
> On Fri, 23 Aug 2024 20:17:14 +0200
> Vasileios Amoiridis <vassilisamir@gmail.com> wrote:
> 
> > This commit intends to add the old BMP085 sensor to the new IRQ interface
> > of the sensor consistence. No functional changes intended.
> 
> interface of the driver for consistence
>
 
ACK.

> > 
> > The BMP085 sensor is equivalent with the BMP180 with the only difference of
> > BMP085 having an extra interrupt pin to inform about an End of Conversion.
> > 
> > Signed-off-by: Vasileios Amoiridis <vassilisamir@gmail.com>
> > ---
> >  drivers/iio/pressure/bmp280-core.c | 68 ++++++++++++++++++++++--------
> >  drivers/iio/pressure/bmp280-i2c.c  |  4 +-
> >  drivers/iio/pressure/bmp280-spi.c  |  4 +-
> >  drivers/iio/pressure/bmp280.h      |  1 +
> >  4 files changed, 56 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> > index f5268a13bfdb..1d27777d8a2c 100644
> > --- a/drivers/iio/pressure/bmp280-core.c
> > +++ b/drivers/iio/pressure/bmp280-core.c
> > @@ -3058,13 +3058,19 @@ static irqreturn_t bmp085_eoc_irq(int irq, void *d)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > -static int bmp085_fetch_eoc_irq(struct device *dev,
> > -				const char *name,
> > -				int irq,
> > -				struct bmp280_data *data)
> > +static int bmp085_trigger_probe(struct iio_dev *indio_dev)
> >  {
> > +	struct bmp280_data *data = iio_priv(indio_dev);
> > +	struct device *dev = data->dev;
> > +	struct fwnode_handle *fwnode;
> >  	unsigned long irq_trig;
> > -	int ret;
> > +	int ret, irq;
> > +
> > +	fwnode = dev_fwnode(data->dev);
> > +	if (!fwnode)
> > +		return -ENODEV;
> > +
> > +	irq = fwnode_irq_get(fwnode, 0);
> 
> Should check this + as Andy pointed out in earlier review this
> call will return an error if fwnode not present anyway so can skip
> earlier check.
> see fwnode_call_int_op() definition.
> 
> Otherwise, Andy has given some detailed review. I wouldn't suggest
> applying the style cleanup to existing code but he's entirely
> correct that we can make the stuff being touched anyway easier
> to read.
> 
> The more functional stuff maybe needs to be looked at in other
> drivers though.
> 
> Jonathan


Hi Jonathan,

Thanks for the feedback. Indeed Andy has given a very detailed
review feedback, I will also check what you said.

Cheers,
Vasilis