2020-03-04 08:39:47

by Alexandru Ardelean

[permalink] [raw]
Subject: [PATCH v2 2/2] iio: at91-sama5d2_adc: adjust iio_triggered_buffer_{predisable,postenable} positions

The iio_triggered_buffer_{predisable,postenable} functions attach/detach
poll functions.

In most cases the iio_triggered_buffer_postenable() should be called first
to attach the poll function, and then the driver can init the data to be
triggered.
In this case it's the other way around: the DMA code should be initialized
before the attaching the poll function and the reverse should be done when
un-initializing.

To make things easier when removing the iio_triggered_buffer_postenable() &
iio_triggered_buffer_predisable() functions from the IIO core API, the DMA
code has been moved into preenable() for init, and postdisable() for
uninit.

Signed-off-by: Alexandru Ardelean <[email protected]>
---
drivers/iio/adc/at91-sama5d2_adc.c | 32 ++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index f2a74c47c768..2e01073d401d 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -882,7 +882,7 @@ static bool at91_adc_current_chan_is_touch(struct iio_dev *indio_dev)
AT91_SAMA5D2_MAX_CHAN_IDX + 1);
}

-static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
+static int at91_adc_buffer_preenable(struct iio_dev *indio_dev)
{
int ret;
struct at91_adc_state *st = iio_priv(indio_dev);
@@ -902,13 +902,20 @@ static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
return ret;
}

+ return 0;
+}
+
+static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
+{
+ if (at91_adc_current_chan_is_touch(indio_dev))
+ return 0;
+
return iio_triggered_buffer_postenable(indio_dev);
}

-static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
+static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
{
struct at91_adc_state *st = iio_priv(indio_dev);
- int ret;
u8 bit;

/* check if we are disabling triggered buffer or the touchscreen */
@@ -919,13 +926,8 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
if (!(indio_dev->currentmode & INDIO_ALL_TRIGGERED_MODES))
return -EINVAL;

- /* continue with the triggered buffer */
- ret = iio_triggered_buffer_predisable(indio_dev);
- if (ret < 0)
- dev_err(&indio_dev->dev, "buffer predisable failed\n");
-
if (!st->dma_st.dma_chan)
- return ret;
+ return 0;

/* if we are using DMA we must clear registers and end DMA */
dmaengine_terminate_sync(st->dma_st.dma_chan);
@@ -952,10 +954,20 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)

/* read overflow register to clear possible overflow status */
at91_adc_readl(st, AT91_SAMA5D2_OVER);
- return ret;
+ return 0;
+}
+
+static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
+{
+ if (at91_adc_current_chan_is_touch(indio_dev))
+ return 0;
+
+ return iio_triggered_buffer_predisable(indio_dev);
}

static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
+ .preenable = &at91_adc_buffer_preenable,
+ .postdisable = &at91_adc_buffer_postdisable,
.postenable = &at91_adc_buffer_postenable,
.predisable = &at91_adc_buffer_predisable,
};
--
2.20.1


2020-04-19 10:07:37

by Ludovic Desroches

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] iio: at91-sama5d2_adc: adjust iio_triggered_buffer_{predisable,postenable} positions

On Wed, Mar 04, 2020 at 10:42:19AM +0200, Alexandru Ardelean wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The iio_triggered_buffer_{predisable,postenable} functions attach/detach
> poll functions.
>
> In most cases the iio_triggered_buffer_postenable() should be called first
> to attach the poll function, and then the driver can init the data to be
> triggered.
> In this case it's the other way around: the DMA code should be initialized
> before the attaching the poll function and the reverse should be done when
> un-initializing.
>
> To make things easier when removing the iio_triggered_buffer_postenable() &
> iio_triggered_buffer_predisable() functions from the IIO core API, the DMA
> code has been moved into preenable() for init, and postdisable() for
> uninit.
>
> Signed-off-by: Alexandru Ardelean <[email protected]>

Reviewed-by: Ludovic Desroches <[email protected]>

