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; 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 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() ? > + if (ret) > + goto set_opmode_unlock; -- With Best Regards, Andy Shevchenko