Messages in this thread |  | | Date | Sat, 23 Apr 2022 17:15:35 +0530 | From | Manivannan Sadhasivam <> | Subject | Re: [PATCH 3/5] scsi: ufs: qcom: Add a readl() to make sure ref_clk gets enabled |
| |
On Fri, Apr 22, 2022 at 10:03:22PM -0700, Bart Van Assche wrote: > On 4/22/22 06:21, Manivannan Sadhasivam wrote: > > In ufs_qcom_dev_ref_clk_ctrl(), it was noted that the ref_clk needs to be > > stable for atleast 1us. Eventhough there is wmb() to make sure the write > ^ ^ > Some spaces are missing. > > > 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, eventhough the CPU waits for 1us, the > ^ > missing space---------------------------- >
Kind of used to it ;) Will fix it.
> > 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(). > > > > 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 | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c > > index 5f0a8f646eb5..5b9986c63eed 100644 > > --- a/drivers/scsi/ufs/ufs-qcom.c > > +++ b/drivers/scsi/ufs/ufs-qcom.c > > @@ -690,6 +690,12 @@ static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_qcom_host *host, bool enable) > > /* 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 > > * device ref_clk is stable for at least 1us before the hibern8 > > The comment above the wmb() call looks wrong to me. How about removing that > wmb() call? >
Hmm, yes it could be removed as well. readl() on weakly ordered architectures include a control dependency [1] so there is no way the instructions after it can be speculated.
Thanks, Mani
[1] https://www.spinics.net/lists/arm-kernel/msg689858.html
> Thanks, > > Bart.
|  |