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

[LI Qingwu]

> -----Original Message-----
> From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> Sent: Wednesday, May 4, 2022 10:39 PM
> To: LI Qingwu <qing-wu.li@xxxxxxxxxxxxxxxxxxxxxxx>
> Cc: Jonathan Cameron <jic23@xxxxxxxxxx>; Lars-Peter Clausen
> <lars@xxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; Tomas Melin
> <tomas.melin@xxxxxxxxxxx>; devicetree <devicetree@xxxxxxxxxxxxxxx>; Linux
> Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>; linux-iio
> <linux-iio@xxxxxxxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>
> 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 4:35 PM LI Qingwu
> <qing-wu.li@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > > From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
> > > Sent: Wednesday, May 4, 2022 10:20 PM On Wed, May 4, 2022 at 3:36 PM
> > > LI Qingwu <Qing-wu.Li@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> ...
> 
> > > > +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?
> 
> But why const qualifier? What is the point of it for example for u8 members if
> the entire object is qualified as const below in the same patch?
> 
> On top of that, please explain what in your opinion the "const ...
> *foo" gives us, and what we will lose if we remove the "const" part out of them.

Ah, you are right, those const are unnecessary for nonpointer members.
for the pointers, the contexts that the pointer points to are still writable.
what about if I remove all the const from nonpointer and keep it for the pointers?
Like：
const struct iio_chan_spec *channels;
const int (*accel_scale_table)[2];
const int (*incli_scale_table)[2];
const int *accel_scale_modes_map;
const int *incli_scale_modes_map;
const unsigned long *scan_masks;
const int *avail_modes_table;
const int *freq_modes_map;
const int *freq_table;
const char *name;
u8 num_accel_scales;
u8 num_incli_scales;
u8 num_avail_modes;
u8 num_channels;
u8 num_freqs;
u8 chip_id;
bool angle;



> 
> > > > +       const char *name;
> > > > +};
> 
> --
> With Best Regards,
> Andy Shevchenko