Re: [PATCH] libata: automatically use DMADIR if drive/bridge requires it
Tejun Heo wrote: Back in 2.6.17-rc2, a libata module parameter was added for atapi_dmadir. That's nice, but most SATA devices which need it will tell us about it in their IDENTIFY PACKET response, as bit-15 of word-62 of the returned data (as per ATA7, ATA8 specifications). So for those which specify it, we should automatically use the DMADIR bit. Otherwise, disc writing will fail by default on many SATA-ATAPI drives. This patch adds ATA_DFLAG_DMADIR and make ata_dev_configure() set it if atapi_dmadir is set or identify data indicates DMADIR is necessary. atapi_xlat() is converted to check ATA_DFLAG_DMADIR before setting DMADIR. Original patch is from Mark Lord. Signed-off-by: Tejun Heo [EMAIL PROTECTED] Cc: Mark Lord [EMAIL PROTECTED] --- I don't have a bridge which sets DMADIR but so only checked atapi_dmadir parameter. Thanks. The patch looks good. However, it seems there is no realworld IDE-to-SATA bridge that requires DMADIR and also mangles IDENTIFY PACKET bit-15 of word-62 to indicate its presence. From the previous test of the IDE-to-SATA bridges, only the Sil 3611 requires the host software to hint on ATAPI DMADIR. But Sil 3611 doesn't mangle IDENTIFY PACKET word-62: http://www.spinics.net/lists/linux-ide/msg01514.html Maybe we could use ata_dev_knobble() instead? i.e. If it's an ATAPI device ata_dev_knobble() then we turn on DMADIR (regardless the bridge chip used since setting DMADIR won't hurt those don't need it.) -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/15] libata: zero xfer length on ATAPI data xfer IRQ is HSM violation
Tejun Heo wrote: From: Albert Lee [EMAIL PROTECTED] Treat zero xfer length as HSM violation. While at it, add unlikely()'s to ATAPI ireason and transfer length checks. tj: Formatted patch and added unlikely()'s. Signed-off-by: Albert Lee [EMAIL PROTECTED] Signed-off-by: Tejun Heo [EMAIL PROTECTED] --- drivers/ata/libata-core.c |7 +-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 597e07c..88f7637 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -5280,12 +5280,15 @@ static void atapi_pio_bytes(struct ata_queued_cmd *qc) bytes = (bc_hi 8) | bc_lo; /* shall be cleared to zero, indicating xfer of data */ - if (ireason (1 0)) + if (unlikely(ireason (1 0))) goto err_out; /* make sure transfer direction matches expected */ i_write = ((ireason (1 1)) == 0) ? 1 : 0; - if (do_write != i_write) + if (unlikely(do_write != i_write)) + goto err_out; + + if (unlikely(!bytes)) goto err_out; VPRINTK(ata%u: xfering %d bytes\n, ap-print_id, bytes); Looks good. Acked-by: Albert Lee [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/15] libata: improve ATAPI draining
Tejun Heo wrote: For misc ATAPI commands which transfer variable length data to the host, overflow can occur due to application or hardware bug. Such overflows can be ignored safely as long as overflow data is properly drained. libata HSM implementation has this implemented in __atapi_pio_bytes() but it isn't enough. Improve drain logic such that... * Multiple PIO data phases are allowed. Not allowing this used to be okay when transfer chunk size was set to 8k unconditionally but with transfer hcunk size set to allocation size, treating extra PIO data phases as HSM violations cause a lot of trouble. * Limit the amount of draining to ATAPI_MAX_DRAIN (16k currently). * Don't whine if overflow is allowed and safe. When unexpected overflow occurs, trigger HSM violation and report the problem using ehi error description. * Properly calculate the number of bytes to be drained considering actual number of consumed bytes for partial draining. * Add and use ata_drain_page for draining. This change fixes the problem where LLDs which do 32bit IOs consumes 4 bytes on each 2 byte draining resulting in draining twice more data than requested. This patch fixes ATAPI regressions introduced by setting transfer chunk size to allocation size. Signed-off-by: Tejun Heo [EMAIL PROTECTED] Cc: Albert Lee [EMAIL PROTECTED] Acked-by: Albert Lee [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/14] libata: improve ATAPI draining
Tejun Heo wrote: For misc ATAPI commands which transfer variable length data to the host, overflow can occur due to application or hardware bug. Such overflows can be ignored safely as long as overflow data is properly drained. libata HSM implementation has this implemented in __atapi_pio_bytes() but it isn't enough. Improve drain logic such that... * Multiple PIO data phases are allowed. Not allowing this used to be okay when transfer chunk size was set to 8k unconditionally but with transfer hcunk size set to allocation size, treating extra PIO data phases as HSM violations cause a lot of trouble. * Limit the amount of draining to ATAPI_MAX_DRAIN (16k currently). * Don't whine if overflow is allowed and safe. When unexpected overflow occurs, trigger HSM violation and report the problem using ehi error description. * Properly calculate the number of bytes to be drained considering actual number of consumed bytes for partial draining. * Add and use ata_drain_page for draining. This change fixes the problem where LLDs which do 32bit IOs consumes 4 bytes on each 2 byte draining resulting in draining twice more data than requested. This patch fixes ATAPI regressions introduced by setting transfer chunk size to allocation size. Signed-off-by: Tejun Heo [EMAIL PROTECTED] Cc: Albert Lee [EMAIL PROTECTED] Sorry for the late reply. The new patch looks good. However, I am a little worried about the following code segment in __atapi_pio_bytes(): next_sg: if (!bytes) return 0; Maybe we should add an additional check to atapi_pio_bytes() such that DRQ asserted with bytes = 0 is considered as AC_ERR_HSM? (patch attached below.) Otherwise the device might keep interruping us with DRQ asserted + zero byte count, and we are in infinite loop... -- albert diff -Nrup 00_libata-dev/drivers/ata/libata-core.c 07_more_check/drivers/ata/libata-core.c --- 00_libata-dev/drivers/ata/libata-core.c 2007-11-14 10:08:36.0 +0800 +++ 07_more_check/drivers/ata/libata-core.c 2007-12-04 18:03:48.0 +0800 @@ -5341,6 +5341,9 @@ static void atapi_pio_bytes(struct ata_q if (do_write != i_write) goto err_out; + if (!bytes) + goto err_out; + VPRINTK(ata%u: xfering %d bytes\n, ap-print_id, bytes); __atapi_pio_bytes(qc, bytes); - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/13] libata: improve ATAPI draining
Tejun Heo wrote: For misc ATAPI commands which transfer variable length data to the host, overflow can occur due to application or hardware bug. Such overflows can be ignored safely as long as overflow data is properly drained. libata HSM implementation has this implemented in __atapi_pio_bytes() but it isn't enough. Improve drain logic such that... * Multiple PIO data phases are allowed. Not allowing this used to be okay when transfer chunk size was set to 8k unconditionally but with transfer hcunk size set to allocation size, treating extra PIO data phases as HSM violations cause a lot of trouble. * Limit the amount of draining to ATAPI_MAX_DRAIN (16k currently). * Don't whine if overflow is allowed and safe. When unexpected overflow occurs, trigger HSM violation and report the problem using ehi error description. * If the device indicates that it wants to transfer odd number of bytes, it should be rounded up not down. If the trailing data is odd-lengthed, normally the situation is that we have odd-lengthed real data before the trailing data. e.g. The real data is 9 bytes, but the drive returns 10 bytes (so, the trailing data is 1 byte). In ata_data_xfer(), we have the following code: /* Transfer trailing 1 byte, if any. */ ... (for write case) ... iowrite16(le16_to_cpu(align_buf[0]), ap-ioaddr.data_addr); or ... (for read case) ... ioread16(ap-ioaddr.data_addr) The PATA bus is actually 16-bit wide. So, ata_data_xfer() actually implicitly transfers one more byte than we see if it's odd-lengthed. That's why in atapi_pio_bytes(), the trailing length was round down instead round up. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] libata: use ATA_HORKAGE_STUCK_ERR for ATAPI tape drives
Per Mark's comments, maybe all ATAPI tape drives need ATA_HORKAGE_STUCK_ERR.
This patch applys ATA_HORKAGE_STUCK_ERR for all ATAPI tape drives.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
Cc: Mark Lord [EMAIL PROTECTED]
---
diff -Nrup 01_ide_tape_stuck_err/drivers/ata/libata-core.c
02_black_ide_tape_drives/drivers/ata/libata-core.c
--- 01_ide_tape_stuck_err/drivers/ata/libata-core.c 2007-11-14
11:20:31.0 +0800
+++ 02_black_ide_tape_drives/drivers/ata/libata-core.c 2007-11-14
11:45:33.0 +0800
@@ -2307,8 +2307,10 @@ int ata_dev_configure(struct ata_device
}
if ((dev-class == ATA_DEV_ATAPI)
- (atapi_command_packet_set(id) == TYPE_TAPE))
+ (atapi_command_packet_set(id) == TYPE_TAPE)) {
dev-max_sectors = ATA_MAX_SECTORS_TAPE;
+ dev-horkage |= ATA_HORKAGE_STUCK_ERR;
+ }
if (dev-horkage ATA_HORKAGE_MAX_SEC_128)
dev-max_sectors = min_t(unsigned int, ATA_MAX_SECTORS_128,
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] libata: workaround DRQ=1 ERR=1 for ATAPI tape drives
After an error condition, some ATAPI tape drives set DRQ=1 together with ERR=1
when asking the host to transfer the CDB of the next packet command (i.e.
request sense).
This patch, a revised version of Alan/Mark's previous patch, adds
ATA_HORKAGE_STUCK_ERR
to workaround the problem by ignoring the ERR bit and proceed sending the CDB.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
Cc: Alan Cox [EMAIL PROTECTED]
Cc: Mark Lord [EMAIL PROTECTED]
---
Revised per Alan, Mark and Tejun's comments. Tested ok with the Seagate
STT8000A tape drive.
Patch against the libata-dev tree.
diff -Nrup 00_libata-dev/drivers/ata/libata-core.c
01_ide_tape_stuck_err/drivers/ata/libata-core.c
--- 00_libata-dev/drivers/ata/libata-core.c 2007-11-14 10:08:36.0
+0800
+++ 01_ide_tape_stuck_err/drivers/ata/libata-core.c 2007-11-14
11:20:31.0 +0800
@@ -5490,11 +5490,19 @@ fsm_start:
* let the EH abort the command or reset the device.
*/
if (unlikely(status (ATA_ERR | ATA_DF))) {
- ata_port_printk(ap, KERN_WARNING, DRQ=1 with device
- error, dev_stat 0x%X\n, status);
- qc-err_mask |= AC_ERR_HSM;
- ap-hsm_task_state = HSM_ST_ERR;
- goto fsm_start;
+ /* Some ATAPI tape drives forget to clear the ERR bit
+* when doing the next command (mostly request sense).
+* We ignore ERR here to workaround and proceed sending
+* the CDB.
+*/
+ if (!(qc-dev-horkage ATA_HORKAGE_STUCK_ERR)) {
+ ata_port_printk(ap, KERN_WARNING,
+ DRQ=1 with device error,
+ dev_stat 0x%X\n, status);
+ qc-err_mask |= AC_ERR_HSM;
+ ap-hsm_task_state = HSM_ST_ERR;
+ goto fsm_start;
+ }
}
/* Send the CDB (atapi) or the first data block (ata pio out).
diff -Nrup 00_libata-dev/include/linux/libata.h
01_ide_tape_stuck_err/include/linux/libata.h
--- 00_libata-dev/include/linux/libata.h2007-11-14 10:08:59.0
+0800
+++ 01_ide_tape_stuck_err/include/linux/libata.h2007-11-14
11:19:32.0 +0800
@@ -340,6 +340,7 @@ enum {
ATA_HORKAGE_HPA_SIZE= (1 6), /* native size off by one */
ATA_HORKAGE_IPM = (1 7), /* Link PM problems */
ATA_HORKAGE_IVB = (1 8), /* cbl det validity bit bugs */
+ ATA_HORKAGE_STUCK_ERR = (1 9), /* stuck ERR on next PACKET */
/* DMA mask for user DMA control: User visible values; DO NOT
renumber */
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fix ATAPI transfer lengths causes CD writing regression
Tejun Heo wrote:
Daniel Drake wrote:
Tejun Heo wrote:
4ata2.00: HSM violation: eh_analyze_tf: BUSY|DRQ
3ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen
3ata2.00: cmd a0/00:00:00:0a:00/00:00:00:00:00/a0 tag 0 cdb 0x5a data
10 in
4 res 58/00:02:00:0a:00/00:00:00:00:00/a0 Emask 0x2 (HSM
violation)
3ata2.00: status: { DRDY DRQ }
6ata2: soft resetting link
6ata2.00: configured for UDMA/33
6ata2: EH complete
Does this patch fix the problem?
That fixes it, thanks! There is no more ugly error in dmesg, the test
prog doesn't print any sense data, and brasero works OK too. However,
these messages appear in the kernel log every time I run the test app
(or when brasero does its thing):
4ata2.00: 10 bytes trailing data
4ata2.00: 10 bytes trailing data
4ata2.00: 10 bytes trailing data
4ata2.00: 10 bytes trailing data
4ata2.00: 10 bytes trailing data
4ata2.00: 10 bytes trailing data
4ata2.00: 6 bytes trailing data
Yeah, that's expected. What's going on here is that your drive sends
full mode sense data (76bytes) regardless of allocation size in CDB but
it does honor transfer chunk set in the PACKET TF, which is set to the
same value as allocation size by Alan's patch. So, now the drive sends
the 76 bytes in 8 chunks. The first chunk is transferred into the sg
buffer and the following chunks are thrown away.
Previously, transfer chunk was set to 8k, so the drive claims to
transfer 76 bytes from the buegging, libata transfers leading 10 bytes
got transferred into the user buffer and throws away what's remaining.
The change caused problem because libata HSM always switches to
HSM_ST_LAST (command sequence completion) after filling the command
buffer completely. So, throwing away is activated iff the extra data to
throw away is transfered together with the last chunk of useful data.
With the chunk size reduced to allocation size, the initial chunk fills
the data buffer completely and all the extra bytes are transfered in
separate chunks. However, libata HSM expects command sequence to
complete after the initial chunk but the drive asserts DRQ for the next
chunk on the following interrupt, so HSM violation is triggered.
The patch modifies HSM such that it keeps throwing away extra data as
long as the drive asserts DRQ which is how IDE driver does it.
From past experience, some dead ATAPI devices stuck in DRQ=1. We should
take care of such situation, otherwise the HSM might get into an infinite
loop of waiting for the dead ATAPI device to say DRQ=0 and discarding
endless trailing data.
Maybe we could set a limit here. If the ATAPI device keeps DRQ=1 and
exceeds the limit, we consider it as HSM violation and have EH handle it.
--
albert
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] libata: change the last state of pio read to HSM_ST_IDLE
Jeff Garzik wrote: Albert Lee wrote: Patch 2/2: After reading the last pio data block, the HSM is waiting for device to be idle, not waiting for the last interrupt. This patch changes the state after PIO data-in to HSM_ST_IDLE instead of HSM_ST_LAST for accuracy. Signed-off-by: Albert Lee [EMAIL PROTECTED] Is this still needed? Not quite needed; it only makes the state transition after reading the last PIO block more accurate. However, if we want to do part of the irq PIO in the workqueue sometime in the future, this patch will be needed (otherwise the HSM might think it expecting another irq). For the time being, maybe we can just skip this patch until sometime it's really needed. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ATAPI tape drives broken with libata
Mark Lord wrote:
I can read and write the tape successfully, and the written data matches
what is later read back from it.
Here's the hack I used. Not ready for mainline, but perhaps it will
help Alan or whomever come up with something nice for Jeff.
--- 2.6.23-rc6/drivers/ata/libata-core.c 2007-09-13 09:49:16.0
-0400
+++ linux/drivers/ata/libata-core.c 2007-09-13 14:15:57.0 -0400
@@ -4932,9 +4932,9 @@
if (unlikely(status (ATA_ERR | ATA_DF))) {
ata_port_printk(ap, KERN_WARNING, DRQ=1 with
device
error, dev_stat 0x%X\n, status);
- qc-err_mask |= AC_ERR_HSM;
- ap-hsm_task_state = HSM_ST_ERR;
- goto fsm_start;
+ //qc-err_mask |= AC_ERR_HSM;
+ //ap-hsm_task_state = HSM_ST_ERR;
+ //goto fsm_start;
}
/* Send the CDB (atapi) or the first data block (ata pio
out).
It's strange that the device reports DRQ + ERR at this point since if
it knows something wrong, asking to transfer the CDB doesn't help...
Is the device configured to MWDMA? If yes, maybe it's ATAPI DMA of
the previous command makes the device state machine confused...
Could you please try if the attached debug patch helps? Thanks.
--
albert
diff -Nrup 00_libata-dev/drivers/ata/libata-core.c
01_atapi_dma/drivers/ata/libata-core.c
--- 00_libata-dev/drivers/ata/libata-core.c 2007-09-21 10:35:54.0
+0800
+++ 01_atapi_dma/drivers/ata/libata-core.c 2007-09-21 10:49:20.0
+0800
@@ -4193,6 +4193,8 @@ static void ata_fill_sg_dumb(struct ata_
int ata_check_atapi_dma(struct ata_queued_cmd *qc)
{
struct ata_port *ap = qc-ap;
+ struct scsi_cmnd *cmd = qc-scsicmd;
+ u8 *scsicmd = cmd-cmnd;
/* Don't allow DMA if it isn't multiple of 16 bytes. Quite a
* few ATAPI devices choke on such DMA requests.
@@ -4200,6 +4202,21 @@ int ata_check_atapi_dma(struct ata_queue
if (unlikely(qc-nbytes 15))
return 1;
+ switch (scsicmd[0]) {
+ case READ_10:
+ case WRITE_10:
+ case READ_12:
+ case WRITE_12:
+ case READ_6:
+ case WRITE_6:
+ case 0xad: /* READ_DVD_STRUCTURE */
+ case 0xbe: /* READ_CD */
+ /* ATAPI DMA maybe ok */
+ break;
+ default:
+ return 1;
+ }
+
if (ap-ops-check_atapi_dma)
return ap-ops-check_atapi_dma(qc);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.23-rc3] pata_pdc2027x: PLL detection fixes
Sergei Shtylyov wrote: Hello. Mikael Pettersson wrote: Previously I reported that the pata_pdc2027x PLL detection changes in kernel 2.6.22 broke the driver on my PowerMac: pata_pdc2027x: Invalid PLL input clock 1691742kHz, give up! This is followed by a number of errors and speed reduction steps on the affected ports. There are two bugs in pata_pdc2027x's PLL detection code: 1. The PLL counter's start value is read before the chip is put in test mode. Outside of test mode the counter is halted, and on the PowerMac the counter is zero because the chip hasn't been initialised by its BIOS. So what? a) causes an unnecessary wraparound, which in turn is one of the causes for PLL detection failures on non-x86 It is *not* the cause of failure -- the old IDE driver copes well with this on non-x86. ;-) b) puts more work [the enter test mode stuff] in between the start and and sampling points, reducing the precision of the PLL detection; I actually observed quite noticeable differences in detected PLL frequency based on whether the start was sampled before or after the test mode enter code I'd think this differnce is negligible with 100 ms delay. But why not -- shouldn't harm indeed (except it's better to read a stable counter before than unstable one after entering test mode)? The fix is to move the read of the start value to after test mode is started, but before the mdelay() in test mode. This is not an issue, so no fix is needed. This also improves the precision of the PLL detection. BTW, looks like we don't even need to bother reading the darn counter beforehand: bit 1 of the indexed register 1 (the same used to enter/exit test mode by twiddling its bit 6) when being cleared should reset the counter to 0 Or maybe to 0x7fff? I can't remember already -- never seen those infamous Promise papers, and I was testing this code looong ago already... :-) I've tested reloading the pata_pdc2027x module before. The counter seems not cleared when leaving the test mode. -- I'm looking at the internal sources which were written based on the *fragment* of the PDC20270 datasheet (yeah, Promise didn't even give us the whole datasheet!) about the PLL calibration. Well, I have no data sheet and no sources except what's in the kernel and what debug info PDC_DEBUG generates. IIRC, Albert should have the docs but he's under NDA. What have really surpried me about Promise was that they gave their SATA chip docs to Jeff who made them public and yet they continue to conceal the old PATA chip docs... :-/ 2. The code to compute the number of PLL decrements during the mdelay() in test mode fails to consider that the PLL counter only is 30 bits wide. If there is a wraparound, it will compute an incorrect and much too large value. On the PowerMac, the start count is zero, the end count is a large 30-bit value, so wraparound occurs and an out of bounds PLL clock is detected. The fix is to mask the (start - end) computation to 30 bits. Yeah, that's what I've done for the old IDE driver... Except that due to what may be a typo pdc202xx_new masks to 26 bits, not 30. Indeed! :- Thanks for noticing -- this is a typo, of course... And it's a pity that Albert failed to notice it when he last touched that driver... I was too blind to notice the wrong 26-bit mask. :( Fortunately with the 10ms delay used by the IDE pdc202xx_new driver, (start_count - end_count) is smaller than the 26-bit mask. So it doesn't actually damage anything. I was going to address that if/when this patch goes in. Please do, I'm too short of time. But I guess the difference was never that large, so the clipped mask worked... I wonder if the true reason of the former issue which was attibuted mdelay() being imprecise... mdelay() is actually imprecise on my x86 PC. This can be measured by doing some do_gettimeofday() tests. It's quite precise when tested on ppc64... -- albert While debugging this I also noticed that pdc_read_counter() reads the two halves of the 30-bit PLL counter as 16-bit values, and then combines them as if the halves only are 15 bits wide. To avoid confusion, the halves should be read as 15-bit values. Shouldn't matter, the bit is most probably reseved and so always remains 0. Actually, those 2 counters count the data bytes transferred over PCI bus when the chip in not in the test mode. It matters when someone reads the code and wonders why two 16-bit values (readl() 0x) are combined with a 15-bit shift ((x 15) | y). Well, let it be. :-) This patch implements all three changes. It fixes the PLL detection failure on my PowerMac, and doesn't cause any regressions on an x86 with an identical card. Signed-off-by: Mikael Pettersson [EMAIL PROTECTED] - To unsubscribe from
[PATCH 2.6.23-rc3] libata: pata_pdc2027x PLL detection minor cleanup
Minor cleanup to remove the unneeded rmb()s per Jeff's advice. Also removed the pll_clock 0 check since pll_clock now guaranteed to be = 0 after Mikael's patch. Signed-off-by: Albert Lee [EMAIL PROTECTED] --- Tested ok on both x86 and ppc64, together with Mikael's patch. diff -Nrup 01_mikael/drivers/ata/pata_pdc2027x.c 02_pdc_pll_fix2/drivers/ata/pata_pdc2027x.c --- 01_mikael/drivers/ata/pata_pdc2027x.c 2007-08-20 10:43:38.0 +0800 +++ 02_pdc_pll_fix2/drivers/ata/pata_pdc2027x.c 2007-08-20 10:48:22.0 +0800 @@ -565,12 +565,10 @@ static long pdc_read_counter(struct ata_ retry: bccrl = readl(mmio_base + PDC_BYTE_COUNT) 0x7fff; bccrh = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x7fff; - rmb(); /* Read the counter values again for verification */ bccrlv = readl(mmio_base + PDC_BYTE_COUNT) 0x7fff; bccrhv = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x7fff; - rmb(); counter = (bccrh 15) | bccrl; @@ -745,9 +743,6 @@ static int pdc_hardware_init(struct ata_ */ pll_clock = pdc_detect_pll_input_clock(host); - if (pll_clock 0) /* counter overflow? Try again. */ - pll_clock = pdc_detect_pll_input_clock(host); - dev_printk(KERN_INFO, host-dev, PLL input clock %ld kHz\n, pll_clock/1000); /* Adjust PLL control register */ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.23-rc3] pata_pdc2027x: PLL detection fixes
Mikael Pettersson wrote: Previously I reported that the pata_pdc2027x PLL detection changes in kernel 2.6.22 broke the driver on my PowerMac: pata_pdc2027x: Invalid PLL input clock 1691742kHz, give up! This is followed by a number of errors and speed reduction steps on the affected ports. There are two bugs in pata_pdc2027x's PLL detection code: 1. The PLL counter's start value is read before the chip is put in test mode. Outside of test mode the counter is halted, and on the PowerMac the counter is zero because the chip hasn't been initialised by its BIOS. The fix is to move the read of the start value to after test mode is started, but before the mdelay() in test mode. This also improves the precision of the PLL detection. 2. The code to compute the number of PLL decrements during the mdelay() in test mode fails to consider that the PLL counter only is 30 bits wide. If there is a wraparound, it will compute an incorrect and much too large value. On the PowerMac, the start count is zero, the end count is a large 30-bit value, so wraparound occurs and an out of bounds PLL clock is detected. The fix is to mask the (start - end) computation to 30 bits. The wrap-around situation was handled by checking if (pll_clock 0) in pdc_hardware_init() then retrying pdc_detect_pll_input_clock(). But it seems not working correctly. :( While debugging this I also noticed that pdc_read_counter() reads the two halves of the 30-bit PLL counter as 16-bit values, and then combines them as if the halves only are 15 bits wide. To avoid confusion, the halves should be read as 15-bit values. This patch implements all three changes. It fixes the PLL detection failure on my PowerMac, and doesn't cause any regressions on an x86 with an identical card. Signed-off-by: Mikael Pettersson [EMAIL PROTECTED] --- drivers/ata/pata_pdc2027x.c | 18 +- 1 files changed, 9 insertions(+), 9 deletions(-) diff -rupN linux-2.6.23-rc3/drivers/ata/pata_pdc2027x.c linux-2.6.23-rc3.pata_pdc2027x-pll-detection-fixes/drivers/ata/pata_pdc2027x.c --- linux-2.6.23-rc3/drivers/ata/pata_pdc2027x.c 2007-07-09 22:01:31.0 +0200 +++ linux-2.6.23-rc3.pata_pdc2027x-pll-detection-fixes/drivers/ata/pata_pdc2027x.c 2007-08-18 21:53:40.0 +0200 @@ -563,13 +563,13 @@ static long pdc_read_counter(struct ata_ u32 bccrl, bccrh, bccrlv, bccrhv; retry: - bccrl = readl(mmio_base + PDC_BYTE_COUNT) 0x; - bccrh = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x; + bccrl = readl(mmio_base + PDC_BYTE_COUNT) 0x7fff; + bccrh = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x7fff; rmb(); /* Read the counter values again for verification */ - bccrlv = readl(mmio_base + PDC_BYTE_COUNT) 0x; - bccrhv = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x; + bccrlv = readl(mmio_base + PDC_BYTE_COUNT) 0x7fff; + bccrhv = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x7fff; rmb(); Agreed this looks safer. (Although the high bit 15 of the counter is always zero during my test.) counter = (bccrh 15) | bccrl; @@ -692,16 +692,16 @@ static long pdc_detect_pll_input_clock(s struct timeval start_time, end_time; long pll_clock, usec_elapsed; - /* Read current counter value */ - start_count = pdc_read_counter(host); - do_gettimeofday(start_time); - /* Start the test mode */ scr = readl(mmio_base + PDC_SYS_CTL); PDPRINTK(scr[%X]\n, scr); writel(scr | (0x01 14), mmio_base + PDC_SYS_CTL); readl(mmio_base + PDC_SYS_CTL); /* flush */ + /* Read current counter value */ + start_count = pdc_read_counter(host); + do_gettimeofday(start_time); + /* Let the counter run for 100 ms. */ mdelay(100); Looks good (though reading the start_count before or after doesn't really matter since we can treat start the test mode as part of the 100ms delay time). @@ -719,7 +719,7 @@ static long pdc_detect_pll_input_clock(s usec_elapsed = (end_time.tv_sec - start_time.tv_sec) * 100 + (end_time.tv_usec - start_time.tv_usec); - pll_clock = (start_count - end_count) / 100 * + pll_clock = ((start_count - end_count) 0x3fff) / 100 * (1 / usec_elapsed); PDPRINTK(start[%ld] end[%ld] \n, start_count, end_count); Hmm, this one alone looks like the real fix for the problem. I guess my previous patch changing pll_clock from (start_count - end_count) * 10 to (start_count - end_count) / 100 * (1 / usec_elapsed) somehow broke the (pll_clock 0) check... Thanks for fixing it. Maybe we also need this fix for the IDE pdc202xx_new.c driver... Acked-by: Albert Lee [EMAIL PROTECTED] -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More
Re: [PATCH 2.6.23-rc3] pata_pdc2027x: PLL detection fixes
Jeff Garzik wrote: Mikael Pettersson wrote: Previously I reported that the pata_pdc2027x PLL detection changes in kernel 2.6.22 broke the driver on my PowerMac: pata_pdc2027x: Invalid PLL input clock 1691742kHz, give up! This is followed by a number of errors and speed reduction steps on the affected ports. There are two bugs in pata_pdc2027x's PLL detection code: 1. The PLL counter's start value is read before the chip is put in test mode. Outside of test mode the counter is halted, and on the PowerMac the counter is zero because the chip hasn't been initialised by its BIOS. The fix is to move the read of the start value to after test mode is started, but before the mdelay() in test mode. This also improves the precision of the PLL detection. 2. The code to compute the number of PLL decrements during the mdelay() in test mode fails to consider that the PLL counter only is 30 bits wide. If there is a wraparound, it will compute an incorrect and much too large value. On the PowerMac, the start count is zero, the end count is a large 30-bit value, so wraparound occurs and an out of bounds PLL clock is detected. The fix is to mask the (start - end) computation to 30 bits. While debugging this I also noticed that pdc_read_counter() reads the two halves of the 30-bit PLL counter as 16-bit values, and then combines them as if the halves only are 15 bits wide. To avoid confusion, the halves should be read as 15-bit values. This patch implements all three changes. It fixes the PLL detection failure on my PowerMac, and doesn't cause any regressions on an x86 with an identical card. Signed-off-by: Mikael Pettersson [EMAIL PROTECTED] Fantastic! Thanks for putting in a great effort to track these down. I'll queue it up [unless someone responds with a problem requiring revision, of course] diff -rupN linux-2.6.23-rc3/drivers/ata/pata_pdc2027x.c linux-2.6.23-rc3.pata_pdc2027x-pll-detection-fixes/drivers/ata/pata_pdc2027x.c --- linux-2.6.23-rc3/drivers/ata/pata_pdc2027x.c2007-07-09 22:01:31.0 +0200 +++ linux-2.6.23-rc3.pata_pdc2027x-pll-detection-fixes/drivers/ata/pata_pdc2027x.c 2007-08-18 21:53:40.0 +0200 @@ -563,13 +563,13 @@ static long pdc_read_counter(struct ata_ u32 bccrl, bccrh, bccrlv, bccrhv; retry: -bccrl = readl(mmio_base + PDC_BYTE_COUNT) 0x; -bccrh = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x; +bccrl = readl(mmio_base + PDC_BYTE_COUNT) 0x7fff; +bccrh = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x7fff; rmb(); /* Read the counter values again for verification */ -bccrlv = readl(mmio_base + PDC_BYTE_COUNT) 0x; -bccrhv = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x; +bccrlv = readl(mmio_base + PDC_BYTE_COUNT) 0x7fff; +bccrhv = readl(mmio_base + PDC_BYTE_COUNT + 0x100) 0x7fff; rmb(); counter = (bccrh 15) | bccrl; Unrelated to your changes, but, I wonder why those rmb() are there at all...? The first rmb() is to make sure bccrl is read before bccrlv for later (bccrl = bccrlv) check since both reading the same memory address. The second rmb() looks like leftover when the code copy-and-paste'ed. Maybe we can remove this one. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2.6.23-rc3] pata_pdc2027x: PLL detection fixes
Jeff Garzik wrote: Albert Lee wrote: The first rmb() is to make sure bccrl is read before bccrlv for later (bccrl = bccrlv) check since both reading the same memory address. That's already guaranteed without the rmb(), AFAICS. Hmm, thanks for the advice. Will remove both rmb()s. Will also remove the now unused (pll_clock 0) check from pdc_hardware_init() and test/verify Mikael's patch when I go back to office tomorrow. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: unexpected scsi timeout
Vasily Averin wrote: Tejun Heo wrote: [cc'ing Albert] Vasily Averin wrote: Tejun, Jeff I've noticed that some scsi commands for DVD-drive attached to pata_via successfully finishes without any delays but reports about TIMEOUT condition. It happens because of ATA_ERR bit is set in status register. As result for each command Error Handler thread awakened, requests sense buffer and go to sleep again. Need more info. Please post boot dmesg and the result of 'lspci -nn' and 'hdparm -I /dev/srX' and when such errors occur. It was 2.6.22 kernel with pata_via and sata_via drivers, scsi and cdrom debug was temporally enabled via sysctl (please see logs near Jul 24 13:42:46 timestamp) Btw. I'm not sure that it was an error, I've looked on the sources and IMHO it's normal command processing cdrom without disk inserted into drive. I've checked 2.6.19, 2.6.20 and 2.6.22 kernels and got the same behavior. Hi Vasily, Your log looks ok. It's normal for TEST_UNIT_READY to return ATA_ERR when no disc inside and libata EH triggered to request sense. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/7] libata: remove irq_on from ata_bus_reset() and ata_std_postreset()
It seems irq_on() in ata_bus_reset() and ata_std_postreset()
are leftover of the EDD reset. Remove them.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 00_libata-dev/drivers/ata/libata-core.c
01_remove_leftover_irqon/drivers/ata/libata-core.c
--- 00_libata-dev/drivers/ata/libata-core.c 2007-07-07 09:58:55.0
+0800
+++ 01_remove_leftover_irqon/drivers/ata/libata-core.c 2007-07-07
10:38:55.0 +0800
@@ -3194,9 +3194,6 @@ void ata_bus_reset(struct ata_port *ap)
if ((slave_possible) (err != 0x81))
ap-device[1].class = ata_dev_try_classify(ap, 1, err);
- /* re-enable interrupts */
- ap-ops-irq_on(ap);
-
/* is double-select really necessary? */
if (ap-device[1].class != ATA_DEV_NONE)
ap-ops-dev_select(ap, 1);
@@ -3581,10 +3578,6 @@ void ata_std_postreset(struct ata_port *
if (sata_scr_read(ap, SCR_ERROR, serror) == 0)
sata_scr_write(ap, SCR_ERROR, serror);
- /* re-enable interrupts */
- if (!ap-ops-error_handler)
- ap-ops-irq_on(ap);
-
/* is double-select really necessary? */
if (classes[0] != ATA_DEV_NONE)
ap-ops-dev_select(ap, 1);
diff -Nrup 00_libata-dev/drivers/ata/pata_scc.c
01_remove_leftover_irqon/drivers/ata/pata_scc.c
--- 00_libata-dev/drivers/ata/pata_scc.c2007-07-07 09:58:55.0
+0800
+++ 01_remove_leftover_irqon/drivers/ata/pata_scc.c 2007-07-07
10:38:55.0 +0800
@@ -892,10 +892,6 @@ static void scc_std_postreset (struct at
{
DPRINTK(ENTER\n);
- /* re-enable interrupts */
- if (!ap-ops-error_handler)
- ap-ops-irq_on(ap);
-
/* is double-select really necessary? */
if (classes[0] != ATA_DEV_NONE)
ap-ops-dev_select(ap, 1);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] sata_promise: pdc_freeze() semantic change
After checking the current implementations of freeze()/thaw(), it seems only pdc_freeze() does more than simple irq masking. Remove the DMA stop code from pdc_freeze(). Signed-off-by: Albert Lee [EMAIL PROTECTED] --- diff -Nrup 01_remove_leftover_irqon/drivers/ata/sata_promise.c 02_sata_pdc_freeze/drivers/ata/sata_promise.c --- 01_remove_leftover_irqon/drivers/ata/sata_promise.c 2007-07-07 09:58:55.0 +0800 +++ 02_sata_pdc_freeze/drivers/ata/sata_promise.c 2007-07-07 10:39:22.0 +0800 @@ -578,7 +578,6 @@ static void pdc_freeze(struct ata_port * tmp = readl(mmio + PDC_CTLSTAT); tmp |= PDC_IRQ_DISABLE; - tmp = ~PDC_DMA_ENABLE; writel(tmp, mmio + PDC_CTLSTAT); readl(mmio + PDC_CTLSTAT); /* flush */ } - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/7] libata: use freeze/thaw for polling PIO
This patch let polling pio codes to use freeze()/thaw() for irq off/on.
The reason is: some ATAPI devices raises INTRQ even of nIEN = 1.
Using the host adapter irq mask mechanism, if available, is more reliable than
the device nIEN bit.
ata_qc_set_polling() is also removed since now unused.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Considerations:
Polling PIO, the new user of freeze()/thaw(), might have higher calling
frequency to freeze()/thaw() than EH.
Can current implemention of freeze()/thaw() handle that?
We might need more testing on sata_nv, sata_promise, sata_sil, sata_sil24,
sata_via and sata_vsc.
diff -Nrup 03_add_thaw_freeze/drivers/ata/libata-core.c
04_use_freeze_polling/drivers/ata/libata-core.c
--- 03_add_thaw_freeze/drivers/ata/libata-core.c2007-07-07
10:38:55.0 +0800
+++ 04_use_freeze_polling/drivers/ata/libata-core.c 2007-07-07
10:40:10.0 +0800
@@ -4753,7 +4753,7 @@ static void ata_hsm_qc_complete(struct a
qc = ata_qc_from_tag(ap, qc-tag);
if (qc) {
if (likely(!(qc-err_mask AC_ERR_HSM))) {
- ap-ops-irq_on(ap);
+ ap-ops-thaw(ap);
ata_qc_complete(qc);
} else
ata_port_freeze(ap);
@@ -4769,7 +4769,7 @@ static void ata_hsm_qc_complete(struct a
} else {
if (in_wq) {
spin_lock_irqsave(ap-lock, flags);
- ap-ops-irq_on(ap);
+ ap-ops-thaw(ap);
ata_qc_complete(qc);
spin_unlock_irqrestore(ap-lock, flags);
} else
@@ -5411,7 +5411,7 @@ unsigned int ata_qc_issue_prot(struct at
switch (qc-tf.protocol) {
case ATA_PROT_NODATA:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ata_qc_set_polling(qc);
+ ap-ops-freeze(ap);
ata_tf_to_host(ap, qc-tf);
ap-hsm_task_state = HSM_ST_LAST;
@@ -5432,7 +5432,7 @@ unsigned int ata_qc_issue_prot(struct at
case ATA_PROT_PIO:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ata_qc_set_polling(qc);
+ ap-ops-freeze(ap);
ata_tf_to_host(ap, qc-tf);
@@ -5461,7 +5461,7 @@ unsigned int ata_qc_issue_prot(struct at
case ATA_PROT_ATAPI:
case ATA_PROT_ATAPI_NODATA:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ata_qc_set_polling(qc);
+ ap-ops-freeze(ap);
ata_tf_to_host(ap, qc-tf);
diff -Nrup 03_add_thaw_freeze/include/linux/libata.h
04_use_freeze_polling/include/linux/libata.h
--- 03_add_thaw_freeze/include/linux/libata.h 2007-07-07 09:59:42.0
+0800
+++ 04_use_freeze_polling/include/linux/libata.h2007-07-07
10:40:10.0 +0800
@@ -1094,11 +1094,6 @@ static inline u8 ata_wait_idle(struct at
return status;
}
-static inline void ata_qc_set_polling(struct ata_queued_cmd *qc)
-{
- qc-tf.ctl |= ATA_NIEN;
-}
-
static inline struct ata_queued_cmd *__ata_qc_from_tag(struct ata_port *ap,
unsigned int tag)
{
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/7] libata: remove nIEN handling from ata_tf_load()
The relevant bits in the ctl register are HOB, SRST and nIEN.
- HOB is only used by ata_tf_read().
- For SRST, soft reset is not the duty of tf_load.
- For nIEN, explicit irq_on()/irq_off and freeze()/thaw() are provided.
Remove the implicit nIEN handling from ata_tf_load() since EH/HSM now
calls irq_on/irq_off explicitly. vsc_intr_mask_update() also removed
since now unused.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
The bitmasks used by vsc_intr_mask_update() are different to the bit masks
in vsc_irq_on() and vsc_irq_off(). I don't know which one is more correct.
Maybe we need more test and see if vsc_irq_on()/vsc_irq_off() works for
turning irq on/off for polling pio.
diff -Nrup 05_rename_thaw_lldd/drivers/ata/libata-sff.c
06_tf_load_cleanup/drivers/ata/libata-sff.c
--- 05_rename_thaw_lldd/drivers/ata/libata-sff.c2007-07-07
10:49:04.0 +0800
+++ 06_tf_load_cleanup/drivers/ata/libata-sff.c 2007-07-07 10:50:10.0
+0800
@@ -143,11 +143,13 @@ void ata_tf_load(struct ata_port *ap, co
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- if (tf-ctl != ap-last_ctl) {
- iowrite8(tf-ctl, ioaddr-ctl_addr);
- ap-last_ctl = tf-ctl;
- ata_wait_idle(ap);
- }
+ /*
+* The relevant bits in the ctl register are HOB, SRST and nIEN.
+* HOB is only used by ata_tf_read().
+* For SRST, soft reset is not the duty of tf_load.
+* For nIEN, explicit -irq_on() and -irq_off are provided.
+* That's why tf-ctl is ignored here.
+*/
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
iowrite8(tf-hob_feature, ioaddr-feature_addr);
diff -Nrup 05_rename_thaw_lldd/drivers/ata/pata_scc.c
06_tf_load_cleanup/drivers/ata/pata_scc.c
--- 05_rename_thaw_lldd/drivers/ata/pata_scc.c 2007-07-07 10:49:30.0
+0800
+++ 06_tf_load_cleanup/drivers/ata/pata_scc.c 2007-07-07 10:50:10.0
+0800
@@ -271,12 +271,6 @@ static void scc_tf_load (struct ata_port
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- if (tf-ctl != ap-last_ctl) {
- out_be32(ioaddr-ctl_addr, tf-ctl);
- ap-last_ctl = tf-ctl;
- ata_wait_idle(ap);
- }
-
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
out_be32(ioaddr-feature_addr, tf-hob_feature);
out_be32(ioaddr-nsect_addr, tf-hob_nsect);
diff -Nrup 05_rename_thaw_lldd/drivers/ata/sata_svw.c
06_tf_load_cleanup/drivers/ata/sata_svw.c
--- 05_rename_thaw_lldd/drivers/ata/sata_svw.c 2007-07-07 10:49:30.0
+0800
+++ 06_tf_load_cleanup/drivers/ata/sata_svw.c 2007-07-07 10:50:10.0
+0800
@@ -125,11 +125,6 @@ static void k2_sata_tf_load(struct ata_p
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- if (tf-ctl != ap-last_ctl) {
- writeb(tf-ctl, ioaddr-ctl_addr);
- ap-last_ctl = tf-ctl;
- ata_wait_idle(ap);
- }
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
writew(tf-feature | (((u16)tf-hob_feature) 8),
ioaddr-feature_addr);
diff -Nrup 05_rename_thaw_lldd/drivers/ata/sata_vsc.c
06_tf_load_cleanup/drivers/ata/sata_vsc.c
--- 05_rename_thaw_lldd/drivers/ata/sata_vsc.c 2007-07-07 10:49:30.0
+0800
+++ 06_tf_load_cleanup/drivers/ata/sata_vsc.c 2007-07-07 10:50:10.0
+0800
@@ -137,36 +137,19 @@ static void vsc_irq_on(struct ata_port *
}
-static void vsc_intr_mask_update(struct ata_port *ap, u8 ctl)
-{
- void __iomem *mask_addr;
- u8 mask;
-
- mask_addr = ap-host-iomap[VSC_MMIO_BAR] +
- VSC_SATA_INT_MASK_OFFSET + ap-port_no;
- mask = readb(mask_addr);
- if (ctl ATA_NIEN)
- mask |= 0x80;
- else
- mask = 0x7F;
- writeb(mask, mask_addr);
-}
-
-
static void vsc_sata_tf_load(struct ata_port *ap, const struct ata_taskfile
*tf)
{
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
/*
-* The only thing the ctl register is used for is SRST.
-* That is not enabled or disabled via tf_load.
-* However, if ATA_NIEN is changed, then we need to change the
interrupt register.
+* The relevant bits in the ctl register are HOB, SRST and nIEN.
+* HOB is only used by ata_tf_read().
+* For SRST, soft reset is not the duty of tf_load.
+* For nIEN, explicit -irq_on() and -irq_off are provided.
+* That's why tf-ctl is ignored here.
*/
- if ((tf-ctl ATA_NIEN) != (ap-last_ctl ATA_NIEN)) {
- ap-last_ctl = tf-ctl;
- vsc_intr_mask_update(ap, tf-ctl ATA_NIEN);
- }
+
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
writew
[PATCH 7/7] libata: remove ap-last_ctl
With the previous changes made to tf_load(), ap-last_ctl is now unused. Remove
it.
(The purpose of ap-last_ctl is to cache the last status of nIEN bit
and hopefully avoid writing the Control register unnecessarily.
However, the libata polling code always call irq_on() in ata_hsm_qc_complete()
and such mechanism isn't much utilized.)
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 06_tf_load_cleanup/drivers/ata/libata-core.c
07_last_ctl_cleanup/drivers/ata/libata-core.c
--- 06_tf_load_cleanup/drivers/ata/libata-core.c2007-07-07
10:49:04.0 +0800
+++ 07_last_ctl_cleanup/drivers/ata/libata-core.c 2007-07-07
10:50:36.0 +0800
@@ -5987,7 +5987,6 @@ struct ata_port *ata_port_alloc(struct a
ap-hw_sata_spd_limit = UINT_MAX;
ap-active_tag = ATA_TAG_POISON;
- ap-last_ctl = 0xFF;
#if defined(ATA_VERBOSE_DEBUG)
/* turn on all debugging levels */
diff -Nrup 06_tf_load_cleanup/drivers/ata/libata-sff.c
07_last_ctl_cleanup/drivers/ata/libata-sff.c
--- 06_tf_load_cleanup/drivers/ata/libata-sff.c 2007-07-07 10:50:10.0
+0800
+++ 07_last_ctl_cleanup/drivers/ata/libata-sff.c2007-07-07
10:50:36.0 +0800
@@ -53,7 +53,6 @@ void ata_irq_on(struct ata_port *ap)
struct ata_ioports *ioaddr = ap-ioaddr;
ap-ctl = ~ATA_NIEN;
- ap-last_ctl = ap-ctl;
iowrite8(ap-ctl, ioaddr-ctl_addr);
ata_wait_idle(ap);
@@ -76,7 +75,6 @@ void ata_irq_off(struct ata_port *ap)
struct ata_ioports *ioaddr = ap-ioaddr;
ap-ctl |= ATA_NIEN;
- ap-last_ctl = ap-ctl;
iowrite8(ap-ctl, ioaddr-ctl_addr);
diff -Nrup 06_tf_load_cleanup/drivers/ata/pata_scc.c
07_last_ctl_cleanup/drivers/ata/pata_scc.c
--- 06_tf_load_cleanup/drivers/ata/pata_scc.c 2007-07-07 10:50:10.0
+0800
+++ 07_last_ctl_cleanup/drivers/ata/pata_scc.c 2007-07-07 10:50:36.0
+0800
@@ -794,7 +794,6 @@ static void scc_irq_on (struct ata_port
struct ata_ioports *ioaddr = ap-ioaddr;
ap-ctl = ~ATA_NIEN;
- ap-last_ctl = ap-ctl;
out_be32(ioaddr-ctl_addr, ap-ctl);
ata_wait_idle(ap);
@@ -814,7 +813,6 @@ static void scc_irq_off (struct ata_port
struct ata_ioports *ioaddr = ap-ioaddr;
ap-ctl |= ATA_NIEN;
- ap-last_ctl = ap-ctl;
out_be32(ioaddr-ctl_addr, ap-ctl);
diff -Nrup 06_tf_load_cleanup/include/linux/libata.h
07_last_ctl_cleanup/include/linux/libata.h
--- 06_tf_load_cleanup/include/linux/libata.h 2007-07-07 10:49:04.0
+0800
+++ 07_last_ctl_cleanup/include/linux/libata.h 2007-07-07 10:50:36.0
+0800
@@ -507,7 +507,6 @@ struct ata_port {
struct ata_ioports ioaddr; /* ATA cmd/ctl/dma register blocks */
u8 ctl;/* cache of ATA control register */
- u8 last_ctl; /* Cache last written value */
unsigned intpio_mask;
unsigned intmwdma_mask;
unsigned intudma_mask;
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/10] libata: irq_on/off restructuring
Tejun Heo wrote: I like the whole series. Just a few nits. * 9 and 10 need to be oen patch. As it currently stands, compile will fail after 9. * Please don't include Patch n/10 in message body. * Add freeze/thaw to all, merge, kill freeze/thaw from all sequence is a bit odd. It would be better if we can do things more directly but I haven't thought about how that can be done too hard, so if it's too difficult, it's probably not worth it. Alan, Jeff, I think we'll need to do what IDE has been doing for IRQ masking to get acceptable PIO behavior after all and that will also make us more resistant against deadly IRQ storms on controllers which don't have pending IRQ bits (most SFF ones), and this change will ease our way toward that direction. What do you guys think? Thanks for the review/advice. Revised patches to follow. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [info] What's in Jeff's libata-dev inbox?
Jeff Garzik wrote: I have patches from Alan (pata_sis FIFO whack, pata_dma option), Tejun, Albert and Kristen still to be reviewed. Will get to those on Friday, after the July 4th US holiday.tions(-) Just to be more specific, my to-review inbox contains: Alan: pata_sis FIFO whack, pata_dma option Tejun: PMP patchbomb Albert: irq_on/off restructure, irq-driven PIO to wq, minor PIO fixes Please ignore the irq-driven PIO to wq patchset; it's not ready. For the other two patchsets, I'll rediff and resend for your review. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] ide: pdc202xx_new PLL input clock fix
Sergei Shtylyov wrote: Bartlomiej Zolnierkiewicz wrote: typos fixed manually Signed-off-by: Albert Lee [EMAIL PROTECTED] Except for types in description: Said he, while making his own typo. :-) :) -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] libata: minor pio fixes (resend)
Minor pio fixes: 1/2: move ata_altstatus() to pio data xfer functions 2/2: change the last state of pio read to HSM_ST_IDLE (The previous remove unneeded ata_altstatus() from ata_hsm_qc_complete() has been accepted.) Patch against the libata-dev tree for your review. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] libata: move ata_altstatus() to pio data xfer functions
Patch 1/2:
Move ata_altstatus() out from ata_hsm_move() to the pio data xfer functions
like ata_pio_sectors() and atapi_pio_bytes() where it makes more sense.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
atapi_send_cdb() already calls ata_altstatus() inside.
This patch makes ata_pio_sectors() and atapi_pio_bytes() do the same.
diff -Nrup 00_libata-dev/drivers/ata/libata-core.c
01_move_altstatus/drivers/ata/libata-core.c
--- 00_libata-dev/drivers/ata/libata-core.c 2007-07-04 11:26:30.0
+0800
+++ 01_move_altstatus/drivers/ata/libata-core.c 2007-07-04 11:32:36.0
+0800
@@ -4524,6 +4524,8 @@ static void ata_pio_sectors(struct ata_q
ata_pio_sector(qc);
} else
ata_pio_sector(qc);
+
+ ata_altstatus(ap); /* flush */
}
/**
@@ -4698,6 +4700,7 @@ static void atapi_pio_bytes(struct ata_q
VPRINTK(ata%u: xfering %d bytes\n, ap-print_id, bytes);
__atapi_pio_bytes(qc, bytes);
+ ata_altstatus(ap); /* flush */
return;
@@ -4869,7 +4872,6 @@ fsm_start:
*/
ap-hsm_task_state = HSM_ST;
ata_pio_sectors(qc);
- ata_altstatus(ap); /* flush */
} else
/* send CDB */
atapi_send_cdb(ap, qc);
@@ -4950,7 +4952,6 @@ fsm_start:
if (!(qc-tf.flags ATA_TFLAG_WRITE)) {
ata_pio_sectors(qc);
- ata_altstatus(ap);
status = ata_wait_idle(ap);
}
@@ -4970,13 +4971,11 @@ fsm_start:
if (ap-hsm_task_state == HSM_ST_LAST
(!(qc-tf.flags ATA_TFLAG_WRITE))) {
/* all data read */
- ata_altstatus(ap);
status = ata_wait_idle(ap);
goto fsm_start;
}
}
- ata_altstatus(ap); /* flush */
poll_next = 1;
break;
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/10] libata: irq_on/off restructuring
For ATA, there are two levels of mechanism available to turn irq on/off. - device level: nIEN bit in the control register. This masks INTRQ from the device. - host adapter level: some controller can mask out per-port irq from the host adapter. Currently various parts of libata deal with irq on/off. ex. tf_load() can alter the nIEN bit. ex. irq_on() also alters the device level nIEN bit. ex. freeze()/thaw() deal with the host adapter irq mask. It seems these irq on/off codes could be better structured. Patches against libata-dev tree for your review/advice, thanks. 1/10: remove irq_on from ata_bus_reset() and ata_std_postreset() 2/10: add -irq_off() for symmetry 3/10: implement -irq_off() in LLDDs 4/10: use irq_off from bmdma_freeze() 5/10: use freeze()/thaw() for polling 6/10: add freeze()/thaw() to old EH LLDDs 7/10: pdc_freeze() semantic change 8/10: remove writing of tf-ctl from ata_tf_load() 9/10: integrate freeze()/thaw() with irq_on/off. 10/10: integrate freeze/thaw with irq_on/off in LLDDs - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/10] libata: add irq_off
Patch 2/10:
Currently there is -irq_on but no -irq_off.
Turning irq off is done via altering the nIEN bit of qc-tf, together with
tf_load().
This patch adds -irq_off for symmetry.
tf_load() and ata_qc_set_polling() will be fixed/removed in later patches.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 01_remove_leftover_irqon/drivers/ata/libata-core.c
02_add_irq_off/drivers/ata/libata-core.c
--- 01_remove_leftover_irqon/drivers/ata/libata-core.c 2007-07-04
11:43:30.0 +0800
+++ 02_add_irq_off/drivers/ata/libata-core.c2007-07-04 11:57:33.0
+0800
@@ -6901,6 +6901,8 @@ EXPORT_SYMBOL_GPL(ata_eh_qc_retry);
EXPORT_SYMBOL_GPL(ata_do_eh);
EXPORT_SYMBOL_GPL(ata_irq_on);
EXPORT_SYMBOL_GPL(ata_dummy_irq_on);
+EXPORT_SYMBOL_GPL(ata_irq_off);
+EXPORT_SYMBOL_GPL(ata_dummy_irq_off);
EXPORT_SYMBOL_GPL(ata_irq_ack);
EXPORT_SYMBOL_GPL(ata_dummy_irq_ack);
EXPORT_SYMBOL_GPL(ata_dev_try_classify);
diff -Nrup 01_remove_leftover_irqon/drivers/ata/libata-sff.c
02_add_irq_off/drivers/ata/libata-sff.c
--- 01_remove_leftover_irqon/drivers/ata/libata-sff.c 2007-07-04
11:26:30.0 +0800
+++ 02_add_irq_off/drivers/ata/libata-sff.c 2007-07-04 11:57:33.0
+0800
@@ -67,6 +67,39 @@ u8 ata_irq_on(struct ata_port *ap)
u8 ata_dummy_irq_on (struct ata_port *ap) { return 0; }
/**
+ * ata_irq_off - Disable interrupts on a port.
+ * @ap: Port on which interrupts are disabled.
+ *
+ * Disable interrupts on a legacy IDE device using MMIO or PIO,
+ * wait for idle, clear any pending interrupts.
+ *
+ * LOCKING:
+ * Inherited from caller.
+ */
+u8 ata_irq_off(struct ata_port *ap)
+{
+ struct ata_ioports *ioaddr = ap-ioaddr;
+ u8 tmp;
+
+ ap-ctl |= ATA_NIEN;
+ ap-last_ctl = ap-ctl;
+
+ iowrite8(ap-ctl, ioaddr-ctl_addr);
+
+ /* Under certain circumstances, some controllers raise IRQ on
+* ATA_NIEN manipulation. Also, many controllers fail to mask
+* previously pending IRQ on ATA_NIEN assertion. Clear it.
+*/
+ tmp = ata_wait_idle(ap);
+
+ ap-ops-irq_clear(ap);
+
+ return tmp;
+}
+
+u8 ata_dummy_irq_off (struct ata_port *ap) { return 0; }
+
+/**
* ata_irq_ack - Acknowledge a device interrupt.
* @ap: Port on which interrupts are enabled.
*
diff -Nrup 01_remove_leftover_irqon/drivers/ata/libata.h
02_add_irq_off/drivers/ata/libata.h
--- 01_remove_leftover_irqon/drivers/ata/libata.h 2007-07-04
11:26:30.0 +0800
+++ 02_add_irq_off/drivers/ata/libata.h 2007-07-04 11:57:33.0 +0800
@@ -156,7 +156,5 @@ extern void ata_port_wait_eh(struct ata_
extern void ata_qc_schedule_eh(struct ata_queued_cmd *qc);
/* libata-sff.c */
-extern u8 ata_irq_on(struct ata_port *ap);
-
#endif /* __LIBATA_H__ */
diff -Nrup 01_remove_leftover_irqon/include/linux/libata.h
02_add_irq_off/include/linux/libata.h
--- 01_remove_leftover_irqon/include/linux/libata.h 2007-07-04
11:26:45.0 +0800
+++ 02_add_irq_off/include/linux/libata.h 2007-07-04 11:57:33.0
+0800
@@ -597,6 +597,7 @@ struct ata_port_operations {
irq_handler_t irq_handler;
void (*irq_clear) (struct ata_port *);
u8 (*irq_on) (struct ata_port *);
+ u8 (*irq_off) (struct ata_port *);
u8 (*irq_ack) (struct ata_port *ap, unsigned int chk_drq);
u32 (*scr_read) (struct ata_port *ap, unsigned int sc_reg);
@@ -804,6 +805,8 @@ extern struct ata_device *ata_dev_pair(s
extern int ata_do_set_mode(struct ata_port *ap, struct ata_device
**r_failed_dev);
extern u8 ata_irq_on(struct ata_port *ap);
extern u8 ata_dummy_irq_on(struct ata_port *ap);
+extern u8 ata_irq_off(struct ata_port *ap);
+extern u8 ata_dummy_irq_off(struct ata_port *ap);
extern u8 ata_irq_ack(struct ata_port *ap, unsigned int chk_drq);
extern u8 ata_dummy_irq_ack(struct ata_port *ap, unsigned int chk_drq);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/10] libata: implement -irq_off in LLDDs
Patch 3/10: Implement the newly added -irq_off in LLDDs. Signed-off-by: Albert Lee [EMAIL PROTECTED] --- diff -Nrup 02_add_irq_off/drivers/ata/ahci.c 03_add_irq_off_lldd/drivers/ata/ahci.c --- 02_add_irq_off/drivers/ata/ahci.c 2007-07-04 11:26:30.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/ahci.c 2007-07-04 12:09:29.0 +0800 @@ -268,6 +268,7 @@ static const struct ata_port_operations .irq_clear = ahci_irq_clear, .irq_on = ata_dummy_irq_on, + .irq_off= ata_dummy_irq_off, .irq_ack= ata_dummy_irq_ack, .scr_read = ahci_scr_read, @@ -302,6 +303,7 @@ static const struct ata_port_operations .irq_clear = ahci_irq_clear, .irq_on = ata_dummy_irq_on, + .irq_off= ata_dummy_irq_off, .irq_ack= ata_dummy_irq_ack, .scr_read = ahci_scr_read, diff -Nrup 02_add_irq_off/drivers/ata/ata_generic.c 03_add_irq_off_lldd/drivers/ata/ata_generic.c --- 02_add_irq_off/drivers/ata/ata_generic.c2007-07-04 11:26:30.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/ata_generic.c 2007-07-04 12:09:29.0 +0800 @@ -121,6 +121,7 @@ static struct ata_port_operations generi .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, diff -Nrup 02_add_irq_off/drivers/ata/ata_piix.c 03_add_irq_off_lldd/drivers/ata/ata_piix.c --- 02_add_irq_off/drivers/ata/ata_piix.c 2007-07-04 11:26:30.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/ata_piix.c 2007-07-04 12:09:29.0 +0800 @@ -305,6 +305,7 @@ static const struct ata_port_operations .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -339,6 +340,7 @@ static const struct ata_port_operations .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -369,6 +371,7 @@ static const struct ata_port_operations .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, diff -Nrup 02_add_irq_off/drivers/ata/pata_ali.c 03_add_irq_off_lldd/drivers/ata/pata_ali.c --- 02_add_irq_off/drivers/ata/pata_ali.c 2007-07-04 11:26:30.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/pata_ali.c 2007-07-04 12:09:29.0 +0800 @@ -320,6 +320,7 @@ static struct ata_port_operations ali_ea .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -362,6 +363,7 @@ static struct ata_port_operations ali_20 .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -401,6 +403,7 @@ static struct ata_port_operations ali_c2 .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -439,6 +442,7 @@ static struct ata_port_operations ali_c5 .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, diff -Nrup 02_add_irq_off/drivers/ata/pata_amd.c 03_add_irq_off_lldd/drivers/ata/pata_amd.c --- 02_add_irq_off/drivers/ata/pata_amd.c 2007-07-04 11:26:30.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/pata_amd.c 2007-07-04 12:09:29.0 +0800 @@ -356,6 +356,7 @@ static struct ata_port_operations amd33_ .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start
[PATCH 5/10] libata: use freeze/thaw for polling
Patch 5/10:
This patch changes polling codes to use freeze()/thaw() for irq off/on.
The reason is: some ATAPI devices raises INTRQ even of nIEN = 1.
Using the host adapter irq mask mechanism, if available, is more reliable than
the device nIEN bit.
ata_qc_set_polling() is also removed since now unused.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Considerations:
HSM, the new user of freeze()/thaw(), will call freeze()/thaw() in higher
frequency than EH.
Can current implemention of freeze()/thaw() handle it?
diff -Nrup 04_convert_freeze/drivers/ata/libata-core.c
05_convert_hsm_to_freeze/drivers/ata/libata-core.c
--- 04_convert_freeze/drivers/ata/libata-core.c 2007-07-04 11:57:33.0
+0800
+++ 05_convert_hsm_to_freeze/drivers/ata/libata-core.c 2007-07-04
13:13:56.0 +0800
@@ -4753,7 +4753,7 @@ static void ata_hsm_qc_complete(struct a
qc = ata_qc_from_tag(ap, qc-tag);
if (qc) {
if (likely(!(qc-err_mask AC_ERR_HSM))) {
- ap-ops-irq_on(ap);
+ ap-ops-thaw(ap);
ata_qc_complete(qc);
} else
ata_port_freeze(ap);
@@ -4769,7 +4769,7 @@ static void ata_hsm_qc_complete(struct a
} else {
if (in_wq) {
spin_lock_irqsave(ap-lock, flags);
- ap-ops-irq_on(ap);
+ ap-ops-thaw(ap);
ata_qc_complete(qc);
spin_unlock_irqrestore(ap-lock, flags);
} else
@@ -5411,7 +5411,7 @@ unsigned int ata_qc_issue_prot(struct at
switch (qc-tf.protocol) {
case ATA_PROT_NODATA:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ata_qc_set_polling(qc);
+ ap-ops-freeze(ap);
ata_tf_to_host(ap, qc-tf);
ap-hsm_task_state = HSM_ST_LAST;
@@ -5432,7 +5432,7 @@ unsigned int ata_qc_issue_prot(struct at
case ATA_PROT_PIO:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ata_qc_set_polling(qc);
+ ap-ops-freeze(ap);
ata_tf_to_host(ap, qc-tf);
@@ -5461,7 +5461,7 @@ unsigned int ata_qc_issue_prot(struct at
case ATA_PROT_ATAPI:
case ATA_PROT_ATAPI_NODATA:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ata_qc_set_polling(qc);
+ ap-ops-freeze(ap);
ata_tf_to_host(ap, qc-tf);
diff -Nrup 04_convert_freeze/include/linux/libata.h
05_convert_hsm_to_freeze/include/linux/libata.h
--- 04_convert_freeze/include/linux/libata.h2007-07-04 11:57:33.0
+0800
+++ 05_convert_hsm_to_freeze/include/linux/libata.h 2007-07-04
13:13:56.0 +0800
@@ -1097,11 +1097,6 @@ static inline u8 ata_wait_idle(struct at
return status;
}
-static inline void ata_qc_set_polling(struct ata_queued_cmd *qc)
-{
- qc-tf.ctl |= ATA_NIEN;
-}
-
static inline struct ata_queued_cmd *__ata_qc_from_tag(struct ata_port *ap,
unsigned int tag)
{
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/10] libata: add freeze/thaw to old EH LLDDs
Patch 6/10:
Now that HSM polling code path uses freeze()/thaw() regardless of old EH or new
EH.
Add freeze()/thaw() to old EH LLDDs for the HSM polling code.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 05_convert_hsm_to_freeze/drivers/ata/pata_ixp4xx_cf.c
06_add_freeze_thaw_to_lldd/drivers/ata/pata_ixp4xx_cf.c
--- 05_convert_hsm_to_freeze/drivers/ata/pata_ixp4xx_cf.c 2007-07-04
12:09:29.0 +0800
+++ 06_add_freeze_thaw_to_lldd/drivers/ata/pata_ixp4xx_cf.c 2007-07-04
13:17:13.0 +0800
@@ -131,6 +131,9 @@ static struct ata_port_operations ixp4xx
.data_xfer = ixp4xx_mmio_data_xfer,
.cable_detect = ata_cable_40wire,
+ .freeze = ata_bmdma_freeze,
+ .thaw = ata_bmdma_thaw,
+
.irq_clear = ixp4xx_irq_clear,
.irq_on = ata_irq_on,
.irq_off= ata_irq_off,
diff -Nrup 05_convert_hsm_to_freeze/drivers/ata/pata_scc.c
06_add_freeze_thaw_to_lldd/drivers/ata/pata_scc.c
--- 05_convert_hsm_to_freeze/drivers/ata/pata_scc.c 2007-07-04
13:12:38.0 +0800
+++ 06_add_freeze_thaw_to_lldd/drivers/ata/pata_scc.c 2007-07-04
13:17:13.0 +0800
@@ -879,6 +879,18 @@ static void scc_bmdma_freeze (struct ata
}
/**
+ * scc_bmdma_thaw - Thaw BMDMA controller port
+ * @ap: port to thaw
+ *
+ * Note: Original code is ata_bmdma_thaw().
+ */
+
+static void scc_bmdma_thaw (struct ata_port *ap)
+{
+ scc_irq_on(ap);
+}
+
+/**
* scc_pata_prereset - prepare for reset
* @ap: ATA port to be reset
* @deadline: deadline jiffies for the operation
@@ -1025,6 +1037,7 @@ static const struct ata_port_operations
.qc_issue = ata_qc_issue_prot,
.freeze = scc_bmdma_freeze,
+ .thaw = scc_bmdma_thaw,
.error_handler = scc_error_handler,
.post_internal_cmd = scc_bmdma_stop,
diff -Nrup 05_convert_hsm_to_freeze/drivers/ata/pdc_adma.c
06_add_freeze_thaw_to_lldd/drivers/ata/pdc_adma.c
--- 05_convert_hsm_to_freeze/drivers/ata/pdc_adma.c 2007-07-04
12:09:29.0 +0800
+++ 06_add_freeze_thaw_to_lldd/drivers/ata/pdc_adma.c 2007-07-04
13:17:13.0 +0800
@@ -171,6 +171,8 @@ static const struct ata_port_operations
.qc_issue = adma_qc_issue,
.eng_timeout= adma_eng_timeout,
.data_xfer = ata_data_xfer,
+ .freeze = ata_bmdma_freeze,
+ .thaw = ata_bmdma_thaw,
.irq_clear = adma_irq_clear,
.irq_on = ata_irq_on,
.irq_off= ata_irq_off,
diff -Nrup 05_convert_hsm_to_freeze/drivers/ata/sata_mv.c
06_add_freeze_thaw_to_lldd/drivers/ata/sata_mv.c
--- 05_convert_hsm_to_freeze/drivers/ata/sata_mv.c 2007-07-04
12:09:29.0 +0800
+++ 06_add_freeze_thaw_to_lldd/drivers/ata/sata_mv.c2007-07-04
13:17:13.0 +0800
@@ -453,6 +453,9 @@ static const struct ata_port_operations
.eng_timeout= mv_eng_timeout,
+ .freeze = ata_bmdma_freeze,
+ .thaw = ata_bmdma_thaw,
+
.irq_clear = mv_irq_clear,
.irq_on = ata_irq_on,
.irq_off= ata_irq_off,
@@ -483,6 +486,9 @@ static const struct ata_port_operations
.eng_timeout= mv_eng_timeout,
+ .freeze = ata_bmdma_freeze,
+ .thaw = ata_bmdma_thaw,
+
.irq_clear = mv_irq_clear,
.irq_on = ata_irq_on,
.irq_off= ata_irq_off,
@@ -513,6 +519,9 @@ static const struct ata_port_operations
.eng_timeout= mv_eng_timeout,
+ .freeze = ata_bmdma_freeze,
+ .thaw = ata_bmdma_thaw,
+
.irq_clear = mv_irq_clear,
.irq_on = ata_irq_on,
.irq_off= ata_irq_off,
diff -Nrup 05_convert_hsm_to_freeze/drivers/ata/sata_qstor.c
06_add_freeze_thaw_to_lldd/drivers/ata/sata_qstor.c
--- 05_convert_hsm_to_freeze/drivers/ata/sata_qstor.c 2007-07-04
12:09:29.0 +0800
+++ 06_add_freeze_thaw_to_lldd/drivers/ata/sata_qstor.c 2007-07-04
13:17:13.0 +0800
@@ -157,6 +157,8 @@ static const struct ata_port_operations
.qc_issue = qs_qc_issue,
.data_xfer = ata_data_xfer,
.eng_timeout= qs_eng_timeout,
+ .freeze = ata_bmdma_freeze,
+ .thaw = ata_bmdma_thaw,
.irq_clear = qs_irq_clear,
.irq_on = ata_irq_on,
.irq_off= ata_irq_off,
diff -Nrup 05_convert_hsm_to_freeze/drivers/ata/sata_sx4.c
06_add_freeze_thaw_to_lldd/drivers/ata/sata_sx4.c
--- 05_convert_hsm_to_freeze/drivers/ata/sata_sx4.c 2007
[PATCH 7/10] libata: pdc_freeze() semantic change
Patch 7/10: After checking the current implementations of freeze()/thaw(), it seems only pdc_freeze() do more than simple irq masking. Remove the DMA stop code from pdc_freeze(). Signed-off-by: Albert Lee [EMAIL PROTECTED] --- diff -Nrup 06_add_freeze_thaw_to_lldd/drivers/ata/sata_promise.c 07_sata_promise_freeze/drivers/ata/sata_promise.c --- 06_add_freeze_thaw_to_lldd/drivers/ata/sata_promise.c 2007-07-04 12:09:29.0 +0800 +++ 07_sata_promise_freeze/drivers/ata/sata_promise.c 2007-07-04 13:17:59.0 +0800 @@ -581,7 +581,6 @@ static void pdc_freeze(struct ata_port * tmp = readl(mmio + PDC_CTLSTAT); tmp |= PDC_IRQ_DISABLE; - tmp = ~PDC_DMA_ENABLE; writel(tmp, mmio + PDC_CTLSTAT); readl(mmio + PDC_CTLSTAT); /* flush */ } - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/10] libata: remove writing of tf-ctl from ata_tf_load()
Patch 8/10:
The relevant bits in the ctl register are HOB, SRST and nIEN.
- HOB is only used by ata_tf_read().
- For SRST, soft reset is not the duty of tf_load.
- For nIEN, explicit irq_on()/irq_off and freeze()/thaw() are provided.
Since EH/HSM now call explicit freeze()/thaw() for irq off/on.
Remove the implicit nIEN handling from ata_tf_load().
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 07_sata_promise_freeze/drivers/ata/libata-sff.c
08_tfload_cleanup/drivers/ata/libata-sff.c
--- 07_sata_promise_freeze/drivers/ata/libata-sff.c 2007-07-04
13:12:38.0 +0800
+++ 08_tfload_cleanup/drivers/ata/libata-sff.c 2007-07-04 13:20:30.0
+0800
@@ -153,11 +153,13 @@ void ata_tf_load(struct ata_port *ap, co
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- if (tf-ctl != ap-last_ctl) {
- iowrite8(tf-ctl, ioaddr-ctl_addr);
- ap-last_ctl = tf-ctl;
- ata_wait_idle(ap);
- }
+ /*
+* The relevant bits in the ctl register are HOB, SRST and nIEN.
+* HOB is only used by ata_tf_read().
+* For SRST, soft reset is not the duty of tf_load.
+* For nIEN, explicit -irq_on() and -irq_off are provided.
+* That's why tf-ctl is ignored here.
+*/
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
iowrite8(tf-hob_feature, ioaddr-feature_addr);
diff -Nrup 07_sata_promise_freeze/drivers/ata/pata_scc.c
08_tfload_cleanup/drivers/ata/pata_scc.c
--- 07_sata_promise_freeze/drivers/ata/pata_scc.c 2007-07-04
13:17:13.0 +0800
+++ 08_tfload_cleanup/drivers/ata/pata_scc.c2007-07-04 13:20:30.0
+0800
@@ -271,12 +271,6 @@ static void scc_tf_load (struct ata_port
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- if (tf-ctl != ap-last_ctl) {
- out_be32(ioaddr-ctl_addr, tf-ctl);
- ap-last_ctl = tf-ctl;
- ata_wait_idle(ap);
- }
-
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
out_be32(ioaddr-feature_addr, tf-hob_feature);
out_be32(ioaddr-nsect_addr, tf-hob_nsect);
diff -Nrup 07_sata_promise_freeze/drivers/ata/sata_svw.c
08_tfload_cleanup/drivers/ata/sata_svw.c
--- 07_sata_promise_freeze/drivers/ata/sata_svw.c 2007-07-04
12:09:29.0 +0800
+++ 08_tfload_cleanup/drivers/ata/sata_svw.c2007-07-04 13:20:30.0
+0800
@@ -125,11 +125,6 @@ static void k2_sata_tf_load(struct ata_p
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- if (tf-ctl != ap-last_ctl) {
- writeb(tf-ctl, ioaddr-ctl_addr);
- ap-last_ctl = tf-ctl;
- ata_wait_idle(ap);
- }
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
writew(tf-feature | (((u16)tf-hob_feature) 8),
ioaddr-feature_addr);
diff -Nrup 07_sata_promise_freeze/drivers/ata/sata_vsc.c
08_tfload_cleanup/drivers/ata/sata_vsc.c
--- 07_sata_promise_freeze/drivers/ata/sata_vsc.c 2007-07-04
12:09:29.0 +0800
+++ 08_tfload_cleanup/drivers/ata/sata_vsc.c2007-07-04 13:20:30.0
+0800
@@ -137,36 +137,11 @@ static void vsc_thaw(struct ata_port *ap
}
-static void vsc_intr_mask_update(struct ata_port *ap, u8 ctl)
-{
- void __iomem *mask_addr;
- u8 mask;
-
- mask_addr = ap-host-iomap[VSC_MMIO_BAR] +
- VSC_SATA_INT_MASK_OFFSET + ap-port_no;
- mask = readb(mask_addr);
- if (ctl ATA_NIEN)
- mask |= 0x80;
- else
- mask = 0x7F;
- writeb(mask, mask_addr);
-}
-
-
static void vsc_sata_tf_load(struct ata_port *ap, const struct ata_taskfile
*tf)
{
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- /*
-* The only thing the ctl register is used for is SRST.
-* That is not enabled or disabled via tf_load.
-* However, if ATA_NIEN is changed, then we need to change the
interrupt register.
-*/
- if ((tf-ctl ATA_NIEN) != (ap-last_ctl ATA_NIEN)) {
- ap-last_ctl = tf-ctl;
- vsc_intr_mask_update(ap, tf-ctl ATA_NIEN);
- }
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
writew(tf-feature | (((u16)tf-hob_feature) 8),
ioaddr-feature_addr);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/10] libata: Integrate freeze/thaw with irq_on/off
Patch 9/10:
irq_on/irq_off are now only wrapped by freeze/thaw (and unused otherwise).
We can integrate freeze/thaw with irq_on/irq_off.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
This is for libata-core. The LLDDs will be fixed in the next patch.
diff -Nrup 08_tfload_cleanup/drivers/ata/libata-core.c
09_integrate_irq_on_off/drivers/ata/libata-core.c
--- 08_tfload_cleanup/drivers/ata/libata-core.c 2007-07-04 13:13:56.0
+0800
+++ 09_integrate_irq_on_off/drivers/ata/libata-core.c 2007-07-04
13:35:05.0 +0800
@@ -4753,7 +4753,7 @@ static void ata_hsm_qc_complete(struct a
qc = ata_qc_from_tag(ap, qc-tag);
if (qc) {
if (likely(!(qc-err_mask AC_ERR_HSM))) {
- ap-ops-thaw(ap);
+ ap-ops-irq_on(ap);
ata_qc_complete(qc);
} else
ata_port_freeze(ap);
@@ -4769,7 +4769,7 @@ static void ata_hsm_qc_complete(struct a
} else {
if (in_wq) {
spin_lock_irqsave(ap-lock, flags);
- ap-ops-thaw(ap);
+ ap-ops-irq_on(ap);
ata_qc_complete(qc);
spin_unlock_irqrestore(ap-lock, flags);
} else
@@ -5411,7 +5411,7 @@ unsigned int ata_qc_issue_prot(struct at
switch (qc-tf.protocol) {
case ATA_PROT_NODATA:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ap-ops-freeze(ap);
+ ap-ops-irq_off(ap);
ata_tf_to_host(ap, qc-tf);
ap-hsm_task_state = HSM_ST_LAST;
@@ -5432,7 +5432,7 @@ unsigned int ata_qc_issue_prot(struct at
case ATA_PROT_PIO:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ap-ops-freeze(ap);
+ ap-ops-irq_off(ap);
ata_tf_to_host(ap, qc-tf);
@@ -5461,7 +5461,7 @@ unsigned int ata_qc_issue_prot(struct at
case ATA_PROT_ATAPI:
case ATA_PROT_ATAPI_NODATA:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ap-ops-freeze(ap);
+ ap-ops-irq_off(ap);
ata_tf_to_host(ap, qc-tf);
@@ -6758,8 +6758,6 @@ const struct ata_port_operations ata_dum
.dev_select = ata_noop_dev_select,
.qc_prep= ata_noop_qc_prep,
.qc_issue = ata_dummy_qc_issue,
- .freeze = ata_dummy_noret,
- .thaw = ata_dummy_noret,
.error_handler = ata_dummy_noret,
.post_internal_cmd = ata_dummy_qc_noret,
.irq_clear = ata_dummy_noret,
@@ -6821,8 +6819,6 @@ EXPORT_SYMBOL_GPL(ata_bmdma_start);
EXPORT_SYMBOL_GPL(ata_bmdma_irq_clear);
EXPORT_SYMBOL_GPL(ata_bmdma_status);
EXPORT_SYMBOL_GPL(ata_bmdma_stop);
-EXPORT_SYMBOL_GPL(ata_bmdma_freeze);
-EXPORT_SYMBOL_GPL(ata_bmdma_thaw);
EXPORT_SYMBOL_GPL(ata_bmdma_drive_eh);
EXPORT_SYMBOL_GPL(ata_bmdma_error_handler);
EXPORT_SYMBOL_GPL(ata_bmdma_post_internal_cmd);
diff -Nrup 08_tfload_cleanup/drivers/ata/libata-eh.c
09_integrate_irq_on_off/drivers/ata/libata-eh.c
--- 08_tfload_cleanup/drivers/ata/libata-eh.c 2007-07-04 11:26:30.0
+0800
+++ 09_integrate_irq_on_off/drivers/ata/libata-eh.c 2007-07-04
13:36:27.0 +0800
@@ -617,8 +617,7 @@ static void __ata_port_freeze(struct ata
{
WARN_ON(!ap-ops-error_handler);
- if (ap-ops-freeze)
- ap-ops-freeze(ap);
+ ap-ops-irq_off(ap);
ap-pflags |= ATA_PFLAG_FROZEN;
@@ -690,8 +689,7 @@ void ata_eh_thaw_port(struct ata_port *a
ap-pflags = ~ATA_PFLAG_FROZEN;
- if (ap-ops-thaw)
- ap-ops-thaw(ap);
+ ap-ops-irq_on(ap);
spin_unlock_irqrestore(ap-lock, flags);
diff -Nrup 08_tfload_cleanup/drivers/ata/libata-sff.c
09_integrate_irq_on_off/drivers/ata/libata-sff.c
--- 08_tfload_cleanup/drivers/ata/libata-sff.c 2007-07-04 13:20:30.0
+0800
+++ 09_integrate_irq_on_off/drivers/ata/libata-sff.c2007-07-04
13:57:21.0 +0800
@@ -48,23 +48,20 @@
* LOCKING:
* Inherited from caller.
*/
-u8 ata_irq_on(struct ata_port *ap)
+void ata_irq_on(struct ata_port *ap)
{
struct ata_ioports *ioaddr = ap-ioaddr;
- u8 tmp;
ap-ctl = ~ATA_NIEN;
ap-last_ctl = ap-ctl;
iowrite8(ap-ctl, ioaddr-ctl_addr);
- tmp = ata_wait_idle(ap);
+ ata_wait_idle(ap);
ap-ops-irq_clear(ap);
-
- return tmp;
}
-u8 ata_dummy_irq_on (struct ata_port *ap) { return 0; }
+void ata_dummy_irq_on (struct ata_port *ap){ }
/**
* ata_irq_off - Disable interrupts on a port.
@@ -76,10 +73,9 @@ u8 ata_dummy_irq_on (struct ata_port *ap
* LOCKING:
* Inherited
Re: CF flash PATA on libata failure to attach
Alan Cox wrote: On Fri, Jun 29, 2007 at 05:34:36PM +1000, Andrew Hall wrote: Further to this the PATA to SATA bridge being used in this case is: http://www.jmicron.com/JM20330.html ..as you will see only PIO and UDMA modes are supported. In which case their microcontroller in the middle should be masking ident bits. Without a way to detect the presence of the device I don't see an easy way for us to handle it automatically at all I didn't test whether MWDMA2 is supported by the JMicron bridge or not. But I'm sure that the JMicron bridge doesn't mangle the IDENTIFY to indicat anything about MWDMA unsupported: (Previous test report: http://www.spinics.net/lists/linux-ide/msg01514.html) The Acard ARC-770 bridge from another vendor does mangle the IDENTIFY data and indicates MWDMA unsupported as Alan advised. Andrew, maybe you could test if such bridge works with your CF device. (http://www.acard.com/english/fb0101.jsp?type1_idno=1type2_idno=17) -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] ide: pdc202xx_new PLL input clock fix
Recently the PLL input clock of Promise 2027x is sometimes detected
higer than expected (e.g. 20.027 MHz compared to 16.714 MHz).
It seems sometimes the mdelay() function is not as precise as it
used to be. Per Alan's advice, HT or power management might affect
the precision of mdelay().
This patch calls gettimeofday() to mesure the time elapsed and
calculate the PLL input clock accordingly.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
Cc: Alan Cox [EMAIL PROTECTED]
---
(I have the Promise adapter at hand, so it's easier for me to verify.)
Attached please find the patch and the test result on my box for your review:
[ 72.186805] PDC20275: chipset revision 1
[ 72.196768] start[1039788039] end[1039620652] pll[16804952]
[ 72.196819] PDC20275: PLL input clock is 16804 kHz
[ 72.226586] PDC20275: 100% native mode on irq 10
Maybe Bahadir could also help to verify if it helps to his hda lost interrupt
problem.
--- linux-2.6.22-rc7/drivers/ide/pci/pdc202xx_new.c.ori 2007-07-02
14:40:08.0 +0800
+++ linux-2.6.22-rc7/drivers/ide/pci/pdc202xx_new.c 2007-07-03
13:06:40.0 +0800
@@ -306,11 +306,13 @@ static long __devinit read_counter(u32 d
*/
static long __devinit detect_pll_input_clock(unsigned long dma_base)
{
+ struct timeval start_time, end_time;
long start_count, end_count;
- long pll_input;
+ long pll_input, usec_elapsed;
u8 scr1;
start_count = read_counter(dma_base);
+ do_gettimeofday(start_time);
/* Start the test mode */
outb(0x01, dma_base + 0x01);
@@ -322,6 +324,7 @@ static long __devinit detect_pll_input_c
mdelay(10);
end_count = read_counter(dma_base);
+ do_gettimeofday(end_time);
/* Stop the test mode */
outb(0x01, dma_base + 0x01);
@@ -333,7 +336,10 @@ static long __devinit detect_pll_input_c
* Calculate the input clock in Hz
* (the clock counter is 30 bit wide and counts down)
*/
- pll_input = ((start_count - end_count) 0x3ff) * 100;
+ usec_elapsed = (end_time.tv_sec - start_time.tv_sec) * 100 +
+ (end_time.tv_usec - start_time.tv_usec);
+ pll_input = ((start_count - end_count) 0x3ff) / 10 *
+ (1000 / usec_elapsed);
DBG(start[%ld] end[%ld]\n, start_count, end_count);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2.6.22-rc6] libata: remove reading alt_status from ata_hsm_qc_complete()
In ata_hsm_qc_complete(): Calling ata_altstatus() after the qc is completed might race with next qc. Remove it. Signed-off-by: Albert Lee [EMAIL PROTECTED] --- Jeff, (Sorry for re-submitting this patch so late.) The unneeded reading of alt_status might cause trouble when another command doing activity like pio data xfer. After checking, the flush here is leftover/merge damage of ata_pio_block( ) in 2.6.17-rc5: http://git.kernel.org/?p=linux/kernel/git/jgarzik/libata-dev.git;a=commitdiff;h=bb31a8faa270beafcc51a65880c5564c6b718bd6 We can safely remove it. --- linux-2.6.22-rc6/drivers/ata/libata-core.c.ori 2007-06-28 09:47:31.0 +0800 +++ linux-2.6.22-rc6/drivers/ata/libata-core.c 2007-06-28 09:49:26.0 +0800 @@ -4799,8 +4799,6 @@ static void ata_hsm_qc_complete(struct a } else ata_qc_complete(qc); } - - ata_altstatus(ap); /* flush */ } /** - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata and legacy ide pcmcia failure
Robert de Rooy wrote: Albert Lee wrote: Mark Lord wrote: ... Mmm.. I don't know about the first failure there, but after that it gets into the stuck DRQ state which libata makes no attempt to handle at present. It seems the pata_pcmcia driver is using IRQ driven PIO. Maybe Robert could try the following pio_polling patch first. I did not get the chance to try Marks patch, but the pio_polling patch from Albert works!! The patch just workarounds the lost irq problem by polling; not real fix. We still need to find out why irq is lost per Mark's comment: This proves that the device does work correctly in most respects except for interrupt delivery. The status bits are working and it can be probed for, configured, and used. So, next step might be to try and understand the interrupt mis-delivery problem some more. I've lost the history of the original issue, but we now know that everything except the actual interrupt seems good. I am not familiar with the PCMCIA interrupt delivery. It seems the Dazzle 4in1 Card Adapter works under windows but somehow lost irq under both IDE and libata. Maybe Alan/Bart or the PCMCIA developers know better... -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ide/dma not working from 2.6.19 to 2.6.21
Alan Cox wrote: Reloading the pata_pdc2027x module then the problem goes away. I guess maybe somehow mdelay() is not as precise as before... It's not the most accurate of things once power management and the like get invovled. HT doesn't help it either. You should have get much more accurate timing information if you query gettimeofday before/after and do a few loops. Thanks for the advice. Patch to follow. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] libata: pata_pdc2027x PLL input clock fix
Recently the PLL input clock of pata_pdc2027x is sometimes detected higer than expected (e.g. 20.027 MHz compared to 16.714 MHz). It seems sometimes the mdelay() function is not as precise as it used to be. Per Alan's advice, HT or power management might affect the precision of mdelay(). This patch calls gettimeofday() to mesure the time elapsed and calculate the PLL input clock accordingly. Signed-off-by: Albert Lee [EMAIL PROTECTED] Cc: Alan Cox [EMAIL PROTECTED] --- Did more test. For mdelay(100) the usec_elapsed is usually 99287. However, sometimes the usec_elapsed is 118934, longer than expected. Jun 26 12:12:29 p4ht-s kernel: [ 9156.490991] ACPI: PCI Interrupt :02:05.0[A] - Link [LNK1] - GSI 10 (level, low) - IRQ 10 Jun 26 12:12:29 p4ht-s kernel: [ 9156.610175] usec_elapsed[118934] Jun 26 12:12:29 p4ht-s kernel: [ 9156.610511] pata_pdc2027x :02:05.0: PLL input clock 16817 kHz After the patch, the PLL input clock detected looks more accurate. For your review, thanks. diff -Nrup 00_libata-dev/drivers/ata/pata_pdc2027x.c 01_gettimeofday/drivers/ata/pata_pdc2027x.c --- 00_libata-dev/drivers/ata/pata_pdc2027x.c 2007-06-01 12:08:21.0 +0800 +++ 01_gettimeofday/drivers/ata/pata_pdc2027x.c 2007-06-26 13:08:34.0 +0800 @@ -689,10 +689,12 @@ static long pdc_detect_pll_input_clock(s void __iomem *mmio_base = host-iomap[PDC_MMIO_BAR]; u32 scr; long start_count, end_count; - long pll_clock; + struct timeval start_time, end_time; + long pll_clock, usec_elapsed; /* Read current counter value */ start_count = pdc_read_counter(host); + do_gettimeofday(start_time); /* Start the test mode */ scr = readl(mmio_base + PDC_SYS_CTL); @@ -705,6 +707,7 @@ static long pdc_detect_pll_input_clock(s /* Read the counter values again */ end_count = pdc_read_counter(host); + do_gettimeofday(end_time); /* Stop the test mode */ scr = readl(mmio_base + PDC_SYS_CTL); @@ -713,7 +716,11 @@ static long pdc_detect_pll_input_clock(s readl(mmio_base + PDC_SYS_CTL); /* flush */ /* calculate the input clock in Hz */ - pll_clock = (start_count - end_count) * 10; + usec_elapsed = (end_time.tv_sec - start_time.tv_sec) * 100 + + (end_time.tv_usec - start_time.tv_usec); + + pll_clock = (start_count - end_count) / 100 * + (1 / usec_elapsed); PDPRINTK(start[%ld] end[%ld] \n, start_count, end_count); PDPRINTK(PLL input clock[%ld]Hz\n, pll_clock); - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: libata and legacy ide pcmcia failure
Mark Lord wrote: Robert de Rooy wrote: I did another try with libata pcmcia support using 2.6.22-rc5 which already includes the nodata polling fix, in combination with disable-dev_init_param-and-setxfermode-for-CFA.patch and the timing-debug.patch ... Jun 22 13:19:44 localhost kernel: ata3.00: issuing IDENTIFY Jun 22 13:19:45 localhost kernel: ata3.00: IDENTIFY complete Jun 22 13:19:45 localhost kernel: ata3.00: CFA: Memory Card Adapter, 20011212, max PIO1 Jun 22 13:19:45 localhost kernel: ata3.00: 253696 sectors, multi 0: LBA Jun 22 13:19:45 localhost kernel: ata3.00: issuing IDENTIFY Jun 22 13:19:45 localhost kernel: ata3.00: IDENTIFY complete Jun 22 13:19:45 localhost kernel: ata3.00: configured for PIO0 Jun 22 13:19:45 localhost kernel: ata3: EH complete Jun 22 13:19:45 localhost kernel: scsi 2:0:0:0: Direct-Access ATA Memory Card Adap 2001 PQ: 0 ANSI: 5 Jun 22 13:19:45 localhost kernel: sd 2:0:0:0: [sdb] 253696 512-byte hardware sectors (130 MB) Jun 22 13:19:45 localhost kernel: sd 2:0:0:0: [sdb] Write Protect is off Jun 22 13:19:45 localhost kernel: sd 2:0:0:0: [sdb] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA Jun 22 13:19:45 localhost kernel: sd 2:0:0:0: [sdb] 253696 512-byte hardware sectors (130 MB) Jun 22 13:19:45 localhost kernel: sd 2:0:0:0: [sdb] Write Protect is off Jun 22 13:19:45 localhost kernel: sd 2:0:0:0: [sdb] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA Jun 22 13:20:15 localhost kernel: sdb:3ata3.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen Jun 22 13:20:15 localhost kernel: ata3.00: cmd 20/00:08:00:00:00/00:00:00:00:00/e0 tag 0 cdb 0x0 data 4096 in Jun 22 13:20:15 localhost kernel: res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) Jun 22 13:20:15 localhost kernel: ata3: soft resetting port Jun 22 13:20:15 localhost kernel: ata3: reset complete Jun 22 13:20:15 localhost kernel: ATA: abnormal status 0x80 on port 0x00014107 Jun 22 13:20:15 localhost kernel: ATA: abnormal status 0x80 on port 0x00014107 Jun 22 13:20:15 localhost kernel: ATA: abnormal status 0x58 on port 0x00014107 Jun 22 13:20:15 localhost kernel: ata3.00: issuing IDENTIFY Jun 22 13:20:15 localhost kernel: ATA: abnormal status 0x58 on port 0x00014107 ... Mmm.. I don't know about the first failure there, but after that it gets into the stuck DRQ state which libata makes no attempt to handle at present. It seems the pata_pcmcia driver is using IRQ driven PIO. Maybe Robert could try the following pio_polling patch first. -- albert --- --- libata-dev/drivers/ata/pata_pcmcia.c~ 2007-06-12 16:44:43.0 +0800 +++ libata-dev/drivers/ata/pata_pcmcia.c2007-06-25 11:53:37.0 +0800 @@ -299,7 +299,7 @@ next_entry: ap-ops = pcmcia_port_ops; ap-pio_mask = 1; /* ISA so PIO 0 cycles */ - ap-flags |= ATA_FLAG_SLAVE_POSS; + ap-flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_PIO_POLLING; ap-ioaddr.cmd_addr = io_addr; ap-ioaddr.altstatus_addr = ctl_addr; ap-ioaddr.ctl_addr = ctl_addr; - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 2.6.22-rc5 libata/ata_piix failure with older CDROM
Mikael Pettersson wrote: I tried (again) to convert an Intel i815EP-chipset machine from IDE to libata, but libata still fails to initialise the CDROM on the machine. With 2.6.22-rc5, IDE says: Uniform Multi-Platform E-IDE driver Revision: 7.00alpha2 ide: Assuming 33MHz system bus speed for PIO modes; override with idebus=xx ICH2: IDE controller at PCI slot :00:1f.1 ICH2: chipset revision 5 ICH2: not 100% native mode: will probe irqs later ide0: BM-DMA at 0xf000-0xf007, BIOS settings: hda:DMA, hdb:pio ide1: BM-DMA at 0xf008-0xf00f, BIOS settings: hdc:DMA, hdd:DMA Probing IDE interface ide0... hda: IC35L080AVVA07-0, ATA DISK drive hda: selected mode 0x45 ide0 at 0x1f0-0x1f7,0x3f6 on irq 14 Probing IDE interface ide1... hdc: CRD-8320B, ATAPI CD/DVD-ROM drive hdd: Hewlett-Packard CD-Writer Plus 9100, ATAPI CD/DVD-ROM drive hdc: selected mode 0x22 hdd: selected mode 0x42 ide1 at 0x170-0x177,0x376 on irq 15 hda: max request size: 128KiB hda: 160836480 sectors (82348 MB) w/1863KiB Cache, CHS=65535/16/63, UDMA(100) hda: cache flushes supported hda: hda1 hda2 hda3 hda4 hda5 hda6 Switching to libata (ata_piix) results in: ata_piix :00:1f.1: version 2.11 PCI: Setting latency timer of device :00:1f.1 to 64 scsi0 : ata_piix scsi1 : ata_piix ata1: PATA max UDMA/100 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x0001f000 irq 14 ata2: PATA max UDMA/100 cmd 0x00010170 ctl 0x00010376 bmdma 0x0001f008 irq 15 ata1.00: ata_hpa_resize 1: sectors = 160836480, hpa_sectors = 160836480 ata1.00: ATA-5: IC35L080AVVA07-0, VA4OA52A, max UDMA/100 ata1.00: 160836480 sectors, multi 16: LBA ata1.00: ata_hpa_resize 1: sectors = 160836480, hpa_sectors = 160836480 ata1.00: configured for UDMA/100 ata2.00: ATAPI: CRD-8320B, 1.12, max MWDMA2 ata2.01: ATAPI: Hewlett-Packard CD-Writer Plus 9100, 1.0c, max UDMA/33 ata2.00: configured for MWDMA2 ata2.01: configured for UDMA/33 scsi 0:0:0:0: Direct-Access ATA IC35L080AVVA07-0 VA4O PQ: 0 ANSI: 5 sd 0:0:0:0: [sda] 160836480 512-byte hardware sectors (82348 MB) sd 0:0:0:0: [sda] Write Protect is off sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00 sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sd 0:0:0:0: [sda] 160836480 512-byte hardware sectors (82348 MB) sd 0:0:0:0: [sda] Write Protect is off sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00 sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA sda: sda1 sda2 sda3 sda4 sda5 sda6 sd 0:0:0:0: [sda] Attached SCSI disk ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata2.00: cmd a0/01:00:00:00:00/00:00:00:00:00/a0 tag 0 cdb 0x12 data 36 in res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) ata2: port is slow to respond, please be patient (Status 0xd0) ata2: device not ready (errno=-16), forcing hardreset ata2: BUG: prereset() requested invalid reset type ata2: soft resetting port ATA: abnormal status 0x80 on port 0x00010177 ATA: abnormal status 0x80 on port 0x00010177 ATA: abnormal status 0x80 on port 0x00010177 The INQUIRY timeout looks like the ATAPI DMA problem that Tejun is working on. Could you please check if Tejun's patch that limits ATAPI DMA to multiple of 16-bytes works: https://bugzilla.novell.com/attachment.cgi?id=147389 (The original bug is: https://bugzilla.novell.com/show_bug.cgi?id=229260#c35) -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ide/dma not working from 2.6.19 to 2.6.21
Sergei Shtylyov wrote: Hello. Bahadir Balban wrote: I have a PCI Promise TX2 Ultra133 controller with a harddisk on an ARM platform (which is a barebone system with no BIOS). This setup used to work with the old linux-ide drivers on 2.6.19 You were very lucky then bacause actually it usually failed on anything non-x86. ;-) but it does not work with 2.6.22-rc4, or 2.6.21. Here's the error output: I guess this might be related to the driver update in 2.6.20-rc1 then -- I made the driver calibrate the chip's PLL, so that it might work for non-x86 case too (where Promise BIOS isn't available to do this). The code is basically the same as in the libata's pata_2027x driver... PDC20269: chipset revision 2 6PDC20269: ROM enabled at 0xa021 PDC20269: ROM enabled at 0xa021 Hm, why all the messages are printed twice? :-O (I'll cut out the dupes to save bandwidth). PDC20269: PLL input clock is 37736 kHz 37.736 MHz is a pretty strange PLL clock... Basically, it's the halved PCI clock, and that value signifies your PCI is probably running at 75 MHz -- which is serious overclocking... :-/ That could explain why the driver managed to work before -- the default DPLL settings fit for 66 MHz PCI but made the UltraDMA fail with CRC errors on the plain 33 MHz PCI, IIRC... I may suggest replacing #undef DEBUG by #define DEBUG in this driver, rebuilding the kernel and posting the driver's output... I don't know whether this is related or not. Recently I noticed that the PLL clock being detected higher than expected as 20MHz on my box. (It used be 16.6 MHz since my PCI bus is running at 33MHz...) Reloading the pata_pdc2027x module then the problem goes away. I guess maybe somehow mdelay() is not as precise as before... -- albert (dmesg attached for reference) The PLL clock is wrongly detected as 20MHz on my box, kernel 2.6.22-rc3. [ 5545.255266] pata_pdc2027x :02:05.0: version 0.9 [ 5545.255489] ACPI: PCI Interrupt :02:05.0[A] - Link [LNK1] - GSI 10 (level, low) - IRQ 10 [ 5545.374703] pata_pdc2027x :02:05.0: PLL input clock 20027 kHz [ 5545.404872] scsi2 : pata_pdc2027x [ 5545.430357] scsi3 : pata_pdc2027x [ 5545.48] ata3: PATA max UDMA/133 cmd 0xe09897c0 ctl 0xe0989fda bmdma 0xe0989000 irq 0 [ 5545.433671] ata4: PATA max UDMA/133 cmd 0xe09895c0 ctl 0xe0989dda bmdma 0xe0989008 irq 0 [ 5545.916581] ata3.00: ATAPI, max UDMA/33 [ 5545.916590] ata3.01: ATAPI, max UDMA/33 [ 5546.080361] ata3.00: configured for UDMA/33 [ 5546.244197] ata3.01: configured for UDMA/33 [ 5546.410493] scsi 2:0:0:0: CD-ROMLITE-ON CD-RW SOHR-5238S 4S07 PQ: 0 ANSI: 5 [ 5546.437658] scsi 2:0:1:0: CD-ROMHL-DT-ST DVDRAM GSA-4163B A105 PQ: 0 ANSI: 5 [ 5546.604481] sr0: scsi3-mmc drive: 52x/52x writer cd/rw xa/form2 cdda tray [ 5546.604808] Uniform CD-ROM driver Revision: 3.20 [ 5546.607596] sr 2:0:0:0: Attached scsi CD-ROM sr0 [ 5546.612168] sr1: scsi3-mmc drive: 40x/40x writer dvd-ram cd/rw xa/form2 cdda tray [ 5546.614075] sr 2:0:1:0: Attached scsi CD-ROM sr1 [ 5640.334685] ata3.00: disabled [ 5640.334695] ata3.01: disabled [ 5640.403451] ACPI: PCI interrupt for device :02:05.0 disabled == The PLL clock clock is correct after rmmod and reload the module: [ 5644.153643] pata_pdc2027x :02:05.0: version 0.9 [ 5644.153723] ACPI: PCI Interrupt :02:05.0[A] - Link [LNK1] - GSI 10 (level, low) - IRQ 10 [ 5644.252954] pata_pdc2027x :02:05.0: PLL input clock 16714 kHz [ 5644.303470] scsi4 : pata_pdc2027x [ 5644.319285] scsi5 : pata_pdc2027x [ 5644.321207] ata5: PATA max UDMA/133 cmd 0xe09897c0 ctl 0xe0989fda bmdma 0xe0989000 irq 0 [ 5644.321215] ata6: PATA max UDMA/133 cmd 0xe09895c0 ctl 0xe0989dda bmdma 0xe0989008 irq 0 [ 5644.803308] ata5.00: ATAPI, max UDMA/33 [ 5644.803629] ata5.01: ATAPI, max UDMA/33 [ 5644.967144] ata5.00: configured for UDMA/33 [ 5645.130975] ata5.01: configured for UDMA/33 [ 5645.287539] scsi 4:0:0:0: CD-ROMLITE-ON CD-RW SOHR-5238S 4S07 PQ: 0 ANSI: 5 [ 5645.291669] sr0: scsi3-mmc drive: 52x/52x writer cd/rw xa/form2 cdda tray [ 5645.294295] sr 4:0:0:0: Attached scsi CD-ROM sr0 [ 5645.300189] scsi 4:0:1:0: CD-ROMHL-DT-ST DVDRAM GSA-4163B A105 PQ: 0 ANSI: 5 [ 5645.305716] sr1: scsi3-mmc drive: 40x/40x writer dvd-ram cd/rw xa/form2 cdda tray [ 5645.306736] sr 4:0:1:0: Attached scsi CD-ROM sr1 [EMAIL PROTECTED] june_2007]$ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 0/9] libata: irq_on/off restructuring
For ATA, there are two levels of mechanism available to turn irq on/off. - device level: nIEN bit in the control register. nIEN masks INTRQ from the device. - host adapter level: some controller can mask out per-port irq from the host adapter. Currently various parts of libata deal with irq on/off. ex. tf_load() can alter the nIEN bit. ex. irq_on() also alters the device level nIEN bit. ex. freeze()/thaw() deal with the host adapter irq mask. It seems these irq on/off codes could be better structured. Draft patches for your review/advice, thanks. 1/9: remove irq_on from ata_bus_reset() and ata_std_postreset() 2/9: add -irq_off() for symmetry 3/9: implement -irq_off() in LLDDs 4/9: call irq_off from bmdma_freeze() 5/9: use freeze()/thaw() for polling 6/9: add freeze()/thaw() to old EH LLDDs 7/9: pdc_freeze() semantic change 8/9: remove writing of tf-ctl from ata_tf_load() 9/9: Remove irq_on/off. Rename freeze()/thaw() to irq_on/off. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 1/9] libata: remove irq_on from ata_bus_reset() and ata_std_postreset()
Patch 1/9:
It looks the calling of irq_on() in ata_bus_reset() and ata_std_postreset()
are leftover of the earlier EDD reset. Remove them.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 00_libata-dev/drivers/ata/libata-core.c
01_remove_leftover_irqon/drivers/ata/libata-core.c
--- 00_libata-dev/drivers/ata/libata-core.c 2007-06-01 12:08:21.0
+0800
+++ 01_remove_leftover_irqon/drivers/ata/libata-core.c 2007-06-11
17:31:53.0 +0800
@@ -3190,9 +3190,6 @@ void ata_bus_reset(struct ata_port *ap)
if ((slave_possible) (err != 0x81))
ap-device[1].class = ata_dev_try_classify(ap, 1, err);
- /* re-enable interrupts */
- ap-ops-irq_on(ap);
-
/* is double-select really necessary? */
if (ap-device[1].class != ATA_DEV_NONE)
ap-ops-dev_select(ap, 1);
@@ -3577,10 +3574,6 @@ void ata_std_postreset(struct ata_port *
if (sata_scr_read(ap, SCR_ERROR, serror) == 0)
sata_scr_write(ap, SCR_ERROR, serror);
- /* re-enable interrupts */
- if (!ap-ops-error_handler)
- ap-ops-irq_on(ap);
-
/* is double-select really necessary? */
if (classes[0] != ATA_DEV_NONE)
ap-ops-dev_select(ap, 1);
diff -Nrup 00_libata-dev/drivers/ata/pata_scc.c
01_remove_leftover_irqon/drivers/ata/pata_scc.c
--- 00_libata-dev/drivers/ata/pata_scc.c2007-06-01 12:08:21.0
+0800
+++ 01_remove_leftover_irqon/drivers/ata/pata_scc.c 2007-06-11
17:32:07.0 +0800
@@ -892,10 +892,6 @@ static void scc_std_postreset (struct at
{
DPRINTK(ENTER\n);
- /* re-enable interrupts */
- if (!ap-ops-error_handler)
- ap-ops-irq_on(ap);
-
/* is double-select really necessary? */
if (classes[0] != ATA_DEV_NONE)
ap-ops-dev_select(ap, 1);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 2/9] libata: add irq_off() for symmetry
Patch 2/9:
Currently there is irq_on() but no irq_off().
Turning irq off is done via altering the nIEN bit of qc-tf, together with
tf_load().
This patch adds irq_off() for symmetry.
tf_load() and ata_qc_set_polling() will be fixed/removed in later patches.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 01_remove_leftover_irqon/drivers/ata/libata-core.c
02_add_irq_off/drivers/ata/libata-core.c
--- 01_remove_leftover_irqon/drivers/ata/libata-core.c 2007-06-11
17:31:53.0 +0800
+++ 02_add_irq_off/drivers/ata/libata-core.c2007-06-12 13:20:05.0
+0800
@@ -6906,6 +6906,8 @@ EXPORT_SYMBOL_GPL(ata_eh_qc_retry);
EXPORT_SYMBOL_GPL(ata_do_eh);
EXPORT_SYMBOL_GPL(ata_irq_on);
EXPORT_SYMBOL_GPL(ata_dummy_irq_on);
+EXPORT_SYMBOL_GPL(ata_irq_off);
+EXPORT_SYMBOL_GPL(ata_dummy_irq_off);
EXPORT_SYMBOL_GPL(ata_irq_ack);
EXPORT_SYMBOL_GPL(ata_dummy_irq_ack);
EXPORT_SYMBOL_GPL(ata_dev_try_classify);
diff -Nrup 01_remove_leftover_irqon/drivers/ata/libata-sff.c
02_add_irq_off/drivers/ata/libata-sff.c
--- 01_remove_leftover_irqon/drivers/ata/libata-sff.c 2007-06-01
12:08:21.0 +0800
+++ 02_add_irq_off/drivers/ata/libata-sff.c 2007-06-12 13:19:27.0
+0800
@@ -67,6 +67,39 @@ u8 ata_irq_on(struct ata_port *ap)
u8 ata_dummy_irq_on (struct ata_port *ap) { return 0; }
/**
+ * ata_irq_off - Disable interrupts on a port.
+ * @ap: Port on which interrupts are disabled.
+ *
+ * Disable interrupts on a legacy IDE device using MMIO or PIO,
+ * wait for idle, clear any pending interrupts.
+ *
+ * LOCKING:
+ * Inherited from caller.
+ */
+u8 ata_irq_off(struct ata_port *ap)
+{
+ struct ata_ioports *ioaddr = ap-ioaddr;
+ u8 tmp;
+
+ ap-ctl |= ATA_NIEN;
+ ap-last_ctl = ap-ctl;
+
+ iowrite8(ap-ctl, ioaddr-ctl_addr);
+
+ /* Under certain circumstances, some controllers raise IRQ on
+* ATA_NIEN manipulation. Also, many controllers fail to mask
+* previously pending IRQ on ATA_NIEN assertion. Clear it.
+*/
+ tmp = ata_wait_idle(ap);
+
+ ap-ops-irq_clear(ap);
+
+ return tmp;
+}
+
+u8 ata_dummy_irq_off (struct ata_port *ap) { return 0; }
+
+/**
* ata_irq_ack - Acknowledge a device interrupt.
* @ap: Port on which interrupts are enabled.
*
diff -Nrup 01_remove_leftover_irqon/drivers/ata/libata.h
02_add_irq_off/drivers/ata/libata.h
--- 01_remove_leftover_irqon/drivers/ata/libata.h 2007-06-01
12:08:21.0 +0800
+++ 02_add_irq_off/drivers/ata/libata.h 2007-06-12 13:20:23.0 +0800
@@ -156,7 +156,5 @@ extern void ata_port_wait_eh(struct ata_
extern void ata_qc_schedule_eh(struct ata_queued_cmd *qc);
/* libata-sff.c */
-extern u8 ata_irq_on(struct ata_port *ap);
-
#endif /* __LIBATA_H__ */
diff -Nrup 01_remove_leftover_irqon/include/linux/libata.h
02_add_irq_off/include/linux/libata.h
--- 01_remove_leftover_irqon/include/linux/libata.h 2007-06-01
12:08:32.0 +0800
+++ 02_add_irq_off/include/linux/libata.h 2007-06-12 13:43:50.0
+0800
@@ -599,6 +599,7 @@ struct ata_port_operations {
irq_handler_t irq_handler;
void (*irq_clear) (struct ata_port *);
u8 (*irq_on) (struct ata_port *);
+ u8 (*irq_off) (struct ata_port *);
u8 (*irq_ack) (struct ata_port *ap, unsigned int chk_drq);
u32 (*scr_read) (struct ata_port *ap, unsigned int sc_reg);
@@ -805,6 +806,8 @@ extern struct ata_device *ata_dev_pair(s
extern int ata_do_set_mode(struct ata_port *ap, struct ata_device
**r_failed_dev);
extern u8 ata_irq_on(struct ata_port *ap);
extern u8 ata_dummy_irq_on(struct ata_port *ap);
+extern u8 ata_irq_off(struct ata_port *ap);
+extern u8 ata_dummy_irq_off(struct ata_port *ap);
extern u8 ata_irq_ack(struct ata_port *ap, unsigned int chk_drq);
extern u8 ata_dummy_irq_ack(struct ata_port *ap, unsigned int chk_drq);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 3/9] libata: add -irq_off() to LLDDs
Patch 3/9: Minor patch to add the newly added -irq_off() to LLDDs. Signed-off-by: Albert Lee [EMAIL PROTECTED] --- diff -Nrup 02_add_irq_off/drivers/ata/ahci.c 03_add_irq_off_lldd/drivers/ata/ahci.c --- 02_add_irq_off/drivers/ata/ahci.c 2007-06-01 12:08:21.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/ahci.c 2007-06-11 17:37:24.0 +0800 @@ -268,6 +268,7 @@ static const struct ata_port_operations .irq_clear = ahci_irq_clear, .irq_on = ata_dummy_irq_on, + .irq_off= ata_dummy_irq_off, .irq_ack= ata_dummy_irq_ack, .scr_read = ahci_scr_read, @@ -302,6 +303,7 @@ static const struct ata_port_operations .irq_clear = ahci_irq_clear, .irq_on = ata_dummy_irq_on, + .irq_off= ata_dummy_irq_off, .irq_ack= ata_dummy_irq_ack, .scr_read = ahci_scr_read, diff -Nrup 02_add_irq_off/drivers/ata/ata_generic.c 03_add_irq_off_lldd/drivers/ata/ata_generic.c --- 02_add_irq_off/drivers/ata/ata_generic.c2007-06-01 12:08:21.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/ata_generic.c 2007-06-11 17:46:08.0 +0800 @@ -121,6 +121,7 @@ static struct ata_port_operations generi .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, diff -Nrup 02_add_irq_off/drivers/ata/ata_piix.c 03_add_irq_off_lldd/drivers/ata/ata_piix.c --- 02_add_irq_off/drivers/ata/ata_piix.c 2007-06-01 12:08:21.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/ata_piix.c 2007-06-11 17:37:24.0 +0800 @@ -305,6 +305,7 @@ static const struct ata_port_operations .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -339,6 +340,7 @@ static const struct ata_port_operations .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -369,6 +371,7 @@ static const struct ata_port_operations .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, diff -Nrup 02_add_irq_off/drivers/ata/pata_ali.c 03_add_irq_off_lldd/drivers/ata/pata_ali.c --- 02_add_irq_off/drivers/ata/pata_ali.c 2007-06-01 12:08:21.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/pata_ali.c 2007-06-11 17:37:24.0 +0800 @@ -320,6 +320,7 @@ static struct ata_port_operations ali_ea .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -362,6 +363,7 @@ static struct ata_port_operations ali_20 .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -401,6 +403,7 @@ static struct ata_port_operations ali_c2 .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, @@ -439,6 +442,7 @@ static struct ata_port_operations ali_c5 .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start, diff -Nrup 02_add_irq_off/drivers/ata/pata_amd.c 03_add_irq_off_lldd/drivers/ata/pata_amd.c --- 02_add_irq_off/drivers/ata/pata_amd.c 2007-06-01 12:08:21.0 +0800 +++ 03_add_irq_off_lldd/drivers/ata/pata_amd.c 2007-06-11 17:37:24.0 +0800 @@ -356,6 +356,7 @@ static struct ata_port_operations amd33_ .irq_handler= ata_interrupt, .irq_clear = ata_bmdma_irq_clear, .irq_on = ata_irq_on, + .irq_off= ata_irq_off, .irq_ack= ata_irq_ack, .port_start = ata_port_start
[PATCH/RFC 4/9] libata: call irq_off from bmdma_freeze()
Patch 4/9:
Minor patch to call irq_off() from bmdma_freeze() to avoid duplicated code.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 03_add_irq_off_lldd/drivers/ata/libata-sff.c
04_convert_freeze/drivers/ata/libata-sff.c
--- 03_add_irq_off_lldd/drivers/ata/libata-sff.c2007-06-12
13:15:13.0 +0800
+++ 04_convert_freeze/drivers/ata/libata-sff.c 2007-06-12 13:15:17.0
+0800
@@ -413,20 +413,7 @@ void ata_bmdma_stop(struct ata_queued_cm
*/
void ata_bmdma_freeze(struct ata_port *ap)
{
- struct ata_ioports *ioaddr = ap-ioaddr;
-
- ap-ctl |= ATA_NIEN;
- ap-last_ctl = ap-ctl;
-
- iowrite8(ap-ctl, ioaddr-ctl_addr);
-
- /* Under certain circumstances, some controllers raise IRQ on
-* ATA_NIEN manipulation. Also, many controllers fail to mask
-* previously pending IRQ on ATA_NIEN assertion. Clear it.
-*/
- ata_chk_status(ap);
-
- ap-ops-irq_clear(ap);
+ ap-ops-irq_off(ap);
}
/**
diff -Nrup 03_add_irq_off_lldd/drivers/ata/pata_scc.c
04_convert_freeze/drivers/ata/pata_scc.c
--- 03_add_irq_off_lldd/drivers/ata/pata_scc.c 2007-06-12 13:06:12.0
+0800
+++ 04_convert_freeze/drivers/ata/pata_scc.c2007-06-12 14:58:03.0
+0800
@@ -875,20 +875,7 @@ static u8 scc_irq_ack (struct ata_port *
static void scc_bmdma_freeze (struct ata_port *ap)
{
- struct ata_ioports *ioaddr = ap-ioaddr;
-
- ap-ctl |= ATA_NIEN;
- ap-last_ctl = ap-ctl;
-
- out_be32(ioaddr-ctl_addr, ap-ctl);
-
- /* Under certain circumstances, some controllers raise IRQ on
-* ATA_NIEN manipulation. Also, many controllers fail to mask
-* previously pending IRQ on ATA_NIEN assertion. Clear it.
-*/
- ata_chk_status(ap);
-
- ap-ops-irq_clear(ap);
+ scc_irq_off(ap);
}
/**
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 5/9] libata: use freeze()/thaw() for polling
Patch 5/9:
This patch changes polling codes to use freeze()/thaw() for irq off/on.
ata_qc_set_polling() is also removed since now unused.
The reason for freeze()/thaw(): some ATAPI devices raises INTRQ even if nIEN =
1.
Using the host adapter irq mask mechanism, if available, is more reliable than
using the device nIEN bit.
Considerations:
1. the semantic of freeze()/thaw() maybe more than irq off/on?
2. HSM, the new user of freeze()/thaw(), will call freeze()/thaw() more
frequently than EH.
Can current implemention of freeze()/thaw() handle it?
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 04_convert_freeze/drivers/ata/libata-core.c
05_convert_hsm_to_freeze/drivers/ata/libata-core.c
--- 04_convert_freeze/drivers/ata/libata-core.c 2007-06-11 17:37:24.0
+0800
+++ 05_convert_hsm_to_freeze/drivers/ata/libata-core.c 2007-06-12
13:33:03.0 +0800
@@ -4753,7 +4753,7 @@ static void ata_hsm_qc_complete(struct a
qc = ata_qc_from_tag(ap, qc-tag);
if (qc) {
if (likely(!(qc-err_mask AC_ERR_HSM))) {
- ap-ops-irq_on(ap);
+ ap-ops-thaw(ap);
ata_qc_complete(qc);
} else
ata_port_freeze(ap);
@@ -4769,7 +4769,7 @@ static void ata_hsm_qc_complete(struct a
} else {
if (in_wq) {
spin_lock_irqsave(ap-lock, flags);
- ap-ops-irq_on(ap);
+ ap-ops-thaw(ap);
ata_qc_complete(qc);
spin_unlock_irqrestore(ap-lock, flags);
} else
@@ -5421,7 +5421,7 @@ unsigned int ata_qc_issue_prot(struct at
switch (qc-tf.protocol) {
case ATA_PROT_NODATA:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ata_qc_set_polling(qc);
+ ap-ops-freeze(ap);
ata_tf_to_host(ap, qc-tf);
ap-hsm_task_state = HSM_ST_LAST;
@@ -5442,7 +5442,7 @@ unsigned int ata_qc_issue_prot(struct at
case ATA_PROT_PIO:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ata_qc_set_polling(qc);
+ ap-ops-freeze(ap);
ata_tf_to_host(ap, qc-tf);
@@ -5471,7 +5471,7 @@ unsigned int ata_qc_issue_prot(struct at
case ATA_PROT_ATAPI:
case ATA_PROT_ATAPI_NODATA:
if (qc-tf.flags ATA_TFLAG_POLLING)
- ata_qc_set_polling(qc);
+ ap-ops-freeze(ap);
ata_tf_to_host(ap, qc-tf);
diff -Nrup 04_convert_freeze/include/linux/libata.h
05_convert_hsm_to_freeze/include/linux/libata.h
--- 04_convert_freeze/include/linux/libata.h2007-06-11 17:37:24.0
+0800
+++ 05_convert_hsm_to_freeze/include/linux/libata.h 2007-06-12
13:40:14.0 +0800
@@ -1100,11 +1100,6 @@ static inline u8 ata_wait_idle(struct at
return status;
}
-static inline void ata_qc_set_polling(struct ata_queued_cmd *qc)
-{
- qc-tf.ctl |= ATA_NIEN;
-}
-
static inline struct ata_queued_cmd *__ata_qc_from_tag(struct ata_port *ap,
unsigned int tag)
{
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 7/9] libata: pdc_freeze() semantic change
Patch 7/9: After checking the current implementations of freeze()/thaw(), it seems only pdc_freeze() do more than simple irq masking. Remove the DMA disable code from pdc_freeze(). The question is the design/semantic of freeze()/thaw(). Maybe we should limit them to simple irq on/off? Signed-off-by: Albert Lee [EMAIL PROTECTED] --- diff -Nrup 06_add_freeze_thaw_to_lldd/drivers/ata/sata_promise.c 07_sata_promise_freeze/drivers/ata/sata_promise.c --- 06_add_freeze_thaw_to_lldd/drivers/ata/sata_promise.c 2007-06-11 17:23:47.0 +0800 +++ 07_sata_promise_freeze/drivers/ata/sata_promise.c 2007-06-13 13:39:33.0 +0800 @@ -581,7 +581,6 @@ static void pdc_freeze(struct ata_port * tmp = readl(mmio + PDC_CTLSTAT); tmp |= PDC_IRQ_DISABLE; - tmp = ~PDC_DMA_ENABLE; writel(tmp, mmio + PDC_CTLSTAT); readl(mmio + PDC_CTLSTAT); /* flush */ } - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 8/9] libata: remove writing of tf-ctl from ata_tf_load()
Patch 8/9:
Currently ata_tf_load() writes to the Control register if it is changed.
The relevant bits in the ctl register are HOB, SRST and nIEN.
- HOB is only used by ata_tf_read().
- For SRST, soft reset is not the duty of tf_load.
- For nIEN, explicit irq_on()/irq_off and freeze()/thaw() are provided.
Since EH/HSM now call freeze()/thaw() for irq off/on explicitly.
The implicit nIEN handling is removed from ata_tf_load().
The nIEN snoop codes are also removed from sata_vsc.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 07_sata_promise_freeze/drivers/ata/libata-sff.c
08_tfload_cleanup/drivers/ata/libata-sff.c
--- 07_sata_promise_freeze/drivers/ata/libata-sff.c 2007-06-12
13:15:17.0 +0800
+++ 08_tfload_cleanup/drivers/ata/libata-sff.c 2007-06-12 14:52:57.0
+0800
@@ -153,11 +153,13 @@ void ata_tf_load(struct ata_port *ap, co
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- if (tf-ctl != ap-last_ctl) {
- iowrite8(tf-ctl, ioaddr-ctl_addr);
- ap-last_ctl = tf-ctl;
- ata_wait_idle(ap);
- }
+ /*
+* The relevant bits in the ctl register are HOB, SRST and nIEN.
+* HOB is only used by ata_tf_read().
+* For SRST, soft reset is not the duty of tf_load.
+* For nIEN, explicit -irq_on() and -irq_off are provided.
+* That's why tf-ctl is ignored here.
+*/
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
iowrite8(tf-hob_feature, ioaddr-feature_addr);
diff -Nrup 07_sata_promise_freeze/drivers/ata/pata_scc.c
08_tfload_cleanup/drivers/ata/pata_scc.c
--- 07_sata_promise_freeze/drivers/ata/pata_scc.c 2007-06-12
14:58:15.0 +0800
+++ 08_tfload_cleanup/drivers/ata/pata_scc.c2007-06-12 14:58:20.0
+0800
@@ -271,12 +271,6 @@ static void scc_tf_load (struct ata_port
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- if (tf-ctl != ap-last_ctl) {
- out_be32(ioaddr-ctl_addr, tf-ctl);
- ap-last_ctl = tf-ctl;
- ata_wait_idle(ap);
- }
-
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
out_be32(ioaddr-feature_addr, tf-hob_feature);
out_be32(ioaddr-nsect_addr, tf-hob_nsect);
diff -Nrup 07_sata_promise_freeze/drivers/ata/sata_svw.c
08_tfload_cleanup/drivers/ata/sata_svw.c
--- 07_sata_promise_freeze/drivers/ata/sata_svw.c 2007-06-11
17:24:51.0 +0800
+++ 08_tfload_cleanup/drivers/ata/sata_svw.c2007-06-12 13:37:46.0
+0800
@@ -125,11 +125,6 @@ static void k2_sata_tf_load(struct ata_p
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- if (tf-ctl != ap-last_ctl) {
- writeb(tf-ctl, ioaddr-ctl_addr);
- ap-last_ctl = tf-ctl;
- ata_wait_idle(ap);
- }
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
writew(tf-feature | (((u16)tf-hob_feature) 8),
ioaddr-feature_addr);
diff -Nrup 07_sata_promise_freeze/drivers/ata/sata_vsc.c
08_tfload_cleanup/drivers/ata/sata_vsc.c
--- 07_sata_promise_freeze/drivers/ata/sata_vsc.c 2007-06-11
17:25:50.0 +0800
+++ 08_tfload_cleanup/drivers/ata/sata_vsc.c2007-06-13 13:47:24.0
+0800
@@ -137,36 +137,11 @@ static void vsc_thaw(struct ata_port *ap
}
-static void vsc_intr_mask_update(struct ata_port *ap, u8 ctl)
-{
- void __iomem *mask_addr;
- u8 mask;
-
- mask_addr = ap-host-iomap[VSC_MMIO_BAR] +
- VSC_SATA_INT_MASK_OFFSET + ap-port_no;
- mask = readb(mask_addr);
- if (ctl ATA_NIEN)
- mask |= 0x80;
- else
- mask = 0x7F;
- writeb(mask, mask_addr);
-}
-
-
static void vsc_sata_tf_load(struct ata_port *ap, const struct ata_taskfile
*tf)
{
struct ata_ioports *ioaddr = ap-ioaddr;
unsigned int is_addr = tf-flags ATA_TFLAG_ISADDR;
- /*
-* The only thing the ctl register is used for is SRST.
-* That is not enabled or disabled via tf_load.
-* However, if ATA_NIEN is changed, then we need to change the
interrupt register.
-*/
- if ((tf-ctl ATA_NIEN) != (ap-last_ctl ATA_NIEN)) {
- ap-last_ctl = tf-ctl;
- vsc_intr_mask_update(ap, tf-ctl ATA_NIEN);
- }
if (is_addr (tf-flags ATA_TFLAG_LBA48)) {
writew(tf-feature | (((u16)tf-hob_feature) 8),
ioaddr-feature_addr);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 9/9] libata: remove irq_on/off and rename freeze()/thaw() to irq_on/off
Patch 9/9:
It seems -irq_on and -irq_off are now only wrapped by freeze()/thaw() and
unused elsewhere.
This patch
- Remove -irq_on and -irq_off.
- Rename -freeze and -thaw to irq_on() and irq_off() to be specific.
Hopefully the LLDDs need to implement only one irq on/off by either host
adapter masking,
nIEN manipulation, or both or something else.
No patch for this yet. Just some idea like below...
diff -Nrup 09_freeze_thaw_pdc2027x/include/linux/libata.h
10_replace_irq_on_off/include/linux/libata.h
--- 09_freeze_thaw_pdc2027x/include/linux/libata.h 2007-06-12
13:35:19.0 +0800
+++ 10_replace_irq_on_off/include/linux/libata.h2007-06-15
11:43:47.0 +0800
@@ -591,15 +591,13 @@ struct ata_port_operations {
*/
void (*eng_timeout) (struct ata_port *ap); /* obsolete */
- void (*freeze) (struct ata_port *ap);
- void (*thaw) (struct ata_port *ap);
+ u8 (*irq_on) (struct ata_port *);
+ u8 (*irq_off) (struct ata_port *);
void (*error_handler) (struct ata_port *ap);
void (*post_internal_cmd) (struct ata_queued_cmd *qc);
irq_handler_t irq_handler;
void (*irq_clear) (struct ata_port *);
- u8 (*irq_on) (struct ata_port *);
- u8 (*irq_off) (struct ata_port *);
u8 (*irq_ack) (struct ata_port *ap, unsigned int chk_drq);
u32 (*scr_read) (struct ata_port *ap, unsigned int sc_reg);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] libata: ATA passthru fixes
1/6: update protocol numbers 2/6: support PIO multi commands 3/6: map UDMA protocols 4/6: always enforce correct DEV bit 5/6: support ATAPI devices 6/6: update cached device parameters Patch against the libata-dev tree for your review, thanks. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] libata: update protocol numbers
Patch 1/6: Update the ATA passthru protocol numbers according to the new spec. Signed-off-by: Albert Lee [EMAIL PROTECTED] --- diff -Nrup 00_libata-dev/drivers/ata/libata-scsi.c 01_protocol_update/drivers/ata/libata-scsi.c --- 00_libata-dev/drivers/ata/libata-scsi.c 2007-06-01 12:08:21.0 +0800 +++ 01_protocol_update/drivers/ata/libata-scsi.c2007-06-07 11:38:50.0 +0800 @@ -2512,16 +2512,15 @@ ata_scsi_map_proto(u8 byte1) case 5: /* PIO Data-out */ return ATA_PROT_PIO; - case 10:/* Device Reset */ case 0: /* Hard Reset */ case 1: /* SRST */ - case 2: /* Bus Idle */ - case 7: /* Packet */ - case 8: /* DMA Queued */ - case 9: /* Device Diagnostic */ - case 11:/* UDMA Data-in */ - case 12:/* UDMA Data-Out */ - case 13:/* FPDMA */ + case 8: /* Device Diagnostic */ + case 9: /* Device Reset */ + case 7: /* DMA Queued */ + case 10:/* UDMA Data-in */ + case 11:/* UDMA Data-Out */ + case 12:/* FPDMA */ + case 15:/* Return Response Info */ default:/* Reserved */ break; } - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] libata: support PIO multi commands
Patch 2/6:
support the pass through of PIO multi commands.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 01_protocol_update/drivers/ata/libata-scsi.c
02_pio_multi/drivers/ata/libata-scsi.c
--- 01_protocol_update/drivers/ata/libata-scsi.c2007-06-07
11:38:50.0 +0800
+++ 02_pio_multi/drivers/ata/libata-scsi.c 2007-06-07 11:38:53.0
+0800
@@ -2551,10 +2551,6 @@ static unsigned int ata_scsi_pass_thru(s
if (tf-protocol == ATA_PROT_DMA dev-dma_mode == 0)
goto invalid_fld;
- if (cdb[1] 0xe0)
- /* PIO multi not supported yet */
- goto invalid_fld;
-
/*
* 12 and 16 byte CDBs use different offsets to
* provide the various register values.
@@ -2606,6 +2602,22 @@ static unsigned int ata_scsi_pass_thru(s
tf-device = qc-dev-devno ?
tf-device | ATA_DEV1 : tf-device ~ATA_DEV1;
+ /* sanity check for pio multi commands */
+ if ((cdb[1] 0xe0) !is_multi_taskfile(tf))
+ goto invalid_fld;
+
+ if (is_multi_taskfile(tf)) {
+ unsigned int multi_count = 1 (cdb[1] 5);
+
+ /* compare the passed through multi_count
+* with the cached multi_count of libata
+*/
+ if (multi_count != dev-multi_count)
+ ata_dev_printk(dev, KERN_WARNING,
+ invalid multi_count %u ignored\n,
+ multi_count);
+ }
+
/* READ/WRITE LONG use a non-standard sect_size */
qc-sect_size = ATA_SECT_SIZE;
switch (tf-command) {
diff -Nrup 01_protocol_update/include/linux/ata.h
02_pio_multi/include/linux/ata.h
--- 01_protocol_update/include/linux/ata.h 2007-06-01 12:08:32.0
+0800
+++ 02_pio_multi/include/linux/ata.h2007-06-06 13:34:05.0 +0800
@@ -249,7 +249,7 @@ enum ata_tf_protocols {
/* ATA taskfile protocols */
ATA_PROT_UNKNOWN, /* unknown/invalid */
ATA_PROT_NODATA,/* no data */
- ATA_PROT_PIO, /* PIO single sector */
+ ATA_PROT_PIO, /* PIO data xfer */
ATA_PROT_DMA, /* DMA */
ATA_PROT_NCQ, /* NCQ */
ATA_PROT_ATAPI, /* packet command, PIO data xfer*/
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] libata: map UDMA protocols
Patch 3/6: Map the ATA passthru UDMA protocols to ATA_PROT_DMA. Signed-off-by: Albert Lee [EMAIL PROTECTED] --- Don't know why SAT distinguishs UDMA Data In and UDMA Data Out from DMA. These UDMA protocols only matter for on-the-wire-protocol and are mostly transparent to the software. Anyway, map both of UDMA protocols to ATA_PROT_DMA here. diff -Nrup 02_pio_multi/drivers/ata/libata-scsi.c 03_udma_supp/drivers/ata/libata-scsi.c --- 02_pio_multi/drivers/ata/libata-scsi.c 2007-06-07 11:38:53.0 +0800 +++ 03_udma_supp/drivers/ata/libata-scsi.c 2007-06-07 11:41:30.0 +0800 @@ -2506,6 +2506,8 @@ ata_scsi_map_proto(u8 byte1) return ATA_PROT_NODATA; case 6: /* DMA */ + case 10:/* UDMA Data-in */ + case 11:/* UDMA Data-Out */ return ATA_PROT_DMA; case 4: /* PIO Data-in */ @@ -2517,8 +2519,6 @@ ata_scsi_map_proto(u8 byte1) case 8: /* Device Diagnostic */ case 9: /* Device Reset */ case 7: /* DMA Queued */ - case 10:/* UDMA Data-in */ - case 11:/* UDMA Data-Out */ case 12:/* FPDMA */ case 15:/* Return Response Info */ default:/* Reserved */ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] libata: always enforce correct DEV bit
Patch 4/6: Always enforce correct DEV bit since we know which drive the command is targeted. SAT demands to ignore the DEV bit, too. Signed-off-by: Albert Lee [EMAIL PROTECTED] --- diff -Nrup 03_udma_supp/drivers/ata/libata-scsi.c 04_dev_bit/drivers/ata/libata-scsi.c --- 03_udma_supp/drivers/ata/libata-scsi.c 2007-06-07 11:41:30.0 +0800 +++ 04_dev_bit/drivers/ata/libata-scsi.c2007-06-07 14:44:27.0 +0800 @@ -2595,12 +2595,10 @@ static unsigned int ata_scsi_pass_thru(s tf-device = cdb[8]; tf-command = cdb[9]; } - /* -* If slave is possible, enforce correct master/slave bit - */ - if (qc-ap-flags ATA_FLAG_SLAVE_POSS) - tf-device = qc-dev-devno ? - tf-device | ATA_DEV1 : tf-device ~ATA_DEV1; + + /* enforce correct master/slave bit */ + tf-device = dev-devno ? + tf-device | ATA_DEV1 : tf-device ~ATA_DEV1; /* sanity check for pio multi commands */ if ((cdb[1] 0xe0) !is_multi_taskfile(tf)) - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] libata: support ATAPI devices
Patch 5/6:
Support ATA passthru to ATAPI devices.
Revised based on Mark's patch and Jeff's comments.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
Cc: Mark Lord [EMAIL PROTECTED]
---
After reading Mark's patch, I guess we can safely by-pass the
cdb_len check: the ATA_12/ATA_16 are for the libata SATL and
get unwrapped to ATA taskfiles before the devices ever see them.
diff -Nrup 04_dev_bit/drivers/ata/libata-scsi.c
05_supp_atapi/drivers/ata/libata-scsi.c
--- 04_dev_bit/drivers/ata/libata-scsi.c2007-06-07 14:44:27.0
+0800
+++ 05_supp_atapi/drivers/ata/libata-scsi.c 2007-06-07 14:44:30.0
+0800
@@ -2701,10 +2701,6 @@ static inline ata_xlat_func_t ata_get_xl
case VERIFY_16:
return ata_scsi_verify_xlat;
- case ATA_12:
- case ATA_16:
- return ata_scsi_pass_thru;
-
case START_STOP:
return ata_scsi_start_stop_xlat;
}
@@ -2740,8 +2736,14 @@ static inline int __ata_scsi_queuecmd(st
void (*done)(struct scsi_cmnd *),
struct ata_device *dev)
{
+ u8 cmd = scmd-cmnd[0];
int rc = 0;
+ if (unlikely(cmd == ATA_12 || cmd == ATA_16)) {
+ rc = ata_scsi_translate(dev, scmd, done, ata_scsi_pass_thru);
+ return rc;
+ }
+
if (unlikely(!scmd-cmd_len || scmd-cmd_len dev-cdb_len)) {
DPRINTK(bad CDB len=%u, max=%u\n,
scmd-cmd_len, dev-cdb_len);
@@ -2751,8 +2753,7 @@ static inline int __ata_scsi_queuecmd(st
}
if (dev-class == ATA_DEV_ATA) {
- ata_xlat_func_t xlat_func = ata_get_xlat_func(dev,
- scmd-cmnd[0]);
+ ata_xlat_func_t xlat_func = ata_get_xlat_func(dev, cmd);
if (xlat_func)
rc = ata_scsi_translate(dev, scmd, done, xlat_func);
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] libata: update cached device paramters
Patch 6/6:
INIT_DEV_PARAMS and SET_MULTI_MODE change the device parameters cached by
libata.
Re-read IDENTIFY DEVICE info and update the cached device paramters when seeing
these commands.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 05_supp_atapi/drivers/ata/libata-scsi.c
06_sync_param/drivers/ata/libata-scsi.c
--- 05_supp_atapi/drivers/ata/libata-scsi.c 2007-06-07 14:44:30.0
+0800
+++ 06_sync_param/drivers/ata/libata-scsi.c 2007-06-07 14:44:33.0
+0800
@@ -1363,12 +1363,22 @@ static void ata_scsi_qc_complete(struct
* schedule EH_REVALIDATE operation to update the IDENTIFY DEVICE
* cache
*/
- if (ap-ops-error_handler
- !need_sense (qc-tf.command == ATA_CMD_SET_FEATURES)
- ((qc-tf.feature == SETFEATURES_WC_ON) ||
-(qc-tf.feature == SETFEATURES_WC_OFF))) {
- ap-eh_info.action |= ATA_EH_REVALIDATE;
- ata_port_schedule_eh(ap);
+ if (ap-ops-error_handler !need_sense) {
+ switch (qc-tf.command) {
+ case ATA_CMD_SET_FEATURES:
+ if ((qc-tf.feature == SETFEATURES_WC_ON) ||
+ (qc-tf.feature == SETFEATURES_WC_OFF)) {
+ ap-eh_info.action |= ATA_EH_REVALIDATE;
+ ata_port_schedule_eh(ap);
+ }
+ break;
+
+ case ATA_CMD_INIT_DEV_PARAMS: /* CHS translation changed */
+ case ATA_CMD_SET_MULTI: /* multi_count changed */
+ ap-eh_info.action |= ATA_EH_REVALIDATE;
+ ata_port_schedule_eh(ap);
+ break;
+ }
}
/* For ATA pass thru (SAT) commands, generate a sense block if
diff -Nrup 05_supp_atapi/include/linux/ata.h 06_sync_param/include/linux/ata.h
--- 05_supp_atapi/include/linux/ata.h 2007-06-06 13:34:05.0 +0800
+++ 06_sync_param/include/linux/ata.h 2007-06-07 10:50:26.0 +0800
@@ -151,6 +151,7 @@ enum {
ATA_CMD_WRITE_MULTI_EXT = 0x39,
ATA_CMD_WRITE_MULTI_FUA_EXT = 0xCE,
ATA_CMD_SET_FEATURES= 0xEF,
+ ATA_CMD_SET_MULTI = 0xC6,
ATA_CMD_PACKET = 0xA0,
ATA_CMD_VERIFY = 0x40,
ATA_CMD_VERIFY_EXT = 0x42,
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] libata: minor pio fixes
Minor pio fixes: 1/3: remove unneeded ata_altstatus() from ata_hsm_qc_complete() 2/3: move the ata_altstatus() to pio data xfer functions 3/3: change the last state of pio read to HSM_ST_IDLE Patch against the libata-dev tree for your review. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] libata: remove unneeded ata_altstatus() from ata_hsm_qc_complete()
Patch 1/3: In ata_hsm_qc_complete(): Calling ata_altstatus() after the qc completed looks wrong. Remove it. Signed-off-by: Albert Lee [EMAIL PROTECTED] --- After checking, it is leftover of ata_pio_block( ) in 2.6.17-rc5 of the following merge: http://git.kernel.org/?p=linux/kernel/git/jgarzik/libata-dev.git;a=commitdiff;h=bb31a8faa270beafcc51a65880c5564c6b718bd6 We can safely remove it. diff -Nrup 00_libata-dev/drivers/ata/libata-core.c 01_remove_bad_flush/drivers/ata/libata-core.c --- 00_libata-dev/drivers/ata/libata-core.c 2007-06-01 12:08:21.0 +0800 +++ 01_remove_bad_flush/drivers/ata/libata-core.c 2007-06-04 18:10:43.0 +0800 @@ -4782,8 +4782,6 @@ static void ata_hsm_qc_complete(struct a } else ata_qc_complete(qc); } - - ata_altstatus(ap); /* flush */ } /** - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] libata: move ata_altstatus() to pio data xfer functions
Patch 2/3:
Move ata_altstatus() out from ata_hsm_move() to the pio data xfer functions
such as ata_pio_sectors() and atapi_pio_bytes() where it makes more sense.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
atapi_send_cdb() already calls ata_altstatus() inside.
This patch makes ata_pio_sectors() and atapi_pio_bytes() do the same for
consistency.
diff -Nrup 01_remove_bad_flush/drivers/ata/libata-core.c
02_move_altstatus/drivers/ata/libata-core.c
--- 01_remove_bad_flush/drivers/ata/libata-core.c 2007-06-04
18:10:43.0 +0800
+++ 02_move_altstatus/drivers/ata/libata-core.c 2007-06-04 18:16:24.0
+0800
@@ -4524,6 +4524,8 @@ static void ata_pio_sectors(struct ata_q
ata_pio_sector(qc);
} else
ata_pio_sector(qc);
+
+ ata_altstatus(ap); /* flush */
}
/**
@@ -4698,6 +4700,7 @@ static void atapi_pio_bytes(struct ata_q
VPRINTK(ata%u: xfering %d bytes\n, ap-print_id, bytes);
__atapi_pio_bytes(qc, bytes);
+ ata_altstatus(ap); /* flush */
return;
@@ -4869,7 +4872,6 @@ fsm_start:
*/
ap-hsm_task_state = HSM_ST;
ata_pio_sectors(qc);
- ata_altstatus(ap); /* flush */
} else
/* send CDB */
atapi_send_cdb(ap, qc);
@@ -4950,7 +4952,6 @@ fsm_start:
if (!(qc-tf.flags ATA_TFLAG_WRITE)) {
ata_pio_sectors(qc);
- ata_altstatus(ap);
status = ata_wait_idle(ap);
}
@@ -4970,13 +4971,11 @@ fsm_start:
if (ap-hsm_task_state == HSM_ST_LAST
(!(qc-tf.flags ATA_TFLAG_WRITE))) {
/* all data read */
- ata_altstatus(ap);
status = ata_wait_idle(ap);
goto fsm_start;
}
}
- ata_altstatus(ap); /* flush */
poll_next = 1;
break;
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] libata: change the last state of pio read to HSM_ST_IDLE
Patch 3/3:
After reading the last pio block, the HSM is waiting for device
to be idle, not waiting for the last interrupt.
Change the state after PIO data-in to HSM_ST_IDLE instead of HSM_ST_LAST for
accuracy.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 02_move_altstatus/drivers/ata/libata-core.c
03_read_state/drivers/ata/libata-core.c
--- 02_move_altstatus/drivers/ata/libata-core.c 2007-06-04 18:16:24.0
+0800
+++ 03_read_state/drivers/ata/libata-core.c 2007-06-04 18:25:00.0
+0800
@@ -4462,7 +4462,7 @@ static void ata_pio_sector(struct ata_qu
unsigned char *buf;
if (qc-curbytes == qc-nbytes - qc-sect_size)
- ap-hsm_task_state = HSM_ST_LAST;
+ ap-hsm_task_state = do_write ? HSM_ST_LAST : HSM_ST_IDLE;
page = sg[qc-cursg].page;
offset = sg[qc-cursg].offset + qc-cursg_ofs;
@@ -4811,6 +4811,8 @@ int ata_hsm_move(struct ata_port *ap, st
*/
WARN_ON(in_wq != ata_hsm_ok_in_wq(ap, qc));
+ WARN_ON(ap-hsm_task_state == HSM_ST_IDLE);
+
fsm_start:
DPRINTK(ata%u: protocol %d task_state %d (dev_stat 0x%X)\n,
ap-print_id, qc-tf.protocol, ap-hsm_task_state, status);
@@ -4968,8 +4970,7 @@ fsm_start:
ata_pio_sectors(qc);
- if (ap-hsm_task_state == HSM_ST_LAST
- (!(qc-tf.flags ATA_TFLAG_WRITE))) {
+ if (ap-hsm_task_state == HSM_ST_IDLE) {
/* all data read */
status = ata_wait_idle(ap);
goto fsm_start;
@@ -4980,6 +4981,7 @@ fsm_start:
break;
case HSM_ST_LAST:
+ case HSM_ST_IDLE:
if (unlikely(!ata_ok(status))) {
qc-err_mask |= __ac_err_mask(status);
ap-hsm_task_state = HSM_ST_ERR;
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] libata: print device model and firmware revision for ATAPI devices
For ATA/CFA devices, libata prints out the device model and firmware revision.
Do the same for ATAPI devices.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Sometimes the error happens after identify but before the
SCSI messages. Knowing the device model and firmware revision
could also help.
Dmesg looks like below after the patch.
ata1.00: ATA-6: WDC WD2000JB-00KFA0, 08.05J08, max UDMA/100
ata1.00: 390721968 sectors, multi 16: LBA48
ata5.00: ATAPI: LITE-ON CD-RW SOHR-5238S, 4S07, max UDMA/33
For your review, thanks.
diff -Nrup 00_libata-dev/drivers/ata/libata-core.c
01_atapi_dmesg/drivers/ata/libata-core.c
--- 00_libata-dev/drivers/ata/libata-core.c 2007-06-01 12:08:21.0
+0800
+++ 01_atapi_dmesg/drivers/ata/libata-core.c2007-06-04 15:33:19.0
+0800
@@ -1900,6 +1900,13 @@ int ata_dev_configure(struct ata_device
if (ata_msg_probe(ap))
ata_dump_id(id);
+ /* SCSI only uses 4-char revisions, dump full 8 chars from ATA */
+ ata_id_c_string(dev-id, fwrevbuf, ATA_ID_FW_REV,
+ sizeof(fwrevbuf));
+
+ ata_id_c_string(dev-id, modelbuf, ATA_ID_PROD,
+ sizeof(modelbuf));
+
/* ATA-specific feature tests */
if (dev-class == ATA_DEV_ATA) {
if (ata_id_is_cfa(id)) {
@@ -1914,13 +1921,6 @@ int ata_dev_configure(struct ata_device
dev-n_sectors = ata_id_n_sectors(id);
- /* SCSI only uses 4-char revisions, dump full 8 chars from ATA
*/
- ata_id_c_string(dev-id, fwrevbuf, ATA_ID_FW_REV,
- sizeof(fwrevbuf));
-
- ata_id_c_string(dev-id, modelbuf, ATA_ID_PROD,
- sizeof(modelbuf));
-
if (dev-id[59] 0x100)
dev-multi_count = dev-id[59] 0xff;
@@ -2009,7 +2009,9 @@ int ata_dev_configure(struct ata_device
/* print device info to dmesg */
if (ata_msg_drv(ap) print_info)
- ata_dev_printk(dev, KERN_INFO, ATAPI, max %s%s\n,
+ ata_dev_printk(dev, KERN_INFO,
+ ATAPI: %s, %s, max %s%s\n,
+ modelbuf, fwrevbuf,
ata_mode_string(xfer_mask),
cdb_intr_string);
}
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] libata: ack more unsolicited INTRQ
Alan Cox wrote: As previously discussed, the possible issue with this patch is: Some ATA/ATAPI devices might be unhappy if the STATUS register is read during data transfer (not sure if this is true or not). (Patch 5/8 doesn't have such issue.) Some older intel eats your disk if you do that. Ah, too bad. Thanks for the advice. Neither of these approaches quite work. Much as I dislike the old IDE disable/enable irq approach it does look like that is the only safe answer for some controllers. Agreed reading the status register during data transfer looks bad. Please disregard this patch. Back to the problem that the patch was trying to solve, i.e. unsolicited interrupt when HSM doing data transfer in the wq, is there any experience about how often such situation occurs? IMHO, it seems not something that happens often. If the cable/devices are well configured, the intrq would mostly occur when it should. Even if the unsolicited irq happens, maybe the current code has good chance to handle it? e.g. ata_irq_task() already reads the status before data transfer, thus possibly clearing some of unsolicited irqs. e.g. maybe the data transfer in the workqueue is quick enough? If yes, hopefully the ATA_PFLAG_HSM_WQ is soon cleared, and then the interrupt handler can come in ack the irq. (Much the same way when the irq handler encounters early irq by bad devices.) -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 0/8] libata: delegate irq driven pio to workqueue (take 2)
1/8: fix the ata_altstatus() in ata_hsm_qc_complete() 2/8: move ata_altstatus() from ata_hsm_move() to pio data xfer functions 3/8: change the state after PIO data-in to HSM_ST_IDLE instead of HSM_ST_LAST 4/8: move and reduce locking to the pio data xfer functions 5/8: ack possibly unsolicited irq when polling 6/8: delegate irq driven pio to workqueue 7/8: ata_port_flush_task() fix for irq pio delegation 8/8: ack more unsolicited irq --- Revised and reorder the patches per previous comments. Patch against the libata-dev tree for your review. (af3b146d26550f0c8e0d77b2117c6f8aec5d8146) - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] libata: fix the ata_altstatus() in ata_hsm_qc_complete()
patch 1/8:
In ata_hsm_qc_complete():
Calling ata_altstatus() after the qc completed looks unsafe.
Move it to be before completing the qc.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Don't know what the ata_altstatus() is doing here?
Anyway, move it to be before ata_qc_complete().
diff -Nrup 00_libata-dev/drivers/ata/libata-core.c
01_flush_fix/drivers/ata/libata-core.c
--- 00_libata-dev/drivers/ata/libata-core.c 2007-05-14 12:18:45.0
+0800
+++ 01_flush_fix/drivers/ata/libata-core.c 2007-05-15 10:05:33.0
+0800
@@ -4740,6 +4740,8 @@ static void ata_hsm_qc_complete(struct a
struct ata_port *ap = qc-ap;
unsigned long flags;
+ ata_altstatus(ap); /* flush */
+
if (ap-ops-error_handler) {
if (in_wq) {
spin_lock_irqsave(ap-lock, flags);
@@ -4772,8 +4774,6 @@ static void ata_hsm_qc_complete(struct a
} else
ata_qc_complete(qc);
}
-
- ata_altstatus(ap); /* flush */
}
/**
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/8] libata: move ata_altstatus() out from ata_hsm_move() to the pio data xfer functions
patch 2/8:
Move ata_altstatus() out from ata_hsm_move() to the pio data xfer functions,
like ata_pio_sectors() and atapi_pio_bytes() that know better if
ata_altstatus() is needed.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
atapi_send_cdb() already did ata_altstatus() in itself.
This patch makes ata_pio_sector(), ata_pio_sectors() and atapi_pio_bytes() do
the same.
diff -Nrup 01_flush_fix/drivers/ata/libata-core.c
02_smart_flush/drivers/ata/libata-core.c
--- 01_flush_fix/drivers/ata/libata-core.c 2007-05-15 10:05:33.0
+0800
+++ 02_smart_flush/drivers/ata/libata-core.c2007-05-16 10:37:53.0
+0800
@@ -4435,6 +4435,7 @@ void ata_data_xfer_noirq(struct ata_devi
/**
* ata_pio_sector - Transfer a sector of data.
* @qc: Command on going
+ * @drq_last: Last sector of pio DRQ transfer
*
* Transfer qc-sect_size bytes of data from/to the ATA device.
*
@@ -4442,7 +4443,7 @@ void ata_data_xfer_noirq(struct ata_devi
* Inherited from caller.
*/
-static void ata_pio_sector(struct ata_queued_cmd *qc)
+static void ata_pio_sector(struct ata_queued_cmd *qc, int drq_last)
{
int do_write = (qc-tf.flags ATA_TFLAG_WRITE);
struct scatterlist *sg = qc-__sg;
@@ -4480,6 +4481,9 @@ static void ata_pio_sector(struct ata_qu
ap-ops-data_xfer(qc-dev, buf + offset, qc-sect_size,
do_write);
}
+ if (drq_last)
+ ata_altstatus(ap); /* flush */
+
qc-curbytes += qc-sect_size;
qc-cursg_ofs += qc-sect_size;
@@ -4511,9 +4515,9 @@ static void ata_pio_sectors(struct ata_q
nsect = min((qc-nbytes - qc-curbytes) / qc-sect_size,
qc-dev-multi_count);
while (nsect--)
- ata_pio_sector(qc);
+ ata_pio_sector(qc, !nsect);
} else
- ata_pio_sector(qc);
+ ata_pio_sector(qc, 1);
}
/**
@@ -4596,6 +4600,7 @@ next_sg:
for (i = 0; i words; i++)
ap-ops-data_xfer(qc-dev, (unsigned char*)pad_buf, 2,
do_write);
+ ata_altstatus(ap); /* flush */
ap-hsm_task_state = HSM_ST_LAST;
return;
}
@@ -4645,6 +4650,8 @@ next_sg:
if (bytes)
goto next_sg;
+
+ ata_altstatus(ap); /* flush */
}
/**
@@ -4861,7 +4868,6 @@ fsm_start:
*/
ap-hsm_task_state = HSM_ST;
ata_pio_sectors(qc);
- ata_altstatus(ap); /* flush */
} else
/* send CDB */
atapi_send_cdb(ap, qc);
@@ -4942,7 +4948,6 @@ fsm_start:
if (!(qc-tf.flags ATA_TFLAG_WRITE)) {
ata_pio_sectors(qc);
- ata_altstatus(ap);
status = ata_wait_idle(ap);
}
@@ -4962,13 +4967,11 @@ fsm_start:
if (ap-hsm_task_state == HSM_ST_LAST
(!(qc-tf.flags ATA_TFLAG_WRITE))) {
/* all data read */
- ata_altstatus(ap);
status = ata_wait_idle(ap);
goto fsm_start;
}
}
- ata_altstatus(ap); /* flush */
poll_next = 1;
break;
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] libata: move and reduce locking to the pio data xfer functions
patch 4/8:
- Added the ATA_PFLAG_HSM_WQ (i.e. HSM running in workqueue) flag to serialize
irq and workqueue access to the port.
- Moved the locking out from the ata_hsm_move() to the data xfer functions like
ata_pio_sectors().
- The time holding ap-lock is reduced to only part of last pio transfer and
clearing of the ATA_PFLAG_HSM_WQ flag.
- The transfer of head is made to be multiple of 8-bytes such that
-data_xfer() could possibly utilize 32-bit pio/mmio.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
Cc: Alan Cox [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
---
The variable name is changed to irq_handover per Tejun's review.
sata_sil is also modified per Tejun's advice.
Alan's concern about unsolicited irq will be addressed later in patch 5/8 and
8/8.
The chang to __atapi_pio_bytes() is the hardest part. Hopefully this patch gets
it right.
diff -Nrup 03_read_state/drivers/ata/libata-core.c
04_move_narrow_lock/drivers/ata/libata-core.c
--- 03_read_state/drivers/ata/libata-core.c 2007-05-16 10:37:57.0
+0800
+++ 04_move_narrow_lock/drivers/ata/libata-core.c 2007-05-16
13:53:16.0 +0800
@@ -4436,6 +4436,7 @@ void ata_data_xfer_noirq(struct ata_devi
* ata_pio_sector - Transfer a sector of data.
* @qc: Command on going
* @drq_last: Last sector of pio DRQ transfer
+ * @irq_handover: workqueue is going to handover to the irq handler
*
* Transfer qc-sect_size bytes of data from/to the ATA device.
*
@@ -4443,7 +,7 @@ void ata_data_xfer_noirq(struct ata_devi
* Inherited from caller.
*/
-static void ata_pio_sector(struct ata_queued_cmd *qc, int drq_last)
+static void ata_pio_sector(struct ata_queued_cmd *qc, int drq_last, int
irq_handover)
{
int do_write = (qc-tf.flags ATA_TFLAG_WRITE);
struct scatterlist *sg = qc-__sg;
@@ -4451,6 +4452,7 @@ static void ata_pio_sector(struct ata_qu
struct page *page;
unsigned int offset;
unsigned char *buf;
+ unsigned long irq_flags = 0;
if (qc-curbytes == qc-nbytes - qc-sect_size)
ap-hsm_task_state = do_write ? HSM_ST_LAST : HSM_ST_IDLE;
@@ -4467,6 +4469,16 @@ static void ata_pio_sector(struct ata_qu
if (PageHighMem(page)) {
unsigned long flags;
+ if (irq_handover)
+ /* Data transfer will trigger INTRQ.
+* Acquire ap-lock to
+* - transfer the last sector of data
+* - read and discard altstatus
+* - clear ATA_PFLAG_HSM_WQ
+* atomically.
+*/
+ spin_lock_irqsave(ap-lock, irq_flags);
+
/* FIXME: use a bounce buffer */
local_irq_save(flags);
buf = kmap_atomic(page, KM_IRQ0);
@@ -4477,8 +4489,34 @@ static void ata_pio_sector(struct ata_qu
kunmap_atomic(buf, KM_IRQ0);
local_irq_restore(flags);
} else {
+ unsigned int head = 0, tail = qc-sect_size;
+
buf = page_address(page);
- ap-ops-data_xfer(qc-dev, buf + offset, qc-sect_size,
do_write);
+
+ if (irq_handover) {
+ tail = 8;
+ head = qc-sect_size - tail;
+
+ /* Data transfer of head is done without holding
+* ap-lock to improve interrupt latency.
+* Hopefully no unsolicited INTRQ at this point,
+* otherwise we may have nobody cared irq.
+* Make head to be multiple of 8 bytes such that
+* -data_xfer() could utilize 32-bit pio/mmio.
+*/
+ ap-ops-data_xfer(qc-dev, buf + offset, head,
do_write);
+
+ /* Data transfer of tail will trigger INTRQ.
+* Acquire ap-lock to
+* - transfer the last 8 bytes of data
+* - read and discard altstatus
+* - clear ATA_PFLAG_HSM_WQ
+* atomically.
+*/
+ spin_lock_irqsave(ap-lock, irq_flags);
+ }
+
+ ap-ops-data_xfer(qc-dev, buf + offset + head, tail,
do_write);
}
if (drq_last)
@@ -4491,11 +4529,21 @@ static void ata_pio_sector(struct ata_qu
qc-cursg++;
qc-cursg_ofs = 0;
}
+
+ if (irq_handover) {
+ ap-pflags = ~ATA_PFLAG_HSM_WQ;
+ spin_unlock_irqrestore(ap-lock, irq_flags);
+ }
+
+ /* irq handler or another command might
+* be running at this point
+*/
}
/**
* ata_pio_sectors - Transfer one or many sectors.
* @qc: Command on going
+ * @irq_handover: workqueue is going to handover to the irq
[PATCH 5/8] libata: ack unsolicited INTRQ when polling
patch 5/8:
- Move the ATA_TFLAG_POLLING check from ata_interrupt to ata_host_intr
- Ack unsolicited INTRQ if polling and before the HSM accessing the port.
(e.g. some device asserts INTRQ even if polling and nIEN = 1.)
Signed-off-by: Albert Lee [EMAIL PROTECTED]
Cc: Alan Cox [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
---
1.
This fixes the Benq irq nobody cared problem.
(Some device asserts INTRQ even if nIEN = 1 and polling.
http://bugzilla.kernel.org/show_bug.cgi?id=8441)
Other unsolicited irqs that Alan concerned about will be address in patch 8/8.
2. Per Tejun's advice, I have checked the LLDDs.
The following LLDDs checks ATA_TFLAG_POLLING in their interrupt handler:
pdc_adma, sata_inic162x, sata_mv, sata_nv, sata_promise,
sata_qstor, sata_sil, sata_sx4 and sata_vsc.
After review, it seems non of them requires change due to this patch.
BTW, sata_sil and some other LLDDs already have such workaround in it.
diff -Nrup 04_move_narrow_lock/drivers/ata/libata-core.c
05_polling_ack/drivers/ata/libata-core.c
--- 04_move_narrow_lock/drivers/ata/libata-core.c 2007-05-16
13:53:16.0 +0800
+++ 05_polling_ack/drivers/ata/libata-core.c2007-05-16 13:53:20.0
+0800
@@ -5694,6 +5694,13 @@ inline unsigned int ata_host_intr (struc
if (ap-pflags ATA_PFLAG_HSM_WQ)
goto idle_irq;
+ /* polling, while HSM not yet active in wq */
+ if (qc-tf.flags ATA_TFLAG_POLLING) {
+ /* ack unsolicited irq */
+ ata_chk_status(ap);
+ goto idle_irq;
+ }
+
/* Check whether we are expecting interrupt in this state */
switch (ap-hsm_task_state) {
case HSM_ST_FIRST:
@@ -5804,8 +5811,7 @@ irqreturn_t ata_interrupt (int irq, void
struct ata_queued_cmd *qc;
qc = ata_qc_from_tag(ap, ap-active_tag);
- if (qc (!(qc-tf.flags ATA_TFLAG_POLLING))
- (qc-flags ATA_QCFLAG_ACTIVE))
+ if (qc (qc-flags ATA_QCFLAG_ACTIVE))
handled |= ata_host_intr(ap, qc);
}
}
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] libata: delegate irq driven pio to workqueue
patch 6/8:
- Delegate irq driven pio to workqueue.
- HSM_ST_LAST is kept in the interrupt since this is not CPU intensive.
(Mostly for DMA and non-data.)
Signed-off-by: Albert Lee [EMAIL PROTECTED]
Cc: Alan Cox [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
---
sata_sil is not changed to delegate to workqueue at this moment.
Maybe we can add such feature in the future if needed.
diff -Nrup 05_polling_ack/drivers/ata/libata-core.c
06_irq_wq/drivers/ata/libata-core.c
--- 05_polling_ack/drivers/ata/libata-core.c2007-05-16 13:53:20.0
+0800
+++ 06_irq_wq/drivers/ata/libata-core.c 2007-05-16 13:53:25.0 +0800
@@ -4864,20 +4864,15 @@ err_out:
static inline int ata_hsm_ok_in_wq(struct ata_port *ap, struct ata_queued_cmd
*qc)
{
- if (qc-tf.flags ATA_TFLAG_POLLING)
- return 1;
-
- if (ap-hsm_task_state == HSM_ST_FIRST) {
- if (qc-tf.protocol == ATA_PROT_PIO
- (qc-tf.flags ATA_TFLAG_WRITE))
- return 1;
-
- if (is_atapi_taskfile(qc-tf)
- !(qc-dev-flags ATA_DFLAG_CDB_INTR))
- return 1;
+ switch (qc-tf.protocol) {
+ case ATA_PROT_DMA:
+ return 0;
+ case ATA_PROT_ATAPI_DMA:
+ if (ap-hsm_task_state != HSM_ST_FIRST)
+ return 0;
}
- return 0;
+ return (ap-pflags ATA_PFLAG_HSM_WQ) ? 1 : 0;
}
/**
@@ -5217,6 +5212,39 @@ fsm_start:
}
/**
+ * ata_irq_task - queue task for irq pio
+ * @work: associated work_struct
+ *
+ * LOCKING:
+ * None.
+ */
+
+static void ata_irq_task(struct work_struct *work)
+{
+ struct ata_port *ap =
+ container_of(work, struct ata_port, port_task.work);
+ struct ata_queued_cmd *qc = ap-port_task_data;
+ u8 status;
+
+ WARN_ON(ap-hsm_task_state == HSM_ST_IDLE);
+
+ /* double check the device is not busy */
+ status = ata_chk_status(ap);
+ if (status ATA_BUSY) {
+ ata_port_printk(ap, KERN_ERR, Unexpected device busy
+ (Status 0x%x)\n, status);
+ return;
+ }
+
+ /* move the HSM */
+ ata_hsm_move(ap, qc, status, 1);
+
+ /* another command or interrupt handler
+* may be running at this point.
+*/
+}
+
+/**
* ata_qc_new - Request an available ATA command, for queueing
* @ap: Port associated with device @dev
* @dev: Device from whom we request an available command structure
@@ -5756,11 +5784,21 @@ inline unsigned int ata_host_intr (struc
/* ack bmdma irq events */
ap-ops-irq_clear(ap);
- ata_hsm_move(ap, qc, status, 0);
+ /* move the HSM */
+ switch (ap-hsm_task_state) {
+ case HSM_ST_FIRST:
+ case HSM_ST:
+ /* delegate PIO data transfer to workqueue */
+ ap-pflags |= ATA_PFLAG_HSM_WQ;
+ ata_port_queue_task(ap, ata_irq_task, qc, 0);
+ break;
+ default:
+ ata_hsm_move(ap, qc, status, 0);
- if (unlikely(qc-err_mask) (qc-tf.protocol == ATA_PROT_DMA ||
- qc-tf.protocol == ATA_PROT_ATAPI_DMA))
- ata_ehi_push_desc(ehi, BMDMA stat 0x%x, host_stat);
+ if (unlikely(qc-err_mask) (qc-tf.protocol == ATA_PROT_DMA
||
+ qc-tf.protocol ==
ATA_PROT_ATAPI_DMA))
+ ata_ehi_push_desc(ehi, BMDMA stat 0x%x, host_stat);
+ }
return 1; /* irq handled */
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] libata: fix ata_port_flush_task() for irq pio delegation
7/8:
Submitting tasks from ata_host_intr() breaks ata_port_flush_task().
This patch adds code to disable irq pio delegation to ata_port_flush_task().
Irq pio delegation is re-enabled when next qc is issued.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
---
Don't know if this is good enough. Maybe Tejun have better ideas.
diff -Nrup 06_irq_wq/drivers/ata/libata-core.c
07_irq_wq_fix/drivers/ata/libata-core.c
--- 06_irq_wq/drivers/ata/libata-core.c 2007-05-16 13:53:25.0 +0800
+++ 07_irq_wq_fix/drivers/ata/libata-core.c 2007-05-16 13:53:37.0
+0800
@@ -1321,6 +1321,7 @@ void ata_port_flush_task(struct ata_port
spin_lock_irqsave(ap-lock, flags);
ap-pflags |= ATA_PFLAG_FLUSH_PORT_TASK;
+ ap-pflags = ~ATA_PFLAG_IRQ_DELEGATE;
spin_unlock_irqrestore(ap-lock, flags);
DPRINTK(flush #1\n);
@@ -5604,6 +5605,9 @@ unsigned int ata_qc_issue_prot(struct at
(ap-flags ATA_FLAG_SETXFER_POLLING))
qc-tf.flags |= ATA_TFLAG_POLLING;
+ /* delegate data transfer of irq driven pio to workqueue */
+ ap-pflags |= ATA_PFLAG_IRQ_DELEGATE;
+
/* select the device */
ata_dev_select(ap, qc-dev-devno, 1, 0);
@@ -5788,9 +5792,12 @@ inline unsigned int ata_host_intr (struc
switch (ap-hsm_task_state) {
case HSM_ST_FIRST:
case HSM_ST:
- /* delegate PIO data transfer to workqueue */
- ap-pflags |= ATA_PFLAG_HSM_WQ;
- ata_port_queue_task(ap, ata_irq_task, qc, 0);
+ if (ap-pflags ATA_PFLAG_IRQ_DELEGATE) {
+ /* delegate PIO data transfer to workqueue */
+ ap-pflags |= ATA_PFLAG_HSM_WQ;
+ ata_port_queue_task(ap, ata_irq_task, qc, 0);
+ } else
+ ata_hsm_move(ap, qc, status, 0);
break;
default:
ata_hsm_move(ap, qc, status, 0);
diff -Nrup 06_irq_wq/include/linux/libata.h 07_irq_wq_fix/include/linux/libata.h
--- 06_irq_wq/include/linux/libata.h2007-05-14 14:45:28.0 +0800
+++ 07_irq_wq_fix/include/linux/libata.h2007-05-15 18:14:49.0
+0800
@@ -196,6 +196,7 @@ enum {
ATA_PFLAG_SUSPENDED = (1 17), /* port is suspended (power) */
ATA_PFLAG_PM_PENDING= (1 18), /* PM operation pending */
ATA_PFLAG_HSM_WQ= (1 19), /* hsm accessing the port in wq */
+ ATA_PFLAG_IRQ_DELEGATE = (1 20), /* delegate irq pio to wq */
/* struct ata_queued_cmd flags */
ATA_QCFLAG_ACTIVE = (1 0), /* cmd not yet ack'd to scsi lyer */
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] libata: ack more unsolicited INTRQ
patch 8/8: ack more unsolicited irq
Signed-off-by: Albert Lee [EMAIL PROTECTED]
Cc: Alan Cox [EMAIL PROTECTED]
Cc: Mark Lord [EMAIL PROTECTED]
Cc: Tejun Heo [EMAIL PROTECTED]
---
To address Alan's concern about the unsolicited irq (hopefully),
this patch tries to ack irq that happens after HSM accessing the port
but before the data transfer that actually triggers INTRQ.
For a typical transaction:
0. wait for irq or polling for device not busy
1. hsm starts accessing the port (ATA_PFLAG_HSM_WQ set)
2. hsm transfer the head part of the data
3. hsm acquires ap-lock
4. hsm transfer the tail part of the data (which may trigger INTRQ)
5. hsm clears ATA_PFLAG_HSM_WQ and release ap-lock
This patch allows the irq handler to ack irqs that occur during
1 and 2. (Another patch 5/8 acks irq before 1 and when polling.)
As previously discussed, the possible issue with this patch is:
Some ATA/ATAPI devices might be unhappy if the STATUS register
is read during data transfer (not sure if this is true or not).
(Patch 5/8 doesn't have such issue.)
diff -Nrup 07_irq_wq_fix/drivers/ata/libata-core.c
08_possible_ack/drivers/ata/libata-core.c
--- 07_irq_wq_fix/drivers/ata/libata-core.c 2007-05-16 13:53:37.0
+0800
+++ 08_possible_ack/drivers/ata/libata-core.c 2007-05-16 13:53:45.0
+0800
@@ -5723,8 +5723,14 @@ inline unsigned int ata_host_intr (struc
ap-print_id, qc-tf.protocol, ap-hsm_task_state);
/* HSM accessing the port from the workqueue */
- if (ap-pflags ATA_PFLAG_HSM_WQ)
+ if (ap-pflags ATA_PFLAG_HSM_WQ) {
+ /* HSM is not transfering the last piece
+* of data that triggers INTRQ yet.
+* ack unsolicited irq.
+*/
+ ata_chk_status(ap);
goto idle_irq;
+ }
/* polling, while HSM not yet active in wq */
if (qc-tf.flags ATA_TFLAG_POLLING) {
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 0/7] libata: push part of irq driven pio to workqueue
1/7: set the state after PIO data-in to HSM_ST_IDLE instead of HSM_ST_LAST 2/7: fix the ata_altstatus() in ata_hsm_qc_complete() 3/7: move ata_altstatus() out from ata_hsm_move() to the pio data xfer functions 4/7: move polling idle irq check to ata_host_intr() 5/7: move and reduce locking to the pio data xfer functions 6/7: push part of the irq driven pio out to workqueue 7/7: ack unexpected INTRQ when polling This is not complete yet, still needs to check if this creates new race conditions with EH or causing trouble to other part. Draft patch against 2.6.21.1 for your review. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/7] libata: move the ata_altstatus() in ata_hsm_qc_complete()
patch 2/7:
Calling ata_altstatus() after the qc completed looks incorrect.
Move it to before the qc is completed.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 01_last_idle/drivers/ata/libata-core.c
02_flush_fix/drivers/ata/libata-core.c
--- 01_last_idle/drivers/ata/libata-core.c 2007-05-11 10:24:16.0
+0800
+++ 02_flush_fix/drivers/ata/libata-core.c 2007-05-11 11:14:04.0
+0800
@@ -4329,6 +4329,8 @@ static void ata_hsm_qc_complete(struct a
struct ata_port *ap = qc-ap;
unsigned long flags;
+ ata_altstatus(ap); /* flush */
+
if (ap-ops-error_handler) {
if (in_wq) {
spin_lock_irqsave(ap-lock, flags);
@@ -4361,8 +4363,6 @@ static void ata_hsm_qc_complete(struct a
} else
ata_qc_complete(qc);
}
-
- ata_altstatus(ap); /* flush */
}
/**
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/7] libata: move ata_altstatus() out to the pio data xfer functions
patch 3/7:
move ata_altstatus() out from ata_hsm_move() to the pio data xfer functions.
Functions like ata_pio_sectors() and atapi_pio_bytes() know better if the
flush is needed.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 02_flush_fix/drivers/ata/libata-core.c
03_smart_flush/drivers/ata/libata-core.c
--- 02_flush_fix/drivers/ata/libata-core.c 2007-05-11 11:14:04.0
+0800
+++ 03_smart_flush/drivers/ata/libata-core.c2007-05-11 10:24:19.0
+0800
@@ -4031,7 +4031,7 @@ void ata_data_xfer_noirq(struct ata_devi
* Inherited from caller.
*/
-static void ata_pio_sector(struct ata_queued_cmd *qc)
+static void ata_pio_sector(struct ata_queued_cmd *qc, int last)
{
int do_write = (qc-tf.flags ATA_TFLAG_WRITE);
struct scatterlist *sg = qc-__sg;
@@ -4069,6 +4069,9 @@ static void ata_pio_sector(struct ata_qu
ap-ops-data_xfer(qc-dev, buf + offset, ATA_SECT_SIZE,
do_write);
}
+ if (last)
+ ata_altstatus(ap); /* flush */
+
qc-curbytes += ATA_SECT_SIZE;
qc-cursg_ofs += ATA_SECT_SIZE;
@@ -4100,9 +4103,9 @@ static void ata_pio_sectors(struct ata_q
nsect = min((qc-nbytes - qc-curbytes) / ATA_SECT_SIZE,
qc-dev-multi_count);
while (nsect--)
- ata_pio_sector(qc);
+ ata_pio_sector(qc, !nsect);
} else
- ata_pio_sector(qc);
+ ata_pio_sector(qc, 1);
}
/**
@@ -4185,6 +4188,8 @@ next_sg:
for (i = 0; i words; i++)
ap-ops-data_xfer(qc-dev, (unsigned char*)pad_buf, 2,
do_write);
+ ata_altstatus(ap); /* flush */
+
ap-hsm_task_state = HSM_ST_LAST;
return;
}
@@ -4234,6 +4239,8 @@ next_sg:
if (bytes)
goto next_sg;
+
+ ata_altstatus(ap); /* flush */
}
/**
@@ -4452,7 +4459,6 @@ fsm_start:
*/
ap-hsm_task_state = HSM_ST;
ata_pio_sectors(qc);
- ata_altstatus(ap); /* flush */
} else
/* send CDB */
atapi_send_cdb(ap, qc);
@@ -4533,7 +4539,6 @@ fsm_start:
if (!(qc-tf.flags ATA_TFLAG_WRITE)) {
ata_pio_sectors(qc);
- ata_altstatus(ap);
status = ata_wait_idle(ap);
}
@@ -4552,13 +4557,11 @@ fsm_start:
if (ap-hsm_task_state == HSM_ST_IDLE) {
/* all data read */
- ata_altstatus(ap);
status = ata_wait_idle(ap);
goto fsm_start;
}
}
- ata_altstatus(ap); /* flush */
poll_next = 1;
break;
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/7] libata: move polling idle irq check to ata_host_intr()
patch 4/7:
move the polling idle irq check from ata_interrupt() to ata_host_intr(),
where it makes more sense.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 03_smart_flush/drivers/ata/libata-core.c
04_polling_check/drivers/ata/libata-core.c
--- 03_smart_flush/drivers/ata/libata-core.c2007-05-11 10:24:19.0
+0800
+++ 04_polling_check/drivers/ata/libata-core.c 2007-05-11 10:25:09.0
+0800
@@ -5119,6 +5119,10 @@ inline unsigned int ata_host_intr (struc
VPRINTK(ata%u: protocol %d task_state %d\n,
ap-print_id, qc-tf.protocol, ap-hsm_task_state);
+ /* polling */
+ if (qc-tf.flags ATA_TFLAG_POLLING)
+ goto idle_irq;
+
/* Check whether we are expecting interrupt in this state */
switch (ap-hsm_task_state) {
case HSM_ST_FIRST:
@@ -5229,8 +5233,7 @@ irqreturn_t ata_interrupt (int irq, void
struct ata_queued_cmd *qc;
qc = ata_qc_from_tag(ap, ap-active_tag);
- if (qc (!(qc-tf.flags ATA_TFLAG_POLLING))
- (qc-flags ATA_QCFLAG_ACTIVE))
+ if (qc (qc-flags ATA_QCFLAG_ACTIVE))
handled |= ata_host_intr(ap, qc);
}
}
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/7] libata: move and reduce locking to the pio data xfer functions
patch 5/7:
- move the locking out from the ata_hsm_move() to the data xfer functions like
ata_pio_sectors().
- added the ATA_PFLAG_HSM_WQ (HSM running in workqueue) flag to serialize irq
and workqueue.
- the time holding ap-lock is reduced to last piece of pio transfer and
clearing the ATA_PFLAG_HSM_WQ flag
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 04_polling_check/drivers/ata/libata-core.c
05_narrow_lock/drivers/ata/libata-core.c
--- 04_polling_check/drivers/ata/libata-core.c 2007-05-11 10:25:09.0
+0800
+++ 05_narrow_lock/drivers/ata/libata-core.c2007-05-11 11:46:41.0
+0800
@@ -4031,7 +4031,7 @@ void ata_data_xfer_noirq(struct ata_devi
* Inherited from caller.
*/
-static void ata_pio_sector(struct ata_queued_cmd *qc, int last)
+static void ata_pio_sector(struct ata_queued_cmd *qc, int last, int lock)
{
int do_write = (qc-tf.flags ATA_TFLAG_WRITE);
struct scatterlist *sg = qc-__sg;
@@ -4039,6 +4039,7 @@ static void ata_pio_sector(struct ata_qu
struct page *page;
unsigned int offset;
unsigned char *buf;
+ unsigned long irq_flags = 0;
if (qc-curbytes == qc-nbytes - ATA_SECT_SIZE)
ap-hsm_task_state = do_write ? HSM_ST_LAST : HSM_ST_IDLE;
@@ -4055,6 +4056,9 @@ static void ata_pio_sector(struct ata_qu
if (PageHighMem(page)) {
unsigned long flags;
+ if (lock)
+ spin_lock_irqsave(ap-lock, irq_flags);
+
/* FIXME: use a bounce buffer */
local_irq_save(flags);
buf = kmap_atomic(page, KM_IRQ0);
@@ -4065,8 +4069,18 @@ static void ata_pio_sector(struct ata_qu
kunmap_atomic(buf, KM_IRQ0);
local_irq_restore(flags);
} else {
+ unsigned int head = 0, tail = ATA_SECT_SIZE;
+
buf = page_address(page);
- ap-ops-data_xfer(qc-dev, buf + offset, ATA_SECT_SIZE,
do_write);
+
+ if (lock) {
+ tail = 8;
+ head = ATA_SECT_SIZE - tail; /* multiple of 8 bytes */
+ ap-ops-data_xfer(qc-dev, buf + offset, head,
do_write);
+ spin_lock_irqsave(ap-lock, irq_flags);
+ }
+
+ ap-ops-data_xfer(qc-dev, buf + offset + head, tail,
do_write);
}
if (last)
@@ -4079,6 +4093,11 @@ static void ata_pio_sector(struct ata_qu
qc-cursg++;
qc-cursg_ofs = 0;
}
+
+ if (lock) {
+ ap-pflags = ~ATA_PFLAG_HSM_WQ;
+ spin_unlock_irqrestore(ap-lock, irq_flags);
+ }
}
/**
@@ -4092,7 +4111,7 @@ static void ata_pio_sector(struct ata_qu
* Inherited from caller.
*/
-static void ata_pio_sectors(struct ata_queued_cmd *qc)
+static void ata_pio_sectors(struct ata_queued_cmd *qc, int lock)
{
if (is_multi_taskfile(qc-tf)) {
/* READ/WRITE MULTIPLE */
@@ -4103,9 +4122,9 @@ static void ata_pio_sectors(struct ata_q
nsect = min((qc-nbytes - qc-curbytes) / ATA_SECT_SIZE,
qc-dev-multi_count);
while (nsect--)
- ata_pio_sector(qc, !nsect);
+ ata_pio_sector(qc, !nsect, lock !nsect);
} else
- ata_pio_sector(qc, 1);
+ ata_pio_sector(qc, 1, lock);
}
/**
@@ -4120,12 +4139,17 @@ static void ata_pio_sectors(struct ata_q
* caller.
*/
-static void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc)
+static void atapi_send_cdb(struct ata_port *ap, struct ata_queued_cmd *qc, int
lock)
{
+ unsigned long irq_flags = 0;
+
/* send SCSI cdb */
DPRINTK(send cdb\n);
WARN_ON(qc-dev-cdb_len 12);
+ if (lock)
+ spin_lock_irqsave(ap-lock, irq_flags);
+
ap-ops-data_xfer(qc-dev, qc-cdb, qc-dev-cdb_len, 1);
ata_altstatus(ap); /* flush */
@@ -4142,6 +4166,11 @@ static void atapi_send_cdb(struct ata_po
ap-ops-bmdma_start(qc);
break;
}
+
+ if (lock) {
+ ap-pflags = ~ATA_PFLAG_HSM_WQ;
+ spin_unlock_irqrestore(ap-lock, irq_flags);
+ }
}
/**
@@ -4156,7 +4185,7 @@ static void atapi_send_cdb(struct ata_po
*
*/
-static void __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes)
+static void __atapi_pio_bytes(struct ata_queued_cmd *qc, unsigned int bytes,
int lock)
{
int do_write = (qc-tf.flags ATA_TFLAG_WRITE);
struct scatterlist *sg = qc-__sg;
@@ -4164,6 +4193,8 @@ static void __atapi_pio_bytes(struct ata
struct page *page;
unsigned char *buf;
unsigned int offset, count;
+ unsigned long irq_flags = 0;
+ int transfer_lock = 0;
if (qc-curbytes + bytes = qc-nbytes)
ap-hsm_task_state = HSM_ST_LAST;
@@ -4181,6 +4212,10 @@ next_sg
[PATCH 6/7] libata: push part of the irq driven pio out to workqueue
patch 6/7:
Push part of the irq driven pio out to workqueue.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
It seems this creates new race condition with ata_exec_internal_sg() and EH?
diff -Nrup 05_narrow_lock/drivers/ata/libata-core.c
06_irq_wq/drivers/ata/libata-core.c
--- 05_narrow_lock/drivers/ata/libata-core.c2007-05-11 11:46:41.0
+0800
+++ 06_irq_wq/drivers/ata/libata-core.c 2007-05-11 11:54:56.0 +0800
@@ -4370,20 +4370,15 @@ err_out:
static inline int ata_hsm_ok_in_wq(struct ata_port *ap, struct ata_queued_cmd
*qc)
{
- if (qc-tf.flags ATA_TFLAG_POLLING)
- return 1;
-
- if (ap-hsm_task_state == HSM_ST_FIRST) {
- if (qc-tf.protocol == ATA_PROT_PIO
- (qc-tf.flags ATA_TFLAG_WRITE))
- return 1;
-
- if (is_atapi_taskfile(qc-tf)
- !(qc-dev-flags ATA_DFLAG_CDB_INTR))
- return 1;
+ switch (qc-tf.protocol) {
+ case ATA_PROT_DMA:
+ return 0;
+ case ATA_PROT_ATAPI_DMA:
+ if (ap-hsm_task_state != HSM_ST_FIRST)
+ return 0;
}
- return 0;
+ return (ap-pflags ATA_PFLAG_HSM_WQ) ? 1 : 0;
}
/**
@@ -4559,7 +4554,7 @@ fsm_start:
goto fsm_start;
}
- atapi_pio_bytes(qc, 0);
+ atapi_pio_bytes(qc, in_wq);
if (unlikely(ap-hsm_task_state == HSM_ST_ERR))
/* bad ireason reported by device */
@@ -4614,7 +4609,7 @@ fsm_start:
goto fsm_start;
}
- ata_pio_sectors(qc, 0);
+ ata_pio_sectors(qc, in_wq);
if (ap-hsm_task_state == HSM_ST_IDLE) {
/* all data read */
@@ -4713,6 +4708,31 @@ fsm_start:
goto fsm_start;
}
+static void ata_irq_task(struct work_struct *work)
+{
+ struct ata_port *ap =
+ container_of(work, struct ata_port, port_task.work);
+ struct ata_queued_cmd *qc = ap-port_task_data;
+ u8 status;
+
+ WARN_ON(ap-hsm_task_state == HSM_ST_IDLE);
+
+ /* double check the device is not busy */
+ status = ata_chk_status(ap);
+ if (status ATA_BUSY) {
+ ata_port_printk(ap, KERN_ERR, Unexpected device busy
+ (Status 0x%x)\n, status);
+ return;
+ }
+
+ /* move the HSM */
+ ata_hsm_move(ap, qc, status, 1);
+
+ /* another command or interrupt handler
+* may be running at this point.
+*/
+}
+
/**
* ata_qc_new - Request an available ATA command, for queueing
* @ap: Port associated with device @dev
@@ -5250,11 +5270,20 @@ inline unsigned int ata_host_intr (struc
/* ack bmdma irq events */
ap-ops-irq_clear(ap);
- ata_hsm_move(ap, qc, status, 0);
+ /* invoke HSM */
+ switch (ap-hsm_task_state) {
+ case HSM_ST_FIRST:
+ case HSM_ST:
+ ap-pflags |= ATA_PFLAG_HSM_WQ;
+ ata_port_queue_task(ap, ata_irq_task, qc, 0);
+ break;
+ default:
+ ata_hsm_move(ap, qc, status, 0);
- if (unlikely(qc-err_mask) (qc-tf.protocol == ATA_PROT_DMA ||
- qc-tf.protocol == ATA_PROT_ATAPI_DMA))
- ata_ehi_push_desc(ehi, BMDMA stat 0x%x, host_stat);
+ if (unlikely(qc-err_mask) (qc-tf.protocol == ATA_PROT_DMA
||
+ qc-tf.protocol ==
ATA_PROT_ATAPI_DMA))
+ ata_ehi_push_desc(ehi, BMDMA stat 0x%x, host_stat);
+ }
return 1; /* irq handled */
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/7] libata: ack unexpected INTRQ when polling
patch 7/7:
ack unexpected INTRQ when polling.
(Some device asserts INTRQ even if polling and nIEN = 1.
http://bugzilla.kernel.org/show_bug.cgi?id=8441)
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
diff -Nrup 06_irq_wq/drivers/ata/libata-core.c
07_ack_random_irq/drivers/ata/libata-core.c
--- 06_irq_wq/drivers/ata/libata-core.c 2007-05-11 11:54:56.0 +0800
+++ 07_ack_random_irq/drivers/ata/libata-core.c 2007-05-11 15:40:37.0
+0800
@@ -5211,9 +5211,12 @@ inline unsigned int ata_host_intr (struc
if (ap-pflags ATA_PFLAG_HSM_WQ)
goto idle_irq;
- /* polling */
- if (qc-tf.flags ATA_TFLAG_POLLING)
+ /* polling, while HSM not yet active in wq */
+ if (qc-tf.flags ATA_TFLAG_POLLING) {
+ /* ack random irq */
+ ata_chk_status(ap);
goto idle_irq;
+ }
/* Check whether we are expecting interrupt in this state */
switch (ap-hsm_task_state) {
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] libata: move polling idle irq check to ata_host_intr()
Tejun Heo wrote:
diff -Nrup 03_smart_flush/drivers/ata/libata-core.c
04_polling_check/drivers/ata/libata-core.c
--- 03_smart_flush/drivers/ata/libata-core.c 2007-05-11 10:24:19.0
+0800
+++ 04_polling_check/drivers/ata/libata-core.c2007-05-11
10:25:09.0 +0800
@@ -5119,6 +5119,10 @@ inline unsigned int ata_host_intr (struc
VPRINTK(ata%u: protocol %d task_state %d\n,
ap-print_id, qc-tf.protocol, ap-hsm_task_state);
+ /* polling */
+ if (qc-tf.flags ATA_TFLAG_POLLING)
+ goto idle_irq;
+
/* Check whether we are expecting interrupt in this state */
switch (ap-hsm_task_state) {
case HSM_ST_FIRST:
@@ -5229,8 +5233,7 @@ irqreturn_t ata_interrupt (int irq, void
struct ata_queued_cmd *qc;
qc = ata_qc_from_tag(ap, ap-active_tag);
- if (qc (!(qc-tf.flags ATA_TFLAG_POLLING))
- (qc-flags ATA_QCFLAG_ACTIVE))
+ if (qc (qc-flags ATA_QCFLAG_ACTIVE))
There are several LLD specific IRQ handlers which have similar part.
Care to update them together?
Sure, will do.
--
albert
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] libata: move and reduce locking to the pio data xfer functions
Tejun Heo wrote:
Albert Lee wrote:
-static void ata_pio_sector(struct ata_queued_cmd *qc, int last)
+static void ata_pio_sector(struct ata_queued_cmd *qc, int last, int lock)
I think the naming of @lock is a bit confusing here. @clr_hsm_wq or
@last_sector, maybe?
How about irq_handover? When set to true, it means the workqueue is going to
handover the control of the port to the irq handler.
+ if (lock) {
+ tail = 8;
+ head = ATA_SECT_SIZE - tail; /* multiple of 8 bytes */
+ ap-ops-data_xfer(qc-dev, buf + offset, head,
do_write);
+ spin_lock_irqsave(ap-lock, irq_flags);
+ }
+
+ ap-ops-data_xfer(qc-dev, buf + offset + head, tail,
do_write);
Aieee, we have to transfer the whole last sector while holding the spin
lock and IRQ disabled. That's sad but pushing locking into -data_xfer
doesn't sound attractive either. Any better ideas?
Why need to transfer the last sector as a whole?
Spliting it into 504 (unlocked) + 8 (holding ap-lock) works on my machine...
--
albert
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)
Tejun Heo wrote: I guess it's about time to listen what Jeff thinks. Albert, if Jeff agrees with pushing HSM or data transfer to workqueue, are you interested in doing that? The HSM is your baby after all. :-) Moving HSM task to the workqueue looks good idea. The mouse cursor of my X desktop doesn't move smoothly when blocks of PIO are going in hard irq. Hopefully we can do better with that. Yes, I am interested in doing it, if Jeff also ack. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: disable_irq() during polling IDENTIFY
Tejun Heo wrote:
[cc'ing Bartlomiej and Mark, hi]
Hello, Albert.
Albert Lee wrote:
Problem:
Kernel got irq 5: nobody cared when using
libata + polling IDENTIFY + Promise 20275 adapter + Benq DW1620 drive.
Detail message available in bug 8441
(http://bugzilla.kernel.org/show_bug.cgi?id=8441).
Cause:
The Benq DW1620 drive raises INTRQ during polling IDENTIFY PACKET DEVICE,
even if nIEN = 1.
Ai...
Proposed fix:
disable_irq() during polling IDENTIFY to work around, the same as what IDE
subsystem does.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Some controller like Intel ICH4 is immune from the problem, with the same
kernel
and the same Benq DW1620 drive. So, adding the ATA_FLAG_IDENT_IRQ_OFF flag for
those adapters that needs the workaround. Patch against 2.6.21.1 for your
review, thanks.
I guess piix is masking the interrupt at the host side.
Another interesting aspect is that the SATA spec says the device is
recommended to ignore nIEN while the controller is recommended to not
set nIEN when it sends FIS to the device. ie. nIEN should be
implemented on the host controller. I bet there are controllers out
there which doesn't do host-side masking and there will be more and more
devices which ignore nIEN, so we're likely to see similar problems on
SATA too.
+ /* Disable IRQ since some devices like Benq DW1620 raises INTRQ
+ * when IDENTIFY PACKET DEVICE, even with polling IDENTIFY.
+ */
+ if (ap-flags ATA_FLAG_IDENT_IRQ_OFF) {
+ if (host-irq)
+ disable_irq(host-irq);
+
+ if (host-irq2)
+ disable_irq(host-irq2);
+ }
+
err_mask = ata_exec_internal(dev, tf, NULL, DMA_FROM_DEVICE,
id, sizeof(id[0]) * ATA_ID_WORDS);
+
+ /* Re-enable IRQ */
+ if (ap-flags ATA_FLAG_IDENT_IRQ_OFF) {
+ if (host-irq)
+ enable_irq(host-irq);
+
+ if (host-irq2)
+ enable_irq(host-irq2);
+ }
+
Yeap, this is how IDE deals with polling commands but I'm not sure how
it's supposed to work with PCI IRQ sharing. Bartlomiej, can you
enlighten me here?
Also, this is a problem for not only IDENTIFY but all polling commands.
Yes, other command might also assert INTRQ during polling.
However, for the specific BENQ DW1620 drive, only IDENTIFY_PACKET_DEVICE
has such behavior; other commands like READ or REQUEST_SENSE are ok.
One solution I can think of is to let IRQ handler ack IRQ
unconditionally during polling commands - ie. just read the TF Status
register once and tell the IRQ subsystem that the IRQ is handled. This
shouldn't affect the operation of polling as the only side effect of
reading Status is clearing pending IRQ will give us a nice way to
deal with the SATA bridge chip which chokes on nIEN. Considering the
sorry state of nIEN in SATA, I guess this might be the best way to deal
with this.
Albert, can you please test whether this works? Modifying
ata_interrupt() such that it reads TF Status if ATA_TFLAG_POLLING should
do the trick.
Yes, reading the Status register and acking interrupt also fixes the
problem (patch attached below).
--
albert
diff -Nrup linux-2.6.21.1-ori/drivers/ata/libata-core.c
linux-2.6.21.1-mod3/drivers/ata/libata-core.c
--- linux-2.6.21.1-ori/drivers/ata/libata-core.c2007-05-04
11:22:23.0 +0800
+++ linux-2.6.21.1-mod3/drivers/ata/libata-core.c 2007-05-07
17:44:21.0 +0800
@@ -5224,9 +5224,14 @@ irqreturn_t ata_interrupt (int irq, void
struct ata_queued_cmd *qc;
qc = ata_qc_from_tag(ap, ap-active_tag);
- if (qc (!(qc-tf.flags ATA_TFLAG_POLLING))
- (qc-flags ATA_QCFLAG_ACTIVE))
- handled |= ata_host_intr(ap, qc);
+ if (qc (qc-flags ATA_QCFLAG_ACTIVE)) {
+ if (qc-tf.flags ATA_TFLAG_POLLING) {
+ ata_chk_status(ap);
+ handled = 1;
+ } else {
+ handled |= ata_host_intr(ap, qc);
+ }
+ }
}
}
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] libata: disable_irq() during polling IDENTIFY
Tejun Heo wrote:
Hello, Albert.
Albert Lee wrote:
Tejun Heo wrote:
Also, this is a problem for not only IDENTIFY but all polling commands.
Yes, other command might also assert INTRQ during polling.
However, for the specific BENQ DW1620 drive, only IDENTIFY_PACKET_DEVICE
has such behavior; other commands like READ or REQUEST_SENSE are ok.
Oh I see.
One solution I can think of is to let IRQ handler ack IRQ
unconditionally during polling commands - ie. just read the TF Status
register once and tell the IRQ subsystem that the IRQ is handled. This
shouldn't affect the operation of polling as the only side effect of
reading Status is clearing pending IRQ will give us a nice way to
deal with the SATA bridge chip which chokes on nIEN. Considering the
sorry state of nIEN in SATA, I guess this might be the best way to deal
with this.
Albert, can you please test whether this works? Modifying
ata_interrupt() such that it reads TF Status if ATA_TFLAG_POLLING should
do the trick.
Yes, reading the Status register and acking interrupt also fixes the
problem (patch attached below).
Good to know it works. With or without nIEN, I think this change is a
good thing to have. IRQ handler shouldn't interfere with polling as
both acquire lock during operation.
This reminds me of a possible issue with the patch:
Previously the polling code assumes that the interrupt handler won't interfere
with it and the polling code runs without holding ap-lock.
However, with the above patch, the interrupt handler might read the
Status register when the polling code is transfering data, etc. from the port.
Would this cause trouble to the ATA/ATAPI devices?
Should we have something like ATA_PFLAG_HSM_BUSY below, such that the interrupt
handler won't read the Status register when HSM is busy accessing the port?
--
albert
(Revised patch: Don't read the Status register when HSM is busy accessing the
port)
diff -Nrup linux-2.6.21.1-ori/drivers/ata/libata-core.c
linux-2.6.21.1-mod3/drivers/ata/libata-core.c
--- linux-2.6.21.1-ori/drivers/ata/libata-core.c2007-05-04
11:22:23.0 +0800
+++ linux-2.6.21.1-mod3/drivers/ata/libata-core.c 2007-05-07
19:17:58.0 +0800
@@ -4389,6 +4389,14 @@ int ata_hsm_move(struct ata_port *ap, st
*/
WARN_ON(in_wq != ata_hsm_ok_in_wq(ap, qc));
+ if (in_wq) {
+ spin_lock_irqsave(ap-lock, flags);
+ ap-pflags |= ATA_PFLAG_HSM_BUSY;
+ spin_unlock_irqrestore(ap-lock, flags);
+ } else {
+ ap-pflags |= ATA_PFLAG_HSM_BUSY;
+ }
+
fsm_start:
DPRINTK(ata%u: protocol %d task_state %d (dev_stat 0x%X)\n,
ap-print_id, qc-tf.protocol, ap-hsm_task_state, status);
@@ -4600,6 +4608,14 @@ fsm_start:
BUG();
}
+ if (in_wq) {
+ spin_lock_irqsave(ap-lock, flags);
+ ap-pflags = ~ATA_PFLAG_HSM_BUSY;
+ spin_unlock_irqrestore(ap-lock, flags);
+ } else {
+ ap-pflags = ~ATA_PFLAG_HSM_BUSY;
+ }
+
return poll_next;
}
@@ -5224,9 +5240,14 @@ irqreturn_t ata_interrupt (int irq, void
struct ata_queued_cmd *qc;
qc = ata_qc_from_tag(ap, ap-active_tag);
- if (qc (!(qc-tf.flags ATA_TFLAG_POLLING))
- (qc-flags ATA_QCFLAG_ACTIVE))
- handled |= ata_host_intr(ap, qc);
+ if (qc (qc-flags ATA_QCFLAG_ACTIVE)) {
+ if (!(qc-tf.flags ATA_TFLAG_POLLING)) {
+ handled |= ata_host_intr(ap, qc);
+ } else if (!(ap-pflags ATA_PFLAG_HSM_BUSY)) {
+ ata_chk_status(ap);
+ handled = 1;
+ }
+ }
}
}
diff -Nrup linux-2.6.21.1-ori/include/linux/libata.h
linux-2.6.21.1-mod3/include/linux/libata.h
--- linux-2.6.21.1-ori/include/linux/libata.h 2007-04-28 05:49:26.0
+0800
+++ linux-2.6.21.1-mod3/include/linux/libata.h 2007-05-07 18:41:01.0
+0800
@@ -195,6 +195,7 @@ enum {
ATA_PFLAG_FLUSH_PORT_TASK = (1 16), /* flush port task */
ATA_PFLAG_SUSPENDED = (1 17), /* port is suspended (power) */
ATA_PFLAG_PM_PENDING= (1 18), /* PM operation pending */
+ ATA_PFLAG_HSM_BUSY = (1 19), /* HSM accessing the port */
/* struct ata_queued_cmd flags */
ATA_QCFLAG_ACTIVE = (1 0), /* cmd not yet ack'd to scsi lyer */
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libata: disable_irq() during polling IDENTIFY
Problem:
Kernel got irq 5: nobody cared when using
libata + polling IDENTIFY + Promise 20275 adapter + Benq DW1620 drive.
Detail message available in bug 8441
(http://bugzilla.kernel.org/show_bug.cgi?id=8441).
Cause:
The Benq DW1620 drive raises INTRQ during polling IDENTIFY PACKET DEVICE,
even if nIEN = 1.
Proposed fix:
disable_irq() during polling IDENTIFY to work around, the same as what IDE
subsystem does.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Some controller like Intel ICH4 is immune from the problem, with the same kernel
and the same Benq DW1620 drive. So, adding the ATA_FLAG_IDENT_IRQ_OFF flag for
those adapters that needs the workaround. Patch against 2.6.21.1 for your
review, thanks.
diff -Nrup linux-2.6.21.1-ori/drivers/ata/libata-core.c
linux-2.6.21.1-mod2/drivers/ata/libata-core.c
--- linux-2.6.21.1-ori/drivers/ata/libata-core.c2007-05-04
11:22:23.0 +0800
+++ linux-2.6.21.1-mod2/drivers/ata/libata-core.c 2007-05-07
11:00:02.0 +0800
@@ -1427,6 +1427,7 @@ int ata_dev_read_id(struct ata_device *d
unsigned int flags, u16 *id)
{
struct ata_port *ap = dev-ap;
+ struct ata_host *host = ap-host;
unsigned int class = *p_class;
struct ata_taskfile tf;
unsigned int err_mask = 0;
@@ -1466,8 +1467,29 @@ int ata_dev_read_id(struct ata_device *d
*/
tf.flags |= ATA_TFLAG_POLLING;
+ /* Disable IRQ since some devices like Benq DW1620 raises INTRQ
+* when IDENTIFY PACKET DEVICE, even with polling IDENTIFY.
+*/
+ if (ap-flags ATA_FLAG_IDENT_IRQ_OFF) {
+ if (host-irq)
+ disable_irq(host-irq);
+
+ if (host-irq2)
+ disable_irq(host-irq2);
+ }
+
err_mask = ata_exec_internal(dev, tf, NULL, DMA_FROM_DEVICE,
id, sizeof(id[0]) * ATA_ID_WORDS);
+
+ /* Re-enable IRQ */
+ if (ap-flags ATA_FLAG_IDENT_IRQ_OFF) {
+ if (host-irq)
+ enable_irq(host-irq);
+
+ if (host-irq2)
+ enable_irq(host-irq2);
+ }
+
if (err_mask) {
if (err_mask AC_ERR_NODEV_HINT) {
DPRINTK(ata%u.%d: NODEV after polling detection\n,
diff -Nrup linux-2.6.21.1-ori/drivers/ata/pata_pdc2027x.c
linux-2.6.21.1-mod2/drivers/ata/pata_pdc2027x.c
--- linux-2.6.21.1-ori/drivers/ata/pata_pdc2027x.c 2007-04-28
05:49:26.0 +0800
+++ linux-2.6.21.1-mod2/drivers/ata/pata_pdc2027x.c 2007-05-07
10:44:33.0 +0800
@@ -214,7 +214,7 @@ static struct ata_port_info pdc2027x_por
{
.sht= pdc2027x_sht,
.flags = ATA_FLAG_NO_LEGACY | ATA_FLAG_SLAVE_POSS |
- ATA_FLAG_MMIO,
+ ATA_FLAG_MMIO | ATA_FLAG_IDENT_IRQ_OFF,
.pio_mask = 0x1f, /* pio0-4 */
.mwdma_mask = 0x07, /* mwdma0-2 */
.udma_mask = ATA_UDMA5, /* udma0-5 */
@@ -224,7 +224,7 @@ static struct ata_port_info pdc2027x_por
{
.sht= pdc2027x_sht,
.flags = ATA_FLAG_NO_LEGACY | ATA_FLAG_SLAVE_POSS |
- ATA_FLAG_MMIO,
+ ATA_FLAG_MMIO | ATA_FLAG_IDENT_IRQ_OFF,
.pio_mask = 0x1f, /* pio0-4 */
.mwdma_mask = 0x07, /* mwdma0-2 */
.udma_mask = ATA_UDMA6, /* udma0-6 */
diff -Nrup linux-2.6.21.1-ori/include/linux/libata.h
linux-2.6.21.1-mod2/include/linux/libata.h
--- linux-2.6.21.1-ori/include/linux/libata.h 2007-04-28 05:49:26.0
+0800
+++ linux-2.6.21.1-mod2/include/linux/libata.h 2007-05-07 12:25:05.0
+0800
@@ -174,6 +174,7 @@ enum {
ATA_FLAG_SETXFER_POLLING= (1 14), /* use polling for SETXFER */
ATA_FLAG_IGN_SIMPLEX= (1 15), /* ignore SIMPLEX */
ATA_FLAG_NO_IORDY = (1 16), /* controller lacks iordy */
+ ATA_FLAG_IDENT_IRQ_OFF = (1 17), /* disable irq when IDENTIFY */
/* The following flag belongs to ap-pflags but is kept in
* ap-flags because it's referenced in many LLDs and will be
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: CD problems booting FC7 RC3
Stephen Clark wrote: Hi, When trying to boot FC7 RC3 on my HP N5430 laptop I get the following: ... Write protecting the kernel read-only data: 845k input: PS/2 Generic Mouse as /class/input/input2 SCSI subsystem initialized libata version 2.20 loaded. ACPI: Unable to derive IRQ for device :00:0f.0 ACPI: PCI Interrupt :00:0f.0[A]: no GSI ata1: PATA max UDMA/100 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x00011000 irq 14 ata2: PATA max UDMA/100 cmd 0x00010170 ctl 0x00010376 bmdma 0x00011008 irq 15 scsi0 : pata_ali PM: Adding info for No Bus:host0 ata1.00: ATA-5: HITACHI_DK23CA-20, 00H1A0A3, max UDMA/100 ata1.00: 39070080 sectors, multi 16: LBA ata1.00: configured for UDMA/33 TCP bic registered Initializing XFRM netlink socket NET: Registered protocol family 1 NET: Registered protocol family 17 powernow: PowerNOW! Technology present. Can scale: frequency and voltage. powernow: SGTC: 1 Initializing XFRM netlink socket NET: Registered protocol family 1 NET: Registered protocol family 17 powernow: PowerNOW! Technology present. Can scale: frequency and voltage. powernow: SGTC: 1 powernow: Minimum speed 300 MHz. Maximum speed 850 MHz. powernow-k8: Processor cpuid 670 not supported Using IPI No-Shortcut mode Magic number: 3:342:541 Freeing unused kernel memory: 248k freed Write protecting the kernel read-only data: 845k input: PS/2 Generic Mouse as /class/input/input2 SCSI subsystem initialized libata version 2.20 loaded. ACPI: Unable to derive IRQ for device :00:0f.0 ACPI: PCI Interrupt :00:0f.0[A]: no GSI ata1: PATA max UDMA/100 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x00011000 irq 14 ata2: PATA max UDMA/100 cmd 0x00010170 ctl 0x00010376 bmdma 0x00011008 irq 15 scsi0 : pata_ali PM: Adding info for No Bus:host0 ata1.00: ATA-5: HITACHI_DK23CA-20, 00H1A0A3, max UDMA/100 ata1.00: 39070080 sectors, multi 16: LBA ata1.00: configured for UDMA/33 scsi1 : pata_ali PM: Adding info for No Bus:host1 ata2.00: ATAPI, max UDMA/33 ata2.00: configured for UDMA/33 PM: Adding info for No Bus:target0:0:0 scsi 0:0:0:0: Direct-Access ATA HITACHI_DK23CA-2 00H1 PQ: 0 ANSI: 5 PM: Adding info for scsi:0:0:0:0 PM: Adding info for No Bus:target1:0:0 ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x2 frozen ata2.00: cmd a0/01:00:00:00:00/00:00:00:00:00/a0 tag 0 cdb 0x12 data 36 in res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout) Looks similar to the AOpen timeout problem (http://bugzilla.kernel.org/show_bug.cgi?id=8244) Please try the following patch (http://marc.info/?l=linux-idem=117548454130046q=raw) Thanks, Albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] libata: Workaround/fixes for ATAPI devices (take 3)
patch 1/4: Reorder HSM_ST_FIRST patch 2/4: Clear tf before doing request sense patch 3/4: Limit max sector to 128 for TORiSAN DVD drives patch 4/4: Limit ATAPI DMA to R/W commands only for TORiSAN DVD drives (patch 2/4 revised per Tejun's advice.) (patch 3/4 revised per Vlad's new test result.) - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] libata: reorder HSM_ST_FIRST for easier decoding (take 3)
patch 1/4:
Reorder HSM_ST_FIRST, such that the task state transition is easier decoded
with human eyes.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 00_libata-dev.ori/include/linux/libata.h
01_hsm_st/include/linux/libata.h
--- 00_libata-dev.ori/include/linux/libata.h2007-03-23 16:56:24.0
+0800
+++ 01_hsm_st/include/linux/libata.h2007-04-02 10:50:50.0 +0800
@@ -315,11 +315,11 @@ enum {
enum hsm_task_states {
HSM_ST_IDLE,/* no command on going */
+ HSM_ST_FIRST, /* (waiting the device to)
+ write CDB or first data block */
HSM_ST, /* (waiting the device to) transfer data */
HSM_ST_LAST,/* (waiting the device to) complete command */
HSM_ST_ERR, /* error */
- HSM_ST_FIRST, /* (waiting the device to)
- write CDB or first data block */
};
enum ata_completion_errors {
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] libata: Clear tf before doing request sense (take 3)
patch 2/4:
Clear tf before doing request sense.
This fixes the AOpen 56X/AKH timeout problem.
(http://bugzilla.kernel.org/show_bug.cgi?id=8244)
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Per Tejun's advice to use result_tf instead.
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 01_hsm_st/drivers/ata/libata-eh.c 02_aopen_rs/drivers/ata/libata-eh.c
--- 01_hsm_st/drivers/ata/libata-eh.c 2007-03-23 16:56:13.0 +0800
+++ 02_aopen_rs/drivers/ata/libata-eh.c 2007-04-02 11:04:11.0 +0800
@@ -982,26 +982,27 @@ static int ata_eh_read_log_10h(struct at
* RETURNS:
* 0 on success, AC_ERR_* mask on failure
*/
-static unsigned int atapi_eh_request_sense(struct ata_device *dev,
- unsigned char *sense_buf)
+static unsigned int atapi_eh_request_sense(struct ata_queued_cmd *qc)
{
+ struct ata_device *dev = qc-dev;
+ unsigned char *sense_buf = qc-scsicmd-sense_buffer;
struct ata_port *ap = dev-ap;
struct ata_taskfile tf;
u8 cdb[ATAPI_CDB_LEN];
DPRINTK(ATAPI request sense\n);
- ata_tf_init(dev, tf);
-
/* FIXME: is this needed? */
memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);
- /* XXX: why tf_read here? */
- ap-ops-tf_read(ap, tf);
-
- /* fill these in, for the case where they are -not- overwritten */
+ /* initialize sense_buf with the error register,
+* for the case where they are -not- overwritten
+*/
sense_buf[0] = 0x70;
- sense_buf[2] = tf.feature 4;
+ sense_buf[2] = qc-result_tf.feature 4;
+
+ /* some devices time out if garbage left in tf */
+ ata_tf_init(dev, tf);
memset(cdb, 0, ATAPI_CDB_LEN);
cdb[0] = REQUEST_SENSE;
@@ -1165,8 +1166,7 @@ static unsigned int ata_eh_analyze_tf(st
case ATA_DEV_ATAPI:
if (!(qc-ap-pflags ATA_PFLAG_FROZEN)) {
- tmp = atapi_eh_request_sense(qc-dev,
-qc-scsicmd-sense_buffer);
+ tmp = atapi_eh_request_sense(qc);
if (!tmp) {
/* ATA_QCFLAG_SENSE_VALID is used to
* tell atapi_qc_complete() that sense
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] libata: Limit max sector to 128 for TORiSAN DVD drives (take 3)
patch 3/4:
The TORiSAN drive locks up when max sector == 256.
Limit max sector to 128 for the TORiSAN DRD-N216 drives.
(http://bugzilla.kernel.org/show_bug.cgi?id=6710)
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Revised to use max sector = 128 per Vlad's new test result.
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 02_aopen_rs/drivers/ata/libata-core.c
03_max_sect/drivers/ata/libata-core.c
--- 02_aopen_rs/drivers/ata/libata-core.c 2007-03-23 16:56:13.0
+0800
+++ 03_max_sect/drivers/ata/libata-core.c 2007-04-02 10:56:59.0
+0800
@@ -1784,6 +1784,9 @@ int ata_dev_configure(struct ata_device
dev-max_sectors = ATA_MAX_SECTORS;
}
+ if (ata_device_blacklisted(dev) ATA_HORKAGE_MAX_SEC_128)
+ dev-max_sectors = min(ATA_MAX_SECTORS_128, dev-max_sectors);
+
if (ap-ops-dev_config)
ap-ops-dev_config(ap, dev);
@@ -3352,6 +3355,9 @@ static const struct ata_blacklist_entry
{ _NEC DV5800A, NULL, ATA_HORKAGE_NODMA },
{ SAMSUNG CD-ROM SN-124,N001, ATA_HORKAGE_NODMA },
+ /* Weird ATAPI devices */
+ { TORiSAN DVD-ROM DRD-N216, NULL, ATA_HORKAGE_MAX_SEC_128 },
+
/* Devices we expect to fail diagnostics */
/* Devices where NCQ should be avoided */
diff -Nrup 02_aopen_rs/include/linux/ata.h 03_max_sect/include/linux/ata.h
--- 02_aopen_rs/include/linux/ata.h 2007-03-23 16:56:24.0 +0800
+++ 03_max_sect/include/linux/ata.h 2007-04-02 10:56:59.0 +0800
@@ -40,6 +40,7 @@ enum {
ATA_MAX_DEVICES = 2,/* per bus/port */
ATA_MAX_PRD = 256, /* we could make these 256/256 */
ATA_SECT_SIZE = 512,
+ ATA_MAX_SECTORS_128 = 128,
ATA_MAX_SECTORS = 256,
ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */
diff -Nrup 02_aopen_rs/include/linux/libata.h 03_max_sect/include/linux/libata.h
--- 02_aopen_rs/include/linux/libata.h 2007-04-02 10:50:50.0 +0800
+++ 03_max_sect/include/linux/libata.h 2007-04-02 10:56:59.0 +0800
@@ -311,6 +311,7 @@ enum {
ATA_HORKAGE_DIAGNOSTIC = (1 0), /* Failed boot diag */
ATA_HORKAGE_NODMA = (1 1), /* DMA problems */
ATA_HORKAGE_NONCQ = (1 2), /* Don't use NCQ */
+ ATA_HORKAGE_MAX_SEC_128 = (1 3), /* Limit max sects to 128 */
};
enum hsm_task_states {
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] libata: Workaround/fixes for ATAPI devices
patch 1/4: Reorder HSM_ST_FIRST patch 2/4: Clear tf before doing request sense patch 3/4: Limit max sector to 240 for TORiSAN DVD drives patch 4/4: Limit ATAPI DMA to R/W commands only for TORiSAN DVD drives - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] libata: reorder HSM_ST_FIRST for easier decoding
patch 1/4:
Reorder HSM_ST_FIRST, such that the task state transition is easier decoded
with human eyes.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 00_libata-dev.ori/include/linux/libata.h
01_hsm_st/include/linux/libata.h
--- 00_libata-dev.ori/include/linux/libata.h2007-03-23 16:56:24.0
+0800
+++ 01_hsm_st/include/linux/libata.h2007-03-26 12:33:28.0 +0800
@@ -315,11 +315,11 @@ enum {
enum hsm_task_states {
HSM_ST_IDLE,/* no command on going */
+ HSM_ST_FIRST, /* (waiting the device to)
+ write CDB or first data block */
HSM_ST, /* (waiting the device to) transfer data */
HSM_ST_LAST,/* (waiting the device to) complete command */
HSM_ST_ERR, /* error */
- HSM_ST_FIRST, /* (waiting the device to)
- write CDB or first data block */
};
enum ata_completion_errors {
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] libata: Clear tf before doing request sense
patch 2/4: Clear tf before doing request sense. This fixes the AOpen 56X/AKH timeout problem. (http://bugzilla.kernel.org/show_bug.cgi?id=8244) Signed-off-by: Albert Lee [EMAIL PROTECTED] --- Patch against libata-dev tree, for your review, thanks. diff -Nrup 01_hsm_st/drivers/ata/libata-eh.c 02_aopen_rs/drivers/ata/libata-eh.c --- 01_hsm_st/drivers/ata/libata-eh.c 2007-03-23 16:56:13.0 +0800 +++ 02_aopen_rs/drivers/ata/libata-eh.c 2007-03-31 01:11:01.0 +0800 @@ -991,18 +991,19 @@ static unsigned int atapi_eh_request_sen DPRINTK(ATAPI request sense\n); - ata_tf_init(dev, tf); - /* FIXME: is this needed? */ memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE); - /* XXX: why tf_read here? */ + /* read error register to initialize sense_buf */ ap-ops-tf_read(ap, tf); /* fill these in, for the case where they are -not- overwritten */ sense_buf[0] = 0x70; sense_buf[2] = tf.feature 4; + /* some devices time out if garbage left in tf */ + ata_tf_init(dev, tf); + memset(cdb, 0, ATAPI_CDB_LEN); cdb[0] = REQUEST_SENSE; cdb[4] = SCSI_SENSE_BUFFERSIZE; - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] libata: Limit max sector to 240 for TORiSAN DVD drives
patch 3/4:
The TORiSAN drive locks up when max sector == 256.
Limit max sector to 240 for TORiSAN DRD-N216 drives.
(http://bugzilla.kernel.org/show_bug.cgi?id=6710)
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 02_aopen_rs/drivers/ata/libata-core.c
03_max_sect/drivers/ata/libata-core.c
--- 02_aopen_rs/drivers/ata/libata-core.c 2007-03-23 16:56:13.0
+0800
+++ 03_max_sect/drivers/ata/libata-core.c 2007-03-31 14:11:33.0
+0800
@@ -1784,6 +1784,9 @@ int ata_dev_configure(struct ata_device
dev-max_sectors = ATA_MAX_SECTORS;
}
+ if (ata_device_blacklisted(dev) ATA_HORKAGE_MAX_SEC_240)
+ dev-max_sectors = min(ATA_MAX_SECTORS_240, dev-max_sectors);
+
if (ap-ops-dev_config)
ap-ops-dev_config(ap, dev);
@@ -3352,6 +3355,9 @@ static const struct ata_blacklist_entry
{ _NEC DV5800A, NULL, ATA_HORKAGE_NODMA },
{ SAMSUNG CD-ROM SN-124,N001, ATA_HORKAGE_NODMA },
+ /* Weird ATAPI devices */
+ { TORiSAN DVD-ROM DRD-N216, NULL, ATA_HORKAGE_MAX_SEC_240 },
+
/* Devices we expect to fail diagnostics */
/* Devices where NCQ should be avoided */
diff -Nrup 02_aopen_rs/include/linux/ata.h 03_max_sect/include/linux/ata.h
--- 02_aopen_rs/include/linux/ata.h 2007-03-23 16:56:24.0 +0800
+++ 03_max_sect/include/linux/ata.h 2007-03-31 10:06:53.0 +0800
@@ -40,6 +40,7 @@ enum {
ATA_MAX_DEVICES = 2,/* per bus/port */
ATA_MAX_PRD = 256, /* we could make these 256/256 */
ATA_SECT_SIZE = 512,
+ ATA_MAX_SECTORS_240 = 240,
ATA_MAX_SECTORS = 256,
ATA_MAX_SECTORS_LBA48 = 65535,/* TODO: 65536? */
diff -Nrup 02_aopen_rs/include/linux/libata.h 03_max_sect/include/linux/libata.h
--- 02_aopen_rs/include/linux/libata.h 2007-03-26 12:33:28.0 +0800
+++ 03_max_sect/include/linux/libata.h 2007-03-31 14:07:49.0 +0800
@@ -311,6 +311,7 @@ enum {
ATA_HORKAGE_DIAGNOSTIC = (1 0), /* Failed boot diag */
ATA_HORKAGE_NODMA = (1 1), /* DMA problems */
ATA_HORKAGE_NONCQ = (1 2), /* Don't use NCQ */
+ ATA_HORKAGE_MAX_SEC_240 = (1 3), /* Limit max sects below 256 */
};
enum hsm_task_states {
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] libata: Limit ATAPI DMA to R/W commands only for TORiSAN DVD drives
patch 4/4:
Limit ATAPI DMA to R/W commands only for TORiSAN DRD-N216 DVD-ROM drives
(http://bugzilla.kernel.org/show_bug.cgi?id=6710)
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
I guess maybe other commands like READ_DVD_STRUCTURE and READ_CD also
work under ATAPI DMA, but not confirmed by Vlad yet. So, allow ATAPI DMA to
only for Read/Write here for safty.
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 03_max_sect/drivers/ata/libata-core.c
04_dma_rw_only/drivers/ata/libata-core.c
--- 03_max_sect/drivers/ata/libata-core.c 2007-03-31 14:11:33.0
+0800
+++ 04_dma_rw_only/drivers/ata/libata-core.c2007-03-31 14:27:47.0
+0800
@@ -1787,6 +1787,10 @@ int ata_dev_configure(struct ata_device
if (ata_device_blacklisted(dev) ATA_HORKAGE_MAX_SEC_240)
dev-max_sectors = min(ATA_MAX_SECTORS_240, dev-max_sectors);
+ /* limit ATAPI DMA to R/W commands only */
+ if (ata_device_blacklisted(dev) ATA_HORKAGE_DMA_RW_ONLY)
+ dev-horkage |= ATA_HORKAGE_DMA_RW_ONLY;
+
if (ap-ops-dev_config)
ap-ops-dev_config(ap, dev);
@@ -3356,7 +3360,8 @@ static const struct ata_blacklist_entry
{ SAMSUNG CD-ROM SN-124,N001, ATA_HORKAGE_NODMA },
/* Weird ATAPI devices */
- { TORiSAN DVD-ROM DRD-N216, NULL, ATA_HORKAGE_MAX_SEC_240 },
+ { TORiSAN DVD-ROM DRD-N216, NULL, ATA_HORKAGE_MAX_SEC_240 |
+ ATA_HORKAGE_DMA_RW_ONLY },
/* Devices we expect to fail diagnostics */
@@ -3676,6 +3681,26 @@ int ata_check_atapi_dma(struct ata_queue
struct ata_port *ap = qc-ap;
int rc = 0; /* Assume ATAPI DMA is OK by default */
+ /* some drives can only do ATAPI DMA on read/write */
+ if (unlikely(qc-dev-horkage ATA_HORKAGE_DMA_RW_ONLY)) {
+ struct scsi_cmnd *cmd = qc-scsicmd;
+ u8 *scsicmd = cmd-cmnd;
+
+ switch (scsicmd[0]) {
+ case READ_10:
+ case WRITE_10:
+ case READ_12:
+ case WRITE_12:
+ case READ_6:
+ case WRITE_6:
+ /* atapi dma maybe ok */
+ break;
+ default:
+ /* turn off atapi dma */
+ return 1;
+ }
+ }
+
if (ap-ops-check_atapi_dma)
rc = ap-ops-check_atapi_dma(qc);
diff -Nrup 03_max_sect/include/linux/libata.h
04_dma_rw_only/include/linux/libata.h
--- 03_max_sect/include/linux/libata.h 2007-03-31 14:07:49.0 +0800
+++ 04_dma_rw_only/include/linux/libata.h 2007-03-31 14:08:31.0
+0800
@@ -312,6 +312,7 @@ enum {
ATA_HORKAGE_NODMA = (1 1), /* DMA problems */
ATA_HORKAGE_NONCQ = (1 2), /* Don't use NCQ */
ATA_HORKAGE_MAX_SEC_240 = (1 3), /* Limit max sects below 256 */
+ ATA_HORKAGE_DMA_RW_ONLY = (1 4), /* ATAPI DMA for RW only */
};
enum hsm_task_states {
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] libata: Clear tf before doing request sense
Tejun Heo wrote: Albert Lee wrote: diff -Nrup 01_hsm_st/drivers/ata/libata-eh.c 02_aopen_rs/drivers/ata/libata-eh.c --- 01_hsm_st/drivers/ata/libata-eh.c2007-03-23 16:56:13.0 +0800 +++ 02_aopen_rs/drivers/ata/libata-eh.c2007-03-31 01:11:01.0 +0800 @@ -991,18 +991,19 @@ static unsigned int atapi_eh_request_sen DPRINTK(ATAPI request sense\n); -ata_tf_init(dev, tf); - /* FIXME: is this needed? */ memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE); -/* XXX: why tf_read here? */ +/* read error register to initialize sense_buf */ ap-ops-tf_read(ap, tf); /* fill these in, for the case where they are -not- overwritten */ sense_buf[0] = 0x70; sense_buf[2] = tf.feature 4; Oh, now I see why it's there. Thanks for spotting this. We don't need tf_read here, you can simply use the value in qc-result_tf.feature for this purpose. Thanks for the advice. Will revise this patch. -- albert - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] libata: Workaround/fixes for ATAPI devices (revised)
patch 1/4: Reorder HSM_ST_FIRST patch 2/4: Clear tf before doing request sense patch 3/4: Limit max sector to 240 for TORiSAN DVD drives patch 4/4: Limit ATAPI DMA to R/W commands only for TORiSAN DVD drives (patch 2/4 revised per Tejun's advice.) - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] libata: reorder HSM_ST_FIRST for easier decoding
patch 1/4:
Reorder HSM_ST_FIRST, such that the task state transition is easier decoded
with human eyes.
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 00_libata-dev.ori/include/linux/libata.h
01_hsm_st/include/linux/libata.h
--- 00_libata-dev.ori/include/linux/libata.h2007-03-23 16:56:24.0
+0800
+++ 01_hsm_st/include/linux/libata.h2007-03-26 12:33:28.0 +0800
@@ -315,11 +315,11 @@ enum {
enum hsm_task_states {
HSM_ST_IDLE,/* no command on going */
+ HSM_ST_FIRST, /* (waiting the device to)
+ write CDB or first data block */
HSM_ST, /* (waiting the device to) transfer data */
HSM_ST_LAST,/* (waiting the device to) complete command */
HSM_ST_ERR, /* error */
- HSM_ST_FIRST, /* (waiting the device to)
- write CDB or first data block */
};
enum ata_completion_errors {
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] libata: Clear tf before doing request sense (revised)
patch 2/4:
Clear tf before doing request sense.
This fixes the AOpen 56X/AKH timeout problem.
(http://bugzilla.kernel.org/show_bug.cgi?id=8244)
Signed-off-by: Albert Lee [EMAIL PROTECTED]
---
Revised per Tejun's advice to use result_tf instead.
Patch against libata-dev tree, for your review, thanks.
diff -Nrup 01_hsm_st/drivers/ata/libata-eh.c 02_aopen_rs/drivers/ata/libata-eh.c
--- 01_hsm_st/drivers/ata/libata-eh.c 2007-03-23 16:56:13.0 +0800
+++ 02_aopen_rs/drivers/ata/libata-eh.c 2007-03-31 15:55:30.0 +0800
@@ -982,26 +982,27 @@ static int ata_eh_read_log_10h(struct at
* RETURNS:
* 0 on success, AC_ERR_* mask on failure
*/
-static unsigned int atapi_eh_request_sense(struct ata_device *dev,
- unsigned char *sense_buf)
+static unsigned int atapi_eh_request_sense(struct ata_queued_cmd *qc)
{
+ struct ata_device *dev = qc-dev;
+ unsigned char *sense_buf = qc-scsicmd-sense_buffer;
struct ata_port *ap = dev-ap;
struct ata_taskfile tf;
u8 cdb[ATAPI_CDB_LEN];
DPRINTK(ATAPI request sense\n);
- ata_tf_init(dev, tf);
-
/* FIXME: is this needed? */
memset(sense_buf, 0, SCSI_SENSE_BUFFERSIZE);
- /* XXX: why tf_read here? */
- ap-ops-tf_read(ap, tf);
-
- /* fill these in, for the case where they are -not- overwritten */
+ /* initialize sense_buf with error register,
+* for the case where they are -not- overwritten
+*/
sense_buf[0] = 0x70;
- sense_buf[2] = tf.feature 4;
+ sense_buf[2] = qc-result_tf.feature 4;
+
+ /* some devices time out if garbage left in tf */
+ ata_tf_init(dev, tf);
memset(cdb, 0, ATAPI_CDB_LEN);
cdb[0] = REQUEST_SENSE;
@@ -1165,8 +1166,7 @@ static unsigned int ata_eh_analyze_tf(st
case ATA_DEV_ATAPI:
if (!(qc-ap-pflags ATA_PFLAG_FROZEN)) {
- tmp = atapi_eh_request_sense(qc-dev,
-qc-scsicmd-sense_buffer);
+ tmp = atapi_eh_request_sense(qc);
if (!tmp) {
/* ATA_QCFLAG_SENSE_VALID is used to
* tell atapi_qc_complete() that sense
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
