All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] hpsa update
@ 2015-12-09 21:18 Don Brace
  2015-12-09 21:18 ` [PATCH 1/3] hpsa: fix path_info_show Don Brace
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Don Brace @ 2015-12-09 21:18 UTC (permalink / raw)
  To: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley
  Cc: linux-scsi

These patches are based on Linus's tree

The changes are:
 - correct missing changes from snprintf to scnprintf
   in path_info_show by Rasmus Villemoes
 - fix reported bus for SAS transport devices
 - add in enclosure information

---

Don Brace (3):
      hpsa: fix path_info_show
      hpsa: change SAS transport devices to bus 0.
      hpsa: add box and bay information for enclosure devices


 drivers/scsi/hpsa.c     |  118 ++++++++++++++++++++++++++++++++++++++++++-----
 drivers/scsi/hpsa.h     |    2 -
 drivers/scsi/hpsa_cmd.h |   13 +++++
 3 files changed, 120 insertions(+), 13 deletions(-)

--
Signature

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] hpsa: fix path_info_show
  2015-12-09 21:18 [PATCH 0/3] hpsa update Don Brace
@ 2015-12-09 21:18 ` Don Brace
  2015-12-09 23:15   ` Matthew R. Ochs
  2015-12-09 21:18 ` [PATCH 2/3] hpsa: change SAS transport devices to bus 0 Don Brace
  2015-12-09 21:18 ` [PATCH 3/3] hpsa: add box and bay information for enclosure devices Don Brace
  2 siblings, 1 reply; 11+ messages in thread
From: Don Brace @ 2015-12-09 21:18 UTC (permalink / raw)
  To: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley
  Cc: linux-scsi

left off some changes from Rasmus Villemoes where he changed
snprintf to scnprintf

Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Justin Lindley <justin.lindley@pmcs.com>
Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com>
Reviewed-by: Scott Teel <scott.teel@pmcs.com>
Signed-off-by: Don Brace <don.brace@pmcs.com>
---
 drivers/scsi/hpsa.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index a386036..f8370b8 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -795,7 +795,7 @@ static ssize_t path_info_show(struct device *dev,
 		if (hdev->external ||
 			hdev->devtype == TYPE_RAID ||
 			is_logical_device(hdev)) {
-			output_len += snprintf(buf + output_len,
+			output_len += scnprintf(buf + output_len,
 						PAGE_SIZE - output_len,
 						"%s\n", active);
 			continue;
@@ -809,28 +809,28 @@ static ssize_t path_info_show(struct device *dev,
 		if (phys_connector[1] < '0')
 			phys_connector[1] = '0';
 		if (hdev->phys_connector[i] > 0)
-			output_len += snprintf(buf + output_len,
+			output_len += scnprintf(buf + output_len,
 				PAGE_SIZE - output_len,
 				"PORT: %.2s ",
 				phys_connector);
 		if (hdev->devtype == TYPE_DISK && hdev->expose_device) {
 			if (box == 0 || box == 0xFF) {
-				output_len += snprintf(buf + output_len,
+				output_len += scnprintf(buf + output_len,
 					PAGE_SIZE - output_len,
 					"BAY: %hhu %s\n",
 					bay, active);
 			} else {
-				output_len += snprintf(buf + output_len,
+				output_len += scnprintf(buf + output_len,
 					PAGE_SIZE - output_len,
 					"BOX: %hhu BAY: %hhu %s\n",
 					box, bay, active);
 			}
 		} else if (box != 0 && box != 0xFF) {
-			output_len += snprintf(buf + output_len,
+			output_len += scnprintf(buf + output_len,
 				PAGE_SIZE - output_len, "BOX: %hhu %s\n",
 				box, active);
 		} else
-			output_len += snprintf(buf + output_len,
+			output_len += scnprintf(buf + output_len,
 				PAGE_SIZE - output_len, "%s\n", active);
 	}
 


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] hpsa: change SAS transport devices to bus 0.
  2015-12-09 21:18 [PATCH 0/3] hpsa update Don Brace
  2015-12-09 21:18 ` [PATCH 1/3] hpsa: fix path_info_show Don Brace
@ 2015-12-09 21:18 ` Don Brace
  2015-12-09 23:21   ` Matthew R. Ochs
  2015-12-09 21:18 ` [PATCH 3/3] hpsa: add box and bay information for enclosure devices Don Brace
  2 siblings, 1 reply; 11+ messages in thread
From: Don Brace @ 2015-12-09 21:18 UTC (permalink / raw)
  To: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley
  Cc: linux-scsi

sas transport places devices on bus 0 but driver was setting
the bus to 3.

Reviewed-by: Justin Lindley <justin.lindley@pmcs.com>
Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com>
Reviewed-by: Scott Teel <scott.teel@pmcs.com>
Signed-off-by: Don Brace <don.brace@pmcs.com>
---
 drivers/scsi/hpsa.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
index ae5beda..fdd39fc 100644
--- a/drivers/scsi/hpsa.h
+++ b/drivers/scsi/hpsa.h
@@ -400,7 +400,7 @@ struct offline_device_entry {
 #define HPSA_PHYSICAL_DEVICE_BUS	0
 #define HPSA_RAID_VOLUME_BUS		1
 #define HPSA_EXTERNAL_RAID_VOLUME_BUS	2
-#define HPSA_HBA_BUS			3
+#define HPSA_HBA_BUS			0
 
 /*
 	Send the command to the hardware


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] hpsa: add box and bay information for enclosure devices
  2015-12-09 21:18 [PATCH 0/3] hpsa update Don Brace
  2015-12-09 21:18 ` [PATCH 1/3] hpsa: fix path_info_show Don Brace
  2015-12-09 21:18 ` [PATCH 2/3] hpsa: change SAS transport devices to bus 0 Don Brace
@ 2015-12-09 21:18 ` Don Brace
  2015-12-09 23:41   ` Matthew R. Ochs
  2 siblings, 1 reply; 11+ messages in thread
From: Don Brace @ 2015-12-09 21:18 UTC (permalink / raw)
  To: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley
  Cc: linux-scsi

Reviewed-by: Justin Lindley <justin.lindley@pmcs.com>
Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com>
Reviewed-by: Scott Teel <scott.teel@pmcs.com>
Signed-off-by: Don Brace <don.brace@pmcs.com>
---
 drivers/scsi/hpsa.c     |  108 ++++++++++++++++++++++++++++++++++++++++++++---
 drivers/scsi/hpsa_cmd.h |   13 ++++++
 2 files changed, 114 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index f8370b8..6f84ec7 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -750,7 +750,6 @@ static ssize_t host_show_hp_ssd_smart_path_enabled(struct device *dev,
 }
 
 #define MAX_PATHS 8
-
 static ssize_t path_info_show(struct device *dev,
 	     struct device_attribute *attr, char *buf)
 {
@@ -792,9 +791,7 @@ static ssize_t path_info_show(struct device *dev,
 				hdev->bus, hdev->target, hdev->lun,
 				scsi_device_type(hdev->devtype));
 
-		if (hdev->external ||
-			hdev->devtype == TYPE_RAID ||
-			is_logical_device(hdev)) {
+		if (hdev->devtype == TYPE_RAID || is_logical_device(hdev)) {
 			output_len += scnprintf(buf + output_len,
 						PAGE_SIZE - output_len,
 						"%s\n", active);
@@ -808,8 +805,7 @@ static ssize_t path_info_show(struct device *dev,
 			phys_connector[0] = '0';
 		if (phys_connector[1] < '0')
 			phys_connector[1] = '0';
-		if (hdev->phys_connector[i] > 0)
-			output_len += scnprintf(buf + output_len,
+		output_len += scnprintf(buf + output_len,
 				PAGE_SIZE - output_len,
 				"PORT: %.2s ",
 				phys_connector);
@@ -3191,6 +3187,90 @@ out:
 	return rc;
 }
 
+/*
+ * get enclosure information
+ * struct ReportExtendedLUNdata *rlep - Used for BMIC drive number
+ * struct hpsa_scsi_dev_t *encl_dev - device entry for enclosure
+ * Uses id_physical_device to determine the box_index.
+ */
+static int hpsa_get_enclosure_info(struct ctlr_info *h,
+			unsigned char *scsi3addr,
+			struct ReportExtendedLUNdata *rlep, int rle_index,
+			struct hpsa_scsi_dev_t *encl_dev)
+{
+	int rc = -1;
+	struct CommandList *c = NULL;
+	struct ErrorInfo *ei = NULL;
+	struct bmic_sense_storage_box_params *bssbp = NULL;
+	struct bmic_identify_physical_device *id_phys = NULL;
+	struct ext_report_lun_entry *rle = &rlep->LUN[rle_index];
+	u16 bmic_device_index = 0;
+
+
+	bssbp = kzalloc(sizeof(*bssbp), GFP_KERNEL);
+	if (!bssbp)
+		goto out;
+
+	id_phys = kzalloc(sizeof(*id_phys), GFP_KERNEL);
+	if (!id_phys)
+		goto out;
+
+	bmic_device_index = GET_BMIC_DRIVE_NUMBER(&rle->lunid[0]);
+
+	if (bmic_device_index == 0xFF00)
+		goto out;
+
+	memset(id_phys, 0, sizeof(*id_phys));
+	rc = hpsa_bmic_id_physical_device(h, scsi3addr, bmic_device_index,
+						id_phys, sizeof(*id_phys));
+	if (rc) {
+		dev_warn(&h->pdev->dev, "%s: id_phys failed %d bdi[0x%x]\n",
+			__func__, encl_dev->external, bmic_device_index);
+		goto out;
+	}
+
+	c = cmd_alloc(h);
+
+	rc = fill_cmd(c, BMIC_SENSE_STORAGE_BOX_PARAMS, h, bssbp,
+			sizeof(*bssbp), 0, RAID_CTLR_LUNID, TYPE_CMD);
+
+	if (rc)
+		goto out;
+
+	if (id_phys->phys_connector[1] == 'E')
+		c->Request.CDB[5] = id_phys->box_index;
+	else
+		c->Request.CDB[5] = 0;
+
+	rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, PCI_DMA_FROMDEVICE,
+						NO_TIMEOUT);
+	if (rc)
+		goto out;
+
+	ei = c->err_info;
+	if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) {
+		rc = -1;
+		goto out;
+	}
+
+	encl_dev->box[id_phys->active_path_number] = bssbp->phys_box_on_port;
+	memcpy(&encl_dev->phys_connector[id_phys->active_path_number],
+		bssbp->phys_connector, sizeof(bssbp->phys_connector));
+
+	rc = IO_OK;
+out:
+	kfree(bssbp);
+	kfree(id_phys);
+
+	if (c)
+		cmd_free(h, c);
+
+	if (rc != IO_OK)
+		hpsa_show_dev_msg(KERN_INFO, h, encl_dev,
+			"Error, could not get enclosure information\n");
+	return rc;
+}
+
 static u64 hpsa_get_sas_address_from_report_physical(struct ctlr_info *h,
 						unsigned char *scsi3addr)
 {
@@ -4032,7 +4112,8 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
 
 		/* skip masked non-disk devices */
 		if (MASKED_DEVICE(lunaddrbytes) && physical_device &&
-			(physdev_list->LUN[phys_dev_index].device_flags & 0x01))
+		   (physdev_list->LUN[phys_dev_index].device_type != 0x06) &&
+		   (physdev_list->LUN[phys_dev_index].device_flags & 0x01))
 			continue;
 
 		/* Get device type, vendor, model, device id */
@@ -4117,6 +4198,9 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
 		case TYPE_TAPE:
 		case TYPE_MEDIUM_CHANGER:
 		case TYPE_ENCLOSURE:
+			hpsa_get_enclosure_info(h, lunaddrbytes,
+						physdev_list, phys_dev_index,
+						this_device);
 			ncurrent++;
 			break;
 		case TYPE_RAID:
@@ -6629,6 +6713,16 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
 			c->Request.CDB[7] = (size >> 16) & 0xFF;
 			c->Request.CDB[8] = (size >> 8) & 0XFF;
 			break;
+		case BMIC_SENSE_STORAGE_BOX_PARAMS:
+			c->Request.CDBLen = 10;
+			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_STORAGE_BOX_PARAMS;
+			c->Request.CDB[7] = (size >> 16) & 0xFF;
+			c->Request.CDB[8] = (size >> 8) & 0XFF;
+			break;
 		case BMIC_IDENTIFY_CONTROLLER:
 			c->Request.CDBLen = 10;
 			c->Request.type_attr_dir =
diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
index d92ef0d..6a919ad 100644
--- a/drivers/scsi/hpsa_cmd.h
+++ b/drivers/scsi/hpsa_cmd.h
@@ -291,6 +291,7 @@ struct SenseSubsystem_info {
 #define BMIC_SENSE_DIAG_OPTIONS 0xF5
 #define HPSA_DIAG_OPTS_DISABLE_RLD_CACHING 0x40000000
 #define BMIC_SENSE_SUBSYSTEM_INFORMATION 0x66
+#define BMIC_SENSE_STORAGE_BOX_PARAMS 0x65
 
 /* Command List Structure */
 union SCSI3Addr {
@@ -842,5 +843,17 @@ struct bmic_sense_subsystem_info {
 	u8	pad[332];
 };
 
+struct bmic_sense_storage_box_params {
+	u8	reserved[36];
+	u8	inquiry_valid;
+	u8	reserved_1[68];
+	u8	phys_box_on_port;
+	u8	reserved_2[22];
+	u16	connection_info;
+	u8	reserver_3[84];
+	u8	phys_connector[2];
+	u8	reserved_4[296];
+};
+
 #pragma pack()
 #endif /* HPSA_CMD_H */


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] hpsa: fix path_info_show
  2015-12-09 21:18 ` [PATCH 1/3] hpsa: fix path_info_show Don Brace
