Linux SCSI subsystem development
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Himanshu Madhani <himanshu.madhani@cavium.com>,
	martin.petersen@oracle.com
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH] qla2xxx: Fix compile warning
Date: Fri, 30 Jun 2017 17:10:23 -0700	[thread overview]
Message-ID: <1498867823.8086.17.camel@HansenPartnership.com> (raw)
In-Reply-To: <20170630235622.4602-1-himanshu.madhani@cavium.com>

OK, the subject is technically true, but to most people "fix compile
warning" means yet another pointless patch.  This patch isn't
pointless, though, is it?  The driver is actually broken on big endian
systems without it, so the subject should probably say this because
it's important for distros triaging whether to backport this fix if
they backport the nvme components.

On Fri, 2017-06-30 at 16:56 -0700, Himanshu Madhani wrote:
> drivers/scsi/qla2xxx/qla_nvme.c: In function 'qla2x00_start_nvme_mq':
> include/uapi/linux/byteorder/big_endian.h:32:26: warning: large
> integer implicitly truncated to unsigned type [-Woverflow]
>  #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))

This text doesn't explain why you've fixed the bug this way.  It has to
do with the 32 byte reads the card does for the mailbox, so you should
explain what the actual problem the compile warning picked up is and
why you fixed it by doing an entire 32 bit write.

Preferably ASAP because the merge window will open on Monday.

James

  reply	other threads:[~2017-07-01  0:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-30 23:56 [PATCH] qla2xxx: Fix compile warning Himanshu Madhani
2017-07-01  0:10 ` James Bottomley [this message]
  -- strict thread matches above, loose matches on Subject: below --
2017-06-28 22:00 Himanshu Madhani
2017-06-28 22:12 ` James Bottomley
2017-06-28 23:03   ` Madhani, Himanshu
2017-06-06 20:55 Himanshu Madhani
2017-06-13  1:14 ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1498867823.8086.17.camel@HansenPartnership.com \
    --to=james.bottomley@hansenpartnership.com \
    --cc=himanshu.madhani@cavium.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox