[PATCH 0/3] Add the Renesas X9250 potentiometers IIO support

[PATCH 0/3] Add the Renesas X9250 potentiometers IIO support

[Herve Codina]

Hi,

The Renesas X9250 integrated four digitally controlled potentiometers.
On each potentiometer, the X9250T has a 100 kOhms total resistance and
the X9250U has a 50 kOhms total resistance.

Best regards,
Herve Codina

Herve Codina (3):
  dt-bindings: iio: potentiometer: Add the Renesas X9250 potentiometers
  iio: potentiometer: Add support for the Renesas X9250 potentiometers
  MAINTAINERS: add the Renesas X9250 driver entry

 .../iio/potentiometer/renesas,x9250.yaml      |  56 +++++
 MAINTAINERS                                   |   7 +
 drivers/iio/potentiometer/Kconfig             |  10 +
 drivers/iio/potentiometer/Makefile            |   1 +
 drivers/iio/potentiometer/x9250.c             | 233 ++++++++++++++++++
 5 files changed, 307 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.yaml
 create mode 100644 drivers/iio/potentiometer/x9250.c

-- 
2.39.2

[PATCH 1/3] dt-bindings: iio: potentiometer: Add the Renesas X9250 potentiometers

[Herve Codina]

The Renesas X9250 is a quad digitally controlled potentiometers.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 .../iio/potentiometer/renesas,x9250.yaml      | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.yaml

diff --git a/Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.yaml b/Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.yaml
new file mode 100644
index 000000000000..e0433cd522aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/potentiometer/renesas,x9250.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas X9250 quad potentiometers
+
+maintainers:
+  - Herve Codina <herve.codina@bootlin.com>
+
+description:
+  The Renesas X9250 integrates four digitally controlled potentiometers.
+  On each potentiometer, the X9250T has a 100 kOhms total resistance and the
+  X9250U has a 50 kOhms total resistance.
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml
+  - $ref: /schemas/iio/iio.yaml
+
+properties:
+  compatible:
+    enum:
+      - renesas,x9250t
+      - renesas,x9250u
+
+  reg:
+    description:
+      SPI device address.
+    maxItems: 1
+
+  '#io-channel-cells':
+    const: 1
+
+  spi-max-frequency:
+    maximum: 2000000
+
+required:
+  - compatible
+  - reg
+  - '#io-channel-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        x9250@0 {
+            compatible = "renesas,x9250t";
+            reg = <0>;
+            spi-max-frequency = <2000000>;
+            #io-channel-cells = <1>;
+        };
+    };
-- 
2.39.2

Re: [PATCH 1/3] dt-bindings: iio: potentiometer: Add the Renesas X9250 potentiometers

[Krzysztof Kozlowski]

On 20/04/2023 14:13, Herve Codina wrote:
> The Renesas X9250 is a quad digitally controlled potentiometers.
> 

Thank you for your patch. There is something to discuss/improve.

> +maintainers:
> +  - Herve Codina <herve.codina@bootlin.com>
> +
> +description:
> +  The Renesas X9250 integrates four digitally controlled potentiometers.
> +  On each potentiometer, the X9250T has a 100 kOhms total resistance and the
> +  X9250U has a 50 kOhms total resistance.
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml
> +  - $ref: /schemas/iio/iio.yaml
> +
> +properties:
> +  compatible:
> +    enum:
> +      - renesas,x9250t
> +      - renesas,x9250u
> +
> +  reg:
> +    description:
> +      SPI device address.

SPI bus does not have device addresses, AFAIR. Drop description.

> +    maxItems: 1
> +
> +  '#io-channel-cells':
> +    const: 1
> +
> +  spi-max-frequency:
> +    maximum: 2000000
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#io-channel-cells'
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        x9250@0 {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +            compatible = "renesas,x9250t";
> +            reg = <0>;
> +            spi-max-frequency = <2000000>;
> +            #io-channel-cells = <1>;
> +        };
> +    };

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: iio: potentiometer: Add the Renesas X9250 potentiometers

[Herve Codina]

Hi Krzysztof,

