[PATCH 1/2] dt-bindings: usb: mediatek,mtk-xhci: add keep-clock-on

[PATCH 1/2] dt-bindings: usb: mediatek,mtk-xhci: add keep-clock-on

[Macpaul Lin]

Option "mediatek,keep-clock-on" means to keep clock on during system
suspend and resume. Some platform will flush register settings if clock has
been disabled when system is suspended. Set this option to avoid clock off.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index ea696c8..a956dde 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -104,6 +104,12 @@ properties:
     description: enable USB remote wakeup, see power/wakeup-source.txt
     type: boolean
 
+  mediatek,keep-clock-on:
+    description: |
+      Keep clock on during system suspend and resume. Some platform will flush
+      register settings if clock has been disabled when system is suspended.
+    type: boolean
+
   mediatek,syscon-wakeup:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 1
@@ -175,6 +181,7 @@ examples:
         imod-interval-ns = <10000>;
         mediatek,syscon-wakeup = <&pericfg 0x400 1>;
         wakeup-source;
+        mediatek,keep-clock-on;
         usb3-lpm-capable;
     };
 ...
-- 
1.7.9.5

[PATCH 2/2] usb: host: XHCI: xhci-mtk.c: support mediatek,keep-clock-on

[Macpaul Lin]

Some platform dose not support turn off clock when system suspending.
We add an option "mediatek,keep-clock-on" for distinquish these platforms.
When "mediatek,keep-clock-on" has been set, xhci-mtk driver will skip
turning clock on and off during system suspend and resume.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c |    9 +++++++--
 drivers/usb/host/xhci-mtk.h |    1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 4311d4c..c6c2804 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -464,6 +464,9 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	of_property_read_u32(node, "mediatek,u3p-dis-msk",
 			     &mtk->u3p_dis_msk);
 
+	/* keep clock on when suspending on some platform */
+	mtk->keep_clk_on = of_property_read_bool(node, "mediatek,keep-clock-on");
+
 	ret = usb_wakeup_of_property_parse(mtk, node);
 	if (ret) {
 		dev_err(dev, "failed to parse uwk property\n");
@@ -624,7 +627,8 @@ static int __maybe_unused xhci_mtk_suspend(struct device *dev)
 	del_timer_sync(&xhci->shared_hcd->rh_timer);
 
 	xhci_mtk_host_disable(mtk);
-	xhci_mtk_clks_disable(mtk);
+	if (!mtk->keep_clk_on)
+		xhci_mtk_clks_disable(mtk);
 	usb_wakeup_set(mtk, true);
 	return 0;
 }
@@ -636,7 +640,8 @@ static int __maybe_unused xhci_mtk_resume(struct device *dev)
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 
 	usb_wakeup_set(mtk, false);
-	xhci_mtk_clks_enable(mtk);
+	if (!mtk->keep_clk_on)
+		xhci_mtk_clks_enable(mtk);
 	xhci_mtk_host_enable(mtk);
 
 	xhci_dbg(xhci, "%s: restart port polling\n", __func__);
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index a93cfe8..37639c5 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -152,6 +152,7 @@ struct xhci_hcd_mtk {
 	struct regmap *uwk;
 	u32 uwk_reg_base;
 	u32 uwk_vers;
+	bool keep_clk_on;
 };
 
 static inline struct xhci_hcd_mtk *hcd_to_mtk(struct usb_hcd *hcd)
-- 
1.7.9.5

Re: [PATCH 1/2] dt-bindings: usb: mediatek,mtk-xhci: add keep-clock-on

[Chunfeng Yun]

Hi Macpaul,

On Mon, 2020-11-02 at 15:18 +0800, Macpaul Lin wrote:
> Option "mediatek,keep-clock-on" means to keep clock on during system
> suspend and resume. Some platform will flush register settings if clock has
> been disabled when system is suspended. Set this option to avoid clock off.
> 
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
>  .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml |    7 +++++++

This depends on reviewing patch:
https://patchwork.kernel.org/project/linux-mediatek/patch/20201014014434.62239-6-chunfeng.yun@mediatek.com/
[v2,6/8] dt-bindings: usb: convert mediatek, mtk-xhci.txt to YAML schema

Please add it after "---", thanks


