Skip to content

Commit

Permalink
hpsa: get rid of type/attribute/direction bit field where possible
Browse files Browse the repository at this point in the history 
Using bit fields for hardware command fields isn't portable and
relies on assumptions about how the compiler lays out the bits.
We can fix this in the driver's internal command structure, but the
ioctl interface we can't change because it is part of the
userland ABI.

Signed-off-by: Don Brace <don.brace@pmcs.com>
Reviewed-by: Webb Scales <webb.scales@hp.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
  • Loading branch information
Stephen M. Cameron authored and Christoph Hellwig committed Nov 20, 2014
1 parent 50a0dec commit a505b86
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 34 deletions.
58 changes: 29 additions & 29 deletions drivers/scsi/hpsa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4019,25 +4019,27 @@ static int hpsa_scsi_queue_command_lck(struct scsi_cmnd *cmd,
BUG_ON(cmd->cmd_len > sizeof(c->Request.CDB));
c->Request.CDBLen = cmd->cmd_len;
memcpy(c->Request.CDB, cmd->cmnd, cmd->cmd_len);
c->Request.Type.Type = TYPE_CMD;
c->Request.Type.Attribute = ATTR_SIMPLE;
switch (cmd->sc_data_direction) {
case DMA_TO_DEVICE:
c->Request.Type.Direction = XFER_WRITE;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_WRITE);
break;
case DMA_FROM_DEVICE:
c->Request.Type.Direction = XFER_READ;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_READ);
break;
case DMA_NONE:
c->Request.Type.Direction = XFER_NONE;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_NONE);
break;
case DMA_BIDIRECTIONAL:
/* This can happen if a buggy application does a scsi passthru
* and sets both inlen and outlen to non-zero. ( see
* ../scsi/scsi_ioctl.c:scsi_ioctl_send_command() )
*/

c->Request.Type.Direction = XFER_RSVD;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(TYPE_CMD, ATTR_SIMPLE, XFER_RSVD);
/* This is technically wrong, and hpsa controllers should
* reject it with CMD_INVALID, which is the most correct
* response, but non-fibre backends appear to let it
Expand Down Expand Up @@ -5257,7 +5259,6 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
c->Header.tag = c->busaddr;
memcpy(c->Header.LUN.LunAddrBytes, scsi3addr, 8);

c->Request.Type.Type = cmd_type;
if (cmd_type == TYPE_CMD) {
switch (cmd) {
case HPSA_INQUIRY:
Expand All @@ -5267,8 +5268,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
c->Request.CDB[2] = (page_code & 0xff);
}
c->Request.CDBLen = 6;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_READ;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
c->Request.Timeout = 0;
c->Request.CDB[0] = HPSA_INQUIRY;
c->Request.CDB[4] = size & 0xFF;
Expand All @@ -5279,8 +5280,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
mode = 00 target = 0. Nothing to write.
*/
c->Request.CDBLen = 12;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_READ;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
c->Request.Timeout = 0;
c->Request.CDB[0] = cmd;
c->Request.CDB[6] = (size >> 24) & 0xFF; /* MSB */
Expand All @@ -5290,8 +5291,9 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
break;
case HPSA_CACHE_FLUSH:
c->Request.CDBLen = 12;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_WRITE;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(cmd_type,
ATTR_SIMPLE, XFER_WRITE);
c->Request.Timeout = 0;
c->Request.CDB[0] = BMIC_WRITE;
c->Request.CDB[6] = BMIC_CACHE_FLUSH;
Expand All @@ -5300,14 +5302,14 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
break;
case TEST_UNIT_READY:
c->Request.CDBLen = 6;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_NONE;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_NONE);
c->Request.Timeout = 0;
break;
case HPSA_GET_RAID_MAP:
c->Request.CDBLen = 12;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_READ;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
c->Request.Timeout = 0;
c->Request.CDB[0] = HPSA_CISS_READ;
c->Request.CDB[1] = cmd;
Expand All @@ -5318,8 +5320,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
break;
case BMIC_SENSE_CONTROLLER_PARAMETERS:
c->Request.CDBLen = 10;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_READ;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_READ);
c->Request.Timeout = 0;
c->Request.CDB[0] = BMIC_READ;
c->Request.CDB[6] = BMIC_SENSE_CONTROLLER_PARAMETERS;
Expand All @@ -5336,9 +5338,8 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,

case HPSA_DEVICE_RESET_MSG:
c->Request.CDBLen = 16;
c->Request.Type.Type = 1; /* It is a MSG not a CMD */
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_NONE;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(cmd_type, ATTR_SIMPLE, XFER_NONE);
c->Request.Timeout = 0; /* Don't time out */
memset(&c->Request.CDB[0], 0, sizeof(c->Request.CDB));
c->Request.CDB[0] = cmd;
Expand All @@ -5357,9 +5358,9 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
tlower = (u32) (a->Header.tag >> 32);
tupper = (u32) (a->Header.tag & 0x0ffffffffULL);
c->Request.CDBLen = 16;
c->Request.Type.Type = TYPE_MSG;
c->Request.Type.Attribute = ATTR_SIMPLE;
c->Request.Type.Direction = XFER_WRITE;
c->Request.type_attr_dir =
TYPE_ATTR_DIR(cmd_type,
ATTR_SIMPLE, XFER_WRITE);
c->Request.Timeout = 0; /* Don't time out */
c->Request.CDB[0] = HPSA_TASK_MANAGEMENT;
c->Request.CDB[1] = HPSA_TMF_ABORT_TASK;
Expand Down Expand Up @@ -5389,7 +5390,7 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
BUG();
}

switch (c->Request.Type.Direction) {
switch (GET_DIR(c->Request.type_attr_dir)) {
case XFER_READ:
pci_dir = PCI_DMA_FROMDEVICE;
break;
Expand Down Expand Up @@ -5747,9 +5748,8 @@ static int hpsa_message(struct pci_dev *pdev, unsigned char opcode,
memset(&cmd->CommandHeader.LUN.LunAddrBytes, 0, 8);

cmd->Request.CDBLen = 16;
cmd->Request.Type.Type = TYPE_MSG;
cmd->Request.Type.Attribute = ATTR_HEADOFQUEUE;
cmd->Request.Type.Direction = XFER_NONE;
cmd->Request.type_attr_dir =
TYPE_ATTR_DIR(TYPE_MSG, ATTR_HEADOFQUEUE, XFER_NONE);
cmd->Request.Timeout = 0; /* Don't time out */
cmd->Request.CDB[0] = opcode;
cmd->Request.CDB[1] = type;
Expand Down
18 changes: 13 additions & 5 deletions drivers/scsi/hpsa_cmd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,19 @@ struct CommandListHeader {

struct RequestBlock {
u8 CDBLen;
struct {
u8 Type:3;
u8 Attribute:3;
u8 Direction:2;
} Type;
/*
* type_attr_dir:
* type: low 3 bits
* attr: middle 3 bits
* dir: high 2 bits
*/
u8 type_attr_dir;
#define TYPE_ATTR_DIR(t, a, d) ((((d) & 0x03) << 6) |\
(((a) & 0x07) << 3) |\
((t) & 0x07))
#define GET_TYPE(tad) ((tad) & 0x07)
#define GET_ATTR(tad) (((tad) >> 3) & 0x07)
#define GET_DIR(tad) (((tad) >> 6) & 0x03)
u16 Timeout;
u8 CDB[16];
};
Expand Down

0 comments on commit a505b86

Please sign in to comment.