Message ID | 20241210-ad7606_add_iio_backend_software_mode-v2-6-6619c3e50d81@baylibre.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | Add support for Software mode on AD7606's iio backend driver | expand |
On Tue, 10 Dec 2024 10:46:46 +0000 Guillaume Stols <gstols@baylibre.com> wrote: > Since we must access the bus parallel bus using a custom procedure, > let's add a specialized compatible, and define specialized callbacks for > writing the registers using the parallel interface. > > Signed-off-by: Guillaume Stols <gstols@baylibre.com> Hi Guillaume, A few comments inline. Thanks, Jonathan > --- > drivers/iio/adc/ad7606_bi.h | 16 +++++++ > drivers/iio/adc/adi-axi-adc.c | 100 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 116 insertions(+) > > diff --git a/drivers/iio/adc/ad7606_bi.h b/drivers/iio/adc/ad7606_bi.h > new file mode 100644 > index 000000000000..9ade23ec61dd > --- /dev/null > +++ b/drivers/iio/adc/ad7606_bi.h Why bi? Bus interface? I'd spell it out. Header name lengths don't usually need to be quite this short. > @@ -0,0 +1,16 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright (c) 2010-2024 Analog Devices Inc. > + * Copyright (c) 2024 Baylibre, SAS > + */ > +#ifndef __LINUX_PLATFORM_DATA_AD7606_H__ > +#define __LINUX_PLATFORM_DATA_AD7606_H__ > + > +#include <linux/iio/backend.h> Only a pointer type needed, so a forward def better than an include (which costs some trivial amount of compile time). struct iio_backend; > + > +struct ad7606_platform_data { > + int (*bus_reg_read)(struct iio_backend *back, u32 reg, u32 *val); > + int (*bus_reg_write)(struct iio_backend *back, u32 reg, u32 val); > +}; > + > +#endif /* __LINUX_PLATFORM_DATA_AD7606_H__ */ > diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c > index 7ff636643e56..b8bcf89417b0 100644 > --- a/drivers/iio/adc/adi-axi-adc.c > +++ b/drivers/iio/adc/adi-axi-adc.c > @@ -27,6 +27,7 @@ > #include <linux/iio/buffer.h> > #include <linux/iio/iio.h> > > +#include "ad7606_bi.h" > /* > * Register definitions: > * https://wiki.analog.com/resources/fpga/docs/axi_adc_ip#register_map > @@ -73,6 +74,12 @@ > #define ADI_AXI_ADC_REG_DELAY(l) (0x0800 + (l) * 0x4) > #define AXI_ADC_DELAY_CTRL_MASK GENMASK(4, 0) > > +#define ADI_AXI_REG_CONFIG_WR 0x0080 > +#define ADI_AXI_REG_CONFIG_RD 0x0084 > +#define ADI_AXI_REG_CONFIG_CTRL 0x008c > +#define ADI_AXI_REG_CONFIG_CTRL_READ 0x03 > +#define ADI_AXI_REG_CONFIG_CTRL_WRITE 0x01 > + > #define ADI_AXI_ADC_MAX_IO_NUM_LANES 15 > > #define ADI_AXI_REG_CHAN_CTRL_DEFAULTS \ > @@ -80,6 +87,11 @@ > ADI_AXI_REG_CHAN_CTRL_FMT_EN | \ > ADI_AXI_REG_CHAN_CTRL_ENABLE) > > +/* AD7606's specific */ > +#define AD7606_REG_READ_BIT 0x8000 > +#define AD7606_REG_ADDRESS_MASK 0xff00 > +#define AD7606_REG_VALUE_MASK 0x00ff > + Maybe name these to keep the ADI_AXI_ prefix as well. Otherwise it might get a little confusing wrt to registers in teh deivce. They are nice short names currently so that shouldn't make the code to messy. > struct axi_adc_info { > unsigned int version; > const struct iio_backend_info *backend_info; > @@ -313,6 +325,80 @@ static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back, > return iio_dmaengine_buffer_setup(st->dev, indio_dev, dma_name); > } > > +static int axi_adc_raw_write(struct iio_backend *back, void *buf, unsigned int len) > +{ > + struct adi_axi_adc_state *st = iio_backend_get_priv(back); > + u32 data; > + u32 *bdata = buf; data = *(u32)(buf); Is fine here I think rather than the extra local variable that is just used to do the type cast. > + > + data = *bdata; > + regmap_write(st->regmap, ADI_AXI_REG_CONFIG_WR, data); > + regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL, > + ADI_AXI_REG_CONFIG_CTRL_WRITE); > + usleep_range(50, 100); > + regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL, 0x00); > + usleep_range(50, 100); > + > + return 0; > +} ... > + > +static int ad7606_bi_reg_write(struct iio_backend *back, u32 reg, u32 val) > +{ > + struct adi_axi_adc_state *st = iio_backend_get_priv(back); > + u32 buf; > + > + guard(mutex)(&st->lock); > + > + /* Read any register to switch to register mode */ What follows looks like a write... > + buf = 0xaf00; > + axi_adc_raw_write(back, &buf, sizeof(buf)); > + > + buf = FIELD_PREP(AD7606_REG_ADDRESS_MASK, reg) | FIELD_PREP(AD7606_REG_VALUE_MASK, val); > + axi_adc_raw_write(back, &buf, sizeof(buf)); > + > + /* Write 0x0 on the bus to get back to ADC mode */ > + buf = 0; > + axi_adc_raw_write(back, &buf, sizeof(buf)); > + > + return 0; > +}
diff --git a/drivers/iio/adc/ad7606_bi.h b/drivers/iio/adc/ad7606_bi.h new file mode 100644 index 000000000000..9ade23ec61dd --- /dev/null +++ b/drivers/iio/adc/ad7606_bi.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (c) 2010-2024 Analog Devices Inc. + * Copyright (c) 2024 Baylibre, SAS + */ +#ifndef __LINUX_PLATFORM_DATA_AD7606_H__ +#define __LINUX_PLATFORM_DATA_AD7606_H__ + +#include <linux/iio/backend.h> + +struct ad7606_platform_data { + int (*bus_reg_read)(struct iio_backend *back, u32 reg, u32 *val); + int (*bus_reg_write)(struct iio_backend *back, u32 reg, u32 val); +}; + +#endif /* __LINUX_PLATFORM_DATA_AD7606_H__ */ diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c index 7ff636643e56..b8bcf89417b0 100644 --- a/drivers/iio/adc/adi-axi-adc.c +++ b/drivers/iio/adc/adi-axi-adc.c @@ -27,6 +27,7 @@ #include <linux/iio/buffer.h> #include <linux/iio/iio.h> +#include "ad7606_bi.h" /* * Register definitions: * https://wiki.analog.com/resources/fpga/docs/axi_adc_ip#register_map @@ -73,6 +74,12 @@ #define ADI_AXI_ADC_REG_DELAY(l) (0x0800 + (l) * 0x4) #define AXI_ADC_DELAY_CTRL_MASK GENMASK(4, 0) +#define ADI_AXI_REG_CONFIG_WR 0x0080 +#define ADI_AXI_REG_CONFIG_RD 0x0084 +#define ADI_AXI_REG_CONFIG_CTRL 0x008c +#define ADI_AXI_REG_CONFIG_CTRL_READ 0x03 +#define ADI_AXI_REG_CONFIG_CTRL_WRITE 0x01 + #define ADI_AXI_ADC_MAX_IO_NUM_LANES 15 #define ADI_AXI_REG_CHAN_CTRL_DEFAULTS \ @@ -80,6 +87,11 @@ ADI_AXI_REG_CHAN_CTRL_FMT_EN | \ ADI_AXI_REG_CHAN_CTRL_ENABLE) +/* AD7606's specific */ +#define AD7606_REG_READ_BIT 0x8000 +#define AD7606_REG_ADDRESS_MASK 0xff00 +#define AD7606_REG_VALUE_MASK 0x00ff + struct axi_adc_info { unsigned int version; const struct iio_backend_info *backend_info; @@ -313,6 +325,80 @@ static struct iio_buffer *axi_adc_request_buffer(struct iio_backend *back, return iio_dmaengine_buffer_setup(st->dev, indio_dev, dma_name); } +static int axi_adc_raw_write(struct iio_backend *back, void *buf, unsigned int len) +{ + struct adi_axi_adc_state *st = iio_backend_get_priv(back); + u32 data; + u32 *bdata = buf; + + data = *bdata; + regmap_write(st->regmap, ADI_AXI_REG_CONFIG_WR, data); + regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL, + ADI_AXI_REG_CONFIG_CTRL_WRITE); + usleep_range(50, 100); + regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL, 0x00); + usleep_range(50, 100); + + return 0; +} + +static int axi_adc_raw_read(struct iio_backend *back, void *buf, unsigned int len) +{ + struct adi_axi_adc_state *st = iio_backend_get_priv(back); + u32 *bdata = buf; + + regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL, + ADI_AXI_REG_CONFIG_CTRL_READ); + usleep_range(50, 100); + regmap_read(st->regmap, ADI_AXI_REG_CONFIG_RD, bdata); + regmap_write(st->regmap, ADI_AXI_REG_CONFIG_CTRL, 0x00); + usleep_range(50, 100); + + return 0; +} + +static int ad7606_bi_reg_read(struct iio_backend *back, u32 reg, u32 *val) +{ + struct adi_axi_adc_state *st = iio_backend_get_priv(back); + u32 buf; + + guard(mutex)(&st->lock); + + /* + * The address is written on the highest weight byte, and the MSB set at 1 + * indicates a read operation. + */ + buf = FIELD_PREP(AD7606_REG_ADDRESS_MASK, reg) | AD7606_REG_READ_BIT; + axi_adc_raw_write(back, &buf, sizeof(buf)); + axi_adc_raw_read(back, val, 4); + + /* Write 0x0 on the bus to get back to ADC mode */ + buf = 0; + axi_adc_raw_write(back, &buf, sizeof(buf)); + return 0; +} + +static int ad7606_bi_reg_write(struct iio_backend *back, u32 reg, u32 val) +{ + struct adi_axi_adc_state *st = iio_backend_get_priv(back); + u32 buf; + + guard(mutex)(&st->lock); + + /* Read any register to switch to register mode */ + buf = 0xaf00; + axi_adc_raw_write(back, &buf, sizeof(buf)); + + buf = FIELD_PREP(AD7606_REG_ADDRESS_MASK, reg) | FIELD_PREP(AD7606_REG_VALUE_MASK, val); + axi_adc_raw_write(back, &buf, sizeof(buf)); + + /* Write 0x0 on the bus to get back to ADC mode */ + buf = 0; + axi_adc_raw_write(back, &buf, sizeof(buf)); + + return 0; +} + static void axi_adc_free_buffer(struct iio_backend *back, struct iio_buffer *buffer) { @@ -487,9 +573,23 @@ static const struct axi_adc_info adc_generic = { .backend_info = &adi_axi_adc_generic, }; +static const struct ad7606_platform_data ad7606_pdata = { + .bus_reg_read = ad7606_bi_reg_read, + .bus_reg_write = ad7606_bi_reg_write, +}; + +static const struct axi_adc_info adc_ad7606 = { + .version = ADI_AXI_PCORE_VER(10, 0, 'a'), + .backend_info = &adi_axi_adc_generic, + .bus_controller = true, + .pdata = &ad7606_pdata, + .pdata_sz = sizeof(ad7606_pdata), +}; + /* Match table for of_platform binding */ static const struct of_device_id adi_axi_adc_of_match[] = { { .compatible = "adi,axi-adc-10.0.a", .data = &adc_generic }, + { .compatible = "adi,axi-ad7606x", .data = &adc_ad7606 }, { /* end of list */ } }; MODULE_DEVICE_TABLE(of, adi_axi_adc_of_match);
Since we must access the bus parallel bus using a custom procedure, let's add a specialized compatible, and define specialized callbacks for writing the registers using the parallel interface. Signed-off-by: Guillaume Stols <gstols@baylibre.com> --- drivers/iio/adc/ad7606_bi.h | 16 +++++++ drivers/iio/adc/adi-axi-adc.c | 100 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+)