2020-11-06 16:59:58

by Fabrice Gasnier

Subject: [PATCH] iio: adc: stm32-adc: adapt clock duty cycle for proper operation

For proper operation, STM32 ADC should be used with a clock duty cycle
of 50%, in the range of 49% to 51%. Depending on the clock tree, divider
can be used in case clock duty cycle is out of this range.
In case clk_get_scaled_duty_cycle() returns an error, kindly apply a
divider by default (don't make the probe fail).

Signed-off-by: Fabrice Gasnier <[email protected]>
---
drivers/iio/adc/stm32-adc-core.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
index cd870c0..d64a9e8 100644
--- a/drivers/iio/adc/stm32-adc-core.c
+++ b/drivers/iio/adc/stm32-adc-core.c
@@ -202,7 +202,7 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
{
u32 ckmode, presc, val;
unsigned long rate;
- int i, div;
+ int i, div, duty;

/* stm32h7 bus clock is common for all ADC instances (mandatory) */
if (!priv->bclk) {
@@ -226,6 +226,11 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
return -EINVAL;
}

+ /* If duty is an error, kindly use at least /2 divider */
+ duty = clk_get_scaled_duty_cycle(priv->aclk, 100);
+ if (duty < 0)
+ dev_warn(&pdev->dev, "adc clock duty: %d\n", duty);
+
for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
presc = stm32h7_adc_ckmodes_spec[i].presc;
@@ -234,6 +239,13 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
if (ckmode)
continue;

+ /*
+ * For proper operation, clock duty cycle range is 49%
+ * to 51%. Apply at least /2 prescaler otherwise.
+ */
+ if (div == 1 && (duty < 49 || duty > 51))
+ continue;
+
if ((rate / div) <= priv->max_clk_rate)
goto out;
}
@@ -246,6 +258,10 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
return -EINVAL;
}

+ duty = clk_get_scaled_duty_cycle(priv->bclk, 100);
+ if (duty < 0)
+ dev_warn(&pdev->dev, "bus clock duty: %d\n", duty);
+
for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
presc = stm32h7_adc_ckmodes_spec[i].presc;
@@ -254,6 +270,9 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
if (!ckmode)
continue;

+ if (div == 1 && (duty < 49 || duty > 51))
+ continue;
+
if ((rate / div) <= priv->max_clk_rate)
goto out;
}
--
2.7.4

2020-11-08 15:20:23

by Jonathan Cameron

Subject: Re: [PATCH] iio: adc: stm32-adc: adapt clock duty cycle for proper operation

On Fri, 6 Nov 2020 17:57:26 +0100
Fabrice Gasnier <[email protected]> wrote:

> For proper operation, STM32 ADC should be used with a clock duty cycle
> of 50%, in the range of 49% to 51%. Depending on the clock tree, divider
> can be used in case clock duty cycle is out of this range.
> In case clk_get_scaled_duty_cycle() returns an error, kindly apply a
> divider by default (don't make the probe fail).
>
> Signed-off-by: Fabrice Gasnier <[email protected]>
Hi Fabrice,

This sounds like it's a fix for a situation in which the device is not
currently working? If so, please let me know a fixes tag.

Thanks,

Jonathan

> ---
> drivers/iio/adc/stm32-adc-core.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
> index cd870c0..d64a9e8 100644
> --- a/drivers/iio/adc/stm32-adc-core.c
> +++ b/drivers/iio/adc/stm32-adc-core.c
> @@ -202,7 +202,7 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> {
> u32 ckmode, presc, val;
> unsigned long rate;
> - int i, div;
> + int i, div, duty;
>
> /* stm32h7 bus clock is common for all ADC instances (mandatory) */
> if (!priv->bclk) {
> @@ -226,6 +226,11 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> return -EINVAL;
> }
>
> + /* If duty is an error, kindly use at least /2 divider */
> + duty = clk_get_scaled_duty_cycle(priv->aclk, 100);
> + if (duty < 0)
> + dev_warn(&pdev->dev, "adc clock duty: %d\n", duty);
> +
> for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
> ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
> presc = stm32h7_adc_ckmodes_spec[i].presc;
> @@ -234,6 +239,13 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> if (ckmode)
> continue;
>
> + /*
> + * For proper operation, clock duty cycle range is 49%
> + * to 51%. Apply at least /2 prescaler otherwise.
> + */
> + if (div == 1 && (duty < 49 || duty > 51))
> + continue;
> +
> if ((rate / div) <= priv->max_clk_rate)
> goto out;
> }
> @@ -246,6 +258,10 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> return -EINVAL;
> }
>
> + duty = clk_get_scaled_duty_cycle(priv->bclk, 100);
> + if (duty < 0)
> + dev_warn(&pdev->dev, "bus clock duty: %d\n", duty);
> +
> for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
> ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
> presc = stm32h7_adc_ckmodes_spec[i].presc;
> @@ -254,6 +270,9 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> if (!ckmode)
> continue;
>
> + if (div == 1 && (duty < 49 || duty > 51))
> + continue;
> +
> if ((rate / div) <= priv->max_clk_rate)
> goto out;
> }

