[PATCH v4 0/3] Support ROHM BD79112 ADC

[PATCH v4 0/3] Support ROHM BD79112 ADC

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 2935 bytes --]

Support ROHM BD79112 ADC/GPIO

The ROHM BD79112 is a 12-bit, 32 channel SAR ADC / GPIO IC. Or, a "Signal
Monitor Hub IC" as data-sheet describes it.

Data sheet states the maximum sampling rate to be 1 MSPS, but achieving
this would probably require the SPI and samples to be processed by
something else but the CPU running Linux. This could work with the "SPI
offloading" which has recently landed upstream - but I have no HW to test
this so nothing fancy is implemented here. It's still worth mentioning
if someone needs the speed and wants to try implementing it :)

The SPI protocol is slightly peculiar. Accesses are done in 16-bit
sequences, separated by releasing and re-aquiring the chip-select.

Register write takes 1 such sequence. The 8-bit register data to write,
is stored in the last 8 bits. The high 8 bits contain register address
and an I/O-bit which needs to be set for register accesses.

Register read consists of two 16-bit sequences (separated by
chip-select). First sequence has again the register address and an IO
bit in the high byte. Additionally, reads must have a 'read bit' set.
The last 8 bits must be zero. The register data will be carried in the
last 8 bits of the next 16-bit sequence while high bits in reply are zero.

ADC data reading is similar to register reading except:
 - No R/W bit or I/O bit should be set.
 - Register address is replaced by channel number (0 - 31).
 - Reply data is carried in the 12 low bits (instead of 8 bits) of the
   reply sequence.

The protocol is implemented using custom regmap read() and write()
operations.

Other than that, pretty standard device and driver.

Revision history:
 v3 => v4:
 - Fix Kconfig dependency (I2C => SPI)
 - Styling as suggested by Andy and Jonathan
 - Moved I/O documentation comment and read/write functions next to each
   other and tried clarifying the comment

 v2 => v3:
 - Mostly cosmetic changes to the driver
 - dt-bindings and MAINTAINERS unchanged

 v1 => v2:
 - Plenty of fixes to the driver (thanks to reviewers, Andy and David)
 - Add gpio-controller information to the device-tree bindings

See individual patches for more accurate changelog

---
Matti Vaittinen (3):
      dt-bindings: iio: adc: ROHM BD79112 ADC/GPIO
      iio: adc: Support ROHM BD79112 ADC/GPIO
      MAINTAINERS: Support ROHM BD79112 ADC

 .../devicetree/bindings/iio/adc/rohm,bd79112.yaml  | 104 ++++
 MAINTAINERS                                        |   3 +-
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/rohm-bd79112.c                     | 553 +++++++++++++++++++++
 5 files changed, 670 insertions(+), 1 deletion(-)
---
base-commit: d1487b0b78720b86ec2a2ac7acc683ec90627e5b
change-id: 20250910-bd79112-436a087a5013

Best regards,
-- 
Matti Vaittinen <mazziesaccount@gmail.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v4 1/3] dt-bindings: iio: adc: ROHM BD79112 ADC/GPIO

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 3386 bytes --]

The ROHM BD79112 is an ADC/GPIO with 32 channels. The channel inputs can
be used as ADC or GPIO. Using the GPIOs as IRQ sources isn't supported.

The ADC is 12-bit, supporting input voltages up to 5.7V, and separate I/O
voltage supply. Maximum SPI clock rate is 20 MHz (10 MHz with
daisy-chain configuration) and maximum sampling rate is 1MSPS.

Add a device tree binding document for the ROHM BD79112.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

---
Revision history:
v3 => v4:
 - shorten the example by dropping some channels.

v1 => v2:
 - BD79112 can act as a GPIO controller.
---
 .../devicetree/bindings/iio/adc/rohm,bd79112.yaml  | 104 +++++++++++++++++++++
 1 file changed, 104 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/rohm,bd79112.yaml b/Documentation/devicetree/bindings/iio/adc/rohm,bd79112.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..aa8b07c3fac1096c0d48ec64361263624f2bb9fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/rohm,bd79112.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/rohm,bd79112.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD79112 ADC/GPO
+
+maintainers:
+  - Matti Vaittinen <mazziesaccount@gmail.com>
+
+description: |
+  The ROHM BD79112 is a 12-bit, 32-channel, SAR ADC. ADC input pins can be
+  also configured as general purpose inputs/outputs. SPI should use MODE 3.
+
+properties:
+  compatible:
+    const: rohm,bd79112
+
+  reg:
+    maxItems: 1
+
+  spi-cpha: true
+  spi-cpol: true
+
+  gpio-controller: true
+  "#gpio-cells":
+    const: 2
+
+  vdd-supply: true
+
+  iovdd-supply: true
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+patternProperties:
+  "^channel@([0-9]|[12][0-9]|3[01])$":
+    type: object
+    $ref: /schemas/iio/adc/adc.yaml#
+    description: Represents ADC channel. Omitted channels' inputs are GPIOs.
+
+    properties:
+      reg:
+        description: AIN pin number
+        minimum: 0
+        maximum: 31
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - iovdd-supply
+  - vdd-supply
+  - spi-cpha
+  - spi-cpol
+
+additionalProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        adc: adc@0 {
+            compatible = "rohm,bd79112";
+            reg = <0x0>;
+
+            spi-cpha;
+            spi-cpol;
+
+            vdd-supply = <&dummyreg>;
+            iovdd-supply = <&dummyreg>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            gpio-controller;
+            #gpio-cells = <2>;
+
+            channel@0 {
+                reg = <0>;
+            };
+            channel@1 {
+                reg = <1>;
+            };
+            channel@2 {
+                reg = <2>;
+            };
+            channel@16 {
+                reg = <16>;
+            };
+            channel@20 {
+                reg = <20>;
+            };
+        };
+    };

-- 
2.51.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v4 2/3] iio: adc: Support ROHM BD79112 ADC/GPIO

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 19866 bytes --]

The ROHM BD79112 is an ADC/GPIO with 32 channels. The channel inputs can
be used as ADC or GPIO. Using the GPIOs as IRQ sources isn't supported.

The ADC is 12-bit, supporting input voltages up to 5.7V, and separate I/O
voltage supply. Maximum SPI clock rate is 20 MHz (10 MHz with
daisy-chain configuration) and maximum sampling rate is 1MSPS.

The IC does also support CRC but it is not implemented in the driver.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
Revision history:
v3 => v4:
 - Fix Kconfig dependency (I2C => SPI)
 - Styling as suggested by Andy and Jonathan
 - Moved I/O documentation comment and read/write functions next to each
   other and tried clarifying the comment

v2 => v3: (mainly based on review by Andy, thanks!)
 - fix broken indentiation
 - re-order includes
 - drop useless < 0 check for an unsigned offset
 - use gc->ngpios instead of hard coded pincount in
   for_each_set_clump8()
 - drop useless check for zero mask inside for_each_set_clump8() body
 - reorder return value checks for the
   devm_iio_adc_device_alloc_chaninfo_se() as suggested by Andy. (Well,
   I am not 100% happy with it as it results an extra check in the
   hopefully most common 'success' -case. But yeah, probe is not exactly
   a fast path).

v1 => v2 (mainly based on reviews by Andy and David, thanks!)
 - Fix Kconfig dependency to REGMAP_SPI instead of REGMAP_I2C
 - Add a few header includes
 - Drop unnecessary alignments
 - plenty of styling
 - use for_each_set_clump8 instead of open-coding it
 - change order of direction setting writes to avoid receiving 'event'
   when direction is changed from input to output.
 - fix data-sheet names and assigning of them to iio_dev
