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

[Eugen.Hristev]

On 06.03.2021 19:20, Jonathan Cameron wrote:
> On Mon, 1 Mar 2021 16:32:54 +0200
> Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
>> ---
>>   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);
>