[PATCH 0/4] Add support for the GE HealthCare PMC ADC

[PATCH 0/4] Add support for the GE HealthCare PMC ADC

[Herve Codina]

Hi,

The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
voltage and current, 16-Bit ADC with an I2C Interface.

Best regards,
Herve Codina

Herve Codina (4):
  dt-bindings: vendor-prefixes: Add an entry for GE HealthCare
  dt-bindings: iio: adc: Add the GE HealthCare PMC ADC
  iio: adc: Add support for the GE HealthCare PMC ADC
  MAINTAINERS: add the GE HealthCare PMC ADC driver entry

 .../bindings/iio/adc/gehc,pmc-adc.yaml        |  82 ++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   8 +
 drivers/iio/adc/Kconfig                       |  10 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/gehc-pmc-adc.c                | 233 ++++++++++++++++++
 include/dt-bindings/iio/adc/gehc,pmc-adc.h    |  10 +
 7 files changed, 346 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml
 create mode 100644 drivers/iio/adc/gehc-pmc-adc.c
 create mode 100644 include/dt-bindings/iio/adc/gehc,pmc-adc.h

-- 
2.46.1

[PATCH 1/4] dt-bindings: vendor-prefixes: Add an entry for GE HealthCare

[Herve Codina]

Add the "gehc" entry for GE HealthCare.
https://www.gehealthcare.com

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index b320a39de7fe..15877574a417 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -561,6 +561,8 @@ patternProperties:
     description: GE Fanuc Intelligent Platforms Embedded Systems, Inc.
   "^GEFanuc,.*":
     description: GE Fanuc Intelligent Platforms Embedded Systems, Inc.
+  "^gehc,.*":
+    description: GE HealthCare
   "^gemei,.*":
     description: Gemei Digital Technology Co., Ltd.
   "^gemtek,.*":
-- 
2.46.1

[PATCH 2/4] dt-bindings: iio: adc: Add the GE HealthCare PMC ADC

[Herve Codina]

The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
(voltage and current), 16-Bit ADC with an I2C Interface.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 .../bindings/iio/adc/gehc,pmc-adc.yaml        | 82 +++++++++++++++++++
 include/dt-bindings/iio/adc/gehc,pmc-adc.h    | 10 +++
 2 files changed, 92 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml
 create mode 100644 include/dt-bindings/iio/adc/gehc,pmc-adc.h

diff --git a/Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml b/Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml
new file mode 100644
index 000000000000..6b2bb1309767
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/gehc,pmc-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GE HealthCare PMC Analog to Digital Converter (ADC)
+
+maintainers:
+  - Herve Codina <herve.codina@bootlin.com>
+
+description:
+  The GE HealthCare PMC ADC is a 16-Channel (voltage and current), 16-Bit ADC
+  with an I2C Interface.
+
+properties:
+  compatible:
+    const: gehc,pmc-adc
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description:
+      Regulator for the VDD power supply.
+
+  vdda-supply:
+    description:
+      Regulator for the VDD analog (VDDA) power supply.
+
+  vddio-supply:
+    description:
+      Regulator for the VDD IO (VDDIO) power supply.
+
+  vref-supply:
+    description:
+      Regulator for the voltage reference power supply.
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: osc
+
+  "#io-channel-cells":
+    const: 2
+    description: |
+      The first cell is the channel type (dt-bindings/iio/adc/gehc,pmc-adc.h
+      defines these values):
+       - 0: voltage
+       - 1: current
+      The second cell is the channel number from 0 to 15.
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - vdda-supply
+  - vddio-supply
+  - vref-supply
+  - '#io-channel-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@14 {
+            compatible = "gehc,pmc-adc";
+            reg = <0x14>;
+            vdd-supply = <&reg_vdd>;
+            vdda-supply = <&reg_vdda>;
+            vddio-supply = <&reg_vddio>;
+            vref-supply = <&reg_vref>;
+            #io-channel-cells = <2>;
+        };
+    };
+...
diff --git a/include/dt-bindings/iio/adc/gehc,pmc-adc.h b/include/dt-bindings/iio/adc/gehc,pmc-adc.h
new file mode 100644
index 000000000000..2f291e3c76ae
--- /dev/null
+++ b/include/dt-bindings/iio/adc/gehc,pmc-adc.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+
+#ifndef _DT_BINDINGS_IIO_ADC_GEHC_PMC_ADC_H
+#define _DT_BINDINGS_IIO_ADC_GEHC_PMC_ADC_H
+
+/* ADC channel type */
+#define GEHC_PMC_ADC_VOLTAGE	0
+#define GEHC_PMC_ADC_CURRENT	1
+
+#endif
-- 
2.46.1

