diff mbox series

[V6,5/5] iio: accel: sca3300: Add inclination channels

Message ID 20220513124135.1295822-6-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 
Different from SCA3300, SCL3300 can output inclination angles.
Angles are formed from acceleration with following equations:
ANG_X = atan2(accx , sqrt(pow(accy , 2) + pow(accz , 2)))
ANG_Y = atan2(accy , sqrt(pow(accx , 2) + pow(accz , 2)))
ANG_Z = atan2(accz , sqrt(pow(accx , 2) + pow(accy , 2)))

The commit adds the output of the raw value, scale
and scale_available of angles.

New interfaces:
  in_incli_scale
  in_incli_scale_available
  in_incli_x_raw
  in_incli_y_raw
  in_incli_z_raw
Data converted by application of scale to degrees.

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

Comments

Tomas Melin May 16, 2022, 7:50 a.m. UTC | #1 
Hi Li,

A few comments below.

On 13/05/2022 15:41, LI Qingwu wrote:
> Different from SCA3300, SCL3300 can output inclination angles.
> Angles are formed from acceleration with following equations:
> ANG_X = atan2(accx , sqrt(pow(accy , 2) + pow(accz , 2)))
> ANG_Y = atan2(accy , sqrt(pow(accx , 2) + pow(accz , 2)))
> ANG_Z = atan2(accz , sqrt(pow(accx , 2) + pow(accy , 2)))
> 
> The commit adds the output of the raw value, scale
> and scale_available of angles.
> 
> New interfaces:
>   in_incli_scale
>   in_incli_scale_available
>   in_incli_x_raw
>   in_incli_y_raw
>   in_incli_z_raw
> Data converted by application of scale to degrees.
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> ---
>  drivers/iio/accel/sca3300.c | 82 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 79 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
> index 3dcb4296fad2..7060eba360ae 100644
> --- a/drivers/iio/accel/sca3300.c
> +++ b/drivers/iio/accel/sca3300.c
> @@ -44,12 +44,18 @@
>  #define SCA3300_VALUE_RS_ERROR	0x3
>  #define SCA3300_MASK_RS_STATUS	GENMASK(1, 0)
>  
> +#define SCA3300_REG_ANG_CTRL 0x0C
> +#define SCA3300_ANG_ENABLE   0x1F
Since these are not available for SCA3300 I suggest using
SCL3300 prefix here. Also note that the other groups have
a comment block in place.


> +
>  enum sca3300_scan_indexes {
>  	SCA3300_ACC_X = 0,
>  	SCA3300_ACC_Y,
>  	SCA3300_ACC_Z,
>  	SCA3300_TEMP,
>  	SCA3300_TIMESTAMP,
> +	SCA3300_INCLI_X,
> +	SCA3300_INCLI_Y,
> +	SCA3300_INCLI_Z,
>  };
>  
>  #define SCA3300_ACCEL_CHANNEL(index, reg, axis) {			\
> @@ -73,6 +79,24 @@ enum sca3300_scan_indexes {
>  	},								\
>  }
>  
> +#define SCA3300_INCLI_CHANNEL(index, reg, axis) {			\
> +	.type = IIO_INCLI,						\
> +	.address = reg,							\
> +	.modified = 1,							\
> +	.channel2 = IIO_MOD_##axis,					\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.info_mask_shared_by_type_available =				\
> +	BIT(IIO_CHAN_INFO_SCALE),					\
> +	.scan_index = index,						\
> +	.scan_type = {							\
> +		.sign = 's',						\
> +		.realbits = 16,						\
> +		.storagebits = 16,					\
> +		.endianness = IIO_CPU,					\
> +	},								\
> +}
> +
>  #define SCA3300_TEMP_CHANNEL(index, reg) {				\
>  		.type = IIO_TEMP,					\
>  		.address = reg,						\
> @@ -106,15 +130,36 @@ static const int sca3300_accel_scale_map[] = {0, 1, 2, 2};
>  static const int scl3300_accel_scale[][2] = {{0, 167}, {0, 333}, {0, 83}};
>  static const int scl3300_accel_scale_map[] = {0, 1, 2};
>  
> +static const int scl3300_incli_scale[][2] = {{0, 5495}};
> +static const int scl3300_incli_scale_map[] = {0, 0, 0};
> +
>  static const int sca3300_avail_modes_map[] = {0, 1, 2, 3};
>  static const int scl3300_avail_modes_map[] = {0, 1, 3};
>  
> +static const struct iio_chan_spec scl3300_channels[] = {
> +	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_X, 0x1, X),
> +	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Y, 0x2, Y),
> +	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Z, 0x3, Z),
> +	SCA3300_TEMP_CHANNEL(SCA3300_TEMP, 0x05),
> +	IIO_CHAN_SOFT_TIMESTAMP(SCA3300_TIMESTAMP),

