Hello Heinrich,
On 13.10.23 10:32, Heinrich Schuchardt wrote:
> 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?
No.
>> 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.
Yep, I looked at the exact same code, and you are correct, with
"All invocations of i2c_dw_scl_hcnt() have cond = 0, offset = 0."
> 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.
Can you try this out? And if it works may I ask if you send a v2
with some explanation added in U-Boot code, that we sync here with
that what you found in kernel driver?
Many Thanks!
bye,
Heiko
>
> Best regards
>
> Heinrich
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
