Re: [PATCH v3 1/1] iio: adc: ad7124: allow more than 8 channels
On Tue, 23 Feb 2021 18:44:04 +0200
wrote:
> From: Alexandru Tachici
>
> Currently AD7124-8 driver cannot use more than 8 IIO channels
> because it was assigning the channel configurations bijectively
> to channels specified in the device-tree. This is not possible
> to do when using more than 8 channels as AD7124-8 has only 8
> configuration registers.
>
> To allow the user to use all channels at once the driver
> will keep in memory configurations for all channels but
> will program only 8 of them at a time on the device.
> If multiple channels have the same configuration, only
> one configuration register will be used. If there
> are more configurations than available registers only
> the last 8 used configurations will be allowed to exist
> on the device in a LRU fashion.
>
> Signed-off-by: Alexandru Tachici
Hi Alexandru,
I like the solution you've gone with here. Its fairly intuitive
solution to a fiddly problem :)
All comments inline are fairly minor tweaks.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7124.c | 461 ++-
> 1 file changed, 308 insertions(+), 153 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index 766c7604..d5d0596eb6ed 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -86,6 +87,10 @@
> #define AD7124_SINC3_FILTER 2
> #define AD7124_SINC4_FILTER 0
>
> +#define AD7124_CONF_ADDR_OFFSET 20
> +#define AD7124_MAX_CONFIGS 8
> +#define AD7124_MAX_CHANNELS 16
> +
> enum ad7124_ids {
> ID_AD7124_4,
> ID_AD7124_8,
> @@ -136,25 +141,36 @@ struct ad7124_chip_info {
> };
>
> struct ad7124_channel_config {
> + bool live;
> + unsigned int cfg_slot;
> enum ad7124_ref_sel refsel;
> bool bipolar;
> bool buf_positive;
> bool buf_negative;
> - unsigned int ain;
> unsigned int vref_mv;
> unsigned int pga_bits;
> unsigned int odr;
> + unsigned int odr_sel_bits;
> unsigned int filter_type;
> };
>
> +struct ad7124_channel {
> + unsigned int nr;
> + struct ad7124_channel_config cfg;
> + unsigned int ain;
> + unsigned int slot;
> +};
> +
> struct ad7124_state {
> const struct ad7124_chip_info *chip_info;
> struct ad_sigma_delta sd;
> - struct ad7124_channel_config *channel_config;
> + struct ad7124_channel *channels;
> struct regulator *vref[4];
> struct clk *mclk;
> unsigned int adc_control;
> unsigned int num_channels;
> + struct mutex cfgs_lock; /* lock for configs access */
> + DECLARE_KFIFO(live_cfgs_fifo, struct ad7124_channel_config *,
> AD7124_MAX_CONFIGS);
> };
>
> static const struct iio_chan_spec ad7124_channel_template = {
> @@ -238,33 +254,9 @@ static int ad7124_set_mode(struct ad_sigma_delta *sd,
> return ad_sd_write_reg(>sd, AD7124_ADC_CONTROL, 2, st->adc_control);
> }
>
> -static int ad7124_set_channel(struct ad_sigma_delta *sd, unsigned int
> channel)
> -{
> - struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
> - unsigned int val;
> -
> - val = st->channel_config[channel].ain | AD7124_CHANNEL_EN(1) |
> - AD7124_CHANNEL_SETUP(channel);
> -
> - return ad_sd_write_reg(>sd, AD7124_CHANNEL(channel), 2, val);
> -}
> -
> -static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
> - .set_channel = ad7124_set_channel,
> - .set_mode = ad7124_set_mode,
> - .has_registers = true,
> - .addr_shift = 0,
> - .read_mask = BIT(6),
> - .data_reg = AD7124_DATA,
> - .irq_flags = IRQF_TRIGGER_FALLING,
> -};
> -
> -static int ad7124_set_channel_odr(struct ad7124_state *st,
> - unsigned int channel,
> - unsigned int odr)
> +static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int
> channel, unsigned int odr)
> {
> unsigned int fclk, odr_sel_bits;
> - int ret;
>
> fclk = clk_get_rate(st->mclk);
> /*
> @@ -280,36 +272,10 @@ static int ad7124_set_channel_odr(struct ad7124_state
> *st,
> else if (odr_sel_bits > 2047)
> odr_sel_bits = 2047;
>
> - ret = ad7124_spi_write_mask(st, AD7124_FILTER(channel),
> - AD7124_FILTER_FS_MSK,
> - AD7124_FILTER_FS(odr_sel_bits), 3);
> - if (ret < 0)
> - return ret;
> - /* fADC = fCLK / (FS[10:0] x 32) */
> - st->channel_config[channel].odr =
> - DIV_ROUND_CLOSEST(fclk, odr_sel_bits * 32);
> -
> - return 0;
> -}
> -
> -static int ad7124_set_channel_gain(struct ad7124_state *st,
> -unsigned int channel,
> -unsigned int gain)
> -{
> - unsigned int res;
> - int ret;
> -
> - res =
[PATCH v3 1/1] iio: adc: ad7124: allow more than 8 channels
From: Alexandru Tachici
Currently AD7124-8 driver cannot use more than 8 IIO channels
because it was assigning the channel configurations bijectively
to channels specified in the device-tree. This is not possible
to do when using more than 8 channels as AD7124-8 has only 8
configuration registers.
To allow the user to use all channels at once the driver
will keep in memory configurations for all channels but
will program only 8 of them at a time on the device.
If multiple channels have the same configuration, only
one configuration register will be used. If there
are more configurations than available registers only
the last 8 used configurations will be allowed to exist
on the device in a LRU fashion.
Signed-off-by: Alexandru Tachici
---
drivers/iio/adc/ad7124.c | 461 ++-
1 file changed, 308 insertions(+), 153 deletions(-)
diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index 766c7604..d5d0596eb6ed 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -86,6 +87,10 @@
#define AD7124_SINC3_FILTER 2
#define AD7124_SINC4_FILTER 0
+#define AD7124_CONF_ADDR_OFFSET20
+#define AD7124_MAX_CONFIGS 8
+#define AD7124_MAX_CHANNELS16
+
enum ad7124_ids {
ID_AD7124_4,
ID_AD7124_8,
@@ -136,25 +141,36 @@ struct ad7124_chip_info {
};
struct ad7124_channel_config {
+ bool live;
+ unsigned int cfg_slot;
enum ad7124_ref_sel refsel;
bool bipolar;
bool buf_positive;
bool buf_negative;
- unsigned int ain;
unsigned int vref_mv;
unsigned int pga_bits;
unsigned int odr;
+ unsigned int odr_sel_bits;
unsigned int filter_type;
};
+struct ad7124_channel {
+ unsigned int nr;
+ struct ad7124_channel_config cfg;
+ unsigned int ain;
+ unsigned int slot;
+};
+
struct ad7124_state {
const struct ad7124_chip_info *chip_info;
struct ad_sigma_delta sd;
- struct ad7124_channel_config *channel_config;
+ struct ad7124_channel *channels;
struct regulator *vref[4];
struct clk *mclk;
unsigned int adc_control;
unsigned int num_channels;
+ struct mutex cfgs_lock; /* lock for configs access */
+ DECLARE_KFIFO(live_cfgs_fifo, struct ad7124_channel_config *,
AD7124_MAX_CONFIGS);
};
static const struct iio_chan_spec ad7124_channel_template = {
@@ -238,33 +254,9 @@ static int ad7124_set_mode(struct ad_sigma_delta *sd,
return ad_sd_write_reg(>sd, AD7124_ADC_CONTROL, 2, st->adc_control);
}
-static int ad7124_set_channel(struct ad_sigma_delta *sd, unsigned int channel)
-{
- struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
- unsigned int val;
-
- val = st->channel_config[channel].ain | AD7124_CHANNEL_EN(1) |
- AD7124_CHANNEL_SETUP(channel);
-
- return ad_sd_write_reg(>sd, AD7124_CHANNEL(channel), 2, val);
-}
-
-static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
- .set_channel = ad7124_set_channel,
- .set_mode = ad7124_set_mode,
- .has_registers = true,
- .addr_shift = 0,
- .read_mask = BIT(6),
- .data_reg = AD7124_DATA,
- .irq_flags = IRQF_TRIGGER_FALLING,
-};
-
-static int ad7124_set_channel_odr(struct ad7124_state *st,
- unsigned int channel,
- unsigned int odr)
+static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int
channel, unsigned int odr)
{
unsigned int fclk, odr_sel_bits;
- int ret;
fclk = clk_get_rate(st->mclk);
/*
@@ -280,36 +272,10 @@ static int ad7124_set_channel_odr(struct ad7124_state *st,
else if (odr_sel_bits > 2047)
odr_sel_bits = 2047;
- ret = ad7124_spi_write_mask(st, AD7124_FILTER(channel),
- AD7124_FILTER_FS_MSK,
- AD7124_FILTER_FS(odr_sel_bits), 3);
- if (ret < 0)
- return ret;
- /* fADC = fCLK / (FS[10:0] x 32) */
- st->channel_config[channel].odr =
- DIV_ROUND_CLOSEST(fclk, odr_sel_bits * 32);
-
- return 0;
-}
-
-static int ad7124_set_channel_gain(struct ad7124_state *st,
- unsigned int channel,
- unsigned int gain)
-{
- unsigned int res;
- int ret;
-
- res = ad7124_find_closest_match(ad7124_gain,
- ARRAY_SIZE(ad7124_gain), gain);
- ret = ad7124_spi_write_mask(st, AD7124_CONFIG(channel),
- AD7124_CONFIG_PGA_MSK,
- AD7124_CONFIG_PGA(res), 2);
- if (ret < 0)
- return ret;
-
- st->channel_config[channel].pga_bits = res;
-
