[3/4] i2c: qup: Correct duty cycle for FM and FM+

Message ID 1522871100-3621-4-git-send-email-austinwc@codeaurora.org
State Superseded
Headers show
Series
  • Add Fast Mode Plus and other fixes
Related show

Commit Message

Christ, Austin April 4, 2018, 7:44 p.m.
The I2C spec UM10204 Rev. 6 specifies the following timings.

           Standard      Fast Mode     Fast Mode Plus
SCL low    4.7us         1.3us         0.5us
SCL high   4.0us         0.6us         0.26us

This results in a 33%/66% duty cycle as opposed to the 50%/50% duty cycle
used for Standard-mode.

Add High Time Divider settings to correct duty cycle for FM(400kHz) and
FM+(1MHz).

Signed-off-by: Austin Christ <austinwc@codeaurora.org>
---
 drivers/i2c/busses/i2c-qup.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Sricharan R April 13, 2018, 6:14 a.m. | #1
Hi Austin,

On 4/5/2018 1:14 AM, Austin Christ wrote:
> The I2C spec UM10204 Rev. 6 specifies the following timings.
> 
>            Standard      Fast Mode     Fast Mode Plus
> SCL low    4.7us         1.3us         0.5us
> SCL high   4.0us         0.6us         0.26us
> 
> This results in a 33%/66% duty cycle as opposed to the 50%/50% duty cycle
> used for Standard-mode.
> 
> Add High Time Divider settings to correct duty cycle for FM(400kHz) and
> FM+(1MHz).
> 
> Signed-off-by: Austin Christ <austinwc@codeaurora.org>
> ---
>  drivers/i2c/busses/i2c-qup.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> index cb14687..2a88611 100644
> --- a/drivers/i2c/busses/i2c-qup.c
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -1858,9 +1858,15 @@ static int qup_i2c_probe(struct platform_device *pdev)
>  	size = QUP_INPUT_FIFO_SIZE(io_mode);
>  	qup->in_fifo_sz = qup->in_blk_sz * (2 << size);
>  
> -	fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
>  	hs_div = 3;
> -	qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
> +	if (clk_freq <= I2C_STANDARD_FREQ) {
> +		fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
> +		qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
> +	} else {
> +		/* 33%/66% duty cycle */
> +		fs_div = ((src_clk_freq / clk_freq) - 6) * 2 / 3;
> +		qup->clk_ctl = ((fs_div / 2) << 16) | (hs_div << 8) | (fs_div & 0xff);
> +	}
 while here, can we add a small comment for the above calculation. why the -3 above and -6 ?

Regards,
 Sricharan
Christ, Austin April 13, 2018, 3:48 p.m. | #2
Hey Sricharan,

On 4/13/2018 12:14 AM, Sricharan R wrote:
> Hi Austin,
> 
> On 4/5/2018 1:14 AM, Austin Christ wrote:
>> The I2C spec UM10204 Rev. 6 specifies the following timings.
>>
>>             Standard      Fast Mode     Fast Mode Plus
>> SCL low    4.7us         1.3us         0.5us
>> SCL high   4.0us         0.6us         0.26us
>>
>> This results in a 33%/66% duty cycle as opposed to the 50%/50% duty cycle
>> used for Standard-mode.
>>
>> Add High Time Divider settings to correct duty cycle for FM(400kHz) and
>> FM+(1MHz).
>>
>> Signed-off-by: Austin Christ <austinwc@codeaurora.org>
>> ---
>>   drivers/i2c/busses/i2c-qup.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>> index cb14687..2a88611 100644
>> --- a/drivers/i2c/busses/i2c-qup.c
>> +++ b/drivers/i2c/busses/i2c-qup.c
>> @@ -1858,9 +1858,15 @@ static int qup_i2c_probe(struct platform_device *pdev)
>>   	size = QUP_INPUT_FIFO_SIZE(io_mode);
>>   	qup->in_fifo_sz = qup->in_blk_sz * (2 << size);
>>   
>> -	fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
>>   	hs_div = 3;
>> -	qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
>> +	if (clk_freq <= I2C_STANDARD_FREQ) {
>> +		fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
>> +		qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
>> +	} else {
>> +		/* 33%/66% duty cycle */
>> +		fs_div = ((src_clk_freq / clk_freq) - 6) * 2 / 3;
>> +		qup->clk_ctl = ((fs_div / 2) << 16) | (hs_div << 8) | (fs_div & 0xff);
>> +	}
>   while here, can we add a small comment for the above calculation. why the -3 above and -6 ?
> 
Good question. This is the exact math recommended in the HPG for fs_div 
with and without the high time divider set. There is not explanation for 
the differences in the math in the hardware documentation that I've 
seen. What is the preferred way for documenting something like this in 
comments?

> Regards,
>   Sricharan
>
Sricharan R April 17, 2018, 11:42 a.m. | #3
Hi Austin,

On 4/13/2018 9:18 PM, Christ, Austin wrote:
> Hey Sricharan,
> 
> On 4/13/2018 12:14 AM, Sricharan R wrote:
>> Hi Austin,
>>
>> On 4/5/2018 1:14 AM, Austin Christ wrote:
>>> The I2C spec UM10204 Rev. 6 specifies the following timings.
>>>
>>>             Standard      Fast Mode     Fast Mode Plus
>>> SCL low    4.7us         1.3us         0.5us
>>> SCL high   4.0us         0.6us         0.26us
>>>
>>> This results in a 33%/66% duty cycle as opposed to the 50%/50% duty cycle
>>> used for Standard-mode.
>>>
>>> Add High Time Divider settings to correct duty cycle for FM(400kHz) and
>>> FM+(1MHz).
>>>
>>> Signed-off-by: Austin Christ <austinwc@codeaurora.org>
>>> ---
>>>   drivers/i2c/busses/i2c-qup.c | 10 ++++++++--
>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>>> index cb14687..2a88611 100644
>>> --- a/drivers/i2c/busses/i2c-qup.c
>>> +++ b/drivers/i2c/busses/i2c-qup.c
>>> @@ -1858,9 +1858,15 @@ static int qup_i2c_probe(struct platform_device *pdev)
>>>       size = QUP_INPUT_FIFO_SIZE(io_mode);
>>>       qup->in_fifo_sz = qup->in_blk_sz * (2 << size);
>>>   -    fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
>>>       hs_div = 3;
>>> -    qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
>>> +    if (clk_freq <= I2C_STANDARD_FREQ) {
>>> +        fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
>>> +        qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
>>> +    } else {
>>> +        /* 33%/66% duty cycle */
>>> +        fs_div = ((src_clk_freq / clk_freq) - 6) * 2 / 3;
>>> +        qup->clk_ctl = ((fs_div / 2) << 16) | (hs_div << 8) | (fs_div & 0xff);
>>> +    }
>>   while here, can we add a small comment for the above calculation. why the -3 above and -6 ?
>>
> Good question. This is the exact math recommended in the HPG for fs_div with and without the high time divider set. There is not explanation for the differences in the math in the hardware documentation that I've seen. What is the preferred way for documenting something like this in comments?

 ok, so you might have verified clk output with the new settings, right ?. That should be fine.

Regards,
 Sricharan
Christ, Austin April 18, 2018, 3:31 p.m. | #4
On 4/17/2018 5:42 AM, Sricharan R wrote:
> Hi Austin,
> 
> On 4/13/2018 9:18 PM, Christ, Austin wrote:
>> Hey Sricharan,
>>
>> On 4/13/2018 12:14 AM, Sricharan R wrote:
>>> Hi Austin,
>>>
>>> On 4/5/2018 1:14 AM, Austin Christ wrote:
>>>> The I2C spec UM10204 Rev. 6 specifies the following timings.
>>>>
>>>>              Standard      Fast Mode     Fast Mode Plus
>>>> SCL low    4.7us         1.3us         0.5us
>>>> SCL high   4.0us         0.6us         0.26us
>>>>
>>>> This results in a 33%/66% duty cycle as opposed to the 50%/50% duty cycle
>>>> used for Standard-mode.
>>>>
>>>> Add High Time Divider settings to correct duty cycle for FM(400kHz) and
>>>> FM+(1MHz).
>>>>
>>>> Signed-off-by: Austin Christ <austinwc@codeaurora.org>
>>>> ---
>>>>    drivers/i2c/busses/i2c-qup.c | 10 ++++++++--
>>>>    1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>>>> index cb14687..2a88611 100644
>>>> --- a/drivers/i2c/busses/i2c-qup.c
>>>> +++ b/drivers/i2c/busses/i2c-qup.c
>>>> @@ -1858,9 +1858,15 @@ static int qup_i2c_probe(struct platform_device *pdev)
>>>>        size = QUP_INPUT_FIFO_SIZE(io_mode);
>>>>        qup->in_fifo_sz = qup->in_blk_sz * (2 << size);
>>>>    -    fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
>>>>        hs_div = 3;
>>>> -    qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
>>>> +    if (clk_freq <= I2C_STANDARD_FREQ) {
>>>> +        fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
>>>> +        qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
>>>> +    } else {
>>>> +        /* 33%/66% duty cycle */
>>>> +        fs_div = ((src_clk_freq / clk_freq) - 6) * 2 / 3;
>>>> +        qup->clk_ctl = ((fs_div / 2) << 16) | (hs_div << 8) | (fs_div & 0xff);
>>>> +    }
>>>    while here, can we add a small comment for the above calculation. why the -3 above and -6 ?
>>>
>> Good question. This is the exact math recommended in the HPG for fs_div with and without the high time divider set. There is not explanation for the differences in the math in the hardware documentation that I've seen. What is the preferred way for documenting something like this in comments?
> 
>   ok, so you might have verified clk output with the new settings, right ?. That should be fine.
> 
Yes clk output was verified as well as the characteristics of the clock 
signal on the wire for 100kHz, 400kHz, and 1MHz to make sure frequency 
and duty cycle were within the I2C spec.
> Regards,
>   Sricharan
>

Patch

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index cb14687..2a88611 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -1858,9 +1858,15 @@  static int qup_i2c_probe(struct platform_device *pdev)
 	size = QUP_INPUT_FIFO_SIZE(io_mode);
 	qup->in_fifo_sz = qup->in_blk_sz * (2 << size);
 
-	fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
 	hs_div = 3;
-	qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
+	if (clk_freq <= I2C_STANDARD_FREQ) {
+		fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
+		qup->clk_ctl = (hs_div << 8) | (fs_div & 0xff);
+	} else {
+		/* 33%/66% duty cycle */
+		fs_div = ((src_clk_freq / clk_freq) - 6) * 2 / 3;
+		qup->clk_ctl = ((fs_div / 2) << 16) | (hs_div << 8) | (fs_div & 0xff);
+	}
 
 	/*
 	 * Time it takes for a byte to be clocked out on the bus.