eg20t i2c driver: missing some changes?
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
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
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
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