[PATCH 3/4] iio: adc: Add support for the GE HealthCare PMC ADC

[Herve Codina]

The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
(voltage and current), 16-Bit ADC with an I2C Interface.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/iio/adc/Kconfig        |  10 ++
 drivers/iio/adc/Makefile       |   1 +
 drivers/iio/adc/gehc-pmc-adc.c | 233 +++++++++++++++++++++++++++++++++
 3 files changed, 244 insertions(+)
 create mode 100644 drivers/iio/adc/gehc-pmc-adc.c

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 97ece1a4b7e3..87b20f972c25 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -571,6 +571,16 @@ config FSL_MX25_ADC
 	  Generic Conversion Queue driver used for general purpose ADC in the
 	  MX25. This driver supports single measurements using the MX25 ADC.
 
+config GEHC_PMC_ADC
+	tristate "GE HealthCare PMC ADC driver"
+	depends on I2C
+	help
+	  Say yes here to build support for the GE HealthCare PMC 16-bit
+	  16-Channel ADC.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called gehc-pmc-adc.
+
 config HI8435
 	tristate "Holt Integrated Circuits HI-8435 threshold detector"
 	select IIO_TRIGGERED_EVENT
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7b91cd98c0e0..66b36dfe9a28 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_ENVELOPE_DETECTOR) += envelope-detector.o
 obj-$(CONFIG_EP93XX_ADC) += ep93xx_adc.o
 obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
 obj-$(CONFIG_FSL_MX25_ADC) += fsl-imx25-gcq.o
+obj-$(CONFIG_GEHC_PMC_ADC) += gehc-pmc-adc.o
 obj-$(CONFIG_HI8435) += hi8435.o
 obj-$(CONFIG_HX711) += hx711.o
 obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o