>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> index ea696c8..a956dde 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -104,6 +104,12 @@ properties:
>      description: enable USB remote wakeup, see power/wakeup-source.txt
>      type: boolean
>  
> +  mediatek,keep-clock-on:
> +    description: |
> +      Keep clock on during system suspend and resume. Some platform will flush
> +      register settings if clock has been disabled when system is suspended.
> +    type: boolean
> +
>    mediatek,syscon-wakeup:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      maxItems: 1
> @@ -175,6 +181,7 @@ examples:
>          imod-interval-ns = <10000>;
>          mediatek,syscon-wakeup = <&pericfg 0x400 1>;
>          wakeup-source;
> +        mediatek,keep-clock-on;
>          usb3-lpm-capable;
>      };
>  ...

Re: [PATCH 1/2] dt-bindings: usb: mediatek,mtk-xhci: add keep-clock-on

[Rob Herring]

On Mon, Nov 02, 2020 at 03:18:48PM +0800, Macpaul Lin wrote:
> Option "mediatek,keep-clock-on" means to keep clock on during system
> suspend and resume. Some platform will flush register settings if clock has
> been disabled when system is suspended. Set this option to avoid clock off.
> 
> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
>  .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> index ea696c8..a956dde 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> @@ -104,6 +104,12 @@ properties:
>      description: enable USB remote wakeup, see power/wakeup-source.txt
>      type: boolean
>  
> +  mediatek,keep-clock-on:
> +    description: |
> +      Keep clock on during system suspend and resume. Some platform will flush
> +      register settings if clock has been disabled when system is suspended.
> +    type: boolean
> +

This should be implied by the compatible string.

>    mediatek,syscon-wakeup:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      maxItems: 1
> @@ -175,6 +181,7 @@ examples:
>          imod-interval-ns = <10000>;
>          mediatek,syscon-wakeup = <&pericfg 0x400 1>;
>          wakeup-source;
> +        mediatek,keep-clock-on;
>          usb3-lpm-capable;
>      };
>  ...
> -- 
> 1.7.9.5

Re: [PATCH 1/2] dt-bindings: usb: mediatek,mtk-xhci: add keep-clock-on

[Macpaul Lin]

On Wed, 2020-11-04 at 16:39 -0600, Rob Herring wrote:
> On Mon, Nov 02, 2020 at 03:18:48PM +0800, Macpaul Lin wrote:
> > Option "mediatek,keep-clock-on" means to keep clock on during system
> > suspend and resume. Some platform will flush register settings if clock has
> > been disabled when system is suspended. Set this option to avoid clock off.
> > 
> > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> > ---
> >  .../devicetree/bindings/usb/mediatek,mtk-xhci.yaml |    7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > index ea696c8..a956dde 100644
> > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
> > @@ -104,6 +104,12 @@ properties:
> >      description: enable USB remote wakeup, see power/wakeup-source.txt
> >      type: boolean
> >  
> > +  mediatek,keep-clock-on:
> > +    description: |
> > +      Keep clock on during system suspend and resume. Some platform will flush
> > +      register settings if clock has been disabled when system is suspended.
> > +    type: boolean
> > +
> 
> This should be implied by the compatible string.

This should be an property according to system-wide design.

Mtk-xhci may be applied to different product lines for the same platform
(IC). Suspend on phone system is different to tablet or laptop. Phone's
power management module will turn off XHCI's power once the clock has
been turned off. For example, the headset plugged into phone won't do
disconnect and re-enumeration during system suspend. Click a button on
the headset to wake-up phone is necessary, and then, XHCI needs ready to
work immediately.

If the IC has been applied to a tablet or laptop product. When system is
suspending, the headset will be disconnected. The headset will do
re-enumerate when system is waking up. In this kind of applications, the
power of XHCI can be turned off with clock.

> >    mediatek,syscon-wakeup:
> >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >      maxItems: 1
> > @@ -175,6 +181,7 @@ examples:
> >          imod-interval-ns = <10000>;
> >          mediatek,syscon-wakeup = <&pericfg 0x400 1>;
> >          wakeup-source;
> > +        mediatek,keep-clock-on;
> >          usb3-lpm-capable;
> >      };
> >  ...
> > -- 
> > 1.7.9.5

After a discussion with Chunfeng, I'll send a new version for 
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt because the
YAML file still need to be revised. The property
"mediatek,keep-clock-on" will be renamed to "mediatek,str-clock-on" for
implying it relates to suspend/resume capability.

Thanks.
Macpaul Lin

[PATCH v2 1/2] dt-bindings: usb: mediatek,mtk-xhci: add str-clock-on

[Macpaul Lin]

Option "mediatek,str-clock-on" means to keep clock on during system
suspend and resume. Some platform will flush register settings if clock has
been disabled when system is suspended. Set this option to avoid clock off.

