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.5On 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.
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");
>
© 2016 - 2024 Red Hat, Inc.