All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300
@ 2022-01-24  9:39 LI Qingwu
  2022-01-24  9:39 ` [PATCH V1 1/6] iio: accel: sca3300: add define for temp channel for reuse LI Qingwu
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: LI Qingwu @ 2022-01-24  9:39 UTC (permalink / raw)
  To: qing-wu.li, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: LI Qingwu

The current driver support sca3300 only.
Modifed for support SCL3300.
Verifed with SCL3300 on IMX8MM.
Splited the change for review.

LI Qingwu (6):
  iio: accel: sca3300: add define for temp channel for reuse.
  iio: accel: sca3300: Add interface for operation modes.
  iio: accel: sca3300: modified to support multi chips
  iio: accel: sca3300: Add support for SCL3300
  iio: accel: sca3300: Add inclination channels.
  dt-bindings: iio: accel: sca3300: Document murata,scl3300

 .../bindings/iio/accel/murata,sca3300.yaml    |   1 +
 drivers/iio/accel/sca3300.c                   | 284 +++++++++++++++---
 2 files changed, 238 insertions(+), 47 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH V1 1/6] iio: accel: sca3300: add define for temp channel for reuse.
  2022-01-24  9:39 [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 LI Qingwu
@ 2022-01-24  9:39 ` LI Qingwu
  2022-01-24  9:39 ` [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes LI Qingwu
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 25+ messages in thread
From: LI Qingwu @ 2022-01-24  9:39 UTC (permalink / raw)
  To: qing-wu.li, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: LI Qingwu

Add define of SCA3300_TEMP_CHANNEL for reuse.

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

diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
index f7ef8ecfd34a..083ae2a47ad9 100644
--- a/drivers/iio/accel/sca3300.c
+++ b/drivers/iio/accel/sca3300.c
@@ -72,23 +72,25 @@ enum sca3300_scan_indexes {
 	},								\
 }
 
+#define SCA3300_TEMP_CHANNEL(index, reg) {				\
+		.type = IIO_TEMP,					\
+		.address = reg,						\
+		.scan_index = index,					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+		.scan_type = {						\
+			.sign = 's',					\
+			.realbits = 16,					\
+			.storagebits = 16,				\
+			.endianness = IIO_CPU,				\
+		},							\
+}
+
 static const struct iio_chan_spec sca3300_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),
-	{
-		.type = IIO_TEMP,
-		.address = 0x5,
-		.scan_index = SCA3300_TEMP,
-		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
-		.scan_type = {
-			.sign = 's',
-			.realbits = 16,
-			.storagebits = 16,
-			.endianness = IIO_CPU,
-		},
-	},
-	IIO_CHAN_SOFT_TIMESTAMP(4),
+	SCA3300_TEMP_CHANNEL(SCA3300_TEMP, 0x05),
+	IIO_CHAN_SOFT_TIMESTAMP(4)
 };
 
 static const int sca3300_lp_freq[] = {70, 70, 70, 10};
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes.
  2022-01-24  9:39 [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 LI Qingwu
  2022-01-24  9:39 ` [PATCH V1 1/6] iio: accel: sca3300: add define for temp channel for reuse LI Qingwu
@ 2022-01-24  9:39 ` LI Qingwu
  2022-01-24 13:03   ` Andy Shevchenko
                     ` (3 more replies)
  2022-01-24  9:39 ` [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips LI Qingwu
                   ` (4 subsequent siblings)
  6 siblings, 4 replies; 25+ messages in thread
From: LI Qingwu @ 2022-01-24  9:39 UTC (permalink / raw)
  To: qing-wu.li, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: LI Qingwu

The acceleration scale and the frequency were set via operation modes,
the scal and frequency are both non-uniqueness,
this leads to logic confusion for setting scale.and.frequency.
it getting worse if add more different sensor types into the driver.

The commit add an interface for set and get the operation modes.
the following interfaces added:
in_accel_op_mode_available
in_op_mode

SCA3300 operation modes table:
| Mode | Full-scale | low pass filter frequency |
| ---- | ---------- | ------------------------- |
| 1    | ± 3 g      | 70 Hz                     |
| 2    | ± 6 g      | 70 Hz                     |
| 3    | ± 1.5 g    | 70 Hz                     |
| 4    | ± 1.5 g    | 10 Hz                     |

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

diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
index 083ae2a47ad9..e26b3175b3c6 100644
--- a/drivers/iio/accel/sca3300.c
+++ b/drivers/iio/accel/sca3300.c
@@ -42,6 +42,38 @@
 /* Device return status and mask */
 #define SCA3300_VALUE_RS_ERROR	0x3
 #define SCA3300_MASK_RS_STATUS	GENMASK(1, 0)
+enum sca3300_op_mode_indexes {
+	OP_MOD_1 = 0,
+	OP_MOD_2,
+	OP_MOD_3,
+	OP_MOD_4,
+	OP_MOD_CNT
+};
+
+static const char * const sca3300_op_modes[] = {
+	[OP_MOD_1] = "1",
+	[OP_MOD_2] = "2",
+	[OP_MOD_3] = "3",
+	[OP_MOD_4] = "4"
+};
+
+static int sca3300_get_op_mode(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan);
+static int sca3300_set_op_mode(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, unsigned int mode);
+
+static const struct iio_enum sca3300_op_mode_enum = {
+	.items = sca3300_op_modes,
+	.num_items = ARRAY_SIZE(sca3300_op_modes),
+	.get = sca3300_get_op_mode,
+	.set = sca3300_set_op_mode,
+};
+
+static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
+	IIO_ENUM("op_mode", IIO_SHARED_BY_DIR, &sca3300_op_mode_enum),
+	IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
+	{ }
+};
 
 enum sca3300_scan_indexes {
 	SCA3300_ACC_X = 0,
@@ -70,6 +102,7 @@ enum sca3300_scan_indexes {
 		.storagebits = 16,					\
 		.endianness = IIO_CPU,					\
 	},								\
+	.ext_info = sca3300_ext_info,					\
 }
 
 #define SCA3300_TEMP_CHANNEL(index, reg) {				\
@@ -400,6 +433,28 @@ static int sca3300_read_avail(struct iio_dev *indio_dev,
 	}
 }
 
+static int sca3300_get_op_mode(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan)
+{
+	int mode;
+	int ret;
+	struct sca3300_data *data = iio_priv(indio_dev);
+
+	ret = sca3300_read_reg(data, SCA3300_REG_MODE, &mode);
+	if (ret)
+		return ret;
+	return mode;
+
+}
+
+static int sca3300_set_op_mode(struct iio_dev *indio_dev,
+		const struct iio_chan_spec *chan, unsigned int mode)
+{
+	struct sca3300_data *data = iio_priv(indio_dev);
+
+	return sca3300_write_reg(data, SCA3300_REG_MODE, mode);
+}
+
 static const struct iio_info sca3300_info = {
 	.read_raw = sca3300_read_raw,
 	.write_raw = sca3300_write_raw,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips
  2022-01-24  9:39 [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 LI Qingwu
  2022-01-24  9:39 ` [PATCH V1 1/6] iio: accel: sca3300: add define for temp channel for reuse LI Qingwu
  2022-01-24  9:39 ` [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes LI Qingwu
@ 2022-01-24  9:39 ` LI Qingwu
  2022-01-24 13:08   ` Andy Shevchenko
                     ` (2 more replies)
  2022-01-24  9:39 ` [PATCH V1 4/6] iio: accel: sca3300: Add support for SCL3300 LI Qingwu
                   ` (3 subsequent siblings)
  6 siblings, 3 replies; 25+ messages in thread
From: LI Qingwu @ 2022-01-24  9:39 UTC (permalink / raw)
  To: qing-wu.li, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: LI Qingwu

The drive support sca3300 only,there are some other similar chips,
for instance, SCL3300.
modified the driver to read the device id,
add the tables for the corresponding id to support multiple chips.

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

diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
index e26b3175b3c6..26e6fab88a8f 100644
--- a/drivers/iio/accel/sca3300.c
+++ b/drivers/iio/accel/sca3300.c
@@ -37,11 +37,11 @@
 
 /* Device ID */
 #define SCA3300_REG_WHOAMI	0x10
-#define SCA3300_WHOAMI_ID	0x51
 
 /* Device return status and mask */
 #define SCA3300_VALUE_RS_ERROR	0x3
 #define SCA3300_MASK_RS_STATUS	GENMASK(1, 0)
+
 enum sca3300_op_mode_indexes {
 	OP_MOD_1 = 0,
 	OP_MOD_2,
@@ -75,6 +75,11 @@ static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
 	{ }
 };
 
+enum sca3300_chip_type {
+	CHIP_SCA3300 = 0,
+	CHIP_CNT
+};
+
 enum sca3300_scan_indexes {
 	SCA3300_ACC_X = 0,
 	SCA3300_ACC_Y,
@@ -126,8 +131,13 @@ 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[CHIP_CNT][OP_MOD_CNT] = {
+	[CHIP_SCA3300] = {70, 70, 70, 10},
+};
+
+static const int sca3300_accel_scale[CHIP_CNT][OP_MOD_CNT][2] = {
+	[CHIP_SCA3300] = {{0, 370}, {0, 741}, {0, 185}, {0, 185}},
+};
 
 static const unsigned long sca3300_scan_masks[] = {
 	BIT(SCA3300_ACC_X) | BIT(SCA3300_ACC_Y) | BIT(SCA3300_ACC_Z) |
@@ -135,6 +145,15 @@ static const unsigned long sca3300_scan_masks[] = {
 	0
 };
 
+struct sca3300_chip_info {
+	enum sca3300_chip_type chip_type;
+	const char *name;
+	u8 chip_id;
+	const struct iio_chan_spec *channels;
+	int num_channels;
+	unsigned long scan_masks;
+};
+
 /**
  * struct sca3300_data - device data
  * @spi: SPI device structure
@@ -150,8 +169,21 @@ struct sca3300_data {
 		s16 channels[4];
 		s64 ts __aligned(sizeof(s64));
 	} scan;
+	const struct sca3300_chip_info *chip_info;
 	u8 txbuf[4] ____cacheline_aligned;
 	u8 rxbuf[4];
+
+};
+
+static const struct sca3300_chip_info sca3300_chip_info_tbl[] = {
+	[CHIP_SCA3300] = {
+		.chip_type = CHIP_SCA3300,
+		.name = "sca3300",
+		.chip_id = 0x51,
+		.channels = sca3300_channels,
+		.num_channels = ARRAY_SIZE(sca3300_channels),
+		.scan_masks = sca3300_scan_masks,
+	},
 };
 
 DECLARE_CRC8_TABLE(sca3300_crc_table);
@@ -271,23 +303,20 @@ static int sca3300_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		if (val)
-			return -EINVAL;
-
-		for (i = 0; i < ARRAY_SIZE(sca3300_accel_scale); i++) {
-			if (val2 == sca3300_accel_scale[i][1])
+		for (i = 0; i < OP_MOD_CNT; i++) {
+			if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
+			    (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
 				return sca3300_write_reg(data, SCA3300_REG_MODE, 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])
+		if (reg_val == 2 && val == sca3300_lp_freq[data->chip_info->chip_type][3])
 			return sca3300_write_reg(data, SCA3300_REG_MODE, 3);
-		if (reg_val == 3 && val == sca3300_lp_freq[2])
+		if (reg_val == 3 && val == sca3300_lp_freq[data->chip_info->chip_type][2])
 			return sca3300_write_reg(data, SCA3300_REG_MODE, 2);
 		return -EINVAL;
 	default:
@@ -313,14 +342,18 @@ static int sca3300_read_raw(struct iio_dev *indio_dev,
 		ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
 		if (ret)
 			return ret;
-		*val = 0;
-		*val2 = sca3300_accel_scale[reg_val][1];
+		if (chan->type == IIO_ACCEL) {
+			*val = sca3300_accel_scale[data->chip_info->chip_type][reg_val][0];
+			*val2 = sca3300_accel_scale[data->chip_info->chip_type][reg_val][1];
+		} else {
+			return -EINVAL;
+		}
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
 		ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
 		if (ret)
 			return ret;
-		*val = sca3300_lp_freq[reg_val];
+		*val = sca3300_lp_freq[data->chip_info->chip_type][reg_val];
 		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
@@ -364,6 +397,7 @@ static int sca3300_init(struct sca3300_data *sca_data,
 {
 	int value = 0;
 	int ret;
+	int i = 0;
 
 	ret = sca3300_write_reg(sca_data, SCA3300_REG_MODE,
 				SCA3300_MODE_SW_RESET);
@@ -375,15 +409,22 @@ static int sca3300_init(struct sca3300_data *sca_data,
 	 * Wait 15ms for settling of signal paths.
 	 */
 	usleep_range(16e3, 50e3);
-
-	ret = sca3300_read_reg(sca_data, SCA3300_REG_WHOAMI, &value);
-	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_info_tbl); i++) {
+		ret = sca3300_read_reg(sca_data, SCA3300_REG_WHOAMI, &value);
+		if (ret)
+			return ret;
+		if (sca3300_chip_info_tbl[i].chip_id == value) {
+			sca_data->chip_info = &sca3300_chip_info_tbl[i];
+			indio_dev->name = sca3300_chip_info_tbl[i].name;
+			indio_dev->channels = sca3300_chip_info_tbl[i].channels;
+			indio_dev->num_channels = sca3300_chip_info_tbl[i].num_channels;
+			indio_dev->modes = INDIO_DIRECT_MODE;
+			indio_dev->available_scan_masks = sca3300_chip_info_tbl[i].scan_masks;
+			break;
+		}
+	}
+	if (i == ARRAY_SIZE(sca3300_chip_info_tbl)) {
+		dev_err(&sca_data->spi->dev, "Invalid chip %x\n", value);
 		return -ENODEV;
 	}
 	return 0;
@@ -417,15 +458,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;
+		if (chan->type == IIO_ACCEL) {
+			*vals = (const int *)sca3300_accel_scale[data->chip_info->chip_type];
+			*length = ARRAY_SIZE(sca3300_accel_scale[data->chip_info->chip_type]) * 2;
+		} else {
+			return -EINVAL;
+		}
 		*type = IIO_VAL_INT_PLUS_MICRO;
 		return IIO_AVAIL_LIST;
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
-		*vals = &sca3300_lp_freq[2];
-		*length = 2;
+		*vals = (const int *)sca3300_lp_freq[data->chip_info->chip_type];
+		*length = ARRAY_SIZE(sca3300_lp_freq[data->chip_info->chip_type]);
 		*type = IIO_VAL_INT;
 		return IIO_AVAIL_LIST;
 	default:
@@ -471,7 +518,6 @@ static int sca3300_probe(struct spi_device *spi)
 	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*sca_data));
 	if (!indio_dev)
 		return -ENOMEM;
