Re: [PATCH 1/6] ibmvfc: byte swap login_buf.resp values in attribute show functions

[Martin K. Petersen]

Tyrel,

> The checkpatch script only warns at 100 char lines these days. To be
> fair though I did have two lines go over that limit by a couple
> characters, there are a couple commit log typos, and I had an if
> keyword with no space after before the opening parenthesis. So, I'll
> happily re-spin.

Please tweak the little things that need fixing and resubmit.

> However, for my info going forward is the SCSI subsystem sticking to
> 80 char lines as a hard limit?

As far as I'm concerned the 80 char limit is mainly about ensuring that
the code is structured in a sensible way. Typesetting best practices
also suggest that longer lines are harder to read. So while I generally
don't strictly enforce the 80 char limit for drivers, I do push back if
I feel that readability could be improved by breaking the line or
restructuring the code.

Use your best judgment to optimize for readability.

Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 1/6] ibmvfc: byte swap login_buf.resp values in attribute show functions

[Tyrel Datwyler]

On 11/12/20 1:37 AM, Christoph Hellwig wrote:
> On Wed, Nov 11, 2020 at 07:04:37PM -0600, Tyrel Datwyler wrote:
>> Both ibmvfc_show_host_(capabilities|npiv_version) functions retrieve
>> values from vhost->login_buf.resp buffer. This is the MAD response
>> buffer from the VIOS and as such any multi-byte non-string values are in
>> big endian format.
>>
>> Byte swap these values to host cpu endian format for better human
>> readability.
> 
> The whole series creates tons of pointlessly over 80 char lines.
> Please do a quick fixup.
> 

The checkpatch script only warns at 100 char lines these days. To be fair though
I did have two lines go over that limit by a couple characters, there are a
couple commit log typos, and I had an if keyword with no space after before the
opening parenthesis. So, I'll happily re-spin.

However, for my info going forward is the SCSI subsystem sticking to 80 char
lines as a hard limit?

-Tyrel

Re: [PATCH 1/6] ibmvfc: byte swap login_buf.resp values in attribute show functions

[Christoph Hellwig]

On Wed, Nov 11, 2020 at 07:04:37PM -0600, Tyrel Datwyler wrote:
> Both ibmvfc_show_host_(capabilities|npiv_version) functions retrieve
> values from vhost->login_buf.resp buffer. This is the MAD response
> buffer from the VIOS and as such any multi-byte non-string values are in
> big endian format.
> 
> Byte swap these values to host cpu endian format for better human
> readability.

The whole series creates tons of pointlessly over 80 char lines.
Please do a quick fixup.

[PATCH 1/6] ibmvfc: byte swap login_buf.resp values in attribute show functions

[Tyrel Datwyler]

Both ibmvfc_show_host_(capabilities|npiv_version) functions retrieve
values from vhost->login_buf.resp buffer. This is the MAD response
buffer from the VIOS and as such any multi-byte non-string values are in
big endian format.

Byte swap these values to host cpu endian format for better human
readability.

Signed-off-by: Tyrel Datwyler 
---
 drivers/scsi/ibmvscsi/ibmvfc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 070cf516b98f..01fe65de9086 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -3025,7 +3025,7 @@ static ssize_t ibmvfc_show_host_npiv_version(struct 
device *dev,
 {
struct Scsi_Host *shost = class_to_shost(dev);
struct ibmvfc_host *vhost = shost_priv(shost);
-   return snprintf(buf, PAGE_SIZE, "%d\n", vhost->login_buf->resp.version);
+   return snprintf(buf, PAGE_SIZE, "%d\n", 
be32_to_cpu(vhost->login_buf->resp.version));
 }
 
 static ssize_t ibmvfc_show_host_capabilities(struct device *dev,
@@ -3033,7 +3033,7 @@ static ssize_t ibmvfc_show_host_capabilities(struct 
device *dev,
 {
struct Scsi_Host *shost = class_to_shost(dev);
struct ibmvfc_host *vhost = shost_priv(shost);
-   return snprintf(buf, PAGE_SIZE, "%llx\n", 
vhost->login_buf->resp.capabilities);
+   return snprintf(buf, PAGE_SIZE, "%llx\n", 
be64_to_cpu(vhost->login_buf->resp.capabilities));
 }
 
 /**
-- 
2.27.0