| 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
| ||
|
Message-ID: <20250210185421.67d06a57@jic23-huawei>
Date: Mon, 10 Feb 2025 18:54:21 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Nuno Sá <nuno.sa@...log.com>, Uwe
Kleine-König <ukleinek@...nel.org>, Michael Hennerich
<Michael.Hennerich@...log.com>, Lars-Peter Clausen <lars@...afoo.de>, David
Jander <david@...tonic.nl>, Martin Sperl <kernel@...tin.sperl.org>,
linux-spi@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org, Jonathan Cameron
<Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v8 14/17] iio: adc: ad4695: Add support for SPI offload
On Mon, 10 Feb 2025 10:01:57 -0600
David Lechner <dlechner@...libre.com> wrote:
> On 2/7/25 2:09 PM, David Lechner wrote:
> > Add support for SPI offload to the ad4695 driver. SPI offload allows
> > sampling data at the max sample rate (500kSPS or 1MSPS).
> >
> > This is developed and tested against the ADI example FPGA design for
> > this family of ADCs [1].
> >
> > [1]: http://analogdevicesinc.github.io/hdl/projects/ad469x_fmc/index.html
> >
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> > Reviewed-by: Nuno Sa <nuno.sa@...log.com>
> > Signed-off-by: David Lechner <dlechner@...libre.com>
> > ---
> >
>
> ...
>
> > ---
> > drivers/iio/adc/Kconfig | 1 +
> > drivers/iio/adc/ad4695.c | 445 +++++++++++++++++++++++++++++++++++++++++++++--
> > 2 files changed, 429 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 995b9cacbaa964d26424346120c139858f93cdcd..ec60b64c46e187e2be18ab1f8ca9e6f4f03299f9 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -52,6 +52,7 @@ config AD4695
> > tristate "Analog Device AD4695 ADC Driver"
> > depends on SPI
> > select IIO_BUFFER
> > + select IIO_BUFFER_DMAENGINE
> > select IIO_TRIGGERED_BUFFER
> > select REGMAP
>
> I missed adding
>
> select SPI_OFFLOAD
>
> Closes: https://lore.kernel.org/oe-kbuild-all/202502090910.ganYXEeF-lkp@intel.com/
>
> > help
>
Thanks. Fixed up whilst applying.
> ...
>
>
> > +static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
> > +{
> > + struct ad4695_state *st = iio_priv(indio_dev);
> > + struct spi_offload_trigger_config config = {
> > + .type = SPI_OFFLOAD_TRIGGER_DATA_READY,
> > + };
> > + struct spi_transfer *xfer = &st->buf_read_xfer[0];
> > + struct pwm_state state;
> > + u8 temp_chan_bit = st->chip_info->num_voltage_inputs;
> > + u8 num_slots = 0;
> > + u8 temp_en = 0;
> > + unsigned int bit;
> > + int ret;
> > +
> > + iio_for_each_active_channel(indio_dev, bit) {
> > + if (bit == temp_chan_bit) {
> > + temp_en = 1;
> > + continue;
> > + }
> > +
> > + ret = regmap_write(st->regmap, AD4695_REG_AS_SLOT(num_slots),
> > + FIELD_PREP(AD4695_REG_AS_SLOT_INX, bit));
> > + if (ret)
> > + return ret;
> > +
> > + num_slots++;
> > + }
> > +
> > + /*
> > + * For non-offload, we could discard data to work around this
> > + * restriction, but with offload, that is not possible.
> > + */
> > + if (num_slots < 2) {
> > + dev_err(&st->spi->dev,
> > + "At least two voltage channels must be enabled.\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = regmap_update_bits(st->regmap, AD4695_REG_TEMP_CTRL,
> > + AD4695_REG_TEMP_CTRL_TEMP_EN,
> > + FIELD_PREP(AD4695_REG_TEMP_CTRL_TEMP_EN,
> > + temp_en));
> > + if (ret)
> > + return ret;
> > +
> > + /* Each BUSY event means just one sample for one channel is ready. */
> > + memset(xfer, 0, sizeof(*xfer));
> > + xfer->offload_flags = SPI_OFFLOAD_XFER_RX_STREAM;
> > + /* Using 19 bits per word to allow for possible oversampling */
> > + xfer->bits_per_word = 19;
> > + xfer->len = 4;
> > +
> > + spi_message_init_with_transfers(&st->buf_read_msg, xfer, 1);
> > + st->buf_read_msg.offload = st->offload;
> > +
> > + ret = spi_optimize_message(st->spi, &st->buf_read_msg);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * NB: technically, this is part the SPI offload trigger enable, but it
> > + * doesn't work to call it from the offload trigger enable callback
> > + * because it requires accessing the SPI bus. Calling it from the
> > + * trigger enable callback could cause a deadlock.
> > + */
> > + ret = regmap_set_bits(st->regmap, AD4695_REG_GP_MODE,
> > + AD4695_REG_GP_MODE_BUSY_GP_EN);
> > + if (ret)
> > + goto err_unoptimize_message;
> > +
> > + ret = spi_offload_trigger_enable(st->offload, st->offload_trigger,
> > + &config);
> > + if (ret)
> > + goto err_disable_busy_output;
> > +
> > + ret = ad4695_enter_advanced_sequencer_mode(st, num_slots);
> > + if (ret)
> > + goto err_offload_trigger_disable;
> > +
> > + guard(mutex)(&st->cnv_pwm_lock);
>
> Apparently clang doesn't like this guard() after goto, so I'll have to figure
> out what to do about that. Probably need to add a helper function to avoid
> goto below from jumping out of scoped_guard().
>
> https://lore.kernel.org/oe-kbuild-all/202502090806.KxEvxCZC-lkp@intel.com/
Easiest option don't use guard(). Sometimes they are not the right choice!
diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
index d73d2ff77625..d1e7634001b3 100644
--- a/drivers/iio/adc/ad4695.c
+++ b/drivers/iio/adc/ad4695.c
@@ -800,7 +800,7 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
if (ret)
goto err_offload_trigger_disable;
- guard(mutex)(&st->cnv_pwm_lock);
+ mutex_lock(&st->cnv_pwm_lock);
pwm_get_state(st->cnv_pwm, &state);
/*
* PWM subsystem generally rounds down, so requesting 2x minimum high
@@ -808,6 +808,7 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
*/
state.duty_cycle = AD4695_T_CNVH_NS * 2;
ret = pwm_apply_might_sleep(st->cnv_pwm, &state);
+ mutex_unlock(st->cnv_pwm_lock);
if (ret)
goto err_offload_exit_conversion_mode;
For now I've applied this as I want to get some build testing on this series, but
feel free to send a patch to tweak this to a different solution.
Jonathan
>
> > + pwm_get_state(st->cnv_pwm, &state);
> > + /*
> > + * PWM subsystem generally rounds down, so requesting 2x minimum high
> > + * time ensures that we meet the minimum high time in any case.
> > + */
> > + state.duty_cycle = AD4695_T_CNVH_NS * 2;
> > + ret = pwm_apply_might_sleep(st->cnv_pwm, &state);
> > + if (ret)
> > + goto err_offload_exit_conversion_mode;
> > +
> > + return 0;
> > +
> > +err_offload_exit_conversion_mode:
> > + /*
> > + * We have to unwind in a different order to avoid triggering offload.
> > + * ad4695_exit_conversion_mode() triggers a conversion, so it has to be
> > + * done after spi_offload_trigger_disable().
> > + */
> > + spi_offload_trigger_disable(st->offload, st->offload_trigger);
> > + ad4695_exit_conversion_mode(st);
> > + goto err_disable_busy_output;
> > +
> > +err_offload_trigger_disable:
> > + spi_offload_trigger_disable(st->offload, st->offload_trigger);
> > +
> > +err_disable_busy_output:
> > + regmap_clear_bits(st->regmap, AD4695_REG_GP_MODE,
> > + AD4695_REG_GP_MODE_BUSY_GP_EN);
> > +
> > +err_unoptimize_message:
> > + spi_unoptimize_message(&st->buf_read_msg);
> > +
> > + return ret;
> > +}
Powered by blists - more mailing lists