This happens to match with 0x4 in enum, but would be better to instead
use direct reference 0x4 here to not mix enum with channel ref?

> +	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_X, 0x09, X),
> +	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Y, 0x0A, Y),
> +	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Z, 0x0B, Z),
> +};
> +
>  static const unsigned long sca3300_scan_masks[] = {
>  	BIT(SCA3300_ACC_X) | BIT(SCA3300_ACC_Y) | BIT(SCA3300_ACC_Z) |
>  	BIT(SCA3300_TEMP),
>  	0
>  };
>  
> +static const unsigned long scl3300_scan_masks[] = {
> +	BIT(SCA3300_ACC_X) | BIT(SCA3300_ACC_Y) | BIT(SCA3300_ACC_Z) |
> +	BIT(SCA3300_TEMP) |
> +	BIT(SCA3300_INCLI_X) | BIT(SCA3300_INCLI_Y) | BIT(SCA3300_INCLI_Z),
> +	0
> +};
> +
>  struct sca3300_chip_info {
>  	const char *name;
>  	const unsigned long *scan_masks;
> @@ -123,14 +168,19 @@ struct sca3300_chip_info {
>  	u8 num_accel_scales;
>  	const int (*accel_scale)[2];
>  	const int *accel_scale_map;
> +	const int (*incli_scale)[2];
> +	const int *incli_scale_map;
> +	u8 num_incli_scales;
>  	u8 num_freqs;
>  	const int *freq_table;
>  	const int *freq_map;
>  	const int *avail_modes_table;
>  	u8 num_avail_modes;
>  	u8 chip_id;
> +	bool angle;
Suggest removing this variable, see below.
>  };
>  
> +
>  /**
>   * struct sca3300_data - device data
>   * @spi: SPI device structure
> @@ -160,19 +210,27 @@ static const struct sca3300_chip_info sca3300_chip_tbl[] = {
>  		.num_accel_scales = ARRAY_SIZE(sca3300_accel_scale)*2,
>  		.accel_scale = sca3300_accel_scale,
>  		.accel_scale_map = sca3300_accel_scale_map,
> +		.incli_scale = NULL,
> +		.incli_scale_map = NULL,
> +		.num_incli_scales = 0,
I suppose these are redundant lines, can be removed?

>  		.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,
> +		.angle = false,
>  	},
> -	{	.scan_masks = sca3300_scan_masks,
> -		.channels = sca3300_channels,
> -		.num_channels = ARRAY_SIZE(sca3300_channels),
> +	{
> +		.scan_masks = scl3300_scan_masks,
> +		.channels = scl3300_channels,
> +		.num_channels = ARRAY_SIZE(scl3300_channels),
>  		.num_accel_scales = ARRAY_SIZE(scl3300_accel_scale)*2,
>  		.accel_scale = scl3300_accel_scale,
>  		.accel_scale_map = scl3300_accel_scale_map,
> +		.incli_scale = scl3300_incli_scale,
> +		.incli_scale_map = scl3300_incli_scale_map,
> +		.num_incli_scales =  ARRAY_SIZE(scl3300_incli_scale)*2,
>  		.num_freqs = ARRAY_SIZE(scl3300_lp_freq),
>  		.freq_table = scl3300_lp_freq,
>  		.freq_map = scl3300_lp_freq_map,
> @@ -180,6 +238,7 @@ static const struct sca3300_chip_info sca3300_chip_tbl[] = {
>  		.avail_modes_table = scl3300_avail_modes_map,
>  		.num_avail_modes = 3,
>  		.chip_id = SCL3300_WHOAMI_ID,
> +		.angle = true,
>  	},
>  };
>  
> @@ -387,6 +446,11 @@ static int sca3300_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  		switch (chan->type) {
> +		case IIO_INCLI:
> +			index = data->chip->incli_scale_map[index];
> +			*val = data->chip->incli_scale[index][0];
> +			*val2 = data->chip->incli_scale[index][1];
> +			return IIO_VAL_INT_PLUS_MICRO;
>  		case IIO_ACCEL:
>  			index = data->chip->accel_scale_map[index];
>  			*val = data->chip->accel_scale[index][0];

Please check function comment for sca3300_init, it has references to
data sheet which should now be updated, too.

Check also startup delay after sw reset. For scl3300 is seems to be 25ms
instead of 15ms for sca3300, so that could also be updated to be 26ms in
order to safely support both chips and be in line with data sheet.

> @@ -473,6 +537,13 @@ static int sca3300_init(struct sca3300_data *sca_data,
>  
>  	sca_data->chip = &sca3300_chip_tbl[i];
>  
> +	if (sca_data->chip->angle) {
Instead of resorting to additional variable, can the difference scl/sca
be handled here like?
    if (value == SCL3300_WHOAMI_ID)

(assuming there are not many more chip variants to be handled here, this
would keep it somewhat more simple)

Thanks,
Tomas



> +		ret = sca3300_write_reg(sca_data, SCA3300_REG_ANG_CTRL,
> +					SCA3300_ANG_ENABLE);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -508,6 +579,11 @@ static int sca3300_read_avail(struct iio_dev *indio_dev,
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SCALE:
>  		switch (chan->type) {
> +		case IIO_INCLI:
> +			*vals = (const int *)data->chip->incli_scale;
> +			*length = data->chip->num_incli_scales;
> +			*type = IIO_VAL_INT_PLUS_MICRO;
> +			return IIO_AVAIL_LIST;
>  		case IIO_ACCEL:
>  			*vals = (const int *)data->chip->accel_scale;
>  			*length = data->chip->num_accel_scales;
diff mbox series

Patch

diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
index 3dcb4296fad2..7060eba360ae 100644
--- a/drivers/iio/accel/sca3300.c
+++ b/drivers/iio/accel/sca3300.c
@@ -44,12 +44,18 @@ 
 #define SCA3300_VALUE_RS_ERROR	0x3
 #define SCA3300_MASK_RS_STATUS	GENMASK(1, 0)
 
+#define SCA3300_REG_ANG_CTRL 0x0C
+#define SCA3300_ANG_ENABLE   0x1F
+
 enum sca3300_scan_indexes {
 	SCA3300_ACC_X = 0,
 	SCA3300_ACC_Y,
 	SCA3300_ACC_Z,
 	SCA3300_TEMP,
 	SCA3300_TIMESTAMP,
+	SCA3300_INCLI_X,
+	SCA3300_INCLI_Y,
+	SCA3300_INCLI_Z,
 };
 
 #define SCA3300_ACCEL_CHANNEL(index, reg, axis) {			\
@@ -73,6 +79,24 @@  enum sca3300_scan_indexes {
 	},								\
 }
 
+#define SCA3300_INCLI_CHANNEL(index, reg, axis) {			\
+	.type = IIO_INCLI,						\
+	.address = reg,							\
+	.modified = 1,							\
+	.channel2 = IIO_MOD_##axis,					\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type_available =				\
+	BIT(IIO_CHAN_INFO_SCALE),					\
+	.scan_index = index,						\
+	.scan_type = {							\
+		.sign = 's',						\
+		.realbits = 16,						\
+		.storagebits = 16,					\
+		.endianness = IIO_CPU,					\
+	},								\
+}
+
 #define SCA3300_TEMP_CHANNEL(index, reg) {				\
 		.type = IIO_TEMP,					\
 		.address = reg,						\
@@ -106,15 +130,36 @@  static const int sca3300_accel_scale_map[] = {0, 1, 2, 2};
 static const int scl3300_accel_scale[][2] = {{0, 167}, {0, 333}, {0, 83}};
 static const int scl3300_accel_scale_map[] = {0, 1, 2};
 
+static const int scl3300_incli_scale[][2] = {{0, 5495}};
+static const int scl3300_incli_scale_map[] = {0, 0, 0};
+
 static const int sca3300_avail_modes_map[] = {0, 1, 2, 3};
 static const int scl3300_avail_modes_map[] = {0, 1, 3};
 
+static const struct iio_chan_spec scl3300_channels[] = {
+	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_X, 0x1, X),
+	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Y, 0x2, Y),
+	SCA3300_ACCEL_CHANNEL(SCA3300_ACC_Z, 0x3, Z),
+	SCA3300_TEMP_CHANNEL(SCA3300_TEMP, 0x05),
+	IIO_CHAN_SOFT_TIMESTAMP(SCA3300_TIMESTAMP),
+	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_X, 0x09, X),
+	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Y, 0x0A, Y),
+	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Z, 0x0B, Z),
+};
+
 static const unsigned long sca3300_scan_masks[] = {
 	BIT(SCA3300_ACC_X) | BIT(SCA3300_ACC_Y) | BIT(SCA3300_ACC_Z) |
 	BIT(SCA3300_TEMP),
 	0
 };
 
+static const unsigned long scl3300_scan_masks[] = {
+	BIT(SCA3300_ACC_X) | BIT(SCA3300_ACC_Y) | BIT(SCA3300_ACC_Z) |
+	BIT(SCA3300_TEMP) |
+	BIT(SCA3300_INCLI_X) | BIT(SCA3300_INCLI_Y) | BIT(SCA3300_INCLI_Z),
+	0
+};
+
 struct sca3300_chip_info {
 	const char *name;
 	const unsigned long *scan_masks;
@@ -123,14 +168,19 @@  struct sca3300_chip_info {
 	u8 num_accel_scales;
 	const int (*accel_scale)[2];
 	const int *accel_scale_map;
+	const int (*incli_scale)[2];
+	const int *incli_scale_map;
+	u8 num_incli_scales;
 	u8 num_freqs;
 	const int *freq_table;
 	const int *freq_map;
 	const int *avail_modes_table;
 	u8 num_avail_modes;
 	u8 chip_id;
+	bool angle;
 };
 
+
 /**
  * struct sca3300_data - device data
  * @spi: SPI device structure
@@ -160,19 +210,27 @@  static const struct sca3300_chip_info sca3300_chip_tbl[] = {
 		.num_accel_scales = ARRAY_SIZE(sca3300_accel_scale)*2,
 		.accel_scale = sca3300_accel_scale,
 		.accel_scale_map = sca3300_accel_scale_map,
+		.incli_scale = NULL,
+		.incli_scale_map = NULL,
+		.num_incli_scales = 0,
 		.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,
+		.angle = false,
 	},
-	{	.scan_masks = sca3300_scan_masks,
-		.channels = sca3300_channels,
-		.num_channels = ARRAY_SIZE(sca3300_channels),
+	{
+		.scan_masks = scl3300_scan_masks,
+		.channels = scl3300_channels,
+		.num_channels = ARRAY_SIZE(scl3300_channels),
 		.num_accel_scales = ARRAY_SIZE(scl3300_accel_scale)*2,
 		.accel_scale = scl3300_accel_scale,
 		.accel_scale_map = scl3300_accel_scale_map,
+		.incli_scale = scl3300_incli_scale,
+		.incli_scale_map = scl3300_incli_scale_map,
+		.num_incli_scales =  ARRAY_SIZE(scl3300_incli_scale)*2,
 		.num_freqs = ARRAY_SIZE(scl3300_lp_freq),
 		.freq_table = scl3300_lp_freq,
 		.freq_map = scl3300_lp_freq_map,
@@ -180,6 +238,7 @@  static const struct sca3300_chip_info sca3300_chip_tbl[] = {
 		.avail_modes_table = scl3300_avail_modes_map,
 		.num_avail_modes = 3,
 		.chip_id = SCL3300_WHOAMI_ID,
+		.angle = true,
 	},
 };
 
@@ -387,6 +446,11 @@  static int sca3300_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 		switch (chan->type) {
+		case IIO_INCLI:
+			index = data->chip->incli_scale_map[index];
+			*val = data->chip->incli_scale[index][0];
+			*val2 = data->chip->incli_scale[index][1];
+			return IIO_VAL_INT_PLUS_MICRO;
 		case IIO_ACCEL:
 			index = data->chip->accel_scale_map[index];
 			*val = data->chip->accel_scale[index][0];
@@ -473,6 +537,13 @@  static int sca3300_init(struct sca3300_data *sca_data,
 
 	sca_data->chip = &sca3300_chip_tbl[i];
 
+	if (sca_data->chip->angle) {
+		ret = sca3300_write_reg(sca_data, SCA3300_REG_ANG_CTRL,
+					SCA3300_ANG_ENABLE);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -508,6 +579,11 @@  static int sca3300_read_avail(struct iio_dev *indio_dev,
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
 		switch (chan->type) {
+		case IIO_INCLI:
+			*vals = (const int *)data->chip->incli_scale;
+			*length = data->chip->num_incli_scales;
+			*type = IIO_VAL_INT_PLUS_MICRO;
+			return IIO_AVAIL_LIST;
 		case IIO_ACCEL:
 			*vals = (const int *)data->chip->accel_scale;
 			*length = data->chip->num_accel_scales;