Re: [PATCH 1/1] i2c: designware_i2c: adjust timing calculation

On 10/13/23 06:37, Heiko Schocher wrote:

Hello Heinrich,

On 11.10.23 06:48, Heinrich Schuchardt wrote:

...

In SPL probing of the designware_i2c device on the StarFive VisionFive 2 board fails with

 dw_i2c: mode 0, ic_clk 1000000, speed 100000,
 period 10 rise 1 fall 1 tlow 5 thigh 4 spk 0
 dw_i2c: bad counts. hcnt = -4 lcnt = 4
 device_probe: i2c@12050000 failed to probe -22

When changing the offset for the high phase from 7 to 1 the device is probed correctly.

Without this fix the memory size of the StarFive VisionFive 2 board cannot be read from EEPROM.

Fixes: e71b6f6622d6 ("i2c: designware_i2c: Rewrite timing calculation") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

---

drivers/i2c/designware_i2c.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index e54de42abc..55e582091c 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -155,10 +155,10 @@ static int dw_i2c_calc_timing(struct dw_i2c *priv, enum i2c_speed_mode mode,

/* * Back-solve for hcnt and lcnt according to the following equations:

• * SCL_High_time = [(HCNT + IC_*_SPKLEN + 7) * ic_clk] + SCL_Fall_time
  

• * SCL_High_time = [(HCNT + IC_*_SPKLEN + 1) * ic_clk] + SCL_Fall_time
  

  • SCL_Low_time = [(LCNT + 1) * ic_clk] - SCL_Fall_time + SCL_Rise_time
  */

• hcnt = min_thigh_cnt - fall_cnt - 7 - spk_cnt;

• hcnt = min_thigh_cnt - fall_cnt - 1 - spk_cnt; lcnt = min_tlow_cnt - rise_cnt + fall_cnt - 1;

  if (hcnt < 0 || lcnt < 0) {

@@ -170,13 +170,13 @@ static int dw_i2c_calc_timing(struct dw_i2c *priv, enum i2c_speed_mode mode, * Now add things back up to ensure the period is hit. If it is off, * split the difference and bias to lcnt for remainder */

• tot = hcnt + lcnt + 7 + spk_cnt + rise_cnt + 1;

• tot = hcnt + lcnt + 1 + spk_cnt + rise_cnt + 1;

  if (tot < period_cnt) { diff = (period_cnt - tot) / 2; hcnt += diff; lcnt += diff;

• tot = hcnt + lcnt + 7 + spk_cnt + rise_cnt + 1;
  

• tot = hcnt + lcnt + 1 + spk_cnt + rise_cnt + 1;
  

  lcnt += period_cnt - tot; }

What are this magic constants? Are they somewhere documented in the RM?

I have no access to Designware reference manuals. Do you have?

Hmm... and does this may break other boards? Should we have this in someway configurable?

Tried to look fast in the linux driver... and it seems to me, this constants depend at least on i2c_speed_mode?

bye, Heiko

https://www.nxp.com/docs/en/user-guide/UM10204.pdf has I2C timing diagrams in Figure 38 and Figure 39.

Linux has a function i2c_dw_scl_hcnt() where depending on parameter cond either a value of 8 or 3 is used. All invocations of i2c_dw_scl_hcnt() have cond = 0, offset = 0. Linux' i2c_dw_scl_lcnt() always uses 1.

The Linux comments are:

cond==True IC_[FS]S_SCL_HCNT + (1+4+3) >= IC_CLK * tHIGH

cond==False IC_[FS]S_SCL_HCNT + 3 >= IC_CLK * (tHD;STA + tf)

The U-Boot comment has:

SCL_High_time = [(HCNT + IC_*_SPKLEN + 7) * ic_clk] + SCL_Fall_time

This lets me assume that these are the matching pieces of code.

While my patch had %s/ 7 / 1 / we would need %s/ 7 / 3 / to match the Linux formula. This should be enough to avoid the observed "bad count" error on the VisionFive 2.

Best regards

Heinrich