* [PATCH 0/6] ibmvscsi: code cleanup
@ 2016-02-03 23:28 Tyrel Datwyler
2016-02-03 23:28 ` [PATCH 1/6] ibmvscsi: Correct values for several viosrp_crq_format enums Tyrel Datwyler
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-03 23:28 UTC (permalink / raw)
To: JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, linuxppc-dev, nfont,
brking, hare, Tyrel Datwyler
Fixed up a couple spots that were out of line with the PAPR in regards
to its defined VSCSI protocol. Did away with some magic numbers directly
in the code. Fixed a minor endian issue.
Tyrel Datwyler (6):
ibmvscsi: Correct values for several viosrp_crq_format enums
ibmvscsi: Add and use enums for valid CRQ header values
ibmvscsi: Replace magic values in set_adpater_info() with defines
ibmvscsi: Use of_root to access OF device tree root node
ibmvscsi: Remove unsupported host config MAD and sysfs interface
ibmvscsi: Add endian conversions to sysfs attribute show functions
drivers/scsi/ibmvscsi/ibmvscsi.c | 117 +++++++--------------------------------
drivers/scsi/ibmvscsi/viosrp.h | 20 ++++---
2 files changed, 30 insertions(+), 107 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/6] ibmvscsi: Correct values for several viosrp_crq_format enums
2016-02-03 23:28 [PATCH 0/6] ibmvscsi: code cleanup Tyrel Datwyler
@ 2016-02-03 23:28 ` Tyrel Datwyler
2016-02-04 8:38 ` Johannes Thumshirn
2016-02-03 23:28 ` [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values Tyrel Datwyler
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-03 23:28 UTC (permalink / raw)
To: JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, linuxppc-dev, nfont,
brking, hare, Tyrel Datwyler
The enum values for VIOSRP_LINUX_FORMAT and VIOSRP_INLINE_FORMAT are
off by one. They are currently defined as 0x06 and 0x07 respetively.
These values are defined in PAPR correctly as 0x05 and 0x06. This
inconsistency has gone unnoticed as neither enum is currently used.
The possible future support of PING messages between the VIOS and
client adapter relies on VIOSRP_INLINE_FORMAT crq messages.
Corrected these enum values to match PAPR definitions.
Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
drivers/scsi/ibmvscsi/viosrp.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index 1162430..d1044e9 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -56,8 +56,8 @@ enum viosrp_crq_formats {
VIOSRP_MAD_FORMAT = 0x02,
VIOSRP_OS400_FORMAT = 0x03,
VIOSRP_AIX_FORMAT = 0x04,
- VIOSRP_LINUX_FORMAT = 0x06,
- VIOSRP_INLINE_FORMAT = 0x07
+ VIOSRP_LINUX_FORMAT = 0x05,
+ VIOSRP_INLINE_FORMAT = 0x06
};
enum viosrp_crq_status {
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values
2016-02-03 23:28 [PATCH 0/6] ibmvscsi: code cleanup Tyrel Datwyler
2016-02-03 23:28 ` [PATCH 1/6] ibmvscsi: Correct values for several viosrp_crq_format enums Tyrel Datwyler
@ 2016-02-03 23:28 ` Tyrel Datwyler
2016-02-04 8:39 ` Johannes Thumshirn
2016-02-04 19:55 ` Manoj Kumar
2016-02-03 23:28 ` [PATCH 3/6] ibmvscsi: Replace magic values in set_adpater_info() with defines Tyrel Datwyler
` (3 subsequent siblings)
5 siblings, 2 replies; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-03 23:28 UTC (permalink / raw)
To: JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, linuxppc-dev, nfont,
brking, hare, Tyrel Datwyler
The PAPR defines four valid header values for the first byte of a
CRQ message. Namely, an unused/empty message (0x00), a valid
command/response entry (0x80), a valid initialization entry (0xC0),
and a transport event (0xFF). Define these values as enums and use
them in the code in place of their magic number equivalents.
Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvscsi.c | 14 +++++++-------
drivers/scsi/ibmvscsi/viosrp.h | 7 +++++++
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index adfef9d..176260d 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -182,7 +182,7 @@ static struct viosrp_crq *crq_queue_next_crq(struct crq_queue *queue)
spin_lock_irqsave(&queue->lock, flags);
crq = &queue->msgs[queue->cur];
- if (crq->valid & 0x80) {
+ if (crq->valid & VIOSRP_CRQ_VALID) {
if (++queue->cur == queue->size)
queue->cur = 0;
@@ -231,7 +231,7 @@ static void ibmvscsi_task(void *data)
/* Pull all the valid messages off the CRQ */
while ((crq = crq_queue_next_crq(&hostdata->queue)) != NULL) {
ibmvscsi_handle_crq(crq, hostdata);
- crq->valid = 0x00;
+ crq->valid = VIOSRP_CRQ_FREE;
}
vio_enable_interrupts(vdev);
@@ -239,7 +239,7 @@ static void ibmvscsi_task(void *data)
if (crq != NULL) {
vio_disable_interrupts(vdev);
ibmvscsi_handle_crq(crq, hostdata);
- crq->valid = 0x00;
+ crq->valid = VIOSRP_CRQ_FREE;
} else {
done = 1;
}
@@ -474,7 +474,7 @@ static int initialize_event_pool(struct event_pool *pool,
struct srp_event_struct *evt = &pool->events[i];
memset(&evt->crq, 0x00, sizeof(evt->crq));
atomic_set(&evt->free, 1);
- evt->crq.valid = 0x80;
+ evt->crq.valid = VIOSRP_CRQ_VALID;
evt->crq.IU_length = cpu_to_be16(sizeof(*evt->xfer_iu));
evt->crq.IU_data_ptr = cpu_to_be64(pool->iu_token +
sizeof(*evt->xfer_iu) * i);
@@ -1767,7 +1767,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
struct srp_event_struct *evt_struct =
(__force struct srp_event_struct *)crq->IU_data_ptr;
switch (crq->valid) {
- case 0xC0: /* initialization */
+ case VIOSRP_CRQ_INIT: /* initialization */
switch (crq->format) {
case 0x01: /* Initialization message */
dev_info(hostdata->dev, "partner initialized\n");
@@ -1791,7 +1791,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
dev_err(hostdata->dev, "unknown crq message type: %d\n", crq->format);
}
return;
- case 0xFF: /* Hypervisor telling us the connection is closed */
+ case VIOSRP_CRQ_TRANSPORT: /* Hypervisor telling us the connection is closed */
scsi_block_requests(hostdata->host);
atomic_set(&hostdata->request_limit, 0);
if (crq->format == 0x06) {
@@ -1807,7 +1807,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
ibmvscsi_reset_host(hostdata);
}
return;
- case 0x80: /* real payload */
+ case VIOSRP_CRQ_VALID: /* real payload */
break;
default:
dev_err(hostdata->dev, "got an invalid message type 0x%02x\n",
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index d1044e9..17f2de0 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -51,6 +51,13 @@ union srp_iu {
u8 reserved[SRP_MAX_IU_LEN];
};
+enum viosrp_crq_headers {
+ VIOSRP_CRQ_FREE = 0x00,
+ VIOSRP_CRQ_VALID = 0x80,
+ VIOSRP_CRQ_INIT = 0xC0,
+ VIOSRP_CRQ_TRANSPORT = 0xFF
+};
+
enum viosrp_crq_formats {
VIOSRP_SRP_FORMAT = 0x01,
VIOSRP_MAD_FORMAT = 0x02,
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/6] ibmvscsi: Replace magic values in set_adpater_info() with defines
2016-02-03 23:28 [PATCH 0/6] ibmvscsi: code cleanup Tyrel Datwyler
2016-02-03 23:28 ` [PATCH 1/6] ibmvscsi: Correct values for several viosrp_crq_format enums Tyrel Datwyler
2016-02-03 23:28 ` [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values Tyrel Datwyler
@ 2016-02-03 23:28 ` Tyrel Datwyler
2016-02-04 8:44 ` Johannes Thumshirn
2016-02-03 23:28 ` [PATCH 4/6] ibmvscsi: Use of_root to access OF device tree root node Tyrel Datwyler
` (2 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-03 23:28 UTC (permalink / raw)
To: JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, linuxppc-dev, nfont,
brking, hare, Tyrel Datwyler
Add defines for mad version and mad os_type, and replace the magic
numbers in set_adapter_info() accordingly.
Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvscsi.c | 4 ++--
drivers/scsi/ibmvscsi/viosrp.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 176260d..7e51615 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -283,8 +283,8 @@ static void set_adapter_info(struct ibmvscsi_host_data *hostdata)
hostdata->madapter_info.partition_number =
cpu_to_be32(partition_number);
- hostdata->madapter_info.mad_version = cpu_to_be32(1);
- hostdata->madapter_info.os_type = cpu_to_be32(2);
+ hostdata->madapter_info.mad_version = cpu_to_be32(SRP_MAD_VERSION_1);
+ hostdata->madapter_info.os_type = cpu_to_be32(SRP_MAD_OS_LINUX);
}
/**
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index 17f2de0..da9bb29 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -38,6 +38,8 @@
#define SRP_VERSION "16.a"
#define SRP_MAX_IU_LEN 256
#define SRP_MAX_LOC_LEN 32
+#define SRP_MAD_VERSION_1 1
+#define SRP_MAD_OS_LINUX 2
union srp_iu {
struct srp_login_req login_req;
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/6] ibmvscsi: Use of_root to access OF device tree root node
2016-02-03 23:28 [PATCH 0/6] ibmvscsi: code cleanup Tyrel Datwyler
` (2 preceding siblings ...)
2016-02-03 23:28 ` [PATCH 3/6] ibmvscsi: Replace magic values in set_adpater_info() with defines Tyrel Datwyler
@ 2016-02-03 23:28 ` Tyrel Datwyler
2016-02-04 8:45 ` Johannes Thumshirn
2016-02-03 23:28 ` [PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface Tyrel Datwyler
2016-02-03 23:28 ` [PATCH 6/6] ibmvscsi: Add endian conversions to sysfs attribute show functions Tyrel Datwyler
5 siblings, 1 reply; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-03 23:28 UTC (permalink / raw)
To: JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, linuxppc-dev, nfont,
brking, hare, Tyrel Datwyler
The root node of the OF device tree is exported as of_root. No need
to look up the root by path name. Instead just get a reference
directly via of_root.
Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvscsi.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 7e51615..47cfe33 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -248,25 +248,23 @@ static void ibmvscsi_task(void *data)
static void gather_partition_info(void)
{
- struct device_node *rootdn;
-
const char *ppartition_name;
const __be32 *p_number_ptr;
/* Retrieve information about this partition */
- rootdn = of_find_node_by_path("/");
- if (!rootdn) {
+ if (!of_root)
return;
- }
- ppartition_name = of_get_property(rootdn, "ibm,partition-name", NULL);
+ of_node_get(of_root);
+
+ ppartition_name = of_get_property(of_root, "ibm,partition-name", NULL);
if (ppartition_name)
strncpy(partition_name, ppartition_name,
sizeof(partition_name));
- p_number_ptr = of_get_property(rootdn, "ibm,partition-no", NULL);
+ p_number_ptr = of_get_property(of_root, "ibm,partition-no", NULL);
if (p_number_ptr)
partition_number = of_read_number(p_number_ptr, 1);
- of_node_put(rootdn);
+ of_node_put(of_root);
}
static void set_adapter_info(struct ibmvscsi_host_data *hostdata)
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface
2016-02-03 23:28 [PATCH 0/6] ibmvscsi: code cleanup Tyrel Datwyler
` (3 preceding siblings ...)
2016-02-03 23:28 ` [PATCH 4/6] ibmvscsi: Use of_root to access OF device tree root node Tyrel Datwyler
@ 2016-02-03 23:28 ` Tyrel Datwyler
2016-02-04 8:03 ` Johannes Thumshirn
2016-02-03 23:28 ` [PATCH 6/6] ibmvscsi: Add endian conversions to sysfs attribute show functions Tyrel Datwyler
5 siblings, 1 reply; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-03 23:28 UTC (permalink / raw)
To: JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, linuxppc-dev, nfont,
brking, hare, Tyrel Datwyler
A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in
the code for some time. From what information I've gathered from
Brian King this was likely implemented on the host side in a SLES 9
based VIOS, which is no longer supported anywhere. Further, it is
not defined in PAPR or supported by any AIX based VIOS.
Treating as bit rot and removing the sysfs interface and associated
host config code accordingly.
Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvscsi.c | 78 ----------------------------------------
drivers/scsi/ibmvscsi/viosrp.h | 7 ----
2 files changed, 85 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 47cfe33..a2b8db1 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -1853,62 +1853,6 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
}
/**
- * ibmvscsi_get_host_config: Send the command to the server to get host
- * configuration data. The data is opaque to us.
- */
-static int ibmvscsi_do_host_config(struct ibmvscsi_host_data *hostdata,
- unsigned char *buffer, int length)
-{
- struct viosrp_host_config *host_config;
- struct srp_event_struct *evt_struct;
- unsigned long flags;
- dma_addr_t addr;
- int rc;
-
- evt_struct = get_event_struct(&hostdata->pool);
- if (!evt_struct) {
- dev_err(hostdata->dev, "couldn't allocate event for HOST_CONFIG!\n");
- return -1;
- }
-
- init_event_struct(evt_struct,
- sync_completion,
- VIOSRP_MAD_FORMAT,
- info_timeout);
-
- host_config = &evt_struct->iu.mad.host_config;
-
- /* The transport length field is only 16-bit */
- length = min(0xffff, length);
-
- /* Set up a lun reset SRP command */
- memset(host_config, 0x00, sizeof(*host_config));
- host_config->common.type = cpu_to_be32(VIOSRP_HOST_CONFIG_TYPE);
- host_config->common.length = cpu_to_be16(length);
- addr = dma_map_single(hostdata->dev, buffer, length, DMA_BIDIRECTIONAL);
-
- if (dma_mapping_error(hostdata->dev, addr)) {
- if (!firmware_has_feature(FW_FEATURE_CMO))
- dev_err(hostdata->dev,
- "dma_mapping error getting host config\n");
- free_event_struct(&hostdata->pool, evt_struct);
- return -1;
- }
-
- host_config->buffer = cpu_to_be64(addr);
-
- init_completion(&evt_struct->comp);
- spin_lock_irqsave(hostdata->host->host_lock, flags);
- rc = ibmvscsi_send_srp_event(evt_struct, hostdata, info_timeout * 2);
- spin_unlock_irqrestore(hostdata->host->host_lock, flags);
- if (rc == 0)
- wait_for_completion(&evt_struct->comp);
- dma_unmap_single(hostdata->dev, addr, length, DMA_BIDIRECTIONAL);
-
- return rc;
-}
-
-/**
* ibmvscsi_slave_configure: Set the "allow_restart" flag for each disk.
* @sdev: struct scsi_device device to configure
*
@@ -2090,27 +2034,6 @@ static struct device_attribute ibmvscsi_host_os_type = {
.show = show_host_os_type,
};
-static ssize_t show_host_config(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct Scsi_Host *shost = class_to_shost(dev);
- struct ibmvscsi_host_data *hostdata = shost_priv(shost);
-
- /* returns null-terminated host config data */
- if (ibmvscsi_do_host_config(hostdata, buf, PAGE_SIZE) == 0)
- return strlen(buf);
- else
- return 0;
-}
-
-static struct device_attribute ibmvscsi_host_config = {
- .attr = {
- .name = "config",
- .mode = S_IRUGO,
- },
- .show = show_host_config,
-};
-
static struct device_attribute *ibmvscsi_attrs[] = {
&ibmvscsi_host_vhost_loc,
&ibmvscsi_host_vhost_name,
@@ -2119,7 +2042,6 @@ static struct device_attribute *ibmvscsi_attrs[] = {
&ibmvscsi_host_partition_number,
&ibmvscsi_host_mad_version,
&ibmvscsi_host_os_type,
- &ibmvscsi_host_config,
NULL
};
diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
index da9bb29..04bae63 100644
--- a/drivers/scsi/ibmvscsi/viosrp.h
+++ b/drivers/scsi/ibmvscsi/viosrp.h
@@ -96,7 +96,6 @@ enum viosrp_mad_types {
VIOSRP_EMPTY_IU_TYPE = 0x01,
VIOSRP_ERROR_LOG_TYPE = 0x02,
VIOSRP_ADAPTER_INFO_TYPE = 0x03,
- VIOSRP_HOST_CONFIG_TYPE = 0x04,
VIOSRP_CAPABILITIES_TYPE = 0x05,
VIOSRP_ENABLE_FAST_FAIL = 0x08,
};
@@ -162,11 +161,6 @@ struct viosrp_adapter_info {
__be64 buffer;
};
-struct viosrp_host_config {
- struct mad_common common;
- __be64 buffer;
-};
-
struct viosrp_fast_fail {
struct mad_common common;
};
@@ -204,7 +198,6 @@ union mad_iu {
struct viosrp_empty_iu empty_iu;
struct viosrp_error_log error_log;
struct viosrp_adapter_info adapter_info;
- struct viosrp_host_config host_config;
struct viosrp_fast_fail fast_fail;
struct viosrp_capabilities capabilities;
};
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 6/6] ibmvscsi: Add endian conversions to sysfs attribute show functions
2016-02-03 23:28 [PATCH 0/6] ibmvscsi: code cleanup Tyrel Datwyler
` (4 preceding siblings ...)
2016-02-03 23:28 ` [PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface Tyrel Datwyler
@ 2016-02-03 23:28 ` Tyrel Datwyler
2016-02-04 8:45 ` Johannes Thumshirn
5 siblings, 1 reply; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-03 23:28 UTC (permalink / raw)
To: JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, linuxppc-dev, nfont,
brking, hare, Tyrel Datwyler
The values returned by the show functions for the host os_type,
mad_version, and partition_number attributes get their values
directly from the madapter_info struct whose associated fields are
__be32 typed. Added endian conversion to ensure these values are
sane on LE platforms.
Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvscsi.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index a2b8db1..3621ac5 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -1983,7 +1983,7 @@ static ssize_t show_host_partition_number(struct device *dev,
int len;
len = snprintf(buf, PAGE_SIZE, "%d\n",
- hostdata->madapter_info.partition_number);
+ be32_to_cpu(hostdata->madapter_info.partition_number));
return len;
}
@@ -2003,7 +2003,7 @@ static ssize_t show_host_mad_version(struct device *dev,
int len;
len = snprintf(buf, PAGE_SIZE, "%d\n",
- hostdata->madapter_info.mad_version);
+ be32_to_cpu(hostdata->madapter_info.mad_version));
return len;
}
@@ -2022,7 +2022,8 @@ static ssize_t show_host_os_type(struct device *dev,
struct ibmvscsi_host_data *hostdata = shost_priv(shost);
int len;
- len = snprintf(buf, PAGE_SIZE, "%d\n", hostdata->madapter_info.os_type);
+ len = snprintf(buf, PAGE_SIZE, "%d\n",
+ be32_to_cpu(hostdata->madapter_info.os_type));
return len;
}
--
2.5.0
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface
2016-02-03 23:28 ` [PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface Tyrel Datwyler
@ 2016-02-04 8:03 ` Johannes Thumshirn
2016-02-04 17:48 ` Tyrel Datwyler
0 siblings, 1 reply; 19+ messages in thread
From: Johannes Thumshirn @ 2016-02-04 8:03 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: JBottomley, martin.petersen, linux-scsi, linux-kernel,
linuxppc-dev, nfont, brking, hare
On Wed, Feb 03, 2016 at 05:28:33PM -0600, Tyrel Datwyler wrote:
> A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in
> the code for some time. From what information I've gathered from
> Brian King this was likely implemented on the host side in a SLES 9
> based VIOS, which is no longer supported anywhere. Further, it is
> not defined in PAPR or supported by any AIX based VIOS.
>
> Treating as bit rot and removing the sysfs interface and associated
> host config code accordingly.
Doesn't removing a sysfs interface potentially break userspace code?
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/6] ibmvscsi: Correct values for several viosrp_crq_format enums
2016-02-03 23:28 ` [PATCH 1/6] ibmvscsi: Correct values for several viosrp_crq_format enums Tyrel Datwyler
@ 2016-02-04 8:38 ` Johannes Thumshirn
0 siblings, 0 replies; 19+ messages in thread
From: Johannes Thumshirn @ 2016-02-04 8:38 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: JBottomley, martin.petersen, linux-scsi, linux-kernel,
linuxppc-dev, nfont, brking, hare
On Wed, Feb 03, 2016 at 05:28:29PM -0600, Tyrel Datwyler wrote:
> The enum values for VIOSRP_LINUX_FORMAT and VIOSRP_INLINE_FORMAT are
> off by one. They are currently defined as 0x06 and 0x07 respetively.
> These values are defined in PAPR correctly as 0x05 and 0x06. This
> inconsistency has gone unnoticed as neither enum is currently used.
> The possible future support of PING messages between the VIOS and
> client adapter relies on VIOSRP_INLINE_FORMAT crq messages.
> Corrected these enum values to match PAPR definitions.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> ---
> drivers/scsi/ibmvscsi/viosrp.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
> index 1162430..d1044e9 100644
> --- a/drivers/scsi/ibmvscsi/viosrp.h
> +++ b/drivers/scsi/ibmvscsi/viosrp.h
> @@ -56,8 +56,8 @@ enum viosrp_crq_formats {
> VIOSRP_MAD_FORMAT = 0x02,
> VIOSRP_OS400_FORMAT = 0x03,
> VIOSRP_AIX_FORMAT = 0x04,
> - VIOSRP_LINUX_FORMAT = 0x06,
> - VIOSRP_INLINE_FORMAT = 0x07
> + VIOSRP_LINUX_FORMAT = 0x05,
> + VIOSRP_INLINE_FORMAT = 0x06
> };
>
> enum viosrp_crq_status {
> --
> 2.5.0
>
> --
> 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
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values
2016-02-03 23:28 ` [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values Tyrel Datwyler
@ 2016-02-04 8:39 ` Johannes Thumshirn
2016-02-04 19:55 ` Manoj Kumar
1 sibling, 0 replies; 19+ messages in thread
From: Johannes Thumshirn @ 2016-02-04 8:39 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: JBottomley, martin.petersen, linux-scsi, linux-kernel,
linuxppc-dev, nfont, brking, hare
On Wed, Feb 03, 2016 at 05:28:30PM -0600, Tyrel Datwyler wrote:
> The PAPR defines four valid header values for the first byte of a
> CRQ message. Namely, an unused/empty message (0x00), a valid
> command/response entry (0x80), a valid initialization entry (0xC0),
> and a transport event (0xFF). Define these values as enums and use
> them in the code in place of their magic number equivalents.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvscsi.c | 14 +++++++-------
> drivers/scsi/ibmvscsi/viosrp.h | 7 +++++++
> 2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index adfef9d..176260d 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -182,7 +182,7 @@ static struct viosrp_crq *crq_queue_next_crq(struct crq_queue *queue)
>
> spin_lock_irqsave(&queue->lock, flags);
> crq = &queue->msgs[queue->cur];
> - if (crq->valid & 0x80) {
> + if (crq->valid & VIOSRP_CRQ_VALID) {
> if (++queue->cur == queue->size)
> queue->cur = 0;
>
> @@ -231,7 +231,7 @@ static void ibmvscsi_task(void *data)
> /* Pull all the valid messages off the CRQ */
> while ((crq = crq_queue_next_crq(&hostdata->queue)) != NULL) {
> ibmvscsi_handle_crq(crq, hostdata);
> - crq->valid = 0x00;
> + crq->valid = VIOSRP_CRQ_FREE;
> }
>
> vio_enable_interrupts(vdev);
> @@ -239,7 +239,7 @@ static void ibmvscsi_task(void *data)
> if (crq != NULL) {
> vio_disable_interrupts(vdev);
> ibmvscsi_handle_crq(crq, hostdata);
> - crq->valid = 0x00;
> + crq->valid = VIOSRP_CRQ_FREE;
> } else {
> done = 1;
> }
> @@ -474,7 +474,7 @@ static int initialize_event_pool(struct event_pool *pool,
> struct srp_event_struct *evt = &pool->events[i];
> memset(&evt->crq, 0x00, sizeof(evt->crq));
> atomic_set(&evt->free, 1);
> - evt->crq.valid = 0x80;
> + evt->crq.valid = VIOSRP_CRQ_VALID;
> evt->crq.IU_length = cpu_to_be16(sizeof(*evt->xfer_iu));
> evt->crq.IU_data_ptr = cpu_to_be64(pool->iu_token +
> sizeof(*evt->xfer_iu) * i);
> @@ -1767,7 +1767,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
> struct srp_event_struct *evt_struct =
> (__force struct srp_event_struct *)crq->IU_data_ptr;
> switch (crq->valid) {
> - case 0xC0: /* initialization */
> + case VIOSRP_CRQ_INIT: /* initialization */
> switch (crq->format) {
> case 0x01: /* Initialization message */
> dev_info(hostdata->dev, "partner initialized\n");
> @@ -1791,7 +1791,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
> dev_err(hostdata->dev, "unknown crq message type: %d\n", crq->format);
> }
> return;
> - case 0xFF: /* Hypervisor telling us the connection is closed */
> + case VIOSRP_CRQ_TRANSPORT: /* Hypervisor telling us the connection is closed */
> scsi_block_requests(hostdata->host);
> atomic_set(&hostdata->request_limit, 0);
> if (crq->format == 0x06) {
> @@ -1807,7 +1807,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
> ibmvscsi_reset_host(hostdata);
> }
> return;
> - case 0x80: /* real payload */
> + case VIOSRP_CRQ_VALID: /* real payload */
> break;
> default:
> dev_err(hostdata->dev, "got an invalid message type 0x%02x\n",
> diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
> index d1044e9..17f2de0 100644
> --- a/drivers/scsi/ibmvscsi/viosrp.h
> +++ b/drivers/scsi/ibmvscsi/viosrp.h
> @@ -51,6 +51,13 @@ union srp_iu {
> u8 reserved[SRP_MAX_IU_LEN];
> };
>
> +enum viosrp_crq_headers {
> + VIOSRP_CRQ_FREE = 0x00,
> + VIOSRP_CRQ_VALID = 0x80,
> + VIOSRP_CRQ_INIT = 0xC0,
> + VIOSRP_CRQ_TRANSPORT = 0xFF
> +};
> +
> enum viosrp_crq_formats {
> VIOSRP_SRP_FORMAT = 0x01,
> VIOSRP_MAD_FORMAT = 0x02,
> --
> 2.5.0
>
> --
> 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
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/6] ibmvscsi: Replace magic values in set_adpater_info() with defines
2016-02-03 23:28 ` [PATCH 3/6] ibmvscsi: Replace magic values in set_adpater_info() with defines Tyrel Datwyler
@ 2016-02-04 8:44 ` Johannes Thumshirn
0 siblings, 0 replies; 19+ messages in thread
From: Johannes Thumshirn @ 2016-02-04 8:44 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: JBottomley, martin.petersen, linux-scsi, linux-kernel,
linuxppc-dev, nfont, brking, hare
On Wed, Feb 03, 2016 at 05:28:31PM -0600, Tyrel Datwyler wrote:
> Add defines for mad version and mad os_type, and replace the magic
> numbers in set_adapter_info() accordingly.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvscsi.c | 4 ++--
> drivers/scsi/ibmvscsi/viosrp.h | 2 ++
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index 176260d..7e51615 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -283,8 +283,8 @@ static void set_adapter_info(struct ibmvscsi_host_data *hostdata)
> hostdata->madapter_info.partition_number =
> cpu_to_be32(partition_number);
>
> - hostdata->madapter_info.mad_version = cpu_to_be32(1);
> - hostdata->madapter_info.os_type = cpu_to_be32(2);
> + hostdata->madapter_info.mad_version = cpu_to_be32(SRP_MAD_VERSION_1);
> + hostdata->madapter_info.os_type = cpu_to_be32(SRP_MAD_OS_LINUX);
> }
>
> /**
> diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
> index 17f2de0..da9bb29 100644
> --- a/drivers/scsi/ibmvscsi/viosrp.h
> +++ b/drivers/scsi/ibmvscsi/viosrp.h
> @@ -38,6 +38,8 @@
> #define SRP_VERSION "16.a"
> #define SRP_MAX_IU_LEN 256
> #define SRP_MAX_LOC_LEN 32
> +#define SRP_MAD_VERSION_1 1
> +#define SRP_MAD_OS_LINUX 2
>
> union srp_iu {
> struct srp_login_req login_req;
> --
> 2.5.0
>
> --
> 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
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/6] ibmvscsi: Use of_root to access OF device tree root node
2016-02-03 23:28 ` [PATCH 4/6] ibmvscsi: Use of_root to access OF device tree root node Tyrel Datwyler
@ 2016-02-04 8:45 ` Johannes Thumshirn
0 siblings, 0 replies; 19+ messages in thread
From: Johannes Thumshirn @ 2016-02-04 8:45 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: JBottomley, martin.petersen, linux-scsi, linux-kernel,
linuxppc-dev, nfont, brking, hare
On Wed, Feb 03, 2016 at 05:28:32PM -0600, Tyrel Datwyler wrote:
> The root node of the OF device tree is exported as of_root. No need
> to look up the root by path name. Instead just get a reference
> directly via of_root.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvscsi.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index 7e51615..47cfe33 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -248,25 +248,23 @@ static void ibmvscsi_task(void *data)
>
> static void gather_partition_info(void)
> {
> - struct device_node *rootdn;
> -
> const char *ppartition_name;
> const __be32 *p_number_ptr;
>
> /* Retrieve information about this partition */
> - rootdn = of_find_node_by_path("/");
> - if (!rootdn) {
> + if (!of_root)
> return;
> - }
>
> - ppartition_name = of_get_property(rootdn, "ibm,partition-name", NULL);
> + of_node_get(of_root);
> +
> + ppartition_name = of_get_property(of_root, "ibm,partition-name", NULL);
> if (ppartition_name)
> strncpy(partition_name, ppartition_name,
> sizeof(partition_name));
> - p_number_ptr = of_get_property(rootdn, "ibm,partition-no", NULL);
> + p_number_ptr = of_get_property(of_root, "ibm,partition-no", NULL);
> if (p_number_ptr)
> partition_number = of_read_number(p_number_ptr, 1);
> - of_node_put(rootdn);
> + of_node_put(of_root);
> }
>
> static void set_adapter_info(struct ibmvscsi_host_data *hostdata)
> --
> 2.5.0
>
> --
> 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
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 6/6] ibmvscsi: Add endian conversions to sysfs attribute show functions
2016-02-03 23:28 ` [PATCH 6/6] ibmvscsi: Add endian conversions to sysfs attribute show functions Tyrel Datwyler
@ 2016-02-04 8:45 ` Johannes Thumshirn
0 siblings, 0 replies; 19+ messages in thread
From: Johannes Thumshirn @ 2016-02-04 8:45 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: JBottomley, martin.petersen, linux-scsi, linux-kernel,
linuxppc-dev, nfont, brking, hare
On Wed, Feb 03, 2016 at 05:28:34PM -0600, Tyrel Datwyler wrote:
> The values returned by the show functions for the host os_type,
> mad_version, and partition_number attributes get their values
> directly from the madapter_info struct whose associated fields are
> __be32 typed. Added endian conversion to ensure these values are
> sane on LE platforms.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvscsi.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index a2b8db1..3621ac5 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -1983,7 +1983,7 @@ static ssize_t show_host_partition_number(struct device *dev,
> int len;
>
> len = snprintf(buf, PAGE_SIZE, "%d\n",
> - hostdata->madapter_info.partition_number);
> + be32_to_cpu(hostdata->madapter_info.partition_number));
> return len;
> }
>
> @@ -2003,7 +2003,7 @@ static ssize_t show_host_mad_version(struct device *dev,
> int len;
>
> len = snprintf(buf, PAGE_SIZE, "%d\n",
> - hostdata->madapter_info.mad_version);
> + be32_to_cpu(hostdata->madapter_info.mad_version));
> return len;
> }
>
> @@ -2022,7 +2022,8 @@ static ssize_t show_host_os_type(struct device *dev,
> struct ibmvscsi_host_data *hostdata = shost_priv(shost);
> int len;
>
> - len = snprintf(buf, PAGE_SIZE, "%d\n", hostdata->madapter_info.os_type);
> + len = snprintf(buf, PAGE_SIZE, "%d\n",
> + be32_to_cpu(hostdata->madapter_info.os_type));
> return len;
> }
>
> --
> 2.5.0
>
> --
> 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
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface
2016-02-04 8:03 ` Johannes Thumshirn
@ 2016-02-04 17:48 ` Tyrel Datwyler
2016-02-05 8:34 ` Johannes Thumshirn
0 siblings, 1 reply; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-04 17:48 UTC (permalink / raw)
To: Johannes Thumshirn, Tyrel Datwyler
Cc: martin.petersen, linux-scsi, JBottomley, linux-kernel, hare,
brking, nfont, linuxppc-dev
On 02/04/2016 12:03 AM, Johannes Thumshirn wrote:
> On Wed, Feb 03, 2016 at 05:28:33PM -0600, Tyrel Datwyler wrote:
>> A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in
>> the code for some time. From what information I've gathered from
>> Brian King this was likely implemented on the host side in a SLES 9
>> based VIOS, which is no longer supported anywhere. Further, it is
>> not defined in PAPR or supported by any AIX based VIOS.
>>
>> Treating as bit rot and removing the sysfs interface and associated
>> host config code accordingly.
>
> Doesn't removing a sysfs interface potentially break userspace code?
>
In the general case yes, but I feel in this case no. First, Reading from
this config attribute of a vscsi host adapter always returns nothing.
Second, any userspace code using this attribute better be checking for
the existence of config. Just a quick look for
/sys/class/scsi_host/host*/config under other host adapters on my system
I find that attribute doesn't exist for any of them.
If there is truly enough concern that somebody may actually be accessing
this useless attribute from userspace then we can still strip out the
unsupported code, but leave the attribute and return nothing directly
from the show function.
-Tyrel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values
2016-02-03 23:28 ` [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values Tyrel Datwyler
2016-02-04 8:39 ` Johannes Thumshirn
@ 2016-02-04 19:55 ` Manoj Kumar
2016-02-04 21:25 ` Tyrel Datwyler
1 sibling, 1 reply; 19+ messages in thread
From: Manoj Kumar @ 2016-02-04 19:55 UTC (permalink / raw)
To: Tyrel Datwyler, JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, linuxppc-dev, nfont,
brking, hare
On 2/3/2016 5:28 PM, Tyrel Datwyler wrote:
> The PAPR defines four valid header values for the first byte of a
> CRQ message. Namely, an unused/empty message (0x00), a valid
> command/response entry (0x80), a valid initialization entry (0xC0),
> and a transport event (0xFF). Define these values as enums and use
> them in the code in place of their magic number equivalents.
>
> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> ---
> drivers/scsi/ibmvscsi/ibmvscsi.c | 14 +++++++-------
> drivers/scsi/ibmvscsi/viosrp.h | 7 +++++++
> 2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index adfef9d..176260d 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -182,7 +182,7 @@ static struct viosrp_crq *crq_queue_next_crq(struct crq_queue *queue)
>
> spin_lock_irqsave(&queue->lock, flags);
> crq = &queue->msgs[queue->cur];
> - if (crq->valid & 0x80) {
> + if (crq->valid & VIOSRP_CRQ_VALID) {
After the switch to enums, bitwise operators are a bit misleading.
Especially in this case since multiple values would satisfy this
condition: VIOSRP_CRQ_VALID, VIOSRP_CRQ_INIT and
VIOSRP_CRQ_TRANSPORT.
If 'valid' will only have one of these four enums defined, would
this be better written as:
if (crq->valid != VIOSRP_CRQ_FREE)
> if (++queue->cur == queue->size)
> queue->cur = 0;
>
> @@ -231,7 +231,7 @@ static void ibmvscsi_task(void *data)
> /* Pull all the valid messages off the CRQ */
> while ((crq = crq_queue_next_crq(&hostdata->queue)) != NULL) {
> ibmvscsi_handle_crq(crq, hostdata);
> - crq->valid = 0x00;
> + crq->valid = VIOSRP_CRQ_FREE;
> }
>
> vio_enable_interrupts(vdev);
> @@ -239,7 +239,7 @@ static void ibmvscsi_task(void *data)
> if (crq != NULL) {
> vio_disable_interrupts(vdev);
> ibmvscsi_handle_crq(crq, hostdata);
> - crq->valid = 0x00;
> + crq->valid = VIOSRP_CRQ_FREE;
> } else {
> done = 1;
> }
> @@ -474,7 +474,7 @@ static int initialize_event_pool(struct event_pool *pool,
> struct srp_event_struct *evt = &pool->events[i];
> memset(&evt->crq, 0x00, sizeof(evt->crq));
> atomic_set(&evt->free, 1);
> - evt->crq.valid = 0x80;
> + evt->crq.valid = VIOSRP_CRQ_VALID;
> evt->crq.IU_length = cpu_to_be16(sizeof(*evt->xfer_iu));
> evt->crq.IU_data_ptr = cpu_to_be64(pool->iu_token +
> sizeof(*evt->xfer_iu) * i);
> @@ -1767,7 +1767,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
> struct srp_event_struct *evt_struct =
> (__force struct srp_event_struct *)crq->IU_data_ptr;
> switch (crq->valid) {
> - case 0xC0: /* initialization */
> + case VIOSRP_CRQ_INIT: /* initialization */
> switch (crq->format) {
> case 0x01: /* Initialization message */
> dev_info(hostdata->dev, "partner initialized\n");
> @@ -1791,7 +1791,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
> dev_err(hostdata->dev, "unknown crq message type: %d\n", crq->format);
> }
> return;
> - case 0xFF: /* Hypervisor telling us the connection is closed */
> + case VIOSRP_CRQ_TRANSPORT: /* Hypervisor telling us the connection is closed */
> scsi_block_requests(hostdata->host);
> atomic_set(&hostdata->request_limit, 0);
> if (crq->format == 0x06) {
> @@ -1807,7 +1807,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
> ibmvscsi_reset_host(hostdata);
> }
> return;
> - case 0x80: /* real payload */
> + case VIOSRP_CRQ_VALID: /* real payload */
> break;
> default:
> dev_err(hostdata->dev, "got an invalid message type 0x%02x\n",
> diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
> index d1044e9..17f2de0 100644
> --- a/drivers/scsi/ibmvscsi/viosrp.h
> +++ b/drivers/scsi/ibmvscsi/viosrp.h
> @@ -51,6 +51,13 @@ union srp_iu {
> u8 reserved[SRP_MAX_IU_LEN];
> };
>
> +enum viosrp_crq_headers {
> + VIOSRP_CRQ_FREE = 0x00,
> + VIOSRP_CRQ_VALID = 0x80,
> + VIOSRP_CRQ_INIT = 0xC0,
> + VIOSRP_CRQ_TRANSPORT = 0xFF
> +};
> +
> enum viosrp_crq_formats {
> VIOSRP_SRP_FORMAT = 0x01,
> VIOSRP_MAD_FORMAT = 0x02,
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values
2016-02-04 19:55 ` Manoj Kumar
@ 2016-02-04 21:25 ` Tyrel Datwyler
2016-02-09 17:41 ` Manoj Kumar
0 siblings, 1 reply; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-04 21:25 UTC (permalink / raw)
To: manoj, Tyrel Datwyler, JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, hare, brking, nfont,
linuxppc-dev
On 02/04/2016 11:55 AM, Manoj Kumar wrote:
> On 2/3/2016 5:28 PM, Tyrel Datwyler wrote:
>> The PAPR defines four valid header values for the first byte of a
>> CRQ message. Namely, an unused/empty message (0x00), a valid
>> command/response entry (0x80), a valid initialization entry (0xC0),
>> and a transport event (0xFF). Define these values as enums and use
>> them in the code in place of their magic number equivalents.
>>
>> Signed-off-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
>> ---
>> drivers/scsi/ibmvscsi/ibmvscsi.c | 14 +++++++-------
>> drivers/scsi/ibmvscsi/viosrp.h | 7 +++++++
>> 2 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> index adfef9d..176260d 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
>> @@ -182,7 +182,7 @@ static struct viosrp_crq *crq_queue_next_crq(struct crq_queue *queue)
>>
>> spin_lock_irqsave(&queue->lock, flags);
>> crq = &queue->msgs[queue->cur];
>> - if (crq->valid & 0x80) {
>> + if (crq->valid & VIOSRP_CRQ_VALID) {
>
> After the switch to enums, bitwise operators are a bit misleading.
> Especially in this case since multiple values would satisfy this
> condition: VIOSRP_CRQ_VALID, VIOSRP_CRQ_INIT and
> VIOSRP_CRQ_TRANSPORT.
Yeah, I can see how that is confusing. Since, all three possible valid
crq message types have the first bit set I think this was originally a
cute hack to grab anything that was likely valid. Then in
ibmvscsi_handle_crq() we explicitly match the full header value in a
switch statement logging anything that turned out actually invalid.
>
> If 'valid' will only have one of these four enums defined, would
> this be better written as:
>
> if (crq->valid != VIOSRP_CRQ_FREE)
This definitely would make the logic easier to read and follow. Also,
this would make sure any crq with an invalid header that doesn't have
its first bit set will also be logged by the ibmvscsi_handle_crq()
switch statement default block and not silently ignored.
-Tyrel
>
>> if (++queue->cur == queue->size)
>> queue->cur = 0;
>>
>> @@ -231,7 +231,7 @@ static void ibmvscsi_task(void *data)
>> /* Pull all the valid messages off the CRQ */
>> while ((crq = crq_queue_next_crq(&hostdata->queue)) != NULL) {
>> ibmvscsi_handle_crq(crq, hostdata);
>> - crq->valid = 0x00;
>> + crq->valid = VIOSRP_CRQ_FREE;
>> }
>>
>> vio_enable_interrupts(vdev);
>> @@ -239,7 +239,7 @@ static void ibmvscsi_task(void *data)
>> if (crq != NULL) {
>> vio_disable_interrupts(vdev);
>> ibmvscsi_handle_crq(crq, hostdata);
>> - crq->valid = 0x00;
>> + crq->valid = VIOSRP_CRQ_FREE;
>> } else {
>> done = 1;
>> }
>> @@ -474,7 +474,7 @@ static int initialize_event_pool(struct event_pool *pool,
>> struct srp_event_struct *evt = &pool->events[i];
>> memset(&evt->crq, 0x00, sizeof(evt->crq));
>> atomic_set(&evt->free, 1);
>> - evt->crq.valid = 0x80;
>> + evt->crq.valid = VIOSRP_CRQ_VALID;
>> evt->crq.IU_length = cpu_to_be16(sizeof(*evt->xfer_iu));
>> evt->crq.IU_data_ptr = cpu_to_be64(pool->iu_token +
>> sizeof(*evt->xfer_iu) * i);
>> @@ -1767,7 +1767,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
>> struct srp_event_struct *evt_struct =
>> (__force struct srp_event_struct *)crq->IU_data_ptr;
>> switch (crq->valid) {
>> - case 0xC0: /* initialization */
>> + case VIOSRP_CRQ_INIT: /* initialization */
>> switch (crq->format) {
>> case 0x01: /* Initialization message */
>> dev_info(hostdata->dev, "partner initialized\n");
>> @@ -1791,7 +1791,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
>> dev_err(hostdata->dev, "unknown crq message type: %d\n", crq->format);
>> }
>> return;
>> - case 0xFF: /* Hypervisor telling us the connection is closed */
>> + case VIOSRP_CRQ_TRANSPORT: /* Hypervisor telling us the connection is closed */
>> scsi_block_requests(hostdata->host);
>> atomic_set(&hostdata->request_limit, 0);
>> if (crq->format == 0x06) {
>> @@ -1807,7 +1807,7 @@ static void ibmvscsi_handle_crq(struct viosrp_crq *crq,
>> ibmvscsi_reset_host(hostdata);
>> }
>> return;
>> - case 0x80: /* real payload */
>> + case VIOSRP_CRQ_VALID: /* real payload */
>> break;
>> default:
>> dev_err(hostdata->dev, "got an invalid message type 0x%02x\n",
>> diff --git a/drivers/scsi/ibmvscsi/viosrp.h b/drivers/scsi/ibmvscsi/viosrp.h
>> index d1044e9..17f2de0 100644
>> --- a/drivers/scsi/ibmvscsi/viosrp.h
>> +++ b/drivers/scsi/ibmvscsi/viosrp.h
>> @@ -51,6 +51,13 @@ union srp_iu {
>> u8 reserved[SRP_MAX_IU_LEN];
>> };
>>
>> +enum viosrp_crq_headers {
>> + VIOSRP_CRQ_FREE = 0x00,
>> + VIOSRP_CRQ_VALID = 0x80,
>> + VIOSRP_CRQ_INIT = 0xC0,
>> + VIOSRP_CRQ_TRANSPORT = 0xFF
>> +};
>> +
>> enum viosrp_crq_formats {
>> VIOSRP_SRP_FORMAT = 0x01,
>> VIOSRP_MAD_FORMAT = 0x02,
>>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface
2016-02-04 17:48 ` Tyrel Datwyler
@ 2016-02-05 8:34 ` Johannes Thumshirn
0 siblings, 0 replies; 19+ messages in thread
From: Johannes Thumshirn @ 2016-02-05 8:34 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, JBottomley,
linux-kernel, hare, brking, nfont, linuxppc-dev
On Thu, Feb 04, 2016 at 09:48:23AM -0800, Tyrel Datwyler wrote:
> On 02/04/2016 12:03 AM, Johannes Thumshirn wrote:
> > On Wed, Feb 03, 2016 at 05:28:33PM -0600, Tyrel Datwyler wrote:
> >> A VIOSRP_HOST_CONFIG_TYPE management datagram (MAD) has existed in
> >> the code for some time. From what information I've gathered from
> >> Brian King this was likely implemented on the host side in a SLES 9
> >> based VIOS, which is no longer supported anywhere. Further, it is
> >> not defined in PAPR or supported by any AIX based VIOS.
> >>
> >> Treating as bit rot and removing the sysfs interface and associated
> >> host config code accordingly.
> >
> > Doesn't removing a sysfs interface potentially break userspace code?
> >
>
> In the general case yes, but I feel in this case no. First, Reading from
> this config attribute of a vscsi host adapter always returns nothing.
> Second, any userspace code using this attribute better be checking for
> the existence of config. Just a quick look for
> /sys/class/scsi_host/host*/config under other host adapters on my system
> I find that attribute doesn't exist for any of them.
>
> If there is truly enough concern that somebody may actually be accessing
> this useless attribute from userspace then we can still strip out the
> unsupported code, but leave the attribute and return nothing directly
> from the show function.
Which is what I kinda prefer. Slowly deprecate and phase out, but don't break
userspace. You never know who is writing some obscure piece of code relaying on
some sysfs attribute.
Thanks,
Johannes
--
Johannes Thumshirn Storage
jthumshirn@suse.de +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values
2016-02-04 21:25 ` Tyrel Datwyler
@ 2016-02-09 17:41 ` Manoj Kumar
2016-02-09 18:27 ` Tyrel Datwyler
0 siblings, 1 reply; 19+ messages in thread
From: Manoj Kumar @ 2016-02-09 17:41 UTC (permalink / raw)
To: Tyrel Datwyler, JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, hare, brking, nfont,
linuxppc-dev
> Yeah, I can see how that is confusing. Since, all three possible valid
> crq message types have the first bit set I think this was originally a
> cute hack to grab anything that was likely valid. Then in
> ibmvscsi_handle_crq() we explicitly match the full header value in a
> switch statement logging anything that turned out actually invalid.
>
>>
>> If 'valid' will only have one of these four enums defined, would
>> this be better written as:
>>
>> if (crq->valid != VIOSRP_CRQ_FREE)
>
> This definitely would make the logic easier to read and follow. Also,
> this would make sure any crq with an invalid header that doesn't have
> its first bit set will also be logged by the ibmvscsi_handle_crq()
> switch statement default block and not silently ignored.
>
> -Tyrel
Sounds good, Tyrel. Does this mean I should expect a v2 of this patch
series?
- Manoj N. Kumar
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values
2016-02-09 17:41 ` Manoj Kumar
@ 2016-02-09 18:27 ` Tyrel Datwyler
0 siblings, 0 replies; 19+ messages in thread
From: Tyrel Datwyler @ 2016-02-09 18:27 UTC (permalink / raw)
To: manoj, JBottomley
Cc: martin.petersen, linux-scsi, linux-kernel, hare, brking, nfont,
linuxppc-dev
On 02/09/2016 09:41 AM, Manoj Kumar wrote:
>> Yeah, I can see how that is confusing. Since, all three possible valid
>> crq message types have the first bit set I think this was originally a
>> cute hack to grab anything that was likely valid. Then in
>> ibmvscsi_handle_crq() we explicitly match the full header value in a
>> switch statement logging anything that turned out actually invalid.
>>
>>>
>>> If 'valid' will only have one of these four enums defined, would
>>> this be better written as:
>>>
>>> if (crq->valid != VIOSRP_CRQ_FREE)
>>
>> This definitely would make the logic easier to read and follow. Also,
>> this would make sure any crq with an invalid header that doesn't have
>> its first bit set will also be logged by the ibmvscsi_handle_crq()
>> switch statement default block and not silently ignored.
>>
>> -Tyrel
>
> Sounds good, Tyrel. Does this mean I should expect a v2 of this patch
> series?
>
> - Manoj N. Kumar
Haven't had a chance to clean up and resubmit, but yes there will be a
v2 coming along soon.
-Tyrel
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-02-09 18:27 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-03 23:28 [PATCH 0/6] ibmvscsi: code cleanup Tyrel Datwyler
2016-02-03 23:28 ` [PATCH 1/6] ibmvscsi: Correct values for several viosrp_crq_format enums Tyrel Datwyler
2016-02-04 8:38 ` Johannes Thumshirn
2016-02-03 23:28 ` [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values Tyrel Datwyler
2016-02-04 8:39 ` Johannes Thumshirn
2016-02-04 19:55 ` Manoj Kumar
2016-02-04 21:25 ` Tyrel Datwyler
2016-02-09 17:41 ` Manoj Kumar
2016-02-09 18:27 ` Tyrel Datwyler
2016-02-03 23:28 ` [PATCH 3/6] ibmvscsi: Replace magic values in set_adpater_info() with defines Tyrel Datwyler
2016-02-04 8:44 ` Johannes Thumshirn
2016-02-03 23:28 ` [PATCH 4/6] ibmvscsi: Use of_root to access OF device tree root node Tyrel Datwyler
2016-02-04 8:45 ` Johannes Thumshirn
2016-02-03 23:28 ` [PATCH 5/6] ibmvscsi: Remove unsupported host config MAD and sysfs interface Tyrel Datwyler
2016-02-04 8:03 ` Johannes Thumshirn
2016-02-04 17:48 ` Tyrel Datwyler
2016-02-05 8:34 ` Johannes Thumshirn
2016-02-03 23:28 ` [PATCH 6/6] ibmvscsi: Add endian conversions to sysfs attribute show functions Tyrel Datwyler
2016-02-04 8:45 ` Johannes Thumshirn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).