-
 	sca_data = iio_priv(indio_dev);
 	mutex_init(&sca_data->lock);
 	sca_data->spi = spi;
@@ -479,11 +525,7 @@ 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) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH V1 4/6] iio: accel: sca3300: Add support for SCL3300
  2022-01-24  9:39 [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 LI Qingwu
                   ` (2 preceding siblings ...)
  2022-01-24  9:39 ` [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips LI Qingwu
@ 2022-01-24  9:39 ` LI Qingwu
  2022-01-24 13:12   ` Andy Shevchenko
  2022-01-24  9:39 ` [PATCH V1 5/6] iio: accel: sca3300: Add inclination channels LI Qingwu
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: LI Qingwu @ 2022-01-24  9:39 UTC (permalink / raw)
  To: qing-wu.li, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: LI Qingwu

Add support for Murata SCL3300, a 3-axis MEMS inclination.
same as SCA3300, it has accel and temperature output.

Datasheet link:
www.murata.com/en-us/products/sensor/inclinometer/overview/lineup/scl3300

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

diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
index 26e6fab88a8f..7ea3e202d474 100644
--- a/drivers/iio/accel/sca3300.c
+++ b/drivers/iio/accel/sca3300.c
@@ -68,7 +68,6 @@ static const struct iio_enum sca3300_op_mode_enum = {
 	.get = sca3300_get_op_mode,
 	.set = sca3300_set_op_mode,
 };
-
 static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
 	IIO_ENUM("op_mode", IIO_SHARED_BY_DIR, &sca3300_op_mode_enum),
 	IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
@@ -77,6 +76,7 @@ static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
 
 enum sca3300_chip_type {
 	CHIP_SCA3300 = 0,
+	CHIP_SCL3300,
 	CHIP_CNT
 };
 
@@ -131,12 +131,23 @@ static const struct iio_chan_spec sca3300_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(4)
 };
 
+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(4),
+};
+
+
 static const int sca3300_lp_freq[CHIP_CNT][OP_MOD_CNT] = {
 	[CHIP_SCA3300] = {70, 70, 70, 10},
+	[CHIP_SCL3300] = {40, 70, 10, 10},
 };
 
 static const int sca3300_accel_scale[CHIP_CNT][OP_MOD_CNT][2] = {
 	[CHIP_SCA3300] = {{0, 370}, {0, 741}, {0, 185}, {0, 185}},
+	[CHIP_SCL3300] = {{0, 167}, {0, 333}, {0, 83}, {0, 83}}
 };
 
 static const unsigned long sca3300_scan_masks[] = {
@@ -184,6 +195,14 @@ static const struct sca3300_chip_info sca3300_chip_info_tbl[] = {
 		.num_channels = ARRAY_SIZE(sca3300_channels),
 		.scan_masks = sca3300_scan_masks,
 	},
+	[CHIP_SCL3300] = {
+		.chip_type = CHIP_SCL3300,
+		.name = "scl3300",
+		.chip_id = 0xC1,
+		.channels = scl3300_channels,
+		.num_channels = ARRAY_SIZE(scl3300_channels),
+		.scan_masks = sca3300_scan_masks,
+	},
 };
 
 DECLARE_CRC8_TABLE(sca3300_crc_table);
@@ -310,10 +329,14 @@ static int sca3300_write_raw(struct iio_dev *indio_dev,
 		}
 		return -EINVAL;
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		if (data->chip_info->chip_type == CHIP_SCL3300) {
+			/*SCL3300 freq.tied to accel scale, not allowed to set separately.*/
+			return -EINVAL;
+		}
 		ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
 		if (ret)
 			return ret;
-		/* freq. change is possible only for mode 3 and 4 */
+		/* SCA330 freq. change is possible only for mode 3 and 4 */
 		if (reg_val == 2 && val == sca3300_lp_freq[data->chip_info->chip_type][3])
 			return sca3300_write_reg(data, SCA3300_REG_MODE, 3);
 		if (reg_val == 3 && val == sca3300_lp_freq[data->chip_info->chip_type][2])
@@ -459,7 +482,6 @@ static int sca3300_read_avail(struct iio_dev *indio_dev,
 			      long mask)
 {
 	struct sca3300_data *data = iio_priv(indio_dev);
-
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
 		if (chan->type == IIO_ACCEL) {
@@ -553,6 +575,7 @@ static int sca3300_probe(struct spi_device *spi)
 
 static const struct of_device_id sca3300_dt_ids[] = {
 	{ .compatible = "murata,sca3300"},
+	{ .compatible = "murata,scl3300"},
 	{}
 };
 MODULE_DEVICE_TABLE(of, sca3300_dt_ids);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH V1 5/6] iio: accel: sca3300: Add inclination channels.
  2022-01-24  9:39 [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 LI Qingwu
                   ` (3 preceding siblings ...)
  2022-01-24  9:39 ` [PATCH V1 4/6] iio: accel: sca3300: Add support for SCL3300 LI Qingwu
@ 2022-01-24  9:39 ` LI Qingwu
  2022-01-24 13:19   ` Andy Shevchenko
  2022-01-24  9:39 ` [PATCH V1 6/6] dt-bindings: iio: accel: sca3300: Document murata,scl3300 LI Qingwu
  2022-01-30 11:35 ` [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 Jonathan Cameron
  6 siblings, 1 reply; 25+ messages in thread
From: LI Qingwu @ 2022-01-24  9:39 UTC (permalink / raw)
  To: qing-wu.li, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: LI Qingwu

Different with SCA3300, SCL3300 can output inclination angles.
Angles are formed from acceleration with following equations:
ANG_X = atan2(accx / √(accy^2 + accz^2)),
ANG_Y = atan2(accy / √(accx^2 + accz^2)),
ANG_Z = atan2(accz / √(accx^2 + accy^2)),

The commit add output of the raw value,scale
and scale_available of angles.
add interface for enable/disable of the angle output.

new interfaces:
in_incli_en
in_incli_scale
in_incli_scale_available
in_incli_x_raw
in_incli_y_raw
in_incli_z_raw

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

diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
index 7ea3e202d474..535d14b58acf 100644
--- a/drivers/iio/accel/sca3300.c
+++ b/drivers/iio/accel/sca3300.c
@@ -42,6 +42,8 @@
 #define SCA3300_VALUE_RS_ERROR	0x3
 #define SCA3300_MASK_RS_STATUS	GENMASK(1, 0)
 
+#define SCA3300_REG_ANG_CTRL 0x0C
+
 enum sca3300_op_mode_indexes {
 	OP_MOD_1 = 0,
 	OP_MOD_2,
@@ -85,6 +87,9 @@ enum sca3300_scan_indexes {
 	SCA3300_ACC_Y,
 	SCA3300_ACC_Z,
 	SCA3300_TEMP,
+	SCA3300_INCLI_X,
+	SCA3300_INCLI_Y,
+	SCA3300_INCLI_Z,
 	SCA3300_TIMESTAMP,
 };
 
@@ -110,6 +115,26 @@ enum sca3300_scan_indexes {
 	.ext_info = sca3300_ext_info,					\
 }
 
+#define SCA3300_INCLI_CHANNEL(index, reg, axis) {			\
+	.type = IIO_INCLI,						\
+	.address = reg,							\
+	.modified = 1,							\
+	.channel2 = IIO_MOD_##axis,					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type =					\
+	BIT(IIO_CHAN_INFO_SCALE) |					\
+	BIT(IIO_CHAN_INFO_ENABLE),					\
+	.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,						\
@@ -128,7 +153,7 @@ static const struct iio_chan_spec sca3300_channels[] = {
 	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(4)
+	IIO_CHAN_SOFT_TIMESTAMP(SCA3300_TIMESTAMP)
 };
 
 static const struct iio_chan_spec scl3300_channels[] = {
@@ -136,7 +161,10 @@ static const struct iio_chan_spec scl3300_channels[] = {
 	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(4),
+	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_X, 0x09, X),
+	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Y, 0x0A, Y),
+	SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Z, 0x0B, Z),
+	IIO_CHAN_SOFT_TIMESTAMP(SCA3300_TIMESTAMP)
 };
 
 
@@ -150,12 +178,24 @@ static const int sca3300_accel_scale[CHIP_CNT][OP_MOD_CNT][2] = {
 	[CHIP_SCL3300] = {{0, 167}, {0, 333}, {0, 83}, {0, 83}}
 };
 
+static const int sca3300_incli_scale[CHIP_CNT][OP_MOD_CNT][2] = {
+	[CHIP_SCA3300] = {{0, 0}, {0, 0}, {0, 0}, {0, 0}},
+	[CHIP_SCL3300] = {{0, 5495}, {0, 5495}, {0, 5495}, {0, 5495}}
+};
+
 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 {
 	enum sca3300_chip_type chip_type;
 	const char *name;
@@ -177,13 +217,12 @@ struct sca3300_data {
 	struct spi_device *spi;
 	struct mutex lock;
 	struct {
-		s16 channels[4];
+		s16 channels[SCA3300_TIMESTAMP-1];
 		s64 ts __aligned(sizeof(s64));
 	} scan;
 	const struct sca3300_chip_info *chip_info;
 	u8 txbuf[4] ____cacheline_aligned;
 	u8 rxbuf[4];
-
 };
 
 static const struct sca3300_chip_info sca3300_chip_info_tbl[] = {
@@ -201,7 +240,7 @@ static const struct sca3300_chip_info sca3300_chip_info_tbl[] = {
 		.chip_id = 0xC1,
 		.channels = scl3300_channels,
 		.num_channels = ARRAY_SIZE(scl3300_channels),
-		.scan_masks = sca3300_scan_masks,
+		.scan_masks = scl3300_scan_masks,
 	},
 };
 
@@ -322,11 +361,15 @@ static int sca3300_write_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
+		if (chan->type != IIO_ACCEL)
+			return -EINVAL;
 		for (i = 0; i < OP_MOD_CNT; i++) {
 			if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
 			    (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
 				return sca3300_write_reg(data, SCA3300_REG_MODE, i);
 		}
+		/*Inclination scale info tied to accel scale.*/
+		/*not allowed to set separately.      */
 		return -EINVAL;
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
 		if (data->chip_info->chip_type == CHIP_SCL3300) {
@@ -342,6 +385,17 @@ static int sca3300_write_raw(struct iio_dev *indio_dev,
 		if (reg_val == 3 && val == sca3300_lp_freq[data->chip_info->chip_type][2])
 			return sca3300_write_reg(data, SCA3300_REG_MODE, 2);
 		return -EINVAL;
+	case IIO_CHAN_INFO_ENABLE:
+		if (data->chip_info->chip_type == CHIP_SCL3300) {
+			if (chan->type == IIO_INCLI) {
+				if (val != 0)
+					reg_val = 0x1F;
+				else
+					reg_val = 0x00;
+				return sca3300_write_reg(data, SCA3300_REG_ANG_CTRL, reg_val);
+			}
+		}
+		return -EINVAL;
 	default:
 		return -EINVAL;
 	}
@@ -365,7 +419,11 @@ static int sca3300_read_raw(struct iio_dev *indio_dev,
 		ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
 		if (ret)
 			return ret;
-		if (chan->type == IIO_ACCEL) {
+
+		if (chan->type == IIO_INCLI) {
+			*val = sca3300_incli_scale[data->chip_info->chip_type][reg_val][0];
+			*val2 = sca3300_incli_scale[data->chip_info->chip_type][reg_val][1];
+		} else if (chan->type == IIO_ACCEL) {
 			*val = sca3300_accel_scale[data->chip_info->chip_type][reg_val][0];
 			*val2 = sca3300_accel_scale[data->chip_info->chip_type][reg_val][1];
 		} else {
@@ -378,6 +436,13 @@ static int sca3300_read_raw(struct iio_dev *indio_dev,
 			return ret;
 		*val = sca3300_lp_freq[data->chip_info->chip_type][reg_val];
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_ENABLE:
+		if (chan->type == IIO_INCLI) {
+			ret = sca3300_read_reg(data, SCA3300_REG_ANG_CTRL, &reg_val);
+			*val = reg_val;
+			return IIO_VAL_INT;
+		}
+		return -EINVAL;
 	default:
 		return -EINVAL;
 	}
@@ -484,7 +549,10 @@ static int sca3300_read_avail(struct iio_dev *indio_dev,
 	struct sca3300_data *data = iio_priv(indio_dev);
 	switch (mask) {
 	case IIO_CHAN_INFO_SCALE:
-		if (chan->type == IIO_ACCEL) {
+		if (chan->type == IIO_INCLI) {
+			*vals = (const int *)sca3300_incli_scale[data->chip_info->chip_type];
+			*length = ARRAY_SIZE(sca3300_incli_scale[data->chip_info->chip_type]) * 2;
+		} else if (chan->type == IIO_ACCEL) {
 			*vals = (const int *)sca3300_accel_scale[data->chip_info->chip_type];
 			*length = ARRAY_SIZE(sca3300_accel_scale[data->chip_info->chip_type]) * 2;
 		} else {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH V1 6/6] dt-bindings: iio: accel: sca3300: Document murata,scl3300
  2022-01-24  9:39 [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 LI Qingwu
                   ` (4 preceding siblings ...)
  2022-01-24  9:39 ` [PATCH V1 5/6] iio: accel: sca3300: Add inclination channels LI Qingwu
@ 2022-01-24  9:39 ` LI Qingwu
  2022-02-09  3:08   ` Rob Herring
  2022-01-30 11:35 ` [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 Jonathan Cameron
  6 siblings, 1 reply; 25+ messages in thread
From: LI Qingwu @ 2022-01-24  9:39 UTC (permalink / raw)
  To: qing-wu.li, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: LI Qingwu

initial DT bindings for Murata scl3300 inclinometer.

Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
---
 Documentation/devicetree/bindings/iio/accel/murata,sca3300.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/accel/murata,sca3300.yaml b/Documentation/devicetree/bindings/iio/accel/murata,sca3300.yaml
index 55fd3548e3b6..f6e2a16a710b 100644
--- a/Documentation/devicetree/bindings/iio/accel/murata,sca3300.yaml
+++ b/Documentation/devicetree/bindings/iio/accel/murata,sca3300.yaml
@@ -17,6 +17,7 @@ properties:
   compatible:
     enum:
       - murata,sca3300
+      - murata,scl3300
 
   reg:
     maxItems: 1
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes.
  2022-01-24  9:39 ` [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes LI Qingwu
@ 2022-01-24 13:03   ` Andy Shevchenko
  2022-01-24 13:49     ` kernel test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2022-01-24 13:03 UTC (permalink / raw)
  To: LI Qingwu
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Tomas Melin,
	devicetree, Linux Kernel Mailing List

On Mon, Jan 24, 2022 at 11:39 AM LI Qingwu
<Qing-wu.Li@leica-geosystems.com.cn> wrote:
>
> The acceleration scale and the frequency were set via operation modes,
> the scal and frequency are both non-uniqueness,

scale

> this leads to logic confusion for setting scale.and.frequency.
> it getting worse if add more different sensor types into the driver.

It gets
if you add

> The commit add an interface for set and get the operation modes.
> the following interfaces added:

> in_accel_op_mode_available
> in_op_mode

Please indent them by let's say 2 spaces.

> SCA3300 operation modes table:
> | Mode | Full-scale | low pass filter frequency |
> | ---- | ---------- | ------------------------- |
> | 1    | ± 3 g      | 70 Hz                     |
> | 2    | ± 6 g      | 70 Hz                     |
> | 3    | ± 1.5 g    | 70 Hz                     |
> | 4    | ± 1.5 g    | 10 Hz                     |

...

> +static const char * const sca3300_op_modes[] = {
> +       [OP_MOD_1] = "1",
> +       [OP_MOD_2] = "2",
> +       [OP_MOD_3] = "3",

> +       [OP_MOD_4] = "4"

+ Comma.

> +};

...

> +       int mode;
> +       int ret;
> +       struct sca3300_data *data = iio_priv(indio_dev);

Here and everywhere else, can we keep reversed xmas tree order?

       struct sca3300_data *data = iio_priv(indio_dev);
      int mode;
       int ret;

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips
  2022-01-24  9:39 ` [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips LI Qingwu
@ 2022-01-24 13:08   ` Andy Shevchenko
  2022-01-24 15:01     ` kernel test robot
  2022-01-24 15:32     ` kernel test robot
  2 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2022-01-24 13:08 UTC (permalink / raw)
  To: LI Qingwu
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Tomas Melin,
	devicetree, Linux Kernel Mailing List

On Mon, Jan 24, 2022 at 11:39 AM LI Qingwu
<Qing-wu.Li@leica-geosystems.com.cn> wrote:
>
> The drive support sca3300 only,there are some other similar chips,
> for instance, SCL3300.
> modified the driver to read the device id,
> add the tables for the corresponding id to support multiple chips.

...

>  /* Device return status and mask */
>  #define SCA3300_VALUE_RS_ERROR 0x3
>  #define SCA3300_MASK_RS_STATUS GENMASK(1, 0)
> +
>  enum sca3300_op_mode_indexes {
>         OP_MOD_1 = 0,
>         OP_MOD_2,

Stray change ?

...

> +struct sca3300_chip_info {
> +       enum sca3300_chip_type chip_type;
> +       const char *name;

> +       u8 chip_id;
> +       const struct iio_chan_spec *channels;
> +       int num_channels;

Ordering these as

       const struct iio_chan_spec *channels;
       int num_channels;
       u8 chip_id;

will save 4 bytes in some cases.

> +       unsigned long scan_masks;
> +};

...

>                 }
>                 return -EINVAL;
> -
>         case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
>                 ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);

Another stray change?

...

>         int value = 0;
>         int ret;
> +       int i = 0;

Reversed xmas tree order?

...

>          * Wait 15ms for settling of signal paths.
>          */
>         usleep_range(16e3, 50e3);
> -

Leave this blank line, so you will have a better reading of the code
and comment.

> +       for (i = 0; i < ARRAY_SIZE(sca3300_chip_info_tbl); i++) {
> +               ret = sca3300_read_reg(sca_data, SCA3300_REG_WHOAMI, &value);
> +               if (ret)
> +                       return ret;

> +               if (sca3300_chip_info_tbl[i].chip_id == value) {

> +                       sca_data->chip_info = &sca3300_chip_info_tbl[i];
> +                       indio_dev->name = sca3300_chip_info_tbl[i].name;
> +                       indio_dev->channels = sca3300_chip_info_tbl[i].channels;
> +                       indio_dev->num_channels = sca3300_chip_info_tbl[i].num_channels;
> +                       indio_dev->modes = INDIO_DIRECT_MODE;
> +                       indio_dev->available_scan_masks = sca3300_chip_info_tbl[i].scan_masks;

You may do it after the below check. Thus here

               if (sca3300_chip_info_tbl[i].chip_id == value)
                       break;

> +                       break;
> +               }
> +       }
> +       if (i == ARRAY_SIZE(sca3300_chip_info_tbl)) {
> +               dev_err(&sca_data->spi->dev, "Invalid chip %x\n", value);
>                 return -ENODEV;
>         }
>         return 0;

...

>         indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*sca_data));
>         if (!indio_dev)
>                 return -ENOMEM;
> -
>         sca_data = iio_priv(indio_dev);
>         mutex_init(&sca_data->lock);

Stray change for sure.

...

>         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;

> +

Ditto.

>
>         ret = sca3300_init(sca_data, indio_dev);

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 4/6] iio: accel: sca3300: Add support for SCL3300
  2022-01-24  9:39 ` [PATCH V1 4/6] iio: accel: sca3300: Add support for SCL3300 LI Qingwu
@ 2022-01-24 13:12   ` Andy Shevchenko
  0 siblings, 0 replies; 25+ messages in thread
From: Andy Shevchenko @ 2022-01-24 13:12 UTC (permalink / raw)
  To: LI Qingwu
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Tomas Melin,
	devicetree, Linux Kernel Mailing List

On Mon, Jan 24, 2022 at 11:39 AM LI Qingwu
<Qing-wu.Li@leica-geosystems.com.cn> wrote:
>
> Add support for Murata SCL3300, a 3-axis MEMS inclination.
> same as SCA3300, it has accel and temperature output.

accelerometer

> Datasheet link:
> www.murata.com/en-us/products/sensor/inclinometer/overview/lineup/scl3300
>

Make this a Datasheet: tag (change name and drop blank line(s) in the
tag block).

> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>

...

> @@ -68,7 +68,6 @@ static const struct iio_enum sca3300_op_mode_enum = {
>         .get = sca3300_get_op_mode,
>         .set = sca3300_set_op_mode,
>  };
> -

Stray change.

...

> +};
> +
> +

One blank line is enough.

...

>  static const int sca3300_accel_scale[CHIP_CNT][OP_MOD_CNT][2] = {
>         [CHIP_SCA3300] = {{0, 370}, {0, 741}, {0, 185}, {0, 185}},
> +       [CHIP_SCL3300] = {{0, 167}, {0, 333}, {0, 83}, {0, 83}}

+ Comma.

>  };

...

> +                       /*SCL3300 freq.tied to accel scale, not allowed to set separately.*/

Missed spaces.

...

>         struct sca3300_data *data = iio_priv(indio_dev);
> -

This even checkpatch should complain of, besides being a stray change.

>         switch (mask) {

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 5/6] iio: accel: sca3300: Add inclination channels.
  2022-01-24  9:39 ` [PATCH V1 5/6] iio: accel: sca3300: Add inclination channels LI Qingwu
@ 2022-01-24 13:19   ` Andy Shevchenko
  2022-01-30 11:46     ` Jonathan Cameron
  0 siblings, 1 reply; 25+ messages in thread
From: Andy Shevchenko @ 2022-01-24 13:19 UTC (permalink / raw)
  To: LI Qingwu
  Cc: Jonathan Cameron, Lars-Peter Clausen, Rob Herring, Tomas Melin,
	devicetree, Linux Kernel Mailing List

On Mon, Jan 24, 2022 at 11:39 AM LI Qingwu
<Qing-wu.Li@leica-geosystems.com.cn> wrote:
>
> Different with SCA3300, SCL3300 can output inclination angles.
> Angles are formed from acceleration with following equations:
> ANG_X = atan2(accx / √(accy^2 + accz^2)),
> ANG_Y = atan2(accy / √(accx^2 + accz^2)),
> ANG_Z = atan2(accz / √(accx^2 + accy^2)),
>
> The commit add output of the raw value,scale
> and scale_available of angles.
> add interface for enable/disable of the angle output.
>
> new interfaces:

New

> in_incli_en
> in_incli_scale
> in_incli_scale_available
> in_incli_x_raw
> in_incli_y_raw
> in_incli_z_raw

Indent them by 2 spaces.

Wondering if these need to be described in ABI documentation.

...

>         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(4)

> +       IIO_CHAN_SOFT_TIMESTAMP(SCA3300_TIMESTAMP)

+ Comma (while at it)?

...

> -       IIO_CHAN_SOFT_TIMESTAMP(4),
> +       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_X, 0x09, X),
> +       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Y, 0x0A, Y),
> +       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Z, 0x0B, Z),
> +       IIO_CHAN_SOFT_TIMESTAMP(SCA3300_TIMESTAMP)

Ditto.

> +static const int sca3300_incli_scale[CHIP_CNT][OP_MOD_CNT][2] = {
> +       [CHIP_SCA3300] = {{0, 0}, {0, 0}, {0, 0}, {0, 0}},

> +       [CHIP_SCL3300] = {{0, 5495}, {0, 5495}, {0, 5495}, {0, 5495}}

+ Comma.

> +};

...

>         struct {
> -               s16 channels[4];
> +               s16 channels[SCA3300_TIMESTAMP-1];

Missed spaces around the arithmetic operator.

>                 s64 ts __aligned(sizeof(s64));
>         } scan;
>         const struct sca3300_chip_info *chip_info;
>         u8 txbuf[4] ____cacheline_aligned;
>         u8 rxbuf[4];

> -

Stray change.

>  };

...

> +               /*Inclination scale info tied to accel scale.*/
> +               /*not allowed to set separately.      */

Please, follow the proper style for multi-line comments, including
necessary spaces, periods, starting and ending lines.

...

> +       case IIO_CHAN_INFO_ENABLE:
> +               if (data->chip_info->chip_type == CHIP_SCL3300) {

> +                       if (chan->type == IIO_INCLI) {

See below.

> +                               if (val != 0)

   if (val)

> +                                       reg_val = 0x1F;
> +                               else
> +                                       reg_val = 0x00;
> +                               return sca3300_write_reg(data, SCA3300_REG_ANG_CTRL, reg_val);
> +                       }
> +               }

...

> -               if (chan->type == IIO_ACCEL) {
> +
> +               if (chan->type == IIO_INCLI) {

> +               } else if (chan->type == IIO_ACCEL) {

I would recommend using switch-case for channel type as well.

...

> +       case IIO_CHAN_INFO_ENABLE:
> +               if (chan->type == IIO_INCLI) {

> +                       ret = sca3300_read_reg(data, SCA3300_REG_ANG_CTRL, &reg_val);

How is ret supposed to be used?

> +                       *val = reg_val;
> +                       return IIO_VAL_INT;
> +               }
> +               return -EINVAL;

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes.
  2022-01-24  9:39 ` [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes LI Qingwu
@ 2022-01-24 13:49     ` kernel test robot
  2022-01-24 13:49     ` kernel test robot
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2022-01-24 13:49 UTC (permalink / raw)
  To: LI Qingwu, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: llvm, kbuild-all, LI Qingwu

Hi LI,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on v5.17-rc1 next-20220124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: hexagon-randconfig-r033-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242108.VNQCotc3-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 2e58a18910867ba6795066e044293e6daf89edf5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1521580933c9c08143fc14b23a363eb7961d4520
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
        git checkout 1521580933c9c08143fc14b23a363eb7961d4520
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/iio/accel/sca3300.c:74:53: error: too few arguments provided to function-like macro invocation
           IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
                                                              ^
   include/linux/iio/iio.h:112:9: note: macro 'IIO_ENUM_AVAILABLE' defined here
   #define IIO_ENUM_AVAILABLE(_name, _shared, _e) \
           ^
>> drivers/iio/accel/sca3300.c:74:2: error: use of undeclared identifier 'IIO_ENUM_AVAILABLE'
           IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
           ^
   2 errors generated.


vim +74 drivers/iio/accel/sca3300.c

    71	
    72	static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
    73		IIO_ENUM("op_mode", IIO_SHARED_BY_DIR, &sca3300_op_mode_enum),
  > 74		IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
    75		{ }
    76	};
    77	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes.
@ 2022-01-24 13:49     ` kernel test robot
  0 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2022-01-24 13:49 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2681 bytes --]

Hi LI,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on v5.17-rc1 next-20220124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: hexagon-randconfig-r033-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242108.VNQCotc3-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 2e58a18910867ba6795066e044293e6daf89edf5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1521580933c9c08143fc14b23a363eb7961d4520
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
        git checkout 1521580933c9c08143fc14b23a363eb7961d4520
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/iio/accel/sca3300.c:74:53: error: too few arguments provided to function-like macro invocation
           IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
                                                              ^
   include/linux/iio/iio.h:112:9: note: macro 'IIO_ENUM_AVAILABLE' defined here
   #define IIO_ENUM_AVAILABLE(_name, _shared, _e) \
           ^
>> drivers/iio/accel/sca3300.c:74:2: error: use of undeclared identifier 'IIO_ENUM_AVAILABLE'
           IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
           ^
   2 errors generated.


vim +74 drivers/iio/accel/sca3300.c

    71	
    72	static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
    73		IIO_ENUM("op_mode", IIO_SHARED_BY_DIR, &sca3300_op_mode_enum),
  > 74		IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
    75		{ }
    76	};
    77	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes.
  2022-01-24  9:39 ` [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes LI Qingwu
@ 2022-01-24 13:59     ` kernel test robot
  2022-01-24 13:49     ` kernel test robot
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2022-01-24 13:59 UTC (permalink / raw)
  To: LI Qingwu, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: kbuild-all, LI Qingwu

Hi LI,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on v5.17-rc1 next-20220124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: parisc-randconfig-r015-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242143.uZzucUii-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1521580933c9c08143fc14b23a363eb7961d4520
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
        git checkout 1521580933c9c08143fc14b23a363eb7961d4520
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=parisc SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/iio/accel/sca3300.c:74:60: error: macro "IIO_ENUM_AVAILABLE" requires 3 arguments, but only 2 given
      74 |         IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
         |                                                            ^
   In file included from include/linux/iio/buffer.h:10,
                    from drivers/iio/accel/sca3300.c:17:
   include/linux/iio/iio.h:112: note: macro "IIO_ENUM_AVAILABLE" defined here
     112 | #define IIO_ENUM_AVAILABLE(_name, _shared, _e) \
         | 
>> drivers/iio/accel/sca3300.c:74:9: error: 'IIO_ENUM_AVAILABLE' undeclared here (not in a function)
      74 |         IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
         |         ^~~~~~~~~~~~~~~~~~


vim +/IIO_ENUM_AVAILABLE +74 drivers/iio/accel/sca3300.c

    71	
    72	static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
    73		IIO_ENUM("op_mode", IIO_SHARED_BY_DIR, &sca3300_op_mode_enum),
  > 74		IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
    75		{ }
    76	};
    77	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes.
@ 2022-01-24 13:59     ` kernel test robot
  0 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2022-01-24 13:59 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2785 bytes --]