diff --git a/drivers/iio/adc/gehc-pmc-adc.c b/drivers/iio/adc/gehc-pmc-adc.c
new file mode 100644
index 000000000000..c46c2fb84d35
--- /dev/null
+++ b/drivers/iio/adc/gehc-pmc-adc.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * The GE HealthCare PMC ADC is a 16-Channel (Voltage and current), 16-Bit
+ * ADC with an I2C Interface.
+ *
+ * Copyright (C) 2024, GE HealthCare
+ *
+ * Authors:
+ * Herve Codina <herve.codina@bootlin.com>
+ */
+#include <dt-bindings/iio/adc/gehc,pmc-adc.h>
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+
+struct pmc_adc {
+	struct i2c_client *client;
+};
+
+#define PMC_ADC_CMD_REQUEST_PROTOCOL_VERSION	0x01
+#define PMC_ADC_CMD_READ_VOLTAGE(_ch)		(0x10 | (_ch))
+#define PMC_ADC_CMD_READ_CURRENT(_ch)		(0x20 | (_ch))
+
+#define PMC_ADC_VOLTAGE_CHANNEL(_ch, _ds_name) {			\
+	.type = IIO_VOLTAGE,						\
+	.indexed = 1,							\
+	.channel = (_ch),						\
+	.address = PMC_ADC_CMD_READ_VOLTAGE(_ch),			\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.datasheet_name = (_ds_name),					\
+}
+
+#define PMC_ADC_CURRENT_CHANNEL(_ch, _ds_name) {			\
+	.type = IIO_CURRENT,						\
+	.indexed = 1,							\
+	.channel = (_ch),						\
+	.address = PMC_ADC_CMD_READ_CURRENT(_ch),			\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.datasheet_name = (_ds_name),					\
+}
+
+static const struct iio_chan_spec pmc_adc_channels[] = {
+	PMC_ADC_VOLTAGE_CHANNEL(0, "CH0_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(1, "CH1_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(2, "CH2_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(3, "CH3_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(4, "CH4_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(5, "CH5_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(6, "CH6_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(7, "CH7_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(8, "CH8_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(9, "CH9_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(10, "CH10_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(11, "CH11_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(12, "CH12_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(13, "CH13_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(14, "CH14_V"),
+	PMC_ADC_VOLTAGE_CHANNEL(15, "CH15_V"),
+
+	PMC_ADC_CURRENT_CHANNEL(0, "CH0_I"),
+	PMC_ADC_CURRENT_CHANNEL(1, "CH1_I"),
+	PMC_ADC_CURRENT_CHANNEL(2, "CH2_I"),
+	PMC_ADC_CURRENT_CHANNEL(3, "CH3_I"),
+	PMC_ADC_CURRENT_CHANNEL(4, "CH4_I"),
+	PMC_ADC_CURRENT_CHANNEL(5, "CH5_I"),
+	PMC_ADC_CURRENT_CHANNEL(6, "CH6_I"),
+	PMC_ADC_CURRENT_CHANNEL(7, "CH7_I"),
+	PMC_ADC_CURRENT_CHANNEL(8, "CH8_I"),
+	PMC_ADC_CURRENT_CHANNEL(9, "CH9_I"),
+	PMC_ADC_CURRENT_CHANNEL(10, "CH10_I"),
+	PMC_ADC_CURRENT_CHANNEL(11, "CH11_I"),
+	PMC_ADC_CURRENT_CHANNEL(12, "CH12_I"),
+	PMC_ADC_CURRENT_CHANNEL(13, "CH13_I"),
+	PMC_ADC_CURRENT_CHANNEL(14, "CH14_I"),
+	PMC_ADC_CURRENT_CHANNEL(15, "CH15_I"),
+};
+
+static int pmc_adc_read_raw_ch(struct pmc_adc *pmc_adc, u8 cmd, int *val)
+{
+	s32 ret;
+
+	ret = i2c_smbus_read_word_swapped(pmc_adc->client, cmd);
+	if (ret < 0) {
+		dev_err(&pmc_adc->client->dev, "i2c read word failed (%d)\n", ret);
+		return ret;
+	}
+
+	*val = sign_extend32(ret, 16);
+	return 0;
+}
+
+static int pmc_adc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct pmc_adc *pmc_adc = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = pmc_adc_read_raw_ch(pmc_adc, chan->address, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = 1; /* Raw values are directly read in mV or mA */
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int pmc_adc_fwnode_xlate(struct iio_dev *indio_dev,
+				const struct fwnode_reference_args *iiospec)
+{
+	enum iio_chan_type expected_type;
+	unsigned int i;
+
+	/*
+	 * args[0]: Acquisition type (i.e. voltage or current)
+	 * args[1]: PMC ADC channel number
+	 */
+	if (iiospec->nargs != 2)
+		return -EINVAL;
+
+	switch (iiospec->args[0]) {
+	case GEHC_PMC_ADC_VOLTAGE:
+		expected_type = IIO_VOLTAGE;
+		break;
+	case GEHC_PMC_ADC_CURRENT:
+		expected_type = IIO_CURRENT;
+		break;
+	default:
+		dev_err(&indio_dev->dev, "Invalid channel type %llu\n",
+			iiospec->args[0]);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < indio_dev->num_channels; i++)
+		if (indio_dev->channels[i].type == expected_type &&
+		    indio_dev->channels[i].channel == iiospec->args[1])
+			return i;
+
+	dev_err(&indio_dev->dev, "Invalid channel type %llu number %llu\n",
+		iiospec->args[0], iiospec->args[1]);
+	return -EINVAL;
+}
+
+static const struct iio_info pmc_adc_info = {
+	.read_raw = pmc_adc_read_raw,
+	.fwnode_xlate = pmc_adc_fwnode_xlate,
+};
+
+static const char *const pmc_adc_regulator_names[] = {
+	"vdd",
+	"vdda",
+	"vddio",
+	"vref",
+};
+
+static int pmc_adc_probe(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev;
+	struct pmc_adc *pmc_adc;
+	struct clk *clk;
+	s32 val;
+	int ret;
+
+	ret = devm_regulator_bulk_get_enable(&client->dev, ARRAY_SIZE(pmc_adc_regulator_names),
+					     pmc_adc_regulator_names);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Failed to get regulators\n");
+
+	clk = devm_clk_get_optional_enabled(&client->dev, "osc");
+	if (IS_ERR(clk))
+		return dev_err_probe(&client->dev, PTR_ERR(clk), "Failed to get osc clock\n");
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*pmc_adc));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	pmc_adc = iio_priv(indio_dev);
+	pmc_adc->client = client;
+
+	val = i2c_smbus_read_byte_data(pmc_adc->client, PMC_ADC_CMD_REQUEST_PROTOCOL_VERSION);
+	if (val < 0)
+		return dev_err_probe(&client->dev, val, "Failed to get protocol version\n");
+
+	if (val != 0x01) {
+		dev_err(&client->dev, "Unsupported protocol version 0x%02x\n", val);
+		return -EINVAL;
+	}
+
+	indio_dev->name = "pmc_adc";
+	indio_dev->info = &pmc_adc_info;
+	indio_dev->channels = pmc_adc_channels;
+	indio_dev->num_channels = ARRAY_SIZE(pmc_adc_channels);
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct of_device_id pmc_adc_of_match[] = {
+	{ .compatible = "gehc,pmc-adc"},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pmc_adc_of_match);
+
+static const struct i2c_device_id pmc_adc_id_table[] = {
+	{ "pmc-adc" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pmc_adc_id_table);
+
+static struct i2c_driver pmc_adc_i2c_driver = {
+	.driver  = {
+		.name = "pmc-adc",
+		.of_match_table = pmc_adc_of_match,
+	},
+	.id_table = pmc_adc_id_table,
+	.probe  = pmc_adc_probe,
+};
+
+module_i2c_driver(pmc_adc_i2c_driver);
+
+MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>");
+MODULE_DESCRIPTION("GE HealthCare PMC ADC driver");
+MODULE_LICENSE("GPL");
-- 
2.46.1

[PATCH 4/4] MAINTAINERS: add the GE HealthCare PMC ADC driver entry

[Herve Codina]

After contributing the driver, add myself as the maintainer for the
GE HealthCare PCM ADC IIO driver.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bd288a97c770..6cccbe4e3d72 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9459,6 +9459,14 @@ M:	Kieran Bingham <kbingham@kernel.org>
 S:	Supported
 F:	scripts/gdb/
 
+GE HEALTHCARE PMC ADC DRIVER
+M:	Herve Codina <herve.codina@bootlin.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml
+F:	drivers/iio/adc/gehc-pmc-adc.c
+F:	include/dt-bindings/iio/adc/gehc,pmc-adc.h
+
 GEMINI CRYPTO DRIVER
 M:	Corentin Labbe <clabbe@baylibre.com>
 L:	linux-crypto@vger.kernel.org
-- 
2.46.1

Re: [PATCH 2/4] dt-bindings: iio: adc: Add the GE HealthCare PMC ADC

[Conor Dooley]

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

On Tue, Oct 01, 2024 at 09:46:16AM +0200, Herve Codina wrote:
> The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> (voltage and current), 16-Bit ADC with an I2C Interface.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  .../bindings/iio/adc/gehc,pmc-adc.yaml        | 82 +++++++++++++++++++
>  include/dt-bindings/iio/adc/gehc,pmc-adc.h    | 10 +++
>  2 files changed, 92 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml
>  create mode 100644 include/dt-bindings/iio/adc/gehc,pmc-adc.h
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml b/Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml
> new file mode 100644
> index 000000000000..6b2bb1309767
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/gehc,pmc-adc.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/gehc,pmc-adc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GE HealthCare PMC Analog to Digital Converter (ADC)
> +
> +maintainers:
> +  - Herve Codina <herve.codina@bootlin.com>
> +
> +description:
> +  The GE HealthCare PMC ADC is a 16-Channel (voltage and current), 16-Bit ADC
> +  with an I2C Interface.
> +
> +properties:
> +  compatible:
> +    const: gehc,pmc-adc
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description:
> +      Regulator for the VDD power supply.
> +
> +  vdda-supply:
> +    description:
> +      Regulator for the VDD analog (VDDA) power supply.
> +
> +  vddio-supply:
> +    description:
> +      Regulator for the VDD IO (VDDIO) power supply.
> +
> +  vref-supply:
> +    description:
> +      Regulator for the voltage reference power supply.
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: osc

Since there's no datasheet for me to look up, why is the clock optional?

> +
> +  "#io-channel-cells":
> +    const: 2
> +    description: |
> +      The first cell is the channel type (dt-bindings/iio/adc/gehc,pmc-adc.h
> +      defines these values):
> +       - 0: voltage
> +       - 1: current
> +      The second cell is the channel number from 0 to 15.
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +  - vdda-supply
> +  - vddio-supply
> +  - vref-supply
> +  - '#io-channel-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@14 {
> +            compatible = "gehc,pmc-adc";
> +            reg = <0x14>;
> +            vdd-supply = <&reg_vdd>;
> +            vdda-supply = <&reg_vdda>;
> +            vddio-supply = <&reg_vddio>;
> +            vref-supply = <&reg_vref>;
> +            #io-channel-cells = <2>;
> +        };
> +    };
> +...
> diff --git a/include/dt-bindings/iio/adc/gehc,pmc-adc.h b/include/dt-bindings/iio/adc/gehc,pmc-adc.h
> new file mode 100644
> index 000000000000..2f291e3c76ae
> --- /dev/null
> +++ b/include/dt-bindings/iio/adc/gehc,pmc-adc.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +
> +#ifndef _DT_BINDINGS_IIO_ADC_GEHC_PMC_ADC_H
> +#define _DT_BINDINGS_IIO_ADC_GEHC_PMC_ADC_H
> +
> +/* ADC channel type */
> +#define GEHC_PMC_ADC_VOLTAGE	0
> +#define GEHC_PMC_ADC_CURRENT	1
> +
> +#endif
> -- 
> 2.46.1
> 

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

Re: [PATCH 1/4] dt-bindings: vendor-prefixes: Add an entry for GE HealthCare

[Conor Dooley]

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

On Tue, Oct 01, 2024 at 09:46:15AM +0200, Herve Codina wrote:
> Add the "gehc" entry for GE HealthCare.
> https://www.gehealthcare.com
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

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

Re: [PATCH 2/4] dt-bindings: iio: adc: Add the GE HealthCare PMC ADC

[Herve Codina]

On Tue, 1 Oct 2024 17:18:30 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Tue, Oct 01, 2024 at 09:46:16AM +0200, Herve Codina wrote:
> > The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> > (voltage and current), 16-Bit ADC with an I2C Interface.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---
> >  .../bindings/iio/adc/gehc,pmc-adc.yaml        | 82 +++++++++++++++++++
> >  include/dt-bindings/iio/adc/gehc,pmc-adc.h    | 10 +++
...
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: osc  
> 
> Since there's no datasheet for me to look up, why is the clock optional?

The component uses an external oscillator (osc) if an external oscillator is
connected to its clock pins. Otherwise, it uses an internal reference clock.

Best regards,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/4] dt-bindings: iio: adc: Add the GE HealthCare PMC ADC

[Conor Dooley]

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

On Tue, Oct 01, 2024 at 06:46:02PM +0200, Herve Codina wrote:
> On Tue, 1 Oct 2024 17:18:30 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Tue, Oct 01, 2024 at 09:46:16AM +0200, Herve Codina wrote:
> > > The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> > > (voltage and current), 16-Bit ADC with an I2C Interface.
> > > 
> > > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > > ---
> > >  .../bindings/iio/adc/gehc,pmc-adc.yaml        | 82 +++++++++++++++++++
> > >  include/dt-bindings/iio/adc/gehc,pmc-adc.h    | 10 +++
> ...
> > > +  clocks:
> > > +    maxItems: 1
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: osc  
> > 
> > Since there's no datasheet for me to look up, why is the clock optional?
> 
> The component uses an external oscillator (osc) if an external oscillator is
> connected to its clock pins. Otherwise, it uses an internal reference clock.

Could you please add a description of the clock with this statement,
given there's no datasheet or w/e linked here?

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

Re: [PATCH 3/4] iio: adc: Add support for the GE HealthCare PMC ADC

[David Lechner]

On Tue, Oct 1, 2024 at 2:47 AM Herve Codina <herve.codina@bootlin.com> wrote:
>
> The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> (voltage and current), 16-Bit ADC with an I2C Interface.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---

...

> +static int pmc_adc_read_raw_ch(struct pmc_adc *pmc_adc, u8 cmd, int *val)
> +{
> +       s32 ret;
> +
> +       ret = i2c_smbus_read_word_swapped(pmc_adc->client, cmd);
> +       if (ret < 0) {
> +               dev_err(&pmc_adc->client->dev, "i2c read word failed (%d)\n", ret);
> +               return ret;
> +       }
> +
> +       *val = sign_extend32(ret, 16);

Shouldn't this be 15, not 16?

> +       return 0;
> +}
> +

...

> +
> +static int pmc_adc_probe(struct i2c_client *client)
> +{
> +       struct iio_dev *indio_dev;
> +       struct pmc_adc *pmc_adc;
> +       struct clk *clk;
> +       s32 val;
> +       int ret;
> +
> +       ret = devm_regulator_bulk_get_enable(&client->dev, ARRAY_SIZE(pmc_adc_regulator_names),
> +                                            pmc_adc_regulator_names);
> +       if (ret)
> +               return dev_err_probe(&client->dev, ret, "Failed to get regulators\n");
> +
> +       clk = devm_clk_get_optional_enabled(&client->dev, "osc");
> +       if (IS_ERR(clk))
> +               return dev_err_probe(&client->dev, PTR_ERR(clk), "Failed to get osc clock\n");
> +
> +       indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*pmc_adc));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       pmc_adc = iio_priv(indio_dev);
> +       pmc_adc->client = client;
> +
> +       val = i2c_smbus_read_byte_data(pmc_adc->client, PMC_ADC_CMD_REQUEST_PROTOCOL_VERSION);
> +       if (val < 0)
> +               return dev_err_probe(&client->dev, val, "Failed to get protocol version\n");
> +
> +       if (val != 0x01) {
> +               dev_err(&client->dev, "Unsupported protocol version 0x%02x\n", val);

Use dev_err_probe?

> +               return -EINVAL;
> +       }
> +
> +       indio_dev->name = "pmc_adc";
> +       indio_dev->info = &pmc_adc_info;
> +       indio_dev->channels = pmc_adc_channels;
> +       indio_dev->num_channels = ARRAY_SIZE(pmc_adc_channels);
> +
> +       return devm_iio_device_register(&client->dev, indio_dev);
> +}

Re: [PATCH 3/4] iio: adc: Add support for the GE HealthCare PMC ADC

[Jonathan Cameron]

On Tue,  1 Oct 2024 09:46:17 +0200
Herve Codina <herve.codina@bootlin.com> wrote:

> The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> (voltage and current), 16-Bit ADC with an I2C Interface.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Just one thing to add to David's review.

I'm going to guess this isn't a general purpose ADC? Can you share any info
on what sort of device it is used in?

No problem if not - I'm just curious as I've not seen GE HealthCare I2C parts
before!

> diff --git a/drivers/iio/adc/gehc-pmc-adc.c b/drivers/iio/adc/gehc-pmc-adc.c
> new file mode 100644
> index 000000000000..c46c2fb84d35
> --- /dev/null
> +++ b/drivers/iio/adc/gehc-pmc-adc.c
> @@ -0,0 +1,233 @@


> +
> +static int pmc_adc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct pmc_adc *pmc_adc = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = pmc_adc_read_raw_ch(pmc_adc, chan->address, val);
> +		if (ret)
> +			return ret;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 1; /* Raw values are directly read in mV or mA */

Drop this scale and make the channels processed. That saves userspace even applying
the *1 this indicates.  Rare to find a device that outputs in our base units
but might as well take advantage of one that does :)

> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}

Re: [PATCH 2/4] dt-bindings: iio: adc: Add the GE HealthCare PMC ADC

[Herve Codina]

Hi Conor,

On Tue, 1 Oct 2024 18:03:37 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Tue, Oct 01, 2024 at 06:46:02PM +0200, Herve Codina wrote:
> > On Tue, 1 Oct 2024 17:18:30 +0100
> > Conor Dooley <conor@kernel.org> wrote:
> >   
> > > On Tue, Oct 01, 2024 at 09:46:16AM +0200, Herve Codina wrote:  
> > > > The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> > > > (voltage and current), 16-Bit ADC with an I2C Interface.
> > > > 
> > > > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > > > ---
> > > >  .../bindings/iio/adc/gehc,pmc-adc.yaml        | 82 +++++++++++++++++++
> > > >  include/dt-bindings/iio/adc/gehc,pmc-adc.h    | 10 +++  
> > ...  
> > > > +  clocks:
> > > > +    maxItems: 1
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: osc    
> > > 
> > > Since there's no datasheet for me to look up, why is the clock optional?  
> > 
> > The component uses an external oscillator (osc) if an external oscillator is
> > connected to its clock pins. Otherwise, it uses an internal reference clock.  
> 
> Could you please add a description of the clock with this statement,
> given there's no datasheet or w/e linked here?

Sure, I will add a description in the next iteration.

There is no datasheet or any similar document available for this component.
This is reason why I cannot add a link to this kind of dos.

Best regards,
Hervé

Re: [PATCH 3/4] iio: adc: Add support for the GE HealthCare PMC ADC

[Herve Codina]

Hi David,

On Tue, 1 Oct 2024 12:19:16 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On Tue, Oct 1, 2024 at 2:47 AM Herve Codina <herve.codina@bootlin.com> wrote:
> >
> > The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> > (voltage and current), 16-Bit ADC with an I2C Interface.
> >
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > ---  
> 
> ...
> 
> > +static int pmc_adc_read_raw_ch(struct pmc_adc *pmc_adc, u8 cmd, int *val)
> > +{
> > +       s32 ret;
> > +
> > +       ret = i2c_smbus_read_word_swapped(pmc_adc->client, cmd);
> > +       if (ret < 0) {
> > +               dev_err(&pmc_adc->client->dev, "i2c read word failed (%d)\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       *val = sign_extend32(ret, 16);  
> 
> Shouldn't this be 15, not 16?

Yes, indeed, I double checked on my side and it should be 15!
Thanks for catching it. It will be fixed in the next iteration.

> 
> > +       return 0;
> > +}
> > +  
> 
> ...
> 
> > +
> > +static int pmc_adc_probe(struct i2c_client *client)
> > +{
> > +       struct iio_dev *indio_dev;
> > +       struct pmc_adc *pmc_adc;
> > +       struct clk *clk;
> > +       s32 val;
> > +       int ret;
> > +
> > +       ret = devm_regulator_bulk_get_enable(&client->dev, ARRAY_SIZE(pmc_adc_regulator_names),
> > +                                            pmc_adc_regulator_names);
> > +       if (ret)
> > +               return dev_err_probe(&client->dev, ret, "Failed to get regulators\n");
> > +
> > +       clk = devm_clk_get_optional_enabled(&client->dev, "osc");
> > +       if (IS_ERR(clk))
> > +               return dev_err_probe(&client->dev, PTR_ERR(clk), "Failed to get osc clock\n");
> > +
> > +       indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*pmc_adc));
> > +       if (!indio_dev)
> > +               return -ENOMEM;
> > +
> > +       pmc_adc = iio_priv(indio_dev);
> > +       pmc_adc->client = client;
> > +
> > +       val = i2c_smbus_read_byte_data(pmc_adc->client, PMC_ADC_CMD_REQUEST_PROTOCOL_VERSION);
> > +       if (val < 0)
> > +               return dev_err_probe(&client->dev, val, "Failed to get protocol version\n");
> > +
> > +       if (val != 0x01) {
> > +               dev_err(&client->dev, "Unsupported protocol version 0x%02x\n", val);  
> 
> Use dev_err_probe?

Yes, will be changed in the next iteration.

> 
> > +               return -EINVAL;
> > +       }
> > +

Best regards,
Hervé

Re: [PATCH 3/4] iio: adc: Add support for the GE HealthCare PMC ADC

[Herve Codina]

Hi Jonathan,

On Tue, 1 Oct 2024 20:24:30 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue,  1 Oct 2024 09:46:17 +0200
> Herve Codina <herve.codina@bootlin.com> wrote:
> 
> > The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> > (voltage and current), 16-Bit ADC with an I2C Interface.
> > 
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> 
> Just one thing to add to David's review.
> 
> I'm going to guess this isn't a general purpose ADC? Can you share any info
> on what sort of device it is used in?
> 
> No problem if not - I'm just curious as I've not seen GE HealthCare I2C parts
> before!

I cannot tell about the product it is used in :(
One sure thing I can say is that the component itself is not available off
the shelf, and is fully designed to be used in a specific product.

> 
> > diff --git a/drivers/iio/adc/gehc-pmc-adc.c b/drivers/iio/adc/gehc-pmc-adc.c
> > new file mode 100644
> > index 000000000000..c46c2fb84d35
> > --- /dev/null
> > +++ b/drivers/iio/adc/gehc-pmc-adc.c
> > @@ -0,0 +1,233 @@  
> 
> 
> > +
> > +static int pmc_adc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> > +			    int *val, int *val2, long mask)
> > +{
> > +	struct pmc_adc *pmc_adc = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		ret = pmc_adc_read_raw_ch(pmc_adc, chan->address, val);
> > +		if (ret)
> > +			return ret;
> > +		return IIO_VAL_INT;
> > +
> > +	case IIO_CHAN_INFO_SCALE:
> > +		*val = 1; /* Raw values are directly read in mV or mA */  
> 
> Drop this scale and make the channels processed. That saves userspace even applying

I thought that scale was mandatory.

From the userspace, offset is clearly optional
  https://elixir.bootlin.com/linux/v6.11/source/Documentation/ABI/testing/sysfs-bus-iio#L458
But nothing about a default value is mentioned in the scale description
  https://elixir.bootlin.com/linux/v6.11/source/Documentation/ABI/testing/sysfs-bus-iio#L515

> the *1 this indicates.  Rare to find a device that outputs in our base units
> but might as well take advantage of one that does :)

Yes, the device is a custom designed device and it has a fully knowledge (by
design) of the board it is soldered on. As I was involved in the communication
protocol definition, units were chosen to fit well with IIO.

> 
> > +		return IIO_VAL_INT;
> > +	}
> > +
> > +	return -EINVAL;
> > +}  

