[PATCH v3 0/4] iio: cdc: add Texas Instruments FDC1004 driver

[PATCH v3 0/4] iio: cdc: add Texas Instruments FDC1004 driver

[Victor Chernyakin]

This series adds support for the FDC1004 4-channel I2C capacitive sensor.
This sensor can report single-ended measurements of any one of its
channels, as well as differential measurements between any pair
of them. It has a measurement range of +-15 pF and a resolution of 0.5 fF.

Patch 1 prepares iio_format_value to handle IIO_VAL_DECIMAL64_* formats
introduced in Patch 2.
Patch 2 adds higher-precision IIO_VAL_DECIMAL64_* formats, used
in the driver.
Patch 3 adds the devicetree bindings.
Patch 4 adds the driver itself.

Tested with the sensor connected over an i2c-tiny-usb adapter.

---
Changes in v3:

Split Patch 1 into two patches.

- Patch 1
  - Expanded the comment above abs_s64_no_overflow()

- Patch 4 (previously Patch 3)
  - Added comment on what a negative capacitance bias is used for
  - Reduced some line lengths by introducing a variable with a short name
  - Joined consecutive assignments into single ones
  - Fixed indentation

- Link to v2: https://lore.kernel.org/SN1PR12MB2528DD1E6067CC4C0B76746FA5622@SN1PR12MB2528.namprd12.prod.outlook.com/

---
Changes in v2:
- Patch 1
  - Added tests
  - Fixed overflow in iio_format_value on 10^scale and UB on abs(LLONG_MIN)
- Patch 2
  - Made the power supply mandatory
- Patch 3
  - Switched from direct I2C calls to regmap
  - Added of device table
  - Removed scan indexes
  - Build up complex values incrementally
  - Adjusted list of includes
  - More designed initializers
  - Various style/whitespace tweaks
- Link to v1: https://lore.kernel.org/SN1PR12MB25286BE4E1EA3B75F8E563A9A5652@SN1PR12MB2528.namprd12.prod.outlook.com

---
Victor Chernyakin (4):
  iio: prepare iio_format_value to handle higher-precision
    IIO_VAL_DECIMAL64_* formats
  iio: add IIO_VAL_DECIMAL64_{ATTO,ZEPTO,YOCTO} formats
  dt-bindings: iio: cdc: add Texas Instruments FDC1004
  iio: cdc: add Texas Instruments FDC1004 driver

 .../bindings/iio/cdc/ti,fdc1004.yaml          |  46 ++
 MAINTAINERS                                   |   6 +
 drivers/iio/cdc/Kconfig                       |  10 +
 drivers/iio/cdc/Makefile                      |   1 +
 drivers/iio/cdc/fdc1004.c                     | 488 ++++++++++++++++++
 drivers/iio/industrialio-core.c               |  32 +-
 drivers/iio/test/iio-test-format.c            |  16 +
 include/linux/iio/types.h                     |   3 +
 8 files changed, 598 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml
 create mode 100644 drivers/iio/cdc/fdc1004.c

-- 
2.53.0

[PATCH v3 1/4] iio: prepare iio_format_value to handle higher-precision IIO_VAL_DECIMAL64_* formats

[Victor Chernyakin]

Prepare iio_format_value for an upcoming patch that adds formats up
to IIO_VAL_DECIMAL64_YOCTO (a scale of 24 digits) by avoiding
computing 10^scale when doing so would overflow, and by avoiding
undefined behavior due to abs(LLONG_MIN) (which couldn't happen
previously).

Signed-off-by: Victor Chernyakin <chernyakin.victor.j@outlook.com>
---
 drivers/iio/industrialio-core.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 767a7794624a..c30659ca0544 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -653,6 +653,18 @@ int iio_read_mount_matrix(struct device *dev, struct iio_mount_matrix *matrix)
 }
 EXPORT_SYMBOL(iio_read_mount_matrix);
 
+/*
+ * Regular abs(LLONG_MIN) causes signed overflow, which is undefined
+ * behavior. This variant avoids that by performing the computation in an
+ * unsigned type, for which negation is always well-defined.
+ */
+static inline u64 abs_s64_no_overflow(s64 x)
+{
+	return x < 0 ? -(u64)x : (u64)x;
+}
+
+#define GREATEST_POWER_OF_10_THAT_FITS_IN_S64 18
+
 static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
 				  int size, const int *vals)
 {
@@ -720,13 +732,19 @@ static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
 		int scale = type - IIO_VAL_DECIMAL64_BASE;
 		s64 frac;
 
-		tmp2 = div64_s64_rem(iio_val_s64_compose(vals[0], vals[1]),
-				     int_pow(10, scale), &frac);
+		if (scale <= GREATEST_POWER_OF_10_THAT_FITS_IN_S64) {
+			tmp2 = div64_s64_rem(iio_val_s64_compose(vals[0], vals[1]),
+					     int_pow(10, scale), &frac);
+		} else {
+			tmp2 = 0;
+			frac = iio_val_s64_compose(vals[0], vals[1]);
+		}
+
 		if (tmp2 == 0 && frac < 0)
 			l += sysfs_emit_at(buf, offset, "-");
 
-		l += sysfs_emit_at(buf, offset + l, "%lld.%0*lld", tmp2, scale,
-				   abs(frac));
+		l += sysfs_emit_at(buf, offset + l, "%lld.%0*llu", tmp2, scale,
+				   abs_s64_no_overflow(frac));
 		return l;
 	}
 	default:
-- 
2.53.0

[PATCH v3 2/4] iio: add IIO_VAL_DECIMAL64_{ATTO,ZEPTO,YOCTO} formats

[Victor Chernyakin]

The highest-precision format IIO has right now is IIO_VAL_DECIMAL64_FEMTO,
which gives 15 decimal digits of precision after the decimal point. This
is not enough for the upcoming TI FDC1004 driver, which needs 22 digits
of precision to exactly express its scale value. This patch adds higher-
precision formats, up to IIO_VAL_DECIMAL64_YOCTO (24 digits of precision).

Signed-off-by: Victor Chernyakin <chernyakin.victor.j@outlook.com>
---
 drivers/iio/industrialio-core.c    |  6 ++++++
 drivers/iio/test/iio-test-format.c | 16 ++++++++++++++++
 include/linux/iio/types.h          |  3 +++
 3 files changed, 25 insertions(+)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index c30659ca0544..632d91e09ef2 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -728,6 +728,9 @@ static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
 	case IIO_VAL_DECIMAL64_NANO:
 	case IIO_VAL_DECIMAL64_PICO:
 	case IIO_VAL_DECIMAL64_FEMTO:
+	case IIO_VAL_DECIMAL64_ATTO:
+	case IIO_VAL_DECIMAL64_ZEPTO:
+	case IIO_VAL_DECIMAL64_YOCTO:
 	{
 		int scale = type - IIO_VAL_DECIMAL64_BASE;
 		s64 frac;
@@ -1052,6 +1055,9 @@ static ssize_t iio_write_channel_info(struct device *dev,
 		case IIO_VAL_DECIMAL64_NANO:
 		case IIO_VAL_DECIMAL64_PICO:
 		case IIO_VAL_DECIMAL64_FEMTO:
+		case IIO_VAL_DECIMAL64_ATTO:
+		case IIO_VAL_DECIMAL64_ZEPTO:
+		case IIO_VAL_DECIMAL64_YOCTO:
 			dec_scale = type - IIO_VAL_DECIMAL64_BASE;
 			fallthrough;
 		case IIO_VAL_INT_64:
diff --git a/drivers/iio/test/iio-test-format.c b/drivers/iio/test/iio-test-format.c
index e5d0a2ac4670..43a80952a50f 100644
--- a/drivers/iio/test/iio-test-format.c
+++ b/drivers/iio/test/iio-test-format.c
@@ -286,6 +286,14 @@ static void iio_test_iio_format_value_decimal_64(struct kunit *test)
 
 	/* limits */
 	iio_val_s64_decompose(LLONG_MAX, &values[0], &values[1]);
+	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_YOCTO, ARRAY_SIZE(values), values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000009223372036854775807\n");
+	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_ZEPTO, ARRAY_SIZE(values), values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.009223372036854775807\n");
+	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_ATTO, ARRAY_SIZE(values), values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "9.223372036854775807\n");
+	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_FEMTO, ARRAY_SIZE(values), values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "9223.372036854775807\n");
 	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_PICO, ARRAY_SIZE(values), values);
 	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "9223372.036854775807\n");
 	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_NANO, ARRAY_SIZE(values), values);
@@ -294,6 +302,14 @@ static void iio_test_iio_format_value_decimal_64(struct kunit *test)
 	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "9223372036854.775807\n");
 
 	iio_val_s64_decompose(LLONG_MIN, &values[0], &values[1]);
