Skip to contentSkip to conversation

pm80xx: Added SPCv/ve specific ids, variables and modify for SPC

PATCH Superseded Anand 3 participants 6 messages Apr 18, 2013 – May 6, 2013

Subsystems (2)
SCSI PMC sierra PM8001 driver

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Anand

author
#0 lore raw

PATCH v4 2/12 pm80xx: Added SPCv/ve specific ids, variables and modify for SPC

Quoted 1 line

From 6e057b9cd46d2f0f1838cf727bf8c0abc6a7b399 Mon Sep 17 00:00:00 2001

From: Sakthivel K <[email protected]>
Date: Wed, 17 Apr 2013 16:26:36 +0530
Subject: [PATCH V4 2/12] pm80xx: Added SPCv/ve specific ids, variables and modify for SPC

Updated pci id table with device, vendor, subdevice and subvendor ids for 8081, 8088, 8089 SAS/SATA controllers. Added SPCv/ve related macros. Updated macros, hba info structure and other structures for SPCv/ve. Update of structure and variable names for SPC hardware functionalities.

Signed-off-by Sakthivel K <[email protected]>
Signed-off-by Anand Kumar S <[email protected]>
Ack-by Jack Wang <[email protected]>
Ack-by Hannes Reinecke <[email protected]>
---
 drivers/scsi/pm8001/pm8001_ctl.c  |   69 ++++++++++---
 drivers/scsi/pm8001/pm8001_defs.h |   19 +++-
 drivers/scsi/pm8001/pm8001_hwi.c  |  213 ++++++++++++++++++++++---------------
 drivers/scsi/pm8001/pm8001_init.c |   48 +++++++-
 drivers/scsi/pm8001/pm8001_sas.h  |   92 ++++++++++++++--
 5 files changed, 320 insertions(+), 121 deletions(-)