---
 drivers/iio/adc/Kconfig        |  10 +
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/rohm-bd79112.c | 553 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 564 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index e3d3826c335714652726f012018aa5ecb9124a6d..984246b6f57f34e8c097fb92b59df158dd2a14a4 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1309,6 +1309,16 @@ config RN5T618_ADC
 	  This driver can also be built as a module. If so, the module
 	  will be called rn5t618-adc.
 
+config ROHM_BD79112
+	tristate "Rohm BD79112 ADC driver"
+	depends on SPI && GPIOLIB
+	select REGMAP_SPI
+	select IIO_ADC_HELPER
+	help
+	  Say yes here to build support for the ROHM BD79112 ADC. The
+	  ROHM BD79112 is a 12-bit, 32-channel, SAR ADC. Analog inputs
+	  can also be used for GPIO.
+
 config ROHM_BD79124
 	tristate "Rohm BD79124 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 89d72bf9ce70ac1e225fbef83a83dd1a13aef27c..34b40c34cf7191c3367c6c57dcad6d1dbe374824 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -117,6 +117,7 @@ obj-$(CONFIG_QCOM_VADC_COMMON) += qcom-vadc-common.o
 obj-$(CONFIG_RCAR_GYRO_ADC) += rcar-gyroadc.o
 obj-$(CONFIG_RICHTEK_RTQ6056) += rtq6056.o
 obj-$(CONFIG_RN5T618_ADC) += rn5t618-adc.o
+obj-$(CONFIG_ROHM_BD79112) += rohm-bd79112.o
 obj-$(CONFIG_ROHM_BD79124) += rohm-bd79124.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
 obj-$(CONFIG_RZG2L_ADC) += rzg2l_adc.o
