[PATCH V2 0/3] pm80xx : Updates for the driver version 0.1.39.

[PATCH V2 0/3] pm80xx : Updates for the driver version 0.1.39.

[Deepak Ukey]

From: Deepak Ukey <Deepak.Ukey@microchip.com>

This patch set includes some bug fixes and features for pm80xx driver.

Changes from V1:
	For "Support for get phy profile functionality."
		-Initalized the completion and removed the phy_prof_resp
		structure.
	For "Staggered spin up support."
		-Removed the lock_flags from pm8001_hba_info structure.
	For "Wait for PHY startup before draining libsas queue."
		-Moved the completion initalization in first patch.
		-Added module parameter to wait for phy startup.
		
Viswas G (2):
  pm80xx : Support for get phy profile functionality.
  pm80xx : Staggered spin up support.

peter chang (1):
  pm80xx : Wait for PHY startup before draining libsas queue.

 drivers/scsi/pm8001/pm8001_ctl.c  |  36 ++++++
 drivers/scsi/pm8001/pm8001_ctl.h  |  20 +++
 drivers/scsi/pm8001/pm8001_defs.h |   9 +-
 drivers/scsi/pm8001/pm8001_hwi.c  |  14 ++-
 drivers/scsi/pm8001/pm8001_init.c |  77 +++++++++++-
 drivers/scsi/pm8001/pm8001_sas.c  | 103 +++++++++++++++-
 drivers/scsi/pm8001/pm8001_sas.h  |  22 ++++
 drivers/scsi/pm8001/pm80xx_hwi.c  | 253 +++++++++++++++++++++++++++++++++++---
 drivers/scsi/pm8001/pm80xx_hwi.h  |   2 +
 9 files changed, 505 insertions(+), 31 deletions(-)

-- 
2.16.3

[PATCH V2 1/3] pm80xx : Support for get phy profile functionality.

[Deepak Ukey]

From: Viswas G <Viswas.G@microchip.com>

Added the support to get the phy profile which gives information
about the phy states, port and errors on phy.

Signed-off-by: Deepak Ukey <deepak.ukey@microchip.com>
Signed-off-by: Viswas G <Viswas.G@microchip.com>
Signed-off-by: Radha Ramachandran <radha@google.com>
---
 drivers/scsi/pm8001/pm8001_ctl.h  | 20 ++++++++++
 drivers/scsi/pm8001/pm8001_init.c |  2 +
 drivers/scsi/pm8001/pm8001_sas.h  |  5 +++
 drivers/scsi/pm8001/pm80xx_hwi.c  | 83 ++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/pm8001/pm80xx_hwi.h  |  2 +
 5 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_ctl.h b/drivers/scsi/pm8001/pm8001_ctl.h
index d0d43a250b9e..02dc04a1fbe8 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.h
+++ b/drivers/scsi/pm8001/pm8001_ctl.h
@@ -41,6 +41,26 @@
 #ifndef PM8001_CTL_H_INCLUDED
 #define PM8001_CTL_H_INCLUDED
 
+struct phy_status {
+	char		phy_id;
+	unsigned int	phy_state:4;
+	unsigned int	nlr:4;
+	unsigned int	plr:4;
+	unsigned int	reserved1:12;
+	unsigned char	port_id;
+	unsigned int	prts:4;
+	unsigned int	reserved2:20;
+} __packed;
+
+struct phy_errcnt {
+	unsigned int	InvalidDword;
+	unsigned int	runningDisparityError;
+	unsigned int	codeViolation;
+	unsigned int	LossOfSyncDW;
+	unsigned int	phyResetProblem;
+	unsigned int	inboundCRCError;
+};
+
 #define IOCTL_BUF_SIZE		4096
 #define HEADER_LEN			28
 #define SIZE_OFFSET			16
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index a8f5344fdfda..38a65e1da823 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -397,6 +397,8 @@ static int pm8001_alloc(struct pm8001_hba_info *pm8001_ha,
 		pm8001_ha->ccb_info[i].task = NULL;
 		pm8001_ha->ccb_info[i].ccb_tag = 0xffffffff;
 		pm8001_ha->ccb_info[i].device = NULL;
+		pm8001_ha->ccb_info[i].completion = NULL;
+		pm8001_ha->ccb_info[i].resp_buf = NULL;
 		++pm8001_ha->tags_num;
 	}
 	pm8001_ha->flags = PM8001F_INIT_TIME;
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index ae7ba9b3c4bc..488af79dec47 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -56,6 +56,7 @@
 #include <scsi/sas_ata.h>
 #include <linux/atomic.h>
 #include "pm8001_defs.h"
+#include "pm8001_ctl.h"
 
 #define DRV_NAME		"pm80xx"
 #define DRV_VERSION		"0.1.39"