+	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_YOCTO, ARRAY_SIZE(values), values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000009223372036854775808\n");
+	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_ZEPTO, ARRAY_SIZE(values), values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.009223372036854775808\n");
+	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_ATTO, ARRAY_SIZE(values), values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-9.223372036854775808\n");
+	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_FEMTO, ARRAY_SIZE(values), values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-9223.372036854775808\n");
 	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_PICO, ARRAY_SIZE(values), values);
 	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-9223372.036854775808\n");
 	ret = iio_format_value(buf, IIO_VAL_DECIMAL64_NANO, ARRAY_SIZE(values), values);
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index d8944c9e5e90..2ef9b2a8b119 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -43,6 +43,9 @@ enum iio_event_info {
 #define IIO_VAL_DECIMAL64_NANO		(IIO_VAL_DECIMAL64_BASE + 9)
 #define IIO_VAL_DECIMAL64_PICO		(IIO_VAL_DECIMAL64_BASE + 12)
 #define IIO_VAL_DECIMAL64_FEMTO		(IIO_VAL_DECIMAL64_BASE + 15)
+#define IIO_VAL_DECIMAL64_ATTO		(IIO_VAL_DECIMAL64_BASE + 18)
+#define IIO_VAL_DECIMAL64_ZEPTO		(IIO_VAL_DECIMAL64_BASE + 21)
+#define IIO_VAL_DECIMAL64_YOCTO		(IIO_VAL_DECIMAL64_BASE + 24)
 
 static inline s64 iio_val_s64_compose(s32 val0, s32 val1)
 {
-- 
2.53.0

[PATCH v3 3/4] dt-bindings: iio: cdc: add Texas Instruments FDC1004

[Victor Chernyakin]

Add device tree bindings for the Texas Instruments FDC1004
capacitive sensor.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Victor Chernyakin <chernyakin.victor.j@outlook.com>
---
 .../bindings/iio/cdc/ti,fdc1004.yaml          | 46 +++++++++++++++++++
 MAINTAINERS                                   |  5 ++
 2 files changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml

diff --git a/Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml b/Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml
new file mode 100644
index 000000000000..8366afd025a6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/cdc/ti,fdc1004.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments FDC1004 capacitive sensor
+
+maintainers:
+  - Victor Chernyakin <chernyakin.victor.j@outlook.com>
+
+description: |
+  The FDC1004 is a high-resolution 4-channel capacitance to digital converter
+  that works over I2C.
+
+  https://www.ti.com/product/FDC1004
+
+properties:
+  compatible:
+    const: ti,fdc1004
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        cdc@50 {
+            compatible = "ti,fdc1004";
+            reg = <0x50>;
+            vdd-supply = <&vdd_3v3>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 360977678f70..69c23ae21016 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27388,6 +27388,11 @@ S:	Maintained
 F:	drivers/net/ethernet/ti/cpsw*
 F:	drivers/net/ethernet/ti/davinci*
 
+TI FDC1004 DRIVER
+M:	Victor Chernyakin <chernyakin.victor.j@outlook.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml
+
 TI FLASH MEDIA MEMORYSTICK/MMC DRIVERS
 M:	Alex Dubov <oakad@yahoo.com>
 S:	Maintained
-- 
2.53.0

[PATCH v3 4/4] iio: cdc: add Texas Instruments FDC1004 driver

[Victor Chernyakin]

The FDC1004 is a high-precision 4-channel capacitive sensor that talks
over I2C. It can report the values of each of its 4 channels individually,
as well as the difference between any two of them.

This initial driver allows performing single reads and configuring
the sensor's various settings (sample frequency, calibration
gain/offset, etc.). It could be extended with support for triggered
buffers, repeated measurement mode, and/or power management.

Signed-off-by: Victor Chernyakin <chernyakin.victor.j@outlook.com>
---
 MAINTAINERS               |   1 +
 drivers/iio/cdc/Kconfig   |  10 +
 drivers/iio/cdc/Makefile  |   1 +
 drivers/iio/cdc/fdc1004.c | 488 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 500 insertions(+)
 create mode 100644 drivers/iio/cdc/fdc1004.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 69c23ae21016..4d926c47ad8d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27392,6 +27392,7 @@ TI FDC1004 DRIVER
 M:	Victor Chernyakin <chernyakin.victor.j@outlook.com>
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml
+F:	drivers/iio/cdc/fdc1004.c
 
 TI FLASH MEDIA MEMORYSTICK/MMC DRIVERS
 M:	Alex Dubov <oakad@yahoo.com>
diff --git a/drivers/iio/cdc/Kconfig b/drivers/iio/cdc/Kconfig
index e0a5ce66a984..6b9ea2cd37bd 100644
--- a/drivers/iio/cdc/Kconfig
+++ b/drivers/iio/cdc/Kconfig
@@ -24,4 +24,14 @@ config AD7746
 	  To compile this driver as a module, choose M here: the
 	  module will be called ad7746.
 
+config FDC1004
+	tristate "Texas Instruments FDC1004 capacitive sensor driver"
+	depends on I2C
+	help
+	  Say yes here to build support for the Texas Instruments FDC1004
+	  capacitive sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called fdc1004.
+
 endmenu
diff --git a/drivers/iio/cdc/Makefile b/drivers/iio/cdc/Makefile
index 41db756d8020..88253f33c5a7 100644
--- a/drivers/iio/cdc/Makefile
+++ b/drivers/iio/cdc/Makefile
@@ -5,3 +5,4 @@
 
 obj-$(CONFIG_AD7150) += ad7150.o
 obj-$(CONFIG_AD7746) += ad7746.o
+obj-$(CONFIG_FDC1004) += fdc1004.o
diff --git a/drivers/iio/cdc/fdc1004.c b/drivers/iio/cdc/fdc1004.c
new file mode 100644
index 000000000000..1fe2874cd65a
--- /dev/null
+++ b/drivers/iio/cdc/fdc1004.c
@@ -0,0 +1,488 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Texas Instruments FDC1004 capacitive sensor driver
+ *
+ * Copyright (c) 2026 Victor Chernyakin <chernyakin.victor.j@outlook.com>
+ *
+ * Datasheet: https://www.ti.com/lit/gpn/FDC1004
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/math.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/types.h>
+
+#include <linux/iio/iio.h>
+
+#define FDC1004_REG_MEASn_MSB(n)	(2 * (n))
+#define FDC1004_REG_MEASn_LSB(n)	(2 * (n) + 1)
+#define FDC1004_REG_CONF_MEASn(n)	(0x08 + (n))
+#define FDC1004_REG_FDC_CONF		0x0C
+#define FDC1004_REG_OFFSET_CAL_CINn(n)	(0x0D + (n))
+#define FDC1004_REG_GAIN_CAL_CINn(n)	(0x11 + (n))
+
+#define FDC1004_CONF_MEAS_CHA_MASK	GENMASK(15, 13)
+#define FDC1004_CONF_MEAS_CHB_MASK	GENMASK(12, 10)
+#define FDC1004_CONF_MEAS_CHB_CAPDAC	0b100
+#define FDC1004_CONF_MEAS_CAPDAC_MASK	GENMASK(9, 5)
+
+#define FDC1004_FDC_CONF_DONE_n_MASK(n)	BIT(3 - (n))
+#define FDC1004_FDC_CONF_MEAS_n_MASK(n)	BIT(7 - (n))
+#define FDC1004_FDC_CONF_RATE_MASK	GENMASK(11, 10)
+#define FDC1004_FDC_CONF_RST_MASK	BIT(15)
+
+static const int fdc1004_samp_freqs[] = { 100, 200, 400 };
+
+/*
+ * MEASn_{MSB,LSB}: 24 bits, signed. Format: IIO_VAL_DECIMAL64_YOCTO.
+ * 1 step = 1/(2^19) pF = 1/(1000 * 2^19) nF = 0.000000001907348632812500 nF.
+ */
+static const s64 fdc1004_scale = 1907348632812500;
+
+/*
+ * CAPDAC field: 5 bits, unsigned. Format: IIO_VAL_INT.
+ * 1 step = 3.125 pF = 3.125 / 1/(2^19) raw value steps = 1638400 raw value steps.
+ */
+static const int fdc1004_offset[] = {
+	0x00 * 1638400, /* min */
+	0x01 * 1638400, /* step */
+	0x1F * 1638400, /* max */
+};
+
+/*
+ * OFFSET_CAL_CINn: 16 bits, signed. Format: IIO_VAL_DECIMAL64_FEMTO.
+ * 1 step = 1/(2^11) pF = 1/(1000 * 2^11) nF = 0.000000488281250 nF.
+ * Negative values are used to cancel out any parasitic capacitance
+ * of the sensor's external circuitry.
+ */
+static const s64 fdc1004_calibbias[] = {
+	-0x8000 * 488281250UL, /* min */
+	+0x0001 * 488281250UL, /* step */
+	+0x7FFF * 488281250UL, /* max */
+};
+
+/*
+ * GAIN_CAL_CINn: 16 bits, unsigned. Format: IIO_VAL_DECIMAL64_FEMTO.
+ * 1 step = 1/(2^14) pF/pF = 0.000061035156250 pF/pF.
+ */
+static const s64 fdc1004_calibscale[] = {
+	0x0000 * 61035156250UL, /* min */
+	0x0001 * 61035156250UL, /* step */
+	0xFFFF * 61035156250UL, /* max */
+};
+
+static const struct regmap_range fdc1004_readable_reg_ranges[] = {
+	regmap_reg_range(FDC1004_REG_MEASn_MSB(0), FDC1004_REG_GAIN_CAL_CINn(3)),
+};
+
+static const struct regmap_access_table fdc1004_readable_reg_table = {
+	.yes_ranges = fdc1004_readable_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(fdc1004_readable_reg_ranges),
+};
+
+static const struct regmap_range fdc1004_writeable_reg_ranges[] = {
+	regmap_reg_range(FDC1004_REG_CONF_MEASn(0), FDC1004_REG_GAIN_CAL_CINn(3)),
+};
+
+static const struct regmap_access_table fdc1004_writeable_reg_table = {
+	.yes_ranges = fdc1004_writeable_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(fdc1004_writeable_reg_ranges),
+};
+
+static const struct regmap_range fdc1004_volatile_reg_ranges[] = {
+	regmap_reg_range(FDC1004_REG_MEASn_MSB(0), FDC1004_REG_MEASn_LSB(3)),
+	regmap_reg_range(FDC1004_REG_FDC_CONF, FDC1004_REG_FDC_CONF),
+};
+
+static const struct regmap_access_table fdc1004_volatile_reg_table = {
+	.yes_ranges = fdc1004_volatile_reg_ranges,
+	.n_yes_ranges = ARRAY_SIZE(fdc1004_volatile_reg_ranges),
+};
+
+static const struct regmap_config fdc1004_regmap_config = {
+	.name = "fdc1004",
+	.reg_bits = 8,
+	.val_bits = 16,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+	.max_register = FDC1004_REG_GAIN_CAL_CINn(3),
+	.rd_table = &fdc1004_readable_reg_table,
+	.wr_table = &fdc1004_writeable_reg_table,
+	.volatile_table = &fdc1004_volatile_reg_table,
+	.cache_type = REGCACHE_MAPLE,
+};
+
+struct fdc1004_data {
+	struct regmap *regmap;
+
+	/*
+	 * Serializes accesses to the chip's measurement and config
+	 * registers, and to the other struct members below.
+	 */
+	struct mutex lock;
+
+	u8 offsets[4];
+	u8 samp_freq_index;
+};
+
+static int fdc1004_soft_reset(struct fdc1004_data *data)
+{
+	int ret;
+	unsigned int val;
+
+	ret = regmap_write(data->regmap, FDC1004_REG_FDC_CONF,
+			   FDC1004_FDC_CONF_RST_MASK);
+	if (ret)
+		return ret;
+
+	return regmap_read_poll_timeout(data->regmap, FDC1004_REG_FDC_CONF, val,
+					!(ret & FDC1004_FDC_CONF_RST_MASK),
+					1 * USEC_PER_MSEC, 200 * USEC_PER_MSEC);
+}
+
+static int fdc1004_read_raw(struct iio_dev *indio_dev,
+			    const struct iio_chan_spec *chan,
+			    int *val, int *val2, long mask)
+{
+	struct fdc1004_data *data = iio_priv(indio_dev);
+	struct regmap *map = data->regmap;
+	unsigned int reg_val;
+	u16 reg_vals[2];
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		iio_val_s64_decompose(fdc1004_scale, val, val2);
+		return IIO_VAL_DECIMAL64_YOCTO;
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&data->lock);
+
+		/* Configure the measurement we want. */
+		reg_val = FIELD_PREP(FDC1004_CONF_MEAS_CHA_MASK, chan->channel)
+			| FIELD_PREP(FDC1004_CONF_MEAS_CHB_MASK, chan->channel2)
+			| FIELD_PREP(FDC1004_CONF_MEAS_CAPDAC_MASK,
+				      data->offsets[chan->channel]);
+		ret = regmap_write(map, FDC1004_REG_CONF_MEASn(0), reg_val);
+		if (ret)
+			return ret;
+
+		/* Start the measurement. */
+		reg_val = FDC1004_FDC_CONF_MEAS_n_MASK(0)
+			/* "+ 1" because the chip wants a 1-indexed value. */
+			| FIELD_PREP(FDC1004_FDC_CONF_RATE_MASK,
+				     data->samp_freq_index + 1);
+		ret = regmap_write(map, FDC1004_REG_FDC_CONF, reg_val);
+		if (ret)
+			return ret;
+
+		/*
+		 * Wait for the measurement to complete.
+		 * The slowest sampling frequency the chip supports is 100 SPS,
+		 * i.e. 1 sample every 10 ms, so our timeout can't be shorter
+		 * than that. Go with 40 ms to be safe.
+		 */
+		ret = regmap_read_poll_timeout(map, FDC1004_REG_FDC_CONF, reg_val,
+					       reg_val & FDC1004_FDC_CONF_DONE_n_MASK(0),
+					       1 * USEC_PER_MSEC, 40 * USEC_PER_MSEC);
+		if (ret)
+			return ret;
+
+		/*
+		 * Now read the two registers making up the measurement.
+		 * The datasheet says to read MSB first, LSB second.
+		 */
+		ret = regmap_bulk_read(map, FDC1004_REG_MEASn_MSB(0),
+				       reg_vals, ARRAY_SIZE(reg_vals));
+		if (ret)
+			return ret;
+
+		*val = (sign_extend32(reg_vals[0], 15) << 8) | (reg_vals[1] >> 8);
+		return IIO_VAL_INT;
+	}
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		guard(mutex)(&data->lock);
+
+		*val = fdc1004_samp_freqs[data->samp_freq_index];
+		return IIO_VAL_INT;
+	}
+	case IIO_CHAN_INFO_OFFSET: {
+		guard(mutex)(&data->lock);
+
+		*val = data->offsets[chan->channel] * fdc1004_offset[1];
+		return IIO_VAL_INT;
+	}
+	case IIO_CHAN_INFO_CALIBBIAS:
+		ret = regmap_read(map, FDC1004_REG_OFFSET_CAL_CINn(chan->channel),
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		iio_val_s64_decompose(sign_extend64(reg_val, 15) *
+				      fdc1004_calibbias[1], val, val2);
+		return IIO_VAL_DECIMAL64_FEMTO;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		ret = regmap_read(map, FDC1004_REG_GAIN_CAL_CINn(chan->channel),
+				  &reg_val);
+		if (ret)
+			return ret;
+
+		iio_val_s64_decompose(reg_val * fdc1004_calibscale[1], val, val2);
+		return IIO_VAL_DECIMAL64_FEMTO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const int fdc1004_calibbias_decomposed[] = {
+	lower_32_bits(fdc1004_calibbias[0]), upper_32_bits(fdc1004_calibbias[0]),
+	lower_32_bits(fdc1004_calibbias[1]), upper_32_bits(fdc1004_calibbias[1]),
+	lower_32_bits(fdc1004_calibbias[2]), upper_32_bits(fdc1004_calibbias[2]),
+};
+
+static const int fdc1004_calibscale_decomposed[] = {
+	lower_32_bits(fdc1004_calibscale[0]), upper_32_bits(fdc1004_calibscale[0]),
+	lower_32_bits(fdc1004_calibscale[1]), upper_32_bits(fdc1004_calibscale[1]),
+	lower_32_bits(fdc1004_calibscale[2]), upper_32_bits(fdc1004_calibscale[2]),
+};
+
+static int fdc1004_read_avail(struct iio_dev *indio_dev,
+			      const struct iio_chan_spec *chan,
+			      const int **vals, int *type, int *length,
+			      long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = fdc1004_samp_freqs;
+		*length = ARRAY_SIZE(fdc1004_samp_freqs);
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_OFFSET:
+		*vals = fdc1004_offset;
+		*length = ARRAY_SIZE(fdc1004_offset);
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_RANGE;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		*vals = fdc1004_calibbias_decomposed;
+		*length = ARRAY_SIZE(fdc1004_calibbias_decomposed);
+		*type = IIO_VAL_DECIMAL64_FEMTO;
+		return IIO_AVAIL_RANGE;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		*vals = fdc1004_calibscale_decomposed;
+		*length = ARRAY_SIZE(fdc1004_calibscale_decomposed);
+		*type = IIO_VAL_DECIMAL64_FEMTO;
+		return IIO_AVAIL_RANGE;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int fdc1004_write_raw_get_fmt(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+	case IIO_CHAN_INFO_OFFSET:
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_CALIBBIAS:
+	case IIO_CHAN_INFO_CALIBSCALE:
+		return IIO_VAL_DECIMAL64_FEMTO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int fdc1004_write_raw(struct iio_dev *indio_dev,
+			     const struct iio_chan_spec *chan,
+			     int val, int val2, long mask)
+{
+	struct fdc1004_data *data = iio_priv(indio_dev);
+	struct regmap *map = data->regmap;
+	s64 val64;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		for (unsigned int i = 0; i < ARRAY_SIZE(fdc1004_samp_freqs); i++) {
+			if (fdc1004_samp_freqs[i] == val) {
+				guard(mutex)(&data->lock);
+
+				data->samp_freq_index = i;
+				return 0;
+			}
+		}
+
+		return -EINVAL;
+	case IIO_CHAN_INFO_OFFSET: {
+		if (val < fdc1004_offset[0] || val > fdc1004_offset[2] ||
+		    val % fdc1004_offset[1])
+			return -EINVAL;
+
+		guard(mutex)(&data->lock);
+
+		data->offsets[chan->channel] = val / fdc1004_offset[1];
+		return 0;
+	}
+	case IIO_CHAN_INFO_CALIBBIAS:
+		val64 = iio_val_s64_compose(val, val2);
+		if (val64 < fdc1004_calibbias[0] || val64 > fdc1004_calibbias[2])
+			return -EINVAL;
+
+		return regmap_write(map, FDC1004_REG_OFFSET_CAL_CINn(chan->channel),
+				    DIV_ROUND_CLOSEST(val64, fdc1004_calibbias[1]));
+	case IIO_CHAN_INFO_CALIBSCALE:
+		val64 = iio_val_s64_compose(val, val2);
+		if (val64 < fdc1004_calibscale[0] || val64 > fdc1004_calibscale[2])
+			return -EINVAL;
+
+		return regmap_write(map, FDC1004_REG_GAIN_CAL_CINn(chan->channel),
+				    DIV_ROUND_CLOSEST(val64, fdc1004_calibscale[1]));
+	default:
+		return -EINVAL;
+	}
+}
+
+static int fdc1004_debugfs_reg_access(struct iio_dev *indio_dev,
+				      unsigned int reg, unsigned int writeval,
+				      unsigned int *readval)
+{
+	struct fdc1004_data *data = iio_priv(indio_dev);
+
+	guard(mutex)(&data->lock);
+
+	if (readval)
+		return regmap_read(data->regmap, reg, readval);
+
+	return regmap_write(data->regmap, reg, writeval);
+}
+
+#define FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(_channel)               \
+	{                                                                \
+		.type = IIO_CAPACITANCE,                                 \
+		.indexed = true,                                         \
+		.channel = _channel,                                     \
+		.channel2 = FDC1004_CONF_MEAS_CHB_CAPDAC,                \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |           \
+				      BIT(IIO_CHAN_INFO_OFFSET) |        \
+				      BIT(IIO_CHAN_INFO_CALIBBIAS) |     \
+				      BIT(IIO_CHAN_INFO_CALIBSCALE),     \
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),    \
+		.info_mask_shared_by_type_available =                    \
+			BIT(IIO_CHAN_INFO_OFFSET) |                      \
+			BIT(IIO_CHAN_INFO_CALIBBIAS) |                   \
+			BIT(IIO_CHAN_INFO_CALIBSCALE),                   \
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+		.info_mask_shared_by_all_available =                     \
+			BIT(IIO_CHAN_INFO_SAMP_FREQ),                    \
+	}
+
+#define FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(_chan1, _chan2)         \
+	{                                                                \
+		.type = IIO_CAPACITANCE,                                 \
+		.indexed = true,                                         \
+		.differential = true,                                    \
+		.channel = _chan1,                                       \
+		.channel2 = _chan2,                                      \
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),            \
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),    \
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+		.info_mask_shared_by_all_available =                     \
+			BIT(IIO_CHAN_INFO_SAMP_FREQ),                    \
+	}
+
+static const struct iio_chan_spec fdc1004_channels[] = {
+	FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(0),
+	FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(1),
+	FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(2),
+	FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(3),
+
+	/* The sensor only supports differential measurements where CHA < CHB. */
+	FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(0, 1),
+	FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(0, 2),
+	FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(0, 3),
+
+	FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(1, 2),
+	FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(1, 3),
+
+	FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(2, 3),
+};
+
+static const struct iio_info fdc1004_info = {
+	.read_raw = fdc1004_read_raw,
+	.read_avail = fdc1004_read_avail,
+	.write_raw = fdc1004_write_raw,
+	.write_raw_get_fmt = fdc1004_write_raw_get_fmt,
+	.debugfs_reg_access = fdc1004_debugfs_reg_access,
+};
+
+static int fdc1004_probe(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev;
+	struct fdc1004_data *data;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
+		return -EOPNOTSUPP;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+
+	data->regmap = devm_regmap_init_i2c(client, &fdc1004_regmap_config);
+	if (IS_ERR(data->regmap))
+		return PTR_ERR(data->regmap);
+
+	ret = devm_mutex_init(&client->dev, &data->lock);
+	if (ret)
+		return ret;
+
+	ret = devm_regulator_get_enable(&client->dev, "vdd");
+	if (ret)
+		return ret;
+
+	ret = fdc1004_soft_reset(data);
+	if (ret)
+		return ret;
+
+	indio_dev->name = "fdc1004";
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &fdc1004_info;
+	indio_dev->channels = fdc1004_channels;
+	indio_dev->num_channels = ARRAY_SIZE(fdc1004_channels);
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id fdc1004_id[] = {
+	{ .name = "fdc1004" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, fdc1004_id);
+
+static const struct of_device_id fdc1004_of_match[] = {
+	{ .compatible = "ti,fdc1004" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, fdc1004_of_match);
+
+static struct i2c_driver fdc1004_driver = {
+	.driver = {
+		.name = "fdc1004",
+		.of_match_table = fdc1004_of_match,
+	},
+	.probe = fdc1004_probe,
+	.id_table = fdc1004_id,
+};
+module_i2c_driver(fdc1004_driver);
+
+MODULE_AUTHOR("Victor Chernyakin <chernyakin.victor.j@outlook.com>");
+MODULE_DESCRIPTION("Texas Instruments FDC1004 capacitive sensor driver");
+MODULE_LICENSE("GPL");
-- 
2.53.0

Re: [PATCH v3 4/4] iio: cdc: add Texas Instruments FDC1004 driver

[Andy Shevchenko]

On Mon, Oct 26, 2026 at 11:51:14AM -0500, Victor Chernyakin wrote:
> The FDC1004 is a high-precision 4-channel capacitive sensor that talks
> over I2C. It can report the values of each of its 4 channels individually,
> as well as the difference between any two of them.
> 
> This initial driver allows performing single reads and configuring
> the sensor's various settings (sample frequency, calibration
> gain/offset, etc.). It could be extended with support for triggered
> buffers, repeated measurement mode, and/or power management.

...

> +static int fdc1004_soft_reset(struct fdc1004_data *data)
> +{
> +	int ret;
> +	unsigned int val;

Try to keep all of them in the reversed xmas tree order.

> +	ret = regmap_write(data->regmap, FDC1004_REG_FDC_CONF,
> +			   FDC1004_FDC_CONF_RST_MASK);

	struct regmap *map = data->regmap;
	...

	ret = regmap_write(map, FDC1004_REG_FDC_CONF, FDC1004_FDC_CONF_RST_MASK);

(yes, it's fine to have 81 characters here)

> +	if (ret)
> +		return ret;
> +
> +	return regmap_read_poll_timeout(data->regmap, FDC1004_REG_FDC_CONF, val,
> +					!(ret & FDC1004_FDC_CONF_RST_MASK),
> +					1 * USEC_PER_MSEC, 200 * USEC_PER_MSEC);
> +}

...

> +static int fdc1004_read_raw(struct iio_dev *indio_dev,
> +			    const struct iio_chan_spec *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct fdc1004_data *data = iio_priv(indio_dev);
> +	struct regmap *map = data->regmap;
> +	unsigned int reg_val;
> +	u16 reg_vals[2];
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		iio_val_s64_decompose(fdc1004_scale, val, val2);
> +		return IIO_VAL_DECIMAL64_YOCTO;
> +	case IIO_CHAN_INFO_RAW: {
> +		guard(mutex)(&data->lock);
> +
> +		/* Configure the measurement we want. */
> +		reg_val = FIELD_PREP(FDC1004_CONF_MEAS_CHA_MASK, chan->channel)
> +			| FIELD_PREP(FDC1004_CONF_MEAS_CHB_MASK, chan->channel2)
> +			| FIELD_PREP(FDC1004_CONF_MEAS_CAPDAC_MASK,
> +				      data->offsets[chan->channel]);

We don't practice this style in IIO.

		reg_val = FIELD_PREP(FDC1004_CONF_MEAS_CHA_MASK, chan->channel) |
			  FIELD_PREP(FDC1004_CONF_MEAS_CHB_MASK, chan->channel2) |
			  FIELD_PREP(FDC1004_CONF_MEAS_CAPDAC_MASK,
				      data->offsets[chan->channel]);

> +		ret = regmap_write(map, FDC1004_REG_CONF_MEASn(0), reg_val);
> +		if (ret)
> +			return ret;
> +
> +		/* Start the measurement. */
> +		reg_val = FDC1004_FDC_CONF_MEAS_n_MASK(0)
> +			/* "+ 1" because the chip wants a 1-indexed value. */

Move comment on top of the assignment.

> +			| FIELD_PREP(FDC1004_FDC_CONF_RATE_MASK,
> +				     data->samp_freq_index + 1);

Same about the style.

> +		ret = regmap_write(map, FDC1004_REG_FDC_CONF, reg_val);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Wait for the measurement to complete.
> +		 * The slowest sampling frequency the chip supports is 100 SPS,
> +		 * i.e. 1 sample every 10 ms, so our timeout can't be shorter
> +		 * than that. Go with 40 ms to be safe.
> +		 */
> +		ret = regmap_read_poll_timeout(map, FDC1004_REG_FDC_CONF, reg_val,
> +					       reg_val & FDC1004_FDC_CONF_DONE_n_MASK(0),
> +					       1 * USEC_PER_MSEC, 40 * USEC_PER_MSEC);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Now read the two registers making up the measurement.
> +		 * The datasheet says to read MSB first, LSB second.
> +		 */
> +		ret = regmap_bulk_read(map, FDC1004_REG_MEASn_MSB(0),
> +				       reg_vals, ARRAY_SIZE(reg_vals));
> +		if (ret)
> +			return ret;
> +
> +		*val = (sign_extend32(reg_vals[0], 15) << 8) | (reg_vals[1] >> 8);

We still have two shift operators, so it might be not a bad idea after all to
have a combined variable with wordswap done in case of LITTLE_ENDIAN
architecture. Jonathan, what do you think about this? Reading the whole set as
32-bit value and convert it if required

		// also note in this case we don't need to have _MSB in the name
		ret = regmap_bulk_read(map, FDC1004_REG_MEASn(0), reg_val, sizeof(reg_val));
		if (ret)
			return ret;

#ifdef __LITTLE_ENDIAN
		reg_val = swahw32(reg_val);
#endif

		*val = sign_extend32(reg_val >> 8, 23);

		// I assume the following is also possible in this case
		// *val = (int)reg_val >> 8;
		// but usually we use sign_extend32() in IIO even when simple
		// casting is okay

This also reduces stack usage.

> +		return IIO_VAL_INT;
> +	}
> +	case IIO_CHAN_INFO_SAMP_FREQ: {
> +		guard(mutex)(&data->lock);
> +
> +		*val = fdc1004_samp_freqs[data->samp_freq_index];
> +		return IIO_VAL_INT;
> +	}
> +	case IIO_CHAN_INFO_OFFSET: {
> +		guard(mutex)(&data->lock);
> +
> +		*val = data->offsets[chan->channel] * fdc1004_offset[1];
> +		return IIO_VAL_INT;
> +	}
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		ret = regmap_read(map, FDC1004_REG_OFFSET_CAL_CINn(chan->channel),
> +				  &reg_val);
> +		if (ret)
> +			return ret;
> +
> +		iio_val_s64_decompose(sign_extend64(reg_val, 15) *
> +				      fdc1004_calibbias[1], val, val2);
> +		return IIO_VAL_DECIMAL64_FEMTO;
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		ret = regmap_read(map, FDC1004_REG_GAIN_CAL_CINn(chan->channel),
> +				  &reg_val);
> +		if (ret)
> +			return ret;
> +
> +		iio_val_s64_decompose(reg_val * fdc1004_calibscale[1], val, val2);
> +		return IIO_VAL_DECIMAL64_FEMTO;
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +static const int fdc1004_calibbias_decomposed[] = {
> +	lower_32_bits(fdc1004_calibbias[0]), upper_32_bits(fdc1004_calibbias[0]),
> +	lower_32_bits(fdc1004_calibbias[1]), upper_32_bits(fdc1004_calibbias[1]),
> +	lower_32_bits(fdc1004_calibbias[2]), upper_32_bits(fdc1004_calibbias[2]),
> +};
> +
> +static const int fdc1004_calibscale_decomposed[] = {
> +	lower_32_bits(fdc1004_calibscale[0]), upper_32_bits(fdc1004_calibscale[0]),
> +	lower_32_bits(fdc1004_calibscale[1]), upper_32_bits(fdc1004_calibscale[1]),
> +	lower_32_bits(fdc1004_calibscale[2]), upper_32_bits(fdc1004_calibscale[2]),
> +};

We still haven't decided on this one. I think the 2D array is better for these.
And with the above we effectively have a duplication of the tables.

...

> +#define FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(_chan1, _chan2)         \
> +	{                                                                \
> +		.type = IIO_CAPACITANCE,                                 \
> +		.indexed = true,                                         \
> +		.differential = true,                                    \
> +		.channel = _chan1,                                       \
> +		.channel2 = _chan2,                                      \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),            \
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),    \
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +		.info_mask_shared_by_all_available =                     \
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ),                    \
> +	}

Please, use tab for indenting the \ slashes (and only spaces at the end if you
want to shorten that a bit).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/4] iio: prepare iio_format_value to handle higher-precision IIO_VAL_DECIMAL64_* formats

