Re: [PATCH 1/6] ibmvfc: byte swap login_buf.resp values in attribute show functions
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
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
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
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