Hello, I wrote:
* Convert cmd64x, hpt366 and pdc202xx_old host drivers to use
pci_resource_start(hwif->pci_dev, 4) instead of hwif->dma_master.
Before using pci_resource_start(), the code should check
pci_resource_len() to ensure it is the appropriate size.
It would be very wise to ensure that
pci_resource_flags()&IORESOURCE_IO is true, too.
Sure but since the old code hasn't been doing these checks (old code
just did pci_resource_start() -> hwif->dma_base -> hwif->dma_master)
this is a separate issue from what the patch tries to achieve.
Nonetheless this is duplicating an existing bug $N times... not ideal :)
The existing bug is that the methods converted to use
pci_resource_start()
may be set in hwif instance and used even if the BAR4 is invalid.
Therefore
the above patch is correct and it isn't duplicating the existing bug. :)
However I agree that all these pci_resource_start()s could be a bit
confusing
so here is 'take 2'.
Yeah, they are. Actually, I was thinking about such patch ~1.5 years
ago *but* using a different approach -- namely, hwif->extra_base.
Yes, the code introducing this field was written with replacing dma_master
by it in mind... It's a pity I haven't gotten around to it.
[...]
Index: b/drivers/ide/pci/hpt366.c
===================================================================
--- a/drivers/ide/pci/hpt366.c
+++ b/drivers/ide/pci/hpt366.c
@@ -845,32 +845,33 @@ static int hpt374_ide_dma_end(ide_drive_
This just asks to be:
No, it doesn't cause that way one would only break it.
static void hpt3xxn_set_clock(ide_hwif_t *hwif, u8 mode)
{
unsigned long base = hwif->extra_base;
Sorry for a thinko. All offsets below shoud be really 0x6x, not 0x5x...
u8 scr2 = inb(base + 0x5b);
if ((scr2 & 0x7f) == mode)
return;
/* Tristate the bus */
outb(0x80, base + 0x53);
outb(0x80, base + 0x57);
/* Switch clock and reset channels */
outb(mode, base + 0x5b);
outb(0xc0, base + 0x59);
/*
* Reset the state machines.
* NOTE: avoid accidentally enabling the disabled channels.
*/
outb(inb(base + 0x50) | 0x32, base + 0x50);
outb(inb(base + 0x54) | 0x32, base + 0x54);
/* Complete reset */
outb(0x00, base + 0x59);
/* Reconnect channels to bus */
outb(0x00, base + 0x53);
outb(0x00, base + 0x57);
}
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
