On Fri, Aug 13, 2021 at 11:35 AM Puranjay Mohan <puranjay12@xxxxxxxxx> 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