[Andy Shevchenko]

On Mon, Oct 26, 2026 at 11:51:11AM -0500, Victor Chernyakin wrote:
> Prepare iio_format_value for an upcoming patch that adds formats up
> to IIO_VAL_DECIMAL64_YOCTO (a scale of 24 digits) by avoiding
> computing 10^scale when doing so would overflow, and by avoiding
> undefined behavior due to abs(LLONG_MIN) (which couldn't happen
> previously).

...

> +#define GREATEST_POWER_OF_10_THAT_FITS_IN_S64 18

It's an interesting name for log10(2^63).

This, along with log10(2^64 - 1) should be added as definitions at least
IIO-wide. There is a series that wanted similar for u64.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/4] iio: prepare iio_format_value to handle higher-precision IIO_VAL_DECIMAL64_* formats

[Victor Chernyakin]

> > +#define GREATEST_POWER_OF_10_THAT_FITS_IN_S64 18
> 
> It's an interesting name for log10(2^63).
> 
> This, along with log10(2^64 - 1) should be added as definitions at least
> IIO-wide. There is a series that wanted similar for u64.

In that case, might as well go all the way and add them to linux/limits.h?

Re: [PATCH v3 4/4] iio: cdc: add Texas Instruments FDC1004 driver

[Victor Chernyakin]

