Skip to contentSkip to conversation

Question on i2c->real_clk when MPC_I2C_CLOCK_PRESERVE is set

DISCUSSION Richard Retanubun 1 participant 1 message Feb 3, 2012 I2C subsystem host drivers

lore
Share & tools
lore
#0 lore raw

Hi Albrecht,

I am working on an MPC8360e platfrom and linux-3.0.0
I am seeing some stability issue with i2c and have some questions.

In git commit "powerpc/5200/i2c: improve i2c bus error recovery" 0c2daaafcdec726e89cbccca61d576de8429c537

The concept of i2c->real_clk is introduced to better optimize mpc_i2c_fixup(). Unless I am mistaken, in the current implementation, when MPC_I2C_CLOCK_PRESERVE is set, i2c->real_clk is never initialized.

So lets presume that its value is likely 0.

That makes mpc_i2c_fixup() look like this:

int k;
u32 delay_val = 1000000 / i2c->real_clk + 1; /* delay_val = 1000000 */

if (delay_val < 2)

	delay_val = 2;
for (k = 9; k; k--) {
	writeccr(i2c, 0);
	writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN);
	udelay(delay_val); /* 1000000us = 1 sec */
	writeccr(i2c, CCR_MEN);
	udelay(delay_val << 1); /* 2000000 = 2 sec */
}

Which I think is not the intended value, no?

I tried to see if I can populate i2c->real_clk using the preserved values inside fdr and dfsrr but I don't think that's possible without a lookup table (or another dts binding)

The other thought I have to in addition of a mininum delay_val of 2, we can also add a maximum limit for delay_val (of 30us ? to maintain the delay of the previous version)

As an aside, uboot seems to have a way to figure out the correct values of fdr and dfsrr without lookup /drivers/i2c/fsl-i2c.c::set_i2c_bus_speed(). This will prevent needing to use MPC_I2C_CLOCK_PRESERVE and get i2c->real_clk calculated properly.

Thanks for your time.

Signature · 1 line
Richard Retanubun