Hi LI,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on v5.17-rc1 next-20220124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: parisc-randconfig-r015-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242143.uZzucUii-lkp(a)intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1521580933c9c08143fc14b23a363eb7961d4520
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
        git checkout 1521580933c9c08143fc14b23a363eb7961d4520
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=parisc SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/iio/accel/sca3300.c:74:60: error: macro "IIO_ENUM_AVAILABLE" requires 3 arguments, but only 2 given
      74 |         IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
         |                                                            ^
   In file included from include/linux/iio/buffer.h:10,
                    from drivers/iio/accel/sca3300.c:17:
   include/linux/iio/iio.h:112: note: macro "IIO_ENUM_AVAILABLE" defined here
     112 | #define IIO_ENUM_AVAILABLE(_name, _shared, _e) \
         | 
>> drivers/iio/accel/sca3300.c:74:9: error: 'IIO_ENUM_AVAILABLE' undeclared here (not in a function)
      74 |         IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
         |         ^~~~~~~~~~~~~~~~~~


vim +/IIO_ENUM_AVAILABLE +74 drivers/iio/accel/sca3300.c

    71	
    72	static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
    73		IIO_ENUM("op_mode", IIO_SHARED_BY_DIR, &sca3300_op_mode_enum),
  > 74		IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
    75		{ }
    76	};
    77	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips
  2022-01-24  9:39 ` [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips LI Qingwu
@ 2022-01-24 15:01     ` kernel test robot
  2022-01-24 15:01     ` kernel test robot
  2022-01-24 15:32     ` kernel test robot
  2 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2022-01-24 15:01 UTC (permalink / raw)
  To: LI Qingwu, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: llvm, kbuild-all, LI Qingwu

Hi LI,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on v5.17-rc1 next-20220124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: hexagon-randconfig-r033-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242207.0V8eUHu2-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 2e58a18910867ba6795066e044293e6daf89edf5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7dc3bc68cdfcb252dd79fea28a5e944d76784fe8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
        git checkout 7dc3bc68cdfcb252dd79fea28a5e944d76784fe8
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/iio/accel/sca3300.c:74:53: error: too few arguments provided to function-like macro invocation
           IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
                                                              ^
   include/linux/iio/iio.h:112:9: note: macro 'IIO_ENUM_AVAILABLE' defined here
   #define IIO_ENUM_AVAILABLE(_name, _shared, _e) \
           ^
   drivers/iio/accel/sca3300.c:74:2: error: use of undeclared identifier 'IIO_ENUM_AVAILABLE'
           IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
           ^
>> drivers/iio/accel/sca3300.c:185:17: warning: incompatible pointer to integer conversion initializing 'unsigned long' with an expression of type 'const unsigned long[2]' [-Wint-conversion]
                   .scan_masks = sca3300_scan_masks,
                                 ^~~~~~~~~~~~~~~~~~
