diff mbox

[11/15] mpt3sas: Report Firmware Package Version from HBA Driver.

Message ID 1522402644-3016-12-git-send-email-chaitra.basappa@broadcom.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Chaitra P B March 30, 2018, 9:37 a.m. UTC 
Added function _base_display_fwpkg_version, which sends FWUpload
request to pull FW package version from FW Image Header.
Now driver prints FW package version in addition to FW
version if the PackageVersion is valid.

Signed-off-by: Chaitra P B <chaitra.basappa@broadcom.com>
Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 115 +++++++++++++++++++++++++++++++++++-
 drivers/scsi/mpt3sas/mpt3sas_base.h |   1 +
 2 files changed, 114 insertions(+), 2 deletions(-)

Comments

Bart Van Assche March 30, 2018, 4:34 p.m. UTC | #1 
On Fri, 2018-03-30 at 15:07 +0530, Chaitra P B wrote:
> +					pr_info(MPT3SAS_FMT "FW Package Version"

> +					"(%02d.%02d.%02d.%02d)\n",

> +					ioc->name,

> +					((FWImgHdr->PackageVersion.Word)

> +							& 0xFF000000) >> 24,

> +					((FWImgHdr->PackageVersion.Word)

> +							& 0x00FF0000) >> 16,

> +					((FWImgHdr->PackageVersion.Word)

> +							& 0x0000FF00) >> 8,

> +					(FWImgHdr->PackageVersion.Word)

> +							& 0x000000FF);


Since FWImgHdr->PackageVersion.Word has type __le32 I don't think that the
above code will work correctly on big endian systems. Please use the Dev,
Unit, Minor and Major members of MPI2_VERSION_STRUCT instead of open-coding
access to these members.

Thanks,

Bart.
kernel test robot March 31, 2018, 8:44 a.m. UTC | #2 
Hi Chaitra,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on next-20180329]
[cannot apply to scsi/for-next v4.16-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chaitra-P-B/mpt3sas-Enhancements-and-Defect-fixes/20180331-123801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/scsi/mpt3sas/mpt3sas_base.c:3878:32: sparse: incorrect type in assignment (different base types) @@    expected restricted __le32 [usertype] ImageSize @@    got unsigned long [unsrestricted __le32 [usertype] ImageSize @@
   drivers/scsi/mpt3sas/mpt3sas_base.c:3878:32:    expected restricted __le32 [usertype] ImageSize
   drivers/scsi/mpt3sas/mpt3sas_base.c:3878:32:    got unsigned long [unsigned] [assigned] [usertype] data_length
>> drivers/scsi/mpt3sas/mpt3sas_base.c:3903:63: sparse: incorrect type in assignment (different base types) @@    expected restricted __le32 [usertype] Word @@    got unsignrestricted __le32 [usertype] Word @@
   drivers/scsi/mpt3sas/mpt3sas_base.c:3903:63:    expected restricted __le32 [usertype] Word
   drivers/scsi/mpt3sas/mpt3sas_base.c:3903:63:    got unsigned int [unsigned] [usertype] <noident>
   drivers/scsi/mpt3sas/mpt3sas_base.c:3906:41: sparse: restricted __le32 degrades to integer
   drivers/scsi/mpt3sas/mpt3sas_base.c:3906:41: sparse: restricted __le32 degrades to integer
   drivers/scsi/mpt3sas/mpt3sas_base.c:3906:41: sparse: restricted __le32 degrades to integer
   drivers/scsi/mpt3sas/mpt3sas_base.c:3906:41: sparse: restricted __le32 degrades to integer

vim +3878 drivers/scsi/mpt3sas/mpt3sas_base.c

  3826	
  3827	/**
  3828	 * _base_display_fwpkg_version - sends FWUpload request to pull FWPkg
  3829	 *				version from FW Image Header.
  3830	 * @ioc: per adapter object
  3831	 *
  3832	 * Returns 0 for success, non-zero for failure.
  3833	 */
  3834		static int
  3835	_base_display_fwpkg_version(struct MPT3SAS_ADAPTER *ioc)
  3836	{
  3837		Mpi2FWImageHeader_t *FWImgHdr;
  3838		Mpi25FWUploadRequest_t *mpi_request;
  3839		Mpi2FWUploadReply_t mpi_reply;
  3840		int r = 0;
  3841		void *fwpkg_data = NULL;
  3842		dma_addr_t fwpkg_data_dma;
  3843		u16 smid, ioc_status;
  3844		size_t data_length;
  3845	
  3846		dinitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
  3847					__func__));
  3848	
  3849		if (ioc->base_cmds.status & MPT3_CMD_PENDING) {
  3850			pr_err(MPT3SAS_FMT "%s: internal command already in use\n",
  3851					ioc->name, __func__);
  3852			return -EAGAIN;
  3853		}
  3854	
  3855		data_length = sizeof(Mpi2FWImageHeader_t);
  3856		fwpkg_data = pci_alloc_consistent(ioc->pdev, data_length,
  3857				&fwpkg_data_dma);
  3858		if (!fwpkg_data) {
  3859			pr_err(MPT3SAS_FMT "failure at %s:%d/%s()!\n",
  3860					ioc->name, __FILE__, __LINE__, __func__);
  3861			return -ENOMEM;
  3862		}
  3863	
  3864		smid = mpt3sas_base_get_smid(ioc, ioc->base_cb_idx);
  3865		if (!smid) {
  3866			pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n",
  3867					ioc->name, __func__);
  3868			r = -EAGAIN;
  3869			goto out;
  3870		}
  3871	
  3872		ioc->base_cmds.status = MPT3_CMD_PENDING;
  3873		mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
  3874		ioc->base_cmds.smid = smid;
  3875		memset(mpi_request, 0, sizeof(Mpi25FWUploadRequest_t));
  3876		mpi_request->Function = MPI2_FUNCTION_FW_UPLOAD;
  3877		mpi_request->ImageType = MPI2_FW_UPLOAD_ITYPE_FW_FLASH;
