diff mbox series

[2/4] iio: adc: at91-sama5d2: initialize hardware after clock is started

Message ID 20210301143256.16502-3-eugen.hristev@microchip.com (mailing list archive)
State New
Headers show
Series iio: adc: at91-sama5d2: add support for sama7g5 | expand

Commit Message

Eugen Hristev March 1, 2021, 2:32 p.m. UTC
The hw_init hardware init call must happen after the clock is prepared and
enabled. Otherwise, writing to the registers might lead to a block or
external abort.

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/iio/adc/at91-sama5d2_adc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jonathan Cameron March 6, 2021, 5:20 p.m. UTC | #1
On Mon, 1 Mar 2021 16:32:54 +0200
Eugen Hristev <eugen.hristev@microchip.com> wrote:

> The hw_init hardware init call must happen after the clock is prepared and
> enabled. Otherwise, writing to the registers might lead to a block or
> external abort.

Fix for existing parts or something only needed for the new devices?
If it's a fix we should be looking to back port it so please
provide me with a fixes tag.

If it's a fix but not super urgent then let me know and we can
take it with the rest of this series (and hence keep things simple)

Thanks,

Jonathan

> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
>  drivers/iio/adc/at91-sama5d2_adc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
> index a7826f097b95..63325f037f09 100644
> --- a/drivers/iio/adc/at91-sama5d2_adc.c
> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
> @@ -1832,12 +1832,12 @@ static int at91_adc_probe(struct platform_device *pdev)
>  		goto vref_disable;
>  	}
>  
> -	at91_adc_hw_init(indio_dev);
> -
>  	ret = clk_prepare_enable(st->per_clk);
>  	if (ret)
>  		goto vref_disable;
>  
> +	at91_adc_hw_init(indio_dev);
> +
>  	platform_set_drvdata(pdev, indio_dev);
>  
>  	ret = at91_adc_buffer_and_trigger_init(&pdev->dev, indio_dev);
Eugen Hristev March 8, 2021, 1:33 p.m. UTC | #2
On 06.03.2021 19:20, Jonathan Cameron wrote:
> On Mon, 1 Mar 2021 16:32:54 +0200
> Eugen Hristev <eugen.hristev@microchip.com> wrote:
> 
>> The hw_init hardware init call must happen after the clock is prepared and
>> enabled. Otherwise, writing to the registers might lead to a block or
>> external abort.
> 
> Fix for existing parts or something only needed for the new devices?
> If it's a fix we should be looking to back port it so please
> provide me with a fixes tag.
> 
> If it's a fix but not super urgent then let me know and we can
> take it with the rest of this series (and hence keep things simple)

Hi Jonathan,

It's not super urgent.
Actually this issue could potentially impact other parts, but it was 
never hit, as the clocking of the ADC block in older products is done 
differently and the bridge connected to the block does not halt if 
requests are sent to an unclocked ADC. Most likely they are buffered and 
served once clock ticks.

For the sama7g5, the ADC is in an asynchronous part of the architecture 
that is clocked by a generic clock that must be on, otherwise the full 
AXI4 will stall because no reply is coming as the ADC is not clocked.
I do not fully grasp it, but this is my understanding.

Anyway it makes sense for all products to not read/write registers in 
the block if the clock is not started yet.

Eugen

> 
> Thanks,
> 
> Jonathan
> 
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
>>   drivers/iio/adc/at91-sama5d2_adc.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
>> index a7826f097b95..63325f037f09 100644
>> --- a/drivers/iio/adc/at91-sama5d2_adc.c
>> +++ b/drivers/iio/adc/at91-sama5d2_adc.c
>> @@ -1832,12 +1832,12 @@ static int at91_adc_probe(struct platform_device *pdev)
>>                goto vref_disable;
>>        }
>>
>> -     at91_adc_hw_init(indio_dev);
>> -
>>        ret = clk_prepare_enable(st->per_clk);
>>        if (ret)
>>                goto vref_disable;
>>
>> +     at91_adc_hw_init(indio_dev);
>> +
>>        platform_set_drvdata(pdev, indio_dev);
>>
>>        ret = at91_adc_buffer_and_trigger_init(&pdev->dev, indio_dev);
>
diff mbox series

Patch

diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index a7826f097b95..63325f037f09 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -1832,12 +1832,12 @@  static int at91_adc_probe(struct platform_device *pdev)
 		goto vref_disable;
 	}
 
-	at91_adc_hw_init(indio_dev);
-
 	ret = clk_prepare_enable(st->per_clk);
 	if (ret)
 		goto vref_disable;
 
+	at91_adc_hw_init(indio_dev);
+
 	platform_set_drvdata(pdev, indio_dev);
 
 	ret = at91_adc_buffer_and_trigger_init(&pdev->dev, indio_dev);