Re: [PATCH v4 4/6] iio: accel: fxls8962af: add interrupt support

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Thu, Apr 29, 2021 at 3:28 PM Sean Nyekjaer <sean@geanix.com> wrote:
>
> Preparation commit for the next that adds hw buffered sampling

It misses a better description of what's going on here.
It misses the period at the end of the line.

...

> +#include <linux/of_irq.h>

See below.

...

> +static irqreturn_t fxls8962af_interrupt(int irq, void *p)
> +{
> +       struct iio_dev *indio_dev = p;
> +       struct fxls8962af_data *data = iio_priv(indio_dev);
> +       unsigned int reg;
> +       int ret;
> +
> +       ret = regmap_read(data->regmap, FXLS8962AF_INT_STATUS, &reg);
> +       if (ret < 0)
> +               return IRQ_NONE;
> +
> +       return IRQ_NONE;

And how is it handled?

> +}

...

> +static void fxls8962af_get_irq(struct device *dev, enum fxls8962af_int_pin *pin)
> +{
> +       if (!device_property_match_string(dev, "interrupt-names", "INT2")) {

This doesn't feel right.
You should use fwnode_get_irq() or introduce fwnode_get_irq_by_name().

Since ACPI has no given names for Interrupt resources (and moreover
there are two types of them), it also needs a support from the
software like we have for GPIOs with struct acpi_gpio_table (luckily
it's part of _DSD() nowadays, but in general it's not required by the
spec).

> +               *pin = FXLS8962AF_PIN_INT2;
> +               return;
> +       }
> +
> +       *pin = FXLS8962AF_PIN_INT1;
> +}


-- 
With Best Regards,
Andy Shevchenko