* [PATCH 1/6] ide: pass command to ide_map_sg()
2009-02-09 23:19 [PATCH 0/6] ide: more unifications of ATA and ATAPI support Bartlomiej Zolnierkiewicz
@ 2009-02-09 23:19 ` Bartlomiej Zolnierkiewicz
2009-02-11 6:36 ` Borislav Petkov
2009-02-09 23:19 ` [PATCH 2/6] ide: use do_rw_taskfile() for ATA_CMD_PACKET commands Bartlomiej Zolnierkiewicz
` (5 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-09 23:19 UTC (permalink / raw)
To: linux-ide; +Cc: Borislav Petkov, Bartlomiej Zolnierkiewicz, linux-kernel
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: pass command to ide_map_sg()
* Set IDE_TFLAG_WRITE flag and ->rq also for ATA_CMD_PACKET
commands.
* Pass command to ->dma_setup method and update all its
implementations accordingly.
* Pass command instead of request to ide_build_sglist(),
*_build_dmatable() and ide_map_sg().
While at it:
* Fix scc_dma_setup() documentation + use ATA_DMA_WR define.
* Rename sgiioc4_build_dma_table() to sgiioc4_build_dmatable(),
change return value type to 'int' and drop unused 'ddir'
argument.
* Do some minor cleanups in [tx4939]ide_dma_setup().
There should be no functional changes caused by this patch.
Cc: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/alim15x3.c | 7 ++++---
drivers/ide/au1xxx-ide.c | 15 ++++++---------
drivers/ide/icside.c | 7 +++----
drivers/ide/ide-atapi.c | 16 ++++++++++++----
drivers/ide/ide-disk.c | 10 +++++-----
drivers/ide/ide-dma-sff.c | 18 +++++++++---------
drivers/ide/ide-dma.c | 15 +++++++--------
drivers/ide/ide-floppy.c | 6 +++++-
drivers/ide/ide-io.c | 6 +++---
drivers/ide/ide-taskfile.c | 4 ++--
drivers/ide/ns87415.c | 4 ++--
drivers/ide/pmac.c | 19 ++++++++-----------
drivers/ide/scc_pata.c | 19 +++++++------------
drivers/ide/sgiioc4.c | 21 +++++++--------------
drivers/ide/trm290.c | 10 +++++-----
drivers/ide/tx4939ide.c | 26 ++++++++++----------------
include/linux/ide.h | 12 ++++++------
17 files changed, 101 insertions(+), 114 deletions(-)
Index: b/drivers/ide/alim15x3.c
===================================================================
--- a/drivers/ide/alim15x3.c
+++ b/drivers/ide/alim15x3.c
@@ -191,17 +191,18 @@ static void ali_set_dma_mode(ide_drive_t
/**
* ali15x3_dma_setup - begin a DMA phase
* @drive: target device
+ * @cmd: command
*
* Returns 1 if the DMA cannot be performed, zero on success.
*/
-static int ali15x3_dma_setup(ide_drive_t *drive)
+static int ali15x3_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
if (m5229_revision < 0xC2 && drive->media != ide_disk) {
- if (rq_data_dir(drive->hwif->rq))
+ if (cmd->tf_flags & IDE_TFLAG_WRITE)
return 1; /* try PIO instead of DMA */
}
- return ide_dma_setup(drive);
+ return ide_dma_setup(drive, cmd);
}
/**
Index: b/drivers/ide/au1xxx-ide.c
===================================================================
--- a/drivers/ide/au1xxx-ide.c
+++ b/drivers/ide/au1xxx-ide.c
@@ -209,15 +209,14 @@ static void auide_set_dma_mode(ide_drive
*/
#ifdef CONFIG_BLK_DEV_IDE_AU1XXX_MDMA2_DBDMA
-static int auide_build_dmatable(ide_drive_t *drive)
+static int auide_build_dmatable(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
- struct request *rq = hwif->rq;
_auide_hwif *ahwif = &auide_hwif;
struct scatterlist *sg;
- int i = hwif->cmd.sg_nents, iswrite, count = 0;
+ int i = cmd->sg_nents, count = 0;
+ int iswrite = !!(cmd->tf_flags & IDE_TFLAG_WRITE);
- iswrite = (rq_data_dir(rq) == WRITE);
/* Save for interrupt context */
ahwif->drive = drive;
@@ -298,12 +297,10 @@ static void auide_dma_exec_cmd(ide_drive
(2*WAIT_CMD), NULL);
}
-static int auide_dma_setup(ide_drive_t *drive)
+static int auide_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
- struct request *rq = drive->hwif->rq;
-
- if (!auide_build_dmatable(drive)) {
- ide_map_sg(drive, rq);
+ if (auide_build_dmatable(drive, cmd) == 0) {
+ ide_map_sg(drive, cmd);
return 1;
}
Index: b/drivers/ide/icside.c
===================================================================
--- a/drivers/ide/icside.c
+++ b/drivers/ide/icside.c
@@ -307,15 +307,14 @@ static void icside_dma_start(ide_drive_t
enable_dma(ec->dma);
}
-static int icside_dma_setup(ide_drive_t *drive)
+static int icside_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
struct expansion_card *ec = ECARD_DEV(hwif->dev);
struct icside_state *state = ecard_get_drvdata(ec);
- struct request *rq = hwif->rq;
unsigned int dma_mode;
- if (rq_data_dir(rq))
+ if (cmd->tf_flags & IDE_TFLAG_WRITE)
dma_mode = DMA_MODE_WRITE;
else
dma_mode = DMA_MODE_READ;
@@ -344,7 +343,7 @@ static int icside_dma_setup(ide_drive_t
* Tell the DMA engine about the SG table and
* data direction.
*/
- set_dma_sg(ec->dma, hwif->sg_table, hwif->cmd.sg_nents);
+ set_dma_sg(ec->dma, hwif->sg_table, cmd->sg_nents);
set_dma_mode(ec->dma, dma_mode);
drive->waiting_for_dma = 1;
Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -626,12 +626,20 @@ ide_startstop_t ide_issue_pc(ide_drive_t
{
struct ide_atapi_pc *pc;
ide_hwif_t *hwif = drive->hwif;
+ const struct ide_dma_ops *dma_ops = hwif->dma_ops;
+ struct ide_cmd *cmd = &hwif->cmd;
ide_expiry_t *expiry = NULL;
struct request *rq = hwif->rq;
unsigned int timeout;
u32 tf_flags;
u16 bcount;
+ if (drive->media != ide_floppy) {
+ if (rq_data_dir(rq))
+ cmd->tf_flags |= IDE_TFLAG_WRITE;
+ cmd->rq = rq;
+ }
+
if (dev_is_idecd(drive)) {
tf_flags = IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL;
bcount = ide_cd_get_xferlen(rq);
@@ -639,8 +647,8 @@ ide_startstop_t ide_issue_pc(ide_drive_t
timeout = ATAPI_WAIT_PC;
if (drive->dma) {
- if (ide_build_sglist(drive, rq))
- drive->dma = !hwif->dma_ops->dma_setup(drive);
+ if (ide_build_sglist(drive, cmd))
+ drive->dma = !dma_ops->dma_setup(drive, cmd);
else
drive->dma = 0;
}
@@ -663,8 +671,8 @@ ide_startstop_t ide_issue_pc(ide_drive_t
if ((pc->flags & PC_FLAG_DMA_OK) &&
(drive->dev_flags & IDE_DFLAG_USING_DMA)) {
- if (ide_build_sglist(drive, rq))
- drive->dma = !hwif->dma_ops->dma_setup(drive);
+ if (ide_build_sglist(drive, cmd))
+ drive->dma = !dma_ops->dma_setup(drive, cmd);
else
drive->dma = 0;
}
Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -99,11 +99,6 @@ static ide_startstop_t __ide_do_rw_disk(
memset(&cmd, 0, sizeof(cmd));
cmd.tf_flags = IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
- if (dma == 0) {
- ide_init_sg_cmd(&cmd, nsectors);
- ide_map_sg(drive, rq);
- }
-
if (drive->dev_flags & IDE_DFLAG_LBA) {
if (lba48) {
pr_debug("%s: LBA=0x%012llx\n", drive->name,
@@ -156,6 +151,11 @@ static ide_startstop_t __ide_do_rw_disk(
ide_tf_set_cmd(drive, &cmd, dma);
cmd.rq = rq;
+ if (dma == 0) {
+ ide_init_sg_cmd(&cmd, nsectors);
+ ide_map_sg(drive, &cmd);
+ }
+
rc = do_rw_taskfile(drive, &cmd);
if (rc == ide_stopped && dma) {
Index: b/drivers/ide/ide-dma-sff.c
===================================================================
--- a/drivers/ide/ide-dma-sff.c
+++ b/drivers/ide/ide-dma-sff.c
@@ -111,7 +111,7 @@ EXPORT_SYMBOL_GPL(ide_dma_host_set);
* May also be invoked from trm290.c
*/
-int ide_build_dmatable(ide_drive_t *drive, struct request *rq)
+int ide_build_dmatable(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
__le32 *table = (__le32 *)hwif->dmatable_cpu;
@@ -120,7 +120,7 @@ int ide_build_dmatable(ide_drive_t *driv
struct scatterlist *sg;
u8 is_trm290 = !!(hwif->host_flags & IDE_HFLAG_TRM290);
- for_each_sg(hwif->sg_table, sg, hwif->cmd.sg_nents, i) {
+ for_each_sg(hwif->sg_table, sg, cmd->sg_nents, i) {
u32 cur_addr, cur_len, xcount, bcount;
cur_addr = sg_dma_address(sg);
@@ -175,6 +175,7 @@ EXPORT_SYMBOL_GPL(ide_build_dmatable);
/**
* ide_dma_setup - begin a DMA phase
* @drive: target device
+ * @cmd: command
*
* Build an IDE DMA PRD (IDE speak for scatter gather table)
* and then set up the DMA transfer registers for a device
@@ -185,17 +186,16 @@ EXPORT_SYMBOL_GPL(ide_build_dmatable);
* is returned.
*/
-int ide_dma_setup(ide_drive_t *drive)
+int ide_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
- struct request *rq = hwif->rq;
- unsigned int reading = rq_data_dir(rq) ? 0 : ATA_DMA_WR;
u8 mmio = (hwif->host_flags & IDE_HFLAG_MMIO) ? 1 : 0;
+ u8 rw = (cmd->tf_flags & IDE_TFLAG_WRITE) ? 0 : ATA_DMA_WR;
u8 dma_stat;
/* fall back to pio! */
- if (!ide_build_dmatable(drive, rq)) {
- ide_map_sg(drive, rq);
+ if (ide_build_dmatable(drive, cmd) == 0) {
+ ide_map_sg(drive, cmd);
return 1;
}
@@ -208,9 +208,9 @@ int ide_dma_setup(ide_drive_t *drive)
/* specify r/w */
if (mmio)
- writeb(reading, (void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
+ writeb(rw, (void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
else
- outb(reading, hwif->dma_base + ATA_DMA_CMD);
+ outb(rw, hwif->dma_base + ATA_DMA_CMD);
/* read DMA status for INTR & ERROR flags */
dma_stat = hwif->dma_ops->dma_sff_read_status(hwif);
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -120,7 +120,7 @@ int ide_dma_good_drive(ide_drive_t *driv
/**
* ide_build_sglist - map IDE scatter gather for DMA I/O
* @drive: the drive to build the DMA table for
- * @rq: the request holding the sg list
+ * @cmd: command
*
* Perform the DMA mapping magic necessary to access the source or
* target buffers of a request via DMA. The lower layers of the
@@ -128,23 +128,22 @@ int ide_dma_good_drive(ide_drive_t *driv
* operate in a portable fashion.
*/
-int ide_build_sglist(ide_drive_t *drive, struct request *rq)
+int ide_build_sglist(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
struct scatterlist *sg = hwif->sg_table;
- struct ide_cmd *cmd = &hwif->cmd;
int i;
- ide_map_sg(drive, rq);
+ ide_map_sg(drive, cmd);
- if (rq_data_dir(rq) == READ)
- cmd->sg_dma_direction = DMA_FROM_DEVICE;
- else
+ if (cmd->tf_flags & IDE_TFLAG_WRITE)
cmd->sg_dma_direction = DMA_TO_DEVICE;
+ else
+ cmd->sg_dma_direction = DMA_FROM_DEVICE;
i = dma_map_sg(hwif->dev, sg, cmd->sg_nents, cmd->sg_dma_direction);
if (i == 0)
- ide_map_sg(drive, rq);
+ ide_map_sg(drive, cmd);
return i;
}
Index: b/drivers/ide/ide-floppy.c
===================================================================
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -285,8 +285,12 @@ static ide_startstop_t ide_floppy_do_req
goto out_end;
}
+ if (rq_data_dir(rq))
+ cmd->tf_flags |= IDE_TFLAG_WRITE;
+ cmd->rq = rq;
+
ide_init_sg_cmd(cmd, rq->nr_sectors);
- ide_map_sg(drive, rq);
+ ide_map_sg(drive, cmd);
pc->sg = hwif->sg_table;
pc->sg_cnt = cmd->sg_nents;
Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -224,11 +224,11 @@ static ide_startstop_t do_special (ide_d
return ide_stopped;
}
-void ide_map_sg(ide_drive_t *drive, struct request *rq)
+void ide_map_sg(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
- struct ide_cmd *cmd = &hwif->cmd;
struct scatterlist *sg = hwif->sg_table;
+ struct request *rq = cmd->rq;
if (rq->cmd_type == REQ_TYPE_ATA_TASKFILE) {
sg_init_one(sg, rq->buffer, rq->nr_sectors * SECTOR_SIZE);
@@ -269,7 +269,7 @@ static ide_startstop_t execute_drive_cmd
if (cmd) {
if (cmd->protocol == ATA_PROT_PIO) {
ide_init_sg_cmd(cmd, rq->nr_sectors);
- ide_map_sg(drive, rq);
+ ide_map_sg(drive, cmd);
}
return do_rw_taskfile(drive, cmd);
Index: b/drivers/ide/ide-taskfile.c
===================================================================
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -102,8 +102,8 @@ ide_startstop_t do_rw_taskfile(ide_drive
return ide_started;
default:
if ((drive->dev_flags & IDE_DFLAG_USING_DMA) == 0 ||
- ide_build_sglist(drive, hwif->rq) == 0 ||
- dma_ops->dma_setup(drive))
+ ide_build_sglist(drive, cmd) == 0 ||
+ dma_ops->dma_setup(drive, cmd))
return ide_stopped;
dma_ops->dma_exec_cmd(drive, tf->command);
dma_ops->dma_start(drive);
Index: b/drivers/ide/ns87415.c
===================================================================
--- a/drivers/ide/ns87415.c
+++ b/drivers/ide/ns87415.c
@@ -216,11 +216,11 @@ static int ns87415_dma_end(ide_drive_t *
return (dma_stat & 7) != 4;
}
-static int ns87415_dma_setup(ide_drive_t *drive)
+static int ns87415_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
/* select DMA xfer */
ns87415_prepare_drive(drive, 1);
- if (!ide_dma_setup(drive))
+ if (ide_dma_setup(drive, cmd) == 0)
return 0;
/* DMA failed: select PIO xfer */
ns87415_prepare_drive(drive, 0);
Index: b/drivers/ide/pmac.c
===================================================================
--- a/drivers/ide/pmac.c
+++ b/drivers/ide/pmac.c
@@ -404,7 +404,6 @@ kauai_lookup_timing(struct kauai_timing*
#define IDE_WAKEUP_DELAY (1*HZ)
static int pmac_ide_init_dma(ide_hwif_t *, const struct ide_port_info *);
-static int pmac_ide_build_dmatable(ide_drive_t *drive, struct request *rq);
static void pmac_ide_selectproc(ide_drive_t *drive);
static void pmac_ide_kauai_selectproc(ide_drive_t *drive);
@@ -1422,8 +1421,7 @@ out:
* pmac_ide_build_dmatable builds the DBDMA command list
* for a transfer and sets the DBDMA channel to point to it.
*/
-static int
-pmac_ide_build_dmatable(ide_drive_t *drive, struct request *rq)
+static int pmac_ide_build_dmatable(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
pmac_ide_hwif_t *pmif =
@@ -1431,8 +1429,8 @@ pmac_ide_build_dmatable(ide_drive_t *dri
struct dbdma_cmd *table;
volatile struct dbdma_regs __iomem *dma = pmif->dma_regs;
struct scatterlist *sg;
- int wr = (rq_data_dir(rq) == WRITE);
- int i = hwif->cmd.sg_nents, count = 0;
+ int wr = !!(cmd->tf_flags & IDE_TFLAG_WRITE);
+ int i = cmd->sg_nents, count = 0;
/* DMA table is already aligned */
table = (struct dbdma_cmd *) pmif->dma_table_cpu;
@@ -1504,23 +1502,22 @@ use_pio_instead:
* Prepare a DMA transfer. We build the DMA table, adjust the timings for
* a read on KeyLargo ATA/66 and mark us as waiting for DMA completion
*/
-static int
-pmac_ide_dma_setup(ide_drive_t *drive)
+static int pmac_ide_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
pmac_ide_hwif_t *pmif =
(pmac_ide_hwif_t *)dev_get_drvdata(hwif->gendev.parent);
- struct request *rq = hwif->rq;
u8 unit = drive->dn & 1, ata4 = (pmif->kind == controller_kl_ata4);
+ u8 write = !!(cmd->tf_flags & IDE_TFLAG_WRITE);
- if (!pmac_ide_build_dmatable(drive, rq)) {
- ide_map_sg(drive, rq);
+ if (pmac_ide_build_dmatable(drive, cmd) == 0) {
+ ide_map_sg(drive, cmd);
return 1;
}
/* Apple adds 60ns to wrDataSetup on reads */
if (ata4 && (pmif->timings[unit] & TR_66_UDMA_EN)) {
- writel(pmif->timings[unit] + (!rq_data_dir(rq) ? 0x00800000UL : 0),
+ writel(pmif->timings[unit] + (write ? 0 : 0x00800000UL),
PMAC_IDE_REG(IDE_TIMING_CONFIG));
(void)readl(PMAC_IDE_REG(IDE_TIMING_CONFIG));
}
Index: b/drivers/ide/scc_pata.c
===================================================================
--- a/drivers/ide/scc_pata.c
+++ b/drivers/ide/scc_pata.c
@@ -303,8 +303,9 @@ static void scc_dma_host_set(ide_drive_t
}
/**
- * scc_ide_dma_setup - begin a DMA phase
+ * scc_dma_setup - begin a DMA phase
* @drive: target device
+ * @cmd: command
*
* Build an IDE DMA PRD (IDE speak for scatter gather table)
* and then set up the DMA transfer registers.
@@ -313,21 +314,15 @@ static void scc_dma_host_set(ide_drive_t
* is returned.
*/
-static int scc_dma_setup(ide_drive_t *drive)
+static int scc_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
- struct request *rq = hwif->rq;
- unsigned int reading;
+ u32 rw = (cmd->tf_flags & IDE_TFLAG_WRITE) ? 0 : ATA_DMA_WR;
u8 dma_stat;
- if (rq_data_dir(rq))
- reading = 0;
- else
- reading = 1 << 3;
-
/* fall back to pio! */
- if (!ide_build_dmatable(drive, rq)) {
- ide_map_sg(drive, rq);
+ if (ide_build_dmatable(drive, cmd) == 0) {
+ ide_map_sg(drive, cmd);
return 1;
}
@@ -335,7 +330,7 @@ static int scc_dma_setup(ide_drive_t *dr
out_be32((void __iomem *)(hwif->dma_base + 8), hwif->dmatable_dma);
/* specify r/w */
- out_be32((void __iomem *)hwif->dma_base, reading);
+ out_be32((void __iomem *)hwif->dma_base, rw);
/* read DMA status for INTR & ERROR flags */
dma_stat = scc_dma_sff_read_status(hwif);
Index: b/drivers/ide/sgiioc4.c
===================================================================
--- a/drivers/ide/sgiioc4.c
+++ b/drivers/ide/sgiioc4.c
@@ -424,12 +424,11 @@ sgiioc4_configure_for_dma(int dma_direct
/* | Upper 32 bits - Zero |EOL| 15 unused | 16 Bit Length| */
/* --------------------------------------------------------------------- */
/* Creates the scatter gather list, DMA Table */
-static unsigned int
-sgiioc4_build_dma_table(ide_drive_t * drive, struct request *rq, int ddir)
+static int sgiioc4_build_dmatable(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
unsigned int *table = hwif->dmatable_cpu;
- unsigned int count = 0, i = hwif->cmd.sg_nents;
+ unsigned int count = 0, i = cmd->sg_nents;
struct scatterlist *sg = hwif->sg_table;
while (i && sg_dma_len(sg)) {
@@ -484,24 +483,18 @@ use_pio_instead:
return 0; /* revert to PIO for this request */
}
-static int sgiioc4_dma_setup(ide_drive_t *drive)
+static int sgiioc4_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
- struct request *rq = drive->hwif->rq;
- unsigned int count = 0;
int ddir;
+ u8 write = !!(cmd->tf_flags & IDE_TFLAG_WRITE);
- if (rq_data_dir(rq))
- ddir = PCI_DMA_TODEVICE;
- else
- ddir = PCI_DMA_FROMDEVICE;
-
- if (!(count = sgiioc4_build_dma_table(drive, rq, ddir))) {
+ if (sgiioc4_build_dmatable(drive, cmd) == 0) {
/* try PIO instead of DMA */
- ide_map_sg(drive, rq);
+ ide_map_sg(drive, cmd);
return 1;
}
- if (rq_data_dir(rq))
+ if (write)
/* Writes TO the IOC4 FROM Main Memory */
ddir = IOC4_DMA_READ;
else
Index: b/drivers/ide/trm290.c
===================================================================
--- a/drivers/ide/trm290.c
+++ b/drivers/ide/trm290.c
@@ -181,13 +181,12 @@ static void trm290_dma_exec_cmd(ide_driv
ide_execute_command(drive, command, &ide_dma_intr, WAIT_CMD, NULL);
}
-static int trm290_dma_setup(ide_drive_t *drive)
+static int trm290_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
- struct request *rq = hwif->rq;
unsigned int count, rw;
- if (rq_data_dir(rq)) {
+ if (cmd->tf_flags & IDE_TFLAG_WRITE) {
#ifdef TRM290_NO_DMA_WRITES
/* always use PIO for writes */
trm290_prepare_drive(drive, 0); /* select PIO xfer */
@@ -197,8 +196,9 @@ static int trm290_dma_setup(ide_drive_t
} else
rw = 2;
- if (!(count = ide_build_dmatable(drive, rq))) {
- ide_map_sg(drive, rq);
+ count = ide_build_dmatable(drive, cmd);
+ if (count == 0) {
+ ide_map_sg(drive, cmd);
/* try PIO instead of DMA */
trm290_prepare_drive(drive, 0); /* select PIO xfer */
return 1;
Index: b/drivers/ide/tx4939ide.c
===================================================================
--- a/drivers/ide/tx4939ide.c
+++ b/drivers/ide/tx4939ide.c
@@ -232,7 +232,7 @@ static u8 tx4939ide_clear_dma_status(voi
#ifdef __BIG_ENDIAN
/* custom ide_build_dmatable to handle swapped layout */
-static int tx4939ide_build_dmatable(ide_drive_t *drive, struct request *rq)
+static int tx4939ide_build_dmatable(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
u32 *table = (u32 *)hwif->dmatable_cpu;
@@ -240,7 +240,7 @@ static int tx4939ide_build_dmatable(ide_
int i;
struct scatterlist *sg;
- for_each_sg(hwif->sg_table, sg, hwif->cmd.sg_nents, i) {
+ for_each_sg(hwif->sg_table, sg, cmd->sg_nents, i) {
u32 cur_addr, cur_len, bcount;
cur_addr = sg_dma_address(sg);
@@ -287,23 +287,15 @@ use_pio_instead:
#define tx4939ide_build_dmatable ide_build_dmatable
#endif
-static int tx4939ide_dma_setup(ide_drive_t *drive)
+static int tx4939ide_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
void __iomem *base = TX4939IDE_BASE(hwif);
- struct request *rq = hwif->rq;
- u8 reading;
- int nent;
-
- if (rq_data_dir(rq))
- reading = 0;
- else
- reading = ATA_DMA_WR;
+ u8 rw = (cmd->tf_flags & IDE_TFLAG_WRITE) ? 0 : ATA_DMA_WR;
/* fall back to PIO! */
- nent = tx4939ide_build_dmatable(drive, rq);
- if (!nent) {
- ide_map_sg(drive, rq);
+ if (tx4939ide_build_dmatable(drive, cmd) == 0) {
+ ide_map_sg(drive, cmd);
return 1;
}
@@ -311,7 +303,7 @@ static int tx4939ide_dma_setup(ide_drive
tx4939ide_writel(hwif->dmatable_dma, base, TX4939IDE_PRD_Ptr);
/* specify r/w */
- tx4939ide_writeb(reading, base, TX4939IDE_DMA_Cmd);
+ tx4939ide_writeb(rw, base, TX4939IDE_DMA_Cmd);
/* clear INTR & ERROR flags */
tx4939ide_clear_dma_status(base);
@@ -320,7 +312,9 @@ static int tx4939ide_dma_setup(ide_drive
tx4939ide_writew(SECTOR_SIZE / 2, base, drive->dn ?
TX4939IDE_Xfer_Cnt_2 : TX4939IDE_Xfer_Cnt_1);
- tx4939ide_writew(rq->nr_sectors, base, TX4939IDE_Sec_Cnt);
+
+ tx4939ide_writew(cmd->rq->nr_sectors, base, TX4939IDE_Sec_Cnt);
+
return 0;
}
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -713,7 +713,7 @@ struct ide_port_ops {
struct ide_dma_ops {
void (*dma_host_set)(struct ide_drive_s *, int);
- int (*dma_setup)(struct ide_drive_s *);
+ int (*dma_setup)(struct ide_drive_s *, struct ide_cmd *);
void (*dma_exec_cmd)(struct ide_drive_s *, u8);
void (*dma_start)(struct ide_drive_s *);
int (*dma_end)(struct ide_drive_s *);
@@ -1409,7 +1409,7 @@ int ide_pci_resume(struct pci_dev *);
#define ide_pci_resume NULL
#endif
-void ide_map_sg(ide_drive_t *, struct request *);
+void ide_map_sg(ide_drive_t *, struct ide_cmd *);
void ide_init_sg_cmd(struct ide_cmd *, int);
#define BAD_DMA_DRIVE 0
@@ -1444,14 +1444,14 @@ ide_startstop_t ide_dma_intr(ide_drive_t
int ide_allocate_dma_engine(ide_hwif_t *);
void ide_release_dma_engine(ide_hwif_t *);
-int ide_build_sglist(ide_drive_t *, struct request *);
+int ide_build_sglist(ide_drive_t *, struct ide_cmd *);
void ide_destroy_dmatable(ide_drive_t *);
#ifdef CONFIG_BLK_DEV_IDEDMA_SFF
int config_drive_for_dma(ide_drive_t *);
-extern int ide_build_dmatable(ide_drive_t *, struct request *);
+int ide_build_dmatable(ide_drive_t *, struct ide_cmd *);
void ide_dma_host_set(ide_drive_t *, int);
-extern int ide_dma_setup(ide_drive_t *);
+int ide_dma_setup(ide_drive_t *, struct ide_cmd *);
void ide_dma_exec_cmd(ide_drive_t *, u8);
extern void ide_dma_start(ide_drive_t *);
int ide_dma_end(ide_drive_t *);
@@ -1479,7 +1479,7 @@ static inline void ide_check_dma_crc(ide
static inline ide_startstop_t ide_dma_timeout_retry(ide_drive_t *drive, int error) { return ide_stopped; }
static inline void ide_release_dma_engine(ide_hwif_t *hwif) { ; }
static inline int ide_build_sglist(ide_drive_t *drive,
- struct request *rq) { return 0; }
+ struct ide_cmd *cmd) { return 0; }
#endif /* CONFIG_BLK_DEV_IDEDMA */
#ifdef CONFIG_BLK_DEV_IDEACPI
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/6] ide: pass command to ide_map_sg()
2009-02-09 23:19 ` [PATCH 1/6] ide: pass command to ide_map_sg() Bartlomiej Zolnierkiewicz
@ 2009-02-11 6:36 ` Borislav Petkov
2009-02-11 16:28 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 27+ messages in thread
From: Borislav Petkov @ 2009-02-11 6:36 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel
On Tue, Feb 10, 2009 at 12:19:52AM +0100, Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [PATCH] ide: pass command to ide_map_sg()
>
> * Set IDE_TFLAG_WRITE flag and ->rq also for ATA_CMD_PACKET
> commands.
>
> * Pass command to ->dma_setup method and update all its
> implementations accordingly.
>
> * Pass command instead of request to ide_build_sglist(),
> *_build_dmatable() and ide_map_sg().
>
> While at it:
>
> * Fix scc_dma_setup() documentation + use ATA_DMA_WR define.
>
> * Rename sgiioc4_build_dma_table() to sgiioc4_build_dmatable(),
> change return value type to 'int' and drop unused 'ddir'
> argument.
>
> * Do some minor cleanups in [tx4939]ide_dma_setup().
>
> There should be no functional changes caused by this patch.
>
> Cc: Borislav Petkov <petkovbb@gmail.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
> drivers/ide/alim15x3.c | 7 ++++---
> drivers/ide/au1xxx-ide.c | 15 ++++++---------
> drivers/ide/icside.c | 7 +++----
> drivers/ide/ide-atapi.c | 16 ++++++++++++----
> drivers/ide/ide-disk.c | 10 +++++-----
> drivers/ide/ide-dma-sff.c | 18 +++++++++---------
> drivers/ide/ide-dma.c | 15 +++++++--------
> drivers/ide/ide-floppy.c | 6 +++++-
> drivers/ide/ide-io.c | 6 +++---
> drivers/ide/ide-taskfile.c | 4 ++--
> drivers/ide/ns87415.c | 4 ++--
> drivers/ide/pmac.c | 19 ++++++++-----------
> drivers/ide/scc_pata.c | 19 +++++++------------
> drivers/ide/sgiioc4.c | 21 +++++++--------------
> drivers/ide/trm290.c | 10 +++++-----
> drivers/ide/tx4939ide.c | 26 ++++++++++----------------
> include/linux/ide.h | 12 ++++++------
> 17 files changed, 101 insertions(+), 114 deletions(-)
>
> Index: b/drivers/ide/alim15x3.c
> ===================================================================
> --- a/drivers/ide/alim15x3.c
> +++ b/drivers/ide/alim15x3.c
> @@ -191,17 +191,18 @@ static void ali_set_dma_mode(ide_drive_t
> /**
> * ali15x3_dma_setup - begin a DMA phase
> * @drive: target device
> + * @cmd: command
> *
> * Returns 1 if the DMA cannot be performed, zero on success.
> */
>
> -static int ali15x3_dma_setup(ide_drive_t *drive)
> +static int ali15x3_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
> {
> if (m5229_revision < 0xC2 && drive->media != ide_disk) {
> - if (rq_data_dir(drive->hwif->rq))
> + if (cmd->tf_flags & IDE_TFLAG_WRITE)
> return 1; /* try PIO instead of DMA */
> }
> - return ide_dma_setup(drive);
> + return ide_dma_setup(drive, cmd);
> }
>
> /**
> Index: b/drivers/ide/au1xxx-ide.c
> ===================================================================
> --- a/drivers/ide/au1xxx-ide.c
> +++ b/drivers/ide/au1xxx-ide.c
> @@ -209,15 +209,14 @@ static void auide_set_dma_mode(ide_drive
> */
>
> #ifdef CONFIG_BLK_DEV_IDE_AU1XXX_MDMA2_DBDMA
> -static int auide_build_dmatable(ide_drive_t *drive)
> +static int auide_build_dmatable(ide_drive_t *drive, struct ide_cmd *cmd)
> {
> ide_hwif_t *hwif = drive->hwif;
> - struct request *rq = hwif->rq;
> _auide_hwif *ahwif = &auide_hwif;
> struct scatterlist *sg;
> - int i = hwif->cmd.sg_nents, iswrite, count = 0;
> + int i = cmd->sg_nents, count = 0;
> + int iswrite = !!(cmd->tf_flags & IDE_TFLAG_WRITE);
>
> - iswrite = (rq_data_dir(rq) == WRITE);
> /* Save for interrupt context */
> ahwif->drive = drive;
>
> @@ -298,12 +297,10 @@ static void auide_dma_exec_cmd(ide_drive
> (2*WAIT_CMD), NULL);
> }
>
> -static int auide_dma_setup(ide_drive_t *drive)
> +static int auide_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
> {
> - struct request *rq = drive->hwif->rq;
> -
> - if (!auide_build_dmatable(drive)) {
> - ide_map_sg(drive, rq);
> + if (auide_build_dmatable(drive, cmd) == 0) {
> + ide_map_sg(drive, cmd);
> return 1;
> }
>
> Index: b/drivers/ide/icside.c
> ===================================================================
> --- a/drivers/ide/icside.c
> +++ b/drivers/ide/icside.c
> @@ -307,15 +307,14 @@ static void icside_dma_start(ide_drive_t
> enable_dma(ec->dma);
> }
>
> -static int icside_dma_setup(ide_drive_t *drive)
> +static int icside_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
> {
> ide_hwif_t *hwif = drive->hwif;
> struct expansion_card *ec = ECARD_DEV(hwif->dev);
> struct icside_state *state = ecard_get_drvdata(ec);
> - struct request *rq = hwif->rq;
> unsigned int dma_mode;
>
> - if (rq_data_dir(rq))
> + if (cmd->tf_flags & IDE_TFLAG_WRITE)
> dma_mode = DMA_MODE_WRITE;
> else
> dma_mode = DMA_MODE_READ;
> @@ -344,7 +343,7 @@ static int icside_dma_setup(ide_drive_t
> * Tell the DMA engine about the SG table and
> * data direction.
> */
> - set_dma_sg(ec->dma, hwif->sg_table, hwif->cmd.sg_nents);
> + set_dma_sg(ec->dma, hwif->sg_table, cmd->sg_nents);
> set_dma_mode(ec->dma, dma_mode);
>
> drive->waiting_for_dma = 1;
> Index: b/drivers/ide/ide-atapi.c
> ===================================================================
> --- a/drivers/ide/ide-atapi.c
> +++ b/drivers/ide/ide-atapi.c
> @@ -626,12 +626,20 @@ ide_startstop_t ide_issue_pc(ide_drive_t
> {
> struct ide_atapi_pc *pc;
> ide_hwif_t *hwif = drive->hwif;
> + const struct ide_dma_ops *dma_ops = hwif->dma_ops;
> + struct ide_cmd *cmd = &hwif->cmd;
> ide_expiry_t *expiry = NULL;
> struct request *rq = hwif->rq;
> unsigned int timeout;
> u32 tf_flags;
> u16 bcount;
>
> + if (drive->media != ide_floppy) {
> + if (rq_data_dir(rq))
> + cmd->tf_flags |= IDE_TFLAG_WRITE;
> + cmd->rq = rq;
> + }
> +
> if (dev_is_idecd(drive)) {
> tf_flags = IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL;
> bcount = ide_cd_get_xferlen(rq);
> @@ -639,8 +647,8 @@ ide_startstop_t ide_issue_pc(ide_drive_t
> timeout = ATAPI_WAIT_PC;
>
> if (drive->dma) {
> - if (ide_build_sglist(drive, rq))
> - drive->dma = !hwif->dma_ops->dma_setup(drive);
> + if (ide_build_sglist(drive, cmd))
> + drive->dma = !dma_ops->dma_setup(drive, cmd);
> else
> drive->dma = 0;
> }
> @@ -663,8 +671,8 @@ ide_startstop_t ide_issue_pc(ide_drive_t
>
> if ((pc->flags & PC_FLAG_DMA_OK) &&
> (drive->dev_flags & IDE_DFLAG_USING_DMA)) {
> - if (ide_build_sglist(drive, rq))
> - drive->dma = !hwif->dma_ops->dma_setup(drive);
> + if (ide_build_sglist(drive, cmd))
> + drive->dma = !dma_ops->dma_setup(drive, cmd);
> else
> drive->dma = 0;
> }
> Index: b/drivers/ide/ide-disk.c
> ===================================================================
> --- a/drivers/ide/ide-disk.c
> +++ b/drivers/ide/ide-disk.c
> @@ -99,11 +99,6 @@ static ide_startstop_t __ide_do_rw_disk(
> memset(&cmd, 0, sizeof(cmd));
> cmd.tf_flags = IDE_TFLAG_TF | IDE_TFLAG_DEVICE;
>
> - if (dma == 0) {
> - ide_init_sg_cmd(&cmd, nsectors);
> - ide_map_sg(drive, rq);
> - }
> -
> if (drive->dev_flags & IDE_DFLAG_LBA) {
> if (lba48) {
> pr_debug("%s: LBA=0x%012llx\n", drive->name,
> @@ -156,6 +151,11 @@ static ide_startstop_t __ide_do_rw_disk(
> ide_tf_set_cmd(drive, &cmd, dma);
> cmd.rq = rq;
>
> + if (dma == 0) {
> + ide_init_sg_cmd(&cmd, nsectors);
> + ide_map_sg(drive, &cmd);
> + }
> +
> rc = do_rw_taskfile(drive, &cmd);
>
> if (rc == ide_stopped && dma) {
> Index: b/drivers/ide/ide-dma-sff.c
> ===================================================================
> --- a/drivers/ide/ide-dma-sff.c
> +++ b/drivers/ide/ide-dma-sff.c
> @@ -111,7 +111,7 @@ EXPORT_SYMBOL_GPL(ide_dma_host_set);
> * May also be invoked from trm290.c
> */
>
> -int ide_build_dmatable(ide_drive_t *drive, struct request *rq)
> +int ide_build_dmatable(ide_drive_t *drive, struct ide_cmd *cmd)
> {
> ide_hwif_t *hwif = drive->hwif;
> __le32 *table = (__le32 *)hwif->dmatable_cpu;
> @@ -120,7 +120,7 @@ int ide_build_dmatable(ide_drive_t *driv
> struct scatterlist *sg;
> u8 is_trm290 = !!(hwif->host_flags & IDE_HFLAG_TRM290);
>
> - for_each_sg(hwif->sg_table, sg, hwif->cmd.sg_nents, i) {
> + for_each_sg(hwif->sg_table, sg, cmd->sg_nents, i) {
> u32 cur_addr, cur_len, xcount, bcount;
>
> cur_addr = sg_dma_address(sg);
> @@ -175,6 +175,7 @@ EXPORT_SYMBOL_GPL(ide_build_dmatable);
> /**
> * ide_dma_setup - begin a DMA phase
> * @drive: target device
> + * @cmd: command
> *
> * Build an IDE DMA PRD (IDE speak for scatter gather table)
> * and then set up the DMA transfer registers for a device
> @@ -185,17 +186,16 @@ EXPORT_SYMBOL_GPL(ide_build_dmatable);
> * is returned.
> */
>
> -int ide_dma_setup(ide_drive_t *drive)
> +int ide_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
> {
> ide_hwif_t *hwif = drive->hwif;
> - struct request *rq = hwif->rq;
> - unsigned int reading = rq_data_dir(rq) ? 0 : ATA_DMA_WR;
> u8 mmio = (hwif->host_flags & IDE_HFLAG_MMIO) ? 1 : 0;
> + u8 rw = (cmd->tf_flags & IDE_TFLAG_WRITE) ? 0 : ATA_DMA_WR;
> u8 dma_stat;
>
> /* fall back to pio! */
> - if (!ide_build_dmatable(drive, rq)) {
> - ide_map_sg(drive, rq);
> + if (ide_build_dmatable(drive, cmd) == 0) {
> + ide_map_sg(drive, cmd);
> return 1;
> }
>
> @@ -208,9 +208,9 @@ int ide_dma_setup(ide_drive_t *drive)
>
> /* specify r/w */
> if (mmio)
> - writeb(reading, (void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
> + writeb(rw, (void __iomem *)(hwif->dma_base + ATA_DMA_CMD));
> else
> - outb(reading, hwif->dma_base + ATA_DMA_CMD);
> + outb(rw, hwif->dma_base + ATA_DMA_CMD);
>
> /* read DMA status for INTR & ERROR flags */
> dma_stat = hwif->dma_ops->dma_sff_read_status(hwif);
> Index: b/drivers/ide/ide-dma.c
> ===================================================================
> --- a/drivers/ide/ide-dma.c
> +++ b/drivers/ide/ide-dma.c
> @@ -120,7 +120,7 @@ int ide_dma_good_drive(ide_drive_t *driv
> /**
> * ide_build_sglist - map IDE scatter gather for DMA I/O
> * @drive: the drive to build the DMA table for
> - * @rq: the request holding the sg list
> + * @cmd: command
> *
> * Perform the DMA mapping magic necessary to access the source or
> * target buffers of a request via DMA. The lower layers of the
> @@ -128,23 +128,22 @@ int ide_dma_good_drive(ide_drive_t *driv
> * operate in a portable fashion.
> */
>
> -int ide_build_sglist(ide_drive_t *drive, struct request *rq)
> +int ide_build_sglist(ide_drive_t *drive, struct ide_cmd *cmd)
> {
> ide_hwif_t *hwif = drive->hwif;
> struct scatterlist *sg = hwif->sg_table;
> - struct ide_cmd *cmd = &hwif->cmd;
> int i;
>
> - ide_map_sg(drive, rq);
> + ide_map_sg(drive, cmd);
>
> - if (rq_data_dir(rq) == READ)
> - cmd->sg_dma_direction = DMA_FROM_DEVICE;
> - else
> + if (cmd->tf_flags & IDE_TFLAG_WRITE)
> cmd->sg_dma_direction = DMA_TO_DEVICE;
> + else
> + cmd->sg_dma_direction = DMA_FROM_DEVICE;
>
> i = dma_map_sg(hwif->dev, sg, cmd->sg_nents, cmd->sg_dma_direction);
> if (i == 0)
> - ide_map_sg(drive, rq);
> + ide_map_sg(drive, cmd);
>
> return i;
> }
> Index: b/drivers/ide/ide-floppy.c
> ===================================================================
> --- a/drivers/ide/ide-floppy.c
> +++ b/drivers/ide/ide-floppy.c
> @@ -285,8 +285,12 @@ static ide_startstop_t ide_floppy_do_req
> goto out_end;
> }
>
> + if (rq_data_dir(rq))
> + cmd->tf_flags |= IDE_TFLAG_WRITE;
> + cmd->rq = rq;
> +
> ide_init_sg_cmd(cmd, rq->nr_sectors);
> - ide_map_sg(drive, rq);
> + ide_map_sg(drive, cmd);
How about we push those mappings in ide_issue_pc() in the else-branch
after we've tried setting up dma and we've failed? This way we don't
have to do that in the ->do_request of every device and do it for all at
one place instead. Only ide-cd will have to have ->ide_io_buffers for
PIO transfers (which I was working on before bugs :))
[..]
--
Regards/Gruss,
Boris.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/6] ide: pass command to ide_map_sg()
2009-02-11 6:36 ` Borislav Petkov
@ 2009-02-11 16:28 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-11 16:28 UTC (permalink / raw)
To: petkovbb; +Cc: linux-ide, linux-kernel
On Wednesday 11 February 2009, Borislav Petkov wrote:
> On Tue, Feb 10, 2009 at 12:19:52AM +0100, Bartlomiej Zolnierkiewicz wrote:
> > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Subject: [PATCH] ide: pass command to ide_map_sg()
> >
> > * Set IDE_TFLAG_WRITE flag and ->rq also for ATA_CMD_PACKET
> > commands.
> >
> > * Pass command to ->dma_setup method and update all its
> > implementations accordingly.
> >
> > * Pass command instead of request to ide_build_sglist(),
> > *_build_dmatable() and ide_map_sg().
> >
> > While at it:
> >
> > * Fix scc_dma_setup() documentation + use ATA_DMA_WR define.
> >
> > * Rename sgiioc4_build_dma_table() to sgiioc4_build_dmatable(),
> > change return value type to 'int' and drop unused 'ddir'
> > argument.
> >
> > * Do some minor cleanups in [tx4939]ide_dma_setup().
> >
> > There should be no functional changes caused by this patch.
> >
> > Cc: Borislav Petkov <petkovbb@gmail.com>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
[...]
[ please cut needless parts when replying, thanks ]
> > Index: b/drivers/ide/ide-floppy.c
> > ===================================================================
> > --- a/drivers/ide/ide-floppy.c
> > +++ b/drivers/ide/ide-floppy.c
> > @@ -285,8 +285,12 @@ static ide_startstop_t ide_floppy_do_req
> > goto out_end;
> > }
> >
> > + if (rq_data_dir(rq))
> > + cmd->tf_flags |= IDE_TFLAG_WRITE;
> > + cmd->rq = rq;
> > +
> > ide_init_sg_cmd(cmd, rq->nr_sectors);
> > - ide_map_sg(drive, rq);
> > + ide_map_sg(drive, cmd);
>
> How about we push those mappings in ide_issue_pc() in the else-branch
> after we've tried setting up dma and we've failed? This way we don't
> have to do that in the ->do_request of every device and do it for all at
Sounds OK to me.
> one place instead. Only ide-cd will have to have ->ide_io_buffers for
> PIO transfers (which I was working on before bugs :))
BTW I have now a draft patch making ide-cd use ide_pio_bytes() for fs
requests (I still need to do it for non-fs requests, polish, document
and do more testing). The hard parts (== more like mind melting ones)
were in untagling cdrom_end_request() uses and getting rid of partial
completions without making the whole transfer fail on errors...
Thanks,
Bart
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/6] ide: use do_rw_taskfile() for ATA_CMD_PACKET commands
2009-02-09 23:19 [PATCH 0/6] ide: more unifications of ATA and ATAPI support Bartlomiej Zolnierkiewicz
2009-02-09 23:19 ` [PATCH 1/6] ide: pass command to ide_map_sg() Bartlomiej Zolnierkiewicz
@ 2009-02-09 23:19 ` Bartlomiej Zolnierkiewicz
2009-02-09 23:20 ` [PATCH 3/6] ide: set hwif->expiry prior to calling [__]ide_set_handler() Bartlomiej Zolnierkiewicz
` (4 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-09 23:19 UTC (permalink / raw)
To: linux-ide; +Cc: Borislav Petkov, Bartlomiej Zolnierkiewicz, linux-kernel
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: use do_rw_taskfile() for ATA_CMD_PACKET commands
* Pass command to ide_issue_pc() and update ->do_request methods
in ide-{cd,floppy,tape}.c accordingly.
* Convert ide_pktcmd_tf_load() to ide_init_packet_cmd() which
just initializes command structure and use do_rw_taskfile()
to load ATA_CMD_PACKET commands.
While at it:
* Rename ide{floppy,tape}_issue_pc() to ide_{floppy,tape}_issue_pc().
There should be no functional changes caused by this patch.
Cc: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-atapi.c | 37 ++++++++++++-------------------------
drivers/ide/ide-cd.c | 11 ++++++++++-
drivers/ide/ide-floppy.c | 24 ++++++++++++++----------
drivers/ide/ide-tape.c | 19 ++++++++++++++-----
drivers/ide/ide-taskfile.c | 3 ++-
include/linux/ide.h | 2 +-
6 files changed, 53 insertions(+), 43 deletions(-)
Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -475,23 +475,15 @@ next_irq:
return ide_started;
}
-static void ide_pktcmd_tf_load(ide_drive_t *drive, u32 tf_flags, u16 bcount)
+static void ide_init_packet_cmd(struct ide_cmd *cmd, u32 tf_flags,
+ u16 bcount, u8 dma)
{
- ide_hwif_t *hwif = drive->hwif;
- struct ide_cmd cmd;
- u8 dma = drive->dma;
-
- memset(&cmd, 0, sizeof(cmd));
- cmd.tf_flags = IDE_TFLAG_OUT_LBAH | IDE_TFLAG_OUT_LBAM |
- IDE_TFLAG_OUT_FEATURE | tf_flags;
- cmd.tf.feature = dma; /* Use PIO/DMA */
- cmd.tf.lbam = bcount & 0xff;
- cmd.tf.lbah = (bcount >> 8) & 0xff;
-
- ide_tf_dump(drive->name, &cmd.tf);
- hwif->tp_ops->set_irq(hwif, 1);
- SELECT_MASK(drive, 0);
- hwif->tp_ops->tf_load(drive, &cmd);
+ cmd->protocol = dma ? ATAPI_PROT_DMA : ATAPI_PROT_PIO;
+ cmd->tf_flags |= IDE_TFLAG_OUT_LBAH | IDE_TFLAG_OUT_LBAM |
+ IDE_TFLAG_OUT_FEATURE | tf_flags;
+ cmd->tf.feature = dma; /* Use PIO/DMA */
+ cmd->tf.lbam = bcount & 0xff;
+ cmd->tf.lbah = (bcount >> 8) & 0xff;
}
static u8 ide_read_ireason(ide_drive_t *drive)
@@ -622,24 +614,17 @@ static ide_startstop_t ide_transfer_pc(i
return ide_started;
}
-ide_startstop_t ide_issue_pc(ide_drive_t *drive)
+ide_startstop_t ide_issue_pc(ide_drive_t *drive, struct ide_cmd *cmd)
{
struct ide_atapi_pc *pc;
ide_hwif_t *hwif = drive->hwif;
const struct ide_dma_ops *dma_ops = hwif->dma_ops;
- struct ide_cmd *cmd = &hwif->cmd;
ide_expiry_t *expiry = NULL;
struct request *rq = hwif->rq;
unsigned int timeout;
u32 tf_flags;
u16 bcount;
- if (drive->media != ide_floppy) {
- if (rq_data_dir(rq))
- cmd->tf_flags |= IDE_TFLAG_WRITE;
- cmd->rq = rq;
- }
-
if (dev_is_idecd(drive)) {
tf_flags = IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL;
bcount = ide_cd_get_xferlen(rq);
@@ -684,7 +669,9 @@ ide_startstop_t ide_issue_pc(ide_drive_t
: WAIT_TAPE_CMD;
}
- ide_pktcmd_tf_load(drive, tf_flags, bcount);
+ ide_init_packet_cmd(cmd, tf_flags, bcount, drive->dma);
+
+ (void)do_rw_taskfile(drive, cmd);
/* Issue the packet command */
if (drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT) {
Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -1060,6 +1060,8 @@ static void cdrom_do_block_pc(ide_drive_
static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
sector_t block)
{
+ struct ide_cmd cmd;
+
ide_debug_log(IDE_DBG_RQ, "cmd: 0x%x, block: %llu",
rq->cmd[0], (unsigned long long)block);
@@ -1088,7 +1090,14 @@ static ide_startstop_t ide_cd_do_request
return ide_stopped;
}
- return ide_issue_pc(drive);
+ memset(&cmd, 0, sizeof(cmd));
+
+ if (rq_data_dir(rq))
+ cmd.tf_flags |= IDE_TFLAG_WRITE;
+
+ cmd.rq = rq;
+
+ return ide_issue_pc(drive, &cmd);
}
/*
Index: b/drivers/ide/ide-floppy.c
===================================================================
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -130,8 +130,9 @@ static void ide_floppy_report_error(stru
}
-static ide_startstop_t idefloppy_issue_pc(ide_drive_t *drive,
- struct ide_atapi_pc *pc)
+static ide_startstop_t ide_floppy_issue_pc(ide_drive_t *drive,
+ struct ide_cmd *cmd,
+ struct ide_atapi_pc *pc)
{
struct ide_disk_obj *floppy = drive->driver_data;
@@ -157,7 +158,7 @@ static ide_startstop_t idefloppy_issue_p
pc->retries++;
- return ide_issue_pc(drive);
+ return ide_issue_pc(drive, cmd);
}
void ide_floppy_create_read_capacity_cmd(struct ide_atapi_pc *pc)
@@ -244,7 +245,7 @@ static ide_startstop_t ide_floppy_do_req
{
struct ide_disk_obj *floppy = drive->driver_data;
ide_hwif_t *hwif = drive->hwif;
- struct ide_cmd *cmd = &hwif->cmd;
+ struct ide_cmd cmd;
struct ide_atapi_pc *pc;
if (drive->debug_mask & IDE_DBG_RQ)
@@ -285,19 +286,22 @@ static ide_startstop_t ide_floppy_do_req
goto out_end;
}
+ memset(&cmd, 0, sizeof(cmd));
+
if (rq_data_dir(rq))
- cmd->tf_flags |= IDE_TFLAG_WRITE;
- cmd->rq = rq;
+ cmd.tf_flags |= IDE_TFLAG_WRITE;
+
+ cmd.rq = rq;
- ide_init_sg_cmd(cmd, rq->nr_sectors);
- ide_map_sg(drive, cmd);
+ ide_init_sg_cmd(&cmd, rq->nr_sectors);
+ ide_map_sg(drive, &cmd);
pc->sg = hwif->sg_table;
- pc->sg_cnt = cmd->sg_nents;
+ pc->sg_cnt = cmd.sg_nents;
pc->rq = rq;
- return idefloppy_issue_pc(drive, pc);
+ return ide_floppy_issue_pc(drive, &cmd, pc);
out_end:
drive->failed_pc = NULL;
if (blk_fs_request(rq) == 0 && rq->errors == 0)
Index: b/drivers/ide/ide-tape.c
===================================================================
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -580,7 +580,7 @@ static int ide_tape_io_buffers(ide_drive
*
* The handling will be done in three stages:
*
- * 1. idetape_issue_pc will send the packet command to the drive, and will set
+ * 1. ide_tape_issue_pc will send the packet command to the drive, and will set
* the interrupt handler to ide_pc_intr.
*
* 2. On each interrupt, ide_pc_intr will be called. This step will be
@@ -608,8 +608,9 @@ static int ide_tape_io_buffers(ide_drive
* request.
*/
-static ide_startstop_t idetape_issue_pc(ide_drive_t *drive,
- struct ide_atapi_pc *pc)
+static ide_startstop_t ide_tape_issue_pc(ide_drive_t *drive,
+ struct ide_cmd *cmd,
+ struct ide_atapi_pc *pc)
{
idetape_tape_t *tape = drive->driver_data;
@@ -654,7 +655,7 @@ static ide_startstop_t idetape_issue_pc(
pc->retries++;
- return ide_issue_pc(drive);
+ return ide_issue_pc(drive, cmd);
}
/* A mode sense command is used to "sense" tape parameters. */
@@ -749,6 +750,7 @@ static ide_startstop_t idetape_do_reques
idetape_tape_t *tape = drive->driver_data;
struct ide_atapi_pc *pc = NULL;
struct request *postponed_rq = tape->postponed_rq;
+ struct ide_cmd cmd;
u8 stat;
debug_log(DBG_SENSE, "sector: %llu, nr_sectors: %lu,"
@@ -844,7 +846,14 @@ static ide_startstop_t idetape_do_reques
BUG();
out:
- return idetape_issue_pc(drive, pc);
+ memset(&cmd, 0, sizeof(cmd));
+
+ if (rq_data_dir(rq))
+ cmd.tf_flags |= IDE_TFLAG_WRITE;
+
+ cmd.rq = rq;
+
+ return ide_tape_issue_pc(drive, &cmd, pc);
}
/*
Index: b/drivers/ide/ide-taskfile.c
===================================================================
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -100,13 +100,14 @@ ide_startstop_t do_rw_taskfile(ide_drive
ide_execute_command(drive, tf->command, handler,
WAIT_WORSTCASE, NULL);
return ide_started;
- default:
+ case ATA_PROT_DMA:
if ((drive->dev_flags & IDE_DFLAG_USING_DMA) == 0 ||
ide_build_sglist(drive, cmd) == 0 ||
dma_ops->dma_setup(drive, cmd))
return ide_stopped;
dma_ops->dma_exec_cmd(drive, tf->command);
dma_ops->dma_start(drive);
+ default:
return ide_started;
}
}
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1223,7 +1223,7 @@ int ide_cd_expiry(ide_drive_t *);
int ide_cd_get_xferlen(struct request *);
-ide_startstop_t ide_issue_pc(ide_drive_t *);
+ide_startstop_t ide_issue_pc(ide_drive_t *, struct ide_cmd *);
ide_startstop_t do_rw_taskfile(ide_drive_t *, struct ide_cmd *);
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 3/6] ide: set hwif->expiry prior to calling [__]ide_set_handler()
2009-02-09 23:19 [PATCH 0/6] ide: more unifications of ATA and ATAPI support Bartlomiej Zolnierkiewicz
2009-02-09 23:19 ` [PATCH 1/6] ide: pass command to ide_map_sg() Bartlomiej Zolnierkiewicz
2009-02-09 23:19 ` [PATCH 2/6] ide: use do_rw_taskfile() for ATA_CMD_PACKET commands Bartlomiej Zolnierkiewicz
@ 2009-02-09 23:20 ` Bartlomiej Zolnierkiewicz
2009-03-16 14:23 ` Sergei Shtylyov
2009-02-09 23:20 ` [PATCH 4/6] ide: add ->dma_expiry method and remove ->dma_exec_cmd one Bartlomiej Zolnierkiewicz
` (3 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-09 23:20 UTC (permalink / raw)
To: linux-ide; +Cc: Borislav Petkov, Bartlomiej Zolnierkiewicz, linux-kernel
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: set hwif->expiry prior to calling [__]ide_set_handler()
* Set hwif->expiry prior to calling [__]ide_set_handler()
and drop 'expiry' argument.
* Set hwif->expiry to NULL in ide_{timer_expiry,intr}()
and remove 'hwif->expiry = NULL' assignments.
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-atapi.c | 6 ++++--
drivers/ide/ide-cd.c | 3 ++-
drivers/ide/ide-eh.c | 9 ++++-----
drivers/ide/ide-io.c | 2 ++
drivers/ide/ide-iops.c | 13 +++++++------
drivers/ide/ide-taskfile.c | 6 +++---
include/linux/ide.h | 6 ++----
7 files changed, 24 insertions(+), 21 deletions(-)
Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -471,7 +471,7 @@ static ide_startstop_t ide_pc_intr(ide_d
rq->cmd[0], bcount);
next_irq:
/* And set the interrupt handler again */
- ide_set_handler(drive, ide_pc_intr, timeout, NULL);
+ ide_set_handler(drive, ide_pc_intr, timeout);
return ide_started;
}
@@ -590,11 +590,13 @@ static ide_startstop_t ide_transfer_pc(i
}
}
+ hwif->expiry = expiry;
+
/* Set the interrupt routine */
ide_set_handler(drive,
(dev_is_idecd(drive) ? drive->irq_handler
: ide_pc_intr),
- timeout, expiry);
+ timeout);
/* Begin DMA, if necessary */
if (dev_is_idecd(drive)) {
Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -953,7 +953,8 @@ static ide_startstop_t cdrom_newpc_intr(
expiry = ide_cd_expiry;
}
- ide_set_handler(drive, cdrom_newpc_intr, timeout, expiry);
+ hwif->expiry = expiry;
+ ide_set_handler(drive, cdrom_newpc_intr, timeout);
return ide_started;
end_request:
Index: b/drivers/ide/ide-eh.c
===================================================================
--- a/drivers/ide/ide-eh.c
+++ b/drivers/ide/ide-eh.c
@@ -175,8 +175,7 @@ static ide_startstop_t atapi_reset_pollf
printk(KERN_INFO "%s: ATAPI reset complete\n", drive->name);
else {
if (time_before(jiffies, hwif->poll_timeout)) {
- ide_set_handler(drive, &atapi_reset_pollfunc, HZ/20,
- NULL);
+ ide_set_handler(drive, &atapi_reset_pollfunc, HZ/20);
/* continue polling */
return ide_started;
}
@@ -238,7 +237,7 @@ static ide_startstop_t reset_pollfunc(id
if (!OK_STAT(tmp, 0, ATA_BUSY)) {
if (time_before(jiffies, hwif->poll_timeout)) {
- ide_set_handler(drive, &reset_pollfunc, HZ/20, NULL);
+ ide_set_handler(drive, &reset_pollfunc, HZ/20);
/* continue polling */
return ide_started;
}
@@ -355,7 +354,7 @@ static ide_startstop_t do_reset1(ide_dri
ndelay(400);
hwif->poll_timeout = jiffies + WAIT_WORSTCASE;
hwif->polling = 1;
- __ide_set_handler(drive, &atapi_reset_pollfunc, HZ/20, NULL);
+ __ide_set_handler(drive, &atapi_reset_pollfunc, HZ/20);
spin_unlock_irqrestore(&hwif->lock, flags);
return ide_started;
}
@@ -415,7 +414,7 @@ static ide_startstop_t do_reset1(ide_dri
udelay(10);
hwif->poll_timeout = jiffies + WAIT_WORSTCASE;
hwif->polling = 1;
- __ide_set_handler(drive, &reset_pollfunc, HZ/20, NULL);
+ __ide_set_handler(drive, &reset_pollfunc, HZ/20);
/*
* Some weird controller like resetting themselves to a strange
Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -647,6 +647,7 @@ void ide_timer_expiry (unsigned long dat
}
}
hwif->handler = NULL;
+ hwif->expiry = NULL;
/*
* We need to simulate a real interrupt when invoking
* the handler() function, which means we need to
@@ -826,6 +827,7 @@ irqreturn_t ide_intr (int irq, void *dev
goto out;
hwif->handler = NULL;
+ hwif->expiry = NULL;
hwif->req_gen++;
del_timer(&hwif->timer);
spin_unlock(&hwif->lock);
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -425,26 +425,25 @@ int ide_config_drive_speed(ide_drive_t *
* See also ide_execute_command
*/
void __ide_set_handler(ide_drive_t *drive, ide_handler_t *handler,
- unsigned int timeout, ide_expiry_t *expiry)
+ unsigned int timeout)
{
ide_hwif_t *hwif = drive->hwif;
BUG_ON(hwif->handler);
hwif->handler = handler;
- hwif->expiry = expiry;
hwif->timer.expires = jiffies + timeout;
hwif->req_gen_timer = hwif->req_gen;
add_timer(&hwif->timer);
}
-void ide_set_handler (ide_drive_t *drive, ide_handler_t *handler,
- unsigned int timeout, ide_expiry_t *expiry)
+void ide_set_handler(ide_drive_t *drive, ide_handler_t *handler,
+ unsigned int timeout)
{
ide_hwif_t *hwif = drive->hwif;
unsigned long flags;
spin_lock_irqsave(&hwif->lock, flags);
- __ide_set_handler(drive, handler, timeout, expiry);
+ __ide_set_handler(drive, handler, timeout);
spin_unlock_irqrestore(&hwif->lock, flags);
}
EXPORT_SYMBOL(ide_set_handler);
@@ -469,8 +468,10 @@ void ide_execute_command(ide_drive_t *dr
ide_hwif_t *hwif = drive->hwif;
unsigned long flags;
+ hwif->expiry = expiry;
+
spin_lock_irqsave(&hwif->lock, flags);
- __ide_set_handler(drive, handler, timeout, expiry);
+ __ide_set_handler(drive, handler, timeout);
hwif->tp_ops->exec_command(hwif, cmd);
/*
* Drive takes 400nS to respond, we must avoid the IRQ being
Index: b/drivers/ide/ide-taskfile.c
===================================================================
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -140,7 +140,7 @@ static ide_startstop_t task_no_data_intr
} else if (custom && tf->command == ATA_CMD_INIT_DEV_PARAMS) {
if ((stat & (ATA_ERR | ATA_DRQ)) == 0) {
ide_set_handler(drive, &task_no_data_intr,
- WAIT_WORSTCASE, NULL);
+ WAIT_WORSTCASE);
return ide_started;
}
}
@@ -347,7 +347,7 @@ static ide_startstop_t task_pio_intr(ide
}
out_wait:
/* Still data left to transfer. */
- ide_set_handler(drive, &task_pio_intr, WAIT_WORSTCASE, NULL);
+ ide_set_handler(drive, &task_pio_intr, WAIT_WORSTCASE);
return ide_started;
out_end:
if ((cmd->tf_flags & IDE_TFLAG_FS) == 0)
@@ -377,7 +377,7 @@ static ide_startstop_t pre_task_out_intr
if ((drive->dev_flags & IDE_DFLAG_UNMASK) == 0)
local_irq_disable();
- ide_set_handler(drive, &task_pio_intr, WAIT_WORSTCASE, NULL);
+ ide_set_handler(drive, &task_pio_intr, WAIT_WORSTCASE);
ide_pio_datablock(drive, cmd, 1);
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1132,10 +1132,8 @@ unsigned int ide_rq_bytes(struct request
int ide_end_rq(ide_drive_t *, struct request *, int, unsigned int);
void ide_kill_rq(ide_drive_t *, struct request *);
-void __ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int,
- ide_expiry_t *);
-void ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int,
- ide_expiry_t *);
+void __ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
+void ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
void ide_execute_command(ide_drive_t *, u8, ide_handler_t *, unsigned int,
ide_expiry_t *);
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 4/6] ide: add ->dma_expiry method and remove ->dma_exec_cmd one
2009-02-09 23:19 [PATCH 0/6] ide: more unifications of ATA and ATAPI support Bartlomiej Zolnierkiewicz
` (2 preceding siblings ...)
2009-02-09 23:20 ` [PATCH 3/6] ide: set hwif->expiry prior to calling [__]ide_set_handler() Bartlomiej Zolnierkiewicz
@ 2009-02-09 23:20 ` Bartlomiej Zolnierkiewicz
2009-02-10 18:18 ` Sergei Shtylyov
2009-02-09 23:20 ` [PATCH 5/6] ide: remove ide_execute_pkt_cmd() Bartlomiej Zolnierkiewicz
` (2 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-09 23:20 UTC (permalink / raw)
To: linux-ide; +Cc: Borislav Petkov, Bartlomiej Zolnierkiewicz, linux-kernel
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: add ->dma_expiry method and remove ->dma_exec_cmd one
* Rename dma_timer_expiry() to ide_dma_sff_expiry() and export it.
* Add ->dma_expiry method and use it to set hwif->expiry for
ATA_PROT_DMA protocol in do_rw_taskfile().
* Initialize ->dma_expiry to ide_dma_sff_expiry() for SFF hosts.
* Move setting hwif->expiry from ide_execute_command() to its
users and drop 'expiry' argument.
* Use ide_execute_command() instead of ->dma_exec_cmd in
do_rw_taskfile().
* Remove ->dma_exec_cmd method and its implementations.
* Unexport ide_execute_command() and ide_dma_intr().
There should be no functional changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/alim15x3.c | 2 +-
drivers/ide/au1xxx-ide.c | 8 --------
drivers/ide/cmd64x.c | 6 +++---
drivers/ide/cs5536.c | 2 +-
drivers/ide/hpt366.c | 6 +++---
drivers/ide/icside.c | 7 -------
drivers/ide/ide-atapi.c | 3 ++-
drivers/ide/ide-dma-sff.c | 15 ++++-----------
drivers/ide/ide-dma.c | 1 -
drivers/ide/ide-iops.c | 6 +-----
drivers/ide/ide-taskfile.c | 6 ++++--
drivers/ide/it821x.c | 2 +-
drivers/ide/ns87415.c | 2 +-
drivers/ide/pdc202xx_old.c | 4 ++--
drivers/ide/pmac.c | 8 --------
drivers/ide/sc1200.c | 2 +-
drivers/ide/scc_pata.c | 2 +-
drivers/ide/siimage.c | 2 +-
drivers/ide/sl82c105.c | 2 +-
drivers/ide/tc86c001.c | 2 +-
drivers/ide/trm290.c | 6 ------
drivers/ide/tx4939ide.c | 2 +-
include/linux/ide.h | 7 +++----
23 files changed, 32 insertions(+), 71 deletions(-)
Index: b/drivers/ide/alim15x3.c
===================================================================
--- a/drivers/ide/alim15x3.c
+++ b/drivers/ide/alim15x3.c
@@ -504,11 +504,11 @@ static const struct ide_port_ops ali_por
static const struct ide_dma_ops ali_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ali15x3_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = ide_dma_end,
.dma_test_irq = ide_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
Index: b/drivers/ide/au1xxx-ide.c
===================================================================
--- a/drivers/ide/au1xxx-ide.c
+++ b/drivers/ide/au1xxx-ide.c
@@ -290,13 +290,6 @@ static void auide_dma_start(ide_drive_t
}
-static void auide_dma_exec_cmd(ide_drive_t *drive, u8 command)
-{
- /* issue cmd to drive */
- ide_execute_command(drive, command, &ide_dma_intr,
- (2*WAIT_CMD), NULL);
-}
-
static int auide_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
if (auide_build_dmatable(drive, cmd) == 0) {
@@ -356,7 +349,6 @@ static void auide_init_dbdma_dev(dbdev_t
static const struct ide_dma_ops au1xxx_dma_ops = {
.dma_host_set = auide_dma_host_set,
.dma_setup = auide_dma_setup,
- .dma_exec_cmd = auide_dma_exec_cmd,
.dma_start = auide_dma_start,
.dma_end = auide_dma_end,
.dma_test_irq = auide_dma_test_irq,
Index: b/drivers/ide/cmd64x.c
===================================================================
--- a/drivers/ide/cmd64x.c
+++ b/drivers/ide/cmd64x.c
@@ -379,11 +379,11 @@ static const struct ide_port_ops cmd64x_
static const struct ide_dma_ops cmd64x_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = cmd64x_dma_end,
.dma_test_irq = cmd64x_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
@@ -391,11 +391,11 @@ static const struct ide_dma_ops cmd64x_d
static const struct ide_dma_ops cmd646_rev1_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = cmd646_1_dma_end,
.dma_test_irq = ide_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
@@ -403,11 +403,11 @@ static const struct ide_dma_ops cmd646_r
static const struct ide_dma_ops cmd648_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = cmd648_dma_end,
.dma_test_irq = cmd648_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
Index: b/drivers/ide/cs5536.c
===================================================================
--- a/drivers/ide/cs5536.c
+++ b/drivers/ide/cs5536.c
@@ -231,11 +231,11 @@ static const struct ide_port_ops cs5536_
static const struct ide_dma_ops cs5536_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = cs5536_dma_start,
.dma_end = cs5536_dma_end,
.dma_test_irq = ide_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
};
Index: b/drivers/ide/hpt366.c
===================================================================
--- a/drivers/ide/hpt366.c
+++ b/drivers/ide/hpt366.c
@@ -1418,11 +1418,11 @@ static const struct ide_port_ops hpt3xx_
static const struct ide_dma_ops hpt37x_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = hpt374_dma_end,
.dma_test_irq = hpt374_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
@@ -1430,11 +1430,11 @@ static const struct ide_dma_ops hpt37x_d
static const struct ide_dma_ops hpt370_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = hpt370_dma_start,
.dma_end = hpt370_dma_end,
.dma_test_irq = ide_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = hpt370_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
@@ -1442,11 +1442,11 @@ static const struct ide_dma_ops hpt370_d
static const struct ide_dma_ops hpt36x_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = ide_dma_end,
.dma_test_irq = ide_dma_test_irq,
.dma_lost_irq = hpt366_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
Index: b/drivers/ide/icside.c
===================================================================
--- a/drivers/ide/icside.c
+++ b/drivers/ide/icside.c
@@ -351,12 +351,6 @@ static int icside_dma_setup(ide_drive_t
return 0;
}
-static void icside_dma_exec_cmd(ide_drive_t *drive, u8 cmd)
-{
- /* issue cmd to drive */
- ide_execute_command(drive, cmd, ide_dma_intr, 2 * WAIT_CMD, NULL);
-}
-
static int icside_dma_test_irq(ide_drive_t *drive)
{
ide_hwif_t *hwif = drive->hwif;
@@ -380,7 +374,6 @@ static int icside_dma_init(ide_hwif_t *h
static const struct ide_dma_ops icside_v6_dma_ops = {
.dma_host_set = icside_dma_host_set,
.dma_setup = icside_dma_setup,
- .dma_exec_cmd = icside_dma_exec_cmd,
.dma_start = icside_dma_start,
.dma_end = icside_dma_end,
.dma_test_irq = icside_dma_test_irq,
Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -679,8 +679,9 @@ ide_startstop_t ide_issue_pc(ide_drive_t
if (drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT) {
if (drive->dma)
drive->waiting_for_dma = 0;
+ hwif->expiry = expiry;
ide_execute_command(drive, ATA_CMD_PACKET, ide_transfer_pc,
- timeout, expiry);
+ timeout);
return ide_started;
} else {
ide_execute_pkt_cmd(drive);
Index: b/drivers/ide/ide-dma-sff.c
===================================================================
--- a/drivers/ide/ide-dma-sff.c
+++ b/drivers/ide/ide-dma-sff.c
@@ -224,7 +224,7 @@ int ide_dma_setup(ide_drive_t *drive, st
EXPORT_SYMBOL_GPL(ide_dma_setup);
/**
- * dma_timer_expiry - handle a DMA timeout
+ * ide_dma_sff_expiry - handle a DMA timeout
* @drive: Drive that timed out
*
* An IDE DMA transfer timed out. In the event of an error we ask
@@ -237,7 +237,7 @@ EXPORT_SYMBOL_GPL(ide_dma_setup);
* This can occur if an interrupt is lost or due to hang or bugs.
*/
-static int dma_timer_expiry(ide_drive_t *drive)
+int ide_dma_sff_expiry(ide_drive_t *drive)
{
ide_hwif_t *hwif = drive->hwif;
u8 dma_stat = hwif->dma_ops->dma_sff_read_status(hwif);
@@ -261,14 +261,7 @@ static int dma_timer_expiry(ide_drive_t
return 0; /* Status is unknown -- reset the bus */
}
-
-void ide_dma_exec_cmd(ide_drive_t *drive, u8 command)
-{
- /* issue cmd to drive */
- ide_execute_command(drive, command, &ide_dma_intr, 2 * WAIT_CMD,
- dma_timer_expiry);
-}
-EXPORT_SYMBOL_GPL(ide_dma_exec_cmd);
+EXPORT_SYMBOL_GPL(ide_dma_sff_expiry);
void ide_dma_start(ide_drive_t *drive)
{
@@ -342,10 +335,10 @@ EXPORT_SYMBOL_GPL(ide_dma_test_irq);
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,
.dma_start = ide_dma_start,
.dma_end = ide_dma_end,
.dma_test_irq = ide_dma_test_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_lost_irq = ide_dma_lost_irq,
.dma_sff_read_status = ide_dma_sff_read_status,
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -110,7 +110,6 @@ ide_startstop_t ide_dma_intr(ide_drive_t
}
return ide_error(drive, "dma_intr", stat);
}
-EXPORT_SYMBOL_GPL(ide_dma_intr);
int ide_dma_good_drive(ide_drive_t *drive)
{
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -454,7 +454,6 @@ EXPORT_SYMBOL(ide_set_handler);
* @command: command byte to write
* @handler: handler for next phase
* @timeout: timeout for command
- * @expiry: handler to run on timeout
*
* Helper function to issue an IDE command. This handles the
* atomicity requirements, command timing and ensures that the
@@ -463,13 +462,11 @@ EXPORT_SYMBOL(ide_set_handler);
*/
void ide_execute_command(ide_drive_t *drive, u8 cmd, ide_handler_t *handler,
- unsigned timeout, ide_expiry_t *expiry)
+ unsigned timeout)
{
ide_hwif_t *hwif = drive->hwif;
unsigned long flags;
- hwif->expiry = expiry;
-
spin_lock_irqsave(&hwif->lock, flags);
__ide_set_handler(drive, handler, timeout);
hwif->tp_ops->exec_command(hwif, cmd);
@@ -482,7 +479,6 @@ void ide_execute_command(ide_drive_t *dr
ndelay(400);
spin_unlock_irqrestore(&hwif->lock, flags);
}
-EXPORT_SYMBOL(ide_execute_command);
void ide_execute_pkt_cmd(ide_drive_t *drive)
{
Index: b/drivers/ide/ide-taskfile.c
===================================================================
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -98,14 +98,16 @@ ide_startstop_t do_rw_taskfile(ide_drive
if (handler == NULL)
handler = task_no_data_intr;
ide_execute_command(drive, tf->command, handler,
- WAIT_WORSTCASE, NULL);
+ WAIT_WORSTCASE);
return ide_started;
case ATA_PROT_DMA:
if ((drive->dev_flags & IDE_DFLAG_USING_DMA) == 0 ||
ide_build_sglist(drive, cmd) == 0 ||
dma_ops->dma_setup(drive, cmd))
return ide_stopped;
- dma_ops->dma_exec_cmd(drive, tf->command);
+ hwif->expiry = dma_ops->dma_expiry;
+ ide_execute_command(drive, tf->command, ide_dma_intr,
+ 2 * WAIT_CMD);
dma_ops->dma_start(drive);
default:
return ide_started;
Index: b/drivers/ide/it821x.c
===================================================================
--- a/drivers/ide/it821x.c
+++ b/drivers/ide/it821x.c
@@ -509,10 +509,10 @@ static void it821x_quirkproc(ide_drive_t
static struct ide_dma_ops it821x_pass_through_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = it821x_dma_start,
.dma_end = it821x_dma_end,
.dma_test_irq = ide_dma_test_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_lost_irq = ide_dma_lost_irq,
.dma_sff_read_status = ide_dma_sff_read_status,
Index: b/drivers/ide/ns87415.c
===================================================================
--- a/drivers/ide/ns87415.c
+++ b/drivers/ide/ns87415.c
@@ -301,11 +301,11 @@ static const struct ide_port_ops ns87415
static const struct ide_dma_ops ns87415_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ns87415_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = ns87415_dma_end,
.dma_test_irq = ide_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = superio_dma_sff_read_status,
};
Index: b/drivers/ide/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pdc202xx_old.c
+++ b/drivers/ide/pdc202xx_old.c
@@ -331,11 +331,11 @@ static const struct ide_port_ops pdc2026
static const struct ide_dma_ops pdc20246_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = ide_dma_end,
.dma_test_irq = pdc202xx_dma_test_irq,
.dma_lost_irq = pdc202xx_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = pdc202xx_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
@@ -343,11 +343,11 @@ static const struct ide_dma_ops pdc20246
static const struct ide_dma_ops pdc2026x_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = pdc202xx_dma_start,
.dma_end = pdc202xx_dma_end,
.dma_test_irq = pdc202xx_dma_test_irq,
.dma_lost_irq = pdc202xx_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = pdc202xx_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
Index: b/drivers/ide/pmac.c
===================================================================
--- a/drivers/ide/pmac.c
+++ b/drivers/ide/pmac.c
@@ -1527,13 +1527,6 @@ static int pmac_ide_dma_setup(ide_drive_
return 0;
}
-static void
-pmac_ide_dma_exec_cmd(ide_drive_t *drive, u8 command)
-{
- /* issue cmd to drive */
- ide_execute_command(drive, command, &ide_dma_intr, 2*WAIT_CMD, NULL);
-}
-
/*
* Kick the DMA controller into life after the DMA command has been issued
* to the drive.
@@ -1654,7 +1647,6 @@ pmac_ide_dma_lost_irq (ide_drive_t *driv
static const struct ide_dma_ops pmac_dma_ops = {
.dma_host_set = pmac_ide_dma_host_set,
.dma_setup = pmac_ide_dma_setup,
- .dma_exec_cmd = pmac_ide_dma_exec_cmd,
.dma_start = pmac_ide_dma_start,
.dma_end = pmac_ide_dma_end,
.dma_test_irq = pmac_ide_dma_test_irq,
Index: b/drivers/ide/sc1200.c
===================================================================
--- a/drivers/ide/sc1200.c
+++ b/drivers/ide/sc1200.c
@@ -286,11 +286,11 @@ static const struct ide_port_ops sc1200_
static const struct ide_dma_ops sc1200_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = sc1200_dma_end,
.dma_test_irq = ide_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
Index: b/drivers/ide/scc_pata.c
===================================================================
--- a/drivers/ide/scc_pata.c
+++ b/drivers/ide/scc_pata.c
@@ -868,12 +868,12 @@ static const struct ide_port_ops scc_por
static const struct ide_dma_ops scc_dma_ops = {
.dma_host_set = scc_dma_host_set,
.dma_setup = scc_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = scc_dma_start,
.dma_end = scc_dma_end,
.dma_test_irq = scc_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
.dma_timeout = ide_dma_timeout,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_sff_read_status = scc_dma_sff_read_status,
};
Index: b/drivers/ide/siimage.c
===================================================================
--- a/drivers/ide/siimage.c
+++ b/drivers/ide/siimage.c
@@ -711,10 +711,10 @@ static const struct ide_port_ops sil_sat
static const struct ide_dma_ops sil_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = ide_dma_end,
.dma_test_irq = siimage_dma_test_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_lost_irq = ide_dma_lost_irq,
.dma_sff_read_status = ide_dma_sff_read_status,
Index: b/drivers/ide/sl82c105.c
===================================================================
--- a/drivers/ide/sl82c105.c
+++ b/drivers/ide/sl82c105.c
@@ -293,11 +293,11 @@ static const struct ide_port_ops sl82c10
static const struct ide_dma_ops sl82c105_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = sl82c105_dma_start,
.dma_end = sl82c105_dma_end,
.dma_test_irq = ide_dma_test_irq,
.dma_lost_irq = sl82c105_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = sl82c105_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
Index: b/drivers/ide/tc86c001.c
===================================================================
--- a/drivers/ide/tc86c001.c
+++ b/drivers/ide/tc86c001.c
@@ -182,11 +182,11 @@ static const struct ide_port_ops tc86c00
static const struct ide_dma_ops tc86c001_dma_ops = {
.dma_host_set = ide_dma_host_set,
.dma_setup = ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = tc86c001_dma_start,
.dma_end = ide_dma_end,
.dma_test_irq = ide_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = ide_dma_sff_read_status,
};
Index: b/drivers/ide/trm290.c
===================================================================
--- a/drivers/ide/trm290.c
+++ b/drivers/ide/trm290.c
@@ -176,11 +176,6 @@ static void trm290_selectproc (ide_drive
trm290_prepare_drive(drive, !!(drive->dev_flags & IDE_DFLAG_USING_DMA));
}
-static void trm290_dma_exec_cmd(ide_drive_t *drive, u8 command)
-{
- ide_execute_command(drive, command, &ide_dma_intr, WAIT_CMD, NULL);
-}
-
static int trm290_dma_setup(ide_drive_t *drive, struct ide_cmd *cmd)
{
ide_hwif_t *hwif = drive->hwif;
@@ -315,7 +310,6 @@ static const struct ide_port_ops trm290_
static struct ide_dma_ops trm290_dma_ops = {
.dma_host_set = trm290_dma_host_set,
.dma_setup = trm290_dma_setup,
- .dma_exec_cmd = trm290_dma_exec_cmd,
.dma_start = trm290_dma_start,
.dma_end = trm290_dma_end,
.dma_test_irq = trm290_dma_test_irq,
Index: b/drivers/ide/tx4939ide.c
===================================================================
--- a/drivers/ide/tx4939ide.c
+++ b/drivers/ide/tx4939ide.c
@@ -627,11 +627,11 @@ static const struct ide_port_ops tx4939i
static const struct ide_dma_ops tx4939ide_dma_ops = {
.dma_host_set = tx4939ide_dma_host_set,
.dma_setup = tx4939ide_dma_setup,
- .dma_exec_cmd = ide_dma_exec_cmd,
.dma_start = ide_dma_start,
.dma_end = tx4939ide_dma_end,
.dma_test_irq = tx4939ide_dma_test_irq,
.dma_lost_irq = ide_dma_lost_irq,
+ .dma_expiry = ide_dma_sff_expiry,
.dma_timeout = ide_dma_timeout,
.dma_sff_read_status = tx4939ide_dma_sff_read_status,
};
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -714,11 +714,11 @@ struct ide_port_ops {
struct ide_dma_ops {
void (*dma_host_set)(struct ide_drive_s *, int);
int (*dma_setup)(struct ide_drive_s *, struct ide_cmd *);
- void (*dma_exec_cmd)(struct ide_drive_s *, u8);
void (*dma_start)(struct ide_drive_s *);
int (*dma_end)(struct ide_drive_s *);
int (*dma_test_irq)(struct ide_drive_s *);
void (*dma_lost_irq)(struct ide_drive_s *);
+ int (*dma_expiry)(struct ide_drive_s *);
void (*dma_timeout)(struct ide_drive_s *);
/*
* The following method is optional and only required to be
@@ -1135,8 +1135,7 @@ void ide_kill_rq(ide_drive_t *, struct r
void __ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
void ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
-void ide_execute_command(ide_drive_t *, u8, ide_handler_t *, unsigned int,
- ide_expiry_t *);
+void ide_execute_command(ide_drive_t *, u8, ide_handler_t *, unsigned int);
void ide_execute_pkt_cmd(ide_drive_t *);
@@ -1450,10 +1449,10 @@ int config_drive_for_dma(ide_drive_t *);
int ide_build_dmatable(ide_drive_t *, struct ide_cmd *);
void ide_dma_host_set(ide_drive_t *, int);
int ide_dma_setup(ide_drive_t *, struct ide_cmd *);
-void ide_dma_exec_cmd(ide_drive_t *, u8);
extern void ide_dma_start(ide_drive_t *);
int ide_dma_end(ide_drive_t *);
int ide_dma_test_irq(ide_drive_t *);
+int ide_dma_sff_expiry(ide_drive_t *);
u8 ide_dma_sff_read_status(ide_hwif_t *);
extern const struct ide_dma_ops sff_dma_ops;
#else
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/6] ide: add ->dma_expiry method and remove ->dma_exec_cmd one
2009-02-09 23:20 ` [PATCH 4/6] ide: add ->dma_expiry method and remove ->dma_exec_cmd one Bartlomiej Zolnierkiewicz
@ 2009-02-10 18:18 ` Sergei Shtylyov
2009-02-11 16:30 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 27+ messages in thread
From: Sergei Shtylyov @ 2009-02-10 18:18 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, Borislav Petkov, linux-kernel
Bartlomiej Zolnierkiewicz wrote:
> * Rename dma_timer_expiry() to ide_dma_sff_expiry() and export it.
> * Add ->dma_expiry method
This name doesn't seem to make much sense. Why not leave it dma_timer_expiry?
MBR, Sergei
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/6] ide: add ->dma_expiry method and remove ->dma_exec_cmd one
2009-02-10 18:18 ` Sergei Shtylyov
@ 2009-02-11 16:30 ` Bartlomiej Zolnierkiewicz
2009-02-11 17:30 ` Sergei Shtylyov
0 siblings, 1 reply; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-11 16:30 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-ide, Borislav Petkov, linux-kernel
On Tuesday 10 February 2009, Sergei Shtylyov wrote:
> Bartlomiej Zolnierkiewicz wrote:
>
> > * Rename dma_timer_expiry() to ide_dma_sff_expiry() and export it.
>
> > * Add ->dma_expiry method
>
> This name doesn't seem to make much sense. Why not leave it dma_timer_expiry?
Well, the name matches with hwif->expiry one + the downside of leaving it at
dma_timer_expiry is a need to reformat all struct ide_dma_ops instances...
Thanks,
Bart
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/6] ide: add ->dma_expiry method and remove ->dma_exec_cmd one
2009-02-11 16:30 ` Bartlomiej Zolnierkiewicz
@ 2009-02-11 17:30 ` Sergei Shtylyov
2009-02-17 14:16 ` Bartlomiej Zolnierkiewicz
0 siblings, 1 reply; 27+ messages in thread
From: Sergei Shtylyov @ 2009-02-11 17:30 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, Borislav Petkov, linux-kernel
Hello.
Bartlomiej Zolnierkiewicz wrote:
>>Bartlomiej Zolnierkiewicz wrote:
>>>* Rename dma_timer_expiry() to ide_dma_sff_expiry() and export it.
>>>* Add ->dma_expiry method
>> This name doesn't seem to make much sense. Why not leave it dma_timer_expiry?
> Well, the name matches with hwif->expiry one + the downside of leaving it at
> dma_timer_expiry is a need to reformat all struct ide_dma_ops instances...
How about just timer_expiry() then?
> Thanks,
> Bart
MBR, Sergei
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/6] ide: add ->dma_expiry method and remove ->dma_exec_cmd one
2009-02-11 17:30 ` Sergei Shtylyov
@ 2009-02-17 14:16 ` Bartlomiej Zolnierkiewicz
2009-02-17 14:29 ` Sergei Shtylyov
0 siblings, 1 reply; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-17 14:16 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-ide, Borislav Petkov, linux-kernel
On Wednesday 11 February 2009, Sergei Shtylyov wrote:
> Hello.
>
> Bartlomiej Zolnierkiewicz wrote:
>
> >>Bartlomiej Zolnierkiewicz wrote:
>
> >>>* Rename dma_timer_expiry() to ide_dma_sff_expiry() and export it.
>
> >>>* Add ->dma_expiry method
>
> >> This name doesn't seem to make much sense. Why not leave it dma_timer_expiry?
>
> > Well, the name matches with hwif->expiry one + the downside of leaving it at
> > dma_timer_expiry is a need to reformat all struct ide_dma_ops instances...
On the 2nd look it doesn't seem to be a problem (I must have been looking at
some odd case previously) -- I'll fix it up later.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/6] ide: add ->dma_expiry method and remove ->dma_exec_cmd one
2009-02-17 14:16 ` Bartlomiej Zolnierkiewicz
@ 2009-02-17 14:29 ` Sergei Shtylyov
0 siblings, 0 replies; 27+ messages in thread
From: Sergei Shtylyov @ 2009-02-17 14:29 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, Borislav Petkov, linux-kernel
Bartlomiej Zolnierkiewicz wrote:
>>Hello.
>>Bartlomiej Zolnierkiewicz wrote:
>>>>Bartlomiej Zolnierkiewicz wrote:
>>>>>* Rename dma_timer_expiry() to ide_dma_sff_expiry() and export it.
>>>>>* Add ->dma_expiry method
>>>> This name doesn't seem to make much sense. Why not leave it dma_timer_expiry?
>>>Well, the name matches with hwif->expiry one + the downside of leaving it at
>>>dma_timer_expiry is a need to reformat all struct ide_dma_ops instances...
> On the 2nd look it doesn't seem to be a problem (I must have been looking at
> some odd case previously) -- I'll fix it up later.
I vote for timer_expiry().
MBR, Sergei
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-09 23:19 [PATCH 0/6] ide: more unifications of ATA and ATAPI support Bartlomiej Zolnierkiewicz
` (3 preceding siblings ...)
2009-02-09 23:20 ` [PATCH 4/6] ide: add ->dma_expiry method and remove ->dma_exec_cmd one Bartlomiej Zolnierkiewicz
@ 2009-02-09 23:20 ` Bartlomiej Zolnierkiewicz
2009-02-11 6:55 ` Borislav Petkov
2009-02-09 23:20 ` [PATCH 6/6] ide: keep track of number of bytes instead of sectors in struct ide_cmd Bartlomiej Zolnierkiewicz
2009-02-11 7:16 ` [PATCH 0/6] ide: more unifications of ATA and ATAPI support Borislav Petkov
6 siblings, 1 reply; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-09 23:20 UTC (permalink / raw)
To: linux-ide; +Cc: Borislav Petkov, Bartlomiej Zolnierkiewicz, linux-kernel
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: remove ide_execute_pkt_cmd()
* Pass command structure to ide_execute_command() and skip
__ide_set_handler() for ATAPI protocols.
* Convert ide_issue_pc() to always use ide_execute_command()
and remove no longer needed ide_execute_pkt_cmd().
There should be no functional changes caused by this patch.
Cc: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-atapi.c | 15 +++++++--------
drivers/ide/ide-iops.c | 23 ++++++-----------------
drivers/ide/ide-taskfile.c | 6 ++----
include/linux/ide.h | 5 ++---
4 files changed, 17 insertions(+), 32 deletions(-)
Index: b/drivers/ide/ide-atapi.c
===================================================================
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -481,6 +481,7 @@ static void ide_init_packet_cmd(struct i
cmd->protocol = dma ? ATAPI_PROT_DMA : ATAPI_PROT_PIO;
cmd->tf_flags |= IDE_TFLAG_OUT_LBAH | IDE_TFLAG_OUT_LBAM |
IDE_TFLAG_OUT_FEATURE | tf_flags;
+ cmd->tf.command = ATA_CMD_PACKET;
cmd->tf.feature = dma; /* Use PIO/DMA */
cmd->tf.lbam = bcount & 0xff;
cmd->tf.lbah = (bcount >> 8) & 0xff;
@@ -626,6 +627,7 @@ ide_startstop_t ide_issue_pc(ide_drive_t
unsigned int timeout;
u32 tf_flags;
u16 bcount;
+ u8 drq_int = !!(drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT);
if (dev_is_idecd(drive)) {
tf_flags = IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL;
@@ -675,17 +677,14 @@ ide_startstop_t ide_issue_pc(ide_drive_t
(void)do_rw_taskfile(drive, cmd);
- /* Issue the packet command */
- if (drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT) {
+ if (drq_int) {
if (drive->dma)
drive->waiting_for_dma = 0;
hwif->expiry = expiry;
- ide_execute_command(drive, ATA_CMD_PACKET, ide_transfer_pc,
- timeout);
- return ide_started;
- } else {
- ide_execute_pkt_cmd(drive);
- return ide_transfer_pc(drive);
}
+
+ ide_execute_command(drive, cmd, ide_transfer_pc, timeout);
+
+ return drq_int ? ide_started : ide_transfer_pc(drive);
}
EXPORT_SYMBOL_GPL(ide_issue_pc);
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -451,7 +451,7 @@ EXPORT_SYMBOL(ide_set_handler);
/**
* ide_execute_command - execute an IDE command
* @drive: IDE drive to issue the command against
- * @command: command byte to write
+ * @cmd: command
* @handler: handler for next phase
* @timeout: timeout for command
*
@@ -461,15 +461,16 @@ EXPORT_SYMBOL(ide_set_handler);
* should go via this function or do equivalent locking.
*/
-void ide_execute_command(ide_drive_t *drive, u8 cmd, ide_handler_t *handler,
- unsigned timeout)
+void ide_execute_command(ide_drive_t *drive, struct ide_cmd *cmd,
+ ide_handler_t *handler, unsigned timeout)
{
ide_hwif_t *hwif = drive->hwif;
unsigned long flags;
spin_lock_irqsave(&hwif->lock, flags);
- __ide_set_handler(drive, handler, timeout);
- hwif->tp_ops->exec_command(hwif, cmd);
+ if (cmd->protocol != ATAPI_PROT_DMA && cmd->protocol != ATAPI_PROT_PIO)
+ __ide_set_handler(drive, handler, timeout);
+ hwif->tp_ops->exec_command(hwif, cmd->tf.command);
/*
* Drive takes 400nS to respond, we must avoid the IRQ being
* serviced before that.
@@ -480,18 +481,6 @@ void ide_execute_command(ide_drive_t *dr
spin_unlock_irqrestore(&hwif->lock, flags);
}
-void ide_execute_pkt_cmd(ide_drive_t *drive)
-{
- ide_hwif_t *hwif = drive->hwif;
- unsigned long flags;
-
- spin_lock_irqsave(&hwif->lock, flags);
- hwif->tp_ops->exec_command(hwif, ATA_CMD_PACKET);
- ndelay(400);
- spin_unlock_irqrestore(&hwif->lock, flags);
-}
-EXPORT_SYMBOL_GPL(ide_execute_pkt_cmd);
-
/*
* ide_wait_not_busy() waits for the currently selected device on the hwif
* to report a non-busy status, see comments in ide_probe_port().
Index: b/drivers/ide/ide-taskfile.c
===================================================================
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -97,8 +97,7 @@ ide_startstop_t do_rw_taskfile(ide_drive
case ATA_PROT_NODATA:
if (handler == NULL)
handler = task_no_data_intr;
- ide_execute_command(drive, tf->command, handler,
- WAIT_WORSTCASE);
+ ide_execute_command(drive, cmd, handler, WAIT_WORSTCASE);
return ide_started;
case ATA_PROT_DMA:
if ((drive->dev_flags & IDE_DFLAG_USING_DMA) == 0 ||
@@ -106,8 +105,7 @@ ide_startstop_t do_rw_taskfile(ide_drive
dma_ops->dma_setup(drive, cmd))
return ide_stopped;
hwif->expiry = dma_ops->dma_expiry;
- ide_execute_command(drive, tf->command, ide_dma_intr,
- 2 * WAIT_CMD);
+ ide_execute_command(drive, cmd, ide_dma_intr, 2 * WAIT_CMD);
dma_ops->dma_start(drive);
default:
return ide_started;
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1135,9 +1135,8 @@ void ide_kill_rq(ide_drive_t *, struct r
void __ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
void ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
-void ide_execute_command(ide_drive_t *, u8, ide_handler_t *, unsigned int);
-
-void ide_execute_pkt_cmd(ide_drive_t *);
+void ide_execute_command(ide_drive_t *, struct ide_cmd *, ide_handler_t *,
+ unsigned int);
void ide_pad_transfer(ide_drive_t *, int, int);
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-09 23:20 ` [PATCH 5/6] ide: remove ide_execute_pkt_cmd() Bartlomiej Zolnierkiewicz
@ 2009-02-11 6:55 ` Borislav Petkov
2009-02-11 13:22 ` Sergei Shtylyov
0 siblings, 1 reply; 27+ messages in thread
From: Borislav Petkov @ 2009-02-11 6:55 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel
On Tue, Feb 10, 2009 at 12:20:19AM +0100, Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [PATCH] ide: remove ide_execute_pkt_cmd()
>
> * Pass command structure to ide_execute_command() and skip
> __ide_set_handler() for ATAPI protocols.
>
> * Convert ide_issue_pc() to always use ide_execute_command()
> and remove no longer needed ide_execute_pkt_cmd().
>
> There should be no functional changes caused by this patch.
>
> Cc: Borislav Petkov <petkovbb@gmail.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
> drivers/ide/ide-atapi.c | 15 +++++++--------
> drivers/ide/ide-iops.c | 23 ++++++-----------------
> drivers/ide/ide-taskfile.c | 6 ++----
> include/linux/ide.h | 5 ++---
> 4 files changed, 17 insertions(+), 32 deletions(-)
>
> Index: b/drivers/ide/ide-atapi.c
> ===================================================================
> --- a/drivers/ide/ide-atapi.c
> +++ b/drivers/ide/ide-atapi.c
> @@ -481,6 +481,7 @@ static void ide_init_packet_cmd(struct i
> cmd->protocol = dma ? ATAPI_PROT_DMA : ATAPI_PROT_PIO;
> cmd->tf_flags |= IDE_TFLAG_OUT_LBAH | IDE_TFLAG_OUT_LBAM |
> IDE_TFLAG_OUT_FEATURE | tf_flags;
> + cmd->tf.command = ATA_CMD_PACKET;
> cmd->tf.feature = dma; /* Use PIO/DMA */
> cmd->tf.lbam = bcount & 0xff;
> cmd->tf.lbah = (bcount >> 8) & 0xff;
> @@ -626,6 +627,7 @@ ide_startstop_t ide_issue_pc(ide_drive_t
> unsigned int timeout;
> u32 tf_flags;
> u16 bcount;
> + u8 drq_int = !!(drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT);
How about we finally add those check macros in block layer fashion like
blk_pc_request et al and do
#define drv_can_drq_interrupt(drive) ((drive)->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT)
...
or similar instead of wasting stack space? It'll also read better in the if()
check:
if (drv_can_irq_interrupt(drive)) { ...
If it's ok with you, I'll whip up patch(es) later.
>
> if (dev_is_idecd(drive)) {
> tf_flags = IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL;
> @@ -675,17 +677,14 @@ ide_startstop_t ide_issue_pc(ide_drive_t
>
> (void)do_rw_taskfile(drive, cmd);
>
> - /* Issue the packet command */
> - if (drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT) {
> + if (drq_int) {
> if (drive->dma)
> drive->waiting_for_dma = 0;
> hwif->expiry = expiry;
> - ide_execute_command(drive, ATA_CMD_PACKET, ide_transfer_pc,
> - timeout);
> - return ide_started;
> - } else {
> - ide_execute_pkt_cmd(drive);
> - return ide_transfer_pc(drive);
> }
> +
> + ide_execute_command(drive, cmd, ide_transfer_pc, timeout);
> +
> + return drq_int ? ide_started : ide_transfer_pc(drive);
> }
> EXPORT_SYMBOL_GPL(ide_issue_pc);
> Index: b/drivers/ide/ide-iops.c
> ===================================================================
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -451,7 +451,7 @@ EXPORT_SYMBOL(ide_set_handler);
> /**
> * ide_execute_command - execute an IDE command
> * @drive: IDE drive to issue the command against
> - * @command: command byte to write
> + * @cmd: command
> * @handler: handler for next phase
> * @timeout: timeout for command
> *
> @@ -461,15 +461,16 @@ EXPORT_SYMBOL(ide_set_handler);
> * should go via this function or do equivalent locking.
> */
>
> -void ide_execute_command(ide_drive_t *drive, u8 cmd, ide_handler_t *handler,
> - unsigned timeout)
> +void ide_execute_command(ide_drive_t *drive, struct ide_cmd *cmd,
> + ide_handler_t *handler, unsigned timeout)
> {
> ide_hwif_t *hwif = drive->hwif;
> unsigned long flags;
>
> spin_lock_irqsave(&hwif->lock, flags);
> - __ide_set_handler(drive, handler, timeout);
> - hwif->tp_ops->exec_command(hwif, cmd);
> + if (cmd->protocol != ATAPI_PROT_DMA && cmd->protocol != ATAPI_PROT_PIO)
> + __ide_set_handler(drive, handler, timeout);
> + hwif->tp_ops->exec_command(hwif, cmd->tf.command);
> /*
> * Drive takes 400nS to respond, we must avoid the IRQ being
> * serviced before that.
> @@ -480,18 +481,6 @@ void ide_execute_command(ide_drive_t *dr
> spin_unlock_irqrestore(&hwif->lock, flags);
> }
>
> -void ide_execute_pkt_cmd(ide_drive_t *drive)
> -{
> - ide_hwif_t *hwif = drive->hwif;
> - unsigned long flags;
> -
> - spin_lock_irqsave(&hwif->lock, flags);
> - hwif->tp_ops->exec_command(hwif, ATA_CMD_PACKET);
> - ndelay(400);
> - spin_unlock_irqrestore(&hwif->lock, flags);
> -}
> -EXPORT_SYMBOL_GPL(ide_execute_pkt_cmd);
> -
> /*
> * ide_wait_not_busy() waits for the currently selected device on the hwif
> * to report a non-busy status, see comments in ide_probe_port().
> Index: b/drivers/ide/ide-taskfile.c
> ===================================================================
> --- a/drivers/ide/ide-taskfile.c
> +++ b/drivers/ide/ide-taskfile.c
> @@ -97,8 +97,7 @@ ide_startstop_t do_rw_taskfile(ide_drive
> case ATA_PROT_NODATA:
> if (handler == NULL)
> handler = task_no_data_intr;
> - ide_execute_command(drive, tf->command, handler,
> - WAIT_WORSTCASE);
> + ide_execute_command(drive, cmd, handler, WAIT_WORSTCASE);
> return ide_started;
> case ATA_PROT_DMA:
> if ((drive->dev_flags & IDE_DFLAG_USING_DMA) == 0 ||
> @@ -106,8 +105,7 @@ ide_startstop_t do_rw_taskfile(ide_drive
> dma_ops->dma_setup(drive, cmd))
> return ide_stopped;
> hwif->expiry = dma_ops->dma_expiry;
> - ide_execute_command(drive, tf->command, ide_dma_intr,
> - 2 * WAIT_CMD);
> + ide_execute_command(drive, cmd, ide_dma_intr, 2 * WAIT_CMD);
> dma_ops->dma_start(drive);
> default:
> return ide_started;
> Index: b/include/linux/ide.h
> ===================================================================
> --- a/include/linux/ide.h
> +++ b/include/linux/ide.h
> @@ -1135,9 +1135,8 @@ void ide_kill_rq(ide_drive_t *, struct r
> void __ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
> void ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
>
> -void ide_execute_command(ide_drive_t *, u8, ide_handler_t *, unsigned int);
> -
> -void ide_execute_pkt_cmd(ide_drive_t *);
> +void ide_execute_command(ide_drive_t *, struct ide_cmd *, ide_handler_t *,
> + unsigned int);
>
> void ide_pad_transfer(ide_drive_t *, int, int);
>
--
Regards/Gruss,
Boris.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-11 6:55 ` Borislav Petkov
@ 2009-02-11 13:22 ` Sergei Shtylyov
2009-02-11 13:37 ` Borislav Petkov
0 siblings, 1 reply; 27+ messages in thread
From: Sergei Shtylyov @ 2009-02-11 13:22 UTC (permalink / raw)
To: petkovbb, Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel
Hello.
Borislav Petkov wrote:
>> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>> Subject: [PATCH] ide: remove ide_execute_pkt_cmd()
>>
>> * Pass command structure to ide_execute_command() and skip
>> __ide_set_handler() for ATAPI protocols.
>>
>> * Convert ide_issue_pc() to always use ide_execute_command()
>> and remove no longer needed ide_execute_pkt_cmd().
>>
>> There should be no functional changes caused by this patch.
>>
>> Cc: Borislav Petkov <petkovbb@gmail.com>
>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>> ---
>> drivers/ide/ide-atapi.c | 15 +++++++--------
>> drivers/ide/ide-iops.c | 23 ++++++-----------------
>> drivers/ide/ide-taskfile.c | 6 ++----
>> include/linux/ide.h | 5 ++---
>> 4 files changed, 17 insertions(+), 32 deletions(-)
>>
>> Index: b/drivers/ide/ide-atapi.c
>> ===================================================================
>> --- a/drivers/ide/ide-atapi.c
>> +++ b/drivers/ide/ide-atapi.c
>> @@ -481,6 +481,7 @@ static void ide_init_packet_cmd(struct i
>> cmd->protocol = dma ? ATAPI_PROT_DMA : ATAPI_PROT_PIO;
>> cmd->tf_flags |= IDE_TFLAG_OUT_LBAH | IDE_TFLAG_OUT_LBAM |
>> IDE_TFLAG_OUT_FEATURE | tf_flags;
>> + cmd->tf.command = ATA_CMD_PACKET;
>> cmd->tf.feature = dma; /* Use PIO/DMA */
>> cmd->tf.lbam = bcount & 0xff;
>> cmd->tf.lbah = (bcount >> 8) & 0xff;
>> @@ -626,6 +627,7 @@ ide_startstop_t ide_issue_pc(ide_drive_t
>> unsigned int timeout;
>> u32 tf_flags;
>> u16 bcount;
>> + u8 drq_int = !!(drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT);
>>
>
> How about we finally add those check macros in block layer fashion like
> blk_pc_request et al and do
>
> #define drv_can_drq_interrupt(drive) ((drive)->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT)
>
I suppose it's for the devices that interrupt on packet DRQ? Then
it's hardly a good name because it's not like this is some optional
capability.
> ...
>
>
> or similar instead of wasting stack space?
It doesn't necessarily waste stack space. Haven't you heard about
compiler putting local vairables into registers?
> It'll also read better in the if() check:
>
> if (drv_can_irq_interrupt(drive)) { ...
>
It's faster to checj a local variable than to dereference drv several
times -- unless gcc optimizes that away (by creating an implicit local
variable :-).
> If it's ok with you, I'll whip up patch(es) later.
>
>> if (dev_is_idecd(drive)) {
>> tf_flags = IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL;
>> @@ -675,17 +677,14 @@ ide_startstop_t ide_issue_pc(ide_drive_t
>>
Don't leave uncommented fragment behind please.
MBR, Sergei
>>
>> (void)do_rw_taskfile(drive, cmd);
>>
>> - /* Issue the packet command */
>> - if (drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT) {
>> + if (drq_int) {
>> if (drive->dma)
>> drive->waiting_for_dma = 0;
>> hwif->expiry = expiry;
>> - ide_execute_command(drive, ATA_CMD_PACKET, ide_transfer_pc,
>> - timeout);
>> - return ide_started;
>> - } else {
>> - ide_execute_pkt_cmd(drive);
>> - return ide_transfer_pc(drive);
>> }
>> +
>> + ide_execute_command(drive, cmd, ide_transfer_pc, timeout);
>> +
>> + return drq_int ? ide_started : ide_transfer_pc(drive);
>> }
>> EXPORT_SYMBOL_GPL(ide_issue_pc);
>> Index: b/drivers/ide/ide-iops.c
>> ===================================================================
>> --- a/drivers/ide/ide-iops.c
>> +++ b/drivers/ide/ide-iops.c
>> @@ -451,7 +451,7 @@ EXPORT_SYMBOL(ide_set_handler);
>> /**
>> * ide_execute_command - execute an IDE command
>> * @drive: IDE drive to issue the command against
>> - * @command: command byte to write
>> + * @cmd: command
>> * @handler: handler for next phase
>> * @timeout: timeout for command
>> *
>> @@ -461,15 +461,16 @@ EXPORT_SYMBOL(ide_set_handler);
>> * should go via this function or do equivalent locking.
>> */
>>
>> -void ide_execute_command(ide_drive_t *drive, u8 cmd, ide_handler_t *handler,
>> - unsigned timeout)
>> +void ide_execute_command(ide_drive_t *drive, struct ide_cmd *cmd,
>> + ide_handler_t *handler, unsigned timeout)
>> {
>> ide_hwif_t *hwif = drive->hwif;
>> unsigned long flags;
>>
>> spin_lock_irqsave(&hwif->lock, flags);
>> - __ide_set_handler(drive, handler, timeout);
>> - hwif->tp_ops->exec_command(hwif, cmd);
>> + if (cmd->protocol != ATAPI_PROT_DMA && cmd->protocol != ATAPI_PROT_PIO)
>> + __ide_set_handler(drive, handler, timeout);
>> + hwif->tp_ops->exec_command(hwif, cmd->tf.command);
>> /*
>> * Drive takes 400nS to respond, we must avoid the IRQ being
>> * serviced before that.
>> @@ -480,18 +481,6 @@ void ide_execute_command(ide_drive_t *dr
>> spin_unlock_irqrestore(&hwif->lock, flags);
>> }
>>
>> -void ide_execute_pkt_cmd(ide_drive_t *drive)
>> -{
>> - ide_hwif_t *hwif = drive->hwif;
>> - unsigned long flags;
>> -
>> - spin_lock_irqsave(&hwif->lock, flags);
>> - hwif->tp_ops->exec_command(hwif, ATA_CMD_PACKET);
>> - ndelay(400);
>> - spin_unlock_irqrestore(&hwif->lock, flags);
>> -}
>> -EXPORT_SYMBOL_GPL(ide_execute_pkt_cmd);
>> -
>> /*
>> * ide_wait_not_busy() waits for the currently selected device on the hwif
>> * to report a non-busy status, see comments in ide_probe_port().
>> Index: b/drivers/ide/ide-taskfile.c
>> ===================================================================
>> --- a/drivers/ide/ide-taskfile.c
>> +++ b/drivers/ide/ide-taskfile.c
>> @@ -97,8 +97,7 @@ ide_startstop_t do_rw_taskfile(ide_drive
>> case ATA_PROT_NODATA:
>> if (handler == NULL)
>> handler = task_no_data_intr;
>> - ide_execute_command(drive, tf->command, handler,
>> - WAIT_WORSTCASE);
>> + ide_execute_command(drive, cmd, handler, WAIT_WORSTCASE);
>> return ide_started;
>> case ATA_PROT_DMA:
>> if ((drive->dev_flags & IDE_DFLAG_USING_DMA) == 0 ||
>> @@ -106,8 +105,7 @@ ide_startstop_t do_rw_taskfile(ide_drive
>> dma_ops->dma_setup(drive, cmd))
>> return ide_stopped;
>> hwif->expiry = dma_ops->dma_expiry;
>> - ide_execute_command(drive, tf->command, ide_dma_intr,
>> - 2 * WAIT_CMD);
>> + ide_execute_command(drive, cmd, ide_dma_intr, 2 * WAIT_CMD);
>> dma_ops->dma_start(drive);
>> default:
>> return ide_started;
>> Index: b/include/linux/ide.h
>> ===================================================================
>> --- a/include/linux/ide.h
>> +++ b/include/linux/ide.h
>> @@ -1135,9 +1135,8 @@ void ide_kill_rq(ide_drive_t *, struct r
>> void __ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
>> void ide_set_handler(ide_drive_t *, ide_handler_t *, unsigned int);
>>
>> -void ide_execute_command(ide_drive_t *, u8, ide_handler_t *, unsigned int);
>> -
>> -void ide_execute_pkt_cmd(ide_drive_t *);
>> +void ide_execute_command(ide_drive_t *, struct ide_cmd *, ide_handler_t *,
>> + unsigned int);
>>
>> void ide_pad_transfer(ide_drive_t *, int, int);
>>
>>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-11 13:22 ` Sergei Shtylyov
@ 2009-02-11 13:37 ` Borislav Petkov
2009-02-11 13:49 ` Sergei Shtylyov
0 siblings, 1 reply; 27+ messages in thread
From: Borislav Petkov @ 2009-02-11 13:37 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel
Hi,
On Wed, Feb 11, 2009 at 2:22 PM, Sergei Shtylyov
<sshtylyov@ru.mvista.com> wrote:
> Hello.
>
> Borislav Petkov wrote:
>
>>> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>> Subject: [PATCH] ide: remove ide_execute_pkt_cmd()
>>>
>>> * Pass command structure to ide_execute_command() and skip
>>> __ide_set_handler() for ATAPI protocols.
>>>
>>> * Convert ide_issue_pc() to always use ide_execute_command()
>>> and remove no longer needed ide_execute_pkt_cmd().
>>>
>>> There should be no functional changes caused by this patch.
>>>
>>> Cc: Borislav Petkov <petkovbb@gmail.com>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>> ---
>>> drivers/ide/ide-atapi.c | 15 +++++++--------
>>> drivers/ide/ide-iops.c | 23 ++++++-----------------
>>> drivers/ide/ide-taskfile.c | 6 ++----
>>> include/linux/ide.h | 5 ++---
>>> 4 files changed, 17 insertions(+), 32 deletions(-)
>>>
>>> Index: b/drivers/ide/ide-atapi.c
>>> ===================================================================
>>> --- a/drivers/ide/ide-atapi.c
>>> +++ b/drivers/ide/ide-atapi.c
>>> @@ -481,6 +481,7 @@ static void ide_init_packet_cmd(struct i
>>> cmd->protocol = dma ? ATAPI_PROT_DMA : ATAPI_PROT_PIO;
>>> cmd->tf_flags |= IDE_TFLAG_OUT_LBAH | IDE_TFLAG_OUT_LBAM |
>>> IDE_TFLAG_OUT_FEATURE | tf_flags;
>>> + cmd->tf.command = ATA_CMD_PACKET;
>>> cmd->tf.feature = dma; /* Use PIO/DMA */
>>> cmd->tf.lbam = bcount & 0xff;
>>> cmd->tf.lbah = (bcount >> 8) & 0xff;
>>> @@ -626,6 +627,7 @@ ide_startstop_t ide_issue_pc(ide_drive_t
>>> unsigned int timeout;
>>> u32 tf_flags;
>>> u16 bcount;
>>> + u8 drq_int = !!(drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT);
>>>
>>
>> How about we finally add those check macros in block layer fashion like
>> blk_pc_request et al and do
>>
>> #define drv_can_drq_interrupt(drive) ((drive)->atapi_flags &
>> IDE_AFLAG_DRQ_INTERRUPT)
>>
>
> I suppose it's for the devices that interrupt on packet DRQ? Then it's
> hardly a good name because it's not like this is some optional capability.
No, I was alluding to the command packet DRQ type used by the device as it is
put in SFF8020i, 7.1.7.1 General Configuration Word.
>> or similar instead of wasting stack space?
>
> It doesn't necessarily waste stack space. Haven't you heard about compiler
> putting local vairables into registers?
Yes, have you heard of unnecessary register spilling?
>> It'll also read better in the if() check:
>>
>> if (drv_can_irq_interrupt(drive)) { ...
>>
>
> It's faster to checj a local variable than to dereference drv several times
> -- unless gcc optimizes that away (by creating an implicit local variable
> :-).
I hope gcc is smart enough to do that.
>> If it's ok with you, I'll whip up patch(es) later.
>>
>
>>> if (dev_is_idecd(drive)) {
>>> tf_flags = IDE_TFLAG_OUT_NSECT | IDE_TFLAG_OUT_LBAL;
>>> @@ -675,17 +677,14 @@ ide_startstop_t ide_issue_pc(ide_drive_t
>>>
>
> Don't leave uncommented fragment behind please.
>
> MBR, Sergei
>
>
[..]
--
Regards/Gruss,
Boris
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-11 13:37 ` Borislav Petkov
@ 2009-02-11 13:49 ` Sergei Shtylyov
2009-02-11 16:32 ` Borislav Petkov
2009-02-11 16:37 ` Bartlomiej Zolnierkiewicz
0 siblings, 2 replies; 27+ messages in thread
From: Sergei Shtylyov @ 2009-02-11 13:49 UTC (permalink / raw)
To: petkovbb; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel
Hello.
Borislav Petkov wrote:
>>>> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>>> Subject: [PATCH] ide: remove ide_execute_pkt_cmd()
>>>>
>>>> * Pass command structure to ide_execute_command() and skip
>>>> __ide_set_handler() for ATAPI protocols.
>>>>
>>>> * Convert ide_issue_pc() to always use ide_execute_command()
>>>> and remove no longer needed ide_execute_pkt_cmd().
>>>>
>>>> There should be no functional changes caused by this patch.
>>>>
>>>> Cc: Borislav Petkov <petkovbb@gmail.com>
>>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>>> ---
>>>> drivers/ide/ide-atapi.c | 15 +++++++--------
>>>> drivers/ide/ide-iops.c | 23 ++++++-----------------
>>>> drivers/ide/ide-taskfile.c | 6 ++----
>>>> include/linux/ide.h | 5 ++---
>>>> 4 files changed, 17 insertions(+), 32 deletions(-)
>>>>
>>>> Index: b/drivers/ide/ide-atapi.c
>>>> ===================================================================
>>>> --- a/drivers/ide/ide-atapi.c
>>>> +++ b/drivers/ide/ide-atapi.c
>>>> @@ -481,6 +481,7 @@ static void ide_init_packet_cmd(struct i
>>>> cmd->protocol = dma ? ATAPI_PROT_DMA : ATAPI_PROT_PIO;
>>>> cmd->tf_flags |= IDE_TFLAG_OUT_LBAH | IDE_TFLAG_OUT_LBAM |
>>>> IDE_TFLAG_OUT_FEATURE | tf_flags;
>>>> + cmd->tf.command = ATA_CMD_PACKET;
>>>> cmd->tf.feature = dma; /* Use PIO/DMA */
>>>> cmd->tf.lbam = bcount & 0xff;
>>>> cmd->tf.lbah = (bcount >> 8) & 0xff;
>>>> @@ -626,6 +627,7 @@ ide_startstop_t ide_issue_pc(ide_drive_t
>>>> unsigned int timeout;
>>>> u32 tf_flags;
>>>> u16 bcount;
>>>> + u8 drq_int = !!(drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT);
>>>>
>>>>
>>> How about we finally add those check macros in block layer fashion like
>>> blk_pc_request et al and do
>>>
>>> #define drv_can_drq_interrupt(drive) ((drive)->atapi_flags &
>>> IDE_AFLAG_DRQ_INTERRUPT)
>>>
>>>
>> I suppose it's for the devices that interrupt on packet DRQ? Then it's
>> hardly a good name because it's not like this is some optional capability.
>>
>
> No, I was alluding to the command packet DRQ type used by the device as it is
> put in SFF8020i, 7.1.7.1 General Configuration Word.
>
I was talking about exactly the same feature. :-)
>>> or similar instead of wasting stack space?
>>>
>> It doesn't necessarily waste stack space. Haven't you heard about compiler
>> putting local vairables into registers?
>>
>
> Yes, have you heard of unnecessary register spilling?
>
No -- only about stack spilling on CPUs "caching" the top of stack in
their register file (like SPARC).
Linux runs not only on x86 and many RISCs can store several local
variables in the dedicated registers -- it's the part of say MIPS ABIs...
>>> It'll also read better in the if() check:
>>>
>>> if (drv_can_irq_interrupt(drive)) { ...
>>>
>>>
>> It's faster to checj a local variable than to dereference drv several times
>> -- unless gcc optimizes that away (by creating an implicit local variable
>> :-).
>>
>
> I hope gcc is smart enough to do that.
>
Then where's the win?
MBR, Sergei
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-11 13:49 ` Sergei Shtylyov
@ 2009-02-11 16:32 ` Borislav Petkov
2009-02-15 12:24 ` Sergei Shtylyov
2009-02-11 16:37 ` Bartlomiej Zolnierkiewicz
1 sibling, 1 reply; 27+ messages in thread
From: Borislav Petkov @ 2009-02-11 16:32 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel
[..]
>>>>
>>>> How about we finally add those check macros in block layer fashion like
>>>> blk_pc_request et al and do
>>>>
>>>> #define drv_can_drq_interrupt(drive) ((drive)->atapi_flags &
>>>> IDE_AFLAG_DRQ_INTERRUPT)
>>>>
>>>>
>>>
>>> I suppose it's for the devices that interrupt on packet DRQ? Then it's
>>> hardly a good name because it's not like this is some optional
>>> capability.
>>>
>>
>> No, I was alluding to the command packet DRQ type used by the device as it
>> is
>> put in SFF8020i, 7.1.7.1 General Configuration Word.
>>
>
> I was talking about exactly the same feature. :-)
Ok, I agree, the names could be a bit more explanatory w.r.t of the DRQ type:
ide_drv_microprocessor_drq
ide_drv_interrupt_drq
ide_drv_accelerated_drq
although we use only one type currently.
>>>> or similar instead of wasting stack space?
>>>>
>>>
>>> It doesn't necessarily waste stack space. Haven't you heard about
>>> compiler
>>> putting local vairables into registers?
>>>
>>
>> Yes, have you heard of unnecessary register spilling?
>>
>
> No -- only about stack spilling on CPUs "caching" the top of stack in their
> register file (like SPARC).
> Linux runs not only on x86 and many RISCs can store several local variables
> in the dedicated registers -- it's the part of say MIPS ABIs...
>>>> It'll also read better in the if() check:
>>>>
>>>> if (drv_can_irq_interrupt(drive)) { ...
>>>>
>>>>
>>>
>>> It's faster to checj a local variable than to dereference drv several
>>> times
>>> -- unless gcc optimizes that away (by creating an implicit local variable
>>> :-).
Let's look at an example:
In ide-cd.c:cdrom_newpc_intr() you have the following code snippet:
<ide-cd.c>
799 thislen = blk_fs_request(rq) ? len : rq->data_len;
800 if (thislen > len)
801 thislen = len;
802
803 ide_debug_log(IDE_DBG_PC, "%s: DRQ: stat: 0x%x, thislen: %d\n",
804 __func__, stat, thislen);
805
806 /* If DRQ is clear, the command has completed. */
807 if ((stat & ATA_DRQ) == 0) {
808 if (blk_fs_request(rq)) {
</ide-cd.c>
Now watch the blk_fs_request() thing.
Here's what my gcc¹ spits out:
<ide-cd.s>
.LVL174:
.loc 1 799 0
movl 76(%r12), %ecx # <variable>.cmd_type, prephitmp.1128
cmpl $1, %ecx #, prephitmp.1128
movl %ecx, %r8d # prephitmp.1128, prephitmp.1047
je .L225 #,
.LVL175:
.loc 1 800 0
movzwl -44(%rbp), %r15d # len, thislen
.LVL176:
.loc 1 799 0
movl 280(%r12), %edx # <variable>.data_len, thislen.1129
.LVL177:
.loc 1 800 0
cmpl %r15d, %edx # thislen, thislen.1129
movl %r15d, %ebx # thislen, thislen.1163
jg .L145 #,
.LVL178:
movl %edx, %r15d # thislen.1129, thislen
.LVL179:
.L145:
.loc 1 807 0
testb $8, -64(%rbp) #, stat
jne .L147 #,
.LVL180:
.loc 1 808 0
cmpl $1, %ecx #, prephitmp.1128
je .L226 #,
.loc 1 825 0
cmpl $2, %ecx #, prephitmp.1128
.p2align 4,,3
.p2align 3
je .L152 #,
.LBB408:
</ide-cd.s>
and at label .LVL174 you see the blk_fs_request() check from line
799 above. Later, at label .LVL180 you see the next blk_fs_request() check from
line 808 and this is cached in %ecx so gcc is smart enough to do that. So,
actually you get the same thing/or even better with variables in registers
instead of on stack and the code is more readable. A win-win
situation, I'd say :).
>>
>> I hope gcc is smart enough to do that.
>>
>
> Then where's the win?
Readability, of course. Also you have smaller, cleaner code. This is very
important, IMO.
---
¹gcc (Debian 4.3.3-3) 4.3.3
Copyright (C) 2008 Free Software Foundation, Inc.
--
Regards/Gruss,
Boris
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-11 16:32 ` Borislav Petkov
@ 2009-02-15 12:24 ` Sergei Shtylyov
2009-02-15 17:39 ` Borislav Petkov
0 siblings, 1 reply; 27+ messages in thread
From: Sergei Shtylyov @ 2009-02-15 12:24 UTC (permalink / raw)
To: petkovbb; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel
Hello.
Borislav Petkov wrote:
>>>>> or similar instead of wasting stack space?
>>>>>
>>>>>
>>>> It doesn't necessarily waste stack space. Haven't you heard about
>>>> compiler
>>>> putting local vairables into registers?
>>>>
>>>>
>>> Yes, have you heard of unnecessary register spilling?
>>>
>>>
>> No -- only about stack spilling on CPUs "caching" the top of stack in their
>> register file (like SPARC).
>> Linux runs not only on x86 and many RISCs can store several local variables
>> in the dedicated registers -- it's the part of say MIPS ABIs...
>>
Oh, it's really the same on x86, only there are only 3 registers
dedicated to that. RISCs typically have more, however x86_64 wiuth 16
its registers is close to that already. However, you're right -- these
registers need saving at the function entry, so they effectively take
the stack space.
>>>>> It'll also read better in the if() check:
>>>>>
>>>>> if (drv_can_irq_interrupt(drive)) { ...
>>>>>
>>>>>
>>>>>
>>>> It's faster to checj a local variable than to dereference drv several
>>>> times
>>>> -- unless gcc optimizes that away (by creating an implicit local variable
>>>> :-).
>>>>
>
>
> Let's look at an example:
>
> In ide-cd.c:cdrom_newpc_intr() you have the following code snippet:
>
> <ide-cd.c>
> 799 thislen = blk_fs_request(rq) ? len : rq->data_len;
> 800 if (thislen > len)
> 801 thislen = len;
> 802
> 803 ide_debug_log(IDE_DBG_PC, "%s: DRQ: stat: 0x%x, thislen: %d\n",
> 804 __func__, stat, thislen);
> 805
> 806 /* If DRQ is clear, the command has completed. */
> 807 if ((stat & ATA_DRQ) == 0) {
> 808 if (blk_fs_request(rq)) {
> </ide-cd.c>
>
> Now watch the blk_fs_request() thing.
>
> Here's what my gcc¹ spits out:
>
Thsi code is somewhat confused. Also, I was of a better opinion of gcc...
> <ide-cd.s>
> .LVL174:
> .loc 1 799 0
> movl 76(%r12), %ecx # <variable>.cmd_type, prephitmp.1128
> cmpl $1, %ecx #, prephitmp.1128
> movl %ecx, %r8d # prephitmp.1128, prephitmp.1047
> je .L225 #,
>
Now where is that label?
> .LVL175:
> .loc 1 800 0
> movzwl -44(%rbp), %r15d # len, thislen
>
Oh, that AT&T syntax... it took me a while to realize that it's a
movzx insn. :-)
> .LVL176:
> .loc 1 799 0
> movl 280(%r12), %edx # <variable>.data_len, thislen.1129
> .LVL177:
> .loc 1 800 0
> cmpl %r15d, %edx # thislen, thislen.1129
> movl %r15d, %ebx # thislen, thislen.1163
> jg .L145 #,
> .LVL178:
> movl %edx, %r15d # thislen.1129, thislen
>
I wonder why it doesn't generate cmovng ISO jg and mov...
> .LVL179:
> .L145:
> .loc 1 807 0
> testb $8, -64(%rbp) #, stat
> jne .L147 #,
> .LVL180:
> .loc 1 808 0
> cmpl $1, %ecx #, prephitmp.1128
> je .L226 #,
> .loc 1 825 0
> cmpl $2, %ecx #, prephitmp.1128
> .p2align 4,,3
> .p2align 3
> je .L152 #,
> .LBB408:
> </ide-cd.s>
>
> and at label .LVL174 you see the blk_fs_request() check from line
> 799 above. Later, at label .LVL180 you see the next blk_fs_request() check from
> line 808 and this is cached in %ecx so gcc is smart enough to do that. So,
>
Yes, CSE optimization does work...
> actually you get the same thing/or even better with variables in registers
> instead of on stack
I still don't undestand why you assume that such variable will be
alloceted on stack -- gcc has 3 registers available for local variables
(which doesn't have to save across function calls). However, the
register variables have to take stack space indeed as they need to be
saved on funciton entry... though I'm not sure that gcc will necessary
put such variable in one of those 3 registers if it figures out that
there are no function calls going to happen during its life time.
> and the code is more readable. A win-win situation, I'd say :).
>
You haven't presented the code which gets generated when the local
variable is used, so it's impossible to compare.
MBR, Sergei
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-15 12:24 ` Sergei Shtylyov
@ 2009-02-15 17:39 ` Borislav Petkov
2009-02-15 23:18 ` Sergei Shtylyov
0 siblings, 1 reply; 27+ messages in thread
From: Borislav Petkov @ 2009-02-15 17:39 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel
Hi,
> I still don't undestand why you assume that such variable will be
> alloceted on stack -- gcc has 3 registers available for local variables
> (which doesn't have to save across function calls). However, the
> register variables have to take stack space indeed as they need to be
> saved on funciton entry... though I'm not sure that gcc will necessary
> put such variable in one of those 3 registers if it figures out that
> there are no function calls going to happen during its life time.
>
>> and the code is more readable. A win-win situation, I'd say :).
>>
>
> You haven't presented the code which gets generated when the local
> variable is used, so it's impossible to compare.
Here's another example from ide-disk.c where you have stack variables cashing
those flags checks:
<ide-disk.c>
static ide_startstop_t __ide_do_rw_disk(ide_drive_t *drive, struct request *rq,
sector_t block)
{
ide_hwif_t *hwif = drive->hwif;
u16 nsectors = (u16)rq->nr_sectors;
u8 lba48 = !!(drive->dev_flags & IDE_DFLAG_LBA48);
u8 dma = !!(drive->dev_flags & IDE_DFLAG_USING_DMA);
ide_task_t task;
struct ide_taskfile *tf = &task.tf;
ide_startstop_t rc;
if ((hwif->host_flags & IDE_HFLAG_NO_LBA48_DMA) && lba48 && dma) {
if (block + rq->nr_sectors > 1ULL << 28)
dma = 0;
else
lba48 = 0;
}
</ide-disk.c>
Corresponding asm (this time i386 but I don't think it matters since we
need at least one arch to prove my point).
<ide-disk.s>
.LVL48:
.loc 1 94 0
movl %eax, %edx # D.32119, tmp93
.loc 1 95 0
shrl %eax # D.32119
andb $1, %al #,
movb %al, -58(%ebp) #, dma
.LVL49:
.loc 1 100 0
movl -52(%ebp), %eax # hwif,
.loc 1 94 0
shrl $21, %edx #, tmp93
andb $1, %dl #,
movb %dl, -57(%ebp) #, lba48
.LVL50:
.loc 1 100 0
testb $4, 90(%eax) #, <variable>.host_flags
je .L37 #,
testb %dl, %dl #
je .L37 #,
cmpb $0, -58(%ebp) # dma
movb $1, -57(%ebp) #, lba48
.LVL51:
</ide-disk.s>
Now look at the last lines at labels .LVL48 and .LVL49 - they both save
those 1-byte u8's called dma and lba48 on the stack at -57(%ebp) and
-58(%ebp), respectively. And guess what, later on label LVL50 they get
accessed in the check. And several times more later, which in most sane
architectures still means cache accesses but when you have registers its
even faster :).
--
Regards/Gruss,
Boris.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-15 17:39 ` Borislav Petkov
@ 2009-02-15 23:18 ` Sergei Shtylyov
2009-02-16 8:56 ` Borislav Petkov
0 siblings, 1 reply; 27+ messages in thread
From: Sergei Shtylyov @ 2009-02-15 23:18 UTC (permalink / raw)
To: petkovbb, Sergei Shtylyov, Bartlomiej Zolnierkiewicz, linux-ide,
linux-kernel
Hello.
Borislav Petkov wrote:
>> I still don't undestand why you assume that such variable will be
>> alloceted on stack -- gcc has 3 registers available for local variables
>> (which doesn't have to save across function calls). However, the
>> register variables have to take stack space indeed as they need to be
>> saved on funciton entry... though I'm not sure that gcc will necessary
>> put such variable in one of those 3 registers if it figures out that
>> there are no function calls going to happen during its life time.
>>
>>> and the code is more readable. A win-win situation, I'd say :).
>>>
>> You haven't presented the code which gets generated when the local
>> variable is used, so it's impossible to compare.
>>
>
> Here's another example from ide-disk.c where you have stack variables cashing
> those flags checks:
>
They are caching the result of !! in 'u8' variables -- which is not
the same as cahing the flags. I suspect gcc avoid putting byte-sized
variables into registers...
> <ide-disk.c>
> static ide_startstop_t __ide_do_rw_disk(ide_drive_t *drive, struct request *rq,
> sector_t block)
> {
> ide_hwif_t *hwif = drive->hwif;
> u16 nsectors = (u16)rq->nr_sectors;
> u8 lba48 = !!(drive->dev_flags & IDE_DFLAG_LBA48);
> u8 dma = !!(drive->dev_flags & IDE_DFLAG_USING_DMA);
> ide_task_t task;
> struct ide_taskfile *tf = &task.tf;
> ide_startstop_t rc;
>
> if ((hwif->host_flags & IDE_HFLAG_NO_LBA48_DMA) && lba48 && dma) {
> if (block + rq->nr_sectors > 1ULL << 28)
> dma = 0;
> else
> lba48 = 0;
> }
> </ide-disk.c>
>
> Corresponding asm (this time i386 but I don't think it matters since we
> need at least one arch to prove my point).
>
> <ide-disk.s>
> .LVL48:
> .loc 1 94 0
> movl %eax, %edx # D.32119, tmp93
> .loc 1 95 0
> shrl %eax # D.32119
>
Where's the shift count I wonder?
> andb $1, %al #,
> movb %al, -58(%ebp) #, dma
> .LVL49:
> .loc 1 100 0
> movl -52(%ebp), %eax # hwif,
> .loc 1 94 0
> shrl $21, %edx #, tmp93
> andb $1, %dl #,
> movb %dl, -57(%ebp) #, lba48
> .LVL50:
> .loc 1 100 0
> testb $4, 90(%eax) #, <variable>.host_flags
> je .L37 #,
> testb %dl, %dl #
> je .L37 #,
> cmpb $0, -58(%ebp) # dma
> movb $1, -57(%ebp) #, lba48
> .LVL51:
> </ide-disk.s>
>
> Now look at the last lines at labels .LVL48 and .LVL49 - they both save
> those 1-byte u8's called dma and lba48 on the stack at -57(%ebp) and
> -58(%ebp), respectively. And guess what, later on label LVL50 they get
> accessed in the check.
If you look better, you'll see that the copy of 'lba48' in the %dl
register gets used.
> And several times more later, which in most sane
> architectures still means cache accesses but when you have registers its
> even faster :).
Didn't quite get that statement.
Well, this example wasn't very convincing...
MBR. Sergei
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-15 23:18 ` Sergei Shtylyov
@ 2009-02-16 8:56 ` Borislav Petkov
0 siblings, 0 replies; 27+ messages in thread
From: Borislav Petkov @ 2009-02-16 8:56 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel
>
> Didn't quite get that statement.
> Well, this example wasn't very convincing...
Oh, I'm really getting tired of this!
Here's your final example:
A stack variable caching the flags check. REMEMBER, no write accesses to
it!:
<ide-cd.c>
int ide_cd_read_toc(ide_drive_t *drive, struct request_sense *sense)
{
int stat, ntracks, i;
struct cdrom_info *info = drive->driver_data;
struct cdrom_device_info *cdi = &info->devinfo;
struct atapi_toc *toc = info->toc;
struct {
struct atapi_toc_header hdr;
struct atapi_toc_entry ent;
} ms_tmp;
long last_written;
unsigned long sectors_per_frame = SECTORS_PER_FRAME;
u8 tmp = drive->atapi_flags & IDE_AFLAG_TOCTRACKS_AS_BCD;
</ide-cd.c>
later you access the _same_ variable in an if-check:
<ide-cd.c>
if (tmp) {
toc->hdr.first_track = bcd2bin(toc->hdr.first_track);
toc->hdr.last_track = bcd2bin(toc->hdr.last_track);
}
</ide-cd.c>
Resulting assembly snippets:
<ide-cd.s>
.LCFI166:
.loc 1 1202 0
movl %edx, -52(%ebp) # sense, sense
.loc 1 1213 0
movl 500(%edi), %ecx # <variable>.atapi_flags,
.loc 1 1204 0
movl 24(%eax), %eax # <variable>.driver_data,
.LVL399:
movl %eax, -40(%ebp) #, info
.LVL400:
.loc 1 1206 0
movl 16(%eax), %esi # <variable>.toc, toc
.LVL401:
.loc 1 1212 0
movl $4, -20(%ebp) #, sectors_per_frame
.LVL402:
.loc 1 1213 0
movl %ecx, -48(%ebp) #, D.31862
</ide-cd.s>
you see here the ->atapi_flags, dereferenced above, being saved on the
stack. Here's the asm for the check later:
<ide-cd.s>
.LVL415:
jne .L283 #,
.loc 1 1256 0
movb -48(%ebp), %al # D.31862,
.LVL416:
andb $16, %al #,
movb %al, -33(%ebp) #, tmp
.LVL417:
je .L287 #,
.loc 1 1257 0
</ide-cd.s>
So, the ->atapi_flags is moved into 8bit %al from the stack! Then the
$16 is the IDE_AFLAG_TOCTRACKS_AS_BCD, so the binary & against the
->atapi_flags is done actually here. Then the tmp variable is SAVED ON
THE STACK at -33(%ebp) and it is dereferenced AGAIN in the next check:
.LVL426:
jne .L283 #,
.loc 1 1297 0
cmpb $0, -33(%ebp) # tmp
je .L290 #
If you still don't see it, go play with your own assembler and do some
code staring on your own which doesn't involve me!
--
Regards/Gruss,
Boris.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()
2009-02-11 13:49 ` Sergei Shtylyov
2009-02-11 16:32 ` Borislav Petkov
@ 2009-02-11 16:37 ` Bartlomiej Zolnierkiewicz
1 sibling, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-11 16:37 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: petkovbb, linux-ide, linux-kernel
On Wednesday 11 February 2009, Sergei Shtylyov wrote:
> Hello.
>
> Borislav Petkov wrote:
>
> >>>> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >>>> Subject: [PATCH] ide: remove ide_execute_pkt_cmd()
> >>>>
> >>>> * Pass command structure to ide_execute_command() and skip
> >>>> __ide_set_handler() for ATAPI protocols.
> >>>>
> >>>> * Convert ide_issue_pc() to always use ide_execute_command()
> >>>> and remove no longer needed ide_execute_pkt_cmd().
> >>>>
> >>>> There should be no functional changes caused by this patch.
> >>>>
> >>>> Cc: Borislav Petkov <petkovbb@gmail.com>
> >>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >>>> ---
> >>>> drivers/ide/ide-atapi.c | 15 +++++++--------
> >>>> drivers/ide/ide-iops.c | 23 ++++++-----------------
> >>>> drivers/ide/ide-taskfile.c | 6 ++----
> >>>> include/linux/ide.h | 5 ++---
> >>>> 4 files changed, 17 insertions(+), 32 deletions(-)
> >>>>
> >>>> Index: b/drivers/ide/ide-atapi.c
> >>>> ===================================================================
> >>>> --- a/drivers/ide/ide-atapi.c
> >>>> +++ b/drivers/ide/ide-atapi.c
> >>>> @@ -481,6 +481,7 @@ static void ide_init_packet_cmd(struct i
> >>>> cmd->protocol = dma ? ATAPI_PROT_DMA : ATAPI_PROT_PIO;
> >>>> cmd->tf_flags |= IDE_TFLAG_OUT_LBAH | IDE_TFLAG_OUT_LBAM |
> >>>> IDE_TFLAG_OUT_FEATURE | tf_flags;
> >>>> + cmd->tf.command = ATA_CMD_PACKET;
> >>>> cmd->tf.feature = dma; /* Use PIO/DMA */
> >>>> cmd->tf.lbam = bcount & 0xff;
> >>>> cmd->tf.lbah = (bcount >> 8) & 0xff;
> >>>> @@ -626,6 +627,7 @@ ide_startstop_t ide_issue_pc(ide_drive_t
> >>>> unsigned int timeout;
> >>>> u32 tf_flags;
> >>>> u16 bcount;
> >>>> + u8 drq_int = !!(drive->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT);
> >>>>
> >>>>
> >>> How about we finally add those check macros in block layer fashion like
> >>> blk_pc_request et al and do
> >>>
> >>> #define drv_can_drq_interrupt(drive) ((drive)->atapi_flags &
> >>> IDE_AFLAG_DRQ_INTERRUPT)
> >>>
> >>>
> >> I suppose it's for the devices that interrupt on packet DRQ? Then it's
> >> hardly a good name because it's not like this is some optional capability.
> >>
> >
> > No, I was alluding to the command packet DRQ type used by the device as it is
> > put in SFF8020i, 7.1.7.1 General Configuration Word.
> >
>
> I was talking about exactly the same feature. :-)
>
> >>> or similar instead of wasting stack space?
> >>>
> >> It doesn't necessarily waste stack space. Haven't you heard about compiler
> >> putting local vairables into registers?
> >>
> >
> > Yes, have you heard of unnecessary register spilling?
> >
>
> No -- only about stack spilling on CPUs "caching" the top of stack in
> their register file (like SPARC).
> Linux runs not only on x86 and many RISCs can store several local
> variables in the dedicated registers -- it's the part of say MIPS ABIs...
>
> >>> It'll also read better in the if() check:
> >>>
> >>> if (drv_can_irq_interrupt(drive)) { ...
> >>>
> >>>
> >> It's faster to checj a local variable than to dereference drv several times
> >> -- unless gcc optimizes that away (by creating an implicit local variable
> >> :-).
> >>
> >
> > I hope gcc is smart enough to do that.
> >
>
> Then where's the win?
In having shorter & cleaner code:
ide_dev_drq_int(drive)
vs
((drive)->atapi_flags & IDE_AFLAG_DRQ_INTERRUPT)
We can of course have both things -- using macros for all checks
but still cache the result when it makes sense.
The idea of having macros for ->dev_flags and ->atapi_flags sounds
fine to me (+ it abstracts the actual ->dev_flags + ->atapi_flags
split allowing easier sanitizations / enhancements later).
Thanks,
Bart
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 6/6] ide: keep track of number of bytes instead of sectors in struct ide_cmd
2009-02-09 23:19 [PATCH 0/6] ide: more unifications of ATA and ATAPI support Bartlomiej Zolnierkiewicz
` (4 preceding siblings ...)
2009-02-09 23:20 ` [PATCH 5/6] ide: remove ide_execute_pkt_cmd() Bartlomiej Zolnierkiewicz
@ 2009-02-09 23:20 ` Bartlomiej Zolnierkiewicz
2009-02-11 7:16 ` [PATCH 0/6] ide: more unifications of ATA and ATAPI support Borislav Petkov
6 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-09 23:20 UTC (permalink / raw)
To: linux-ide; +Cc: Borislav Petkov, Bartlomiej Zolnierkiewicz, linux-kernel
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: keep track of number of bytes instead of sectors in struct ide_cmd
* Pass number of bytes instead of sectors to ide_init_sg_cmd().
* Pass number of bytes to process to ide_pio_sector() and rename
it to ide_pio_bytes().
* Rename ->nsect field to ->nbytes in struct ide_cmd and use
->nbytes, ->nleft and ->cursg_ofs to keep track of number of
bytes instead of sectors.
There should be no functional changes caused by this patch.
Cc: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-disk.c | 4 ++--
drivers/ide/ide-floppy.c | 2 +-
drivers/ide/ide-io.c | 6 +++---
drivers/ide/ide-taskfile.c | 32 ++++++++++++++++----------------
include/linux/ide.h | 4 ++--
5 files changed, 24 insertions(+), 24 deletions(-)
Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -152,7 +152,7 @@ static ide_startstop_t __ide_do_rw_disk(
cmd.rq = rq;
if (dma == 0) {
- ide_init_sg_cmd(&cmd, nsectors);
+ ide_init_sg_cmd(&cmd, nsectors << 9);
ide_map_sg(drive, &cmd);
}
@@ -162,7 +162,7 @@ static ide_startstop_t __ide_do_rw_disk(
/* fallback to PIO */
cmd.tf_flags |= IDE_TFLAG_DMA_PIO_FALLBACK;
ide_tf_set_cmd(drive, &cmd, 0);
- ide_init_sg_cmd(&cmd, nsectors);
+ ide_init_sg_cmd(&cmd, nsectors << 9);
rc = do_rw_taskfile(drive, &cmd);
}
Index: b/drivers/ide/ide-floppy.c
===================================================================
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -293,7 +293,7 @@ static ide_startstop_t ide_floppy_do_req
cmd.rq = rq;
- ide_init_sg_cmd(&cmd, rq->nr_sectors);
+ ide_init_sg_cmd(&cmd, rq->nr_sectors << 9);
ide_map_sg(drive, &cmd);
pc->sg = hwif->sg_table;
Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -241,9 +241,9 @@ void ide_map_sg(ide_drive_t *drive, stru
}
EXPORT_SYMBOL_GPL(ide_map_sg);
-void ide_init_sg_cmd(struct ide_cmd *cmd, int nsect)
+void ide_init_sg_cmd(struct ide_cmd *cmd, unsigned int nr_bytes)
{
- cmd->nsect = cmd->nleft = nsect;
+ cmd->nbytes = cmd->nleft = nr_bytes;
cmd->cursg_ofs = 0;
cmd->cursg = NULL;
}
@@ -268,7 +268,7 @@ static ide_startstop_t execute_drive_cmd
if (cmd) {
if (cmd->protocol == ATA_PROT_PIO) {
- ide_init_sg_cmd(cmd, rq->nr_sectors);
+ ide_init_sg_cmd(cmd, rq->nr_sectors << 9);
ide_map_sg(drive, cmd);
}
Index: b/drivers/ide/ide-taskfile.c
===================================================================
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -188,8 +188,8 @@ static u8 wait_drive_not_busy(ide_drive_
return stat;
}
-static void ide_pio_sector(ide_drive_t *drive, struct ide_cmd *cmd,
- unsigned int write)
+static void ide_pio_bytes(ide_drive_t *drive, struct ide_cmd *cmd,
+ unsigned int write, unsigned int nr_bytes)
{
ide_hwif_t *hwif = drive->hwif;
struct scatterlist *sg = hwif->sg_table;
@@ -208,7 +208,7 @@ static void ide_pio_sector(ide_drive_t *
}
page = sg_page(cursg);
- offset = cursg->offset + cmd->cursg_ofs * SECTOR_SIZE;
+ offset = cursg->offset + cmd->cursg_ofs;
/* get the current page and offset */
page = nth_page(page, (offset >> PAGE_SHIFT));
@@ -219,19 +219,19 @@ static void ide_pio_sector(ide_drive_t *
#endif
buf = kmap_atomic(page, KM_BIO_SRC_IRQ) + offset;
- cmd->nleft--;
- cmd->cursg_ofs++;
+ cmd->nleft -= nr_bytes;
+ cmd->cursg_ofs += nr_bytes;
- if ((cmd->cursg_ofs * SECTOR_SIZE) == cursg->length) {
+ if (cmd->cursg_ofs == cursg->length) {
cmd->cursg = sg_next(cmd->cursg);
cmd->cursg_ofs = 0;
}
/* do the actual data transfer */
if (write)
- hwif->tp_ops->output_data(drive, cmd, buf, SECTOR_SIZE);
+ hwif->tp_ops->output_data(drive, cmd, buf, nr_bytes);
else
- hwif->tp_ops->input_data(drive, cmd, buf, SECTOR_SIZE);
+ hwif->tp_ops->input_data(drive, cmd, buf, nr_bytes);
kunmap_atomic(buf, KM_BIO_SRC_IRQ);
#ifdef CONFIG_HIGHMEM
@@ -244,9 +244,9 @@ static void ide_pio_multi(ide_drive_t *d
{
unsigned int nsect;
- nsect = min_t(unsigned int, cmd->nleft, drive->mult_count);
+ nsect = min_t(unsigned int, cmd->nleft >> 9, drive->mult_count);
while (nsect--)
- ide_pio_sector(drive, cmd, write);
+ ide_pio_bytes(drive, cmd, write, SECTOR_SIZE);
}
static void ide_pio_datablock(ide_drive_t *drive, struct ide_cmd *cmd,
@@ -265,7 +265,7 @@ static void ide_pio_datablock(ide_drive_
if (cmd->tf_flags & IDE_TFLAG_MULTI_PIO)
ide_pio_multi(drive, cmd, write);
else
- ide_pio_sector(drive, cmd, write);
+ ide_pio_bytes(drive, cmd, write, SECTOR_SIZE);
drive->io_32bit = saved_io_32bit;
}
@@ -273,18 +273,18 @@ static void ide_pio_datablock(ide_drive_
static void ide_error_cmd(ide_drive_t *drive, struct ide_cmd *cmd)
{
if (cmd->tf_flags & IDE_TFLAG_FS) {
- int sectors = cmd->nsect - cmd->nleft;
+ int nr_bytes = cmd->nbytes - cmd->nleft;
if (cmd->protocol == ATA_PROT_PIO &&
((cmd->tf_flags & IDE_TFLAG_WRITE) || cmd->nleft == 0)) {
if (cmd->tf_flags & IDE_TFLAG_MULTI_PIO)
- sectors -= drive->mult_count;
+ nr_bytes -= drive->mult_count << 9;
else
- sectors--;
+ nr_bytes -= SECTOR_SIZE;
}
- if (sectors > 0)
- ide_complete_rq(drive, 0, sectors << 9);
+ if (nr_bytes > 0)
+ ide_complete_rq(drive, 0, nr_bytes);
}
}
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -349,7 +349,7 @@ struct ide_cmd {
int sg_nents; /* number of sg entries */
int sg_dma_direction; /* DMA transfer direction */
- unsigned int nsect;
+ unsigned int nbytes;
unsigned int nleft;
struct scatterlist *cursg;
unsigned int cursg_ofs;
@@ -1406,7 +1406,7 @@ int ide_pci_resume(struct pci_dev *);
#endif
void ide_map_sg(ide_drive_t *, struct ide_cmd *);
-void ide_init_sg_cmd(struct ide_cmd *, int);
+void ide_init_sg_cmd(struct ide_cmd *, unsigned int);
#define BAD_DMA_DRIVE 0
#define GOOD_DMA_DRIVE 1
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 0/6] ide: more unifications of ATA and ATAPI support
2009-02-09 23:19 [PATCH 0/6] ide: more unifications of ATA and ATAPI support Bartlomiej Zolnierkiewicz
` (5 preceding siblings ...)
2009-02-09 23:20 ` [PATCH 6/6] ide: keep track of number of bytes instead of sectors in struct ide_cmd Bartlomiej Zolnierkiewicz
@ 2009-02-11 7:16 ` Borislav Petkov
2009-02-23 22:51 ` Bartlomiej Zolnierkiewicz
6 siblings, 1 reply; 27+ messages in thread
From: Borislav Petkov @ 2009-02-11 7:16 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel
On Tue, Feb 10, 2009 at 12:19:45AM +0100, Bartlomiej Zolnierkiewicz wrote:
>
> After this patchset we have a valid struct ide_cmd available also for
> ATA_CMD_PACKET commands and comparing struct ide_atapi_pc with ide_cmd it
> seems that there are many similarities between them and that we may just
> merge both structs (this should also allow us to unify ide-cd code with
> non-ide-cd one in ide-atapi.c later). From the quick look the only gotcha
> is REQUEST SENSE handling, ->request_sense_pc needs to be converted to
> ->request_sense_cmd and we have to be careful with choosing right 'cmd'
> in *_issue_pc()...
>
> Borislav, please take a look and tell me what do you think about it
> (also feel free to go ahead with patches :-)...
Yep, like it, especially the idea of a unified ide_cmd thingy and
removing the atapi_pc :).
ACK.
--
Regards/Gruss,
Boris.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 0/6] ide: more unifications of ATA and ATAPI support
2009-02-11 7:16 ` [PATCH 0/6] ide: more unifications of ATA and ATAPI support Borislav Petkov
@ 2009-02-23 22:51 ` Bartlomiej Zolnierkiewicz
0 siblings, 0 replies; 27+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2009-02-23 22:51 UTC (permalink / raw)
To: petkovbb; +Cc: linux-ide, linux-kernel
On Wednesday 11 February 2009, Borislav Petkov wrote:
> On Tue, Feb 10, 2009 at 12:19:45AM +0100, Bartlomiej Zolnierkiewicz wrote:
> >
> > After this patchset we have a valid struct ide_cmd available also for
> > ATA_CMD_PACKET commands and comparing struct ide_atapi_pc with ide_cmd it
> > seems that there are many similarities between them and that we may just
> > merge both structs (this should also allow us to unify ide-cd code with
> > non-ide-cd one in ide-atapi.c later). From the quick look the only gotcha
There is another gotcha there and this one is worth fixing indpendently of
struct ide_atapi_pc and struct ide_cmd merge:
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ide: decrease size of ->pc_buf field in struct ide_atapi_pc
struct ide_atapi_pc is often allocated on the stack and size of ->pc_buf
size is 256 bytes. However since only ide_floppy_create_read_capacity_cmd()
and idetape_create_inquiry_cmd() require such size allocate buffers for
these pc-s explicitely and decrease ->pc_buf size to 64 bytes.
Cc: Borislav Petkov <petkovbb@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ide/ide-floppy.c | 5 ++++-
drivers/ide/ide-floppy_ioctl.c | 5 ++++-
drivers/ide/ide-tape.c | 4 ++++
include/linux/ide.h | 2 +-
4 files changed, 13 insertions(+), 3 deletions(-)
Index: b/drivers/ide/ide-floppy.c
===================================================================
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -377,7 +377,7 @@ static int ide_floppy_get_capacity(ide_d
struct gendisk *disk = floppy->disk;
struct ide_atapi_pc pc;
u8 *cap_desc;
- u8 header_len, desc_cnt;
+ u8 pc_buf[256], header_len, desc_cnt;
int i, rc = 1, blocks, length;
drive->bios_cyl = 0;
@@ -387,6 +387,9 @@ static int ide_floppy_get_capacity(ide_d
drive->capacity64 = 0;
ide_floppy_create_read_capacity_cmd(&pc);
+ pc.buf = &pc_buf[0];
+ pc.buf_size = sizeof(pc_buf);
+
if (ide_queue_pc_tail(drive, disk, &pc)) {
printk(KERN_ERR PFX "Can't get floppy parameters\n");
return 1;
Index: b/drivers/ide/ide-floppy_ioctl.c
===================================================================
--- a/drivers/ide/ide-floppy_ioctl.c
+++ b/drivers/ide/ide-floppy_ioctl.c
@@ -36,9 +36,9 @@ static int ide_floppy_get_format_capacit
int __user *arg)
{
struct ide_disk_obj *floppy = drive->driver_data;
- u8 header_len, desc_cnt;
int i, blocks, length, u_array_size, u_index;
int __user *argp;
+ u8 pc_buf[256], header_len, desc_cnt;
if (get_user(u_array_size, arg))
return -EFAULT;
@@ -47,6 +47,9 @@ static int ide_floppy_get_format_capacit
return -EINVAL;
ide_floppy_create_read_capacity_cmd(pc);
+ pc->buf = &pc_buf[0];
+ pc->buf_size = sizeof(pc_buf);
+
if (ide_queue_pc_tail(drive, floppy->disk, pc)) {
printk(KERN_ERR "ide-floppy: Can't get floppy parameters\n");
return -EIO;
Index: b/drivers/ide/ide-tape.c
===================================================================
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -2014,9 +2014,13 @@ static void idetape_get_inquiry_results(
{
idetape_tape_t *tape = drive->driver_data;
struct ide_atapi_pc pc;
+ u8 pc_buf[256];
char fw_rev[4], vendor_id[8], product_id[16];
idetape_create_inquiry_cmd(&pc);
+ pc.buf = &pc_buf[0];
+ pc.buf_size = sizeof(pc_buf);
+
if (ide_queue_pc_tail(drive, tape->disk, &pc)) {
printk(KERN_ERR "ide-tape: %s: can't get INQUIRY results\n",
tape->name);
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -377,7 +377,7 @@ enum {
* With each packet command, we allocate a buffer of IDE_PC_BUFFER_SIZE bytes.
* This is used for several packet commands (not for READ/WRITE commands).
*/
-#define IDE_PC_BUFFER_SIZE 256
+#define IDE_PC_BUFFER_SIZE 64
#define ATAPI_WAIT_PC (60 * HZ)
struct ide_atapi_pc {
^ permalink raw reply [flat|nested] 27+ messages in thread