@@ -244,6 +245,8 @@ struct pm8001_dispatch {
 	int (*sas_diag_execute_req)(struct pm8001_hba_info *pm8001_ha,
 		u32 state);
 	int (*sas_re_init_req)(struct pm8001_hba_info *pm8001_ha);
+	int (*get_phy_profile_req)(struct pm8001_hba_info *pm8001_ha,
+		int phy, int page, struct completion *comp, void *buf);
 };
 
 struct pm8001_chip_info {
@@ -318,6 +321,8 @@ struct pm8001_ccb_info {
 	struct pm8001_prd	buf_prd[PM8001_MAX_DMA_SG];
 	struct fw_control_ex	*fw_control_context;
 	u8			open_retry;
+	struct completion	*completion;
+	void			*resp_buf;
 };
 
 struct mpi_mem {
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 4d205ebaee87..a909d00c6897 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -3792,7 +3792,6 @@ static int mpi_set_controller_config_resp(struct pm8001_hba_info *pm8001_ha,
 	PM8001_MSG_DBG(pm8001_ha, pm8001_printk(
 			"SET CONTROLLER RESP: status 0x%x qlfr_pgcd 0x%x\n",
 			status, err_qlfr_pgcd));
-
 	return 0;
 }
 
@@ -3818,9 +3817,58 @@ static int mpi_get_controller_config_resp(struct pm8001_hba_info *pm8001_ha,
 static int mpi_get_phy_profile_resp(struct pm8001_hba_info *pm8001_ha,
 			void *piomb)
 {
+	u32 tag, page_code;
+	struct phy_status *phy_status, *phy_stat;
+	struct phy_errcnt *phy_err, *phy_err_cnt;
+	struct pm8001_ccb_info *ccb;
+	struct get_phy_profile_resp *pPayload =
+		(struct get_phy_profile_resp *)(piomb + 4);
+	u32 status = le32_to_cpu(pPayload->status);
+
+	page_code = (u8)((pPayload->ppc_phyid & 0xFF00) >> 8);
+
 	PM8001_MSG_DBG(pm8001_ha,
-			pm8001_printk(" pm80xx_addition_functionality\n"));
+		pm8001_printk(" pm80xx_addition_functionality\n"));
+	if (status) {
+		/* status is FAILED */
+		PM8001_FAIL_DBG(pm8001_ha, pm8001_printk(
+			"mpiGetPhyProfileReq failed  with status 0x%08x\n",
+			status));
+	}
 
+	tag = le32_to_cpu(pPayload->tag);
+	ccb = &pm8001_ha->ccb_info[tag];
+	if (ccb->completion != NULL) {
+		if (status) {
+			/* signal fail status */
+			memset(&ccb->resp_buf, 0xff, sizeof(ccb->resp_buf));
+		} else if (page_code == SAS_PHY_GENERAL_STATUS_PAGE) {
+			phy_status = (struct phy_status *)ccb->resp_buf;
+			phy_stat =
+			(struct phy_status *)pPayload->ppc_specific_rsp;
+			phy_status->phy_id = le32_to_cpu(phy_stat->phy_id);
+			phy_status->phy_state =
+					le32_to_cpu(phy_stat->phy_state);
+			phy_status->plr = le32_to_cpu(phy_stat->plr);
+			phy_status->nlr = le32_to_cpu(phy_stat->nlr);
+			phy_status->port_id = le32_to_cpu(phy_stat->port_id);
+			phy_status->prts = le32_to_cpu(phy_stat->prts);
+		} else if (page_code == SAS_PHY_ERR_COUNTERS_PAGE) {
+			phy_err = (struct phy_errcnt *)ccb->resp_buf;
+			phy_err_cnt =
+			(struct phy_errcnt *)pPayload->ppc_specific_rsp;
+			phy_err->InvalidDword =
+			le32_to_cpu(phy_err_cnt->InvalidDword);
+			phy_err->runningDisparityError =
+			le32_to_cpu(phy_err_cnt->runningDisparityError);
+			phy_err->LossOfSyncDW =
+			le32_to_cpu(phy_err_cnt->LossOfSyncDW);
+			phy_err->phyResetProblem =
+			le32_to_cpu(phy_err_cnt->phyResetProblem);
+		}
+		complete(ccb->completion);
+	}
+	pm8001_tag_free(pm8001_ha, tag);
 	return 0;
 }
 
@@ -5013,6 +5061,36 @@ pm80xx_chip_isr(struct pm8001_hba_info *pm8001_ha, u8 vec)
 	return IRQ_HANDLED;
 }
 
+int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
+		int page_code, struct completion *comp, void *buf)
+{
+	u32 tag;
+	struct get_phy_profile_req payload;
+	struct inbound_queue_table *circularQ;
+	struct pm8001_ccb_info *ccb;
+	int rc, ppc_phyid;
+	u32 opc = OPC_INB_GET_PHY_PROFILE;
+
+	memset(&payload, 0, sizeof(payload));
+
+	rc = pm8001_tag_alloc(pm8001_ha, &tag);
+	if (rc)
+		PM8001_FAIL_DBG(pm8001_ha, pm8001_printk("Invalid tag\n"));
+
+	ccb = &pm8001_ha->ccb_info[tag];
+	ccb->completion = comp;
+	ccb->resp_buf = buf;
+	circularQ = &pm8001_ha->inbnd_q_tbl[0];
+
+	payload.tag = cpu_to_le32(tag);
+	ppc_phyid = (page_code & 0xFF)  << 8 | (phy_id & 0xFF);
+	payload.ppc_phyid = cpu_to_le32(ppc_phyid);
+
+	pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &payload,
+			sizeof(payload), 0);
+	return rc;
+}
+
 void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
 	u32 operation, u32 phyid, u32 length, u32 *buf)
 {
@@ -5113,4 +5191,5 @@ const struct pm8001_dispatch pm8001_80xx_dispatch = {
 	.set_nvmd_req		= pm8001_chip_set_nvmd_req,
 	.fw_flash_update_req	= pm8001_chip_fw_flash_update_req,
 	.set_dev_state_req	= pm8001_chip_set_dev_state_req,
+	.get_phy_profile_req	= pm8001_chip_get_phy_profile,
 };
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.h b/drivers/scsi/pm8001/pm80xx_hwi.h
index 701951a0f715..b5119c5479da 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.h
+++ b/drivers/scsi/pm8001/pm80xx_hwi.h
@@ -175,7 +175,9 @@
 #define PHY_STOP_ERR_DEVICE_ATTACHED	0x1046
 
 /* phy_profile */
+#define SAS_PHY_ERR_COUNTERS_PAGE	0x01
 #define SAS_PHY_ANALOG_SETTINGS_PAGE	0x04
+#define SAS_PHY_GENERAL_STATUS_PAGE	0x05
 #define PHY_DWORD_LENGTH		0xC
 
 /* Thermal related */
-- 
2.16.3

[PATCH V2 2/3] pm80xx : Staggered spin up support.

[Deepak Ukey]

From: Viswas G <Viswas.G@microchip.com>

As a part of drive discovery, driver will initaite the drive spin up.
If all drives do spin up together, it will result in large power
consumption. To reduce the power consumption, driver provide an option
to make a small group of drives (say 3 or 4 drives together) to do the
spin up. The delay between two spin up group and no of drives to
spin up (group) can be programmed by the customer in seeprom and
driver will use it to control the spinup.

Signed-off-by: Viswas G <Viswas.G@microchip.com>
Signed-off-by: Radha Ramachandran <radha@google.com>
Signed-off-by: Deepak Ukey <Deepak.Ukey@microchip.com>
---
 drivers/scsi/pm8001/pm8001_defs.h |   3 +
 drivers/scsi/pm8001/pm8001_hwi.c  |  14 ++++-
 drivers/scsi/pm8001/pm8001_init.c |  53 +++++++++++++++-
 drivers/scsi/pm8001/pm8001_sas.c  |  36 ++++++++++-
 drivers/scsi/pm8001/pm8001_sas.h  |  13 ++++
 drivers/scsi/pm8001/pm80xx_hwi.c  | 125 +++++++++++++++++++++++++++++++++-----
 6 files changed, 221 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_defs.h b/drivers/scsi/pm8001/pm8001_defs.h
index 1c7f15fd69ce..fd700ce5e80c 100644
--- a/drivers/scsi/pm8001/pm8001_defs.h
+++ b/drivers/scsi/pm8001/pm8001_defs.h
@@ -101,6 +101,9 @@ enum port_type {
 #define USI_MAX_MEMCNT		(PI + PM8001_MAX_SPCV_OUTB_NUM)
 #define	CONFIG_SCSI_PM8001_MAX_DMA_SG	528
 #define PM8001_MAX_DMA_SG	CONFIG_SCSI_PM8001_MAX_DMA_SG
+#define SPINUP_DELAY_OFFSET		0x890 /* 0x890 - delay */
+#define SPINUP_GROUP_OFFSET		0x892 /* 0x892 - group */
+#define PM80XX_MAX_SPINUP_DELAY	10000 /* 10000 ms */
 enum memory_region_num {
 	AAP1 = 0x0, /* application acceleration processor */
 	IOP,	    /* IO processor */
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index fb9848e1d481..6378c8e8d6b2 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -3237,7 +3237,7 @@ int pm8001_mpi_local_phy_ctl(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		(struct local_phy_ctl_resp *)(piomb + 4);
 	u32 status = le32_to_cpu(pPayload->status);
 	u32 phy_id = le32_to_cpu(pPayload->phyop_phyid) & ID_BITS;
-	u32 phy_op = le32_to_cpu(pPayload->phyop_phyid) & OP_BITS;
+	u32 phy_op = (le32_to_cpu(pPayload->phyop_phyid) & OP_BITS) >> 8;
 	tag = le32_to_cpu(pPayload->tag);
 	if (status != 0) {
 		PM8001_MSG_DBG(pm8001_ha,
@@ -3248,6 +3248,13 @@ int pm8001_mpi_local_phy_ctl(struct pm8001_hba_info *pm8001_ha, void *piomb)
 			pm8001_printk("%x phy execute %x phy op success!\n",
 			phy_id, phy_op));
 		pm8001_ha->phy[phy_id].reset_success = true;
+		if (phy_op == PHY_NOTIFY_ENABLE_SPINUP &&
+			!pm8001_ha->reset_in_progress){
+			/* Notify the sas layer to discover
+			 * the the whole sas domain
+			 */
+			pm8001_bytes_dmaed(pm8001_ha, phy_id);
+		}
 	}
 	if (pm8001_ha->phy[phy_id].enable_completion) {
 		complete(pm8001_ha->phy[phy_id].enable_completion);
@@ -3643,7 +3650,10 @@ int pm8001_mpi_reg_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
 			pm8001_printk("DEVREG_FAILURE_DEVICE_TYPE_NOT_SUPPORTED\n"));
 		break;
 	}
-	complete(pm8001_dev->dcompletion);
+	if (pm8001_dev->dcompletion) {
+		complete(pm8001_dev->dcompletion);
+		pm8001_dev->dcompletion = NULL;
+	}
 	ccb->task = NULL;
 	ccb->ccb_tag = 0xFFFFFFFF;
 	pm8001_tag_free(pm8001_ha, htag);
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 38a65e1da823..46a2f86f3d9b 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -55,6 +55,12 @@ MODULE_PARM_DESC(link_rate, "Enable link rate.\n"
 		" 4: Link rate 6.0G\n"
 		" 8: Link rate 12.0G\n");
 
+bool staggered_spinup;
+module_param(staggered_spinup, bool, 0644);
+MODULE_PARM_DESC(staggered_spinup, "enable the staggered spinup feature.\n"
+		" 0/N: false\n"
+		" 1/Y: true\n");
+
 static struct scsi_transport_template *pm8001_stt;
 
 /**
@@ -164,7 +170,7 @@ static void pm8001_free(struct pm8001_hba_info *pm8001_ha)
 
 	if (!pm8001_ha)
 		return;
-
+	del_timer(&pm8001_ha->spinup_timer);
 	for (i = 0; i < USI_MAX_MEMCNT; i++) {
 		if (pm8001_ha->memoryMap.region[i].virt_ptr != NULL) {
 			dma_free_coherent(&pm8001_ha->pdev->dev,
@@ -488,6 +494,7 @@ static struct pm8001_hba_info *pm8001_pci_alloc(struct pci_dev *pdev,
 	pm8001_ha->shost = shost;
 	pm8001_ha->id = pm8001_id++;
 	pm8001_ha->logging_level = logging_level;
+	pm8001_ha->staggered_spinup = staggered_spinup;
 	pm8001_ha->non_fatal_count = 0;
 	if (link_rate >= 1 && link_rate <= 15)
 		pm8001_ha->link_rate = (link_rate << 8);
@@ -621,7 +628,8 @@ static void  pm8001_post_sas_ha_init(struct Scsi_Host *shost,
  * Currently we just set the fixed SAS address to our HBA,for manufacture,
  * it should read from the EEPROM
  */
-static void pm8001_init_sas_add(struct pm8001_hba_info *pm8001_ha)
+static void pm8001_init_sas_add_and_spinup_config
+		(struct pm8001_hba_info *pm8001_ha)
 {
 	u8 i, j;
 	u8 sas_add[8];
@@ -707,6 +715,45 @@ static void pm8001_init_sas_add(struct pm8001_hba_info *pm8001_ha)
 	memcpy(pm8001_ha->sas_addr, &pm8001_ha->phy[0].dev_sas_addr,
 		SAS_ADDR_SIZE);
 #endif
+
+	/* For spinning up drives in group */
+	pm8001_ha->phy_head = -1;
+	pm8001_ha->phy_tail = -1;
+
+	for (i = 0; i < PM8001_MAX_PHYS; i++)
+		pm8001_ha->phy_up[i] = 0xff;
+
+	timer_setup(&pm8001_ha->spinup_timer,
+		(void *)pm8001_spinup_timedout, 0);
+
+	if (pm8001_ha->staggered_spinup == true) {
+		/* spinup interval in unit of 100 ms */
+		pm8001_ha->spinup_interval =
+			payload.func_specific[SPINUP_DELAY_OFFSET] * 100;
+		pm8001_ha->spinup_group =
+			payload.func_specific[SPINUP_GROUP_OFFSET];
+	} else {
+		pm8001_ha->spinup_interval = 0;
+		pm8001_ha->spinup_group = pm8001_ha->chip->n_phy;
+	}
+
+	if (pm8001_ha->spinup_interval > PM80XX_MAX_SPINUP_DELAY) {
+		PM8001_DISC_DBG(pm8001_ha, pm8001_printk(
+		"Spinup delay from Seeprom is %d ms, reset to %d ms\n",
+		pm8001_ha->spinup_interval * 100, PM80XX_MAX_SPINUP_DELAY));
+		pm8001_ha->spinup_interval = PM80XX_MAX_SPINUP_DELAY;
+	}
+
+	if (pm8001_ha->spinup_group > pm8001_ha->chip->n_phy) {
+		PM8001_DISC_DBG(pm8001_ha, pm8001_printk(
+		"Spinup group from Seeprom is %d, reset to %d\n",
+		pm8001_ha->spinup_group, pm8001_ha->chip->n_phy));
+		pm8001_ha->spinup_group = pm8001_ha->chip->n_phy;
+	}
+
+	PM8001_MSG_DBG(pm8001_ha, pm8001_printk(
+		"Spinup interval : %d Spinup group %d\n",
+		pm8001_ha->spinup_interval, pm8001_ha->spinup_group));
 }
 
 /*
@@ -1105,7 +1152,7 @@ static int pm8001_pci_probe(struct pci_dev *pdev,
 		pm80xx_set_thermal_config(pm8001_ha);
 	}
 
-	pm8001_init_sas_add(pm8001_ha);
+	pm8001_init_sas_add_and_spinup_config(pm8001_ha);
 	/* phy setting support for motherboard controller */
 	if (pm8001_configure_phy_settings(pm8001_ha))
 		goto err_out_shost;
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index b7cbc312843e..cf1c5e68953b 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -265,14 +265,39 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
 void pm8001_scan_start(struct Scsi_Host *shost)
 {
 	int i;
+	unsigned long lock_flags;
 	struct pm8001_hba_info *pm8001_ha;
 	struct sas_ha_struct *sha = SHOST_TO_SAS_HA(shost);
+	DECLARE_COMPLETION(comp);
 	pm8001_ha = sha->lldd_ha;
 	/* SAS_RE_INITIALIZATION not available in SPCv/ve */
 	if (pm8001_ha->chip_id == chip_8001)
 		PM8001_CHIP_DISP->sas_re_init_req(pm8001_ha);
-	for (i = 0; i < pm8001_ha->chip->n_phy; ++i)
-		PM8001_CHIP_DISP->phy_start_req(pm8001_ha, i);
+
+	if (pm8001_ha->pdev->device == 0x8001 ||
+		pm8001_ha->pdev->device == 0x8081 ||
+		(pm8001_ha->spinup_interval != 0)) {
+		for (i = 0; i < pm8001_ha->chip->n_phy; ++i)
+			PM8001_CHIP_DISP->phy_start_req(pm8001_ha, i);
+	} else {
+		for (i = 0; i < pm8001_ha->chip->n_phy; ++i) {
+			spin_lock_irqsave(&pm8001_ha->lock, lock_flags);
+			pm8001_ha->phy_started = i;
+			pm8001_ha->scan_completion = &comp;
+			pm8001_ha->phystart_timedout = 1;
+			spin_unlock_irqrestore(&pm8001_ha->lock, lock_flags);
+			PM8001_CHIP_DISP->phy_start_req(pm8001_ha, i);
+			wait_for_completion_timeout(&comp,
+				msecs_to_jiffies(500));
+			if (pm8001_ha->phystart_timedout)
+				PM8001_MSG_DBG(pm8001_ha, pm8001_printk(
+				"Timeout happened for phyid = %d\n", i));
+		}
+		spin_lock_irqsave(&pm8001_ha->lock, lock_flags);
+		pm8001_ha->phy_started = -1;
+		pm8001_ha->scan_completion = NULL;
+		spin_unlock_irqrestore(&pm8001_ha->lock, lock_flags);
+	}
 }
 
 int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time)
@@ -662,6 +687,13 @@ static int pm8001_dev_found_notify(struct domain_device *dev)
 			flag = 1; /* directly sata */
 		}
 	} /*register this device to HBA*/
