* [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) @ 2007-02-02 15:18 Tejun Heo 2007-02-02 15:34 ` Sergei Shtylyov ` (3 more replies) 0 siblings, 4 replies; 33+ messages in thread From: Tejun Heo @ 2007-02-02 15:18 UTC (permalink / raw) To: ahaas, Alan Cox, linux-ide Hello, Art Haas, Alan. Okay, here's another try at fixing the detection bug. I went through intel ich docs and compared with the ide piix driver. This patch fixes the following problems. * Control bits in the timing register wasn't cleared properly while programming PIO mode. * MWDMA mode programming cleared the wrong part of control bits. I think this can fix your problem. * MWDMA mode programming cleared udma_mask even when the controller doesn't support UDMA. This doesn't matter for your case. Please test and report the result. Thanks. FOR TESTING. DO NOT APPLY. diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c index c6bf1a3..51f55a0 100644 --- a/drivers/ata/ata_piix.c +++ b/drivers/ata/ata_piix.c @@ -728,8 +728,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) if (adev->class == ATA_DEV_ATA) control |= 4; /* PPE enable */ + /* PIO configuration clears DTE unconditionally. It will be + * programmed in set_dmamode which is guaranteed to be called + * after set_piomode if any DMA mode is available. + */ pci_read_config_word(dev, master_port, &master_data); if (is_slave) { + /* clear TIME1|IE1|PPE1|DTE1 */ + master_data &= 0xff0f; /* Enable SITRE (seperate slave timing register) */ master_data |= 0x4000; /* enable PPE1, IE1 and TIME1 as needed */ @@ -737,12 +743,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) pci_read_config_byte(dev, slave_port, &slave_data); slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the timing nibble for this slave */ - slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); + slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) + << (ap->port_no ? 4 : 0); } else { - /* Master keeps the bits in a different format */ - master_data &= 0xccf8; + /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */ + master_data &= 0xccf0; /* Enable PPE, IE and TIME as appropriate */ master_data |= control; + /* load ISP and RCT */ master_data |= (timings[pio][0] << 12) | (timings[pio][1] << 8); @@ -859,7 +867,7 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */ master_data |= control << 4; pci_read_config_byte(dev, 0x44, &slave_data); - slave_data &= (0x0F + 0xE1 * ap->port_no); + slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the matching timing */ slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); pci_write_config_byte(dev, 0x44, slave_data); @@ -871,8 +879,11 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i (timings[pio][0] << 12) | (timings[pio][1] << 8); } - udma_enable &= ~(1 << devid); - pci_write_config_word(dev, master_port, master_data); + + if (ap->udma_mask) { + udma_enable &= ~(1 << devid); + pci_write_config_word(dev, master_port, master_data); + } } /* Don't scribble on 0x48 if the controller does not support UDMA */ if (ap->udma_mask) ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 15:18 [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) Tejun Heo @ 2007-02-02 15:34 ` Sergei Shtylyov 2007-02-02 16:38 ` Jeff Garzik 2007-02-02 16:41 ` Tejun Heo 2007-02-02 17:42 ` Alan ` (2 subsequent siblings) 3 siblings, 2 replies; 33+ messages in thread From: Sergei Shtylyov @ 2007-02-02 15:34 UTC (permalink / raw) To: Tejun Heo; +Cc: ahaas, Alan Cox, linux-ide Hello. Tejun Heo wrote: > Okay, here's another try at fixing the detection bug. I went through > intel ich docs and compared with the ide piix driver. This patch > fixes the following problems. > diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c > index c6bf1a3..51f55a0 100644 > --- a/drivers/ata/ata_piix.c > +++ b/drivers/ata/ata_piix.c > @@ -728,8 +728,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) > if (adev->class == ATA_DEV_ATA) > control |= 4; /* PPE enable */ > > + /* PIO configuration clears DTE unconditionally. It will be > + * programmed in set_dmamode which is guaranteed to be called > + * after set_piomode if any DMA mode is available. > + */ Actually, I think ata_timing_merge() should just be performed when setting MWDMA mode... This should be the right thing to do in most cases (however, this hardware has some complications in the form of only 2-bit wide active/recovery counts and 2 fast timing bank select bits)... > pci_read_config_word(dev, master_port, &master_data); > if (is_slave) { > + /* clear TIME1|IE1|PPE1|DTE1 */ > + master_data &= 0xff0f; Yeah, I've fixed this oversight in piix.c... > /* Enable SITRE (seperate slave timing register) */ > master_data |= 0x4000; > /* enable PPE1, IE1 and TIME1 as needed */ > @@ -737,12 +743,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) > pci_read_config_byte(dev, slave_port, &slave_data); > slave_data &= (ap->port_no ? 0x0f : 0xf0); > /* Load the timing nibble for this slave */ > - slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); > + slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) > + << (ap->port_no ? 4 : 0); > } else { > - /* Master keeps the bits in a different format */ > - master_data &= 0xccf8; > + /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */ > + master_data &= 0xccf0; > /* Enable PPE, IE and TIME as appropriate */ > master_data |= control; > + /* load ISP and RCT */ > master_data |= > (timings[pio][0] << 12) | > (timings[pio][1] << 8); > @@ -859,7 +867,7 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i > master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */ > master_data |= control << 4; > pci_read_config_byte(dev, 0x44, &slave_data); > - slave_data &= (0x0F + 0xE1 * ap->port_no); > + slave_data &= (ap->port_no ? 0x0f : 0xf0); > /* Load the matching timing */ > slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); > pci_write_config_byte(dev, 0x44, slave_data); > @@ -871,8 +879,11 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i > (timings[pio][0] << 12) | > (timings[pio][1] << 8); > } > - udma_enable &= ~(1 << devid); > - pci_write_config_word(dev, master_port, master_data); > + > + if (ap->udma_mask) { > + udma_enable &= ~(1 << devid); > + pci_write_config_word(dev, master_port, master_data); > + } I've also noticed that this is done at the end of piix_set_piomode() and I see no reason why. Isn't it just a leftover from the piix.c brokenness? This driver coupled PIO and UDMA timing updates for no conceivable reason? MBR, Sergei ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 15:34 ` Sergei Shtylyov @ 2007-02-02 16:38 ` Jeff Garzik 2007-02-02 16:57 ` Mark Lord 2007-02-02 16:41 ` Tejun Heo 1 sibling, 1 reply; 33+ messages in thread From: Jeff Garzik @ 2007-02-02 16:38 UTC (permalink / raw) To: Sergei Shtylyov, Tejun Heo; +Cc: ahaas, Alan Cox, linux-ide It's amazing how poorly we have programmed PIIX, for the lifespan of Linux... Jeff ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 16:38 ` Jeff Garzik @ 2007-02-02 16:57 ` Mark Lord 2007-02-02 18:34 ` Sergei Shtylyov 0 siblings, 1 reply; 33+ messages in thread From: Mark Lord @ 2007-02-02 16:57 UTC (permalink / raw) To: Jeff Garzik; +Cc: Sergei Shtylyov, Tejun Heo, ahaas, Alan Cox, linux-ide Jeff Garzik wrote: > It's amazing how poorly we have programmed PIIX, for the lifespan of > Linux... Actually, we've only been fiddling timings on PIIX since 1998 or so, when AH began taking over Linux IDE. Still, 10 years is an eternity. Cheers ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 16:57 ` Mark Lord @ 2007-02-02 18:34 ` Sergei Shtylyov 0 siblings, 0 replies; 33+ messages in thread From: Sergei Shtylyov @ 2007-02-02 18:34 UTC (permalink / raw) To: Mark Lord; +Cc: Jeff Garzik, Tejun Heo, ahaas, Alan Cox, linux-ide Hello. Mark Lord wrote: >> It's amazing how poorly we have programmed PIIX, for the lifespan of >> Linux... > Actually, we've only been fiddling timings on PIIX since 1998 or so, > when AH began taking over Linux IDE. Still, 10 years is an eternity. The most funny thing is that despite being called piix.c the driver seems to have always been broken for the original PIIX which didn't have separate mastre/slave timings. :-) > Cheers MBR, Sergei ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 15:34 ` Sergei Shtylyov 2007-02-02 16:38 ` Jeff Garzik @ 2007-02-02 16:41 ` Tejun Heo 2007-02-02 18:49 ` Alan 1 sibling, 1 reply; 33+ messages in thread From: Tejun Heo @ 2007-02-02 16:41 UTC (permalink / raw) To: Sergei Shtylyov; +Cc: ahaas, Alan Cox, linux-ide Sergei Shtylyov wrote: >> + /* PIO configuration clears DTE unconditionally. It will be >> + * programmed in set_dmamode which is guaranteed to be called >> + * after set_piomode if any DMA mode is available. >> + */ > > Actually, I think ata_timing_merge() should just be performed when > setting MWDMA mode... This should be the right thing to do in most cases > (however, this hardware has some complications in the form of only 2-bit > wide active/recovery counts and 2 fast timing bank select bits)... Yeap, that'll be nice. Dunno whether modifying piix/ata_piix too much would be a good idea tho considering the wide usage. >> pci_read_config_word(dev, master_port, &master_data); >> if (is_slave) { >> + /* clear TIME1|IE1|PPE1|DTE1 */ >> + master_data &= 0xff0f; > > Yeah, I've fixed this oversight in piix.c... Great, please consider updating ata_piix together next time. libata can really use some help from a someone who knows a lot about PATA including mode programming. >> + if (ap->udma_mask) { >> + udma_enable &= ~(1 << devid); >> + pci_write_config_word(dev, master_port, master_data); >> + } > > I've also noticed that this is done at the end of piix_set_piomode() > and I see no reason why. Isn't it just a leftover from the piix.c > brokenness? This driver coupled PIO and UDMA timing updates for no > conceivable reason? In both mwdma and pio cases, they're just turning off UDMA. Don't know whether it's actually necessary but still afraid to change it unless there is a good reason. -- tejun ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 16:41 ` Tejun Heo @ 2007-02-02 18:49 ` Alan 2007-02-02 19:04 ` Sergei Shtylyov 0 siblings, 1 reply; 33+ messages in thread From: Alan @ 2007-02-02 18:49 UTC (permalink / raw) To: Tejun Heo; +Cc: Sergei Shtylyov, ahaas, linux-ide On Sat, 03 Feb 2007 01:41:41 +0900 Tejun Heo <htejun@gmail.com> wrote: > > Actually, I think ata_timing_merge() should just be performed when > > setting MWDMA mode... This should be the right thing to do in most cases > > (however, this hardware has some complications in the form of only 2-bit > > wide active/recovery counts and 2 fast timing bank select bits)... > > Yeap, that'll be nice. Dunno whether modifying piix/ata_piix too much > would be a good idea tho considering the wide usage. If I remember rightly the 2bits is ok because you can set the bit to say that timings are for DMA only, the device then uses slower than PIO0 for all other cycles. > In both mwdma and pio cases, they're just turning off UDMA. Don't know > whether it's actually necessary but still afraid to change it unless > there is a good reason. The tuning manual I have does it, so I do it 8) Alan ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 18:49 ` Alan @ 2007-02-02 19:04 ` Sergei Shtylyov 0 siblings, 0 replies; 33+ messages in thread From: Sergei Shtylyov @ 2007-02-02 19:04 UTC (permalink / raw) To: Alan; +Cc: Tejun Heo, ahaas, linux-ide Hello. Alan wrote: >>> Actually, I think ata_timing_merge() should just be performed when >>>setting MWDMA mode... This should be the right thing to do in most cases >>>(however, this hardware has some complications in the form of only 2-bit >>>wide active/recovery counts and 2 fast timing bank select bits)... >>Yeap, that'll be nice. Dunno whether modifying piix/ata_piix too much >>would be a good idea tho considering the wide usage. > If I remember rightly the 2bits is ok because you can set the bit to say > that timings are for DMA only, Yeah, after thinking a bit more, the current logic seems good enough, i. e. it's better to slow down PIO to mode 0 than further slow down DMA to match the current PIO speed -- however, this is already happening with MWDMA1 which is actually faster than PIO3 (150 vs 180 ns). But well, who cares... :-) > the device then uses slower than PIO0 for all other cycles. Well, thankfully, compatible mode is PIO0 exactly (except for taskfile accesses which are way slower indeed). >>In both mwdma and pio cases, they're just turning off UDMA. Don't know >>whether it's actually necessary but still afraid to change it unless >>there is a good reason. > The tuning manual I have does it, so I do it 8) Do you mean 29860004.pdf? Actually, I'm not seeing anything alike here. It would've been stupid idea to couple UDMA to PIO but well... after looking at the HighPoint datasheets, one becomes hard to surprise. :-) > Alan Well, after looking at do_pata_set_dmamode(), I have another question: why IORDY enable is forced here? It has *nothing* to do with the IDE DMA protocol. (This also seems to be an issue with piix.c...) MBR, Sergei ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 15:18 [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) Tejun Heo 2007-02-02 15:34 ` Sergei Shtylyov @ 2007-02-02 17:42 ` Alan 2007-02-03 1:40 ` Tejun Heo 2007-02-02 21:14 ` Art Haas 2007-04-30 18:29 ` [PATCH] Fix pio/mwdma programming on ata_piix.c Art Haas 3 siblings, 1 reply; 33+ messages in thread From: Alan @ 2007-02-02 17:42 UTC (permalink / raw) To: Tejun Heo; +Cc: ahaas, linux-ide > * Control bits in the timing register wasn't cleared properly while > programming PIO mode. Yep and if the BIOS programmed the slave into DMA that might not be ideal. > * MWDMA mode programming cleared the wrong part of control bits. I > think this can fix your problem. Your change does nothing here. 0x0F + 1 * 0xE1 = 0xF0 It's just a construct to avoid the use of the ugly C "?" operator. The rest of the code uses ? so the change makes sense for style, but it doesn't appear to be a bug. > * MWDMA mode programming cleared udma_mask even when the controller > doesn't support UDMA. This doesn't matter for your case. Or on the actual hardware. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 17:42 ` Alan @ 2007-02-03 1:40 ` Tejun Heo 2007-02-03 20:04 ` Alan 0 siblings, 1 reply; 33+ messages in thread From: Tejun Heo @ 2007-02-03 1:40 UTC (permalink / raw) To: Alan; +Cc: ahaas, linux-ide Alan wrote: >> * Control bits in the timing register wasn't cleared properly while >> programming PIO mode. > > Yep and if the BIOS programmed the slave into DMA that might not be ideal. How so? The bit will be programmed by set_dmamode() right after set_piomode() is complete. >> * MWDMA mode programming cleared the wrong part of control bits. I >> think this can fix your problem. > > Your change does nothing here. > > 0x0F + 1 * 0xE1 = 0xF0 > > It's just a construct to avoid the use of the ugly C "?" operator. The > rest of the code uses ? so the change makes sense for style, but it > doesn't appear to be a bug. (0x0F + 0xE1 * ap->port_no) 1. when ap->port_no == 0 (0x0f + 0xe1 * 0) == 0x0f 2. when ap->port-no == 1 (0x0f + 0xe1 * 1) == 0xf0 (ap->port_no ? 0x0f : 0xf0) 1. when ap->port_no == 0 (0 ? 0x0f : 0xf0) == 0xf0 2. when ap->port_no == 1 (1 ? 0x0f : 0xf0) == 0x0f See the difference? Smart one liners are dangerous. ?* is _much_ better than cryptic arithmetic. >> * MWDMA mode programming cleared udma_mask even when the controller >> doesn't support UDMA. This doesn't matter for your case. > > Or on the actual hardware. I was trying to make it more consistent with pio counterpart. We can remove if() from set_piomode too. Let's just keep things in sync between stuff including ide piix driver. -- tejun ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-03 1:40 ` Tejun Heo @ 2007-02-03 20:04 ` Alan 2007-02-04 2:47 ` Tejun Heo 0 siblings, 1 reply; 33+ messages in thread From: Alan @ 2007-02-03 20:04 UTC (permalink / raw) To: Tejun Heo; +Cc: ahaas, linux-ide > > Yep and if the BIOS programmed the slave into DMA that might not be ideal. > > How so? The bit will be programmed by set_dmamode() right after > set_piomode() is complete. IFF we also put the device into a DMA mode. A blacklisted device would be wrong. > See the difference? Smart one liners are dangerous. ?* is _much_ > better than cryptic arithmetic. Agreed > > >> * MWDMA mode programming cleared udma_mask even when the controller > >> doesn't support UDMA. This doesn't matter for your case. > > > > Or on the actual hardware. > > I was trying to make it more consistent with pio counterpart. We can > remove if() from set_piomode too. Let's just keep things in sync > between stuff including ide piix driver. The one you must not touch is the UDMA register rather than added UDMA bits in older registers. Alan ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-03 20:04 ` Alan @ 2007-02-04 2:47 ` Tejun Heo 0 siblings, 0 replies; 33+ messages in thread From: Tejun Heo @ 2007-02-04 2:47 UTC (permalink / raw) To: Alan; +Cc: ahaas, linux-ide Alan wrote: >>> Yep and if the BIOS programmed the slave into DMA that might not be ideal. >> How so? The bit will be programmed by set_dmamode() right after >> set_piomode() is complete. > > IFF we also put the device into a DMA mode. A blacklisted device would be > wrong. Hmmm... I might be misunderstanding, but if libata is going to put the device into PIO mode, why does it matter what BIOS configured slave to? The slave is going to be put into PIO mode libata selected so the control bits should match, no? [--snip--] >>>> * MWDMA mode programming cleared udma_mask even when the controller >>>> doesn't support UDMA. This doesn't matter for your case. >>> Or on the actual hardware. >> I was trying to make it more consistent with pio counterpart. We can >> remove if() from set_piomode too. Let's just keep things in sync >> between stuff including ide piix driver. > > The one you must not touch is the UDMA register rather than added UDMA > bits in older registers. OIC. I was confused there. Thanks. -- tejun ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 15:18 [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) Tejun Heo 2007-02-02 15:34 ` Sergei Shtylyov 2007-02-02 17:42 ` Alan @ 2007-02-02 21:14 ` Art Haas 2007-02-03 2:09 ` Tejun Heo 2007-04-30 18:29 ` [PATCH] Fix pio/mwdma programming on ata_piix.c Art Haas 3 siblings, 1 reply; 33+ messages in thread From: Art Haas @ 2007-02-02 21:14 UTC (permalink / raw) To: Tejun Heo; +Cc: Alan Cox, linux-ide On Sat, Feb 03, 2007 at 12:18:56AM +0900, Tejun Heo wrote: > Hello, Art Haas, Alan. > > Okay, here's another try at fixing the detection bug. I went through > intel ich docs and compared with the ide piix driver. This patch > fixes the following problems. > > * Control bits in the timing register wasn't cleared properly while > programming PIO mode. > > * MWDMA mode programming cleared the wrong part of control bits. I > think this can fix your problem. > > * MWDMA mode programming cleared udma_mask even when the controller > doesn't support UDMA. This doesn't matter for your case. > > Please test and report the result. Thanks. Hi. Sorry to say the CD-ROM is still not found. Full 'dmesg' output listed below in hopes it provides clues. $ dmesg Linux version 2.6.20-rc7-ajh (arth@pcdebian) (gcc version 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)) #3 Fri Feb 2 12:15:03 CST 2007 BIOS-provided physical RAM map: sanitize start sanitize end copy_e820_map() start: 0000000000000000 size: 00000000000a0000 end: 00000000000a0000 type: 1 copy_e820_map() type is E820_RAM copy_e820_map() start: 00000000000f0000 size: 0000000000010000 end: 0000000000100000 type: 2 copy_e820_map() start: 0000000000100000 size: 000000001fef0000 end: 000000001fff0000 type: 1 copy_e820_map() type is E820_RAM copy_e820_map() start: 000000001fff0000 size: 0000000000003000 end: 000000001fff3000 type: 4 copy_e820_map() start: 000000001fff3000 size: 000000000000d000 end: 0000000020000000 type: 3 copy_e820_map() start: 00000000ffff0000 size: 0000000000010000 end: 0000000100000000 type: 2 BIOS-e820: 0000000000000000 - 00000000000a0000 (usable) BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved) BIOS-e820: 0000000000100000 - 000000001fff0000 (usable) BIOS-e820: 000000001fff0000 - 000000001fff3000 (ACPI NVS) BIOS-e820: 000000001fff3000 - 0000000020000000 (ACPI data) BIOS-e820: 00000000ffff0000 - 0000000100000000 (reserved) 511MB LOWMEM available. Entering add_active_range(0, 0, 131056) 0 entries of 256 used Zone PFN ranges: DMA 0 -> 4096 Normal 4096 -> 131056 early_node_map[1] active PFN ranges 0: 0 -> 131056 On node 0 totalpages: 131056 DMA zone: 32 pages used for memmap DMA zone: 0 pages reserved DMA zone: 4064 pages, LIFO batch:0 Normal zone: 991 pages used for memmap Normal zone: 125969 pages, LIFO batch:31 DMI 2.1 present. ACPI: RSDP (v000 GBT ) @ 0x000f6c60 ACPI: RSDT (v001 GBT AWRDACPI 0x00000000 0x00000000) @ 0x1fff3000 ACPI: FADT (v001 GBT AWRDACPI 0x00000000 0x00000000) @ 0x1fff3040 ACPI: DSDT (v001 GBT AWRDACPI 0x00001000 MSFT 0x01000007) @ 0x00000000 ACPI: PM-Timer IO Port: 0x4008 Allocating PCI resources starting at 30000000 (gap: 20000000:dfff0000) Detected 398.969 MHz processor. Built 1 zonelists. Total pages: 130033 Kernel command line: BOOT_IMAGE=2.6.20-rc7-ajh ro root=LABEL=/ root=/dev/sda1 Local APIC disabled by BIOS -- you can enable it with "lapic" mapped APIC to ffffd000 (01402000) Enabling fast FPU save and restore... done. Initializing CPU#0 PID hash table entries: 2048 (order: 11, 8192 bytes) Console: colour VGA+ 80x25 Dentry cache hash table entries: 65536 (order: 6, 262144 bytes) Inode-cache hash table entries: 32768 (order: 5, 131072 bytes) Memory: 516956k/524224k available (1461k kernel code, 6724k reserved, 484k data, 148k init, 0k highmem) virtual kernel memory layout: fixmap : 0xffff7000 - 0xfffff000 ( 32 kB) vmalloc : 0xe0800000 - 0xffff5000 ( 503 MB) lowmem : 0xc0000000 - 0xdfff0000 ( 511 MB) .init : 0xc02ea000 - 0xc030f000 ( 148 kB) .data : 0xc026d68f - 0xc02e6a50 ( 484 kB) .text : 0xc0100000 - 0xc026d68f (1461 kB) Checking if this processor honours the WP bit even in supervisor mode... Ok. Calibrating delay using timer specific routine.. 798.28 BogoMIPS (lpj=399143) Mount-cache hash table entries: 512 CPU: After generic identify, caps: 0183f9ff 00000000 00000000 00000000 00000000 00000000 00000000 CPU: L1 I cache: 16K, L1 D cache: 16K CPU: L2 cache: 512K CPU: After all inits, caps: 0183f9ff 00000000 00000000 00000040 00000000 00000000 00000000 Intel machine check architecture supported. Intel machine check reporting enabled on CPU#0. CPU: Intel Pentium II (Deschutes) stepping 01 Checking 'hlt' instruction... OK. ACPI: Core revision 20060707 ACPI: setting ELCR to 0200 (from 1c00) NET: Registered protocol family 16 ACPI: bus type pci registered PCI: PCI BIOS revision 2.10 entry at 0xfb3c0, last bus=1 PCI: Using configuration type 1 Setting up standard PCI resources ACPI: Interpreter enabled ACPI: Using PIC for interrupt routing ACPI: PCI Root Bridge [PCI0] (0000:00) PCI: Probing PCI hardware (bus 00) ACPI: Assume root bridge [\_SB_.PCI0] bus is 0 * Found PM-Timer Bug on the chipset. Due to workarounds for a bug, * this clock source is slow. Consider trying other clock sources PCI quirk: region 4000-403f claimed by PIIX4 ACPI PCI quirk: region 5000-500f claimed by PIIX4 SMB Boot video device is 0000:00:08.0 ACPI: PCI Interrupt Routing Table [\_SB_.PCI0._PRT] ACPI: PCI Interrupt Link [LNKA] (IRQs 3 4 5 6 7 *10 11 12 14 15) ACPI: PCI Interrupt Link [LNKB] (IRQs 3 4 5 6 7 10 11 12 14 15) *0, disabled. ACPI: PCI Interrupt Link [LNKC] (IRQs 3 4 5 6 7 10 11 *12 14 15) ACPI: PCI Interrupt Link [LNKD] (IRQs 3 4 5 6 7 10 *11 12 14 15) ACPI: Power Resource [PFAN] (on) Linux Plug and Play Support v0.97 (c) Adam Belay pnp: PnP ACPI init pnp: ACPI device : hid PNP0A03 pnp: ACPI device : hid PNP0C01 pnp: ACPI device : hid PNP0C02 pnp: ACPI device : hid PNP0200 pnp: ACPI device : hid PNP0B00 pnp: ACPI device : hid PNP0800 pnp: ACPI device : hid PNP0C04 pnp: ACPI device : hid PNP0700 pnp: ACPI device : hid PNP0501 pnp: ACPI device : hid PNP0501 pnp: ACPI device : hid PNP0400 pnp: ACPI device : hid PNP0303 pnp: PnP ACPI: found 12 devices PnPBIOS: Disabled by ACPI PNP SCSI subsystem initialized libata version 2.00 loaded. PCI: Using ACPI for IRQ routing PCI: If a device doesn't work, try "pci=routeirq". If it helps, post a report pnp: the driver 'system' has been registered pnp: match found with the PnP device '00:01' and the driver 'system' pnp: match found with the PnP device '00:02' and the driver 'system' PCI: Bridge: 0000:00:01.0 IO window: d000-dfff MEM window: disabled. PREFETCH window: disabled. NET: Registered protocol family 2 IP route cache hash table entries: 4096 (order: 2, 16384 bytes) TCP established hash table entries: 16384 (order: 4, 65536 bytes) TCP bind hash table entries: 8192 (order: 3, 32768 bytes) TCP: Hash tables configured (established 16384 bind 8192) TCP reno registered io scheduler noop registered io scheduler anticipatory registered (default) io scheduler deadline registered io scheduler cfq registered Limiting direct PCI/PCI transfers. isapnp: Scanning for PnP cards... isapnp: No Plug & Play device found Serial: 8250/16550 driver $Revision: 1.90 $ 4 ports, IRQ sharing disabled serial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A serial8250: ttyS1 at I/O 0x2f8 (irq = 3) is a 16550A pnp: the driver 'serial' has been registered pnp: match found with the PnP device '00:08' and the driver 'serial' 00:08: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A pnp: match found with the PnP device '00:09' and the driver 'serial' 00:09: ttyS1 at I/O 0x2f8 (irq = 3) is a 16550A ata_piix 0000:00:07.1: version 2.00ac7 ata1: PATA max UDMA/33 cmd 0x1F0 ctl 0x3F6 bmdma 0xF000 irq 14 ata2: PATA max UDMA/33 cmd 0x170 ctl 0x376 bmdma 0xF008 irq 15 scsi0 : ata_piix ata1.00: ATA-2, max UDMA/33, 6303024 sectors: LBA ata1.00: ata1: dev 0 multi count 16 ata1.01: ATA-4, max UDMA/66, 16514064 sectors: LBA ata1.01: ata1: dev 1 multi count 16 ata1.00: configured for UDMA/33 ata1.01: configured for UDMA/33 scsi1 : ata_piix ata2.00: ATAPI, max MWDMA1 ata2.00: revalidation failed (errno=-2) ata2.00: limiting speed to PIO4 ata2: failed to recover some devices, retrying in 5 secs ata2.00: revalidation failed (errno=-2) ata2.00: limiting speed to PIO0 ata2: failed to recover some devices, retrying in 5 secs ata2.00: revalidation failed (errno=-2) ata2.00: disabled scsi 0:0:0:0: Direct-Access ATA ST33232A 3.02 PQ: 0 ANSI: 5 SCSI device sda: 6303024 512-byte hdwr sectors (3227 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: disabled, read cache: enabled, doesn't support DPO or FUA SCSI device sda: 6303024 512-byte hdwr sectors (3227 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: disabled, read cache: enabled, doesn't support DPO or FUA sda: sda1 sda2 < sda5 sda6 sda7 > sd 0:0:0:0: Attached scsi disk sda scsi 0:0:1:0: Direct-Access ATA FUJITSU MPD3084A DD-0 PQ: 0 ANSI: 5 SCSI device sdb: 16514064 512-byte hdwr sectors (8455 MB) sdb: Write Protect is off sdb: Mode Sense: 00 3a 00 00 SCSI device sdb: write cache: enabled, read cache: enabled, doesn't support DPO or FUA SCSI device sdb: 16514064 512-byte hdwr sectors (8455 MB) sdb: Write Protect is off sdb: Mode Sense: 00 3a 00 00 SCSI device sdb: write cache: enabled, read cache: enabled, doesn't support DPO or FUA sdb: sdb1 sdb2 < sdb5 sdb6 > sdb3 sd 0:0:1:0: Attached scsi disk sdb pnp: the driver 'i8042 kbd' has been registered pnp: match found with the PnP device '00:0b' and the driver 'i8042 kbd' pnp: the driver 'i8042 aux' has been registered PNP: PS/2 Controller [PNP0303:PS2K] at 0x60,0x64 irq 1 PNP: PS/2 controller doesn't have AUX irq; using default 12 serio: i8042 KBD port at 0x60,0x64 irq 1 mice: PS/2 mouse device common for all mice input: AT Translated Set 2 keyboard as /class/input/input0 TCP cubic registered Using IPI Shortcut mode Time: tsc clocksource has been installed. VFS: Mounted root (ext2 filesystem) readonly. Freeing unused kernel memory: 148k freed NET: Registered protocol family 1 Floppy drive(s): fd0 is 1.44M FDC 0 is a post-1991 82077 Real Time Clock Driver v1.12ac pnp: the driver 'parport_pc' has been registered pnp: match found with the PnP device '00:0a' and the driver 'parport_pc' parport: PnPBIOS parport detected. parport0: PC-style at 0x378, irq 7 [PCSPP(,...)] 8139too Fast Ethernet driver 0.9.28 ACPI: PCI Interrupt Link [LNKC] enabled at IRQ 12 PCI: setting IRQ 12 as level-triggered ACPI: PCI Interrupt 0000:00:0a.0[A] -> Link [LNKC] -> GSI 12 (level, low) -> IRQ 12 eth0: RealTek RTL8139 at 0xe0816000, 00:05:5d:45:47:96, IRQ 12 eth0: Identified 8139 chip type 'RTL-8139C' Adding 100760k swap on /dev/sda7. Priority:-1 extents:1 across:100760k kjournald starting. Commit interval 5 seconds EXT3-fs: mounted filesystem with ordered data mode. kjournald starting. Commit interval 5 seconds EXT3-fs: mounted filesystem with ordered data mode. kjournald starting. Commit interval 5 seconds EXT3-fs: mounted filesystem with ordered data mode. kjournald starting. Commit interval 5 seconds EXT3-fs: mounted filesystem with ordered data mode. NET: Registered protocol family 17 eth0: link up, 100Mbps, full-duplex, lpa 0x45E1 eth0: link up, 100Mbps, full-duplex, lpa 0x45E1 ttyS3: LSR safety check engaged! ttyS3: LSR safety check engaged! $ -- Man once surrendering his reason, has no remaining guard against absurdities the most monstrous, and like a ship without rudder, is the sport of every wind. -Thomas Jefferson to James Smith, 1822 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-02 21:14 ` Art Haas @ 2007-02-03 2:09 ` Tejun Heo 2007-02-03 14:35 ` Art Haas 2007-02-03 19:47 ` Mark Lord 0 siblings, 2 replies; 33+ messages in thread From: Tejun Heo @ 2007-02-03 2:09 UTC (permalink / raw) To: Art Haas; +Cc: Alan Cox, linux-ide, Albert Lee, Mark Lord, Sergei Shtylyov (cc'ing Albert, Mark and Sergei. Hi!) Art Haas wrote: > Hi. Sorry to say the CD-ROM is still not found. Full 'dmesg' output > listed below in hopes it provides clues. Erggghh.. I'm sorry too. I thought I found it this time. > ata_piix 0000:00:07.1: version 2.00ac7 > ata2: PATA max UDMA/33 cmd 0x170 ctl 0x376 bmdma 0xF008 irq 15 You have UDMA/33 PATA port. > ata2.00: ATAPI, max MWDMA1 The initial reset and PACKET_IDENTIFY went well. > ata2.00: revalidation failed (errno=-2) Then, PACKET_IDENTIFY after configuring transfer mode fails with -ENOENT. Meaning it saw (status & (ATA_BUSY|ATA_DRQ|ATA_ERR|ATA_DF)) == 0 in HSM_ST. libata resets silently and tries to revalidate the device. > ata2.00: limiting speed to PIO4 > ata2: failed to recover some devices, retrying in 5 secs > ata2.00: revalidation failed (errno=-2) > ata2.00: limiting speed to PIO0 > ata2: failed to recover some devices, retrying in 5 secs > ata2.00: revalidation failed (errno=-2) > ata2.00: disabled All of those tries fail and libata gives up the device. * Something went wrong with mode programming. It seems pretty old (reporting max UDMA/33). Art, can you post the result of 'lspci -nn'? * After mode programming, it might need some delay between clearing ATA_BUSY and setting ATA_DRQ. Dunno if this is possible. Just throwing out thoughts here. So, PATA gurus, can you bless us with enlightenment? :-) -- tejun ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-03 2:09 ` Tejun Heo @ 2007-02-03 14:35 ` Art Haas 2007-02-03 19:47 ` Mark Lord 1 sibling, 0 replies; 33+ messages in thread From: Art Haas @ 2007-02-03 14:35 UTC (permalink / raw) To: Tejun Heo; +Cc: Alan Cox, linux-ide, Albert Lee, Mark Lord, Sergei Shtylyov On Sat, Feb 03, 2007 at 11:09:19AM +0900, Tejun Heo wrote: > (cc'ing Albert, Mark and Sergei. Hi!) > > Art Haas wrote: > > Hi. Sorry to say the CD-ROM is still not found. Full 'dmesg' output > > listed below in hopes it provides clues. > > Erggghh.. I'm sorry too. I thought I found it this time. > > > ata_piix 0000:00:07.1: version 2.00ac7 > > ata2: PATA max UDMA/33 cmd 0x170 ctl 0x376 bmdma 0xF008 irq 15 > > You have UDMA/33 PATA port. > > > ata2.00: ATAPI, max MWDMA1 > > The initial reset and PACKET_IDENTIFY went well. > > > ata2.00: revalidation failed (errno=-2) > > Then, PACKET_IDENTIFY after configuring transfer mode fails with > -ENOENT. Meaning it saw (status & (ATA_BUSY|ATA_DRQ|ATA_ERR|ATA_DF)) == > 0 in HSM_ST. > > libata resets silently and tries to revalidate the device. > > > ata2.00: limiting speed to PIO4 > > ata2: failed to recover some devices, retrying in 5 secs > > ata2.00: revalidation failed (errno=-2) > > ata2.00: limiting speed to PIO0 > > ata2: failed to recover some devices, retrying in 5 secs > > ata2.00: revalidation failed (errno=-2) > > ata2.00: disabled > > All of those tries fail and libata gives up the device. > > * Something went wrong with mode programming. It seems pretty old > (reporting max UDMA/33). Art, can you post the result of 'lspci -nn'? Here you are ... # lspci -nn 00:00.0 Host bridge [0600]: Intel Corporation 440BX/ZX/DX - 82443BX/ZX/DX Host bridge [8086:7190] (rev 02) 00:01.0 PCI bridge [0604]: Intel Corporation 440BX/ZX/DX - 82443BX/ZX/DX AGP bridge [8086:7191] (rev 02) 00:07.0 ISA bridge [0601]: Intel Corporation 82371AB/EB/MB PIIX4 ISA [8086:7110] (rev 02) 00:07.1 IDE interface [0101]: Intel Corporation 82371AB/EB/MB PIIX4 IDE [8086:7111] (rev 01) 00:07.2 USB Controller [0c03]: Intel Corporation 82371AB/EB/MB PIIX4 USB [8086:7112] (rev 01) 00:07.3 Bridge [0680]: Intel Corporation 82371AB/EB/MB PIIX4 ACPI [8086:7113] (rev 02) 00:08.0 VGA compatible controller [0300]: S3 Inc. ViRGE/DX or /GX [5333:8a01] (rev 01) 00:0a.0 Ethernet controller [0200]: D-Link System Inc RTL8139 Ethernet [1186:1300] (rev 10) # Please let me know of anything thing else I can send that may help. Art Haas -- Man once surrendering his reason, has no remaining guard against absurdities the most monstrous, and like a ship without rudder, is the sport of every wind. -Thomas Jefferson to James Smith, 1822 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-03 2:09 ` Tejun Heo 2007-02-03 14:35 ` Art Haas @ 2007-02-03 19:47 ` Mark Lord 2007-02-06 9:11 ` Tejun Heo 1 sibling, 1 reply; 33+ messages in thread From: Mark Lord @ 2007-02-03 19:47 UTC (permalink / raw) To: Tejun Heo; +Cc: Art Haas, Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov Tejun Heo wrote: > .. > Then, PACKET_IDENTIFY after configuring transfer mode fails with > -ENOENT. Meaning it saw (status & (ATA_BUSY|ATA_DRQ|ATA_ERR|ATA_DF)) == > 0 in HSM_ST. .. > So, PATA gurus, can you bless us with enlightenment? :-) Heh.. guaranteeing detection of all the strange implementations out there is part black magic. But the simple thing to do here is, just for fun, hack the code to do the infamous 50 millisecond hard wait before issuing the PACKET_IDENTIFY. If that fixes it, then it's just a matter of tuning to discover the real amount of delay required, and a nicer way of doing the delay. Also, zero out the features register before issuing PACKET_IDENTIFY, if the code isn't already doing that. After the drive asserts BUSY, and later deasserts BUSY, there might be a slight delay before the drive asserts DRQ. So, it is possible for the status to read zeros in the important bits. My suggestion is to wait up to the infamous 50 milliseconds again here, if needed. ???? Cheers -- Mark Lord Real-Time Remedies Inc. mlord@pobox.com ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-03 19:47 ` Mark Lord @ 2007-02-06 9:11 ` Tejun Heo 2007-02-06 16:33 ` Art Haas 0 siblings, 1 reply; 33+ messages in thread From: Tejun Heo @ 2007-02-06 9:11 UTC (permalink / raw) To: Mark Lord, Art Haas; +Cc: Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov [-- Attachment #1: Type: text/plain, Size: 1230 bytes --] Mark Lord wrote: > Tejun Heo wrote: >> .. >> Then, PACKET_IDENTIFY after configuring transfer mode fails with >> -ENOENT. Meaning it saw (status & (ATA_BUSY|ATA_DRQ|ATA_ERR|ATA_DF)) == >> 0 in HSM_ST. > .. >> So, PATA gurus, can you bless us with enlightenment? :-) > > Heh.. guaranteeing detection of all the strange implementations out there > is part black magic. > > But the simple thing to do here is, just for fun, hack the code > to do the infamous 50 millisecond hard wait before issuing the > PACKET_IDENTIFY. > If that fixes it, then it's just a matter of tuning to discover the real > amount of delay required, and a nicer way of doing the delay. Okay. > Also, zero out the features register before issuing PACKET_IDENTIFY, > if the code isn't already doing that. Okay. > After the drive asserts BUSY, and later deasserts BUSY, > there might be a slight delay before the drive asserts DRQ. > So, it is possible for the status to read zeros in the important bits. > > My suggestion is to wait up to the infamous 50 milliseconds again here, > if needed. Okay, the attached patch does what Mark suggested. Art, can you please give it a shot and report dmesg? My thanks for sticking around till now. -- tejun [-- Attachment #2: wait-for-drq.patch --] [-- Type: text/x-patch, Size: 1089 bytes --] diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 667acd2..4c81433 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1478,7 +1478,8 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class, } tf.protocol = ATA_PROT_PIO; - tf.flags |= ATA_TFLAG_POLLING; /* for polling presence detection */ + /* POLLING for polling detection, ISADDR|DEVICE to clear TF */ + tf.flags |= ATA_TFLAG_POLLING | ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE, id, sizeof(id[0]) * ATA_ID_WORDS); @@ -4477,6 +4478,18 @@ fsm_start: goto fsm_start; } else { + unsigned long timeout = jiffies + msecs_to_jiffies(50); + + while (time_before(jiffies, timeout)) { + if (status & ATA_DRQ) + break; + ata_dev_printk(qc->dev, KERN_INFO, + "XXX: waiting for DRQ, status=0x%x\n", + status); + msleep(10); + status = ata_chk_status(ap); + } + /* ATA PIO protocol */ if (unlikely((status & ATA_DRQ) == 0)) { /* handle BSY=0, DRQ=0 as error */ ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-06 9:11 ` Tejun Heo @ 2007-02-06 16:33 ` Art Haas 2007-02-07 2:53 ` Tejun Heo 0 siblings, 1 reply; 33+ messages in thread From: Art Haas @ 2007-02-06 16:33 UTC (permalink / raw) To: Tejun Heo; +Cc: Mark Lord, Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov On Tue, Feb 06, 2007 at 06:11:44PM +0900, Tejun Heo wrote: > Mark Lord wrote: > > Tejun Heo wrote: > >> .. > >> Then, PACKET_IDENTIFY after configuring transfer mode fails with > >> -ENOENT. Meaning it saw (status & (ATA_BUSY|ATA_DRQ|ATA_ERR|ATA_DF)) == > >> 0 in HSM_ST. > > .. > >> So, PATA gurus, can you bless us with enlightenment? :-) > > > > Heh.. guaranteeing detection of all the strange implementations out there > > is part black magic. > > > > But the simple thing to do here is, just for fun, hack the code > > to do the infamous 50 millisecond hard wait before issuing the > > PACKET_IDENTIFY. > > If that fixes it, then it's just a matter of tuning to discover the real > > amount of delay required, and a nicer way of doing the delay. > > Okay. > > > Also, zero out the features register before issuing PACKET_IDENTIFY, > > if the code isn't already doing that. > > Okay. > > > After the drive asserts BUSY, and later deasserts BUSY, > > there might be a slight delay before the drive asserts DRQ. > > So, it is possible for the status to read zeros in the important bits. > > > > My suggestion is to wait up to the infamous 50 milliseconds again here, > > if needed. > > Okay, the attached patch does what Mark suggested. Art, can you please > give it a shot and report dmesg? My thanks for sticking around till now. > SUCCESS!!!!! $ dmesg Linux version 2.6.20-ajh (arth@pcdebian) (gcc version 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)) #2 Tue Feb 6 08:36:33 CST 2007 BIOS-provided physical RAM map: sanitize start sanitize end copy_e820_map() start: 0000000000000000 size: 00000000000a0000 end: 00000000000a0000 type: 1 copy_e820_map() type is E820_RAM copy_e820_map() start: 00000000000f0000 size: 0000000000010000 end: 0000000000100000 type: 2 copy_e820_map() start: 0000000000100000 size: 000000001fef0000 end: 000000001fff0000 type: 1 copy_e820_map() type is E820_RAM copy_e820_map() start: 000000001fff0000 size: 0000000000003000 end: 000000001fff3000 type: 4 copy_e820_map() start: 000000001fff3000 size: 000000000000d000 end: 0000000020000000 type: 3 copy_e820_map() start: 00000000ffff0000 size: 0000000000010000 end: 0000000100000000 type: 2 BIOS-e820: 0000000000000000 - 00000000000a0000 (usable) BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved) BIOS-e820: 0000000000100000 - 000000001fff0000 (usable) BIOS-e820: 000000001fff0000 - 000000001fff3000 (ACPI NVS) BIOS-e820: 000000001fff3000 - 0000000020000000 (ACPI data) BIOS-e820: 00000000ffff0000 - 0000000100000000 (reserved) 511MB LOWMEM available. Entering add_active_range(0, 0, 131056) 0 entries of 256 used Zone PFN ranges: DMA 0 -> 4096 Normal 4096 -> 131056 early_node_map[1] active PFN ranges 0: 0 -> 131056 On node 0 totalpages: 131056 DMA zone: 32 pages used for memmap DMA zone: 0 pages reserved DMA zone: 4064 pages, LIFO batch:0 Normal zone: 991 pages used for memmap Normal zone: 125969 pages, LIFO batch:31 DMI 2.1 present. ACPI: RSDP (v000 GBT ) @ 0x000f6c60 ACPI: RSDT (v001 GBT AWRDACPI 0x00000000 0x00000000) @ 0x1fff3000 ACPI: FADT (v001 GBT AWRDACPI 0x00000000 0x00000000) @ 0x1fff3040 ACPI: DSDT (v001 GBT AWRDACPI 0x00001000 MSFT 0x01000007) @ 0x00000000 ACPI: PM-Timer IO Port: 0x4008 Allocating PCI resources starting at 30000000 (gap: 20000000:dfff0000) Detected 398.959 MHz processor. Built 1 zonelists. Total pages: 130033 Kernel command line: BOOT_IMAGE=2.6.20-ajh ro root=LABEL=/ root=/dev/sda1 Local APIC disabled by BIOS -- you can enable it with "lapic" mapped APIC to ffffd000 (01402000) Enabling fast FPU save and restore... done. Initializing CPU#0 PID hash table entries: 2048 (order: 11, 8192 bytes) Console: colour VGA+ 80x25 Dentry cache hash table entries: 65536 (order: 6, 262144 bytes) Inode-cache hash table entries: 32768 (order: 5, 131072 bytes) Memory: 516956k/524224k available (1460k kernel code, 6724k reserved, 485k data, 148k init, 0k highmem) virtual kernel memory layout: fixmap : 0xffff7000 - 0xfffff000 ( 32 kB) vmalloc : 0xe0800000 - 0xffff5000 ( 503 MB) lowmem : 0xc0000000 - 0xdfff0000 ( 511 MB) .init : 0xc02ea000 - 0xc030f000 ( 148 kB) .data : 0xc026d38f - 0xc02e6a50 ( 485 kB) .text : 0xc0100000 - 0xc026d38f (1460 kB) Checking if this processor honours the WP bit even in supervisor mode... Ok. Calibrating delay using timer specific routine.. 798.30 BogoMIPS (lpj=399150) Mount-cache hash table entries: 512 CPU: After generic identify, caps: 0183f9ff 00000000 00000000 00000000 00000000 00000000 00000000 CPU: L1 I cache: 16K, L1 D cache: 16K CPU: L2 cache: 512K CPU: After all inits, caps: 0183f9ff 00000000 00000000 00000040 00000000 00000000 00000000 Intel machine check architecture supported. Intel machine check reporting enabled on CPU#0. CPU: Intel Pentium II (Deschutes) stepping 01 Checking 'hlt' instruction... OK. ACPI: Core revision 20060707 ACPI: setting ELCR to 0200 (from 1c00) NET: Registered protocol family 16 ACPI: bus type pci registered PCI: PCI BIOS revision 2.10 entry at 0xfb3c0, last bus=1 PCI: Using configuration type 1 Setting up standard PCI resources ACPI: Interpreter enabled ACPI: Using PIC for interrupt routing ACPI: PCI Root Bridge [PCI0] (0000:00) PCI: Probing PCI hardware (bus 00) ACPI: Assume root bridge [\_SB_.PCI0] bus is 0 * Found PM-Timer Bug on the chipset. Due to workarounds for a bug, * this clock source is slow. Consider trying other clock sources PCI quirk: region 4000-403f claimed by PIIX4 ACPI PCI quirk: region 5000-500f claimed by PIIX4 SMB Boot video device is 0000:00:08.0 ACPI: PCI Interrupt Routing Table [\_SB_.PCI0._PRT] ACPI: PCI Interrupt Link [LNKA] (IRQs 3 4 5 6 7 *10 11 12 14 15) ACPI: PCI Interrupt Link [LNKB] (IRQs 3 4 5 6 7 10 11 12 14 15) *0, disabled. ACPI: PCI Interrupt Link [LNKC] (IRQs 3 4 5 6 7 10 11 *12 14 15) ACPI: PCI Interrupt Link [LNKD] (IRQs 3 4 5 6 7 10 *11 12 14 15) ACPI: Power Resource [PFAN] (on) Linux Plug and Play Support v0.97 (c) Adam Belay pnp: PnP ACPI init pnp: ACPI device : hid PNP0A03 pnp: ACPI device : hid PNP0C01 pnp: ACPI device : hid PNP0C02 pnp: ACPI device : hid PNP0200 pnp: ACPI device : hid PNP0B00 pnp: ACPI device : hid PNP0800 pnp: ACPI device : hid PNP0C04 pnp: ACPI device : hid PNP0700 pnp: ACPI device : hid PNP0501 pnp: ACPI device : hid PNP0501 pnp: ACPI device : hid PNP0400 pnp: ACPI device : hid PNP0303 pnp: PnP ACPI: found 12 devices PnPBIOS: Disabled by ACPI PNP SCSI subsystem initialized libata version 2.00 loaded. PCI: Using ACPI for IRQ routing PCI: If a device doesn't work, try "pci=routeirq". If it helps, post a report pnp: the driver 'system' has been registered pnp: match found with the PnP device '00:01' and the driver 'system' pnp: match found with the PnP device '00:02' and the driver 'system' PCI: Bridge: 0000:00:01.0 IO window: d000-dfff MEM window: disabled. PREFETCH window: disabled. NET: Registered protocol family 2 IP route cache hash table entries: 4096 (order: 2, 16384 bytes) TCP established hash table entries: 16384 (order: 4, 65536 bytes) TCP bind hash table entries: 8192 (order: 3, 32768 bytes) TCP: Hash tables configured (established 16384 bind 8192) TCP reno registered io scheduler noop registered io scheduler anticipatory registered (default) io scheduler deadline registered io scheduler cfq registered Limiting direct PCI/PCI transfers. isapnp: Scanning for PnP cards... isapnp: No Plug & Play device found Serial: 8250/16550 driver $Revision: 1.90 $ 4 ports, IRQ sharing disabled serial8250: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A serial8250: ttyS1 at I/O 0x2f8 (irq = 3) is a 16550A pnp: the driver 'serial' has been registered pnp: match found with the PnP device '00:08' and the driver 'serial' 00:08: ttyS0 at I/O 0x3f8 (irq = 4) is a 16550A pnp: match found with the PnP device '00:09' and the driver 'serial' 00:09: ttyS1 at I/O 0x2f8 (irq = 3) is a 16550A ata_piix 0000:00:07.1: version 2.00ac7 ata1: PATA max UDMA/33 cmd 0x1F0 ctl 0x3F6 bmdma 0xF000 irq 14 ata2: PATA max UDMA/33 cmd 0x170 ctl 0x376 bmdma 0xF008 irq 15 scsi0 : ata_piix ata1.00: ATA-2, max UDMA/33, 6303024 sectors: LBA ata1.00: ata1: dev 0 multi count 16 ata1.01: ATA-4, max UDMA/66, 16514064 sectors: LBA ata1.01: ata1: dev 1 multi count 16 ata1.00: configured for UDMA/33 ata1.01: configured for UDMA/33 scsi1 : ata_piix ata2.00: ATAPI, max MWDMA1 ata2.00: configured for MWDMA1 scsi 0:0:0:0: Direct-Access ATA ST33232A 3.02 PQ: 0 ANSI: 5 SCSI device sda: 6303024 512-byte hdwr sectors (3227 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: disabled, read cache: enabled, doesn't support DPO or FUA SCSI device sda: 6303024 512-byte hdwr sectors (3227 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: disabled, read cache: enabled, doesn't support DPO or FUA sda: sda1 sda2 < sda5 sda6 sda7 > sd 0:0:0:0: Attached scsi disk sda scsi 0:0:1:0: Direct-Access ATA FUJITSU MPD3084A DD-0 PQ: 0 ANSI: 5 SCSI device sdb: 16514064 512-byte hdwr sectors (8455 MB) sdb: Write Protect is off sdb: Mode Sense: 00 3a 00 00 SCSI device sdb: write cache: enabled, read cache: enabled, doesn't support DPO or FUA SCSI device sdb: 16514064 512-byte hdwr sectors (8455 MB) sdb: Write Protect is off sdb: Mode Sense: 00 3a 00 00 SCSI device sdb: write cache: enabled, read cache: enabled, doesn't support DPO or FUA sdb: sdb1 sdb2 < sdb5 sdb6 > sdb3 sd 0:0:1:0: Attached scsi disk sdb scsi 1:0:0:0: CD-ROM ATAPI CDROM 1.80 PQ: 0 ANSI: 5 pnp: the driver 'i8042 kbd' has been registered pnp: match found with the PnP device '00:0b' and the driver 'i8042 kbd' pnp: the driver 'i8042 aux' has been registered PNP: PS/2 Controller [PNP0303:PS2K] at 0x60,0x64 irq 1 PNP: PS/2 controller doesn't have AUX irq; using default 12 serio: i8042 KBD port at 0x60,0x64 irq 1 mice: PS/2 mouse device common for all mice input: AT Translated Set 2 keyboard as /class/input/input0 TCP cubic registered Using IPI Shortcut mode Time: tsc clocksource has been installed. VFS: Mounted root (ext2 filesystem) readonly. Freeing unused kernel memory: 148k freed NET: Registered protocol family 1 sr0: scsi3-mmc drive: 4x/24x cd/rw xa/form2 cdda tray Uniform CD-ROM driver Revision: 3.20 sr 1:0:0:0: Attached scsi CD-ROM sr0 Real Time Clock Driver v1.12ac pnp: the driver 'parport_pc' has been registered pnp: match found with the PnP device '00:0a' and the driver 'parport_pc' parport: PnPBIOS parport detected. parport0: PC-style at 0x378, irq 7 [PCSPP(,...)] 8139too Fast Ethernet driver 0.9.28 ACPI: PCI Interrupt Link [LNKC] enabled at IRQ 12 PCI: setting IRQ 12 as level-triggered ACPI: PCI Interrupt 0000:00:0a.0[A] -> Link [LNKC] -> GSI 12 (level, low) -> IRQ 12 eth0: RealTek RTL8139 at 0xe0840000, 00:05:5d:45:47:96, IRQ 12 eth0: Identified 8139 chip type 'RTL-8139C' Floppy drive(s): fd0 is 1.44M FDC 0 is a post-1991 82077 Adding 100760k swap on /dev/sda7. Priority:-1 extents:1 across:100760k kjournald starting. Commit interval 5 seconds EXT3-fs: mounted filesystem with ordered data mode. kjournald starting. Commit interval 5 seconds EXT3-fs: mounted filesystem with ordered data mode. kjournald starting. Commit interval 5 seconds EXT3-fs: mounted filesystem with ordered data mode. kjournald starting. Commit interval 5 seconds EXT3-fs: mounted filesystem with ordered data mode. NET: Registered protocol family 17 eth0: link up, 100Mbps, full-duplex, lpa 0x45E1 eth0: link up, 100Mbps, full-duplex, lpa 0x45E1 ttyS3: LSR safety check engaged! ttyS3: LSR safety check engaged! EXT3 FS on sda5, internal journal EXT3 FS on sda6, internal journal EXT3 FS on sdb5, internal journal EXT3 FS on sdb6, internal journal kjournald starting. Commit interval 5 seconds EXT3 FS on sdb3, internal journal EXT3-fs: mounted filesystem with ordered data mode. $ Yay! In addition to the patch above, the working kernel also has the 'ata_piix.c' patch applied. The combined diffs are listed below. Thanks for fixing the problems, and I'm glad I could help test patches. I hope these changes can make it into the 2.6.21 kernel. Art Haas diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c index 47701b2..71f5756 100644 --- a/drivers/ata/ata_piix.c +++ b/drivers/ata/ata_piix.c @@ -723,8 +723,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) if (adev->class == ATA_DEV_ATA) control |= 4; /* PPE enable */ + /* PIO configuration clears DTE unconditionally. It will be + * programmed in set_dmamode which is guaranteed to be called + * after set_piomode if any DMA mode is available. + */ pci_read_config_word(dev, master_port, &master_data); if (is_slave) { + /* clear TIME1|IE1|PPE1|DTE1 */ + master_data &= 0xff0f; /* Enable SITRE (seperate slave timing register) */ master_data |= 0x4000; /* enable PPE1, IE1 and TIME1 as needed */ @@ -732,12 +738,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) pci_read_config_byte(dev, slave_port, &slave_data); slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the timing nibble for this slave */ - slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); + slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) + << (ap->port_no ? 4 : 0); } else { - /* Master keeps the bits in a different format */ - master_data &= 0xccf8; + /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */ + master_data &= 0xccf0; /* Enable PPE, IE and TIME as appropriate */ master_data |= control; + /* load ISP and RCT */ master_data |= (timings[pio][0] << 12) | (timings[pio][1] << 8); @@ -853,7 +861,7 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */ master_data |= control << 4; pci_read_config_byte(dev, 0x44, &slave_data); - slave_data &= (0x0F + 0xE1 * ap->port_no); + slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the matching timing */ slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); pci_write_config_byte(dev, 0x44, slave_data); @@ -865,8 +873,11 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i (timings[pio][0] << 12) | (timings[pio][1] << 8); } - udma_enable &= ~(1 << devid); - pci_write_config_word(dev, master_port, master_data); + + if (ap->udma_mask) { + udma_enable &= ~(1 << devid); + pci_write_config_word(dev, master_port, master_data); + } } /* Don't scribble on 0x48 if the controller does not support UDMA */ if (ap->udma_mask) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 667acd2..4c81433 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1478,7 +1478,8 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class, } tf.protocol = ATA_PROT_PIO; - tf.flags |= ATA_TFLAG_POLLING; /* for polling presence detection */ + /* POLLING for polling detection, ISADDR|DEVICE to clear TF */ + tf.flags |= ATA_TFLAG_POLLING | ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE, id, sizeof(id[0]) * ATA_ID_WORDS); @@ -4477,6 +4478,18 @@ fsm_start: goto fsm_start; } else { + unsigned long timeout = jiffies + msecs_to_jiffies(50); + + while (time_before(jiffies, timeout)) { + if (status & ATA_DRQ) + break; + ata_dev_printk(qc->dev, KERN_INFO, + "XXX: waiting for DRQ, status=0x%x\n", + status); + msleep(10); + status = ata_chk_status(ap); + } + /* ATA PIO protocol */ if (unlikely((status & ATA_DRQ) == 0)) { /* handle BSY=0, DRQ=0 as error */ -- Man once surrendering his reason, has no remaining guard against absurdities the most monstrous, and like a ship without rudder, is the sport of every wind. -Thomas Jefferson to James Smith, 1822 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-06 16:33 ` Art Haas @ 2007-02-07 2:53 ` Tejun Heo 2007-02-07 19:35 ` Art Haas 0 siblings, 1 reply; 33+ messages in thread From: Tejun Heo @ 2007-02-07 2:53 UTC (permalink / raw) To: Art Haas; +Cc: Mark Lord, Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov [-- Attachment #1: Type: text/plain, Size: 1437 bytes --] Art Haas wrote: >>> Also, zero out the features register before issuing PACKET_IDENTIFY, >>> if the code isn't already doing that. >> Okay. >> >>> After the drive asserts BUSY, and later deasserts BUSY, >>> there might be a slight delay before the drive asserts DRQ. >>> So, it is possible for the status to read zeros in the important bits. >>> >>> My suggestion is to wait up to the infamous 50 milliseconds again here, >>> if needed. >> Okay, the attached patch does what Mark suggested. Art, can you please >> give it a shot and report dmesg? My thanks for sticking around till now. > > SUCCESS!!!!! Yay! [--snip--] > ata_piix 0000:00:07.1: version 2.00ac7 > ata1: PATA max UDMA/33 cmd 0x1F0 ctl 0x3F6 bmdma 0xF000 irq 14 > ata2: PATA max UDMA/33 cmd 0x170 ctl 0x376 bmdma 0xF008 irq 15 > scsi0 : ata_piix > ata1.00: ATA-2, max UDMA/33, 6303024 sectors: LBA > ata1.00: ata1: dev 0 multi count 16 > ata1.01: ATA-4, max UDMA/66, 16514064 sectors: LBA > ata1.01: ata1: dev 1 multi count 16 > ata1.00: configured for UDMA/33 > ata1.01: configured for UDMA/33 > scsi1 : ata_piix > ata2.00: ATAPI, max MWDMA1 > ata2.00: configured for MWDMA1 So, it succeeded without any DRQ wait. Can you please apply only the attached patch over vanilla 2.6.20 and see if your problem is fixed? This problem has been around for quite a while now and there probably have been other users hit by this out there. Thanks a lot, Mark. -- tejun [-- Attachment #2: clear-TF-for-IDENTIFY.patch --] [-- Type: text/x-patch, Size: 780 bytes --] diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 667acd2..d6fcf0a 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1478,7 +1478,16 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class, } tf.protocol = ATA_PROT_PIO; - tf.flags |= ATA_TFLAG_POLLING; /* for polling presence detection */ + + /* Some devices choke if TF registers contain garbage. Make + * sure those are properly initialized. + */ + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + + /* Device presence detection is unreliable on some + * controllers. Always poll IDENTIFY if available. + */ + tf.flags |= ATA_TFLAG_POLLING; err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE, id, sizeof(id[0]) * ATA_ID_WORDS); ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-07 2:53 ` Tejun Heo @ 2007-02-07 19:35 ` Art Haas 2007-02-07 19:51 ` Mark Lord 0 siblings, 1 reply; 33+ messages in thread From: Art Haas @ 2007-02-07 19:35 UTC (permalink / raw) To: Tejun Heo; +Cc: Mark Lord, Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov On Wed, Feb 07, 2007 at 11:53:17AM +0900, Tejun Heo wrote: > Art Haas wrote: > >>>Also, zero out the features register before issuing PACKET_IDENTIFY, > >>>if the code isn't already doing that. > >>Okay. > >> > >>>After the drive asserts BUSY, and later deasserts BUSY, > >>>there might be a slight delay before the drive asserts DRQ. > >>>So, it is possible for the status to read zeros in the important bits. > >>> > >>>My suggestion is to wait up to the infamous 50 milliseconds again here, > >>>if needed. > >>Okay, the attached patch does what Mark suggested. Art, can you please > >>give it a shot and report dmesg? My thanks for sticking around till now. > > > >SUCCESS!!!!! > > Yay! > > [--snip--] > >ata_piix 0000:00:07.1: version 2.00ac7 > >ata1: PATA max UDMA/33 cmd 0x1F0 ctl 0x3F6 bmdma 0xF000 irq 14 > >ata2: PATA max UDMA/33 cmd 0x170 ctl 0x376 bmdma 0xF008 irq 15 > >scsi0 : ata_piix > >ata1.00: ATA-2, max UDMA/33, 6303024 sectors: LBA > >ata1.00: ata1: dev 0 multi count 16 > >ata1.01: ATA-4, max UDMA/66, 16514064 sectors: LBA > >ata1.01: ata1: dev 1 multi count 16 > >ata1.00: configured for UDMA/33 > >ata1.01: configured for UDMA/33 > >scsi1 : ata_piix > >ata2.00: ATAPI, max MWDMA1 > >ata2.00: configured for MWDMA1 > > So, it succeeded without any DRQ wait. Can you please apply only the > attached patch over vanilla 2.6.20 and see if your problem is fixed? > > This problem has been around for quite a while now and there probably > have been other users hit by this out there. Thanks a lot, Mark. SUCCESS (again)!!!!! $ dmesg [ ... snip ... ] ata_piix 0000:00:07.1: version 2.00ac7 ata1: PATA max UDMA/33 cmd 0x1F0 ctl 0x3F6 bmdma 0xF000 irq 14 ata2: PATA max UDMA/33 cmd 0x170 ctl 0x376 bmdma 0xF008 irq 15 scsi0 : ata_piix ata1.00: ATA-2, max UDMA/33, 6303024 sectors: LBA ata1.00: ata1: dev 0 multi count 16 ata1.01: ATA-4, max UDMA/66, 16514064 sectors: LBA ata1.01: ata1: dev 1 multi count 16 ata1.00: configured for UDMA/33 ata1.01: configured for UDMA/33 scsi1 : ata_piix ata2.00: ATAPI, max MWDMA1 ata2.00: configured for MWDMA1 scsi 0:0:0:0: Direct-Access ATA ST33232A 3.02 PQ: 0 ANSI: 5 SCSI device sda: 6303024 512-byte hdwr sectors (3227 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: disabled, read cache: enabled, doesn't support DPO or FUA SCSI device sda: 6303024 512-byte hdwr sectors (3227 MB) sda: Write Protect is off sda: Mode Sense: 00 3a 00 00 SCSI device sda: write cache: disabled, read cache: enabled, doesn't support DPO or FUA sda: sda1 sda2 < sda5 sda6 sda7 > sd 0:0:0:0: Attached scsi disk sda scsi 0:0:1:0: Direct-Access ATA FUJITSU MPD3084A DD-0 PQ: 0 ANSI: 5 SCSI device sdb: 16514064 512-byte hdwr sectors (8455 MB) sdb: Write Protect is off sdb: Mode Sense: 00 3a 00 00 SCSI device sdb: write cache: enabled, read cache: enabled, doesn't support DPO or FUA SCSI device sdb: 16514064 512-byte hdwr sectors (8455 MB) sdb: Write Protect is off sdb: Mode Sense: 00 3a 00 00 SCSI device sdb: write cache: enabled, read cache: enabled, doesn't support DPO or FUA sdb: sdb1 sdb2 < sdb5 sdb6 > sdb3 sd 0:0:1:0: Attached scsi disk sdb scsi 1:0:0:0: CD-ROM ATAPI CDROM 1.80 PQ: 0 ANSI: 5 [ ... snip ... ] $ The minimal patch is a keeper. Nice to have the CD-ROM available using the libata code on this machine. My other machine with the PIIX setup has not had these problems. Guess this box has quirky/old hardware. Thanks again for the patch(es); I'm glad to help test them out. Art Haas -- Man once surrendering his reason, has no remaining guard against absurdities the most monstrous, and like a ship without rudder, is the sport of every wind. -Thomas Jefferson to James Smith, 1822 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) 2007-02-07 19:35 ` Art Haas @ 2007-02-07 19:51 ` Mark Lord 2007-02-07 20:37 ` [PATCH] libata: clear TF before IDENTIFYing Tejun Heo 0 siblings, 1 reply; 33+ messages in thread From: Mark Lord @ 2007-02-07 19:51 UTC (permalink / raw) To: Art Haas; +Cc: Tejun Heo, Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov Art Haas wrote: > On Wed, Feb 07, 2007 at 11:53:17AM +0900, Tejun Heo wrote: >> Art Haas wrote: >>>>> Also, zero out the features register before issuing PACKET_IDENTIFY, >>>>> if the code isn't already doing that. >>>> Okay. >>>SUCCESS!!!!! >> Yay! ... >> So, it succeeded without any DRQ wait. Can you please apply only the >> attached patch over vanilla 2.6.20 and see if your problem is fixed? >> >> This problem has been around for quite a while now and there probably >> have been other users hit by this out there. Thanks a lot, Mark. > > SUCCESS (again)!!!!! Good stuff, guys. I'm happy to have been of service. Cheers -- Mark Lord Real-Time Remedies Inc. mlord@pobox.com ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH] libata: clear TF before IDENTIFYing 2007-02-07 19:51 ` Mark Lord @ 2007-02-07 20:37 ` Tejun Heo 2007-02-08 14:56 ` Mark Lord ` (2 more replies) 0 siblings, 3 replies; 33+ messages in thread From: Tejun Heo @ 2007-02-07 20:37 UTC (permalink / raw) To: Mark Lord, Jeff Garzik Cc: Art Haas, Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov, stable Some devices chock if Feature is not clear when IDENTIFY is issued. Set ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE for IDENTIFY such that whole TF is cleared when reading ID data. Kudos to Art Haas for testing various futile patches over several months and Mark Lord for pointing out the fix. Signed-off-by: Tejun Heo <htejun@gmail.com> Cc: Art Haas <ahaas@airmail.net> Cc: Mark Lord <mlord@pobox.com> --- I think this should go into -stable but a little bit hesitant because the code hasn't been tested widely. This patch should really be harmless but who knows. Jeff, Mark, what do you think? diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 667acd2..d6fcf0a 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1478,7 +1478,16 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class, } tf.protocol = ATA_PROT_PIO; - tf.flags |= ATA_TFLAG_POLLING; /* for polling presence detection */ + + /* Some devices choke if TF registers contain garbage. Make + * sure those are properly initialized. + */ + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; + + /* Device presence detection is unreliable on some + * controllers. Always poll IDENTIFY if available. + */ + tf.flags |= ATA_TFLAG_POLLING; err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE, id, sizeof(id[0]) * ATA_ID_WORDS); ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] libata: clear TF before IDENTIFYing 2007-02-07 20:37 ` [PATCH] libata: clear TF before IDENTIFYing Tejun Heo @ 2007-02-08 14:56 ` Mark Lord 2007-02-13 19:38 ` Art Haas 2007-02-15 23:08 ` Jeff Garzik 2 siblings, 0 replies; 33+ messages in thread From: Mark Lord @ 2007-02-08 14:56 UTC (permalink / raw) To: Tejun Heo Cc: Jeff Garzik, Art Haas, Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov, stable Tejun Heo wrote: > Some devices chock if Feature is not clear when IDENTIFY is issued. > Set ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE for IDENTIFY such that whole > TF is cleared when reading ID data. > > Kudos to Art Haas for testing various futile patches over several > months and Mark Lord for pointing out the fix. > > Signed-off-by: Tejun Heo <htejun@gmail.com> > Cc: Art Haas <ahaas@airmail.net> > Cc: Mark Lord <mlord@pobox.com> Signed-off-by: Mark Lord <mlord@pobox.com> > I think this should go into -stable but a little bit hesitant because > the code hasn't been tested widely. This patch should really be > harmless but who knows. Jeff, Mark, what do you think? If it was up to me, I'd plunk it directly into 2.6.20.1 and earlier. But Jeff has his own ways.. > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index 667acd2..d6fcf0a 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -1478,7 +1478,16 @@ int ata_dev_read_id(struct ata_device *dev, > unsigned int *p_class, > } > > tf.protocol = ATA_PROT_PIO; > - tf.flags |= ATA_TFLAG_POLLING; /* for polling presence detection */ > + > + /* Some devices choke if TF registers contain garbage. Make > + * sure those are properly initialized. > + */ > + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; > + > + /* Device presence detection is unreliable on some > + * controllers. Always poll IDENTIFY if available. > + */ > + tf.flags |= ATA_TFLAG_POLLING; > > err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE, > id, sizeof(id[0]) * ATA_ID_WORDS); > - > To unsubscribe from this list: send the line "unsubscribe linux-ide" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] libata: clear TF before IDENTIFYing 2007-02-07 20:37 ` [PATCH] libata: clear TF before IDENTIFYing Tejun Heo 2007-02-08 14:56 ` Mark Lord @ 2007-02-13 19:38 ` Art Haas 2007-02-15 23:08 ` Jeff Garzik 2 siblings, 0 replies; 33+ messages in thread From: Art Haas @ 2007-02-13 19:38 UTC (permalink / raw) To: Tejun Heo Cc: Mark Lord, Jeff Garzik, Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov On Wed, Feb 07, 2007 at 12:37:41PM -0800, Tejun Heo wrote: > Some devices chock if Feature is not clear when IDENTIFY is issued. > Set ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE for IDENTIFY such that whole > TF is cleared when reading ID data. > > Kudos to Art Haas for testing various futile patches over several > months and Mark Lord for pointing out the fix. > > Signed-off-by: Tejun Heo <htejun@gmail.com> > Cc: Art Haas <ahaas@airmail.net> > Cc: Mark Lord <mlord@pobox.com> > --- > I think this should go into -stable but a little bit hesitant because > the code hasn't been tested widely. This patch should really be > harmless but who knows. Jeff, Mark, what do you think? > > [ ... snip patch ... ] Hi. I applied this patch for a kernel build on another PIIX based machine around here, plus the 'ata_piix.c' patch that was tested earlier, and the patched kernel booted without problems. I'm guessing that between the time the patch was initially posted and now other people have tried it out, but I can confirm the patches did not cause problems on the machine (SMP PIII box). A little more testing info for those wanting some results. The kernel, btw, was Linus' git tree from this morning. Art Haas -- Man once surrendering his reason, has no remaining guard against absurdities the most monstrous, and like a ship without rudder, is the sport of every wind. -Thomas Jefferson to James Smith, 1822 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] libata: clear TF before IDENTIFYing 2007-02-07 20:37 ` [PATCH] libata: clear TF before IDENTIFYing Tejun Heo 2007-02-08 14:56 ` Mark Lord 2007-02-13 19:38 ` Art Haas @ 2007-02-15 23:08 ` Jeff Garzik 2 siblings, 0 replies; 33+ messages in thread From: Jeff Garzik @ 2007-02-15 23:08 UTC (permalink / raw) To: Tejun Heo Cc: Mark Lord, Art Haas, Alan Cox, linux-ide, Albert Lee, Sergei Shtylyov, stable Tejun Heo wrote: > Some devices chock if Feature is not clear when IDENTIFY is issued. > Set ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE for IDENTIFY such that whole > TF is cleared when reading ID data. > > Kudos to Art Haas for testing various futile patches over several > months and Mark Lord for pointing out the fix. > > Signed-off-by: Tejun Heo <htejun@gmail.com> > Cc: Art Haas <ahaas@airmail.net> > Cc: Mark Lord <mlord@pobox.com> > --- > I think this should go into -stable but a little bit hesitant because > the code hasn't been tested widely. This patch should really be > harmless but who knows. Jeff, Mark, what do you think? applied ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH] Fix pio/mwdma programming on ata_piix.c 2007-02-02 15:18 [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) Tejun Heo ` (2 preceding siblings ...) 2007-02-02 21:14 ` Art Haas @ 2007-04-30 18:29 ` Art Haas 2007-05-01 3:02 ` Tejun Heo 3 siblings, 1 reply; 33+ messages in thread From: Art Haas @ 2007-04-30 18:29 UTC (permalink / raw) To: Tejun Heo; +Cc: Alan Cox, linux-ide Hi. Back in the beginning of February I was sent a patch to test during the time the CD-ROM identification bug was affecting my computer. The first posting of the patch appeared on Feb. 3 with Tejun Heo writing: > Hello, Art Haas, Alan. > > Okay, here's another try at fixing the detection bug. I went through > intel ich docs and compared with the ide piix driver. This patch > fixes the following problems. > > * Control bits in the timing register wasn't cleared properly while > programming PIO mode. > > * MWDMA mode programming cleared the wrong part of control bits. I > think this can fix your problem. > > * MWDMA mode programming cleared udma_mask even when the controller > doesn't support UDMA. This doesn't matter for your case. > > Please test and report the result. Thanks. The patch and complete thread can be reviewed here: http://marc.info/?l=linux-ide&m=117042956705812&w=2 Now that 2.6.22 is open for big modifications, and the queued libata changes have been pulled by Linus, I'm wondering if the ata_piix.c patch posted above should be sent. I've built all my kernels since the posting with the patch and had no problems. As you write above, the patch does fix some bugs, and as the thread progressed a cleanup or two in the code was explained as a bug fix - the setting of the 'slave_data' variable between lines 826 and 834 in particular. Here's the patch against today's Linus tree if you feel it should make it into 2.6.22. Art Haas diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c index 55d306a..26c8b5f 100644 --- a/drivers/ata/ata_piix.c +++ b/drivers/ata/ata_piix.c @@ -695,8 +695,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) if (adev->class == ATA_DEV_ATA) control |= 4; /* PPE enable */ + /* PIO configuration clears DTE unconditionally. It will be + * programmed in set_dmamode which is guaranteed to be called + * after set_piomode if any DMA mode is available. + */ pci_read_config_word(dev, master_port, &master_data); if (is_slave) { + /* clear TIME1|IE1|PPE1|DTE1 */ + master_data &= 0xff0f; /* Enable SITRE (seperate slave timing register) */ master_data |= 0x4000; /* enable PPE1, IE1 and TIME1 as needed */ @@ -704,12 +710,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) pci_read_config_byte(dev, slave_port, &slave_data); slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the timing nibble for this slave */ - slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); + slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) + << (ap->port_no ? 4 : 0); } else { - /* Master keeps the bits in a different format */ - master_data &= 0xccf8; + /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */ + master_data &= 0xccf0; /* Enable PPE, IE and TIME as appropriate */ master_data |= control; + /* load ISP and RCT */ master_data |= (timings[pio][0] << 12) | (timings[pio][1] << 8); @@ -826,7 +834,7 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */ master_data |= control << 4; pci_read_config_byte(dev, 0x44, &slave_data); - slave_data &= (0x0F + 0xE1 * ap->port_no); + slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the matching timing */ slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); pci_write_config_byte(dev, 0x44, slave_data); @@ -838,8 +846,11 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i (timings[pio][0] << 12) | (timings[pio][1] << 8); } - udma_enable &= ~(1 << devid); - pci_write_config_word(dev, master_port, master_data); + + if (ap->udma_mask) { + udma_enable &= ~(1 << devid); + pci_write_config_word(dev, master_port, master_data); + } } /* Don't scribble on 0x48 if the controller does not support UDMA */ if (ap->udma_mask) -- Man once surrendering his reason, has no remaining guard against absurdities the most monstrous, and like a ship without rudder, is the sport of every wind. -Thomas Jefferson to James Smith, 1822 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] Fix pio/mwdma programming on ata_piix.c 2007-04-30 18:29 ` [PATCH] Fix pio/mwdma programming on ata_piix.c Art Haas @ 2007-05-01 3:02 ` Tejun Heo 2007-05-24 19:59 ` Art Haas 0 siblings, 1 reply; 33+ messages in thread From: Tejun Heo @ 2007-05-01 3:02 UTC (permalink / raw) To: Art Haas; +Cc: Alan Cox, linux-ide, Jeff Garzik Hello, Art Haas. Art Haas wrote: > Hi. > > Back in the beginning of February I was sent a patch to test during the > time the CD-ROM identification bug was affecting my computer. The > first posting of the patch appeared on Feb. 3 with Tejun Heo writing: Ah.. I completely forgot about this one. This wasn't directly related to the detection bug, so I got oblivious as usual. :-) I think Alan has reviewed it back then. If there's no objection, I'll reformat the patch and submit it properly. Quoting whole message for Jeff. >> Hello, Art Haas, Alan. >> >> Okay, here's another try at fixing the detection bug. I went through >> intel ich docs and compared with the ide piix driver. This patch >> fixes the following problems. >> >> * Control bits in the timing register wasn't cleared properly while >> programming PIO mode. >> >> * MWDMA mode programming cleared the wrong part of control bits. I >> think this can fix your problem. >> >> * MWDMA mode programming cleared udma_mask even when the controller >> doesn't support UDMA. This doesn't matter for your case. >> >> Please test and report the result. Thanks. > > The patch and complete thread can be reviewed here: > > http://marc.info/?l=linux-ide&m=117042956705812&w=2 > > Now that 2.6.22 is open for big modifications, and the queued libata > changes have been pulled by Linus, I'm wondering if the ata_piix.c patch > posted above should be sent. I've built all my kernels since the posting > with the patch and had no problems. As you write above, the patch does > fix some bugs, and as the thread progressed a cleanup or two in the code > was explained as a bug fix - the setting of the 'slave_data' variable > between lines 826 and 834 in particular. > > Here's the patch against today's Linus tree if you feel it should make > it into 2.6.22. > > Art Haas > > diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c > index 55d306a..26c8b5f 100644 > --- a/drivers/ata/ata_piix.c > +++ b/drivers/ata/ata_piix.c > @@ -695,8 +695,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) > if (adev->class == ATA_DEV_ATA) > control |= 4; /* PPE enable */ > > + /* PIO configuration clears DTE unconditionally. It will be > + * programmed in set_dmamode which is guaranteed to be called > + * after set_piomode if any DMA mode is available. > + */ > pci_read_config_word(dev, master_port, &master_data); > if (is_slave) { > + /* clear TIME1|IE1|PPE1|DTE1 */ > + master_data &= 0xff0f; > /* Enable SITRE (seperate slave timing register) */ > master_data |= 0x4000; > /* enable PPE1, IE1 and TIME1 as needed */ > @@ -704,12 +710,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) > pci_read_config_byte(dev, slave_port, &slave_data); > slave_data &= (ap->port_no ? 0x0f : 0xf0); > /* Load the timing nibble for this slave */ > - slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); > + slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) > + << (ap->port_no ? 4 : 0); > } else { > - /* Master keeps the bits in a different format */ > - master_data &= 0xccf8; > + /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */ > + master_data &= 0xccf0; > /* Enable PPE, IE and TIME as appropriate */ > master_data |= control; > + /* load ISP and RCT */ > master_data |= > (timings[pio][0] << 12) | > (timings[pio][1] << 8); > @@ -826,7 +834,7 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i > master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */ > master_data |= control << 4; > pci_read_config_byte(dev, 0x44, &slave_data); > - slave_data &= (0x0F + 0xE1 * ap->port_no); > + slave_data &= (ap->port_no ? 0x0f : 0xf0); > /* Load the matching timing */ > slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); > pci_write_config_byte(dev, 0x44, slave_data); > @@ -838,8 +846,11 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i > (timings[pio][0] << 12) | > (timings[pio][1] << 8); > } > - udma_enable &= ~(1 << devid); > - pci_write_config_word(dev, master_port, master_data); > + > + if (ap->udma_mask) { > + udma_enable &= ~(1 << devid); > + pci_write_config_word(dev, master_port, master_data); > + } > } > /* Don't scribble on 0x48 if the controller does not support UDMA */ > if (ap->udma_mask) > -- tejun ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] Fix pio/mwdma programming on ata_piix.c 2007-05-01 3:02 ` Tejun Heo @ 2007-05-24 19:59 ` Art Haas 2007-05-24 20:55 ` Jeff Garzik 0 siblings, 1 reply; 33+ messages in thread From: Art Haas @ 2007-05-24 19:59 UTC (permalink / raw) To: Tejun Heo; +Cc: Alan Cox, linux-ide, Jeff Garzik On Tue, May 01, 2007 at 05:02:22AM +0200, Tejun Heo wrote: > Hello, Art Haas. > > Art Haas wrote: > > Hi. > > > > Back in the beginning of February I was sent a patch to test during the > > time the CD-ROM identification bug was affecting my computer. The > > first posting of the patch appeared on Feb. 3 with Tejun Heo writing: > > Ah.. I completely forgot about this one. This wasn't directly related > to the detection bug, so I got oblivious as usual. :-) > > I think Alan has reviewed it back then. If there's no objection, I'll > reformat the patch and submit it properly. Quoting whole message for Jeff. > > >> Hello, Art Haas, Alan. > >> > >> Okay, here's another try at fixing the detection bug. I went through > >> intel ich docs and compared with the ide piix driver. This patch > >> fixes the following problems. > >> > >> * Control bits in the timing register wasn't cleared properly while > >> programming PIO mode. > >> > >> * MWDMA mode programming cleared the wrong part of control bits. I > >> think this can fix your problem. > >> > >> * MWDMA mode programming cleared udma_mask even when the controller > >> doesn't support UDMA. This doesn't matter for your case. > >> > >> Please test and report the result. Thanks. > > > > The patch and complete thread can be reviewed here: > > > > http://marc.info/?l=linux-ide&m=117042956705812&w=2 > > > > Now that 2.6.22 is open for big modifications, and the queued libata > > changes have been pulled by Linus, I'm wondering if the ata_piix.c patch > > posted above should be sent. I've built all my kernels since the posting > > with the patch and had no problems. As you write above, the patch does > > fix some bugs, and as the thread progressed a cleanup or two in the code > > was explained as a bug fix - the setting of the 'slave_data' variable > > between lines 826 and 834 in particular. > > > > Here's the patch against today's Linus tree if you feel it should make > > it into 2.6.22. > > [ ... snip ... ] Ping? Here's the patch diffed to this afternoon's tree. Art Haas diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c index 0458811..51de738 100644 --- a/drivers/ata/ata_piix.c +++ b/drivers/ata/ata_piix.c @@ -684,8 +684,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) if (adev->class == ATA_DEV_ATA) control |= 4; /* PPE enable */ + /* PIO configuration clears DTE unconditionally. It will be + * programmed in set_dmamode which is guaranteed to be called + * after set_piomode if any DMA mode is available. + */ pci_read_config_word(dev, master_port, &master_data); if (is_slave) { + /* clear TIME1|IE1|PPE1|DTE1 */ + master_data &= 0xff0f; /* Enable SITRE (seperate slave timing register) */ master_data |= 0x4000; /* enable PPE1, IE1 and TIME1 as needed */ @@ -693,12 +699,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) pci_read_config_byte(dev, slave_port, &slave_data); slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the timing nibble for this slave */ - slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); + slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) + << (ap->port_no ? 4 : 0); } else { - /* Master keeps the bits in a different format */ - master_data &= 0xccf8; + /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */ + master_data &= 0xccf0; /* Enable PPE, IE and TIME as appropriate */ master_data |= control; + /* load ISP and RCT */ master_data |= (timings[pio][0] << 12) | (timings[pio][1] << 8); @@ -815,7 +823,7 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */ master_data |= control << 4; pci_read_config_byte(dev, 0x44, &slave_data); - slave_data &= (0x0F + 0xE1 * ap->port_no); + slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the matching timing */ slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); pci_write_config_byte(dev, 0x44, slave_data); @@ -827,8 +835,11 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i (timings[pio][0] << 12) | (timings[pio][1] << 8); } - udma_enable &= ~(1 << devid); - pci_write_config_word(dev, master_port, master_data); + + if (ap->udma_mask) { + udma_enable &= ~(1 << devid); + pci_write_config_word(dev, master_port, master_data); + } } /* Don't scribble on 0x48 if the controller does not support UDMA */ if (ap->udma_mask) -- Man once surrendering his reason, has no remaining guard against absurdities the most monstrous, and like a ship without rudder, is the sport of every wind. -Thomas Jefferson to James Smith, 1822 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] Fix pio/mwdma programming on ata_piix.c 2007-05-24 19:59 ` Art Haas @ 2007-05-24 20:55 ` Jeff Garzik 2007-05-24 21:03 ` Tejun Heo 0 siblings, 1 reply; 33+ messages in thread From: Jeff Garzik @ 2007-05-24 20:55 UTC (permalink / raw) To: Art Haas; +Cc: Tejun Heo, Alan Cox, linux-ide Art Haas wrote: > On Tue, May 01, 2007 at 05:02:22AM +0200, Tejun Heo wrote: >> Hello, Art Haas. >> >> Art Haas wrote: >>> Hi. >>> >>> Back in the beginning of February I was sent a patch to test during the >>> time the CD-ROM identification bug was affecting my computer. The >>> first posting of the patch appeared on Feb. 3 with Tejun Heo writing: >> Ah.. I completely forgot about this one. This wasn't directly related >> to the detection bug, so I got oblivious as usual. :-) >> >> I think Alan has reviewed it back then. If there's no objection, I'll >> reformat the patch and submit it properly. Quoting whole message for Jeff. >> >>>> Hello, Art Haas, Alan. >>>> >>>> Okay, here's another try at fixing the detection bug. I went through >>>> intel ich docs and compared with the ide piix driver. This patch >>>> fixes the following problems. >>>> >>>> * Control bits in the timing register wasn't cleared properly while >>>> programming PIO mode. >>>> >>>> * MWDMA mode programming cleared the wrong part of control bits. I >>>> think this can fix your problem. >>>> >>>> * MWDMA mode programming cleared udma_mask even when the controller >>>> doesn't support UDMA. This doesn't matter for your case. >>>> >>>> Please test and report the result. Thanks. >>> The patch and complete thread can be reviewed here: >>> >>> http://marc.info/?l=linux-ide&m=117042956705812&w=2 >>> >>> Now that 2.6.22 is open for big modifications, and the queued libata >>> changes have been pulled by Linus, I'm wondering if the ata_piix.c patch >>> posted above should be sent. I've built all my kernels since the posting >>> with the patch and had no problems. As you write above, the patch does >>> fix some bugs, and as the thread progressed a cleanup or two in the code >>> was explained as a bug fix - the setting of the 'slave_data' variable >>> between lines 826 and 834 in particular. >>> >>> Here's the patch against today's Linus tree if you feel it should make >>> it into 2.6.22. >>> [ ... snip ... ] > > Ping? > > Here's the patch diffed to this afternoon's tree. > > Art Haas > > diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c > index 0458811..51de738 100644 > --- a/drivers/ata/ata_piix.c > +++ b/drivers/ata/ata_piix.c > @@ -684,8 +684,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) > if (adev->class == ATA_DEV_ATA) > control |= 4; /* PPE enable */ > > + /* PIO configuration clears DTE unconditionally. It will be > + * programmed in set_dmamode which is guaranteed to be called > + * after set_piomode if any DMA mode is available. > + */ > pci_read_config_word(dev, master_port, &master_data); > if (is_slave) { > + /* clear TIME1|IE1|PPE1|DTE1 */ > + master_data &= 0xff0f; > /* Enable SITRE (seperate slave timing register) */ > master_data |= 0x4000; > /* enable PPE1, IE1 and TIME1 as needed */ > @@ -693,12 +699,14 @@ static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev) > pci_read_config_byte(dev, slave_port, &slave_data); > slave_data &= (ap->port_no ? 0x0f : 0xf0); > /* Load the timing nibble for this slave */ > - slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); > + slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) > + << (ap->port_no ? 4 : 0); > } else { > - /* Master keeps the bits in a different format */ > - master_data &= 0xccf8; > + /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */ > + master_data &= 0xccf0; > /* Enable PPE, IE and TIME as appropriate */ > master_data |= control; > + /* load ISP and RCT */ > master_data |= > (timings[pio][0] << 12) | > (timings[pio][1] << 8); > @@ -815,7 +823,7 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i > master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */ > master_data |= control << 4; > pci_read_config_byte(dev, 0x44, &slave_data); > - slave_data &= (0x0F + 0xE1 * ap->port_no); > + slave_data &= (ap->port_no ? 0x0f : 0xf0); > /* Load the matching timing */ > slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); > pci_write_config_byte(dev, 0x44, slave_data); > @@ -827,8 +835,11 @@ static void do_pata_set_dmamode (struct ata_port *ap, struct ata_device *adev, i > (timings[pio][0] << 12) | > (timings[pio][1] << 8); > } > - udma_enable &= ~(1 << devid); > - pci_write_config_word(dev, master_port, master_data); > + > + if (ap->udma_mask) { > + udma_enable &= ~(1 << devid); > + pci_write_config_word(dev, master_port, master_data); > + } > } > /* Don't scribble on 0x48 if the controller does not support UDMA */ > if (ap->udma_mask) > Please resend with proper sign-offs. All patches in the kernel need that... http://linux.yyz.us/patch-format.html Thanks, Jeff ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] Fix pio/mwdma programming on ata_piix.c 2007-05-24 20:55 ` Jeff Garzik @ 2007-05-24 21:03 ` Tejun Heo 2007-05-25 17:16 ` [PATCH] ata_piix: fix pio/mwdma programming Tejun Heo 0 siblings, 1 reply; 33+ messages in thread From: Tejun Heo @ 2007-05-24 21:03 UTC (permalink / raw) To: Jeff Garzik; +Cc: Art Haas, Alan Cox, linux-ide Jeff Garzik wrote: > Please resend with proper sign-offs. All patches in the kernel need > that... http://linux.yyz.us/patch-format.html I was supposed to do that and Art reminded me once but I managed to forget again, duh..... I'll do it first thing tomorrow. Sorry, Art. -- tejun ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH] ata_piix: fix pio/mwdma programming 2007-05-24 21:03 ` Tejun Heo @ 2007-05-25 17:16 ` Tejun Heo 2007-05-25 18:05 ` Alan Cox 2007-05-28 13:02 ` Jeff Garzik 0 siblings, 2 replies; 33+ messages in thread From: Tejun Heo @ 2007-05-25 17:16 UTC (permalink / raw) To: Jeff Garzik; +Cc: Art Haas, Alan Cox, linux-ide Fix various bugs in pio/mwdma mode programming. * Control bits in the timing register wasn't cleared properly while programming PIO mode. * MWDMA mode programming cleared the wrong part of control bits. * MWDMA mode programming cleared udma_mask even when the controller doesn't support UDMA. Signed-off-by: Tejun Heo <htejun@gmail.com> Cc: Art Haas <ahaas@airmail.net> --- Patch is against libata-dev#upstream. Thanks. drivers/ata/ata_piix.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c index 9c07b88..924e447 100644 --- a/drivers/ata/ata_piix.c +++ b/drivers/ata/ata_piix.c @@ -685,8 +685,14 @@ static void piix_set_piomode (struct ata if (adev->class == ATA_DEV_ATA) control |= 4; /* PPE enable */ + /* PIO configuration clears DTE unconditionally. It will be + * programmed in set_dmamode which is guaranteed to be called + * after set_piomode if any DMA mode is available. + */ pci_read_config_word(dev, master_port, &master_data); if (is_slave) { + /* clear TIME1|IE1|PPE1|DTE1 */ + master_data &= 0xff0f; /* Enable SITRE (seperate slave timing register) */ master_data |= 0x4000; /* enable PPE1, IE1 and TIME1 as needed */ @@ -694,12 +700,14 @@ static void piix_set_piomode (struct ata pci_read_config_byte(dev, slave_port, &slave_data); slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the timing nibble for this slave */ - slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); + slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) + << (ap->port_no ? 4 : 0); } else { - /* Master keeps the bits in a different format */ - master_data &= 0xccf8; + /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */ + master_data &= 0xccf0; /* Enable PPE, IE and TIME as appropriate */ master_data |= control; + /* load ISP and RCT */ master_data |= (timings[pio][0] << 12) | (timings[pio][1] << 8); @@ -816,7 +824,7 @@ static void do_pata_set_dmamode (struct master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */ master_data |= control << 4; pci_read_config_byte(dev, 0x44, &slave_data); - slave_data &= (0x0F + 0xE1 * ap->port_no); + slave_data &= (ap->port_no ? 0x0f : 0xf0); /* Load the matching timing */ slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); pci_write_config_byte(dev, 0x44, slave_data); @@ -828,8 +836,11 @@ static void do_pata_set_dmamode (struct (timings[pio][0] << 12) | (timings[pio][1] << 8); } - udma_enable &= ~(1 << devid); - pci_write_config_word(dev, master_port, master_data); + + if (ap->udma_mask) { + udma_enable &= ~(1 << devid); + pci_write_config_word(dev, master_port, master_data); + } } /* Don't scribble on 0x48 if the controller does not support UDMA */ if (ap->udma_mask) ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming 2007-05-25 17:16 ` [PATCH] ata_piix: fix pio/mwdma programming Tejun Heo @ 2007-05-25 18:05 ` Alan Cox 2007-05-28 13:02 ` Jeff Garzik 1 sibling, 0 replies; 33+ messages in thread From: Alan Cox @ 2007-05-25 18:05 UTC (permalink / raw) To: Tejun Heo; +Cc: Jeff Garzik, Art Haas, linux-ide On Fri, 25 May 2007 19:16:58 +0200 Tejun Heo <htejun@gmail.com> wrote: > Fix various bugs in pio/mwdma mode programming. > > * Control bits in the timing register wasn't cleared properly while > programming PIO mode. > > * MWDMA mode programming cleared the wrong part of control bits. > > * MWDMA mode programming cleared udma_mask even when the controller > doesn't support UDMA. > > Signed-off-by: Tejun Heo <htejun@gmail.com> > Cc: Art Haas <ahaas@airmail.net> Acked-by: Alan Cox <alan@redhat.com> > --- > Patch is against libata-dev#upstream. Thanks. > > drivers/ata/ata_piix.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c > index 9c07b88..924e447 100644 > --- a/drivers/ata/ata_piix.c > +++ b/drivers/ata/ata_piix.c > @@ -685,8 +685,14 @@ static void piix_set_piomode (struct ata > if (adev->class == ATA_DEV_ATA) > control |= 4; /* PPE enable */ > > + /* PIO configuration clears DTE unconditionally. It will be > + * programmed in set_dmamode which is guaranteed to be called > + * after set_piomode if any DMA mode is available. > + */ > pci_read_config_word(dev, master_port, &master_data); > if (is_slave) { > + /* clear TIME1|IE1|PPE1|DTE1 */ > + master_data &= 0xff0f; > /* Enable SITRE (seperate slave timing register) */ > master_data |= 0x4000; > /* enable PPE1, IE1 and TIME1 as needed */ > @@ -694,12 +700,14 @@ static void piix_set_piomode (struct ata > pci_read_config_byte(dev, slave_port, &slave_data); > slave_data &= (ap->port_no ? 0x0f : 0xf0); > /* Load the timing nibble for this slave */ > - slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); > + slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) > + << (ap->port_no ? 4 : 0); > } else { > - /* Master keeps the bits in a different format */ > - master_data &= 0xccf8; > + /* clear ISP|RCT|TIME0|IE0|PPE0|DTE0 */ > + master_data &= 0xccf0; > /* Enable PPE, IE and TIME as appropriate */ > master_data |= control; > + /* load ISP and RCT */ > master_data |= > (timings[pio][0] << 12) | > (timings[pio][1] << 8); > @@ -816,7 +824,7 @@ static void do_pata_set_dmamode (struct > master_data &= 0xFF4F; /* Mask out IORDY|TIME1|DMAONLY */ > master_data |= control << 4; > pci_read_config_byte(dev, 0x44, &slave_data); > - slave_data &= (0x0F + 0xE1 * ap->port_no); > + slave_data &= (ap->port_no ? 0x0f : 0xf0); > /* Load the matching timing */ > slave_data |= ((timings[pio][0] << 2) | timings[pio][1]) << (ap->port_no ? 4 : 0); > pci_write_config_byte(dev, 0x44, slave_data); > @@ -828,8 +836,11 @@ static void do_pata_set_dmamode (struct > (timings[pio][0] << 12) | > (timings[pio][1] << 8); > } > - udma_enable &= ~(1 << devid); > - pci_write_config_word(dev, master_port, master_data); > + > + if (ap->udma_mask) { > + udma_enable &= ~(1 << devid); > + pci_write_config_word(dev, master_port, master_data); > + } > } > /* Don't scribble on 0x48 if the controller does not support UDMA */ > if (ap->udma_mask) -- -- Sick of rip off UK rail fares ? Learn how to get far cheaper fares http://zeniv.linux.org.uk/~alan/GTR/ ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] ata_piix: fix pio/mwdma programming 2007-05-25 17:16 ` [PATCH] ata_piix: fix pio/mwdma programming Tejun Heo 2007-05-25 18:05 ` Alan Cox @ 2007-05-28 13:02 ` Jeff Garzik 1 sibling, 0 replies; 33+ messages in thread From: Jeff Garzik @ 2007-05-28 13:02 UTC (permalink / raw) To: Tejun Heo; +Cc: Art Haas, Alan Cox, linux-ide Tejun Heo wrote: > Fix various bugs in pio/mwdma mode programming. > > * Control bits in the timing register wasn't cleared properly while > programming PIO mode. > > * MWDMA mode programming cleared the wrong part of control bits. > > * MWDMA mode programming cleared udma_mask even when the controller > doesn't support UDMA. > > Signed-off-by: Tejun Heo <htejun@gmail.com> > Cc: Art Haas <ahaas@airmail.net> > --- > Patch is against libata-dev#upstream. Thanks. > > drivers/ata/ata_piix.c | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) applied to #upstream ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2007-05-28 13:02 UTC | newest] Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-02-02 15:18 [PATCH] ata_piix: fix pio/mwdma programming (for testing, don't apply) Tejun Heo 2007-02-02 15:34 ` Sergei Shtylyov 2007-02-02 16:38 ` Jeff Garzik 2007-02-02 16:57 ` Mark Lord 2007-02-02 18:34 ` Sergei Shtylyov 2007-02-02 16:41 ` Tejun Heo 2007-02-02 18:49 ` Alan 2007-02-02 19:04 ` Sergei Shtylyov 2007-02-02 17:42 ` Alan 2007-02-03 1:40 ` Tejun Heo 2007-02-03 20:04 ` Alan 2007-02-04 2:47 ` Tejun Heo 2007-02-02 21:14 ` Art Haas 2007-02-03 2:09 ` Tejun Heo 2007-02-03 14:35 ` Art Haas 2007-02-03 19:47 ` Mark Lord 2007-02-06 9:11 ` Tejun Heo 2007-02-06 16:33 ` Art Haas 2007-02-07 2:53 ` Tejun Heo 2007-02-07 19:35 ` Art Haas 2007-02-07 19:51 ` Mark Lord 2007-02-07 20:37 ` [PATCH] libata: clear TF before IDENTIFYing Tejun Heo 2007-02-08 14:56 ` Mark Lord 2007-02-13 19:38 ` Art Haas 2007-02-15 23:08 ` Jeff Garzik 2007-04-30 18:29 ` [PATCH] Fix pio/mwdma programming on ata_piix.c Art Haas 2007-05-01 3:02 ` Tejun Heo 2007-05-24 19:59 ` Art Haas 2007-05-24 20:55 ` Jeff Garzik 2007-05-24 21:03 ` Tejun Heo 2007-05-25 17:16 ` [PATCH] ata_piix: fix pio/mwdma programming Tejun Heo 2007-05-25 18:05 ` Alan Cox 2007-05-28 13:02 ` Jeff Garzik
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.