@ 2015-12-09 23:15   ` Matthew R. Ochs
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew R. Ochs @ 2015-12-09 23:15 UTC (permalink / raw)
  To: Don Brace
  Cc: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley, linux-scsi

Reviewed-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] hpsa: change SAS transport devices to bus 0.
  2015-12-09 21:18 ` [PATCH 2/3] hpsa: change SAS transport devices to bus 0 Don Brace
@ 2015-12-09 23:21   ` Matthew R. Ochs
  2015-12-15 15:25     ` Don Brace
  0 siblings, 1 reply; 11+ messages in thread
From: Matthew R. Ochs @ 2015-12-09 23:21 UTC (permalink / raw)
  To: Don Brace
  Cc: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley, linux-scsi

> On Dec 9, 2015, at 3:18 PM, Don Brace <don.brace@pmcs.com> wrote:
> 
> sas transport places devices on bus 0 but driver was setting
> the bus to 3.
> 
> Reviewed-by: Justin Lindley <justin.lindley@pmcs.com>
> Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com>
> Reviewed-by: Scott Teel <scott.teel@pmcs.com>
> Signed-off-by: Don Brace <don.brace@pmcs.com>
> ---
> drivers/scsi/hpsa.h |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
> index ae5beda..fdd39fc 100644
> --- a/drivers/scsi/hpsa.h
> +++ b/drivers/scsi/hpsa.h
> @@ -400,7 +400,7 @@ struct offline_device_entry {
> #define HPSA_PHYSICAL_DEVICE_BUS	0
> #define HPSA_RAID_VOLUME_BUS		1
> #define HPSA_EXTERNAL_RAID_VOLUME_BUS	2
> -#define HPSA_HBA_BUS			3
> +#define HPSA_HBA_BUS			0

Is this not the same as using HPSA_PHYSICAL_DEVICE_BUS?


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] hpsa: add box and bay information for enclosure devices
  2015-12-09 21:18 ` [PATCH 3/3] hpsa: add box and bay information for enclosure devices Don Brace
@ 2015-12-09 23:41   ` Matthew R. Ochs
  2015-12-18 20:19     ` Matthew R. Ochs
  0 siblings, 1 reply; 11+ messages in thread
From: Matthew R. Ochs @ 2015-12-09 23:41 UTC (permalink / raw)
  To: Don Brace
  Cc: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley, linux-scsi

> On Dec 9, 2015, at 3:18 PM, Don Brace <don.brace@pmcs.com> wrote:

No commit message?

> 
> Reviewed-by: Justin Lindley <justin.lindley@pmcs.com>
> Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com>
> Reviewed-by: Scott Teel <scott.teel@pmcs.com>
> Signed-off-by: Don Brace <don.brace@pmcs.com>
> ---
> drivers/scsi/hpsa.c     |  108 ++++++++++++++++++++++++++++++++++++++++++++---
> drivers/scsi/hpsa_cmd.h |   13 ++++++
> 2 files changed, 114 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index f8370b8..6f84ec7 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -750,7 +750,6 @@ static ssize_t host_show_hp_ssd_smart_path_enabled(struct device *dev,
> }
> 
> #define MAX_PATHS 8
> -
> static ssize_t path_info_show(struct device *dev,
> 	     struct device_attribute *attr, char *buf)
> {
> @@ -792,9 +791,7 @@ static ssize_t path_info_show(struct device *dev,
> 				hdev->bus, hdev->target, hdev->lun,
> 				scsi_device_type(hdev->devtype));
> 
> -		if (hdev->external ||
> -			hdev->devtype == TYPE_RAID ||
> -			is_logical_device(hdev)) {
> +		if (hdev->devtype == TYPE_RAID || is_logical_device(hdev)) {

How does removing the check for external here relate to the rest of this commit?

> 			output_len += scnprintf(buf + output_len,
> 						PAGE_SIZE - output_len,
> 						"%s\n", active);
> @@ -808,8 +805,7 @@ static ssize_t path_info_show(struct device *dev,
> 			phys_connector[0] = '0';
> 		if (phys_connector[1] < '0')
> 			phys_connector[1] = '0';
> -		if (hdev->phys_connector[i] > 0)
> -			output_len += scnprintf(buf + output_len,
> +		output_len += scnprintf(buf + output_len,

Same question regarding the removal of the > 0 check.

> 				PAGE_SIZE - output_len,
> 				"PORT: %.2s ",
> 				phys_connector);
> @@ -3191,6 +3187,90 @@ out:
> 	return rc;
> }
> 
> +/*
> + * get enclosure information
> + * struct ReportExtendedLUNdata *rlep - Used for BMIC drive number
> + * struct hpsa_scsi_dev_t *encl_dev - device entry for enclosure
> + * Uses id_physical_device to determine the box_index.
> + */
> +static int hpsa_get_enclosure_info(struct ctlr_info *h,
> +			unsigned char *scsi3addr,
> +			struct ReportExtendedLUNdata *rlep, int rle_index,
> +			struct hpsa_scsi_dev_t *encl_dev)
> +{
> +	int rc = -1;
> +	struct CommandList *c = NULL;
> +	struct ErrorInfo *ei = NULL;
> +	struct bmic_sense_storage_box_params *bssbp = NULL;
> +	struct bmic_identify_physical_device *id_phys = NULL;
> +	struct ext_report_lun_entry *rle = &rlep->LUN[rle_index];
> +	u16 bmic_device_index = 0;
> +
> +
> +	bssbp = kzalloc(sizeof(*bssbp), GFP_KERNEL);
> +	if (!bssbp)
> +		goto out;
> +
> +	id_phys = kzalloc(sizeof(*id_phys), GFP_KERNEL);
> +	if (!id_phys)
> +		goto out;
> +
> +	bmic_device_index = GET_BMIC_DRIVE_NUMBER(&rle->lunid[0]);
> +
> +	if (bmic_device_index == 0xFF00)
> +		goto out;

Why not put this check before the memory allocations so you can
avoid them if this condition is not met?

> +
> +	memset(id_phys, 0, sizeof(*id_phys));

Didn't you just obtain zeroed memory from kzalloc()?

> +	rc = hpsa_bmic_id_physical_device(h, scsi3addr, bmic_device_index,
> +						id_phys, sizeof(*id_phys));
> +	if (rc) {
> +		dev_warn(&h->pdev->dev, "%s: id_phys failed %d bdi[0x%x]\n",
> +			__func__, encl_dev->external, bmic_device_index);
> +		goto out;
> +	}
> +
> +	c = cmd_alloc(h);
> +
> +	rc = fill_cmd(c, BMIC_SENSE_STORAGE_BOX_PARAMS, h, bssbp,
> +			sizeof(*bssbp), 0, RAID_CTLR_LUNID, TYPE_CMD);
> +
> +	if (rc)
> +		goto out;
> +
> +	if (id_phys->phys_connector[1] == 'E')
> +		c->Request.CDB[5] = id_phys->box_index;
> +	else
> +		c->Request.CDB[5] = 0;
> +
> +	rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, PCI_DMA_FROMDEVICE,
> +						NO_TIMEOUT);
> +	if (rc)
> +		goto out;
> +
> +	ei = c->err_info;
> +	if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) {
> +		rc = -1;
> +		goto out;
> +	}
> +
> +	encl_dev->box[id_phys->active_path_number] = bssbp->phys_box_on_port;
> +	memcpy(&encl_dev->phys_connector[id_phys->active_path_number],
> +		bssbp->phys_connector, sizeof(bssbp->phys_connector));
> +
> +	rc = IO_OK;
> +out:
> +	kfree(bssbp);
> +	kfree(id_phys);
> +
> +	if (c)
> +		cmd_free(h, c);
> +
> +	if (rc != IO_OK)
> +		hpsa_show_dev_msg(KERN_INFO, h, encl_dev,
> +			"Error, could not get enclosure information\n");
> +	return rc;
> +}
> +
> static u64 hpsa_get_sas_address_from_report_physical(struct ctlr_info *h,
> 						unsigned char *scsi3addr)
> {
> @@ -4032,7 +4112,8 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
> 
> 		/* skip masked non-disk devices */
> 		if (MASKED_DEVICE(lunaddrbytes) && physical_device &&
> -			(physdev_list->LUN[phys_dev_index].device_flags & 0x01))
> +		   (physdev_list->LUN[phys_dev_index].device_type != 0x06) &&
> +		   (physdev_list->LUN[phys_dev_index].device_flags & 0x01))
> 			continue;
> 
> 		/* Get device type, vendor, model, device id */
> @@ -4117,6 +4198,9 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
> 		case TYPE_TAPE:
> 		case TYPE_MEDIUM_CHANGER:

Is it 'okay' that these two types are falling through and will call this new
enclosure info routine?

> 		case TYPE_ENCLOSURE:
> +			hpsa_get_enclosure_info(h, lunaddrbytes,
> +						physdev_list, phys_dev_index,
> +						this_device);

Any reason this routine wasn't made a void? The return code is not being used
and the other similar helpers in this path are void.

> 			ncurrent++;
> 			break;
> 		case TYPE_RAID:
> @@ -6629,6 +6713,16 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
> 			c->Request.CDB[7] = (size >> 16) & 0xFF;
> 			c->Request.CDB[8] = (size >> 8) & 0XFF;
> 			break;
> +		case BMIC_SENSE_STORAGE_BOX_PARAMS:
> +			c->Request.CDBLen = 10;
> +			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_STORAGE_BOX_PARAMS;
> +			c->Request.CDB[7] = (size >> 16) & 0xFF;
> +			c->Request.CDB[8] = (size >> 8) & 0XFF;
> +			break;
> 		case BMIC_IDENTIFY_CONTROLLER:
> 			c->Request.CDBLen = 10;
> 			c->Request.type_attr_dir =
> diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
> index d92ef0d..6a919ad 100644
> --- a/drivers/scsi/hpsa_cmd.h
> +++ b/drivers/scsi/hpsa_cmd.h
> @@ -291,6 +291,7 @@ struct SenseSubsystem_info {
> #define BMIC_SENSE_DIAG_OPTIONS 0xF5
> #define HPSA_DIAG_OPTS_DISABLE_RLD_CACHING 0x40000000
> #define BMIC_SENSE_SUBSYSTEM_INFORMATION 0x66
> +#define BMIC_SENSE_STORAGE_BOX_PARAMS 0x65
> 
> /* Command List Structure */
> union SCSI3Addr {
> @@ -842,5 +843,17 @@ struct bmic_sense_subsystem_info {
> 	u8	pad[332];
> };
> 
> +struct bmic_sense_storage_box_params {
> +	u8	reserved[36];
> +	u8	inquiry_valid;
> +	u8	reserved_1[68];
> +	u8	phys_box_on_port;
> +	u8	reserved_2[22];
> +	u16	connection_info;
> +	u8	reserver_3[84];
> +	u8	phys_connector[2];
> +	u8	reserved_4[296];
> +};
> +
> #pragma pack()
> #endif /* HPSA_CMD_H */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] hpsa: change SAS transport devices to bus 0.
  2015-12-09 23:21   ` Matthew R. Ochs