>> drivers/iio/accel/sca3300.c:307:13: warning: comparison between pointer and integer ('int' and 'const int *') [-Wpointer-integer-compare]
                           if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
                                ~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/sca3300.c:308:14: warning: comparison between pointer and integer ('int' and 'const int *') [-Wpointer-integer-compare]
                               (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
                                ~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/accel/sca3300.c:422:36: warning: incompatible integer to pointer conversion assigning to 'const unsigned long *' from 'const unsigned long'; take the address with & [-Wint-conversion]
                           indio_dev->available_scan_masks = sca3300_chip_info_tbl[i].scan_masks;
                                                           ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                                             &
   4 warnings and 2 errors generated.


vim +185 drivers/iio/accel/sca3300.c

   177	
   178	static const struct sca3300_chip_info sca3300_chip_info_tbl[] = {
   179		[CHIP_SCA3300] = {
   180			.chip_type = CHIP_SCA3300,
   181			.name = "sca3300",
   182			.chip_id = 0x51,
   183			.channels = sca3300_channels,
   184			.num_channels = ARRAY_SIZE(sca3300_channels),
 > 185			.scan_masks = sca3300_scan_masks,
   186		},
   187	};
   188	
   189	DECLARE_CRC8_TABLE(sca3300_crc_table);
   190	
   191	static int sca3300_transfer(struct sca3300_data *sca_data, int *val)
   192	{
   193		/* Consecutive requests min. 10 us delay (Datasheet section 5.1.2) */
   194		struct spi_delay delay = { .value = 10, .unit = SPI_DELAY_UNIT_USECS };
   195		int32_t ret;
   196		int rs;
   197		u8 crc;
   198		struct spi_transfer xfers[2] = {
   199			{
   200				.tx_buf = sca_data->txbuf,
   201				.len = ARRAY_SIZE(sca_data->txbuf),
   202				.delay = delay,
   203				.cs_change = 1,
   204			},
   205			{
   206				.rx_buf = sca_data->rxbuf,
   207				.len = ARRAY_SIZE(sca_data->rxbuf),
   208				.delay = delay,
   209			}
   210		};
   211	
   212		/* inverted crc value as described in device data sheet */
   213		crc = ~crc8(sca3300_crc_table, &sca_data->txbuf[0], 3, CRC8_INIT_VALUE);
   214		sca_data->txbuf[3] = crc;
   215	
   216		ret = spi_sync_transfer(sca_data->spi, xfers, ARRAY_SIZE(xfers));
   217		if (ret) {
   218			dev_err(&sca_data->spi->dev,
   219				"transfer error, error: %d\n", ret);
   220			return -EIO;
   221		}
   222	
   223		crc = ~crc8(sca3300_crc_table, &sca_data->rxbuf[0], 3, CRC8_INIT_VALUE);
   224		if (sca_data->rxbuf[3] != crc) {
   225			dev_err(&sca_data->spi->dev, "CRC checksum mismatch");
   226			return -EIO;
   227		}
   228	
   229		/* get return status */
   230		rs = sca_data->rxbuf[0] & SCA3300_MASK_RS_STATUS;
   231		if (rs == SCA3300_VALUE_RS_ERROR)
   232			ret = -EINVAL;
   233	
   234		*val = sign_extend32(get_unaligned_be16(&sca_data->rxbuf[1]), 15);
   235	
   236		return ret;
   237	}
   238	
   239	static int sca3300_error_handler(struct sca3300_data *sca_data)
   240	{
   241		int ret;
   242		int val;
   243	
   244		mutex_lock(&sca_data->lock);
   245		sca_data->txbuf[0] = SCA3300_REG_STATUS << 2;
   246		ret = sca3300_transfer(sca_data, &val);
   247		mutex_unlock(&sca_data->lock);
   248		/*
   249		 * Return status error is cleared after reading status register once,
   250		 * expect EINVAL here.
   251		 */
   252		if (ret != -EINVAL) {
   253			dev_err(&sca_data->spi->dev,
   254				"error reading device status: %d\n", ret);
   255			return ret;
   256		}
   257	
   258		dev_err(&sca_data->spi->dev, "device status: 0x%lx\n",
   259			val & SCA3300_STATUS_MASK);
   260	
   261		return 0;
   262	}
   263	
   264	static int sca3300_read_reg(struct sca3300_data *sca_data, u8 reg, int *val)
   265	{
   266		int ret;
   267	
   268		mutex_lock(&sca_data->lock);
   269		sca_data->txbuf[0] = reg << 2;
   270		ret = sca3300_transfer(sca_data, val);
   271		mutex_unlock(&sca_data->lock);
   272		if (ret != -EINVAL)
   273			return ret;
   274	
   275		return sca3300_error_handler(sca_data);
   276	}
   277	
   278	static int sca3300_write_reg(struct sca3300_data *sca_data, u8 reg, int val)
   279	{
   280		int reg_val = 0;
   281		int ret;
   282	
   283		mutex_lock(&sca_data->lock);
   284		/* BIT(7) for write operation */
   285		sca_data->txbuf[0] = BIT(7) | (reg << 2);
   286		put_unaligned_be16(val, &sca_data->txbuf[1]);
   287		ret = sca3300_transfer(sca_data, &reg_val);
   288		mutex_unlock(&sca_data->lock);
   289		if (ret != -EINVAL)
   290			return ret;
   291	
   292		return sca3300_error_handler(sca_data);
   293	}
   294	
   295	static int sca3300_write_raw(struct iio_dev *indio_dev,
   296				     struct iio_chan_spec const *chan,
   297				     int val, int val2, long mask)
   298	{
   299		struct sca3300_data *data = iio_priv(indio_dev);
   300		int reg_val;
   301		int ret;
   302		int i;
   303	
   304		switch (mask) {
   305		case IIO_CHAN_INFO_SCALE:
   306			for (i = 0; i < OP_MOD_CNT; i++) {
 > 307				if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
   308				    (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
   309					return sca3300_write_reg(data, SCA3300_REG_MODE, i);
   310			}
   311			return -EINVAL;
   312		case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
   313			ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
   314			if (ret)
   315				return ret;
   316			/* freq. change is possible only for mode 3 and 4 */
   317			if (reg_val == 2 && val == sca3300_lp_freq[data->chip_info->chip_type][3])
   318				return sca3300_write_reg(data, SCA3300_REG_MODE, 3);
   319			if (reg_val == 3 && val == sca3300_lp_freq[data->chip_info->chip_type][2])
   320				return sca3300_write_reg(data, SCA3300_REG_MODE, 2);
   321			return -EINVAL;
   322		default:
   323			return -EINVAL;
   324		}
   325	}
   326	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips
@ 2022-01-24 15:01     ` kernel test robot
  0 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2022-01-24 15:01 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 8932 bytes --]

Hi LI,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on v5.17-rc1 next-20220124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: hexagon-randconfig-r033-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242207.0V8eUHu2-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 2e58a18910867ba6795066e044293e6daf89edf5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7dc3bc68cdfcb252dd79fea28a5e944d76784fe8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
        git checkout 7dc3bc68cdfcb252dd79fea28a5e944d76784fe8
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/iio/accel/sca3300.c:74:53: error: too few arguments provided to function-like macro invocation
           IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
                                                              ^
   include/linux/iio/iio.h:112:9: note: macro 'IIO_ENUM_AVAILABLE' defined here
   #define IIO_ENUM_AVAILABLE(_name, _shared, _e) \
           ^
   drivers/iio/accel/sca3300.c:74:2: error: use of undeclared identifier 'IIO_ENUM_AVAILABLE'
           IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
           ^
>> drivers/iio/accel/sca3300.c:185:17: warning: incompatible pointer to integer conversion initializing 'unsigned long' with an expression of type 'const unsigned long[2]' [-Wint-conversion]
                   .scan_masks = sca3300_scan_masks,
                                 ^~~~~~~~~~~~~~~~~~
>> drivers/iio/accel/sca3300.c:307:13: warning: comparison between pointer and integer ('int' and 'const int *') [-Wpointer-integer-compare]
                           if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
                                ~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/accel/sca3300.c:308:14: warning: comparison between pointer and integer ('int' and 'const int *') [-Wpointer-integer-compare]
                               (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
                                ~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/accel/sca3300.c:422:36: warning: incompatible integer to pointer conversion assigning to 'const unsigned long *' from 'const unsigned long'; take the address with & [-Wint-conversion]
                           indio_dev->available_scan_masks = sca3300_chip_info_tbl[i].scan_masks;
                                                           ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                                             &
   4 warnings and 2 errors generated.


vim +185 drivers/iio/accel/sca3300.c

   177	
   178	static const struct sca3300_chip_info sca3300_chip_info_tbl[] = {
   179		[CHIP_SCA3300] = {
   180			.chip_type = CHIP_SCA3300,
   181			.name = "sca3300",
   182			.chip_id = 0x51,
   183			.channels = sca3300_channels,
   184			.num_channels = ARRAY_SIZE(sca3300_channels),
 > 185			.scan_masks = sca3300_scan_masks,
   186		},
   187	};
   188	
   189	DECLARE_CRC8_TABLE(sca3300_crc_table);
   190	
   191	static int sca3300_transfer(struct sca3300_data *sca_data, int *val)
   192	{
   193		/* Consecutive requests min. 10 us delay (Datasheet section 5.1.2) */
   194		struct spi_delay delay = { .value = 10, .unit = SPI_DELAY_UNIT_USECS };
   195		int32_t ret;
   196		int rs;
   197		u8 crc;
   198		struct spi_transfer xfers[2] = {
   199			{
   200				.tx_buf = sca_data->txbuf,
   201				.len = ARRAY_SIZE(sca_data->txbuf),
   202				.delay = delay,
   203				.cs_change = 1,
   204			},
   205			{
   206				.rx_buf = sca_data->rxbuf,
   207				.len = ARRAY_SIZE(sca_data->rxbuf),
   208				.delay = delay,
   209			}
   210		};
   211	
   212		/* inverted crc value as described in device data sheet */
   213		crc = ~crc8(sca3300_crc_table, &sca_data->txbuf[0], 3, CRC8_INIT_VALUE);
   214		sca_data->txbuf[3] = crc;
   215	
   216		ret = spi_sync_transfer(sca_data->spi, xfers, ARRAY_SIZE(xfers));
   217		if (ret) {
   218			dev_err(&sca_data->spi->dev,
   219				"transfer error, error: %d\n", ret);
   220			return -EIO;
   221		}
   222	
   223		crc = ~crc8(sca3300_crc_table, &sca_data->rxbuf[0], 3, CRC8_INIT_VALUE);
   224		if (sca_data->rxbuf[3] != crc) {
   225			dev_err(&sca_data->spi->dev, "CRC checksum mismatch");
   226			return -EIO;
   227		}
   228	
   229		/* get return status */
   230		rs = sca_data->rxbuf[0] & SCA3300_MASK_RS_STATUS;
   231		if (rs == SCA3300_VALUE_RS_ERROR)
   232			ret = -EINVAL;
   233	
   234		*val = sign_extend32(get_unaligned_be16(&sca_data->rxbuf[1]), 15);
   235	
   236		return ret;
   237	}
   238	
   239	static int sca3300_error_handler(struct sca3300_data *sca_data)
   240	{
   241		int ret;
   242		int val;
   243	
   244		mutex_lock(&sca_data->lock);
   245		sca_data->txbuf[0] = SCA3300_REG_STATUS << 2;
   246		ret = sca3300_transfer(sca_data, &val);
   247		mutex_unlock(&sca_data->lock);
   248		/*
   249		 * Return status error is cleared after reading status register once,
   250		 * expect EINVAL here.
   251		 */
   252		if (ret != -EINVAL) {
   253			dev_err(&sca_data->spi->dev,
   254				"error reading device status: %d\n", ret);
   255			return ret;
   256		}
   257	
   258		dev_err(&sca_data->spi->dev, "device status: 0x%lx\n",
   259			val & SCA3300_STATUS_MASK);
   260	
   261		return 0;
   262	}
   263	
   264	static int sca3300_read_reg(struct sca3300_data *sca_data, u8 reg, int *val)
   265	{
   266		int ret;
   267	
   268		mutex_lock(&sca_data->lock);
   269		sca_data->txbuf[0] = reg << 2;
   270		ret = sca3300_transfer(sca_data, val);
   271		mutex_unlock(&sca_data->lock);
   272		if (ret != -EINVAL)
   273			return ret;
   274	
   275		return sca3300_error_handler(sca_data);
   276	}
   277	
   278	static int sca3300_write_reg(struct sca3300_data *sca_data, u8 reg, int val)
   279	{
   280		int reg_val = 0;
   281		int ret;
   282	
   283		mutex_lock(&sca_data->lock);
   284		/* BIT(7) for write operation */
   285		sca_data->txbuf[0] = BIT(7) | (reg << 2);
   286		put_unaligned_be16(val, &sca_data->txbuf[1]);
   287		ret = sca3300_transfer(sca_data, &reg_val);
   288		mutex_unlock(&sca_data->lock);
   289		if (ret != -EINVAL)
   290			return ret;
   291	
   292		return sca3300_error_handler(sca_data);
   293	}
   294	
   295	static int sca3300_write_raw(struct iio_dev *indio_dev,
   296				     struct iio_chan_spec const *chan,
   297				     int val, int val2, long mask)
   298	{
   299		struct sca3300_data *data = iio_priv(indio_dev);
   300		int reg_val;
   301		int ret;
   302		int i;
   303	
   304		switch (mask) {
   305		case IIO_CHAN_INFO_SCALE:
   306			for (i = 0; i < OP_MOD_CNT; i++) {
 > 307				if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
   308				    (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
   309					return sca3300_write_reg(data, SCA3300_REG_MODE, i);
   310			}
   311			return -EINVAL;
   312		case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
   313			ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
   314			if (ret)
   315				return ret;
   316			/* freq. change is possible only for mode 3 and 4 */
   317			if (reg_val == 2 && val == sca3300_lp_freq[data->chip_info->chip_type][3])
   318				return sca3300_write_reg(data, SCA3300_REG_MODE, 3);
   319			if (reg_val == 3 && val == sca3300_lp_freq[data->chip_info->chip_type][2])
   320				return sca3300_write_reg(data, SCA3300_REG_MODE, 2);
   321			return -EINVAL;
   322		default:
   323			return -EINVAL;
   324		}
   325	}
   326	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips
  2022-01-24  9:39 ` [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips LI Qingwu
@ 2022-01-24 15:32     ` kernel test robot
  2022-01-24 15:01     ` kernel test robot
  2022-01-24 15:32     ` kernel test robot
  2 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2022-01-24 15:32 UTC (permalink / raw)
  To: LI Qingwu, jic23, lars, robh+dt, tomas.melin, andy.shevchenko,
	devicetree, linux-kernel
  Cc: kbuild-all, LI Qingwu

Hi LI,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on v5.17-rc1 next-20220124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: parisc-randconfig-r015-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242336.1dSTFe7a-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7dc3bc68cdfcb252dd79fea28a5e944d76784fe8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
        git checkout 7dc3bc68cdfcb252dd79fea28a5e944d76784fe8
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=parisc SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/iio/accel/sca3300.c:74:60: error: macro "IIO_ENUM_AVAILABLE" requires 3 arguments, but only 2 given
      74 |         IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
         |                                                            ^
   In file included from include/linux/iio/buffer.h:10,
                    from drivers/iio/accel/sca3300.c:17:
   include/linux/iio/iio.h:112: note: macro "IIO_ENUM_AVAILABLE" defined here
     112 | #define IIO_ENUM_AVAILABLE(_name, _shared, _e) \
         | 
   drivers/iio/accel/sca3300.c:74:9: error: 'IIO_ENUM_AVAILABLE' undeclared here (not in a function)
      74 |         IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
         |         ^~~~~~~~~~~~~~~~~~
>> drivers/iio/accel/sca3300.c:185:31: warning: initialization of 'long unsigned int' from 'const long unsigned int *' makes integer from pointer without a cast [-Wint-conversion]
     185 |                 .scan_masks = sca3300_scan_masks,
         |                               ^~~~~~~~~~~~~~~~~~
   drivers/iio/accel/sca3300.c:185:31: note: (near initialization for 'sca3300_chip_info_tbl[0].scan_masks')
   drivers/iio/accel/sca3300.c: In function 'sca3300_write_raw':
>> drivers/iio/accel/sca3300.c:307:34: warning: comparison between pointer and integer
     307 |                         if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
         |                                  ^~
   drivers/iio/accel/sca3300.c:308:35: warning: comparison between pointer and integer
     308 |                             (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
         |                                   ^~
   drivers/iio/accel/sca3300.c: In function 'sca3300_init':
>> drivers/iio/accel/sca3300.c:422:57: warning: assignment to 'const long unsigned int *' from 'long unsigned int' makes pointer from integer without a cast [-Wint-conversion]
     422 |                         indio_dev->available_scan_masks = sca3300_chip_info_tbl[i].scan_masks;
         |                                                         ^


vim +185 drivers/iio/accel/sca3300.c

   177	
   178	static const struct sca3300_chip_info sca3300_chip_info_tbl[] = {
   179		[CHIP_SCA3300] = {
   180			.chip_type = CHIP_SCA3300,
   181			.name = "sca3300",
   182			.chip_id = 0x51,
   183			.channels = sca3300_channels,
   184			.num_channels = ARRAY_SIZE(sca3300_channels),
 > 185			.scan_masks = sca3300_scan_masks,
   186		},
   187	};
   188	
   189	DECLARE_CRC8_TABLE(sca3300_crc_table);
   190	
   191	static int sca3300_transfer(struct sca3300_data *sca_data, int *val)
   192	{
   193		/* Consecutive requests min. 10 us delay (Datasheet section 5.1.2) */
   194		struct spi_delay delay = { .value = 10, .unit = SPI_DELAY_UNIT_USECS };
   195		int32_t ret;
   196		int rs;
   197		u8 crc;
   198		struct spi_transfer xfers[2] = {
   199			{
   200				.tx_buf = sca_data->txbuf,
   201				.len = ARRAY_SIZE(sca_data->txbuf),
   202				.delay = delay,
   203				.cs_change = 1,
   204			},
   205			{
   206				.rx_buf = sca_data->rxbuf,
   207				.len = ARRAY_SIZE(sca_data->rxbuf),
   208				.delay = delay,
   209			}
   210		};
   211	
   212		/* inverted crc value as described in device data sheet */
   213		crc = ~crc8(sca3300_crc_table, &sca_data->txbuf[0], 3, CRC8_INIT_VALUE);
   214		sca_data->txbuf[3] = crc;
   215	
   216		ret = spi_sync_transfer(sca_data->spi, xfers, ARRAY_SIZE(xfers));
   217		if (ret) {
   218			dev_err(&sca_data->spi->dev,
   219				"transfer error, error: %d\n", ret);
   220			return -EIO;
   221		}
   222	
   223		crc = ~crc8(sca3300_crc_table, &sca_data->rxbuf[0], 3, CRC8_INIT_VALUE);
   224		if (sca_data->rxbuf[3] != crc) {
   225			dev_err(&sca_data->spi->dev, "CRC checksum mismatch");
   226			return -EIO;
   227		}
   228	
   229		/* get return status */
   230		rs = sca_data->rxbuf[0] & SCA3300_MASK_RS_STATUS;
   231		if (rs == SCA3300_VALUE_RS_ERROR)
   232			ret = -EINVAL;
   233	
   234		*val = sign_extend32(get_unaligned_be16(&sca_data->rxbuf[1]), 15);
   235	
   236		return ret;
   237	}
   238	
   239	static int sca3300_error_handler(struct sca3300_data *sca_data)
   240	{
   241		int ret;
   242		int val;
   243	
   244		mutex_lock(&sca_data->lock);
   245		sca_data->txbuf[0] = SCA3300_REG_STATUS << 2;
   246		ret = sca3300_transfer(sca_data, &val);
   247		mutex_unlock(&sca_data->lock);
   248		/*
   249		 * Return status error is cleared after reading status register once,
   250		 * expect EINVAL here.
   251		 */
   252		if (ret != -EINVAL) {
   253			dev_err(&sca_data->spi->dev,
   254				"error reading device status: %d\n", ret);
   255			return ret;
   256		}
   257	
   258		dev_err(&sca_data->spi->dev, "device status: 0x%lx\n",
   259			val & SCA3300_STATUS_MASK);
   260	
   261		return 0;
   262	}
   263	
   264	static int sca3300_read_reg(struct sca3300_data *sca_data, u8 reg, int *val)
   265	{
   266		int ret;
   267	
   268		mutex_lock(&sca_data->lock);
   269		sca_data->txbuf[0] = reg << 2;
   270		ret = sca3300_transfer(sca_data, val);
   271		mutex_unlock(&sca_data->lock);
   272		if (ret != -EINVAL)
   273			return ret;
   274	
   275		return sca3300_error_handler(sca_data);
   276	}
   277	
   278	static int sca3300_write_reg(struct sca3300_data *sca_data, u8 reg, int val)
   279	{
   280		int reg_val = 0;
   281		int ret;
   282	
   283		mutex_lock(&sca_data->lock);
   284		/* BIT(7) for write operation */
   285		sca_data->txbuf[0] = BIT(7) | (reg << 2);
   286		put_unaligned_be16(val, &sca_data->txbuf[1]);
   287		ret = sca3300_transfer(sca_data, &reg_val);
   288		mutex_unlock(&sca_data->lock);
   289		if (ret != -EINVAL)
   290			return ret;
   291	
   292		return sca3300_error_handler(sca_data);
   293	}
   294	
   295	static int sca3300_write_raw(struct iio_dev *indio_dev,
   296				     struct iio_chan_spec const *chan,
   297				     int val, int val2, long mask)
   298	{
   299		struct sca3300_data *data = iio_priv(indio_dev);
   300		int reg_val;
   301		int ret;
   302		int i;
   303	
   304		switch (mask) {
   305		case IIO_CHAN_INFO_SCALE:
   306			for (i = 0; i < OP_MOD_CNT; i++) {
 > 307				if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
   308				    (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
   309					return sca3300_write_reg(data, SCA3300_REG_MODE, i);
   310			}
   311			return -EINVAL;
   312		case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
   313			ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
   314			if (ret)
   315				return ret;
   316			/* freq. change is possible only for mode 3 and 4 */
   317			if (reg_val == 2 && val == sca3300_lp_freq[data->chip_info->chip_type][3])
   318				return sca3300_write_reg(data, SCA3300_REG_MODE, 3);
   319			if (reg_val == 3 && val == sca3300_lp_freq[data->chip_info->chip_type][2])
   320				return sca3300_write_reg(data, SCA3300_REG_MODE, 2);
   321			return -EINVAL;
   322		default:
   323			return -EINVAL;
   324		}
   325	}
   326	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips
@ 2022-01-24 15:32     ` kernel test robot
  0 siblings, 0 replies; 25+ messages in thread
From: kernel test robot @ 2022-01-24 15:32 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 8957 bytes --]

Hi LI,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on v5.17-rc1 next-20220124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: parisc-randconfig-r015-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242336.1dSTFe7a-lkp(a)intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7dc3bc68cdfcb252dd79fea28a5e944d76784fe8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review LI-Qingwu/i-iio-accel-sca3300-add-compitible-for-scl3300/20220124-174021
        git checkout 7dc3bc68cdfcb252dd79fea28a5e944d76784fe8
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=parisc SHELL=/bin/bash drivers/iio/accel/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/iio/accel/sca3300.c:74:60: error: macro "IIO_ENUM_AVAILABLE" requires 3 arguments, but only 2 given
      74 |         IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
         |                                                            ^
   In file included from include/linux/iio/buffer.h:10,
                    from drivers/iio/accel/sca3300.c:17:
   include/linux/iio/iio.h:112: note: macro "IIO_ENUM_AVAILABLE" defined here
     112 | #define IIO_ENUM_AVAILABLE(_name, _shared, _e) \
         | 
   drivers/iio/accel/sca3300.c:74:9: error: 'IIO_ENUM_AVAILABLE' undeclared here (not in a function)
      74 |         IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
         |         ^~~~~~~~~~~~~~~~~~
>> drivers/iio/accel/sca3300.c:185:31: warning: initialization of 'long unsigned int' from 'const long unsigned int *' makes integer from pointer without a cast [-Wint-conversion]
     185 |                 .scan_masks = sca3300_scan_masks,
         |                               ^~~~~~~~~~~~~~~~~~
   drivers/iio/accel/sca3300.c:185:31: note: (near initialization for 'sca3300_chip_info_tbl[0].scan_masks')
   drivers/iio/accel/sca3300.c: In function 'sca3300_write_raw':
>> drivers/iio/accel/sca3300.c:307:34: warning: comparison between pointer and integer
     307 |                         if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
         |                                  ^~
   drivers/iio/accel/sca3300.c:308:35: warning: comparison between pointer and integer
     308 |                             (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
         |                                   ^~
   drivers/iio/accel/sca3300.c: In function 'sca3300_init':
>> drivers/iio/accel/sca3300.c:422:57: warning: assignment to 'const long unsigned int *' from 'long unsigned int' makes pointer from integer without a cast [-Wint-conversion]
     422 |                         indio_dev->available_scan_masks = sca3300_chip_info_tbl[i].scan_masks;
         |                                                         ^


vim +185 drivers/iio/accel/sca3300.c

   177	
   178	static const struct sca3300_chip_info sca3300_chip_info_tbl[] = {
   179		[CHIP_SCA3300] = {
   180			.chip_type = CHIP_SCA3300,
   181			.name = "sca3300",
   182			.chip_id = 0x51,
   183			.channels = sca3300_channels,
   184			.num_channels = ARRAY_SIZE(sca3300_channels),
 > 185			.scan_masks = sca3300_scan_masks,
   186		},
   187	};
   188	
   189	DECLARE_CRC8_TABLE(sca3300_crc_table);
   190	
   191	static int sca3300_transfer(struct sca3300_data *sca_data, int *val)
   192	{
   193		/* Consecutive requests min. 10 us delay (Datasheet section 5.1.2) */
   194		struct spi_delay delay = { .value = 10, .unit = SPI_DELAY_UNIT_USECS };
   195		int32_t ret;
   196		int rs;
   197		u8 crc;
   198		struct spi_transfer xfers[2] = {
   199			{
   200				.tx_buf = sca_data->txbuf,
   201				.len = ARRAY_SIZE(sca_data->txbuf),
   202				.delay = delay,
   203				.cs_change = 1,
   204			},
   205			{
   206				.rx_buf = sca_data->rxbuf,
   207				.len = ARRAY_SIZE(sca_data->rxbuf),
   208				.delay = delay,
   209			}
   210		};
   211	
   212		/* inverted crc value as described in device data sheet */
   213		crc = ~crc8(sca3300_crc_table, &sca_data->txbuf[0], 3, CRC8_INIT_VALUE);
   214		sca_data->txbuf[3] = crc;
   215	
   216		ret = spi_sync_transfer(sca_data->spi, xfers, ARRAY_SIZE(xfers));
   217		if (ret) {
   218			dev_err(&sca_data->spi->dev,
   219				"transfer error, error: %d\n", ret);
   220			return -EIO;
   221		}
   222	
   223		crc = ~crc8(sca3300_crc_table, &sca_data->rxbuf[0], 3, CRC8_INIT_VALUE);
   224		if (sca_data->rxbuf[3] != crc) {
   225			dev_err(&sca_data->spi->dev, "CRC checksum mismatch");
   226			return -EIO;
   227		}
   228	
   229		/* get return status */
   230		rs = sca_data->rxbuf[0] & SCA3300_MASK_RS_STATUS;
   231		if (rs == SCA3300_VALUE_RS_ERROR)
   232			ret = -EINVAL;
   233	
   234		*val = sign_extend32(get_unaligned_be16(&sca_data->rxbuf[1]), 15);
   235	
   236		return ret;
   237	}
   238	
   239	static int sca3300_error_handler(struct sca3300_data *sca_data)
   240	{
   241		int ret;
   242		int val;
   243	
   244		mutex_lock(&sca_data->lock);
   245		sca_data->txbuf[0] = SCA3300_REG_STATUS << 2;
   246		ret = sca3300_transfer(sca_data, &val);
   247		mutex_unlock(&sca_data->lock);
   248		/*
   249		 * Return status error is cleared after reading status register once,
   250		 * expect EINVAL here.
   251		 */
   252		if (ret != -EINVAL) {
   253			dev_err(&sca_data->spi->dev,
   254				"error reading device status: %d\n", ret);
   255			return ret;
   256		}
   257	
   258		dev_err(&sca_data->spi->dev, "device status: 0x%lx\n",
   259			val & SCA3300_STATUS_MASK);
   260	
   261		return 0;
   262	}
   263	
   264	static int sca3300_read_reg(struct sca3300_data *sca_data, u8 reg, int *val)
   265	{
   266		int ret;
   267	
   268		mutex_lock(&sca_data->lock);
   269		sca_data->txbuf[0] = reg << 2;
   270		ret = sca3300_transfer(sca_data, val);
   271		mutex_unlock(&sca_data->lock);
   272		if (ret != -EINVAL)
   273			return ret;
   274	
   275		return sca3300_error_handler(sca_data);
   276	}
   277	
   278	static int sca3300_write_reg(struct sca3300_data *sca_data, u8 reg, int val)
   279	{
   280		int reg_val = 0;
   281		int ret;
   282	
   283		mutex_lock(&sca_data->lock);
   284		/* BIT(7) for write operation */
   285		sca_data->txbuf[0] = BIT(7) | (reg << 2);
   286		put_unaligned_be16(val, &sca_data->txbuf[1]);
   287		ret = sca3300_transfer(sca_data, &reg_val);
   288		mutex_unlock(&sca_data->lock);
   289		if (ret != -EINVAL)
   290			return ret;
   291	
   292		return sca3300_error_handler(sca_data);
   293	}
   294	
   295	static int sca3300_write_raw(struct iio_dev *indio_dev,
   296				     struct iio_chan_spec const *chan,
   297				     int val, int val2, long mask)
   298	{
   299		struct sca3300_data *data = iio_priv(indio_dev);
   300		int reg_val;
   301		int ret;
   302		int i;
   303	
   304		switch (mask) {
   305		case IIO_CHAN_INFO_SCALE:
   306			for (i = 0; i < OP_MOD_CNT; i++) {
 > 307				if ((val == sca3300_accel_scale[data->chip_info->chip_type][0]) &&
   308				    (val2 == sca3300_accel_scale[data->chip_info->chip_type][1]))
   309					return sca3300_write_reg(data, SCA3300_REG_MODE, i);
   310			}
   311			return -EINVAL;
   312		case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
   313			ret = sca3300_read_reg(data, SCA3300_REG_MODE, &reg_val);
   314			if (ret)
   315				return ret;
   316			/* freq. change is possible only for mode 3 and 4 */
   317			if (reg_val == 2 && val == sca3300_lp_freq[data->chip_info->chip_type][3])
   318				return sca3300_write_reg(data, SCA3300_REG_MODE, 3);
   319			if (reg_val == 3 && val == sca3300_lp_freq[data->chip_info->chip_type][2])
   320				return sca3300_write_reg(data, SCA3300_REG_MODE, 2);
   321			return -EINVAL;
   322		default:
   323			return -EINVAL;
   324		}
   325	}
   326	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300
  2022-01-24  9:39 [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 LI Qingwu
                   ` (5 preceding siblings ...)
  2022-01-24  9:39 ` [PATCH V1 6/6] dt-bindings: iio: accel: sca3300: Document murata,scl3300 LI Qingwu
@ 2022-01-30 11:35 ` Jonathan Cameron
  6 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2022-01-30 11:35 UTC (permalink / raw)
  To: LI Qingwu
  Cc: lars, robh+dt, tomas.melin, andy.shevchenko, devicetree, linux-kernel

On Mon, 24 Jan 2022 09:39:06 +0000
LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:

> The current driver support sca3300 only.
> Modifed for support SCL3300.
> Verifed with SCL3300 on IMX8MM.
> Splited the change for review.
> 
> LI Qingwu (6):
>   iio: accel: sca3300: add define for temp channel for reuse.
>   iio: accel: sca3300: Add interface for operation modes.
>   iio: accel: sca3300: modified to support multi chips
>   iio: accel: sca3300: Add support for SCL3300
>   iio: accel: sca3300: Add inclination channels.
>   dt-bindings: iio: accel: sca3300: Document murata,scl3300
> 
>  .../bindings/iio/accel/murata,sca3300.yaml    |   1 +
>  drivers/iio/accel/sca3300.c                   | 284 +++++++++++++++---
>  2 files changed, 238 insertions(+), 47 deletions(-)
> 

For v2, please make sure to cc linux-iio@vger.kernel.org

I won't pick up any IIO changes that haven't been sent to that list.

Thanks,

Jonathan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes.
  2022-01-24  9:39 ` [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes LI Qingwu
                     ` (2 preceding siblings ...)
  2022-01-24 13:59     ` kernel test robot
@ 2022-01-30 11:40   ` Jonathan Cameron
  2022-02-10 10:08     ` LI Qingwu
  3 siblings, 1 reply; 25+ messages in thread
From: Jonathan Cameron @ 2022-01-30 11:40 UTC (permalink / raw)
  To: LI Qingwu
  Cc: lars, robh+dt, tomas.melin, andy.shevchenko, devicetree, linux-kernel

On Mon, 24 Jan 2022 09:39:08 +0000
LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:

> The acceleration scale and the frequency were set via operation modes,
> the scal and frequency are both non-uniqueness,
> this leads to logic confusion for setting scale.and.frequency.
> it getting worse if add more different sensor types into the driver.
> 
> The commit add an interface for set and get the operation modes.
> the following interfaces added:
> in_accel_op_mode_available
> in_op_mode
> 
> SCA3300 operation modes table:
> | Mode | Full-scale | low pass filter frequency |
> | ---- | ---------- | ------------------------- |
> | 1    | ± 3 g      | 70 Hz                     |
> | 2    | ± 6 g      | 70 Hz                     |
> | 3    | ± 1.5 g    | 70 Hz                     |
> | 4    | ± 1.5 g    | 10 Hz                     |
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>

While it may seem convenient to expose this to userspace, the reality is
that generic userspace has no way to know how to use it.

That makes supplying this control a bad idea however convenient it
may seem.  It's not unusual to have these sorts of constraints on
devices and so the ABI always assumes any setting may modify any other
and / or change what is available for a given setting.

If you need a particular combination for your own userspace, then
make the userspace aware of the constraints rather than exposing it
as a 'mode' which the userspace will need to know about anyway.

Jonathan


> ---
>  drivers/iio/accel/sca3300.c | 55 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
> index 083ae2a47ad9..e26b3175b3c6 100644
> --- a/drivers/iio/accel/sca3300.c
> +++ b/drivers/iio/accel/sca3300.c
> @@ -42,6 +42,38 @@
>  /* Device return status and mask */
>  #define SCA3300_VALUE_RS_ERROR	0x3
>  #define SCA3300_MASK_RS_STATUS	GENMASK(1, 0)
> +enum sca3300_op_mode_indexes {
> +	OP_MOD_1 = 0,
> +	OP_MOD_2,
> +	OP_MOD_3,
> +	OP_MOD_4,
> +	OP_MOD_CNT
> +};
> +
> +static const char * const sca3300_op_modes[] = {
> +	[OP_MOD_1] = "1",
> +	[OP_MOD_2] = "2",
> +	[OP_MOD_3] = "3",
> +	[OP_MOD_4] = "4"
> +};
> +
> +static int sca3300_get_op_mode(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan);
> +static int sca3300_set_op_mode(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan, unsigned int mode);
> +
> +static const struct iio_enum sca3300_op_mode_enum = {
> +	.items = sca3300_op_modes,
> +	.num_items = ARRAY_SIZE(sca3300_op_modes),
> +	.get = sca3300_get_op_mode,
> +	.set = sca3300_set_op_mode,
> +};
> +
> +static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
> +	IIO_ENUM("op_mode", IIO_SHARED_BY_DIR, &sca3300_op_mode_enum),
> +	IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
> +	{ }
> +};
>  
>  enum sca3300_scan_indexes {
>  	SCA3300_ACC_X = 0,
> @@ -70,6 +102,7 @@ enum sca3300_scan_indexes {
>  		.storagebits = 16,					\
>  		.endianness = IIO_CPU,					\
>  	},								\
> +	.ext_info = sca3300_ext_info,					\
>  }
>  
>  #define SCA3300_TEMP_CHANNEL(index, reg) {				\
> @@ -400,6 +433,28 @@ static int sca3300_read_avail(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static int sca3300_get_op_mode(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan)
> +{
> +	int mode;
> +	int ret;
> +	struct sca3300_data *data = iio_priv(indio_dev);
> +
> +	ret = sca3300_read_reg(data, SCA3300_REG_MODE, &mode);
> +	if (ret)
> +		return ret;
> +	return mode;
> +
> +}
> +
> +static int sca3300_set_op_mode(struct iio_dev *indio_dev,
> +		const struct iio_chan_spec *chan, unsigned int mode)
> +{
> +	struct sca3300_data *data = iio_priv(indio_dev);
> +
> +	return sca3300_write_reg(data, SCA3300_REG_MODE, mode);
> +}
> +
>  static const struct iio_info sca3300_info = {
>  	.read_raw = sca3300_read_raw,
>  	.write_raw = sca3300_write_raw,


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 5/6] iio: accel: sca3300: Add inclination channels.
  2022-01-24 13:19   ` Andy Shevchenko
@ 2022-01-30 11:46     ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2022-01-30 11:46 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: LI Qingwu, Lars-Peter Clausen, Rob Herring, Tomas Melin,
	devicetree, Linux Kernel Mailing List, linux-iio

On Mon, 24 Jan 2022 15:19:09 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Mon, Jan 24, 2022 at 11:39 AM LI Qingwu
> <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> >
> > Different with SCA3300, SCL3300 can output inclination angles.
> > Angles are formed from acceleration with following equations:
> > ANG_X = atan2(accx / √(accy^2 + accz^2)),
> > ANG_Y = atan2(accy / √(accx^2 + accz^2)),
> > ANG_Z = atan2(accz / √(accx^2 + accy^2)),
> >
> > The commit add output of the raw value,scale
> > and scale_available of angles.
> > add interface for enable/disable of the angle output.
> >
> > new interfaces:  
> 
> New
> 
> > in_incli_en

Why?  There are only a few reasons to have an enable for a
channel and they don't include something that we might only
sometimes read (tend to be temporal channels such as step
counters where we want to be able to pause their counting,
or output channels).

> > in_incli_scale
> > in_incli_scale_available
> > in_incli_x_raw
> > in_incli_y_raw
> > in_incli_z_raw  
> 
> Indent them by 2 spaces.
> 
> Wondering if these need to be described in ABI documentation.
It's standard ABI, though we don't give much description of what
exactly these ease. It might be possible to add more information
to the generic docs, but that would require looking very carefully
at the current supporting devices.

"Inclination raw reading about axis x, y or z (may be
Arbitrarily assigned). Data converted by application of offset
and scale to degrees."

> 
> ...
> 
> >         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(4)  
> 
> > +       IIO_CHAN_SOFT_TIMESTAMP(SCA3300_TIMESTAMP)  
> 
> + Comma (while at it)?
> 
> ...
> 
> > -       IIO_CHAN_SOFT_TIMESTAMP(4),
> > +       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_X, 0x09, X),
> > +       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Y, 0x0A, Y),
> > +       SCA3300_INCLI_CHANNEL(SCA3300_INCLI_Z, 0x0B, Z),
> > +       IIO_CHAN_SOFT_TIMESTAMP(SCA3300_TIMESTAMP)  
> 
> Ditto.
> 
> > +static const int sca3300_incli_scale[CHIP_CNT][OP_MOD_CNT][2] = {
> > +       [CHIP_SCA3300] = {{0, 0}, {0, 0}, {0, 0}, {0, 0}},  
> 
> > +       [CHIP_SCL3300] = {{0, 5495}, {0, 5495}, {0, 5495}, {0, 5495}}  
> 
> + Comma.
> 
> > +};  
> 
> ...
> 
> >         struct {
> > -               s16 channels[4];
> > +               s16 channels[SCA3300_TIMESTAMP-1];  
> 
> Missed spaces around the arithmetic operator.
> 
> >                 s64 ts __aligned(sizeof(s64));
> >         } scan;
> >         const struct sca3300_chip_info *chip_info;
> >         u8 txbuf[4] ____cacheline_aligned;
> >         u8 rxbuf[4];  
> 
> > -  
> 
> Stray change.
> 
> >  };  
> 
> ...
> 
> > +               /*Inclination scale info tied to accel scale.*/
> > +               /*not allowed to set separately.      */  
> 
> Please, follow the proper style for multi-line comments, including
> necessary spaces, periods, starting and ending lines.
> 
> ...
> 
> > +       case IIO_CHAN_INFO_ENABLE:
> > +               if (data->chip_info->chip_type == CHIP_SCL3300) {  
> 
> > +                       if (chan->type == IIO_INCLI) {  
> 
> See below.
> 
> > +                               if (val != 0)  
> 
>    if (val)
> 
> > +                                       reg_val = 0x1F;
> > +                               else
> > +                                       reg_val = 0x00;
> > +                               return sca3300_write_reg(data, SCA3300_REG_ANG_CTRL, reg_val);
> > +                       }
> > +               }  
> 
> ...
> 
> > -               if (chan->type == IIO_ACCEL) {
> > +
> > +               if (chan->type == IIO_INCLI) {  
> 
> > +               } else if (chan->type == IIO_ACCEL) {  
> 
> I would recommend using switch-case for channel type as well.
> 
> ...
> 
> > +       case IIO_CHAN_INFO_ENABLE:
> > +               if (chan->type == IIO_INCLI) {  
> 
> > +                       ret = sca3300_read_reg(data, SCA3300_REG_ANG_CTRL, &reg_val);  
> 
> How is ret supposed to be used?
> 
> > +                       *val = reg_val;
> > +                       return IIO_VAL_INT;
> > +               }
> > +               return -EINVAL;  
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 6/6] dt-bindings: iio: accel: sca3300: Document murata,scl3300
  2022-01-24  9:39 ` [PATCH V1 6/6] dt-bindings: iio: accel: sca3300: Document murata,scl3300 LI Qingwu
@ 2022-02-09  3:08   ` Rob Herring
  0 siblings, 0 replies; 25+ messages in thread
From: Rob Herring @ 2022-02-09  3:08 UTC (permalink / raw)
  To: LI Qingwu
  Cc: andy.shevchenko, linux-kernel, tomas.melin, devicetree,
	qing-wu.li, robh+dt, lars, jic23

On Mon, 24 Jan 2022 09:39:12 +0000, LI Qingwu wrote:
> initial DT bindings for Murata scl3300 inclinometer.
> 
> Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> ---
>  Documentation/devicetree/bindings/iio/accel/murata,sca3300.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes.
  2022-01-30 11:40   ` Jonathan Cameron
@ 2022-02-10 10:08     ` LI Qingwu
  2022-02-10 15:40       ` Jonathan Cameron
  0 siblings, 1 reply; 25+ messages in thread
From: LI Qingwu @ 2022-02-10 10:08 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars, robh+dt, tomas.melin, andy.shevchenko, devicetree, linux-kernel

Thanks a lot all of your inputs, I'm just back from long holiday and start to rework on the patches.

> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, January 30, 2022 7:40 PM
> To: LI Qingwu <qing-wu.li@leica-geosystems.com.cn>
> Cc: lars@metafoo.de; robh+dt@kernel.org; tomas.melin@vaisala.com;
> andy.shevchenko@gmail.com; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation
> modes.
> 
> This email is not from Hexagon’s Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
> 
> 
> On Mon, 24 Jan 2022 09:39:08 +0000
> LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> 
> > The acceleration scale and the frequency were set via operation modes,
> > the scal and frequency are both non-uniqueness, this leads to logic
> > confusion for setting scale.and.frequency.
> > it getting worse if add more different sensor types into the driver.
> >
> > The commit add an interface for set and get the operation modes.
> > the following interfaces added:
> > in_accel_op_mode_available
> > in_op_mode
> >
> > SCA3300 operation modes table:
> > | Mode | Full-scale | low pass filter frequency |
> > | ---- | ---------- | ------------------------- |
> > | 1    | ± 3 g      | 70 Hz                     |
> > | 2    | ± 6 g      | 70 Hz                     |
> > | 3    | ± 1.5 g    | 70 Hz                     |
> > | 4    | ± 1.5 g    | 10 Hz                     |
> >
> > Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>
> 
> While it may seem convenient to expose this to userspace, the reality is that
> generic userspace has no way to know how to use it.
> 
> That makes supplying this control a bad idea however convenient it may seem.
> It's not unusual to have these sorts of constraints on devices and so the ABI
> always assumes any setting may modify any other and / or change what is
> available for a given setting.
> 
> If you need a particular combination for your own userspace, then make the
> userspace aware of the constraints rather than exposing it as a 'mode' which
> the userspace will need to know about anyway.
> 
> Jonathan

Thanks a lot Jonathan, I couldn't agree with you more, the mode is not good for userspace,
I would like to ask you how to handle this.
Since the change for 'mode' was a prepare for support SCL3300,
For SCL3300, mode 3 and mode 4 are totally same for both scale and frequency.
The only different is mode 4 is low noise mode, but no difference from software point of view.
Then it's impossible to set to between mode 3/4, let's say normal noise and low noise mode, with index of frequency and scale.
Set between mode 3 and 4 is necessary, I have no idea how to handle it.

| Mode             | Full-scale   | frequency |
| -------------------      | ----------------- | ------------- |
| 1                 | ± 1.2 g     | 40 Hz    |
| 2                 | ± 2.4 g     | 70 Hz    |
| 3                 | ± 0.6 g     | 10 Hz    |
| 4 (Low noise mode)  | ± 0.6 g     | 10 Hz    |   
               
The link of the SCL3300 datasheet:
https://www.murata.com/-/media/webrenewal/products/sensor/pdf/datasheet/datasheet_scl3300-d01.ashx?la=en&cvid=20210316063715000000

> 
> 
> > ---
> >  drivers/iio/accel/sca3300.c | 55
> > +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> >
> > diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
> > index 083ae2a47ad9..e26b3175b3c6 100644
> > --- a/drivers/iio/accel/sca3300.c
> > +++ b/drivers/iio/accel/sca3300.c
> > @@ -42,6 +42,38 @@
> >  /* Device return status and mask */
> >  #define SCA3300_VALUE_RS_ERROR       0x3
> >  #define SCA3300_MASK_RS_STATUS       GENMASK(1, 0)
> > +enum sca3300_op_mode_indexes {
> > +     OP_MOD_1 = 0,
> > +     OP_MOD_2,
> > +     OP_MOD_3,
> > +     OP_MOD_4,
> > +     OP_MOD_CNT
> > +};
> > +
> > +static const char * const sca3300_op_modes[] = {
> > +     [OP_MOD_1] = "1",
> > +     [OP_MOD_2] = "2",
> > +     [OP_MOD_3] = "3",
> > +     [OP_MOD_4] = "4"
> > +};
> > +
> > +static int sca3300_get_op_mode(struct iio_dev *indio_dev,
> > +             const struct iio_chan_spec *chan); static int
> > +sca3300_set_op_mode(struct iio_dev *indio_dev,
> > +             const struct iio_chan_spec *chan, unsigned int mode);
> > +
> > +static const struct iio_enum sca3300_op_mode_enum = {
> > +     .items = sca3300_op_modes,
> > +     .num_items = ARRAY_SIZE(sca3300_op_modes),
> > +     .get = sca3300_get_op_mode,
> > +     .set = sca3300_set_op_mode,
> > +};
> > +
> > +static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
> > +     IIO_ENUM("op_mode", IIO_SHARED_BY_DIR,
> &sca3300_op_mode_enum),
> > +     IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
> > +     { }
> > +};
> >
> >  enum sca3300_scan_indexes {
> >       SCA3300_ACC_X = 0,
> > @@ -70,6 +102,7 @@ enum sca3300_scan_indexes {
> >               .storagebits = 16,
> \
> >               .endianness = IIO_CPU,
> \
> >       },
> \
> > +     .ext_info = sca3300_ext_info,
> \
> >  }
> >
> >  #define SCA3300_TEMP_CHANNEL(index, reg)
> {                           \
> > @@ -400,6 +433,28 @@ static int sca3300_read_avail(struct iio_dev
> *indio_dev,
> >       }
> >  }
> >
> > +static int sca3300_get_op_mode(struct iio_dev *indio_dev,
> > +             const struct iio_chan_spec *chan) {
> > +     int mode;
> > +     int ret;
> > +     struct sca3300_data *data = iio_priv(indio_dev);
> > +
> > +     ret = sca3300_read_reg(data, SCA3300_REG_MODE, &mode);
> > +     if (ret)
> > +             return ret;
> > +     return mode;
> > +
> > +}
> > +
> > +static int sca3300_set_op_mode(struct iio_dev *indio_dev,
> > +             const struct iio_chan_spec *chan, unsigned int mode) {
> > +     struct sca3300_data *data = iio_priv(indio_dev);
> > +
> > +     return sca3300_write_reg(data, SCA3300_REG_MODE, mode); }
> > +
> >  static const struct iio_info sca3300_info = {
> >       .read_raw = sca3300_read_raw,
> >       .write_raw = sca3300_write_raw,


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes.
  2022-02-10 10:08     ` LI Qingwu
@ 2022-02-10 15:40       ` Jonathan Cameron
  0 siblings, 0 replies; 25+ messages in thread
From: Jonathan Cameron @ 2022-02-10 15:40 UTC (permalink / raw)
  To: LI Qingwu
  Cc: lars, robh+dt, tomas.melin, andy.shevchenko, devicetree,
	linux-kernel, linux-iio

On Thu, 10 Feb 2022 10:08:53 +0000
LI Qingwu <qing-wu.li@leica-geosystems.com.cn> wrote:

> Thanks a lot all of your inputs, I'm just back from long holiday and start to rework on the patches.
> 
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Sunday, January 30, 2022 7:40 PM
> > To: LI Qingwu <qing-wu.li@leica-geosystems.com.cn>
> > Cc: lars@metafoo.de; robh+dt@kernel.org; tomas.melin@vaisala.com;
> > andy.shevchenko@gmail.com; devicetree@vger.kernel.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation
> > modes.
> > 
> > This email is not from Hexagon’s Office 365 instance. Please be careful while
> > clicking links, opening attachments, or replying to this email.
> > 
> > 
> > On Mon, 24 Jan 2022 09:39:08 +0000
> > LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn> wrote:
> >   
> > > The acceleration scale and the frequency were set via operation modes,
> > > the scal and frequency are both non-uniqueness, this leads to logic
> > > confusion for setting scale.and.frequency.
> > > it getting worse if add more different sensor types into the driver.
> > >
> > > The commit add an interface for set and get the operation modes.
> > > the following interfaces added:
> > > in_accel_op_mode_available
> > > in_op_mode
> > >
> > > SCA3300 operation modes table:
> > > | Mode | Full-scale | low pass filter frequency |
> > > | ---- | ---------- | ------------------------- |
> > > | 1    | ± 3 g      | 70 Hz                     |
> > > | 2    | ± 6 g      | 70 Hz                     |
> > > | 3    | ± 1.5 g    | 70 Hz                     |
> > > | 4    | ± 1.5 g    | 10 Hz                     |
> > >
> > > Signed-off-by: LI Qingwu <Qing-wu.Li@leica-geosystems.com.cn>  
> > 
> > While it may seem convenient to expose this to userspace, the reality is that
> > generic userspace has no way to know how to use it.
> > 
> > That makes supplying this control a bad idea however convenient it may seem.
> > It's not unusual to have these sorts of constraints on devices and so the ABI
> > always assumes any setting may modify any other and / or change what is
> > available for a given setting.
> > 
> > If you need a particular combination for your own userspace, then make the
> > userspace aware of the constraints rather than exposing it as a 'mode' which
> > the userspace will need to know about anyway.
> > 
> > Jonathan  
> 

+cc linux-iio@vger.kernel.org

> Thanks a lot Jonathan, I couldn't agree with you more, the mode is not good for userspace,
> I would like to ask you how to handle this.
> Since the change for 'mode' was a prepare for support SCL3300,
> For SCL3300, mode 3 and mode 4 are totally same for both scale and frequency.
> The only different is mode 4 is low noise mode, but no difference from software point of view.
> Then it's impossible to set to between mode 3/4, let's say normal noise and low noise mode, with index of frequency and scale.
> Set between mode 3 and 4 is necessary, I have no idea how to handle it.

Why would a user ever select the 'high noise' option?
My guess is power saving?  Probably not enough to be relevant
in a system running Linux.

I would suggest just not supporting that option.
It is not uncommon for some modes to make limited sense and
to just be there as an artefact of the underlying hardware
architecture.  Doesn't mean we have to support them :)

Jonathan





> 
> | Mode             | Full-scale   | frequency |
> | -------------------      | ----------------- | ------------- |
> | 1                 | ± 1.2 g     | 40 Hz    |
> | 2                 | ± 2.4 g     | 70 Hz    |
> | 3                 | ± 0.6 g     | 10 Hz    |
> | 4 (Low noise mode)  | ± 0.6 g     | 10 Hz    |   
>                
> The link of the SCL3300 datasheet:
> https://www.murata.com/-/media/webrenewal/products/sensor/pdf/datasheet/datasheet_scl3300-d01.ashx?la=en&cvid=20210316063715000000
> 
> > 
> >   
> > > ---
> > >  drivers/iio/accel/sca3300.c | 55
> > > +++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 55 insertions(+)
> > >
> > > diff --git a/drivers/iio/accel/sca3300.c b/drivers/iio/accel/sca3300.c
> > > index 083ae2a47ad9..e26b3175b3c6 100644
> > > --- a/drivers/iio/accel/sca3300.c
> > > +++ b/drivers/iio/accel/sca3300.c
> > > @@ -42,6 +42,38 @@
> > >  /* Device return status and mask */
> > >  #define SCA3300_VALUE_RS_ERROR       0x3
> > >  #define SCA3300_MASK_RS_STATUS       GENMASK(1, 0)
> > > +enum sca3300_op_mode_indexes {
> > > +     OP_MOD_1 = 0,
> > > +     OP_MOD_2,
> > > +     OP_MOD_3,
> > > +     OP_MOD_4,
> > > +     OP_MOD_CNT
> > > +};
> > > +
> > > +static const char * const sca3300_op_modes[] = {
> > > +     [OP_MOD_1] = "1",
> > > +     [OP_MOD_2] = "2",
> > > +     [OP_MOD_3] = "3",
> > > +     [OP_MOD_4] = "4"
> > > +};
> > > +
> > > +static int sca3300_get_op_mode(struct iio_dev *indio_dev,
> > > +             const struct iio_chan_spec *chan); static int
> > > +sca3300_set_op_mode(struct iio_dev *indio_dev,
> > > +             const struct iio_chan_spec *chan, unsigned int mode);
> > > +
> > > +static const struct iio_enum sca3300_op_mode_enum = {
> > > +     .items = sca3300_op_modes,
> > > +     .num_items = ARRAY_SIZE(sca3300_op_modes),
> > > +     .get = sca3300_get_op_mode,
> > > +     .set = sca3300_set_op_mode,
> > > +};
> > > +
> > > +static const struct iio_chan_spec_ext_info sca3300_ext_info[] = {
> > > +     IIO_ENUM("op_mode", IIO_SHARED_BY_DIR,  
> > &sca3300_op_mode_enum),  
> > > +     IIO_ENUM_AVAILABLE("op_mode", &sca3300_op_mode_enum),
> > > +     { }
> > > +};
> > >
> > >  enum sca3300_scan_indexes {
> > >       SCA3300_ACC_X = 0,
> > > @@ -70,6 +102,7 @@ enum sca3300_scan_indexes {
> > >               .storagebits = 16,  
> > \  
> > >               .endianness = IIO_CPU,  
> > \  
> > >       },  
> > \  
> > > +     .ext_info = sca3300_ext_info,  
> > \  
> > >  }
> > >
> > >  #define SCA3300_TEMP_CHANNEL(index, reg)  
> > {                           \  
> > > @@ -400,6 +433,28 @@ static int sca3300_read_avail(struct iio_dev  
> > *indio_dev,  
> > >       }
> > >  }
> > >
> > > +static int sca3300_get_op_mode(struct iio_dev *indio_dev,
> > > +             const struct iio_chan_spec *chan) {
> > > +     int mode;
> > > +     int ret;
> > > +     struct sca3300_data *data = iio_priv(indio_dev);
> > > +
> > > +     ret = sca3300_read_reg(data, SCA3300_REG_MODE, &mode);
> > > +     if (ret)
> > > +             return ret;
> > > +     return mode;
> > > +
> > > +}
> > > +
> > > +static int sca3300_set_op_mode(struct iio_dev *indio_dev,
> > > +             const struct iio_chan_spec *chan, unsigned int mode) {
> > > +     struct sca3300_data *data = iio_priv(indio_dev);
> > > +
> > > +     return sca3300_write_reg(data, SCA3300_REG_MODE, mode); }
> > > +
> > >  static const struct iio_info sca3300_info = {
> > >       .read_raw = sca3300_read_raw,
> > >       .write_raw = sca3300_write_raw,  
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2022-02-10 15:34 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-24  9:39 [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 LI Qingwu
2022-01-24  9:39 ` [PATCH V1 1/6] iio: accel: sca3300: add define for temp channel for reuse LI Qingwu
2022-01-24  9:39 ` [PATCH V1 2/6] iio: accel: sca3300: Add interface for operation modes LI Qingwu
2022-01-24 13:03   ` Andy Shevchenko
2022-01-24 13:49   ` kernel test robot
2022-01-24 13:49     ` kernel test robot
2022-01-24 13:59   ` kernel test robot
2022-01-24 13:59     ` kernel test robot
2022-01-30 11:40   ` Jonathan Cameron
2022-02-10 10:08     ` LI Qingwu
2022-02-10 15:40       ` Jonathan Cameron
2022-01-24  9:39 ` [PATCH V1 3/6] iio: accel: sca3300: modified to support multi chips LI Qingwu
2022-01-24 13:08   ` Andy Shevchenko
2022-01-24 15:01   ` kernel test robot
2022-01-24 15:01     ` kernel test robot
2022-01-24 15:32   ` kernel test robot
2022-01-24 15:32     ` kernel test robot
2022-01-24  9:39 ` [PATCH V1 4/6] iio: accel: sca3300: Add support for SCL3300 LI Qingwu
2022-01-24 13:12   ` Andy Shevchenko
2022-01-24  9:39 ` [PATCH V1 5/6] iio: accel: sca3300: Add inclination channels LI Qingwu
2022-01-24 13:19   ` Andy Shevchenko
2022-01-30 11:46     ` Jonathan Cameron
2022-01-24  9:39 ` [PATCH V1 6/6] dt-bindings: iio: accel: sca3300: Document murata,scl3300 LI Qingwu
2022-02-09  3:08   ` Rob Herring
2022-01-30 11:35 ` [PATCH V1 0/6] i iio: accel: sca3300: add compitible for scl3300 Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.