Re: [PATCH v11 2/2] iio: accel: Add driver support for ADXL355
From: Andy Shevchenko
Date: Wed Aug 11 2021 - 02:55:58 EST
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