| Message ID | 1637117108-230103-14-git-send-email-chenxiang66@hisilicon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add runtime PM support for libsas | expand |
On 17/11/2021 02:45, chenxiang wrote: > From: Xiang Chen<chenxiang66@hisilicon.com> > > It is possible that controller may be suspended between ISR of phyup > and the event being processed, then it can't ensure controller is > active when processing the phyup event which will cause issues. > To avoid the issue, add pm_runtime_get_noresume() in ISR of phyup > and pm_runtime_put_sync() in the work handler exit of a new event > HISI_PHYE_PHY_UP_PM which is called in v3 hw as runtime PM is > only supported for v3 hw. Please consider this rewrite: It is possible that the controller may become suspended between processing a phyup interrupt and the event being processed by libsas. As such, we can't ensure that the controller is active when processing the phyup event - this may cause the phyup event to be lost or other issues. To avoid any possible issues, add pm_runtime_get_noresume() in the phyup interrupt handler and pm_runtime_put_sync() in the work handler exit to ensure that we stay always active. Since we only want pm_runtime_get_noresume() called for v3 hw, signal this will a new event, HISI_PHYE_PHY_UP_PM. Thanks, John
在 2021/12/13 18:44, John Garry 写道: > On 17/11/2021 02:45, chenxiang wrote: >> From: Xiang Chen<chenxiang66@hisilicon.com> >> >> It is possible that controller may be suspended between ISR of phyup >> and the event being processed, then it can't ensure controller is >> active when processing the phyup event which will cause issues. >> To avoid the issue, add pm_runtime_get_noresume() in ISR of phyup >> and pm_runtime_put_sync() in the work handler exit of a new event >> HISI_PHYE_PHY_UP_PM which is called in v3 hw as runtime PM is >> only supported for v3 hw. > > Please consider this rewrite: > > It is possible that the controller may become suspended between > processing a phyup interrupt and the event being processed by libsas. > As such, we can't ensure that the controller is active when processing > the phyup event - this may cause the phyup event to be lost or other > issues. > To avoid any possible issues, add pm_runtime_get_noresume() in the > phyup interrupt handler and pm_runtime_put_sync() in the work handler > exit to ensure that we stay always active. Since we only want > pm_runtime_get_noresume() called for v3 hw, signal this will a new > event, HISI_PHYE_PHY_UP_PM. Ok, i will rewrite it in next version. > > Thanks, > John > > . >
diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h index 2213a91923a5..8269703b978d 100644 --- a/drivers/scsi/hisi_sas/hisi_sas.h +++ b/drivers/scsi/hisi_sas/hisi_sas.h @@ -154,6 +154,7 @@ enum hisi_sas_bit_err_type { enum hisi_sas_phy_event { HISI_PHYE_PHY_UP = 0U, HISI_PHYE_LINK_RESET, + HISI_PHYE_PHY_UP_PM, HISI_PHYES_NUM, }; diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index 44c888e0afd7..9255790a8568 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -854,10 +854,11 @@ int hisi_sas_scan_finished(struct Scsi_Host *shost, unsigned long time) } EXPORT_SYMBOL_GPL(hisi_sas_scan_finished); -static void hisi_sas_phyup_work(struct work_struct *work) +static void hisi_sas_phyup_work_common(struct work_struct *work, + enum hisi_sas_phy_event event) { struct hisi_sas_phy *phy = - container_of(work, typeof(*phy), works[HISI_PHYE_PHY_UP]); + container_of(work, typeof(*phy), works[event]); struct hisi_hba *hisi_hba = phy->hisi_hba; struct asd_sas_phy *sas_phy = &phy->sas_phy; int phy_no = sas_phy->id; @@ -868,6 +869,11 @@ static void hisi_sas_phyup_work(struct work_struct *work) hisi_sas_bytes_dmaed(hisi_hba, phy_no, GFP_KERNEL); } +static void hisi_sas_phyup_work(struct work_struct *work) +{ + hisi_sas_phyup_work_common(work, HISI_PHYE_PHY_UP); +} + static void hisi_sas_linkreset_work(struct work_struct *work) { struct hisi_sas_phy *phy = @@ -877,9 +883,21 @@ static void hisi_sas_linkreset_work(struct work_struct *work) hisi_sas_control_phy(sas_phy, PHY_FUNC_LINK_RESET, NULL); } +static void hisi_sas_phyup_pm_work(struct work_struct *work) +{ + struct hisi_sas_phy *phy = + container_of(work, typeof(*phy), works[HISI_PHYE_PHY_UP_PM]); + struct hisi_hba *hisi_hba = phy->hisi_hba; + struct device *dev = hisi_hba->dev; + + hisi_sas_phyup_work_common(work, HISI_PHYE_PHY_UP_PM); + pm_runtime_put_sync(dev); +} + static const work_func_t hisi_sas_phye_fns[HISI_PHYES_NUM] = { [HISI_PHYE_PHY_UP] = hisi_sas_phyup_work, [HISI_PHYE_LINK_RESET] = hisi_sas_linkreset_work, + [HISI_PHYE_PHY_UP_PM] = hisi_sas_phyup_pm_work, }; bool hisi_sas_notify_phy_event(struct hisi_sas_phy *phy, diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c index 4e9dfdb69757..a5695ae8b73b 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c @@ -1561,7 +1561,10 @@ static irqreturn_t phy_up_v3_hw(int phy_no, struct hisi_hba *hisi_hba) phy->port_id = port_id; phy->phy_attached = 1; - hisi_sas_notify_phy_event(phy, HISI_PHYE_PHY_UP); + + /* Call pm_runtime_put_sync() with pairs in hisi_sas_phyup_pm_work() */ + pm_runtime_get_noresume(dev); + hisi_sas_notify_phy_event(phy, HISI_PHYE_PHY_UP_PM); res = IRQ_HANDLED; end: if (phy->reset_completion)