On Fri, 21 Apr 2023 09:32:57 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 20/04/2023 14:13, Herve Codina wrote:
> > The Renesas X9250 is a quad digitally controlled potentiometers.
> >   
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> > +maintainers:
> > +  - Herve Codina <herve.codina@bootlin.com>
> > +
> > +description:
> > +  The Renesas X9250 integrates four digitally controlled potentiometers.
> > +  On each potentiometer, the X9250T has a 100 kOhms total resistance and the
> > +  X9250U has a 50 kOhms total resistance.
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml
> > +  - $ref: /schemas/iio/iio.yaml
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,x9250t
> > +      - renesas,x9250u
> > +
> > +  reg:
> > +    description:
> > +      SPI device address.  
> 
> SPI bus does not have device addresses, AFAIR. Drop description.

Indeed, SPI has chip-select.
I will drop the description

> 
> > +    maxItems: 1
> > +
> > +  '#io-channel-cells':
> > +    const: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 2000000
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#io-channel-cells'
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    spi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        x9250@0 {  
> 
> Node names should be generic.
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Will be change to potentiometer@0

> 
> > +            compatible = "renesas,x9250t";
> > +            reg = <0>;
> > +            spi-max-frequency = <2000000>;
> > +            #io-channel-cells = <1>;
> > +        };
> > +    };  
> 
> Best regards,
> Krzysztof
> 

The modifications will be present on v3 as v2 was already sent.

Thanks for the review,
Hervé

[PATCH 2/3] iio: potentiometer: Add support for the Renesas X9250 potentiometers

[Herve Codina]

The Renesas X9250 integrates four digitally controlled potentiometers.
On each potentiometer, the X9250T has a 100 kOhms total resistance and
the X9250U has a 50 kOhms total resistance.

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

diff --git a/drivers/iio/potentiometer/Kconfig b/drivers/iio/potentiometer/Kconfig
index 01dd3f858d99..e6a9a3c67845 100644
--- a/drivers/iio/potentiometer/Kconfig
+++ b/drivers/iio/potentiometer/Kconfig
@@ -136,4 +136,14 @@ config TPL0102
 	  To compile this driver as a module, choose M here: the
 	  module will be called tpl0102.
 
+config X9250
+	tristate "Renesas X9250 quad controlled potentiometers"
+	depends on SPI
+	help
+	  Enable support for the Renesas X9250 quad controlled
+	  potentiometers.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called x9250.
+
 endmenu
diff --git a/drivers/iio/potentiometer/Makefile b/drivers/iio/potentiometer/Makefile
index 5ebb8e3bbd76..d11fb739176c 100644
--- a/drivers/iio/potentiometer/Makefile
+++ b/drivers/iio/potentiometer/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_MCP4131) += mcp4131.o
 obj-$(CONFIG_MCP4531) += mcp4531.o
 obj-$(CONFIG_MCP41010) += mcp41010.o
 obj-$(CONFIG_TPL0102) += tpl0102.o
