2020-01-29 07:40:19

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v3 0/4] MediaTek UFS vendor implemenation part III and Auto-Hibern8 fix

Hi,

This series provides MediaTek vendor implementations and some general fixes.

- General fixes
- Fix Auto-Hibern8 error detection

- MediaTek vendor implementations
- Ensure UniPro is powered on before every link startup
- Support linkoff state during suspend
- Gate reference clock for Auto-Hibern8 case

v2 -> v3
- Squash below patches to a single patch (Bean)
- scsi: ufs: add ufshcd_is_auto_hibern8_enabled facility
- scsi: ufs: fix auto-hibern8 error detection
- Add Fixes tag in patch "scsi: ufs: fix Auto-Hibern8 error detection" (Bean)
- Rename VS_LINK_HIBER8 to VS_LINK_HIBERN8 in patch "scsi: ufs-mediatek: gate ref-clk during Auto-Hibern8"

v1 -> v2
- Fix and refine commit messages.

Stanley Chu (4):
scsi: ufs-mediatek: ensure UniPro is not powered down before linkup
scsi: ufs-mediatek: support linkoff state during suspend
scsi: ufs: fix Auto-Hibern8 error detection
scsi: ufs-mediatek: gate ref-clk during Auto-Hibern8

drivers/scsi/ufs/ufs-mediatek.c | 67 +++++++++++++++++++++------------
drivers/scsi/ufs/ufs-mediatek.h | 12 ++++++
drivers/scsi/ufs/ufshcd.c | 3 +-
drivers/scsi/ufs/ufshcd.h | 6 +++
4 files changed, 63 insertions(+), 25 deletions(-)

--
2.18.0


2020-01-29 07:40:25

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v3 4/4] scsi: ufs-mediatek: gate ref-clk during Auto-Hibern8

In current UFS driver design, hba->uic_link_state will not
be changed after link enters Hibern8 state by Auto-Hibern8 mechanism.
In this case, reference clock gating will be skipped unless special
handling is implemented in vendor's callbacks.

Support reference clock gating during Auto-Hibern8 period in
MediaTek Chipsets: If link state is already in Hibern8 while
Auto-Hibern8 feature is enabled, gate reference clock in
setup_clocks callback.

Signed-off-by: Stanley Chu <[email protected]>
---
drivers/scsi/ufs/ufs-mediatek.c | 38 +++++++++++++++++++++++----------
drivers/scsi/ufs/ufs-mediatek.h | 12 +++++++++++
2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index d78897a14905..0ce08872d671 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -143,6 +143,17 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
return 0;
}

+static u32 ufs_mtk_link_get_state(struct ufs_hba *hba)
+{
+ u32 val;
+
+ ufshcd_writel(hba, 0x20, REG_UFS_DEBUG_SEL);
+ val = ufshcd_readl(hba, REG_UFS_PROBE);
+ val = val >> 28;
+
+ return val;
+}
+
/**
* ufs_mtk_setup_clocks - enables/disable clocks
* @hba: host controller instance
@@ -155,7 +166,7 @@ static int ufs_mtk_setup_clocks(struct ufs_hba *hba, bool on,
enum ufs_notify_change_status status)
{
struct ufs_mtk_host *host = ufshcd_get_variant(hba);
- int ret = -EINVAL;
+ int ret = 0;

/*
* In case ufs_mtk_init() is not yet done, simply ignore.
@@ -165,19 +176,24 @@ static int ufs_mtk_setup_clocks(struct ufs_hba *hba, bool on,
if (!host)
return 0;

- switch (status) {
- case PRE_CHANGE:
- if (!on && !ufshcd_is_link_active(hba)) {
+ if (!on && status == PRE_CHANGE) {
+ if (!ufshcd_is_link_active(hba)) {
ufs_mtk_setup_ref_clk(hba, on);
ret = phy_power_off(host->mphy);
+ } else {
+ /*
+ * Gate ref-clk if link state is in Hibern8
+ * triggered by Auto-Hibern8.
+ */
+ if (!ufshcd_can_hibern8_during_gating(hba) &&
+ ufshcd_is_auto_hibern8_enabled(hba) &&
+ ufs_mtk_link_get_state(hba) ==
+ VS_LINK_HIBERN8)
+ ufs_mtk_setup_ref_clk(hba, on);
}
- break;
- case POST_CHANGE:
- if (on) {
- ret = phy_power_on(host->mphy);
- ufs_mtk_setup_ref_clk(hba, on);
- }
- break;
+ } else if (on && status == POST_CHANGE) {
+ ret = phy_power_on(host->mphy);
+ ufs_mtk_setup_ref_clk(hba, on);
}

