[PATCH 1/6] usb: mtu3: get resources that cause deferred probe earlier

[PATCH 1/6] usb: mtu3: get resources that cause deferred probe earlier

[Chunfeng Yun]

Some resources such as regulator, clock usually cause deferred
probe, get them earlier to avoid more ineffective processing.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/mtu3/mtu3_plat.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3_plat.c b/drivers/usb/mtu3/mtu3_plat.c
index 7833678..6344859 100644
--- a/drivers/usb/mtu3/mtu3_plat.c
+++ b/drivers/usb/mtu3/mtu3_plat.c
@@ -204,6 +204,18 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
 	int i;
 	int ret;
 
+	ssusb->vusb33 = devm_regulator_get(&pdev->dev, "vusb33");
+	if (IS_ERR(ssusb->vusb33)) {
+		dev_err(dev, "failed to get vusb33\n");
+		return PTR_ERR(ssusb->vusb33);
+	}
+
+	ssusb->sys_clk = devm_clk_get(dev, "sys_ck");
+	if (IS_ERR(ssusb->sys_clk)) {
+		dev_err(dev, "failed to get sys clock\n");
+		return PTR_ERR(ssusb->sys_clk);
+	}
+
 	ssusb->num_phys = of_count_phandle_with_args(node,
 			"phys", "#phy-cells");
 	if (ssusb->num_phys > 0) {
@@ -230,18 +242,6 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
 		return PTR_ERR(ssusb->ippc_base);
 	}
 
