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

Jonathan Cameron
2024-08-24T10:02:59Z

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

>
> 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