> 3878		mpi_request->ImageSize = data_length;
  3879		ioc->build_sg(ioc, &mpi_request->SGL, 0, 0, fwpkg_data_dma,
  3880				data_length);
  3881		init_completion(&ioc->base_cmds.done);
  3882		mpt3sas_base_put_smid_default(ioc, smid);
  3883		/* Wait for 15 seconds */
  3884		wait_for_completion_timeout(&ioc->base_cmds.done,
  3885				FW_IMG_HDR_READ_TIMEOUT*HZ);
  3886		pr_info(MPT3SAS_FMT "%s: complete\n",
  3887				ioc->name, __func__);
  3888		if (!(ioc->base_cmds.status & MPT3_CMD_COMPLETE)) {
  3889			pr_err(MPT3SAS_FMT "%s: timeout\n",
  3890					ioc->name, __func__);
  3891			_debug_dump_mf(mpi_request,
  3892					sizeof(Mpi25FWUploadRequest_t)/4);
  3893			r = -ETIME;
  3894		} else {
  3895			memset(&mpi_reply, 0, sizeof(Mpi2FWUploadReply_t));
  3896			if (ioc->base_cmds.status & MPT3_CMD_REPLY_VALID) {
  3897				memcpy(&mpi_reply, ioc->base_cmds.reply,
  3898						sizeof(Mpi2FWUploadReply_t));
  3899				ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
  3900							MPI2_IOCSTATUS_MASK;
  3901				if (ioc_status == MPI2_IOCSTATUS_SUCCESS) {
  3902					FWImgHdr = (Mpi2FWImageHeader_t *)fwpkg_data;
> 3903					FWImgHdr->PackageVersion.Word =
  3904					    le32_to_cpu(FWImgHdr->PackageVersion.Word);
  3905					if (FWImgHdr->PackageVersion.Word) {
  3906						pr_info(MPT3SAS_FMT "FW Package Version"
  3907						"(%02d.%02d.%02d.%02d)\n",
  3908						ioc->name,
  3909						((FWImgHdr->PackageVersion.Word)
  3910								& 0xFF000000) >> 24,
  3911						((FWImgHdr->PackageVersion.Word)
  3912								& 0x00FF0000) >> 16,
  3913						((FWImgHdr->PackageVersion.Word)
  3914								& 0x0000FF00) >> 8,
  3915						(FWImgHdr->PackageVersion.Word)
  3916								& 0x000000FF);
  3917					}
  3918				} else {
  3919					_debug_dump_mf(&mpi_reply,
  3920							sizeof(Mpi2FWUploadReply_t)/4);
  3921				}
  3922			}
  3923		}
  3924		ioc->base_cmds.status = MPT3_CMD_NOT_USED;
  3925	out:
  3926		if (fwpkg_data)
  3927			pci_free_consistent(ioc->pdev, data_length, fwpkg_data,
  3928					fwpkg_data_dma);
  3929		return r;
  3930	}
  3931	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Chaitra P B April 2, 2018, 7:27 a.m. UTC | #3 
