| Message ID | 1266433154.2322.0@antares (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Grant Likely |
| Headers | show |
On Wed, Feb 17, 2010 at 11:59 AM, Albrecht Dreß <albrecht.dress@arcor.de> wrote: > This patch improves the recovery of the MPC's I2C bus from errors like bus > hangs resulting in timeouts: > 1. make the bus timeout configurable, as it depends on the bus clock and > the attached slave chip(s); default is still 1 second; > 2. detect any of the cases indicated by the CF, BB and RXAK MSR flags if a > timeout occurs, and add a missing (required) MAL reset; > 3. use a more reliable method to fixup the bus if a hang has been detected. > The sequence is sent 9 times which seems to be necessary if a slave > "misses" more than one clock cycle. For 400 kHz bus speed, the fixup is > also ~70us (81us vs. 150us) faster. > > Tested on a custom Lite5200b derived board, with a Dallas RTC, AD sensors > and NXP IO expander chips attached to the i2c. > > Changes vs. v1: > - use improved bus fixup sequence for all chips (not only the 5200) > - calculate real clock from defaults if no clock is given in the device tree > - better description (I hope) of the changes. > > I didn't split the changes in this file into three parts as recommended by > Grant, as they actually belong together (i.e. they address one single > problem, just in three places of one single source file). > > Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de> Acked-by: Grant Likely <grant.likely@secretlab.ca> > > --- > > Note about the reset sequence: I verified the waveforms for 18.4kHz, 85.9kHz > and 375kHz (drop me a note if you want to see scope screen shots). Not > verified on other mpc chips yet. > @Ira: thanks in advance for giving it a try on your box! > > > --- linux-2.6.32-orig/drivers/i2c/busses/i2c-mpc.c 2009-12-03 > 04:51:21.000000000 +0100 > +++ linux-2.6.32/drivers/i2c/busses/i2c-mpc.c 2010-02-17 > 16:23:11.000000000 +0100 > @@ -59,6 +59,7 @@ struct mpc_i2c { > wait_queue_head_t queue; > struct i2c_adapter adap; > int irq; > + u32 real_clk; > }; > > struct mpc_i2c_divider { > @@ -93,20 +94,23 @@ static irqreturn_t mpc_i2c_isr(int irq, > /* Sometimes 9th clock pulse isn't generated, and slave doesn't release > * the bus, because it wants to send ACK. > * Following sequence of enabling/disabling and sending start/stop generates > - * the pulse, so it's all OK. > + * the 9 pulses, so it's all OK. > */ > static void mpc_i2c_fixup(struct mpc_i2c *i2c) > { > - writeccr(i2c, 0); > - udelay(30); > - writeccr(i2c, CCR_MEN); > - udelay(30); > - writeccr(i2c, CCR_MSTA | CCR_MTX); > - udelay(30); > - writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN); > - udelay(30); > - writeccr(i2c, CCR_MEN); > - udelay(30); > + int k; > + u32 delay_val = 1000000 / i2c->real_clk + 1; > + > + 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); > + writeccr(i2c, CCR_MEN); > + udelay(delay_val << 1); > + } > } > > static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing) > @@ -186,15 +190,19 @@ static const struct mpc_i2c_divider mpc_ > {10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f} > }; > > -int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int > prescaler) > +int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int > prescaler, > + u32 *real_clk) > { > const struct mpc_i2c_divider *div = NULL; > unsigned int pvr = mfspr(SPRN_PVR); > u32 divider; > int i; > > - if (!clock) > + if (!clock) { > + /* see below - default fdr = 0x3f -> div = 2048 */ > + *real_clk = mpc5xxx_get_bus_frequency(node) / 2048; > return -EINVAL; > + } > > /* Determine divider value */ > divider = mpc5xxx_get_bus_frequency(node) / clock; > @@ -212,7 +220,8 @@ int mpc_i2c_get_fdr_52xx(struct device_n > break; > } > > - return div ? (int)div->fdr : -EINVAL; > + *real_clk = mpc5xxx_get_bus_frequency(node) / div->divider; > + return (int)div->fdr; > } > > static void mpc_i2c_setclock_52xx(struct device_node *node, > @@ -221,13 +230,14 @@ static void mpc_i2c_setclock_52xx(struct > { > int ret, fdr; > > - ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler); > + ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler, &i2c->real_clk); > fdr = (ret >= 0) ? ret : 0x3f; /* backward compatibility */ > > writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); > > if (ret >= 0) > - dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr); > + dev_info(i2c->dev, "clock %u Hz (fdr=%d)\n", i2c->real_clk, > + fdr); > } > #else /* !CONFIG_PPC_MPC52xx */ > static void mpc_i2c_setclock_52xx(struct device_node *node, > @@ -287,14 +297,18 @@ u32 mpc_i2c_get_sec_cfg_8xxx(void) > return val; > } > > -int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 > prescaler) > +int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 > prescaler, > + u32 *real_clk) > { > const struct mpc_i2c_divider *div = NULL; > u32 divider; > int i; > > - if (!clock) > + if (!clock) { > + /* see below - default fdr = 0x1031 -> div = 16 * 3072 */ > + *real_clk = fsl_get_sys_freq() / prescaler / (16 * 3072); > return -EINVAL; > + } > > /* Determine proper divider value */ > if (of_device_is_compatible(node, "fsl,mpc8544-i2c")) > @@ -317,6 +331,7 @@ int mpc_i2c_get_fdr_8xxx(struct device_n > break; > } > > + *real_clk = fsl_get_sys_freq() / prescaler / div->divider; > return div ? (int)div->fdr : -EINVAL; > } > > @@ -326,7 +341,7 @@ static void mpc_i2c_setclock_8xxx(struct > { > int ret, fdr; > > - ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler); > + ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler, &i2c->real_clk); > fdr = (ret >= 0) ? ret : 0x1031; /* backward compatibility */ > > writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); > @@ -334,7 +349,7 @@ static void mpc_i2c_setclock_8xxx(struct > > if (ret >= 0) > dev_info(i2c->dev, "clock %d Hz (dfsrr=%d fdr=%d)\n", > - clock, fdr >> 8, fdr & 0xff); > + i2c->real_clk, fdr >> 8, fdr & 0xff); > } > > #else /* !CONFIG_FSL_SOC */ > @@ -446,10 +461,14 @@ static int mpc_xfer(struct i2c_adapter * > return -EINTR; > } > if (time_after(jiffies, orig_jiffies + HZ)) { > + u8 status = readb(i2c->base + MPC_I2C_SR); > + > dev_dbg(i2c->dev, "timeout\n"); > - if (readb(i2c->base + MPC_I2C_SR) == > - (CSR_MCF | CSR_MBB | CSR_RXAK)) > + if ((status & (CSR_MCF | CSR_MBB | CSR_RXAK)) != 0) > { > + writeb(status & ~CSR_MAL, > + i2c->base + MPC_I2C_SR); > mpc_i2c_fixup(i2c); > + } > return -EIO; > } > schedule(); > @@ -540,6 +559,14 @@ static int __devinit fsl_i2c_probe(struc > } > } > > + prop = of_get_property(op->node, "fsl,timeout", &plen); > + if (prop && plen == sizeof(u32)) { > + mpc_ops.timeout = *prop * HZ / 1000000; > + if (mpc_ops.timeout < 5) > + mpc_ops.timeout = 5; > + } > + dev_info(i2c->dev, "timeout %u us\n", mpc_ops.timeout * 1000000 / > HZ); > + > dev_set_drvdata(&op->dev, i2c); > > i2c->adap = mpc_ops; >
> > This patch improves the recovery of the MPC's I2C bus from errors like bus > hangs resulting in timeouts: > 1. make the bus timeout configurable, as it depends on the bus clock and > the attached slave chip(s); default is still 1 second; > 2. detect any of the cases indicated by the CF, BB and RXAK MSR flags if a > timeout occurs, and add a missing (required) MAL reset; > 3. use a more reliable method to fixup the bus if a hang has been detected. > The sequence is sent 9 times which seems to be necessary if a slave > "misses" more than one clock cycle. For 400 kHz bus speed, the fixup is > also ~70us (81us vs. 150us) faster. > > Tested on a custom Lite5200b derived board, with a Dallas RTC, AD sensors > and NXP IO expander chips attached to the i2c. > > Changes vs. v1: > - use improved bus fixup sequence for all chips (not only the 5200) > - calculate real clock from defaults if no clock is given in the device tree > - better description (I hope) of the changes. > > I didn't split the changes in this file into three parts as recommended by > Grant, as they actually belong together (i.e. they address one single > problem, just in three places of one single source file). > > Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de> > > --- > > Note about the reset sequence: I verified the waveforms for 18.4kHz, 85.9kHz > and 375kHz (drop me a note if you want to see scope screen shots). Not > verified on other mpc chips yet. > @Ira: thanks in advance for giving it a try on your box! Does this reset sequence also send a START condition for every clock? The ideal I2C reset sequence should look like this: for(j = 0; j < 9; j++) { if(I2C_READ) send_start(); I2C_SCL(0); I2C_DELAY; I2C_TRISTATE; I2C_SDA(1); I2C_DELAY; I2C_SCL(1); I2C_DELAY; I2C_DELAY; } send_stop(); static void send_start(void) { I2C_DELAY; I2C_TRISTATE; I2C_SDA(1); I2C_DELAY; I2C_SCL(1); I2C_DELAY; I2C_SDA(0); I2C_ACTIVE; I2C_DELAY; } static void send_stop(void) { I2C_SCL(0); I2C_DELAY; I2C_SDA(0); I2C_ACTIVE; I2C_DELAY; I2C_SCL(1); I2C_DELAY; I2C_TRISTATE; I2C_SDA(1); I2C_DELAY; } Jocke
"Albrecht Dreß" <albrecht.dress@arcor.de> wrote on 2010/02/18 10:09:23: > > Hi Joakim: > > > Does this reset sequence also send a START condition for every clock? > > Please see the attached scan from a scope output, showing the first two out of > the 9 sequences at 375 kHz (that's what the 5200's divider makes from 400 kHz > requested). Resolution is 2us/div and 1V/div for both signals. The waveform > itself for each of the 9 sequences is exactly the same we had before with the > old solution, just the timing is faster and adjusted to the ii2c clock, i.e. > the /relative/ waveforms look identical for slower clocks. > > Any insight if this is *really* correct would be great, as I'm not an i2c > expert. I can only say it reliably fixes the bus hangs I saw! Looks like you do a STOP then START each time SCL is high so yes you do a START each SCL and a STOP too. Don't think the STOP will hurt though. Timing is OK for FAST-MODE(400kHz), cannot say for STANDARD-MODE though need a 100Khz scope img for that. The times to look for are: tHD;STA, tSU;STA, tSU;STO and tBUF at least that is what I have identified. Jocke
> @@ -93,20 +94,23 @@ static irqreturn_t mpc_i2c_isr(int irq, > /* Sometimes 9th clock pulse isn't generated, and slave doesn't release > * the bus, because it wants to send ACK. > * Following sequence of enabling/disabling and sending start/stop generates > - * the pulse, so it's all OK. > + * the 9 pulses, so it's all OK. > */ > static void mpc_i2c_fixup(struct mpc_i2c *i2c) > { > - writeccr(i2c, 0); > - udelay(30); > - writeccr(i2c, CCR_MEN); > - udelay(30); > - writeccr(i2c, CCR_MSTA | CCR_MTX); > - udelay(30); > - writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN); > - udelay(30); > - writeccr(i2c, CCR_MEN); > - udelay(30); > + int k; > + u32 delay_val = 1000000 / i2c->real_clk + 1; > + > + 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); > + writeccr(i2c, CCR_MEN); > + udelay(delay_val << 1); > + } > } I am curious, didn't old method work with by just wrapping a for(k=9; k; k--) around it? How did the wave form look? Jocke
On Wed, Feb 17, 2010 at 07:59:14PM +0100, Albrecht Dreß wrote: > This patch improves the recovery of the MPC's I2C bus from errors like bus > hangs resulting in timeouts: > 1. make the bus timeout configurable, as it depends on the bus clock and > the attached slave chip(s); default is still 1 second; > 2. detect any of the cases indicated by the CF, BB and RXAK MSR flags if a > timeout occurs, and add a missing (required) MAL reset; > 3. use a more reliable method to fixup the bus if a hang has been detected. > The sequence is sent 9 times which seems to be necessary if a slave > "misses" more than one clock cycle. For 400 kHz bus speed, the fixup is > also ~70us (81us vs. 150us) faster. > > Tested on a custom Lite5200b derived board, with a Dallas RTC, AD sensors > and NXP IO expander chips attached to the i2c. > > Changes vs. v1: > - use improved bus fixup sequence for all chips (not only the 5200) > - calculate real clock from defaults if no clock is given in the device tree > - better description (I hope) of the changes. > > I didn't split the changes in this file into three parts as recommended by > Grant, as they actually belong together (i.e. they address one single > problem, just in three places of one single source file). > > Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de> > > --- > > Note about the reset sequence: I verified the waveforms for 18.4kHz, 85.9kHz > and 375kHz (drop me a note if you want to see scope screen shots). Not > verified on other mpc chips yet. > @Ira: thanks in advance for giving it a try on your box! > Did this series get forgotten about? I don't see it in bjdook's next-i2c branch or in benh's next branch. I've pulled these into my 2.6.31.13 kernel, and they seem to work fine. You've got my Tested-by if you didn't get one from me already. Ira > --- linux-2.6.32-orig/drivers/i2c/busses/i2c-mpc.c 2009-12-03 04:51:21.000000000 +0100 > +++ linux-2.6.32/drivers/i2c/busses/i2c-mpc.c 2010-02-17 16:23:11.000000000 +0100 > @@ -59,6 +59,7 @@ struct mpc_i2c { > wait_queue_head_t queue; > struct i2c_adapter adap; > int irq; > + u32 real_clk; > }; > > struct mpc_i2c_divider { > @@ -93,20 +94,23 @@ static irqreturn_t mpc_i2c_isr(int irq, > /* Sometimes 9th clock pulse isn't generated, and slave doesn't release > * the bus, because it wants to send ACK. > * Following sequence of enabling/disabling and sending start/stop generates > - * the pulse, so it's all OK. > + * the 9 pulses, so it's all OK. > */ > static void mpc_i2c_fixup(struct mpc_i2c *i2c) > { > - writeccr(i2c, 0); > - udelay(30); > - writeccr(i2c, CCR_MEN); > - udelay(30); > - writeccr(i2c, CCR_MSTA | CCR_MTX); > - udelay(30); > - writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN); > - udelay(30); > - writeccr(i2c, CCR_MEN); > - udelay(30); > + int k; > + u32 delay_val = 1000000 / i2c->real_clk + 1; > + > + 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); > + writeccr(i2c, CCR_MEN); > + udelay(delay_val << 1); > + } > } > > static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing) > @@ -186,15 +190,19 @@ static const struct mpc_i2c_divider mpc_ > {10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f} > }; > > -int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler) > +int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler, > + u32 *real_clk) > { > const struct mpc_i2c_divider *div = NULL; > unsigned int pvr = mfspr(SPRN_PVR); > u32 divider; > int i; > > - if (!clock) > + if (!clock) { > + /* see below - default fdr = 0x3f -> div = 2048 */ > + *real_clk = mpc5xxx_get_bus_frequency(node) / 2048; > return -EINVAL; > + } > > /* Determine divider value */ > divider = mpc5xxx_get_bus_frequency(node) / clock; > @@ -212,7 +220,8 @@ int mpc_i2c_get_fdr_52xx(struct device_n > break; > } > > - return div ? (int)div->fdr : -EINVAL; > + *real_clk = mpc5xxx_get_bus_frequency(node) / div->divider; > + return (int)div->fdr; > } > > static void mpc_i2c_setclock_52xx(struct device_node *node, > @@ -221,13 +230,14 @@ static void mpc_i2c_setclock_52xx(struct > { > int ret, fdr; > > - ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler); > + ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler, &i2c->real_clk); > fdr = (ret >= 0) ? ret : 0x3f; /* backward compatibility */ > > writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); > > if (ret >= 0) > - dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr); > + dev_info(i2c->dev, "clock %u Hz (fdr=%d)\n", i2c->real_clk, > + fdr); > } > #else /* !CONFIG_PPC_MPC52xx */ > static void mpc_i2c_setclock_52xx(struct device_node *node, > @@ -287,14 +297,18 @@ u32 mpc_i2c_get_sec_cfg_8xxx(void) > return val; > } > > -int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler) > +int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler, > + u32 *real_clk) > { > const struct mpc_i2c_divider *div = NULL; > u32 divider; > int i; > > - if (!clock) > + if (!clock) { > + /* see below - default fdr = 0x1031 -> div = 16 * 3072 */ > + *real_clk = fsl_get_sys_freq() / prescaler / (16 * 3072); > return -EINVAL; > + } > > /* Determine proper divider value */ > if (of_device_is_compatible(node, "fsl,mpc8544-i2c")) > @@ -317,6 +331,7 @@ int mpc_i2c_get_fdr_8xxx(struct device_n > break; > } > > + *real_clk = fsl_get_sys_freq() / prescaler / div->divider; > return div ? (int)div->fdr : -EINVAL; > } > > @@ -326,7 +341,7 @@ static void mpc_i2c_setclock_8xxx(struct > { > int ret, fdr; > > - ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler); > + ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler, &i2c->real_clk); > fdr = (ret >= 0) ? ret : 0x1031; /* backward compatibility */ > > writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); > @@ -334,7 +349,7 @@ static void mpc_i2c_setclock_8xxx(struct > > if (ret >= 0) > dev_info(i2c->dev, "clock %d Hz (dfsrr=%d fdr=%d)\n", > - clock, fdr >> 8, fdr & 0xff); > + i2c->real_clk, fdr >> 8, fdr & 0xff); > } > > #else /* !CONFIG_FSL_SOC */ > @@ -446,10 +461,14 @@ static int mpc_xfer(struct i2c_adapter * > return -EINTR; > } > if (time_after(jiffies, orig_jiffies + HZ)) { > + u8 status = readb(i2c->base + MPC_I2C_SR); > + > dev_dbg(i2c->dev, "timeout\n"); > - if (readb(i2c->base + MPC_I2C_SR) == > - (CSR_MCF | CSR_MBB | CSR_RXAK)) > + if ((status & (CSR_MCF | CSR_MBB | CSR_RXAK)) != 0) { > + writeb(status & ~CSR_MAL, > + i2c->base + MPC_I2C_SR); > mpc_i2c_fixup(i2c); > + } > return -EIO; > } > schedule(); > @@ -540,6 +559,14 @@ static int __devinit fsl_i2c_probe(struc > } > } > > + prop = of_get_property(op->node, "fsl,timeout", &plen); > + if (prop && plen == sizeof(u32)) { > + mpc_ops.timeout = *prop * HZ / 1000000; > + if (mpc_ops.timeout < 5) > + mpc_ops.timeout = 5; > + } > + dev_info(i2c->dev, "timeout %u us\n", mpc_ops.timeout * 1000000 / HZ); > + > dev_set_drvdata(&op->dev, i2c); > > i2c->adap = mpc_ops; >
On Thu, May 6, 2010 at 7:54 PM, Albrecht Dreß <albrecht.dress@arcor.de> wrote: > Am 06.05.10 00:09 schrieb(en) Ira W. Snyder: >> >> Did this series get forgotten about? I don't see it in bjdook's next-i2c >> branch or in benh's next branch. > > It's not in Grant's 5xxx tree either - as it's specific for those > processors, I think he might be the responsible person. The patch is still > in a "new" state in patchwork, btw. > >> I've pulled these into my 2.6.31.13 kernel, and they seem to work fine. >> You've got my Tested-by if you didn't get one from me already. > > Tanks a lot! > > I think, though, the whole stuff has been discussed in depth in February, so > I do not understand why it's still pending as "new". Grant, did we miss > something here? I generally let subsystem maintainers pick up the device driver patches for embedded platforms I'm responsible for unless I'm asked to do otherwise. I think ben has asked me to take these through my tree. g.
Am 06.05.10 20:06 schrieb(en) Grant Likely: > > I think, though, the whole stuff has been discussed in depth in February, so > > I do not understand why it's still pending as "new". Grant, did we miss > > something here? > > I generally let subsystem maintainers pick up the device driver > patches for embedded platforms I'm responsible for unless I'm asked to > do otherwise. I think ben has asked me to take these through my tree. That's <http://git.secretlab.ca/?p=linux-2.6.git;a=shortlog>, isn't it? Hmmm, didn't find it there... :-/ Best, Albrecht.
On Sun, May 16, 2010 at 11:47 AM, Albrecht Dreß <albrecht.dress@arcor.de> wrote: > Am 06.05.10 20:06 schrieb(en) Grant Likely: >> >> > I think, though, the whole stuff has been discussed in depth in >> > February, so >> > I do not understand why it's still pending as "new". Grant, did we miss >> > something here? >> >> I generally let subsystem maintainers pick up the device driver >> patches for embedded platforms I'm responsible for unless I'm asked to >> do otherwise. I think ben has asked me to take these through my tree. > > That's <http://git.secretlab.ca/?p=linux-2.6.git;a=shortlog>, isn't it? > Hmmm, didn't find it there... :-/ Ugh... Stupid typing too fast. I meant to say, "I *don't* think ben has asked me to take..." Well this leaves a bit of a mess. I'll make sure it gets in one way or another. g.
Am 19.05.10 18:02 schrieb(en) Grant Likely: > > That's <http://git.secretlab.ca/?p=linux-2.6.git;a=shortlog>, isn't it? > > Hmmm, didn't find it there... :-/ > > Ugh... Stupid typing too fast. I meant to say, "I *don't* think ben > has asked me to take..." > > Well this leaves a bit of a mess. I'll make sure it gets in one way or another. *ping* Any news about this issue? Cheers, Albrecht.
Ash Shoes are one of the leading brands in the modern footwear industry. Lots of people now prefer to wear shoes from this particular brand because they become accustom to their high quality and superb fit. Ash has revolutionized the footwear industry by combining it with the fashion industry. Check out this <http://onlyashshoes.com/goods-109-Virgin-Bis-Sneaker-Blue-Denim-312023.html> These shoes will surely add a new dimension to your wardrobe. The shoes as well as boots from Ash are manufactured using the most advanced shoe making technology in combination with high quality materials and expert craftsmanship. This helps in fulfilling the various requirements of different people. You will get shoes that are ideal for both formal and casual occasions. Innovative collections are introduced each season to satisfy the consumers. Many of the shoes in their collection are both practical and stylish, making them an excellent choice for day to day use.<br /><br /><br /> -- View this message in context: http://linuxppc.10917.n7.nabble.com/Patch-v2-1-2-5200-mpc-improve-i2c-bus-error-recovery-tp9637p69233.html Sent from the linuxppc-dev mailing list archive at Nabble.com.
--- linux-2.6.32-orig/drivers/i2c/busses/i2c-mpc.c 2009-12-03 04:51:21.000000000 +0100 +++ linux-2.6.32/drivers/i2c/busses/i2c-mpc.c 2010-02-17 16:23:11.000000000 +0100 @@ -59,6 +59,7 @@ struct mpc_i2c { wait_queue_head_t queue; struct i2c_adapter adap; int irq; + u32 real_clk; }; struct mpc_i2c_divider { @@ -93,20 +94,23 @@ static irqreturn_t mpc_i2c_isr(int irq, /* Sometimes 9th clock pulse isn't generated, and slave doesn't release * the bus, because it wants to send ACK. * Following sequence of enabling/disabling and sending start/stop generates - * the pulse, so it's all OK. + * the 9 pulses, so it's all OK. */ static void mpc_i2c_fixup(struct mpc_i2c *i2c) { - writeccr(i2c, 0); - udelay(30); - writeccr(i2c, CCR_MEN); - udelay(30); - writeccr(i2c, CCR_MSTA | CCR_MTX); - udelay(30); - writeccr(i2c, CCR_MSTA | CCR_MTX | CCR_MEN); - udelay(30); - writeccr(i2c, CCR_MEN); - udelay(30); + int k; + u32 delay_val = 1000000 / i2c->real_clk + 1; + + 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); + writeccr(i2c, CCR_MEN); + udelay(delay_val << 1); + } } static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing) @@ -186,15 +190,19 @@ static const struct mpc_i2c_divider mpc_ {10240, 0x9d}, {12288, 0x9e}, {15360, 0x9f} }; -int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler) +int mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock, int prescaler, + u32 *real_clk) { const struct mpc_i2c_divider *div = NULL; unsigned int pvr = mfspr(SPRN_PVR); u32 divider; int i; - if (!clock) + if (!clock) { + /* see below - default fdr = 0x3f -> div = 2048 */ + *real_clk = mpc5xxx_get_bus_frequency(node) / 2048; return -EINVAL; + } /* Determine divider value */ divider = mpc5xxx_get_bus_frequency(node) / clock; @@ -212,7 +220,8 @@ int mpc_i2c_get_fdr_52xx(struct device_n break; } - return div ? (int)div->fdr : -EINVAL; + *real_clk = mpc5xxx_get_bus_frequency(node) / div->divider; + return (int)div->fdr; } static void mpc_i2c_setclock_52xx(struct device_node *node, @@ -221,13 +230,14 @@ static void mpc_i2c_setclock_52xx(struct { int ret, fdr; - ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler); + ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler, &i2c->real_clk); fdr = (ret >= 0) ? ret : 0x3f; /* backward compatibility */ writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); if (ret >= 0) - dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr); + dev_info(i2c->dev, "clock %u Hz (fdr=%d)\n", i2c->real_clk, + fdr); } #else /* !CONFIG_PPC_MPC52xx */ static void mpc_i2c_setclock_52xx(struct device_node *node, @@ -287,14 +297,18 @@ u32 mpc_i2c_get_sec_cfg_8xxx(void) return val; } -int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler) +int mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock, u32 prescaler, + u32 *real_clk) { const struct mpc_i2c_divider *div = NULL; u32 divider; int i; - if (!clock) + if (!clock) { + /* see below - default fdr = 0x1031 -> div = 16 * 3072 */ + *real_clk = fsl_get_sys_freq() / prescaler / (16 * 3072); return -EINVAL; + } /* Determine proper divider value */ if (of_device_is_compatible(node, "fsl,mpc8544-i2c")) @@ -317,6 +331,7 @@ int mpc_i2c_get_fdr_8xxx(struct device_n break; } + *real_clk = fsl_get_sys_freq() / prescaler / div->divider; return div ? (int)div->fdr : -EINVAL; } @@ -326,7 +341,7 @@ static void mpc_i2c_setclock_8xxx(struct { int ret, fdr; - ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler); + ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler, &i2c->real_clk); fdr = (ret >= 0) ? ret : 0x1031; /* backward compatibility */ writeb(fdr & 0xff, i2c->base + MPC_I2C_FDR); @@ -334,7 +349,7 @@ static void mpc_i2c_setclock_8xxx(struct if (ret >= 0) dev_info(i2c->dev, "clock %d Hz (dfsrr=%d fdr=%d)\n", - clock, fdr >> 8, fdr & 0xff); + i2c->real_clk, fdr >> 8, fdr & 0xff); } #else /* !CONFIG_FSL_SOC */ @@ -446,10 +461,14 @@ static int mpc_xfer(struct i2c_adapter * return -EINTR; } if (time_after(jiffies, orig_jiffies + HZ)) { + u8 status = readb(i2c->base + MPC_I2C_SR); + dev_dbg(i2c->dev, "timeout\n"); - if (readb(i2c->base + MPC_I2C_SR) == - (CSR_MCF | CSR_MBB | CSR_RXAK)) + if ((status & (CSR_MCF | CSR_MBB | CSR_RXAK)) != 0) { + writeb(status & ~CSR_MAL, + i2c->base + MPC_I2C_SR); mpc_i2c_fixup(i2c); + } return -EIO; } schedule(); @@ -540,6 +559,14 @@ static int __devinit fsl_i2c_probe(struc } } + prop = of_get_property(op->node, "fsl,timeout", &plen); + if (prop && plen == sizeof(u32)) { + mpc_ops.timeout = *prop * HZ / 1000000; + if (mpc_ops.timeout < 5) + mpc_ops.timeout = 5; + } + dev_info(i2c->dev, "timeout %u us\n", mpc_ops.timeout * 1000000 / HZ); + dev_set_drvdata(&op->dev, i2c); i2c->adap = mpc_ops;
This patch improves the recovery of the MPC's I2C bus from errors like bus hangs resulting in timeouts: 1. make the bus timeout configurable, as it depends on the bus clock and the attached slave chip(s); default is still 1 second; 2. detect any of the cases indicated by the CF, BB and RXAK MSR flags if a timeout occurs, and add a missing (required) MAL reset; 3. use a more reliable method to fixup the bus if a hang has been detected. The sequence is sent 9 times which seems to be necessary if a slave "misses" more than one clock cycle. For 400 kHz bus speed, the fixup is also ~70us (81us vs. 150us) faster. Tested on a custom Lite5200b derived board, with a Dallas RTC, AD sensors and NXP IO expander chips attached to the i2c. Changes vs. v1: - use improved bus fixup sequence for all chips (not only the 5200) - calculate real clock from defaults if no clock is given in the device tree - better description (I hope) of the changes. I didn't split the changes in this file into three parts as recommended by Grant, as they actually belong together (i.e. they address one single problem, just in three places of one single source file). Signed-off-by: Albrecht Dreß <albrecht.dress@arcor.de> --- Note about the reset sequence: I verified the waveforms for 18.4kHz, 85.9kHz and 375kHz (drop me a note if you want to see scope screen shots). Not verified on other mpc chips yet. @Ira: thanks in advance for giving it a try on your box!