2020-11-13 08:31:03

by Fabrice Gasnier

Subject: Re: [PATCH] iio: adc: stm32-adc: adapt clock duty cycle for proper operation

On 11/8/20 4:18 PM, Jonathan Cameron wrote:
> On Fri, 6 Nov 2020 17:57:26 +0100
> Fabrice Gasnier <[email protected]> wrote:
>
>> For proper operation, STM32 ADC should be used with a clock duty cycle
>> of 50%, in the range of 49% to 51%. Depending on the clock tree, divider
>> can be used in case clock duty cycle is out of this range.
>> In case clk_get_scaled_duty_cycle() returns an error, kindly apply a
>> divider by default (don't make the probe fail).
>>
>> Signed-off-by: Fabrice Gasnier <[email protected]>
> Hi Fabrice,
>
> This sounds like it's a fix for a situation in which the device is not
> currently working? If so, please let me know a fixes tag.

Hi Jonathan,

That's a good point. I also thought about adding a fixes tag. Currently
I think this can't be hit upstream, as clock tree is tuned to fit duty
cycle constraints. So far, nobody seems to complain about it. So this
can probably go through the normal tree.

Thanks,
Fabrice

>
> Thanks,
>
> Jonathan
>
>> ---
>> drivers/iio/adc/stm32-adc-core.c | 21 ++++++++++++++++++++-
>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
>> index cd870c0..d64a9e8 100644
>> --- a/drivers/iio/adc/stm32-adc-core.c
>> +++ b/drivers/iio/adc/stm32-adc-core.c
>> @@ -202,7 +202,7 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
>> {
>> u32 ckmode, presc, val;
>> unsigned long rate;
>> - int i, div;
>> + int i, div, duty;
>>
>> /* stm32h7 bus clock is common for all ADC instances (mandatory) */
>> if (!priv->bclk) {
>> @@ -226,6 +226,11 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
>> return -EINVAL;
>> }
>>
>> + /* If duty is an error, kindly use at least /2 divider */
>> + duty = clk_get_scaled_duty_cycle(priv->aclk, 100);
>> + if (duty < 0)
>> + dev_warn(&pdev->dev, "adc clock duty: %d\n", duty);
>> +
>> for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
>> ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
>> presc = stm32h7_adc_ckmodes_spec[i].presc;
>> @@ -234,6 +239,13 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
>> if (ckmode)
>> continue;
>>
>> + /*
>> + * For proper operation, clock duty cycle range is 49%
>> + * to 51%. Apply at least /2 prescaler otherwise.
>> + */
>> + if (div == 1 && (duty < 49 || duty > 51))
>> + continue;
>> +
>> if ((rate / div) <= priv->max_clk_rate)
>> goto out;
>> }
>> @@ -246,6 +258,10 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
>> return -EINVAL;
>> }
>>
>> + duty = clk_get_scaled_duty_cycle(priv->bclk, 100);
>> + if (duty < 0)
>> + dev_warn(&pdev->dev, "bus clock duty: %d\n", duty);
>> +
>> for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
>> ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
>> presc = stm32h7_adc_ckmodes_spec[i].presc;
>> @@ -254,6 +270,9 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
>> if (!ckmode)
>> continue;
>>
>> + if (div == 1 && (duty < 49 || duty > 51))
>> + continue;
>> +
>> if ((rate / div) <= priv->max_clk_rate)
>> goto out;
>> }
>

