* [PATCH 0/3] iio: cdc: add Texas Instruments FDC1004 driver
@ 2026-10-22 23:40 Victor Chernyakin
2026-10-22 23:40 ` [PATCH 1/3] iio: add IIO_VAL_DECIMAL64_{ATTO,ZEPTO,YOCTO} formats Victor Chernyakin
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Victor Chernyakin @ 2026-10-22 23:40 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Victor Chernyakin, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, linux-kernel
This series adds support for the FDC1004 I2C capacitive sensor.
Patch 1 adds higher-precision IIO_VAL_DECIMAL64_* formats,
needed for the driver.
Patch 2 adds the devicetree bindings.
Patch 3 adds the driver itself.
Tested with the sensor connected over an i2c-tiny-usb adapter.
Victor Chernyakin (3):
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 | 44 ++
MAINTAINERS | 6 +
drivers/iio/cdc/Kconfig | 10 +
drivers/iio/cdc/Makefile | 1 +
drivers/iio/cdc/fdc1004.c | 460 ++++++++++++++++++
drivers/iio/industrialio-core.c | 6 +
include/linux/iio/types.h | 3 +
7 files changed, 530 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml
create mode 100644 drivers/iio/cdc/fdc1004.c
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] iio: add IIO_VAL_DECIMAL64_{ATTO,ZEPTO,YOCTO} formats 2026-10-22 23:40 [PATCH 0/3] iio: cdc: add Texas Instruments FDC1004 driver Victor Chernyakin @ 2026-10-22 23:40 ` Victor Chernyakin 2026-10-23 3:06 ` Jonathan Cameron 2026-10-22 23:40 ` [PATCH 2/3] dt-bindings: iio: cdc: add Texas Instruments FDC1004 Victor Chernyakin ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Victor Chernyakin @ 2026-10-22 23:40 UTC (permalink / raw) To: Jonathan Cameron Cc: Victor Chernyakin, David Lechner, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel 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 ++++++ include/linux/iio/types.h | 3 +++ 2 files changed, 9 insertions(+) diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 767a7794624a..008387efe96b 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -716,6 +716,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; @@ -1034,6 +1037,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/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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] iio: add IIO_VAL_DECIMAL64_{ATTO,ZEPTO,YOCTO} formats 2026-10-22 23:40 ` [PATCH 1/3] iio: add IIO_VAL_DECIMAL64_{ATTO,ZEPTO,YOCTO} formats Victor Chernyakin @ 2026-10-23 3:06 ` Jonathan Cameron 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2026-10-23 3:06 UTC (permalink / raw) To: Victor Chernyakin Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel On Thu, 22 Oct 2026 18:40:30 -0500 Victor Chernyakin <chernyakin.victor.j@outlook.com> wrote: > 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> Some discussion of whether these are perhaps overkill in the driver patch given device precision doesn't seem to be nearly that good. > --- > drivers/iio/industrialio-core.c | 6 ++++++ > include/linux/iio/types.h | 3 +++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index 767a7794624a..008387efe96b 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -716,6 +716,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: Seems we failed to update the test in drivers/iio/tests/iio-test-format.c for FEMTO. Please add that and a similar test for your new units. > + case IIO_VAL_DECIMAL64_ATTO: > + case IIO_VAL_DECIMAL64_ZEPTO: > + case IIO_VAL_DECIMAL64_YOCTO: > { > int scale = type - IIO_VAL_DECIMAL64_BASE; > s64 frac; > @@ -1034,6 +1037,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/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) > { ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] dt-bindings: iio: cdc: add Texas Instruments FDC1004 2026-10-22 23:40 [PATCH 0/3] iio: cdc: add Texas Instruments FDC1004 driver Victor Chernyakin 2026-10-22 23:40 ` [PATCH 1/3] iio: add IIO_VAL_DECIMAL64_{ATTO,ZEPTO,YOCTO} formats Victor Chernyakin @ 2026-10-22 23:40 ` Victor Chernyakin 2026-10-23 3:09 ` Jonathan Cameron 2026-10-22 23:40 ` [PATCH 3/3] iio: cdc: add Texas Instruments FDC1004 driver Victor Chernyakin 2026-10-23 7:09 ` [PATCH 0/3] " Andy Shevchenko 3 siblings, 1 reply; 9+ messages in thread From: Victor Chernyakin @ 2026-10-22 23:40 UTC (permalink / raw) To: Jonathan Cameron Cc: Victor Chernyakin, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel Add device tree bindings for the Texas Instruments FDC1004 capacitive sensor. Signed-off-by: Victor Chernyakin <chernyakin.victor.j@outlook.com> --- .../bindings/iio/cdc/ti,fdc1004.yaml | 44 +++++++++++++++++++ MAINTAINERS | 5 +++ 2 files changed, 49 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..d22b7bc92a3f --- /dev/null +++ b/Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml @@ -0,0 +1,44 @@ +# 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 + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + cdc@50 { + compatible = "ti,fdc1004"; + reg = <0x50>; + }; + }; +... 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] dt-bindings: iio: cdc: add Texas Instruments FDC1004 2026-10-22 23:40 ` [PATCH 2/3] dt-bindings: iio: cdc: add Texas Instruments FDC1004 Victor Chernyakin @ 2026-10-23 3:09 ` Jonathan Cameron 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2026-10-23 3:09 UTC (permalink / raw) To: Victor Chernyakin Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Thu, 22 Oct 2026 18:40:31 -0500 Victor Chernyakin <chernyakin.victor.j@outlook.com> wrote: > Add device tree bindings for the Texas Instruments FDC1004 > capacitive sensor. > > Signed-off-by: Victor Chernyakin <chernyakin.victor.j@outlook.com> > --- > .../bindings/iio/cdc/ti,fdc1004.yaml | 44 +++++++++++++++++++ > MAINTAINERS | 5 +++ > 2 files changed, 49 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..d22b7bc92a3f > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/cdc/ti,fdc1004.yaml > @@ -0,0 +1,44 @@ > +# 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 Require the power supply. The binding reflects that the sensor needs power to operate. > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + cdc@50 { > + compatible = "ti,fdc1004"; As I mention in the driver review, I think this sort of sensor is pretty much always wired up to specific external sensing elements. As such you may want to provide channel sub nodes in here to make it discoverable which channels are actually useful. > + reg = <0x50>; > + }; > + }; > +... > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] iio: cdc: add Texas Instruments FDC1004 driver 2026-10-22 23:40 [PATCH 0/3] iio: cdc: add Texas Instruments FDC1004 driver Victor Chernyakin 2026-10-22 23:40 ` [PATCH 1/3] iio: add IIO_VAL_DECIMAL64_{ATTO,ZEPTO,YOCTO} formats Victor Chernyakin 2026-10-22 23:40 ` [PATCH 2/3] dt-bindings: iio: cdc: add Texas Instruments FDC1004 Victor Chernyakin @ 2026-10-22 23:40 ` Victor Chernyakin 2026-10-23 3:01 ` Jonathan Cameron 2026-10-23 7:09 ` [PATCH 0/3] " Andy Shevchenko 3 siblings, 1 reply; 9+ messages in thread From: Victor Chernyakin @ 2026-10-22 23:40 UTC (permalink / raw) To: Jonathan Cameron Cc: Victor Chernyakin, David Lechner, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel 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 | 460 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 472 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..852a3723e200 --- /dev/null +++ b/drivers/iio/cdc/fdc1004.c @@ -0,0 +1,460 @@ +// 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/bits.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/iopoll.h> +#include <linux/math.h> +#include <linux/module.h> +#include <linux/mutex.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 = 100/(2^5) pF. + */ +static const int fdc1004_offset[] = { + 0x00 * (100 << (19 - 5)), /* min */ + 0x01 * (100 << (19 - 5)), /* step */ + 0x1F * (100 << (19 - 5)), /* 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. + */ +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 */ +}; + +struct fdc1004_data { + struct i2c_client *client; + struct mutex lock; + u8 samp_freq_index; + u8 offsets[4]; +}; + +static int fdc1004_soft_reset(struct i2c_client *client) +{ + int ret; + + ret = i2c_smbus_write_word_swapped(client, FDC1004_REG_FDC_CONF, + FDC1004_FDC_CONF_RST_MASK); + if (ret) + return ret; + + return poll_timeout_us( + { + ret = i2c_smbus_read_word_swapped(client, + FDC1004_REG_FDC_CONF); + if (ret < 0) + return ret; + }, + !(ret & FDC1004_FDC_CONF_RST_MASK), USEC_PER_MSEC, + 200 * USEC_PER_MSEC, false); +} + +static int fdc1004_wait_until_measurement_is_ready(struct i2c_client *client, + int meas) +{ + int ret; + + /* + * 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. + */ + return poll_timeout_us( + { + ret = i2c_smbus_read_word_swapped(client, + FDC1004_REG_FDC_CONF); + if (ret < 0) + return ret; + }, + ret & FDC1004_FDC_CONF_DONE_n_MASK(meas), USEC_PER_MSEC, + 40 * USEC_PER_MSEC, false); +} + +static int fdc1004_read_measurement(struct i2c_client *client, int meas, + int *out) +{ + int ret; + + /* The datasheet says to read MSB first, LSB second. */ + ret = i2c_smbus_read_word_swapped(client, FDC1004_REG_MEASn_MSB(meas)); + if (ret < 0) + return ret; + + *out = sign_extend32(ret, 15) << 8; + + ret = i2c_smbus_read_word_swapped(client, FDC1004_REG_MEASn_LSB(meas)); + if (ret < 0) + return ret; + + *out |= ret >> 8; + + return 0; +} + +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 i2c_client *client = data->client; + 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. */ + ret = i2c_smbus_write_word_swapped( + client, FDC1004_REG_CONF_MEASn(0), + chan->address | + FIELD_PREP(FDC1004_CONF_MEAS_CAPDAC_MASK, + data->offsets[chan->channel])); + if (ret) + return ret; + + /* Start the measurement. */ + ret = i2c_smbus_write_word_swapped( + client, FDC1004_REG_FDC_CONF, + 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)); + if (ret) + return ret; + + ret = fdc1004_wait_until_measurement_is_ready(client, 0); + if (ret) + return ret; + + ret = fdc1004_read_measurement(client, 0, val); + if (ret) + return ret; + + 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 = i2c_smbus_read_word_swapped( + client, FDC1004_REG_OFFSET_CAL_CINn(chan->channel)); + if (ret < 0) + return ret; + + iio_val_s64_decompose( + sign_extend64(ret, 15) * fdc1004_calibbias[1], + val, val2); + return IIO_VAL_DECIMAL64_FEMTO; + case IIO_CHAN_INFO_CALIBSCALE: + ret = i2c_smbus_read_word_swapped( + client, FDC1004_REG_GAIN_CAL_CINn(chan->channel)); + if (ret < 0) + return ret; + + iio_val_s64_decompose(ret * 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 i2c_client *client = data->client; + s64 val64 = iio_val_s64_compose(val, val2); + int i; + + switch (mask) { + case IIO_CHAN_INFO_SAMP_FREQ: + for (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: + if (val64 < fdc1004_calibbias[0] || val64 > fdc1004_calibbias[2]) + return -EINVAL; + + return i2c_smbus_write_word_swapped( + client, FDC1004_REG_OFFSET_CAL_CINn(chan->channel), + DIV_ROUND_CLOSEST(val64, fdc1004_calibbias[1])); + case IIO_CHAN_INFO_CALIBSCALE: + if (val64 < fdc1004_calibscale[0] || val64 > fdc1004_calibscale[2]) + return -EINVAL; + + return i2c_smbus_write_word_swapped( + client, FDC1004_REG_GAIN_CAL_CINn(chan->channel), + DIV_ROUND_CLOSEST(val64, fdc1004_calibscale[1])); + default: + return -EINVAL; + } +} + +#define FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(_index, _channel) \ + { \ + .type = IIO_CAPACITANCE, \ + .indexed = true, \ + .channel = _channel, \ + .address = FIELD_PREP_CONST(FDC1004_CONF_MEAS_CHA_MASK, \ + _channel) | \ + FIELD_PREP_CONST(FDC1004_CONF_MEAS_CHB_MASK, \ + FDC1004_CONF_MEAS_CHB_CAPDAC), \ + .scan_index = _index, \ + .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(_index, _chan1, _chan2) \ + { \ + .type = IIO_CAPACITANCE, \ + .indexed = true, \ + .differential = true, \ + .channel = _chan1, \ + .channel2 = _chan2, \ + .address = \ + FIELD_PREP_CONST(FDC1004_CONF_MEAS_CHA_MASK, _chan1) | \ + FIELD_PREP_CONST(FDC1004_CONF_MEAS_CHB_MASK, _chan2), \ + .scan_index = _index, \ + .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, 0), + FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(1, 1), + FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(2, 2), + FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(3, 3), + + /* The sensor only supports differential measurements where CHA < CHB. */ + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(4, 0, 1), + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(5, 0, 2), + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(6, 0, 3), + + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(7, 1, 2), + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(8, 1, 3), + + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(9, 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, +}; + +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->client = client; + + 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(client); + 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(indio_dev->dev.parent, indio_dev); +} + +static const struct i2c_device_id fdc1004_id[] = { + { "fdc1004" }, + { } +}; + +MODULE_DEVICE_TABLE(i2c, fdc1004_id); + +static const struct of_device_id fdc1004_of_match[] = { + { "ti,fdc1004" }, + { } +}; + +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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] iio: cdc: add Texas Instruments FDC1004 driver 2026-10-22 23:40 ` [PATCH 3/3] iio: cdc: add Texas Instruments FDC1004 driver Victor Chernyakin @ 2026-10-23 3:01 ` Jonathan Cameron 2026-10-23 7:29 ` Andy Shevchenko 0 siblings, 1 reply; 9+ messages in thread From: Jonathan Cameron @ 2026-10-23 3:01 UTC (permalink / raw) To: Victor Chernyakin Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel On Thu, 22 Oct 2026 18:40:32 -0500 Victor Chernyakin <chernyakin.victor.j@outlook.com> wrote: Hi Victor and welcome to IIO. > 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. Whilst I think I follow the reasoning for the very large units, calibbias and calibscale are generally consider unitless tweak factors. You'll see there are no units expressed anywhere in ABI. This mostly because they tend to rather value defined at best on many parts. So we tend to just go with whatever units feel fairly user friendly. Putting that aside, the base unit of capacitance in the ABI is a nano farad. The part is documented at 0.5 femto farad resolution so I'd kind of expect a scale of only 0.5e-6. Perhaps the issue here is desire for a very high degree of precision on the scale factors? I don't mind that if it is useful but with a documented 0.2% gain error I'm not sure it is. For next version add some reasoning for why such high precision is useful. We frequently round scale factors as often they are swamped by error / noise. Perhaps that doesn't apply here. > > 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 | 460 ++++++++++++++++++++++++++++++++++++++ > 4 files changed, 472 insertions(+) > create mode 100644 drivers/iio/cdc/fdc1004.c > 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..852a3723e200 > --- /dev/null > +++ b/drivers/iio/cdc/fdc1004.c > @@ -0,0 +1,460 @@ > +// 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/bits.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/iopoll.h> > +#include <linux/math.h> > +#include <linux/module.h> > +#include <linux/mutex.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 = 100/(2^5) pF. Given offset is applied prior to scale. Is this in appropriate units? I'm not following that description. > + */ > +static const int fdc1004_offset[] = { > + 0x00 * (100 << (19 - 5)), /* min */ > + 0x01 * (100 << (19 - 5)), /* step */ > + 0x1F * (100 << (19 - 5)), /* 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. > + */ > +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 */ > +}; > + > +struct fdc1004_data { > + struct i2c_client *client; > + struct mutex lock; Add a comment on what data the lock is actually protecting. > + u8 samp_freq_index; > + u8 offsets[4]; > +}; > + > +static int fdc1004_soft_reset(struct i2c_client *client) > +{ > + int ret; > + > + ret = i2c_smbus_write_word_swapped(client, FDC1004_REG_FDC_CONF, > + FDC1004_FDC_CONF_RST_MASK); > + if (ret) > + return ret; > + > + return poll_timeout_us( > + { > + ret = i2c_smbus_read_word_swapped(client, > + FDC1004_REG_FDC_CONF); > + if (ret < 0) > + return ret; > + }, > + !(ret & FDC1004_FDC_CONF_RST_MASK), USEC_PER_MSEC, > + 200 * USEC_PER_MSEC, false); > +} > + > +static int fdc1004_wait_until_measurement_is_ready(struct i2c_client *client, > + int meas) > +{ > + int ret; > + > + /* > + * 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. > + */ > + return poll_timeout_us( > + { > + ret = i2c_smbus_read_word_swapped(client, > + FDC1004_REG_FDC_CONF); > + if (ret < 0) > + return ret; common convention to add the error condition to the exit one rather than do it in here. Then check ret after the exit from teh poll. Tends to give nice code to read. > + }, > + ret & FDC1004_FDC_CONF_DONE_n_MASK(meas), USEC_PER_MSEC, > + 40 * USEC_PER_MSEC, false); > +} > + > +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 i2c_client *client = data->client; > + 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. */ > + ret = i2c_smbus_write_word_swapped( > + client, FDC1004_REG_CONF_MEASn(0), > + chan->address | > + FIELD_PREP(FDC1004_CONF_MEAS_CAPDAC_MASK, > + data->offsets[chan->channel])); > + if (ret) > + return ret; > + > + /* Start the measurement. */ > + ret = i2c_smbus_write_word_swapped( > + client, FDC1004_REG_FDC_CONF, > + 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)); For some of these more complex values I'd use a local variable and build them up in steps. > + if (ret) > + return ret; > + > + ret = fdc1004_wait_until_measurement_is_ready(client, 0); > + if (ret) > + return ret; > + > + ret = fdc1004_read_measurement(client, 0, val); > + if (ret) > + return ret; > + > + 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 = i2c_smbus_read_word_swapped( > + client, FDC1004_REG_OFFSET_CAL_CINn(chan->channel)); Wrap this sort of thing as: > + ret = i2c_smbus_read_word_swapped(client, FDC1004_REG_OFFSET_CAL_CINn(chan->channel)); The long line is fine as it is worth it for readability. > + if (ret < 0) > + return ret; > + > + iio_val_s64_decompose( > + sign_extend64(ret, 15) * fdc1004_calibbias[1], > + val, val2); > + return IIO_VAL_DECIMAL64_FEMTO; As below. I'm not following why the very small units. > + case IIO_CHAN_INFO_CALIBSCALE: > + ret = i2c_smbus_read_word_swapped( > + client, FDC1004_REG_GAIN_CAL_CINn(chan->channel)); > + if (ret < 0) > + return ret; > + > + iio_val_s64_decompose(ret * fdc1004_calibscale[1], val, val2); > + return IIO_VAL_DECIMAL64_FEMTO; > + 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 i2c_client *client = data->client; > + s64 val64 = iio_val_s64_compose(val, val2); Only do this in paths where it is used. Leads to a bit of duplication but less confusing code. > + int i; > + > + switch (mask) { > + case IIO_CHAN_INFO_SAMP_FREQ: > + for (i = 0; i < ARRAY_SIZE(fdc1004_samp_freqs); ++i) { for (unsigned int i = 0; i < ARRAY_SIZE(fdc1004_samp_freq); i++) { Fine to go a bit over 80 chars to help readability and I think that applies here. > + 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: > + if (val64 < fdc1004_calibbias[0] || val64 > fdc1004_calibbias[2]) > + return -EINVAL; > + > + return i2c_smbus_write_word_swapped( > + client, FDC1004_REG_OFFSET_CAL_CINn(chan->channel), > + DIV_ROUND_CLOSEST(val64, fdc1004_calibbias[1])); > + case IIO_CHAN_INFO_CALIBSCALE: > + if (val64 < fdc1004_calibscale[0] || val64 > fdc1004_calibscale[2]) > + return -EINVAL; > + > + return i2c_smbus_write_word_swapped( > + client, FDC1004_REG_GAIN_CAL_CINn(chan->channel), > + DIV_ROUND_CLOSEST(val64, fdc1004_calibscale[1])); > + default: > + return -EINVAL; > + } > +} > + > +#define FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(_index, _channel) \ > + { \ > + .type = IIO_CAPACITANCE, \ > + .indexed = true, \ > + .channel = _channel, \ > + .address = FIELD_PREP_CONST(FDC1004_CONF_MEAS_CHA_MASK, \ > + _channel) | \ > + FIELD_PREP_CONST(FDC1004_CONF_MEAS_CHB_MASK, \ > + FDC1004_CONF_MEAS_CHB_CAPDAC), \ > + .scan_index = _index, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_OFFSET) | \ > + BIT(IIO_CHAN_INFO_CALIBBIAS) | \ > + BIT(IIO_CHAN_INFO_CALIBSCALE), \ Interesting that you have these only for the single ends. I guess they are tweaks on the analog side? > + .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(_index, _chan1, _chan2) \ > + { \ > + .type = IIO_CAPACITANCE, \ > + .indexed = true, \ > + .differential = true, \ > + .channel = _chan1, \ > + .channel2 = _chan2, \ > + .address = \ > + FIELD_PREP_CONST(FDC1004_CONF_MEAS_CHA_MASK, _chan1) | \ > + FIELD_PREP_CONST(FDC1004_CONF_MEAS_CHB_MASK, _chan2), \ Why not build these of channel and channel 2 at the place where you need them? This feels unnecessarily complex in this structure. Much better where we can see the reason to do this used. > + .scan_index = _index, \ Don't add scan index until you have buffer support. > + .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, 0), > + FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(1, 1), > + FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(2, 2), > + FDC1004_SINGLE_ENDED_CAPACITANCE_CHANNEL(3, 3), > + > + /* The sensor only supports differential measurements where CHA < CHB. */ For a capacitance channel I'm going to guess that expectation is wiring is such that only a subset of channels matter for a given configuration? It may be more user friendly to put that in dt (see how we do it in adc/adc.yaml for instance). > + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(4, 0, 1), > + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(5, 0, 2), > + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(6, 0, 3), > + > + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(7, 1, 2), > + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(8, 1, 3), > + > + FDC1004_DIFFERENTIAL_CAPACITANCE_CHANNEL(9, 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, > +}; > + > +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)) I'd look at using regmap even though you only have a single bus. It provides a rich set of helpers and the caching functionality that will save a bunch of bus traffic. > + return -EOPNOTSUPP; > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + data->client = client; > + > + 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(client); > + 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(indio_dev->dev.parent, indio_dev); > +} > + > +static const struct i2c_device_id fdc1004_id[] = { > + { "fdc1004" }, Named initializer as below. > + { } > +}; > + No line break here. We want visual coupling between the array and the macro that uses it. > +MODULE_DEVICE_TABLE(i2c, fdc1004_id); > + > +static const struct of_device_id fdc1004_of_match[] = { > + { "ti,fdc1004" }, Use named initializers. There is some rework of how these IDs work currently going on that requires that + it is good practice anyway. > + { } > +}; This should have a MODULE_DEVICE_TABLE as well. > + ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] iio: cdc: add Texas Instruments FDC1004 driver 2026-10-23 3:01 ` Jonathan Cameron @ 2026-10-23 7:29 ` Andy Shevchenko 0 siblings, 0 replies; 9+ messages in thread From: Andy Shevchenko @ 2026-10-23 7:29 UTC (permalink / raw) To: Jonathan Cameron Cc: Victor Chernyakin, David Lechner, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel On Fri, Oct 23, 2026 at 04:01:43AM +0100, Jonathan Cameron wrote: > On Thu, 22 Oct 2026 18:40:32 -0500 > Victor Chernyakin <chernyakin.victor.j@outlook.com> wrote: ... > > +#include <linux/array_size.h> > > +#include <linux/bitfield.h> > > +#include <linux/bitops.h> > > +#include <linux/bits.h> The latter one is covered by the former, so drop bits.h. + cleanup.h // guard()() With this, please follow IWYU principle (see other IIO drivers that has been submitted in the past couple of years for the style of include section). > > +#include <linux/delay.h> > > +#include <linux/i2c.h> > > +#include <linux/iopoll.h> > > +#include <linux/math.h> > > +#include <linux/module.h> > > +#include <linux/mutex.h> > > +#include <linux/regulator/consumer.h> > > +#include <linux/types.h> ... > > +static const s64 fdc1004_calibbias[] = { > > + -0x8000 * 488281250UL, /* min */ > > + +0x0001 * 488281250UL, /* step */ > > + +0x7FFF * 488281250UL, /* max */ > > +}; Looking at these data types, can you check if linear_range.h APIs can be applied instead? ... > > +struct fdc1004_data { > > + struct i2c_client *client; > > + struct mutex lock; > > Add a comment on what data the lock is actually protecting. > > > + u8 samp_freq_index; > > + u8 offsets[4]; Swap these two, it *might* be that compiler can retrieve all offsets into 32-bit register (due to alignment) at once and optimise the code. You can check the result with `bloat-o-meter`, btw. > > +}; ... > > + return poll_timeout_us( > > + { > > + ret = i2c_smbus_read_word_swapped(client, > > + FDC1004_REG_FDC_CONF); > > + if (ret < 0) > > + return ret; > > common convention to add the error condition to the exit one rather than do it > in here. Then check ret after the exit from teh poll. > Tends to give nice code to read. > > > + }, > > + ret & FDC1004_FDC_CONF_DONE_n_MASK(meas), USEC_PER_MSEC, And please split on logical borders, also use 1 * USEC_PER_MSEC. > > + 40 * USEC_PER_MSEC, false); > > +} Yes, and this macro should not be used like this. Your case is read_poll_timeout(). ... > > +static int fdc1004_read_raw(struct iio_dev *indio_dev, > > + const struct iio_chan_spec *chan, int *val, > > + int *val2, long mask) Logical borders: static int fdc1004_read_raw(struct iio_dev *indio_dev, const struct iio_chan_spec *chan, int *val, int *val2, long mask) ... > > + case IIO_CHAN_INFO_RAW: { > > + guard(mutex)(&data->lock); > > + > > + /* Configure the measurement we want. */ > > + ret = i2c_smbus_write_word_swapped( > > + client, FDC1004_REG_CONF_MEASn(0), > > + chan->address | > > + FIELD_PREP(FDC1004_CONF_MEAS_CAPDAC_MASK, > > + data->offsets[chan->channel])); Indentation is broken... > > + if (ret) > > + return ret; > > + > > + /* Start the measurement. */ > > + ret = i2c_smbus_write_word_swapped( > > + client, FDC1004_REG_FDC_CONF, > > + 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)); > > For some of these more complex values I'd use a local variable and build them > up in steps. ...and this is a good advice to fix readability, > > > + if (ret) > > + return ret; > > + > > + ret = fdc1004_wait_until_measurement_is_ready(client, 0); > > + if (ret) > > + return ret; > > + > > + ret = fdc1004_read_measurement(client, 0, val); > > + if (ret) > > + return ret; > > + > > + return IIO_VAL_INT; > > + } > > + case IIO_CHAN_INFO_SAMP_FREQ: { > > + guard(mutex)(&data->lock); Always add a blank line after guard()(). > > + *val = fdc1004_samp_freqs[data->samp_freq_index]; > > + return IIO_VAL_INT; > > + } ... > > + iio_val_s64_decompose( > > + sign_extend64(ret, 15) * fdc1004_calibbias[1], Again, broken indentation. > > + val, val2); ... > > +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)) > > I'd look at using regmap even though you only have a single bus. It provides > a rich set of helpers and the caching functionality that will save a bunch > of bus traffic. > > > + return -EOPNOTSUPP; > > + > > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + data = iio_priv(indio_dev); > > + data->client = client; > > + > > + 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(client); > > + 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(indio_dev->dev.parent, indio_dev); Why out of a sudden indio_dev->dev.parent? > > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] iio: cdc: add Texas Instruments FDC1004 driver 2026-10-22 23:40 [PATCH 0/3] iio: cdc: add Texas Instruments FDC1004 driver Victor Chernyakin ` (2 preceding siblings ...) 2026-10-22 23:40 ` [PATCH 3/3] iio: cdc: add Texas Instruments FDC1004 driver Victor Chernyakin @ 2026-10-23 7:09 ` Andy Shevchenko 3 siblings, 0 replies; 9+ messages in thread From: Andy Shevchenko @ 2026-10-23 7:09 UTC (permalink / raw) To: Victor Chernyakin Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel On Thu, Oct 22, 2026 at 06:40:29PM -0500, Victor Chernyakin wrote: > This series adds support for the FDC1004 I2C capacitive sensor. Tell us more. Why do we need a brand new driver? > Patch 1 adds higher-precision IIO_VAL_DECIMAL64_* formats, > needed for the driver. > Patch 2 adds the devicetree bindings. > Patch 3 adds the driver itself. > > Tested with the sensor connected over an i2c-tiny-usb adapter. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-23 7:30 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-22 23:40 [PATCH 0/3] iio: cdc: add Texas Instruments FDC1004 driver Victor Chernyakin
2026-10-22 23:40 ` [PATCH 1/3] iio: add IIO_VAL_DECIMAL64_{ATTO,ZEPTO,YOCTO} formats Victor Chernyakin
2026-10-23 3:06 ` Jonathan Cameron
2026-10-22 23:40 ` [PATCH 2/3] dt-bindings: iio: cdc: add Texas Instruments FDC1004 Victor Chernyakin
2026-10-23 3:09 ` Jonathan Cameron
2026-10-22 23:40 ` [PATCH 3/3] iio: cdc: add Texas Instruments FDC1004 driver Victor Chernyakin
2026-10-23 3:01 ` Jonathan Cameron
2026-10-23 7:29 ` Andy Shevchenko
2026-10-23 7:09 ` [PATCH 0/3] " Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®