> > +		/*
> > +		 * Now read the two registers making up the measurement.
> > +		 * The datasheet says to read MSB first, LSB second.
> > +		 */
> > +		ret = regmap_bulk_read(map, FDC1004_REG_MEASn_MSB(0),
> > +				       reg_vals, ARRAY_SIZE(reg_vals));
> > +		if (ret)
> > +			return ret;
> > +
> > +		*val = (sign_extend32(reg_vals[0], 15) << 8) | (reg_vals[1] >> 8);
> 
> We still have two shift operators, so it might be not a bad idea after all to
> have a combined variable with wordswap done in case of LITTLE_ENDIAN
> architecture. Jonathan, what do you think about this? Reading the whole set as
> 32-bit value and convert it if required
> 
> 		// also note in this case we don't need to have _MSB in the name
> 		ret = regmap_bulk_read(map, FDC1004_REG_MEASn(0), reg_val, sizeof(reg_val));

I agree we could drop the _MSB if this was the only use
of FDC1004_REG_MEASn_MSB, but we use both _MSB and _LSB macros in the
volatile register table definition, and it would be weird to have one
macro with _LSB but the other without _MSB.

> 		if (ret)
> 			return ret;
> 
> #ifdef __LITTLE_ENDIAN
> 		reg_val = swahw32(reg_val);
> #endif
> 
> 		*val = sign_extend32(reg_val >> 8, 23);
> 
> 		// I assume the following is also possible in this case
> 		// *val = (int)reg_val >> 8;
> 		// but usually we use sign_extend32() in IIO even when simple
> 		// casting is okay
>
> This also reduces stack usage.

