* [PATCH v2 3/5] scsi: ufs: qcom: Add a readl() to make sure ref_clk gets enabled [not found] <20220423140245.394092-1-manivannan.sadhasivam@linaro.org> @ 2022-04-23 14:02 ` Manivannan Sadhasivam 2022-04-23 15:18 ` Bjorn Andersson 0 siblings, 1 reply; 2+ messages in thread From: Manivannan Sadhasivam @ 2022-04-23 14:02 UTC (permalink / raw) To: martin.petersen, jejb Cc: avri.altman, alim.akhtar, bjorn.andersson, linux-arm-msm, quic_asutoshd, quic_cang, linux-scsi, linux-kernel, bvanassche, ahalaney, Manivannan Sadhasivam, stable In ufs_qcom_dev_ref_clk_ctrl(), it was noted that the ref_clk needs to be stable for at least 1us. Even though there is wmb() to make sure the write gets "completed", there is no guarantee that the write actually reached the UFS device. There is a good chance that the write could be stored in a Write Buffer (WB). In that case, even though the CPU waits for 1us, the ref_clk might not be stable for that period. So lets do a readl() to make sure that the previous write has reached the UFS device before udelay(). Also, the wmb() after writel_relaxed is not really needed. Both writel and readl are ordered on all architectures and the CPU won't speculate instructions after readl() due to the in-built control dependency with read value on weakly ordered architectures. So it can be safely removed. Cc: stable@vger.kernel.org Fixes: f06fcc7155dc ("scsi: ufs-qcom: add QUniPro hardware support and power optimizations") Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/scsi/ufs/ufs-qcom.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index 6ee33cc0ad09..f47a16b7cff5 100644 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -687,8 +687,11 @@ static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) writel_relaxed(temp, host->dev_ref_clk_ctrl_mmio); - /* ensure that ref_clk is enabled/disabled before we return */ - wmb(); + /* + * Make sure the write to ref_clk reaches the destination and + * not stored in a Write Buffer (WB). + */ + readl(host->dev_ref_clk_ctrl_mmio); /* * If we call hibern8 exit after this, we need to make sure that -- 2.25.1 ^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2 3/5] scsi: ufs: qcom: Add a readl() to make sure ref_clk gets enabled 2022-04-23 14:02 ` [PATCH v2 3/5] scsi: ufs: qcom: Add a readl() to make sure ref_clk gets enabled Manivannan Sadhasivam @ 2022-04-23 15:18 ` Bjorn Andersson 0 siblings, 0 replies; 2+ messages in thread From: Bjorn Andersson @ 2022-04-23 15:18 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: martin.petersen, jejb, avri.altman, alim.akhtar, linux-arm-msm, quic_asutoshd, quic_cang, linux-scsi, linux-kernel, bvanassche, ahalaney, stable On Sat 23 Apr 07:02 PDT 2022, Manivannan Sadhasivam wrote: > In ufs_qcom_dev_ref_clk_ctrl(), it was noted that the ref_clk needs to be > stable for at least 1us. Even though there is wmb() to make sure the write > gets "completed", there is no guarantee that the write actually reached > the UFS device. There is a good chance that the write could be stored in > a Write Buffer (WB). In that case, even though the CPU waits for 1us, the > ref_clk might not be stable for that period. > > So lets do a readl() to make sure that the previous write has reached the > UFS device before udelay(). > > Also, the wmb() after writel_relaxed is not really needed. Both writel and > readl are ordered on all architectures and the CPU won't speculate > instructions after readl() due to the in-built control dependency with > read value on weakly ordered architectures. So it can be safely removed. > > Cc: stable@vger.kernel.org > Fixes: f06fcc7155dc ("scsi: ufs-qcom: add QUniPro hardware support and power optimizations") > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/scsi/ufs/ufs-qcom.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c > index 6ee33cc0ad09..f47a16b7cff5 100644 > --- a/drivers/scsi/ufs/ufs-qcom.c > +++ b/drivers/scsi/ufs/ufs-qcom.c > @@ -687,8 +687,11 @@ static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) > > writel_relaxed(temp, host->dev_ref_clk_ctrl_mmio); > > - /* ensure that ref_clk is enabled/disabled before we return */ > - wmb(); > + /* > + * Make sure the write to ref_clk reaches the destination and > + * not stored in a Write Buffer (WB). > + */ > + readl(host->dev_ref_clk_ctrl_mmio); > > /* > * If we call hibern8 exit after this, we need to make sure that > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-04-23 15:16 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220423140245.394092-1-manivannan.sadhasivam@linaro.org>
2022-04-23 14:02 ` [PATCH v2 3/5] scsi: ufs: qcom: Add a readl() to make sure ref_clk gets enabled Manivannan Sadhasivam
2022-04-23 15:18 ` Bjorn Andersson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox