Re: [PATCH v3] Added tilt interrupt support in inv_icm42600

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Hiten,

thanks for your patch.

I see first a big issue at the root. Tilt event is something that should be reported as an IIO event, not in a custom sysfs file. Jonathan can confirm this, but this is my understanding.

Second issue, there is no multiplexing between the tilt and normal data sampling. Meaning turning tilt off will stop the data output of the chip if it was on. And turning data output off will stop tilt functionnality. All these things have to be multiplexed together and chip power off/on must be centralized.

Thanks for your work.

Best regards,
JB

From: Hiten Chauhan <hiten.chauhan@xxxxxxxxxxxxxxxxx>
Sent: Friday, November 17, 2023 16:14
To: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@xxxxxxx>; jic23@xxxxxxxxxx <jic23@xxxxxxxxxx>; lars@xxxxxxxxxx <lars@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx <linux-iio@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>
Cc: Hiten Chauhan <hiten.chauhan@xxxxxxxxxxxxxxxxx>; kernel test robot <lkp@xxxxxxxxx>
Subject: [PATCH v3] Added tilt interrupt support in inv_icm42600 
 
Description: Add new device attribute to enable and disable Tilt interrupt from kernel user space Signed-off-by: Hiten Chauhan <hiten. chauhan@ siliconsignals. io> Reported-by: kernel test robot <lkp@ intel. com> Closes: https: //urldefense. com/v3/__https: //lore. kernel. org/oe-kbuild-all/202311170235. HaVJnmWa-lkp@ intel. com/__;!!FtrhtPsWDhZ6tw!Abqqh_UwyEydZ0xeIy7YQwPWb_knCM2hsJJWavoAq3igeGccV4RZI87CTV__lZgfBjZytNesx5cUc_RXsP6mu9lmvUGZg_rGWg$[lore[. ]kernel[. ]org] 
ZjQcmQRYFpfptBannerStart
This Message Is From an Untrusted Sender 
You have not previously corresponded with this sender. 
 
ZjQcmQRYFpfptBannerEnd
Description:
Add new device attribute to enable and disable
Tilt interrupt from kernel user space

Signed-off-by: Hiten Chauhan <hiten.chauhan@xxxxxxxxxxxxxxxxx>

Reported-by: kernel test robot <lkp@xxxxxxxxx>
Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe-kbuild-all/202311170235.HaVJnmWa-lkp@xxxxxxxxx/__;!!FtrhtPsWDhZ6tw!Abqqh_UwyEydZ0xeIy7YQwPWb_knCM2hsJJWavoAq3igeGccV4RZI87CTV__lZgfBjZytNesx5cUc_RXsP6mu9lmvUGZg_rGWg$[lore[.]kernel[.]org]
---
 drivers/iio/imu/inv_icm42600/inv_icm42600.h   |  24 ++++
 .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 129 ++++++++++++++++++
 2 files changed, 153 insertions(+)

diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
index 0e290c807b0f..39ed39e77deb 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
@@ -187,6 +187,8 @@ struct inv_icm42600_state {
 #define INV_ICM42600_FIFO_CONFIG_STOP_ON_FULL		\
 		FIELD_PREP(INV_ICM42600_FIFO_CONFIG_MASK, 2)
 
+#define INV_ICM42600_REG_MASK        GENMASK(7, 0)
+
 /* all sensor data are 16 bits (2 registers wide) in big-endian */
 #define INV_ICM42600_REG_TEMP_DATA			0x001D
 #define INV_ICM42600_REG_ACCEL_DATA_X			0x001F
@@ -239,6 +241,7 @@ struct inv_icm42600_state {
 #define INV_ICM42600_REG_PWR_MGMT0			0x004E
 #define INV_ICM42600_PWR_MGMT0_TEMP_DIS			BIT(5)
 #define INV_ICM42600_PWR_MGMT0_IDLE			BIT(4)
+#define INV_ICM42600_PWR_ACCEL_MODE			BIT(1)
 #define INV_ICM42600_PWR_MGMT0_GYRO(_mode)		\
 		FIELD_PREP(GENMASK(3, 2), (_mode))
 #define INV_ICM42600_PWR_MGMT0_ACCEL(_mode)		\
@@ -306,6 +309,21 @@ struct inv_icm42600_state {
 #define INV_ICM42600_WHOAMI_ICM42622			0x46
 #define INV_ICM42600_WHOAMI_ICM42631			0x5C
 
+/* Register configs for tilt interrupt */
+#define INV_ICM42605_REG_APEX_CONFIG4                  0x4043
+#define INV_ICM42605_APEX_CONFIG4_MASK                 GENMASK(7, 0)
+
+#define INV_ICM42605_REG_APEX_CONFIG0                  0x0056
+#define INV_ICM42605_APEX_CONFIG0_TILT_ENABLE          BIT(4)
+#define INV_ICM42605_APEX_CONFIG0                      BIT(1)
+
+#define INV_ICM42605_REG_INTF_CONFIG1                   0x404D
+#define INV_ICM42605_INTF_CONFIG1_MASK                  GENMASK(5, 0)
+#define INV_ICM42605_INTF_CONFIG1_TILT_DET_INT1_EN      BIT(3)
+
+#define INV_ICM42605_REG_INT_STATUS3                   0x0038
+
+
 /* User bank 1 (MSB 0x10) */
 #define INV_ICM42600_REG_SENSOR_CONFIG0			0x1003
 #define INV_ICM42600_SENSOR_CONFIG0_ZG_DISABLE		BIT(5)
@@ -364,6 +382,8 @@ typedef int (*inv_icm42600_bus_setup)(struct inv_icm42600_state *);
 extern const struct regmap_config inv_icm42600_regmap_config;
 extern const struct dev_pm_ops inv_icm42600_pm_ops;
 
+extern uint8_t inv_icm42605_int_reg;
+
 const struct iio_mount_matrix *
 inv_icm42600_get_mount_matrix(const struct iio_dev *indio_dev,
 			      const struct iio_chan_spec *chan);
@@ -395,4 +415,8 @@ struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st);
 
 int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev);
 
+int inv_icm42605_generate_tilt_interrupt(struct inv_icm42600_state *st);
+
+int inv_icm42605_disable_tilt_interrupt(struct inv_icm42600_state *st);
+
 #endif
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
index b1e4fde27d25..311f6ea09e64 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
@@ -47,6 +47,8 @@
 		.ext_info = _ext_info,					\
 	}
 
