M Sweger wrote:
>
> Ben,
>
> Ok!
> So what do I add to the aic7xxx.c driver to confirm this idea?
There's actually multiple things in play at the particular piece of code you
were talking about. I've taken the comments you made, the findings you were
able to come up with (good work on that BTW, I love bug reports that can
pin-point the versions of the driver that break and exactly where a workaround
is, it's helps quite a lot), and I've made a test patch. In order to apply
this, you need to upgrade your aic7xxx version to the 5.1.24 driver release
(which is on my web site), then apply this test patch. I haven't booted this
patch, but it should work (I've done compile tests only).
The general issue is that we are dealing with some registers that A) can't
handle write combining and B) are status bits that are driver by external
logic devices that happen to be very *slow* external logic devices (compared
to the chip anyway). When you write to the register it does things to the
external devices, when you read from the register you get the response from
said external devices. I've put in code to force a PCI flush, a memory
barrier because it seems to make a difference on VIA chipsets, and a udelay
after the flush so that we get the delay we need starting from after when the
write is forced. That should solve the problem.
Let me know if this solves your problem, and if so I'll be releasing the
5.1.25 driver very shortly thereafter.
--Doug Ledford <dledford@redhat.com> http://people.redhat.com/dledford Please check my web site for aic7xxx updates/answers before e-mailing me about problems
--- aic7xxx.c.save Tue Feb 8 08:38:20 2000 +++ aic7xxx.c Tue Feb 8 09:05:22 2000 @@ -270,7 +270,7 @@ 0, 0, 0, NULL, NULL, NULL, NULL, NULL, NULL, NULL }; -#define AIC7XXX_C_VERSION "5.1.24" +#define AIC7XXX_C_VERSION "5.1.25" #define NUMBER(arr) (sizeof(arr) / sizeof(arr[0])) #define MIN(a,b) (((a) < (b)) ? (a) : (b)) @@ -1478,6 +1478,7 @@ { x = inb(p->base + port); } + mb(); return(x); #else return(inb(p->base + port)); @@ -7388,6 +7389,21 @@ #undef CLOCK_PULSE } +#define CLOCK_PULSE(p) \ + do { \ + int limit = 0; \ + do { \ + mb(); \ + pause_sequencer(p); /* This is just to generate some PCI */ \ + /* traffic so the PCI read is flushed */ \ + /* it shouldn't be needed, but some */ \ + /* chipsets do indeed appear to need */ \ + /* something to force PCI reads to get */ \ + /* flushed */ \ + udelay(1); /* Do nothing */ \ + } while (((aic_inb(p, SEECTL) & SEERDY) == 0) && (++limit < 1000)); \ + } while(0) + /*+F************************************************************************* * Function: * acquire_seeprom @@ -7398,7 +7414,6 @@ static int acquire_seeprom(struct aic7xxx_host *p) { - int wait; /* * Request access of the memory port. When access is @@ -7408,12 +7423,7 @@ * should be no contention. */ aic_outb(p, SEEMS, SEECTL); - wait = 1000; /* 1000 msec = 1 second */ - while ((wait > 0) && ((aic_inb(p, SEECTL) & SEERDY) == 0)) - { - wait--; - mdelay(1); /* 1 msec */ - } + CLOCK_PULSE(p); if ((aic_inb(p, SEECTL) & SEERDY) == 0) { aic_outb(p, 0, SEECTL); @@ -7432,7 +7442,12 @@ static void release_seeprom(struct aic7xxx_host *p) { + /* + * Make sure the SEEPROM is ready before we release it. + */ + CLOCK_PULSE(p); aic_outb(p, 0, SEECTL); + CLOCK_PULSE(p); } /*+F************************************************************************* @@ -7498,18 +7513,6 @@ }; struct seeprom_cmd seeprom_read = {3, {1, 1, 0}}; -#define CLOCK_PULSE(p) \ - { \ - int limit = 0; \ - while (((aic_inb(p, SEECTL) & SEERDY) == 0) && (++limit < 1000)) \ - { \ - mb(); \ - pause_sequencer(p); /* This is just to generate some PCI */ \ - /* so the PCI read is flushed */ \ - udelay(10); /* Do nothing */ \ - } \ - } - /* * Request access of the memory port. */ @@ -7625,7 +7628,6 @@ } return (1); -#undef CLOCK_PULSE } /*+F************************************************************************* @@ -7640,12 +7642,18 @@ { unsigned char brdctl, value; + /* + * Make sure the SEEPROM is ready before we access it + */ + CLOCK_PULSE(p); if (p->features & AHC_ULTRA2) { brdctl = BRDRW_ULTRA2; aic_outb(p, brdctl, BRDCTL); - udelay(4); - return(aic_inb(p, BRDCTL)); + CLOCK_PULSE(p); + value = aic_inb(p, BRDCTL); + CLOCK_PULSE(p); + return(value); } brdctl = BRDRW; if ( !((p->chip & AHC_CHIPID_MASK) == AHC_AIC7895) || @@ -7654,10 +7662,11 @@ brdctl |= BRDCS; } aic_outb(p, brdctl, BRDCTL); - udelay(1); + CLOCK_PULSE(p); value = aic_inb(p, BRDCTL); + CLOCK_PULSE(p); aic_outb(p, 0, BRDCTL); - udelay(1); + CLOCK_PULSE(p); return (value); } @@ -7673,18 +7682,23 @@ { unsigned char brdctl; + /* + * Make sure the SEEPROM is ready before we access it + */ + CLOCK_PULSE(p); if (p->features & AHC_ULTRA2) { brdctl = value; aic_outb(p, brdctl, BRDCTL); - udelay(4); + CLOCK_PULSE(p); brdctl |= BRDSTB_ULTRA2; aic_outb(p, brdctl, BRDCTL); - udelay(4); + CLOCK_PULSE(p); brdctl &= ~BRDSTB_ULTRA2; aic_outb(p, brdctl, BRDCTL); - udelay(4); + CLOCK_PULSE(p); read_brdctl(p); + CLOCK_PULSE(p); } else { @@ -7696,18 +7710,20 @@ } brdctl = BRDSTB | BRDCS; aic_outb(p, brdctl, BRDCTL); - udelay(1); + CLOCK_PULSE(p); brdctl |= value; aic_outb(p, brdctl, BRDCTL); - udelay(1); + CLOCK_PULSE(p); brdctl &= ~BRDSTB; aic_outb(p, brdctl, BRDCTL); - udelay(1); + CLOCK_PULSE(p); brdctl &= ~BRDCS; aic_outb(p, brdctl, BRDCTL); - udelay(1); + CLOCK_PULSE(p); } } + +#undef CLOCK_PULSE /*+F************************************************************************* * Function:
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Tue Feb 15 2000 - 21:00:13 EST