return ret;
diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
index fccdd979d6fb..492414e5f481 100644
--- a/drivers/scsi/ufs/ufs-mediatek.h
+++ b/drivers/scsi/ufs/ufs-mediatek.h
@@ -53,6 +53,18 @@
#define VS_SAVEPOWERCONTROL 0xD0A6
#define VS_UNIPROPOWERDOWNCONTROL 0xD0A8

+/*
+ * Vendor specific link state
+ */
+enum {
+ VS_LINK_DISABLED = 0,
+ VS_LINK_DOWN = 1,
+ VS_LINK_UP = 2,
+ VS_LINK_HIBERN8 = 3,
+ VS_LINK_LOST = 4,
+ VS_LINK_CFG = 5,
+};
+
/*
* SiP commands
*/
--
2.18.0

2020-01-29 07:40:39

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v3 2/4] scsi: ufs-mediatek: support linkoff state during suspend

If system suspend or runtime suspend mode is configured as
linkoff state, phy can be powered off and reference clock
can be gated in MediaTek Chipsets.

In the same time, remove redundant reference clock control
in suspend and resume callbacks because such control can be
well-handled in setup_clocks callback..

Signed-off-by: Stanley Chu <[email protected]>
---
drivers/scsi/ufs/ufs-mediatek.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 7ac838cc15d1..d78897a14905 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -167,7 +167,7 @@ static int ufs_mtk_setup_clocks(struct ufs_hba *hba, bool on,

switch (status) {
case PRE_CHANGE:
- if (!on) {
+ if (!on && !ufshcd_is_link_active(hba)) {
ufs_mtk_setup_ref_clk(hba, on);
ret = phy_power_off(host->mphy);
}
@@ -437,10 +437,11 @@ static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
err = ufs_mtk_link_set_lpm(hba);
if (err)
return -EAGAIN;
- phy_power_off(host->mphy);
- ufs_mtk_setup_ref_clk(hba, false);
}

+ if (!ufshcd_is_link_active(hba))
+ phy_power_off(host->mphy);
+
return 0;
}

@@ -449,9 +450,10 @@ static int ufs_mtk_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
struct ufs_mtk_host *host = ufshcd_get_variant(hba);
int err;

- if (ufshcd_is_link_hibern8(hba)) {
- ufs_mtk_setup_ref_clk(hba, true);
+ if (!ufshcd_is_link_active(hba))
phy_power_on(host->mphy);
+
+ if (ufshcd_is_link_hibern8(hba)) {
err = ufs_mtk_link_set_hpm(hba);
if (err)
return err;
--
2.18.0

2020-01-29 07:41:09

by Stanley Chu

[permalink] [raw]
Subject: [PATCH v3 3/4] scsi: ufs: fix Auto-Hibern8 error detection

Auto-Hibern8 may be disabled by some vendors or sysfs
in runtime even if Auto-Hibern8 capability is supported
by host. If Auto-Hibern8 capability is supported by host
but not actually enabled, Auto-Hibern8 error shall not happen.

To fix this, provide a way to detect if Auto-Hibern8 is
actually enabled first, and bypass Auto-Hibern8 disabling
case in ufshcd_is_auto_hibern8_error().

Fixes: 8217444 ("scsi: ufs: Add error-handling of Auto-Hibernate")
Cc: [email protected]
Signed-off-by: Stanley Chu <[email protected]>
Reviewed-by: Bean Huo <[email protected]>
---
drivers/scsi/ufs/ufshcd.c | 3 ++-
drivers/scsi/ufs/ufshcd.h | 6 ++++++
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index abd0e6b05f79..214a3f373dd8 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -5479,7 +5479,8 @@ static irqreturn_t ufshcd_update_uic_error(struct ufs_hba *hba)
static bool ufshcd_is_auto_hibern8_error(struct ufs_hba *hba,
u32 intr_mask)
{
- if (!ufshcd_is_auto_hibern8_supported(hba))
+ if (!ufshcd_is_auto_hibern8_supported(hba) ||
+ !ufshcd_is_auto_hibern8_enabled(hba))
return false;

if (!(intr_mask & UFSHCD_UIC_HIBERN8_MASK))
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 2ae6c7c8528c..81c71a3e3474 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -55,6 +55,7 @@
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/regulator/consumer.h>
+#include <linux/bitfield.h>
#include "unipro.h"

#include <asm/irq.h>
@@ -773,6 +774,11 @@ static inline bool ufshcd_is_auto_hibern8_supported(struct ufs_hba *hba)
return (hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT);
}

+static inline bool ufshcd_is_auto_hibern8_enabled(struct ufs_hba *hba)
+{
+ return FIELD_GET(UFSHCI_AHIBERN8_TIMER_MASK, hba->ahit) ? true : false;
+}
+
#define ufshcd_writel(hba, val, reg) \
writel((val), (hba)->mmio_base + (reg))
#define ufshcd_readl(hba, reg) \
--
2.18.0

2020-01-29 07:53:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] scsi: ufs: fix Auto-Hibern8 error detection

On Wed, Jan 29, 2020 at 03:39:01PM +0800, Stanley Chu wrote:
> Auto-Hibern8 may be disabled by some vendors or sysfs
> in runtime even if Auto-Hibern8 capability is supported
> by host. If Auto-Hibern8 capability is supported by host
> but not actually enabled, Auto-Hibern8 error shall not happen.
>
> To fix this, provide a way to detect if Auto-Hibern8 is
> actually enabled first, and bypass Auto-Hibern8 disabling
> case in ufshcd_is_auto_hibern8_error().
>
> Fixes: 8217444 ("scsi: ufs: Add error-handling of Auto-Hibernate")

This should be:
Fixes: 821744403913 ("scsi: ufs: Add error-handling of Auto-Hibernate")

2020-01-29 10:48:18

by Stanley Chu

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] scsi: ufs: fix Auto-Hibern8 error detection

Hi Greg,

On Wed, 2020-01-29 at 08:52 +0100, Greg KH wrote:

> > Fixes: 8217444 ("scsi: ufs: Add error-handling of Auto-Hibernate")
>
> This should be:
> Fixes: 821744403913 ("scsi: ufs: Add error-handling of Auto-Hibernate")

Thanks for remind. I'll resend this patch with updated tag.

Stanley

2020-02-02 14:38:37

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] scsi: ufs-mediatek: gate ref-clk during Auto-Hibern8

