diff mbox series

[V6,3/5] iio: accel: sca3300: modified to support multi chips

Message ID 20220513124135.1295822-4-Qing-wu.Li@leica-geosystems.com.cn (mailing list archive)
State Changes Requested
Headers show
Series iio: accel: sca3300: add compatible for scl3300 | expand

Commit Message

LI Qingwu May 13, 2022, 12:41 p.m. UTC
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

Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
---
 drivers/iio/accel/sca3300.c | 186 ++++++++++++++++++++++++++++--------
 1 file changed, 145 insertions(+), 41 deletions(-)

Comments

Jonathan Cameron May 14, 2022, 2:10 p.m. UTC | #1
On Fri, 13 May 2022 12:41:33 +0000
LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:

> 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
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
A few requests for additional comments inline where the handling
is 'unusual' enough to be non obvious to figure out.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/sca3300.c | 186 ++++++++++++++++++++++++++++--------
>  1 file changed, 145 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
> index ff16d2cc8c70..bc6e0213e4aa 100644
> --- a/drivers/iio/accel/sca3300.c
> +++ b/drivers/iio/accel/sca3300.c
> @@ -93,15 +93,35 @@ static const struct iio_chan_spec sca3300_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(4),
>  };
>  
> -static const int sca3300_lp_freq[] = {70, 70, 70, 10};
> -static const int sca3300_accel_scale[][2] = {{0, 370}, {0, 741}, {0, 185}, {0, 185}};
> +static const int sca3300_lp_freq[] = {70, 10};
> +static const int sca3300_lp_freq_map[] = {0, 0, 0, 1};
>  
> +static const int sca3300_accel_scale[][2] = {{0, 370}, {0, 741}, {0, 185}};
> +static const int sca3300_accel_scale_map[] = {0, 1, 2, 2};
> +
> +static const int sca3300_avail_modes_map[] = {0, 1, 2, 3};
>  static const unsigned long sca3300_scan_masks[] = {
>  	BIT(SCA3300_ACC_X) | BIT(SCA3300_ACC_Y) | BIT(SCA3300_ACC_Z) |
>  	BIT(SCA3300_TEMP),
>  	0
>  };
>  
> +struct sca3300_chip_info {
> +	const char *name;
> +	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 int *avail_modes_table;
> +	u8 num_avail_modes;
> +	u8 chip_id;
> +};
> +
>  /**
>   * struct sca3300_data - device data
>   * @spi: SPI device structure
> @@ -117,10 +137,29 @@ struct sca3300_data {
>  		s16 channels[4];
>  		s64 ts __aligned(sizeof(s64));
>  	} scan;
> +	const struct sca3300_chip_info *chip;

Needs documentation as struct sca3300_data has kernel doc.
Also, move this above scan.  That way all the buffers used
for various purposes will remain together.

>  	u8 txbuf[4] ____cacheline_aligned;
>  	u8 rxbuf[4];
>  };
>  
> +static const struct sca3300_chip_info sca3300_chip_tbl[] = {
> +	{
> +		.name = "sca3300",
> +		.scan_masks = sca3300_scan_masks,
> +		.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,
> +		.avail_modes_table = sca3300_avail_modes_map,
> +		.num_avail_modes = 4,
> +		.chip_id = SCA3300_WHOAMI_ID,
> +	},
> +};
> +
>  DECLARE_CRC8_TABLE(sca3300_crc_table);
>  
>  static int sca3300_transfer(struct sca3300_data *sca_data, int *val)
> @@ -227,36 +266,80 @@ static int sca3300_write_reg(struct sca3300_data *sca_data, u8 reg, int val)
>  	return sca3300_error_handler(sca_data);
>  }
>  
> +static int sca3300_set_op_mode(struct sca3300_data *sca_data, int index)
> +{
> +	if ((index < 0) || (index >= sca_data->chip->num_avail_modes))
> +		return -EINVAL;
> +
> +	return sca3300_write_reg(sca_data, SCA3300_REG_MODE,
> +				 sca_data->chip->avail_modes_table[index]);
> +}
> +
> +static int sca3300_get_op_mode(struct sca3300_data *sca_data, int *index)
> +{
> +	int reg_val;
> +	int ret;
> +	int i;
> +
> +	ret = sca3300_read_reg(sca_data, SCA3300_REG_MODE, &reg_val);
> +	if (ret)
> +		return ret;
> +
> +	reg_val &= GENMASK(1, 0);
> +	for (i = 0; i < sca_data->chip->num_avail_modes; i++) {
> +		if (sca_data->chip->avail_modes_table[i] == reg_val)
> +			break;
> +	}
> +	if (i == sca_data->chip->num_avail_modes)
> +		return -EINVAL;
> +
> +	*index = i;
> +	return 0;
> +}
> +
> +static int sca3300_set_frequency(struct sca3300_data *data, int val)
> +{
> +	const struct sca3300_chip_info *chip = data->chip;
> +	unsigned int index;
> +	unsigned int i;
> +
> +	if (sca3300_get_op_mode(data, &index))
> +		return -EINVAL;
> +
> +	for (i = 0; i < chip->num_avail_modes; i++) {
> +		if ((val == chip->freq_table[chip->freq_map[i]]) &&

The conditions being checked here are far from obvious, so I think this would benefit
from an explanatory comment.

Something along the lines of,
"Find a mode in which the requested sampling frequency is available
 and the scaling currently set is retained".



> +		    (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;
> +
> +	return sca3300_set_op_mode(data, i);
> +}
> +
>  static int sca3300_write_raw(struct iio_dev *indio_dev,
>  			     struct iio_chan_spec const *chan,
>  			     int val, int val2, long mask)
>  {
>  	struct sca3300_data *data = iio_priv(indio_dev);
> -	int reg_val;
> -	int ret;
> +	int index;
>  	int i;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SCALE:
> -		if (val)
> +		if (chan->type != IIO_ACCEL)
>  			return -EINVAL;
> -
> -		for (i = 0; i < ARRAY_SIZE(sca3300_accel_scale); i++) {
> -			if (val2 == sca3300_accel_scale[i][1])
> -				return sca3300_write_reg(data, SCA3300_REG_MODE, i);
> +		for (i = 0; i < data->chip->num_avail_modes; i++) {
> +			index = data->chip->accel_scale_map[i];

Also a comment here that we are letting scale take priority over
sampling frequency. That makes sense given we can only ever end up increasing
the sampling frequency which is unlikely to be a problem.

> +			if ((val  == data->chip->accel_scale[index][0]) &&
> +			    (val2 == data->chip->accel_scale[index][1])) {
> +				return sca3300_set_op_mode(data, i);
> +			}
>  		}
>  		return -EINVAL;
> -
>  	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> -		ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
> -		if (ret)
> -			return ret;
> -		/* freq. change is possible only for mode 3 and 4 */
> -		if (reg_val == 2 && val == sca3300_lp_freq[3])
> -			return sca3300_write_reg(data, SCA3300_REG_MODE, 3);
> -		if (reg_val == 3 && val == sca3300_lp_freq[2])
> -			return sca3300_write_reg(data, SCA3300_REG_MODE, 2);
> -		return -EINVAL;
> +		return sca3300_set_frequency(data, val);
>  	default:
>  		return -EINVAL;
>  	}
Tomas Melin May 16, 2022, 6:07 a.m. UTC | #2
Hi,