+
+	if (pm8001_ha->phy_started == pm8001_device->attached_phy) {
+		if (pm8001_ha->scan_completion != NULL) {
+			pm8001_ha->phystart_timedout = 0;
+			complete(pm8001_ha->scan_completion);
+		}
+	}
 	PM8001_DISC_DBG(pm8001_ha, pm8001_printk("Found device\n"));
 	PM8001_CHIP_DISP->reg_dev_req(pm8001_ha, pm8001_device, flag);
 	spin_unlock_irqrestore(&pm8001_ha->lock, flags);
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index 488af79dec47..d2d73cba7f41 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -261,6 +261,7 @@ struct pm8001_port {
 	u8			port_attached;
 	u16			wide_port_phymap;
 	u8			port_state;
+	u8			port_id;
 	struct list_head	list;
 };
 
@@ -566,6 +567,17 @@ struct pm8001_hba_info {
 	u32			reset_in_progress;
 	u32			non_fatal_count;
 	u32			non_fatal_read_length;
+	bool			staggered_spinup;
+	struct completion	*scan_completion;
+	u32			phy_started;
+	u16			phystart_timedout;
+	int			spinup_group;
+	int			spinup_interval;
+	int			phy_up[PM8001_MAX_PHYS];
+	struct timer_list	spinup_timer;
+	int			phy_head;
+	int			phy_tail;
+	spinlock_t		phy_q_lock;
 };
 
 struct pm8001_work {
@@ -684,6 +696,7 @@ void pm8001_open_reject_retry(
 int pm8001_mem_alloc(struct pci_dev *pdev, void **virt_addr,
 	dma_addr_t *pphys_addr, u32 *pphys_addr_hi, u32 *pphys_addr_lo,
 	u32 mem_size, u32 align);
+void pm8001_spinup_timedout(struct timer_list *t);
 
 void pm8001_chip_iounmap(struct pm8001_hba_info *pm8001_ha);
 int pm8001_mpi_build_cmd(struct pm8001_hba_info *pm8001_ha,
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index a909d00c6897..748c7a06262f 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -46,6 +46,72 @@
 #define SMP_DIRECT 1
 #define SMP_INDIRECT 2
 
+static int pm80xx_chip_phy_ctl_req(struct pm8001_hba_info *pm8001_ha,
+	u32 phyId, u32 phy_op);
+
+void  pm8001_queue_phyup(struct pm8001_hba_info *pm8001_ha, int phy_id)
+{
+	int i;
+
+	if (pm8001_ha->phy_head == -1) {
+		pm8001_ha->phy_head = pm8001_ha->phy_tail = 0;
+	} else {
+		/* If the phy id is already queued , discard the phy up */
+		for (i = 0; i < pm8001_ha->chip->n_phy; i++)
+			if (pm8001_ha->phy_up[i] == phy_id)
+				return;
+		pm8001_ha->phy_tail =
+			(pm8001_ha->phy_tail + 1) % PM8001_MAX_PHYS;
+	}
+	pm8001_ha->phy_up[pm8001_ha->phy_tail] = phy_id;
+}
+
+void pm8001_spinup_timedout(struct timer_list *t)
+{
+	struct pm8001_hba_info *pm8001_ha =
+			from_timer(pm8001_ha, t, spinup_timer);
+	struct pm8001_phy *phy;
+	unsigned long flags;
+	int i = 0, phy_id = 0xff;
+
+	spin_lock_irqsave(&pm8001_ha->phy_q_lock, flags);
+
+	do {
+		if (i++ >= pm8001_ha->spinup_group && pm8001_ha->spinup_group)
+			break;
+
+		if (pm8001_ha->phy_head == -1 || pm8001_ha->reset_in_progress)
+			break; /* No phys to spinup */
+
+		phy_id = pm8001_ha->phy_up[pm8001_ha->phy_head];
+		/* Processed phy id, make it invalid 0xff for
+		 * checking repeated phy ups
+		 */
+		pm8001_ha->phy_up[pm8001_ha->phy_head] = 0xff;
+		if (pm8001_ha->phy_head == pm8001_ha->phy_tail) {
+			pm8001_ha->phy_head = pm8001_ha->phy_tail = -1;
+		} else {
+			pm8001_ha->phy_head =
+				(pm8001_ha->phy_head+1) % PM8001_MAX_PHYS;
+		}
+
+		if (phy_id == 0xff)
+			break;
+		phy = &pm8001_ha->phy[phy_id];
+		if (phy->phy_type & PORT_TYPE_SAS) {
+			PM8001_CHIP_DISP->phy_ctl_req(pm8001_ha, phy_id,
+					PHY_NOTIFY_ENABLE_SPINUP);
+		} else {
+			PM8001_CHIP_DISP->phy_ctl_req(pm8001_ha, phy_id,
+					PHY_LINK_RESET);
+		}
+	} while (1);
+
+	if (pm8001_ha->phy_head != -1 && pm8001_ha->spinup_group)
+		mod_timer(&pm8001_ha->spinup_timer,
+			jiffies + msecs_to_jiffies(pm8001_ha->spinup_interval));
+	spin_unlock_irqrestore(&pm8001_ha->phy_q_lock, flags);
+}
 
 int pm80xx_bar4_shift(struct pm8001_hba_info *pm8001_ha, u32 shift_value)
 {
@@ -3302,11 +3368,12 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
 	port->port_state = portstate;
 	port->wide_port_phymap |= (1U << phy_id);
 	phy->phy_state = PHY_STATE_LINK_UP_SPCV;
+	phy->port = port;
 	PM8001_MSG_DBG(pm8001_ha, pm8001_printk(
 		"portid:%d; phyid:%d; linkrate:%d; "
 		"portstate:%x; devicetype:%x\n",
 		port_id, phy_id, link_rate, portstate, deviceType));
-
+	port->port_id = port_id;
 	switch (deviceType) {
 	case SAS_PHY_UNUSED:
 		PM8001_MSG_DBG(pm8001_ha,
@@ -3314,8 +3381,12 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		break;
 	case SAS_END_DEVICE:
 		PM8001_MSG_DBG(pm8001_ha, pm8001_printk("end device.\n"));
-		pm80xx_chip_phy_ctl_req(pm8001_ha, phy_id,
-			PHY_NOTIFY_ENABLE_SPINUP);
+		spin_lock_irqsave(&pm8001_ha->phy_q_lock, flags);
+		pm8001_queue_phyup(pm8001_ha, phy_id);
+		spin_unlock_irqrestore(&pm8001_ha->phy_q_lock, flags);
+		if (!timer_pending(&pm8001_ha->spinup_timer))
+			mod_timer(&pm8001_ha->spinup_timer,
+			jiffies + msecs_to_jiffies(pm8001_ha->spinup_interval));
 		port->port_attached = 1;
 		pm8001_get_lrate_mode(phy, link_rate);
 		break;
@@ -3351,9 +3422,10 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
 	phy->frame_rcvd_size = sizeof(struct sas_identify_frame) - 4;
 	pm8001_get_attached_sas_addr(phy, phy->sas_phy.attached_sas_addr);
 	spin_unlock_irqrestore(&phy->sas_phy.frame_rcvd_lock, flags);
-	if (pm8001_ha->flags == PM8001F_RUN_TIME)
-		msleep(200);/*delay a moment to wait disk to spinup*/
-	pm8001_bytes_dmaed(pm8001_ha, phy_id);
+	if (!pm8001_ha->reset_in_progress) {
+		if (deviceType != SAS_END_DEVICE)
+			pm8001_bytes_dmaed(pm8001_ha, phy_id);
+	}
 }
 
 /**
@@ -3388,11 +3460,17 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
 	port->port_state = portstate;
 	phy->phy_state = PHY_STATE_LINK_UP_SPCV;
 	port->port_attached = 1;
+	phy->port = port;
+	port->port_id = port_id;
 	pm8001_get_lrate_mode(phy, link_rate);
 	phy->phy_type |= PORT_TYPE_SATA;
 	phy->phy_attached = 1;
 	phy->sas_phy.oob_mode = SATA_OOB_MODE;
-	sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_DONE);
+	if (!pm8001_ha->reset_in_progress) {
+		sas_ha->notify_phy_event(&phy->sas_phy, PHYE_OOB_DONE);
+	} else
+		PM8001_MSG_DBG(pm8001_ha, pm8001_printk(
+			"HW_EVENT_PHY_UP: not notified to host\n"));
 	spin_lock_irqsave(&phy->sas_phy.frame_rcvd_lock, flags);
 	memcpy(phy->frame_rcvd, ((u8 *)&pPayload->sata_fis - 4),
 		sizeof(struct dev_to_host_fis));
@@ -3401,7 +3479,8 @@ hw_event_sata_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
 	phy->identify.device_type = SAS_SATA_DEV;
 	pm8001_get_attached_sas_addr(phy, phy->sas_phy.attached_sas_addr);
 	spin_unlock_irqrestore(&phy->sas_phy.frame_rcvd_lock, flags);
-	pm8001_bytes_dmaed(pm8001_ha, phy_id);
+	if (!pm8001_ha->reset_in_progress)
+		pm8001_bytes_dmaed(pm8001_ha, phy_id);
 }
 
 /**
@@ -3497,12 +3576,14 @@ static int mpi_phy_start_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
 				status, phy_id));
 	if (status == 0) {
 		phy->phy_state = PHY_LINK_DOWN;
-		if (pm8001_ha->flags == PM8001F_RUN_TIME &&
-				phy->enable_completion != NULL) {
-			complete(phy->enable_completion);
-			phy->enable_completion = NULL;
-		}
 	}
+
+	if (pm8001_ha->flags == PM8001F_RUN_TIME &&
+		phy->enable_completion != NULL) {
+		complete(phy->enable_completion);
+		phy->enable_completion = NULL;
+	}
+
 	return 0;
 
 }
@@ -3580,7 +3661,14 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
 	case HW_EVENT_SATA_SPINUP_HOLD:
 		PM8001_MSG_DBG(pm8001_ha,
 			pm8001_printk("HW_EVENT_SATA_SPINUP_HOLD\n"));
-		sas_ha->notify_phy_event(&phy->sas_phy, PHYE_SPINUP_HOLD);
+		spin_lock_irqsave(&pm8001_ha->phy_q_lock, flags);
+		pm8001_queue_phyup(pm8001_ha, phy_id);
+		spin_unlock_irqrestore(&pm8001_ha->phy_q_lock, flags);
+
+		/* Start the timer if not started */
+		if (!timer_pending(&pm8001_ha->spinup_timer))
+			mod_timer(&pm8001_ha->spinup_timer,
+			jiffies + msecs_to_jiffies(pm8001_ha->spinup_interval));
 		break;
 	case HW_EVENT_PHY_DOWN:
 		PM8001_MSG_DBG(pm8001_ha,
@@ -4888,7 +4976,7 @@ pm80xx_chip_phy_start_req(struct pm8001_hba_info *pm8001_ha, u8 phy_id)
 	PM8001_INIT_DBG(pm8001_ha,
 		pm8001_printk("PHY START REQ for phy_id %d\n", phy_id));
 
-	payload.ase_sh_lm_slr_phyid = cpu_to_le32(SPINHOLD_DISABLE |
+	payload.ase_sh_lm_slr_phyid = cpu_to_le32(SPINHOLD_ENABLE |
 			LINKMODE_AUTO | pm8001_ha->link_rate | phy_id);
 	/* SSC Disable and SAS Analog ST configuration */
 	/**
@@ -4950,6 +5038,8 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 	u16 ITNT = 2000;
 	struct domain_device *dev = pm8001_dev->sas_device;
 	struct domain_device *parent_dev = dev->parent;
+	struct pm8001_phy *phy;
+	struct pm8001_port *port;
 	circularQ = &pm8001_ha->inbnd_q_tbl[0];
 
 	memset(&payload, 0, sizeof(payload));
@@ -4981,8 +5071,11 @@ static int pm80xx_chip_reg_dev_req(struct pm8001_hba_info *pm8001_ha,
 	linkrate = (pm8001_dev->sas_device->linkrate < dev->port->linkrate) ?
 			pm8001_dev->sas_device->linkrate : dev->port->linkrate;
 
+	phy = &pm8001_ha->phy[phy_id];
+	port = phy->port;
+
 	payload.phyid_portid =
-		cpu_to_le32(((pm8001_dev->sas_device->port->id) & 0xFF) |
+		cpu_to_le32(((port->port_id) & 0xFF) |
 		((phy_id & 0xFF) << 8));
 
 	payload.dtype_dlr_mcn_ir_retry = cpu_to_le32((retryFlag & 0x01) |
-- 
2.16.3

[PATCH V2 3/3] pm80xx : Wait for PHY startup before draining libsas queue.

[Deepak Ukey]

From: peter chang <dpf@google.com>

The host's scan finish waits for the libsas queue to drain. However,
if the PHYs are still in the process of starting then the queue will
be empty. This means that we declare the scan finished before it has
even started. Here we wait for various events from the firmware-side.

The wait behavior can be configured using the module parameter
"wait_for_phy_startup", if the parameter is enabled, the driver will wait
for the phy event from the firmware, before proceeding with load.

Signed-off-by: Viswas G <Viswas.G@microchip.com>
Signed-off-by: peter chang <dpf@google.com>
Signed-off-by: Radha Ramachandran <radha@google.com>
Signed-off-by: Deepak Ukey <Deepak.Ukey@microchip.com>
---
 drivers/scsi/pm8001/pm8001_ctl.c  | 36 +++++++++++++++++++++
 drivers/scsi/pm8001/pm8001_defs.h |  6 ++--
 drivers/scsi/pm8001/pm8001_init.c | 22 +++++++++++++
 drivers/scsi/pm8001/pm8001_sas.c  | 67 +++++++++++++++++++++++++++++++++++++--
 drivers/scsi/pm8001/pm8001_sas.h  |  4 +++
 drivers/scsi/pm8001/pm80xx_hwi.c  | 67 ++++++++++++++++++++++++++++++++-------
 6 files changed, 185 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index 3c9f42779dd0..eae629610a5f 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -88,6 +88,41 @@ static ssize_t controller_fatal_error_show(struct device *cdev,
 }
 static DEVICE_ATTR_RO(controller_fatal_error);
 
+/**
+ * phy_startup_timeout_show - per-phy discovery timeout
+ * @cdev: pointer to embedded class device
+ * @buf: the buffer returned
+ *
+ * A sysfs 'read/write' shost attribute.
+ */
+static ssize_t phy_startup_timeout_show(struct device *cdev,
+	struct device_attribute *attr, char *buf)
+{
+	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;
+
+	return snprintf(buf, PAGE_SIZE, "%08xh\n",
+		pm8001_ha->phy_startup_timeout / HZ);
+}
+
+static ssize_t phy_startup_timeout_store(struct device *cdev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	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;
+	int val = 0;
+
+	if (kstrtoint(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	pm8001_ha->phy_startup_timeout = val * HZ;
+	return strlen(buf);
+}
+
+static DEVICE_ATTR_RW(phy_startup_timeout);
+
 /**
  * pm8001_ctl_fw_version_show - firmware version
  * @cdev: pointer to embedded class device
@@ -867,6 +902,7 @@ static DEVICE_ATTR(update_fw, S_IRUGO|S_IWUSR|S_IWGRP,
 struct device_attribute *pm8001_host_attrs[] = {
 	&dev_attr_interface_rev,
 	&dev_attr_controller_fatal_error,
+	&dev_attr_phy_startup_timeout,
 	&dev_attr_fw_version,
 	&dev_attr_update_fw,
 	&dev_attr_aap_log,
diff --git a/drivers/scsi/pm8001/pm8001_defs.h b/drivers/scsi/pm8001/pm8001_defs.h
index fd700ce5e80c..aaeabb2f2808 100644
--- a/drivers/scsi/pm8001/pm8001_defs.h
+++ b/drivers/scsi/pm8001/pm8001_defs.h
@@ -141,7 +141,9 @@ enum pm8001_hba_info_flags {
  */
 #define PHY_LINK_DISABLE	0x00
 #define PHY_LINK_DOWN		0x01
-#define PHY_STATE_LINK_UP_SPCV	0x2
-#define PHY_STATE_LINK_UP_SPC	0x1
+#define PHY_STATE_LINK_UP_SPCV	0x02
+#define PHY_STATE_LINK_UP_SPC	0x01
+#define PHY_STATE_LINK_RESET	0x03
+#define PHY_STATE_HARD_RESET	0x04
 
 #endif
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 46a2f86f3d9b..55b40e4663e6 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -61,6 +61,26 @@ MODULE_PARM_DESC(staggered_spinup, "enable the staggered spinup feature.\n"
 		" 0/N: false\n"
 		" 1/Y: true\n");
 
+static bool wait_for_phy_startup;
+module_param(wait_for_phy_startup, bool, 0644);
+MODULE_PARM_DESC(wait_for_phy_startup, "wait for phy start.\n"
+		" 0/N: disable\n"
+		" 1/Y: enable\n");
+
+/* if nothing is detected, the PHYs will reset continuously once they
+ * are started. we don't have a good way of differentiating a trained
+ * but waiting-on-signature from one that's never going to train
+ * (nothing attached or dead drive), so we wait an possibly
+ * unreasonable amount of time. this is stuck in start_timeout, and
+ * checked in the host's scan_finished callback for PHYs that haven't
+ * yet come up.
+ *
+ */
+static ulong phy_startup_timeout = 60;
+module_param(phy_startup_timeout, ulong, 0644);
+MODULE_PARM_DESC(phy_startup_timeout_s,
+		" seconds to wait for discovery, per-PHY.");
+
 static struct scsi_transport_template *pm8001_stt;
 
 /**
@@ -495,6 +515,8 @@ static struct pm8001_hba_info *pm8001_pci_alloc(struct pci_dev *pdev,
 	pm8001_ha->id = pm8001_id++;
 	pm8001_ha->logging_level = logging_level;
 	pm8001_ha->staggered_spinup = staggered_spinup;
+	pm8001_ha->phy_startup_timeout = phy_startup_timeout * HZ;
+	pm8001_ha->wait_for_phy_startup = wait_for_phy_startup;
 	pm8001_ha->non_fatal_count = 0;
 	if (link_rate >= 1 && link_rate <= 15)
 		pm8001_ha->link_rate = (link_rate << 8);
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index cf1c5e68953b..21086bdae6a9 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -39,6 +39,7 @@
  */
 
 #include <linux/slab.h>
+#include "pm80xx_hwi.h"
 #include "pm8001_sas.h"
 
 /**
@@ -257,6 +258,59 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
 	return rc;
 }
 
+static int pm8001_update_phy_mask(struct pm8001_hba_info *pm8001_ha)
+{
+	DECLARE_COMPLETION_ONSTACK(comp);
+	unsigned long timeout = msecs_to_jiffies(2000);
+	int ret = 0;
+	int i;
+	struct phy_status *phy_status =
+		kmalloc(sizeof(struct phy_status), GFP_KERNEL);
+	if (phy_status == NULL) {
+		pm8001_printk("Allocation failed\n");
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < pm8001_ha->chip->n_phy; i++) {
+		struct pm8001_phy *phy = pm8001_ha->phy + i;
+
+		if (pm8001_ha->phy_mask & (1 << i) && phy->phy_state) {
+			ret = PM8001_CHIP_DISP->get_phy_profile_req(pm8001_ha,
+					i, SAS_PHY_GENERAL_STATUS_PAGE,
+					&comp, (void *)phy_status);
+			if (ret != 0) {
+				pm8001_printk("get_phy_profile_req:%d:%d\n",
+					i, ret);
+				return ret;
+			}
+			timeout = wait_for_completion_timeout(&comp, timeout);
+			if (timeout == 0)
+				return ret;
+			PM8001_DISC_DBG(pm8001_ha,
+				pm8001_printk("phy_id:%d state:%x nlr:%d plr:%d\n",
+					i,
+					phy_status->phy_state,
+					phy_status->nlr, phy_status->plr));
+			switch (phy_status->phy_state) {
+			case PHY_STATE_LINK_UP_SPCV:
+			case PHY_STATE_LINK_RESET:
+			case PHY_STATE_HARD_RESET:
+				/* these are handled by mpi_hw_event() */
+				break;
+			case PHY_LINK_DOWN:
+				if (time_is_after_jiffies(phy->start_timeout))
+					break;
+				clear_bit(i, &pm8001_ha->phy_mask);
+				break;
+			default:
+				clear_bit(i, &pm8001_ha->phy_mask);
+				break;
+			}
+		}
+	}
+	return ret;
+}
+
 /**
   * pm8001_scan_start - we should enable all HBA phys by sending the phy_start
   * command to HBA.
@@ -274,6 +328,8 @@ void pm8001_scan_start(struct Scsi_Host *shost)
 	if (pm8001_ha->chip_id == chip_8001)
 		PM8001_CHIP_DISP->sas_re_init_req(pm8001_ha);
 
+	pm8001_ha->phy_mask = 0;
+
 	if (pm8001_ha->pdev->device == 0x8001 ||
 		pm8001_ha->pdev->device == 0x8081 ||
 		(pm8001_ha->spinup_interval != 0)) {
@@ -303,11 +359,16 @@ void pm8001_scan_start(struct Scsi_Host *shost)
 int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time)
 {
 	struct sas_ha_struct *ha = SHOST_TO_SAS_HA(shost);
+	struct pm8001_hba_info *pm8001_ha = ha->lldd_ha;
 
-	/* give the phy enabling interrupt event time to come in (1s
-	* is empirically about all it takes) */
-	if (time < HZ)
+	/* Wait for PHY startup to finish */
+	PM8001_DISC_DBG(pm8001_ha,
+		pm8001_printk("mask:%lx\n", pm8001_ha->phy_mask));
+	if (pm8001_ha->phy_mask && pm8001_ha->wait_for_phy_startup) {
+		pm8001_update_phy_mask(pm8001_ha);
 		return 0;
+	}
+
 	/* Wait for discovery to finish */
 	sas_drain_work(ha);
 	return 1;
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index d2d73cba7f41..37c526be55bc 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -283,6 +283,7 @@ struct pm8001_phy {
 	struct completion	*reset_completion;
 	bool			port_reset_status;
 	bool			reset_success;
+	unsigned long		start_timeout;
 };
 
 /* port reset status */
@@ -578,6 +579,9 @@ struct pm8001_hba_info {
 	int			phy_head;
 	int			phy_tail;
 	spinlock_t		phy_q_lock;
+	unsigned long		phy_mask;
+	u32			phy_startup_timeout;
+	bool			wait_for_phy_startup;
 };
 
 struct pm8001_work {
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 748c7a06262f..91dc9d6d3657 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -80,18 +80,26 @@ void pm8001_spinup_timedout(struct timer_list *t)
 		if (i++ >= pm8001_ha->spinup_group && pm8001_ha->spinup_group)
 			break;
 
-		if (pm8001_ha->phy_head == -1 || pm8001_ha->reset_in_progress)
-			break; /* No phys to spinup */
-
-		phy_id = pm8001_ha->phy_up[pm8001_ha->phy_head];
-		/* Processed phy id, make it invalid 0xff for
-		 * checking repeated phy ups
-		 */
-		pm8001_ha->phy_up[pm8001_ha->phy_head] = 0xff;
-		if (pm8001_ha->phy_head == pm8001_ha->phy_tail) {
-			pm8001_ha->phy_head = pm8001_ha->phy_tail = -1;
+		if (pm8001_ha->phy_head == -1 ||
+				pm8001_ha->reset_in_progress) {
+			/* No phys to spinup or timeout */
+			if (pm8001_ha->phy_mask) {
+				pm8001_printk("spinup timeout phy_mask:%lx\n",
+					pm8001_ha->phy_mask);
+				pm8001_ha->phy_mask = 0;
+			}
+			break;
 		} else {
-			pm8001_ha->phy_head =
+			phy_id = pm8001_ha->phy_up[pm8001_ha->phy_head];
+			/* Processed phy id, make it invalid 0xff for
+			 * checking repeated phy ups
+			 */
+			pm8001_ha->phy_up[pm8001_ha->phy_head] = 0xff;
+			if (pm8001_ha->phy_head == pm8001_ha->phy_tail)
+				pm8001_ha->phy_head =
+					pm8001_ha->phy_tail = -1;
+			else
+				pm8001_ha->phy_head =
 				(pm8001_ha->phy_head+1) % PM8001_MAX_PHYS;
 		}
 
@@ -3584,8 +3592,10 @@ static int mpi_phy_start_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		phy->enable_completion = NULL;
 	}
 
-	return 0;
+	if (!phy->phy_state)
+		clear_bit(phy_id, &pm8001_ha->phy_mask);
 
+	return 0;
 }
 
 /**
@@ -3646,6 +3656,36 @@ static int mpi_hw_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
 		pm8001_printk("portid:%d phyid:%d event:0x%x status:0x%x\n",
 				port_id, phy_id, eventType, status));
 
+	switch (eventType) {
+		/* the PHY has completed startup and possibly queued
+		 * stuff to the libsas side, so we no longer have to
+		 * track that it is in the middle of doing something
+		 * that libsas doesn't know about.
+		 */
+	case HW_EVENT_SAS_PHY_UP:
+	case HW_EVENT_SATA_PHY_UP:
+	case HW_EVENT_PHY_DOWN:
+	case HW_EVENT_PORT_INVALID:
+	case HW_EVENT_BROADCAST_CHANGE:
+	case HW_EVENT_PHY_ERROR:
+	case HW_EVENT_HARD_RESET_RECEIVED:
+	case HW_EVENT_ID_FRAME_TIMEOUT:
+	case HW_EVENT_LINK_ERR_PHY_RESET_FAILED:
+	case HW_EVENT_PORT_RESET_TIMER_TMO:
+	case HW_EVENT_PORT_RECOVERY_TIMER_TMO:
+	case HW_EVENT_PORT_RECOVER:
+		clear_bit(phy_id, &pm8001_ha->phy_mask);
+		break;
+		/* we may have already gotten the response from the
+		 * PHY, but if the PHY was powered off then this event
+		 * signals that there is more in-flight and we have
+		 * restarted the the spinup timer.
+		 */
+	case HW_EVENT_SATA_SPINUP_HOLD:
+		set_bit(phy_id, &pm8001_ha->phy_mask);
+		break;
+	}
+
 	switch (eventType) {
 
 	case HW_EVENT_SAS_PHY_UP:
@@ -4975,6 +5015,9 @@ pm80xx_chip_phy_start_req(struct pm8001_hba_info *pm8001_ha, u8 phy_id)
 
 	PM8001_INIT_DBG(pm8001_ha,
 		pm8001_printk("PHY START REQ for phy_id %d\n", phy_id));
+	set_bit(phy_id, &pm8001_ha->phy_mask);
+	pm8001_ha->phy[phy_id].start_timeout =
+		jiffies + pm8001_ha->phy_startup_timeout;
 
 	payload.ase_sh_lm_slr_phyid = cpu_to_le32(SPINHOLD_ENABLE |
 			LINKMODE_AUTO | pm8001_ha->link_rate | phy_id);
-- 
2.16.3

Re: [PATCH V2 1/3] pm80xx : Support for get phy profile functionality.

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2893 bytes --]

Hi Deepak,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.8-rc2 next-20200624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Deepak-Ukey/pm80xx-Updates-for-the-driver-version-0-1-39/20200624-195528
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-rhel-7.6 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/scsi/pm8001/pm80xx_hwi.c:69:6: warning: no previous prototype for 'pm80xx_pci_mem_copy' [-Wmissing-prototypes]
      69 | void pm80xx_pci_mem_copy(struct pm8001_hba_info  *pm8001_ha, u32 soffset,
         |      ^~~~~~~~~~~~~~~~~~~
>> drivers/scsi/pm8001/pm80xx_hwi.c:5064:5: warning: no previous prototype for 'pm8001_chip_get_phy_profile' [-Wmissing-prototypes]
    5064 | int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/scsi/pm8001/pm80xx_hwi.c:5094:6: warning: no previous prototype for 'mpi_set_phy_profile_req' [-Wmissing-prototypes]
    5094 | void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
         |      ^~~~~~~~~~~~~~~~~~~~~~~

vim +/pm8001_chip_get_phy_profile +5064 drivers/scsi/pm8001/pm80xx_hwi.c

  5063	
> 5064	int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
  5065			int page_code, struct completion *comp, void *buf)
  5066	{
  5067		u32 tag;
  5068		struct get_phy_profile_req payload;
  5069		struct inbound_queue_table *circularQ;
  5070		struct pm8001_ccb_info *ccb;
  5071		int rc, ppc_phyid;
  5072		u32 opc = OPC_INB_GET_PHY_PROFILE;
  5073	
  5074		memset(&payload, 0, sizeof(payload));
  5075	
  5076		rc = pm8001_tag_alloc(pm8001_ha, &tag);
  5077		if (rc)
  5078			PM8001_FAIL_DBG(pm8001_ha, pm8001_printk("Invalid tag\n"));
  5079	
  5080		ccb = &pm8001_ha->ccb_info[tag];
  5081		ccb->completion = comp;
  5082		ccb->resp_buf = buf;
  5083		circularQ = &pm8001_ha->inbnd_q_tbl[0];
  5084	
  5085		payload.tag = cpu_to_le32(tag);
  5086		ppc_phyid = (page_code & 0xFF)  << 8 | (phy_id & 0xFF);
  5087		payload.ppc_phyid = cpu_to_le32(ppc_phyid);
  5088	
  5089		pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &payload,
  5090				sizeof(payload), 0);
  5091		return rc;
  5092	}
  5093	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49244 bytes --]

Re: [PATCH V2 2/3] pm80xx : Staggered spin up support.

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2794 bytes --]

Hi Deepak,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.8-rc2 next-20200624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Deepak-Ukey/pm80xx-Updates-for-the-driver-version-0-1-39/20200624-195528
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-rhel-7.6 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/scsi/pm8001/pm80xx_hwi.c:52:7: warning: no previous prototype for 'pm8001_queue_phyup' [-Wmissing-prototypes]
      52 | void  pm8001_queue_phyup(struct pm8001_hba_info *pm8001_ha, int phy_id)
         |       ^~~~~~~~~~~~~~~~~~
   drivers/scsi/pm8001/pm80xx_hwi.c:135:6: warning: no previous prototype for 'pm80xx_pci_mem_copy' [-Wmissing-prototypes]
     135 | void pm80xx_pci_mem_copy(struct pm8001_hba_info  *pm8001_ha, u32 soffset,
         |      ^~~~~~~~~~~~~~~~~~~
   drivers/scsi/pm8001/pm80xx_hwi.c:5157:5: warning: no previous prototype for 'pm8001_chip_get_phy_profile' [-Wmissing-prototypes]
    5157 | int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/scsi/pm8001/pm80xx_hwi.c:5187:6: warning: no previous prototype for 'mpi_set_phy_profile_req' [-Wmissing-prototypes]
    5187 | void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
         |      ^~~~~~~~~~~~~~~~~~~~~~~

vim +/pm8001_queue_phyup +52 drivers/scsi/pm8001/pm80xx_hwi.c

    48	
    49	static int pm80xx_chip_phy_ctl_req(struct pm8001_hba_info *pm8001_ha,
    50		u32 phyId, u32 phy_op);
    51	
  > 52	void  pm8001_queue_phyup(struct pm8001_hba_info *pm8001_ha, int phy_id)
    53	{
    54		int i;
    55	
    56		if (pm8001_ha->phy_head == -1) {
    57			pm8001_ha->phy_head = pm8001_ha->phy_tail = 0;
    58		} else {
    59			/* If the phy id is already queued , discard the phy up */
    60			for (i = 0; i < pm8001_ha->chip->n_phy; i++)
    61				if (pm8001_ha->phy_up[i] == phy_id)
    62					return;
    63			pm8001_ha->phy_tail =
    64				(pm8001_ha->phy_tail + 1) % PM8001_MAX_PHYS;
    65		}
    66		pm8001_ha->phy_up[pm8001_ha->phy_tail] = phy_id;
    67	}
    68	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49244 bytes --]

Re: [PATCH V2 1/3] pm80xx : Support for get phy profile functionality.

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3911 bytes --]

Hi Deepak,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.8-rc2 next-20200624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Deepak-Ukey/pm80xx-Updates-for-the-driver-version-0-1-39/20200624-195528
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1d4c87335d5236ea1f35937e1014980ba961ae34)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/scsi/pm8001/pm80xx_hwi.c:69:6: warning: no previous prototype for function 'pm80xx_pci_mem_copy' [-Wmissing-prototypes]
   void pm80xx_pci_mem_copy(struct pm8001_hba_info  *pm8001_ha, u32 soffset,
        ^
   drivers/scsi/pm8001/pm80xx_hwi.c:69:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void pm80xx_pci_mem_copy(struct pm8001_hba_info  *pm8001_ha, u32 soffset,
   ^
   static 
>> drivers/scsi/pm8001/pm80xx_hwi.c:5064:5: warning: no previous prototype for function 'pm8001_chip_get_phy_profile' [-Wmissing-prototypes]
   int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
       ^
   drivers/scsi/pm8001/pm80xx_hwi.c:5064:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
   ^
   static 
   drivers/scsi/pm8001/pm80xx_hwi.c:5094:6: warning: no previous prototype for function 'mpi_set_phy_profile_req' [-Wmissing-prototypes]
   void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
        ^
   drivers/scsi/pm8001/pm80xx_hwi.c:5094:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
   ^
   static 
   3 warnings generated.

vim +/pm8001_chip_get_phy_profile +5064 drivers/scsi/pm8001/pm80xx_hwi.c

  5063	
> 5064	int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
  5065			int page_code, struct completion *comp, void *buf)
  5066	{
  5067		u32 tag;
  5068		struct get_phy_profile_req payload;
  5069		struct inbound_queue_table *circularQ;
  5070		struct pm8001_ccb_info *ccb;
  5071		int rc, ppc_phyid;
  5072		u32 opc = OPC_INB_GET_PHY_PROFILE;
  5073	
  5074		memset(&payload, 0, sizeof(payload));
  5075	
  5076		rc = pm8001_tag_alloc(pm8001_ha, &tag);
  5077		if (rc)
  5078			PM8001_FAIL_DBG(pm8001_ha, pm8001_printk("Invalid tag\n"));
  5079	
  5080		ccb = &pm8001_ha->ccb_info[tag];
  5081		ccb->completion = comp;
  5082		ccb->resp_buf = buf;
  5083		circularQ = &pm8001_ha->inbnd_q_tbl[0];
  5084	
  5085		payload.tag = cpu_to_le32(tag);
  5086		ppc_phyid = (page_code & 0xFF)  << 8 | (phy_id & 0xFF);
  5087		payload.ppc_phyid = cpu_to_le32(ppc_phyid);
  5088	
  5089		pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &payload,
  5090				sizeof(payload), 0);
  5091		return rc;
  5092	}
  5093	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 75309 bytes --]