diff --git a/drivers/iio/adc/rohm-bd79112.c b/drivers/iio/adc/rohm-bd79112.c
new file mode 100644
index 0000000000000000000000000000000000000000..a2a3affe2c6dc86a237a164139c27ec66dc9d131
--- /dev/null
+++ b/drivers/iio/adc/rohm-bd79112.c
@@ -0,0 +1,553 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ROHM ADC driver for BD79112 signal monitoring hub.
+ * Copyright (C) 2025, ROHM Semiconductor.
+ *
+ * SPI communication derived from ad7923.c and ti-ads7950.c
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/bits.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/gpio/driver.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <asm/byteorder.h>
+
+#include <linux/iio/adc-helpers.h>
+#include <linux/iio/iio.h>
+
+#define BD79112_MAX_NUM_CHANNELS 32
+
+struct bd79112_data {
+	struct spi_device *spi;
+	struct regmap *map;
+	struct device *dev;
+	struct gpio_chip gc;
+	unsigned long gpio_valid_mask;
+	unsigned int vref_mv;
+	struct spi_transfer read_xfer[2];
+	struct spi_transfer write_xfer;
+	struct spi_message read_msg;
+	struct spi_message write_msg;
+	/* 16-bit TX, valid data in high byte */
+	u8 read_tx[2] __aligned(IIO_DMA_MINALIGN);
+	/* 8-bit address followed by 8-bit data */
+	u8 reg_write_tx[2];
+	/* 12-bit of ADC data or 8 bit of reg data */
+	__be16 read_rx;
+};
+
+/*
+ * The ADC data is read issuing SPI-command matching the channel number.
+ * We treat this as a register address.
+ */
+#define BD79112_REG_AGIO0A		0x00
+#define BD79112_REG_AGIO15B		0x1f
+
+/*
+ * ADC STATUS_FLAG appended to ADC data will be set, if the ADC result is being
+ * read for a channel, which input pin is muxed to be a GPIO.
+ */
+#define BD79112_ADC_STATUS_FLAG BIT(14)
+
+/*
+ * The BD79112 requires "R/W bit" to be set for SPI register (not ADC data)
+ * reads and an "IOSET bit" to be set for read/write operations (which aren't
+ * reading the ADC data).
+ */
+#define BD79112_BIT_RW			BIT(4)
+#define BD79112_BIT_IO			BIT(5)
+
+#define BD79112_REG_GPI_VALUE_B8_15	(BD79112_BIT_IO | 0x0)
+#define BD79112_REG_GPI_VALUE_B0_B7	(BD79112_BIT_IO | 0x1)
+#define BD79112_REG_GPI_VALUE_A8_15	(BD79112_BIT_IO | 0x2)
+#define BD79112_REG_GPI_VALUE_A0_A7	(BD79112_BIT_IO | 0x3)
+
+#define BD79112_REG_GPI_EN_B7_B15	(BD79112_BIT_IO | 0x4)
+#define BD79112_REG_GPI_EN_B0_B7	(BD79112_BIT_IO | 0x5)
+#define BD79112_REG_GPI_EN_A8_A15	(BD79112_BIT_IO | 0x6)
+#define BD79112_REG_GPI_EN_A0_A7	(BD79112_BIT_IO | 0x7)
+
+#define BD79112_REG_GPO_EN_B7_B15	(BD79112_BIT_IO | 0x8)
+#define BD79112_REG_GPO_EN_B0_B7	(BD79112_BIT_IO | 0x9)
+#define BD79112_REG_GPO_EN_A8_A15	(BD79112_BIT_IO | 0xa)
+#define BD79112_REG_GPO_EN_A0_A7	(BD79112_BIT_IO | 0xb)
+
+#define BD79112_NUM_GPIO_EN_REGS	8
+#define BD79112_FIRST_GPIO_EN_REG	BD79112_REG_GPI_EN_B7_B15
+
+#define BD79112_REG_GPO_VALUE_B8_15	(BD79112_BIT_IO | 0xc)
+#define BD79112_REG_GPO_VALUE_B0_B7	(BD79112_BIT_IO | 0xd)
+#define BD79112_REG_GPO_VALUE_A8_15	(BD79112_BIT_IO | 0xe)
+#define BD79112_REG_GPO_VALUE_A0_A7	(BD79112_BIT_IO | 0xf)
+
+#define BD79112_REG_MAX BD79112_REG_GPO_VALUE_A0_A7
+
+/*
+ * Read transaction consists of two 16-bit sequences separated by CSB.
+ * For register read, 'IOSET' bit must be set. For ADC read, IOSET is cleared
+ * and ADDR equals the channel number (0 ... 31).
+ *
+ * First 16-bit sequence, MOSI as below, MISO data ignored:
+ * - SCK: | 1 | 2 |   3   |    4   | 5 .. 8 | 9 .. 16 |
+ * - MOSI:| 0 | 0 | IOSET | RW (1) |  ADDR  |  8'b0   |
+ *
+ * CSB released and re-acquired between these sequences
+ *
+ * Second 16-bit sequence, MISO as below, MOSI data ignored:
+ *   For Register read data is 8 bits:
+ *   - SCK: | 1 .. 8 |   9 .. 16   |
+ *   - MISO:|  8'b0  | 8-bit data  |
+ *
+ *   For ADC read data is 12 bits:
+ *   - SCK: | 1 .. 4 |   4 .. 16   |
+ *   - MISO:|  4'b0  | 12-bit data |
+ */
+static int bd79112_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct bd79112_data *data = context;
+	int ret;
+
+	if (reg & BD79112_BIT_IO)
+		reg |= BD79112_BIT_RW;
+
+	data->read_tx[0] = reg;
+
+	ret = spi_sync(data->spi, &data->read_msg);
+	if (!ret)
+		*val = be16_to_cpu(data->read_rx);
+
+	if (reg & BD79112_BIT_IO && *val & BD79112_ADC_STATUS_FLAG)
+		dev_err(data->dev, "ADC pin configured as GPIO\n");
+
+	return ret;
+}
+
+/*
+ * Write, single 16-bit sequence (broken down below):
+ *
+ * First 8-bit, MOSI as below, MISO data ignored:
+ * - SCK: | 1 | 2 | 3   | 4     | 5 .. 8 |
+ * - MOSI:| 0 | 0 |IOSET| RW(0) | ADDR   |
+ *
+ * Last 8 SCK cycles (b8 ... b15), MISO contains register data, MOSI ignored.
+ * - SCK: | 9 .. 16 |
+ * - MISO:|  data   |
+ */
+static int bd79112_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct bd79112_data *data = context;
+
+	data->reg_write_tx[0] = reg;
+	data->reg_write_tx[1] = val;
+
+	return spi_sync(data->spi, &data->write_msg);
+}
+
+static int _get_gpio_reg(unsigned int offset, unsigned int base)
+{
+	int regoffset = offset / 8;
+
+	if (offset > 31)
+		return -EINVAL;
+
+	return base - regoffset;
+}
+
+#define GET_GPIO_BIT(offset) BIT((offset) % 8)
+#define GET_GPO_EN_REG(offset)  _get_gpio_reg((offset), BD79112_REG_GPO_EN_A0_A7)
+#define GET_GPI_EN_REG(offset)  _get_gpio_reg((offset), BD79112_REG_GPI_EN_A0_A7)
+#define GET_GPO_VAL_REG(offset)  _get_gpio_reg((offset), BD79112_REG_GPO_VALUE_A0_A7)
+#define GET_GPI_VAL_REG(offset)  _get_gpio_reg((offset), BD79112_REG_GPI_VALUE_A0_A7)
+
+static const struct regmap_range bd71815_volatile_ro_ranges[] = {
+	{
+		/* Read ADC data */
+		.range_min = BD79112_REG_AGIO0A,
+		.range_max = BD79112_REG_AGIO15B,
+	}, {
+		/* GPI state */
+		.range_min = BD79112_REG_GPI_VALUE_B8_15,
+		.range_max = BD79112_REG_GPI_VALUE_A0_A7,
+	},
+};
+
+static const struct regmap_access_table bd79112_volatile_regs = {
+	.yes_ranges = &bd71815_volatile_ro_ranges[0],
+	.n_yes_ranges = ARRAY_SIZE(bd71815_volatile_ro_ranges),
+};
+
+static const struct regmap_access_table bd79112_ro_regs = {
+	.no_ranges = &bd71815_volatile_ro_ranges[0],
+	.n_no_ranges = ARRAY_SIZE(bd71815_volatile_ro_ranges),
+};
+
+static const struct regmap_config bd79112_regmap = {
+	.reg_read = bd79112_reg_read,
+	.reg_write = bd79112_reg_write,
+	.volatile_table = &bd79112_volatile_regs,
+	.wr_table = &bd79112_ro_regs,
+	.cache_type = REGCACHE_MAPLE,
+	.max_register = BD79112_REG_MAX,
+};
+
+static int bd79112_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long m)
+{
+	struct bd79112_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (m) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_read(data->map, chan->channel, val);
+		if (ret < 0)
+			return ret;
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = data->vref_mv;
+		*val2 = 12;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info bd79112_info = {
+	.read_raw = bd79112_read_raw,
+};
+
+static const struct iio_chan_spec bd79112_chan_template = {
+	.type = IIO_VOLTAGE,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+	.indexed = 1,
+};
+
+static int bd79112_gpio_init_valid_mask(struct gpio_chip *gc,
+					unsigned long *valid_mask,
+					unsigned int ngpios)
+{
+	struct bd79112_data *data = gpiochip_get_data(gc);
+
+	*valid_mask = data->gpio_valid_mask;
+
+	return 0;
+}
+
+static int bd79112_gpio_dir_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct bd79112_data *data = gpiochip_get_data(gc);
+	unsigned int reg, bit, val;
+	int ret;
+
+	bit = GET_GPIO_BIT(offset);
+	reg = GET_GPO_EN_REG(offset);
+
+	ret = regmap_read(data->map, reg, &val);
+	if (ret)
+		return ret;
+
+	if (bit & val)
+		return GPIO_LINE_DIRECTION_OUT;
+
+	reg = GET_GPI_EN_REG(offset);
+	ret = regmap_read(data->map, reg, &val);
+	if (ret)
+		return ret;
+
+	if (bit & val)
+		return GPIO_LINE_DIRECTION_IN;
+
+	/*
+	 * Ouch. Seems the pin is ADC input - shouldn't happen as changing mux
+	 * at runtime is not supported and non GPIO pins should be invalidated
+	 * by the valid_mask at probe. Maybe someone wrote a register bypassing
+	 * the driver?
+	 */
+	dev_err(data->dev, "Pin not a GPIO\n");
+
+	return -EINVAL;
+}
+
+static int bd79112_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct bd79112_data *data = gpiochip_get_data(gc);
+	unsigned int reg, bit, val;
+	int ret;
+
+	bit = GET_GPIO_BIT(offset);
+	reg = GET_GPI_VAL_REG(offset);
+
+	ret = regmap_read(data->map, reg, &val);
+	if (ret)
+		return ret;
+
+	return !!(val & bit);
+}
+
+static int bd79112_gpio_set(struct gpio_chip *gc, unsigned int offset,
+			    int value)
+{
+	struct bd79112_data *data = gpiochip_get_data(gc);
+	unsigned int reg, bit;
+
+	bit = GET_GPIO_BIT(offset);
+	reg = GET_GPO_VAL_REG(offset);
+
+	return regmap_assign_bits(data->map, reg, bit, value);
+}
+
+static int bd79112_gpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
+				     unsigned long *bits)
+{
+	struct bd79112_data *data = gpiochip_get_data(gc);
+	unsigned long i, bank_mask;
+
+	for_each_set_clump8(i, bank_mask, mask, gc->ngpio) {
+		unsigned long bank_bits;
+		unsigned int reg;
+		int ret;
+
+		bank_bits = bitmap_get_value8(bits, i);
+		reg = BD79112_REG_GPO_VALUE_A0_A7 - i / 8;
+		ret = regmap_update_bits(data->map, reg, bank_mask, bank_bits);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int bd79112_gpio_dir_set(struct bd79112_data *data, unsigned int offset,
+				int dir)
+{
+	unsigned int gpi_reg, gpo_reg, bit;
+	int ret;
+
+	bit = GET_GPIO_BIT(offset);
+	gpi_reg = GET_GPI_EN_REG(offset);
+	gpo_reg =  GET_GPO_EN_REG(offset);
+
+	if (dir == GPIO_LINE_DIRECTION_OUT) {
+		ret = regmap_clear_bits(data->map, gpi_reg, bit);
+		if (ret)
+			return ret;
+
+		return regmap_set_bits(data->map, gpo_reg, bit);
+	}
+
+	ret = regmap_set_bits(data->map, gpi_reg, bit);
+	if (ret)
+		return ret;
+
+	return regmap_clear_bits(data->map, gpo_reg, bit);
+}
+
+static int bd79112_gpio_input(struct gpio_chip *gc, unsigned int offset)
+{
+	struct bd79112_data *data = gpiochip_get_data(gc);
+
+	return bd79112_gpio_dir_set(data, offset, GPIO_LINE_DIRECTION_IN);
+}
+
+static int bd79112_gpio_output(struct gpio_chip *gc, unsigned int offset,
+			       int value)
+{
+	struct bd79112_data *data = gpiochip_get_data(gc);
+	int ret;
+
+	ret = bd79112_gpio_set(gc, offset, value);
+	if (ret)
+		return ret;
+
+	return bd79112_gpio_dir_set(data, offset, GPIO_LINE_DIRECTION_OUT);
+}
+
+static const struct gpio_chip bd79112_gpio_chip = {
+	.label			= "bd79112-gpio",
+	.get_direction		= bd79112_gpio_dir_get,
+	.direction_input	= bd79112_gpio_input,
+	.direction_output	= bd79112_gpio_output,
+	.get			= bd79112_gpio_get,
+	.set			= bd79112_gpio_set,
+	.set_multiple		= bd79112_gpio_set_multiple,
+	.init_valid_mask	= bd79112_gpio_init_valid_mask,
+	.can_sleep		= true,
+	.ngpio			= 32,
+	.base			= -1,
+};
+
+static int bd79112_get_gpio_pins(const struct iio_chan_spec *cs, int num_channels)
+{
+	int i, gpio_channels;
+
+	/*
+	 * Let's initialize the mux config to say that all 32 channels are
+	 * GPIOs. Then we can just loop through the iio_chan_spec and clear the
+	 * bits for found ADC channels.
+	 */
+	gpio_channels = GENMASK(31, 0);
+	for (i = 0; i < num_channels; i++)
+		gpio_channels &= ~BIT(cs[i].channel);
+
+	return gpio_channels;
+}
+
+/* ADC channels as named in the data-sheet */
+static const char * const bd79112_chan_names[] = {
+	"AGIO0A", "AGIO1A", "AGIO2A", "AGIO3A",		/* 0 - 3 */
+	"AGIO4A", "AGIO5A", "AGIO6A", "AGIO7A",		/* 4 - 7 */
+	"AGIO8A", "AGIO9A", "AGIO10A", "AGIO11A",	/* 8 - 11 */
+	"AGIO12A", "AGIO13A", "AGIO14A", "AGIO15A",	/* 12 - 15 */
+	"AGIO0B", "AGIO1B", "AGIO2B", "AGIO3B",		/* 16 - 19 */
+	"AGIO4B", "AGIO5B", "AGIO6B", "AGIO7B",		/* 20 - 23 */
+	"AGIO8B", "AGIO9B", "AGIO10B", "AGIO11B",	/* 24 - 27 */
+	"AGIO12B", "AGIO13B", "AGIO14B", "AGIO15B",	/* 28 - 31 */
+};
+
+static int bd79112_probe(struct spi_device *spi)
+{
+	struct bd79112_data *data;
+	struct iio_dev *iio_dev;
+	struct iio_chan_spec *cs;
+	struct device *dev = &spi->dev;
+	unsigned long gpio_pins, pin;
+	unsigned int i;
+	int ret;
+
+	iio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!iio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(iio_dev);
+	data->spi = spi;
+	data->dev = dev;
+	data->map = devm_regmap_init(&spi->dev, NULL, data, &bd79112_regmap);
+	if (IS_ERR(data->map))
+		return dev_err_probe(dev, PTR_ERR(data->map),
+				     "Failed to initialize Regmap\n");
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "vdd");
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to get the Vdd\n");
+
+	data->vref_mv = ret / 1000;
+
+	ret = devm_regulator_get_enable(dev, "iovdd");
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to enable I/O voltage\n");
+
+	data->read_xfer[0].tx_buf = &data->read_tx[0];
+	data->read_xfer[0].len = sizeof(data->read_tx);
+	data->read_xfer[0].cs_change = 1;
+	data->read_xfer[1].rx_buf = &data->read_rx;
+	data->read_xfer[1].len = sizeof(data->read_rx);
+	spi_message_init_with_transfers(&data->read_msg, data->read_xfer, 2);
+
+	data->write_xfer.tx_buf = &data->reg_write_tx[0];
+	data->write_xfer.len = sizeof(data->reg_write_tx);
+	spi_message_init_with_transfers(&data->write_msg, &data->write_xfer, 1);
+
+	ret = devm_iio_adc_device_alloc_chaninfo_se(dev, &bd79112_chan_template,
+						    BD79112_MAX_NUM_CHANNELS - 1,
+						    &cs);
+
+	/* Register all pins as GPIOs if there are no ADC channels */
+	if (ret == -ENOENT)
+		goto register_gpios;
+
+	if (ret < 0)
+		return ret;
+
+	iio_dev->num_channels = ret;
+	iio_dev->channels = cs;
+
+	for (i = 0; i < iio_dev->num_channels; i++) {
+		unsigned int ch = cs[i].channel;
+
+		cs[i].datasheet_name = bd79112_chan_names[ch];
+	}
+
+	iio_dev->info = &bd79112_info;
+	iio_dev->name = "bd79112";
+	iio_dev->modes = INDIO_DIRECT_MODE;
+
+	/*
+	 * Ensure all channels are ADCs. This allows us to register the IIO
+	 * device early (before checking which pins are to be used for GPIO)
+	 * without having to worry about some pins being initially used for
+	 * GPIO.
+	 */
+	for (i = 0; i < BD79112_NUM_GPIO_EN_REGS; i++) {
+		ret = regmap_write(data->map, BD79112_FIRST_GPIO_EN_REG + i, 0);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to initialize channels\n");
+	}
+
+	ret = devm_iio_device_register(data->dev, iio_dev);
+	if (ret)
+		return dev_err_probe(data->dev, ret, "Failed to register ADC\n");
+
+register_gpios:
+	gpio_pins = bd79112_get_gpio_pins(iio_dev->channels,
+					  iio_dev->num_channels);
+
+	/* If all channels are reserved for ADC, then we're done. */
+	if (!gpio_pins)
+		return 0;
+
+	/* Default all the GPIO pins to GPI */
+	for_each_set_bit(pin, &gpio_pins, BD79112_MAX_NUM_CHANNELS) {
+		ret = bd79112_gpio_dir_set(data, pin, GPIO_LINE_DIRECTION_IN);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to mark pin as GPI\n");
+	}
+
+	data->gpio_valid_mask = gpio_pins;
+	data->gc = bd79112_gpio_chip;
+	data->gc.parent = dev;
+
+	return devm_gpiochip_add_data(dev, &data->gc, data);
+}
+
+static const struct of_device_id bd79112_of_match[] = {
+	{ .compatible = "rohm,bd79112" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, bd79112_of_match);
+
+static const struct spi_device_id bd79112_id[] = {
+	{ "bd79112" },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, bd79112_id);
+
+static struct spi_driver bd79112_driver = {
+	.driver = {
+		.name = "bd79112",
+		.of_match_table = bd79112_of_match,
+	},
+	.probe = bd79112_probe,
+	.id_table = bd79112_id,
+};
+module_spi_driver(bd79112_driver);
+
+MODULE_AUTHOR("Matti Vaittinen <mazziesaccount@gmail.com>");
+MODULE_DESCRIPTION("Driver for ROHM BD79112 ADC/GPIO");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("IIO_DRIVER");

-- 
2.51.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v4 3/3] MAINTAINERS: Support ROHM BD79112 ADC

[Matti Vaittinen]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

Add the ROHM BD79112 ADC in the list of the BD791xx ADC drivers
which are maintained by undersigned.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

---
Revision history:
v1 => :
 - no changes
---
 MAINTAINERS | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index af1c8d2bfb3deb870d8df44b8bae22e7cffb5aca..8e78a1168c17d8c2c7056e99e191d542ef0b95a6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21864,9 +21864,10 @@ S:	Supported
 F:	drivers/power/supply/bd99954-charger.c
 F:	drivers/power/supply/bd99954-charger.h
 
-ROHM BD79124 ADC / GPO IC
+ROHM BD791xx ADC / GPO IC
 M:	Matti Vaittinen <mazziesaccount@gmail.com>
 S:	Supported
+F:	drivers/iio/adc/rohm-bd79112.c
 F:	drivers/iio/adc/rohm-bd79124.c
 
 ROHM BH1745 COLOUR SENSOR

-- 
2.51.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v4 2/3] iio: adc: Support ROHM BD79112 ADC/GPIO

