| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 27 Feb 2021 17:41:46 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: <alexandru.tachici@...log.com>
Cc: <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/1] iio: adc: ad7124: allow more than 8 channels
On Tue, 23 Feb 2021 18:44:04 +0200
<alexandru.tachici@...log.com> wrote:
> From: Alexandru Tachici <alexandru.tachici@...log.com>
>
> 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 <alexandru.tachici@...log.com>
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 766c73333604..d5d0596eb6ed 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -11,6 +11,7 @@
> #include <linux/err.h>
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> +#include <linux/kfifo.h>
> #include <linux/module.h>
> #include <linux/of_device.h>
> #include <linux/regulator/consumer.h>
> @@ -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(&st->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(&st->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;
>
> - return 0;
> + /* fADC = fCLK / (FS[10:0] x 32) */
> + st->channels[channel].cfg.odr = DIV_ROUND_CLOSEST(fclk, odr_sel_bits * 32);
> + st->channels[channel].cfg.odr_sel_bits = odr_sel_bits;
> }
>
> static int ad7124_get_3db_filter_freq(struct ad7124_state *st,
> @@ -317,9 +283,9 @@ static int ad7124_get_3db_filter_freq(struct ad7124_state *st,
> {
> unsigned int fadc;
>
> - fadc = st->channel_config[channel].odr;
> + fadc = st->channels[channel].cfg.odr;
>
> - switch (st->channel_config[channel].filter_type) {
> + switch (st->channels[channel].cfg.filter_type) {
> case AD7124_SINC3_FILTER:
> return DIV_ROUND_CLOSEST(fadc * 230, 1000);
> case AD7124_SINC4_FILTER:
> @@ -329,9 +295,8 @@ static int ad7124_get_3db_filter_freq(struct ad7124_state *st,
> }
> }
>
> -static int ad7124_set_3db_filter_freq(struct ad7124_state *st,
> - unsigned int channel,
> - unsigned int freq)
> +static void ad7124_set_3db_filter_freq(struct ad7124_state *st, unsigned int channel,
> + unsigned int freq)
> {
> unsigned int sinc4_3db_odr;
> unsigned int sinc3_3db_odr;
> @@ -349,21 +314,224 @@ static int ad7124_set_3db_filter_freq(struct ad7124_state *st,
> new_odr = sinc3_3db_odr;
> }
>
> - if (st->channel_config[channel].filter_type != new_filter) {
> - int ret;
> + st->channels[channel].cfg.filter_type = new_filter;
> + st->channels[channel].cfg.odr = new_odr;
> +}
>
> - st->channel_config[channel].filter_type = new_filter;
> - ret = ad7124_spi_write_mask(st, AD7124_FILTER(channel),
> - AD7124_FILTER_TYPE_MSK,
> - AD7124_FILTER_TYPE_SEL(new_filter),
> - 3);
> - if (ret < 0)
> - return ret;
> +static bool ad7124_configs_equal(struct ad7124_channel_config *cfg1,
> + struct ad7124_channel_config *cfg2)
> +{
> + return ((cfg1->refsel == cfg2->refsel) &&
> + (cfg1->bipolar == cfg2->bipolar) &&
> + (cfg1->buf_positive == cfg2->buf_positive) &&
> + (cfg1->buf_negative == cfg2->buf_negative) &&
> + (cfg1->vref_mv == cfg2->vref_mv) &&
> + (cfg1->pga_bits == cfg2->pga_bits) &&
> + (cfg1->odr == cfg2->odr) &&
> + (cfg1->odr_sel_bits == cfg2->odr_sel_bits) &&
> + (cfg1->filter_type == cfg2->filter_type));
> +}
> +
> +static struct ad7124_channel_config *ad7124_find_similar_live_cfg(struct ad7124_state *st,
> + struct ad7124_channel_config *cfg)
> +{
> + struct ad7124_channel_config *cfg_aux;
> + int i;
> +
> + for (i = 0; i < st->num_channels; i++) {
> + cfg_aux = &st->channels[i].cfg;
> +
> + if (cfg_aux->live && ad7124_configs_equal(cfg, cfg_aux))
> + return cfg_aux;
> + }
> +
> + return NULL;
> +}
> +
> +static int ad7124_find_free_config_slot(struct ad7124_state *st)
> +{
> + int slots[AD7124_MAX_CONFIGS] = {0};
> + struct ad7124_channel_config *cfg;
> + int i = 0;
> +
> + /* mark all currently used configuration slots */
> + for (i = 0; i < st->num_channels; i++) {
> + cfg = &st->channels[i].cfg;
> +
> + if (cfg->live)
> + slots[cfg->cfg_slot] = 1;
Worth maintaining a bitmap as we make use of configs
rather than computing this here?
Defintiely use a bitmap here if not and find_next_zero_bit() to
find an empty hole.
> + }
> +
> + /* find a free one */
> + for (i = 0; i < AD7124_MAX_CONFIGS; i++) {
> + if (slots[i] == 0)
> + return i;
> + }
> +
> + return -1;
> +}
> +
> +static int ad7124_init_config_vref(struct ad7124_state *st, struct ad7124_channel_config *cfg)
> +{
> + unsigned int refsel = cfg->refsel;
> +
> + switch (refsel) {
> + case AD7124_REFIN1:
> + case AD7124_REFIN2:
> + case AD7124_AVDD_REF:
> + if (IS_ERR(st->vref[refsel])) {
> + dev_err(&st->sd.spi->dev,
> + "Error, trying to use external voltage reference without a %s regulator.\n",
> + ad7124_ref_names[refsel]);
> + return PTR_ERR(st->vref[refsel]);
> + }
> + cfg->vref_mv = regulator_get_voltage(st->vref[refsel]);
> + /* Conversion from uV to mV */
> + cfg->vref_mv /= 1000;
> + break;
> + case AD7124_INT_REF:
> + cfg->vref_mv = 2500;
> + st->adc_control &= ~AD7124_ADC_CTRL_REF_EN_MSK;
> + st->adc_control |= AD7124_ADC_CTRL_REF_EN(1);
> + return ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL,
> + 2, st->adc_control);
> + default:
> + dev_err(&st->sd.spi->dev, "Invalid reference %d\n", refsel);
> + return -EINVAL;
> + }
> +
> + return 0;
Move this up to the the one break path above? (trivial!)
> +}
> +
> +static int ad7124_write_config(struct ad7124_state *st, struct ad7124_channel_config *cfg,
> + unsigned int cfg_slot)
> +{
> + unsigned int tmp;
> + unsigned int val;
> + int ret;
> +
> + cfg->cfg_slot = cfg_slot;
> +
> + tmp = (cfg->buf_positive << 1) + cfg->buf_negative;
> + val = AD7124_CONFIG_BIPOLAR(cfg->bipolar) | AD7124_CONFIG_REF_SEL(cfg->refsel) |
> + AD7124_CONFIG_IN_BUFF(tmp);
> + ret = ad_sd_write_reg(&st->sd, AD7124_CONFIG(cfg->cfg_slot), 2, val);
> + if (ret < 0)
> + return ret;
> +
> + tmp = AD7124_FILTER_TYPE_SEL(cfg->filter_type);
> + ret = ad7124_spi_write_mask(st, AD7124_FILTER(cfg->cfg_slot), AD7124_FILTER_TYPE_MSK,
> + tmp, 3);
> + if (ret < 0)
> + return ret;
> +
> + ret = ad7124_spi_write_mask(st, AD7124_FILTER(cfg->cfg_slot), AD7124_FILTER_FS_MSK,
> + AD7124_FILTER_FS(cfg->odr_sel_bits), 3);
> + if (ret < 0)
> + return ret;
> +
> + return ad7124_spi_write_mask(st, AD7124_CONFIG(cfg->cfg_slot), AD7124_CONFIG_PGA_MSK,
> + AD7124_CONFIG_PGA(cfg->pga_bits), 2);
> +}
> +
> +static struct ad7124_channel_config *ad7124_pop_config(struct ad7124_state *st)
> +{
> + struct ad7124_channel_config *lru_cfg;
> + struct ad7124_channel_config *cfg;
> + int ret;
> + int i;
> +
> + /* pop least recently used config from the fifo
/*
* Pop
> + * in order to make room for the new one
> + */
> + ret = kfifo_get(&st->live_cfgs_fifo, &lru_cfg);
> + if (ret <= 0)
> + return NULL;
> +
> + lru_cfg->live = false;
> +
> + /* invalidate all other configs that pointed to this one */
> + for (i = 0; i < st->num_channels; i++) {
> + cfg = &st->channels[i].cfg;
> +
> + if (cfg->cfg_slot == lru_cfg->cfg_slot)
> + cfg->live = false;
> }
>
> - return ad7124_set_channel_odr(st, channel, new_odr);
> + return lru_cfg;
> +}
> +
> +static int ad7124_enable_channel(struct ad7124_state *st, struct ad7124_channel *ch)
> +{
> + ch->cfg.live = true;
> + return ad_sd_write_reg(&st->sd, AD7124_CHANNEL(ch->nr), 2, ch->ain |
> + AD7124_CHANNEL_SETUP(ch->cfg.cfg_slot) | AD7124_CHANNEL_EN(1));
> }
>
> +static int ad7124_prepare_read(struct ad7124_state *st, int address)
> +{
> + struct ad7124_channel_config *cfg = &st->channels[address].cfg;
> + struct ad7124_channel_config *live_cfg;
> + struct ad7124_channel_config *lru_cfg;
> + int free_cfg_slot;
> + int ret;
> +
> + /* Before doing any reads assign the channel a configuration.
/*
* Before
> + * Check if channel's config is on the device
> + */
> + if (!cfg->live) {
> + /* check if config matches another one */
> + live_cfg = ad7124_find_similar_live_cfg(st, cfg);
> + if (!live_cfg) {
> + free_cfg_slot = ad7124_find_free_config_slot(st);
> + if (free_cfg_slot >= 0) {
> + ret = ad7124_write_config(st, cfg, free_cfg_slot);
> + if (ret < 0)
> + return ret;
> +
> + /* push the new config in configs queue */
> + cfg->cfg_slot = free_cfg_slot;
> + kfifo_put(&st->live_cfgs_fifo, cfg);
> + } else {
> + lru_cfg = ad7124_pop_config(st);
> + if (!lru_cfg)
> + return -EINVAL;
> +
> + /* push the new config in configs queue */
> + cfg->cfg_slot = lru_cfg->cfg_slot;
> + kfifo_put(&st->live_cfgs_fifo, cfg);
> + }
> + } else {
> + cfg->cfg_slot = live_cfg->cfg_slot;
> + }
> + }
> +
> + /* point channel to the config slot and enable */
> + return ad7124_enable_channel(st, &st->channels[address]);
> +}
> +
> +static int ad7124_set_channel(struct ad_sigma_delta *sd, unsigned int channel)
> +{
> + struct ad7124_state *st = container_of(sd, struct ad7124_state, sd);
> + int ret;
> +
> + mutex_lock(&st->cfgs_lock);
> + ret = ad7124_prepare_read(st, channel);
> + mutex_unlock(&st->cfgs_lock);
> +
> + return ret;
> +}
> +
> +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_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long info)
> @@ -378,36 +546,44 @@ static int ad7124_read_raw(struct iio_dev *indio_dev,
> return ret;
>
> /* After the conversion is performed, disable the channel */
> - ret = ad_sd_write_reg(&st->sd,
> - AD7124_CHANNEL(chan->address), 2,
> - st->channel_config[chan->address].ain |
> - AD7124_CHANNEL_EN(0));
> + ret = ad_sd_write_reg(&st->sd, AD7124_CHANNEL(chan->address), 2,
> + st->channels[chan->address].ain | AD7124_CHANNEL_EN(0));
> if (ret < 0)
> return ret;
>
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> - idx = st->channel_config[chan->address].pga_bits;
> - *val = st->channel_config[chan->address].vref_mv;
> - if (st->channel_config[chan->address].bipolar)
> + mutex_lock(&st->cfgs_lock);
> +
> + idx = st->channels[chan->address].cfg.pga_bits;
> + *val = st->channels[chan->address].cfg.vref_mv;
> + if (st->channels[chan->address].cfg.bipolar)
> *val2 = chan->scan_type.realbits - 1 + idx;
> else
> *val2 = chan->scan_type.realbits + idx;
>
> + mutex_unlock(&st->cfgs_lock);
> return IIO_VAL_FRACTIONAL_LOG2;
> case IIO_CHAN_INFO_OFFSET:
> - if (st->channel_config[chan->address].bipolar)
> + mutex_lock(&st->cfgs_lock);
> + if (st->channels[chan->address].cfg.bipolar)
> *val = -(1 << (chan->scan_type.realbits - 1));
> else
> *val = 0;
>
> + mutex_unlock(&st->cfgs_lock);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SAMP_FREQ:
> - *val = st->channel_config[chan->address].odr;
> + mutex_lock(&st->cfgs_lock);
> + *val = st->channels[chan->address].cfg.odr;
> + mutex_unlock(&st->cfgs_lock);
>
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> + mutex_lock(&st->cfgs_lock);
> *val = ad7124_get_3db_filter_freq(st, chan->scan_index);
> + mutex_unlock(&st->cfgs_lock);
> +
> return IIO_VAL_INT;
> default:
> return -EINVAL;
> @@ -421,32 +597,53 @@ static int ad7124_write_raw(struct iio_dev *indio_dev,
> struct ad7124_state *st = iio_priv(indio_dev);
> unsigned int res, gain, full_scale, vref;
>
> + mutex_lock(&st->cfgs_lock);
Taking a lock outside of a switch but then always unlocking it
in the case statements is a little unintuitive.
Can we move a
goto err;
or
break; as appropriate.
err:
mutex_unlock() path and move the unlock out of the switch
as well?
> + /* any change to the configs will make them different to
/*
* Any change...
> + * the ones written on the device
> + */
Technically we don't yet know it is a change. - Could be a rewrite
of the existing config. It might be worth identifying that there
is an actual change before we do this.
> + st->channels[chan->address].cfg.live = false;
> +
> switch (info) {
> case IIO_CHAN_INFO_SAMP_FREQ:
> - if (val2 != 0)
> + if (val2 != 0) {
> + mutex_unlock(&st->cfgs_lock);
> return -EINVAL;
> + }
>
> - return ad7124_set_channel_odr(st, chan->address, val);
> + ad7124_set_channel_odr(st, chan->address, val);
> +
> + mutex_unlock(&st->cfgs_lock);
> + return 0;
> case IIO_CHAN_INFO_SCALE:
> - if (val != 0)
> + if (val != 0) {
> + mutex_unlock(&st->cfgs_lock);
> return -EINVAL;
> + }
>
> - if (st->channel_config[chan->address].bipolar)
> + if (st->channels[chan->address].cfg.bipolar)
> full_scale = 1 << (chan->scan_type.realbits - 1);
> else
> full_scale = 1 << chan->scan_type.realbits;
>
> - vref = st->channel_config[chan->address].vref_mv * 1000000LL;
> + vref = st->channels[chan->address].cfg.vref_mv * 1000000LL;
> res = DIV_ROUND_CLOSEST(vref, full_scale);
> gain = DIV_ROUND_CLOSEST(res, val2);
> + res = ad7124_find_closest_match(ad7124_gain, ARRAY_SIZE(ad7124_gain), gain);
> + st->channels[chan->address].cfg.pga_bits = res;
>
> - return ad7124_set_channel_gain(st, chan->address, gain);
> + mutex_unlock(&st->cfgs_lock);
> + return 0;
> case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> - if (val2 != 0)
> + if (val2 != 0) {
> + mutex_unlock(&st->cfgs_lock);
> return -EINVAL;
> + }
>
> - return ad7124_set_3db_filter_freq(st, chan->address, val);
> + ad7124_set_3db_filter_freq(st, chan->address, val);
> + mutex_unlock(&st->cfgs_lock);
> + return 0;
> default:
> + mutex_unlock(&st->cfgs_lock);
> return -EINVAL;
> }
> }
> @@ -547,47 +744,14 @@ static int ad7124_check_chip_id(struct ad7124_state *st)
> return 0;
> }
>
> -static int ad7124_init_channel_vref(struct ad7124_state *st,
> - unsigned int channel_number)
> -{
> - unsigned int refsel = st->channel_config[channel_number].refsel;
> -
> - switch (refsel) {
> - case AD7124_REFIN1:
> - case AD7124_REFIN2:
> - case AD7124_AVDD_REF:
> - if (IS_ERR(st->vref[refsel])) {
> - dev_err(&st->sd.spi->dev,
> - "Error, trying to use external voltage reference without a %s regulator.\n",
> - ad7124_ref_names[refsel]);
> - return PTR_ERR(st->vref[refsel]);
> - }
> - st->channel_config[channel_number].vref_mv =
> - regulator_get_voltage(st->vref[refsel]);
> - /* Conversion from uV to mV */
> - st->channel_config[channel_number].vref_mv /= 1000;
> - break;
> - case AD7124_INT_REF:
> - st->channel_config[channel_number].vref_mv = 2500;
> - st->adc_control &= ~AD7124_ADC_CTRL_REF_EN_MSK;
> - st->adc_control |= AD7124_ADC_CTRL_REF_EN(1);
> - return ad_sd_write_reg(&st->sd, AD7124_ADC_CONTROL,
> - 2, st->adc_control);
> - default:
> - dev_err(&st->sd.spi->dev, "Invalid reference %d\n", refsel);
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
> struct device_node *np)
> {
> struct ad7124_state *st = iio_priv(indio_dev);
> + struct ad7124_channel_config *cfg;
> + struct ad7124_channel *channels;
> struct device_node *child;
> struct iio_chan_spec *chan;
> - struct ad7124_channel_config *chan_config;
> unsigned int ain[2], channel = 0, tmp;
> int ret;
>
> @@ -602,16 +766,18 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
> if (!chan)
> return -ENOMEM;
>
> - chan_config = devm_kcalloc(indio_dev->dev.parent, st->num_channels,
> - sizeof(*chan_config), GFP_KERNEL);
> - if (!chan_config)
> + channels = devm_kcalloc(indio_dev->dev.parent, st->num_channels, sizeof(*channels),
> + GFP_KERNEL);
> + if (!channels)
> return -ENOMEM;
>
> indio_dev->channels = chan;
> indio_dev->num_channels = st->num_channels;
> - st->channel_config = chan_config;
> + st->channels = channels;
>
> for_each_available_child_of_node(np, child) {
> + cfg = &st->channels[channel].cfg;
> +
> ret = of_property_read_u32(child, "reg", &channel);
> if (ret)
> goto err;
> @@ -621,21 +787,20 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
> if (ret)
> goto err;
>
> - st->channel_config[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
> + st->channels[channel].nr = channel;
> + st->channels[channel].ain = AD7124_CHANNEL_AINP(ain[0]) |
> AD7124_CHANNEL_AINM(ain[1]);
> - st->channel_config[channel].bipolar =
> - of_property_read_bool(child, "bipolar");
> +
> + cfg->bipolar = of_property_read_bool(child, "bipolar");
>
> ret = of_property_read_u32(child, "adi,reference-select", &tmp);
> if (ret)
> - st->channel_config[channel].refsel = AD7124_INT_REF;
> + cfg->refsel = AD7124_INT_REF;
> else
> - st->channel_config[channel].refsel = tmp;
> + cfg->refsel = tmp;
>
> - st->channel_config[channel].buf_positive =
> - of_property_read_bool(child, "adi,buffered-positive");
> - st->channel_config[channel].buf_negative =
> - of_property_read_bool(child, "adi,buffered-negative");
> + cfg->buf_positive = of_property_read_bool(child, "adi,buffered-positive");
> + cfg->buf_negative = of_property_read_bool(child, "adi,buffered-negative");
>
> chan[channel] = ad7124_channel_template;
> chan[channel].address = channel;
> @@ -654,7 +819,7 @@ static int ad7124_of_parse_channel_config(struct iio_dev *indio_dev,
> static int ad7124_setup(struct ad7124_state *st)
> {
> unsigned int val, fclk, power_mode;
> - int i, ret, tmp;
> + int i, ret;
>
> fclk = clk_get_rate(st->mclk);
> if (!fclk)
> @@ -677,31 +842,21 @@ static int ad7124_setup(struct ad7124_state *st)
> if (ret < 0)
> return ret;
>
> + mutex_init(&st->cfgs_lock);
> + INIT_KFIFO(st->live_cfgs_fifo);
> for (i = 0; i < st->num_channels; i++) {
> - val = st->channel_config[i].ain | AD7124_CHANNEL_SETUP(i);
> - ret = ad_sd_write_reg(&st->sd, AD7124_CHANNEL(i), 2, val);
> - if (ret < 0)
> - return ret;
> + val = st->channels[i].ain;
>
> - ret = ad7124_init_channel_vref(st, i);
> + ret = ad7124_init_config_vref(st, &st->channels[i].cfg);
> if (ret < 0)
> return ret;
>
> - tmp = (st->channel_config[i].buf_positive << 1) +
> - st->channel_config[i].buf_negative;
> -
> - val = AD7124_CONFIG_BIPOLAR(st->channel_config[i].bipolar) |
> - AD7124_CONFIG_REF_SEL(st->channel_config[i].refsel) |
> - AD7124_CONFIG_IN_BUFF(tmp);
> - ret = ad_sd_write_reg(&st->sd, AD7124_CONFIG(i), 2, val);
> - if (ret < 0)
> - return ret;
> /*
> * 9.38 SPS is the minimum output data rate supported
> * regardless of the selected power mode. Round it up to 10 and
> - * set all the enabled channels to this default value.
> + * set all channels to this default value.
> */
> - ret = ad7124_set_channel_odr(st, i, 10);
> + ad7124_set_channel_odr(st, i, 10);
> }
>
> return ret;
Powered by blists - more mailing lists