+obj-$(CONFIG_X9250) += x9250.o
diff --git a/drivers/iio/potentiometer/x9250.c b/drivers/iio/potentiometer/x9250.c
new file mode 100644
index 000000000000..9d3831f36054
--- /dev/null
+++ b/drivers/iio/potentiometer/x9250.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *
+ * x9250.c  --  Renesas X9250 potentiometers IIO driver
+ *
+ * Copyright 2023 CS GROUP France
+ *
+ * Author: Herve Codina <herve.codina@bootlin.com>
+ */
+
+#include <linux/iio/iio.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+
+struct x9250_cfg {
+	int kohms;
+};
+
+struct x9250 {
+	struct spi_device *spi;
+	const struct x9250_cfg *cfg;
+	struct mutex lock; /* Protect tx and rx buffers */
+	/* Cannot use stack area for SPI (dma-safe memory) */
+	u8 spi_tx_buf[3] __aligned(IIO_DMA_MINALIGN);
+	u8 spi_rx_buf[3] __aligned(IIO_DMA_MINALIGN);
+};
+
+#define X9250_CMD_RD_WCR(_p)    (0x90 | (_p))
+#define X9250_CMD_WR_WCR(_p)    (0xa0 | (_p))
+
+static int x9250_write8(struct x9250 *x9250, u8 cmd, u8 val)
+{
+	struct spi_transfer xfer = {
+		.tx_buf = &x9250->spi_tx_buf,
+		.len = 3,
+	};
+	int ret;
+
+	BUILD_BUG_ON(sizeof(x9250->spi_tx_buf) < 3);
+
+	mutex_lock(&x9250->lock);
+
+	x9250->spi_tx_buf[0] = 0x50;
+	x9250->spi_tx_buf[1] = cmd;
+	x9250->spi_tx_buf[2] = val;
+
+	ret = spi_sync_transfer(x9250->spi, &xfer, 1);
+
+	mutex_unlock(&x9250->lock);
+
+	return ret;
+}
+
+static int x9250_read8(struct x9250 *x9250, u8 cmd, u8 *val)
+{
+	struct spi_transfer xfer = {
+		.tx_buf = &x9250->spi_tx_buf,
+		.rx_buf = &x9250->spi_rx_buf,
+		.len = 3,
+	};
+	int ret;
+
+	BUILD_BUG_ON(sizeof(x9250->spi_tx_buf) < 3);
+	BUILD_BUG_ON(sizeof(x9250->spi_rx_buf) < 3);
+
+	mutex_lock(&x9250->lock);
+
+	x9250->spi_tx_buf[0] = 0x50;
+	x9250->spi_tx_buf[1] = cmd;
+
+	ret = spi_sync_transfer(x9250->spi, &xfer, 1);
+	if (ret)
+		goto end;
+
+	*val = x9250->spi_rx_buf[2];
+	ret = 0;
+end:
+	mutex_unlock(&x9250->lock);
+	return ret;
+}
+
+#define X9250_CHANNEL(ch) {						\
+	.type = IIO_RESISTANCE,						\
+	.indexed = 1,							\
+	.output = 1,							\
+	.channel = (ch),						\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_RAW),	\
+}
+
+static const struct iio_chan_spec x9250_channels[] = {
+	X9250_CHANNEL(0),
+	X9250_CHANNEL(1),
+	X9250_CHANNEL(2),
+	X9250_CHANNEL(3),
+};
+
+static int x9250_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			  int *val, int *val2, long mask)
+{
+	struct x9250 *x9250 = iio_priv(indio_dev);
+	int ch = chan->channel;
+	int ret;
+	u8 v;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = x9250_read8(x9250, X9250_CMD_RD_WCR(ch), &v);
+		if (ret)
+			return ret;
+		*val = v;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = 1000 * x9250->cfg->kohms;
+		*val2 = U8_MAX;
+		return IIO_VAL_FRACTIONAL;
+	}
+
+	return -EINVAL;
+}
+
+static int x9250_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			    const int **vals, int *type, int *length, long mask)
+{
+	static const int range[] = {0, 1, 255}; /* min, step, max */
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		*length = ARRAY_SIZE(range);
+		*vals = range;
+		*type = IIO_VAL_INT;
+		return IIO_AVAIL_RANGE;
+	}
+
+	return -EINVAL;
+}
+
+static int x9250_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			   int val, int val2, long mask)
+{
+	struct x9250 *x9250 = iio_priv(indio_dev);
+	int ch = chan->channel;
+
+	if (mask != IIO_CHAN_INFO_RAW)
+		return -EINVAL;
+
+	if (val > U8_MAX || val < 0)
+		return -EINVAL;
+
+	return x9250_write8(x9250, X9250_CMD_WR_WCR(ch), val);
+}
+
+static const struct iio_info x9250_info = {
+	.read_raw = x9250_read_raw,
+	.read_avail = x9250_read_avail,
+	.write_raw = x9250_write_raw,
+};
+
+enum x9250_type {
+	X9250T,
+	X9250U,
+};
+
+static const struct x9250_cfg x9250_cfg[] = {
+	[X9250T] = { .kohms =  100, },
+	[X9250U] = { .kohms =  50, },
+};
+
+static int x9250_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct x9250 *x9250;
+	int ret;
+
+	spi->bits_per_word = 8;
+	ret = spi_setup(spi);
+	if (ret < 0)
+		return ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*x9250));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	x9250 = iio_priv(indio_dev);
+	x9250->spi = spi;
+	x9250->cfg = device_get_match_data(&spi->dev);
+	if (!x9250->cfg)
+		x9250->cfg = &x9250_cfg[spi_get_device_id(spi)->driver_data];
+
+	mutex_init(&x9250->lock);
+
+	indio_dev->info = &x9250_info;
+	indio_dev->channels = x9250_channels;
+	indio_dev->num_channels = ARRAY_SIZE(x9250_channels);
+	indio_dev->name = spi_get_device_id(spi)->name;
+
+	spi_set_drvdata(spi, indio_dev);
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+static const struct of_device_id x9250_of_match[] = {
+	{ .compatible = "renesas,x9250t", &x9250_cfg[X9250T]},
+	{ .compatible = "renesas,x9250u", &x9250_cfg[X9250U]},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, x9250_of_match);
+
+static const struct spi_device_id x9250_id_table[] = {
+	{ "x9250t", X9250T },
+	{ "x9250u", X9250U },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, x9250_id_table);
+
+static struct spi_driver x9250_spi_driver = {
+	.driver  = {
+		.name   = "x9250",
+		.of_match_table = x9250_of_match,
+	},
+	.id_table = x9250_id_table,
+	.probe  = x9250_probe,
+};
+
+module_spi_driver(x9250_spi_driver);
+
+MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>");
+MODULE_DESCRIPTION("X9250 ALSA SoC driver");
+MODULE_LICENSE("GPL");
-- 
2.39.2

Re: [PATCH 2/3] iio: potentiometer: Add support for the Renesas X9250 potentiometers

[Lars-Peter Clausen]

On 4/20/23 05:13, Herve Codina wrote:
> The Renesas X9250 integrates four digitally controlled potentiometers.
> On each potentiometer, the X9250T has a 100 kOhms total resistance and
> the X9250U has a 50 kOhms total resistance.
>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Hi,

Looks perfect! Just one small comment.

> +static int x9250_write8(struct x9250 *x9250, u8 cmd, u8 val)
> +{
> +	struct spi_transfer xfer = {
> +		.tx_buf = &x9250->spi_tx_buf,
> +		.len = 3,
> +	};
> +	int ret;
> +
> +	BUILD_BUG_ON(sizeof(x9250->spi_tx_buf) < 3);
> +
> +	mutex_lock(&x9250->lock);
> +
> +	x9250->spi_tx_buf[0] = 0x50;
The 0x50 shows up as a magic constant in multiple places, a define for 
it would be nice.
> +	x9250->spi_tx_buf[1] = cmd;
> +	x9250->spi_tx_buf[2] = val;
> +
> +	ret = spi_sync_transfer(x9250->spi, &xfer, 1);
> +
> +	mutex_unlock(&x9250->lock);
> +
> +	return ret;
> +}

Re: [PATCH 2/3] iio: potentiometer: Add support for the Renesas X9250 potentiometers

[Herve Codina]

On Thu, 20 Apr 2023 06:47:32 -0700
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 4/20/23 05:13, Herve Codina wrote:
> > The Renesas X9250 integrates four digitally controlled potentiometers.
> > On each potentiometer, the X9250T has a 100 kOhms total resistance and
> > the X9250U has a 50 kOhms total resistance.
> >
> > Signed-off-by: Herve Codina <herve.codina@bootlin.com>  
> 
> Hi,
> 
> Looks perfect! Just one small comment.
> 
> > +static int x9250_write8(struct x9250 *x9250, u8 cmd, u8 val)
> > +{
> > +	struct spi_transfer xfer = {
> > +		.tx_buf = &x9250->spi_tx_buf,
> > +		.len = 3,
> > +	};
> > +	int ret;
> > +
> > +	BUILD_BUG_ON(sizeof(x9250->spi_tx_buf) < 3);
> > +
> > +	mutex_lock(&x9250->lock);
> > +
> > +	x9250->spi_tx_buf[0] = 0x50;  
> The 0x50 shows up as a magic constant in multiple places, a define for 
> it would be nice.

Sure, a define will be present in next iteration.

> > +	x9250->spi_tx_buf[1] = cmd;
> > +	x9250->spi_tx_buf[2] = val;
> > +
> > +	ret = spi_sync_transfer(x9250->spi, &xfer, 1);
> > +
> > +	mutex_unlock(&x9250->lock);
> > +
> > +	return ret;
> > +}  

Thanks for the review,
Hervé

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

[PATCH 3/3] MAINTAINERS: add the Renesas X9250 driver entry

[Herve Codina]

After contributing the driver, add myself as the maintainer for the
Renesas X9250 IIO driver.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 8ec7ccba9848..0027af3e14cb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17926,6 +17926,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/clock/renesas,versaclock7.yaml
 F:	drivers/clk/clk-versaclock7.c
 
+RENESAS X9250 DIGITAL POTENTIOMETERS DRIVER
+M:	Herve Codina <herve.codina@bootlin.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.yaml
+F:	drivers/iio/potentiometer/x9250.c
+
 RESET CONTROLLER FRAMEWORK
 M:	Philipp Zabel <p.zabel@pengutronix.de>
 S:	Maintained
-- 
2.39.2