Skip to contentSkip to conversation

ipr: fix resource address formatting and add attribute for device ID

PATCH In review Wayne Boyer 2 participants 5 messages Aug 10, 2010 – Aug 11, 2010 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 1/2 ipr: fix resource address formatting and add attribute for device ID

This patch fixes a resource address formatting problem where the first byte was being zeroed out.

Also, the device ID is now made available as a sysfs attribute.

---

 drivers/scsi/ipr.c |   43 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/ipr.h |    3 ++-
 2 files changed, 44 insertions(+), 2 deletions(-)
drivers/scsi/ipr.c +42 -1
Index: b/drivers/scsi/ipr.c
===================================================================
@@ -1096,6 +1096,7 @@ static void ipr_init_res_entry(struct ip
 		res->bus = cfgtew->u.cfgte->res_addr.bus;
 		res->target = cfgtew->u.cfgte->res_addr.target;
 		res->lun = cfgtew->u.cfgte->res_addr.lun;
+		res->lun_wwn = cfgtew->u.cfgte->lun_wwn;
 	}
 
 	ipr_update_ata_class(res, proto);
@@ -1142,7 +1143,7 @@ static char *ipr_format_res_path(u8 *res
 	int i;
 	char *p = buffer;
 
-	res_path[0] = '\0';
+	*p = '\0';
 	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
 	for (i = 1; res_path[i] != 0xff && ((i * 3) < len); i++)
 		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
@@ -4086,6 +4087,7 @@ static int ipr_change_queue_type(struct 
 /**
  * ipr_show_adapter_handle - Show the adapter's resource handle for this device
  * @dev:	device struct
+ * @attr:	device attribute structure
  * @buf:	buffer
  *
  * Return value:
@@ -4119,6 +4121,7 @@ static struct device_attribute ipr_adapt
  * ipr_show_resource_path - Show the resource path or the resource address for
  *			    this device.
  * @dev:	device struct
+ * @attr:	device attribute structure
  * @buf:	buffer
  *
  * Return value:
@@ -4156,8 +4159,45 @@ static struct device_attribute ipr_resou
 };
 
 /**
+ * ipr_show_device_id - Show the device_id for this device.
+ * @dev:	device struct
+ * @attr:	device attribute structure
+ * @buf:	buffer
+ *
+ * Return value:
+ *	number of bytes printed to buffer
+ **/
+static ssize_t ipr_show_device_id(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)sdev->host->hostdata;
+	struct ipr_resource_entry *res;
+	unsigned long lock_flags = 0;
+	ssize_t len = -ENXIO;
+
+	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+	res = (struct ipr_resource_entry *)sdev->hostdata;
+	if (res && ioa_cfg->sis64)
+		len = snprintf(buf, PAGE_SIZE, "0x%llx\n", res->dev_id);
+	else if (res)
+		len = snprintf(buf, PAGE_SIZE, "0x%llx\n", res->lun_wwn);
+
+	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+	return len;
+}
+
+static struct device_attribute ipr_device_id_attr = {
+	.attr = {
+		.name =		"device_id",
+		.mode =		S_IRUGO,
+	},
+	.show = ipr_show_device_id
+};
+
+/**
  * ipr_show_resource_type - Show the resource type for this device.
  * @dev:	device struct
+ * @attr:	device attribute structure
  * @buf:	buffer
  *
  * Return value:
@@ -4192,6 +4232,7 @@ static struct device_attribute ipr_resou
 static struct device_attribute *ipr_dev_attrs[] = {
 	&ipr_adapter_handle_attr,
 	&ipr_resource_path_attr,
+	&ipr_device_id_attr,
 	&ipr_resource_type_attr,
 	NULL,
 };
drivers/scsi/ipr.h +2 -1
Index: b/drivers/scsi/ipr.h
===================================================================
@@ -372,7 +372,7 @@ struct ipr_config_table_entry {
 
 	struct ipr_res_addr res_addr;
 	__be32 res_handle;
-	__be32 reserved4[2];
+	__be64 lun_wwn;
 	struct ipr_std_inq_data std_inq_data;
 }__attribute__ ((packed, aligned (4)));
 
@@ -1210,6 +1210,7 @@ struct ipr_resource_entry {
 
 	__be32 res_handle;
 	__be64 dev_id;
+	__be64 lun_wwn;
 	struct scsi_lun dev_lun;
 	u8 res_path[8];
 
#2 lore raw
Quoted 14 lines · Wayne Boyer

Index: b/drivers/scsi/ipr.h
===================================================================

drivers/scsi/ipr.h +1 -1
@@ -372,7 +372,7 @@ struct ipr_config_table_entry {
 
 	struct ipr_res_addr res_addr;
 	__be32 res_handle;
-	__be32 reserved4[2];
+	__be64 lun_wwn;
 	struct ipr_std_inq_data std_inq_data;
 }__attribute__ ((packed, aligned (4)));
 

drivers/scsi/ipr.h struct ipr_resource_entry { · line 1213

Quoted 4 lines
drivers/scsi/ipr.h +1 -0
@@ -1210,6 +1210,7 @@ struct ipr_resource_entry {
 
 	__be32 res_handle;
 	__be64 dev_id;
+	__be64 lun_wwn;

This should be two __be32's or just an array of u8's instead, since this field is not aligned on an 8 byte boundary. You can use the unaligned accessors to make it easier to deal with.

Thanks,

Brian

Signature · 3 lines
Brian King
Linux on Power Virtualization
IBM Linux Technology Center
author Acked-by Brian King
#3 lore raw
Quoted 7 lines · Brian King
Quoted 6 lines · Wayne Boyer

Index: b/drivers/scsi/ipr.h
===================================================================

drivers/scsi/ipr.h +0 -0
@@ -372,7 +372,7 @@ struct ipr_config_table_entry {
 
Quoted 10 lines

drivers/scsi/ipr.h struct ipr_config_table_entry { · line 375

Quoted 4 lines
drivers/scsi/ipr.h +1 -1
 	struct ipr_res_addr res_addr;
 	__be32 res_handle;
-	__be32 reserved4[2];
+	__be64 lun_wwn;

This should be two __be32's or just an array of u8's instead, since this field is not aligned on an 8 byte boundary. You can use the unaligned accessors to make it easier to deal with.

Here's the updated patch. The field in question was lun_wwn in the ipr_res_addr structure.

---

This patch fixes a resource address formatting problem where the first byte was being zeroed out.

Also, the device ID is now made available as a sysfs attribute.

---

 drivers/scsi/ipr.c |   43 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/ipr.h |    4 +++-
 2 files changed, 45 insertions(+), 2 deletions(-)
drivers/scsi/ipr.c +42 -1
Index: b/drivers/scsi/ipr.c
===================================================================
@@ -1096,6 +1096,7 @@ static void ipr_init_res_entry(struct ip
 		res->bus = cfgtew->u.cfgte->res_addr.bus;
 		res->target = cfgtew->u.cfgte->res_addr.target;
 		res->lun = cfgtew->u.cfgte->res_addr.lun;
+		res->lun_wwn = get_unaligned_be64(cfgtew->u.cfgte->lun_wwn);
 	}
 
 	ipr_update_ata_class(res, proto);
@@ -1142,7 +1143,7 @@ static char *ipr_format_res_path(u8 *res
 	int i;
 	char *p = buffer;
 
-	res_path[0] = '\0';
+	*p = '\0';
 	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
 	for (i = 1; res_path[i] != 0xff && ((i * 3) < len); i++)
 		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
@@ -4086,6 +4087,7 @@ static int ipr_change_queue_type(struct 
 /**
  * ipr_show_adapter_handle - Show the adapter's resource handle for this device
  * @dev:	device struct
+ * @attr:	device attribute structure
  * @buf:	buffer
  *
  * Return value:
@@ -4119,6 +4121,7 @@ static struct device_attribute ipr_adapt
  * ipr_show_resource_path - Show the resource path or the resource address for
  *			    this device.
  * @dev:	device struct
+ * @attr:	device attribute structure
  * @buf:	buffer
  *
  * Return value:
@@ -4156,8 +4159,45 @@ static struct device_attribute ipr_resou
 };
 
 /**
+ * ipr_show_device_id - Show the device_id for this device.
+ * @dev:	device struct
+ * @attr:	device attribute structure
+ * @buf:	buffer
+ *
+ * Return value:
+ *	number of bytes printed to buffer
+ **/
+static ssize_t ipr_show_device_id(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)sdev->host->hostdata;
+	struct ipr_resource_entry *res;
+	unsigned long lock_flags = 0;
+	ssize_t len = -ENXIO;
+
+	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+	res = (struct ipr_resource_entry *)sdev->hostdata;
+	if (res && ioa_cfg->sis64)
+		len = snprintf(buf, PAGE_SIZE, "0x%llx\n", res->dev_id);
+	else if (res)
+		len = snprintf(buf, PAGE_SIZE, "0x%llx\n", res->lun_wwn);
+
+	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+	return len;
+}
+
+static struct device_attribute ipr_device_id_attr = {
+	.attr = {
+		.name =		"device_id",
+		.mode =		S_IRUGO,
+	},
+	.show = ipr_show_device_id
+};
+
+/**
  * ipr_show_resource_type - Show the resource type for this device.
  * @dev:	device struct
+ * @attr:	device attribute structure
  * @buf:	buffer
  *
  * Return value:
@@ -4192,6 +4232,7 @@ static struct device_attribute ipr_resou
 static struct device_attribute *ipr_dev_attrs[] = {
 	&ipr_adapter_handle_attr,
 	&ipr_resource_path_attr,
+	&ipr_device_id_attr,
 	&ipr_resource_type_attr,
 	NULL,
 };
drivers/scsi/ipr.h +3 -1
Index: b/drivers/scsi/ipr.h
===================================================================
@@ -26,6 +26,7 @@
 #ifndef _IPR_H
 #define _IPR_H
 
+#include <asm/unaligned.h>
 #include <linux/types.h>
 #include <linux/completion.h>
 #include <linux/libata.h>
@@ -372,7 +373,7 @@ struct ipr_config_table_entry {
 
 	struct ipr_res_addr res_addr;
 	__be32 res_handle;
-	__be32 reserved4[2];
+	__be32 lun_wwn[2];
 	struct ipr_std_inq_data std_inq_data;
 }__attribute__ ((packed, aligned (4)));
 
@@ -1210,6 +1211,7 @@ struct ipr_resource_entry {
 
 	__be32 res_handle;
 	__be64 dev_id;
+	__be64 lun_wwn;
 	struct scsi_lun dev_lun;
 	u8 res_path[8];
 
Acked-by
#4 lore raw
Quoted 163 lines · Wayne Boyer
Quoted 7 lines · Brian King
Quoted 6 lines · Wayne Boyer

Index: b/drivers/scsi/ipr.h
===================================================================

drivers/scsi/ipr.h +0 -0
@@ -372,7 +372,7 @@ struct ipr_config_table_entry {
 
Quoted 10 lines

drivers/scsi/ipr.h struct ipr_config_table_entry {

Quoted 4 lines
drivers/scsi/ipr.h +1 -1
 	struct ipr_res_addr res_addr;
 	__be32 res_handle;
-	__be32 reserved4[2];
+	__be64 lun_wwn;

This should be two __be32's or just an array of u8's instead, since this field is not aligned on an 8 byte boundary. You can use the unaligned accessors to make it easier to deal with.

Here's the updated patch. The field in question was lun_wwn in the ipr_res_addr structure.

---

This patch fixes a resource address formatting problem where the first byte was being zeroed out.

Also, the device ID is now made available as a sysfs attribute.

---

 drivers/scsi/ipr.c |   43 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/ipr.h |    4 +++-
 2 files changed, 45 insertions(+), 2 deletions(-)

Index: b/drivers/scsi/ipr.c
===================================================================

drivers/scsi/ipr.c +42 -1
@@ -1096,6 +1096,7 @@ static void ipr_init_res_entry(struct ip
 		res->bus = cfgtew->u.cfgte->res_addr.bus;
 		res->target = cfgtew->u.cfgte->res_addr.target;
 		res->lun = cfgtew->u.cfgte->res_addr.lun;
+		res->lun_wwn = get_unaligned_be64(cfgtew->u.cfgte->lun_wwn);
 	}
 
 	ipr_update_ata_class(res, proto);
@@ -1142,7 +1143,7 @@ static char *ipr_format_res_path(u8 *res
 	int i;
 	char *p = buffer;
 
-	res_path[0] = '\0';
+	*p = '\0';
 	p += snprintf(p, buffer + len - p, "%02X", res_path[0]);
 	for (i = 1; res_path[i] != 0xff && ((i * 3) < len); i++)
 		p += snprintf(p, buffer + len - p, "-%02X", res_path[i]);
@@ -4086,6 +4087,7 @@ static int ipr_change_queue_type(struct 
 /**
  * ipr_show_adapter_handle - Show the adapter's resource handle for this device
  * @dev:	device struct
+ * @attr:	device attribute structure
  * @buf:	buffer
  *
  * Return value:
@@ -4119,6 +4121,7 @@ static struct device_attribute ipr_adapt
  * ipr_show_resource_path - Show the resource path or the resource address for
  *			    this device.
  * @dev:	device struct
+ * @attr:	device attribute structure
  * @buf:	buffer
  *
  * Return value:
@@ -4156,8 +4159,45 @@ static struct device_attribute ipr_resou
 };
 
 /**
+ * ipr_show_device_id - Show the device_id for this device.
+ * @dev:	device struct
+ * @attr:	device attribute structure
+ * @buf:	buffer
+ *
+ * Return value:
+ *	number of bytes printed to buffer
+ **/
+static ssize_t ipr_show_device_id(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct ipr_ioa_cfg *ioa_cfg = (struct ipr_ioa_cfg *)sdev->host->hostdata;
+	struct ipr_resource_entry *res;
+	unsigned long lock_flags = 0;
+	ssize_t len = -ENXIO;
+
+	spin_lock_irqsave(ioa_cfg->host->host_lock, lock_flags);
+	res = (struct ipr_resource_entry *)sdev->hostdata;
+	if (res && ioa_cfg->sis64)
+		len = snprintf(buf, PAGE_SIZE, "0x%llx\n", res->dev_id);
+	else if (res)
+		len = snprintf(buf, PAGE_SIZE, "0x%llx\n", res->lun_wwn);
+
+	spin_unlock_irqrestore(ioa_cfg->host->host_lock, lock_flags);
+	return len;
+}
+
+static struct device_attribute ipr_device_id_attr = {
+	.attr = {
+		.name =		"device_id",
+		.mode =		S_IRUGO,
+	},
+	.show = ipr_show_device_id
+};
+
+/**
  * ipr_show_resource_type - Show the resource type for this device.
  * @dev:	device struct
+ * @attr:	device attribute structure
  * @buf:	buffer
  *
  * Return value:
@@ -4192,6 +4232,7 @@ static struct device_attribute ipr_resou
 static struct device_attribute *ipr_dev_attrs[] = {
 	&ipr_adapter_handle_attr,
 	&ipr_resource_path_attr,
+	&ipr_device_id_attr,
 	&ipr_resource_type_attr,
 	NULL,
 };

Index: b/drivers/scsi/ipr.h
===================================================================

drivers/scsi/ipr.h +3 -2
@@ -26,6 +26,7 @@
 #ifndef _IPR_H
 #define _IPR_H
 
+#include <asm/unaligned.h>
 #include <linux/types.h>
 #include <linux/completion.h>
 #include <linux/libata.h>
@@ -372,7 +373,7 @@ struct ipr_config_table_entry {
 
 	struct ipr_res_addr res_addr;
 	__be32 res_handle;
-	__be32 reserved4[2];
+	__be32 lun_wwn[2];
 	struct ipr_std_inq_data std_inq_data;
 }__attribute__ ((packed, aligned (4)));
 
@@ -1210,6 +1211,7 @@ struct ipr_resource_entry {
 
 	__be32 res_handle;
 	__be64 dev_id;
+	__be64 lun_wwn;
 	struct scsi_lun dev_lun;
 	u8 res_path[8];
 
 
 
--

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

Signature · 3 lines
Brian King
Linux on Power Virtualization
IBM Linux Technology Center
#5 lore raw

PATCH 2/2 ipr: Driver version 2.5.1

In reply to a message not in the archive

Bump the driver version.

---

 drivers/scsi/ipr.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/ipr.h +2 -2
Index: b/drivers/scsi/ipr.h
===================================================================
@@ -37,8 +37,8 @@
 /*
  * Literals
  */
-#define IPR_DRIVER_VERSION "2.5.0"
-#define IPR_DRIVER_DATE "(February 11, 2010)"
+#define IPR_DRIVER_VERSION "2.5.1"
+#define IPR_DRIVER_DATE "(August 10, 2010)"
 
 /*
  * IPR_MAX_CMD_PER_LUN: This defines the maximum number of outstanding