On Sun, 2019-04-14 at 13:45 +0100, Jonathan Cameron wrote:
> [External]
>
>
> On Tue, 9 Apr 2019 10:58:21 +0300
> Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> wrote:
>
> > From: Lars-Peter Clausen <lars@xxxxxxxxxx>
> >
> > Use a heap allocated memory for the SPI transfer buffer. Using stack
> > memory
> > will not work on some architectures when using DMA.
> >
> > Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
>
> Yikes, surprised this one snuck in here.
> > ---
> >
> > Note: this doesn't need a stable/fixes tag. It's more of an
> > enhancement.
>
> Talk me through this... Right now I think this is a bug that is going
> to cause random and extremely hard to debug problems. I've had these
> in the past and they are really really nasty as it's random stack
> corruption.
> The nasty is not the fact it's on the stack, but the fact that it isn't
> in it's own cacheline.
>
> So my inclination is this definitely needs stable and fixes tag.
> What am I missing?
I guess I'm not that great with discerning stuff (about whether it needs a
stable & fixes tag).
I guess I'm still in an early phase with this one.
My sentiment, is that there haven't been enough support questions about
weird issues with sigma-delta to have categorized this earlier.
But, I guess you're right. This could do with a Fixes/stable tag.
Thanks
Alex
>
> Jonathan
>
> >
> > drivers/iio/adc/ad_sigma_delta.c | 22 +++++++++++-----------
> > include/linux/iio/adc/ad_sigma_delta.h | 3 ++-
> > 2 files changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad_sigma_delta.c
> > b/drivers/iio/adc/ad_sigma_delta.c
> > index af6cbc683214..02d9049b9218 100644
> > --- a/drivers/iio/adc/ad_sigma_delta.c
> > +++ b/drivers/iio/adc/ad_sigma_delta.c
> > @@ -58,7 +58,7 @@ EXPORT_SYMBOL_GPL(ad_sd_set_comm);
> > int ad_sd_write_reg(struct ad_sigma_delta *sigma_delta, unsigned int
> > reg,
> > unsigned int size, unsigned int val)
> > {
> > - uint8_t *data = sigma_delta->data;
> > + uint8_t *data = sigma_delta->reg_data;
> > struct spi_transfer t = {
> > .tx_buf = data,
> > .len = size + 1,
> > @@ -102,7 +102,7 @@ EXPORT_SYMBOL_GPL(ad_sd_write_reg);
> > static int ad_sd_read_reg_raw(struct ad_sigma_delta *sigma_delta,
> > unsigned int reg, unsigned int size, uint8_t *val)
> > {
> > - uint8_t *data = sigma_delta->data;
> > + uint8_t *data = sigma_delta->reg_data;
> > int ret;
> > struct spi_transfer t[] = {
> > {
> > @@ -148,24 +148,24 @@ int ad_sd_read_reg(struct ad_sigma_delta
> > *sigma_delta,
> > {
> > int ret;
> >
> > - ret = ad_sd_read_reg_raw(sigma_delta, reg, size, sigma_delta-
> > >data);
> > + ret = ad_sd_read_reg_raw(sigma_delta, reg, size, sigma_delta-
> > >reg_data);
> > if (ret < 0)
> > goto out;
> >
> > switch (size) {
> > case 4:
> > - *val = get_unaligned_be32(sigma_delta->data);
> > + *val = get_unaligned_be32(sigma_delta->reg_data);
> > break;
> > case 3:
> > - *val = (sigma_delta->data[0] << 16) |
> > - (sigma_delta->data[1] << 8) |
> > - sigma_delta->data[2];
> > + *val = (sigma_delta->reg_data[0] << 16) |
> > + (sigma_delta->reg_data[1] << 8) |
> > + sigma_delta->reg_data[2];
> > break;
> > case 2:
> > - *val = get_unaligned_be16(sigma_delta->data);
> > + *val = get_unaligned_be16(sigma_delta->reg_data);
> > break;
> > case 1:
> > - *val = sigma_delta->data[0];
> > + *val = sigma_delta->reg_data[0];
> > break;
> > default:
> > ret = -EINVAL;
> > @@ -403,12 +403,12 @@ static irqreturn_t ad_sd_trigger_handler(int irq,
> > void *p)
> > struct iio_poll_func *pf = p;
> > struct iio_dev *indio_dev = pf->indio_dev;
> > struct ad_sigma_delta *sigma_delta =
> > iio_device_get_drvdata(indio_dev);
> > + uint8_t *data = sigma_delta->buf_data;
> > unsigned int reg_size;
> > unsigned int data_reg;
> > - uint8_t data[16];
> > int ret;
> >
> > - memset(data, 0x00, 16);
> > + memset(sigma_delta->buf_data, 0x00, sizeof(sigma_delta-
> > >buf_data));
> >
> > reg_size = indio_dev->channels[0].scan_type.realbits +
> > indio_dev->channels[0].scan_type.shift;
> > diff --git a/include/linux/iio/adc/ad_sigma_delta.h
> > b/include/linux/iio/adc/ad_sigma_delta.h
> > index 6e9fb1932dde..fb77b2ebe498 100644
> > --- a/include/linux/iio/adc/ad_sigma_delta.h
> > +++ b/include/linux/iio/adc/ad_sigma_delta.h
> > @@ -79,7 +79,8 @@ struct ad_sigma_delta {
> > * DMA (thus cache coherency maintenance) requires the
> > * transfer buffers to live in their own cache lines.
> > */
> > - uint8_t data[4] ____cacheline_aligned;
> > + uint8_t reg_data[4]
> > ____cacheline_aligned;
> > + uint8_t buf_data[16];
> > };
> >
> > static inline int ad_sigma_delta_set_channel(struct ad_sigma_delta
> > *sd,
>
>