Best regards,
Hervé

Re: [PATCH 2/4] dt-bindings: iio: adc: Add the GE HealthCare PMC ADC

[Conor Dooley]

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

On Wed, Oct 02, 2024 at 09:04:53AM +0200, Herve Codina wrote:
> Hi Conor,
> 
> On Tue, 1 Oct 2024 18:03:37 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Tue, Oct 01, 2024 at 06:46:02PM +0200, Herve Codina wrote:
> > > On Tue, 1 Oct 2024 17:18:30 +0100
> > > Conor Dooley <conor@kernel.org> wrote:
> > >   
> > > > On Tue, Oct 01, 2024 at 09:46:16AM +0200, Herve Codina wrote:  
> > > > > The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> > > > > (voltage and current), 16-Bit ADC with an I2C Interface.
> > > > > 
> > > > > Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> > > > > ---
> > > > >  .../bindings/iio/adc/gehc,pmc-adc.yaml        | 82 +++++++++++++++++++
> > > > >  include/dt-bindings/iio/adc/gehc,pmc-adc.h    | 10 +++  
> > > ...  
> > > > > +  clocks:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  clock-names:
> > > > > +    items:
> > > > > +      - const: osc    
> > > > 
> > > > Since there's no datasheet for me to look up, why is the clock optional?  
> > > 
> > > The component uses an external oscillator (osc) if an external oscillator is
> > > connected to its clock pins. Otherwise, it uses an internal reference clock.  
> > 
> > Could you please add a description of the clock with this statement,
> > given there's no datasheet or w/e linked here?
> 
> Sure, I will add a description in the next iteration.
> 
> There is no datasheet or any similar document available for this component.
> This is reason why I cannot add a link to this kind of dos.

