| Message ID | 20210301143256.16502-3-eugen.hristev@microchip.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | iio: adc: at91-sama5d2: add support for sama7g5 | expand |
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);
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 --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);
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(-)