drivers/scsi/pm8001/pm8001_ctl.c +54 -15
index 45bc197..ae2b124 100644
@@ -1,5 +1,5 @@
 /*
- * PMC-Sierra SPC 8001 SAS/SATA based host adapters driver
+ * PMC-Sierra 8001/8081/8088/8089 SAS/SATA based host adapters driver
  *
  * Copyright (c) 2008-2009 USI Co., Ltd.
  * All rights reserved.

@@ -58,8 +58,13 @@ static ssize_t pm8001_ctl_mpi_interface_rev_show(struct device *cdev,
 	struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
 	struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
 
-	return snprintf(buf, PAGE_SIZE, "%d\n",
-		pm8001_ha->main_cfg_tbl.interface_rev);
+	if (pm8001_ha->chip_id == chip_8001) {
+		return snprintf(buf, PAGE_SIZE, "%d\n",
+			pm8001_ha->main_cfg_tbl.pm8001_tbl.interface_rev);
+	} else {
+		return snprintf(buf, PAGE_SIZE, "%d\n",
+			pm8001_ha->main_cfg_tbl.pm80xx_tbl.interface_rev);
+	}
 }
 static
 DEVICE_ATTR(interface_rev, S_IRUGO, pm8001_ctl_mpi_interface_rev_show, NULL);

@@ -78,11 +83,19 @@ static ssize_t pm8001_ctl_fw_version_show(struct device *cdev,
 	struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
 	struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
 
-	return snprintf(buf, PAGE_SIZE, "%02x.%02x.%02x.%02x\n",
-		       (u8)(pm8001_ha->main_cfg_tbl.firmware_rev >> 24),
-		       (u8)(pm8001_ha->main_cfg_tbl.firmware_rev >> 16),
-		       (u8)(pm8001_ha->main_cfg_tbl.firmware_rev >> 8),
-		       (u8)(pm8001_ha->main_cfg_tbl.firmware_rev));
+	if (pm8001_ha->chip_id == chip_8001) {
+		return snprintf(buf, PAGE_SIZE, "%02x.%02x.%02x.%02x\n",
+		(u8)(pm8001_ha->main_cfg_tbl.pm8001_tbl.firmware_rev >> 24),
+		(u8)(pm8001_ha->main_cfg_tbl.pm8001_tbl.firmware_rev >> 16),
+		(u8)(pm8001_ha->main_cfg_tbl.pm8001_tbl.firmware_rev >> 8),
+		(u8)(pm8001_ha->main_cfg_tbl.pm8001_tbl.firmware_rev));
+	} else {
+		return snprintf(buf, PAGE_SIZE, "%02x.%02x.%02x.%02x\n",
+		(u8)(pm8001_ha->main_cfg_tbl.pm80xx_tbl.firmware_rev >> 24),
+		(u8)(pm8001_ha->main_cfg_tbl.pm80xx_tbl.firmware_rev >> 16),
+		(u8)(pm8001_ha->main_cfg_tbl.pm80xx_tbl.firmware_rev >> 8),
+		(u8)(pm8001_ha->main_cfg_tbl.pm80xx_tbl.firmware_rev));
+	}
 }
 static DEVICE_ATTR(fw_version, S_IRUGO, pm8001_ctl_fw_version_show, NULL);
 /**

@@ -99,8 +112,13 @@ static ssize_t pm8001_ctl_max_out_io_show(struct device *cdev,
 	struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
 	struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
 
-	return snprintf(buf, PAGE_SIZE, "%d\n",
-			pm8001_ha->main_cfg_tbl.max_out_io);
+	if (pm8001_ha->chip_id == chip_8001) {
+		return snprintf(buf, PAGE_SIZE, "%d\n",
+			pm8001_ha->main_cfg_tbl.pm8001_tbl.max_out_io);
+	} else {
+		return snprintf(buf, PAGE_SIZE, "%d\n",
+			pm8001_ha->main_cfg_tbl.pm80xx_tbl.max_out_io);
+	}
 }
 static DEVICE_ATTR(max_out_io, S_IRUGO, pm8001_ctl_max_out_io_show, NULL);
 /**

@@ -117,8 +135,15 @@ static ssize_t pm8001_ctl_max_devices_show(struct device *cdev,
 	struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
 	struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
 
-	return snprintf(buf, PAGE_SIZE, "%04d\n",
-			(u16)(pm8001_ha->main_cfg_tbl.max_sgl >> 16));
+	if (pm8001_ha->chip_id == chip_8001) {
+		return snprintf(buf, PAGE_SIZE, "%04d\n",
+			(u16)(pm8001_ha->main_cfg_tbl.pm8001_tbl.max_sgl >> 16)
+			);
+	} else {
+		return snprintf(buf, PAGE_SIZE, "%04d\n",
+			(u16)(pm8001_ha->main_cfg_tbl.pm80xx_tbl.max_sgl >> 16)
+			);
+	}
 }
 static DEVICE_ATTR(max_devices, S_IRUGO, pm8001_ctl_max_devices_show, NULL);
 /**

@@ -136,8 +161,15 @@ static ssize_t pm8001_ctl_max_sg_list_show(struct device *cdev,
 	struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
 	struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
 
-	return snprintf(buf, PAGE_SIZE, "%04d\n",
-			pm8001_ha->main_cfg_tbl.max_sgl & 0x0000FFFF);
+	if (pm8001_ha->chip_id == chip_8001) {
+		return snprintf(buf, PAGE_SIZE, "%04d\n",
+			pm8001_ha->main_cfg_tbl.pm8001_tbl.max_sgl & 0x0000FFFF
+			);
+	} else {
+		return snprintf(buf, PAGE_SIZE, "%04d\n",
+			pm8001_ha->main_cfg_tbl.pm80xx_tbl.max_sgl & 0x0000FFFF
+			);
+	}
 }
 static DEVICE_ATTR(max_sg_list, S_IRUGO, pm8001_ctl_max_sg_list_show, NULL);
 

@@ -173,7 +205,14 @@ static ssize_t pm8001_ctl_sas_spec_support_show(struct device *cdev,
 	struct Scsi_Host *shost = class_to_shost(cdev);
 	struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
 	struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
-	mode = (pm8001_ha->main_cfg_tbl.ctrl_cap_flag & 0xfe000000)>>25;
+	/* fe000000 means supports SAS2.1 */
+	if (pm8001_ha->chip_id == chip_8001)
+		mode = (pm8001_ha->main_cfg_tbl.pm8001_tbl.ctrl_cap_flag &
+							0xfe000000)>>25;
+	else
+		/* fe000000 means supports SAS2.1 */
+		mode = (pm8001_ha->main_cfg_tbl.pm80xx_tbl.ctrl_cap_flag &
+							0xfe000000)>>25;
 	return show_sas_spec_support_status(mode, buf);
 }
 static DEVICE_ATTR(sas_spec_support, S_IRUGO,
drivers/scsi/pm8001/pm8001_defs.h +15 -4
index c3d20c8..b25f87c 100644
@@ -1,5 +1,5 @@
 /*
- * PMC-Sierra SPC 8001 SAS/SATA based host adapters driver
+ * PMC-Sierra 8001/8081/8088/8089 SAS/SATA based host adapters driver
  *
  * Copyright (c) 2008-2009 USI Co., Ltd.
  * All rights reserved.

@@ -43,6 +43,10 @@
 
 enum chip_flavors {
 	chip_8001,
+	chip_8008,
+	chip_8009,
+	chip_8018,
+	chip_8019
 };
 #define USI_MAX_MEMCNT			9
 #define PM8001_MAX_DMA_SG		SG_ALL

@@ -69,12 +73,19 @@ enum port_type {
 #define PM8001_MPI_QUEUE         1024   /* maximum mpi queue entries */
 #define	PM8001_MAX_INB_NUM	 1
 #define	PM8001_MAX_OUTB_NUM	 1
+#define	PM8001_MAX_SPCV_INB_NUM		1
+#define	PM8001_MAX_SPCV_OUTB_NUM	4
 #define	PM8001_CAN_QUEUE	 508	/* SCSI Queue depth */
 
+/* Inbound/Outbound queue size */
+#define IOMB_SIZE_SPC		64
+#define IOMB_SIZE_SPCV		128
+
 /* unchangeable hardware details */
-#define	PM8001_MAX_PHYS		 8	/* max. possible phys */
-#define	PM8001_MAX_PORTS	 8	/* max. possible ports */
-#define	PM8001_MAX_DEVICES	 1024	/* max supported device */
+#define	PM8001_MAX_PHYS		 16	/* max. possible phys */
+#define	PM8001_MAX_PORTS	 16	/* max. possible ports */
+#define	PM8001_MAX_DEVICES	 2048	/* max supported device */
+#define	PM8001_MAX_MSIX_VEC	 64	/* max msi-x int for spcv/ve */
 
 enum memory_region_num {
 	AAP1 = 0x0, /* application acceleration processor */
drivers/scsi/pm8001/pm8001_hwi.c +128 -85

282 lines not on this page · raw message

drivers/scsi/pm8001/pm8001_init.c +42 -6

76 lines not on this page · raw message

drivers/scsi/pm8001/pm8001_sas.h +81 -11

138 lines not on this page · raw message

Signature · 1 line
1.7.1
maintainer
#1 lore raw
Quoted 10 lines · Anand
Quoted 1 line

From 6e057b9cd46d2f0f1838cf727bf8c0abc6a7b399 Mon Sep 17 00:00:00 2001

From: Sakthivel K <[email protected]>
Date: Wed, 17 Apr 2013 16:26:36 +0530
Subject: [PATCH V4 2/12] pm80xx: Added SPCv/ve specific ids, variables and modify for SPC

Updated pci id table with device, vendor, subdevice and subvendor ids for 8081, 8088, 8089 SAS/SATA controllers. Added SPCv/ve related macros. Updated macros, hba info structure and other structures for SPCv/ve. Update of structure and variable names for SPC hardware functionalities.

Quoted 4 lines
Signed-off-by Sakthivel K <[email protected]>
Signed-off-by Anand Kumar S <[email protected]>
Ack-by Jack Wang <[email protected]>
Ack-by Hannes Reinecke <[email protected]>

I think this should be Acked-by: and Reviewed-by: respectively

[...]

Quoted 112 lines
drivers/scsi/pm8001/pm8001_hwi.c +46 -40
index b8dd050..dc45fdc 100644
@@ -50,32 +50,39 @@
 static void read_main_config_table(struct pm8001_hba_info *pm8001_ha)
 {
 	void __iomem *address = pm8001_ha->main_cfg_tbl_addr;
-	pm8001_ha->main_cfg_tbl.signature	= pm8001_mr32(address, 0x00);
-	pm8001_ha->main_cfg_tbl.interface_rev	= pm8001_mr32(address, 0x04);
-	pm8001_ha->main_cfg_tbl.firmware_rev	= pm8001_mr32(address, 0x08);
-	pm8001_ha->main_cfg_tbl.max_out_io	= pm8001_mr32(address, 0x0C);
-	pm8001_ha->main_cfg_tbl.max_sgl		= pm8001_mr32(address, 0x10);
-	pm8001_ha->main_cfg_tbl.ctrl_cap_flag	= pm8001_mr32(address, 0x14);
-	pm8001_ha->main_cfg_tbl.gst_offset	= pm8001_mr32(address, 0x18);
-	pm8001_ha->main_cfg_tbl.inbound_queue_offset =
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.signature	=
+				pm8001_mr32(address, 0x00);
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.interface_rev =
+				pm8001_mr32(address, 0x04);
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.firmware_rev	=
+				pm8001_mr32(address, 0x08);
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.max_out_io	=
+				pm8001_mr32(address, 0x0C);
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.max_sgl	=
+				pm8001_mr32(address, 0x10);
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.ctrl_cap_flag =
+				pm8001_mr32(address, 0x14);
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.gst_offset	=
+				pm8001_mr32(address, 0x18);
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.inbound_queue_offset =
 		pm8001_mr32(address, MAIN_IBQ_OFFSET);
-	pm8001_ha->main_cfg_tbl.outbound_queue_offset =
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.outbound_queue_offset =
 		pm8001_mr32(address, MAIN_OBQ_OFFSET);
-	pm8001_ha->main_cfg_tbl.hda_mode_flag	=
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.hda_mode_flag	=
 		pm8001_mr32(address, MAIN_HDA_FLAGS_OFFSET);
 
 	/* read analog Setting offset from the configuration table */
-	pm8001_ha->main_cfg_tbl.anolog_setup_table_offset =
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.anolog_setup_table_offset =
 		pm8001_mr32(address, MAIN_ANALOG_SETUP_OFFSET);
 
 	/* read Error Dump Offset and Length */
-	pm8001_ha->main_cfg_tbl.fatal_err_dump_offset0 =
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_dump_offset0 =
 		pm8001_mr32(address, MAIN_FATAL_ERROR_RDUMP0_OFFSET);
-	pm8001_ha->main_cfg_tbl.fatal_err_dump_length0 =
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_dump_length0 =
 		pm8001_mr32(address, MAIN_FATAL_ERROR_RDUMP0_LENGTH);
-	pm8001_ha->main_cfg_tbl.fatal_err_dump_offset1 =
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_dump_offset1 =
 		pm8001_mr32(address, MAIN_FATAL_ERROR_RDUMP1_OFFSET);
-	pm8001_ha->main_cfg_tbl.fatal_err_dump_length1 =
+	pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_dump_length1 =
 		pm8001_mr32(address, MAIN_FATAL_ERROR_RDUMP1_LENGTH);
 }
 

@@ -86,31 +93,56 @@ static void read_main_config_table(struct pm8001_hba_info *pm8001_ha)
 static void read_general_status_table(struct pm8001_hba_info *pm8001_ha)
 {
 	void __iomem *address = pm8001_ha->general_stat_tbl_addr;
-	pm8001_ha->gs_tbl.gst_len_mpistate	= pm8001_mr32(address, 0x00);
-	pm8001_ha->gs_tbl.iq_freeze_state0	= pm8001_mr32(address, 0x04);
-	pm8001_ha->gs_tbl.iq_freeze_state1	= pm8001_mr32(address, 0x08);
-	pm8001_ha->gs_tbl.msgu_tcnt		= pm8001_mr32(address, 0x0C);
-	pm8001_ha->gs_tbl.iop_tcnt		= pm8001_mr32(address, 0x10);
-	pm8001_ha->gs_tbl.reserved		= pm8001_mr32(address, 0x14);
-	pm8001_ha->gs_tbl.phy_state[0]	= pm8001_mr32(address, 0x18);
-	pm8001_ha->gs_tbl.phy_state[1]	= pm8001_mr32(address, 0x1C);
-	pm8001_ha->gs_tbl.phy_state[2]	= pm8001_mr32(address, 0x20);
-	pm8001_ha->gs_tbl.phy_state[3]	= pm8001_mr32(address, 0x24);
-	pm8001_ha->gs_tbl.phy_state[4]	= pm8001_mr32(address, 0x28);
-	pm8001_ha->gs_tbl.phy_state[5]	= pm8001_mr32(address, 0x2C);
-	pm8001_ha->gs_tbl.phy_state[6]	= pm8001_mr32(address, 0x30);
-	pm8001_ha->gs_tbl.phy_state[7]	= pm8001_mr32(address, 0x34);
-	pm8001_ha->gs_tbl.reserved1		= pm8001_mr32(address, 0x38);
-	pm8001_ha->gs_tbl.reserved2		= pm8001_mr32(address, 0x3C);
-	pm8001_ha->gs_tbl.reserved3		= pm8001_mr32(address, 0x40);
-	pm8001_ha->gs_tbl.recover_err_info[0]	= pm8001_mr32(address, 0x44);
-	pm8001_ha->gs_tbl.recover_err_info[1]	= pm8001_mr32(address, 0x48);
-	pm8001_ha->gs_tbl.recover_err_info[2]	= pm8001_mr32(address, 0x4C);
-	pm8001_ha->gs_tbl.recover_err_info[3]	= pm8001_mr32(address, 0x50);
-	pm8001_ha->gs_tbl.recover_err_info[4]	= pm8001_mr32(address, 0x54);
-	pm8001_ha->gs_tbl.recover_err_info[5]	= pm8001_mr32(address, 0x58);
-	pm8001_ha->gs_tbl.recover_err_info[6]	= pm8001_mr32(address, 0x5C);
-	pm8001_ha->gs_tbl.recover_err_info[7]	= pm8001_mr32(address, 0x60);
+	pm8001_ha->gs_tbl.pm8001_tbl.gst_len_mpistate	=
+				pm8001_mr32(address, 0x00);
+	pm8001_ha->gs_tbl.pm8001_tbl.iq_freeze_state0	=
+				pm8001_mr32(address, 0x04);
+	pm8001_ha->gs_tbl.pm8001_tbl.iq_freeze_state1	=
+				pm8001_mr32(address, 0x08);
+	pm8001_ha->gs_tbl.pm8001_tbl.msgu_tcnt		=
+				pm8001_mr32(address, 0x0C);
+	pm8001_ha->gs_tbl.pm8001_tbl.iop_tcnt		=
+				pm8001_mr32(address, 0x10);
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd		=
+				pm8001_mr32(address, 0x14);
+	pm8001_ha->gs_tbl.pm8001_tbl.phy_state[0]	=
+				pm8001_mr32(address, 0x18);
+	pm8001_ha->gs_tbl.pm8001_tbl.phy_state[1]	=
+				pm8001_mr32(address, 0x1C);
+	pm8001_ha->gs_tbl.pm8001_tbl.phy_state[2]	=
+				pm8001_mr32(address, 0x20);
+	pm8001_ha->gs_tbl.pm8001_tbl.phy_state[3]	=
+				pm8001_mr32(address, 0x24);
+	pm8001_ha->gs_tbl.pm8001_tbl.phy_state[4]	=
+				pm8001_mr32(address, 0x28);
+	pm8001_ha->gs_tbl.pm8001_tbl.phy_state[5]	=
+				pm8001_mr32(address, 0x2C);

drivers/scsi/pm8001/pm8001_hwi.c static void read_main_config_table(struct pm8001_hba_info *pm8001_ha) · line 129

Quoted 10 lines
drivers/scsi/pm8001/pm8001_hwi.c +10 -0
+	pm8001_ha->gs_tbl.pm8001_tbl.phy_state[6]	=
+				pm8001_mr32(address, 0x30);
+	pm8001_ha->gs_tbl.pm8001_tbl.phy_state[7]	=
+				pm8001_mr32(address, 0x34);
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[0]		=
+				pm8001_mr32(address, 0x38);
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[1]		=
+				pm8001_mr32(address, 0x3C);
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[2]		=
+				pm8001_mr32(address, 0x40);

The compiler doesn't like this bit:

drivers/scsi/pm8001/pm8001_hwi.c: In function ‘pm8001_chip_init’:
drivers/scsi/pm8001/pm8001_hwi.c:128:36: warning: array subscript is above array bounds [-Warray-bounds]

And it's clearly right: the resvd1[] array only has two elements in pm8001_sas.h, so Either the array has to grow or something else needs fixing in here.

James

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw

James,

Can you pls let us know the warning flags you are including in your makefile such that we can check from our end. Obviously it looks like we did not use -Warray-bounds in this case.

Thanks.

Regards
Anand

________________________________

From: James Bottomley [mailto:[email protected]]
Sent: Thu 5/2/2013 3:12 PM
To: Anand Kumar Santhanam
Cc: [email protected]; Harry Yang; Vishwanath Maram; [email protected]; Rich Bono; Sangeetha Gnanasekaran
Subject: Re: [PATCH V4 2/12] pm80xx: Added SPCv/ve specific ids, variables and modify for SPC

Quoted 10 lines · Anand
Quoted 1 line

From 6e057b9cd46d2f0f1838cf727bf8c0abc6a7b399 Mon Sep 17 00:00:00 2001

From: Sakthivel K <[email protected]>
Date: Wed, 17 Apr 2013 16:26:36 +0530
Subject: [PATCH V4 2/12] pm80xx: Added SPCv/ve specific ids, variables and modify for SPC

Updated pci id table with device, vendor, subdevice and subvendor ids for 8081, 8088, 8089 SAS/SATA controllers. Added SPCv/ve related macros. Updated macros, hba info structure and other structures for SPCv/ve. Update of structure and variable names for SPC hardware functionalities.

Quoted 4 lines
Signed-off-by Sakthivel K <[email protected]>
Signed-off-by Anand Kumar S <[email protected]>
Ack-by Jack Wang <[email protected]>
Ack-by Hannes Reinecke <[email protected]>

I think this should be Acked-by: and Reviewed-by: respectively

[...]

Quoted 112 lines
drivers/scsi/pm8001/pm8001_hwi.c +46 -40
index b8dd050..dc45fdc 100644
@@ -50,32 +50,39 @@
 static void read_main_config_table(struct pm8001_hba_info *pm8001_ha)
 {
      void __iomem *address = pm8001_ha->main_cfg_tbl_addr;
-     pm8001_ha->main_cfg_tbl.signature       = pm8001_mr32(address, 0x00);
-     pm8001_ha->main_cfg_tbl.interface_rev   = pm8001_mr32(address, 0x04);
-     pm8001_ha->main_cfg_tbl.firmware_rev    = pm8001_mr32(address, 0x08);
-     pm8001_ha->main_cfg_tbl.max_out_io      = pm8001_mr32(address, 0x0C);
-     pm8001_ha->main_cfg_tbl.max_sgl         = pm8001_mr32(address, 0x10);
-     pm8001_ha->main_cfg_tbl.ctrl_cap_flag   = pm8001_mr32(address, 0x14);
-     pm8001_ha->main_cfg_tbl.gst_offset      = pm8001_mr32(address, 0x18);
-     pm8001_ha->main_cfg_tbl.inbound_queue_offset =
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.signature    =
+                             pm8001_mr32(address, 0x00);
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.interface_rev =
+                             pm8001_mr32(address, 0x04);
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.firmware_rev =
+                             pm8001_mr32(address, 0x08);
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.max_out_io   =
+                             pm8001_mr32(address, 0x0C);
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.max_sgl      =
+                             pm8001_mr32(address, 0x10);
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.ctrl_cap_flag =
+                             pm8001_mr32(address, 0x14);
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.gst_offset   =
+                             pm8001_mr32(address, 0x18);
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.inbound_queue_offset =
              pm8001_mr32(address, MAIN_IBQ_OFFSET);
-     pm8001_ha->main_cfg_tbl.outbound_queue_offset =
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.outbound_queue_offset =
              pm8001_mr32(address, MAIN_OBQ_OFFSET);
-     pm8001_ha->main_cfg_tbl.hda_mode_flag   =
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.hda_mode_flag        =
              pm8001_mr32(address, MAIN_HDA_FLAGS_OFFSET);
 
      /* read analog Setting offset from the configuration table */
-     pm8001_ha->main_cfg_tbl.anolog_setup_table_offset =
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.anolog_setup_table_offset =
              pm8001_mr32(address, MAIN_ANALOG_SETUP_OFFSET);
 
      /* read Error Dump Offset and Length */
-     pm8001_ha->main_cfg_tbl.fatal_err_dump_offset0 =
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_dump_offset0 =
              pm8001_mr32(address, MAIN_FATAL_ERROR_RDUMP0_OFFSET);
-     pm8001_ha->main_cfg_tbl.fatal_err_dump_length0 =
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_dump_length0 =
              pm8001_mr32(address, MAIN_FATAL_ERROR_RDUMP0_LENGTH);
-     pm8001_ha->main_cfg_tbl.fatal_err_dump_offset1 =
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_dump_offset1 =
              pm8001_mr32(address, MAIN_FATAL_ERROR_RDUMP1_OFFSET);
-     pm8001_ha->main_cfg_tbl.fatal_err_dump_length1 =
+     pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_dump_length1 =
              pm8001_mr32(address, MAIN_FATAL_ERROR_RDUMP1_LENGTH);
 }
 

@@ -86,31 +93,56 @@ static void read_main_config_table(struct pm8001_hba_info *pm8001_ha)
 static void read_general_status_table(struct pm8001_hba_info *pm8001_ha)
 {
      void __iomem *address = pm8001_ha->general_stat_tbl_addr;
-     pm8001_ha->gs_tbl.gst_len_mpistate      = pm8001_mr32(address, 0x00);
-     pm8001_ha->gs_tbl.iq_freeze_state0      = pm8001_mr32(address, 0x04);
-     pm8001_ha->gs_tbl.iq_freeze_state1      = pm8001_mr32(address, 0x08);
-     pm8001_ha->gs_tbl.msgu_tcnt             = pm8001_mr32(address, 0x0C);
-     pm8001_ha->gs_tbl.iop_tcnt              = pm8001_mr32(address, 0x10);
-     pm8001_ha->gs_tbl.reserved              = pm8001_mr32(address, 0x14);
-     pm8001_ha->gs_tbl.phy_state[0]  = pm8001_mr32(address, 0x18);
-     pm8001_ha->gs_tbl.phy_state[1]  = pm8001_mr32(address, 0x1C);
-     pm8001_ha->gs_tbl.phy_state[2]  = pm8001_mr32(address, 0x20);
-     pm8001_ha->gs_tbl.phy_state[3]  = pm8001_mr32(address, 0x24);
-     pm8001_ha->gs_tbl.phy_state[4]  = pm8001_mr32(address, 0x28);
-     pm8001_ha->gs_tbl.phy_state[5]  = pm8001_mr32(address, 0x2C);
-     pm8001_ha->gs_tbl.phy_state[6]  = pm8001_mr32(address, 0x30);
-     pm8001_ha->gs_tbl.phy_state[7]  = pm8001_mr32(address, 0x34);
-     pm8001_ha->gs_tbl.reserved1             = pm8001_mr32(address, 0x38);
-     pm8001_ha->gs_tbl.reserved2             = pm8001_mr32(address, 0x3C);
-     pm8001_ha->gs_tbl.reserved3             = pm8001_mr32(address, 0x40);
-     pm8001_ha->gs_tbl.recover_err_info[0]   = pm8001_mr32(address, 0x44);
-     pm8001_ha->gs_tbl.recover_err_info[1]   = pm8001_mr32(address, 0x48);
-     pm8001_ha->gs_tbl.recover_err_info[2]   = pm8001_mr32(address, 0x4C);
-     pm8001_ha->gs_tbl.recover_err_info[3]   = pm8001_mr32(address, 0x50);
-     pm8001_ha->gs_tbl.recover_err_info[4]   = pm8001_mr32(address, 0x54);
-     pm8001_ha->gs_tbl.recover_err_info[5]   = pm8001_mr32(address, 0x58);
-     pm8001_ha->gs_tbl.recover_err_info[6]   = pm8001_mr32(address, 0x5C);
-     pm8001_ha->gs_tbl.recover_err_info[7]   = pm8001_mr32(address, 0x60);
+     pm8001_ha->gs_tbl.pm8001_tbl.gst_len_mpistate   =
+                             pm8001_mr32(address, 0x00);
+     pm8001_ha->gs_tbl.pm8001_tbl.iq_freeze_state0   =
+                             pm8001_mr32(address, 0x04);
+     pm8001_ha->gs_tbl.pm8001_tbl.iq_freeze_state1   =
+                             pm8001_mr32(address, 0x08);
+     pm8001_ha->gs_tbl.pm8001_tbl.msgu_tcnt          =
+                             pm8001_mr32(address, 0x0C);
+     pm8001_ha->gs_tbl.pm8001_tbl.iop_tcnt           =
+                             pm8001_mr32(address, 0x10);
+     pm8001_ha->gs_tbl.pm8001_tbl.rsvd               =
+                             pm8001_mr32(address, 0x14);
+     pm8001_ha->gs_tbl.pm8001_tbl.phy_state[0]       =
+                             pm8001_mr32(address, 0x18);
+     pm8001_ha->gs_tbl.pm8001_tbl.phy_state[1]       =
+                             pm8001_mr32(address, 0x1C);
+     pm8001_ha->gs_tbl.pm8001_tbl.phy_state[2]       =
+                             pm8001_mr32(address, 0x20);
+     pm8001_ha->gs_tbl.pm8001_tbl.phy_state[3]       =
+                             pm8001_mr32(address, 0x24);
+     pm8001_ha->gs_tbl.pm8001_tbl.phy_state[4]       =
+                             pm8001_mr32(address, 0x28);
+     pm8001_ha->gs_tbl.pm8001_tbl.phy_state[5]       =
+                             pm8001_mr32(address, 0x2C);

drivers/scsi/pm8001/pm8001_hwi.c static void read_main_config_table(struct pm8001_hba_info *pm8001_ha) · line 129

Quoted 10 lines
drivers/scsi/pm8001/pm8001_hwi.c +10 -0
+     pm8001_ha->gs_tbl.pm8001_tbl.phy_state[6]       =
+                             pm8001_mr32(address, 0x30);
+     pm8001_ha->gs_tbl.pm8001_tbl.phy_state[7]       =
+                             pm8001_mr32(address, 0x34);
+     pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[0]           =
+                             pm8001_mr32(address, 0x38);
+     pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[1]           =
+                             pm8001_mr32(address, 0x3C);
+     pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[2]           =
+                             pm8001_mr32(address, 0x40);

The compiler doesn't like this bit:

drivers/scsi/pm8001/pm8001_hwi.c: In function 'pm8001_chip_init':
drivers/scsi/pm8001/pm8001_hwi.c:128:36: warning: array subscript is above array bounds [-Warray-bounds]

And it's clearly right: the resvd1[] array only has two elements in pm8001_sas.h, so Either the array has to grow or something else needs fixing in here.

James

maintainer
#3 lore raw
Quoted 5 lines · Anand Kumar Santhanam

James,

Can you pls let us know the warning flags you are including in your makefile such that we can check from our end.
Obviously it looks like we did not use -Warray-bounds in this case.

Oh good grief, please stop wasting my time. You have the definition here in your patch:

Review

Quoted 1 line
(quoted diff) +1 -0
+	u32			rsvd1[2];

And then you use it as:

drivers/scsi/pm8001/pm8001_hwi.c static void read_main_config_table(struct pm8001_hba_info *pm8001_ha) · line 129

Quoted 6 lines
drivers/scsi/pm8001/pm8001_hwi.c +6 -0
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[0]		=
+				pm8001_mr32(address, 0x38);
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[1]		=
+				pm8001_mr32(address, 0x3C);
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[2]		=
+				pm8001_mr32(address, 0x40);

That last one goes over the array bounds because an array declared as rsvdl1[2] only has two elements: 0 and 1 ... this isn't rocket science or esoteric compiler flags, this is basic C.

Stop quibbling and fix it, please.

James

#4 lore raw

Hi James,

I have resubmitted the patchset after fixing the array out of bounds issue. Pls pull the same.

I guess I did not make myself clear with my earlier mail. I compiled my earlier patchset using the
Kernel Makefile and I did not get the array out of bounds warning even I had -Wall -O2 flags enabled.
So I was just wondering whether you are using any special settings such that you are able to see the
warning.

Regards
Anand

-----Original Message-----
From: James Bottomley [mailto:[email protected]]
Sent: Friday, May 03, 2013 11:29 AM
To: Anand Kumar Santhanam
Cc: [email protected]; Harry Yang; Vishwanath Maram; [email protected]; Rich Bono; Sangeetha Gnanasekaran
Subject: Re: [PATCH V4 2/12] pm80xx: Added SPCv/ve specific ids, variables and modify for SPC

Quoted 5 lines · Anand Kumar Santhanam

James,

Can you pls let us know the warning flags you are including in your makefile such that we can check from our end.
Obviously it looks like we did not use -Warray-bounds in this case.

Oh good grief, please stop wasting my time. You have the definition here in your patch:

Review

Quoted 1 line
(quoted diff) +1 -0
+	u32			rsvd1[2];

And then you use it as:

drivers/scsi/pm8001/pm8001_hwi.c static void read_main_config_table(struct pm8001_hba_info *pm8001_ha) · line 129

Quoted 6 lines
drivers/scsi/pm8001/pm8001_hwi.c +6 -0
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[0]		=
+				pm8001_mr32(address, 0x38);
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[1]		=
+				pm8001_mr32(address, 0x3C);
+	pm8001_ha->gs_tbl.pm8001_tbl.rsvd1[2]		=
+				pm8001_mr32(address, 0x40);

That last one goes over the array bounds because an array declared as rsvdl1[2] only has two elements: 0 and 1 ... this isn't rocket science or esoteric compiler flags, this is basic C.

Stop quibbling and fix it, please.

James

maintainer
#5 lore raw
Quoted 5 lines · Anand Kumar Santhanam

Hi James,

I have resubmitted the patchset after fixing the array out of bounds issue. Pls pull the same.

Quoted 7 lines

I guess I did not make myself clear with my earlier mail. I compiled my earlier patchset using the
Kernel Makefile and I did not get the array out of bounds warning even I had -Wall -O2 flags enabled.
So I was just wondering whether you are using any special settings such that you are able to see the
warning.

Oh, OK, that's not what I read in the email, so sorry for being brusque. What -Wall actually does is highly gcc version dependent. What I did was simply a standard kernel compile (no extra flags), but with gcc-4.7.2

James