Sonic, Doesn't work - I send you an ADXL345 board with STAMP TWI connector assembled.
-Michael >-----Original Message----- >From: Zhang, Sonic >Sent: Tuesday, June 16, 2009 10:48 AM >To: Hennerich, Michael; '[email protected]' >Cc: '[email protected]' >Subject: RE: [Linux-kernel-commits] [6649] trunk/drivers/i2c/busses/i2c-bfin-twi.c: More replace >timeout timer with wait_for_completion_timeout > >Then, I have another patch for you to solve the lost interrupt issue. If it works with high SCL, I >will check it into SVN. > > > >Sonic > >-----Original Message----- >From: Hennerich, Michael >Sent: Tuesday, June 16, 2009 4:29 PM >To: Zhang, Sonic; '[email protected]' >Cc: '[email protected]' >Subject: RE: [Linux-kernel-commits] [6649] trunk/drivers/i2c/busses/i2c-bfin-twi.c: More replace >timeout timer with wait_for_completion_timeout > >Sonic, > >The issue that the system sometimes freezes is fixed now, due to proper timeout handling. > >The i2c driver as it is today is therefore better than before. > >However with the ADXL345 driver in I2C mode, SCL set to 400kHz - I sometimes see that >wait_for_completion_timeout() runs into timeout, due to a lost interrupt. >bfin_twi_handle_interrupt() returns without signaling complete() and the HW doesn't issue a follow up >interrupt, as the implementation would assume. > >As far as I can tell this happens with the block read transfers, and only at high SCL. > >I will send you an ADXL board you can test with. > >-Michael > > >>-----Original Message----- >>From: Zhang, Sonic >>Sent: Tuesday, June 16, 2009 4:15 AM >>To: Zhang, Sonic; Hennerich, Michael; [email protected] >>Cc: [email protected] >>Subject: RE: [Linux-kernel-commits] [6649] >>trunk/drivers/i2c/busses/i2c-bfin-twi.c: More replace timeout timer >>with wait_for_completion_timeout >> >>Hi Michael, >> >>I ran the complete regression after applying your patch. Both >>twi_smbus_test and twi_master_test passed on the regression machine. >>Could you file a bug with the error information under ADXL34x driver? >>Maybe the problem is the default timeout value is too short. You can changed it in ADXL34x driver at >runtime. >> >> >>Sonic >> >> >>-----Original Message----- >>From: Zhang, Sonic [mailto:[email protected]] >>Sent: Monday, June 15, 2009 5:48 PM >>To: Hennerich, Michael; [email protected] >>Cc: [email protected] >>Subject: RE: [Linux-kernel-commits] [6649] >>trunk/drivers/i2c/busses/i2c-bfin-twi.c: More replace timeout timer >>with wait_for_completion_timeout >> >>I am still looking at the regression result. >> >>Sonic >> >>-----Original Message----- >>From: Hennerich, Michael [mailto:[email protected]] >>Sent: Monday, June 15, 2009 5:06 PM >>To: [email protected] >>Cc: [email protected] >>Subject: RE: [Linux-kernel-commits] [6649] >>trunk/drivers/i2c/busses/i2c-bfin-twi.c: More replace timeout timer >>with wait_for_completion_timeout >> >>Sonic, >> >>Did you test these changes? >> >>i2c-bfin-twi i2c-bfin-twi.0: Blackfin BF5xx on-chip I2C TWI Contoller, >>regs_b...@ffc00700 >>schedule_timeout: wrong timeout value fffedda6 Hardware Trace: >> 0 Target : <0x00004c50> { __etext + 0xffe50890 } >> >> >>IIRC: >> >>Timeout is in jiffies but not jiffies + timeout. >> >>Index: drivers/i2c/busses/i2c-bfin-twi.c >>=================================================================== >>--- drivers/i2c/busses/i2c-bfin-twi.c (revision 6714) >>+++ drivers/i2c/busses/i2c-bfin-twi.c (working copy) >>@@ -335,7 +335,7 @@ >> >> while (!iface->result) { >> if (!wait_for_completion_timeout(&iface->complete, >>- jiffies + adap->timeout*HZ)) >>+ adap->timeout*HZ)) >> iface->result = -1; >> } >> >>@@ -551,7 +551,7 @@ >> >> while (!iface->result) { >> if (!wait_for_completion_timeout(&iface->complete, >>- jiffies + adap->timeout*HZ)) >>+ adap->timeout*HZ)) >> iface->result = -1; >> } >> >> >>With this being fixed - the kernel boots without an error messages. >>However the ADXL34x driver doesn't work at all now. >> >>-Michael >> >> >> >>________________________________________ >>From: [email protected] >>[mailto:[email protected]] On Behalf Of >>[email protected] >>Sent: Thursday, June 11, 2009 11:30 AM >>To: [email protected] >>Subject: [Linux-kernel-commits] [6649] >>trunk/drivers/i2c/busses/i2c-bfin-twi.c: More replace timeout timer >>with wait_for_completion_timeout >> >>Revision >>6649 >>Author >>sonicz >>Date >>2009-06-11 04:29:59 -0500 (Thu, 11 Jun 2009) Log Message More replace >>timeout timer with wait_for_completion_timeout >> >>1. retry i2c transfer according to user defined parameter retires in struct i2c_adapter. >>2. wait timeout based on user defined parameter timeout in struct i2c_adapter. >>3. wake up i2c transfer process in each twi interrupt. >>Modified Paths >>* trunk/drivers/i2c/busses/i2c-bfin-twi.c >>Diff >>Modified: trunk/drivers/i2c/busses/i2c-bfin-twi.c (6648 => 6649) >> >>--- trunk/drivers/i2c/busses/i2c-bfin-twi.c 2009-06-11 09:26:47 UTC >>(rev 6648) >>+++ trunk/drivers/i2c/busses/i2c-bfin-twi.c 2009-06-11 09:29:59 UTC >>(rev 6649) >>@@ -23,8 +23,6 @@ >> #include <asm/portmux.h> >> #include <asm/irq.h> >> >>-#define POLL_TIMEOUT (20 * HZ) >>- >> /* SMBus mode*/ >> #define TWI_I2C_MODE_STANDARD 1 >> #define TWI_I2C_MODE_STANDARDSUB 2 >>@@ -165,16 +163,13 @@ >> write_INT_MASK(iface, 0); >> write_MASTER_CTL(iface, 0); >> SSYNC(); >>- /* If it is a quick transfer, only address bug >>no data, >>+ /* If it is a quick transfer, only address >>without data, >> * not an err, return 1. >>+ * If address is acknowledged return 1. >> */ >>- if (iface->writeNum == 0 && (mast_stat & >>BUFRDERR)) >>+ if ((iface->writeNum == 0 && (mast_stat & >>BUFRDERR)) >>+ || !(mast_stat & ANAK)) >> iface->result = 1; >>- /* If address not acknowledged return -1, >>- * else return 0. >>- */ >>- else if (!(mast_stat & ANAK)) >>- iface->result = 0; >> } >> complete(&iface->complete); >> return; >>@@ -246,9 +241,9 @@ >> write_INT_MASK(iface, 0); >> write_MASTER_CTL(iface, 0); >> SSYNC(); >>- complete(&iface->complete); >> } >> } >>+ complete(&iface->complete); >> } >> >> /* Interrupt handler */ >>@@ -264,9 +259,9 @@ >> } >> >> /* >>- * Generic i2c master transfer entrypoint >>+ * One i2c master transfer >> */ >>-static int bfin_twi_master_xfer(struct i2c_adapter *adap, >>+static int bfin_twi_do_master_xfer(struct i2c_adapter *adap, >> struct i2c_msg *msgs, int num) >> { >> struct bfin_twi_iface *iface = adap->algo_data; @@ -338,23 >>+333,41 @@ >> ((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ > 100) ? FAST : 0)); >> SSYNC(); >> >>- if (!wait_for_completion_timeout(&iface->complete, >>- jiffies + POLL_TIMEOUT)) >>- iface->result = -1; >>+ while (!iface->result) { >>+ if (!wait_for_completion_timeout(&iface->complete, >>+ jiffies + adap->timeout*HZ)) >>+ iface->result = -1; >>+ } >> >>- rc = iface->result; >>+ if (iface->result == 1) >>+ rc = iface->cur_msg + 1; >>+ else >>+ rc = iface->result; >> >>- if (rc == 1) >>- return num; >>- else >>- return rc; >>+ return rc; >> } >> >> /* >>- * SMBus type transfer entrypoint >>+ * Generic i2c master transfer entrypoint >> */ >>+static int bfin_twi_master_xfer(struct i2c_adapter *adap, >>+ struct i2c_msg *msgs, int num) >>+{ >>+ int i, ret = 0; >> >>-int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr, >>+ for (i = 0; i < adap->retries; i++) { >>+ ret = bfin_twi_do_master_xfer(adap, msgs, num); >>+ if (ret > 0) >>+ break; >>+ } >>+ >>+ return ret; >>+} >>+ >>+/* >>+ * One I2C SMBus transfer >>+ */ >>+int bfin_twi_do_smbus_xfer(struct i2c_adapter *adap, u16 addr, >> unsigned short flags, char read_write, >> u8 command, int size, union i2c_smbus_data >>*data) >> { >>@@ -536,9 +549,11 @@ >> } >> SSYNC(); >> >>- if (!wait_for_completion_timeout(&iface->complete, >>- jiffies + POLL_TIMEOUT)) >>- iface->result = -1; >>+ while (!iface->result) { >>+ if (!wait_for_completion_timeout(&iface->complete, >>+ jiffies + adap->timeout*HZ)) >>+ iface->result = -1; >>+ } >> >> rc = (iface->result >= 0) ? 0 : -1; >> >>@@ -546,6 +561,25 @@ >> } >> >> /* >>+ * Generic I2C SMBus transfer entrypoint */ int >>+bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr, >>+ unsigned short flags, char read_write, >>+ u8 command, int size, union i2c_smbus_data >>*data) >>+{ >>+ int i, ret = 0; >>+ >>+ for (i = 0; i < adap->retries; i++) { >>+ ret = bfin_twi_do_smbus_xfer(adap, addr, flags, >>+ read_write, command, size, data); >>+ if (ret == 0) >>+ break; >>+ } >>+ >>+ return ret; >>+} >>+ >>+/* >> * Return what the adapter supports >> */ >> static u32 bfin_twi_functionality(struct i2c_adapter *adap) @@ -643,6 >>+677,8 @@ >> p_adap->algo_data = iface; >> p_adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD; >> p_adap->dev.parent = &pdev->dev; >>+ p_adap->timeout = 1; >>+ p_adap->retries = 3; >> >> rc = peripheral_request_list(pin_req[pdev->id], "i2c-bfin-twi"); >> if (rc) { _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