@ 2015-12-15 15:25     ` Don Brace
  2015-12-18 20:14       ` Matthew R. Ochs
  0 siblings, 1 reply; 11+ messages in thread
From: Don Brace @ 2015-12-15 15:25 UTC (permalink / raw)
  To: Matthew R. Ochs
  Cc: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley, linux-scsi

On 12/09/2015 05:21 PM, Matthew R. Ochs wrote:
>> On Dec 9, 2015, at 3:18 PM, Don Brace <don.brace@pmcs.com> wrote:
>>
>> sas transport places devices on bus 0 but driver was setting
>> the bus to 3.
>>
>> Reviewed-by: Justin Lindley <justin.lindley@pmcs.com>
>> Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com>
>> Reviewed-by: Scott Teel <scott.teel@pmcs.com>
>> Signed-off-by: Don Brace <don.brace@pmcs.com>
>> ---
>> drivers/scsi/hpsa.h |    2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
>> index ae5beda..fdd39fc 100644
>> --- a/drivers/scsi/hpsa.h
>> +++ b/drivers/scsi/hpsa.h
>> @@ -400,7 +400,7 @@ struct offline_device_entry {
>> #define HPSA_PHYSICAL_DEVICE_BUS	0
>> #define HPSA_RAID_VOLUME_BUS		1
>> #define HPSA_EXTERNAL_RAID_VOLUME_BUS	2
>> -#define HPSA_HBA_BUS			3
>> +#define HPSA_HBA_BUS			0
> Is this not the same as using HPSA_PHYSICAL_DEVICE_BUS?
I was just trying to minimize the changes. I can update the driver
is necessary.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] hpsa: change SAS transport devices to bus 0.
  2015-12-15 15:25     ` Don Brace
@ 2015-12-18 20:14       ` Matthew R. Ochs
  0 siblings, 0 replies; 11+ messages in thread