Re: [PATCH V2 3/3] pm80xx : Wait for PHY startup before draining libsas queue.

[John Garry]

On 24/06/2020 13:03, Deepak Ukey wrote:


> From: peter chang <dpf@google.com>
> 
> The host's scan finish waits for the libsas queue to drain. However,
> if the PHYs are still in the process of starting then the queue will
> be empty. This means that we declare the scan finished before it has
> even started. Here we wait for various events from the firmware-side.

But we still report the phy up even to libsas later, so should 
revalidate the domain and discover the device.

> 
> The wait behavior can be configured using the module parameter
> "wait_for_phy_startup", if the parameter is enabled, the driver will wait
> for the phy event from the firmware, before proceeding with load.

If this is to circumvent a deficiency in udev (which you mentioned in 
the v1 in this series, but fail to mention here), then better to fix udev.

> 
> Signed-off-by: Viswas G <Viswas.G@microchip.com>

Author != first Signed-off-by

> Signed-off-by: peter chang <dpf@google.com>
> Signed-off-by: Radha Ramachandran <radha@google.com>
> Signed-off-by: Deepak Ukey <Deepak.Ukey@microchip.com>
> ---
>   drivers/scsi/pm8001/pm8001_ctl.c  | 36 +++++++++++++++++++++
>   drivers/scsi/pm8001/pm8001_defs.h |  6 ++--
>   drivers/scsi/pm8001/pm8001_init.c | 22 +++++++++++++
>   drivers/scsi/pm8001/pm8001_sas.c  | 67 +++++++++++++++++++++++++++++++++++++--
>   drivers/scsi/pm8001/pm8001_sas.h  |  4 +++
>   drivers/scsi/pm8001/pm80xx_hwi.c  | 67 ++++++++++++++++++++++++++++++++-------
>   6 files changed, 185 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
> index 3c9f42779dd0..eae629610a5f 100644
> --- a/drivers/scsi/pm8001/pm8001_ctl.c
> +++ b/drivers/scsi/pm8001/pm8001_ctl.c
> @@ -88,6 +88,41 @@ static ssize_t controller_fatal_error_show(struct device *cdev,
>   }
>   static DEVICE_ATTR_RO(controller_fatal_error);
>   
> +/**
> + * phy_startup_timeout_show - per-phy discovery timeout
> + * @cdev: pointer to embedded class device
> + * @buf: the buffer returned
> + *
> + * A sysfs 'read/write' shost attribute.
> + */
> +static ssize_t phy_startup_timeout_show(struct device *cdev,
> +	struct device_attribute *attr, char *buf)
> +{
> +	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;
> +
> +	return snprintf(buf, PAGE_SIZE, "%08xh\n",
> +		pm8001_ha->phy_startup_timeout / HZ);
> +}
> +
> +static ssize_t phy_startup_timeout_store(struct device *cdev,
> +	struct device_attribute *attr, const char *buf, size_t count)
> +{
> +	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;
> +	int val = 0;
> +
> +	if (kstrtoint(buf, 0, &val) < 0)
> +		return -EINVAL;
> +
> +	pm8001_ha->phy_startup_timeout = val * HZ;
> +	return strlen(buf);

I don't see how this can be used.

> +}
> +
> +static DEVICE_ATTR_RW(phy_startup_timeout);
> +

 >
 >   	case HW_EVENT_SAS_PHY_UP:
 > @@ -4975,6 +5015,9 @@ pm80xx_chip_phy_start_req(struct 
pm8001_hba_info *pm8001_ha, u8 phy_id)
 >
 >   	PM8001_INIT_DBG(pm8001_ha,
 >   		pm8001_printk("PHY START REQ for phy_id %d\n", phy_id));
 > +	set_bit(phy_id, &pm8001_ha->phy_mask);
 > +	pm8001_ha->phy[phy_id].start_timeout =
 > +		jiffies + pm8001_ha->phy_startup_timeout;
 >
 >   	payload.ase_sh_lm_slr_phyid = cpu_to_le32(SPINHOLD_ENABLE |
 >   			LINKMODE_AUTO | pm8001_ha->link_rate | phy_id);
 > --

