* [PATCH v1 0/2] scsi: ufs: fix waiting time for reference clock
@ 2020-02-17 9:35 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 9:35 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: beanhuo, asutoshd, cang, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng, Stanley Chu
Hi,
This patchset adds waiting time for reference clock in both common path and MediaTek UFS implementions.
Stanley Chu (2):
scsi: ufs: add required delay after gating reference clock
scsi: ufs: ufs-mediatek: add waiting time for reference clock
drivers/scsi/ufs/ufs-mediatek.c | 46 +++++++++++++++++++++++++++++++--
drivers/scsi/ufs/ufs-mediatek.h | 2 ++
drivers/scsi/ufs/ufshcd.c | 8 +++++-
3 files changed, 53 insertions(+), 3 deletions(-)
--
2.18.0
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v1 0/2] scsi: ufs: fix waiting time for reference clock
@ 2020-02-17 9:35 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 9:35 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, chun-hung.wu, kuohong.wang,
linux-kernel, cang, linux-mediatek, peter.wang, matthias.bgg,
beanhuo, linux-arm-kernel, asutoshd
Hi,
This patchset adds waiting time for reference clock in both common path and MediaTek UFS implementions.
Stanley Chu (2):
scsi: ufs: add required delay after gating reference clock
scsi: ufs: ufs-mediatek: add waiting time for reference clock
drivers/scsi/ufs/ufs-mediatek.c | 46 +++++++++++++++++++++++++++++++--
drivers/scsi/ufs/ufs-mediatek.h | 2 ++
drivers/scsi/ufs/ufshcd.c | 8 +++++-
3 files changed, 53 insertions(+), 3 deletions(-)
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v1 0/2] scsi: ufs: fix waiting time for reference clock
@ 2020-02-17 9:35 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 9:35 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, chun-hung.wu, kuohong.wang,
linux-kernel, cang, linux-mediatek, peter.wang, matthias.bgg,
beanhuo, linux-arm-kernel, asutoshd
Hi,
This patchset adds waiting time for reference clock in both common path and MediaTek UFS implementions.
Stanley Chu (2):
scsi: ufs: add required delay after gating reference clock
scsi: ufs: ufs-mediatek: add waiting time for reference clock
drivers/scsi/ufs/ufs-mediatek.c | 46 +++++++++++++++++++++++++++++++--
drivers/scsi/ufs/ufs-mediatek.h | 2 ++
drivers/scsi/ufs/ufshcd.c | 8 +++++-
3 files changed, 53 insertions(+), 3 deletions(-)
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-17 9:35 ` Stanley Chu
(?)
@ 2020-02-17 9:35 ` Stanley Chu
-1 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 9:35 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: beanhuo, asutoshd, cang, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng, Stanley Chu
In UFS version 3.0, a newly added attribute bRefClkGatingWaitTime defines
the minimum time for which the reference clock is required by device during
transition to LS-MODE or HIBERN8 state.
Currently this time is detected and stored in
hba->dev_info.clk_gating_wait_us but applied to vendor implementatios only.
Make it applied to reference clock named as "ref_clk" in device tree in
common path.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufshcd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 744b8254220c..7f60721f54d1 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7417,8 +7417,10 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
struct ufs_clk_info *clki;
struct list_head *head = &hba->clk_list_head;
unsigned long flags;
+ unsigned long wait_us;
ktime_t start = ktime_get();
bool clk_state_changed = false;
+ bool ref_clk;
if (list_empty(head))
goto out;
@@ -7436,7 +7438,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
list_for_each_entry(clki, head, list) {
if (!IS_ERR_OR_NULL(clki->clk)) {
- if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
+ ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
+ if (skip_ref_clk && ref_clk)
continue;
clk_state_changed = on ^ clki->enabled;
@@ -7449,6 +7452,9 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
}
} else if (!on && clki->enabled) {
clk_disable_unprepare(clki->clk);
+ wait_us = hba->dev_info.clk_gating_wait_us;
+ if (ref_clk && wait_us)
+ usleep_range(wait_us, wait_us + 10);
}
clki->enabled = on;
dev_dbg(hba->dev, "%s: clk: %s %sabled\n", __func__,
--
2.18.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 9:35 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 9:35 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, chun-hung.wu, kuohong.wang,
linux-kernel, cang, linux-mediatek, peter.wang, matthias.bgg,
beanhuo, linux-arm-kernel, asutoshd
In UFS version 3.0, a newly added attribute bRefClkGatingWaitTime defines
the minimum time for which the reference clock is required by device during
transition to LS-MODE or HIBERN8 state.
Currently this time is detected and stored in
hba->dev_info.clk_gating_wait_us but applied to vendor implementatios only.
Make it applied to reference clock named as "ref_clk" in device tree in
common path.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufshcd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 744b8254220c..7f60721f54d1 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7417,8 +7417,10 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
struct ufs_clk_info *clki;
struct list_head *head = &hba->clk_list_head;
unsigned long flags;
+ unsigned long wait_us;
ktime_t start = ktime_get();
bool clk_state_changed = false;
+ bool ref_clk;
if (list_empty(head))
goto out;
@@ -7436,7 +7438,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
list_for_each_entry(clki, head, list) {
if (!IS_ERR_OR_NULL(clki->clk)) {
- if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
+ ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
+ if (skip_ref_clk && ref_clk)
continue;
clk_state_changed = on ^ clki->enabled;
@@ -7449,6 +7452,9 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
}
} else if (!on && clki->enabled) {
clk_disable_unprepare(clki->clk);
+ wait_us = hba->dev_info.clk_gating_wait_us;
+ if (ref_clk && wait_us)
+ usleep_range(wait_us, wait_us + 10);
}
clki->enabled = on;
dev_dbg(hba->dev, "%s: clk: %s %sabled\n", __func__,
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 9:35 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 9:35 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, chun-hung.wu, kuohong.wang,
linux-kernel, cang, linux-mediatek, peter.wang, matthias.bgg,
beanhuo, linux-arm-kernel, asutoshd
In UFS version 3.0, a newly added attribute bRefClkGatingWaitTime defines
the minimum time for which the reference clock is required by device during
transition to LS-MODE or HIBERN8 state.
Currently this time is detected and stored in
hba->dev_info.clk_gating_wait_us but applied to vendor implementatios only.
Make it applied to reference clock named as "ref_clk" in device tree in
common path.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufshcd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 744b8254220c..7f60721f54d1 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7417,8 +7417,10 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
struct ufs_clk_info *clki;
struct list_head *head = &hba->clk_list_head;
unsigned long flags;
+ unsigned long wait_us;
ktime_t start = ktime_get();
bool clk_state_changed = false;
+ bool ref_clk;
if (list_empty(head))
goto out;
@@ -7436,7 +7438,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
list_for_each_entry(clki, head, list) {
if (!IS_ERR_OR_NULL(clki->clk)) {
- if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
+ ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
+ if (skip_ref_clk && ref_clk)
continue;
clk_state_changed = on ^ clki->enabled;
@@ -7449,6 +7452,9 @@ static int __ufshcd_setup_clocks(struct ufs_hba *hba, bool on,
}
} else if (!on && clki->enabled) {
clk_disable_unprepare(clki->clk);
+ wait_us = hba->dev_info.clk_gating_wait_us;
+ if (ref_clk && wait_us)
+ usleep_range(wait_us, wait_us + 10);
}
clki->enabled = on;
dev_dbg(hba->dev, "%s: clk: %s %sabled\n", __func__,
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v1 2/2] scsi: ufs: ufs-mediatek: add waiting time for reference clock
2020-02-17 9:35 ` Stanley Chu
(?)
@ 2020-02-17 9:35 ` Stanley Chu
-1 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 9:35 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: beanhuo, asutoshd, cang, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng, Stanley Chu
Some delays may be required either after gating or before ungating
reference clock for device according to vendor requirements.
Note that in UFS 3.0, the delay time after gating reference
clock can be defined by attribute bRefClkGatingWaitTime. Use the
formal value instead if it can be queried from device.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufs-mediatek.c | 46 +++++++++++++++++++++++++++++++--
drivers/scsi/ufs/ufs-mediatek.h | 2 ++
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 9d05962feb15..de650822c9d9 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -100,6 +100,17 @@ static int ufs_mtk_bind_mphy(struct ufs_hba *hba)
return err;
}
+static void ufs_mtk_udelay(unsigned long us)
+{
+ if (!us)
+ return;
+
+ if (us < 10)
+ udelay(us);
+ else
+ usleep_range(us, us + 10);
+}
+
static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
{
struct ufs_mtk_host *host = ufshcd_get_variant(hba);
@@ -112,6 +123,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
if (on) {
ufs_mtk_ref_clk_notify(on, res);
+ ufs_mtk_udelay(host->ref_clk_ungating_wait_us);
ufshcd_writel(hba, REFCLK_REQUEST, REG_UFS_REFCLK_CTRL);
} else {
ufshcd_writel(hba, REFCLK_RELEASE, REG_UFS_REFCLK_CTRL);
@@ -137,12 +149,29 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
out:
host->ref_clk_enabled = on;
- if (!on)
+ if (!on) {
+ ufs_mtk_udelay(host->ref_clk_gating_wait_us);
ufs_mtk_ref_clk_notify(on, res);
+ }
return 0;
}
+static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba,
+ u16 gating_us, u16 ungating_us)
+{
+ struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+
+ if (hba->dev_info.clk_gating_wait_us) {
+ host->ref_clk_gating_wait_us =
+ hba->dev_info.clk_gating_wait_us;
+ } else {
+ host->ref_clk_gating_wait_us = gating_us;
+ }
+
+ host->ref_clk_ungating_wait_us = ungating_us;
+}
+
static u32 ufs_mtk_link_get_state(struct ufs_hba *hba)
{
u32 val;
@@ -502,10 +531,23 @@ static void ufs_mtk_dbg_register_dump(struct ufs_hba *hba)
static int ufs_mtk_apply_dev_quirks(struct ufs_hba *hba)
{
struct ufs_dev_info *dev_info = &hba->dev_info;
+ u16 mid = dev_info->wmanufacturerid;
- if (dev_info->wmanufacturerid == UFS_VENDOR_SAMSUNG)
+ if (mid == UFS_VENDOR_SAMSUNG)
ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TACTIVATE), 6);
+ /*
+ * Decide waiting time before gating reference clock and
+ * after ungating reference clock according to vendors'
+ * requirements.
+ */
+ if (mid == UFS_VENDOR_SAMSUNG)
+ ufs_mtk_setup_ref_clk_wait_us(hba, 1, 1);
+ else if (mid == UFS_VENDOR_SKHYNIX)
+ ufs_mtk_setup_ref_clk_wait_us(hba, 30, 30);
+ else if (mid == UFS_VENDOR_TOSHIBA)
+ ufs_mtk_setup_ref_clk_wait_us(hba, 100, 32);
+
return 0;
}
diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
index 492414e5f481..4c787b99fe41 100644
--- a/drivers/scsi/ufs/ufs-mediatek.h
+++ b/drivers/scsi/ufs/ufs-mediatek.h
@@ -92,6 +92,8 @@ struct ufs_mtk_host {
struct ufs_hba *hba;
struct phy *mphy;
bool ref_clk_enabled;
+ u16 ref_clk_ungating_wait_us;
+ u16 ref_clk_gating_wait_us;
};
#endif /* !_UFS_MEDIATEK_H */
--
2.18.0
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v1 2/2] scsi: ufs: ufs-mediatek: add waiting time for reference clock
@ 2020-02-17 9:35 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 9:35 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, chun-hung.wu, kuohong.wang,
linux-kernel, cang, linux-mediatek, peter.wang, matthias.bgg,
beanhuo, linux-arm-kernel, asutoshd
Some delays may be required either after gating or before ungating
reference clock for device according to vendor requirements.
Note that in UFS 3.0, the delay time after gating reference
clock can be defined by attribute bRefClkGatingWaitTime. Use the
formal value instead if it can be queried from device.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufs-mediatek.c | 46 +++++++++++++++++++++++++++++++--
drivers/scsi/ufs/ufs-mediatek.h | 2 ++
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 9d05962feb15..de650822c9d9 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -100,6 +100,17 @@ static int ufs_mtk_bind_mphy(struct ufs_hba *hba)
return err;
}
+static void ufs_mtk_udelay(unsigned long us)
+{
+ if (!us)
+ return;
+
+ if (us < 10)
+ udelay(us);
+ else
+ usleep_range(us, us + 10);
+}
+
static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
{
struct ufs_mtk_host *host = ufshcd_get_variant(hba);
@@ -112,6 +123,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
if (on) {
ufs_mtk_ref_clk_notify(on, res);
+ ufs_mtk_udelay(host->ref_clk_ungating_wait_us);
ufshcd_writel(hba, REFCLK_REQUEST, REG_UFS_REFCLK_CTRL);
} else {
ufshcd_writel(hba, REFCLK_RELEASE, REG_UFS_REFCLK_CTRL);
@@ -137,12 +149,29 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
out:
host->ref_clk_enabled = on;
- if (!on)
+ if (!on) {
+ ufs_mtk_udelay(host->ref_clk_gating_wait_us);
ufs_mtk_ref_clk_notify(on, res);
+ }
return 0;
}
+static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba,
+ u16 gating_us, u16 ungating_us)
+{
+ struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+
+ if (hba->dev_info.clk_gating_wait_us) {
+ host->ref_clk_gating_wait_us =
+ hba->dev_info.clk_gating_wait_us;
+ } else {
+ host->ref_clk_gating_wait_us = gating_us;
+ }
+
+ host->ref_clk_ungating_wait_us = ungating_us;
+}
+
static u32 ufs_mtk_link_get_state(struct ufs_hba *hba)
{
u32 val;
@@ -502,10 +531,23 @@ static void ufs_mtk_dbg_register_dump(struct ufs_hba *hba)
static int ufs_mtk_apply_dev_quirks(struct ufs_hba *hba)
{
struct ufs_dev_info *dev_info = &hba->dev_info;
+ u16 mid = dev_info->wmanufacturerid;
- if (dev_info->wmanufacturerid == UFS_VENDOR_SAMSUNG)
+ if (mid == UFS_VENDOR_SAMSUNG)
ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TACTIVATE), 6);
+ /*
+ * Decide waiting time before gating reference clock and
+ * after ungating reference clock according to vendors'
+ * requirements.
+ */
+ if (mid == UFS_VENDOR_SAMSUNG)
+ ufs_mtk_setup_ref_clk_wait_us(hba, 1, 1);
+ else if (mid == UFS_VENDOR_SKHYNIX)
+ ufs_mtk_setup_ref_clk_wait_us(hba, 30, 30);
+ else if (mid == UFS_VENDOR_TOSHIBA)
+ ufs_mtk_setup_ref_clk_wait_us(hba, 100, 32);
+
return 0;
}
diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
index 492414e5f481..4c787b99fe41 100644
--- a/drivers/scsi/ufs/ufs-mediatek.h
+++ b/drivers/scsi/ufs/ufs-mediatek.h
@@ -92,6 +92,8 @@ struct ufs_mtk_host {
struct ufs_hba *hba;
struct phy *mphy;
bool ref_clk_enabled;
+ u16 ref_clk_ungating_wait_us;
+ u16 ref_clk_gating_wait_us;
};
#endif /* !_UFS_MEDIATEK_H */
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH v1 2/2] scsi: ufs: ufs-mediatek: add waiting time for reference clock
@ 2020-02-17 9:35 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 9:35 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, chun-hung.wu, kuohong.wang,
linux-kernel, cang, linux-mediatek, peter.wang, matthias.bgg,
beanhuo, linux-arm-kernel, asutoshd
Some delays may be required either after gating or before ungating
reference clock for device according to vendor requirements.
Note that in UFS 3.0, the delay time after gating reference
clock can be defined by attribute bRefClkGatingWaitTime. Use the
formal value instead if it can be queried from device.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
---
drivers/scsi/ufs/ufs-mediatek.c | 46 +++++++++++++++++++++++++++++++--
drivers/scsi/ufs/ufs-mediatek.h | 2 ++
2 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 9d05962feb15..de650822c9d9 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -100,6 +100,17 @@ static int ufs_mtk_bind_mphy(struct ufs_hba *hba)
return err;
}
+static void ufs_mtk_udelay(unsigned long us)
+{
+ if (!us)
+ return;
+
+ if (us < 10)
+ udelay(us);
+ else
+ usleep_range(us, us + 10);
+}
+
static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
{
struct ufs_mtk_host *host = ufshcd_get_variant(hba);
@@ -112,6 +123,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
if (on) {
ufs_mtk_ref_clk_notify(on, res);
+ ufs_mtk_udelay(host->ref_clk_ungating_wait_us);
ufshcd_writel(hba, REFCLK_REQUEST, REG_UFS_REFCLK_CTRL);
} else {
ufshcd_writel(hba, REFCLK_RELEASE, REG_UFS_REFCLK_CTRL);
@@ -137,12 +149,29 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
out:
host->ref_clk_enabled = on;
- if (!on)
+ if (!on) {
+ ufs_mtk_udelay(host->ref_clk_gating_wait_us);
ufs_mtk_ref_clk_notify(on, res);
+ }
return 0;
}
+static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba,
+ u16 gating_us, u16 ungating_us)
+{
+ struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+
+ if (hba->dev_info.clk_gating_wait_us) {
+ host->ref_clk_gating_wait_us =
+ hba->dev_info.clk_gating_wait_us;
+ } else {
+ host->ref_clk_gating_wait_us = gating_us;
+ }
+
+ host->ref_clk_ungating_wait_us = ungating_us;
+}
+
static u32 ufs_mtk_link_get_state(struct ufs_hba *hba)
{
u32 val;
@@ -502,10 +531,23 @@ static void ufs_mtk_dbg_register_dump(struct ufs_hba *hba)
static int ufs_mtk_apply_dev_quirks(struct ufs_hba *hba)
{
struct ufs_dev_info *dev_info = &hba->dev_info;
+ u16 mid = dev_info->wmanufacturerid;
- if (dev_info->wmanufacturerid == UFS_VENDOR_SAMSUNG)
+ if (mid == UFS_VENDOR_SAMSUNG)
ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TACTIVATE), 6);
+ /*
+ * Decide waiting time before gating reference clock and
+ * after ungating reference clock according to vendors'
+ * requirements.
+ */
+ if (mid == UFS_VENDOR_SAMSUNG)
+ ufs_mtk_setup_ref_clk_wait_us(hba, 1, 1);
+ else if (mid == UFS_VENDOR_SKHYNIX)
+ ufs_mtk_setup_ref_clk_wait_us(hba, 30, 30);
+ else if (mid == UFS_VENDOR_TOSHIBA)
+ ufs_mtk_setup_ref_clk_wait_us(hba, 100, 32);
+
return 0;
}
diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
index 492414e5f481..4c787b99fe41 100644
--- a/drivers/scsi/ufs/ufs-mediatek.h
+++ b/drivers/scsi/ufs/ufs-mediatek.h
@@ -92,6 +92,8 @@ struct ufs_mtk_host {
struct ufs_hba *hba;
struct phy *mphy;
bool ref_clk_enabled;
+ u16 ref_clk_ungating_wait_us;
+ u16 ref_clk_gating_wait_us;
};
#endif /* !_UFS_MEDIATEK_H */
--
2.18.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-17 9:35 ` Stanley Chu
(?)
@ 2020-02-17 12:58 ` Can Guo
-1 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-17 12:58 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng
On 2020-02-17 17:35, Stanley Chu wrote:
> In UFS version 3.0, a newly added attribute bRefClkGatingWaitTime
> defines
> the minimum time for which the reference clock is required by device
> during
> transition to LS-MODE or HIBERN8 state.
>
> Currently this time is detected and stored in
> hba->dev_info.clk_gating_wait_us but applied to vendor implementatios
> only.
> Make it applied to reference clock named as "ref_clk" in device tree in
> common path.
>
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 744b8254220c..7f60721f54d1 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7417,8 +7417,10 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
> struct ufs_clk_info *clki;
> struct list_head *head = &hba->clk_list_head;
> unsigned long flags;
> + unsigned long wait_us;
> ktime_t start = ktime_get();
> bool clk_state_changed = false;
> + bool ref_clk;
>
> if (list_empty(head))
> goto out;
> @@ -7436,7 +7438,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
>
> list_for_each_entry(clki, head, list) {
> if (!IS_ERR_OR_NULL(clki->clk)) {
> - if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> + ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
> + if (skip_ref_clk && ref_clk)
> continue;
>
> clk_state_changed = on ^ clki->enabled;
> @@ -7449,6 +7452,9 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
> }
> } else if (!on && clki->enabled) {
> clk_disable_unprepare(clki->clk);
> + wait_us = hba->dev_info.clk_gating_wait_us;
> + if (ref_clk && wait_us)
> + usleep_range(wait_us, wait_us + 10);
Hi Stanley,
If wait_us is 1us, it would be inappropriate to use usleep_range() here.
You have checks of the delay in patch #2, but why it is not needed here?
Thanks,
Can Guo.
> }
> clki->enabled = on;
> dev_dbg(hba->dev, "%s: clk: %s %sabled\n", __func__,
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 12:58 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-17 12:58 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
On 2020-02-17 17:35, Stanley Chu wrote:
> In UFS version 3.0, a newly added attribute bRefClkGatingWaitTime
> defines
> the minimum time for which the reference clock is required by device
> during
> transition to LS-MODE or HIBERN8 state.
>
> Currently this time is detected and stored in
> hba->dev_info.clk_gating_wait_us but applied to vendor implementatios
> only.
> Make it applied to reference clock named as "ref_clk" in device tree in
> common path.
>
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 744b8254220c..7f60721f54d1 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7417,8 +7417,10 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
> struct ufs_clk_info *clki;
> struct list_head *head = &hba->clk_list_head;
> unsigned long flags;
> + unsigned long wait_us;
> ktime_t start = ktime_get();
> bool clk_state_changed = false;
> + bool ref_clk;
>
> if (list_empty(head))
> goto out;
> @@ -7436,7 +7438,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
>
> list_for_each_entry(clki, head, list) {
> if (!IS_ERR_OR_NULL(clki->clk)) {
> - if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> + ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
> + if (skip_ref_clk && ref_clk)
> continue;
>
> clk_state_changed = on ^ clki->enabled;
> @@ -7449,6 +7452,9 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
> }
> } else if (!on && clki->enabled) {
> clk_disable_unprepare(clki->clk);
> + wait_us = hba->dev_info.clk_gating_wait_us;
> + if (ref_clk && wait_us)
> + usleep_range(wait_us, wait_us + 10);
Hi Stanley,
If wait_us is 1us, it would be inappropriate to use usleep_range() here.
You have checks of the delay in patch #2, but why it is not needed here?
Thanks,
Can Guo.
> }
> clki->enabled = on;
> dev_dbg(hba->dev, "%s: clk: %s %sabled\n", __func__,
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 12:58 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-17 12:58 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
On 2020-02-17 17:35, Stanley Chu wrote:
> In UFS version 3.0, a newly added attribute bRefClkGatingWaitTime
> defines
> the minimum time for which the reference clock is required by device
> during
> transition to LS-MODE or HIBERN8 state.
>
> Currently this time is detected and stored in
> hba->dev_info.clk_gating_wait_us but applied to vendor implementatios
> only.
> Make it applied to reference clock named as "ref_clk" in device tree in
> common path.
>
> Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> ---
> drivers/scsi/ufs/ufshcd.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 744b8254220c..7f60721f54d1 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -7417,8 +7417,10 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
> struct ufs_clk_info *clki;
> struct list_head *head = &hba->clk_list_head;
> unsigned long flags;
> + unsigned long wait_us;
> ktime_t start = ktime_get();
> bool clk_state_changed = false;
> + bool ref_clk;
>
> if (list_empty(head))
> goto out;
> @@ -7436,7 +7438,8 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
>
> list_for_each_entry(clki, head, list) {
> if (!IS_ERR_OR_NULL(clki->clk)) {
> - if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> + ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
> + if (skip_ref_clk && ref_clk)
> continue;
>
> clk_state_changed = on ^ clki->enabled;
> @@ -7449,6 +7452,9 @@ static int __ufshcd_setup_clocks(struct ufs_hba
> *hba, bool on,
> }
> } else if (!on && clki->enabled) {
> clk_disable_unprepare(clki->clk);
> + wait_us = hba->dev_info.clk_gating_wait_us;
> + if (ref_clk && wait_us)
> + usleep_range(wait_us, wait_us + 10);
Hi Stanley,
If wait_us is 1us, it would be inappropriate to use usleep_range() here.
You have checks of the delay in patch #2, but why it is not needed here?
Thanks,
Can Guo.
> }
> clki->enabled = on;
> dev_dbg(hba->dev, "%s: clk: %s %sabled\n", __func__,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-17 12:58 ` Can Guo
(?)
@ 2020-02-17 13:12 ` Stanley Chu
-1 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 13:12 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng
Hi Can,
> > } else if (!on && clki->enabled) {
> > clk_disable_unprepare(clki->clk);
> > + wait_us = hba->dev_info.clk_gating_wait_us;
> > + if (ref_clk && wait_us)
> > + usleep_range(wait_us, wait_us + 10);
>
> Hi Stanley,
>
> If wait_us is 1us, it would be inappropriate to use usleep_range() here.
> You have checks of the delay in patch #2, but why it is not needed here?
>
> Thanks,
> Can Guo.
You are right. I could make that delay checking as common function so it
can be used here as well to cover all possible values.
Thanks for suggestion.
Stanley
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 13:12 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 13:12 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
Hi Can,
> > } else if (!on && clki->enabled) {
> > clk_disable_unprepare(clki->clk);
> > + wait_us = hba->dev_info.clk_gating_wait_us;
> > + if (ref_clk && wait_us)
> > + usleep_range(wait_us, wait_us + 10);
>
> Hi Stanley,
>
> If wait_us is 1us, it would be inappropriate to use usleep_range() here.
> You have checks of the delay in patch #2, but why it is not needed here?
>
> Thanks,
> Can Guo.
You are right. I could make that delay checking as common function so it
can be used here as well to cover all possible values.
Thanks for suggestion.
Stanley
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 13:12 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 13:12 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
Hi Can,
> > } else if (!on && clki->enabled) {
> > clk_disable_unprepare(clki->clk);
> > + wait_us = hba->dev_info.clk_gating_wait_us;
> > + if (ref_clk && wait_us)
> > + usleep_range(wait_us, wait_us + 10);
>
> Hi Stanley,
>
> If wait_us is 1us, it would be inappropriate to use usleep_range() here.
> You have checks of the delay in patch #2, but why it is not needed here?
>
> Thanks,
> Can Guo.
You are right. I could make that delay checking as common function so it
can be used here as well to cover all possible values.
Thanks for suggestion.
Stanley
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-17 13:12 ` Stanley Chu
(?)
@ 2020-02-17 13:22 ` Can Guo
-1 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-17 13:22 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng
On 2020-02-17 21:12, Stanley Chu wrote:
> Hi Can,
>
>
>> > } else if (!on && clki->enabled) {
>> > clk_disable_unprepare(clki->clk);
>> > + wait_us = hba->dev_info.clk_gating_wait_us;
>> > + if (ref_clk && wait_us)
>> > + usleep_range(wait_us, wait_us + 10);
>>
>> Hi St,anley,
>>
>> If wait_us is 1us, it would be inappropriate to use usleep_range()
>> here.
>> You have checks of the delay in patch #2, but why it is not needed
>> here?
>>
>> Thanks,
>> Can Guo.
>
> You are right. I could make that delay checking as common function so
> it
> can be used here as well to cover all possible values.
>
> Thanks for suggestion.
> Stanley
Hi Stanley,
One more thing, as in patch #2, you have already added delays in your
ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
As the delay added in your vops also delays the actions of turning
off all the other clocks in ufshcd_setup_clocks(), you don't need the
delay here again, do you agree?
Thanks,
Can Guo.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 13:22 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-17 13:22 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
On 2020-02-17 21:12, Stanley Chu wrote:
> Hi Can,
>
>
>> > } else if (!on && clki->enabled) {
>> > clk_disable_unprepare(clki->clk);
>> > + wait_us = hba->dev_info.clk_gating_wait_us;
>> > + if (ref_clk && wait_us)
>> > + usleep_range(wait_us, wait_us + 10);
>>
>> Hi St,anley,
>>
>> If wait_us is 1us, it would be inappropriate to use usleep_range()
>> here.
>> You have checks of the delay in patch #2, but why it is not needed
>> here?
>>
>> Thanks,
>> Can Guo.
>
> You are right. I could make that delay checking as common function so
> it
> can be used here as well to cover all possible values.
>
> Thanks for suggestion.
> Stanley
Hi Stanley,
One more thing, as in patch #2, you have already added delays in your
ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
As the delay added in your vops also delays the actions of turning
off all the other clocks in ufshcd_setup_clocks(), you don't need the
delay here again, do you agree?
Thanks,
Can Guo.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 13:22 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-17 13:22 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
On 2020-02-17 21:12, Stanley Chu wrote:
> Hi Can,
>
>
>> > } else if (!on && clki->enabled) {
>> > clk_disable_unprepare(clki->clk);
>> > + wait_us = hba->dev_info.clk_gating_wait_us;
>> > + if (ref_clk && wait_us)
>> > + usleep_range(wait_us, wait_us + 10);
>>
>> Hi St,anley,
>>
>> If wait_us is 1us, it would be inappropriate to use usleep_range()
>> here.
>> You have checks of the delay in patch #2, but why it is not needed
>> here?
>>
>> Thanks,
>> Can Guo.
>
> You are right. I could make that delay checking as common function so
> it
> can be used here as well to cover all possible values.
>
> Thanks for suggestion.
> Stanley
Hi Stanley,
One more thing, as in patch #2, you have already added delays in your
ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
As the delay added in your vops also delays the actions of turning
off all the other clocks in ufshcd_setup_clocks(), you don't need the
delay here again, do you agree?
Thanks,
Can Guo.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-17 13:22 ` Can Guo
(?)
@ 2020-02-17 13:34 ` Stanley Chu
-1 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 13:34 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng
Hi Can,
On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote:
> On 2020-02-17 21:12, Stanley Chu wrote:
> > Hi Can,
> >
> >
> >> > } else if (!on && clki->enabled) {
> >> > clk_disable_unprepare(clki->clk);
> >> > + wait_us = hba->dev_info.clk_gating_wait_us;
> >> > + if (ref_clk && wait_us)
> >> > + usleep_range(wait_us, wait_us + 10);
> >>
> >> Hi St,anley,
> >>
> >> If wait_us is 1us, it would be inappropriate to use usleep_range()
> >> here.
> >> You have checks of the delay in patch #2, but why it is not needed
> >> here?
> >>
> >> Thanks,
> >> Can Guo.
> >
> > You are right. I could make that delay checking as common function so
> > it
> > can be used here as well to cover all possible values.
> >
> > Thanks for suggestion.
> > Stanley
>
> Hi Stanley,
>
> One more thing, as in patch #2, you have already added delays in your
> ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
> don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
> As the delay added in your vops also delays the actions of turning
> off all the other clocks in ufshcd_setup_clocks(), you don't need the
> delay here again, do you agree?
MediaTek driver is not using reference clocks named as "ref_clk" defined
in device tree, thus the delay specific for "ref_clk" in
ufshcd_setup_clocks() will not be applied in MediaTek platform.
This patch is aimed to add delay for this kind of "ref_clk" used by any
future vendors.
Anyway thanks for the reminding : )
>
> Thanks,
> Can Guo.
Thanks,
Stanley
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 13:34 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 13:34 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
Hi Can,
On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote:
> On 2020-02-17 21:12, Stanley Chu wrote:
> > Hi Can,
> >
> >
> >> > } else if (!on && clki->enabled) {
> >> > clk_disable_unprepare(clki->clk);
> >> > + wait_us = hba->dev_info.clk_gating_wait_us;
> >> > + if (ref_clk && wait_us)
> >> > + usleep_range(wait_us, wait_us + 10);
> >>
> >> Hi St,anley,
> >>
> >> If wait_us is 1us, it would be inappropriate to use usleep_range()
> >> here.
> >> You have checks of the delay in patch #2, but why it is not needed
> >> here?
> >>
> >> Thanks,
> >> Can Guo.
> >
> > You are right. I could make that delay checking as common function so
> > it
> > can be used here as well to cover all possible values.
> >
> > Thanks for suggestion.
> > Stanley
>
> Hi Stanley,
>
> One more thing, as in patch #2, you have already added delays in your
> ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
> don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
> As the delay added in your vops also delays the actions of turning
> off all the other clocks in ufshcd_setup_clocks(), you don't need the
> delay here again, do you agree?
MediaTek driver is not using reference clocks named as "ref_clk" defined
in device tree, thus the delay specific for "ref_clk" in
ufshcd_setup_clocks() will not be applied in MediaTek platform.
This patch is aimed to add delay for this kind of "ref_clk" used by any
future vendors.
Anyway thanks for the reminding : )
>
> Thanks,
> Can Guo.
Thanks,
Stanley
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 13:34 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-17 13:34 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
Hi Can,
On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote:
> On 2020-02-17 21:12, Stanley Chu wrote:
> > Hi Can,
> >
> >
> >> > } else if (!on && clki->enabled) {
> >> > clk_disable_unprepare(clki->clk);
> >> > + wait_us = hba->dev_info.clk_gating_wait_us;
> >> > + if (ref_clk && wait_us)
> >> > + usleep_range(wait_us, wait_us + 10);
> >>
> >> Hi St,anley,
> >>
> >> If wait_us is 1us, it would be inappropriate to use usleep_range()
> >> here.
> >> You have checks of the delay in patch #2, but why it is not needed
> >> here?
> >>
> >> Thanks,
> >> Can Guo.
> >
> > You are right. I could make that delay checking as common function so
> > it
> > can be used here as well to cover all possible values.
> >
> > Thanks for suggestion.
> > Stanley
>
> Hi Stanley,
>
> One more thing, as in patch #2, you have already added delays in your
> ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
> don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
> As the delay added in your vops also delays the actions of turning
> off all the other clocks in ufshcd_setup_clocks(), you don't need the
> delay here again, do you agree?
MediaTek driver is not using reference clocks named as "ref_clk" defined
in device tree, thus the delay specific for "ref_clk" in
ufshcd_setup_clocks() will not be applied in MediaTek platform.
This patch is aimed to add delay for this kind of "ref_clk" used by any
future vendors.
Anyway thanks for the reminding : )
>
> Thanks,
> Can Guo.
Thanks,
Stanley
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-17 13:34 ` Stanley Chu
(?)
@ 2020-02-17 13:42 ` Can Guo
-1 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-17 13:42 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng
On 2020-02-17 21:34, Stanley Chu wrote:
> Hi Can,
>
> On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote:
>> On 2020-02-17 21:12, Stanley Chu wrote:
>> > Hi Can,
>> >
>> >
>> >> > } else if (!on && clki->enabled) {
>> >> > clk_disable_unprepare(clki->clk);
>> >> > + wait_us = hba->dev_info.clk_gating_wait_us;
>> >> > + if (ref_clk && wait_us)
>> >> > + usleep_range(wait_us, wait_us + 10);
>> >>
>> >> Hi St,anley,
>> >>
>> >> If wait_us is 1us, it would be inappropriate to use usleep_range()
>> >> here.
>> >> You have checks of the delay in patch #2, but why it is not needed
>> >> here?
>> >>
>> >> Thanks,
>> >> Can Guo.
>> >
>> > You are right. I could make that delay checking as common function so
>> > it
>> > can be used here as well to cover all possible values.
>> >
>> > Thanks for suggestion.
>> > Stanley
>>
>> Hi Stanley,
>>
>> One more thing, as in patch #2, you have already added delays in your
>> ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
>> don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
>> As the delay added in your vops also delays the actions of turning
>> off all the other clocks in ufshcd_setup_clocks(), you don't need the
>> delay here again, do you agree?
>
> MediaTek driver is not using reference clocks named as "ref_clk"
> defined
> in device tree, thus the delay specific for "ref_clk" in
> ufshcd_setup_clocks() will not be applied in MediaTek platform.
>
> This patch is aimed to add delay for this kind of "ref_clk" used by any
> future vendors.
>
> Anyway thanks for the reminding : )
>
>>
>> Thanks,
>> Can Guo.
>
>
> Thanks,
> Stanley
Hi Stanley,
Then we are unluckily hit by this change. We have ref_clk in DT, thus
this change would add unwanted delays to our platforms. but still we
disable device's ref_clk in vops. :)
Could you please hold on patch #1 first? I need sometime to have a
dicussion with my colleagues on this.
Thanks.
Can Guo.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 13:42 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-17 13:42 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
On 2020-02-17 21:34, Stanley Chu wrote:
> Hi Can,
>
> On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote:
>> On 2020-02-17 21:12, Stanley Chu wrote:
>> > Hi Can,
>> >
>> >
>> >> > } else if (!on && clki->enabled) {
>> >> > clk_disable_unprepare(clki->clk);
>> >> > + wait_us = hba->dev_info.clk_gating_wait_us;
>> >> > + if (ref_clk && wait_us)
>> >> > + usleep_range(wait_us, wait_us + 10);
>> >>
>> >> Hi St,anley,
>> >>
>> >> If wait_us is 1us, it would be inappropriate to use usleep_range()
>> >> here.
>> >> You have checks of the delay in patch #2, but why it is not needed
>> >> here?
>> >>
>> >> Thanks,
>> >> Can Guo.
>> >
>> > You are right. I could make that delay checking as common function so
>> > it
>> > can be used here as well to cover all possible values.
>> >
>> > Thanks for suggestion.
>> > Stanley
>>
>> Hi Stanley,
>>
>> One more thing, as in patch #2, you have already added delays in your
>> ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
>> don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
>> As the delay added in your vops also delays the actions of turning
>> off all the other clocks in ufshcd_setup_clocks(), you don't need the
>> delay here again, do you agree?
>
> MediaTek driver is not using reference clocks named as "ref_clk"
> defined
> in device tree, thus the delay specific for "ref_clk" in
> ufshcd_setup_clocks() will not be applied in MediaTek platform.
>
> This patch is aimed to add delay for this kind of "ref_clk" used by any
> future vendors.
>
> Anyway thanks for the reminding : )
>
>>
>> Thanks,
>> Can Guo.
>
>
> Thanks,
> Stanley
Hi Stanley,
Then we are unluckily hit by this change. We have ref_clk in DT, thus
this change would add unwanted delays to our platforms. but still we
disable device's ref_clk in vops. :)
Could you please hold on patch #1 first? I need sometime to have a
dicussion with my colleagues on this.
Thanks.
Can Guo.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 13:42 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-17 13:42 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
On 2020-02-17 21:34, Stanley Chu wrote:
> Hi Can,
>
> On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote:
>> On 2020-02-17 21:12, Stanley Chu wrote:
>> > Hi Can,
>> >
>> >
>> >> > } else if (!on && clki->enabled) {
>> >> > clk_disable_unprepare(clki->clk);
>> >> > + wait_us = hba->dev_info.clk_gating_wait_us;
>> >> > + if (ref_clk && wait_us)
>> >> > + usleep_range(wait_us, wait_us + 10);
>> >>
>> >> Hi St,anley,
>> >>
>> >> If wait_us is 1us, it would be inappropriate to use usleep_range()
>> >> here.
>> >> You have checks of the delay in patch #2, but why it is not needed
>> >> here?
>> >>
>> >> Thanks,
>> >> Can Guo.
>> >
>> > You are right. I could make that delay checking as common function so
>> > it
>> > can be used here as well to cover all possible values.
>> >
>> > Thanks for suggestion.
>> > Stanley
>>
>> Hi Stanley,
>>
>> One more thing, as in patch #2, you have already added delays in your
>> ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
>> don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
>> As the delay added in your vops also delays the actions of turning
>> off all the other clocks in ufshcd_setup_clocks(), you don't need the
>> delay here again, do you agree?
>
> MediaTek driver is not using reference clocks named as "ref_clk"
> defined
> in device tree, thus the delay specific for "ref_clk" in
> ufshcd_setup_clocks() will not be applied in MediaTek platform.
>
> This patch is aimed to add delay for this kind of "ref_clk" used by any
> future vendors.
>
> Anyway thanks for the reminding : )
>
>>
>> Thanks,
>> Can Guo.
>
>
> Thanks,
> Stanley
Hi Stanley,
Then we are unluckily hit by this change. We have ref_clk in DT, thus
this change would add unwanted delays to our platforms. but still we
disable device's ref_clk in vops. :)
Could you please hold on patch #1 first? I need sometime to have a
dicussion with my colleagues on this.
Thanks.
Can Guo.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-17 9:35 ` Stanley Chu
(?)
@ 2020-02-17 16:17 ` Bart Van Assche
-1 siblings, 0 replies; 42+ messages in thread
From: Bart Van Assche @ 2020-02-17 16:17 UTC (permalink / raw)
To: Stanley Chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: beanhuo, asutoshd, cang, matthias.bgg, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng
On 2020-02-17 01:35, Stanley Chu wrote:
> - if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> + ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
> + if (skip_ref_clk && ref_clk)
Since the " ? true : false" part is superfluous, please leave it out.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 16:17 ` Bart Van Assche
0 siblings, 0 replies; 42+ messages in thread
From: Bart Van Assche @ 2020-02-17 16:17 UTC (permalink / raw)
To: Stanley Chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: andy.teng, chun-hung.wu, kuohong.wang, linux-kernel, cang,
linux-mediatek, peter.wang, matthias.bgg, beanhuo,
linux-arm-kernel, asutoshd
On 2020-02-17 01:35, Stanley Chu wrote:
> - if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> + ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
> + if (skip_ref_clk && ref_clk)
Since the " ? true : false" part is superfluous, please leave it out.
Thanks,
Bart.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-17 16:17 ` Bart Van Assche
0 siblings, 0 replies; 42+ messages in thread
From: Bart Van Assche @ 2020-02-17 16:17 UTC (permalink / raw)
To: Stanley Chu, linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: andy.teng, chun-hung.wu, kuohong.wang, linux-kernel, cang,
linux-mediatek, peter.wang, matthias.bgg, beanhuo,
linux-arm-kernel, asutoshd
On 2020-02-17 01:35, Stanley Chu wrote:
> - if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> + ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
> + if (skip_ref_clk && ref_clk)
Since the " ? true : false" part is superfluous, please leave it out.
Thanks,
Bart.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-17 16:17 ` Bart Van Assche
(?)
@ 2020-02-18 0:50 ` Stanley Chu
-1 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-18 0:50 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, cang, matthias.bgg, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng
Hi Bart,
On Mon, 2020-02-17 at 08:17 -0800, Bart Van Assche wrote:
> On 2020-02-17 01:35, Stanley Chu wrote:
> > - if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> > + ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
> > + if (skip_ref_clk && ref_clk)
>
> Since the " ? true : false" part is superfluous, please leave it out.
Will fix this in next version.
> Thanks,
>
> Bart.
Thanks,
Stanley Chu
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-18 0:50 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-18 0:50 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, cang, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd,
linux-arm-kernel, beanhuo
Hi Bart,
On Mon, 2020-02-17 at 08:17 -0800, Bart Van Assche wrote:
> On 2020-02-17 01:35, Stanley Chu wrote:
> > - if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> > + ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
> > + if (skip_ref_clk && ref_clk)
>
> Since the " ? true : false" part is superfluous, please leave it out.
Will fix this in next version.
> Thanks,
>
> Bart.
Thanks,
Stanley Chu
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-18 0:50 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-18 0:50 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, cang, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd,
linux-arm-kernel, beanhuo
Hi Bart,
On Mon, 2020-02-17 at 08:17 -0800, Bart Van Assche wrote:
> On 2020-02-17 01:35, Stanley Chu wrote:
> > - if (skip_ref_clk && !strcmp(clki->name, "ref_clk"))
> > + ref_clk = !strcmp(clki->name, "ref_clk") ? true : false;
> > + if (skip_ref_clk && ref_clk)
>
> Since the " ? true : false" part is superfluous, please leave it out.
Will fix this in next version.
> Thanks,
>
> Bart.
Thanks,
Stanley Chu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-17 13:42 ` Can Guo
(?)
@ 2020-02-19 2:35 ` Can Guo
-1 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-19 2:35 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng
Hi Stanely,
On 2020-02-17 21:42, Can Guo wrote:
> On 2020-02-17 21:34, Stanley Chu wrote:
>> Hi Can,
>>
>> On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote:
>>> On 2020-02-17 21:12, Stanley Chu wrote:
>>> > Hi Can,
>>> >
>>> >
>>> >> > } else if (!on && clki->enabled) {
>>> >> > clk_disable_unprepare(clki->clk);
>>> >> > + wait_us = hba->dev_info.clk_gating_wait_us;
>>> >> > + if (ref_clk && wait_us)
>>> >> > + usleep_range(wait_us, wait_us + 10);
>>> >>
>>> >> Hi St,anley,
>>> >>
>>> >> If wait_us is 1us, it would be inappropriate to use usleep_range()
>>> >> here.
>>> >> You have checks of the delay in patch #2, but why it is not needed
>>> >> here?
>>> >>
>>> >> Thanks,
>>> >> Can Guo.
>>> >
>>> > You are right. I could make that delay checking as common function so
>>> > it
>>> > can be used here as well to cover all possible values.
>>> >
>>> > Thanks for suggestion.
>>> > Stanley
>>>
>>> Hi Stanley,
>>>
>>> One more thing, as in patch #2, you have already added delays in your
>>> ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
>>> don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
>>> As the delay added in your vops also delays the actions of turning
>>> off all the other clocks in ufshcd_setup_clocks(), you don't need the
>>> delay here again, do you agree?
>>
>> MediaTek driver is not using reference clocks named as "ref_clk"
>> defined
>> in device tree, thus the delay specific for "ref_clk" in
>> ufshcd_setup_clocks() will not be applied in MediaTek platform.
>>
>> This patch is aimed to add delay for this kind of "ref_clk" used by
>> any
>> future vendors.
>>
>> Anyway thanks for the reminding : )
>>
>>>
>>> Thanks,
>>> Can Guo.
>>
>>
>> Thanks,
>> Stanley
>
> Hi Stanley,
>
> Then we are unluckily hit by this change. We have ref_clk in DT, thus
> this change would add unwanted delays to our platforms. but still we
> disable device's ref_clk in vops. :)
>
> Could you please hold on patch #1 first? I need sometime to have a
> dicussion with my colleagues on this.
>
> Thanks.
> Can Guo.
Since we all need this delay here, how about put the delay in the
entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
If so, we can remove all the delays we added in our vops since the
delay anyways delays everything inside ufshcd_setup_clocks().
Meanwhile, if you want to modify the delay
(hba->dev_info.clk_gating_wait_us) for some reasons, say for specific
UFS devices, you still can do it in vops_apply_dev_quirks().
What do you say?
Thanks,
Can Guo.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-19 2:35 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-19 2:35 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
Hi Stanely,
On 2020-02-17 21:42, Can Guo wrote:
> On 2020-02-17 21:34, Stanley Chu wrote:
>> Hi Can,
>>
>> On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote:
>>> On 2020-02-17 21:12, Stanley Chu wrote:
>>> > Hi Can,
>>> >
>>> >
>>> >> > } else if (!on && clki->enabled) {
>>> >> > clk_disable_unprepare(clki->clk);
>>> >> > + wait_us = hba->dev_info.clk_gating_wait_us;
>>> >> > + if (ref_clk && wait_us)
>>> >> > + usleep_range(wait_us, wait_us + 10);
>>> >>
>>> >> Hi St,anley,
>>> >>
>>> >> If wait_us is 1us, it would be inappropriate to use usleep_range()
>>> >> here.
>>> >> You have checks of the delay in patch #2, but why it is not needed
>>> >> here?
>>> >>
>>> >> Thanks,
>>> >> Can Guo.
>>> >
>>> > You are right. I could make that delay checking as common function so
>>> > it
>>> > can be used here as well to cover all possible values.
>>> >
>>> > Thanks for suggestion.
>>> > Stanley
>>>
>>> Hi Stanley,
>>>
>>> One more thing, as in patch #2, you have already added delays in your
>>> ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
>>> don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
>>> As the delay added in your vops also delays the actions of turning
>>> off all the other clocks in ufshcd_setup_clocks(), you don't need the
>>> delay here again, do you agree?
>>
>> MediaTek driver is not using reference clocks named as "ref_clk"
>> defined
>> in device tree, thus the delay specific for "ref_clk" in
>> ufshcd_setup_clocks() will not be applied in MediaTek platform.
>>
>> This patch is aimed to add delay for this kind of "ref_clk" used by
>> any
>> future vendors.
>>
>> Anyway thanks for the reminding : )
>>
>>>
>>> Thanks,
>>> Can Guo.
>>
>>
>> Thanks,
>> Stanley
>
> Hi Stanley,
>
> Then we are unluckily hit by this change. We have ref_clk in DT, thus
> this change would add unwanted delays to our platforms. but still we
> disable device's ref_clk in vops. :)
>
> Could you please hold on patch #1 first? I need sometime to have a
> dicussion with my colleagues on this.
>
> Thanks.
> Can Guo.
Since we all need this delay here, how about put the delay in the
entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
If so, we can remove all the delays we added in our vops since the
delay anyways delays everything inside ufshcd_setup_clocks().
Meanwhile, if you want to modify the delay
(hba->dev_info.clk_gating_wait_us) for some reasons, say for specific
UFS devices, you still can do it in vops_apply_dev_quirks().
What do you say?
Thanks,
Can Guo.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-19 2:35 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-19 2:35 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
Hi Stanely,
On 2020-02-17 21:42, Can Guo wrote:
> On 2020-02-17 21:34, Stanley Chu wrote:
>> Hi Can,
>>
>> On Mon, 2020-02-17 at 21:22 +0800, Can Guo wrote:
>>> On 2020-02-17 21:12, Stanley Chu wrote:
>>> > Hi Can,
>>> >
>>> >
>>> >> > } else if (!on && clki->enabled) {
>>> >> > clk_disable_unprepare(clki->clk);
>>> >> > + wait_us = hba->dev_info.clk_gating_wait_us;
>>> >> > + if (ref_clk && wait_us)
>>> >> > + usleep_range(wait_us, wait_us + 10);
>>> >>
>>> >> Hi St,anley,
>>> >>
>>> >> If wait_us is 1us, it would be inappropriate to use usleep_range()
>>> >> here.
>>> >> You have checks of the delay in patch #2, but why it is not needed
>>> >> here?
>>> >>
>>> >> Thanks,
>>> >> Can Guo.
>>> >
>>> > You are right. I could make that delay checking as common function so
>>> > it
>>> > can be used here as well to cover all possible values.
>>> >
>>> > Thanks for suggestion.
>>> > Stanley
>>>
>>> Hi Stanley,
>>>
>>> One more thing, as in patch #2, you have already added delays in your
>>> ufshcd_vops_setup_clocks(OFF, PRE_CHANGE) path, plus this delay here,
>>> don't you delay for 2*bRefClkGatingWaitTime in ufshcd_setup_clocks()?
>>> As the delay added in your vops also delays the actions of turning
>>> off all the other clocks in ufshcd_setup_clocks(), you don't need the
>>> delay here again, do you agree?
>>
>> MediaTek driver is not using reference clocks named as "ref_clk"
>> defined
>> in device tree, thus the delay specific for "ref_clk" in
>> ufshcd_setup_clocks() will not be applied in MediaTek platform.
>>
>> This patch is aimed to add delay for this kind of "ref_clk" used by
>> any
>> future vendors.
>>
>> Anyway thanks for the reminding : )
>>
>>>
>>> Thanks,
>>> Can Guo.
>>
>>
>> Thanks,
>> Stanley
>
> Hi Stanley,
>
> Then we are unluckily hit by this change. We have ref_clk in DT, thus
> this change would add unwanted delays to our platforms. but still we
> disable device's ref_clk in vops. :)
>
> Could you please hold on patch #1 first? I need sometime to have a
> dicussion with my colleagues on this.
>
> Thanks.
> Can Guo.
Since we all need this delay here, how about put the delay in the
entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
If so, we can remove all the delays we added in our vops since the
delay anyways delays everything inside ufshcd_setup_clocks().
Meanwhile, if you want to modify the delay
(hba->dev_info.clk_gating_wait_us) for some reasons, say for specific
UFS devices, you still can do it in vops_apply_dev_quirks().
What do you say?
Thanks,
Can Guo.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-19 2:35 ` Can Guo
(?)
@ 2020-02-19 9:11 ` Stanley Chu
-1 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-19 9:11 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb,
beanhuo, asutoshd, matthias.bgg, bvanassche, linux-mediatek,
linux-arm-kernel, linux-kernel, kuohong.wang, peter.wang,
chun-hung.wu, andy.teng
Hi Can,
On Wed, 2020-02-19 at 10:35 +0800, Can Guo wrote:
> Since we all need this delay here, how about put the delay in the
> entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
> If so, we can remove all the delays we added in our vops since the
> delay anyways delays everything inside ufshcd_setup_clocks().
>
Always putting the delay in the entrance of ufshcd_setup_clocks() may
add unwanted delay for vendors, just like your current implementation,
or some other vendors who do not want to disable the reference clock.
I think current patch is more reasonable because the delay is applied to
clock only named as "ref_clk" specifically.
If you needs to keep "ref_clk" in DT, would you consider to remove the
delay in your ufs_qcom_dev_ref_clk_ctrl() and let the delay happens via
common ufshcd_setup_clocks() only? However you may still need delay if
call path comes from ufs_qcom_pwr_change_notify().
What do you think?
> Meanwhile, if you want to modify the delay
> (hba->dev_info.clk_gating_wait_us) for some reasons, say for specific
> UFS devices, you still can do it in vops_apply_dev_quirks().
>
> What do you say?
>
> Thanks,
> Can Guo.
Thanks,
Stanley Chu
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-19 9:11 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-19 9:11 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
Hi Can,
On Wed, 2020-02-19 at 10:35 +0800, Can Guo wrote:
> Since we all need this delay here, how about put the delay in the
> entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
> If so, we can remove all the delays we added in our vops since the
> delay anyways delays everything inside ufshcd_setup_clocks().
>
Always putting the delay in the entrance of ufshcd_setup_clocks() may
add unwanted delay for vendors, just like your current implementation,
or some other vendors who do not want to disable the reference clock.
I think current patch is more reasonable because the delay is applied to
clock only named as "ref_clk" specifically.
If you needs to keep "ref_clk" in DT, would you consider to remove the
delay in your ufs_qcom_dev_ref_clk_ctrl() and let the delay happens via
common ufshcd_setup_clocks() only? However you may still need delay if
call path comes from ufs_qcom_pwr_change_notify().
What do you think?
> Meanwhile, if you want to modify the delay
> (hba->dev_info.clk_gating_wait_us) for some reasons, say for specific
> UFS devices, you still can do it in vops_apply_dev_quirks().
>
> What do you say?
>
> Thanks,
> Can Guo.
Thanks,
Stanley Chu
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-19 9:11 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-19 9:11 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, asutoshd, bvanassche,
linux-arm-kernel, beanhuo
Hi Can,
On Wed, 2020-02-19 at 10:35 +0800, Can Guo wrote:
> Since we all need this delay here, how about put the delay in the
> entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
> If so, we can remove all the delays we added in our vops since the
> delay anyways delays everything inside ufshcd_setup_clocks().
>
Always putting the delay in the entrance of ufshcd_setup_clocks() may
add unwanted delay for vendors, just like your current implementation,
or some other vendors who do not want to disable the reference clock.
I think current patch is more reasonable because the delay is applied to
clock only named as "ref_clk" specifically.
If you needs to keep "ref_clk" in DT, would you consider to remove the
delay in your ufs_qcom_dev_ref_clk_ctrl() and let the delay happens via
common ufshcd_setup_clocks() only? However you may still need delay if
call path comes from ufs_qcom_pwr_change_notify().
What do you think?
> Meanwhile, if you want to modify the delay
> (hba->dev_info.clk_gating_wait_us) for some reasons, say for specific
> UFS devices, you still can do it in vops_apply_dev_quirks().
>
> What do you say?
>
> Thanks,
> Can Guo.
Thanks,
Stanley Chu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-19 9:11 ` Stanley Chu
(?)
@ 2020-02-19 10:33 ` Can Guo
-1 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-19 10:33 UTC (permalink / raw)
To: Stanley Chu
Cc: linux-scsi, martin.petersen, Asutosh Das, hongwus, avri.altman,
alim.akhtar, jejb, beanhuo, asutoshd, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng
Hi Stanley,
On 2020-02-19 17:11, Stanley Chu wrote:
> Hi Can,
>
> On Wed, 2020-02-19 at 10:35 +0800, Can Guo wrote:
>
>> Since we all need this delay here, how about put the delay in the
>> entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
>> If so, we can remove all the delays we added in our vops since the
>> delay anyways delays everything inside ufshcd_setup_clocks().
>>
>
> Always putting the delay in the entrance of ufshcd_setup_clocks() may
> add unwanted delay for vendors, just like your current implementation,
> or some other vendors who do not want to disable the reference clock.
>
> I think current patch is more reasonable because the delay is applied
> to
> clock only named as "ref_clk" specifically.
>
> If you needs to keep "ref_clk" in DT, would you consider to remove the
> delay in your ufs_qcom_dev_ref_clk_ctrl() and let the delay happens via
> common ufshcd_setup_clocks() only? However you may still need delay if
> call path comes from ufs_qcom_pwr_change_notify().
>
> What do you think?
>
I agree current change is more reasonable from what it looks, but the
fact
is that I canont remove the delay in ufs_qcom_dev_ref_clk_ctrl() even
with
this change. On our platforms, ref_clk in DT serves multipule purposes,
the ref_clk provided to UFS device is actually controlled in
ufs_qcom_dev_ref_clk_ctrl(), which comes before where this change kicks
start,
so if I remove the delay in ufs_qcom_dev_ref_clk_ctrl(), this change
cannot
provide us the correct delay before gate the ref_clk provided to UFS
device.
> Always putting the delay in the entrance of ufshcd_setup_clocks() may
> add unwanted delay for vendors, just like your current implementation,
> or some other vendors who do not want to disable the reference clock.
I meant if we put the delay in the entrance, I will be able to remove
the delay in ufs_qcom_dev_ref_clk_ctrl(). Meanwhile, we can add proper
checks before the delay to make sure it is initiated only if ref_clk
needs
to be disabled, i.e:
if(!on && !skip_ref_clk && hba->dev_info.clk_gating_wait_us)
usleep_range();
Does this look better to you?
Anyways, we will see regressions with this change on our platforms, can
we
have more discussions before get it merged? It should be OK if you go
with
patch #2 alone first, right? Thanks.
Best regards,
Can Guo.
>> Meanwhile, if you want to modify the delay
>> (hba->dev_info.clk_gating_wait_us) for some reasons, say for specific
>> UFS devices, you still can do it in vops_apply_dev_quirks().
>>
>> What do you say?
>>
>> Thanks,
>> Can Guo.
>
> Thanks,
> Stanley Chu
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-19 10:33 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-19 10:33 UTC (permalink / raw)
To: Stanley Chu
Cc: chun-hung.wu, linux-scsi, martin.petersen, andy.teng, jejb,
peter.wang, kuohong.wang, linux-kernel, avri.altman,
linux-mediatek, linux-arm-kernel, alim.akhtar, matthias.bgg,
beanhuo, bvanassche, hongwus, asutoshd
Hi Stanley,
On 2020-02-19 17:11, Stanley Chu wrote:
> Hi Can,
>
> On Wed, 2020-02-19 at 10:35 +0800, Can Guo wrote:
>
>> Since we all need this delay here, how about put the delay in the
>> entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
>> If so, we can remove all the delays we added in our vops since the
>> delay anyways delays everything inside ufshcd_setup_clocks().
>>
>
> Always putting the delay in the entrance of ufshcd_setup_clocks() may
> add unwanted delay for vendors, just like your current implementation,
> or some other vendors who do not want to disable the reference clock.
>
> I think current patch is more reasonable because the delay is applied
> to
> clock only named as "ref_clk" specifically.
>
> If you needs to keep "ref_clk" in DT, would you consider to remove the
> delay in your ufs_qcom_dev_ref_clk_ctrl() and let the delay happens via
> common ufshcd_setup_clocks() only? However you may still need delay if
> call path comes from ufs_qcom_pwr_change_notify().
>
> What do you think?
>
I agree current change is more reasonable from what it looks, but the
fact
is that I canont remove the delay in ufs_qcom_dev_ref_clk_ctrl() even
with
this change. On our platforms, ref_clk in DT serves multipule purposes,
the ref_clk provided to UFS device is actually controlled in
ufs_qcom_dev_ref_clk_ctrl(), which comes before where this change kicks
start,
so if I remove the delay in ufs_qcom_dev_ref_clk_ctrl(), this change
cannot
provide us the correct delay before gate the ref_clk provided to UFS
device.
> Always putting the delay in the entrance of ufshcd_setup_clocks() may
> add unwanted delay for vendors, just like your current implementation,
> or some other vendors who do not want to disable the reference clock.
I meant if we put the delay in the entrance, I will be able to remove
the delay in ufs_qcom_dev_ref_clk_ctrl(). Meanwhile, we can add proper
checks before the delay to make sure it is initiated only if ref_clk
needs
to be disabled, i.e:
if(!on && !skip_ref_clk && hba->dev_info.clk_gating_wait_us)
usleep_range();
Does this look better to you?
Anyways, we will see regressions with this change on our platforms, can
we
have more discussions before get it merged? It should be OK if you go
with
patch #2 alone first, right? Thanks.
Best regards,
Can Guo.
>> Meanwhile, if you want to modify the delay
>> (hba->dev_info.clk_gating_wait_us) for some reasons, say for specific
>> UFS devices, you still can do it in vops_apply_dev_quirks().
>>
>> What do you say?
>>
>> Thanks,
>> Can Guo.
>
> Thanks,
> Stanley Chu
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-19 10:33 ` Can Guo
0 siblings, 0 replies; 42+ messages in thread
From: Can Guo @ 2020-02-19 10:33 UTC (permalink / raw)
To: Stanley Chu
Cc: chun-hung.wu, linux-scsi, martin.petersen, andy.teng, jejb,
peter.wang, kuohong.wang, linux-kernel, avri.altman,
linux-mediatek, linux-arm-kernel, alim.akhtar, matthias.bgg,
beanhuo, bvanassche, hongwus, asutoshd
Hi Stanley,
On 2020-02-19 17:11, Stanley Chu wrote:
> Hi Can,
>
> On Wed, 2020-02-19 at 10:35 +0800, Can Guo wrote:
>
>> Since we all need this delay here, how about put the delay in the
>> entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
>> If so, we can remove all the delays we added in our vops since the
>> delay anyways delays everything inside ufshcd_setup_clocks().
>>
>
> Always putting the delay in the entrance of ufshcd_setup_clocks() may
> add unwanted delay for vendors, just like your current implementation,
> or some other vendors who do not want to disable the reference clock.
>
> I think current patch is more reasonable because the delay is applied
> to
> clock only named as "ref_clk" specifically.
>
> If you needs to keep "ref_clk" in DT, would you consider to remove the
> delay in your ufs_qcom_dev_ref_clk_ctrl() and let the delay happens via
> common ufshcd_setup_clocks() only? However you may still need delay if
> call path comes from ufs_qcom_pwr_change_notify().
>
> What do you think?
>
I agree current change is more reasonable from what it looks, but the
fact
is that I canont remove the delay in ufs_qcom_dev_ref_clk_ctrl() even
with
this change. On our platforms, ref_clk in DT serves multipule purposes,
the ref_clk provided to UFS device is actually controlled in
ufs_qcom_dev_ref_clk_ctrl(), which comes before where this change kicks
start,
so if I remove the delay in ufs_qcom_dev_ref_clk_ctrl(), this change
cannot
provide us the correct delay before gate the ref_clk provided to UFS
device.
> Always putting the delay in the entrance of ufshcd_setup_clocks() may
> add unwanted delay for vendors, just like your current implementation,
> or some other vendors who do not want to disable the reference clock.
I meant if we put the delay in the entrance, I will be able to remove
the delay in ufs_qcom_dev_ref_clk_ctrl(). Meanwhile, we can add proper
checks before the delay to make sure it is initiated only if ref_clk
needs
to be disabled, i.e:
if(!on && !skip_ref_clk && hba->dev_info.clk_gating_wait_us)
usleep_range();
Does this look better to you?
Anyways, we will see regressions with this change on our platforms, can
we
have more discussions before get it merged? It should be OK if you go
with
patch #2 alone first, right? Thanks.
Best regards,
Can Guo.
>> Meanwhile, if you want to modify the delay
>> (hba->dev_info.clk_gating_wait_us) for some reasons, say for specific
>> UFS devices, you still can do it in vops_apply_dev_quirks().
>>
>> What do you say?
>>
>> Thanks,
>> Can Guo.
>
> Thanks,
> Stanley Chu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
2020-02-19 10:33 ` Can Guo
(?)
@ 2020-02-20 13:30 ` Stanley Chu
-1 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-20 13:30 UTC (permalink / raw)
To: Can Guo
Cc: linux-scsi, martin.petersen, Asutosh Das, hongwus, avri.altman,
alim.akhtar, jejb, beanhuo, matthias.bgg, bvanassche,
linux-mediatek, linux-arm-kernel, linux-kernel, kuohong.wang,
peter.wang, chun-hung.wu, andy.teng
Hi Can,
On Wed, 2020-02-19 at 18:33 +0800, Can Guo wrote:
> Hi Stanley,
>
> On 2020-02-19 17:11, Stanley Chu wrote:
> > Hi Can,
> >
> > On Wed, 2020-02-19 at 10:35 +0800, Can Guo wrote:
> >
> >> Since we all need this delay here, how about put the delay in the
> >> entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
> >> If so, we can remove all the delays we added in our vops since the
> >> delay anyways delays everything inside ufshcd_setup_clocks().
> >>
> >
> > Always putting the delay in the entrance of ufshcd_setup_clocks() may
> > add unwanted delay for vendors, just like your current implementation,
> > or some other vendors who do not want to disable the reference clock.
> >
> > I think current patch is more reasonable because the delay is applied
> > to
> > clock only named as "ref_clk" specifically.
> >
> > If you needs to keep "ref_clk" in DT, would you consider to remove the
> > delay in your ufs_qcom_dev_ref_clk_ctrl() and let the delay happens via
> > common ufshcd_setup_clocks() only? However you may still need delay if
> > call path comes from ufs_qcom_pwr_change_notify().
> >
> > What do you think?
> >
>
> I agree current change is more reasonable from what it looks, but the
> fact
> is that I canont remove the delay in ufs_qcom_dev_ref_clk_ctrl() even
> with
> this change. On our platforms, ref_clk in DT serves multipule purposes,
> the ref_clk provided to UFS device is actually controlled in
> ufs_qcom_dev_ref_clk_ctrl(), which comes before where this change kicks
> start,
> so if I remove the delay in ufs_qcom_dev_ref_clk_ctrl(), this change
> cannot
> provide us the correct delay before gate the ref_clk provided to UFS
> device.
> > Always putting the delay in the entrance of ufshcd_setup_clocks() may
> > add unwanted delay for vendors, just like your current implementation,
> > or some other vendors who do not want to disable the reference clock.
>
> I meant if we put the delay in the entrance, I will be able to remove
> the delay in ufs_qcom_dev_ref_clk_ctrl(). Meanwhile, we can add proper
> checks before the delay to make sure it is initiated only if ref_clk
> needs
> to be disabled, i.e:
>
> if(!on && !skip_ref_clk && hba->dev_info.clk_gating_wait_us)
> usleep_range();
>
> Does this look better to you?
Firstly thanks so much for above details.
Again this statement may also add unwanted delay if some other vendors
does not have "ref_clk" in DT or they don't/can't disable the reference
clock provided to UFS device.
>
> Anyways, we will see regressions with this change on our platforms, can
> we
> have more discussions before get it merged? It should be OK if you go
> with
> patch #2 alone first, right? Thanks.
Now the fact is that this change will impact your flow and it seems no
solid conclusion yet. Sure I could drop patch #1 and submit patch #2
only first : )
Thanks,
Stanley Chu
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-20 13:30 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-20 13:30 UTC (permalink / raw)
To: Can Guo
Cc: chun-hung.wu, linux-scsi, martin.petersen, andy.teng, jejb,
peter.wang, kuohong.wang, linux-kernel, avri.altman,
linux-mediatek, linux-arm-kernel, alim.akhtar, matthias.bgg,
beanhuo, bvanassche, hongwus, Asutosh Das
Hi Can,
On Wed, 2020-02-19 at 18:33 +0800, Can Guo wrote:
> Hi Stanley,
>
> On 2020-02-19 17:11, Stanley Chu wrote:
> > Hi Can,
> >
> > On Wed, 2020-02-19 at 10:35 +0800, Can Guo wrote:
> >
> >> Since we all need this delay here, how about put the delay in the
> >> entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
> >> If so, we can remove all the delays we added in our vops since the
> >> delay anyways delays everything inside ufshcd_setup_clocks().
> >>
> >
> > Always putting the delay in the entrance of ufshcd_setup_clocks() may
> > add unwanted delay for vendors, just like your current implementation,
> > or some other vendors who do not want to disable the reference clock.
> >
> > I think current patch is more reasonable because the delay is applied
> > to
> > clock only named as "ref_clk" specifically.
> >
> > If you needs to keep "ref_clk" in DT, would you consider to remove the
> > delay in your ufs_qcom_dev_ref_clk_ctrl() and let the delay happens via
> > common ufshcd_setup_clocks() only? However you may still need delay if
> > call path comes from ufs_qcom_pwr_change_notify().
> >
> > What do you think?
> >
>
> I agree current change is more reasonable from what it looks, but the
> fact
> is that I canont remove the delay in ufs_qcom_dev_ref_clk_ctrl() even
> with
> this change. On our platforms, ref_clk in DT serves multipule purposes,
> the ref_clk provided to UFS device is actually controlled in
> ufs_qcom_dev_ref_clk_ctrl(), which comes before where this change kicks
> start,
> so if I remove the delay in ufs_qcom_dev_ref_clk_ctrl(), this change
> cannot
> provide us the correct delay before gate the ref_clk provided to UFS
> device.
> > Always putting the delay in the entrance of ufshcd_setup_clocks() may
> > add unwanted delay for vendors, just like your current implementation,
> > or some other vendors who do not want to disable the reference clock.
>
> I meant if we put the delay in the entrance, I will be able to remove
> the delay in ufs_qcom_dev_ref_clk_ctrl(). Meanwhile, we can add proper
> checks before the delay to make sure it is initiated only if ref_clk
> needs
> to be disabled, i.e:
>
> if(!on && !skip_ref_clk && hba->dev_info.clk_gating_wait_us)
> usleep_range();
>
> Does this look better to you?
Firstly thanks so much for above details.
Again this statement may also add unwanted delay if some other vendors
does not have "ref_clk" in DT or they don't/can't disable the reference
clock provided to UFS device.
>
> Anyways, we will see regressions with this change on our platforms, can
> we
> have more discussions before get it merged? It should be OK if you go
> with
> patch #2 alone first, right? Thanks.
Now the fact is that this change will impact your flow and it seems no
solid conclusion yet. Sure I could drop patch #1 and submit patch #2
only first : )
Thanks,
Stanley Chu
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v1 1/2] scsi: ufs: add required delay after gating reference clock
@ 2020-02-20 13:30 ` Stanley Chu
0 siblings, 0 replies; 42+ messages in thread
From: Stanley Chu @ 2020-02-20 13:30 UTC (permalink / raw)
To: Can Guo
Cc: chun-hung.wu, linux-scsi, martin.petersen, andy.teng, jejb,
peter.wang, kuohong.wang, linux-kernel, avri.altman,
linux-mediatek, linux-arm-kernel, alim.akhtar, matthias.bgg,
beanhuo, bvanassche, hongwus, Asutosh Das
Hi Can,
On Wed, 2020-02-19 at 18:33 +0800, Can Guo wrote:
> Hi Stanley,
>
> On 2020-02-19 17:11, Stanley Chu wrote:
> > Hi Can,
> >
> > On Wed, 2020-02-19 at 10:35 +0800, Can Guo wrote:
> >
> >> Since we all need this delay here, how about put the delay in the
> >> entrence of ufshcd_setup_clocks(), before vops_setup_clocks()?
> >> If so, we can remove all the delays we added in our vops since the
> >> delay anyways delays everything inside ufshcd_setup_clocks().
> >>
> >
> > Always putting the delay in the entrance of ufshcd_setup_clocks() may
> > add unwanted delay for vendors, just like your current implementation,
> > or some other vendors who do not want to disable the reference clock.
> >
> > I think current patch is more reasonable because the delay is applied
> > to
> > clock only named as "ref_clk" specifically.
> >
> > If you needs to keep "ref_clk" in DT, would you consider to remove the
> > delay in your ufs_qcom_dev_ref_clk_ctrl() and let the delay happens via
> > common ufshcd_setup_clocks() only? However you may still need delay if
> > call path comes from ufs_qcom_pwr_change_notify().
> >
> > What do you think?
> >
>
> I agree current change is more reasonable from what it looks, but the
> fact
> is that I canont remove the delay in ufs_qcom_dev_ref_clk_ctrl() even
> with
> this change. On our platforms, ref_clk in DT serves multipule purposes,
> the ref_clk provided to UFS device is actually controlled in
> ufs_qcom_dev_ref_clk_ctrl(), which comes before where this change kicks
> start,
> so if I remove the delay in ufs_qcom_dev_ref_clk_ctrl(), this change
> cannot
> provide us the correct delay before gate the ref_clk provided to UFS
> device.
> > Always putting the delay in the entrance of ufshcd_setup_clocks() may
> > add unwanted delay for vendors, just like your current implementation,
> > or some other vendors who do not want to disable the reference clock.
>
> I meant if we put the delay in the entrance, I will be able to remove
> the delay in ufs_qcom_dev_ref_clk_ctrl(). Meanwhile, we can add proper
> checks before the delay to make sure it is initiated only if ref_clk
> needs
> to be disabled, i.e:
>
> if(!on && !skip_ref_clk && hba->dev_info.clk_gating_wait_us)
> usleep_range();
>
> Does this look better to you?
Firstly thanks so much for above details.
Again this statement may also add unwanted delay if some other vendors
does not have "ref_clk" in DT or they don't/can't disable the reference
clock provided to UFS device.
>
> Anyways, we will see regressions with this change on our platforms, can
> we
> have more discussions before get it merged? It should be OK if you go
> with
> patch #2 alone first, right? Thanks.
Now the fact is that this change will impact your flow and it seems no
solid conclusion yet. Sure I could drop patch #1 and submit patch #2
only first : )
Thanks,
Stanley Chu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2020-02-20 13:31 UTC | newest]
Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-17 9:35 [PATCH v1 0/2] scsi: ufs: fix waiting time for reference clock Stanley Chu
2020-02-17 9:35 ` Stanley Chu
2020-02-17 9:35 ` Stanley Chu
2020-02-17 9:35 ` [PATCH v1 1/2] scsi: ufs: add required delay after gating " Stanley Chu
2020-02-17 9:35 ` Stanley Chu
2020-02-17 9:35 ` Stanley Chu
2020-02-17 12:58 ` Can Guo
2020-02-17 12:58 ` Can Guo
2020-02-17 12:58 ` Can Guo
2020-02-17 13:12 ` Stanley Chu
2020-02-17 13:12 ` Stanley Chu
2020-02-17 13:12 ` Stanley Chu
2020-02-17 13:22 ` Can Guo
2020-02-17 13:22 ` Can Guo
2020-02-17 13:22 ` Can Guo
2020-02-17 13:34 ` Stanley Chu
2020-02-17 13:34 ` Stanley Chu
2020-02-17 13:34 ` Stanley Chu
2020-02-17 13:42 ` Can Guo
2020-02-17 13:42 ` Can Guo
2020-02-17 13:42 ` Can Guo
2020-02-19 2:35 ` Can Guo
2020-02-19 2:35 ` Can Guo
2020-02-19 2:35 ` Can Guo
2020-02-19 9:11 ` Stanley Chu
2020-02-19 9:11 ` Stanley Chu
2020-02-19 9:11 ` Stanley Chu
2020-02-19 10:33 ` Can Guo
2020-02-19 10:33 ` Can Guo
2020-02-19 10:33 ` Can Guo
2020-02-20 13:30 ` Stanley Chu
2020-02-20 13:30 ` Stanley Chu
2020-02-20 13:30 ` Stanley Chu
2020-02-17 16:17 ` Bart Van Assche
2020-02-17 16:17 ` Bart Van Assche
2020-02-17 16:17 ` Bart Van Assche
2020-02-18 0:50 ` Stanley Chu
2020-02-18 0:50 ` Stanley Chu
2020-02-18 0:50 ` Stanley Chu
2020-02-17 9:35 ` [PATCH v1 2/2] scsi: ufs: ufs-mediatek: add waiting time for " Stanley Chu
2020-02-17 9:35 ` Stanley Chu
2020-02-17 9:35 ` Stanley Chu
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.