From: Stanley Chu <stanley.chu@mediatek.com>
To: Avri Altman <Avri.Altman@wdc.com>
Cc: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
"alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
"jejb@linux.ibm.com" <jejb@linux.ibm.com>,
"beanhuo@micron.com" <beanhuo@micron.com>,
"asutoshd@codeaurora.org" <asutoshd@codeaurora.org>,
"cang@codeaurora.org" <cang@codeaurora.org>,
"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
"bvanassche@acm.org" <bvanassche@acm.org>,
"linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"kuohong.wang@mediatek.com" <kuohong.wang@mediatek.com>,
"peter.wang@mediatek.com" <peter.wang@mediatek.com>,
"chun-hung.wu@mediatek.com" <chun-hung.wu@mediatek.com>,
"andy.teng@mediatek.com" <andy.teng@mediatek.com>,
"cc.chou@mediatek.com" <cc.chou@mediatek.com>,
"chaotian.jing@mediatek.com" <chaotian.jing@mediatek.com>,
"pengshun.zhao@mediatek.com" <pengshun.zhao@mediatek.com>
Subject: RE: [PATCH v2 1/5] scsi: ufs-mediatek: Fix imprecise waiting time for ref-clk control
Date: Sun, 31 May 2020 21:45:06 +0800 [thread overview]
Message-ID: <1590932706.25636.14.camel@mtkswgap22> (raw)
In-Reply-To: <SN6PR04MB464015BDF84DF7A9779BEB41FC8D0@SN6PR04MB4640.namprd04.prod.outlook.com>
Hi Avri,
On Sun, 2020-05-31 at 07:10 +0000, Avri Altman wrote:
> >
> > Currently ref-clk control timeout is implemented by Jiffies. However
> > jiffies is not accurate enough thus "false timeout" may happen.
> >
> > Use more accurate delay mechanism instead, for example, ktime.
> >
> > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> > Reviewed-by: Andy Teng <andy.teng@mediatek.com>
> Reviewed-by: Avri Altman <avri.altman@wdc.com>
>
Thanks for your review.
> >
> > /* Wait for ack */
> > - timeout = jiffies + msecs_to_jiffies(REFCLK_REQ_TIMEOUT_MS);
> > + timeout = ktime_add_us(ktime_get(), REFCLK_REQ_TIMEOUT_US);
> > do {
> > + time_checked = ktime_get();
> > value = ufshcd_readl(hba, REG_UFS_REFCLK_CTRL);
> >
> > /* Wait until ack bit equals to req bit */
> > @@ -144,7 +145,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba,
> > bool on)
> > goto out;
> >
> > usleep_range(100, 200);
> > - } while (time_before(jiffies, timeout));
> > + } while (ktime_before(time_checked, timeout));
> Nit: you could get rid of time_checked if you would use ktime_compare(ktime_get(), timeout) > 0
>
> Thanks,
> Avri
If this context is preempted and scheduled out between ufshcd_readl()
and ktime_compare(ktime_get(), timeout), then the ktime_get() may get a
"timed-out" time even though the last ufshcd_readl() is actually
executed before the "timed-out" time. In this case, false alarm will
show up. Using "time_checked" here could solve above issue.
Thanks,
Stanley Chu
WARNING: multiple messages have this Message-ID (diff)
From: Stanley Chu <stanley.chu@mediatek.com>
To: Avri Altman <Avri.Altman@wdc.com>
Cc: "pengshun.zhao@mediatek.com" <pengshun.zhao@mediatek.com>,
"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
"andy.teng@mediatek.com" <andy.teng@mediatek.com>,
"jejb@linux.ibm.com" <jejb@linux.ibm.com>,
"chun-hung.wu@mediatek.com" <chun-hung.wu@mediatek.com>,
"kuohong.wang@mediatek.com" <kuohong.wang@mediatek.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"cc.chou@mediatek.com" <cc.chou@mediatek.com>,
"cang@codeaurora.org" <cang@codeaurora.org>,
"linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"peter.wang@mediatek.com" <peter.wang@mediatek.com>,
"alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
"beanhuo@micron.com" <beanhuo@micron.com>,
"chaotian.jing@mediatek.com" <chaotian.jing@mediatek.com>,
"bvanassche@acm.org" <bvanassche@acm.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"asutoshd@codeaurora.org" <asutoshd@codeaurora.org>
Subject: RE: [PATCH v2 1/5] scsi: ufs-mediatek: Fix imprecise waiting time for ref-clk control
Date: Sun, 31 May 2020 21:45:06 +0800 [thread overview]
Message-ID: <1590932706.25636.14.camel@mtkswgap22> (raw)
In-Reply-To: <SN6PR04MB464015BDF84DF7A9779BEB41FC8D0@SN6PR04MB4640.namprd04.prod.outlook.com>
Hi Avri,
On Sun, 2020-05-31 at 07:10 +0000, Avri Altman wrote:
> >
> > Currently ref-clk control timeout is implemented by Jiffies. However
> > jiffies is not accurate enough thus "false timeout" may happen.
> >
> > Use more accurate delay mechanism instead, for example, ktime.
> >
> > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> > Reviewed-by: Andy Teng <andy.teng@mediatek.com>
> Reviewed-by: Avri Altman <avri.altman@wdc.com>
>
Thanks for your review.
> >
> > /* Wait for ack */
> > - timeout = jiffies + msecs_to_jiffies(REFCLK_REQ_TIMEOUT_MS);
> > + timeout = ktime_add_us(ktime_get(), REFCLK_REQ_TIMEOUT_US);
> > do {
> > + time_checked = ktime_get();
> > value = ufshcd_readl(hba, REG_UFS_REFCLK_CTRL);
> >
> > /* Wait until ack bit equals to req bit */
> > @@ -144,7 +145,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba,
> > bool on)
> > goto out;
> >
> > usleep_range(100, 200);
> > - } while (time_before(jiffies, timeout));
> > + } while (ktime_before(time_checked, timeout));
> Nit: you could get rid of time_checked if you would use ktime_compare(ktime_get(), timeout) > 0
>
> Thanks,
> Avri
If this context is preempted and scheduled out between ufshcd_readl()
and ktime_compare(ktime_get(), timeout), then the ktime_get() may get a
"timed-out" time even though the last ufshcd_readl() is actually
executed before the "timed-out" time. In this case, false alarm will
show up. Using "time_checked" here could solve above issue.
Thanks,
Stanley Chu
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
WARNING: multiple messages have this Message-ID (diff)
From: Stanley Chu <stanley.chu@mediatek.com>
To: Avri Altman <Avri.Altman@wdc.com>
Cc: "pengshun.zhao@mediatek.com" <pengshun.zhao@mediatek.com>,
"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
"martin.petersen@oracle.com" <martin.petersen@oracle.com>,
"andy.teng@mediatek.com" <andy.teng@mediatek.com>,
"jejb@linux.ibm.com" <jejb@linux.ibm.com>,
"chun-hung.wu@mediatek.com" <chun-hung.wu@mediatek.com>,
"kuohong.wang@mediatek.com" <kuohong.wang@mediatek.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"cc.chou@mediatek.com" <cc.chou@mediatek.com>,
"cang@codeaurora.org" <cang@codeaurora.org>,
"linux-mediatek@lists.infradead.org"
<linux-mediatek@lists.infradead.org>,
"peter.wang@mediatek.com" <peter.wang@mediatek.com>,
"alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
"beanhuo@micron.com" <beanhuo@micron.com>,
"chaotian.jing@mediatek.com" <chaotian.jing@mediatek.com>,
"bvanassche@acm.org" <bvanassche@acm.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"asutoshd@codeaurora.org" <asutoshd@codeaurora.org>
Subject: RE: [PATCH v2 1/5] scsi: ufs-mediatek: Fix imprecise waiting time for ref-clk control
Date: Sun, 31 May 2020 21:45:06 +0800 [thread overview]
Message-ID: <1590932706.25636.14.camel@mtkswgap22> (raw)
In-Reply-To: <SN6PR04MB464015BDF84DF7A9779BEB41FC8D0@SN6PR04MB4640.namprd04.prod.outlook.com>
Hi Avri,
On Sun, 2020-05-31 at 07:10 +0000, Avri Altman wrote:
> >
> > Currently ref-clk control timeout is implemented by Jiffies. However
> > jiffies is not accurate enough thus "false timeout" may happen.
> >
> > Use more accurate delay mechanism instead, for example, ktime.
> >
> > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> > Reviewed-by: Andy Teng <andy.teng@mediatek.com>
> Reviewed-by: Avri Altman <avri.altman@wdc.com>
>
Thanks for your review.
> >
> > /* Wait for ack */
> > - timeout = jiffies + msecs_to_jiffies(REFCLK_REQ_TIMEOUT_MS);
> > + timeout = ktime_add_us(ktime_get(), REFCLK_REQ_TIMEOUT_US);
> > do {
> > + time_checked = ktime_get();
> > value = ufshcd_readl(hba, REG_UFS_REFCLK_CTRL);
> >
> > /* Wait until ack bit equals to req bit */
> > @@ -144,7 +145,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba,
> > bool on)
> > goto out;
> >
> > usleep_range(100, 200);
> > - } while (time_before(jiffies, timeout));
> > + } while (ktime_before(time_checked, timeout));
> Nit: you could get rid of time_checked if you would use ktime_compare(ktime_get(), timeout) > 0
>
> Thanks,
> Avri
If this context is preempted and scheduled out between ufshcd_readl()
and ktime_compare(ktime_get(), timeout), then the ktime_get() may get a
"timed-out" time even though the last ufshcd_readl() is actually
executed before the "timed-out" time. In this case, false alarm will
show up. Using "time_checked" here could solve above issue.
Thanks,
Stanley Chu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-05-31 13:45 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-29 9:23 [PATCH v2 0/5] scsi: ufs-mediatek: Fix clk-gating and introduce low-power mode for vccq2 Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` [PATCH v2 1/5] scsi: ufs-mediatek: Fix imprecise waiting time for ref-clk control Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-31 7:10 ` Avri Altman
2020-05-31 7:10 ` Avri Altman
2020-05-31 7:10 ` Avri Altman
2020-05-31 13:45 ` Stanley Chu [this message]
2020-05-31 13:45 ` Stanley Chu
2020-05-31 13:45 ` Stanley Chu
2020-05-29 9:23 ` [PATCH v2 2/5] scsi: ufs-mediatek: Do not gate clocks if auto-hibern8 is not entered yet Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` [PATCH v2 3/5] scsi: ufs-mediatek: Introduce low-power mode for device power supply Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` [PATCH v2 4/5] scsi: ufs-mediatek: Fix unbalanced clock on/off Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` [PATCH v2 5/5] scsi: ufs-mediatek: Allow unbound mphy Stanley Chu
2020-05-29 9:23 ` Stanley Chu
2020-05-29 9:23 ` Stanley Chu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1590932706.25636.14.camel@mtkswgap22 \
--to=stanley.chu@mediatek.com \
--cc=Avri.Altman@wdc.com \
--cc=alim.akhtar@samsung.com \
--cc=andy.teng@mediatek.com \
--cc=asutoshd@codeaurora.org \
--cc=beanhuo@micron.com \
--cc=bvanassche@acm.org \
--cc=cang@codeaurora.org \
--cc=cc.chou@mediatek.com \
--cc=chaotian.jing@mediatek.com \
--cc=chun-hung.wu@mediatek.com \
--cc=jejb@linux.ibm.com \
--cc=kuohong.wang@mediatek.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=matthias.bgg@gmail.com \
--cc=pengshun.zhao@mediatek.com \
--cc=peter.wang@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.