[PATCH v4 0/2] iio: magnetometer: add support for Infineon TLV493D 3D Magnetic Sensor

[PATCH v4 0/2] iio: magnetometer: add support for Infineon TLV493D 3D Magnetic Sensor

[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>
---
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 -
 drivers/iio/magnetometer/Kconfig                   |  13 +
 drivers/iio/magnetometer/Makefile                  |   2 +
 drivers/iio/magnetometer/tlv493d.c                 | 530 +++++++++++++++++++++
 5 files changed, 590 insertions(+), 2 deletions(-)
---
base-commit: d7b8f8e20813f0179d8ef519541a3527e7661d3a
change-id: 20250726-tlv493d-sensor-v6_16-rc5-18c712093b27

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

[PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[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>
---
 drivers/iio/magnetometer/Kconfig   |  13 +
 drivers/iio/magnetometer/Makefile  |   2 +
 drivers/iio/magnetometer/tlv493d.c | 530 +++++++++++++++++++++++++++++++++++++
 3 files changed, 545 insertions(+)

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..ee72211576a6
--- /dev/null
+++ b/drivers/iio/magnetometer/tlv493d.c
@@ -0,0 +1,530 @@
+// 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] = 10000,
+	[TLV493D_OP_MODE_ULTRA_LOWPOWER] = 100000,
+	[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]) == 0,
+			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 < 0)
+		return dev_err_probe(dev, ret, "iio triggered buffer setup failed\n");
+
+	ret = pm_runtime_set_active(dev);
+	if (ret < 0)
+		return ret;
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret < 0)
+		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

[PATCH v4 2/2] dt-bindings: iio: magnetometer: document Infineon TLV493D 3D Magnetic sensor

[Dixit Parmar]

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

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Jonathan Cameron]

On Thu, 14 Aug 2025 08:23:43 +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 couple of really minor things inline. Given Andy has been doing most of the review
work on this one I'll leave it for a few days to give him chance for a final look.

The stuff below is small so if nothing else comes up I can tweak it whilst applying

Thanks,

Jonathan

> diff --git a/drivers/iio/magnetometer/tlv493d.c b/drivers/iio/magnetometer/tlv493d.c
> new file mode 100644
> index 000000000000..ee72211576a6
> --- /dev/null
> +++ b/drivers/iio/magnetometer/tlv493d.c
> @@ -0,0 +1,530 @@

> +	TLV493D_AXIS_X,
> +	TLV493D_AXIS_Y,
> +	TLV493D_AXIS_Z,
> +	TLV493D_TEMPERATURE
As below.

> +};
> +
> +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
This is not a terminating entry, so would typically have a trailing comma.
> +};

> +
> +static int tlv493d_init(struct tlv493d_data *data)

I think this is only called from probe, so it would be appropriate
to use return dev_err_probe() in all the error paths.

If nothing else comes up I might tweak that whilst applying.

> +{
> +	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;
> +}

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Dixit Parmar]

On Sat, Aug 16, 2025 at 02:04:48PM +0100, Jonathan Cameron wrote:
> Hi Dixit,
> 
> A couple of really minor things inline. Given Andy has been doing most of the review
> work on this one I'll leave it for a few days to give him chance for a final look.
> 
> The stuff below is small so if nothing else comes up I can tweak it whilst applying
> 
> Thanks,
> 
> Jonathan
> 
> > diff --git a/drivers/iio/magnetometer/tlv493d.c b/drivers/iio/magnetometer/tlv493d.c
> > new file mode 100644
> > index 000000000000..ee72211576a6
> > --- /dev/null
> > +++ b/drivers/iio/magnetometer/tlv493d.c
> > @@ -0,0 +1,530 @@
> 
> > +	TLV493D_AXIS_X,
> > +	TLV493D_AXIS_Y,
> > +	TLV493D_AXIS_Z,
> > +	TLV493D_TEMPERATURE
> As below.
> 
> > +};
> > +
> > +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
> This is not a terminating entry, so would typically have a trailing comma.
Isn't the last entry in the enum list is termintating entry and it should
not have trailing comma?
> > +};
> 
> > +
> > +static int tlv493d_init(struct tlv493d_data *data)
> 
> I think this is only called from probe, so it would be appropriate
> to use return dev_err_probe() in all the error paths.
There is dev_err_probe() being called based on the return value of this
tlv493d_init(). This function reports the approproiate error(if any) and
the negative return value will result in dev_error_probe().
So I believe having single dev_err_probe() in the _probe() function would
be more appropriate, IMO.
> 
> If nothing else comes up I might tweak that whilst applying.
Much appreciated.