Bart,
 Agreed and patches will be posted with below changes.

Thanks,
 Chaitra


-----Original Message-----
From: Bart Van Assche [mailto:Bart.VanAssche@wdc.com]
Sent: Friday, March 30, 2018 10:05 PM
To: chaitra.basappa@broadcom.com; linux-scsi@vger.kernel.org
Cc: Sathya.Prakash@broadcom.com; suganath-prabu.subramani@broadcom.com;
sreekanth.reddy@broadcom.com
Subject: Re: [PATCH 11/15] mpt3sas: Report Firmware Package Version from HBA
Driver.

On Fri, 2018-03-30 at 15:07 +0530, Chaitra P B wrote:
> +					pr_info(MPT3SAS_FMT "FW Package Version"
> +					"(%02d.%02d.%02d.%02d)\n",
> +					ioc->name,
> +					((FWImgHdr->PackageVersion.Word)
> +							& 0xFF000000) >> 24,
> +					((FWImgHdr->PackageVersion.Word)
> +							& 0x00FF0000) >> 16,
> +					((FWImgHdr->PackageVersion.Word)
> +							& 0x0000FF00) >> 8,
> +					(FWImgHdr->PackageVersion.Word)
> +							& 0x000000FF);

Since FWImgHdr->PackageVersion.Word has type __le32 I don't think that the
above code will work correctly on big endian systems. Please use the Dev,
Unit, Minor and Major members of MPI2_VERSION_STRUCT instead of open-coding
access to these members.

Thanks,

Bart.
diff mbox

