1. Patchew
  2. linux
  3. [PATCH 0/3] iio: adc: ad4695: add new regmap callbacks, timing improvements
  4. View patch

[PATCH 3/3] iio: adc: ad4695: add custom regmap bus callbacks

Trevor Gamblin posted 3 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH 3/3] iio: adc: ad4695: add custom regmap bus callbacks
Posted by Trevor Gamblin 1 year, 3 months ago 
Add a custom implementation of regmap read/write callbacks using the SPI
bus. This allows them to be performed at a lower SCLK rate than data
reads. Previously, all SPI transfers were being performed at a lower
speed, but with this change sample data is read at the max bus speed
while the register reads/writes remain at the lower rate.

Also remove .can_multi_write from the AD4695 driver's regmap_configs, as
this isn't implemented or needed.

For some background context, see:

https://lore.kernel.org/linux-iio/20241028163907.00007e12@Huawei.com/

Suggested-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
---
 drivers/iio/adc/Kconfig  |  2 +-
 drivers/iio/adc/ad4695.c | 74 +++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 68 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 6c4e74420fd2..e0f9d01ce37d 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -51,9 +51,9 @@ config AD4130
 config AD4695
 	tristate "Analog Device AD4695 ADC Driver"
 	depends on SPI
-	select REGMAP_SPI
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
+	select REGMAP
 	help
 	  Say yes here to build support for Analog Devices AD4695 and similar
 	  analog to digital converters (ADC).
diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
index f36c1a1db886..180a0fd4f03c 100644
--- a/drivers/iio/adc/ad4695.c
+++ b/drivers/iio/adc/ad4695.c
@@ -150,6 +150,8 @@ struct ad4695_state {
 	/* Commands to send for single conversion. */
 	u16 cnv_cmd;
 	u8 cnv_cmd2;
+	/* Buffer for storing data from regmap bus reads/writes */
+	u8 regmap_bus_data[4];
 };
 
 static const struct regmap_range ad4695_regmap_rd_ranges[] = {
@@ -194,7 +196,6 @@ static const struct regmap_config ad4695_regmap_config = {
 	.max_register = AD4695_REG_AS_SLOT(127),
 	.rd_table = &ad4695_regmap_rd_table,
 	.wr_table = &ad4695_regmap_wr_table,
-	.can_multi_write = true,
 };
 
 static const struct regmap_range ad4695_regmap16_rd_ranges[] = {
@@ -226,7 +227,67 @@ static const struct regmap_config ad4695_regmap16_config = {
 	.max_register = AD4695_REG_GAIN_IN(15),
 	.rd_table = &ad4695_regmap16_rd_table,
 	.wr_table = &ad4695_regmap16_wr_table,
-	.can_multi_write = true,
+};
+
+static int ad4695_regmap_bus_reg_write(void *context, const void *data,
+				       size_t count)
+{
+	struct ad4695_state *st = context;
+	struct spi_transfer xfer = {
+			.speed_hz = AD4695_REG_ACCESS_SCLK_HZ,
+			.len = count,
+			.tx_buf = st->regmap_bus_data,
+	};
+
+	if (count > ARRAY_SIZE(st->regmap_bus_data))
+		return -EINVAL;
+
+	memcpy(st->regmap_bus_data, data, count);
+
+	return spi_sync_transfer(st->spi, &xfer, 1);
+}
+
+static int ad4695_regmap_bus_reg_read(void *context, const void *reg,
+				      size_t reg_size, void *val,
+				      size_t val_size)
+{
+	struct ad4695_state *st = context;
+	struct spi_transfer xfers[] = {
+		{
+			.speed_hz = AD4695_REG_ACCESS_SCLK_HZ,
+			.len = reg_size,
+			.tx_buf = &st->regmap_bus_data[0],
+		}, {
+			.speed_hz = AD4695_REG_ACCESS_SCLK_HZ,
+			.len = val_size,
+			.rx_buf = &st->regmap_bus_data[2],
+		},
+	};
+	int ret;
+
+	if (reg_size > 2)
+		return -EINVAL;
+
+	if (val_size > 2)
+		return -EINVAL;
+
+	memcpy(&st->regmap_bus_data[0], reg, reg_size);
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+	if (ret)
+		return ret;
+
+	memcpy(val, &st->regmap_bus_data[2], val_size);
+
+	return 0;
+}
+
+static const struct regmap_bus ad4695_regmap_bus = {
+	.write = ad4695_regmap_bus_reg_write,
+	.read = ad4695_regmap_bus_reg_read,
+	.read_flag_mask = 0x80,
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
 };
 
 static const struct iio_chan_spec ad4695_channel_template = {
@@ -1040,15 +1101,14 @@ static int ad4695_probe(struct spi_device *spi)
 	if (!st->chip_info)
 		return -EINVAL;
 
-	/* Registers cannot be read at the max allowable speed */
-	spi->max_speed_hz = AD4695_REG_ACCESS_SCLK_HZ;
-
-	st->regmap = devm_regmap_init_spi(spi, &ad4695_regmap_config);
+	st->regmap = devm_regmap_init(dev, &ad4695_regmap_bus, st,
+				      &ad4695_regmap_config);
 	if (IS_ERR(st->regmap))
 		return dev_err_probe(dev, PTR_ERR(st->regmap),
 				     "Failed to initialize regmap\n");
 
-	st->regmap16 = devm_regmap_init_spi(spi, &ad4695_regmap16_config);
+	st->regmap16 = devm_regmap_init(dev, &ad4695_regmap_bus, st,
+					&ad4695_regmap_config);
 	if (IS_ERR(st->regmap16))
 		return dev_err_probe(dev, PTR_ERR(st->regmap16),
 				     "Failed to initialize regmap16\n");

-- 
2.39.5
Re: [PATCH 3/3] iio: adc: ad4695: add custom regmap bus callbacks
Posted by Trevor Gamblin 1 year, 3 months ago 
On 2024-11-11 10:59, Trevor Gamblin wrote:
> Add a custom implementation of regmap read/write callbacks using the SPI
> bus. This allows them to be performed at a lower SCLK rate than data
> reads. Previously, all SPI transfers were being performed at a lower
> speed, but with this change sample data is read at the max bus speed
> while the register reads/writes remain at the lower rate.
>
> Also remove .can_multi_write from the AD4695 driver's regmap_configs, as
> this isn't implemented or needed.
>
> For some background context, see:
>
> https://lore.kernel.org/linux-iio/20241028163907.00007e12@Huawei.com/
>
> Suggested-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
Found a couple more bugs after some testing. I'll wait until EOD on 
Wednesday to send a v2, in case there are any other review comments.
Re: [PATCH 3/3] iio: adc: ad4695: add custom regmap bus callbacks
Posted by Trevor Gamblin 1 year, 3 months ago 
On 2024-11-11 10:59, Trevor Gamblin wrote:
> Add a custom implementation of regmap read/write callbacks using the SPI
> bus. This allows them to be performed at a lower SCLK rate than data
> reads. Previously, all SPI transfers were being performed at a lower
> speed, but with this change sample data is read at the max bus speed
> while the register reads/writes remain at the lower rate.
>
> Also remove .can_multi_write from the AD4695 driver's regmap_configs, as
> this isn't implemented or needed.
>
> For some background context, see:
>
> https://lore.kernel.org/linux-iio/20241028163907.00007e12@Huawei.com/
>
> Suggested-by: David Lechner <dlechner@baylibre.com>
> Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
> ---
>   drivers/iio/adc/Kconfig  |  2 +-
>   drivers/iio/adc/ad4695.c | 74 +++++++++++++++++++++++++++++++++++++++++++-----
>   2 files changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 6c4e74420fd2..e0f9d01ce37d 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -51,9 +51,9 @@ config AD4130
>   config AD4695
>   	tristate "Analog Device AD4695 ADC Driver"
>   	depends on SPI
> -	select REGMAP_SPI
>   	select IIO_BUFFER
>   	select IIO_TRIGGERED_BUFFER
> +	select REGMAP
>   	help
>   	  Say yes here to build support for Analog Devices AD4695 and similar
>   	  analog to digital converters (ADC).
> diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
> index f36c1a1db886..180a0fd4f03c 100644
> --- a/drivers/iio/adc/ad4695.c
> +++ b/drivers/iio/adc/ad4695.c
> @@ -150,6 +150,8 @@ struct ad4695_state {
>   	/* Commands to send for single conversion. */
>   	u16 cnv_cmd;
>   	u8 cnv_cmd2;
> +	/* Buffer for storing data from regmap bus reads/writes */
> +	u8 regmap_bus_data[4];
>   };
>   
>   static const struct regmap_range ad4695_regmap_rd_ranges[] = {
> @@ -194,7 +196,6 @@ static const struct regmap_config ad4695_regmap_config = {
>   	.max_register = AD4695_REG_AS_SLOT(127),
>   	.rd_table = &ad4695_regmap_rd_table,
>   	.wr_table = &ad4695_regmap_wr_table,
> -	.can_multi_write = true,
>   };
>   
>   static const struct regmap_range ad4695_regmap16_rd_ranges[] = {
> @@ -226,7 +227,67 @@ static const struct regmap_config ad4695_regmap16_config = {
>   	.max_register = AD4695_REG_GAIN_IN(15),
>   	.rd_table = &ad4695_regmap16_rd_table,
>   	.wr_table = &ad4695_regmap16_wr_table,
> -	.can_multi_write = true,
> +};
> +
> +static int ad4695_regmap_bus_reg_write(void *context, const void *data,
> +				       size_t count)
> +{
> +	struct ad4695_state *st = context;
> +	struct spi_transfer xfer = {
> +			.speed_hz = AD4695_REG_ACCESS_SCLK_HZ,
> +			.len = count,
> +			.tx_buf = st->regmap_bus_data,
> +	};
> +
> +	if (count > ARRAY_SIZE(st->regmap_bus_data))
> +		return -EINVAL;
> +
> +	memcpy(st->regmap_bus_data, data, count);
> +
> +	return spi_sync_transfer(st->spi, &xfer, 1);
> +}
> +
> +static int ad4695_regmap_bus_reg_read(void *context, const void *reg,
> +				      size_t reg_size, void *val,
> +				      size_t val_size)
> +{
> +	struct ad4695_state *st = context;
> +	struct spi_transfer xfers[] = {
> +		{
> +			.speed_hz = AD4695_REG_ACCESS_SCLK_HZ,
> +			.len = reg_size,
> +			.tx_buf = &st->regmap_bus_data[0],
> +		}, {
> +			.speed_hz = AD4695_REG_ACCESS_SCLK_HZ,
> +			.len = val_size,
> +			.rx_buf = &st->regmap_bus_data[2],
> +		},
> +	};
> +	int ret;
> +
> +	if (reg_size > 2)
> +		return -EINVAL;
> +
> +	if (val_size > 2)
> +		return -EINVAL;
> +
> +	memcpy(&st->regmap_bus_data[0], reg, reg_size);
> +
> +	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
> +	if (ret)
> +		return ret;
> +
> +	memcpy(val, &st->regmap_bus_data[2], val_size);
> +
> +	return 0;
> +}
> +
> +static const struct regmap_bus ad4695_regmap_bus = {
> +	.write = ad4695_regmap_bus_reg_write,
> +	.read = ad4695_regmap_bus_reg_read,
> +	.read_flag_mask = 0x80,
> +	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
> +	.val_format_endian_default = REGMAP_ENDIAN_BIG,
>   };
>   
>   static const struct iio_chan_spec ad4695_channel_template = {
> @@ -1040,15 +1101,14 @@ static int ad4695_probe(struct spi_device *spi)
>   	if (!st->chip_info)
>   		return -EINVAL;
>   
> -	/* Registers cannot be read at the max allowable speed */
> -	spi->max_speed_hz = AD4695_REG_ACCESS_SCLK_HZ;
> -
> -	st->regmap = devm_regmap_init_spi(spi, &ad4695_regmap_config);
> +	st->regmap = devm_regmap_init(dev, &ad4695_regmap_bus, st,
> +				      &ad4695_regmap_config);
>   	if (IS_ERR(st->regmap))
>   		return dev_err_probe(dev, PTR_ERR(st->regmap),
>   				     "Failed to initialize regmap\n");
>   
> -	st->regmap16 = devm_regmap_init_spi(spi, &ad4695_regmap16_config);
> +	st->regmap16 = devm_regmap_init(dev, &ad4695_regmap_bus, st,
> +					&ad4695_regmap_config);

Note that there's a bug here - that should be

+	st->regmap16 = devm_regmap_init(dev, &ad4695_regmap_bus, st,
+					&ad4695_regmap16_config);

I can fix this in v2, unless it would just be easier to fix during merge 
(assuming there is no other major feedback).

>   	if (IS_ERR(st->regmap16))
>   		return dev_err_probe(dev, PTR_ERR(st->regmap16),
>   				     "Failed to initialize regmap16\n");
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.