Messages in this thread | | | From | Andy Shevchenko <> | Date | Fri, 13 Aug 2021 13:03:25 +0300 | Subject | Re: [PATCH] iio: accel: adxl355: Add triggered buffer support |
| |
On Fri, Aug 13, 2021 at 11:35 AM Puranjay Mohan <puranjay12@gmail.com> wrote: > > Provide a way for continuous data capture by setting up buffer support. The > data ready signal exposed at the DRDY pin of the ADXL355 is exploited as > a hardware interrupt which triggers to fill the buffer.
...
> * > * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adxl354_adxl355.pdf > */ > -
Unrelated change.
...
> +#include <linux/iio/buffer.h> > #include <linux/iio/iio.h> > +#include <linux/iio/trigger.h> > +#include <linux/iio/triggered_buffer.h> > +#include <linux/iio/trigger_consumer.h>
I would rather regroup this like
linux/*
asm/*
linux/iio*
...
> +#include <linux/of_irq.h>
Okay, this is due to the absence of generic fwnode_irq_get_byname() or so.
...
> struct adxl355_data {
> + int irq;
Depending on container_of and frequency of usage this is not a good location for this. Strating from regmap pointer is much better (no pointer arithmetics involved).
> struct regmap *regmap; > struct device *dev; > struct mutex lock; /* lock to protect op_mode */
> };
...
> + ret = regmap_bulk_read(data->regmap, ADXL355_XDATA3_REG, > + &data->buffer.buf[1], > + 3);
ARRAY_SIZE()? Or put this 3 to the previous line, it will be easier to read.
Ditto for the rest of the similar code.
> + if (ret) > + goto out_unlock_notify;
...
> + /* > + * Would be good to move it to the generic version.
Something like "TODO: Would be..." ?
> + */ > + ret = of_irq_get_byname(dev->of_node, "DRDY"); > + if (ret > 0) > + data->irq = ret;
-- With Best Regards, Andy Shevchenko
| |