-	ssusb->vusb33 = devm_regulator_get(&pdev->dev, "vusb33");
-	if (IS_ERR(ssusb->vusb33)) {
-		dev_err(dev, "failed to get vusb33\n");
-		return PTR_ERR(ssusb->vusb33);
-	}
-
-	ssusb->sys_clk = devm_clk_get(dev, "sys_ck");
-	if (IS_ERR(ssusb->sys_clk)) {
-		dev_err(dev, "failed to get sys clock\n");
-		return PTR_ERR(ssusb->sys_clk);
-	}
-
 	ssusb->dr_mode = usb_get_dr_mode(dev);
 	if (ssusb->dr_mode == USB_DR_MODE_UNKNOWN) {
 		dev_err(dev, "dr_mode is error\n");
-- 
1.7.9.5

[PATCH 2/6] usb: mtu3: add reference clock

[Chunfeng Yun]

usually, the reference clock comes from 26M oscillator directly,
but some SoCs are not, add it for compatibility.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/mtu3/mtu3.h      |    1 +
 drivers/usb/mtu3/mtu3_plat.c |   21 +++++++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
index ba9df71..aa6fd6a 100644
--- a/drivers/usb/mtu3/mtu3.h
+++ b/drivers/usb/mtu3/mtu3.h
@@ -225,6 +225,7 @@ struct ssusb_mtk {
 	/* common power & clock */
 	struct regulator *vusb33;
 	struct clk *sys_clk;
+	struct clk *ref_clk;
 	/* otg */
 	struct otg_switch_mtk otg_switch;
 	enum usb_dr_mode dr_mode;
diff --git a/drivers/usb/mtu3/mtu3_plat.c b/drivers/usb/mtu3/mtu3_plat.c
index 6344859..19a345d 100644
--- a/drivers/usb/mtu3/mtu3_plat.c
+++ b/drivers/usb/mtu3/mtu3_plat.c
@@ -123,7 +123,13 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
 	ret = clk_prepare_enable(ssusb->sys_clk);
 	if (ret) {
 		dev_err(ssusb->dev, "failed to enable sys_clk\n");
-		goto clk_err;
+		goto sys_clk_err;
+	}
+
+	ret = clk_prepare_enable(ssusb->ref_clk);
+	if (ret) {
+		dev_err(ssusb->dev, "failed to enable ref_clk\n");
+		goto ref_clk_err;
 	}
 
 	ret = ssusb_phy_init(ssusb);
@@ -143,8 +149,10 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
 phy_err:
 	ssusb_phy_exit(ssusb);
 phy_init_err:
+	clk_disable_unprepare(ssusb->ref_clk);
+ref_clk_err:
 	clk_disable_unprepare(ssusb->sys_clk);
-clk_err:
+sys_clk_err:
 	regulator_disable(ssusb->vusb33);
 vusb33_err:
 
@@ -154,6 +162,7 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
 static void ssusb_rscs_exit(struct ssusb_mtk *ssusb)
 {
 	clk_disable_unprepare(ssusb->sys_clk);
+	clk_disable_unprepare(ssusb->ref_clk);
 	regulator_disable(ssusb->vusb33);
 	ssusb_phy_power_off(ssusb);
 	ssusb_phy_exit(ssusb);
@@ -216,6 +225,12 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
 		return PTR_ERR(ssusb->sys_clk);
 	}
 
+	ssusb->ref_clk = devm_clk_get(dev, "ref_ck");
+	if (IS_ERR(ssusb->ref_clk)) {
+		dev_err(dev, "failed to get ref clock\n");
+		return PTR_ERR(ssusb->ref_clk);
+	}
+
 	ssusb->num_phys = of_count_phandle_with_args(node,
 			"phys", "#phy-cells");
 	if (ssusb->num_phys > 0) {
@@ -428,6 +443,7 @@ static int __maybe_unused mtu3_suspend(struct device *dev)
 	ssusb_host_disable(ssusb, true);
 	ssusb_phy_power_off(ssusb);
 	clk_disable_unprepare(ssusb->sys_clk);
+	clk_disable_unprepare(ssusb->ref_clk);
 	ssusb_wakeup_enable(ssusb);
 
 	return 0;
@@ -445,6 +461,7 @@ static int __maybe_unused mtu3_resume(struct device *dev)
 
 	ssusb_wakeup_disable(ssusb);
 	clk_prepare_enable(ssusb->sys_clk);
+	clk_prepare_enable(ssusb->ref_clk);
 	ssusb_phy_power_on(ssusb);
 	ssusb_host_enable(ssusb);
 
-- 
1.7.9.5

[PATCH 3/6] usb: xhci-mtk: add reference clock

[Chunfeng Yun]

usually, the reference clock comes from 26M oscillator directly,
but some SoCs are not, add it for compatibility.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c |   15 +++++++++++++++
 drivers/usb/host/xhci-mtk.h |    1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 1094ebd..4d75ac5 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -212,6 +212,12 @@ static int xhci_mtk_clks_enable(struct xhci_hcd_mtk *mtk)
 {
 	int ret;
 
+	ret = clk_prepare_enable(mtk->ref_clk);
+	if (ret) {
+		dev_err(mtk->dev, "failed to enable ref_clk\n");
+		goto ref_clk_err;
+	}
+
 	ret = clk_prepare_enable(mtk->sys_clk);
 	if (ret) {
 		dev_err(mtk->dev, "failed to enable sys_clk\n");
@@ -238,6 +244,8 @@ static int xhci_mtk_clks_enable(struct xhci_hcd_mtk *mtk)
 usb_p0_err:
 	clk_disable_unprepare(mtk->sys_clk);
 sys_clk_err:
+	clk_disable_unprepare(mtk->ref_clk);
+ref_clk_err:
 	return -EINVAL;
 }
 
@@ -248,6 +256,7 @@ static void xhci_mtk_clks_disable(struct xhci_hcd_mtk *mtk)
 		clk_disable_unprepare(mtk->wk_deb_p0);
 	}
 	clk_disable_unprepare(mtk->sys_clk);
+	clk_disable_unprepare(mtk->ref_clk);
 }
 
 /* only clocks can be turn off for ip-sleep wakeup mode */
@@ -550,6 +559,12 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 		return PTR_ERR(mtk->sys_clk);
 	}
 
+	mtk->ref_clk = devm_clk_get(dev, "ref_ck");
+	if (IS_ERR(mtk->ref_clk)) {
+		dev_err(dev, "fail to get ref_ck\n");
+		return PTR_ERR(mtk->ref_clk);
+	}
+
 	mtk->lpm_support = of_property_read_bool(node, "usb3-lpm-capable");
 
 	ret = usb_wakeup_of_property_parse(mtk, node);
diff --git a/drivers/usb/host/xhci-mtk.h b/drivers/usb/host/xhci-mtk.h
index 2845c49..3aa5e1d 100644
--- a/drivers/usb/host/xhci-mtk.h
+++ b/drivers/usb/host/xhci-mtk.h
@@ -124,6 +124,7 @@ struct xhci_hcd_mtk {
 	struct regulator *vusb33;
 	struct regulator *vbus;
 	struct clk *sys_clk;	/* sys and mac clock */
+	struct clk *ref_clk;
 	struct clk *wk_deb_p0;	/* port0's wakeup debounce clock */
 	struct clk *wk_deb_p1;
 	struct regmap *pericfg;
-- 
1.7.9.5

[PATCH 4/6] arm64: dts: mt8173: add reference clock for usb

[Chunfeng Yun]

add 26M reference clock for ssusb and xhci nodes

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index 07fd2eb..e2862b6 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -729,9 +729,11 @@
 			       <&u2port1 PHY_TYPE_USB2>;
 			power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
 			clocks = <&topckgen CLK_TOP_USB30_SEL>,
+				 <&clk26m>,
 				 <&pericfg CLK_PERI_USB0>,
 				 <&pericfg CLK_PERI_USB1>;
 			clock-names = "sys_ck",
+				      "ref_ck",
 				      "wakeup_deb_p0",
 				      "wakeup_deb_p1";
 			mediatek,syscon-wakeup = <&pericfg>;
@@ -746,8 +748,8 @@
 				reg-names = "mac";
 				interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>;
 				power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
-				clocks = <&topckgen CLK_TOP_USB30_SEL>;
-				clock-names = "sys_ck";
+				clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
+				clock-names = "sys_ck", "ref_ck";
 				status = "disabled";
 			};
 		};
