linux-kernel - Re: [PATCH v11 2/2] iio: accel: Add driver support for ADXL355

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 11 Aug 2021 12:40:39 +0530
From:   Puranjay Mohan <puranjay12@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Michael Hennerich <Michael.Hennerich@...log.com>,
        Jonathan Cameron <jic23@...nel.org>,
        devicetree <devicetree@...r.kernel.org>,
        linux-iio <linux-iio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        "Bogdan, Dragos" <Dragos.Bogdan@...log.com>,
        Darius <Darius.Berghe@...log.com>
Subject: Re: [PATCH v11 2/2] iio: accel: Add driver support for ADXL355

On Wed, Aug 11, 2021 at 12:25 PM Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
>
> On Wed, Aug 11, 2021 at 8:19 AM Puranjay Mohan <puranjay12@...il.com> 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@...il.com>
>
> ...
>
> > +#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