Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] iio: adc: stm32-adc: adapt clock duty cycle for proper
 operation
To:     Jonathan Cameron <jic23@kernel.org>
CC:     <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <alexandre.torgue@st.com>,
        <olivier.moysan@st.com>, <linux-iio@vger.kernel.org>,
        <linux-stm32@st-md-mailman.stormreply.com>
References: <1604681846-31234-1-git-send-email-fabrice.gasnier@st.com>
 <20201108151835.5d78ebca@archlinux>
From:   Fabrice Gasnier <fabrice.gasnier@st.com>
Message-ID: <0d7c4b81-3f4e-e952-892f-35296c87f987@st.com>
Date:   Fri, 13 Nov 2020 09:27:03 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20201108151835.5d78ebca@archlinux>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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;
>>  	}
>