AFAIK a good optimizer should be able to see that reg_val and reg_vals are
never live at the same time and reuse the same stack slot for both (whether
GCC or Clang actually do that for this function, I can't be bothered to check).

If you and Jonathan prefer that version for other reasons though, I can
switch to it.

> > +static const int fdc1004_calibbias_decomposed[] = {
> > +	lower_32_bits(fdc1004_calibbias[0]), upper_32_bits(fdc1004_calibbias[0]),
> > +	lower_32_bits(fdc1004_calibbias[1]), upper_32_bits(fdc1004_calibbias[1]),
> > +	lower_32_bits(fdc1004_calibbias[2]), upper_32_bits(fdc1004_calibbias[2]),
> > +};
> > +
> > +static const int fdc1004_calibscale_decomposed[] = {
> > +	lower_32_bits(fdc1004_calibscale[0]), upper_32_bits(fdc1004_calibscale[0]),
> > +	lower_32_bits(fdc1004_calibscale[1]), upper_32_bits(fdc1004_calibscale[1]),
> > +	lower_32_bits(fdc1004_calibscale[2]), upper_32_bits(fdc1004_calibscale[2]),
> > +};
> 
> We still haven't decided on this one. I think the 2D array is better for these.
> And with the above we effectively have a duplication of the tables.

I don't like the duplication either, but I think the real problem here is
that read_avail is just a somewhat awkward interface that doesn't allow me
to return s64 values directly. To me, having this separate decomposed table
just for read_avail and otherwise working with s64 values is the least bad
approach.

Re: [PATCH v3 1/4] iio: prepare iio_format_value to handle higher-precision IIO_VAL_DECIMAL64_* formats

[Andy Shevchenko]

On Mon, Oct 26, 2026 at 08:51:53PM -0500, Victor Chernyakin wrote:
> > > +#define GREATEST_POWER_OF_10_THAT_FITS_IN_S64 18
> > 
> > It's an interesting name for log10(2^63).
> > 
> > This, along with log10(2^64 - 1) should be added as definitions at least
> > IIO-wide. There is a series that wanted similar for u64.
> 
> In that case, might as well go all the way and add them to linux/limits.h?

Only if you have existing users outside (which has to be shown
by the respective patch and then it makes the series cross the
subsystem boundaries, much harder to get in :-).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/4] iio: cdc: add Texas Instruments FDC1004 driver

