Hello.
Tejun Heo wrote:
There have been reports of pata_hpt366 locking up the whole machine
and as Mark kindly sent me his hpt366 about a year ago, I played with
it a bit today.
The controller worked perfectly fine with the IDE driver but with
pata_hpt366, PIO works but DMA mode locks up the machine solidly.
After comparing the sources and lspci outputs, I found the following
differences.
1. The case values for PCI clock speed selection seems wrong. IDE
hpt366 uses 9 for 40Mhz and 5 for 25 not the other way around.
Yes, but that only concerns original HPT36x and should hardly matter
as the BIOS seems to setup the timing registers only for 33 MHz PCI, and
these 2 shouldn't even be eached.
2. IDE hpt366 uses different mask values for PIO, MWDMA and UDMA when
combining the old and new timing values but libata uses one value
for all.
Yes, I've fixed that stupidity. UltraDMA modes shouldn't force PIO4
timings. However, the PIO and DMA masks were different from the start
(though incorrect for HPT37x, IIRC)...
3. IDe hpt366 always turns off 0xC0000000 in the timing value but
pata_hpt366 doesn't.
Hm, that's strange... PIO_MST *must* be cleared and it's set in the
timing tables.
Removing the above three differences from pata_hpt366 makes it work
happily, but I don't know anything about this controller so no
sign-off yet.
Alan, Mark, does this change look sane to you?
Alan, it definitely seems we got something about the cable detection
wrong. libata can't see my 80c cable but IDE can.
Thanks.
RFC PATCH. PLEASE DONT APPLY YET.
Acked-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
diff --git a/drivers/ata/pata_hpt366.c b/drivers/ata/pata_hpt366.c
index f2b83ea..64b3107 100644
--- a/drivers/ata/pata_hpt366.c
+++ b/drivers/ata/pata_hpt366.c
@@ -188,25 +188,41 @@ static unsigned long hpt366_filter(struct ata_device *adev, unsigned long mask)
}
/**
- * hpt36x_find_mode - reset the hpt36x bus
+ * hpt36x_calc_timing - calculate timing value for transfer mode
* @ap: ATA port
- * @speed: transfer mode
+ * @cur_timing: current timing value
+ * @speed: target transfer mode
*
* Return the 32bit register programming information for this channel
* that matches the speed provided.
*/
-static u32 hpt36x_find_mode(struct ata_port *ap, int speed)
+static u32 hpt36x_calc_timing(struct ata_port *ap, u32 cur_timing, int speed)
{
struct hpt_clock *clocks = ap->host->private_data;
+ u32 mask;
- while(clocks->xfer_speed) {
+ if (speed < XFER_MW_DMA_0)
+ mask = 0xc1f8ffff;
+ else if (speed < XFER_UDMA_0)
+ mask = 0x303800ff;
+ else
+ mask = 0x30070000;
+
+ while (clocks->xfer_speed) {
if (clocks->xfer_speed == speed)
- return clocks->timing;
+ break;
clocks++;
}
Should really have been a *for* loop...
@@ -247,11 +262,8 @@ static void hpt366_set_piomode(struct ata_port *ap, struct ata_device *adev)
}
pci_read_config_dword(pdev, addr1, ®);
- mode = hpt36x_find_mode(ap, adev->pio_mode);
- mode &= ~0x8000000; /* No FIFO in PIO */
- mode &= ~0x30070000; /* Leave config bits alone */
These are DMA enable bits and UDMA timings...
@ -267,8 +279,7 @@ static void hpt366_set_dmamode(struct ata_port *ap, struct ata_device *adev)
{
struct pci_dev *pdev = to_pci_dev(ap->host->dev);
u32 addr1, addr2;
- u32 reg;
- u32 mode;
+ u32 reg, timing;
u8 fast;
addr1 = 0x40 + 4 * (adev->devno + 2 * ap->port_no);
@@ -282,11 +293,8 @@ static void hpt366_set_dmamode(struct ata_port *ap, struct ata_device *adev)
}
pci_read_config_dword(pdev, addr1, ®);
- mode = hpt36x_find_mode(ap, adev->dma_mode);
- mode |= 0x8000000; /* FIFO in MWDMA or UDMA */
FIFO bit doesn't affect DMA modes.
- mode &= ~0xC0000000; /* Leave config bits alone */
Oh, it's cleared right afterwards...
Looks like with you change the set_dmamode() and set_piomode()
methods are becoming sompletely similar, so it's probably worth to merge
them...
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