> ---
> drivers/iio/adc/at91-sama5d2_adc.c | 32 ++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index f2a74c47c768..2e01073d401d 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -882,7 +882,7 @@ static bool at91_adc_current_chan_is_touch(struct iio_dev *indio_dev)
> AT91_SAMA5D2_MAX_CHAN_IDX + 1);
> }
>
> -static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
> +static int at91_adc_buffer_preenable(struct iio_dev *indio_dev)
> {
> int ret;
> struct at91_adc_state *st = iio_priv(indio_dev);
> @@ -902,13 +902,20 @@ static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
> return ret;
> }
>
> + return 0;
> +}
> +
> +static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + if (at91_adc_current_chan_is_touch(indio_dev))
> + return 0;
> +
> return iio_triggered_buffer_postenable(indio_dev);
> }
>
> -static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> +static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
> {
> struct at91_adc_state *st = iio_priv(indio_dev);
> - int ret;
> u8 bit;
>
> /* check if we are disabling triggered buffer or the touchscreen */
> @@ -919,13 +926,8 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> if (!(indio_dev->currentmode & INDIO_ALL_TRIGGERED_MODES))
> return -EINVAL;
>
> - /* continue with the triggered buffer */
> - ret = iio_triggered_buffer_predisable(indio_dev);
> - if (ret < 0)
> - dev_err(&indio_dev->dev, "buffer predisable failed\n");
> -
> if (!st->dma_st.dma_chan)
> - return ret;
> + return 0;
>
> /* if we are using DMA we must clear registers and end DMA */
> dmaengine_terminate_sync(st->dma_st.dma_chan);
> @@ -952,10 +954,20 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
>
> /* read overflow register to clear possible overflow status */
> at91_adc_readl(st, AT91_SAMA5D2_OVER);
> - return ret;
> + return 0;
> +}
> +
> +static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + if (at91_adc_current_chan_is_touch(indio_dev))
> + return 0;
> +
> + return iio_triggered_buffer_predisable(indio_dev);
> }
>
> static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
> + .preenable = &at91_adc_buffer_preenable,
> + .postdisable = &at91_adc_buffer_postdisable,
> .postenable = &at91_adc_buffer_postenable,
> .predisable = &at91_adc_buffer_predisable,
> };
> --
> 2.20.1
>

2020-04-23 10:58:58

by Eugen Hristev

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] iio: at91-sama5d2_adc: adjust iio_triggered_buffer_{predisable,postenable} positions

On 19.04.2020 13:04, [email protected] wrote:
> On Wed, Mar 04, 2020 at 10:42:19AM +0200, Alexandru Ardelean wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> The iio_triggered_buffer_{predisable,postenable} functions attach/detach
>> poll functions.
>>
>> In most cases the iio_triggered_buffer_postenable() should be called first
>> to attach the poll function, and then the driver can init the data to be
>> triggered.
>> In this case it's the other way around: the DMA code should be initialized
>> before the attaching the poll function and the reverse should be done when
>> un-initializing.
>>
>> To make things easier when removing the iio_triggered_buffer_postenable() &
>> iio_triggered_buffer_predisable() functions from the IIO core API, the DMA
>> code has been moved into preenable() for init, and postdisable() for
>> uninit.
>>
>> Signed-off-by: Alexandru Ardelean <[email protected]>
>
> Reviewed-by: Ludovic Desroches <[email protected]>

Reviewed-by: Eugen Hristev <[email protected]>

Hi Jonathan,

Please don't forget my other 2 patches on this driver, they've been
patiently waiting for this merge window

Thanks!

>
>> ---
>> drivers/iio/adc/at91-sama5d2_adc.c | 32 ++++++++++++++++++++----------
>> 1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
>> index f2a74c47c768..2e01073d401d 100644
>> --- a/drivers/iio/adc/at91-sama5d2_adc.c
>> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
>> @@ -882,7 +882,7 @@ static bool at91_adc_current_chan_is_touch(struct iio_dev *indio_dev)
>> AT91_SAMA5D2_MAX_CHAN_IDX + 1);
>> }
>>
>> -static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
>> +static int at91_adc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> int ret;
>> struct at91_adc_state *st = iio_priv(indio_dev);
>> @@ -902,13 +902,20 @@ static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
>> return ret;
>> }
>>
>> + return 0;
>> +}
>> +
>> +static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
>> +{
>> + if (at91_adc_current_chan_is_touch(indio_dev))
>> + return 0;
>> +
>> return iio_triggered_buffer_postenable(indio_dev);
>> }
>>
>> -static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
>> +static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
>> {
>> struct at91_adc_state *st = iio_priv(indio_dev);
>> - int ret;
>> u8 bit;
>>
>> /* check if we are disabling triggered buffer or the touchscreen */
>> @@ -919,13 +926,8 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
>> if (!(indio_dev->currentmode & INDIO_ALL_TRIGGERED_MODES))
>> return -EINVAL;
>>
>> - /* continue with the triggered buffer */
>> - ret = iio_triggered_buffer_predisable(indio_dev);
>> - if (ret < 0)
>> - dev_err(&indio_dev->dev, "buffer predisable failed\n");
>> -
>> if (!st->dma_st.dma_chan)
>> - return ret;
>> + return 0;
>>
>> /* if we are using DMA we must clear registers and end DMA */
>> dmaengine_terminate_sync(st->dma_st.dma_chan);
>> @@ -952,10 +954,20 @@ static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
>>
>> /* read overflow register to clear possible overflow status */
>> at91_adc_readl(st, AT91_SAMA5D2_OVER);
>> - return ret;
>> + return 0;
>> +}
>> +
>> +static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
>> +{
>> + if (at91_adc_current_chan_is_touch(indio_dev))
>> + return 0;
>> +
>> + return iio_triggered_buffer_predisable(indio_dev);
>> }
>>
>> static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
>> + .preenable = &at91_adc_buffer_preenable,
>> + .postdisable = &at91_adc_buffer_postdisable,
>> .postenable = &at91_adc_buffer_postenable,
>> .predisable = &at91_adc_buffer_predisable,
>> };
>> --
>> 2.20.1
>>