Hello.
Bartlomiej Zolnierkiewicz wrote:
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...
Here we go again... :)
[PATCH] ide: remove ->dma_master field from ide_hwif_t (take 4)
* Convert cmd64x, hpt366 and pdc202xx_old host drivers to use
pci_resource_start(hwif->pci_dev, 4) instead of hwif->dma_master.
* Remove no longer needed ->dma_master field from ide_hwif_t.
v2:
* Use the more readable 'hwif->dma_base - (hwif->channel * 8)' instead of
pci_resource_start(hwif->pci_dev, 4).
v3:
* Use hwif->extra_base in hpt366/pdc20xx_old + some cosmetic fixups over v2
(suggested by Sergei).
Huh? I don't see where you're using extra_base in hpt366.c
v4:
* Correct offsets in hpt3xxn_set_clock().
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
Unfortunately, I have to NAK the patch... :-/
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_
static void hpt3xxn_set_clock(ide_hwif_t *hwif, u8 mode)
{
- u8 scr2 = inb(hwif->dma_master + 0x7b);
+ unsigned long base = hwif->dma_base - (hwif->channel * 8);
Should be hwif->extra_base - 0x10 or else the following code will touch
all the wrong/reserved registers; one also can use hwif->extra_base ISO 'base'
and change all the offsets to 0x6x...
+ u8 scr2 = inb(base + 0x6b);
if ((scr2 & 0x7f) == mode)
return;
/* Tristate the bus */
- outb(0x80, hwif->dma_master + 0x73);
- outb(0x80, hwif->dma_master + 0x77);
+ outb(0x80, base + 0x63);
+ outb(0x80, base + 0x67);
/* Switch clock and reset channels */
- outb(mode, hwif->dma_master + 0x7b);
- outb(0xc0, hwif->dma_master + 0x79);
+ outb(mode, base + 0x6b);
+ outb(0xc0, base + 0x69);
/*
* Reset the state machines.
* NOTE: avoid accidentally enabling the disabled channels.
*/
- outb(inb(hwif->dma_master + 0x70) | 0x32, hwif->dma_master + 0x70);
- outb(inb(hwif->dma_master + 0x74) | 0x32, hwif->dma_master + 0x74);
+ outb(inb(base + 0x60) | 0x32, base + 0x60);
+ outb(inb(base + 0x64) | 0x32, base + 0x64);
/* Complete reset */
- outb(0x00, hwif->dma_master + 0x79);
+ outb(0x00, base + 0x69);
/* Reconnect channels to bus */
- outb(0x00, hwif->dma_master + 0x73);
- outb(0x00, hwif->dma_master + 0x77);
+ outb(0x00, base + 0x63);
+ outb(0x00, base + 0x67);
}
Huh? That's wrong -- if you're setting base to the value of the former
value of hwof->dma_master, the register offsets shouldn't change!
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -193,7 +193,7 @@ static void pdc202xx_old_ide_dma_start(i
if (drive->media != ide_disk || drive->addressing == 1) {
struct request *rq = HWGROUP(drive)->rq;
ide_hwif_t *hwif = HWIF(drive);
- unsigned long high_16 = hwif->dma_master;
+ unsigned long high_16 = hwif->extra_base - 16;
unsigned long atapi_reg = high_16 + (hwif->channel ? 0x24 : 0x20);
That 'high_16' is *not* a good name in this case -- it would've been if we
didn't subtract 16. So, I'd wrote:
unsigned long atapi_reg = hwif->extra_base +
(hwif->channel ? 0x14 : 0x10);
@@ -212,7 +212,7 @@ static int pdc202xx_old_ide_dma_end(ide_
{
if (drive->media != ide_disk || drive->addressing == 1) {
ide_hwif_t *hwif = HWIF(drive);
- unsigned long high_16 = hwif->dma_master;
+ unsigned long high_16 = hwif->extra_base - 16;
unsigned long atapi_reg = high_16 + (hwif->channel ? 0x24 : 0x20);
u8 clock = 0;
Same here...
@@ -228,7 +228,7 @@ static int pdc202xx_old_ide_dma_end(ide_
static int pdc202xx_old_ide_dma_test_irq(ide_drive_t *drive)
{
ide_hwif_t *hwif = HWIF(drive);
- unsigned long high_16 = hwif->dma_master;
+ unsigned long high_16 = hwif->extra_base - 16;
u8 dma_stat = inb(hwif->dma_status);
u8 sc1d = inb(high_16 + 0x001d);
... and here...
@@ -271,7 +271,7 @@ static void pdc202xx_dma_timeout(ide_dri
static void pdc202xx_reset_host (ide_hwif_t *hwif)
{
- unsigned long high_16 = hwif->dma_master;
+ unsigned long high_16 = hwif->extra_base - 16;
u8 udma_speed_flag = inb(high_16 | 0x001f);
... and here too.
outb(udma_speed_flag | 0x10, high_16 | 0x001f);
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
