On Fri, 3 Apr 2020 15:27:16 +0200
Lars-Peter Clausen <lars@xxxxxxxxxx> wrote:
> The XADC supports a samplerate of up to 1MSPS. Unfortunately the hardware
> does not have a FIFO, which means it generates an interrupt for each
> conversion sequence. At one 1MSPS this creates an interrupt storm that
> causes the system to soft-lock.
>
> For this reason the driver limits the maximum samplerate to 150kSPS.
> Currently this check is only done when setting a new samplerate. But it is
> also possible that the initial samplerate configured in the FPGA bitstream
> exceeds the limit.
>
> In this case when starting to capture data without first changing the
> samplerate the system can overload.
>
> To prevent this check the currently configured samplerate in the probe
> function and reduce it to the maximum if necessary.
>
> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
Looks sensible to me. This one is a bit more marginal as a 'fix' or
a workaround for silly fpga configurations. Still probably best to
just give it a fixes tag and merge it with the reset of the series.
Thanks,
Jonathan
> ---
> drivers/iio/adc/xilinx-xadc-core.c | 78 +++++++++++++++++++++++-------
> 1 file changed, 60 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
> index 4acababda4d5..c724b8788007 100644
> --- a/drivers/iio/adc/xilinx-xadc-core.c
> +++ b/drivers/iio/adc/xilinx-xadc-core.c
> @@ -102,6 +102,16 @@ static const unsigned int XADC_ZYNQ_UNMASK_TIMEOUT = 500;
>
> #define XADC_FLAGS_BUFFERED BIT(0)
>
> +/*
> + * The XADC hardware supports a samplerate of up to 1MSPS. Unfortunately it does
> + * not have a hardware FIFO. Which means an interrupt is generated for each
> + * conversion sequence. At 1MSPS sample rate the CPU in ZYNQ7000 is completely
> + * overloaded by the interrupts that it soft-lockups. For this reason the driver
> + * limits the maximum samplerate 150kSPS. At this rate the CPU is fairly busy,
> + * but still responsive.
> + */
> +#define XADC_MAX_SAMPLERATE 150000
> +
> static void xadc_write_reg(struct xadc *xadc, unsigned int reg,
> uint32_t val)
> {
> @@ -834,11 +844,27 @@ static const struct iio_buffer_setup_ops xadc_buffer_ops = {
> .postdisable = &xadc_postdisable,
> };
>
> +static int xadc_read_samplerate(struct xadc *xadc)
> +{
> + unsigned int div;
> + uint16_t val16;
> + int ret;
> +
> + ret = xadc_read_adc_reg(xadc, XADC_REG_CONF2, &val16);
> + if (ret)
> + return ret;
> +
> + div = (val16 & XADC_CONF2_DIV_MASK) >> XADC_CONF2_DIV_OFFSET;
> + if (div < 2)
> + div = 2;
> +
> + return xadc_get_dclk_rate(xadc) / div / 26;
> +}
> +
> static int xadc_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan, int *val, int *val2, long info)
> {
> struct xadc *xadc = iio_priv(indio_dev);
> - unsigned int div;
> uint16_t val16;
> int ret;
>
> @@ -891,41 +917,31 @@ static int xadc_read_raw(struct iio_dev *indio_dev,
> *val = -((273150 << 12) / 503975);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SAMP_FREQ:
> - ret = xadc_read_adc_reg(xadc, XADC_REG_CONF2, &val16);
> - if (ret)
> + ret = xadc_read_samplerate(xadc);
> + if (ret < 0)
> return ret;
>
> - div = (val16 & XADC_CONF2_DIV_MASK) >> XADC_CONF2_DIV_OFFSET;
> - if (div < 2)
> - div = 2;
> -
> - *val = xadc_get_dclk_rate(xadc) / div / 26;
> -
> + *val = ret;
> return IIO_VAL_INT;
> default:
> return -EINVAL;
> }
> }
>
> -static int xadc_write_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan, int val, int val2, long info)
> +static int xadc_write_samplerate(struct xadc *xadc, int val)
> {
> - struct xadc *xadc = iio_priv(indio_dev);
> unsigned long clk_rate = xadc_get_dclk_rate(xadc);
> unsigned int div;
>
> if (!clk_rate)
> return -EINVAL;
>
> - if (info != IIO_CHAN_INFO_SAMP_FREQ)
> - return -EINVAL;
> -
> if (val <= 0)
> return -EINVAL;
>
> /* Max. 150 kSPS */
> - if (val > 150000)
> - val = 150000;
> + if (val > XADC_MAX_SAMPLERATE)
> + val = XADC_MAX_SAMPLERATE;
>
> val *= 26;
>
> @@ -938,7 +954,7 @@ static int xadc_write_raw(struct iio_dev *indio_dev,
> * limit.
> */
> div = clk_rate / val;
> - if (clk_rate / div / 26 > 150000)
> + if (clk_rate / div / 26 > XADC_MAX_SAMPLERATE)
> div++;
> if (div < 2)
> div = 2;
> @@ -949,6 +965,17 @@ static int xadc_write_raw(struct iio_dev *indio_dev,
> div << XADC_CONF2_DIV_OFFSET);
> }
>
> +static int xadc_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int val, int val2, long info)
> +{
> + struct xadc *xadc = iio_priv(indio_dev);
> +
> + if (info != IIO_CHAN_INFO_SAMP_FREQ)
> + return -EINVAL;
> +
> + return xadc_write_samplerate(xadc, val);
> +}
> +
> static const struct iio_event_spec xadc_temp_events[] = {
> {
> .type = IIO_EV_TYPE_THRESH,
> @@ -1234,6 +1261,21 @@ static int xadc_probe(struct platform_device *pdev)
> if (ret)
> goto err_free_samplerate_trigger;
>
> + /*
> + * Make sure not to exceed the maximum samplerate since otherwise the
> + * resulting interrupt storm will soft-lock the system.
> + */
> + if (xadc->ops->flags & XADC_FLAGS_BUFFERED) {
> + ret = xadc_read_samplerate(xadc);
> + if (ret < 0)
> + goto err_free_samplerate_trigger;
> + if (ret > XADC_MAX_SAMPLERATE) {
> + ret = xadc_write_samplerate(xadc, XADC_MAX_SAMPLERATE);
> + if (ret < 0)
> + goto err_free_samplerate_trigger;
> + }
> + }
> +
> ret = request_irq(xadc->irq, xadc->ops->interrupt_handler, 0,
> dev_name(&pdev->dev), indio_dev);
> if (ret)