+uint8_t inv_icm42605_int_reg;
+
 enum inv_icm42600_accel_scan {
 	INV_ICM42600_ACCEL_SCAN_X,
 	INV_ICM42600_ACCEL_SCAN_Y,
@@ -60,6 +62,68 @@ static const struct iio_chan_spec_ext_info inv_icm42600_accel_ext_infos[] = {
 	{},
 };
 
+static ssize_t tilt_interrupt_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct inv_icm42600_state *st = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(st->map, inv_icm42605_int_reg, &val);
+
+	if (ret != 0)
+		return ret;
+
+	snprintf(buf, PAGE_SIZE, "Read reg %x value %x\n", inv_icm42605_int_reg, val);
+
+	return strlen(buf);
+}
+
+static ssize_t tilt_interrupt_store(struct device *dev,
+		struct device_attribute *attr, const char *buf,
+		size_t count)
+{
+	struct inv_icm42600_state *st = dev_get_drvdata(dev);
+	int ret;
+	int value;
+
+	if (!st)
+		return -EINVAL;
+
+	if (kstrtoint(buf, 10, &value))
+		return -EINVAL;
+
+	inv_icm42605_int_reg = INV_ICM42605_REG_INT_STATUS3;
+
+	switch (value) {
+	case 1:
+		ret = inv_icm42605_generate_tilt_interrupt(st);
+		if (ret != 0)
+			return -EIO;
+		break;
+	case 0:
+		ret = inv_icm42605_disable_tilt_interrupt(st);
+		if (ret != 0)
+			return -EIO;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(tilt_interrupt);
+
+static struct attribute *icm42605_attrs[] = {
+	&dev_attr_tilt_interrupt.attr,
+	NULL,
+};
+
+static const struct attribute_group icm42605_attrs_group = {
+	.attrs = icm42605_attrs,
+};
+
 static const struct iio_chan_spec inv_icm42600_accel_channels[] = {
 	INV_ICM42600_ACCEL_CHAN(IIO_MOD_X, INV_ICM42600_ACCEL_SCAN_X,
 				inv_icm42600_accel_ext_infos),
@@ -702,6 +766,7 @@ static const struct iio_info inv_icm42600_accel_info = {
 	.update_scan_mode = inv_icm42600_accel_update_scan_mode,
 	.hwfifo_set_watermark = inv_icm42600_accel_hwfifo_set_watermark,
 	.hwfifo_flush_to_buffer = inv_icm42600_accel_hwfifo_flush,
+	.attrs = &icm42605_attrs_group,
 };
 
 struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st)
@@ -791,3 +856,67 @@ int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev)
 
 	return 0;
 }
+
+int inv_icm42605_generate_tilt_interrupt(struct inv_icm42600_state *st)
+{
+	int ret;
+	int val;
+	char sleep = 10;
+
+	ret = regmap_update_bits(st->map, INV_ICM42605_REG_APEX_CONFIG4,
+				 INV_ICM42605_APEX_CONFIG4_MASK, 0);
+	if (ret)
+		return ret;
+
+	val = INV_ICM42600_PWR_ACCEL_MODE;
+	ret = regmap_write(st->map, INV_ICM42600_REG_PWR_MGMT0, val);
+	if (ret)
+		return ret;
+
+	val = INV_ICM42605_APEX_CONFIG0;
+	ret = regmap_write(st->map, INV_ICM42605_REG_APEX_CONFIG0, val);
+	if (ret)
+		return ret;
+
+	val = INV_ICM42600_SIGNAL_PATH_RESET_DMP_MEM_RESET;
+	ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET, val);
+	if (ret)
+		return ret;
+
+	msleep(sleep);
+
+	val = INV_ICM42600_SIGNAL_PATH_RESET_DMP_INIT_EN;
+	ret = regmap_write(st->map, INV_ICM42600_REG_SIGNAL_PATH_RESET, val);
+	if (ret)
+		return ret;
+
+	val = INV_ICM42605_APEX_CONFIG0_TILT_ENABLE |
+	      INV_ICM42605_APEX_CONFIG0;
+	ret = regmap_write(st->map, INV_ICM42605_REG_APEX_CONFIG0, val);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(st->map, INV_ICM42605_REG_INTF_CONFIG1,
+				 INV_ICM42605_INTF_CONFIG1_MASK,
+				 INV_ICM42605_INTF_CONFIG1_TILT_DET_INT1_EN);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+int inv_icm42605_disable_tilt_interrupt(struct inv_icm42600_state *st)
+{
+	int ret;
+
+	ret = regmap_write(st->map, INV_ICM42605_REG_APEX_CONFIG0, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(st->map, INV_ICM42605_REG_INTF_CONFIG1,
+			INV_ICM42605_INTF_CONFIG1_MASK, 0);
+	if (ret)
+		return ret;
+
+	return 0;
+}

base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
-- 
2.25.1





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux