Hi Jonathan,
On Fri, 11 Aug 2023 at 16:20, Jonathan Cameron
<Jonathan.Cameron@xxxxxxxxxx> wrote:
>
> On Fri, 11 Aug 2023 10:08:00 +0100
> Phil Elwell <phil@xxxxxxxxxxxxxxx> wrote:
>
> > Hi Jonathan and linux-iio,
> >
> > I've run into a problem running a BMP180 over I2C - it's on a
> > Raspberry Pi, but I don't think that's particularly relevant. As it's
> > a BMP180 it has no interrupt signal, but it apparently shares an ID
> > with the BMP085 which does. As a result, the driver and bindings
> > sensibly treat the IRQ as optional.
> >
> > The function bmp280_common_probe contains the following fragment:
> >
> > /*
> > * Attempt to grab an optional EOC IRQ - only the BMP085 has this
> > * however as it happens, the BMP085 shares the chip ID of BMP180
> > * so we look for an IRQ if we have that.
> > */
> > if (irq > 0 || (chip_id == BMP180_CHIP_ID)) {
> > ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
> > if (ret)
> > return ret;
> > }
> >
> > where the irq is passed in from the I2C or SPI framework. Inside
> > bmp085_fetch_eoc_irq it immediately does:
> >
> > irq_trig = irqd_get_trigger_type(irq_get_irq_data(irq));
> >
> > irq_get_irq_data converts irq to an irqd, returning either a pointer
> > to the irq_data or a NULL. irq_get_trigger_type essentially just
> > indirects through the pointer to retrieve the irq_common_data pointer.
> >
> > There is nothing to prevent irq from being 0, and in the case of a
> > BMP180 that is the expected value. This is where it gets strange: on
> > an ARCH=arm build I'm getting a valid-looking irq_data pointer back
> > for IRQ 0, but on ARCH=arm64 I get the more obvious NULL pointer,
> > leading to an exception.
> >
> > I'm hesitant to suggest there's a bug in such old code, but I don't
> > understand why the condition in the probe function isn't:
> >
> > if (irq > 0 && (chip_id == BMP180_CHIP_ID))
>
> Agreed. That was my thought as well when I read the comment you quote
> above. Worst that happens is we don't provide an optional interrupt,
> so this is a fairly safe fix even if the intent was something different.
>
> Mind you, it's also odd if someone provides an irq to a BMP180.
Thanks for the speedy reply. I'll put together a patch to that effect.
Phil
