On Wed, Aug 11, 2021 at 12:25 PM Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > > On Wed, Aug 11, 2021 at 8:19 AM Puranjay Mohan <puranjay12@xxxxxxxxx> wrote: > > > > ADXL355 is a 3-axis MEMS Accelerometer. It offers low noise density, > > low 0g offset drift, low power with selectable measurement ranges. > > It also features programmable high-pass and low-pass filters. > > Thanks for an update! My comments below. > After addressing, feel free to add > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > > ... > > > +#ifndef _ADXL355_H_ > > +#define _ADXL355_H_ > > + > > +#include <linux/regmap.h> > > What I meant here is you need to add a forward declaration, i.e. > > struct device; okay, I will add this. > > line. > > > +extern const struct regmap_access_table adxl355_readable_regs_tbl; > > +extern const struct regmap_access_table adxl355_writeable_regs_tbl; > > + > > +int adxl355_core_probe(struct device *dev, struct regmap *regmap, > > + const char *name); > > + > > +#endif /* _ADXL355_H_ */ > > ... > > > +#define MEGA(x) ((x) * 1000000) > > +#define TERA(x) ((x) * 1000000000000UL) > > What I meant is to define just constants. It will be easier to move > this to a global space (I have a patch, btw, but it may be applied > later). So, just > > #define MEGA 1000000UL > #define TERA 1000000000000UUL will change in v12 > > And note that the second one should have a ULL suffix. > > ... > > > + ret = regmap_bulk_write(data->regmap, > > + adxl355_chans[chan].offset_reg, > > + data->transf_buf, 2); > > ARRAY_SIZE() ? ARRAY SIZE is 3, but here we need to transfer 2 bytes only. > > > + if (ret) > > + goto set_opmode_unlock; > > -- > With Best Regards, > Andy Shevchenko -- Thanks and Regards Yours Truly, Puranjay Mohan