Hello Stanley

On Wed, Jan 29, 2020 at 1:10 PM Stanley Chu <[email protected]> wrote:
>
> In current UFS driver design, hba->uic_link_state will not
> be changed after link enters Hibern8 state by Auto-Hibern8 mechanism.
> In this case, reference clock gating will be skipped unless special
> handling is implemented in vendor's callbacks.
>
> Support reference clock gating during Auto-Hibern8 period in
> MediaTek Chipsets: If link state is already in Hibern8 while
> Auto-Hibern8 feature is enabled, gate reference clock in
> setup_clocks callback.
>
> Signed-off-by: Stanley Chu <[email protected]>

Reviewed-by: Alim Akhtar <[email protected]>
> ---
> drivers/scsi/ufs/ufs-mediatek.c | 38 +++++++++++++++++++++++----------
> drivers/scsi/ufs/ufs-mediatek.h | 12 +++++++++++
> 2 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
> index d78897a14905..0ce08872d671 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.c
> +++ b/drivers/scsi/ufs/ufs-mediatek.c
> @@ -143,6 +143,17 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
> return 0;
> }
>
> +static u32 ufs_mtk_link_get_state(struct ufs_hba *hba)
> +{
> + u32 val;
> +
> + ufshcd_writel(hba, 0x20, REG_UFS_DEBUG_SEL);
> + val = ufshcd_readl(hba, REG_UFS_PROBE);
> + val = val >> 28;
> +
> + return val;
> +}
> +
> /**
> * ufs_mtk_setup_clocks - enables/disable clocks
> * @hba: host controller instance
> @@ -155,7 +166,7 @@ static int ufs_mtk_setup_clocks(struct ufs_hba *hba, bool on,
> enum ufs_notify_change_status status)
> {
> struct ufs_mtk_host *host = ufshcd_get_variant(hba);
> - int ret = -EINVAL;
> + int ret = 0;
>
> /*
> * In case ufs_mtk_init() is not yet done, simply ignore.
> @@ -165,19 +176,24 @@ static int ufs_mtk_setup_clocks(struct ufs_hba *hba, bool on,
> if (!host)
> return 0;
>
> - switch (status) {
> - case PRE_CHANGE:
> - if (!on && !ufshcd_is_link_active(hba)) {
> + if (!on && status == PRE_CHANGE) {
> + if (!ufshcd_is_link_active(hba)) {
> ufs_mtk_setup_ref_clk(hba, on);
> ret = phy_power_off(host->mphy);
> + } else {
> + /*
> + * Gate ref-clk if link state is in Hibern8
> + * triggered by Auto-Hibern8.
> + */
> + if (!ufshcd_can_hibern8_during_gating(hba) &&
> + ufshcd_is_auto_hibern8_enabled(hba) &&
> + ufs_mtk_link_get_state(hba) ==
> + VS_LINK_HIBERN8)
> + ufs_mtk_setup_ref_clk(hba, on);
> }
> - break;
> - case POST_CHANGE:
> - if (on) {
> - ret = phy_power_on(host->mphy);
> - ufs_mtk_setup_ref_clk(hba, on);
> - }
> - break;
> + } else if (on && status == POST_CHANGE) {
> + ret = phy_power_on(host->mphy);
> + ufs_mtk_setup_ref_clk(hba, on);
> }
>
> return ret;
> diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
> index fccdd979d6fb..492414e5f481 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.h
> +++ b/drivers/scsi/ufs/ufs-mediatek.h
> @@ -53,6 +53,18 @@
> #define VS_SAVEPOWERCONTROL 0xD0A6
> #define VS_UNIPROPOWERDOWNCONTROL 0xD0A8
>
> +/*
> + * Vendor specific link state
> + */
> +enum {
> + VS_LINK_DISABLED = 0,
> + VS_LINK_DOWN = 1,
> + VS_LINK_UP = 2,
> + VS_LINK_HIBERN8 = 3,
> + VS_LINK_LOST = 4,
> + VS_LINK_CFG = 5,
> +};
> +
> /*
> * SiP commands
> */
> --
> 2.18.0



--
Regards,
Alim

2020-02-02 14:39:05

by Alim Akhtar

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] scsi: ufs-mediatek: support linkoff state during suspend

Hi Stanley

On Wed, Jan 29, 2020 at 1:11 PM Stanley Chu <[email protected]> wrote:
>
> If system suspend or runtime suspend mode is configured as
> linkoff state, phy can be powered off and reference clock
> can be gated in MediaTek Chipsets.
>
> In the same time, remove redundant reference clock control
> in suspend and resume callbacks because such control can be
> well-handled in setup_clocks callback..
>
> Signed-off-by: Stanley Chu <[email protected]>
Reviewed-by: Alim Akhtar <[email protected]>

> ---
> drivers/scsi/ufs/ufs-mediatek.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
> index 7ac838cc15d1..d78897a14905 100644
> --- a/drivers/scsi/ufs/ufs-mediatek.c
> +++ b/drivers/scsi/ufs/ufs-mediatek.c
> @@ -167,7 +167,7 @@ static int ufs_mtk_setup_clocks(struct ufs_hba *hba, bool on,
>
> switch (status) {
> case PRE_CHANGE:
> - if (!on) {
> + if (!on && !ufshcd_is_link_active(hba)) {
> ufs_mtk_setup_ref_clk(hba, on);
> ret = phy_power_off(host->mphy);
> }
> @@ -437,10 +437,11 @@ static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> err = ufs_mtk_link_set_lpm(hba);
> if (err)
> return -EAGAIN;
> - phy_power_off(host->mphy);
> - ufs_mtk_setup_ref_clk(hba, false);
> }
>
> + if (!ufshcd_is_link_active(hba))
> + phy_power_off(host->mphy);
> +
> return 0;
> }
>
> @@ -449,9 +450,10 @@ static int ufs_mtk_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> struct ufs_mtk_host *host = ufshcd_get_variant(hba);
> int err;
>
> - if (ufshcd_is_link_hibern8(hba)) {
> - ufs_mtk_setup_ref_clk(hba, true);
> + if (!ufshcd_is_link_active(hba))
> phy_power_on(host->mphy);
> +
> + if (ufshcd_is_link_hibern8(hba)) {
> err = ufs_mtk_link_set_hpm(hba);
> if (err)
> return err;
> --
> 2.18.0



--
Regards,
Alim