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.schucha...@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