From: Matthew R. Ochs @ 2015-12-18 20:14 UTC (permalink / raw)
  To: Don Brace
  Cc: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley, linux-scsi

> On Dec 15, 2015, at 9:25 AM, Don Brace <brace77070@gmail.com> wrote:
> 
> On 12/09/2015 05:21 PM, Matthew R. Ochs wrote:
>>> On Dec 9, 2015, at 3:18 PM, Don Brace <don.brace@pmcs.com> wrote:
>>> 
>>> sas transport places devices on bus 0 but driver was setting
>>> the bus to 3.
>>> 
>>> Reviewed-by: Justin Lindley <justin.lindley@pmcs.com>
>>> Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com>
>>> Reviewed-by: Scott Teel <scott.teel@pmcs.com>
>>> Signed-off-by: Don Brace <don.brace@pmcs.com>
>>> ---
>>> drivers/scsi/hpsa.h |    2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/scsi/hpsa.h b/drivers/scsi/hpsa.h
>>> index ae5beda..fdd39fc 100644
>>> --- a/drivers/scsi/hpsa.h
>>> +++ b/drivers/scsi/hpsa.h
>>> @@ -400,7 +400,7 @@ struct offline_device_entry {
>>> #define HPSA_PHYSICAL_DEVICE_BUS	0
>>> #define HPSA_RAID_VOLUME_BUS		1
>>> #define HPSA_EXTERNAL_RAID_VOLUME_BUS	2
>>> -#define HPSA_HBA_BUS			3
>>> +#define HPSA_HBA_BUS			0
>> Is this not the same as using HPSA_PHYSICAL_DEVICE_BUS?
> I was just trying to minimize the changes. I can update the driver
> is necessary.

Understandable.

Reviewed-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] hpsa: add box and bay information for enclosure devices
  2015-12-09 23:41   ` Matthew R. Ochs
@ 2015-12-18 20:19     ` Matthew R. Ochs
  2015-12-21 17:09       ` Don Brace
  0 siblings, 1 reply; 11+ messages in thread
From: Matthew R. Ochs @ 2015-12-18 20:19 UTC (permalink / raw)
  To: Don Brace
  Cc: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley, linux-scsi

Hi Don,

Did you see these comments I had from my review of this patch?


-matt

> On Dec 9, 2015, at 5:41 PM, Matthew R. Ochs <mrochs@linux.vnet.ibm.com> wrote:
> 
>> On Dec 9, 2015, at 3:18 PM, Don Brace <don.brace@pmcs.com> wrote:
> 
> No commit message?

You can disregard this one as I saw you added a message in v1.

> 
>> 
>> Reviewed-by: Justin Lindley <justin.lindley@pmcs.com>
>> Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com>
>> Reviewed-by: Scott Teel <scott.teel@pmcs.com>
>> Signed-off-by: Don Brace <don.brace@pmcs.com>
>> ---
>> drivers/scsi/hpsa.c     |  108 ++++++++++++++++++++++++++++++++++++++++++++---
>> drivers/scsi/hpsa_cmd.h |   13 ++++++
>> 2 files changed, 114 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
>> index f8370b8..6f84ec7 100644
>> --- a/drivers/scsi/hpsa.c
>> +++ b/drivers/scsi/hpsa.c
>> @@ -750,7 +750,6 @@ static ssize_t host_show_hp_ssd_smart_path_enabled(struct device *dev,
>> }
>> 
>> #define MAX_PATHS 8
>> -
>> static ssize_t path_info_show(struct device *dev,
>> 	     struct device_attribute *attr, char *buf)
>> {
>> @@ -792,9 +791,7 @@ static ssize_t path_info_show(struct device *dev,
>> 				hdev->bus, hdev->target, hdev->lun,
>> 				scsi_device_type(hdev->devtype));
>> 
>> -		if (hdev->external ||
>> -			hdev->devtype == TYPE_RAID ||
>> -			is_logical_device(hdev)) {
>> +		if (hdev->devtype == TYPE_RAID || is_logical_device(hdev)) {
> 
> How does removing the check for external here relate to the rest of this commit?
> 
>> 			output_len += scnprintf(buf + output_len,
>> 						PAGE_SIZE - output_len,
>> 						"%s\n", active);
>> @@ -808,8 +805,7 @@ static ssize_t path_info_show(struct device *dev,
>> 			phys_connector[0] = '0';
>> 		if (phys_connector[1] < '0')
>> 			phys_connector[1] = '0';
>> -		if (hdev->phys_connector[i] > 0)
>> -			output_len += scnprintf(buf + output_len,
>> +		output_len += scnprintf(buf + output_len,
> 
> Same question regarding the removal of the > 0 check.
> 
>> 				PAGE_SIZE - output_len,
>> 				"PORT: %.2s ",
>> 				phys_connector);
>> @@ -3191,6 +3187,90 @@ out:
>> 	return rc;
>> }
>> 
>> +/*
>> + * get enclosure information
>> + * struct ReportExtendedLUNdata *rlep - Used for BMIC drive number
>> + * struct hpsa_scsi_dev_t *encl_dev - device entry for enclosure
>> + * Uses id_physical_device to determine the box_index.
>> + */
>> +static int hpsa_get_enclosure_info(struct ctlr_info *h,
>> +			unsigned char *scsi3addr,
>> +			struct ReportExtendedLUNdata *rlep, int rle_index,
>> +			struct hpsa_scsi_dev_t *encl_dev)
>> +{
>> +	int rc = -1;
>> +	struct CommandList *c = NULL;
>> +	struct ErrorInfo *ei = NULL;
>> +	struct bmic_sense_storage_box_params *bssbp = NULL;
>> +	struct bmic_identify_physical_device *id_phys = NULL;
>> +	struct ext_report_lun_entry *rle = &rlep->LUN[rle_index];
>> +	u16 bmic_device_index = 0;
>> +
>> +
>> +	bssbp = kzalloc(sizeof(*bssbp), GFP_KERNEL);
>> +	if (!bssbp)
>> +		goto out;
>> +
>> +	id_phys = kzalloc(sizeof(*id_phys), GFP_KERNEL);
>> +	if (!id_phys)
>> +		goto out;
>> +
>> +	bmic_device_index = GET_BMIC_DRIVE_NUMBER(&rle->lunid[0]);
>> +
>> +	if (bmic_device_index == 0xFF00)
>> +		goto out;
> 
> Why not put this check before the memory allocations so you can
> avoid them if this condition is not met?
> 
>> +
>> +	memset(id_phys, 0, sizeof(*id_phys));
> 
> Didn't you just obtain zeroed memory from kzalloc()?
> 
>> +	rc = hpsa_bmic_id_physical_device(h, scsi3addr, bmic_device_index,
>> +						id_phys, sizeof(*id_phys));
>> +	if (rc) {
>> +		dev_warn(&h->pdev->dev, "%s: id_phys failed %d bdi[0x%x]\n",
>> +			__func__, encl_dev->external, bmic_device_index);
>> +		goto out;
>> +	}
>> +
>> +	c = cmd_alloc(h);
>> +
>> +	rc = fill_cmd(c, BMIC_SENSE_STORAGE_BOX_PARAMS, h, bssbp,
>> +			sizeof(*bssbp), 0, RAID_CTLR_LUNID, TYPE_CMD);
>> +
>> +	if (rc)
>> +		goto out;
>> +
>> +	if (id_phys->phys_connector[1] == 'E')
>> +		c->Request.CDB[5] = id_phys->box_index;
>> +	else
>> +		c->Request.CDB[5] = 0;
>> +
>> +	rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, PCI_DMA_FROMDEVICE,
>> +						NO_TIMEOUT);
>> +	if (rc)
>> +		goto out;
>> +
>> +	ei = c->err_info;
>> +	if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) {
>> +		rc = -1;
>> +		goto out;
>> +	}
>> +
>> +	encl_dev->box[id_phys->active_path_number] = bssbp->phys_box_on_port;
>> +	memcpy(&encl_dev->phys_connector[id_phys->active_path_number],
>> +		bssbp->phys_connector, sizeof(bssbp->phys_connector));
>> +
>> +	rc = IO_OK;
>> +out:
>> +	kfree(bssbp);
>> +	kfree(id_phys);
>> +
>> +	if (c)
>> +		cmd_free(h, c);
>> +
>> +	if (rc != IO_OK)
>> +		hpsa_show_dev_msg(KERN_INFO, h, encl_dev,
>> +			"Error, could not get enclosure information\n");
>> +	return rc;
>> +}
>> +
>> static u64 hpsa_get_sas_address_from_report_physical(struct ctlr_info *h,
>> 						unsigned char *scsi3addr)
>> {
>> @@ -4032,7 +4112,8 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
>> 
>> 		/* skip masked non-disk devices */
>> 		if (MASKED_DEVICE(lunaddrbytes) && physical_device &&
>> -			(physdev_list->LUN[phys_dev_index].device_flags & 0x01))
>> +		   (physdev_list->LUN[phys_dev_index].device_type != 0x06) &&
>> +		   (physdev_list->LUN[phys_dev_index].device_flags & 0x01))
>> 			continue;
>> 
>> 		/* Get device type, vendor, model, device id */
>> @@ -4117,6 +4198,9 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
>> 		case TYPE_TAPE:
>> 		case TYPE_MEDIUM_CHANGER:
> 
> Is it 'okay' that these two types are falling through and will call this new
> enclosure info routine?
> 
>> 		case TYPE_ENCLOSURE:
>> +			hpsa_get_enclosure_info(h, lunaddrbytes,
>> +						physdev_list, phys_dev_index,
>> +						this_device);
> 
> Any reason this routine wasn't made a void? The return code is not being used
> and the other similar helpers in this path are void.
> 
>> 			ncurrent++;
>> 			break;
>> 		case TYPE_RAID:
>> @@ -6629,6 +6713,16 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
>> 			c->Request.CDB[7] = (size >> 16) & 0xFF;
>> 			c->Request.CDB[8] = (size >> 8) & 0XFF;
>> 			break;
>> +		case BMIC_SENSE_STORAGE_BOX_PARAMS:
>> +			c->Request.CDBLen = 10;
>> +			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_STORAGE_BOX_PARAMS;
>> +			c->Request.CDB[7] = (size >> 16) & 0xFF;
>> +			c->Request.CDB[8] = (size >> 8) & 0XFF;
>> +			break;
>> 		case BMIC_IDENTIFY_CONTROLLER:
>> 			c->Request.CDBLen = 10;
>> 			c->Request.type_attr_dir =
>> diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
>> index d92ef0d..6a919ad 100644
>> --- a/drivers/scsi/hpsa_cmd.h
>> +++ b/drivers/scsi/hpsa_cmd.h
>> @@ -291,6 +291,7 @@ struct SenseSubsystem_info {
>> #define BMIC_SENSE_DIAG_OPTIONS 0xF5
>> #define HPSA_DIAG_OPTS_DISABLE_RLD_CACHING 0x40000000
>> #define BMIC_SENSE_SUBSYSTEM_INFORMATION 0x66
>> +#define BMIC_SENSE_STORAGE_BOX_PARAMS 0x65
>> 
>> /* Command List Structure */
>> union SCSI3Addr {
>> @@ -842,5 +843,17 @@ struct bmic_sense_subsystem_info {
>> 	u8	pad[332];
>> };
>> 
>> +struct bmic_sense_storage_box_params {
>> +	u8	reserved[36];
>> +	u8	inquiry_valid;
>> +	u8	reserved_1[68];
>> +	u8	phys_box_on_port;
>> +	u8	reserved_2[22];
>> +	u16	connection_info;
>> +	u8	reserver_3[84];
>> +	u8	phys_connector[2];
>> +	u8	reserved_4[296];
>> +};
>> +
>> #pragma pack()
>> #endif /* HPSA_CMD_H */
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] hpsa: add box and bay information for enclosure devices
  2015-12-18 20:19     ` Matthew R. Ochs