Change-Id: Id841f58e9d7fb3656511072b3eb14d0d355e2dd5
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
Changes for v2:
  - Rename "mediatek,keep-clock-on" to "mediatek,str-clock-on" which implies
    this option related to STR functions.
  - After discussion with Chunfeng, resend dt-bindings descritption based on
    mediatek,mtk-xhci.txt instead of yaml format.

 .../devicetree/bindings/usb/mediatek,mtk-xhci.txt  |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
index 42d8814..fc93bcf 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
@@ -37,6 +37,9 @@ Required properties:
 
 Optional properties:
  - wakeup-source : enable USB remote wakeup;
+ - mediatek,str-clock-on: Keep clock on during system suspend and resume.
+	Some platform will flush register settings if clock has been disabled
+	when system is suspended.
  - mediatek,syscon-wakeup : phandle to syscon used to access the register
 	of the USB wakeup glue layer between xHCI and SPM; it depends on
 	"wakeup-source", and has two arguments:
-- 
1.7.9.5

[PATCH v2 2/2] usb: host: XHCI: xhci-mtk.c: support mediatek,str-clock-on

[Macpaul Lin]

Some platform dose not support turn off clock when system suspending.
We add an option "mediatek,str-clock-on" for distinquish these platforms.
When "mediatek,str-clock-on" has been set, xhci-mtk driver will skip
turning clock on and off during system suspend and resume.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
Changes for v2:
  - Replace "mediatek,keep-clock-on" to "mediatek,str-clock-on" which implies
    this option related to STR functions.

 drivers/usb/host/xhci-mtk.c |    9 +++++++--
 drivers/usb/host/xhci-mtk.h |    1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 4311d4c..77b0d7a 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -464,6 +464,9 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	of_property_read_u32(node, "mediatek,u3p-dis-msk",
 			     &mtk->u3p_dis_msk);
 
+	/* STR: keep clock on when suspending on some platform */
+	mtk->str_clk_on = of_property_read_bool(node, "mediatek,str-clock-on");
+
 	ret = usb_wakeup_of_property_parse(mtk, node);
 	if (ret) {
 		dev_err(dev, "failed to parse uwk property\n");
@@ -624,7 +627,8 @@ static int __maybe_unused xhci_mtk_suspend(struct device *dev)
 	del_timer_sync(&xhci->shared_hcd->rh_timer);
 
 	xhci_mtk_host_disable(mtk);
-	xhci_mtk_clks_disable(mtk);
+	if (!mtk->str_clk_on)
+		xhci_mtk_clks_disable(mtk);
 	usb_wakeup_set(mtk, true);
 	return 0;
 }
@@ -636,7 +640,8 @@ static int __maybe_unused xhci_mtk_resume(struct device *dev)
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 
 	usb_wakeup_set(mtk, false);
-	xhci_mtk_clks_enable(mtk);
+	if (!mtk->str_clk_on)
+		xhci_mtk_clks_enable(mtk);
 	xhci_mtk_host_enable(mtk);
 
 	xhci_dbg(xhci, "%s: restart port polling\n", __func__);
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index a93cfe8..4039b025 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -152,6 +152,7 @@ struct xhci_hcd_mtk {
 	struct regmap *uwk;
 	u32 uwk_reg_base;
 	u32 uwk_vers;
+	bool str_clk_on;
 };
 
 static inline struct xhci_hcd_mtk *hcd_to_mtk(struct usb_hcd *hcd)
-- 
1.7.9.5

Re: [PATCH v2 1/2] dt-bindings: usb: mediatek,mtk-xhci: add str-clock-on

[Macpaul Lin]

On Fri, 2020-11-06 at 17:46 +0800, Macpaul Lin wrote:
> Option "mediatek,str-clock-on" means to keep clock on during system
> suspend and resume. Some platform will flush register settings if clock has
> been disabled when system is suspended. Set this option to avoid clock off.
> 
> Change-Id: Id841f58e9d7fb3656511072b3eb14d0d355e2dd5

Sorry I've found a Change-ID tag here,
I'll send patch v3.

> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
> ---
> Changes for v2:
>   - Rename "mediatek,keep-clock-on" to "mediatek,str-clock-on" which implies
>     this option related to STR functions.
>   - After discussion with Chunfeng, resend dt-bindings descritption based on
>     mediatek,mtk-xhci.txt instead of yaml format.
> 
>  .../devicetree/bindings/usb/mediatek,mtk-xhci.txt  |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> index 42d8814..fc93bcf 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> @@ -37,6 +37,9 @@ Required properties:
>  
>  Optional properties:
>   - wakeup-source : enable USB remote wakeup;
> + - mediatek,str-clock-on: Keep clock on during system suspend and resume.
> +	Some platform will flush register settings if clock has been disabled
> +	when system is suspended.
>   - mediatek,syscon-wakeup : phandle to syscon used to access the register
>  	of the USB wakeup glue layer between xHCI and SPM; it depends on
>  	"wakeup-source", and has two arguments:

Thanks
Macpaul Lin

[PATCH v3 1/2] dt-bindings: usb: mediatek,mtk-xhci: add str-clock-on

[Macpaul Lin]

Option "mediatek,str-clock-on" means to keep clock on during system
suspend and resume. Some platform will flush register settings if clock has
been disabled when system is suspended. Set this option to avoid clock off.

Fixes: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
Cc: stable@vger.kernel.org
---
Changes for v3:
  - Remove unnecessary Change-Id in commit message.
  - Add "Fixes" tag as a bug fix on phone system.
Changes for v2:
  - Rename "mediatek,keep-clock-on" to "mediatek,str-clock-on" which implies
    this option related to STR functions.
  - After discussion with Chunfeng, resend dt-bindings descritption based on
    mediatek,mtk-xhci.txt instead of yaml format.

 .../devicetree/bindings/usb/mediatek,mtk-xhci.txt  |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
index 42d8814..fc93bcf 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
@@ -37,6 +37,9 @@ Required properties:
 
 Optional properties:
  - wakeup-source : enable USB remote wakeup;
+ - mediatek,str-clock-on: Keep clock on during system suspend and resume.
+	Some platform will flush register settings if clock has been disabled
+	when system is suspended.
  - mediatek,syscon-wakeup : phandle to syscon used to access the register
 	of the USB wakeup glue layer between xHCI and SPM; it depends on
 	"wakeup-source", and has two arguments:
-- 
1.7.9.5

[PATCH v3 2/2] usb: host: XHCI: xhci-mtk.c: support mediatek,str-clock-on

[Macpaul Lin]

Some platform dose not support turn off clock when system suspending.
We add an option "mediatek,str-clock-on" for distinquish these platforms.
When "mediatek,str-clock-on" has been set, xhci-mtk driver will skip
turning clock on and off during system suspend and resume.

Fixes: 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
Cc: stable@vger.kernel.org
---
Changes for v3:
  - Add "Fixes" tag as a bug fix on phone system.
Changes for v2:
  - Replace "mediatek,keep-clock-on" to "mediatek,str-clock-on" which implies
    this option related to STR functions.

 drivers/usb/host/xhci-mtk.c |    9 +++++++--
 drivers/usb/host/xhci-mtk.h |    1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 4311d4c..77b0d7a 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -464,6 +464,9 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 	of_property_read_u32(node, "mediatek,u3p-dis-msk",
 			     &mtk->u3p_dis_msk);
 
+	/* STR: keep clock on when suspending on some platform */
+	mtk->str_clk_on = of_property_read_bool(node, "mediatek,str-clock-on");
+
 	ret = usb_wakeup_of_property_parse(mtk, node);
 	if (ret) {
 		dev_err(dev, "failed to parse uwk property\n");
@@ -624,7 +627,8 @@ static int __maybe_unused xhci_mtk_suspend(struct device *dev)
 	del_timer_sync(&xhci->shared_hcd->rh_timer);
 
 	xhci_mtk_host_disable(mtk);
-	xhci_mtk_clks_disable(mtk);
+	if (!mtk->str_clk_on)
+		xhci_mtk_clks_disable(mtk);
 	usb_wakeup_set(mtk, true);
 	return 0;
 }
@@ -636,7 +640,8 @@ static int __maybe_unused xhci_mtk_resume(struct device *dev)
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 
 	usb_wakeup_set(mtk, false);
-	xhci_mtk_clks_enable(mtk);
+	if (!mtk->str_clk_on)
+		xhci_mtk_clks_enable(mtk);
 	xhci_mtk_host_enable(mtk);
 
 	xhci_dbg(xhci, "%s: restart port polling\n", __func__);
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index a93cfe8..4039b025 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -152,6 +152,7 @@ struct xhci_hcd_mtk {
 	struct regmap *uwk;
 	u32 uwk_reg_base;
 	u32 uwk_vers;
+	bool str_clk_on;
 };
 
 static inline struct xhci_hcd_mtk *hcd_to_mtk(struct usb_hcd *hcd)
-- 
1.7.9.5