linux-kernel - RE: [PATCH V3 3/5] iio: accel: sca3300: modified to support multi chips

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <AM9PR06MB7844E8FE0EDF712C769271DAD7C39@AM9PR06MB7844.eurprd06.prod.outlook.com>
Date:   Wed, 4 May 2022 14:35:00 +0000
From:   LI Qingwu <qing-wu.li@...ca-geosystems.com.cn>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
CC:     Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Rob Herring <robh+dt@...nel.org>,
        Tomas Melin <tomas.melin@...sala.com>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-iio <linux-iio@...r.kernel.org>,
        Rob Herring <robh@...nel.org>
Subject: RE: [PATCH V3 3/5] iio: accel: sca3300: modified to support multi
 chips



> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@...il.com>
> Sent: Wednesday, May 4, 2022 10:20 PM
> To: LI Qingwu <Qing-wu.Li@...ca-geosystems.com.cn>
> Cc: Jonathan Cameron <jic23@...nel.org>; Lars-Peter Clausen
> <lars@...afoo.de>; Rob Herring <robh+dt@...nel.org>; Tomas Melin
> <tomas.melin@...sala.com>; devicetree <devicetree@...r.kernel.org>; Linux
> Kernel Mailing List <linux-kernel@...r.kernel.org>; linux-iio
> <linux-iio@...r.kernel.org>; Rob Herring <robh@...nel.org>
> Subject: Re: [PATCH V3 3/5] iio: accel: sca3300: modified to support multi chips
> 
> This email is not from Hexagon’s Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
> 
> 
> On Wed, May 4, 2022 at 3:36 PM LI Qingwu
> <Qing-wu.Li@...ca-geosystems.com.cn> wrote:
> >
> > The drive support sca3300 only.
> 
> driver supports
> 
> > There are some other similar chips, for instance, SCL3300.
> > Prepare the way for multiple chips and additional channels.
> > Modify the driver to read the device id.
> > Add the tables for the corresponding id to support multiple chips.
> > Add prepares for the addition of extra channels.
> > Add prepares for handling the operation modes for multiple chips.
> 
> It seems you need to work more on the commit messages in all patches.
Thanks, Andy. I rely on grammar checking but it didn't point out. 
Let me fix it in the next version.
> 
> ...
> 
> > +struct sca3300_chip_info {
> > +       const struct iio_chan_spec *channels;
> > +       const int (*accel_scale_table)[2];
> > +       const int *accel_scale_modes_map;
> > +       const unsigned long *scan_masks;
> > +       const int *avail_modes_table;
> > +       const int *freq_modes_map;
> > +       const int *freq_table;
> > +       const u8 num_accel_scales;
> > +       const u8 num_avail_modes;
> > +       const u8 num_channels;
> > +       const u8 num_freqs;
> > +       const u8 chip_id;
> 
> Why do you have const qualifier on all members?  The last one is
> understandable, but the rest, esp. pointers should be justified.
Because I thought it was static and has fix value for each chip, unacceptable for you?
> 
> > +       const char *name;
> > +};
> 
> ...
> 
> > +static int sca3300_set_op_mode(struct sca3300_data *sca_data, int
> > +index) {
> > +       int mode;
> > +
> > +       if ((index < 0) || (index >= sca_data->chip->num_avail_modes))
> 
> Too many parentheses.
> 
> > +               return -EINVAL;
> > +
> > +       mode = sca_data->chip->avail_modes_table[index];
> > +
> > +       return sca3300_write_reg(sca_data, SCA3300_REG_MODE,
> mode); }
> 
> --
> With Best Regards,
> Andy Shevchenko