hpsa updates

pulling the rug out from under the reset handler
likewise for ioaccel_cmds_out

Signed-off-by Don Brace <[email protected]>

pulling the rug out from under the reset handler
likewise for ioaccel_cmds_out

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |    2 --
 1 file changed, 2 deletions(-)

drivers/scsi/hpsa.c +0 -0

index 40669f8..e60c652 100644

This function is no longer used.

Signed-off-by Don Brace <[email protected]>

Don: See comment below.

- Manoj Kumar

drivers/scsi/hpsa.c static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h, · line 233

Quoted 4 lines · Don Brace

This function is no longer used.

drivers/scsi/hpsa.c +1 -0

+#define HPSA_PERF_ERROR_BITS ((1 << DIRECT_LOOKUP_SHIFT) - 1)

HPSA_PERF_ERROR_BITS seems to be only used in the function that was removed. Is there a reason to redefine this?

This function is no longer used.

Signed-off-by Don Brace <[email protected]>

---

Check for NULLs.

Check for NULLs.

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |    6 ++++++
 drivers/scsi/hpsa.h |    2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

drivers/scsi/hpsa.c +6 -0

index 864fb03..6b6e9bb 100644
@@ -1139,6 +1139,12 @@ static int hpsa_find_target_lun(struct ctlr_info *h,
 static inline void hpsa_show_dev_msg(const char *level, struct ctlr_info *h,
 	struct hpsa_scsi_dev_t *dev, char *description)
 {
+	if (dev == NULL)
+		return;
+
+	if (h == NULL || h->pdev == NULL || h->scsi_host == NULL)
+		return;
+
 	dev_printk(level, &h->pdev->dev,
 			"scsi %d:%d:%d:%d: %s %s %.8s %.16s RAID-%s SSDSmartPathCap%c En%c Exp=%d\n",
 			h->scsi_host->host_no, dev->bus, dev->target, dev->lun,

drivers/scsi/hpsa.h +0 -0

index 27debb3..d6c4ebf 100644

Quoted 31 lines · Don Brace

Check for NULLs.

Signed-off-by Don Brace <[email protected]>

---
  drivers/scsi/hpsa.c |    6 ++++++
  drivers/scsi/hpsa.h |    2 +-
  2 files changed, 7 insertions(+), 1 deletion(-)

drivers/scsi/hpsa.c +6 -0

index 864fb03..6b6e9bb 100644
@@ -1139,6 +1139,12 @@ static int hpsa_find_target_lun(struct ctlr_info *h,
  static inline void hpsa_show_dev_msg(const char *level, struct ctlr_info *h,
  	struct hpsa_scsi_dev_t *dev, char *description)
  {
+	if (dev == NULL)
+		return;
+
+	if (h == NULL || h->pdev == NULL || h->scsi_host == NULL)
+		return;
+
  	dev_printk(level, &h->pdev->dev,
  			"scsi %d:%d:%d:%d: %s %s %.8s %.16s RAID-%s SSDSmartPathCap%c En%c Exp=%d\n",
  			h->scsi_host->host_no, dev->bus, dev->target, dev->lun,

drivers/scsi/hpsa.h +0 -0

index 27debb3..d6c4ebf 100644

drivers/scsi/hpsa.h struct access_method { · line 40

Quoted 8 lines

drivers/scsi/hpsa.h +1 -1

@@ -34,7 +34,7 @@ struct access_method {
  };
 
  struct hpsa_scsi_dev_t {
-	int devtype;
+	unsigned int devtype;
  	int bus, target, lun;		/* as presented to the OS */
  	unsigned char scsi3addr[8];	/* as presented to the HW */
  #define RAID_CTLR_LUNID "\0\0\0\0\0\0\0\0"

That's not really a check for NULL, isn't it?
Should rather be moved into an individual patch.

Cheers,

Hannes

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

drivers/scsi/hpsa.c +4 -1

index 6b6e9bb..3fe8a18 100644
@@ -1136,7 +1136,7 @@ static int hpsa_find_target_lun(struct ctlr_info *h,
 	return !found;
 }
 
-static inline void hpsa_show_dev_msg(const char *level, struct ctlr_info *h,
+static void hpsa_show_dev_msg(const char *level, struct ctlr_info *h,
 	struct hpsa_scsi_dev_t *dev, char *description)
 {
 	if (dev == NULL)

@@ -1423,6 +1423,9 @@ static int hpsa_scsi_find_entry(struct hpsa_scsi_dev_t *needle,
 #define DEVICE_CHANGED 1
 #define DEVICE_SAME 2
 #define DEVICE_UPDATED 3
+	if (needle == NULL)
+		return DEVICE_NOT_FOUND;
+
 	for (i = 0; i < haystack_size; i++) {
 		if (haystack[i] == NULL) /* previously removed. */
 			continue;

drivers/scsi/hpsa.c +14 -5

+
 	if (nraid_map_entries > RAID_MAP_MAX_ENTRIES)
 		nraid_map_entries = RAID_MAP_MAX_ENTRIES;
 

@@ -1585,6 +1591,8 @@ static void hpsa_figure_phys_disk_ptrs(struct ctlr_info *h,
 		if (!logical_drive->offload_config)
 			continue;
 		for (j = 0; j < ndevices; j++) {
+			if (dev[j] == NULL)
+				continue;
 			if (dev[j]->devtype != TYPE_DISK)
 				continue;
 			if (is_logical_dev_addr_mode(dev[j]->scsi3addr))

@@ -1628,6 +1636,8 @@ static void hpsa_update_log_drive_phys_drive_ptrs(struct ctlr_info *h,
 	int i;
 
 	for (i = 0; i < ndevices; i++) {
+		if (dev[i] == NULL)
+			continue;
 		if (dev[i]->devtype != TYPE_DISK)
 			continue;
 		if (!is_logical_dev_addr_mode(dev[i]->scsi3addr))

@@ -1743,8 +1753,11 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
 	/* Now that h->dev[]->phys_disk[] is coherent, we can enable
 	 * any logical drives that need it enabled.
 	 */
-	for (i = 0; i < h->ndevices; i++)
+	for (i = 0; i < h->ndevices; i++) {
+		if (h->dev[i] == NULL)
+			continue;
 		h->dev[i]->offload_enabled = h->dev[i]->offload_to_be_enabled;
+	}
 
 	spin_unlock_irqrestore(&h->devlock, flags);
 

@@ -1769,6 +1782,8 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
 	sh = h->scsi_host;
 	/* Notify scsi mid layer of any removed devices */
 	for (i = 0; i < nremoved; i++) {
+		if (removed[i] == NULL)
+			continue;
 		if (removed[i]->expose_state & HPSA_SCSI_ADD) {
 			struct scsi_device *sdev =
 				scsi_device_lookup(sh, removed[i]->bus,

@@ -1792,18 +1807,18 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
 
 	/* Notify scsi mid layer of any added devices */
 	for (i = 0; i < nadded; i++) {
+		if (added[i] == NULL)
+			continue;
 		if (!(added[i]->expose_state & HPSA_SCSI_ADD))
 			continue;
 		if (scsi_add_device(sh, added[i]->bus,
 			added[i]->target, added[i]->lun) == 0)
 			continue;
-		hpsa_show_dev_msg(KERN_WARNING, h, added[i],
-					"addition failed, device not added.");
+		dev_warn(&h->pdev->dev, "addition failed, device not added.");
 		/* now we have to remove it from h->dev,
 		 * since it didn't get added to scsi mid layer
 		 */
 		fixup_botched_add(h, added[i]);
-		added[i] = NULL;
 	}
 
 free_and_out:
 
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

drivers/scsi/hpsa.c +0 -0

index 6b6e9bb..3fe8a18 100644

drivers/scsi/hpsa.c +18 -2

@@ -1423,6 +1423,9 @@ static int hpsa_scsi_find_entry(struct hpsa_scsi_dev_t *needle,
 #define DEVICE_CHANGED 1
 #define DEVICE_SAME 2
 #define DEVICE_UPDATED 3
+	if (needle == NULL)
+		return DEVICE_NOT_FOUND;
+
 	for (i = 0; i < haystack_size; i++) {
 		if (haystack[i] == NULL) /* previously removed. */
 			continue;

@@ -1574,6 +1577,9 @@ static void hpsa_figure_phys_disk_ptrs(struct ctlr_info *h,
 				total_disks_per_row;
 	int qdepth;
 
+	if (logical_drive == NULL)
+		return;
+
 	if (nraid_map_entries > RAID_MAP_MAX_ENTRIES)
 		nraid_map_entries = RAID_MAP_MAX_ENTRIES;
 

@@ -1585,6 +1591,8 @@ static void hpsa_figure_phys_disk_ptrs(struct ctlr_info *h,
 		if (!logical_drive->offload_config)
 			continue;
 		for (j = 0; j < ndevices; j++) {
+			if (dev[j] == NULL)
+				continue;
 			if (dev[j]->devtype != TYPE_DISK)
 				continue;
 			if (is_logical_dev_addr_mode(dev[j]->scsi3addr))

@@ -1628,6 +1636,8 @@ static void hpsa_update_log_drive_phys_drive_ptrs(struct ctlr_info *h,
 	int i;
 
 	for (i = 0; i < ndevices; i++) {
+		if (dev[i] == NULL)
+			continue;
 		if (dev[i]->devtype != TYPE_DISK)
 			continue;
 		if (!is_logical_dev_addr_mode(dev[i]->scsi3addr))

@@ -1743,8 +1753,11 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
 	/* Now that h->dev[]->phys_disk[] is coherent, we can enable
 	 * any logical drives that need it enabled.
 	 */
-	for (i = 0; i < h->ndevices; i++)
+	for (i = 0; i < h->ndevices; i++) {
+		if (h->dev[i] == NULL)
+			continue;
 		h->dev[i]->offload_enabled = h->dev[i]->offload_to_be_enabled;
+	}
 
 	spin_unlock_irqrestore(&h->devlock, flags);
 

@@ -1769,6 +1782,8 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
 	sh = h->scsi_host;
 	/* Notify scsi mid layer of any removed devices */
 	for (i = 0; i < nremoved; i++) {
+		if (removed[i] == NULL)
+			continue;
 		if (removed[i]->expose_state & HPSA_SCSI_ADD) {
 			struct scsi_device *sdev =
 				scsi_device_lookup(sh, removed[i]->bus,

@@ -1792,18 +1807,18 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
 
 	/* Notify scsi mid layer of any added devices */
 	for (i = 0; i < nadded; i++) {
+		if (added[i] == NULL)
+			continue;
 		if (!(added[i]->expose_state & HPSA_SCSI_ADD))
 			continue;
 		if (scsi_add_device(sh, added[i]->bus,
 			added[i]->target, added[i]->lun) == 0)
 			continue;
-		hpsa_show_dev_msg(KERN_WARNING, h, added[i],

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Reviewed-by Tomas Henzl <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   17 +++++++++++++++--
 drivers/scsi/hpsa.h |    1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

Abandon and reschedule rescan process only if device inquiries fail due to mem alloc failures, which are likely to occur for all devices.

Otherwise, skip device if inquiry fails for other reasons, and continue rescanning process for other devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Reviewed-by Tomas Henzl <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

drivers/scsi/hpsa.c +4 -1

index a3f671c..ecc6ada 100644
@@ -3375,10 +3375,13 @@ static int hpsa_update_device_info(struct ctlr_info *h,
 
 	unsigned char *inq_buff;
 	unsigned char *obdr_sig;
+	int rc = 0;
 
 	inq_buff = kzalloc(OBDR_TAPE_INQ_SIZE, GFP_KERNEL);
-	if (!inq_buff)
+	if (!inq_buff) {
+		rc = -ENOMEM;
 		goto bail_out;
+	}
 
 	/* Do an inquiry to the device to see what it is. */
 	if (hpsa_scsi_do_inquiry(h, scsi3addr, 0, inq_buff,

drivers/scsi/hpsa.c +8 -4

@@ -3435,7 +3439,7 @@ static int hpsa_update_device_info(struct ctlr_info *h,
 
 bail_out:
 	kfree(inq_buff);
-	return 1;
+	return rc;
 }
 
 static void hpsa_update_device_supports_aborts(struct ctlr_info *h,

@@ -3803,6 +3807,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 	n_ext_target_devs = 0;
 	for (i = 0; i < nphysicals + nlogicals + 1; i++) {
 		u8 *lunaddrbytes, is_OBDR = 0;
+		int rc = 0;
 
 		/* Figure out where the LUN ID info is coming from */
 		lunaddrbytes = figure_lunaddrbytes(h, raid_ctlr_position,

@@ -3815,11 +3820,20 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 				continue;
 
 		/* Get device type, vendor, model, device id */
-		if (hpsa_update_device_info(h, lunaddrbytes, tmpdevice,
-							&is_OBDR)) {
+		rc = hpsa_update_device_info(h, lunaddrbytes, tmpdevice,
+							&is_OBDR);
+		if (rc == -ENOMEM) {
+			dev_warn(&h->pdev->dev,
+				"Out of memory, rescan deferred.\n");
 			h->drv_req_rescan = 1;
-			continue; /* skip it if we can't talk to it. */
+			goto out;

Quoted 33 lines · Don Brace

Abandon and reschedule rescan process only if device inquiries fail due to mem alloc failures, which are likely to occur for all devices.

Otherwise, skip device if inquiry fails for other reasons, and continue rescanning process for other devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Reviewed-by Tomas Henzl <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
  drivers/scsi/hpsa.c |   24 +++++++++++++++++++-----
  1 file changed, 19 insertions(+), 5 deletions(-)

drivers/scsi/hpsa.c +4 -1

index a3f671c..ecc6ada 100644
@@ -3375,10 +3375,13 @@ static int hpsa_update_device_info(struct ctlr_info *h,
 
  	unsigned char *inq_buff;
  	unsigned char *obdr_sig;
+	int rc = 0;
 
  	inq_buff = kzalloc(OBDR_TAPE_INQ_SIZE, GFP_KERNEL);
-	if (!inq_buff)
+	if (!inq_buff) {
+		rc = -ENOMEM;
  		goto bail_out;
+	}
 

drivers/scsi/hpsa.c static int hpsa_update_device_info(struct ctlr_info *h, · line 3395

Quoted 10 lines

drivers/scsi/hpsa.c +1 -0

  	/* Do an inquiry to the device to see what it is. */
  	if (hpsa_scsi_do_inquiry(h, scsi3addr, 0, inq_buff,

@@ -3386,6 +3389,7 @@ static int hpsa_update_device_info(struct ctlr_info *h,
  		/* Inquiry failed (msg printed already) */
  		dev_err(&h->pdev->dev,
  			"hpsa_update_device_info: inquiry failed\n");
+		rc = 1;
  		goto bail_out;
  	}
 

Why '1' and not a normal error code, seeing that you're using -ENOMEM above?

Quoted 33 lines

drivers/scsi/hpsa.c +8 -4

@@ -3435,7 +3439,7 @@ static int hpsa_update_device_info(struct ctlr_info *h,
 
  bail_out:
  	kfree(inq_buff);
-	return 1;
+	return rc;
  }
 
  static void hpsa_update_device_supports_aborts(struct ctlr_info *h,

@@ -3803,6 +3807,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
  	n_ext_target_devs = 0;
  	for (i = 0; i < nphysicals + nlogicals + 1; i++) {
  		u8 *lunaddrbytes, is_OBDR = 0;
+		int rc = 0;
 
  		/* Figure out where the LUN ID info is coming from */
  		lunaddrbytes = figure_lunaddrbytes(h, raid_ctlr_position,

@@ -3815,11 +3820,20 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
  				continue;
 
  		/* Get device type, vendor, model, device id */
-		if (hpsa_update_device_info(h, lunaddrbytes, tmpdevice,
-							&is_OBDR)) {
+		rc = hpsa_update_device_info(h, lunaddrbytes, tmpdevice,
+							&is_OBDR);
+		if (rc == -ENOMEM) {
+			dev_warn(&h->pdev->dev,
+				"Out of memory, rescan deferred.\n");
  			h->drv_req_rescan = 1;
-			continue; /* skip it if we can't talk to it. */
+			goto out;

drivers/scsi/hpsa.c static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno) · line 3839

Quoted 10 lines

drivers/scsi/hpsa.c +6 -0

  		}
+		if (rc) {
+			dev_warn(&h->pdev->dev,
+				"Inquiry failed, skipping device.\n");
+			continue;
+		}
+
  		figure_bus_target_lun(h, lunaddrbytes, tmpdevice);
  		hpsa_update_device_supports_aborts(h, tmpdevice, lunaddrbytes);
  		this_device = currentsd[ncurrent];

Cheers,

Hannes

handle block counts of 0. Cleanup block and block count calculations.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Reviewed-by Tomas Henzl <[email protected]>

Signed-off-by Don Brace <[email protected]>

---

Fix a NULL pointer issue in the driver when devices are removed during a reset.

Signed-off-by Don Brace <[email protected]>

---
 drivers/block/cciss.h |    1 +
 drivers/scsi/hpsa.c   |   16 ++++++++++++++++
 drivers/scsi/hpsa.h   |    1 +
 3 files changed, 18 insertions(+)

drivers/block/cciss.h +1 -0

index 7fda30e..f8b8c6b 100644
@@ -155,6 +155,7 @@ struct ctlr_info
 	size_t reply_pool_size;
 	unsigned char reply_pool_wraparound;
 	u32 *blockFetchTable;
+	u8 reset_in_progress;
 };
 
 /*  Defining the diffent access_methods

drivers/scsi/hpsa.c +0 -0

index ced0d4c..08a761c 100644
@@ -1670,6 +1670,15 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
 	int nadded, nremoved;
 	struct Scsi_Host *sh = NULL;

drivers/scsi/hpsa.c +7 -0

 	added = kzalloc(sizeof(*added) * HPSA_MAX_DEVICES, GFP_KERNEL);
 	removed = kzalloc(sizeof(*removed) * HPSA_MAX_DEVICES, GFP_KERNEL);
 

@@ -1780,6 +1789,10 @@ static void adjust_hpsa_scsi_table(struct ctlr_info *h, int hostno,
 		goto free_and_out;
 
 	sh = h->scsi_host;
+	if (sh == NULL) {
+		dev_warn(&h->pdev->dev, "%s: scsi_host is null\n", __func__);
+		return;
+	}
 	/* Notify scsi mid layer of any removed devices */
 	for (i = 0; i < nremoved; i++) {
 		if (removed[i] == NULL)

@@ -5243,12 +5256,15 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
 
 	hpsa_show_dev_msg(KERN_WARNING, h, dev, "resetting");
 
+	h->reset_in_progress = 1;
+
 	/* send a reset to the SCSI LUN which the command was sent to */
 	rc = hpsa_do_reset(h, dev, dev->scsi3addr, HPSA_RESET_TYPE_LUN,
 			   DEFAULT_REPLY_QUEUE);
 	snprintf(msg, sizeof(msg), "reset %s",
 		 rc == 0 ? "completed successfully" : "failed");
 	hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
+	h->reset_in_progress = 0;
 	return rc == 0 ? SUCCESS : FAILED;
 }
 

drivers/scsi/hpsa.h +1 -1

index b173c0e..dc113c1a 100644
@@ -271,6 +271,7 @@ struct ctlr_info {
 	wait_queue_head_t abort_cmd_wait_queue;
 	wait_queue_head_t event_sync_wait_queue;
 	struct mutex reset_mutex;
+	u8 reset_in_progress;
 };
 
 struct offline_device_entry {
 
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Fix a NULL pointer issue in the driver when devices are removed during a reset.

Signed-off-by Don Brace <[email protected]>

---
 drivers/block/cciss.h |    1 +
 drivers/scsi/hpsa.c   |   16 ++++++++++++++++
 drivers/scsi/hpsa.h   |    1 +
 3 files changed, 18 insertions(+)

drivers/block/cciss.h +0 -0

index 7fda30e..f8b8c6b 100644

Quoted 21 lines · Don Brace

Fix a NULL pointer issue in the driver when devices are removed during a reset.

Signed-off-by Don Brace <[email protected]>

---
  drivers/block/cciss.h |    1 +
  drivers/scsi/hpsa.c   |   16 ++++++++++++++++
  drivers/scsi/hpsa.h   |    1 +
  3 files changed, 18 insertions(+)

drivers/block/cciss.h +1 -0

index 7fda30e..f8b8c6b 100644
@@ -155,6 +155,7 @@ struct ctlr_info
  	size_t reply_pool_size;
  	unsigned char reply_pool_wraparound;
  	u32 *blockFetchTable;
+	u8 reset_in_progress;
  };
 

drivers/block/cciss.h struct ctlr_info · line 161

Quoted 1 line

drivers/block/cciss.h +0 -0

  /*  Defining the diffent access_methods

Why do you need to add it here?
To my knowledge cciss and hpsa are different drivers ...

Cheers,

Hannes

From: Scott Teel <[email protected]>

Set reset type in device_reset_handler to do either logical unit reset for logical devices, or physical target reset, for physical devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   34 ++++++++++++++++++++++++++++------
 drivers/scsi/hpsa.h |    1 +
 2 files changed, 29 insertions(+), 6 deletions(-)

drivers/scsi/hpsa.c +28 -6

index 08a761c..67136fb 100644
@@ -2746,9 +2746,8 @@ static int hpsa_send_reset(struct ctlr_info *h, unsigned char *scsi3addr,
 
 
 	/* fill_cmd can't fail here, no data buffer to map. */
-	(void) fill_cmd(c, HPSA_DEVICE_RESET_MSG, h, NULL, 0, 0,
+	(void) fill_cmd(c, reset_type, h, NULL, 0, 0,
 			scsi3addr, TYPE_MSG);
-	c->Request.CDB[1] = reset_type; /* fill_cmd defaults to LUN reset */
 	rc = hpsa_scsi_do_simple_cmd(h, c, reply_queue, NO_TIMEOUT);
 	if (rc) {
 		dev_warn(&h->pdev->dev, "Failed to send reset command\n");

@@ -5216,6 +5215,7 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
 	int rc;
 	struct ctlr_info *h;
 	struct hpsa_scsi_dev_t *dev;
+	u8 reset_type;
 	char msg[48];
 
 	/* find the controller to which the command to be aborted was sent */

@@ -5254,15 +5254,23 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
 	if (is_hba_lunid(dev->scsi3addr))
 		return SUCCESS;
 
-	hpsa_show_dev_msg(KERN_WARNING, h, dev, "resetting");
+	if (is_logical_dev_addr_mode(dev->scsi3addr))
+		reset_type = HPSA_DEVICE_RESET_MSG;
+	else
+		reset_type = HPSA_PHYS_TARGET_RESET;
+
+	sprintf(msg, "resetting %s",
+		reset_type == HPSA_DEVICE_RESET_MSG ? "logical " : "physical ");
+	hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
 
 	h->reset_in_progress = 1;
 
 	/* send a reset to the SCSI LUN which the command was sent to */
-	rc = hpsa_do_reset(h, dev, dev->scsi3addr, HPSA_RESET_TYPE_LUN,
+	rc = hpsa_do_reset(h, dev, dev->scsi3addr, reset_type,
 			   DEFAULT_REPLY_QUEUE);
-	snprintf(msg, sizeof(msg), "reset %s",
-		 rc == 0 ? "completed successfully" : "failed");
+	sprintf(msg, "reset %s %s",
+		reset_type == HPSA_DEVICE_RESET_MSG ? "logical " : "physical ",
+		rc == 0 ? "completed successfully" : "failed");
 	hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
 	h->reset_in_progress = 0;
 	return rc == 0 ? SUCCESS : FAILED;

@@ -6379,6 +6387,20 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
 	} else if (cmd_type == TYPE_MSG) {
 		switch (cmd) {
 
+		case  HPSA_PHYS_TARGET_RESET:
+			c->Request.CDBLen = 16;
+			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] = HPSA_RESET;
+			c->Request.CDB[1] = HPSA_TARGET_RESET_TYPE;
+			/* Physical target reset needs no control bytes 4-7*/
+			c->Request.CDB[4] = 0x00;
+			c->Request.CDB[5] = 0x00;
+			c->Request.CDB[6] = 0x00;
+			c->Request.CDB[7] = 0x00;
+			break;
 		case  HPSA_DEVICE_RESET_MSG:
 			c->Request.CDBLen = 16;
 			c->Request.type_attr_dir =

drivers/scsi/hpsa.h +0 -0

index dc113c1a..04f98eb 100644

drivers/scsi/hpsa.h +0 -1

 #define HPSA_MSG_SEND_RETRY_LIMIT 10
 #define HPSA_MSG_SEND_RETRY_INTERVAL_MSECS (10000)
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 52 lines · Don Brace

From: Scott Teel <[email protected]>

Set reset type in device_reset_handler to do either logical unit reset for logical devices, or physical target reset, for physical devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
  drivers/scsi/hpsa.c |   34 ++++++++++++++++++++++++++++------
  drivers/scsi/hpsa.h |    1 +
  2 files changed, 29 insertions(+), 6 deletions(-)

drivers/scsi/hpsa.c +10 -3

index 08a761c..67136fb 100644
@@ -2746,9 +2746,8 @@ static int hpsa_send_reset(struct ctlr_info *h, unsigned char *scsi3addr,
 
 
  	/* fill_cmd can't fail here, no data buffer to map. */
-	(void) fill_cmd(c, HPSA_DEVICE_RESET_MSG, h, NULL, 0, 0,
+	(void) fill_cmd(c, reset_type, h, NULL, 0, 0,
  			scsi3addr, TYPE_MSG);
-	c->Request.CDB[1] = reset_type; /* fill_cmd defaults to LUN reset */
  	rc = hpsa_scsi_do_simple_cmd(h, c, reply_queue, NO_TIMEOUT);
  	if (rc) {
  		dev_warn(&h->pdev->dev, "Failed to send reset command\n");

@@ -5216,6 +5215,7 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
  	int rc;
  	struct ctlr_info *h;
  	struct hpsa_scsi_dev_t *dev;
+	u8 reset_type;
  	char msg[48];
 
  	/* find the controller to which the command to be aborted was sent */

@@ -5254,15 +5254,23 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
  	if (is_hba_lunid(dev->scsi3addr))
  		return SUCCESS;
 
-	hpsa_show_dev_msg(KERN_WARNING, h, dev, "resetting");
+	if (is_logical_dev_addr_mode(dev->scsi3addr))
+		reset_type = HPSA_DEVICE_RESET_MSG;
+	else
+		reset_type = HPSA_PHYS_TARGET_RESET;
+
+	sprintf(msg, "resetting %s",
+		reset_type == HPSA_DEVICE_RESET_MSG ? "logical " : "physical ");
+	hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
 

drivers/scsi/hpsa.c static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd) · line 5266

Quoted 1 line

drivers/scsi/hpsa.c +0 -0

 */

Will this new reset command work for the whole range of hpsa devices? (and for the older cciss hw - hpsa_allow_any) ?

Quoted 9 lines

drivers/scsi/hpsa.c +0 -1

  #define HPSA_MSG_SEND_RETRY_LIMIT 10
  #define HPSA_MSG_SEND_RETRY_INTERVAL_MSECS (10000)
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 53 lines · Tomas Henzl

Quoted 52 lines · Don Brace

From: Scott Teel <[email protected]>

Set reset type in device_reset_handler to do either logical unit reset for logical devices, or physical target reset, for physical devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
  drivers/scsi/hpsa.c |   34 ++++++++++++++++++++++++++++------
  drivers/scsi/hpsa.h |    1 +
  2 files changed, 29 insertions(+), 6 deletions(-)

drivers/scsi/hpsa.c +10 -3

index 08a761c..67136fb 100644
@@ -2746,9 +2746,8 @@ static int hpsa_send_reset(struct ctlr_info *h, unsigned char *scsi3addr,
 
 
  	/* fill_cmd can't fail here, no data buffer to map. */
-	(void) fill_cmd(c, HPSA_DEVICE_RESET_MSG, h, NULL, 0, 0,
+	(void) fill_cmd(c, reset_type, h, NULL, 0, 0,
  			scsi3addr, TYPE_MSG);
-	c->Request.CDB[1] = reset_type; /* fill_cmd defaults to LUN reset */
  	rc = hpsa_scsi_do_simple_cmd(h, c, reply_queue, NO_TIMEOUT);
  	if (rc) {
  		dev_warn(&h->pdev->dev, "Failed to send reset command\n");

@@ -5216,6 +5215,7 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
  	int rc;
  	struct ctlr_info *h;
  	struct hpsa_scsi_dev_t *dev;
+	u8 reset_type;
  	char msg[48];
 
  	/* find the controller to which the command to be aborted was sent */

@@ -5254,15 +5254,23 @@ static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd)
  	if (is_hba_lunid(dev->scsi3addr))
  		return SUCCESS;
 
-	hpsa_show_dev_msg(KERN_WARNING, h, dev, "resetting");
+	if (is_logical_dev_addr_mode(dev->scsi3addr))
+		reset_type = HPSA_DEVICE_RESET_MSG;
+	else
+		reset_type = HPSA_PHYS_TARGET_RESET;
+
+	sprintf(msg, "resetting %s",
+		reset_type == HPSA_DEVICE_RESET_MSG ? "logical " : "physical ");
+	hpsa_show_dev_msg(KERN_WARNING, h, dev, msg);
 

Quoted 3 lines

drivers/scsi/hpsa.c static int hpsa_eh_device_reset_handler(struct scsi_cmnd *scsicmd) · line 5266

Quoted 1 line

drivers/scsi/hpsa.c +0 -0

 */

Will this new reset command work for the whole range of hpsa devices? (and for the older cciss hw - hpsa_allow_any) ?

The new reset works for newer controller that support HBA mode or IO bypass mode that allow the driver to send commands directly to a physical device. This new reset is for those situations.

We still do the existing reset for RAID volumes.

From: Scott Teel <[email protected]>

Set reset type in device_reset_handler to do either logical unit reset for logical devices, or physical target reset, for physical devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---

The driver is using two MACROs which seemingly are looking in the wrong location for the device_flags returned from CISS_REPORT_PHYS. Both MACROs, NON_DISK_PHYS_DEV and PHYS_IOACCEL, are using the pointer returned from figure_lunaddrbytes which is the address of the LUN.lunid element in
the extended CISS_REPORT_PHYS. But the MACROS are using offsets beyond the range of the element (offset 17 of an 8 byte element).

These MACROs actually are looking at the correct location but they fail static checker analysis. It also will not work if any new elements are added to the extended LUN structure.

Change the code to use the structure elements directly since this MACRO is only used in one location.

Reported-by Dan Carpenter <[email protected]>

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

The driver is using two MACROs which seemingly are looking in the wrong location for the device_flags returned from CISS_REPORT_PHYS. Both MACROs, NON_DISK_PHYS_DEV and PHYS_IOACCEL, are using the pointer returned from figure_lunaddrbytes which is the address of the LUN.lunid element in
the extended CISS_REPORT_PHYS. But the MACROS are using offsets beyond the range of the element (offset 17 of an 8 byte element).

These MACROs actually are looking at the correct location but they fail static checker analysis. It also will not work if any new elements are added to the extended LUN structure.

Change the code to use the structure elements directly since this MACRO is only used in one location.

Reported-by Dan Carpenter <[email protected]>

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c     |   28 +++++++++++++++-------------
 drivers/scsi/hpsa_cmd.h |    2 --
 2 files changed, 15 insertions(+), 15 deletions(-)

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

From: Kevin Barnett <[email protected]>

remove macros and cleanup device exposure checking

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

From: Kevin Barnett <[email protected]>

remove macros and cleanup device exposure checking

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   25 +++++++++++--------------
 drivers/scsi/hpsa.h |    6 +-----
 2 files changed, 12 insertions(+), 19 deletions(-)

From: Kevin Barnett <[email protected]>

remove repeated calculation that checks for physical or logical devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

From: Kevin Barnett <[email protected]>

remove repeated calculation that checks for physical or logical devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
drivers/scsi/hpsa.c | 23 ++++++++++++++---------
drivers/scsi/hpsa.h | 1 +

2 files changed, 15 insertions(+), 9 deletions(-)

drivers/scsi/hpsa.c +0 -0

index d011540..7c1a552 100644

drivers/scsi/hpsa.c +2 -0

 	DECLARE_BITMAP(lunzerobits, MAX_EXT_TARGETS);
 
 	currentsd = kzalloc(sizeof(*currentsd) * HPSA_MAX_DEVICES, GFP_KERNEL);

@@ -3821,16 +3822,17 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 		int rc = 0;
 		int phys_dev_index = i - (raid_ctlr_position == 0);
 
+		physical_device = i < nphysicals + (raid_ctlr_position == 0);
+
 		/* Figure out where the LUN ID info is coming from */
 		lunaddrbytes = figure_lunaddrbytes(h, raid_ctlr_position,
 			i, nphysicals, nlogicals, physdev_list, logdev_list);
 

drivers/scsi/hpsa.c +7 -4

 		/* Get device type, vendor, model, device id */
 		rc = hpsa_update_device_info(h, lunaddrbytes, tmpdevice,

@@ -3866,10 +3868,13 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 		}
 
 		*this_device = *tmpdevice;
+		this_device->physical_device = physical_device;
 
-		/* do not expose masked devices */
-		if (MASKED_DEVICE(lunaddrbytes) &&
-			i < nphysicals + (raid_ctlr_position == 0))
+		/*
+		 * Expose all devices except for physical devices that
+		 * are masked.
+		 */
+		if (MASKED_DEVICE(lunaddrbytes) && this_device->physical_device)
 			this_device->expose_device = 0;
 		else
 			this_device->expose_device = 1;

@@ -3887,7 +3892,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 				ncurrent++;
 			break;
 		case TYPE_DISK:
-			if (i < nphysicals + (raid_ctlr_position == 0)) {
+			if (this_device->physical_device) {
 				/* The disk is in HBA mode. */
 				/* Never use RAID mapper in HBA mode. */
 				this_device->offload_enabled = 0;

drivers/scsi/hpsa.h +1 -1

index a6ead07..808b520 100644
@@ -37,6 +37,7 @@ struct hpsa_scsi_dev_t {
 	unsigned int devtype;
 	int bus, target, lun;		/* as presented to the OS */
 	unsigned char scsi3addr[8];	/* as presented to the HW */
+	u8 physical_device;
 	u8 expose_device;
 #define RAID_CTLR_LUNID "\0\0\0\0\0\0\0\0"
 	unsigned char device_id[16];    /* from inquiry pg. 0x83 */
 
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 16 lines · Don Brace

From: Kevin Barnett <[email protected]>

remove repeated calculation that checks for physical or logical devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
drivers/scsi/hpsa.c | 23 ++++++++++++++---------
drivers/scsi/hpsa.h | 1 +

2 files changed, 15 insertions(+), 9 deletions(-)

drivers/scsi/hpsa.c static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno) · line 3764

Quoted 9 lines

drivers/scsi/hpsa.c +1 -0

index d011540..7c1a552 100644
@@ -3761,6 +3761,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 	int ncurrent = 0;
 	int i, n_ext_target_devs, ndevs_to_allocate;
 	int raid_ctlr_position;
+	bool physical_device;

Any particular reason for using a bool here and a u8 when you cache the value?

Quoted 14 lines

drivers/scsi/hpsa.c +2 -0

 	DECLARE_BITMAP(lunzerobits, MAX_EXT_TARGETS);
 
 	currentsd = kzalloc(sizeof(*currentsd) * HPSA_MAX_DEVICES, GFP_KERNEL);

@@ -3821,16 +3822,17 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 		int rc = 0;
 		int phys_dev_index = i - (raid_ctlr_position == 0);
 
+		physical_device = i < nphysicals + (raid_ctlr_position == 0);
+
 		/* Figure out where the LUN ID info is coming from */
 		lunaddrbytes = figure_lunaddrbytes(h, raid_ctlr_position,
 			i, nphysicals, nlogicals, physdev_list, logdev_list);
 

drivers/scsi/hpsa.c static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno) · line 3835

Quoted 10 lines

drivers/scsi/hpsa.c +4 -5

 		/* skip masked non-disk devices */
-		if (MASKED_DEVICE(lunaddrbytes))
-			if (i < nphysicals + (raid_ctlr_position == 0) &&
-				(physdev_list->
-				LUN[phys_dev_index].device_flags & 0x01))
-				continue;
+		if (physical_device &&
+			MASKED_DEVICE(lunaddrbytes) &&
+			(physdev_list->LUN[phys_dev_index].device_flags & 0x01))
+			continue;

In this conditional you swapped the ordering, evaluating physical_device first, why?

Quoted 43 lines

drivers/scsi/hpsa.c +7 -4

 		/* Get device type, vendor, model, device id */
 		rc = hpsa_update_device_info(h, lunaddrbytes, tmpdevice,

@@ -3866,10 +3868,13 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 		}
 
 		*this_device = *tmpdevice;
+		this_device->physical_device = physical_device;
 
-		/* do not expose masked devices */
-		if (MASKED_DEVICE(lunaddrbytes) &&
-			i < nphysicals + (raid_ctlr_position == 0))
+		/*
+		 * Expose all devices except for physical devices that
+		 * are masked.
+		 */
+		if (MASKED_DEVICE(lunaddrbytes) && this_device->physical_device)
 			this_device->expose_device = 0;
 		else
 			this_device->expose_device = 1;

@@ -3887,7 +3892,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 				ncurrent++;
 			break;
 		case TYPE_DISK:
-			if (i < nphysicals + (raid_ctlr_position == 0)) {
+			if (this_device->physical_device) {
 				/* The disk is in HBA mode. */
 				/* Never use RAID mapper in HBA mode. */
 				this_device->offload_enabled = 0;

drivers/scsi/hpsa.h +1 -1

index a6ead07..808b520 100644
@@ -37,6 +37,7 @@ struct hpsa_scsi_dev_t {
 	unsigned int devtype;
 	int bus, target, lun;		/* as presented to the OS */
 	unsigned char scsi3addr[8];	/* as presented to the HW */
+	u8 physical_device;
 	u8 expose_device;
 #define RAID_CTLR_LUNID "\0\0\0\0\0\0\0\0"
 	unsigned char device_id[16];    /* from inquiry pg. 0x83 */
 
--

Quoted 4 lines

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 16 lines · Matthew R. Ochs

Quoted 16 lines · Don Brace

From: Kevin Barnett <[email protected]>

remove repeated calculation that checks for physical or logical devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
drivers/scsi/hpsa.c | 23 ++++++++++++++---------
drivers/scsi/hpsa.h | 1 +

2 files changed, 15 insertions(+), 9 deletions(-)

Quoted 10 lines

drivers/scsi/hpsa.c static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno) · line 3764

Quoted 9 lines

drivers/scsi/hpsa.c +1 -0

index d011540..7c1a552 100644
@@ -3761,6 +3761,7 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 	int ncurrent = 0;
 	int i, n_ext_target_devs, ndevs_to_allocate;
 	int raid_ctlr_position;
+	bool physical_device;

Any particular reason for using a bool here and a u8 when you cache the value?

Changed definition to u8 physical_device : 1; in hpsa_scsi_dev_t

Quoted 14 lines

Quoted 14 lines

drivers/scsi/hpsa.c +2 -0

 	DECLARE_BITMAP(lunzerobits, MAX_EXT_TARGETS);
 
 	currentsd = kzalloc(sizeof(*currentsd) * HPSA_MAX_DEVICES, GFP_KERNEL);

@@ -3821,16 +3822,17 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno)
 		int rc = 0;
 		int phys_dev_index = i - (raid_ctlr_position == 0);
 
+		physical_device = i < nphysicals + (raid_ctlr_position == 0);
+
 		/* Figure out where the LUN ID info is coming from */
 		lunaddrbytes = figure_lunaddrbytes(h, raid_ctlr_position,
 			i, nphysicals, nlogicals, physdev_list, logdev_list);
 

Quoted 11 lines

drivers/scsi/hpsa.c static void hpsa_update_scsi_devices(struct ctlr_info *h, int hostno) · line 3835

Quoted 10 lines

drivers/scsi/hpsa.c +4 -5

 		/* skip masked non-disk devices */
-		if (MASKED_DEVICE(lunaddrbytes))
-			if (i < nphysicals + (raid_ctlr_position == 0) &&
-				(physdev_list->
-				LUN[phys_dev_index].device_flags & 0x01))
-				continue;
+		if (physical_device &&
+			MASKED_DEVICE(lunaddrbytes) &&
+			(physdev_list->LUN[phys_dev_index].device_flags & 0x01))
+			continue;

In this conditional you swapped the ordering, evaluating physical_device first, why?

Changed it back. Better to be consistent.

From: Kevin Barnett <[email protected]>

remove repeated calculation that checks for physical or logical devices.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   23 ++++++++++++++---------
 drivers/scsi/hpsa.h |    1 +
 2 files changed, 15 insertions(+), 9 deletions(-)

From: Kevin Barnett <[email protected]>

simplify checking for logical/physical devices

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

From: Kevin Barnett <[email protected]>

simplify checking for logical/physical devices

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

use an index into vpd data for SAS/SATA drives

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

use an index into vpd data for SAS/SATA drives

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
drivers/scsi/hpsa.c | 23 ++++++++++++++++++++---

1 file changed, 20 insertions(+), 3 deletions(-)

drivers/scsi/hpsa.c +6 -2

index c1b053f..1361414 100644
@@ -3137,7 +3137,7 @@ out:
 
 /* Get the device id from inquiry page 0x83 */
 static int hpsa_get_device_id(struct ctlr_info *h, unsigned char *scsi3addr,
-	unsigned char *device_id, int buflen)
+	unsigned char *device_id, int index, int buflen)
 {
 	int rc;
 	unsigned char *buf;

@@ -3149,8 +3149,10 @@ static int hpsa_get_device_id(struct ctlr_info *h, unsigned char *scsi3addr,
 		return -ENOMEM;
 	rc = hpsa_scsi_do_inquiry(h, scsi3addr, VPD_PAGE | 0x83, buf, 64);
 	if (rc == 0)
-		memcpy(device_id, &buf[8], buflen);
+		memcpy(device_id, &buf[index], buflen);
+
 	kfree(buf);
+
 	return rc != 0;
 }
 

@@ -3379,6 +3381,18 @@ static int hpsa_device_supports_aborts(struct ctlr_info *h,
 	return rc;
 }
 
+static void sanitize_inquiry_string(unsigned char *s, int len)
+{

use an index into vpd data for SAS/SATA drives

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

drivers/scsi/hpsa.c +9 -2

index c1b053f..1361414 100644
@@ -3137,7 +3137,7 @@ out:
 
 /* Get the device id from inquiry page 0x83 */
 static int hpsa_get_device_id(struct ctlr_info *h, unsigned char *scsi3addr,
-	unsigned char *device_id, int buflen)
+	unsigned char *device_id, int index, int buflen)
 {
 	int rc;
 	unsigned char *buf;

@@ -3149,8 +3149,10 @@ static int hpsa_get_device_id(struct ctlr_info *h, unsigned char *scsi3addr,
 		return -ENOMEM;
 	rc = hpsa_scsi_do_inquiry(h, scsi3addr, VPD_PAGE | 0x83, buf, 64);
 	if (rc == 0)
-		memcpy(device_id, &buf[8], buflen);
+		memcpy(device_id, &buf[index], buflen);
+
 	kfree(buf);
+
 	return rc != 0;
 }
 

@@ -3379,6 +3381,18 @@ static int hpsa_device_supports_aborts(struct ctlr_info *h,
 	return rc;
 }
 
+static void sanitize_inquiry_string(unsigned char *s, int len)
+{
+	bool terminated = false;
+
+	for (; len > 0; (--len, ++s)) {

Quoted 44 lines · Don Brace

use an index into vpd data for SAS/SATA drives

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
  drivers/scsi/hpsa.c |   23 ++++++++++++++++++++---
  1 file changed, 20 insertions(+), 3 deletions(-)

drivers/scsi/hpsa.c +9 -2

index c1b053f..1361414 100644
@@ -3137,7 +3137,7 @@ out:
 
  /* Get the device id from inquiry page 0x83 */
  static int hpsa_get_device_id(struct ctlr_info *h, unsigned char *scsi3addr,
-	unsigned char *device_id, int buflen)
+	unsigned char *device_id, int index, int buflen)
  {
  	int rc;
  	unsigned char *buf;

@@ -3149,8 +3149,10 @@ static int hpsa_get_device_id(struct ctlr_info *h, unsigned char *scsi3addr,
  		return -ENOMEM;
  	rc = hpsa_scsi_do_inquiry(h, scsi3addr, VPD_PAGE | 0x83, buf, 64);
  	if (rc == 0)
-		memcpy(device_id, &buf[8], buflen);
+		memcpy(device_id, &buf[index], buflen);
+
  	kfree(buf);
+
  	return rc != 0;
  }
 

@@ -3379,6 +3381,18 @@ static int hpsa_device_supports_aborts(struct ctlr_info *h,
  	return rc;
  }
 
+static void sanitize_inquiry_string(unsigned char *s, int len)
+{
+	bool terminated = false;
+
+	for (; len > 0; (--len, ++s)) {

drivers/scsi/hpsa.c static int hpsa_device_supports_aborts(struct ctlr_info *h, · line 3398

Quoted 10 lines

drivers/scsi/hpsa.c +7 -0

+		if (*s == 0)
+			terminated = true;
+		if (terminated || *s < 0x20 || *s > 0x7e)
+			*s = ' ';
+	}
+}
+
  static int hpsa_update_device_info(struct ctlr_info *h,
  	unsigned char scsi3addr[], struct hpsa_scsi_dev_t *this_device,
  	unsigned char *is_OBDR_device)

I would prefer to have the function from scsi_scan.c to be exported. Hardly a point to duplicate it.

Cheers,

Hannes

From: Kevin Barnett <[email protected]>

setup for sas transport. Need to set the
bus and target accordingly.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   25 ++++++++++++-------------
 drivers/scsi/hpsa.h |    5 +++++
 2 files changed, 17 insertions(+), 13 deletions(-)

drivers/scsi/hpsa.c +12 -13

index 1361414..24b3c8c 100644
@@ -3521,38 +3521,37 @@ static int is_ext_target(struct ctlr_info *h, struct hpsa_scsi_dev_t *device)
 	return 0;
 }
 
-/* Helper function to assign bus, target, lun mapping of devices.
- * Puts non-external target logical volumes on bus 0, external target logical
- * volumes on bus 1, physical devices on bus 2. and the hba on bus 3.
+/*
+ * Helper function to assign bus, target, lun mapping of devices.
  * Logical drive target and lun are assigned at this time, but
  * physical device lun and target assignment are deferred (assigned
  * in hpsa_find_target_lun, called by hpsa_scsi_add_entry.)
- */
+*/
 static void figure_bus_target_lun(struct ctlr_info *h,
 	u8 *lunaddrbytes, struct hpsa_scsi_dev_t *device)
 {
-	u32 lunid = le32_to_cpu(*((__le32 *) lunaddrbytes));
+	u32 lunid = get_unaligned_le32(lunaddrbytes);
 
 	if (!is_logical_dev_addr_mode(lunaddrbytes)) {
 		/* physical device, target and lun filled in later */
 		if (is_hba_lunid(lunaddrbytes))
-			hpsa_set_bus_target_lun(device, 3, 0, lunid & 0x3fff);
+			hpsa_set_bus_target_lun(device,
+					HPSA_HBA_BUS, 0, lunid & 0x3fff);
 		else
 			/* defer target, lun assignment for physical devices */
-			hpsa_set_bus_target_lun(device, 2, -1, -1);
+			hpsa_set_bus_target_lun(device,
+					HPSA_PHYSICAL_DEVICE_BUS, -1, -1);
 		return;
 	}
 	/* It's a logical device */
 	if (is_ext_target(h, device)) {
-		/* external target way, put logicals on bus 1
-		 * and match target/lun numbers box
-		 * reports, other smart array, bus 0, target 0, match lunid
-		 */
 		hpsa_set_bus_target_lun(device,
-			1, (lunid >> 16) & 0x3fff, lunid & 0x00ff);
+			HPSA_EXTERNAL_RAID_VOLUME_BUS, (lunid >> 16) & 0x3fff,
+			lunid & 0x00ff);
 		return;
 	}
-	hpsa_set_bus_target_lun(device, 0, 0, lunid & 0x3fff);
+	hpsa_set_bus_target_lun(device, HPSA_RAID_VOLUME_BUS,
+				0, lunid & 0x3fff);
 }
 
 /*

drivers/scsi/hpsa.h +0 -0

index 808b520..38d5534 100644

drivers/scsi/hpsa.h +1 -1

+
 /*
 	Send the command to the hardware
 */
 
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

From: Kevin Barnett <[email protected]>

setup for sas transport. Need to set the
bus and target accordingly.

Reviewed-by Scott Teel <[email protected]>

Reviewed-by Justin Lindley <[email protected]>

Reviewed-by Kevin Barnett <[email protected]>

Signed-off-by Don Brace <[email protected]>

---
 drivers/scsi/hpsa.c |   25 ++++++++++++-------------
 drivers/scsi/hpsa.h |    5 +++++
 2 files changed, 17 insertions(+), 13 deletions(-)