[Jonathan Cameron]

On Wed, 10 Sep 2025 14:24:35 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> The ROHM BD79112 is an ADC/GPIO with 32 channels. The channel inputs can
> be used as ADC or GPIO. Using the GPIOs as IRQ sources isn't supported.
> 
> The ADC is 12-bit, supporting input voltages up to 5.7V, and separate I/O
> voltage supply. Maximum SPI clock rate is 20 MHz (10 MHz with
> daisy-chain configuration) and maximum sampling rate is 1MSPS.
> 
> The IC does also support CRC but it is not implemented in the driver.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>

Hi Matti,

A few trivial things that I'll tidy up if nothing else comes up (I might not
bother given how trivial they are!)

Also one question. I couldn't immediately follow why any random register
read is sanity checking if an ADC pin is configured as GPIO.

Jonathan

> diff --git a/drivers/iio/adc/rohm-bd79112.c b/drivers/iio/adc/rohm-bd79112.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..a2a3affe2c6dc86a237a164139c27ec66dc9d131
> --- /dev/null
> +++ b/drivers/iio/adc/rohm-bd79112.c
> @@ -0,0 +1,553 @@


> +/*
> + * The BD79112 requires "R/W bit" to be set for SPI register (not ADC data)
> + * reads and an "IOSET bit" to be set for read/write operations (which aren't
> + * reading the ADC data).
> + */