If there is nothing attached to the phy, then it would never come up, 
right? If so, how do you handle this?

Thanks,
john

Re: [PATCH V2 2/3] pm80xx : Staggered spin up support.

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4021 bytes --]

Hi Deepak,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.8-rc2 next-20200624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Deepak-Ukey/pm80xx-Updates-for-the-driver-version-0-1-39/20200624-195528
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1d4c87335d5236ea1f35937e1014980ba961ae34)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/scsi/pm8001/pm80xx_hwi.c:52:7: warning: no previous prototype for function 'pm8001_queue_phyup' [-Wmissing-prototypes]
   void  pm8001_queue_phyup(struct pm8001_hba_info *pm8001_ha, int phy_id)
         ^
   drivers/scsi/pm8001/pm80xx_hwi.c:52:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void  pm8001_queue_phyup(struct pm8001_hba_info *pm8001_ha, int phy_id)
   ^
   static 
   drivers/scsi/pm8001/pm80xx_hwi.c:135:6: warning: no previous prototype for function 'pm80xx_pci_mem_copy' [-Wmissing-prototypes]
   void pm80xx_pci_mem_copy(struct pm8001_hba_info  *pm8001_ha, u32 soffset,
        ^
   drivers/scsi/pm8001/pm80xx_hwi.c:135:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void pm80xx_pci_mem_copy(struct pm8001_hba_info  *pm8001_ha, u32 soffset,
   ^
   static 
   drivers/scsi/pm8001/pm80xx_hwi.c:5157:5: warning: no previous prototype for function 'pm8001_chip_get_phy_profile' [-Wmissing-prototypes]
   int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
       ^
   drivers/scsi/pm8001/pm80xx_hwi.c:5157:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
   ^
   static 
   drivers/scsi/pm8001/pm80xx_hwi.c:5187:6: warning: no previous prototype for function 'mpi_set_phy_profile_req' [-Wmissing-prototypes]
   void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
        ^
   drivers/scsi/pm8001/pm80xx_hwi.c:5187:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void mpi_set_phy_profile_req(struct pm8001_hba_info *pm8001_ha,
   ^
   static 
   4 warnings generated.

vim +/pm8001_queue_phyup +52 drivers/scsi/pm8001/pm80xx_hwi.c

    48	
    49	static int pm80xx_chip_phy_ctl_req(struct pm8001_hba_info *pm8001_ha,
    50		u32 phyId, u32 phy_op);
    51	
  > 52	void  pm8001_queue_phyup(struct pm8001_hba_info *pm8001_ha, int phy_id)
    53	{
    54		int i;
    55	
    56		if (pm8001_ha->phy_head == -1) {
    57			pm8001_ha->phy_head = pm8001_ha->phy_tail = 0;
    58		} else {
    59			/* If the phy id is already queued , discard the phy up */
    60			for (i = 0; i < pm8001_ha->chip->n_phy; i++)
    61				if (pm8001_ha->phy_up[i] == phy_id)
    62					return;
    63			pm8001_ha->phy_tail =
    64				(pm8001_ha->phy_tail + 1) % PM8001_MAX_PHYS;
    65		}
    66		pm8001_ha->phy_up[pm8001_ha->phy_tail] = phy_id;
    67	}
    68	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 75309 bytes --]

[RFC PATCH] pm80xx : pm8001_chip_get_phy_profile() can be static

[kernel test robot]

Signed-off-by: kernel test robot <lkp@intel.com>
---
 pm80xx_hwi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index a909d00c6897a..9b60c42af83e1 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -5061,7 +5061,7 @@ pm80xx_chip_isr(struct pm8001_hba_info *pm8001_ha, u8 vec)
 	return IRQ_HANDLED;
 }
 
-int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
+static int pm8001_chip_get_phy_profile(struct pm8001_hba_info *pm8001_ha, int phy_id,
 		int page_code, struct completion *comp, void *buf)
 {
 	u32 tag;

[RFC PATCH] pm80xx : pm8001_queue_phyup() can be static

[kernel test robot]

Signed-off-by: kernel test robot <lkp@intel.com>
---
 pm80xx_hwi.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 748c7a06262fb..80e992d9ea314 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -49,7 +49,7 @@
 static int pm80xx_chip_phy_ctl_req(struct pm8001_hba_info *pm8001_ha,
 	u32 phyId, u32 phy_op);
 
-void  pm8001_queue_phyup(struct pm8001_hba_info *pm8001_ha, int phy_id)
+static void  pm8001_queue_phyup(struct pm8001_hba_info *pm8001_ha, int phy_id)
 {
 	int i;
 

RE: [PATCH V2 3/3] pm80xx : Wait for PHY startup before draining libsas queue.

[Deepak.Ukey]

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On 24/06/2020 13:03, Deepak Ukey wrote:


> From: peter chang <dpf@google.com>
>
> The host's scan finish waits for the libsas queue to drain. However, 
> if the PHYs are still in the process of starting then the queue will 
> be empty. This means that we declare the scan finished before it has 
> even started. Here we wait for various events from the firmware-side.

But we still report the phy up even to libsas later, so should revalidate the domain and discover the device.
- The scsi_host scan_finished callback polls the phy state in pm8001_update_phy_mask. If the phy fails to come up - - after phy->start_timeout, we declare the scan as finished and proceed.
>
> The wait behavior can be configured using the module parameter 
> "wait_for_phy_startup", if the parameter is enabled, the driver will 
> wait for the phy event from the firmware, before proceeding with load.

If this is to circumvent a deficiency in udev (which you mentioned in the v1 in this series, but fail to mention here), then better to fix udev.
- According to recommendation to fix this using udev. It looks like upstreaming this patch is not possible. We will       - investigate using udev for fixing this issue and submit this patch later in  future.

>
> Signed-off-by: Viswas G <Viswas.G@microchip.com>

Author != first Signed-off-by

> Signed-off-by: peter chang <dpf@google.com>
> Signed-off-by: Radha Ramachandran <radha@google.com>
> Signed-off-by: Deepak Ukey <Deepak.Ukey@microchip.com>
> ---
>   drivers/scsi/pm8001/pm8001_ctl.c  | 36 +++++++++++++++++++++
>   drivers/scsi/pm8001/pm8001_defs.h |  6 ++--
>   drivers/scsi/pm8001/pm8001_init.c | 22 +++++++++++++
>   drivers/scsi/pm8001/pm8001_sas.c  | 67 +++++++++++++++++++++++++++++++++++++--
>   drivers/scsi/pm8001/pm8001_sas.h  |  4 +++
>   drivers/scsi/pm8001/pm80xx_hwi.c  | 67 ++++++++++++++++++++++++++++++++-------
>   6 files changed, 185 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_ctl.c 
> b/drivers/scsi/pm8001/pm8001_ctl.c
> index 3c9f42779dd0..eae629610a5f 100644
> --- a/drivers/scsi/pm8001/pm8001_ctl.c
> +++ b/drivers/scsi/pm8001/pm8001_ctl.c
> @@ -88,6 +88,41 @@ static ssize_t controller_fatal_error_show(struct device *cdev,
>   }
>   static DEVICE_ATTR_RO(controller_fatal_error);
>
> +/**
> + * phy_startup_timeout_show - per-phy discovery timeout
> + * @cdev: pointer to embedded class device
> + * @buf: the buffer returned
> + *
> + * A sysfs 'read/write' shost attribute.
> + */
> +static ssize_t phy_startup_timeout_show(struct device *cdev,
> +     struct device_attribute *attr, char *buf) {
> +     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;
> +
> +     return snprintf(buf, PAGE_SIZE, "%08xh\n",
> +             pm8001_ha->phy_startup_timeout / HZ); }
> +
> +static ssize_t phy_startup_timeout_store(struct device *cdev,
> +     struct device_attribute *attr, const char *buf, size_t count) {
> +     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;
> +     int val = 0;
> +
> +     if (kstrtoint(buf, 0, &val) < 0)
> +             return -EINVAL;
> +
> +     pm8001_ha->phy_startup_timeout = val * HZ;
> +     return strlen(buf);

I don't see how this can be used.

> +}
> +
> +static DEVICE_ATTR_RW(phy_startup_timeout);
> +

 >
 >      case HW_EVENT_SAS_PHY_UP:
 > @@ -4975,6 +5015,9 @@ pm80xx_chip_phy_start_req(struct pm8001_hba_info *pm8001_ha, u8 phy_id)  >
 >      PM8001_INIT_DBG(pm8001_ha,
 >              pm8001_printk("PHY START REQ for phy_id %d\n", phy_id));
 > +    set_bit(phy_id, &pm8001_ha->phy_mask);
 > +    pm8001_ha->phy[phy_id].start_timeout =
 > +            jiffies + pm8001_ha->phy_startup_timeout;
 >
 >      payload.ase_sh_lm_slr_phyid = cpu_to_le32(SPINHOLD_ENABLE |
 >                      LINKMODE_AUTO | pm8001_ha->link_rate | phy_id);
 > --

If there is nothing attached to the phy, then it would never come up, right? If so, how do you handle this?

Thanks,
john