* [PATCH 0/3] pm80xx : Updates for the driver version 0.1.39.
@ 2020-04-13 9:49 Deepak Ukey
2020-04-13 9:49 ` [PATCH 1/3] pm80xx : Support for get phy profile functionality Deepak Ukey
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Deepak Ukey @ 2020-04-13 9:49 UTC (permalink / raw)
To: linux-scsi
Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, deepak.ukey, jinpu.wang,
martin.petersen, dpf, yuuzheng, auradkar, vishakhavc, bjashnani,
radha, akshatzen
From: Deepak Ukey <Deepak.Ukey@microchip.com>
This patch set includes some bug fixes and features for pm80xx driver.
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 | 27 ++++
drivers/scsi/pm8001/pm8001_defs.h | 9 +-
drivers/scsi/pm8001/pm8001_hwi.c | 14 ++-
drivers/scsi/pm8001/pm8001_init.c | 79 +++++++++++-
drivers/scsi/pm8001/pm8001_sas.c | 99 ++++++++++++++-
drivers/scsi/pm8001/pm8001_sas.h | 22 ++++
drivers/scsi/pm8001/pm80xx_hwi.c | 252 +++++++++++++++++++++++++++++++++++---
drivers/scsi/pm8001/pm80xx_hwi.h | 2 +
9 files changed, 509 insertions(+), 31 deletions(-)
--
2.16.3
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/3] pm80xx : Support for get phy profile functionality. 2020-04-13 9:49 [PATCH 0/3] pm80xx : Updates for the driver version 0.1.39 Deepak Ukey @ 2020-04-13 9:49 ` Deepak Ukey 2020-04-16 11:31 ` Jinpu Wang 2020-04-13 9:49 ` [PATCH 2/3] pm80xx : Staggered spin up support Deepak Ukey 2020-04-13 9:49 ` [PATCH 3/3] pm80xx : Wait for PHY startup before draining libsas queue Deepak Ukey 2 siblings, 1 reply; 11+ messages in thread From: Deepak Ukey @ 2020-04-13 9:49 UTC (permalink / raw) To: linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, deepak.ukey, jinpu.wang, martin.petersen, dpf, yuuzheng, auradkar, vishakhavc, bjashnani, radha, akshatzen 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 | 27 +++++++++++++ drivers/scsi/pm8001/pm8001_init.c | 1 + drivers/scsi/pm8001/pm8001_sas.c | 1 + drivers/scsi/pm8001/pm8001_sas.h | 5 +++ drivers/scsi/pm8001/pm80xx_hwi.c | 82 ++++++++++++++++++++++++++++++++++++++- drivers/scsi/pm8001/pm80xx_hwi.h | 2 + 6 files changed, 116 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/pm8001/pm8001_ctl.h b/drivers/scsi/pm8001/pm8001_ctl.h index d0d43a250b9e..5a1cbd54164c 100644 --- a/drivers/scsi/pm8001/pm8001_ctl.h +++ b/drivers/scsi/pm8001/pm8001_ctl.h @@ -41,6 +41,33 @@ #ifndef PM8001_CTL_H_INCLUDED #define PM8001_CTL_H_INCLUDED +struct phy_profile { + 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; +}; + +struct phy_prof_resp { + union { + struct phy_profile status; + struct phy_errcnt errcnt; + } phy; +}; + #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..68005d0cb33d 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -502,6 +502,7 @@ static struct pm8001_hba_info *pm8001_pci_alloc(struct pci_dev *pdev, else pm8001_ha->iomb_size = IOMB_SIZE_SPC; + pm8001_ha->phyprofile_completion = NULL; #ifdef PM8001_USE_TASKLET /* Tasklet for non msi-x interrupt handler */ if ((!pdev->msix_cap || !pci_msi_enabled()) diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index b7cbc312843e..3a69d8d1d52e 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -283,6 +283,7 @@ int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time) * is empirically about all it takes) */ if (time < HZ) 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 ae7ba9b3c4bc..cba4e9b95c58 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 pm8001_chip_info { @@ -561,6 +564,8 @@ struct pm8001_hba_info { u32 reset_in_progress; u32 non_fatal_count; u32 non_fatal_read_length; + struct completion *phyprofile_completion; + struct phy_prof_resp phy_profile_resp; }; struct pm8001_work { diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 4d205ebaee87..0f4f18b0e480 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,59 @@ 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_profile *phy_profile, *phy_prof; + struct phy_errcnt *phy_err, *phy_err_cnt; + 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); + if (pm8001_ha->phyprofile_completion != NULL) { + if (status) { + /* signal fail status */ + memset(&pm8001_ha->phy_profile_resp, 0xff, + sizeof(pm8001_ha->phy_profile_resp)); + } else if (page_code == SAS_PHY_GENERAL_STATUS_PAGE) { + phy_profile = + (struct phy_profile *)&pm8001_ha->phy_profile_resp; + phy_prof = + (struct phy_profile *)pPayload->ppc_specific_rsp; + phy_profile->phy_id = le32_to_cpu(phy_prof->phy_id); + phy_profile->phy_state = + le32_to_cpu(phy_prof->phy_state); + phy_profile->plr = le32_to_cpu(phy_prof->plr); + phy_profile->nlr = le32_to_cpu(phy_prof->nlr); + phy_profile->port_id = le32_to_cpu(phy_prof->port_id); + phy_profile->prts = le32_to_cpu(phy_prof->prts); + } else if (page_code == SAS_PHY_ERR_COUNTERS_PAGE) { + phy_err = + (struct phy_errcnt *)&pm8001_ha->phy_profile_resp; + 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(pm8001_ha->phyprofile_completion); + } + pm8001_tag_free(pm8001_ha, tag); return 0; } @@ -5013,6 +5062,34 @@ 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) +{ + + u32 tag; + struct get_phy_profile_req payload; + struct inbound_queue_table *circularQ; + 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")); + + 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 +5190,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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] pm80xx : Support for get phy profile functionality. 2020-04-13 9:49 ` [PATCH 1/3] pm80xx : Support for get phy profile functionality Deepak Ukey @ 2020-04-16 11:31 ` Jinpu Wang 2020-04-20 9:23 ` Deepak.Ukey 0 siblings, 1 reply; 11+ messages in thread From: Jinpu Wang @ 2020-04-16 11:31 UTC (permalink / raw) To: Deepak Ukey Cc: Linux SCSI Mailinglist, Vasanthalakshmi.Tharmarajan, Viswas G, Jinpu Wang, Martin K. Petersen, dpf, yuuzheng, Vikram Auradkar, vishakhavc, bjashnani, Radha Ramachandran, akshatzen On Mon, Apr 13, 2020 at 11:40 AM Deepak Ukey <deepak.ukey@microchip.com> wrote: > > 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> Hi, I failed to find where do you initialize the phyprofile_completion, did I miss something? Thanks! Jinpu > --- > drivers/scsi/pm8001/pm8001_ctl.h | 27 +++++++++++++ > drivers/scsi/pm8001/pm8001_init.c | 1 + > drivers/scsi/pm8001/pm8001_sas.c | 1 + > drivers/scsi/pm8001/pm8001_sas.h | 5 +++ > drivers/scsi/pm8001/pm80xx_hwi.c | 82 ++++++++++++++++++++++++++++++++++++++- > drivers/scsi/pm8001/pm80xx_hwi.h | 2 + > 6 files changed, 116 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/pm8001/pm8001_ctl.h b/drivers/scsi/pm8001/pm8001_ctl.h > index d0d43a250b9e..5a1cbd54164c 100644 > --- a/drivers/scsi/pm8001/pm8001_ctl.h > +++ b/drivers/scsi/pm8001/pm8001_ctl.h > @@ -41,6 +41,33 @@ > #ifndef PM8001_CTL_H_INCLUDED > #define PM8001_CTL_H_INCLUDED > > +struct phy_profile { > + 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; > +}; > + > +struct phy_prof_resp { > + union { > + struct phy_profile status; > + struct phy_errcnt errcnt; > + } phy; > +}; > + > #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..68005d0cb33d 100644 > --- a/drivers/scsi/pm8001/pm8001_init.c > +++ b/drivers/scsi/pm8001/pm8001_init.c > @@ -502,6 +502,7 @@ static struct pm8001_hba_info *pm8001_pci_alloc(struct pci_dev *pdev, > else > pm8001_ha->iomb_size = IOMB_SIZE_SPC; > > + pm8001_ha->phyprofile_completion = NULL; > #ifdef PM8001_USE_TASKLET > /* Tasklet for non msi-x interrupt handler */ > if ((!pdev->msix_cap || !pci_msi_enabled()) > diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c > index b7cbc312843e..3a69d8d1d52e 100644 > --- a/drivers/scsi/pm8001/pm8001_sas.c > +++ b/drivers/scsi/pm8001/pm8001_sas.c > @@ -283,6 +283,7 @@ int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time) > * is empirically about all it takes) */ > if (time < HZ) > 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 ae7ba9b3c4bc..cba4e9b95c58 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 pm8001_chip_info { > @@ -561,6 +564,8 @@ struct pm8001_hba_info { > u32 reset_in_progress; > u32 non_fatal_count; > u32 non_fatal_read_length; > + struct completion *phyprofile_completion; > + struct phy_prof_resp phy_profile_resp; > }; > > struct pm8001_work { > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c > index 4d205ebaee87..0f4f18b0e480 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,59 @@ 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_profile *phy_profile, *phy_prof; > + struct phy_errcnt *phy_err, *phy_err_cnt; > + 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); > + if (pm8001_ha->phyprofile_completion != NULL) { > + if (status) { > + /* signal fail status */ > + memset(&pm8001_ha->phy_profile_resp, 0xff, > + sizeof(pm8001_ha->phy_profile_resp)); > + } else if (page_code == SAS_PHY_GENERAL_STATUS_PAGE) { > + phy_profile = > + (struct phy_profile *)&pm8001_ha->phy_profile_resp; > + phy_prof = > + (struct phy_profile *)pPayload->ppc_specific_rsp; > + phy_profile->phy_id = le32_to_cpu(phy_prof->phy_id); > + phy_profile->phy_state = > + le32_to_cpu(phy_prof->phy_state); > + phy_profile->plr = le32_to_cpu(phy_prof->plr); > + phy_profile->nlr = le32_to_cpu(phy_prof->nlr); > + phy_profile->port_id = le32_to_cpu(phy_prof->port_id); > + phy_profile->prts = le32_to_cpu(phy_prof->prts); > + } else if (page_code == SAS_PHY_ERR_COUNTERS_PAGE) { > + phy_err = > + (struct phy_errcnt *)&pm8001_ha->phy_profile_resp; > + 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(pm8001_ha->phyprofile_completion); > + } > + pm8001_tag_free(pm8001_ha, tag); > return 0; > } > > @@ -5013,6 +5062,34 @@ 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) > +{ > + > + u32 tag; > + struct get_phy_profile_req payload; > + struct inbound_queue_table *circularQ; > + 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")); > + > + 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 +5190,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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/3] pm80xx : Support for get phy profile functionality. 2020-04-16 11:31 ` Jinpu Wang @ 2020-04-20 9:23 ` Deepak.Ukey 0 siblings, 0 replies; 11+ messages in thread From: Deepak.Ukey @ 2020-04-20 9:23 UTC (permalink / raw) To: jinpu.wang Cc: linux-scsi, Vasanthalakshmi.Tharmarajan, Viswas.G, jinpu.wang, martin.petersen, yuuzheng, auradkar, vishakhavc, bjashnani, radha, akshatzen Hi, I failed to find where do you initialize the phyprofile_completion, did I miss something? [DU] We will initalize phyprofile_completion in this patch itself. Thanks, Deepak > 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> Hi, I failed to find where do you initialize the phyprofile_completion, did I miss something? Thanks! Jinpu > --- > drivers/scsi/pm8001/pm8001_ctl.h | 27 +++++++++++++ > drivers/scsi/pm8001/pm8001_init.c | 1 + > drivers/scsi/pm8001/pm8001_sas.c | 1 + > drivers/scsi/pm8001/pm8001_sas.h | 5 +++ > drivers/scsi/pm8001/pm80xx_hwi.c | 82 > ++++++++++++++++++++++++++++++++++++++- > drivers/scsi/pm8001/pm80xx_hwi.h | 2 + > 6 files changed, 116 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/pm8001/pm8001_ctl.h > b/drivers/scsi/pm8001/pm8001_ctl.h > index d0d43a250b9e..5a1cbd54164c 100644 > --- a/drivers/scsi/pm8001/pm8001_ctl.h > +++ b/drivers/scsi/pm8001/pm8001_ctl.h > @@ -41,6 +41,33 @@ > #ifndef PM8001_CTL_H_INCLUDED > #define PM8001_CTL_H_INCLUDED > > +struct phy_profile { > + 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; > +}; > + > +struct phy_prof_resp { > + union { > + struct phy_profile status; > + struct phy_errcnt errcnt; > + } phy; > +}; > + > #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..68005d0cb33d 100644 > --- a/drivers/scsi/pm8001/pm8001_init.c > +++ b/drivers/scsi/pm8001/pm8001_init.c > @@ -502,6 +502,7 @@ static struct pm8001_hba_info *pm8001_pci_alloc(struct pci_dev *pdev, > else > pm8001_ha->iomb_size = IOMB_SIZE_SPC; > > + pm8001_ha->phyprofile_completion = NULL; > #ifdef PM8001_USE_TASKLET > /* Tasklet for non msi-x interrupt handler */ > if ((!pdev->msix_cap || !pci_msi_enabled()) diff --git > a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c > index b7cbc312843e..3a69d8d1d52e 100644 > --- a/drivers/scsi/pm8001/pm8001_sas.c > +++ b/drivers/scsi/pm8001/pm8001_sas.c > @@ -283,6 +283,7 @@ int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time) > * is empirically about all it takes) */ > if (time < HZ) > 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 ae7ba9b3c4bc..cba4e9b95c58 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 pm8001_chip_info { > @@ -561,6 +564,8 @@ struct pm8001_hba_info { > u32 reset_in_progress; > u32 non_fatal_count; > u32 non_fatal_read_length; > + struct completion *phyprofile_completion; > + struct phy_prof_resp phy_profile_resp; > }; > > struct pm8001_work { > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c > b/drivers/scsi/pm8001/pm80xx_hwi.c > index 4d205ebaee87..0f4f18b0e480 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,59 @@ 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_profile *phy_profile, *phy_prof; > + struct phy_errcnt *phy_err, *phy_err_cnt; > + 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); > + if (pm8001_ha->phyprofile_completion != NULL) { > + if (status) { > + /* signal fail status */ > + memset(&pm8001_ha->phy_profile_resp, 0xff, > + sizeof(pm8001_ha->phy_profile_resp)); > + } else if (page_code == SAS_PHY_GENERAL_STATUS_PAGE) { > + phy_profile = > + (struct phy_profile *)&pm8001_ha->phy_profile_resp; > + phy_prof = > + (struct phy_profile *)pPayload->ppc_specific_rsp; > + phy_profile->phy_id = le32_to_cpu(phy_prof->phy_id); > + phy_profile->phy_state = > + le32_to_cpu(phy_prof->phy_state); > + phy_profile->plr = le32_to_cpu(phy_prof->plr); > + phy_profile->nlr = le32_to_cpu(phy_prof->nlr); > + phy_profile->port_id = le32_to_cpu(phy_prof->port_id); > + phy_profile->prts = le32_to_cpu(phy_prof->prts); > + } else if (page_code == SAS_PHY_ERR_COUNTERS_PAGE) { > + phy_err = > + (struct phy_errcnt *)&pm8001_ha->phy_profile_resp; > + 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(pm8001_ha->phyprofile_completion); > + } > + pm8001_tag_free(pm8001_ha, tag); > return 0; > } > > @@ -5013,6 +5062,34 @@ 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) { > + > + u32 tag; > + struct get_phy_profile_req payload; > + struct inbound_queue_table *circularQ; > + 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")); > + > + 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 > +5190,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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] pm80xx : Staggered spin up support. 2020-04-13 9:49 [PATCH 0/3] pm80xx : Updates for the driver version 0.1.39 Deepak Ukey 2020-04-13 9:49 ` [PATCH 1/3] pm80xx : Support for get phy profile functionality Deepak Ukey @ 2020-04-13 9:49 ` Deepak Ukey 2020-04-16 11:42 ` Jinpu Wang 2020-04-13 9:49 ` [PATCH 3/3] pm80xx : Wait for PHY startup before draining libsas queue Deepak Ukey 2 siblings, 1 reply; 11+ messages in thread From: Deepak Ukey @ 2020-04-13 9:49 UTC (permalink / raw) To: linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, deepak.ukey, jinpu.wang, martin.petersen, dpf, yuuzheng, auradkar, vishakhavc, bjashnani, radha, akshatzen 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 | 37 ++++++++++- drivers/scsi/pm8001/pm8001_sas.h | 14 +++++ drivers/scsi/pm8001/pm80xx_hwi.c | 125 +++++++++++++++++++++++++++++++++----- 6 files changed, 223 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 68005d0cb33d..6cbb8fa74456 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, @@ -486,6 +492,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); @@ -620,7 +627,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]; @@ -706,6 +714,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)); } /* @@ -1104,7 +1151,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 3a69d8d1d52e..806845203602 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -267,12 +267,38 @@ void pm8001_scan_start(struct Scsi_Host *shost) int i; 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, + pm8001_ha->lock_flags); + pm8001_ha->phy_started = i; + pm8001_ha->scan_completion = ∁ + pm8001_ha->phystart_timedout = 1; + spin_unlock_irqrestore(&pm8001_ha->lock, + pm8001_ha->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, pm8001_ha->lock_flags); + pm8001_ha->phy_started = -1; + pm8001_ha->scan_completion = NULL; + spin_unlock_irqrestore(&pm8001_ha->lock, pm8001_ha->lock_flags); + } } int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time) @@ -663,6 +689,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 cba4e9b95c58..df02c6cd116a 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; }; @@ -503,6 +504,7 @@ struct pm8001_hba_info { unsigned long flags; spinlock_t lock;/* host-wide lock */ spinlock_t bitmap_lock; + unsigned long lock_flags; struct pci_dev *pdev;/* our device */ struct device *dev; struct pm8001_hba_memspace io_mem[6]; @@ -566,6 +568,17 @@ struct pm8001_hba_info { u32 non_fatal_read_length; struct completion *phyprofile_completion; struct phy_prof_resp phy_profile_resp; + 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 +697,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 0f4f18b0e480..0040bb4e1b71 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, @@ -4889,7 +4977,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 */ /** @@ -4951,6 +5039,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)); @@ -4982,8 +5072,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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] pm80xx : Staggered spin up support. 2020-04-13 9:49 ` [PATCH 2/3] pm80xx : Staggered spin up support Deepak Ukey @ 2020-04-16 11:42 ` Jinpu Wang 2020-04-20 9:21 ` Deepak.Ukey 0 siblings, 1 reply; 11+ messages in thread From: Jinpu Wang @ 2020-04-16 11:42 UTC (permalink / raw) To: Deepak Ukey Cc: Linux SCSI Mailinglist, Vasanthalakshmi.Tharmarajan, Viswas G, Jinpu Wang, Martin K. Petersen, dpf, yuuzheng, Vikram Auradkar, vishakhavc, bjashnani, Radha Ramachandran, akshatzen On Mon, Apr 13, 2020 at 11:40 AM Deepak Ukey <deepak.ukey@microchip.com> wrote: > > 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 | 37 ++++++++++- > drivers/scsi/pm8001/pm8001_sas.h | 14 +++++ > drivers/scsi/pm8001/pm80xx_hwi.c | 125 +++++++++++++++++++++++++++++++++----- > 6 files changed, 223 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 68005d0cb33d..6cbb8fa74456 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, > @@ -486,6 +492,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); > @@ -620,7 +627,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]; > @@ -706,6 +714,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)); > } > > /* > @@ -1104,7 +1151,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 3a69d8d1d52e..806845203602 100644 > --- a/drivers/scsi/pm8001/pm8001_sas.c > +++ b/drivers/scsi/pm8001/pm8001_sas.c > @@ -267,12 +267,38 @@ void pm8001_scan_start(struct Scsi_Host *shost) > int i; > 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, > + pm8001_ha->lock_flags); > + pm8001_ha->phy_started = i; > + pm8001_ha->scan_completion = ∁ > + pm8001_ha->phystart_timedout = 1; > + spin_unlock_irqrestore(&pm8001_ha->lock, > + pm8001_ha->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, pm8001_ha->lock_flags); > + pm8001_ha->phy_started = -1; > + pm8001_ha->scan_completion = NULL; > + spin_unlock_irqrestore(&pm8001_ha->lock, pm8001_ha->lock_flags); > + } > } > > int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time) > @@ -663,6 +689,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 cba4e9b95c58..df02c6cd116a 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; > }; > > @@ -503,6 +504,7 @@ struct pm8001_hba_info { > unsigned long flags; > spinlock_t lock;/* host-wide lock */ > spinlock_t bitmap_lock; > + unsigned long lock_flags; This seems fine, only one question: why do you need this lock_flags in hba? > struct pci_dev *pdev;/* our device */ > struct device *dev; > struct pm8001_hba_memspace io_mem[6]; > @@ -566,6 +568,17 @@ struct pm8001_hba_info { > u32 non_fatal_read_length; > struct completion *phyprofile_completion; > struct phy_prof_resp phy_profile_resp; > + 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 +697,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 0f4f18b0e480..0040bb4e1b71 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, > @@ -4889,7 +4977,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 */ > /** > @@ -4951,6 +5039,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)); > @@ -4982,8 +5072,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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 2/3] pm80xx : Staggered spin up support. 2020-04-16 11:42 ` Jinpu Wang @ 2020-04-20 9:21 ` Deepak.Ukey 0 siblings, 0 replies; 11+ messages in thread From: Deepak.Ukey @ 2020-04-20 9:21 UTC (permalink / raw) To: jinpu.wang Cc: linux-scsi, Vasanthalakshmi.Tharmarajan, Viswas.G, jinpu.wang, martin.petersen, dpf, yuuzheng, auradkar, vishakhavc, bjashnani, radha, akshatzen > 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 | 37 ++++++++++- > drivers/scsi/pm8001/pm8001_sas.h | 14 +++++ > drivers/scsi/pm8001/pm80xx_hwi.c | 125 > +++++++++++++++++++++++++++++++++----- > 6 files changed, 223 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 68005d0cb33d..6cbb8fa74456 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, > @@ -486,6 +492,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); @@ -620,7 > +627,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]; > @@ -706,6 +714,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)); > } > > /* > @@ -1104,7 +1151,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 3a69d8d1d52e..806845203602 100644 > --- a/drivers/scsi/pm8001/pm8001_sas.c > +++ b/drivers/scsi/pm8001/pm8001_sas.c > @@ -267,12 +267,38 @@ void pm8001_scan_start(struct Scsi_Host *shost) > int i; > 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, > + pm8001_ha->lock_flags); > + pm8001_ha->phy_started = i; > + pm8001_ha->scan_completion = ∁ > + pm8001_ha->phystart_timedout = 1; > + spin_unlock_irqrestore(&pm8001_ha->lock, > + pm8001_ha->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, pm8001_ha->lock_flags); > + pm8001_ha->phy_started = -1; > + pm8001_ha->scan_completion = NULL; > + spin_unlock_irqrestore(&pm8001_ha->lock, pm8001_ha->lock_flags); > + } > } > > int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time) > @@ -663,6 +689,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 cba4e9b95c58..df02c6cd116a 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; > }; > > @@ -503,6 +504,7 @@ struct pm8001_hba_info { > unsigned long flags; > spinlock_t lock;/* host-wide lock */ > spinlock_t bitmap_lock; > + unsigned long lock_flags; This seems fine, only one question: why do you need this lock_flags in hba? [DU] Thanks for comment. I will remove the lok_flags from hba structure and add it locally. > struct pci_dev *pdev;/* our device */ > struct device *dev; > struct pm8001_hba_memspace io_mem[6]; @@ -566,6 +568,17 @@ > struct pm8001_hba_info { > u32 non_fatal_read_length; > struct completion *phyprofile_completion; > struct phy_prof_resp phy_profile_resp; > + 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 +697,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 0f4f18b0e480..0040bb4e1b71 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, @@ -4889,7 +4977,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 */ > /** > @@ -4951,6 +5039,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)); @@ -4982,8 +5072,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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] pm80xx : Wait for PHY startup before draining libsas queue. 2020-04-13 9:49 [PATCH 0/3] pm80xx : Updates for the driver version 0.1.39 Deepak Ukey 2020-04-13 9:49 ` [PATCH 1/3] pm80xx : Support for get phy profile functionality Deepak Ukey 2020-04-13 9:49 ` [PATCH 2/3] pm80xx : Staggered spin up support Deepak Ukey @ 2020-04-13 9:49 ` Deepak Ukey 2020-04-16 11:57 ` Jinpu Wang 2 siblings, 1 reply; 11+ messages in thread From: Deepak Ukey @ 2020-04-13 9:49 UTC (permalink / raw) To: linux-scsi Cc: Vasanthalakshmi.Tharmarajan, Viswas.G, deepak.ukey, jinpu.wang, martin.petersen, dpf, yuuzheng, auradkar, vishakhavc, bjashnani, radha, akshatzen From: peter chang <dpf@google.com> Until udev rolls out we can't proceed past module loading w/ device discovery in progress because things like the init scripts only work over the currently discovered block devices. The drivers for disk controllers have various forms of 'barriers' to prevent this from happening depending on their underlying support libraries. 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, and even though we disable staggered spinup we still pretend like it's there. 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> Signed-off-by: peter chang <dpf@google.com> --- drivers/scsi/pm8001/pm8001_ctl.c | 36 +++++++++++++++++++++ drivers/scsi/pm8001/pm8001_defs.h | 6 ++-- drivers/scsi/pm8001/pm8001_init.c | 25 +++++++++++++++ drivers/scsi/pm8001/pm8001_sas.c | 61 +++++++++++++++++++++++++++++++++-- drivers/scsi/pm8001/pm8001_sas.h | 3 ++ drivers/scsi/pm8001/pm80xx_hwi.c | 67 ++++++++++++++++++++++++++++++++------- 6 files changed, 181 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 6cbb8fa74456..560dd9c3f745 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -61,6 +61,30 @@ MODULE_PARM_DESC(staggered_spinup, "enable the staggered spinup feature.\n" " 0/N: false\n" " 1/Y: true\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. + * + * the timeout here was experimentally determined by looking at our + * current worst-case spin-up drive (seagate 8T) which has + * the most drive-to-drive variance, some issues coming up from the + * sleep state (randomly applied ~10s delay to non-data operations), + * and errors from IDENTIFY. + * + * NB: this a workaround to handle current lack of udev. once + * that's everywhere and dynamically dealing w/ device add/remove + * (step one doesn't deal w/ this later condition) then the patches + * can be removed. + */ +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; /** @@ -493,6 +517,7 @@ 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->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 806845203602..470fe4dd3b52 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,54 @@ 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) +{ + struct phy_profile *profile = &pm8001_ha->phy_profile_resp.phy.status; + DECLARE_COMPLETION_ONSTACK(comp); + unsigned long timeout = msecs_to_jiffies(2000); + int ret = 0; + int i; + + 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) { + pm8001_ha->phyprofile_completion = ∁ + ret = PM8001_CHIP_DISP->get_phy_profile_req(pm8001_ha, + i, SAS_PHY_GENERAL_STATUS_PAGE); + 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, + profile->phy_state, + profile->nlr, profile->plr)); + switch (profile->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. @@ -273,6 +322,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)) { @@ -304,11 +355,15 @@ 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_update_phy_mask(pm8001_ha); return 0; + } /* Wait for discovery to finish */ sas_drain_work(ha); diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h index df02c6cd116a..bb4f099a1723 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 */ @@ -579,6 +580,8 @@ struct pm8001_hba_info { int phy_head; int phy_tail; spinlock_t phy_q_lock; + unsigned long phy_mask; + u32 phy_startup_timeout; }; struct pm8001_work { diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 0040bb4e1b71..190a7351c1e8 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: @@ -4976,6 +5016,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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] pm80xx : Wait for PHY startup before draining libsas queue. 2020-04-13 9:49 ` [PATCH 3/3] pm80xx : Wait for PHY startup before draining libsas queue Deepak Ukey @ 2020-04-16 11:57 ` Jinpu Wang 2020-05-12 5:32 ` Deepak.Ukey 0 siblings, 1 reply; 11+ messages in thread From: Jinpu Wang @ 2020-04-16 11:57 UTC (permalink / raw) To: Deepak Ukey Cc: Linux SCSI Mailinglist, Vasanthalakshmi.Tharmarajan, Viswas G, Jinpu Wang, Martin K. Petersen, dpf, yuuzheng, Vikram Auradkar, vishakhavc, bjashnani, Radha Ramachandran, akshatzen On Mon, Apr 13, 2020 at 11:40 AM Deepak Ukey <deepak.ukey@microchip.com> wrote: > > From: peter chang <dpf@google.com> > > Until udev rolls out we can't proceed past module loading w/ device > discovery in progress because things like the init scripts only work > over the currently discovered block devices. Just curious, what's this udev rollout? The drivers for disk > controllers have various forms of 'barriers' to prevent this from > happening depending on their underlying support libraries. > 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, > and even though we disable staggered spinup we still pretend like > it's there. > > 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> > Signed-off-by: peter chang <dpf@google.com> > --- > drivers/scsi/pm8001/pm8001_ctl.c | 36 +++++++++++++++++++++ > drivers/scsi/pm8001/pm8001_defs.h | 6 ++-- > drivers/scsi/pm8001/pm8001_init.c | 25 +++++++++++++++ > drivers/scsi/pm8001/pm8001_sas.c | 61 +++++++++++++++++++++++++++++++++-- > drivers/scsi/pm8001/pm8001_sas.h | 3 ++ > drivers/scsi/pm8001/pm80xx_hwi.c | 67 ++++++++++++++++++++++++++++++++------- > 6 files changed, 181 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 6cbb8fa74456..560dd9c3f745 100644 > --- a/drivers/scsi/pm8001/pm8001_init.c > +++ b/drivers/scsi/pm8001/pm8001_init.c > @@ -61,6 +61,30 @@ MODULE_PARM_DESC(staggered_spinup, "enable the staggered spinup feature.\n" > " 0/N: false\n" > " 1/Y: true\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. > + * > + * the timeout here was experimentally determined by looking at our > + * current worst-case spin-up drive (seagate 8T) which has > + * the most drive-to-drive variance, some issues coming up from the > + * sleep state (randomly applied ~10s delay to non-data operations), > + * and errors from IDENTIFY. > + * > + * NB: this a workaround to handle current lack of udev. once > + * that's everywhere and dynamically dealing w/ device add/remove > + * (step one doesn't deal w/ this later condition) then the patches > + * can be removed. If it's just a workaround for missing proper udev rule, I think we shouldnt' include it in upstream. > + */ > +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; > > /** > @@ -493,6 +517,7 @@ 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->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 806845203602..470fe4dd3b52 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,54 @@ 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) > +{ > + struct phy_profile *profile = &pm8001_ha->phy_profile_resp.phy.status; > + DECLARE_COMPLETION_ONSTACK(comp); > + unsigned long timeout = msecs_to_jiffies(2000); > + int ret = 0; > + int i; > + > + 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) { > + pm8001_ha->phyprofile_completion = ∁ Ok, you have phyprofile_completion init here, I think it should be moved to first patch. Thanks! ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 3/3] pm80xx : Wait for PHY startup before draining libsas queue. 2020-04-16 11:57 ` Jinpu Wang @ 2020-05-12 5:32 ` Deepak.Ukey 2020-05-12 6:32 ` Jinpu Wang 0 siblings, 1 reply; 11+ messages in thread From: Deepak.Ukey @ 2020-05-12 5:32 UTC (permalink / raw) To: jinpu.wang Cc: linux-scsi, Vasanthalakshmi.Tharmarajan, Viswas.G, jinpu.wang, martin.petersen, dpf, yuuzheng, auradkar, vishakhavc, bjashnani, radha, akshatzen On Mon, Apr 13, 2020 at 11:40 AM Deepak Ukey <deepak.ukey@microchip.com> wrote: > > From: peter chang <dpf@google.com> > > Until udev rolls out we can't proceed past module loading w/ device > discovery in progress because things like the init scripts only work > over the currently discovered block devices. Just curious, what's this udev rollout? The drivers for disk > controllers have various forms of 'barriers' to prevent this from > happening depending on their underlying support libraries. > 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, > and even though we disable staggered spinup we still pretend like it's > there. > > 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> > Signed-off-by: peter chang <dpf@google.com> > --- > drivers/scsi/pm8001/pm8001_ctl.c | 36 +++++++++++++++++++++ > drivers/scsi/pm8001/pm8001_defs.h | 6 ++-- > drivers/scsi/pm8001/pm8001_init.c | 25 +++++++++++++++ > drivers/scsi/pm8001/pm8001_sas.c | 61 > +++++++++++++++++++++++++++++++++-- > drivers/scsi/pm8001/pm8001_sas.h | 3 ++ > drivers/scsi/pm8001/pm80xx_hwi.c | 67 > ++++++++++++++++++++++++++++++++------- > 6 files changed, 181 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 6cbb8fa74456..560dd9c3f745 100644 > --- a/drivers/scsi/pm8001/pm8001_init.c > +++ b/drivers/scsi/pm8001/pm8001_init.c > @@ -61,6 +61,30 @@ MODULE_PARM_DESC(staggered_spinup, "enable the staggered spinup feature.\n" > " 0/N: false\n" > " 1/Y: true\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. > + * > + * the timeout here was experimentally determined by looking at our > + * current worst-case spin-up drive (seagate 8T) which has > + * the most drive-to-drive variance, some issues coming up from the > + * sleep state (randomly applied ~10s delay to non-data operations), > + * and errors from IDENTIFY. > + * > + * NB: this a workaround to handle current lack of udev. once > + * that's everywhere and dynamically dealing w/ device add/remove > + * (step one doesn't deal w/ this later condition) then the patches > + * can be removed. If it's just a workaround for missing proper udev rule, I think we shouldnt' include it in upstream. Thank you for your suggestion, we are looking into the possibility of handling this behavior with udev. Since some drives take longer to respond back to that phy start request, the following wait does nothing, since the sas workqueue is empty. /* Wait for discovery to finish */ sas_drain_work(ha); The "wait" behavior could be exposed as a module parameter, if the parameter is enabled, the driver will wait for the phy up event from the firmware, before proceeding with load. Would this be acceptable as an alternative solution? > + */ > +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; > > /** > @@ -493,6 +517,7 @@ 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->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 806845203602..470fe4dd3b52 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,54 @@ 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) > +{ > + struct phy_profile *profile = &pm8001_ha->phy_profile_resp.phy.status; > + DECLARE_COMPLETION_ONSTACK(comp); > + unsigned long timeout = msecs_to_jiffies(2000); > + int ret = 0; > + int i; > + > + 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) { > + pm8001_ha->phyprofile_completion = ∁ Ok, you have phyprofile_completion init here, I think it should be moved to first patch. Thanks! ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] pm80xx : Wait for PHY startup before draining libsas queue. 2020-05-12 5:32 ` Deepak.Ukey @ 2020-05-12 6:32 ` Jinpu Wang 0 siblings, 0 replies; 11+ messages in thread From: Jinpu Wang @ 2020-05-12 6:32 UTC (permalink / raw) To: Deepak Ukey Cc: Linux SCSI Mailinglist, Vasanthalakshmi.Tharmarajan, Viswas G, Jinpu Wang, Martin K. Petersen, dpf, yuuzheng, Vikram Auradkar, vishakhavc, bjashnani, Radha Ramachandran, akshatzen On Tue, May 12, 2020 at 7:32 AM <Deepak.Ukey@microchip.com> wrote: > > On Mon, Apr 13, 2020 at 11:40 AM Deepak Ukey <deepak.ukey@microchip.com> wrote: > > > > From: peter chang <dpf@google.com> > > > > Until udev rolls out we can't proceed past module loading w/ device > > discovery in progress because things like the init scripts only work > > over the currently discovered block devices. > Just curious, what's this udev rollout? > > The drivers for disk > > controllers have various forms of 'barriers' to prevent this from > > happening depending on their underlying support libraries. > > 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, > > and even though we disable staggered spinup we still pretend like it's > > there. > > > > 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> > > Signed-off-by: peter chang <dpf@google.com> > > --- > > drivers/scsi/pm8001/pm8001_ctl.c | 36 +++++++++++++++++++++ > > drivers/scsi/pm8001/pm8001_defs.h | 6 ++-- > > drivers/scsi/pm8001/pm8001_init.c | 25 +++++++++++++++ > > drivers/scsi/pm8001/pm8001_sas.c | 61 > > +++++++++++++++++++++++++++++++++-- > > drivers/scsi/pm8001/pm8001_sas.h | 3 ++ > > drivers/scsi/pm8001/pm80xx_hwi.c | 67 > > ++++++++++++++++++++++++++++++++------- > > 6 files changed, 181 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 6cbb8fa74456..560dd9c3f745 100644 > > --- a/drivers/scsi/pm8001/pm8001_init.c > > +++ b/drivers/scsi/pm8001/pm8001_init.c > > @@ -61,6 +61,30 @@ MODULE_PARM_DESC(staggered_spinup, "enable the staggered spinup feature.\n" > > " 0/N: false\n" > > " 1/Y: true\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. > > + * > > + * the timeout here was experimentally determined by looking at our > > + * current worst-case spin-up drive (seagate 8T) which has > > + * the most drive-to-drive variance, some issues coming up from the > > + * sleep state (randomly applied ~10s delay to non-data operations), > > + * and errors from IDENTIFY. > > + * > > + * NB: this a workaround to handle current lack of udev. once > > + * that's everywhere and dynamically dealing w/ device add/remove > > + * (step one doesn't deal w/ this later condition) then the patches > > + * can be removed. > > If it's just a workaround for missing proper udev rule, I think we shouldnt' include it in upstream. > > Thank you for your suggestion, we are looking into the possibility of handling this behavior with udev. > Since some drives take longer to respond back to that phy start request, the following wait does nothing, since the sas workqueue is empty. > /* Wait for discovery to finish */ > sas_drain_work(ha); > The "wait" behavior could be exposed as a module parameter, if the parameter is enabled, the driver will wait for the phy up event from the firmware, before proceeding with load. Would this be acceptable as an alternative solution? Sounds fine to me! Thanks ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-12 6:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-13 9:49 [PATCH 0/3] pm80xx : Updates for the driver version 0.1.39 Deepak Ukey 2020-04-13 9:49 ` [PATCH 1/3] pm80xx : Support for get phy profile functionality Deepak Ukey 2020-04-16 11:31 ` Jinpu Wang 2020-04-20 9:23 ` Deepak.Ukey 2020-04-13 9:49 ` [PATCH 2/3] pm80xx : Staggered spin up support Deepak Ukey 2020-04-16 11:42 ` Jinpu Wang 2020-04-20 9:21 ` Deepak.Ukey 2020-04-13 9:49 ` [PATCH 3/3] pm80xx : Wait for PHY startup before draining libsas queue Deepak Ukey 2020-04-16 11:57 ` Jinpu Wang 2020-05-12 5:32 ` Deepak.Ukey 2020-05-12 6:32 ` Jinpu Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.