Re: [PATCH 5/6] ide: remove ide_execute_pkt_cmd()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

Borislav Petkov 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)

 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


--
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

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux