| Message ID | 1575721321-8071-3-git-send-email-stanley.chu@mediatek.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 8b0bbf002a1eb9074ab770e21cd454137c76c106 |
| Headers | show |
| Series | scsi: ufs: fixup active period of ufshcd interrupt | expand |
On 12/7/2019 4:22 AM, Stanley Chu wrote: > Similar to suspend, ufshcd interrupt can be disabled since > there won't be any host controller transaction expected till > clocks ungated. > > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> > --- > drivers/scsi/ufs/ufshcd.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index f80bd4e811cb..5de105e82c32 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -1490,6 +1490,8 @@ static void ufshcd_ungate_work(struct work_struct *work) > spin_unlock_irqrestore(hba->host->host_lock, flags); > ufshcd_setup_clocks(hba, true); > > + ufshcd_enable_irq(hba); > + > /* Exit from hibern8 */ > if (ufshcd_can_hibern8_during_gating(hba)) { > /* Prevent gating in this path */ > @@ -1636,6 +1638,8 @@ static void ufshcd_gate_work(struct work_struct *work) > ufshcd_set_link_hibern8(hba); > } > > + ufshcd_disable_irq(hba); > + > if (!ufshcd_is_link_active(hba)) > ufshcd_setup_clocks(hba, false); > else > Hi, Does this save significant power? I see that gate/ungate of clocks happen far too frequently than suspend/resume. Have you considered how much latency this would add to the gating/ungating path? -asd
Hi Asutosh, On Tue, 2019-12-17 at 15:25 -0800, Asutosh Das (asd) wrote: > > > > Hi, > Does this save significant power? I see that gate/ungate of clocks > happen far too frequently than suspend/resume. > > Have you considered how much latency this would add to the > gating/ungating path? > > -asd > Yes, we have measured 200 times clk-gating/clk-ungating and latency data is showed as below, For clk-gating with interrupt disabling toggled, Average latency of each clk-gating: 55.117 us Average latency of irq-disabling during clk-gating: 4.2 us For clk-ungating with interrupt enabling toggled, Average latency of each clk-ungating: 118.324 us Average latency of irq-enabling during clk-ungating: 2.9 us The evaluation here is based on below Can's patch therefore the interrupt control (enable_irq/disable_irq) latency is much shorter than before (request_irq/free_irq). scsi: ufs: Do not free irq in suspend https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/scsi/ufs/ufshcd.c?id=8709c1f68536e256668812788af5b2bb027f49c3 BTW, the main purpose of this patch is aimed to protect ufshcd register from accessing while host clocks are disabled to fix potential system hang issue. The possible scenario is mentioned in commit message of patch "scsi: ufs: disable irq before disabling clocks" in the same series. Thanks Stanley
On 12/17/2019 7:52 PM, Stanley Chu wrote: > Hi Asutosh, > > On Tue, 2019-12-17 at 15:25 -0800, Asutosh Das (asd) wrote: >>> >> >> Hi, >> Does this save significant power? I see that gate/ungate of clocks >> happen far too frequently than suspend/resume. >> >> Have you considered how much latency this would add to the >> gating/ungating path? >> >> -asd >> > > Yes, we have measured 200 times clk-gating/clk-ungating and latency data > is showed as below, > > For clk-gating with interrupt disabling toggled, > > Average latency of each clk-gating: 55.117 us > Average latency of irq-disabling during clk-gating: 4.2 us > > For clk-ungating with interrupt enabling toggled, > Average latency of each clk-ungating: 118.324 us > Average latency of irq-enabling during clk-ungating: 2.9 us > > The evaluation here is based on below Can's patch therefore the > interrupt control (enable_irq/disable_irq) latency is much shorter than > before (request_irq/free_irq). > > scsi: ufs: Do not free irq in suspend > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/scsi/ufs/ufshcd.c?id=8709c1f68536e256668812788af5b2bb027f49c3 > > BTW, the main purpose of this patch is aimed to protect ufshcd register > from accessing while host clocks are disabled to fix potential system > hang issue. The possible scenario is mentioned in commit message of > patch "scsi: ufs: disable irq before disabling clocks" in the same > series. > > Thanks > Stanley > Reviewed-by: Asutosh Das <asutoshd@codeaurora.org> Thanks for the data. It'd be interesting to know more on the - misrouted interrupt recovery feature though. - Thanks Asutosh
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index f80bd4e811cb..5de105e82c32 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1490,6 +1490,8 @@ static void ufshcd_ungate_work(struct work_struct *work) spin_unlock_irqrestore(hba->host->host_lock, flags); ufshcd_setup_clocks(hba, true); + ufshcd_enable_irq(hba); + /* Exit from hibern8 */ if (ufshcd_can_hibern8_during_gating(hba)) { /* Prevent gating in this path */ @@ -1636,6 +1638,8 @@ static void ufshcd_gate_work(struct work_struct *work) ufshcd_set_link_hibern8(hba); } + ufshcd_disable_irq(hba); + if (!ufshcd_is_link_active(hba)) ufshcd_setup_clocks(hba, false); else
Similar to suspend, ufshcd interrupt can be disabled since there won't be any host controller transaction expected till clocks ungated. Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> --- drivers/scsi/ufs/ufshcd.c | 4 ++++ 1 file changed, 4 insertions(+)