eg20t i2c driver: missing some changes?

2012-02-22 Thread Richard Retanubun

Hi

I am using i2c-eg20t on linux 3.1.6 and comparing with the current source on 
3.3-rc4

I think there is a (few) missing cleanup between the version pasted on:
http://www.spinics.net/lists/linux-i2c/msg06664.html

to the one in version 3.3-rc4

one of it is this change in pch_i2c_getack

@@ -348,7 +347,7 @@
reg_val = ioread32(p + PCH_I2CSR) & PCH_GETACK;

if (reg_val != 0) {
-   pch_err(adap, "return%d\n", -EPROTO);
+   pch_dbg(adap, "return%d\n", -EPROTO);
return -EPROTO;
}

Without this change, using i2cdetect will generate a lot of false positive 
errors on devices that are not present.
I'm not sure if there is a patch that is missed or still being reviewed that 
contained this change

There are also some other differences between the mailing list version and 
linux-3.3rc4 in addition to this one,
but I don't have enough background to comment on them.

--
Richard Retanubun
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Question on i2c->real_clk when MPC_I2C_CLOCK_PRESERVE is set

2012-02-03 Thread Richard Retanubun

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 = 100 / i2c->real_clk + 1; /* delay_val = 100 */

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); /* 100us = 1 sec */
writeccr(i2c, CCR_MEN);
udelay(delay_val << 1); /* 200 = 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.

--
Richard Retanubun
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [i2c-mpc.c] adding entry for mpc8360

2012-01-13 Thread Richard Retanubun

On 13/01/12 02:26 AM, Wolfgang Grandegger wrote:

Hello,

I added the "devicetree-discuss" ml.

On 01/12/2012 04:34 PM, Richard Retanubun wrote:

Hello,

I am wondering if it is okay to add .compatible entries for mpc8360 and
declare its data structure
to mpc-i2c.c

something like this:

{.compatible = "fsl,mpc8360-i2c", .data =&mpc_i2c_data_8360, },

static struct mpc_i2c_data mpc_i2c_data_8360 __devinitdata = {
 .setup = mpc_i2c_setup_8xxx,
};

or is the intended approach to match the closest thing to your CPU (in
this case mpc8313) ?


Yes, it is intended to use the name of the compatible device, see:

http://lxr.linux.no/#linux+v3.2.1/arch/powerpc/boot/dts/kmeter1.dts#L69

Wolfgang

Thanks for the speedy response!

Two observations based on recent experiments to share with the ml:

1. The i2c-mpc does not take into account freescale APP note AN2919
   and for boards where the default dfsr setting violates condition #1.
   the i2c_transfer will exit with a -EIO due to timeout once in a while.

2. For boards that uses u-boot as bootloader, its fsl-i2c driver implements
   condition #1 checking. Thus using fsl,preserve-clocking is the way to go
   for now.

Richard
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[i2c-mpc.c] adding entry for mpc8360

2012-01-12 Thread Richard Retanubun

Hello,

I am wondering if it is okay to add .compatible entries for mpc8360 and declare 
its data structure
to mpc-i2c.c

something like this:

{.compatible = "fsl,mpc8360-i2c", .data = &mpc_i2c_data_8360, },

static struct mpc_i2c_data mpc_i2c_data_8360 __devinitdata = {
.setup = mpc_i2c_setup_8xxx,
};

or is the intended approach to match the closest thing to your CPU (in this 
case mpc8313) ?

--
Richard Retanubun
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html