[PATCH v2 0/2] iio: magnetometer: add support for Si7210

[PATCH v2 0/2] iio: magnetometer: add support for Si7210

[Antoni Pokusinski]

This patch series adds support for the Si7210 Hall effect I2C sensor.
The driver currently supports the basic functionalities (i.e. making
temperature and magnetic field measurements and changing the
measurements scale) but I plan to add support for some other features in
the future as well (e.g. the digital output interrupt).

---
Hello,
Thanks for the review. In the v2, I implemented all the necessary
improvements, the most important ones being the `vdd-supply` in DT-binding
together with a regulator, changing the temperature measurement scale to
milli-celsius and removal of the `otp_lock`. The lock removal could be
done since we read the OTP memory in probe only.
Best regards,
Antoni

Changes since v1:
* dt-binding: add `vdd-supply`
* si7210: reorder includes in alphabetic order
* si7210: add comment for `fetch_lock`
* si7210: remove `otp_lock`
* si7210: fetch_measurement: change result type to __be16
* si7210: use guard(mutex) instead of scoped_guard
* si7210: read_raw: use FIELD_GET to get raw temperature value
* si7210: read_raw: return temperature in milli-celsius
* si7210: use regulator for getting the voltage
* si7210: si7210_device_wake: remove unnecessary cast
* si7210: si7210_device_init: use fsleep instead of usleep_range
* si7210: si7210_probe: remove i2c_set_clientdata()
* si7210: minor alignment and formatting fixes

Antoni Pokusinski (2):
  dt-bindings: iio: magnetometer: add binding for Si7210
  iio: magnetometer: si7210: add driver for Si7210

 .../iio/magnetometer/silabs,si7210.yaml       |  48 ++
 drivers/iio/magnetometer/Kconfig              |  11 +
 drivers/iio/magnetometer/Makefile             |   2 +
 drivers/iio/magnetometer/si7210.c             | 428 ++++++++++++++++++
 4 files changed, 489 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/silabs,si7210.yaml
 create mode 100644 drivers/iio/magnetometer/si7210.c

-- 
2.25.1

[PATCH v2 1/2] dt-bindings: iio: magnetometer: add binding for Si7210

[Antoni Pokusinski]

Silicon Labs Si7210 is an I2C Hall effect magnetic position
and temperature sensor.

Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
 .../iio/magnetometer/silabs,si7210.yaml       | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/silabs,si7210.yaml

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/silabs,si7210.yaml b/Documentation/devicetree/bindings/iio/magnetometer/silabs,si7210.yaml
new file mode 100644
index 000000000000..d4a3f7981c36
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/silabs,si7210.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/magnetometer/silabs,si7210.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Si7210 magnetic position and temperature sensor
+
+maintainers:
+  - Antoni Pokusinski <apokusinski01@gmail.com>
+
+description: |
+  Silabs Si7210 I2C Hall effect magnetic position and temperature sensor.
+  https://www.silabs.com/documents/public/data-sheets/si7210-datasheet.pdf
+
+properties:
+  compatible:
+    const: silabs,si7210
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vdd-supply:
+    description: Regulator that provides power to the sensor
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        magnetometer@30 {
+            compatible = "silabs,si7210";
+            reg = <0x30>;
+            interrupt-parent = <&gpio1>;
+            interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
+            vdd-supply = <&vdd_3v3_reg>;
+        };
+    };
-- 
2.25.1

[PATCH v2 2/2] iio: magnetometer: si7210: add driver for Si7210

[Antoni Pokusinski]

Silicon Labs Si7210 is an I2C Hall effect magnetic position and
temperature sensor. The driver supports the following functionalities:
* reading the temperature measurements
* reading the magnetic field measurements in a single-shot mode
* choosing the magnetic field measurement scale (20 or 200 mT)

Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
---
 drivers/iio/magnetometer/Kconfig  |  11 +
 drivers/iio/magnetometer/Makefile |   2 +
 drivers/iio/magnetometer/si7210.c | 428 ++++++++++++++++++++++++++++++
 3 files changed, 441 insertions(+)
 create mode 100644 drivers/iio/magnetometer/si7210.c

diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index 7177cd1d67cb..3debf1320ad1 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -235,6 +235,17 @@ config SENSORS_RM3100_SPI
 	  To compile this driver as a module, choose M here: the module
 	  will be called rm3100-spi.
 
+config SI7210
+	tristate "SI7210 Hall effect sensor"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say Y here to add support for the SI7210 Hall effect 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 si7210.
+
 config TI_TMAG5273
 	tristate "TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor"
 	depends on I2C
diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
index 3e4c2ecd9adf..58f32a855494 100644
--- a/drivers/iio/magnetometer/Makefile
+++ b/drivers/iio/magnetometer/Makefile
@@ -31,6 +31,8 @@ obj-$(CONFIG_SENSORS_RM3100)		+= rm3100-core.o
 obj-$(CONFIG_SENSORS_RM3100_I2C)	+= rm3100-i2c.o
 obj-$(CONFIG_SENSORS_RM3100_SPI)	+= rm3100-spi.o
 
+obj-$(CONFIG_SI7210) += si7210.o
+
 obj-$(CONFIG_TI_TMAG5273)		+= tmag5273.o
 
 obj-$(CONFIG_YAMAHA_YAS530)		+= yamaha-yas530.o
diff --git a/drivers/iio/magnetometer/si7210.c b/drivers/iio/magnetometer/si7210.c
new file mode 100644
index 000000000000..ed779ac24b9d
--- /dev/null
+++ b/drivers/iio/magnetometer/si7210.c
@@ -0,0 +1,428 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Silicon Labs Si7210 Hall Effect sensor driver
+ *
+ * Copyright (c) 2024 Antoni Pokusinski <apokusinski01@gmail.com>
+ *
+ * Datasheet:
+ *  https://www.silabs.com/documents/public/data-sheets/si7210-datasheet.pdf
+ */
+
+#include <linux/bitfield.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/math64.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+
+/* Registers offsets and masks */
+#define SI7210_REG_DSPSIGM	0xC1
+#define SI7210_REG_DSPSIGL	0xC2
+
+#define SI7210_MASK_DSPSIGSEL	GENMASK(2, 0)
+#define SI7210_REG_DSPSIGSEL	0xC3
+
+#define SI7210_MASK_STOP	BIT(1)
+#define SI7210_MASK_ONEBURST	BIT(2)
+#define SI7210_REG_POWER_CTRL	0xC4
+
+#define SI7210_MASK_ARAUTOINC	BIT(0)
+#define SI7210_REG_ARAUTOINC	0xC5
+
+#define SI7210_REG_A0		0xCA
+#define SI7210_REG_A1		0xCB
+#define SI7210_REG_A2		0xCC
+#define SI7210_REG_A3		0xCE
+#define SI7210_REG_A4		0xCF
+#define SI7210_REG_A5		0xD0
+
+#define SI7210_REG_OTP_ADDR	0xE1
+#define SI7210_REG_OTP_DATA	0xE2
+
+#define SI7210_MASK_OTP_READ_EN	BIT(1)
+#define SI7210_REG_OTP_CTRL	0xE3
+
+/* OTP data registers offsets */
+#define SI7210_OTPREG_TMP_OFF	0x1D
+#define SI7210_OTPREG_TMP_GAIN	0x1E
+
+#define SI7210_OTPREG_A0_20	0x21
+#define SI7210_OTPREG_A1_20	0x22
+#define SI7210_OTPREG_A2_20	0x23
+#define SI7210_OTPREG_A3_20	0x24
+#define SI7210_OTPREG_A4_20	0x25
+#define SI7210_OTPREG_A5_20	0x26
+
+#define SI7210_OTPREG_A0_200	0x27
+#define SI7210_OTPREG_A1_200	0x28
+#define SI7210_OTPREG_A2_200	0x29
+#define SI7210_OTPREG_A3_200	0x2A
+#define SI7210_OTPREG_A4_200	0x2B
+#define SI7210_OTPREG_A5_200	0x2C
+
+#define A_REGS_COUNT 6
+
+static const unsigned int a20_otp_regs[A_REGS_COUNT] = {
+	SI7210_OTPREG_A0_20, SI7210_OTPREG_A1_20, SI7210_OTPREG_A2_20,
+	SI7210_OTPREG_A3_20, SI7210_OTPREG_A4_20, SI7210_OTPREG_A5_20
+};
+
+static const unsigned int a200_otp_regs[A_REGS_COUNT] = {
+	SI7210_OTPREG_A0_200, SI7210_OTPREG_A1_200, SI7210_OTPREG_A2_200,
+	SI7210_OTPREG_A3_200, SI7210_OTPREG_A4_200, SI7210_OTPREG_A5_200
+};
+
+static const struct regmap_range si7210_read_reg_ranges[] = {
+	regmap_reg_range(SI7210_REG_DSPSIGM, SI7210_REG_ARAUTOINC),
+	regmap_reg_range(SI7210_REG_A0, SI7210_REG_A2),
+	regmap_reg_range(SI7210_REG_A3, SI7210_REG_A5),
+	regmap_reg_range(SI7210_REG_OTP_ADDR, SI7210_REG_OTP_CTRL),
+};
+
+static const struct regmap_access_table si7210_readable_regs = {
+	.yes_ranges = si7210_read_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(si7210_read_reg_ranges),
+};
+
+static const struct regmap_range si7210_write_reg_ranges[] = {
+	regmap_reg_range(SI7210_REG_DSPSIGSEL, SI7210_REG_ARAUTOINC),
+	regmap_reg_range(SI7210_REG_A0, SI7210_REG_A2),
+	regmap_reg_range(SI7210_REG_A3, SI7210_REG_A5),
+	regmap_reg_range(SI7210_REG_OTP_ADDR, SI7210_REG_OTP_CTRL),
+};
+
+static const struct regmap_access_table si7210_writeable_regs = {
+	.yes_ranges = si7210_write_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(si7210_write_reg_ranges),
+};
+
+static const struct regmap_range si7210_volatile_reg_ranges[] = {
+	regmap_reg_range(SI7210_REG_DSPSIGM, SI7210_REG_DSPSIGL),
+	regmap_reg_range(SI7210_REG_POWER_CTRL, SI7210_REG_POWER_CTRL),
+};
+
+static const struct regmap_access_table si7210_volatile_regs = {
+	.yes_ranges = si7210_volatile_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(si7210_volatile_reg_ranges),
+};
+
+static const struct regmap_config si7210_regmap_conf = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = SI7210_REG_OTP_CTRL,
+
+	.rd_table = &si7210_readable_regs,
+	.wr_table = &si7210_writeable_regs,
+	.volatile_table = &si7210_volatile_regs,
+};
+
+struct si7210_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+	struct regulator *vdd;
+	struct mutex fetch_lock; /* lock for a single measurement fetch */
+	s8 temp_offset;
+	s8 temp_gain;
+	s8 scale_20_a[A_REGS_COUNT];
+	s8 scale_200_a[A_REGS_COUNT];
+	u8 curr_scale;
+};
+
+static const struct iio_chan_spec si7210_channels[] = {
+	{
+		.type = IIO_MAGN,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
+	},
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+	}
+};
+
+static int si7210_fetch_measurement(struct si7210_data *data,
+				    struct iio_chan_spec const *chan,
+				    __be16 *buf)
+{
+	u8 dspsigsel = chan->type == IIO_MAGN ? 0 : 1;
+	int ret;
+
+	guard(mutex)(&data->fetch_lock);
+
+	ret = regmap_update_bits(data->regmap, SI7210_REG_DSPSIGSEL,
+				 SI7210_MASK_DSPSIGSEL, dspsigsel);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_update_bits(data->regmap, SI7210_REG_POWER_CTRL,
+				 SI7210_MASK_ONEBURST | SI7210_MASK_STOP,
+				 SI7210_MASK_ONEBURST & ~SI7210_MASK_STOP);
+	if (ret < 0)
+		return ret;
+
+	/* Read the contents of the registers containing the result: DSPSIGM, DSPSIGL */
+	ret = regmap_bulk_read(data->regmap, SI7210_REG_DSPSIGM, buf, 2);
+	if (ret < 0)
+		return ret;
+
+	*buf = cpu_to_be16(*buf);
+
+	return 0;
+}
+
+static int si7210_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct si7210_data *data = iio_priv(indio_dev);
+	long long tmp;
+	__be16 dspsig;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = si7210_fetch_measurement(data, chan, &dspsig);
+		if (ret < 0)
+			return ret;
+
+		*val = dspsig & GENMASK(14, 0);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		if (data->curr_scale == 20)
+			*val2 = 1250;
+		else /* data->curr_scale == 200 */
+			*val2 = 12500;
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = -16384;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_PROCESSED:
+		ret = si7210_fetch_measurement(data, chan, &dspsig);
+		if (ret < 0)
+			return ret;
+
+		tmp = FIELD_GET(GENMASK(14, 3), dspsig);
+		tmp = (div_s64(-383 * tmp * tmp, 100) + (160940 * tmp - 279800000));
+		tmp = (1 + (data->temp_gain >> 11)) * tmp + 62500 * data->temp_offset;
+
+		ret = regulator_get_voltage(data->vdd);
+		if (ret < 0)
+			return ret;
+
+		tmp -= 222 * div_s64(ret, 1000);
+
+		*val = div_s64(tmp, 1000);
+
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int si7210_set_scale(struct si7210_data *data, unsigned int scale)
+{
+	s8 *a_otp_values;
+	int ret;
+
+	if (scale == 20)
+		a_otp_values = data->scale_20_a;
+	else if (scale == 200)
+		a_otp_values = data->scale_200_a;
+	else
+		return -EINVAL;
+
+	guard(mutex)(&data->fetch_lock);
+
+	/* Write the registers 0xCA - 0xCC*/
+	ret = regmap_bulk_write(data->regmap, SI7210_REG_A0, a_otp_values, 3);
+	if (ret < 0)
+		return ret;
+
+	/* Write the registers 0xCE - 0xD0*/
+	ret = regmap_bulk_write(data->regmap, SI7210_REG_A3, &a_otp_values[3], 3);
+	if (ret < 0)
+		return ret;
+
+	data->curr_scale = scale;
+
+	return 0;
+}
+
+static int si7210_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct si7210_data *data = iio_priv(indio_dev);
+	unsigned int scale;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		if (val == 0 && val2 == 1250)
+			scale = 20;
+		else if (val == 0 && val2 == 12500)
+			scale = 200;
+		else
+			return -EINVAL;
+
+		return si7210_set_scale(data, scale);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int si7210_read_otpreg_val(struct si7210_data *data, unsigned int otpreg, u8 *val)
+{
+	int ret;
+	unsigned int otpdata;
+
+	ret = regmap_write(data->regmap, SI7210_REG_OTP_ADDR, otpreg);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_update_bits(data->regmap, SI7210_REG_OTP_CTRL,
+				 SI7210_MASK_OTP_READ_EN, SI7210_MASK_OTP_READ_EN);
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_read(data->regmap, SI7210_REG_OTP_DATA, &otpdata);
+	if (ret < 0)
+		return ret;
+
+	*val = (u8)otpdata;
+
+	return 0;
+}
+
+static int si7210_device_wake(struct si7210_data *data)
+{
+	/*
+	 * According to the datasheet, the primary method to wake up a
+	 * device is to send an empty write. However this is not feasible
+	 * using current API so we use the other method i.e. read a single
+	 * byte. The device should respond with 0xFF
+	 */
+
+	int ret = 0;
+
+	ret = i2c_smbus_read_byte(data->client);
+	if (ret < 0)
+		return ret;
+
+	if (ret != 0xFF)
+		return -EIO;
+
+	return 0;
+}
+
+static int si7210_device_init(struct si7210_data *data)
+{
+	int ret;
+	unsigned int i;
+
+	ret = si7210_device_wake(data);
+	if (ret < 0)
+		return ret;
+
+	fsleep(1000);
+
+	ret = si7210_read_otpreg_val(data, SI7210_OTPREG_TMP_GAIN, &data->temp_gain);
+	if (ret < 0)
+		return ret;
+	ret = si7210_read_otpreg_val(data, SI7210_OTPREG_TMP_OFF, &data->temp_offset);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < A_REGS_COUNT; i++) {
+		ret = si7210_read_otpreg_val(data, a20_otp_regs[i], &data->scale_20_a[i]);
+		if (ret < 0)
+			return ret;
+	}
+	for (i = 0; i < A_REGS_COUNT; i++) {
+		ret = si7210_read_otpreg_val(data, a200_otp_regs[i], &data->scale_200_a[i]);
+		if (ret < 0)
+			return ret;
+	}
+
+	ret = regmap_update_bits(data->regmap, SI7210_REG_ARAUTOINC,
+				 SI7210_MASK_ARAUTOINC, SI7210_MASK_ARAUTOINC);
+	if (ret < 0)
+		return ret;
+
+	return si7210_set_scale(data, 20);
+}
+
+static const struct iio_info si7210_info = {
+	.read_raw = si7210_read_raw,
+	.write_raw = si7210_write_raw,
+};
+
+static int si7210_probe(struct i2c_client *client)
+{
+	struct si7210_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+
+	mutex_init(&data->fetch_lock);
+
+	data->regmap = devm_regmap_init_i2c(client, &si7210_regmap_conf);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(data->regmap),
+				     "failed to register regmap\n");
+
+	data->vdd = devm_regulator_get(&client->dev, "vdd");
+	if (IS_ERR(data->vdd))
+		return dev_err_probe(&client->dev, PTR_ERR(data->vdd),
+				     "failed to get VDD regulator\n");
+
+	ret = regulator_enable(data->vdd);
+	if (ret)
+		return ret;
+
+	indio_dev->name = dev_name(&client->dev);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &si7210_info;
+	indio_dev->channels = si7210_channels;
+	indio_dev->num_channels = ARRAY_SIZE(si7210_channels);
+
+	ret = si7210_device_init(data);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+				     "device initialization failed\n");
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id si7210_id[] = {
+	{ "si7210" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, si7210_id);
+
+static const struct of_device_id si7210_dt_ids[] = {
+	{ .compatible = "silabs,si7210" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, si7210_dt_ids);
+
+static struct i2c_driver si7210_driver = {
+	.driver = {
+		.name = "si7210",
+		.of_match_table = si7210_dt_ids,
+	},
+	.probe = si7210_probe,
+	.id_table = si7210_id,
+};
+
+module_i2c_driver(si7210_driver);
+MODULE_AUTHOR("Antoni Pokusinski <apokusinski01@gmail.com>");
+MODULE_DESCRIPTION("Silicon Labs Si7210 Hall Effect sensor I2C driver");
+MODULE_LICENSE("Dual BSD/GPL");
-- 
2.25.1

Re: [PATCH v2 2/2] iio: magnetometer: si7210: add driver for Si7210

[kernel test robot]

Hi Antoni,

kernel test robot noticed the following build warnings:

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

url:    https://github.com/intel-lab-lkp/linux/commits/Antoni-Pokusinski/dt-bindings-iio-magnetometer-add-binding-for-Si7210/20250109-074641
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20250108234411.882768-3-apokusinski01%40gmail.com
patch subject: [PATCH v2 2/2] iio: magnetometer: si7210: add driver for Si7210
config: openrisc-randconfig-r123-20250110 (https://download.01.org/0day-ci/archive/20250110/202501101656.F268NchW-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250110/202501101656.F268NchW-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501101656.F268NchW-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/iio/magnetometer/si7210.c:169:16: sparse: sparse: cast from restricted __be16
>> drivers/iio/magnetometer/si7210.c:189:24: sparse: sparse: restricted __be16 degrades to integer
>> drivers/iio/magnetometer/si7210.c:206:23: sparse: sparse: cast to restricted __be16
   drivers/iio/magnetometer/si7210.c:206:23: sparse: sparse: restricted __be16 degrades to integer
   drivers/iio/magnetometer/si7210.c:206:23: sparse: sparse: restricted __be16 degrades to integer

vim +169 drivers/iio/magnetometer/si7210.c

   143	
   144	static int si7210_fetch_measurement(struct si7210_data *data,
   145					    struct iio_chan_spec const *chan,
   146					    __be16 *buf)
   147	{
   148		u8 dspsigsel = chan->type == IIO_MAGN ? 0 : 1;
   149		int ret;
   150	
   151		guard(mutex)(&data->fetch_lock);
   152	
   153		ret = regmap_update_bits(data->regmap, SI7210_REG_DSPSIGSEL,
   154					 SI7210_MASK_DSPSIGSEL, dspsigsel);
   155		if (ret < 0)
   156			return ret;
   157	
   158		ret = regmap_update_bits(data->regmap, SI7210_REG_POWER_CTRL,
   159					 SI7210_MASK_ONEBURST | SI7210_MASK_STOP,
   160					 SI7210_MASK_ONEBURST & ~SI7210_MASK_STOP);
   161		if (ret < 0)
   162			return ret;
   163	
   164		/* Read the contents of the registers containing the result: DSPSIGM, DSPSIGL */
   165		ret = regmap_bulk_read(data->regmap, SI7210_REG_DSPSIGM, buf, 2);
   166		if (ret < 0)
   167			return ret;
   168	
 > 169		*buf = cpu_to_be16(*buf);
   170	
   171		return 0;
   172	}
   173	
   174	static int si7210_read_raw(struct iio_dev *indio_dev,
   175				   struct iio_chan_spec const *chan,
   176				   int *val, int *val2, long mask)
   177	{
   178		struct si7210_data *data = iio_priv(indio_dev);
   179		long long tmp;
   180		__be16 dspsig;
   181		int ret;
   182	
   183		switch (mask) {
   184		case IIO_CHAN_INFO_RAW:
   185			ret = si7210_fetch_measurement(data, chan, &dspsig);
   186			if (ret < 0)
   187				return ret;
   188	
 > 189			*val = dspsig & GENMASK(14, 0);
   190			return IIO_VAL_INT;
   191		case IIO_CHAN_INFO_SCALE:
   192			*val = 0;
   193			if (data->curr_scale == 20)
   194				*val2 = 1250;
   195			else /* data->curr_scale == 200 */
   196				*val2 = 12500;
   197			return IIO_VAL_INT_PLUS_MICRO;
   198		case IIO_CHAN_INFO_OFFSET:
   199			*val = -16384;
   200			return IIO_VAL_INT;
   201		case IIO_CHAN_INFO_PROCESSED:
   202			ret = si7210_fetch_measurement(data, chan, &dspsig);
   203			if (ret < 0)
   204				return ret;
   205	
 > 206			tmp = FIELD_GET(GENMASK(14, 3), dspsig);
   207			tmp = (div_s64(-383 * tmp * tmp, 100) + (160940 * tmp - 279800000));
   208			tmp = (1 + (data->temp_gain >> 11)) * tmp + 62500 * data->temp_offset;
   209	
   210			ret = regulator_get_voltage(data->vdd);
   211			if (ret < 0)
   212				return ret;
   213	
   214			tmp -= 222 * div_s64(ret, 1000);
   215	
   216			*val = div_s64(tmp, 1000);
   217	
   218			return IIO_VAL_INT;
   219		default:
   220			return -EINVAL;
   221		}
   222	}
   223	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] dt-bindings: iio: magnetometer: add binding for Si7210

[Krzysztof Kozlowski]

On 09/01/2025 00:44, Antoni Pokusinski wrote:
> Silicon Labs Si7210 is an I2C Hall effect magnetic position
> and temperature sensor.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 2/2] iio: magnetometer: si7210: add driver for Si7210

[Christophe JAILLET]

Le 09/01/2025 à 00:44, Antoni Pokusinski a écrit :
> Silicon Labs Si7210 is an I2C Hall effect magnetic position and
> temperature sensor. The driver supports the following functionalities:
> * reading the temperature measurements
> * reading the magnetic field measurements in a single-shot mode
> * choosing the magnetic field measurement scale (20 or 200 mT)

...

> diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
> index 3e4c2ecd9adf..58f32a855494 100644
> --- a/drivers/iio/magnetometer/Makefile
> +++ b/drivers/iio/magnetometer/Makefile
> @@ -31,6 +31,8 @@ obj-$(CONFIG_SENSORS_RM3100)		+= rm3100-core.o
>   obj-$(CONFIG_SENSORS_RM3100_I2C)	+= rm3100-i2c.o
>   obj-$(CONFIG_SENSORS_RM3100_SPI)	+= rm3100-spi.o
>   
> +obj-$(CONFIG_SI7210) += si7210.o

Maybe add some tabs, to align?

> +
>   obj-$(CONFIG_TI_TMAG5273)		+= tmag5273.o
>   
>   obj-$(CONFIG_YAMAHA_YAS530)		+= yamaha-yas530.o

...

> +static int si7210_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct si7210_data *data = iio_priv(indio_dev);
> +	long long tmp;

I think that temp is better than tmp
(temperature vs temporary?)

> +	__be16 dspsig;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = si7210_fetch_measurement(data, chan, &dspsig);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = dspsig & GENMASK(14, 0);
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		if (data->curr_scale == 20)
> +			*val2 = 1250;
> +		else /* data->curr_scale == 200 */
> +			*val2 = 12500;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = -16384;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_PROCESSED:
> +		ret = si7210_fetch_measurement(data, chan, &dspsig);
> +		if (ret < 0)
> +			return ret;
> +
> +		tmp = FIELD_GET(GENMASK(14, 3), dspsig);
> +		tmp = (div_s64(-383 * tmp * tmp, 100) + (160940 * tmp - 279800000));

I would keep 279800000 outside of the (), or even write it to better 
match the spec:
	tmp = div_s64(-383 * tmp * tmp, 100) + 160940 * tmp - 279800000;

Here, tmp seems to be Temperature_raw from the sepc * 10^6


> +		tmp = (1 + (data->temp_gain >> 11)) * tmp + 62500 * data->temp_offset;

/ 2048 instead of >> 11 to match the formula in the sepc?

Would it be clearer to have 62500 written as (1000000 / 32)

> +
> +		ret = regulator_get_voltage(data->vdd);
> +		if (ret < 0)
> +			return ret;
> +
> +		tmp -= 222 * div_s64(ret, 1000);
> +
> +		*val = div_s64(tmp, 1000);
> +
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int si7210_set_scale(struct si7210_data *data, unsigned int scale)
> +{
> +	s8 *a_otp_values;
> +	int ret;
> +
> +	if (scale == 20)
> +		a_otp_values = data->scale_20_a;
> +	else if (scale == 200)
> +		a_otp_values = data->scale_200_a;
> +	else
> +		return -EINVAL;
> +
> +	guard(mutex)(&data->fetch_lock);
> +
> +	/* Write the registers 0xCA - 0xCC*/

Missing space before */

> +	ret = regmap_bulk_write(data->regmap, SI7210_REG_A0, a_otp_values, 3);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Write the registers 0xCE - 0xD0*/

Missing space before */

> +	ret = regmap_bulk_write(data->regmap, SI7210_REG_A3, &a_otp_values[3], 3);
> +	if (ret < 0)
> +		return ret;
> +
> +	data->curr_scale = scale;
> +
> +	return 0;
> +}

...

> +static int si7210_device_wake(struct si7210_data *data)
> +{
> +	/*
> +	 * According to the datasheet, the primary method to wake up a
> +	 * device is to send an empty write. However this is not feasible
> +	 * using current API so we use the other method i.e. read a single
> +	 * byte. The device should respond with 0xFF

Nitpick: Missing ending .

> +	 */
> +
> +	int ret = 0;

No need to init.

> +
> +	ret = i2c_smbus_read_byte(data->client);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (ret != 0xFF)
> +		return -EIO;
> +
> +	return 0;
> +}

...

> +MODULE_DESCRIPTION("Silicon Labs Si7210 Hall Effect sensor I2C driver");
> +MODULE_LICENSE("Dual BSD/GPL");

This is not consistent with the SPDX tag.

CJ

Re: [PATCH v2 2/2] iio: magnetometer: si7210: add driver for Si7210

[kernel test robot]

Hi Antoni,

kernel test robot noticed the following build warnings:

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

url:    https://github.com/intel-lab-lkp/linux/commits/Antoni-Pokusinski/dt-bindings-iio-magnetometer-add-binding-for-Si7210/20250109-074641
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20250108234411.882768-3-apokusinski01%40gmail.com
patch subject: [PATCH v2 2/2] iio: magnetometer: si7210: add driver for Si7210
config: arc-randconfig-r111-20250111 (https://download.01.org/0day-ci/archive/20250111/202501110655.qR09D59T-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250111/202501110655.qR09D59T-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501110655.qR09D59T-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/iio/magnetometer/si7210.c:169:16: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned short [usertype] val @@     got restricted __be16 [usertype] @@
   drivers/iio/magnetometer/si7210.c:169:16: sparse:     expected unsigned short [usertype] val
   drivers/iio/magnetometer/si7210.c:169:16: sparse:     got restricted __be16 [usertype]
   drivers/iio/magnetometer/si7210.c:169:16: sparse: sparse: cast from restricted __be16
   drivers/iio/magnetometer/si7210.c:169:16: sparse: sparse: cast from restricted __be16
   drivers/iio/magnetometer/si7210.c:189:24: sparse: sparse: restricted __be16 degrades to integer
   drivers/iio/magnetometer/si7210.c:206:23: sparse: sparse: cast to restricted __be16
   drivers/iio/magnetometer/si7210.c:206:23: sparse: sparse: restricted __be16 degrades to integer
   drivers/iio/magnetometer/si7210.c:206:23: sparse: sparse: restricted __be16 degrades to integer

vim +169 drivers/iio/magnetometer/si7210.c

   143	
   144	static int si7210_fetch_measurement(struct si7210_data *data,
   145					    struct iio_chan_spec const *chan,
   146					    __be16 *buf)
   147	{
   148		u8 dspsigsel = chan->type == IIO_MAGN ? 0 : 1;
   149		int ret;
   150	
   151		guard(mutex)(&data->fetch_lock);
   152	
   153		ret = regmap_update_bits(data->regmap, SI7210_REG_DSPSIGSEL,
   154					 SI7210_MASK_DSPSIGSEL, dspsigsel);
   155		if (ret < 0)
   156			return ret;
   157	
   158		ret = regmap_update_bits(data->regmap, SI7210_REG_POWER_CTRL,
   159					 SI7210_MASK_ONEBURST | SI7210_MASK_STOP,
   160					 SI7210_MASK_ONEBURST & ~SI7210_MASK_STOP);
   161		if (ret < 0)
   162			return ret;
   163	
   164		/* Read the contents of the registers containing the result: DSPSIGM, DSPSIGL */
   165		ret = regmap_bulk_read(data->regmap, SI7210_REG_DSPSIGM, buf, 2);
   166		if (ret < 0)
   167			return ret;
   168	
 > 169		*buf = cpu_to_be16(*buf);
   170	
   171		return 0;
   172	}
   173	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki