On Tue, Feb 10, 2009 at 12:20:19AM +0100, Bartlomiej Zolnierkiewicz wrote:
> From: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> 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@xxxxxxxxx>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> ---
> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