Thank you,
Dixit Parmar

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Andy Shevchenko]

On Wed, Aug 20, 2025 at 10:16:29AM +0530, Dixit Parmar wrote:
> On Sat, Aug 16, 2025 at 02:04:48PM +0100, Jonathan Cameron wrote:

...

> > > +	TLV493D_AXIS_X,
> > > +	TLV493D_AXIS_Y,
> > > +	TLV493D_AXIS_Z,
> > > +	TLV493D_TEMPERATURE
> > As below.
> > 
> > > +};
> > > +
> > > +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
> > This is not a terminating entry, so would typically have a trailing comma.
> Isn't the last entry in the enum list is termintating entry and it should
> not have trailing comma?

No, it's not semantically. (Yes, it's terminating the list syntactically)

> > > +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Andy Shevchenko]

On Thu, Aug 14, 2025 at 08:23:43AM +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].

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

I haven't got the ordering rules here and in Kconfig. Can it be alphabetical?

...

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

+ trailing commas in both cases as discussed in the other email.

...

> +static const u32 tlv493d_sample_rate_us[] = {
> +	[TLV493D_OP_MODE_POWERDOWN] = 0,
> +	[TLV493D_OP_MODE_FAST] = 305,
> +	[TLV493D_OP_MODE_LOWPOWER] = 10000,
> +	[TLV493D_OP_MODE_ULTRA_LOWPOWER] = 100000,

Perhaps
	10 * USEC_PER_MSEC
	100 * USEC_PER_MSEC

respectively?

> +	[TLV493D_OP_MODE_MASTERCONTROLLED] = 305

+ Trailing comma.

> +};

...

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

Wrong indentation, make both 'F':s to be in the same column.

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

Ditto for all of the above.

> +	return sign_extend32(val, 11);
> +}

...

> +static int tlv493d_get_measurements(struct tlv493d_data *data, s16 *x, s16 *y,
> +				s16 *z, s16 *t)

Indentation issue. Please, check fully the code for such issues.

> +{
> +	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]) == 0,

Please, resplit logically, i.e leave the condition on the single line.
Also to make it shorter you can use '!' instead of ' == 0'.

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

...

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

Just wondering if you have tested for negative coordinates, does it propagate
correctly?

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

Interestingly that you have used 100 limit and suddenly don't do it here
and maybe elsewhere. Why inconsistent style? Please, go through the whole
file and make sure the style is consistent in all of the aspects:
- C style used
- comments style (one-line and multi-line)
- indentation
- etc.

> +out_trigger_notify:
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}

...

> +	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
> +				iio_pollfunc_store_time,
> +				tlv493d_trigger_handler,
> +				NULL);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "iio triggered buffer setup failed\n");
> +
> +	ret = pm_runtime_set_active(dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret < 0)
> +		return ret;

For each of 'ret < 0' please double check that this is indeed required.
Otherwise, use common style for all standard cases, i.e.

	if (ret)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Dixit Parmar]

