login
Search
Add ->read_sff_dma_status method for reading DMA Status register
and use it instead of ->INB.
While at it:
* Use inb() directly in ns87415.c::ns87415_dma_end().
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-dma.c | 12 ++++++------
drivers/ide/ide-iops.c | 10 ++++++++++
drivers/ide/pci/ns87415.c | 13 ++++++++++---
drivers/ide/pci/scc_pata.c | 7 +++++++
include/linux/ide.h | 2 ++
5 files changed, 35 insertions(+), 9 deletions(-)
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -334,7 +334,7 @@ static int config_drive_for_dma (ide_dri
static int dma_timer_expiry (ide_drive_t *drive)
{
ide_hwif_t *hwif = HWIF(drive);
- u8 dma_stat = hwif->INB(hwif->dma_status);
+ u8 dma_stat = hwif->read_sff_dma_status(hwif);
printk(KERN_WARNING "%s: dma_timer_expiry: dma status == 0x%02x\n",
drive->name, dma_stat);
@@ -369,7 +369,7 @@ void ide_dma_host_set(ide_drive_t *drive
{
ide_hwif_t *hwif = HWIF(drive);
u8 unit = (drive->select.b.unit & 0x01);
- u8 dma_stat = hwif->INB(hwif->dma_status);
+ u8 dma_stat = hwif->read_sff_dma_status(hwif);
if (on)
dma_stat |= (1 << (5 + unit));
@@ -472,8 +472,8 @@ int ide_dma_setup(ide_drive_t *drive)
/* specify r/w */
hwif->OUTB(reading, hwif->dma_command);
- /* read dma_status for INTR & ERROR flags */
- dma_stat = hwif->INB(hwif->dma_status);
+ /* read DMA status for INTR & ERROR flags */
+ dma_stat = hwif->read_sff_dma_status(hwif);
/* clear INTR & ERROR flags */
hwif->OUTB(dma_stat|6, hwif->dma_status);
@@ -520,7 +520,7 @@ int __ide_dma_end (ide_drive_t *drive)
/* stop DMA */
hwif->OUTB(dma_cmd&~1, hwif->dma_command);
/* get DMA status */
- dma_stat = hwif->INB(hwif->dma_status);
+ dma_stat = hwif->read_sff_dma_status(hwif);
/* clear the INTR & ...Use I/O ops directly in ide_dma_host_set(), ide_dma_setup(),
ide_dma_start() and __ide_dma_end().
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-dma.c | 59 ++++++++++++++++++++++++++++++++++++++------------
1 file changed, 45 insertions(+), 14 deletions(-)
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -376,7 +376,10 @@ void ide_dma_host_set(ide_drive_t *drive
else
dma_stat &= ~(1 << (5 + unit));
- hwif->OUTB(dma_stat, hwif->dma_status);
+ if (hwif->host_flags & IDE_HFLAG_MMIO)
+ writeb(dma_stat, (void __iomem *)hwif->dma_status);
+ else
+ outb(dma_stat, hwif->dma_status);
}
EXPORT_SYMBOL_GPL(ide_dma_host_set);
@@ -449,6 +452,7 @@ int ide_dma_setup(ide_drive_t *drive)
ide_hwif_t *hwif = drive->hwif;
struct request *rq = HWGROUP(drive)->rq;
unsigned int reading;
+ u8 mmio = (hwif->host_flags & IDE_HFLAG_MMIO) ? 1 : 0;
u8 dma_stat;
if (rq_data_dir(rq))
@@ -470,13 +474,20 @@ int ide_dma_setup(ide_drive_t *drive)
outl(hwif->dmatable_dma, hwif->dma_base + ATA_DMA_TABLE_OFS);
/* specify r/w */
- hwif->OUTB(reading, hwif->dma_command);
+ if (mmio)
+ writeb(reading, (void __iomem *)hwif->dma_command);
+ else
+ outb(reading, hwif->dma_command);
/* read DMA status for INTR & ERROR flags */
dma_stat = hwif->read_sff_dma_status(hwif);
/* clear INTR & ERROR flags */
- hwif->OUTB(dma_stat|6, hwif->dma_status);
+ if (mmio)
+ writeb(dma_stat | 6, (void __iomem *)hwif->dma_status);
+ else
+ outb(dma_stat | 6, hwif->dma_status);
+
drive->waiting_for_dma = 1;
return 0;
}
@@ -492,16 +503,23 @@ EXPORT_SYMBOL_GPL(ide_dma_exec_cmd);
void ide_dma_start(ide_drive_t *drive)
{
- ide_hwif_t *hwif = HWIF(drive);
- u8 dma_cmd = hwif->INB(hwif->dma_command);
+ ide_hwif_t *hwif = drive->hwif;
+ u8 dma_cmd;
...Seems like the above 3 sequences are asking to be factored out into ide_dma_write_status(); with the latter two possibly factored out into MBR, Sergei --
* Use ->dma_base + offset instead of ->dma_{status,command}
and remove no longer needed ->dma_{status,command}.
While at it:
* Use ATA_DMA_* defines.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-dma.c | 42 ++++++++++++++++++++---------------------
drivers/ide/ide-iops.c | 4 +--
drivers/ide/pci/cmd64x.c | 12 +++++------
drivers/ide/pci/hpt366.c | 10 ++++-----
drivers/ide/pci/ns87415.c | 14 ++++++-------
drivers/ide/pci/pdc202xx_old.c | 2 -
drivers/ide/pci/piix.c | 4 +--
drivers/ide/pci/scc_pata.c | 38 +++++++++++++++++--------------------
drivers/ide/pci/siimage.c | 4 +--
drivers/ide/pci/sl82c105.c | 4 +--
drivers/ide/pci/tc86c001.c | 13 +++++++-----
include/linux/ide.h | 2 -
12 files changed, 74 insertions(+), 75 deletions(-)
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -377,9 +377,10 @@ void ide_dma_host_set(ide_drive_t *drive
dma_stat &= ~(1 << (5 + unit));
if (hwif->host_flags & IDE_HFLAG_MMIO)
- writeb(dma_stat, (void __iomem *)hwif->dma_status);
+ writeb(dma_stat,
+ (void __iomem *)(hwif->dma_base + ATA_DMA_STATUS));
else
- outb(dma_stat, hwif->dma_status);
+ outb(dma_stat, hwif->dma_base + ATA_DMA_STATUS);
}
EXPORT_SYMBOL_GPL(ide_dma_host_set);
@@ -475,18 +476,19 @@ int ide_dma_setup(ide_drive_t *drive)
/* specify r/w */
if (mmio)
- writeb(reading, (void __iomem *)hwif->dma_command);
+ writeb(reading, (void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
else
- outb(reading, hwif->dma_command);
+ outb(reading, hwif->dma_base + ATA_DMA_CMD);
/* read DMA status for INTR & ERROR flags */
dma_stat = hwif->read_sff_dma_status(hwif);
/* clear INTR & ERROR flags */
if ...Export sff_dma_ops and then remove ide_setup_dma().
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/arm/palm_bk3710.c | 7 +++----
drivers/ide/ide-dma.c | 12 ++----------
drivers/ide/pci/alim15x3.c | 4 +++-
drivers/ide/pci/hpt366.c | 4 +++-
drivers/ide/setup-pci.c | 4 +++-
include/linux/ide.h | 2 +-
6 files changed, 15 insertions(+), 18 deletions(-)
Index: b/drivers/ide/arm/palm_bk3710.c
===================================================================
--- a/drivers/ide/arm/palm_bk3710.c
+++ b/drivers/ide/arm/palm_bk3710.c
@@ -318,15 +318,14 @@ static u8 __devinit palm_bk3710_cable_de
static int __devinit palm_bk3710_init_dma(ide_hwif_t *hwif,
const struct ide_port_info *d)
{
- unsigned long base =
- hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
-
printk(KERN_INFO " %s: MMIO-DMA\n", hwif->name);
if (ide_allocate_dma_engine(hwif))
return -1;
- ide_setup_dma(hwif, base);
+ hwif->dma_base = hwif->io_ports.data_addr - IDE_PALM_ATA_PRI_REG_OFFSET;
+
+ hwif->dma_ops = &sff_dma_ops;
return 0;
}
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -878,7 +878,7 @@ int ide_allocate_dma_engine(ide_hwif_t *
}
EXPORT_SYMBOL_GPL(ide_allocate_dma_engine);
-static const struct ide_dma_ops sff_dma_ops = {
+const struct ide_dma_ops sff_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
.dma_exec_cmd = ide_dma_exec_cmd,
@@ -888,13 +888,5 @@ static const struct ide_dma_ops sff_dma_
.dma_timeout = ide_dma_timeout,
.dma_lost_irq = ide_dma_lost_irq,
};
-
-void ide_setup_dma(ide_hwif_t *hwif, unsigned long base)
-{
- hwif->dma_base = base;
-
- hwif->dma_ops = ...Hello. That's er... too complex. Actually, 'base' has the value for 'dma_base' at that moment... WBR, Sergei --
Well, the above place is the only user of 'base' so isn't the variable kind of superfluous? Thanks, Bart --
Hello. Oops, sorry. I somehow thought this change was in other context (was MBR, Sergei --
Hello.
Unfortunately, this patch broke this driver:
drivers/ide/arm/palm_bk3710.c:327: error: ‘sff_dma_ops’ undeclared (first use
in this function). The declatation of sff_dma_ops should've been surrounded by:
#ifdef CONFIG_BLK_DEV_IDEDMA_SFF
not by:
#ifdef CONFIG_BLK_DEV_IDEDMA_PCI
the same as its definition is in ide-dma.c...
MBR, Sergei
--
OK, Kevin Hilman said he's going to post a patch for this issue... MBR, Sergei --
* Factor out simplex handling from ide_pci_dma_base() to
ide_pci_check_simplex().
* Set hwif->dma_base early in ->init_dma method / ide_hwif_setup_dma()
and reset it in ide_init_port() if DMA initialization fails.
* Use ->read_sff_dma_status instead of ->INB in ide_pci_dma_base().
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-probe.c | 1 +
drivers/ide/pci/alim15x3.c | 12 +++++++++---
drivers/ide/pci/hpt366.c | 12 +++++++++---
drivers/ide/setup-pci.c | 35 +++++++++++++++++++++++------------
include/linux/ide.h | 1 +
5 files changed, 43 insertions(+), 18 deletions(-)
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1369,6 +1369,7 @@ static void ide_init_port(ide_hwif_t *hw
if (rc < 0) {
printk(KERN_INFO "%s: DMA disabled\n", hwif->name);
+ hwif->dma_base = 0;
hwif->swdma_mask = 0;
hwif->mwdma_mask = 0;
hwif->ultra_mask = 0;
Index: b/drivers/ide/pci/alim15x3.c
===================================================================
--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c
@@ -471,7 +471,15 @@ static int __devinit init_dma_ali15x3(id
struct pci_dev *dev = to_pci_dev(hwif->dev);
unsigned long base = ide_pci_dma_base(hwif, d);
- if (base == 0 || ide_pci_set_master(dev, d->name) < 0)
+ if (base == 0)
+ return -1;
+
+ hwif->dma_base = base;
+
+ if (ide_pci_check_simplex(hwif, d) < 0)
+ return -1;
+
+ if (ide_pci_set_master(dev, d->name) < 0)
return -1;
if (!hwif->channel)
@@ -483,8 +491,6 @@ static int __devinit init_dma_ali15x3(id
if (ide_allocate_dma_engine(hwif))
return -1;
- hwif->dma_base = base;
-
hwif->dma_ops = &sff_dma_ops;
return 0;
Index: ...Add ->exec_command method for writing ATA Command register
and use it instead of ->OUTBSYNC.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-io.c | 3 +--
drivers/ide/ide-iops.c | 19 ++++++++++++++-----
drivers/ide/ide-probe.c | 6 +++---
drivers/ide/ide-taskfile.c | 2 +-
drivers/ide/pci/scc_pata.c | 9 +++++++++
drivers/ide/ppc/pmac.c | 9 +++++++++
drivers/scsi/ide-scsi.c | 3 +--
include/linux/ide.h | 3 ++-
8 files changed, 40 insertions(+), 14 deletions(-)
Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -437,8 +437,7 @@ static ide_startstop_t ide_atapi_error(i
if (ide_read_status(drive) & (BUSY_STAT | DRQ_STAT))
/* force an abort */
- hwif->OUTBSYNC(hwif, WIN_IDLEIMMEDIATE,
- hwif->io_ports.command_addr);
+ hwif->exec_command(hwif, WIN_IDLEIMMEDIATE);
if (rq->errors >= ERROR_MAX) {
ide_kill_rq(drive, rq);
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -103,6 +103,14 @@ void SELECT_MASK(ide_drive_t *drive, int
port_ops->maskproc(drive, mask);
}
+static void ide_exec_command(ide_hwif_t *hwif, u8 cmd)
+{
+ if (hwif->host_flags & IDE_HFLAG_MMIO)
+ writeb(cmd, (void __iomem *)hwif->io_ports.command_addr);
+ else
+ outb(cmd, hwif->io_ports.command_addr);
+}
+
static u8 ide_read_sff_dma_status(ide_hwif_t *hwif)
{
if (hwif->host_flags & IDE_HFLAG_MMIO)
@@ -331,6 +339,7 @@ static void ata_output_data(ide_drive_t
void default_hwif_transport(ide_hwif_t *hwif)
{
+ hwif->exec_command = ide_exec_command;
hwif->read_sff_dma_status = ide_read_sff_dma_status;
hwif->tf_load = ide_tf_load;
@@ -696,7 +705,7 @@ int ide_driveid_update(ide_drive_t *driv
...* Remove ide_read_status() inline helper.
* Add ->read_status method for reading ATA Status register
and use it instead of ->INB.
While at it:
* Don't use HWGROUP() macro.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/arm/icside.c | 4 +++-
drivers/ide/ide-atapi.c | 2 +-
drivers/ide/ide-cd.c | 12 ++++++++----
drivers/ide/ide-dma.c | 5 +++--
drivers/ide/ide-floppy.c | 3 ++-
drivers/ide/ide-io.c | 19 ++++++++++---------
drivers/ide/ide-iops.c | 33 ++++++++++++++++++++++-----------
drivers/ide/ide-probe.c | 22 +++++++++++-----------
drivers/ide/ide-tape.c | 6 ++++--
drivers/ide/ide-taskfile.c | 23 ++++++++++++++---------
drivers/ide/pci/ns87415.c | 8 +++++++-
drivers/ide/pci/scc_pata.c | 6 ++++++
drivers/ide/pci/sgiioc4.c | 19 ++++++++++---------
drivers/scsi/ide-scsi.c | 2 +-
include/linux/ide.h | 8 +-------
15 files changed, 103 insertions(+), 69 deletions(-)
Index: b/drivers/ide/arm/icside.c
===================================================================
--- a/drivers/ide/arm/icside.c
+++ b/drivers/ide/arm/icside.c
@@ -375,12 +375,14 @@ static int icside_dma_test_irq(ide_drive
static void icside_dma_timeout(ide_drive_t *drive)
{
+ ide_hwif_t *hwif = drive->hwif;
+
printk(KERN_ERR "%s: DMA timeout occurred: ", drive->name);
if (icside_dma_test_irq(drive))
return;
- ide_dump_status(drive, "DMA timeout", ide_read_status(drive));
+ ide_dump_status(drive, "DMA timeout", hwif->read_status(hwif));
icside_dma_end(drive);
}
Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -35,7 +35,7 @@ ide_startstop_t ide_pc_intr(ide_drive_t
}
/* Clear the interrupt */
- stat = ide_read_status(drive);
+ stat = ...* Remove ide_read_altstatus() inline helper.
* Add ->read_altstatus method for reading ATA Alternate Status
register and use it instead of ->INB.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-iops.c | 13 +++++++++++--
drivers/ide/ide-probe.c | 4 ++--
drivers/ide/pci/scc_pata.c | 6 ++++++
include/linux/ide.h | 8 +-------
4 files changed, 20 insertions(+), 11 deletions(-)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -119,6 +119,14 @@ static u8 ide_read_status(ide_hwif_t *hw
return inb(hwif->io_ports.status_addr);
}
+static u8 ide_read_altstatus(ide_hwif_t *hwif)
+{
+ if (hwif->host_flags & IDE_HFLAG_MMIO)
+ return readb((void __iomem *)hwif->io_ports.ctl_addr);
+ else
+ return inb(hwif->io_ports.ctl_addr);
+}
+
static u8 ide_read_sff_dma_status(ide_hwif_t *hwif)
{
if (hwif->host_flags & IDE_HFLAG_MMIO)
@@ -349,6 +357,7 @@ void default_hwif_transport(ide_hwif_t *
{
hwif->exec_command = ide_exec_command;
hwif->read_status = ide_read_status;
+ hwif->read_altstatus = ide_read_altstatus;
hwif->read_sff_dma_status = ide_read_sff_dma_status;
hwif->tf_load = ide_tf_load;
@@ -511,7 +520,7 @@ int drive_is_ready (ide_drive_t *drive)
* about possible isa-pnp and pci-pnp issues yet.
*/
if (hwif->io_ports.ctl_addr)
- stat = ide_read_altstatus(drive);
+ stat = hwif->read_altstatus(hwif);
else
/* Note: this may clear a pending IRQ!! */
stat = hwif->read_status(hwif);
@@ -724,7 +733,7 @@ int ide_driveid_update(ide_drive_t *driv
}
msleep(50); /* give drive a breather */
- stat = ide_read_altstatus(drive);
+ stat = hwif->read_altstatus(hwif);
} while (stat & BUSY_STAT);
msleep(50); /* wait for IRQ and DRQ_STAT */
Index: ...Add ->set_irq method for setting nIEN bit of ATA Device Control register and use it instead of ide_set_irq(). While at it: * Use ->set_irq in init_irq() and do_reset1(). * Don't use HWIF() macro in ide_check_pm_state(). There should be no functional changes caused by this patch. Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> --- drivers/ide/ide-io.c | 14 ++++++++------ drivers/ide/ide-iops.c | 37 +++++++++++++++++++++++++++---------- drivers/ide/ide-probe.c | 9 ++++----- drivers/ide/ide-taskfile.c | 2 +- drivers/ide/pci/scc_pata.c | 19 +++++++++++++++++++ drivers/ide/ppc/pmac.c | 17 +++++++++++++++++ include/linux/ide.h | 10 ++-------- 7 files changed, 78 insertions(+), 30 deletions(-) Index: b/drivers/ide/ide-io.c =================================================================== --- a/drivers/ide/ide-io.c +++ b/drivers/ide/ide-io.c @@ -783,16 +783,17 @@ static void ide_check_pm_state(ide_drive * the bus may be broken enough to walk on our toes at this * point. */ + ide_hwif_t *hwif = drive->hwif; int rc; #ifdef DEBUG_PM printk("%s: Wakeup request inited, waiting for !BSY...\n", drive->name); #endif - rc = ide_wait_not_busy(HWIF(drive), 35000); + rc = ide_wait_not_busy(hwif, 35000); if (rc) printk(KERN_WARNING "%s: bus not ready on wakeup\n", drive->name); SELECT_DRIVE(drive); - ide_set_irq(drive, 1); - rc = ide_wait_not_busy(HWIF(drive), 100000); + hwif->set_irq(hwif, 1); + rc = ide_wait_not_busy(hwif, 100000); if (rc) printk(KERN_WARNING "%s: drive not ready on wakeup\n", drive->name); } @@ -1069,7 +1070,7 @@ static void ide_do_request (ide_hwgroup_ * quirk_list may not like intr setups/cleanups */ if (drive->quirk_list != 1) - ide_set_irq(drive, 0); + hwif->set_irq(hwif, 0); } hwgroup->hwif = hwif; hwgroup->drive = drive; @@ -1547,6 +1548,7 @@ EXPORT_SYMBOL(ide_do_drive_cmd); void ...
Hello. It seems to me that the whole idea of having 2 separate methods for writing the single device control register is a wrong idea. I'm suggesting to convert set_irq() method to write_devctrl() method and pass the register value directly to it. MBR, Sergei --
Makes sense. Care to make a patch? --
Hello. Perhaps with the method itself forcing the obsolete bit 3 set like I would if I had the time. Can only make another notch for now... MBR, Sergei --
Write ATA Feature register before ATA Sector Count register as a preparation to use ->tf_load in ide_config_drive_speed(). This change shouldn't affect anything (just an usual paranoia). Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> --- drivers/ide/ide-iops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/drivers/ide/ide-iops.c =================================================================== --- a/drivers/ide/ide-iops.c +++ b/drivers/ide/ide-iops.c @@ -828,8 +828,8 @@ int ide_config_drive_speed(ide_drive_t * SELECT_MASK(drive, 0); udelay(1); hwif->set_irq(hwif, 0); - hwif->OUTB(speed, io_ports->nsect_addr); hwif->OUTB(SETFEATURES_XFER, io_ports->feature_addr); + hwif->OUTB(speed, io_ports->nsect_addr); hwif->exec_command(hwif, WIN_SETFEATURES); if (drive->quirk_list == 2) hwif->set_irq(hwif, 1); --
Convert ide_config_drive_speed() to use ->tf_load instead of ->OUTB.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-iops.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -792,9 +792,9 @@ int ide_driveid_update(ide_drive_t *driv
int ide_config_drive_speed(ide_drive_t *drive, u8 speed)
{
ide_hwif_t *hwif = drive->hwif;
- struct ide_io_ports *io_ports = &hwif->io_ports;
int error = 0;
u8 stat;
+ ide_task_t task;
#ifdef CONFIG_BLK_DEV_IDEDMA
if (hwif->dma_ops) /* check if host supports DMA */
@@ -828,9 +828,16 @@ int ide_config_drive_speed(ide_drive_t *
SELECT_MASK(drive, 0);
udelay(1);
hwif->set_irq(hwif, 0);
- hwif->OUTB(SETFEATURES_XFER, io_ports->feature_addr);
- hwif->OUTB(speed, io_ports->nsect_addr);
+
+ memset(&task, 0, sizeof(task));
+ task.tf_flags = IDE_TFLAG_OUT_FEATURE | IDE_TFLAG_OUT_NSECT;
+ task.tf.feature = SETFEATURES_XFER;
+ task.tf.nsect = speed;
+
+ hwif->tf_load(drive, &task);
+
hwif->exec_command(hwif, WIN_SETFEATURES);
+
if (drive->quirk_list == 2)
hwif->set_irq(hwif, 1);
--
Convert actual_try_to_identify() to use ->tf_load instead of ->OUTB.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-probe.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -290,9 +290,15 @@ static int actual_try_to_identify (ide_d
/* set features register for atapi
* identify command to be sure of reply
*/
- if ((cmd == WIN_PIDENTIFY))
- /* disable dma & overlap */
- hwif->OUTB(0, io_ports->feature_addr);
+ if (cmd == WIN_PIDENTIFY) {
+ ide_task_t task;
+
+ memset(&task, 0, sizeof(task));
+ /* disable DMA & overlap */
+ task.tf_flags = IDE_TFLAG_OUT_FEATURE;
+
+ drive->hwif->tf_load(drive, &task);
+ }
/* ask drive for ID */
hwif->exec_command(hwif, cmd);
--
Hello. Eww, this is bulky but should be a part of your plan to switch to tf_load() method... WBR, Sergei --
Yes on both matters: - It is a part of The Grand Plan. ;) - I also don't like the small increase of complexity but these changes make the core code completely independent of the hardware I/O registers (allowing any taskfile transport method). PS Thanks for reviewing these patches. Bart --
Convert SELECT_DRIVE() to use ->tf_load instead of ->OUTB.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-iops.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -88,11 +88,15 @@ void SELECT_DRIVE (ide_drive_t *drive)
{
ide_hwif_t *hwif = drive->hwif;
const struct ide_port_ops *port_ops = hwif->port_ops;
+ ide_task_t task;
if (port_ops && port_ops->selectproc)
port_ops->selectproc(drive);
- hwif->OUTB(drive->select.all, hwif->io_ports.device_addr);
+ memset(&task, 0, sizeof(task));
+ task.tf_flags = IDE_TFLAG_OUT_DEVICE;
+
+ drive->hwif->tf_load(drive, &task);
}
void SELECT_MASK(ide_drive_t *drive, int mask)
--
This actually doesn't seem like a bright idea to me, considering that this gets called when starting every request. How will you look at me adding the transport method for writing this register? :-) MBR, Sergei --
OTOH, adding such a "backdoor" to the taskfile doesn't seem very
consistent... well, I'm not excited about the whole idea conversion to
tf_{load|read}() -- it's not clear what exactly this bought us.
MBR, Sergei
--
We at least could have saved on memset() -- tf_load() method ignores fields other than tf_flags anyway... MBR, Sergei --
This was explained some months ago already, so just to recall -- it was
a part of a bigger work removing duplicated code and allowing abstraction
of the ATA logic.
Anyway this is not set in a stone so if you have proposal of a better
Unless it is huge performance win (unlikely) this is not a good idea
as it would be a maintainance nightmare.
->tf_load does only use cmd->tf_flags today but it might change one day
and nobody will remember to audit all users that they pass a valid cmd...
Thanks,
Bart
[1] coincidentally on the past Saturday I woke up with a bright idea
of doing some profiling of IDE code... I thought this would be a fun...
# readprofile -m System.map | grep ide_
30 ide_intr 0.0714
1 ide_map_sg 0.0112
1 ide_complete_rq 0.0152
78 do_ide_request 0.0643
1 __ide_wait_stat 0.0059
51 ide_execute_command 0.5100
4 ide_set_handler 0.0635
7 ide_outb 1.1667
308 ide_mm_inb 44.0000
...
This was on ICH4 PATA controller... Fun indeed, ain't it?
[ For non-ATA folks: it is _impossible_ for ide_mm_inb to be used
on the above controller. ]
I still have to check what will happen if I would change the order
of following assignments in ide_tf_read():
...
if (mmio) {
tf_outb = ide_mm_outb;
tf_inb = ide_mm_inb;
} else {
tf_outb = ide_outb;
tf_inb = ide_inb;
}
...
However I instead I went ahead and tried to run oprofile to get me some
more trushworthy results:
# opreport -l vmlinux
CPU: Pentium M (P6 core), speed 600 MHz (estimated)
Counted CPU_CLK_UNHALTED events (clocks processor is not halted, and not in a thermal trip) with a unit mask of 0x00 (No unit mask) ...Hello.
Er... I think that the previous IN()/OUT() methods were better. Note
that we ended up using the local version of them in the dafault
ide_tf_{load}read}() anyway -- as Alan has pointed out it might be worth
splitting those into I/O and memory space versions... although given
general slowness of the I/O accesses, this is probably not going to win
It's just quite unbearable to see (especially for a long time
assembly coder) how a single register write is turning into *that*.
MBR, Sergei
--
During ide_tf_{load,read}() addition I was a bit too optimistic about
Maybe it would be worth to add ->tf_{inb,outb} to struct ide_tp_ops
and convert default tp_ops to use them... OTOH we should reinvestigate
I see your point here. If SELECT_DRIVE() is performance sensitive we
may just add another struct ide_tp_ops method for it...
Thanks,
Bart
--
Or we may finally teach selectproc() to also do that, turning it into MBR, Sergei --
What I certainly don't like is how tf_load/read() handle LBA48: there's much of the code duplication going on. I'll think what can be done about it but it may not be easy to tackle... it looks like 'struct ide_taskfile' needs MBR, Sergei --
... which didn't prevent libata from having such backdoor though, in the form of dev_select() method. MBR, Sergei --
Oh, and I forgot about the IDE core's infamous selectproc() method which could easily be taught to also write the device/head register and moved to 'struct ide_tp_ops'. MBR, Sergei --
* Add IDE_TFLAG_IN_FEATURE taskfile flag for reading Feature
register and handle it in ->tf_read.
* Convert ide_read_error() to use ->tf_read instead of ->INB,
then uninline and export it.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/h8300/ide-h8300.c | 2 ++
drivers/ide/ide-iops.c | 15 +++++++++++++++
drivers/ide/pci/ns87415.c | 2 ++
drivers/ide/pci/scc_pata.c | 2 ++
include/linux/ide.h | 10 +++-------
5 files changed, 24 insertions(+), 7 deletions(-)
Index: b/drivers/ide/h8300/ide-h8300.c
===================================================================
--- a/drivers/ide/h8300/ide-h8300.c
+++ b/drivers/ide/h8300/ide-h8300.c
@@ -100,6 +100,8 @@ static void h8300_tf_read(ide_drive_t *d
/* be sure we're looking at the low order bits */
outb(ATA_DEVCTL_OBS & ~0x80, io_ports->ctl_addr);
+ if (task->tf_flags & IDE_TFLAG_IN_FEATURE)
+ tf->feature = inb(io_ports->feature_addr);
if (task->tf_flags & IDE_TFLAG_IN_NSECT)
tf->nsect = inb(io_ports->nsect_addr);
if (task->tf_flags & IDE_TFLAG_IN_LBAL)
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -241,6 +241,8 @@ static void ide_tf_read(ide_drive_t *dri
/* be sure we're looking at the low order bits */
tf_outb(ATA_DEVCTL_OBS & ~0x80, io_ports->ctl_addr);
+ if (task->tf_flags & IDE_TFLAG_IN_FEATURE)
+ tf->feature = tf_inb(io_ports->feature_addr);
if (task->tf_flags & IDE_TFLAG_IN_NSECT)
tf->nsect = tf_inb(io_ports->nsect_addr);
if (task->tf_flags & IDE_TFLAG_IN_LBAL)
@@ -390,6 +392,19 @@ void default_hwif_transport(ide_hwif_t *
hwif->output_data = ata_output_data;
}
+u8 ide_read_error(ide_drive_t *drive)
+{
+ ide_task_t task;
+
+ memset(&task, 0, sizeof(task));
+ task.tf_flags = IDE_TFLAG_IN_FEATURE;
+
+ drive->hwif->tf_read(drive, &task);
+
+ return ...Hello. Doesn't seem like a good name -- you can't read the features register. I should've commented to this patch earlier, of course... MBR, Sergei --
Moreover, the error register shoudn't be affeceted by the HOB bit, so this flag doesn't make sense. MBR, Sergei --
I meant IDE_TFLAG_IN_HOB_FEATURE, of course. MBR, Sergei --
Trivial s/IN_*FEATURE/IN_*ERROR/ should do the job. Once again blame the certain ioctl (you know which one)... :) Thanks, Bart --
Hello. I was too busy acquainting myself with the wonderful world of ARM and Well, if I'm going to get my hands in the transport methods anyway, I Oh, horror... and I know that it wasn't completely ungrounded since both ATA/PI-6 adn -7 have words about reading the features register (depending on HOB). At least ATA/PI-8 got rid of this. Anyway, I think we can safely get rid of this flag and just return the same value in 'features' and 'hob_features'. Besides, union ide_reg_valid_s also has such interesting fields as 'select_hob' and 'control_hob' but they seem MBR, Sergei --
Although, after thinking a bit more, it doesn't seem worth the trouble -- so the rename should suffice... MBR, Sergei --
Add ide_read_device() helper and convert do_probe() to use it
instead of ->INB.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-probe.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -410,6 +410,18 @@ static int ide_busy_sleep(ide_hwif_t *hw
return 1;
}
+static u8 ide_read_device(ide_drive_t *drive)
+{
+ ide_task_t task;
+
+ memset(&task, 0, sizeof(task));
+ task.tf_flags = IDE_TFLAG_IN_DEVICE;
+
+ drive->hwif->tf_read(drive, &task);
+
+ return task.tf.device;
+}
+
/**
* do_probe - probe an IDE device
* @drive: drive to probe
@@ -434,7 +446,6 @@ static int ide_busy_sleep(ide_hwif_t *hw
static int do_probe (ide_drive_t *drive, u8 cmd)
{
ide_hwif_t *hwif = HWIF(drive);
- struct ide_io_ports *io_ports = &hwif->io_ports;
int rc;
u8 stat;
@@ -455,8 +466,8 @@ static int do_probe (ide_drive_t *drive,
msleep(50);
SELECT_DRIVE(drive);
msleep(50);
- if (hwif->INB(io_ports->device_addr) != drive->select.all &&
- !drive->present) {
+
+ if (ide_read_device(drive) != drive->select.all && !drive->present) {
if (drive->select.b.unit != 0) {
/* exit with drive0 selected */
SELECT_DRIVE(&hwif->drives[0]);
--
Add ide_read_ireason() helper and use instead of ->INB for reading
ATAPI Interrupt Reason register.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-atapi.c | 17 ++++++++++++++---
drivers/ide/ide-iops.c | 13 +++++++++++++
2 files changed, 27 insertions(+), 3 deletions(-)
Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -183,16 +183,27 @@ cmd_finished:
}
EXPORT_SYMBOL_GPL(ide_pc_intr);
+static u8 ide_read_ireason(ide_drive_t *drive)
+{
+ ide_task_t task;
+
+ memset(&task, 0, sizeof(task));
+ task.tf_flags = IDE_TFLAG_IN_NSECT;
+
+ drive->hwif->tf_read(drive, &task);
+
+ return task.tf.nsect & 3;
+}
+
static u8 ide_wait_ireason(ide_drive_t *drive, u8 ireason)
{
- ide_hwif_t *hwif = drive->hwif;
int retries = 100;
while (retries-- && ((ireason & CD) == 0 || (ireason & IO))) {
printk(KERN_ERR "%s: (IO,CoD != (0,1) while issuing "
"a packet command, retrying\n", drive->name);
udelay(100);
- ireason = hwif->INB(hwif->io_ports.nsect_addr);
+ ireason = ide_read_ireason(drive);
if (retries == 0) {
printk(KERN_ERR "%s: (IO,CoD != (0,1) while issuing "
"a packet command, ignoring\n",
@@ -219,7 +230,7 @@ ide_startstop_t ide_transfer_pc(ide_driv
return startstop;
}
- ireason = hwif->INB(hwif->io_ports.nsect_addr);
+ ireason = ide_read_ireason(drive);
if (drive->media == ide_tape && !drive->scsi)
ireason = ide_wait_ireason(drive, ireason);
--
Add ide_read_bcount_and_ireason() helper and use it instead of ->INB
in {cdrom_newpc,ide_pc}_intr().
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-atapi.c | 6 ++----
drivers/ide/ide-cd.c | 12 ++++--------
drivers/ide/ide-iops.c | 15 +++++++++++++++
include/linux/ide.h | 1 +
4 files changed, 22 insertions(+), 12 deletions(-)
Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -107,11 +107,9 @@ cmd_finished:
ide_dma_off(drive);
return ide_do_reset(drive);
}
- /* Get the number of bytes to transfer on this interrupt. */
- bcount = (hwif->INB(hwif->io_ports.lbah_addr) << 8) |
- hwif->INB(hwif->io_ports.lbam_addr);
- ireason = hwif->INB(hwif->io_ports.nsect_addr);
+ /* Get the number of bytes to transfer on this interrupt. */
+ ide_read_bcount_and_ireason(drive, &bcount, &ireason);
if (ireason & CD) {
printk(KERN_ERR "%s: CoD != 0 in %s\n", drive->name, __func__);
Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -923,10 +923,11 @@ static ide_startstop_t cdrom_newpc_intr(
struct request *rq = HWGROUP(drive)->rq;
xfer_func_t *xferfunc;
ide_expiry_t *expiry = NULL;
- int dma_error = 0, dma, stat, ireason, len, thislen, uptodate = 0;
+ int dma_error = 0, dma, stat, thislen, uptodate = 0;
int write = (rq_data_dir(rq) == WRITE) ? 1 : 0;
unsigned int timeout;
- u8 lowcyl, highcyl;
+ u16 len;
+ u8 ireason;
/* check for errors */
dma = info->dma;
@@ -954,12 +955,7 @@ static ide_startstop_t cdrom_newpc_intr(
goto end_request;
}
- /* ok we fall to pio :/ */
- ireason = hwif->INB(hwif->io_ports.nsect_addr) & 0x3;
- lowcyl = hwif->INB(hwif->io_ports.lbam_addr);
- highcyl = hwif->INB(hwif->io_ports.lbah_addr);
-
- len = lowcyl + (256 * ...* Remove no longer needed ->INB, ->OUTB and ->OUTBSYNC methods.
Then:
* Remove no longer used default_hwif_[mm]iops() and ide_[mm_]outbsync().
* Cleanup SuperIO handling in ns87415.c.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/arm/icside.c | 5 ----
drivers/ide/arm/palm_bk3710.c | 2 -
drivers/ide/arm/rapide.c | 2 -
drivers/ide/h8300/ide-h8300.c | 2 -
drivers/ide/ide-iops.c | 28 -----------------------
drivers/ide/ide.c | 1
drivers/ide/legacy/ide_platform.c | 4 ---
drivers/ide/mips/swarm.c | 3 --
drivers/ide/pci/ns87415.c | 46 ++++++++++----------------------------
drivers/ide/pci/scc_pata.c | 12 ---------
drivers/ide/pci/sgiioc4.c | 3 --
drivers/ide/pci/siimage.c | 2 -
drivers/ide/ppc/pmac.c | 13 ----------
include/linux/ide.h | 7 -----
14 files changed, 15 insertions(+), 115 deletions(-)
Index: b/drivers/ide/arm/icside.c
===================================================================
--- a/drivers/ide/arm/icside.c
+++ b/drivers/ide/arm/icside.c
@@ -469,8 +469,6 @@ icside_register_v5(struct icside_state *
if (!hwif)
return -ENODEV;
- default_hwif_mmiops(hwif);
-
state->hwif[0] = hwif;
ecard_set_drvdata(ec, state);
@@ -547,14 +545,11 @@ icside_register_v6(struct icside_state *
return -ENODEV;
hwif->chipset = ide_acorn;
- default_hwif_mmiops(hwif);
idx[0] = hwif->index;
mate = ide_find_port();
if (mate) {
- default_hwif_mmiops(mate);
-
hws[1] = &hw[1];
idx[1] = mate->index;
}
Index: b/drivers/ide/arm/palm_bk3710.c
===================================================================
--- a/drivers/ide/arm/palm_bk3710.c
+++ b/drivers/ide/arm/palm_bk3710.c
@@ -401,8 +401,6 @@ static int __devinit palm_bk3710_probe(s
i = hwif->index;
...It's probably too late to object now (and I've been busy before) but I
I also didn't understand the motivation behind putting this method
together with the transport operations... IMO, DMA programming interface
hardly has anything to do with transporting the data over IDE bus.
MBR, Sergei
--
The motivation was that hwif->dma_ops is not available yet when ->read_sff_dma_status is used in ide_pci_check_simplex(). However I agree that it should somehow find its way into ->dma_ops (as usual patches are stongly preffered :). Thanks, Bart --
Hello.
Unless I'm missing something changing the place where hwif->dma_ops is
initialized to sff_dma_ops (along the lines it was changed for hwif->dma_base)
seems pretty trivial, so I wonder why you didn't do it in the same patch...
MBR, Sergei
--
Nevermind, it's just ide_read_sff_dma_status() used for hwif->tp_ops-> read_sff_dma_status() method in the driver where it clearly doesn't make sense. But it seems it never gets called in these cases anyway (only adds a MBR, Sergei --
Indeed, it should be trivial now, one just needs to be careful to: * move 'if (d->dma_ops) ...' from ide_init_port() into ->init_dma/ide_hwif_setup_dma() * unset ->dma_ops on ->init_dma/ide_hwif_setup_dma() failures I guess I overlooked it ATM of making the patch (or the code evolved greatly in the meantime)... [ It is really time consuming and difficult to recall the every small detail of every patch after few months (the patch was posted 10 weeks ago and merged 6 weeks ago)... The most efficient way of handling such issues upon discovery is with sending patches... ] Thanks, Bart --
Hello. Ah, I forgot for a moment that there were two patches and it would have make no sense to do that in the patch that factored out ide_pci_check_simplex()... And then tre was a patch introducing 'struct I think I understand now: it's sticking read_sff_dma_status() method into 'struct ide_tp_ops' that was a wrong move that's worth undoing (by Heh, as if it wasn't time consuming to untange that after a few MBR, Sergei --
