linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/2] iio: magnetometer: add support for Infineon TLV493D 3D Magnetic Sensor
@ 2025-08-29  2:53 Dixit Parmar
  2025-08-29  2:53 ` [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor Dixit Parmar
  2025-08-29  2:53 ` [PATCH v5 2/2] dt-bindings: iio: magnetometer: document Infineon TLV493D 3D Magnetic sensor Dixit Parmar
  0 siblings, 2 replies; 9+ messages in thread
From: Dixit Parmar @ 2025-08-29  2:53 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-kernel, linux-iio, devicetree, Dixit Parmar,
	Krzysztof Kozlowski

The Infineon TLV493D is a Low-Power 3D Magnetic Sensor. The Sensor
applications includes joysticks, control elements (white goods,
multifunction knops), or electric meters (anti tampering) and any
other application that requires accurate angular measurements at
low power consumptions.

The Sensor is configured over I2C, and as part of Sensor measurement
data it provides 3-Axis magnetic fields and temperature core measurement.

The driver supports raw value read and buffered input via external trigger
to allow streaming values with the same sensing timestamp.

While the sensor has an interrupt pin multiplexed with an I2C SCL pin.
But for bus configurations interrupt(INT) is not recommended, unless timing
constraints between I2C data transfers and interrupt pulses are monitored
and aligned.

The Sensor's I2C register map and mode information is described in product
User Manual [1].

Datasheet: https://www.infineon.com/assets/row/public/documents/24/49/infineon-tlv493d-a1b6-datasheet-en.pdf
Link: https://www.mouser.com/pdfDocs/Infineon-TLV493D-A1B6_3DMagnetic-UserManual-v01_03-EN.pdf [1]
Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
---
Changes in v5:
- Use USEC_PER_MSEC for 1000 multiplier.
- Change if (ret < 0) -> if (ret).
- Style and indentation fixes.
- Added entry in MAINTAINERS file.
- Link to v4: https://lore.kernel.org/r/20250814-tlv493d-sensor-v6_16-rc5-v4-0-81b82805aae0@gmail.com

Changes in v4:
- Include required headers.
- Drop struct device *dev from the sensor data structure and use
  i2c_client *client to derive it wherever needed.
- Use FIELD_MODIFY instead of FIELD_PREP to keep other bits intacts.
- Remove unused defines TLV493D_WR_REG_RES*.
- Drop the pm_runtime_mark_last_busy(). As now always called in the
  pm_runtime_put_autosuspend().
- Change goto labels to make it more descriptive.
- Fix style & typo errors.
- Link to v3: https://lore.kernel.org/r/20250807-tlv493d-sensor-v6_16-rc5-v3-0-b80d2cb41232@gmail.com

Changes in v3:
- dt-binding: rename to infineon,tlv493d-a1b6.yaml.
- dt-binding: fix convention errors.
- Switch to using enums for mode and channel where applicable.
- Drop unnecesary input argument checks in functions.
- Switch to u32 array for mode sample rate timing from single member struct.
- Add note describing the I2C communication characteristics at the top.
- Drop tlv493d_setup_ops and use NULL directly.
- Fix typos in comments and commit messages.
- Link to v2: https://lore.kernel.org/r/20250802-tlv493d-sensor-v6_16-rc5-v2-0-e867df86ad93@gmail.com

Changes in v2:
- Drop regmap implementation in favor of using direct i2c APIs to
  have uniform communication APIs across the driver.
- Remove custom device-tree properties as suggested and hardcode
  setting operating mode in probe().
- Derive and hardcode temperature offset from raw offset and compensation.
- Add missing device name(tlv493_) prefix in global variables.
- Change float operation with multiplier to fixed value(1100).
- Change Magnetic field reporting to Guass SI unit.
- User FIELD_PREP instead of direct bitwise ops.
- Convert sensor channel parsing logic from Macro to function for
  better readability.
- Discard unused #define's.
- Discard IIO_CHAN_INFO_PROCESSED.
- Maintain alphabetical order of config options in Makefile and Kconfig.
- Readability fixes.
- Link to v1: https://lore.kernel.org/r/20250726-tlv493d-sensor-v6_16-rc5-v1-0-deac027e6f32@gmail.com

---
Dixit Parmar (2):
      iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor
      dt-bindings: iio: magnetometer: document Infineon TLV493D 3D Magnetic sensor

 .../iio/magnetometer/infineon,tlv493d-a1b6.yaml    |  45 ++
 .../devicetree/bindings/trivial-devices.yaml       |   2 -
 MAINTAINERS                                        |   8 +
 drivers/iio/magnetometer/Kconfig                   |  13 +
 drivers/iio/magnetometer/Makefile                  |   2 +
 drivers/iio/magnetometer/tlv493d.c                 | 533 +++++++++++++++++++++
 6 files changed, 601 insertions(+), 2 deletions(-)
---
base-commit: d7b8f8e20813f0179d8ef519541a3527e7661d3a
change-id: 20250726-tlv493d-sensor-v6_16-rc5-18c712093b27

Best regards,
-- 
Dixit Parmar <dixitparmar19@gmail.com>


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