Patch

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index dca0782..94c69aa 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -3825,6 +3825,111 @@  _base_display_OEMs_branding(struct MPT3SAS_ADAPTER *ioc)
 }
 
 /**
+ * _base_display_fwpkg_version - sends FWUpload request to pull FWPkg
+ *				version from FW Image Header.
+ * @ioc: per adapter object
+ *
+ * Returns 0 for success, non-zero for failure.
+ */
+	static int
+_base_display_fwpkg_version(struct MPT3SAS_ADAPTER *ioc)
+{
+	Mpi2FWImageHeader_t *FWImgHdr;
+	Mpi25FWUploadRequest_t *mpi_request;
+	Mpi2FWUploadReply_t mpi_reply;
+	int r = 0;
+	void *fwpkg_data = NULL;
+	dma_addr_t fwpkg_data_dma;
+	u16 smid, ioc_status;
+	size_t data_length;
+
+	dinitprintk(ioc, pr_info(MPT3SAS_FMT "%s\n", ioc->name,
+				__func__));
+
+	if (ioc->base_cmds.status & MPT3_CMD_PENDING) {
+		pr_err(MPT3SAS_FMT "%s: internal command already in use\n",
+				ioc->name, __func__);
+		return -EAGAIN;
+	}
+
+	data_length = sizeof(Mpi2FWImageHeader_t);
+	fwpkg_data = pci_alloc_consistent(ioc->pdev, data_length,
+			&fwpkg_data_dma);
+	if (!fwpkg_data) {
+		pr_err(MPT3SAS_FMT "failure at %s:%d/%s()!\n",
+				ioc->name, __FILE__, __LINE__, __func__);
+		return -ENOMEM;
+	}
+
+	smid = mpt3sas_base_get_smid(ioc, ioc->base_cb_idx);
+	if (!smid) {
+		pr_err(MPT3SAS_FMT "%s: failed obtaining a smid\n",
+				ioc->name, __func__);
+		r = -EAGAIN;
+		goto out;
+	}
+
+	ioc->base_cmds.status = MPT3_CMD_PENDING;
+	mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
+	ioc->base_cmds.smid = smid;
+	memset(mpi_request, 0, sizeof(Mpi25FWUploadRequest_t));
+	mpi_request->Function = MPI2_FUNCTION_FW_UPLOAD;
+	mpi_request->ImageType = MPI2_FW_UPLOAD_ITYPE_FW_FLASH;
+	mpi_request->ImageSize = data_length;
+	ioc->build_sg(ioc, &mpi_request->SGL, 0, 0, fwpkg_data_dma,
+			data_length);
+	init_completion(&ioc->base_cmds.done);
+	mpt3sas_base_put_smid_default(ioc, smid);
+	/* Wait for 15 seconds */
+	wait_for_completion_timeout(&ioc->base_cmds.done,
+			FW_IMG_HDR_READ_TIMEOUT*HZ);
+	pr_info(MPT3SAS_FMT "%s: complete\n",
+			ioc->name, __func__);
+	if (!(ioc->base_cmds.status & MPT3_CMD_COMPLETE)) {
+		pr_err(MPT3SAS_FMT "%s: timeout\n",
+				ioc->name, __func__);
+		_debug_dump_mf(mpi_request,
+				sizeof(Mpi25FWUploadRequest_t)/4);
+		r = -ETIME;
+	} else {
+		memset(&mpi_reply, 0, sizeof(Mpi2FWUploadReply_t));
+		if (ioc->base_cmds.status & MPT3_CMD_REPLY_VALID) {
+			memcpy(&mpi_reply, ioc->base_cmds.reply,
+					sizeof(Mpi2FWUploadReply_t));
+			ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
+						MPI2_IOCSTATUS_MASK;
+			if (ioc_status == MPI2_IOCSTATUS_SUCCESS) {
+				FWImgHdr = (Mpi2FWImageHeader_t *)fwpkg_data;
+				FWImgHdr->PackageVersion.Word =
+				    le32_to_cpu(FWImgHdr->PackageVersion.Word);
+				if (FWImgHdr->PackageVersion.Word) {
+					pr_info(MPT3SAS_FMT "FW Package Version"
+					"(%02d.%02d.%02d.%02d)\n",
+					ioc->name,
+					((FWImgHdr->PackageVersion.Word)
+							& 0xFF000000) >> 24,
+					((FWImgHdr->PackageVersion.Word)
+							& 0x00FF0000) >> 16,
+					((FWImgHdr->PackageVersion.Word)
+							& 0x0000FF00) >> 8,
+					(FWImgHdr->PackageVersion.Word)
+							& 0x000000FF);
+				}
+			} else {
+				_debug_dump_mf(&mpi_reply,
+						sizeof(Mpi2FWUploadReply_t)/4);
+			}
+		}
+	}
+	ioc->base_cmds.status = MPT3_CMD_NOT_USED;
+out:
+	if (fwpkg_data)
+		pci_free_consistent(ioc->pdev, data_length, fwpkg_data,
+				fwpkg_data_dma);
+	return r;
+}
+
+/**
  * _base_display_ioc_capabilities - Disply IOC's capabilities.
  * @ioc: per adapter object
  *
@@ -6359,12 +6464,18 @@  _base_make_ioc_operational(struct MPT3SAS_ADAPTER *ioc)
  skip_init_reply_post_host_index:
 
 	_base_unmask_interrupts(ioc);
+
+	if (ioc->hba_mpi_version_belonged != MPI2_VERSION) {
+		r = _base_display_fwpkg_version(ioc);
+		if (r)
+			return r;
+	}
+
+	_base_static_config_pages(ioc);
 	r = _base_event_notification(ioc);
 	if (r)
 		return r;
 
-	_base_static_config_pages(ioc);
-
 	if (ioc->is_driver_loading) {
 
 		if (ioc->is_warpdrive && ioc->manu_pg10.OEMIdentifier
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 6f3329e..20fe90d 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -138,6 +138,7 @@ 
 #define MAX_CHAIN_ELEMT_SZ		16
 #define DEFAULT_NUM_FWCHAIN_ELEMTS	8
 
+#define FW_IMG_HDR_READ_TIMEOUT	15
 /*
  * NVMe defines
  */