Re: [PATCH] i2c: adds support for i2c bus on 8xx



Hi Vitaly,

On Thu, 10 May 2007 14:35:54 +0400, Vitaly Bordug wrote:
On Thu, 10 May 2007 11:28:35 +0200 Jean Delvare wrote:
On Tue, 08 May 2007 10:31:31 +0400, Vitaly Bordug wrote:
Interface has been reworked to of_device as well as other feedback
addressed. Repeated start ability is missing from the
implementation.

Did you check the hardware documentation? Is it a hardware limitation?
It is likely to cause problems, so it should be investigated, and if
it really cannot be fixed, then this must be clearly documented (in
Kconfig and in the driver) and the advertised functionalities of the
driver should be reduced accordingly (most SMBus transactions include
a repeated start.)

Yes, I've checked RM about I2C controller, and cannot find anything about repeated
start. There are a few words about multi-master considerations, that SoC features bus arbitration,
but to avoid collisions, "higher-level handshake protocol must be used" (MPC885 RM, p. 32-6).

After reading the MPC885 RM data***, I think the device supports
repeated start. I guess that you simply need to _only_ set the
BD_SC_LAST flag for the last message of the transaction, not all of
them as the driver does. If I am correct, this should be fairly easy to
fix. Please try, test and report.

+static int cpm_iic_init(struct i2c_adapter *adap)
+{
(...)
+ /* Initialize Tx/Rx parameters.
+ */
+ if (cpm->reloc == 0) {
+ cpm8xx_t *cp = cpm->cp;
+ int res;
+
+ u16 v = mk_cr_cmd(CPM_CR_CH_I2C, CPM_CR_INIT_TRX) | CPM_CR_FLG;
+
+ out_be16(&cp->cp_cpcr, v);
+ res = wait_event_timeout(iic_wait,
+ !(in_be16(&cp->cp_cpcr) & CPM_CR_FLG),
+ HZ * 10);

This is a pretty long timeout. Can it realistically take that long?

In fact, it can. Especially when CPM is loaded with Ethernet/UART/Etc
transactions. If you feel it is overkill and I2C shouldn't do that,
I'll trim it down though.

No, if you think it's reasonable, fine with me.

--
Jean Delvare
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/