On 14/05/2022 17:10, Jonathan Cameron wrote:
> On Fri, 13 May 2022 12:41:33 +0000
> LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> 
>> 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
>>
>> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> A few requests for additional comments inline where the handling
> is 'unusual' enough to be non obvious to figure out.
> 
> Thanks,
> 
> Jonathan
> 
>> ---
>>  drivers/iio/accel/sca3300.c | 186 ++++++++++++++++++++++++++++--------
>>  1 file changed, 145 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
>> index ff16d2cc8c70..bc6e0213e4aa 100644
>> --- a/drivers/iio/accel/sca3300.c
>> +++ b/drivers/iio/accel/sca3300.c
>> @@ -93,15 +93,35 @@ static const struct iio_chan_spec sca3300_channels[] = {
>>  	IIO_CHAN_SOFT_TIMESTAMP(4),
>>  };
>>  
>> -static const int sca3300_lp_freq[] = {70, 70, 70, 10};
>> -static const int sca3300_accel_scale[][2] = {{0, 370}, {0, 741}, {0, 185}, {0, 185}};
>> +static const int sca3300_lp_freq[] = {70, 10};
>> +static const int sca3300_lp_freq_map[] = {0, 0, 0, 1};
>>  
>> +static const int sca3300_accel_scale[][2] = {{0, 370}, {0, 741}, {0, 185}};
>> +static const int sca3300_accel_scale_map[] = {0, 1, 2, 2};
>> +
>> +static const int sca3300_avail_modes_map[] = {0, 1, 2, 3};
>>  static const unsigned long sca3300_scan_masks[] = {
>>  	BIT(SCA3300_ACC_X) | BIT(SCA3300_ACC_Y) | BIT(SCA3300_ACC_Z) |
>>  	BIT(SCA3300_TEMP),
>>  	0
>>  };
>>  
>> +struct sca3300_chip_info {
>> +	const char *name;
>> +	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 int *avail_modes_table;
>> +	u8 num_avail_modes;
>> +	u8 chip_id;
>> +};
>> +
>>  /**
>>   * struct sca3300_data - device data
>>   * @spi: SPI device structure
>> @@ -117,10 +137,29 @@ struct sca3300_data {
>>  		s16 channels[4];
>>  		s64 ts __aligned(sizeof(s64));
>>  	} scan;
>> +	const struct sca3300_chip_info *chip;
> 
> Needs documentation as struct sca3300_data has kernel doc.
> Also, move this above scan.  That way all the buffers used
> for various purposes will remain together.
> 
>>  	u8 txbuf[4] ____cacheline_aligned;
>>  	u8 rxbuf[4];
>>  };
>>  
>> +static const struct sca3300_chip_info sca3300_chip_tbl[] = {
>> +	{
>> +		.name = "sca3300",
>> +		.scan_masks = sca3300_scan_masks,
>> +		.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,
>> +		.avail_modes_table = sca3300_avail_modes_map,
>> +		.num_avail_modes = 4,
>> +		.chip_id = SCA3300_WHOAMI_ID,
>> +	},
>> +};
>> +
>>  DECLARE_CRC8_TABLE(sca3300_crc_table);
>>  
>>  static int sca3300_transfer(struct sca3300_data *sca_data, int *val)
>> @@ -227,36 +266,80 @@ static int sca3300_write_reg(struct sca3300_data *sca_data, u8 reg, int val)
>>  	return sca3300_error_handler(sca_data);
>>  }
>>  
>> +static int sca3300_set_op_mode(struct sca3300_data *sca_data, int index)
>> +{
>> +	if ((index < 0) || (index >= sca_data->chip->num_avail_modes))
>> +		return -EINVAL;
>> +
>> +	return sca3300_write_reg(sca_data, SCA3300_REG_MODE,
>> +				 sca_data->chip->avail_modes_table[index]);
>> +}
>> +
>> +static int sca3300_get_op_mode(struct sca3300_data *sca_data, int *index)
>> +{
>> +	int reg_val;
>> +	int ret;
>> +	int i;
>> +
>> +	ret = sca3300_read_reg(sca_data, SCA3300_REG_MODE, &reg_val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	reg_val &= GENMASK(1, 0);
>> +	for (i = 0; i < sca_data->chip->num_avail_modes; i++) {
>> +		if (sca_data->chip->avail_modes_table[i] == reg_val)
>> +			break;
>> +	}
>> +	if (i == sca_data->chip->num_avail_modes)
>> +		return -EINVAL;
>> +
>> +	*index = i;
>> +	return 0;
>> +}
>> +
>> +static int sca3300_set_frequency(struct sca3300_data *data, int val)
>> +{
>> +	const struct sca3300_chip_info *chip = data->chip;
>> +	unsigned int index;
>> +	unsigned int i;
>> +
>> +	if (sca3300_get_op_mode(data, &index))
>> +		return -EINVAL;
>> +
>> +	for (i = 0; i < chip->num_avail_modes; i++) {
>> +		if ((val == chip->freq_table[chip->freq_map[i]]) &&
> 
> The conditions being checked here are far from obvious, so I think this would benefit
> from an explanatory comment.
> 
> Something along the lines of,
> "Find a mode in which the requested sampling frequency is available
>  and the scaling currently set is retained".


In addition to a comment, how about small restructure of loop and giving
local variables that tell the purpose, something like


...

unsigned int opmode_scale, new_scale;

opmode_scale = chip->accel_scale[chip->accel_scale_map[index]];

/*
* Find a mode in which the requested sampling frequency is available
* and the scaling currently set is retained
*/
for (i = 0; i < chip->num_avail_modes; i++) {
    if (val == chip->freq_table[chip->freq_map[i]]) {
        new_scale = chip->accel_scale[chip->accel_scale_map[i]]);	
        if (opmode_scale == new_scale)
            break;
    }
}