[Andy Shevchenko]

On Mon, Oct 26, 2026 at 09:13:25PM -0500, Victor Chernyakin wrote:

...

> > > +		/*
> > > +		 * Now read the two registers making up the measurement.
> > > +		 * The datasheet says to read MSB first, LSB second.
> > > +		 */
> > > +		ret = regmap_bulk_read(map, FDC1004_REG_MEASn_MSB(0),
> > > +				       reg_vals, ARRAY_SIZE(reg_vals));
> > > +		if (ret)
> > > +			return ret;
> > > +
> > > +		*val = (sign_extend32(reg_vals[0], 15) << 8) | (reg_vals[1] >> 8);
> > 
> > We still have two shift operators, so it might be not a bad idea after all to
> > have a combined variable with wordswap done in case of LITTLE_ENDIAN
> > architecture. Jonathan, what do you think about this? Reading the whole set as
> > 32-bit value and convert it if required
> > 
> > 		// also note in this case we don't need to have _MSB in the name
> > 		ret = regmap_bulk_read(map, FDC1004_REG_MEASn(0), reg_val, sizeof(reg_val));
> 
> I agree we could drop the _MSB if this was the only use
> of FDC1004_REG_MEASn_MSB, but we use both _MSB and _LSB macros in the
> volatile register table definition, and it would be weird to have one
> macro with _LSB but the other without _MSB.

You can have three!

> > 		if (ret)
> > 			return ret;
> > 
> > #ifdef __LITTLE_ENDIAN
> > 		reg_val = swahw32(reg_val);
> > #endif
> > 
> > 		*val = sign_extend32(reg_val >> 8, 23);
> > 
> > 		// I assume the following is also possible in this case
> > 		// *val = (int)reg_val >> 8;
> > 		// but usually we use sign_extend32() in IIO even when simple
> > 		// casting is okay
> >
> > This also reduces stack usage.
> 
> AFAIK a good optimizer should be able to see that reg_val and reg_vals are
> never live at the same time and reuse the same stack slot for both (whether
> GCC or Clang actually do that for this function, I can't be bothered to check).
> 
> If you and Jonathan prefer that version for other reasons though, I can
> switch to it.

I gave an idea, I would wait for Jonathan to judge on it.

...

> > > +static const int fdc1004_calibbias_decomposed[] = {
> > > +	lower_32_bits(fdc1004_calibbias[0]), upper_32_bits(fdc1004_calibbias[0]),
> > > +	lower_32_bits(fdc1004_calibbias[1]), upper_32_bits(fdc1004_calibbias[1]),
> > > +	lower_32_bits(fdc1004_calibbias[2]), upper_32_bits(fdc1004_calibbias[2]),
> > > +};
> > > +
> > > +static const int fdc1004_calibscale_decomposed[] = {
> > > +	lower_32_bits(fdc1004_calibscale[0]), upper_32_bits(fdc1004_calibscale[0]),
> > > +	lower_32_bits(fdc1004_calibscale[1]), upper_32_bits(fdc1004_calibscale[1]),
> > > +	lower_32_bits(fdc1004_calibscale[2]), upper_32_bits(fdc1004_calibscale[2]),
> > > +};
> > 
> > We still haven't decided on this one. I think the 2D array is better for these.
> > And with the above we effectively have a duplication of the tables.
> 
> I don't like the duplication either, but I think the real problem here is
> that read_avail is just a somewhat awkward interface that doesn't allow me
> to return s64 values directly. To me, having this separate decomposed table
> just for read_avail and otherwise working with s64 values is the least bad
> approach.

What about adding .read_avail64() (in a separate change)? Problem solved!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/4] iio: cdc: add Texas Instruments FDC1004 driver

[Joshua Crofts]

On Mon, 26 Oct 2026 11:51:14 -0500
Victor Chernyakin <chernyakin.victor.j@outlook.com> wrote:

