2018-09-29 02:30:35

by Chaotian Jing

[permalink] [raw]
Subject: [PATCH v1 1/2] mmc: dt-bindings: add "bus-clk" for MT2712

On MT2712 MSDC0/3, HCLK/bus-clk need gate/ungate together,
or will hang when access MSDC register.

Signed-off-by: Chaotian Jing <[email protected]>
---
Documentation/devicetree/bindings/mmc/mtk-sd.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
index f33467a..f2208f4 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
@@ -22,6 +22,7 @@ Required properties:
"source" - source clock (required)
"hclk" - HCLK which used for host (required)
"source_cg" - independent source clock gate (required for MT2712)
+ "bus_clk" - bus clock used for internal register access (required for MT2712 MSDC0/3)
- pinctrl-names: should be "default", "state_uhs"
- pinctrl-0: should contain default/high speed pin ctrl
- pinctrl-1: should contain uhs mode pin ctrl
--
1.8.1.1.dirty



2018-09-29 02:30:48

by Chaotian Jing

[permalink] [raw]
Subject: [PATCH v1 2/2] mmc: mediatek: add bus_clk control

when gate MSDC0_HCLK, access register will hang, even the MSDC driver
will never accessing register after HCLK was gated, but for safety, need
gate the bus_clk(which used to access register) too.

