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

[Heiko Schocher]

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 at 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 at 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?

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
-- 
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: hs at denx.de