On Wed, Aug 20, 2025 at 05:08:38PM +0300, Andy Shevchenko wrote:
> > --- 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
> 
> I haven't got the ordering rules here and in Kconfig. Can it be alphabetical?
From what I can see, the order is alphabetical based on the CONFIG option in the
Makefile and Kconfig, and I kept CONFIG_INFINEO_TLV493D after CONFIG_IIO_ST*.
Isn't it in correct order? or my understanding is incorrect?
> 
> > +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;
> 
> Wrong indentation, make both 'F':s to be in the same column.
To have 'F' in the same column, it will need spaces after tab (I think its not
advisable to mix spaces and tabs). With just tabs the second FIELD_GET will be
align to 'v' of val. What will be the correct indentation?
> 
> > +static int tlv493d_get_measurements(struct tlv493d_data *data, s16 *x, s16 *y,
> > +				s16 *z, s16 *t)
> 
> Indentation issue. Please, check fully the code for such issues.
I followed the single tab after default as suggested. At which column the s16 *z should be align to?
> > +	/*
> > +	 * 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]) == 0,
> 
> Please, resplit logically, i.e leave the condition on the single line.
> Also to make it shorter you can use '!' instead of ' == 0'.
Having both conditions in same line will go out of 80 char length limit, even with !.
Is it fine?
> 
> Just wondering if you have tested for negative coordinates, does it propagate
> correctly?
Yes I have validated, it reports correct negative coordinate values.
> > +	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> > +				pf->timestamp);
> 
> Interestingly that you have used 100 limit and suddenly don't do it here
> and maybe elsewhere. Why inconsistent style? Please, go through the whole
> file and make sure the style is consistent in all of the aspects:
> - C style used
> - comments style (one-line and multi-line)
> - indentation
> - etc.
I tried to follow 80 limit(except few places where it was just on border or not
clear to read). I belive the standard is to use 80 limit(correct me if I referred
wrong place) and I will recheck to meet that.

Thank you for careful review,
Dixit Parmar

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Andy Shevchenko]

On Thu, Aug 21, 2025 at 6:02 AM Dixit Parmar <dixitparmar19@gmail.com> wrote:
> On Wed, Aug 20, 2025 at 05:08:38PM +0300, Andy Shevchenko wrote:

...

> > >  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
> >
> > I haven't got the ordering rules here and in Kconfig. Can it be alphabetical?
> From what I can see, the order is alphabetical based on the CONFIG option in the
> Makefile and Kconfig, and I kept CONFIG_INFINEO_TLV493D after CONFIG_IIO_ST*.
> Isn't it in correct order? or my understanding is incorrect?

I dunno, The file name there is with the vendor prefix, in many cases
the configuration option is with vendor prefix as well, but the file.

...

> > > +   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;
> >
> > Wrong indentation, make both 'F':s to be in the same column.
> To have 'F' in the same column, it will need spaces after tab (I think its not
> advisable to mix spaces and tabs).

No, mixing tabs and spaces basically means spaces inside TABs or
before TABs. The TABS followed by solely spaces is okay and the
correct way to indent.

> With just tabs the second FIELD_GET will be
> align to 'v' of val. What will be the correct indentation?

...

> > > +static int tlv493d_get_measurements(struct tlv493d_data *data, s16 *x, s16 *y,
> > > +                           s16 *z, s16 *t)
> >
> > Indentation issue. Please, check fully the code for such issues.
> I followed the single tab after default as suggested. At which column the s16 *z should be align to?

s16 starts with the same column as struct.

...

> > > +   ret = read_poll_timeout(i2c_master_recv, err, err ||
> > > +                   FIELD_GET(TLV493D_TEMP_CHANNEL, buff[TLV493D_RD_REG_TEMP]) == 0,
> >
> > Please, resplit logically, i.e leave the condition on the single line.
> > Also to make it shorter you can use '!' instead of ' == 0'.
> Having both conditions in same line will go out of 80 char length limit, even with !.
> Is it fine?

In _this_ case yes. I expect something like this to see

   ret = read_poll_timeout(i2c_master_recv, err,
                   err || !FIELD_GET(TLV493D_TEMP_CHANNEL,
buff[TLV493D_RD_REG_TEMP]),

...

> > Just wondering if you have tested for negative coordinates, does it propagate
> > correctly?
> Yes I have validated, it reports correct negative coordinate values.

OK!

> > > +   iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan),
> > > +                           pf->timestamp);
> >
> > Interestingly that you have used 100 limit and suddenly don't do it here
> > and maybe elsewhere. Why inconsistent style? Please, go through the whole
> > file and make sure the style is consistent in all of the aspects:
> > - C style used
> > - comments style (one-line and multi-line)
> > - indentation
> > - etc.
> I tried to follow 80 limit(except few places where it was just on border or not
> clear to read). I belive the standard is to use 80 limit(correct me if I referred
> wrong place) and I will recheck to meet that.

There are two standards, the old and strict one -- 80 characters, and
this subsystem _tries_ to follow it and relaxed with 100 limit.
The exceptions are possible when it affects readability.

--
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Dixit Parmar]

On Thu, Aug 21, 2025 at 10:41:03AM +0300, Andy Shevchenko wrote:
> On Thu, Aug 21, 2025 at 6:02 AM Dixit Parmar <dixitparmar19@gmail.com> wrote:
> > On Wed, Aug 20, 2025 at 05:08:38PM +0300, Andy Shevchenko wrote:
> 
> ...
> 
> > > >  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
> > >
> > > I haven't got the ordering rules here and in Kconfig. Can it be alphabetical?
> > From what I can see, the order is alphabetical based on the CONFIG option in the
> > Makefile and Kconfig, and I kept CONFIG_INFINEO_TLV493D after CONFIG_IIO_ST*.
> > Isn't it in correct order? or my understanding is incorrect?
> 
> I dunno, The file name there is with the vendor prefix, in many cases
> the configuration option is with vendor prefix as well, but the file.
Hi Jonathan, Can you please suggest best possible way here?

> 
> > > Interestingly that you have used 100 limit and suddenly don't do it here
> > > and maybe elsewhere. Why inconsistent style? Please, go through the whole
> > > file and make sure the style is consistent in all of the aspects:
> > > - C style used
> > > - comments style (one-line and multi-line)
> > > - indentation
> > > - etc.
> > I tried to follow 80 limit(except few places where it was just on border or not
> > clear to read). I belive the standard is to use 80 limit(correct me if I referred
> > wrong place) and I will recheck to meet that.
> 
> There are two standards, the old and strict one -- 80 characters, and
> this subsystem _tries_ to follow it and relaxed with 100 limit.
> The exceptions are possible when it affects readability.
Understood, I will go with 100 limit and make sure everything is well within it.

Thanks,
Dixit

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Andy Shevchenko]

On Fri, Aug 22, 2025 at 5:40 AM Dixit Parmar <dixitparmar19@gmail.com> wrote:
> On Thu, Aug 21, 2025 at 10:41:03AM +0300, Andy Shevchenko wrote:
> > On Thu, Aug 21, 2025 at 6:02 AM Dixit Parmar <dixitparmar19@gmail.com> wrote:
> > > On Wed, Aug 20, 2025 at 05:08:38PM +0300, Andy Shevchenko wrote:

...

> > > > Interestingly that you have used 100 limit and suddenly don't do it here
> > > > and maybe elsewhere. Why inconsistent style? Please, go through the whole
> > > > file and make sure the style is consistent in all of the aspects:
> > > > - C style used
> > > > - comments style (one-line and multi-line)
> > > > - indentation
> > > > - etc.
> > > I tried to follow 80 limit(except few places where it was just on border or not
> > > clear to read). I belive the standard is to use 80 limit(correct me if I referred
> > > wrong place) and I will recheck to meet that.
> >
> > There are two standards, the old and strict one -- 80 characters, and
> > this subsystem _tries_ to follow it and relaxed with 100 limit.
> > The exceptions are possible when it affects readability.
> Understood, I will go with 100 limit and make sure everything is well within it.

It seems I was not clear enough. The IIO hardly tries to use the 80
limit, so don't go for 100 until it's agreed upon with the
maintainers.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Dixit Parmar]

On Fri, Aug 22, 2025 at 8:10 AM Dixit Parmar <dixitparmar19@gmail.com> wrote:
>
> On Thu, Aug 21, 2025 at 10:41:03AM +0300, Andy Shevchenko wrote:
> > On Thu, Aug 21, 2025 at 6:02 AM Dixit Parmar <dixitparmar19@gmail.com> wrote:
> > > On Wed, Aug 20, 2025 at 05:08:38PM +0300, Andy Shevchenko wrote:
> >
> > ...
> >
> > > > >  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
> > > >
> > > > I haven't got the ordering rules here and in Kconfig. Can it be alphabetical?
> > > From what I can see, the order is alphabetical based on the CONFIG option in the
> > > Makefile and Kconfig, and I kept CONFIG_INFINEO_TLV493D after CONFIG_IIO_ST*.
> > > Isn't it in correct order? or my understanding is incorrect?
> >
> > I dunno, The file name there is with the vendor prefix, in many cases
> > the configuration option is with vendor prefix as well, but the file.
> Hi Jonathan, Can you please suggest best possible way here?
Hi Jonathan, When you get a chance, please share your thoughts on this.

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Jonathan Cameron]

On Mon, 25 Aug 2025 08:33:07 +0530
Dixit Parmar <dixitparmar19@gmail.com> wrote:

> On Fri, Aug 22, 2025 at 8:10 AM Dixit Parmar <dixitparmar19@gmail.com> wrote:
> >
> > On Thu, Aug 21, 2025 at 10:41:03AM +0300, Andy Shevchenko wrote:  
> > > On Thu, Aug 21, 2025 at 6:02 AM Dixit Parmar <dixitparmar19@gmail.com> wrote:  
> > > > On Wed, Aug 20, 2025 at 05:08:38PM +0300, Andy Shevchenko wrote:  
> > >
> > > ...
> > >  
> > > > > >  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  
> > > > >
> > > > > I haven't got the ordering rules here and in Kconfig. Can it be alphabetical?  
> > > > From what I can see, the order is alphabetical based on the CONFIG option in the
> > > > Makefile and Kconfig, and I kept CONFIG_INFINEO_TLV493D after CONFIG_IIO_ST*.
> > > > Isn't it in correct order? or my understanding is incorrect?  
> > >
> > > I dunno, The file name there is with the vendor prefix, in many cases
> > > the configuration option is with vendor prefix as well, but the file.  
> > Hi Jonathan, Can you please suggest best possible way here?  
> Hi Jonathan, When you get a chance, please share your thoughts on this.

No hard rules on this.  We should probably make some but don't have
them yet, so try to go with what seems most common in the particular file.

Jonathan

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Dixit Parmar]

> > > > > > >  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
> > > > > >
> > > > > > I haven't got the ordering rules here and in Kconfig. Can it be alphabetical?
> > > > > From what I can see, the order is alphabetical based on the CONFIG option in the
> > > > > Makefile and Kconfig, and I kept CONFIG_INFINEO_TLV493D after CONFIG_IIO_ST*.
> > > > > Isn't it in correct order? or my understanding is incorrect?
> > > >
> > > > I dunno, The file name there is with the vendor prefix, in many cases
> > > > the configuration option is with vendor prefix as well, but the file.
> > > Hi Jonathan, Can you please suggest best possible way here?
> > Hi Jonathan, When you get a chance, please share your thoughts on this.
>
> No hard rules on this.  We should probably make some but don't have
> them yet, so try to go with what seems most common in the particular file.
>
> Jonathan
Okay. I believe we are good considering most of the drivers are having
vendor prefix,
so does this driver too, and its currently in correct alphabetical
order. Considering
the alphabetical order followed is for CONFIG options.
Thanks,
Dixit

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Dixit Parmar]

Jonathan, Andy,
One more query, Do I need to update the MAINTAINERS file with this new
entry as we are adding this new driver in this same patch series?

Re: [PATCH v4 1/2] iio: magnetometer: add support for Infineon TLV493D 3D Magentic sensor

[Jonathan Cameron]

On Tue, 26 Aug 2025 08:32:18 +0530
Dixit Parmar <dixitparmar19@gmail.com> wrote:

> Jonathan, Andy,
> One more query, Do I need to update the MAINTAINERS file with this new
> entry as we are adding this new driver in this same patch series?
> 

That would be ideal.  Do it in steps to include only the files in each patch.

I never specifically push people to commit to maintain drivers unless
they are particularly complex (and hence I'd feel out of my depth reviewing
changes), but the ideal is indeed for people to agree to look after their
code and in particular test changes proposed by others.

Mostly people shouldn't be relying on MAINTAINERS for who to +CC anyway
as it will often miss the people most interested in a given driver so
this is more of a statement of intent than something I consider a practical
part of the upstream process!

Jonathan