> +/*
> + * Read transaction consists of two 16-bit sequences separated by CSB.
> + * For register read, 'IOSET' bit must be set. For ADC read, IOSET is cleared
> + * and ADDR equals the channel number (0 ... 31).
> + *
> + * First 16-bit sequence, MOSI as below, MISO data ignored:
> + * - SCK: | 1 | 2 |   3   |    4   | 5 .. 8 | 9 .. 16 |
> + * - MOSI:| 0 | 0 | IOSET | RW (1) |  ADDR  |  8'b0   |
> + *
> + * CSB released and re-acquired between these sequences
> + *
> + * Second 16-bit sequence, MISO as below, MOSI data ignored:
> + *   For Register read data is 8 bits:
> + *   - SCK: | 1 .. 8 |   9 .. 16   |
> + *   - MISO:|  8'b0  | 8-bit data  |
> + *
> + *   For ADC read data is 12 bits:
> + *   - SCK: | 1 .. 4 |   4 .. 16   |
> + *   - MISO:|  4'b0  | 12-bit data |
> + */
> +static int bd79112_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct bd79112_data *data = context;
> +	int ret;
> +
> +	if (reg & BD79112_BIT_IO)
> +		reg |= BD79112_BIT_RW;
> +
> +	data->read_tx[0] = reg;
> +
> +	ret = spi_sync(data->spi, &data->read_msg);
> +	if (!ret)
> +		*val = be16_to_cpu(data->read_rx);
> +
> +	if (reg & BD79112_BIT_IO && *val & BD79112_ADC_STATUS_FLAG)
> +		dev_err(data->dev, "ADC pin configured as GPIO\n");

Why are we checking this in a regmap callback?
Maybe it needs rewording and the point is some missmatch in what we
can read vs the state?

> +
> +	return ret;
> +}

> +
> +static int bd79112_probe(struct spi_device *spi)
> +{
> +	struct bd79112_data *data;
> +	struct iio_dev *iio_dev;
> +	struct iio_chan_spec *cs;
> +	struct device *dev = &spi->dev;
> +	unsigned long gpio_pins, pin;
> +	unsigned int i;
> +	int ret;
> +
> +	iio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!iio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(iio_dev);
> +	data->spi = spi;
> +	data->dev = dev;
> +	data->map = devm_regmap_init(&spi->dev, NULL, data, &bd79112_regmap);

	data->mpa = devm_regmap_init(dev, ...


> +	if (IS_ERR(data->map))
> +		return dev_err_probe(dev, PTR_ERR(data->map),
> +				     "Failed to initialize Regmap\n");
> +
> +	ret = devm_regulator_get_enable_read_voltage(dev, "vdd");
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to get the Vdd\n");
> +
> +	data->vref_mv = ret / 1000;
> +
> +	ret = devm_regulator_get_enable(dev, "iovdd");
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "Failed to enable I/O voltage\n");
> +
> +	data->read_xfer[0].tx_buf = &data->read_tx[0];
> +	data->read_xfer[0].len = sizeof(data->read_tx);
> +	data->read_xfer[0].cs_change = 1;
> +	data->read_xfer[1].rx_buf = &data->read_rx;
> +	data->read_xfer[1].len = sizeof(data->read_rx);
> +	spi_message_init_with_transfers(&data->read_msg, data->read_xfer, 2);
> +
> +	data->write_xfer.tx_buf = &data->reg_write_tx[0];
> +	data->write_xfer.len = sizeof(data->reg_write_tx);
> +	spi_message_init_with_transfers(&data->write_msg, &data->write_xfer, 1);
> +
> +	ret = devm_iio_adc_device_alloc_chaninfo_se(dev, &bd79112_chan_template,
> +						    BD79112_MAX_NUM_CHANNELS - 1,
> +						    &cs);
> +
> +	/* Register all pins as GPIOs if there are no ADC channels */
> +	if (ret == -ENOENT)
> +		goto register_gpios;
> +
> +	if (ret < 0)
> +		return ret;
> +
> +	iio_dev->num_channels = ret;
> +	iio_dev->channels = cs;
> +
> +	for (i = 0; i < iio_dev->num_channels; i++) {
> +		unsigned int ch = cs[i].channel;
> +
> +		cs[i].datasheet_name = bd79112_chan_names[ch];

Could have done

		cs[i].datasheet_name = bd79112_chan_names[cs[i].channel];

and I don't think it makes it harder to read, but doesn't matter enough to respin.

> +	}

Re: [PATCH v4 2/3] iio: adc: Support ROHM BD79112 ADC/GPIO

[Matti Vaittinen]

Morning Jonathan,

On 10/09/2025 20:46, Jonathan Cameron wrote:
> On Wed, 10 Sep 2025 14:24:35 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> The ROHM BD79112 is an ADC/GPIO with 32 channels. The channel inputs can
>> be used as ADC or GPIO. Using the GPIOs as IRQ sources isn't supported.
>>
>> The ADC is 12-bit, supporting input voltages up to 5.7V, and separate I/O
>> voltage supply. Maximum SPI clock rate is 20 MHz (10 MHz with
>> daisy-chain configuration) and maximum sampling rate is 1MSPS.
>>
>> The IC does also support CRC but it is not implemented in the driver.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> Hi Matti,
> 
> A few trivial things that I'll tidy up if nothing else comes up (I might not
> bother given how trivial they are!)