2020-11-14 15:55:30

by Jonathan Cameron

Subject: Re: [PATCH] iio: adc: stm32-adc: adapt clock duty cycle for proper operation

On Fri, 13 Nov 2020 09:27:03 +0100
Fabrice Gasnier <[email protected]> wrote:

> On 11/8/20 4:18 PM, Jonathan Cameron wrote:
> > On Fri, 6 Nov 2020 17:57:26 +0100
> > Fabrice Gasnier <[email protected]> wrote:
> >
> >> For proper operation, STM32 ADC should be used with a clock duty cycle
> >> of 50%, in the range of 49% to 51%. Depending on the clock tree, divider
> >> can be used in case clock duty cycle is out of this range.
> >> In case clk_get_scaled_duty_cycle() returns an error, kindly apply a
> >> divider by default (don't make the probe fail).
> >>
> >> Signed-off-by: Fabrice Gasnier <[email protected]>
> > Hi Fabrice,
> >
> > This sounds like it's a fix for a situation in which the device is not
> > currently working? If so, please let me know a fixes tag.
>
> Hi Jonathan,
>
> That's a good point. I also thought about adding a fixes tag. Currently
> I think this can't be hit upstream, as clock tree is tuned to fit duty
> cycle constraints. So far, nobody seems to complain about it. So this
> can probably go through the normal tree.
>
Applied. Will be interesting to see if the bot finds this one as a possible
candidate for backports. I'll keep an eye on those coming through and suggest
this isn't backported if it does show up.

Thanks,

Jonathan
> Thanks,
> Fabrice
>
> >
> > Thanks,
> >
> > Jonathan
> >
> >> ---
> >> drivers/iio/adc/stm32-adc-core.c | 21 ++++++++++++++++++++-
> >> 1 file changed, 20 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
> >> index cd870c0..d64a9e8 100644
> >> --- a/drivers/iio/adc/stm32-adc-core.c
> >> +++ b/drivers/iio/adc/stm32-adc-core.c
> >> @@ -202,7 +202,7 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> >> {
> >> u32 ckmode, presc, val;
> >> unsigned long rate;
> >> - int i, div;
> >> + int i, div, duty;
> >>
> >> /* stm32h7 bus clock is common for all ADC instances (mandatory) */
> >> if (!priv->bclk) {
> >> @@ -226,6 +226,11 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> >> return -EINVAL;
> >> }
> >>
> >> + /* If duty is an error, kindly use at least /2 divider */
> >> + duty = clk_get_scaled_duty_cycle(priv->aclk, 100);
> >> + if (duty < 0)
> >> + dev_warn(&pdev->dev, "adc clock duty: %d\n", duty);
> >> +
> >> for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
> >> ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
> >> presc = stm32h7_adc_ckmodes_spec[i].presc;
> >> @@ -234,6 +239,13 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> >> if (ckmode)
> >> continue;
> >>
> >> + /*
> >> + * For proper operation, clock duty cycle range is 49%
> >> + * to 51%. Apply at least /2 prescaler otherwise.
> >> + */
> >> + if (div == 1 && (duty < 49 || duty > 51))
> >> + continue;
> >> +
> >> if ((rate / div) <= priv->max_clk_rate)
> >> goto out;
> >> }
> >> @@ -246,6 +258,10 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> >> return -EINVAL;
> >> }
> >>
> >> + duty = clk_get_scaled_duty_cycle(priv->bclk, 100);
> >> + if (duty < 0)
> >> + dev_warn(&pdev->dev, "bus clock duty: %d\n", duty);
> >> +
> >> for (i = 0; i < ARRAY_SIZE(stm32h7_adc_ckmodes_spec); i++) {
> >> ckmode = stm32h7_adc_ckmodes_spec[i].ckmode;
> >> presc = stm32h7_adc_ckmodes_spec[i].presc;
> >> @@ -254,6 +270,9 @@ static int stm32h7_adc_clk_sel(struct platform_device *pdev,
> >> if (!ckmode)
> >> continue;
> >>
> >> + if (div == 1 && (duty < 49 || duty > 51))
> >> + continue;
> >> +
> >> if ((rate / div) <= priv->max_clk_rate)
> >> goto out;
> >> }
> >