lkml.org 
[lkml]   [2022]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH V5 3/5] iio: accel: sca3300: modified to support multi chips
On Thu, May 12, 2022 at 6:03 PM LI Qingwu
<Qing-wu.Li@leica-geosystems.com.cn> wrote:
>
> The driver supports sca3300 only, there are some other similar chips,
> for instance, SCL3300. This commit prepares the way for multiple chips
> and additional channels. Modify the driver to read the device ID and load
> the corresponding sensor information from the table to support multiple
> chips. add prepares for the addition of extra channels. Add prepares for
> handling the operation modes for multiple chips.

Reading it again I think you may format it better, i.e.

Prepare the way for multiple chips and additional channels:
- Modify the driver to read the device ID and load the corresponding
sensor information from the table to support multiple chips
- Add prepares for the addition of extra channels
- Prepare for handling the operation modes for multiple chips

...

> +struct sca3300_chip_info {
> + const unsigned long *scan_masks;
> + const struct iio_chan_spec *channels;
> + u8 num_channels;
> + u8 num_accel_scales;
> + const int (*accel_scale)[2];
> + const int *accel_scale_map;
> + u8 num_freqs;
> + const int *freq_table;
> + const int *freq_map;

> + const char *name;

You can put it in the first place.

> + const int *avail_modes_table;
> + u8 num_avail_modes;
> + u8 chip_id;
> +};

...

> +static const struct sca3300_chip_info sca3300_chip_tbl[] = {
> + { .scan_masks = sca3300_scan_masks,

Keep { on a separate line.

> + .channels = sca3300_channels,
> + .num_channels = ARRAY_SIZE(sca3300_channels),
> + .num_accel_scales = ARRAY_SIZE(sca3300_accel_scale)*2,
> + .accel_scale = sca3300_accel_scale,
> + .accel_scale_map = sca3300_accel_scale_map,
> + .num_freqs = ARRAY_SIZE(sca3300_lp_freq),
> + .freq_table = sca3300_lp_freq,
> + .freq_map = sca3300_lp_freq_map,
> + .name = "sca3300",
> + .avail_modes_table = sca3300_avail_modes_map,
> + .num_avail_modes = 4,
> + .chip_id = SCA3300_WHOAMI_ID,
> + },
> +};

...

> + ret = sca3300_read_reg(sca_data, SCA3300_REG_MODE, &reg_val);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < sca_data->chip->num_avail_modes; i++) {
> + if (sca_data->chip->avail_modes_table[i] == reg_val&0x03)
> + break;
> + }

> +

This blank line is not needed as I explained.

> + if (i >= sca_data->chip->num_avail_modes)

== is enough and better to understand.

> + return -EINVAL;
> +
> + *index = i;
> + return 0;
> +}

...

> + int index;
> + int i;

Both can be unsigned.

...

> + for (i = 0; i < chip->num_avail_modes; i++) {
> + if ((val == chip->freq_table[chip->freq_map[i]]) &&
> + (chip->accel_scale[chip->accel_scale_map[index]] ==
> + chip->accel_scale[chip->accel_scale_map[i]]))
> + break;
> + }
> +
> + if (i >= chip->num_avail_modes)
> + return -EINVAL;

Two comments as per above for-loop case.

--
With Best Regards,
Andy Shevchenko

\
 
 \ /
  Last update: 2022-05-12 19:33    [W:0.075 / U:0.152 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site