> +config FDC1004
> +	tristate "Texas Instruments FDC1004 capacitive sensor driver"
> +	depends on I2C

Please add "select REGMAP_I2C" here, otherwise building this module
may cause errors with certain configs (e.g. allnoconfig).

> +	help
> +	  Say yes here to build support for the Texas Instruments FDC1004
> +	  capacitive sensor.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called fdc1004.
> +


-- 
Kind regards,
Joshua Crofts

Re: [PATCH v3 1/4] iio: prepare iio_format_value to handle higher-precision IIO_VAL_DECIMAL64_* formats

[Victor Chernyakin]

> > > > +#define GREATEST_POWER_OF_10_THAT_FITS_IN_S64 18
> > > 
> > > It's an interesting name for log10(2^63).
> > > 
> > > This, along with log10(2^64 - 1) should be added as definitions at least
> > > IIO-wide. There is a series that wanted similar for u64.
> > 
> > In that case, might as well go all the way and add them to linux/limits.h?
> 
> Only if you have existing users outside (which has to be shown
> by the respective patch and then it makes the series cross the
> subsystem boundaries, much harder to get in :-).

Yeah I'll keep it within IIO then ;p

Re: [PATCH v3 4/4] iio: cdc: add Texas Instruments FDC1004 driver

[Victor Chernyakin]

> > I don't like the duplication either, but I think the real problem here is
> > that read_avail is just a somewhat awkward interface that doesn't allow me
> > to return s64 values directly. To me, having this separate decomposed table
> > just for read_avail and otherwise working with s64 values is the least bad
> > approach.
> 
> What about adding .read_avail64() (in a separate change)? Problem solved!

If Jonathan is open to such a new API, I could work on that! I think
if we're going to add it, instead of just changing the two int* parameters
to one s64*, we could add a union for the function to write into. That way,
not everything needs to be represented as raw s64's; we could introduce
a proper range type for example:

struct iio_range {
    s64 min;
    s64 step;
    s64 max;
};

struct iio_available_value {
    enum iio_available_type type; /* IIO_AVAIL_RANGE, etc. */
    union {
        const struct iio_range *range;
        struct {
            const s64 *vals;
            int num_vals;
        } list;
    };
};

/* Used like so: */

static const struct iio_range fdc1004_calibbias = {
	.min  = -0x8000 * 488281250UL,
	.step = +0x0001 * 488281250UL,
	.max  = +0x7FFF * 488281250UL,
};

static int fdc1004_read_avail_ext(struct iio_dev *indio_dev,
			                      const struct iio_chan_spec *chan,
			                      iio_available_value *out, long info)
{
	switch (info) {
	case IIO_CHAN_INFO_CALIBBIAS:
        out->range = &fdc1004_calibbias; /* No casts! */
        out->type = IIO_AVAIL_RANGE;
		return IIO_VAL_DECIMAL64_FEMTO;
    }
}

Re: [PATCH v3 4/4] iio: cdc: add Texas Instruments FDC1004 driver

[Victor Chernyakin]

> > +config FDC1004
> > +	tristate "Texas Instruments FDC1004 capacitive sensor driver"
> > +	depends on I2C
> 
> Please add "select REGMAP_I2C" here, otherwise building this module
> may cause errors with certain configs (e.g. allnoconfig).

Added for v4.

Re: [PATCH v3 1/4] iio: prepare iio_format_value to handle higher-precision IIO_VAL_DECIMAL64_* formats

[Rodrigo Alencar]

On 26/10/26 11:51, Victor Chernyakin wrote:
> Prepare iio_format_value for an upcoming patch that adds formats up
> to IIO_VAL_DECIMAL64_YOCTO (a scale of 24 digits) by avoiding
> computing 10^scale when doing so would overflow, and by avoiding
> undefined behavior due to abs(LLONG_MIN) (which couldn't happen
> previously).

...

> +/*
> + * Regular abs(LLONG_MIN) causes signed overflow, which is undefined
> + * behavior. This variant avoids that by performing the computation in an
> + * unsigned type, for which negation is always well-defined.
> + */
> +static inline u64 abs_s64_no_overflow(s64 x)
> +{
> +	return x < 0 ? -(u64)x : (u64)x;
> +}

I think I've seen Andy doing mentions to a needed 'safe' abs() a couple of times.
Don't we have something in the tree already? if not, Why not adding a
'unsigned' abs function to math.h? something like:

/**
 * uabs - return absolute value of an argument as an unsigned type
 * @x: the value.
 *
 * Unlike abs(), the result is well defined when @x is the minimum value of
 * a signed type, since the negation is done in the unsigned domain.
 *
 * Return: an absolute value of @x, as the unsigned counterpart of its type.
 */
#define uabs(x)	__uabs_choose_expr(x, long long,			\
		__uabs_choose_expr(x, long,				\
		__uabs_choose_expr(x, int,				\
		__uabs_choose_expr(x, short,				\
		__uabs_choose_expr(x, char,				\
		__builtin_choose_expr(					\
			__builtin_types_compatible_p(typeof(x), char),	\
			({ signed char __x = (x);			\
			   (unsigned char)(__x < 0 ? -(unsigned char)__x :	\
						     (unsigned char)__x); }),	\
			((void)0)))))))

#define __uabs_choose_expr(x, type, other) __builtin_choose_expr(	\
	__builtin_types_compatible_p(typeof(x),   signed type) ||	\
	__builtin_types_compatible_p(typeof(x), unsigned type),		\
	({ signed type __x = (x);					\
	   (unsigned type)(__x < 0 ? -(unsigned type)__x :		\
				     (unsigned type)__x); }), other)

> +
> +#define GREATEST_POWER_OF_10_THAT_FITS_IN_S64 18
> +

that is readable! being local to this file, maybe define this as
IIO_VAL_INTLOG10_S64_MAX or something to indicate the scope of its usage?

>  static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
>  				  int size, const int *vals)
>  {
> @@ -720,13 +732,19 @@ static ssize_t __iio_format_value(char *buf, size_t offset, unsigned int type,
>  		int scale = type - IIO_VAL_DECIMAL64_BASE;
>  		s64 frac;
>  
> -		tmp2 = div64_s64_rem(iio_val_s64_compose(vals[0], vals[1]),
> -				     int_pow(10, scale), &frac);
> +		if (scale <= GREATEST_POWER_OF_10_THAT_FITS_IN_S64) {
> +			tmp2 = div64_s64_rem(iio_val_s64_compose(vals[0], vals[1]),
> +					     int_pow(10, scale), &frac);
> +		} else {
> +			tmp2 = 0;
> +			frac = iio_val_s64_compose(vals[0], vals[1]);
> +		}
> +
>  		if (tmp2 == 0 && frac < 0)
>  			l += sysfs_emit_at(buf, offset, "-");
>  
> -		l += sysfs_emit_at(buf, offset + l, "%lld.%0*lld", tmp2, scale,
> -				   abs(frac));
> +		l += sysfs_emit_at(buf, offset + l, "%lld.%0*llu", tmp2, scale,
> +				   abs_s64_no_overflow(frac));

...

-- 
Kind regards,

Rodrigo Alencar