That way it's IMHO more clear what we are comparing.

Thanks,
Tomas


> 
> 
> 
>> +		    (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;
>> +
>> +	return sca3300_set_op_mode(data, i);
>> +}
>> +
>>  static int sca3300_write_raw(struct iio_dev *indio_dev,
>>  			     struct iio_chan_spec const *chan,
>>  			     int val, int val2, long mask)
>>  {
>>  	struct sca3300_data *data = iio_priv(indio_dev);
>> -	int reg_val;
>> -	int ret;
>> +	int index;
>>  	int i;
>>  
>>  	switch (mask) {
>>  	case IIO_CHAN_INFO_SCALE:
>> -		if (val)
>> +		if (chan->type != IIO_ACCEL)
>>  			return -EINVAL;
>> -
>> -		for (i = 0; i < ARRAY_SIZE(sca3300_accel_scale); i++) {
>> -			if (val2 == sca3300_accel_scale[i][1])
>> -				return sca3300_write_reg(data, SCA3300_REG_MODE, i);
>> +		for (i = 0; i < data->chip->num_avail_modes; i++) {
>> +			index = data->chip->accel_scale_map[i];
> 
> Also a comment here that we are letting scale take priority over
> sampling frequency. That makes sense given we can only ever end up increasing
> the sampling frequency which is unlikely to be a problem.
> 
>> +			if ((val  == data->chip->accel_scale[index][0]) &&
>> +			    (val2 == data->chip->accel_scale[index][1])) {
>> +				return sca3300_set_op_mode(data, i);
>> +			}
>>  		}
>>  		return -EINVAL;
>> -
>>  	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
>> -		ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
>> -		if (ret)
>> -			return ret;
>> -		/* freq. change is possible only for mode 3 and 4 */
>> -		if (reg_val == 2 && val == sca3300_lp_freq[3])
>> -			return sca3300_write_reg(data, SCA3300_REG_MODE, 3);
>> -		if (reg_val == 3 && val == sca3300_lp_freq[2])
>> -			return sca3300_write_reg(data, SCA3300_REG_MODE, 2);
>> -		return -EINVAL;
>> +		return sca3300_set_frequency(data, val);
>>  	default:
>>  		return -EINVAL;
>>  	}
Jonathan Cameron May 22, 2022, 10:44 a.m. UTC | #3
> >> +static int sca3300_set_frequency(struct sca3300_data *data, int val)
> >> +{
> >> +	const struct sca3300_chip_info *chip = data->chip;
> >> +	unsigned int index;
> >> +	unsigned int i;
> >> +
> >> +	if (sca3300_get_op_mode(data, &index))
> >> +		return -EINVAL;
> >> +
> >> +	for (i = 0; i < chip->num_avail_modes; i++) {
> >> +		if ((val == chip->freq_table[chip->freq_map[i]]) &&  
> > 
> > The conditions being checked here are far from obvious, so I think this would benefit
> > from an explanatory comment.
> > 
> > Something along the lines of,
> > "Find a mode in which the requested sampling frequency is available
> >  and the scaling currently set is retained".  
> 
> 
> In addition to a comment, how about small restructure of loop and giving
> local variables that tell the purpose, something like
> 
> 
> ...
> 
> unsigned int opmode_scale, new_scale;
> 
> opmode_scale = chip->accel_scale[chip->accel_scale_map[index]];
> 
> /*
> * Find a mode in which the requested sampling frequency is available
> * and the scaling currently set is retained
> */
> for (i = 0; i < chip->num_avail_modes; i++) {
>     if (val == chip->freq_table[chip->freq_map[i]]) {
>         new_scale = chip->accel_scale[chip->accel_scale_map[i]]);	
>         if (opmode_scale == new_scale)
>             break;
>     }
> }
> 
> 
> That way it's IMHO more clear what we are comparing.
LGTM.

Thanks,

Jonathan

> 
> Thanks,
> Tomas
diff mbox series

Patch

diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
index ff16d2cc8c70..bc6e0213e4aa 100644
--- a/drivers/iio/accel/sca3300.c
+++ b/drivers/iio/accel/sca3300.c
@@ -93,15 +93,35 @@  static const struct iio_chan_spec sca3300_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
-static const int sca3300_lp_freq[] = {70, 70, 70, 10};
-static const int sca3300_accel_scale[][2] = {{0, 370}, {0, 741}, {0, 185}, {0, 185}};
+static const int sca3300_lp_freq[] = {70, 10};
+static const int sca3300_lp_freq_map[] = {0, 0, 0, 1};
 
+static const int sca3300_accel_scale[][2] = {{0, 370}, {0, 741}, {0, 185}};
+static const int sca3300_accel_scale_map[] = {0, 1, 2, 2};
+
+static const int sca3300_avail_modes_map[] = {0, 1, 2, 3};
 static const unsigned long sca3300_scan_masks[] = {
 	BIT(SCA3300_ACC_X) | BIT(SCA3300_ACC_Y) | BIT(SCA3300_ACC_Z) |
 	BIT(SCA3300_TEMP),
 	0
 };
 
+struct sca3300_chip_info {
+	const char *name;
+	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 int *avail_modes_table;
+	u8 num_avail_modes;
+	u8 chip_id;
+};
+
 /**
  * struct sca3300_data - device data
  * @spi: SPI device structure
@@ -117,10 +137,29 @@  struct sca3300_data {
 		s16 channels[4];
 		s64 ts __aligned(sizeof(s64));
 	} scan;
+	const struct sca3300_chip_info *chip;
 	u8 txbuf[4] ____cacheline_aligned;
 	u8 rxbuf[4];
 };
 
+static const struct sca3300_chip_info sca3300_chip_tbl[] = {
+	{
+		.name = "sca3300",
+		.scan_masks = sca3300_scan_masks,
+		.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,
+		.avail_modes_table = sca3300_avail_modes_map,
+		.num_avail_modes = 4,
+		.chip_id = SCA3300_WHOAMI_ID,
+	},
+};
+
 DECLARE_CRC8_TABLE(sca3300_crc_table);
 
 static int sca3300_transfer(struct sca3300_data *sca_data, int *val)
@@ -227,36 +266,80 @@  static int sca3300_write_reg(struct sca3300_data *sca_data, u8 reg, int val)
 	return sca3300_error_handler(sca_data);
 }
 
+static int sca3300_set_op_mode(struct sca3300_data *sca_data, int index)
+{
+	if ((index < 0) || (index >= sca_data->chip->num_avail_modes))
+		return -EINVAL;
+
+	return sca3300_write_reg(sca_data, SCA3300_REG_MODE,
+				 sca_data->chip->avail_modes_table[index]);
+}
+
+static int sca3300_get_op_mode(struct sca3300_data *sca_data, int *index)
+{
+	int reg_val;
+	int ret;
+	int i;
+
+	ret = sca3300_read_reg(sca_data, SCA3300_REG_MODE, &reg_val);
+	if (ret)
+		return ret;
+
+	reg_val &= GENMASK(1, 0);
+	for (i = 0; i < sca_data->chip->num_avail_modes; i++) {
+		if (sca_data->chip->avail_modes_table[i] == reg_val)
+			break;
+	}
+	if (i == sca_data->chip->num_avail_modes)
+		return -EINVAL;
+
+	*index = i;
+	return 0;
+}
+
+static int sca3300_set_frequency(struct sca3300_data *data, int val)
+{
+	const struct sca3300_chip_info *chip = data->chip;
+	unsigned int index;
+	unsigned int i;
+
+	if (sca3300_get_op_mode(data, &index))
+		return -EINVAL;
+
+	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;
+
+	return sca3300_set_op_mode(data, i);
+}
+
 static int sca3300_write_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
 			     int val, int val2, long mask)
 {
 	struct sca3300_data *data = iio_priv(indio_dev);
-	int reg_val;
-	int ret;
+	int index;
 	int i;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		if (val)
+		if (chan->type != IIO_ACCEL)
 			return -EINVAL;
-
-		for (i = 0; i < ARRAY_SIZE(sca3300_accel_scale); i++) {
-			if (val2 == sca3300_accel_scale[i][1])
-				return sca3300_write_reg(data, SCA3300_REG_MODE, i);
+		for (i = 0; i < data->chip->num_avail_modes; i++) {
+			index = data->chip->accel_scale_map[i];
+			if ((val  == data->chip->accel_scale[index][0]) &&
+			    (val2 == data->chip->accel_scale[index][1])) {
+				return sca3300_set_op_mode(data, i);
+			}
 		}
 		return -EINVAL;
-
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
-		ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
-		if (ret)
-			return ret;
-		/* freq. change is possible only for mode 3 and 4 */
-		if (reg_val == 2 && val == sca3300_lp_freq[3])
-			return sca3300_write_reg(data, SCA3300_REG_MODE, 3);
-		if (reg_val == 3 && val == sca3300_lp_freq[2])
-			return sca3300_write_reg(data, SCA3300_REG_MODE, 2);
-		return -EINVAL;
+		return sca3300_set_frequency(data, val);
 	default:
 		return -EINVAL;
 	}
@@ -267,8 +350,8 @@  static int sca3300_read_raw(struct iio_dev *indio_dev,
 			    int *val, int *val2, long mask)
 {
 	struct sca3300_data *data = iio_priv(indio_dev);
+	int index;
 	int ret;
-	int reg_val;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -277,17 +360,25 @@  static int sca3300_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-		ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
+		ret = sca3300_get_op_mode(data, &index);
 		if (ret)
 			return ret;
-		*val = 0;
-		*val2 = sca3300_accel_scale[reg_val][1];
-		return IIO_VAL_INT_PLUS_MICRO;
+		switch (chan->type) {
+		case IIO_ACCEL:
+			index = data->chip->accel_scale_map[index];
+			*val = data->chip->accel_scale[index][0];
+			*val2 = data->chip->accel_scale[index][1];
+			return IIO_VAL_INT_PLUS_MICRO;
+		default:
+			return -EINVAL;
+		}
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
-		ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
+
+		ret = sca3300_get_op_mode(data, &index);
 		if (ret)
 			return ret;
-		*val = sca3300_lp_freq[reg_val];
+		index = data->chip->freq_map[index];
+		*val = data->chip->freq_table[index];
 		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
@@ -331,6 +422,7 @@  static int sca3300_init(struct sca3300_data *sca_data,
 {
 	int value = 0;
 	int ret;
+	int i;
 
 	ret = sca3300_write_reg(sca_data, SCA3300_REG_MODE,
 				SCA3300_MODE_SW_RESET);
@@ -347,12 +439,17 @@  static int sca3300_init(struct sca3300_data *sca_data,
 	if (ret)
 		return ret;
 
-	if (value != SCA3300_WHOAMI_ID) {
-		dev_err(&sca_data->spi->dev,
-			"device id not expected value, %d != %u\n",
-			value, SCA3300_WHOAMI_ID);
+	for (i = 0; i < ARRAY_SIZE(sca3300_chip_tbl); i++) {
+		if (sca3300_chip_tbl[i].chip_id == value)
+			break;
+	}
+	if (i == ARRAY_SIZE(sca3300_chip_tbl)) {
+		dev_err(&sca_data->spi->dev, "unknown chip id %x\n", value);
 		return -ENODEV;
 	}
+
+	sca_data->chip = &sca3300_chip_tbl[i];
+
 	return 0;
 }
 
@@ -384,15 +481,21 @@  static int sca3300_read_avail(struct iio_dev *indio_dev,
 			      const int **vals, int *type, int *length,
 			      long mask)
 {
+	struct sca3300_data *data = iio_priv(indio_dev);
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		*vals = (const int *)sca3300_accel_scale;
-		*length = ARRAY_SIZE(sca3300_accel_scale) * 2 - 2;
-		*type = IIO_VAL_INT_PLUS_MICRO;
-		return IIO_AVAIL_LIST;
+		switch (chan->type) {
+		case IIO_ACCEL:
+			*vals = (const int *)data->chip->accel_scale;
+			*length = data->chip->num_accel_scales;
+			*type = IIO_VAL_INT_PLUS_MICRO;
+			return IIO_AVAIL_LIST;
+		default:
+			return -EINVAL;
+		}
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
-		*vals = &sca3300_lp_freq[2];
-		*length = 2;
+		*vals = (const int *)data->chip->freq_table;
+		*length = data->chip->num_freqs;
 		*type = IIO_VAL_INT;
 		return IIO_AVAIL_LIST;
 	default:
@@ -424,11 +527,6 @@  static int sca3300_probe(struct spi_device *spi)
 	crc8_populate_msb(sca3300_crc_table, SCA3300_CRC8_POLYNOMIAL);
 
 	indio_dev->info = &sca3300_info;
-	indio_dev->name = SCA3300_ALIAS;
-	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = sca3300_channels;
-	indio_dev->num_channels = ARRAY_SIZE(sca3300_channels);
-	indio_dev->available_scan_masks = sca3300_scan_masks;
 
 	ret = sca3300_init(sca_data, indio_dev);
 	if (ret) {
@@ -436,6 +534,12 @@  static int sca3300_probe(struct spi_device *spi)
 		return ret;
 	}
 
+	indio_dev->name = sca_data->chip->name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = sca_data->chip->channels;
+	indio_dev->num_channels = sca_data->chip->num_channels;
+	indio_dev->available_scan_masks = sca_data->chip->scan_masks;
+
 	ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
 					      iio_pollfunc_store_time,
 					      sca3300_trigger_handler, NULL);