Yeah, I had taken a look last night and couldn't find one, so I figured
that this was the case.

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

Re: [PATCH 3/4] iio: adc: Add support for the GE HealthCare PMC ADC

[Jonathan Cameron]

On Wed, 2 Oct 2024 10:23:24 +0200
Herve Codina <herve.codina@bootlin.com> wrote:

> Hi Jonathan,
> 
> On Tue, 1 Oct 2024 20:24:30 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
> 
> > On Tue,  1 Oct 2024 09:46:17 +0200
> > Herve Codina <herve.codina@bootlin.com> wrote:
> >   
> > > The GE HealthCare PMC Analog to Digital Converter (ADC) is a 16-Channel
> > > (voltage and current), 16-Bit ADC with an I2C Interface.
> > > 
> > > Signed-off-by: Herve Codina <herve.codina@bootlin.com>    
> > 
> > Just one thing to add to David's review.
> > 
> > I'm going to guess this isn't a general purpose ADC? Can you share any info
> > on what sort of device it is used in?
> > 
> > No problem if not - I'm just curious as I've not seen GE HealthCare I2C parts
> > before!  
> 
> I cannot tell about the product it is used in :(
> One sure thing I can say is that the component itself is not available off
> the shelf, and is fully designed to be used in a specific product.
> 
> >   
> > > diff --git a/drivers/iio/adc/gehc-pmc-adc.c b/drivers/iio/adc/gehc-pmc-adc.c
> > > new file mode 100644
> > > index 000000000000..c46c2fb84d35
> > > --- /dev/null
> > > +++ b/drivers/iio/adc/gehc-pmc-adc.c
> > > @@ -0,0 +1,233 @@    
> > 
> >   
> > > +
> > > +static int pmc_adc_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> > > +			    int *val, int *val2, long mask)
> > > +{
> > > +	struct pmc_adc *pmc_adc = iio_priv(indio_dev);
> > > +	int ret;
> > > +
> > > +	switch (mask) {
> > > +	case IIO_CHAN_INFO_RAW:
> > > +		ret = pmc_adc_read_raw_ch(pmc_adc, chan->address, val);
> > > +		if (ret)
> > > +			return ret;
> > > +		return IIO_VAL_INT;
> > > +
> > > +	case IIO_CHAN_INFO_SCALE:
> > > +		*val = 1; /* Raw values are directly read in mV or mA */    
> > 
> > Drop this scale and make the channels processed. That saves userspace even applying  
> 
> I thought that scale was mandatory.
> 
> From the userspace, offset is clearly optional
>   https://elixir.bootlin.com/linux/v6.11/source/Documentation/ABI/testing/sysfs-bus-iio#L458
> But nothing about a default value is mentioned in the scale description
>   https://elixir.bootlin.com/linux/v6.11/source/Documentation/ABI/testing/sysfs-bus-iio#L515
It doesn't get applied to _PROCESSED attributes (see ABI for _input attributes which
simply doesn't say to apply anything.

There is a corner case where _OFFSET is provided but not _SCALE and hence the channel
is _RAW.  In that case I'd say _SCALE is optional, but I'm not sure we've ever seen
it in reality!  The more common case (though still rare) is like this one where 
the reading is in the base units of the ABI so _input is the away to go.
This is fairly ancient ABI lifted from hwmon.


> 
> > the *1 this indicates.  Rare to find a device that outputs in our base units
> > but might as well take advantage of one that does :)  
> 
> Yes, the device is a custom designed device and it has a fully knowledge (by
> design) of the board it is soldered on. As I was involved in the communication
> protocol definition, units were chosen to fit well with IIO.

Nice :)
> 
> >   
> > > +		return IIO_VAL_INT;
> > > +	}
> > > +
> > > +	return -EINVAL;
> > > +}    
> 
> Best regards,
> Hervé