@ 2015-12-21 17:09       ` Don Brace
  0 siblings, 0 replies; 11+ messages in thread
From: Don Brace @ 2015-12-21 17:09 UTC (permalink / raw)
  To: Matthew R. Ochs
  Cc: Viswas.G, scott.teel, Kevin.Barnett, scott.benesh,
	Mahesh.Rajashekhara, hch, Justin.Lindley, elliott,
	james.bottomley, linux-scsi



On 12/18/2015 02:19 PM, Matthew R. Ochs wrote:
> Hi Don,
>
> Did you see these comments I had from my review of this patch?
>
>
> -matt
>
>> On Dec 9, 2015, at 5:41 PM, Matthew R. Ochs <mrochs@linux.vnet.ibm.com> wrote:
>>
>>> On Dec 9, 2015, at 3:18 PM, Don Brace <don.brace@pmcs.com> wrote:
>> No commit message?
> You can disregard this one as I saw you added a message in v1.
>
>>> Reviewed-by: Justin Lindley <justin.lindley@pmcs.com>
>>> Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com>
>>> Reviewed-by: Scott Teel <scott.teel@pmcs.com>
>>> Signed-off-by: Don Brace <don.brace@pmcs.com>
>>> ---
>>> drivers/scsi/hpsa.c     |  108 ++++++++++++++++++++++++++++++++++++++++++++---
>>> drivers/scsi/hpsa_cmd.h |   13 ++++++
>>> 2 files changed, 114 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
>>> index f8370b8..6f84ec7 100644
>>> --- a/drivers/scsi/hpsa.c
>>> +++ b/drivers/scsi/hpsa.c
>>> @@ -750,7 +750,6 @@ static ssize_t host_show_hp_ssd_smart_path_enabled(struct device *dev,
>>> }
>>>
>>> #define MAX_PATHS 8
>>> -
>>> static ssize_t path_info_show(struct device *dev,
>>> 	     struct device_attribute *attr, char *buf)
>>> {
>>> @@ -792,9 +791,7 @@ static ssize_t path_info_show(struct device *dev,
>>> 				hdev->bus, hdev->target, hdev->lun,
>>> 				scsi_device_type(hdev->devtype));
>>>
>>> -		if (hdev->external ||
>>> -			hdev->devtype == TYPE_RAID ||
>>> -			is_logical_device(hdev)) {
>>> +		if (hdev->devtype == TYPE_RAID || is_logical_device(hdev)) {
>> How does removing the check for external here relate to the rest of this commit?
We have enclosure devices that would not show up properly if this check
was not removed.
>>
>>> 			output_len += scnprintf(buf + output_len,
>>> 						PAGE_SIZE - output_len,
>>> 						"%s\n", active);
>>> @@ -808,8 +805,7 @@ static ssize_t path_info_show(struct device *dev,
>>> 			phys_connector[0] = '0';
>>> 		if (phys_connector[1] < '0')
>>> 			phys_connector[1] = '0';
>>> -		if (hdev->phys_connector[i] > 0)
>>> -			output_len += scnprintf(buf + output_len,
>>> +		output_len += scnprintf(buf + output_len,
>> Same question regarding the removal of the > 0 check.
The connector contains alpha-numeric data. We were missing
connector info like "2I" "2E", ...
>>
>>> 				PAGE_SIZE - output_len,
>>> 				"PORT: %.2s ",
>>> 				phys_connector);
>>> @@ -3191,6 +3187,90 @@ out:
>>> 	return rc;
>>> }
>>>
>>> +/*
>>> + * get enclosure information
>>> + * struct ReportExtendedLUNdata *rlep - Used for BMIC drive number
>>> + * struct hpsa_scsi_dev_t *encl_dev - device entry for enclosure
>>> + * Uses id_physical_device to determine the box_index.
>>> + */
>>> +static int hpsa_get_enclosure_info(struct ctlr_info *h,
>>> +			unsigned char *scsi3addr,
>>> +			struct ReportExtendedLUNdata *rlep, int rle_index,
>>> +			struct hpsa_scsi_dev_t *encl_dev)
>>> +{
>>> +	int rc = -1;
>>> +	struct CommandList *c = NULL;
>>> +	struct ErrorInfo *ei = NULL;
>>> +	struct bmic_sense_storage_box_params *bssbp = NULL;
>>> +	struct bmic_identify_physical_device *id_phys = NULL;
>>> +	struct ext_report_lun_entry *rle = &rlep->LUN[rle_index];
>>> +	u16 bmic_device_index = 0;
>>> +
>>> +
>>> +	bssbp = kzalloc(sizeof(*bssbp), GFP_KERNEL);
>>> +	if (!bssbp)
>>> +		goto out;
>>> +
>>> +	id_phys = kzalloc(sizeof(*id_phys), GFP_KERNEL);
>>> +	if (!id_phys)
>>> +		goto out;
>>> +
>>> +	bmic_device_index = GET_BMIC_DRIVE_NUMBER(&rle->lunid[0]);
>>> +
>>> +	if (bmic_device_index == 0xFF00)
>>> +		goto out;
>> Why not put this check before the memory allocations so you can
>> avoid them if this condition is not met?
Good point. I will do this for V3.
>>
>>> +
>>> +	memset(id_phys, 0, sizeof(*id_phys));
>> Didn't you just obtain zeroed memory from kzalloc()?
Another good catch.
>>
>>> +	rc = hpsa_bmic_id_physical_device(h, scsi3addr, bmic_device_index,
>>> +						id_phys, sizeof(*id_phys));
>>> +	if (rc) {
>>> +		dev_warn(&h->pdev->dev, "%s: id_phys failed %d bdi[0x%x]\n",
>>> +			__func__, encl_dev->external, bmic_device_index);
>>> +		goto out;
>>> +	}
>>> +
>>> +	c = cmd_alloc(h);
>>> +
>>> +	rc = fill_cmd(c, BMIC_SENSE_STORAGE_BOX_PARAMS, h, bssbp,
>>> +			sizeof(*bssbp), 0, RAID_CTLR_LUNID, TYPE_CMD);
>>> +
>>> +	if (rc)
>>> +		goto out;
>>> +
>>> +	if (id_phys->phys_connector[1] == 'E')
>>> +		c->Request.CDB[5] = id_phys->box_index;
>>> +	else
>>> +		c->Request.CDB[5] = 0;
>>> +
>>> +	rc = hpsa_scsi_do_simple_cmd_with_retry(h, c, PCI_DMA_FROMDEVICE,
>>> +						NO_TIMEOUT);
>>> +	if (rc)
>>> +		goto out;
>>> +
>>> +	ei = c->err_info;
>>> +	if (ei->CommandStatus != 0 && ei->CommandStatus != CMD_DATA_UNDERRUN) {
>>> +		rc = -1;
>>> +		goto out;
>>> +	}
>>> +
>>> +	encl_dev->box[id_phys->active_path_number] = bssbp->phys_box_on_port;
>>> +	memcpy(&encl_dev->phys_connector[id_phys->active_path_number],
>>> +		bssbp->phys_connector, sizeof(bssbp->phys_connector));
>>> +
>>> +	rc = IO_OK;
>>> +out:
>>> +	kfree(bssbp);
>>> +	kfree(id_phys);
>>> +
>>> +	if (c)
>>> +		cmd_free(h, c);
>>> +
>>> +	if (rc != IO_OK)
>>> +		hpsa_show_dev_msg(KERN_INFO, h, encl_dev,
>>> +			"Error, could not get enclosure information\n");
>>> +	return rc;
>>> +}
>>> +
>>> static u64 hpsa_get_sas_address_from_report_physical(struct ctlr_info *h,
>>> 						unsigned char *scsi3addr)
>>> {
>>> @@ -4032,7 +4112,8 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
>>>
>>> 		/* skip masked non-disk devices */
>>> 		if (MASKED_DEVICE(lunaddrbytes) && physical_device &&
>>> -			(physdev_list->LUN[phys_dev_index].device_flags & 0x01))
>>> +		   (physdev_list->LUN[phys_dev_index].device_type != 0x06) &&
>>> +		   (physdev_list->LUN[phys_dev_index].device_flags & 0x01))
>>> 			continue;
>>>
>>> 		/* Get device type, vendor, model, device id */
>>> @@ -4117,6 +4198,9 @@ static void hpsa_update_scsi_devices(struct ctlr_info *h)
>>> 		case TYPE_TAPE:
>>> 		case TYPE_MEDIUM_CHANGER:
>> Is it 'okay' that these two types are falling through and will call this new
>> enclosure info routine?
Good Point.
>>
>>> 		case TYPE_ENCLOSURE:
>>> +			hpsa_get_enclosure_info(h, lunaddrbytes,
>>> +						physdev_list, phys_dev_index,
>>> +						this_device);
>> Any reason this routine wasn't made a void? The return code is not being used
>> and the other similar helpers in this path are void.
Good point.
>>
>>> 			ncurrent++;
>>> 			break;
>>> 		case TYPE_RAID:
>>> @@ -6629,6 +6713,16 @@ static int fill_cmd(struct CommandList *c, u8 cmd, struct ctlr_info *h,
>>> 			c->Request.CDB[7] = (size >> 16) & 0xFF;
>>> 			c->Request.CDB[8] = (size >> 8) & 0XFF;
>>> 			break;
>>> +		case BMIC_SENSE_STORAGE_BOX_PARAMS:
>>> +			c->Request.CDBLen = 10;
>>> +			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_STORAGE_BOX_PARAMS;
>>> +			c->Request.CDB[7] = (size >> 16) & 0xFF;
>>> +			c->Request.CDB[8] = (size >> 8) & 0XFF;
>>> +			break;
>>> 		case BMIC_IDENTIFY_CONTROLLER:
>>> 			c->Request.CDBLen = 10;
>>> 			c->Request.type_attr_dir =
>>> diff --git a/drivers/scsi/hpsa_cmd.h b/drivers/scsi/hpsa_cmd.h
>>> index d92ef0d..6a919ad 100644
>>> --- a/drivers/scsi/hpsa_cmd.h
>>> +++ b/drivers/scsi/hpsa_cmd.h
>>> @@ -291,6 +291,7 @@ struct SenseSubsystem_info {
>>> #define BMIC_SENSE_DIAG_OPTIONS 0xF5
>>> #define HPSA_DIAG_OPTS_DISABLE_RLD_CACHING 0x40000000
>>> #define BMIC_SENSE_SUBSYSTEM_INFORMATION 0x66
>>> +#define BMIC_SENSE_STORAGE_BOX_PARAMS 0x65
>>>
>>> /* Command List Structure */
>>> union SCSI3Addr {
>>> @@ -842,5 +843,17 @@ struct bmic_sense_subsystem_info {
>>> 	u8	pad[332];
>>> };
>>>
>>> +struct bmic_sense_storage_box_params {
>>> +	u8	reserved[36];
>>> +	u8	inquiry_valid;
>>> +	u8	reserved_1[68];
>>> +	u8	phys_box_on_port;
>>> +	u8	reserved_2[22];
>>> +	u16	connection_info;
>>> +	u8	reserver_3[84];
>>> +	u8	phys_connector[2];
>>> +	u8	reserved_4[296];
>>> +};
>>> +
>>> #pragma pack()
>>> #endif /* HPSA_CMD_H */
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>>> the body of a message to majordomo@vger.kernel.org
>>> 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 majordomo@vger.kernel.org
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-12-21 17:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-09 21:18 [PATCH 0/3] hpsa update Don Brace
2015-12-09 21:18 ` [PATCH 1/3] hpsa: fix path_info_show Don Brace
2015-12-09 23:15   ` Matthew R. Ochs
2015-12-09 21:18 ` [PATCH 2/3] hpsa: change SAS transport devices to bus 0 Don Brace
2015-12-09 23:21   ` Matthew R. Ochs
2015-12-15 15:25     ` Don Brace
2015-12-18 20:14       ` Matthew R. Ochs
2015-12-09 21:18 ` [PATCH 3/3] hpsa: add box and bay information for enclosure devices Don Brace
2015-12-09 23:41   ` Matthew R. Ochs
2015-12-18 20:19     ` Matthew R. Ochs
2015-12-21 17:09       ` Don Brace

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.