Thanks again!

> Also one question. I couldn't immediately follow why any random register
> read is sanity checking if an ADC pin is configured as GPIO.
> 

Ah. Valid question! I see my comment below is partially wrong.


>> +/*
>> + * Read transaction consists of two 16-bit sequences separated by CSB.
>> + * For register read, 'IOSET' bit must be set. For ADC read, IOSET is cleared
>> + * and ADDR equals the channel number (0 ... 31).
>> + *
>> + * First 16-bit sequence, MOSI as below, MISO data ignored:
>> + * - SCK: | 1 | 2 |   3   |    4   | 5 .. 8 | 9 .. 16 |
>> + * - MOSI:| 0 | 0 | IOSET | RW (1) |  ADDR  |  8'b0   |
>> + *
>> + * CSB released and re-acquired between these sequences
>> + *
>> + * Second 16-bit sequence, MISO as below, MOSI data ignored:
>> + *   For Register read data is 8 bits:
>> + *   - SCK: | 1 .. 8 |   9 .. 16   |
>> + *   - MISO:|  8'b0  | 8-bit data  |
>> + *
>> + *   For ADC read data is 12 bits:
>> + *   - SCK: | 1 .. 4 |   4 .. 16   |
>> + *   - MISO:|  4'b0  | 12-bit data |

This is not 100% true. I overlooked the ADC read "status flag" when 
adding this comment for the ADC data reading.

This should be:

  *   For ADC, read data is 12 bits prepended with a status flag:
  *   - SCK: | 1 |      2      | 3  4 |   4 .. 16   |
  *   - MISO:| 0 | STATUS_FLAG | 2'b0 | 12-bit data |

The 'STATUS_FLAG' is set if the input pin is configured as a GPIO.

>> + */
>> +static int bd79112_reg_read(void *context, unsigned int reg, unsigned int *val)
>> +{
>> +	struct bd79112_data *data = context;
>> +	int ret;
>> +
>> +	if (reg & BD79112_BIT_IO)
>> +		reg |= BD79112_BIT_RW;
>> +
>> +	data->read_tx[0] = reg;
>> +
>> +	ret = spi_sync(data->spi, &data->read_msg);
>> +	if (!ret)
>> +		*val = be16_to_cpu(data->read_rx);
>> +
>> +	if (reg & BD79112_BIT_IO && *val & BD79112_ADC_STATUS_FLAG)
>> +		dev_err(data->dev, "ADC pin configured as GPIO\n");
> 
> Why are we checking this in a regmap callback?
> Maybe it needs rewording and the point is some missmatch in what we
> can read vs the state?
> 
>> +
>> +	return ret;
>> +}
> 
>> +
>> +static int bd79112_probe(struct spi_device *spi)
>> +{
>> +	struct bd79112_data *data;
>> +	struct iio_dev *iio_dev;
>> +	struct iio_chan_spec *cs;
>> +	struct device *dev = &spi->dev;
>> +	unsigned long gpio_pins, pin;
>> +	unsigned int i;
>> +	int ret;
>> +
>> +	iio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>> +	if (!iio_dev)
>> +		return -ENOMEM;
>> +
>> +	data = iio_priv(iio_dev);
>> +	data->spi = spi;
>> +	data->dev = dev;
>> +	data->map = devm_regmap_init(&spi->dev, NULL, data, &bd79112_regmap);
> 
> 	data->mpa = devm_regmap_init(dev, ...
> 
> 
>> +	if (IS_ERR(data->map))
>> +		return dev_err_probe(dev, PTR_ERR(data->map),
>> +				     "Failed to initialize Regmap\n");
>> +
>> +	ret = devm_regulator_get_enable_read_voltage(dev, "vdd");
>> +	if (ret < 0)
>> +		return dev_err_probe(dev, ret, "Failed to get the Vdd\n");
>> +
>> +	data->vref_mv = ret / 1000;
>> +
>> +	ret = devm_regulator_get_enable(dev, "iovdd");
>> +	if (ret < 0)
>> +		return dev_err_probe(dev, ret, "Failed to enable I/O voltage\n");
>> +
>> +	data->read_xfer[0].tx_buf = &data->read_tx[0];
>> +	data->read_xfer[0].len = sizeof(data->read_tx);
>> +	data->read_xfer[0].cs_change = 1;
>> +	data->read_xfer[1].rx_buf = &data->read_rx;
>> +	data->read_xfer[1].len = sizeof(data->read_rx);
>> +	spi_message_init_with_transfers(&data->read_msg, data->read_xfer, 2);
>> +
>> +	data->write_xfer.tx_buf = &data->reg_write_tx[0];
>> +	data->write_xfer.len = sizeof(data->reg_write_tx);
>> +	spi_message_init_with_transfers(&data->write_msg, &data->write_xfer, 1);
>> +
>> +	ret = devm_iio_adc_device_alloc_chaninfo_se(dev, &bd79112_chan_template,
>> +						    BD79112_MAX_NUM_CHANNELS - 1,
>> +						    &cs);
>> +
>> +	/* Register all pins as GPIOs if there are no ADC channels */
>> +	if (ret == -ENOENT)
>> +		goto register_gpios;
>> +
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	iio_dev->num_channels = ret;
>> +	iio_dev->channels = cs;
>> +
>> +	for (i = 0; i < iio_dev->num_channels; i++) {
>> +		unsigned int ch = cs[i].channel;
>> +
>> +		cs[i].datasheet_name = bd79112_chan_names[ch];
> 
> Could have done
> 
> 		cs[i].datasheet_name = bd79112_chan_names[cs[i].channel];
> 
> and I don't think it makes it harder to read, but doesn't matter enough to respin.

I kind of agree. It stays short and allows us to get rid of the 
brackets. Thanks!

I can still re-spin this later this week, assuming you rather not fix 
the data-format comment while applying :)

Thanks for pointing this out!

Yours,
	-- Matti

Re: [PATCH v4 2/3] iio: adc: Support ROHM BD79112 ADC/GPIO

[David Lechner]

On 9/10/25 6:24 AM, Matti Vaittinen wrote:

...

> diff --git a/drivers/iio/adc/rohm-bd79112.c b/drivers/iio/adc/rohm-bd79112.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..a2a3affe2c6dc86a237a164139c27ec66dc9d131
> --- /dev/null
> +++ b/drivers/iio/adc/rohm-bd79112.c
> @@ -0,0 +1,553 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ROHM ADC driver for BD79112 signal monitoring hub.
> + * Copyright (C) 2025, ROHM Semiconductor.
> + *
> + * SPI communication derived from ad7923.c and ti-ads7950.c

Really? I wrote the ti-ads7950 driver and I can't say I see the
resemblance. ;-)

> + */
> +

...

> +static int bd79112_get_gpio_pins(const struct iio_chan_spec *cs, int num_channels)

u32 would make more sense when dealing with bit flags.

> +{
> +	int i, gpio_channels;

same for the local variable.

...

> +static int bd79112_probe(struct spi_device *spi)
> +{

...

> +
> +	data->read_xfer[0].tx_buf = &data->read_tx[0];
> +	data->read_xfer[0].len = sizeof(data->read_tx);
> +	data->read_xfer[0].cs_change = 1;
> +	data->read_xfer[1].rx_buf = &data->read_rx;
> +	data->read_xfer[1].len = sizeof(data->read_rx);
> +	spi_message_init_with_transfers(&data->read_msg, data->read_xfer, 2);
> +
> +	data->write_xfer.tx_buf = &data->reg_write_tx[0];
> +	data->write_xfer.len = sizeof(data->reg_write_tx);
> +	spi_message_init_with_transfers(&data->write_msg, &data->write_xfer, 1);

If these messages never change (other than the data in the buffers), you can
call devm_spi_optimize_message() here on each message to get reduced CPU usage
on every SPI message for free.

Re: [PATCH v4 3/3] MAINTAINERS: Support ROHM BD79112 ADC

[David Lechner]

On 9/10/25 6:24 AM, Matti Vaittinen wrote:
> Add the ROHM BD79112 ADC in the list of the BD791xx ADC drivers
> which are maintained by undersigned.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
> Revision history:
> v1 => :
>  - no changes
> ---
>  MAINTAINERS | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index af1c8d2bfb3deb870d8df44b8bae22e7cffb5aca..8e78a1168c17d8c2c7056e99e191d542ef0b95a6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21864,9 +21864,10 @@ S:	Supported
>  F:	drivers/power/supply/bd99954-charger.c
>  F:	drivers/power/supply/bd99954-charger.h
>  
> -ROHM BD79124 ADC / GPO IC
> +ROHM BD791xx ADC / GPO IC
>  M:	Matti Vaittinen <mazziesaccount@gmail.com>
>  S:	Supported

Should we include the devicetree binding files here?

> +F:	drivers/iio/adc/rohm-bd79112.c
>  F:	drivers/iio/adc/rohm-bd79124.c
>  
>  ROHM BH1745 COLOUR SENSOR
> 

Re: [PATCH v4 2/3] iio: adc: Support ROHM BD79112 ADC/GPIO

[Matti Vaittinen]

On 12/09/2025 00:20, David Lechner wrote:
> On 9/10/25 6:24 AM, Matti Vaittinen wrote:
> 
> ...
> 
>> diff --git a/drivers/iio/adc/rohm-bd79112.c b/drivers/iio/adc/rohm-bd79112.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..a2a3affe2c6dc86a237a164139c27ec66dc9d131
>> --- /dev/null
>> +++ b/drivers/iio/adc/rohm-bd79112.c
>> @@ -0,0 +1,553 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * ROHM ADC driver for BD79112 signal monitoring hub.
>> + * Copyright (C) 2025, ROHM Semiconductor.
>> + *
>> + * SPI communication derived from ad7923.c and ti-ads7950.c
> 
> Really? I wrote the ti-ads7950 driver and I can't say I see the
> resemblance. ;-)

Really. :) I picked the idea of populating the transfers in the probe 
and using buffers from the driver-data, from these drivers :) Well, I 
admit it ended up being a bit different - but the starting point was 
those drivers ;)

