[v2,0/2] Ensure High and Low periods of SCL are correct

Message

Lucas Tanure April 9, 2022, 4:43 p.m. UTC

The default duty cycle of 33% is less than the required
by the I2C specs for the LOW period of the SCL clock at
100KHz bus speed.

So, for 100Khz or less, use 50%H/50%L duty cycle, and
for the clock above 100Khz, use 40%H/60%L duty cycle.
That ensures the low period of SCL is always more than
the minimum required by the specs at any given frequency.

I did a few measures on the Khadas Vim3 board:

i2c_AO (i2c@5000):

Before the patchset, I got:
- 100KHz: 3.338us HIGH, 6.746us LOW, 33%/67%, Freq 99KHz (Not Valid tHIGH < 4.0us)
- 400KHz: 860ns HIGH, 1.734us LOW, 33.15%/62.85%, Freq 385.505KHz (Valid)
- 1000KHz: 362ns HIGH, 732ns LOW, 33.09%/66.91%, Freq 914.077KHz (Valid)

With the patchset
- 100KHz: 4.952us HIGH, 5.134us LOW, 49%/51%, Freq 99KHz (Valid)
- 400KHz: 966ns HIGH, 1.628us LOW, 37.24%/62.76%, Freq 385.505KHz (Valid)
- 1000KHz: 372ns HIGH, 720ns LOW, 34.07%/65.93%, Freq 915.741KHz (Valid)

i2c3 (i2c@1c000):

Before the patchset, I got:
- 100KHz: 3.348us HIGH, 6.704us LOW, 33%/67%, Freq 99.5KHz (Not Valid tHIGH < 4.0us)
- 400KHz: 864ns HIGH, 1.69us LOW, 33.83%/62.17%, Freq 391.543KHz (Valid)
- 1000KHz: 360ns HIGH, 690ns LOW, 34.29%/65.71%, Freq 952.381KHz (Valid)

With the patchset
- 100KHz: 4.958us HIGH, 5.092us LOW, 49%/51%, Freq 99KHz (Valid)
- 400KHz: 970ns HIGH, 1.582us LOW, 38%/62%, Freq 391.85KHz (Valid)
- 1000KHz: 370ns HIGH, 680ns LOW, 35.24%/64.76%, Freq 952.57KHz (Valid)

v2 changelog:
 - Keep the previous calculation for Meson6
 - Use I2C_MAX_STANDARD_MODE_FREQ
 - move the comment before the if()
 - use FIELD_PREP for setting div_l
 - Drop removal of meson_i2c_data

Previous versions:
 V1: https://lkml.org/lkml/2022/3/26/109

Lucas Tanure (2):
  i2c: meson: Use _SHIFT and _MASK for register definitions
  i2c: meson: Use 50% duty cycle for I2C clock

 drivers/i2c/busses/i2c-meson.c | 111 ++++++++++++++++++++++++---------
 1 file changed, 82 insertions(+), 29 deletions(-)

Comments

Neil Armstrong April 11, 2022, 7:30 a.m. UTC | #1

Hi,

On 09/04/2022 18:43, Lucas Tanure wrote:
> The default duty cycle of 33% is less than the required
> by the I2C specs for the LOW period of the SCL clock at
> 100KHz bus speed.
> 
> So, for 100Khz or less, use 50%H/50%L duty cycle, and
> for the clock above 100Khz, use 40%H/60%L duty cycle.
> That ensures the low period of SCL is always more than
> the minimum required by the specs at any given frequency.
> 
> I did a few measures on the Khadas Vim3 board:
> 
> i2c_AO (i2c@5000):
> 
> Before the patchset, I got:
> - 100KHz: 3.338us HIGH, 6.746us LOW, 33%/67%, Freq 99KHz (Not Valid tHIGH < 4.0us)
> - 400KHz: 860ns HIGH, 1.734us LOW, 33.15%/62.85%, Freq 385.505KHz (Valid)
> - 1000KHz: 362ns HIGH, 732ns LOW, 33.09%/66.91%, Freq 914.077KHz (Valid)
> 
> With the patchset
> - 100KHz: 4.952us HIGH, 5.134us LOW, 49%/51%, Freq 99KHz (Valid)
> - 400KHz: 966ns HIGH, 1.628us LOW, 37.24%/62.76%, Freq 385.505KHz (Valid)
> - 1000KHz: 372ns HIGH, 720ns LOW, 34.07%/65.93%, Freq 915.741KHz (Valid)
> 
> i2c3 (i2c@1c000):
> 
> Before the patchset, I got:
> - 100KHz: 3.348us HIGH, 6.704us LOW, 33%/67%, Freq 99.5KHz (Not Valid tHIGH < 4.0us)
> - 400KHz: 864ns HIGH, 1.69us LOW, 33.83%/62.17%, Freq 391.543KHz (Valid)
> - 1000KHz: 360ns HIGH, 690ns LOW, 34.29%/65.71%, Freq 952.381KHz (Valid)
> 
> With the patchset
> - 100KHz: 4.958us HIGH, 5.092us LOW, 49%/51%, Freq 99KHz (Valid)
> - 400KHz: 970ns HIGH, 1.582us LOW, 38%/62%, Freq 391.85KHz (Valid)
> - 1000KHz: 370ns HIGH, 680ns LOW, 35.24%/64.76%, Freq 952.57KHz (Valid)
> 
> v2 changelog:
>   - Keep the previous calculation for Meson6
>   - Use I2C_MAX_STANDARD_MODE_FREQ
>   - move the comment before the if()
>   - use FIELD_PREP for setting div_l
>   - Drop removal of meson_i2c_data
> 
> Previous versions:
>   V1: https://lkml.org/lkml/2022/3/26/109
> 
> Lucas Tanure (2):
>    i2c: meson: Use _SHIFT and _MASK for register definitions
>    i2c: meson: Use 50% duty cycle for I2C clock
> 
>   drivers/i2c/busses/i2c-meson.c | 111 ++++++++++++++++++++++++---------
>   1 file changed, 82 insertions(+), 29 deletions(-)
> 

Thanks a lot for the timings !

I think it's ok to be applied,

Neil