Signed-off-by: Chaotian Jing <[email protected]>
---
drivers/mmc/host/mtk-sd.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 0484138..1c1c967 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -387,6 +387,7 @@ struct msdc_host {

struct clk *src_clk; /* msdc source clock */
struct clk *h_clk; /* msdc h_clk */
+ struct clk *bus_clk; /* bus clock which used to access register */
struct clk *src_clk_cg; /* msdc source clock control gate */
u32 mclk; /* mmc subsystem clock frequency */
u32 src_clk_freq; /* source clock frequency */
@@ -660,12 +661,14 @@ static void msdc_gate_clock(struct msdc_host *host)
{
clk_disable_unprepare(host->src_clk_cg);
clk_disable_unprepare(host->src_clk);
+ clk_disable_unprepare(host->bus_clk);
clk_disable_unprepare(host->h_clk);
}

static void msdc_ungate_clock(struct msdc_host *host)
{
clk_prepare_enable(host->h_clk);
+ clk_prepare_enable(host->bus_clk);
clk_prepare_enable(host->src_clk);
clk_prepare_enable(host->src_clk_cg);
while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
@@ -1900,6 +1903,9 @@ static int msdc_drv_probe(struct platform_device *pdev)
goto host_free;
}

+ host->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
+ if (IS_ERR(host->bus_clk))
+ host->bus_clk = NULL;
/*source clock control gate is optional clock*/
host->src_clk_cg = devm_clk_get(&pdev->dev, "source_cg");
if (IS_ERR(host->src_clk_cg))
--
1.8.1.1.dirty


2018-09-29 12:56:40

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mmc: mediatek: add bus_clk control



On 29/09/2018 04:29, Chaotian Jing wrote:
> when gate MSDC0_HCLK, access register will hang, even the MSDC driver
> will never accessing register after HCLK was gated, but for safety, need
> gate the bus_clk(which used to access register) too.
>
> Signed-off-by: Chaotian Jing <[email protected]>
> ---
> drivers/mmc/host/mtk-sd.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 0484138..1c1c967 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -387,6 +387,7 @@ struct msdc_host {
>
> struct clk *src_clk; /* msdc source clock */
> struct clk *h_clk; /* msdc h_clk */
> + struct clk *bus_clk; /* bus clock which used to access register */
> struct clk *src_clk_cg; /* msdc source clock control gate */
> u32 mclk; /* mmc subsystem clock frequency */
> u32 src_clk_freq; /* source clock frequency */
> @@ -660,12 +661,14 @@ static void msdc_gate_clock(struct msdc_host *host)
> {
> clk_disable_unprepare(host->src_clk_cg);
> clk_disable_unprepare(host->src_clk);
> + clk_disable_unprepare(host->bus_clk);
> clk_disable_unprepare(host->h_clk);
> }
>
> static void msdc_ungate_clock(struct msdc_host *host)
> {
> clk_prepare_enable(host->h_clk);
> + clk_prepare_enable(host->bus_clk);
> clk_prepare_enable(host->src_clk);
> clk_prepare_enable(host->src_clk_cg);
> while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
> @@ -1900,6 +1903,9 @@ static int msdc_drv_probe(struct platform_device *pdev)
> goto host_free;
> }
>
> + host->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
> + if (IS_ERR(host->bus_clk))
> + host->bus_clk = NULL;

That means the bus_clk is not requiered, even for mt2712, although the binding
says so. Not sure if this is good enough or if we want to enforce the driver to
actually error out, when the clock is not present in a mt2712 DTS.

I suppose that's up to the maintainer :)

Regards,
Matthias

2018-10-08 11:52:26

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mmc: mediatek: add bus_clk control

On 29 September 2018 at 04:29, Chaotian Jing <[email protected]> wrote:
> when gate MSDC0_HCLK, access register will hang, even the MSDC driver
> will never accessing register after HCLK was gated, but for safety, need
> gate the bus_clk(which used to access register) too.
>
> Signed-off-by: Chaotian Jing <[email protected]>

Applied for next, thanks!

Kind regards
Uffe

> ---
> drivers/mmc/host/mtk-sd.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 0484138..1c1c967 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -387,6 +387,7 @@ struct msdc_host {
>
> struct clk *src_clk; /* msdc source clock */
> struct clk *h_clk; /* msdc h_clk */
> + struct clk *bus_clk; /* bus clock which used to access register */
> struct clk *src_clk_cg; /* msdc source clock control gate */
> u32 mclk; /* mmc subsystem clock frequency */
> u32 src_clk_freq; /* source clock frequency */
> @@ -660,12 +661,14 @@ static void msdc_gate_clock(struct msdc_host *host)
> {
> clk_disable_unprepare(host->src_clk_cg);
> clk_disable_unprepare(host->src_clk);
> + clk_disable_unprepare(host->bus_clk);
> clk_disable_unprepare(host->h_clk);
> }
>
> static void msdc_ungate_clock(struct msdc_host *host)
> {
> clk_prepare_enable(host->h_clk);
> + clk_prepare_enable(host->bus_clk);
> clk_prepare_enable(host->src_clk);
> clk_prepare_enable(host->src_clk_cg);
> while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
> @@ -1900,6 +1903,9 @@ static int msdc_drv_probe(struct platform_device *pdev)
> goto host_free;
> }
>
> + host->bus_clk = devm_clk_get(&pdev->dev, "bus_clk");
> + if (IS_ERR(host->bus_clk))
> + host->bus_clk = NULL;
> /*source clock control gate is optional clock*/
> host->src_clk_cg = devm_clk_get(&pdev->dev, "source_cg");
> if (IS_ERR(host->src_clk_cg))
> --
> 1.8.1.1.dirty
>

2018-10-08 11:53:54

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] mmc: dt-bindings: add "bus-clk" for MT2712

On 29 September 2018 at 04:29, Chaotian Jing <[email protected]> wrote:
> On MT2712 MSDC0/3, HCLK/bus-clk need gate/ungate together,
> or will hang when access MSDC register.
>
> Signed-off-by: Chaotian Jing <[email protected]>

Applied for next, thanks!

Kind regards
Uffe

> ---
> Documentation/devicetree/bindings/mmc/mtk-sd.txt | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> index f33467a..f2208f4 100644
> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> @@ -22,6 +22,7 @@ Required properties:
> "source" - source clock (required)
> "hclk" - HCLK which used for host (required)
> "source_cg" - independent source clock gate (required for MT2712)
> + "bus_clk" - bus clock used for internal register access (required for MT2712 MSDC0/3)
> - pinctrl-names: should be "default", "state_uhs"
> - pinctrl-0: should contain default/high speed pin ctrl
> - pinctrl-1: should contain uhs mode pin ctrl
> --
> 1.8.1.1.dirty
>