> 
>> + */
>> +
> 
> ...
> 
>> +static int bd79112_get_gpio_pins(const struct iio_chan_spec *cs, int num_channels)
> 
> u32 would make more sense when dealing with bit flags.
> 
>> +{
>> +	int i, gpio_channels;
> 
> same for the local variable.

Meh. Ok :)

> ...
> 
>> +static int bd79112_probe(struct spi_device *spi)
>> +{
> 
> ...
> 
>> +
>> +	data->read_xfer[0].tx_buf = &data->read_tx[0];
>> +	data->read_xfer[0].len = sizeof(data->read_tx);
>> +	data->read_xfer[0].cs_change = 1;
>> +	data->read_xfer[1].rx_buf = &data->read_rx;
>> +	data->read_xfer[1].len = sizeof(data->read_rx);
>> +	spi_message_init_with_transfers(&data->read_msg, data->read_xfer, 2);
>> +
>> +	data->write_xfer.tx_buf = &data->reg_write_tx[0];
>> +	data->write_xfer.len = sizeof(data->reg_write_tx);
>> +	spi_message_init_with_transfers(&data->write_msg, &data->write_xfer, 1);
> 
> If these messages never change (other than the data in the buffers), you can
> call devm_spi_optimize_message() here on each message to get reduced CPU usage
> on every SPI message for free.
> 

Thanks!

Yours,
	-- Matti

Re: [PATCH v4 2/3] iio: adc: Support ROHM BD79112 ADC/GPIO

[Jonathan Cameron]

On Thu, 11 Sep 2025 08:13:03 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Morning Jonathan,
> 
> On 10/09/2025 20:46, Jonathan Cameron wrote:
> > On Wed, 10 Sep 2025 14:24:35 +0300
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >   
> >> The ROHM BD79112 is an ADC/GPIO with 32 channels. The channel inputs can
> >> be used as ADC or GPIO. Using the GPIOs as IRQ sources isn't supported.
> >>
> >> The ADC is 12-bit, supporting input voltages up to 5.7V, and separate I/O
> >> voltage supply. Maximum SPI clock rate is 20 MHz (10 MHz with
> >> daisy-chain configuration) and maximum sampling rate is 1MSPS.
> >>
> >> The IC does also support CRC but it is not implemented in the driver.
> >>
> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>  
> > 
> > Hi Matti,
> > 
> > A few trivial things that I'll tidy up if nothing else comes up (I might not
> > bother given how trivial they are!)  
> 
> Thanks again!
> 
> > Also one question. I couldn't immediately follow why any random register
> > read is sanity checking if an ADC pin is configured as GPIO.
> >   
> 
> Ah. Valid question! I see my comment below is partially wrong.
> 
> 
> >> +/*
> >> + * Read transaction consists of two 16-bit sequences separated by CSB.
> >> + * For register read, 'IOSET' bit must be set. For ADC read, IOSET is cleared
> >> + * and ADDR equals the channel number (0 ... 31).
> >> + *
> >> + * First 16-bit sequence, MOSI as below, MISO data ignored:
> >> + * - SCK: | 1 | 2 |   3   |    4   | 5 .. 8 | 9 .. 16 |
> >> + * - MOSI:| 0 | 0 | IOSET | RW (1) |  ADDR  |  8'b0   |
> >> + *
> >> + * CSB released and re-acquired between these sequences
> >> + *
> >> + * Second 16-bit sequence, MISO as below, MOSI data ignored:
> >> + *   For Register read data is 8 bits:
> >> + *   - SCK: | 1 .. 8 |   9 .. 16   |
> >> + *   - MISO:|  8'b0  | 8-bit data  |
> >> + *
> >> + *   For ADC read data is 12 bits:
> >> + *   - SCK: | 1 .. 4 |   4 .. 16   |
> >> + *   - MISO:|  4'b0  | 12-bit data |  
> 
> This is not 100% true. I overlooked the ADC read "status flag" when 
> adding this comment for the ADC data reading.
> 
> This should be:
> 
>   *   For ADC, read data is 12 bits prepended with a status flag:
>   *   - SCK: | 1 |      2      | 3  4 |   4 .. 16   |
>   *   - MISO:| 0 | STATUS_FLAG | 2'b0 | 12-bit data |
> 
> The 'STATUS_FLAG' is set if the input pin is configured as a GPIO.

That's good additional info, but I'm still struggling on why
we are effectively providing a 'debug' check in ever register
read. My assumption is that it should never fire unless you have
a driver bug?  

Jonathan

Re: [PATCH v4 2/3] iio: adc: Support ROHM BD79112 ADC/GPIO

[Matti Vaittinen]

On 13/09/2025 15:24, Jonathan Cameron wrote:
> On Thu, 11 Sep 2025 08:13:03 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> Morning Jonathan,
>>
>> On 10/09/2025 20:46, Jonathan Cameron wrote:
>>> On Wed, 10 Sep 2025 14:24:35 +0300
>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
>>>    
>>>> The ROHM BD79112 is an ADC/GPIO with 32 channels. The channel inputs can
>>>> be used as ADC or GPIO. Using the GPIOs as IRQ sources isn't supported.
>>>>
>>>> The ADC is 12-bit, supporting input voltages up to 5.7V, and separate I/O
>>>> voltage supply. Maximum SPI clock rate is 20 MHz (10 MHz with
>>>> daisy-chain configuration) and maximum sampling rate is 1MSPS.
>>>>
>>>> The IC does also support CRC but it is not implemented in the driver.
>>>>
>>>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>>>
>>> Hi Matti,
>>>
>>> A few trivial things that I'll tidy up if nothing else comes up (I might not
>>> bother given how trivial they are!)
>>
>> Thanks again!
>>
>>> Also one question. I couldn't immediately follow why any random register
>>> read is sanity checking if an ADC pin is configured as GPIO.
>>>    
>>
>> Ah. Valid question! I see my comment below is partially wrong.
>>
>>
>>>> +/*
>>>> + * Read transaction consists of two 16-bit sequences separated by CSB.
>>>> + * For register read, 'IOSET' bit must be set. For ADC read, IOSET is cleared
>>>> + * and ADDR equals the channel number (0 ... 31).
>>>> + *
>>>> + * First 16-bit sequence, MOSI as below, MISO data ignored:
>>>> + * - SCK: | 1 | 2 |   3   |    4   | 5 .. 8 | 9 .. 16 |
>>>> + * - MOSI:| 0 | 0 | IOSET | RW (1) |  ADDR  |  8'b0   |
>>>> + *
>>>> + * CSB released and re-acquired between these sequences
>>>> + *
>>>> + * Second 16-bit sequence, MISO as below, MOSI data ignored:
>>>> + *   For Register read data is 8 bits:
>>>> + *   - SCK: | 1 .. 8 |   9 .. 16   |
>>>> + *   - MISO:|  8'b0  | 8-bit data  |
>>>> + *
>>>> + *   For ADC read data is 12 bits:
>>>> + *   - SCK: | 1 .. 4 |   4 .. 16   |
>>>> + *   - MISO:|  4'b0  | 12-bit data |
>>
>> This is not 100% true. I overlooked the ADC read "status flag" when
>> adding this comment for the ADC data reading.
>>
>> This should be:
>>
>>    *   For ADC, read data is 12 bits prepended with a status flag:
>>    *   - SCK: | 1 |      2      | 3  4 |   4 .. 16   |
>>    *   - MISO:| 0 | STATUS_FLAG | 2'b0 | 12-bit data |
>>
>> The 'STATUS_FLAG' is set if the input pin is configured as a GPIO.
> 
> That's good additional info, but I'm still struggling on why
> we are effectively providing a 'debug' check in ever register
> read. My assumption is that it should never fire unless you have
> a driver bug?

Yes, a driver bug or someone accessing the ADC outside the driver.

I kind of agree the check shouldn't be needed - but I've seen quite a 
few driver bugs during my career. XD The check is _very_ light weight 
compared to the SPI access time - but you're right that it is done at 
every ADC data read - which is 'hot path'. As a result, I am not sure 
whether to leave or drop it.

Yours,
	-- Matti

> 
> Jonathan

Re: [PATCH v4 2/3] iio: adc: Support ROHM BD79112 ADC/GPIO

[Jonathan Cameron]

On Sun, 14 Sep 2025 12:25:06 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 13/09/2025 15:24, Jonathan Cameron wrote:
> > On Thu, 11 Sep 2025 08:13:03 +0300
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >   
> >> Morning Jonathan,
> >>
> >> On 10/09/2025 20:46, Jonathan Cameron wrote:  
> >>> On Wed, 10 Sep 2025 14:24:35 +0300
> >>> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >>>      
> >>>> The ROHM BD79112 is an ADC/GPIO with 32 channels. The channel inputs can
> >>>> be used as ADC or GPIO. Using the GPIOs as IRQ sources isn't supported.
> >>>>
> >>>> The ADC is 12-bit, supporting input voltages up to 5.7V, and separate I/O
> >>>> voltage supply. Maximum SPI clock rate is 20 MHz (10 MHz with
> >>>> daisy-chain configuration) and maximum sampling rate is 1MSPS.
> >>>>
> >>>> The IC does also support CRC but it is not implemented in the driver.
> >>>>
> >>>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>  
> >>>
> >>> Hi Matti,
> >>>
> >>> A few trivial things that I'll tidy up if nothing else comes up (I might not
> >>> bother given how trivial they are!)  
> >>
> >> Thanks again!
> >>  
> >>> Also one question. I couldn't immediately follow why any random register
> >>> read is sanity checking if an ADC pin is configured as GPIO.
> >>>      
> >>
> >> Ah. Valid question! I see my comment below is partially wrong.
> >>
> >>  
> >>>> +/*
> >>>> + * Read transaction consists of two 16-bit sequences separated by CSB.
> >>>> + * For register read, 'IOSET' bit must be set. For ADC read, IOSET is cleared
> >>>> + * and ADDR equals the channel number (0 ... 31).
> >>>> + *
> >>>> + * First 16-bit sequence, MOSI as below, MISO data ignored:
> >>>> + * - SCK: | 1 | 2 |   3   |    4   | 5 .. 8 | 9 .. 16 |
> >>>> + * - MOSI:| 0 | 0 | IOSET | RW (1) |  ADDR  |  8'b0   |
> >>>> + *
> >>>> + * CSB released and re-acquired between these sequences
> >>>> + *
> >>>> + * Second 16-bit sequence, MISO as below, MOSI data ignored:
> >>>> + *   For Register read data is 8 bits:
> >>>> + *   - SCK: | 1 .. 8 |   9 .. 16   |
> >>>> + *   - MISO:|  8'b0  | 8-bit data  |
> >>>> + *
> >>>> + *   For ADC read data is 12 bits:
> >>>> + *   - SCK: | 1 .. 4 |   4 .. 16   |
> >>>> + *   - MISO:|  4'b0  | 12-bit data |  
> >>
> >> This is not 100% true. I overlooked the ADC read "status flag" when
> >> adding this comment for the ADC data reading.
> >>
> >> This should be:
> >>
> >>    *   For ADC, read data is 12 bits prepended with a status flag:
> >>    *   - SCK: | 1 |      2      | 3  4 |   4 .. 16   |
> >>    *   - MISO:| 0 | STATUS_FLAG | 2'b0 | 12-bit data |
> >>
> >> The 'STATUS_FLAG' is set if the input pin is configured as a GPIO.  
> > 
> > That's good additional info, but I'm still struggling on why
> > we are effectively providing a 'debug' check in ever register
> > read. My assumption is that it should never fire unless you have
> > a driver bug?  
> 
> Yes, a driver bug or someone accessing the ADC outside the driver.
> 
> I kind of agree the check shouldn't be needed - but I've seen quite a 
> few driver bugs during my career. XD The check is _very_ light weight 
> compared to the SPI access time - but you're right that it is done at 
> every ADC data read - which is 'hot path'. As a result, I am not sure 
> whether to leave or drop it.
Maybe just add a comment along the lines of
/* Lets check this whilst here, but should never happen! */

> 
> Yours,
> 	-- Matti
> 
> > 
> > Jonathan  
> 
>