Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
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. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
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. pgpzHfLOVT5IJ.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
On Sun, May 16, 2010 at 11:47 AM, Albrecht Dreß albrecht.dr...@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. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
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. pgpiayWmxyQC6.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
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? Thanks, Albrecht. pgpNIA6G1dPFQ.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
On Thu, May 6, 2010 at 7:54 PM, Albrecht Dreß albrecht.dr...@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. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
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.dr...@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.c2009-12-03
04:51:21.0 +0100
+++ linux-2.6.32/drivers/i2c/busses/i2c-mpc.c 2010-02-17 16:23:11.0
+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 = 100 / 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,
@@
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
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.dr...@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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
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! Thanks, Albrecht. Immer auf dem Laufenden! Sport, Auto, Reise, Politik und Promis. Von uns für Sie: der neue Arcor.de-Newsletter! Jetzt anmelden und einfach alles wissen: http://www.arcor.de/rd/footer.newsletterattachment: i2c-fixup.png___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
Albrecht Dreß albrecht.dr...@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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
@@ -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 = 100 / 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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
Hi Joakim:
[snip]
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 = 100 / 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?
Sure does that work! The waveform was somewhat streched, mainly due to the
delays between some of the writeccr() calls which don't change the sda/scl
lines. Unfortunately I didn't take shots from the scope.
However, for *one* cycle, the old code needed (only counting the udelay's) 150
us. For 9 cycles, it's 1.35 ms, which isn't really nice ;-). At 375 kHz real
clock rate, delay_val is 3, i.e. each cycle consumes 9 us, or 81 us for the
whole fixup procedure. If the clock is slower, the gain is of course a lot
smaller, and at 20.5 kHz each cycle again needs 150 us...
My feeling is that the delays used in the old code are just some values which
work for sure, to if you like, my change is basically optimisation...
BTW, related to your earlier question, I checked the timings recorded with the
scope at 100 and at 20 kHz against the nxp's I2C bus specification and user
manual, rev. 03 - everything seems to be fine.
Thanks, Albrecht.
Immer auf dem Laufenden! Sport, Auto, Reise, Politik und Promis. Von uns für
Sie: der neue Arcor.de-Newsletter!
Jetzt anmelden und einfach alles wissen:
http://www.arcor.de/rd/footer.newsletter
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
Albrecht Dreß albrecht.dr...@arcor.de wrote on 2010/02/18 16:04:16:
Hi Joakim:
[snip]
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 = 100 / 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?
Sure does that work! The waveform was somewhat streched, mainly due to the
delays between some of the writeccr() calls which don't change the sda/scl
lines. Unfortunately I didn't take shots from the scope.
Yeah, the long delays has to go. So the wave form was the same but more
stretched
in time? I ask because I don't understand the writeccr(i2c, CCR_MSTA | CCR_MTX);
is supposed to do.
However, for *one* cycle, the old code needed (only counting the udelay's) 150
us. For 9 cycles, it's 1.35 ms, which isn't really nice ;-). At 375 kHz real
clock rate, delay_val is 3, i.e. each cycle consumes 9 us, or 81 us for the
whole fixup procedure. If the clock is slower, the gain is of course a lot
smaller, and at 20.5 kHz each cycle again needs 150 us...
My feeling is that the delays used in the old code are just some values
which work for sure, to if you like, my change is basically optimisation...
The old code only works when the device is stuck at the last bit. To cope
with any bit (worst case is the first bit) you need 9 cycles, 8 bits + ack = 9
Just toggling the clock 9 cycles should unlock any slave stuck in a read
operation.
To unlock slaves stuck in a write operation you also need to generate a START in
every cycle too.
As far as I can tell your patch does all of the above so
Signed-off-by: Joakim Tjernlund joakim.tjernl...@transmode.se
BTW, related to your earlier question, I checked the timings recorded with the
scope at 100 and at 20 kHz against the nxp's I2C bus specification and user
manual, rev. 03 - everything seems to be fine.
Good, thanks.
Jocke
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
On Thu, Feb 18, 2010 at 10:14 AM, Joakim Tjernlund joakim.tjernl...@transmode.se wrote: Albrecht Dreß albrecht.dr...@arcor.de wrote on 2010/02/18 16:04:16: As far as I can tell your patch does all of the above so Signed-off-by: Joakim Tjernlund joakim.tjernl...@transmode.se Thanks Joakim. On a point of process; Signed-off-by means that you've actually handled the patch and forwarded it on, either by reposting or putting it in a git tree for someone to pull. The Signed-off-by trail collects a history of all the people who have touched a patch during its development and merging. Reviewers can use Acked-by: or Reviewed-by: to indicate their consent. Ben can add your annotation when he merges the patch into his i2c tree. Cheers, g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
glik...@secretlab.ca wrote on 2010/02/18 18:41:40: On Thu, Feb 18, 2010 at 10:14 AM, Joakim Tjernlund joakim.tjernl...@transmode.se wrote: Albrecht Dreß albrecht.dr...@arcor.de wrote on 2010/02/18 16:04:16: As far as I can tell your patch does all of the above so Signed-off-by: Joakim Tjernlund joakim.tjernl...@transmode.se Thanks Joakim. On a point of process; Signed-off-by means that you've actually handled the patch and forwarded it on, either by reposting or putting it in a git tree for someone to pull. The Signed-off-by trail collects a history of all the people who have touched a patch during its development and merging. Reviewers can use Acked-by: or Reviewed-by: to indicate their consent. Ben can add your annotation when he merges the patch into his i2c tree. I see, then please change that to Acked-by: Joakim Tjernlund joakim.tjernl...@transmode.se Thanks, Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
Hi Joakim:
Am 18.02.10 18:14 schrieb(en) Joakim Tjernlund:
[snip]
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 = 100 / 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?
Sure does that work! The waveform was somewhat streched, mainly due to the
delays between some of the writeccr() calls which don't change the sda/scl lines.
Unfortunately I didn't take shots from the scope.
Yeah, the long delays has to go. So the wave form was the same but more
stretched in time? I ask because I don't understand the writeccr(i2c, CCR_MSTA
| CCR_MTX); is supposed to do.
Afaict, this is really a no-op. The '5200 user's manual says about MEN
snip
* 0 module is reset and disabled. This is the Power-ON reset. When low the
interface is held in reset, but registers can still be accessed.
* 1 I2C module is enabled. Bit must be set before other CR bits have any effect.
/snip
The change in the MSTA is needed -with the proper delays- as to generate the
START and STOP conditions.
Unfortunately, the data sheet is not very clear (or my English too bad), but
reading it *after* seeing the signals on the scope, I can at least guess what
they mean :-/
Thus, the old code will probably produce SDA and SCL held high for ~90us, then
a SDA/SCL low for ~30us (plus/minus the delays the hw adds internally according
to the clock setting), and then a ~30us SDA/SCL high. It is not possible to
get the necessary delays from the data sheet, but as I said I empirically
verified some cases to be safe.
The old code only works when the device is stuck at the last bit. To cope with
any bit (worst case is the first bit) you need 9 cycles, 8 bits + ack = 9
Just toggling the clock 9 cycles should unlock any slave stuck in a read
operation. To unlock slaves stuck in a write operation you also need to
generate a START in every cycle too.
Yes, of course... this was the initial motivation of the patch, as I *have* a
slave which sometimes needs more than one cycle!
As far as I can tell your patch does all of the above so
Signed-off-by: Joakim Tjernlund joakim.tjernl...@transmode.se
Thanks a lot again for your time, and your helpful comments!
Best, Albrecht.
pgpmK1BnBMsNC.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch v2 1/2] 5200/mpc: improve i2c bus error recovery
On Wed, Feb 17, 2010 at 11:59 AM, Albrecht Dreß albrecht.dr...@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.dr...@arcor.de
Acked-by: Grant Likely grant.lik...@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.0 +0100
+++ linux-2.6.32/drivers/i2c/busses/i2c-mpc.c 2010-02-17
16:23:11.0 +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 = 100 / 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