-- 
1.7.9.5

[PATCH 5/6] dt-bindings: mt8173-xhci: add reference clock

[Chunfeng Yun]

add a reference clock for compatibility

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 .../devicetree/bindings/usb/mt8173-xhci.txt        |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
index 2a930bd..ab8bb27 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mt8173-xhci.txt
@@ -23,6 +23,7 @@ Required properties:
 	entry in clock-names
  - clock-names : must contain
 	"sys_ck": for clock of xHCI MAC
+	"ref_ck": for reference clock of xHCI MAC
 	"wakeup_deb_p0": for USB wakeup debounce clock of port0
 	"wakeup_deb_p1": for USB wakeup debounce clock of port1
 
@@ -47,10 +48,10 @@ usb30: usb@11270000 {
 	reg-names = "mac", "ippc";
 	interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>;
 	power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
-	clocks = <&topckgen CLK_TOP_USB30_SEL>,
+	clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>,
 		 <&pericfg CLK_PERI_USB0>,
 		 <&pericfg CLK_PERI_USB1>;
-	clock-names = "sys_ck",
+	clock-names = "sys_ck", "ref_ck",
 		      "wakeup_deb_p0",
 		      "wakeup_deb_p1";
 	phys = <&phy_port0 PHY_TYPE_USB3>,
@@ -82,6 +83,7 @@ Required properties:
 	entry in clock-names
  - clock-names : must be
 	"sys_ck": for clock of xHCI MAC
+	"ref_ck": for reference clock of xHCI MAC
 
 Optional properties:
  - vbus-supply : reference to the VBUS regulator;
@@ -94,8 +96,8 @@ usb30: usb@11270000 {
 	reg-names = "mac";
 	interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>;
 	power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
-	clocks = <&topckgen CLK_TOP_USB30_SEL>;
-	clock-names = "sys_ck";
+	clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
+	clock-names = "sys_ck", "ref_ck";
 	vusb33-supply = <&mt6397_vusb_reg>;
 	usb3-lpm-capable;
 };
-- 
1.7.9.5

[PATCH 6/6] dt-bindings: mt8173-mtu3: add reference clock

[Chunfeng Yun]

add a reference clock for compatibility

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 .../devicetree/bindings/usb/mt8173-mtu3.txt        |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
index e049d19..8c976cd 100644
--- a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
+++ b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
@@ -10,7 +10,7 @@ Required properties:
  - vusb33-supply : regulator of USB avdd3.3v
  - clocks : a list of phandle + clock-specifier pairs, one for each
 	entry in clock-names
- - clock-names : must contain "sys_ck" for clock of controller;
+ - clock-names : must contain "sys_ck" and "ref_ck" for clock of controller;
 	"wakeup_deb_p0" and "wakeup_deb_p1" are optional, they are
 	depends on "mediatek,enable-wakeup"
  - phys : a list of phandle + phy specifier pairs
@@ -56,10 +56,10 @@ ssusb: usb@11271000 {
 	phys = <&phy_port0 PHY_TYPE_USB3>,
 	       <&phy_port1 PHY_TYPE_USB2>;
 	power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
-	clocks = <&topckgen CLK_TOP_USB30_SEL>,
+	clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>,
 		 <&pericfg CLK_PERI_USB0>,
 		 <&pericfg CLK_PERI_USB1>;
-	clock-names = "sys_ck",
+	clock-names = "sys_ck", "ref_ck",
 		      "wakeup_deb_p0",
 		      "wakeup_deb_p1";
 	vusb33-supply = <&mt6397_vusb_reg>;
@@ -79,8 +79,8 @@ ssusb: usb@11271000 {
 		reg-names = "mac";
 		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>;
 		power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
-		clocks = <&topckgen CLK_TOP_USB30_SEL>;
-		clock-names = "sys_ck";
+		clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
+		clock-names = "sys_ck", "ref_ck";
 		vusb33-supply = <&mt6397_vusb_reg>;
 		status = "disabled";
 	};
-- 
1.7.9.5

Re: [PATCH 4/6] arm64: dts: mt8173: add reference clock for usb

[Greg Kroah-Hartman]

On Wed, Jan 18, 2017 at 02:08:25PM +0800, Chunfeng Yun wrote:
> add 26M reference clock for ssusb and xhci nodes
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

This patch doesn't apply to my tree :(

Re: [PATCH 2/6] usb: mtu3: add reference clock

[Matthias Brugger]

On 18/01/17 07:08, Chunfeng Yun wrote:
> usually, the reference clock comes from 26M oscillator directly,
> but some SoCs are not, add it for compatibility.
>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/mtu3/mtu3.h      |    1 +
>  drivers/usb/mtu3/mtu3_plat.c |   21 +++++++++++++++++++--
>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
> index ba9df71..aa6fd6a 100644
> --- a/drivers/usb/mtu3/mtu3.h
> +++ b/drivers/usb/mtu3/mtu3.h
> @@ -225,6 +225,7 @@ struct ssusb_mtk {
>  	/* common power & clock */
>  	struct regulator *vusb33;
>  	struct clk *sys_clk;
> +	struct clk *ref_clk;
>  	/* otg */
>  	struct otg_switch_mtk otg_switch;
>  	enum usb_dr_mode dr_mode;
> diff --git a/drivers/usb/mtu3/mtu3_plat.c b/drivers/usb/mtu3/mtu3_plat.c
> index 6344859..19a345d 100644
> --- a/drivers/usb/mtu3/mtu3_plat.c
> +++ b/drivers/usb/mtu3/mtu3_plat.c
> @@ -123,7 +123,13 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
>  	ret = clk_prepare_enable(ssusb->sys_clk);
>  	if (ret) {
>  		dev_err(ssusb->dev, "failed to enable sys_clk\n");
> -		goto clk_err;
> +		goto sys_clk_err;
> +	}
> +
> +	ret = clk_prepare_enable(ssusb->ref_clk);
> +	if (ret) {
> +		dev_err(ssusb->dev, "failed to enable ref_clk\n");
> +		goto ref_clk_err;
>  	}
>
>  	ret = ssusb_phy_init(ssusb);
> @@ -143,8 +149,10 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
>  phy_err:
>  	ssusb_phy_exit(ssusb);
>  phy_init_err:
> +	clk_disable_unprepare(ssusb->ref_clk);
> +ref_clk_err:
>  	clk_disable_unprepare(ssusb->sys_clk);
> -clk_err:
> +sys_clk_err:
>  	regulator_disable(ssusb->vusb33);
>  vusb33_err:
>
> @@ -154,6 +162,7 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
>  static void ssusb_rscs_exit(struct ssusb_mtk *ssusb)
>  {
>  	clk_disable_unprepare(ssusb->sys_clk);
> +	clk_disable_unprepare(ssusb->ref_clk);
>  	regulator_disable(ssusb->vusb33);
>  	ssusb_phy_power_off(ssusb);
>  	ssusb_phy_exit(ssusb);
> @@ -216,6 +225,12 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
>  		return PTR_ERR(ssusb->sys_clk);
>  	}
>
> +	ssusb->ref_clk = devm_clk_get(dev, "ref_ck");
> +	if (IS_ERR(ssusb->ref_clk)) {
> +		dev_err(dev, "failed to get ref clock\n");
> +		return PTR_ERR(ssusb->ref_clk);
> +	}
> +

That would break older dts bindings, right?
ref_ck must be optional for the code.

Regards,
Matthias

Re: [PATCH 4/6] arm64: dts: mt8173: add reference clock for usb

[Matthias Brugger]

On 19/01/17 10:37, Greg Kroah-Hartman wrote:
> On Wed, Jan 18, 2017 at 02:08:25PM +0800, Chunfeng Yun wrote:
>> add 26M reference clock for ssusb and xhci nodes
>>
>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>> ---
>>  arch/arm64/boot/dts/mediatek/mt8173.dtsi |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> This patch doesn't apply to my tree :(
>

This patch should go through my tree, but take into account my comment 
on patch 3/6. From my point of view this series is not ready to be merged.

Regards,
Matthias

Re: [PATCH 2/6] usb: mtu3: add reference clock

[Chunfeng Yun]

On Thu, 2017-01-19 at 13:22 +0100, Matthias Brugger wrote:
> 
> On 18/01/17 07:08, Chunfeng Yun wrote:
> > usually, the reference clock comes from 26M oscillator directly,
> > but some SoCs are not, add it for compatibility.
> >
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/mtu3/mtu3.h      |    1 +
> >  drivers/usb/mtu3/mtu3_plat.c |   21 +++++++++++++++++++--
> >  2 files changed, 20 insertions(+), 2 deletions(-)
[...]
> > @@ -154,6 +162,7 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
> >  static void ssusb_rscs_exit(struct ssusb_mtk *ssusb)
> >  {
> >  	clk_disable_unprepare(ssusb->sys_clk);
> > +	clk_disable_unprepare(ssusb->ref_clk);
> >  	regulator_disable(ssusb->vusb33);
> >  	ssusb_phy_power_off(ssusb);
> >  	ssusb_phy_exit(ssusb);
> > @@ -216,6 +225,12 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
> >  		return PTR_ERR(ssusb->sys_clk);
> >  	}
> >
> > +	ssusb->ref_clk = devm_clk_get(dev, "ref_ck");
> > +	if (IS_ERR(ssusb->ref_clk)) {
> > +		dev_err(dev, "failed to get ref clock\n");
> > +		return PTR_ERR(ssusb->ref_clk);
> > +	}
> > +
> 
> That would break older dts bindings, right?
Yes, So I send a new patch for the related dts. Maybe it's not a
problem, only one dts file need be updated currently.

> ref_ck must be optional for the code.
I tend to make it be optional for the dts, but not for the code.
There are some "fixed-clock" which can be treated as dummy ones, and if
a clock is really optional, we can use one fixed-clock in dts, and keep
the code simple.
In fact, the reference clock is essential for usb controller.
> 
> Regards,
> Matthias

Re: [PATCH 5/6] dt-bindings: mt8173-xhci: add reference clock

[Rob Herring]

On Wed, Jan 18, 2017 at 02:08:26PM +0800, Chunfeng Yun wrote:
> add a reference clock for compatibility
> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  .../devicetree/bindings/usb/mt8173-xhci.txt        |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 6/6] dt-bindings: mt8173-mtu3: add reference clock

[Rob Herring]

On Wed, Jan 18, 2017 at 02:08:27PM +0800, Chunfeng Yun wrote:
> add a reference clock for compatibility

Why? This block suddenly has 2 clocks instead of 1?

> 
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  .../devicetree/bindings/usb/mt8173-mtu3.txt        |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
> index e049d19..8c976cd 100644
> --- a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
> +++ b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
> @@ -10,7 +10,7 @@ Required properties:
>   - vusb33-supply : regulator of USB avdd3.3v
>   - clocks : a list of phandle + clock-specifier pairs, one for each
>  	entry in clock-names
> - - clock-names : must contain "sys_ck" for clock of controller;
> + - clock-names : must contain "sys_ck" and "ref_ck" for clock of controller;
>  	"wakeup_deb_p0" and "wakeup_deb_p1" are optional, they are
>  	depends on "mediatek,enable-wakeup"
>   - phys : a list of phandle + phy specifier pairs
> @@ -56,10 +56,10 @@ ssusb: usb@11271000 {
>  	phys = <&phy_port0 PHY_TYPE_USB3>,
>  	       <&phy_port1 PHY_TYPE_USB2>;
>  	power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
> -	clocks = <&topckgen CLK_TOP_USB30_SEL>,
> +	clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>,
>  		 <&pericfg CLK_PERI_USB0>,
>  		 <&pericfg CLK_PERI_USB1>;
> -	clock-names = "sys_ck",
> +	clock-names = "sys_ck", "ref_ck",
>  		      "wakeup_deb_p0",
>  		      "wakeup_deb_p1";
>  	vusb33-supply = <&mt6397_vusb_reg>;
> @@ -79,8 +79,8 @@ ssusb: usb@11271000 {
>  		reg-names = "mac";
>  		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>;
>  		power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
> -		clocks = <&topckgen CLK_TOP_USB30_SEL>;
> -		clock-names = "sys_ck";
> +		clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
> +		clock-names = "sys_ck", "ref_ck";
>  		vusb33-supply = <&mt6397_vusb_reg>;
>  		status = "disabled";
>  	};
> -- 
> 1.7.9.5
> 

Re: [PATCH 5/6] dt-bindings: mt8173-xhci: add reference clock

[Rob Herring]

On Wed, Jan 18, 2017 at 02:08:26PM +0800, Chunfeng Yun wrote:
> add a reference clock for compatibility

Same question here, too.

Re: [PATCH 6/6] dt-bindings: mt8173-mtu3: add reference clock

[Chunfeng Yun]

Hi,

On Sat, 2017-01-21 at 14:11 -0600, Rob Herring wrote:
> On Wed, Jan 18, 2017 at 02:08:27PM +0800, Chunfeng Yun wrote:
> > add a reference clock for compatibility
> 
> Why? This block suddenly has 2 clocks instead of 1?
In fact, there is a reference clock which comes from 26M oscillator
directly. I ignore it because it is a fixed-clock in DTS, and always
turned on for mt8173. But later, I find that I made a mistake before
when I bring up it on a new platform whose reference clock comes from
PLL, and need control it. So here add it, no matter it is a fixed-clock
or not.

> 
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  .../devicetree/bindings/usb/mt8173-mtu3.txt        |   10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
> > index e049d19..8c976cd 100644
> > --- a/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
> > +++ b/Documentation/devicetree/bindings/usb/mt8173-mtu3.txt
> > @@ -10,7 +10,7 @@ Required properties:
> >   - vusb33-supply : regulator of USB avdd3.3v
> >   - clocks : a list of phandle + clock-specifier pairs, one for each
> >  	entry in clock-names
> > - - clock-names : must contain "sys_ck" for clock of controller;
> > + - clock-names : must contain "sys_ck" and "ref_ck" for clock of controller;
> >  	"wakeup_deb_p0" and "wakeup_deb_p1" are optional, they are
> >  	depends on "mediatek,enable-wakeup"
> >   - phys : a list of phandle + phy specifier pairs
> > @@ -56,10 +56,10 @@ ssusb: usb@11271000 {
> >  	phys = <&phy_port0 PHY_TYPE_USB3>,
> >  	       <&phy_port1 PHY_TYPE_USB2>;
> >  	power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
> > -	clocks = <&topckgen CLK_TOP_USB30_SEL>,
> > +	clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>,
> >  		 <&pericfg CLK_PERI_USB0>,
> >  		 <&pericfg CLK_PERI_USB1>;
> > -	clock-names = "sys_ck",
> > +	clock-names = "sys_ck", "ref_ck",
> >  		      "wakeup_deb_p0",
> >  		      "wakeup_deb_p1";
> >  	vusb33-supply = <&mt6397_vusb_reg>;
> > @@ -79,8 +79,8 @@ ssusb: usb@11271000 {
> >  		reg-names = "mac";
> >  		interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_LOW>;
> >  		power-domains = <&scpsys MT8173_POWER_DOMAIN_USB>;
> > -		clocks = <&topckgen CLK_TOP_USB30_SEL>;
> > -		clock-names = "sys_ck";
> > +		clocks = <&topckgen CLK_TOP_USB30_SEL>, <&clk26m>;
> > +		clock-names = "sys_ck", "ref_ck";
> >  		vusb33-supply = <&mt6397_vusb_reg>;
> >  		status = "disabled";
> >  	};
> > -- 
> > 1.7.9.5
> > 

Re: [PATCH 5/6] dt-bindings: mt8173-xhci: add reference clock

[Chunfeng Yun]

On Sat, 2017-01-21 at 14:12 -0600, Rob Herring wrote:
> On Wed, Jan 18, 2017 at 02:08:26PM +0800, Chunfeng Yun wrote:
> > add a reference clock for compatibility
> 
> Same question here, too.
The reason is the same as mt8173-mtu3

thanks a lot
> 

Re: [PATCH 6/6] dt-bindings: mt8173-mtu3: add reference clock

[Rob Herring]

On Sat, Jan 21, 2017 at 7:49 PM, Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> Hi,
>
> On Sat, 2017-01-21 at 14:11 -0600, Rob Herring wrote:
>> On Wed, Jan 18, 2017 at 02:08:27PM +0800, Chunfeng Yun wrote:
>> > add a reference clock for compatibility
>>
>> Why? This block suddenly has 2 clocks instead of 1?
> In fact, there is a reference clock which comes from 26M oscillator
> directly. I ignore it because it is a fixed-clock in DTS, and always
> turned on for mt8173. But later, I find that I made a mistake before
> when I bring up it on a new platform whose reference clock comes from
> PLL, and need control it. So here add it, no matter it is a fixed-clock
> or not.

Add this explanation to the changelog.

Rob

Re: [PATCH 2/6] usb: mtu3: add reference clock

[Matthias Brugger]

On 01/20/2017 03:20 AM, Chunfeng Yun wrote:
> On Thu, 2017-01-19 at 13:22 +0100, Matthias Brugger wrote:
>>
>> On 18/01/17 07:08, Chunfeng Yun wrote:
>>> usually, the reference clock comes from 26M oscillator directly,
>>> but some SoCs are not, add it for compatibility.
>>>
>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>> ---
>>>  drivers/usb/mtu3/mtu3.h      |    1 +
>>>  drivers/usb/mtu3/mtu3_plat.c |   21 +++++++++++++++++++--
>>>  2 files changed, 20 insertions(+), 2 deletions(-)
> [...]
>>> @@ -154,6 +162,7 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
>>>  static void ssusb_rscs_exit(struct ssusb_mtk *ssusb)
>>>  {
>>>  	clk_disable_unprepare(ssusb->sys_clk);
>>> +	clk_disable_unprepare(ssusb->ref_clk);
>>>  	regulator_disable(ssusb->vusb33);
>>>  	ssusb_phy_power_off(ssusb);
>>>  	ssusb_phy_exit(ssusb);
>>> @@ -216,6 +225,12 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
>>>  		return PTR_ERR(ssusb->sys_clk);
>>>  	}
>>>
>>> +	ssusb->ref_clk = devm_clk_get(dev, "ref_ck");
>>> +	if (IS_ERR(ssusb->ref_clk)) {
>>> +		dev_err(dev, "failed to get ref clock\n");
>>> +		return PTR_ERR(ssusb->ref_clk);
>>> +	}
>>> +
>>
>> That would break older dts bindings, right?
> Yes, So I send a new patch for the related dts. Maybe it's not a
> problem, only one dts file need be updated currently.
>
>> ref_ck must be optional for the code.
> I tend to make it be optional for the dts, but not for the code.
> There are some "fixed-clock" which can be treated as dummy ones, and if
> a clock is really optional, we can use one fixed-clock in dts, and keep
> the code simple.
> In fact, the reference clock is essential for usb controller.

Well the thing is that there are devices in the field with an older dtb 
which would break on a newer kernel. That's why we need to make it work 
with the old dtb in the code as well.

Regards,
Matthias

>>
>> Regards,
>> Matthias
>
>

Re: [PATCH 2/6] usb: mtu3: add reference clock

[Chunfeng Yun]

On Wed, 2017-01-25 at 00:23 +0100, Matthias Brugger wrote:
> 
> On 01/20/2017 03:20 AM, Chunfeng Yun wrote:
> > On Thu, 2017-01-19 at 13:22 +0100, Matthias Brugger wrote:
> >>
> >> On 18/01/17 07:08, Chunfeng Yun wrote:
> >>> usually, the reference clock comes from 26M oscillator directly,
> >>> but some SoCs are not, add it for compatibility.
> >>>
> >>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> >>> ---
> >>>  drivers/usb/mtu3/mtu3.h      |    1 +
> >>>  drivers/usb/mtu3/mtu3_plat.c |   21 +++++++++++++++++++--
> >>>  2 files changed, 20 insertions(+), 2 deletions(-)
> > [...]
> >>> @@ -154,6 +162,7 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
> >>>  static void ssusb_rscs_exit(struct ssusb_mtk *ssusb)
> >>>  {
> >>>  	clk_disable_unprepare(ssusb->sys_clk);
> >>> +	clk_disable_unprepare(ssusb->ref_clk);
> >>>  	regulator_disable(ssusb->vusb33);
> >>>  	ssusb_phy_power_off(ssusb);
> >>>  	ssusb_phy_exit(ssusb);
> >>> @@ -216,6 +225,12 @@ static int get_ssusb_rscs(struct platform_device *pdev, struct ssusb_mtk *ssusb)
> >>>  		return PTR_ERR(ssusb->sys_clk);
> >>>  	}
> >>>
> >>> +	ssusb->ref_clk = devm_clk_get(dev, "ref_ck");
> >>> +	if (IS_ERR(ssusb->ref_clk)) {
> >>> +		dev_err(dev, "failed to get ref clock\n");
> >>> +		return PTR_ERR(ssusb->ref_clk);
> >>> +	}
> >>> +
> >>
> >> That would break older dts bindings, right?
> > Yes, So I send a new patch for the related dts. Maybe it's not a
> > problem, only one dts file need be updated currently.
> >
> >> ref_ck must be optional for the code.
> > I tend to make it be optional for the dts, but not for the code.
> > There are some "fixed-clock" which can be treated as dummy ones, and if
> > a clock is really optional, we can use one fixed-clock in dts, and keep
> > the code simple.
> > In fact, the reference clock is essential for usb controller.
> 
> Well the thing is that there are devices in the field with an older dtb 
> which would break on a newer kernel. That's why we need to make it work 
> with the old dtb in the code as well.
> 
Happy Chinese New Year!

Ok, I will make it optional.

Thanks and sorry for later reply.

> Regards,
> Matthias
> 
> >>
> >> Regards,
> >> Matthias
> >
> >

Re: [PATCH 6/6] dt-bindings: mt8173-mtu3: add reference clock

[Chunfeng Yun]

On Mon, 2017-01-23 at 08:02 -0600, Rob Herring wrote:
> On Sat, Jan 21, 2017 at 7:49 PM, Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > Hi,
> >
> > On Sat, 2017-01-21 at 14:11 -0600, Rob Herring wrote:
> >> On Wed, Jan 18, 2017 at 02:08:27PM +0800, Chunfeng Yun wrote:
> >> > add a reference clock for compatibility
> >>
> >> Why? This block suddenly has 2 clocks instead of 1?
> > In fact, there is a reference clock which comes from 26M oscillator
> > directly. I ignore it because it is a fixed-clock in DTS, and always
> > turned on for mt8173. But later, I find that I made a mistake before
> > when I bring up it on a new platform whose reference clock comes from
> > PLL, and need control it. So here add it, no matter it is a fixed-clock
> > or not.
> 
> Add this explanation to the changelog.
> 
Happy Chinese New Year!

Ok, I'll add it.

Thanks and sorry for later reply.

> Rob