| Message ID | 1604681846-31234-1-git-send-email-fabrice.gasnier@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: adc: stm32-adc: adapt clock duty cycle for proper operation | expand |
On Fri, 6 Nov 2020 17:57:26 +0100 Fabrice Gasnier <fabrice.gasnier@st.com> 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 <fabrice.gasnier@st.com> 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; > }
On 11/8/20 4:18 PM, Jonathan Cameron wrote: > On Fri, 6 Nov 2020 17:57:26 +0100 > Fabrice Gasnier <fabrice.gasnier@st.com> 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 <fabrice.gasnier@st.com> > 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; >> } >
On Fri, 13 Nov 2020 09:27:03 +0100 Fabrice Gasnier <fabrice.gasnier@st.com> wrote: > On 11/8/20 4:18 PM, Jonathan Cameron wrote: > > On Fri, 6 Nov 2020 17:57:26 +0100 > > Fabrice Gasnier <fabrice.gasnier@st.com> 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 <fabrice.gasnier@st.com> > > 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; > >> } > >
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; }
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 <fabrice.gasnier@st.com> --- drivers/iio/adc/stm32-adc-core.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)