* [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor
  2025-08-29  2:53 [PATCH v5 0/2] iio: magnetometer: add support for Infineon TLV493D 3D Magnetic Sensor Dixit Parmar
@ 2025-08-29  2:53 ` Dixit Parmar
  2025-08-29  6:52   ` Krzysztof Kozlowski
  2025-08-30 16:19   ` Jonathan Cameron
  2025-08-29  2:53 ` [PATCH v5 2/2] dt-bindings: iio: magnetometer: document Infineon TLV493D 3D Magnetic sensor Dixit Parmar
  1 sibling, 2 replies; 9+ messages in thread
From: Dixit Parmar @ 2025-08-29  2:53 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-kernel, linux-iio, devicetree, Dixit Parmar

The Infineon TLV493D is a Low-Power 3D Magnetic Sensor. The Sensor
applications includes joysticks, control elements (white goods,
multifunction knops), or electric meters (anti tampering) and any
other application that requires accurate angular measurements at
low power consumptions.

The Sensor is configured over I2C, and as part of Sensor measurement
data it provides 3-Axis magnetic fields and temperature core measurement.

The driver supports raw value read and buffered input via external trigger
to allow streaming values with the same sensing timestamp.

While the sensor has an interrupt pin multiplexed with an I2C SCL pin.
But for bus configurations interrupt(INT) is not recommended, unless timing
constraints between I2C data transfers and interrupt pulses are monitored
and aligned.

The Sensor's I2C register map and mode information is described in product
User Manual [1].

Datasheet: https://www.infineon.com/assets/row/public/documents/24/49/infineon-tlv493d-a1b6-datasheet-en.pdf
Link: https://www.mouser.com/pdfDocs/Infineon-TLV493D-A1B6_3DMagnetic-UserManual-v01_03-EN.pdf [1]
Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
---
 MAINTAINERS                        |   8 +
 drivers/iio/magnetometer/Kconfig   |  13 +
 drivers/iio/magnetometer/Makefile  |   2 +
 drivers/iio/magnetometer/tlv493d.c | 533 +++++++++++++++++++++++++++++++++++++
 4 files changed, 556 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fad6cb025a19..cf0a00f5c4d4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11843,6 +11843,14 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/sound/infineon,peb2466.yaml
 F:	sound/soc/codecs/peb2466.c
 
+INFINEON TLV493D Driver
+M:	Dixit Parmar <dixitparmar19@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+W:	https://www.infineon.com/part/TLV493D-A1B6
+F:	Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml
+F:	drivers/iio/magnetometer/tlv493d.c
+
 INFINIBAND SUBSYSTEM
 M:	Jason Gunthorpe <jgg@nvidia.com>
 M:	Leon Romanovsky <leonro@nvidia.com>
diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index 3debf1320ad1..714128df944d 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -173,6 +173,19 @@ config IIO_ST_MAGN_SPI_3AXIS
 	  To compile this driver as a module, choose M here. The module
 	  will be called st_magn_spi.
 
+config INFINEON_TLV493D
+	tristate "Infineon TLV493D Low-Power 3D Magnetic Sensor"
+	depends on I2C
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say Y here to add support for the Infineon TLV493D-A1B6 Low-
+	  Power 3D Magnetic Sensor.
+
+	  This driver can also be compiled as a module.
+	  To compile this driver as a module, choose M here: the module
+	  will be called tlv493d.
+
 config SENSORS_HMC5843
 	tristate
 	select IIO_BUFFER
diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
index 9297723a97d8..dfe970fcacb8 100644
--- a/drivers/iio/magnetometer/Makefile
+++ b/drivers/iio/magnetometer/Makefile
@@ -23,6 +23,8 @@ st_magn-$(CONFIG_IIO_BUFFER) += st_magn_buffer.o
 obj-$(CONFIG_IIO_ST_MAGN_I2C_3AXIS) += st_magn_i2c.o
 obj-$(CONFIG_IIO_ST_MAGN_SPI_3AXIS) += st_magn_spi.o
 
+obj-$(CONFIG_INFINEON_TLV493D)		+= tlv493d.o
+
 obj-$(CONFIG_SENSORS_HMC5843)		+= hmc5843_core.o
 obj-$(CONFIG_SENSORS_HMC5843_I2C)	+= hmc5843_i2c.o
 obj-$(CONFIG_SENSORS_HMC5843_SPI)	+= hmc5843_spi.o
diff --git a/drivers/iio/magnetometer/tlv493d.c b/drivers/iio/magnetometer/tlv493d.c
new file mode 100644
index 000000000000..b723eaac1d9e
--- /dev/null
+++ b/drivers/iio/magnetometer/tlv493d.c
@@ -0,0 +1,533 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/**
+ * Driver for the Infineon TLV493D Low-Power 3D Magnetic Sensor
+ *
+ * Copyright (C) 2025 Dixit Parmar <dixitparmar19@gmail.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/i2c.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+
+/*
+ * TLV493D sensor I2C communication note:
+ *
+ * The sensor supports only direct byte-stream write starting from the
+ * register address 0x0. So for any modification to be made to any write
+ * registers, it must be written starting from the register address  0x0.
+ * I2C write operation should not contain the register address in the I2C
+ * frame, it should contain only raw byte stream for the write registers.
+ * I2C Frame: |S|SlaveAddr Wr|Ack|Byte[0]|Ack|Byte[1]|Ack|.....|Sp|
+ *
+ * Same as the write operation, reading from the sensor registers is also
+ * performed starting from the register address 0x0 for as many bytes as
+ * need to be read.
+ * I2C read operation should not contain the register address in the I2C frame.
+ * I2C Frame: |S|SlaveAddr Rd|Ack|Byte[0]|Ack|Byte[1]|Ack|.....|Sp|
+ */
+
+#define TLV493D_RD_REG_BX	0x00
+#define TLV493D_RD_REG_BY	0x01
+#define TLV493D_RD_REG_BZ	0x02
+#define TLV493D_RD_REG_TEMP	0x03
+#define TLV493D_RD_REG_BX2	0x04
+#define TLV493D_RD_REG_BZ2	0x05
+#define TLV493D_RD_REG_TEMP2	0x06
+#define TLV493D_RD_REG_RES1	0x07
+#define TLV493D_RD_REG_RES2	0x08
+#define TLV493D_RD_REG_RES3	0x09
+#define TLV493D_RD_REG_MAX	0x0a
+
+#define TLV493D_WR_REG_MODE1	0x01
+#define TLV493D_WR_REG_MODE2	0x03
+#define TLV493D_WR_REG_MAX	0x04
+
+#define TLV493D_BX_MAG_X_AXIS_MSB	GENMASK(7, 0)
+#define TLV493D_BX2_MAG_X_AXIS_LSB	GENMASK(7, 4)
+#define TLV493D_BY_MAG_Y_AXIS_MSB	GENMASK(7, 0)
+#define TLV493D_BX2_MAG_Y_AXIS_LSB	GENMASK(3, 0)
+#define TLV493D_BZ_MAG_Z_AXIS_MSB	GENMASK(7, 0)
+#define TLV493D_BZ2_MAG_Z_AXIS_LSB	GENMASK(3, 0)
+#define TLV493D_TEMP_TEMP_MSB		GENMASK(7, 4)
+#define TLV493D_TEMP2_TEMP_LSB		GENMASK(7, 0)
+#define TLV493D_TEMP_CHANNEL		GENMASK(1, 0)
+#define TLV493D_MODE1_MOD_LOWFAST	GENMASK(1, 0)
+#define TLV493D_MODE2_LP_PERIOD	BIT(6)
+#define TLV493D_RD_REG_RES1_WR_MASK	GENMASK(4, 3)
+#define TLV493D_RD_REG_RES2_WR_MASK	GENMASK(7, 0)
+#define TLV493D_RD_REG_RES3_WR_MASK	GENMASK(4, 0)
+
+enum tlv493d_channels {
+	TLV493D_AXIS_X,
+	TLV493D_AXIS_Y,
+	TLV493D_AXIS_Z,
+	TLV493D_TEMPERATURE,
+};
+
+enum tlv493d_op_mode {
+	TLV493D_OP_MODE_POWERDOWN,
+	TLV493D_OP_MODE_FAST,
+	TLV493D_OP_MODE_LOWPOWER,
+	TLV493D_OP_MODE_ULTRA_LOWPOWER,
+	TLV493D_OP_MODE_MASTERCONTROLLED,
+};
+
+struct tlv493d_data {
+	struct i2c_client *client;
+	/* protects from simultaneous sensor access and register readings */
+	struct mutex lock;
+	enum tlv493d_op_mode mode;
+	u8 wr_regs[TLV493D_WR_REG_MAX];
+};
+
+/*
+ * Different mode has different measurement sampling time, this time is
+ * used in deriving the sleep and timeout while reading the data from
+ * sensor in polling.
+ * Power-down mode: No measurement.
+ * Fast mode: Freq:3.3 KHz. Measurement time:305 usec.
+ * Low-power mode: Freq:100 Hz. Measurement time:10 msec.
+ * Ultra low-power mode: Freq:10 Hz. Measurement time:100 msec.
+ * Master controlled mode: Freq:3.3 Khz. Measurement time:305 usec.
+ */
+static const u32 tlv493d_sample_rate_us[] = {
+	[TLV493D_OP_MODE_POWERDOWN] = 0,
+	[TLV493D_OP_MODE_FAST] = 305,
+	[TLV493D_OP_MODE_LOWPOWER] = 10 * USEC_PER_MSEC,
+	[TLV493D_OP_MODE_ULTRA_LOWPOWER] = 100 * USEC_PER_MSEC,
+	[TLV493D_OP_MODE_MASTERCONTROLLED] = 305,
+};
+
+static int tlv493d_write_all_regs(struct tlv493d_data *data)
+{
+	int ret;
+	struct device *dev = &data->client->dev;
+
+	ret = i2c_master_send(data->client, data->wr_regs, ARRAY_SIZE(data->wr_regs));
+	if (ret < 0) {
+		dev_err(dev, "i2c write registers failed, error: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tlv493d_set_operating_mode(struct tlv493d_data *data, enum tlv493d_op_mode mode)
+{
+	u8 *mode1_cfg = &data->wr_regs[TLV493D_WR_REG_MODE1];
+	u8 *mode2_cfg = &data->wr_regs[TLV493D_WR_REG_MODE2];
+
+	switch (mode) {
+	case TLV493D_OP_MODE_POWERDOWN:
+		FIELD_MODIFY(TLV493D_MODE1_MOD_LOWFAST, mode1_cfg, 0);
+		FIELD_MODIFY(TLV493D_MODE2_LP_PERIOD, mode2_cfg, 0);
+		break;
+
+	case TLV493D_OP_MODE_FAST:
+		FIELD_MODIFY(TLV493D_MODE1_MOD_LOWFAST, mode1_cfg, 1);
+		FIELD_MODIFY(TLV493D_MODE2_LP_PERIOD, mode2_cfg, 0);
+		break;
+
+	case TLV493D_OP_MODE_LOWPOWER:
+		FIELD_MODIFY(TLV493D_MODE1_MOD_LOWFAST, mode1_cfg, 2);
+		FIELD_MODIFY(TLV493D_MODE2_LP_PERIOD, mode2_cfg, 1);
+		break;
+
+	case TLV493D_OP_MODE_ULTRA_LOWPOWER:
+		FIELD_MODIFY(TLV493D_MODE1_MOD_LOWFAST, mode1_cfg, 2);
+		FIELD_MODIFY(TLV493D_MODE2_LP_PERIOD, mode2_cfg, 0);
+		break;
+
+	case TLV493D_OP_MODE_MASTERCONTROLLED:
+		FIELD_MODIFY(TLV493D_MODE1_MOD_LOWFAST, mode1_cfg, 3);
+		FIELD_MODIFY(TLV493D_MODE2_LP_PERIOD, mode2_cfg, 0);
+		break;
+	}
+
+	return tlv493d_write_all_regs(data);
+}
+
+static s16 tlv493d_get_channel_data(u8 *b, enum tlv493d_channels ch)
+{
+	u16 val;
+
+	switch (ch) {
+	case TLV493D_AXIS_X:
+		val = FIELD_GET(TLV493D_BX_MAG_X_AXIS_MSB, b[TLV493D_RD_REG_BX]) << 4 |
+		      FIELD_GET(TLV493D_BX2_MAG_X_AXIS_LSB, b[TLV493D_RD_REG_BX2]) >> 4;
+		break;
+	case TLV493D_AXIS_Y:
+		val = FIELD_GET(TLV493D_BY_MAG_Y_AXIS_MSB, b[TLV493D_RD_REG_BY]) << 4 |
+		      FIELD_GET(TLV493D_BX2_MAG_Y_AXIS_LSB, b[TLV493D_RD_REG_BX2]);
+		break;
+	case TLV493D_AXIS_Z:
+		val = FIELD_GET(TLV493D_BZ_MAG_Z_AXIS_MSB, b[TLV493D_RD_REG_BZ]) << 4 |
+		      FIELD_GET(TLV493D_BZ2_MAG_Z_AXIS_LSB, b[TLV493D_RD_REG_BZ2]);
+		break;
+	case TLV493D_TEMPERATURE:
+		val = FIELD_GET(TLV493D_TEMP_TEMP_MSB, b[TLV493D_RD_REG_TEMP]) << 8 |
+		      FIELD_GET(TLV493D_TEMP2_TEMP_LSB, b[TLV493D_RD_REG_TEMP2]);
+		break;
+	}
+
+	return sign_extend32(val, 11);
+}
+
+static int tlv493d_get_measurements(struct tlv493d_data *data, s16 *x, s16 *y,
+				    s16 *z, s16 *t)
+{
+	u8 buff[7] = {};
+	int err, ret;
+	struct device *dev = &data->client->dev;
+	u32 sleep_us = tlv493d_sample_rate_us[data->mode];
+
+	guard(mutex)(&data->lock);
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Poll until data is valid,
+	 * For a valid data TLV493D_TEMP_CHANNEL bit of TLV493D_RD_REG_TEMP
+	 * should be set to 0. The sampling time depends on the sensor mode.
+	 * Poll 3x the time of the sampling time.
+	 */
+	ret = read_poll_timeout(i2c_master_recv, err,
+			err || !FIELD_GET(TLV493D_TEMP_CHANNEL,
+			buff[TLV493D_RD_REG_TEMP]),
+			sleep_us, 3 * sleep_us, false, data->client, buff,
+			ARRAY_SIZE(buff));
+	if (ret) {
+		dev_err(dev, "i2c read poll timeout, error:%d\n", ret);
+		goto out_put_autosuspend;
+	}
+	if (err < 0) {
+		dev_err(dev, "i2c read data failed, error:%d\n", err);
+		ret = err;
+		goto out_put_autosuspend;
+	}
+
+	*x = tlv493d_get_channel_data(buff, TLV493D_AXIS_X);
+	*y = tlv493d_get_channel_data(buff, TLV493D_AXIS_Y);
+	*z = tlv493d_get_channel_data(buff, TLV493D_AXIS_Z);
+	*t = tlv493d_get_channel_data(buff, TLV493D_TEMPERATURE);
+
+out_put_autosuspend:
+	pm_runtime_put_autosuspend(dev);
+	return ret;
+}
+
+static int tlv493d_init(struct tlv493d_data *data)
+{
+	int ret;
+	u8 buff[TLV493D_RD_REG_MAX];
+	struct device *dev = &data->client->dev;
+
+	/*
+	 * The sensor initialization requires below steps to be followed,
+	 * 1. Power-up sensor.
+	 * 2. Read and store read-registers map (0x0-0x9).
+	 * 3. Copy values from read reserved registers to write reserved fields
+	 *    (0x0-0x3).
+	 * 4. Set operating mode.
+	 * 5. Write to all registers.
+	 */
+	ret = i2c_master_recv(data->client, buff, ARRAY_SIZE(buff));
+	if (ret < 0) {
+		dev_err(dev, "i2c read failed, error %d\n", ret);
+		return ret;
+	}
+
+	/* Write register 0x0 is reserved. Does not require to be updated.*/
+	data->wr_regs[0] = 0;
+	data->wr_regs[1] = buff[TLV493D_RD_REG_RES1] & TLV493D_RD_REG_RES1_WR_MASK;
+	data->wr_regs[2] = buff[TLV493D_RD_REG_RES2] & TLV493D_RD_REG_RES2_WR_MASK;
+	data->wr_regs[3] = buff[TLV493D_RD_REG_RES3] & TLV493D_RD_REG_RES3_WR_MASK;
+
+	ret = tlv493d_set_operating_mode(data, data->mode);
+	if (ret < 0) {
+		dev_err(dev, "failed to set operating mode\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tlv493d_read_raw(struct iio_dev *indio_dev,
+			const struct iio_chan_spec *chan, int *val,
+			int *val2, long mask)
+{
+	struct tlv493d_data *data = iio_priv(indio_dev);
+	s16 x, y, z, t;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = tlv493d_get_measurements(data, &x, &y, &z, &t);
+		if (ret)
+			return ret;
+
+		/* Return raw values for requested channel */
+		switch (chan->address) {
+		case TLV493D_AXIS_X:
+			*val = x;
+			return IIO_VAL_INT;
+		case TLV493D_AXIS_Y:
+			*val = y;
+			return IIO_VAL_INT;
+		case TLV493D_AXIS_Z:
+			*val = z;
+			return IIO_VAL_INT;
+		case TLV493D_TEMPERATURE:
+			*val = t;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_MAGN:
+			/*
+			 * Magnetic field scale: 0.0098 mTesla (i.e. 9.8 µT)
+			 * Magnetic field in Gauss: mT * 10 = 0.098.
+			 */
+			*val = 98;
+			*val2 = 1000;
+			return IIO_VAL_FRACTIONAL;
+		case IIO_TEMP:
+			/*
+			 * Temperature scale: 1.1 °C per LSB, expressed as 1100 m°C
+			 * Returned as integer for IIO core to apply:
+			 * temp = (raw + offset) * scale
+			 */
+			*val = 1100;
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_OFFSET:
+		switch (chan->type) {
+		case IIO_TEMP:
+			/*
+			 * Temperature offset includes sensor-specific raw offset
+			 * plus compensation for +25°C bias in formula.
+			 * offset = -raw_offset + (25000 / 1100)
+			 * -340 + 22.72 = -317.28
+			 */
+			*val = -31728;
+			*val2 = 100;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static irqreturn_t tlv493d_trigger_handler(int irq, void *ptr)
+{
+	int ret;
+	s16 x, y, z, t;
+	struct iio_poll_func *pf = ptr;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct tlv493d_data *data = iio_priv(indio_dev);
+	struct device *dev = &data->client->dev;
+	struct {
+		s16 channels[3];
+		s16 temperature;
+		aligned_s64 timestamp;
+	} scan;
+
+	ret = tlv493d_get_measurements(data, &x, &y, &z, &t);
+	if (ret) {
+		dev_err(dev, "failed to read sensor data\n");
+		goto out_trigger_notify;
+	}
+
+	scan.channels[0] = x;
+	scan.channels[1] = y;
+	scan.channels[2] = z;
+	scan.temperature = t;
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), pf->timestamp);
+
+out_trigger_notify:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+#define TLV493D_AXIS_CHANNEL(axis, index)			\
+	{							\
+		.type = IIO_MAGN,				\
+		.modified = 1,					\
+		.channel2 = IIO_MOD_##axis,			\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+				BIT(IIO_CHAN_INFO_SCALE),	\
+		.address = index,				\
+		.scan_index = index,				\
+		.scan_type = {					\
+			.sign = 's',				\
+			.realbits = 12,				\
+			.storagebits = 16,			\
+			.endianness = IIO_CPU,			\
+		},						\
+	}
+
+static const struct iio_chan_spec tlv493d_channels[] = {
+	TLV493D_AXIS_CHANNEL(X, TLV493D_AXIS_X),
+	TLV493D_AXIS_CHANNEL(Y, TLV493D_AXIS_Y),
+	TLV493D_AXIS_CHANNEL(Z, TLV493D_AXIS_Z),
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				BIT(IIO_CHAN_INFO_SCALE) |
+				BIT(IIO_CHAN_INFO_OFFSET),
+		.address = TLV493D_TEMPERATURE,
+		.scan_index = TLV493D_TEMPERATURE,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 12,
+			.storagebits = 16,
+			.endianness = IIO_CPU,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
+static const struct iio_info tlv493d_info = {
+	.read_raw = tlv493d_read_raw,
+};
+
+static const unsigned long tlv493d_scan_masks[] = { GENMASK(3, 0), 0 };
+
+static int tlv493d_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct iio_dev *indio_dev;
+	struct tlv493d_data *data;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+	i2c_set_clientdata(client, indio_dev);
+
+	ret = devm_mutex_init(dev, &data->lock);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to enable regulator\n");
+
+	/*
+	 * Setting Sensor default operating mode to Master-Controlled mode since
+	 * it performs measurement cycle only on-request and stays in Power-Down
+	 * state until next cycle is initiated.
+	 */
+	data->mode = TLV493D_OP_MODE_MASTERCONTROLLED;
+	ret = tlv493d_init(data);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to initialize\n");
+
+	indio_dev->info = &tlv493d_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->name = client->name;
+	indio_dev->channels = tlv493d_channels;
+	indio_dev->num_channels = ARRAY_SIZE(tlv493d_channels);
+	indio_dev->available_scan_masks = tlv493d_scan_masks;
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+				iio_pollfunc_store_time,
+				tlv493d_trigger_handler,
+				NULL);
+	if (ret)
+		return dev_err_probe(dev, ret, "iio triggered buffer setup failed\n");
+
+	ret = pm_runtime_set_active(dev);
+	if (ret)
+		return ret;
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	pm_runtime_get_noresume(dev);
+	pm_runtime_set_autosuspend_delay(dev, 500);
+	pm_runtime_use_autosuspend(dev);
+
+	pm_runtime_put_autosuspend(dev);
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "iio device register failed\n");
+
+	return 0;
+}
+
+static int tlv493d_runtime_suspend(struct device *dev)
+{
+	struct tlv493d_data *data = iio_priv(dev_get_drvdata(dev));
+
+	return tlv493d_set_operating_mode(data, TLV493D_OP_MODE_POWERDOWN);
+}
+
+static int tlv493d_runtime_resume(struct device *dev)
+{
+	struct tlv493d_data *data = iio_priv(dev_get_drvdata(dev));
+
+	return tlv493d_set_operating_mode(data, data->mode);
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(tlv493d_pm_ops, tlv493d_runtime_suspend,
+			tlv493d_runtime_resume, NULL);
+
+static const struct i2c_device_id tlv493d_id[] = {
+	{ "tlv493d" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tlv493d_id);
+
+static const struct of_device_id tlv493d_of_match[] = {
+	{ .compatible = "infineon,tlv493d-a1b6" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, tlv493d_of_match);
+
+static struct i2c_driver tlv493d_driver = {
+	.driver = {
+		.name = "tlv493d",
+		.of_match_table = tlv493d_of_match,
+		.pm = pm_ptr(&tlv493d_pm_ops),
+	},
+	.probe = tlv493d_probe,
+	.id_table = tlv493d_id,
+};
+module_i2c_driver(tlv493d_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Infineon TLV493D Low-Power 3D Magnetic Sensor");
+MODULE_AUTHOR("Dixit Parmar <dixitparmar19@gmail.com>");

-- 
2.43.0


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

* [PATCH v5 2/2] dt-bindings: iio: magnetometer: document Infineon TLV493D 3D Magnetic sensor
  2025-08-29  2:53 [PATCH v5 0/2] iio: magnetometer: add support for Infineon TLV493D 3D Magnetic Sensor Dixit Parmar
  2025-08-29  2:53 ` [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor Dixit Parmar
@ 2025-08-29  2:53 ` Dixit Parmar
  1 sibling, 0 replies; 9+ messages in thread
From: Dixit Parmar @ 2025-08-29  2:53 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-kernel, linux-iio, devicetree, Dixit Parmar,
	Krzysztof Kozlowski

Document the bindings for Infineon TLV493D Low-Power 3D Magnetic Sensor
controlled by I2C interface. Main applications includes joysticks, control
elements (white goods, multifunction knops), or electric meters (anti-
tampering).
Drop duplicate entry for infineon,tlv493d from trivial-devices.yaml as
its documented in this separate dt-binding file now.

Datasheet: https://www.infineon.com/assets/row/public/documents/24/49/infineon-tlv493d-a1b6-datasheet-en.pdf
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
---
 .../iio/magnetometer/infineon,tlv493d-a1b6.yaml    | 45 ++++++++++++++++++++++
 .../devicetree/bindings/trivial-devices.yaml       |  2 -
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml b/Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml
new file mode 100644
index 000000000000..dd23a9370a71
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/magnetometer/infineon,tlv493d-a1b6.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Infineon Technologies TLV493D Low-Power 3D Magnetic Sensor
+
+maintainers:
+  - Dixit Parmar <dixitparmar19@gmail.com>
+
+properties:
+  $nodename:
+    pattern: '^magnetometer@[0-9a-f]+$'
+
+  compatible:
+    const: infineon,tlv493d-a1b6
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: 2.8V to 3.5V VDD supply
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      magnetometer@5e {
+        compatible = "infineon,tlv493d-a1b6";
+        reg = <0x5e>;
+        vdd-supply = <&hall_vcc>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 27930708ccd5..9e0eb5c873d2 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -125,8 +125,6 @@ properties:
           - infineon,ir36021
             # Infineon IRPS5401 Voltage Regulator (PMIC)
           - infineon,irps5401
-            # Infineon TLV493D-A1B6 I2C 3D Magnetic Sensor
-          - infineon,tlv493d-a1b6
             # Infineon Hot-swap controller xdp710
           - infineon,xdp710
             # Infineon Multi-phase Digital VR Controller xdpe11280

-- 
2.43.0


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

* Re: [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor
  2025-08-29  2:53 ` [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor Dixit Parmar
@ 2025-08-29  6:52   ` Krzysztof Kozlowski
  2025-08-29 10:07     ` Dixit Parmar
  2025-08-30 16:19   ` Jonathan Cameron
  1 sibling, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-29  6:52 UTC (permalink / raw)
  To: Dixit Parmar
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

On Fri, Aug 29, 2025 at 08:23:42AM +0530, Dixit Parmar wrote:
> The Infineon TLV493D is a Low-Power 3D Magnetic Sensor. The Sensor
> applications includes joysticks, control elements (white goods,
> multifunction knops), or electric meters (anti tampering) and any
> other application that requires accurate angular measurements at
> low power consumptions.
> 
> The Sensor is configured over I2C, and as part of Sensor measurement
> data it provides 3-Axis magnetic fields and temperature core measurement.
> 
> The driver supports raw value read and buffered input via external trigger
> to allow streaming values with the same sensing timestamp.
> 
> While the sensor has an interrupt pin multiplexed with an I2C SCL pin.
> But for bus configurations interrupt(INT) is not recommended, unless timing
> constraints between I2C data transfers and interrupt pulses are monitored
> and aligned.
> 
> The Sensor's I2C register map and mode information is described in product
> User Manual [1].
> 
> Datasheet: https://www.infineon.com/assets/row/public/documents/24/49/infineon-tlv493d-a1b6-datasheet-en.pdf
> Link: https://www.mouser.com/pdfDocs/Infineon-TLV493D-A1B6_3DMagnetic-UserManual-v01_03-EN.pdf [1]
> Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
> ---
>  MAINTAINERS                        |   8 +
>  drivers/iio/magnetometer/Kconfig   |  13 +
>  drivers/iio/magnetometer/Makefile  |   2 +
>  drivers/iio/magnetometer/tlv493d.c | 533 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 556 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fad6cb025a19..cf0a00f5c4d4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11843,6 +11843,14 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/sound/infineon,peb2466.yaml
>  F:	sound/soc/codecs/peb2466.c
>  
> +INFINEON TLV493D Driver
> +M:	Dixit Parmar <dixitparmar19@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +W:	https://www.infineon.com/part/TLV493D-A1B6
> +F:	Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml

There is no such file here. Apply this *patch* and check by yourself.

Your patchset is still incorrectly organized. See submitting patches in
DT dir.

Best regards,
Krzysztof


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

* Re: [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor
  2025-08-29  6:52   ` Krzysztof Kozlowski
@ 2025-08-29 10:07     ` Dixit Parmar
  2025-08-29 10:31       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 9+ messages in thread
From: Dixit Parmar @ 2025-08-29 10:07 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

> > +INFINEON TLV493D Driver
> > +M:   Dixit Parmar <dixitparmar19@gmail.com>
> > +L:   linux-iio@vger.kernel.org
> > +S:   Maintained
> > +W:   https://www.infineon.com/part/TLV493D-A1B6
> > +F:   Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml
>
> There is no such file here. Apply this *patch* and check by yourself.
That file is being added as a separate patch(Patch 2/2) of this same
patch series. It's already reviewed by you only(based on the name).
https://lore.kernel.org/linux-iio/20250829-tlv493d-sensor-v6_16-rc5-v5-2-746e73bc6c11@gmail.com
>
> Your patchset is still incorrectly organized. See submitting patches in
> DT dir.
By "incorrectly organized" do you mean order of the patches in the
patchset or anything else?
>
Thank you for the review,
Dixit

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

* Re: [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor
  2025-08-29 10:07     ` Dixit Parmar
@ 2025-08-29 10:31       ` Krzysztof Kozlowski
  2025-08-29 12:05         ` Dixit Parmar
  0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-29 10:31 UTC (permalink / raw)
  To: Dixit Parmar
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

On 29/08/2025 12:07, Dixit Parmar wrote:
>>> +INFINEON TLV493D Driver
>>> +M:   Dixit Parmar <dixitparmar19@gmail.com>
>>> +L:   linux-iio@vger.kernel.org
>>> +S:   Maintained
>>> +W:   https://www.infineon.com/part/TLV493D-A1B6
>>> +F:   Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml
>>
>> There is no such file here. Apply this *patch* and check by yourself.
> That file is being added as a separate patch(Patch 2/2) of this same
> patch series. It's already reviewed by you only(based on the name).

No. Read my comment again:

"Apply this *patch* and check by yourself."

It does not matter if you add the file later. The file does not exist
now, here.

> https://lore.kernel.org/linux-iio/20250829-tlv493d-sensor-v6_16-rc5-v5-2-746e73bc6c11@gmail.com
>>
>> Your patchset is still incorrectly organized. See submitting patches in
>> DT dir.
> By "incorrectly organized" do you mean order of the patches in the
> patchset or anything else?

I pointed to the docs, is anything unclear in there?

Best regards,
Krzysztof

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

* Re: [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor
  2025-08-29 10:31       ` Krzysztof Kozlowski
@ 2025-08-29 12:05         ` Dixit Parmar
  2025-08-29 14:33           ` David Lechner
  0 siblings, 1 reply; 9+ messages in thread
From: Dixit Parmar @ 2025-08-29 12:05 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-kernel,
	linux-iio, devicetree

> >>> +INFINEON TLV493D Driver
> >>> +M:   Dixit Parmar <dixitparmar19@gmail.com>
> >>> +L:   linux-iio@vger.kernel.org
> >>> +S:   Maintained
> >>> +W:   https://www.infineon.com/part/TLV493D-A1B6
> >>> +F:   Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml
> >>
> >> There is no such file here. Apply this *patch* and check by yourself.
> > That file is being added as a separate patch(Patch 2/2) of this same
> > patch series. It's already reviewed by you only(based on the name).
>
> No. Read my comment again:
I am not sure If I understood what you exactly mean. Below is what I
understood, please correct me if I am wrong.
>
> "Apply this *patch* and check by yourself."
>
> It does not matter if you add the file later. The file does not exist
> now, here.

I have applied the patch on the latest mainline kernel codebase, it
gets applied successfully.
But yes, as you pointed out the
Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml
file does not exist by applying JUST THIS patch. That is because that
file is introduced by a separate patch under the same
patch series mentioned below.
If this is not what you mean, I'd request to clarify to help me
understand your concern.

> > https://lore.kernel.org/linux-iio/20250829-tlv493d-sensor-v6_16-rc5-v5-2-746e73bc6c11@gmail.com
> >>
> >> Your patchset is still incorrectly organized. See submitting patches in
> >> DT dir.
> > By "incorrectly organized" do you mean order of the patches in the
> > patchset or anything else?
>
> I pointed to the docs, is anything unclear in there?
I did referred the docs you
pointed(Documentation/devicetree/bindings/submitting-patches.rst), as
per that only one point which is
being missed here that is #5 _The Documentation/ portion of the patch
should come in the series before the code implementing the binding._
That is applicable to dt-bindings patch of this patch series as the
dt-bindings patch is a seperate patch.
Apart from that, is there anything you think this patch is missing,
kindly guide me.

Thank you for review,
Dixit

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

* Re: [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor
  2025-08-29 12:05         ` Dixit Parmar
@ 2025-08-29 14:33           ` David Lechner
  0 siblings, 0 replies; 9+ messages in thread
From: David Lechner @ 2025-08-29 14:33 UTC (permalink / raw)
  To: Dixit Parmar, Krzysztof Kozlowski
  Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio,
	devicetree

On 8/29/25 7:05 AM, Dixit Parmar wrote:
>>>>> +INFINEON TLV493D Driver
>>>>> +M:   Dixit Parmar <dixitparmar19@gmail.com>
>>>>> +L:   linux-iio@vger.kernel.org
>>>>> +S:   Maintained
>>>>> +W:   https://www.infineon.com/part/TLV493D-A1B6
>>>>> +F:   Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml
>>>>
>>>> There is no such file here. Apply this *patch* and check by yourself.
>>> That file is being added as a separate patch(Patch 2/2) of this same
>>> patch series. It's already reviewed by you only(based on the name).
>>
>> No. Read my comment again:
> I am not sure If I understood what you exactly mean. Below is what I
> understood, please correct me if I am wrong.
>>
>> "Apply this *patch* and check by yourself."
>>
>> It does not matter if you add the file later. The file does not exist
>> now, here.
> 
> I have applied the patch on the latest mainline kernel codebase, it
> gets applied successfully.
> But yes, as you pointed out the
> Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml
> file does not exist by applying JUST THIS patch. That is because that
> file is introduced by a separate patch under the same
> patch series mentioned below.
> If this is not what you mean, I'd request to clarify to help me
> understand your concern.

Each patch should stand on it's own and not depend on later patches.
As you said below, the dt-bindings patch should come first in the
series. It is OK to also include a change to MAINTAINERS in that patch.
So you could include only this part:

+INFINEON TLV493D Driver
+M:	Dixit Parmar <dixitparmar19@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+W:	https://www.infineon.com/part/TLV493D-A1B6
+F:	Documentation/devicetree/bindings/iio/magnetometer/infineon,tlv493d-a1b6.yaml

in the dt-bindings patch and then add:

+F:	drivers/iio/magnetometer/tlv493d.c

in the later driver patch.

This way, each F: entry is added in the same patch as the new file
it refers to.

> 
>>> https://lore.kernel.org/linux-iio/20250829-tlv493d-sensor-v6_16-rc5-v5-2-746e73bc6c11@gmail.com
>>>>
>>>> Your patchset is still incorrectly organized. See submitting patches in
>>>> DT dir.
>>> By "incorrectly organized" do you mean order of the patches in the
>>> patchset or anything else?
>>
>> I pointed to the docs, is anything unclear in there?
> I did referred the docs you
> pointed(Documentation/devicetree/bindings/submitting-patches.rst), as
> per that only one point which is
> being missed here that is #5 _The Documentation/ portion of the patch
> should come in the series before the code implementing the binding._
> That is applicable to dt-bindings patch of this patch series as the
> dt-bindings patch is a seperate patch.
> Apart from that, is there anything you think this patch is missing,
> kindly guide me.
> 
> Thank you for review,
> Dixit


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

* Re: [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor
  2025-08-29  2:53 ` [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor Dixit Parmar
  2025-08-29  6:52   ` Krzysztof Kozlowski
@ 2025-08-30 16:19   ` Jonathan Cameron
  1 sibling, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2025-08-30 16:19 UTC (permalink / raw)
  To: Dixit Parmar
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-kernel, linux-iio,
	devicetree

On Fri, 29 Aug 2025 08:23:42 +0530
Dixit Parmar <dixitparmar19@gmail.com> wrote:

> The Infineon TLV493D is a Low-Power 3D Magnetic Sensor. The Sensor
> applications includes joysticks, control elements (white goods,
> multifunction knops), or electric meters (anti tampering) and any
> other application that requires accurate angular measurements at
> low power consumptions.
> 
> The Sensor is configured over I2C, and as part of Sensor measurement
> data it provides 3-Axis magnetic fields and temperature core measurement.
> 
> The driver supports raw value read and buffered input via external trigger
> to allow streaming values with the same sensing timestamp.
> 
> While the sensor has an interrupt pin multiplexed with an I2C SCL pin.
> But for bus configurations interrupt(INT) is not recommended, unless timing
> constraints between I2C data transfers and interrupt pulses are monitored
> and aligned.
> 
> The Sensor's I2C register map and mode information is described in product
> User Manual [1].
> 
> Datasheet: https://www.infineon.com/assets/row/public/documents/24/49/infineon-tlv493d-a1b6-datasheet-en.pdf
> Link: https://www.mouser.com/pdfDocs/Infineon-TLV493D-A1B6_3DMagnetic-UserManual-v01_03-EN.pdf [1]
> Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>

Hi Dixit,

A few trivial formatting related things inline.  I'd just have tweaked these
whilst applying but we need to have a v6 for the dt-binding/maintainers stuff
anyway so please tidy these up as well.

Thanks,

Jonathan


> diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
> index 9297723a97d8..dfe970fcacb8 100644
> --- a/drivers/iio/magnetometer/Makefile
> +++ b/drivers/iio/magnetometer/Makefile
> @@ -23,6 +23,8 @@ st_magn-$(CONFIG_IIO_BUFFER) += st_magn_buffer.o
>  obj-$(CONFIG_IIO_ST_MAGN_I2C_3AXIS) += st_magn_i2c.o
>  obj-$(CONFIG_IIO_ST_MAGN_SPI_3AXIS) += st_magn_spi.o
>  
> +obj-$(CONFIG_INFINEON_TLV493D)		+= tlv493d.o
> +
>  obj-$(CONFIG_SENSORS_HMC5843)		+= hmc5843_core.o
>  obj-$(CONFIG_SENSORS_HMC5843_I2C)	+= hmc5843_i2c.o
>  obj-$(CONFIG_SENSORS_HMC5843_SPI)	+= hmc5843_spi.o
> diff --git a/drivers/iio/magnetometer/tlv493d.c b/drivers/iio/magnetometer/tlv493d.c
> new file mode 100644
> index 000000000000..b723eaac1d9e
> --- /dev/null
> +++ b/drivers/iio/magnetometer/tlv493d.c
> @@ -0,0 +1,533 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/**

This isn't kernel doc format. Headers descriptions like this normally aren't
so /* not /** is appropriate. I haven't checked but I'd expect the kernel-doc
script to have warned about that if run on this file

> + * Driver for the Infineon TLV493D Low-Power 3D Magnetic Sensor
> + *
> + * Copyright (C) 2025 Dixit Parmar <dixitparmar19@gmail.com>
> + */
> +/*
> + * Different mode has different measurement sampling time, this time is
> + * used in deriving the sleep and timeout while reading the data from
> + * sensor in polling.
> + * Power-down mode: No measurement.
> + * Fast mode: Freq:3.3 KHz. Measurement time:305 usec.
> + * Low-power mode: Freq:100 Hz. Measurement time:10 msec.
> + * Ultra low-power mode: Freq:10 Hz. Measurement time:100 msec.
> + * Master controlled mode: Freq:3.3 Khz. Measurement time:305 usec.
> + */
> +static const u32 tlv493d_sample_rate_us[] = {
> +	[TLV493D_OP_MODE_POWERDOWN] = 0,
> +	[TLV493D_OP_MODE_FAST] = 305,
> +	[TLV493D_OP_MODE_LOWPOWER] = 10 * USEC_PER_MSEC,
> +	[TLV493D_OP_MODE_ULTRA_LOWPOWER] = 100 * USEC_PER_MSEC,
> +	[TLV493D_OP_MODE_MASTERCONTROLLED] = 305,
> +};
> +
> +static int tlv493d_write_all_regs(struct tlv493d_data *data)
> +{
> +	int ret;
> +	struct device *dev = &data->client->dev;
> +
> +	ret = i2c_master_send(data->client, data->wr_regs, ARRAY_SIZE(data->wr_regs));
> +	if (ret < 0) {
> +		dev_err(dev, "i2c write registers failed, error: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}


> +static int tlv493d_get_measurements(struct tlv493d_data *data, s16 *x, s16 *y,
> +				    s16 *z, s16 *t)
> +{
> +	u8 buff[7] = {};
> +	int err, ret;
> +	struct device *dev = &data->client->dev;
> +	u32 sleep_us = tlv493d_sample_rate_us[data->mode];
> +
> +	guard(mutex)(&data->lock);
> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Poll until data is valid,

. rather than , given next line is a new sentence.

> +	 * For a valid data TLV493D_TEMP_CHANNEL bit of TLV493D_RD_REG_TEMP
> +	 * should be set to 0. The sampling time depends on the sensor mode.
> +	 * Poll 3x the time of the sampling time.
> +	 */
> +	ret = read_poll_timeout(i2c_master_recv, err,
> +			err || !FIELD_GET(TLV493D_TEMP_CHANNEL,
> +			buff[TLV493D_RD_REG_TEMP]),

If you are going to split the FIELD_GET between parameters you must
align the second line of parameters after the (  otherwise this is
unnecessarily hard to read.

	ret = read_poll_timeout(i2c_master_recv, err,
		err || !FIELD_GET(TLV493D_TEMP_CHANNEL,	buff[TLV493D_RD_REG_TEMP]),
		sleep_us, 3 * sleep_us, false, data->client, buff,
		ARRAY_SIZE(buff));

Is a better way to format this.  Once we aren't aligning with the opening
bracket of read_poll_timeout because of the very long line, a single tab of
indent of the second line is enough.  Don't worry about going a little over 80 chars
here as it really hurts readability to split that FIELD_PREP() up.


> +	if (ret) {
> +		dev_err(dev, "i2c read poll timeout, error:%d\n", ret);
> +		goto out_put_autosuspend;
> +	}
> +	if (err < 0) {
> +		dev_err(dev, "i2c read data failed, error:%d\n", err);
> +		ret = err;
> +		goto out_put_autosuspend;
> +	}
> +
> +	*x = tlv493d_get_channel_data(buff, TLV493D_AXIS_X);
> +	*y = tlv493d_get_channel_data(buff, TLV493D_AXIS_Y);
> +	*z = tlv493d_get_channel_data(buff, TLV493D_AXIS_Z);
> +	*t = tlv493d_get_channel_data(buff, TLV493D_TEMPERATURE);
> +
> +out_put_autosuspend:
> +	pm_runtime_put_autosuspend(dev);
> +	return ret;
> +}


> +static int tlv493d_read_raw(struct iio_dev *indio_dev,
> +			const struct iio_chan_spec *chan, int *val,
> +			int *val2, long mask)
> +{
> +	struct tlv493d_data *data = iio_priv(indio_dev);
> +	s16 x, y, z, t;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = tlv493d_get_measurements(data, &x, &y, &z, &t);
> +		if (ret)
> +			return ret;
> +
> +		/* Return raw values for requested channel */

Not return in the C meaning of it, and that comment doesn't
really add anything, so I'd just drop it.

> +		switch (chan->address) {
> +		case TLV493D_AXIS_X:
> +			*val = x;
> +			return IIO_VAL_INT;
> +		case TLV493D_AXIS_Y:
> +			*val = y;
> +			return IIO_VAL_INT;
> +		case TLV493D_AXIS_Z:
> +			*val = z;
> +			return IIO_VAL_INT;
> +		case TLV493D_TEMPERATURE:
> +			*val = t;
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}


> +static int tlv493d_probe(struct i2c_client *client)
> +{

> +	indio_dev->info = &tlv493d_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->name = client->name;
> +	indio_dev->channels = tlv493d_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(tlv493d_channels);
> +	indio_dev->available_scan_masks = tlv493d_scan_masks;
> +
> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> +				iio_pollfunc_store_time,
> +				tlv493d_trigger_handler,
> +				NULL);

If you align later lines of parameters with the start of the 1st
paramater, e.g. after the ( then this is still under 80 chars.

Given that is generally the preferred style where possible (in IIO
anyway) please do that here.

Jonathan

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

end of thread, other threads:[~2025-08-30 16:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29  2:53 [PATCH v5 0/2] iio: magnetometer: add support for Infineon TLV493D 3D Magnetic Sensor Dixit Parmar
2025-08-29  2:53 ` [PATCH v5 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor Dixit Parmar
2025-08-29  6:52   ` Krzysztof Kozlowski
2025-08-29 10:07     ` Dixit Parmar
2025-08-29 10:31       ` Krzysztof Kozlowski
2025-08-29 12:05         ` Dixit Parmar
2025-08-29 14:33           ` David Lechner
2025-08-30 16:19   ` Jonathan Cameron
2025-08-29  2:53 ` [PATCH v5 2/2] dt-bindings: iio: magnetometer: document Infineon TLV493D 3D Magnetic sensor Dixit Parmar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).