* Enabling MMC on MT7628 SoC
@ 2020-02-01 16:06 Mauro Condarelli
2020-02-11 16:15 ` Matthias Brugger
0 siblings, 1 reply; 3+ messages in thread
From: Mauro Condarelli @ 2020-02-01 16:06 UTC (permalink / raw)
To: linux-mediatek
Hi,
I'm trying to enable MMC/SD access on a VoCore2 SOM (based on MT7628)
using mtk_sd driver.
Just enabling mtk_sd will bomb wit undefined function `clk_get_parent`;
this can be trivially cured with:
diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c
index 2f9d5acb38ea..930c2776f6fd 100644
--- a/arch/mips/ralink/clk.c
+++ b/arch/mips/ralink/clk.c
@@ -85,3 +85,9 @@ void __init plat_time_init(void)
clk_put(clk);
timer_probe();
}
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(clk_get_parent);
Naive implementation fails runtime with ENOENT in
devm_clk_get("10130000.mmc", "source") in spite of clock definition in .dts.
I traced the problem to CONFIG_COMMON_CLK not being defined for RALINK.
It cannot be enabled because it will lead to multiple definition of
several clock-related functions (e.g.: `clk_get_rate`).
I found completely disabling clock handling in mtk_sd.c leads to a (for
me) fully working SD card.
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 7726dcf48f2c..464f64bea7c6 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -730,18 +730,22 @@ static void msdc_set_timeout(struct msdc_host
*host, u32 ns, u32 clks)
static void msdc_gate_clock(struct msdc_host *host)
{
+#ifdef CONFIG_COMMON_CLK
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);
+#endif
}
static void msdc_ungate_clock(struct msdc_host *host)
{
+#ifdef CONFIG_COMMON_CLK
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);
+#endif
while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
cpu_relax();
}
@@ -2211,6 +2215,7 @@ static int msdc_drv_probe(struct platform_device
*pdev)
if (ret)
goto host_free;
+#ifdef CONFIG_COMMON_CLK
host->src_clk = devm_clk_get(&pdev->dev, "source");
if (IS_ERR(host->src_clk)) {
ret = PTR_ERR(host->src_clk);
@@ -2230,6 +2235,12 @@ static int msdc_drv_probe(struct platform_device
*pdev)
host->src_clk_cg = devm_clk_get(&pdev->dev, "source_cg");
if (IS_ERR(host->src_clk_cg))
host->src_clk_cg = NULL;
+#else
+ host->src_clk = NULL;
+ host->h_clk = NULL;
+ host->bus_clk = NULL;
+ host->src_clk_cg = NULL;
+#endif
host->irq = platform_get_irq(pdev, 0);
if (host->irq < 0) {
... but I'm unsure this hack-and-slash approach is the Right Thing to do ;)
As said: this works for me, but I would like to fix it properly and have
the fix sent upstream together with my SoM defconfig.
Any hint welcome
Regards
Mauro Condarelli
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: Enabling MMC on MT7628 SoC
2020-02-01 16:06 Enabling MMC on MT7628 SoC Mauro Condarelli
@ 2020-02-11 16:15 ` Matthias Brugger
2020-02-12 16:27 ` Mauro Condarelli
0 siblings, 1 reply; 3+ messages in thread
From: Matthias Brugger @ 2020-02-11 16:15 UTC (permalink / raw)
To: Mauro Condarelli, linux-mediatek, John Crispin, Ralf Baechle,
Paul Burton, linux-mips
[Adding MIPS people to the loop]
On 01/02/2020 17:06, Mauro Condarelli wrote:
> Hi,
> I'm trying to enable MMC/SD access on a VoCore2 SOM (based on MT7628)
> using mtk_sd driver.
>
> Just enabling mtk_sd will bomb wit undefined function `clk_get_parent`;
> this can be trivially cured with:
>
> diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c
> index 2f9d5acb38ea..930c2776f6fd 100644
> --- a/arch/mips/ralink/clk.c
> +++ b/arch/mips/ralink/clk.c
> @@ -85,3 +85,9 @@ void __init plat_time_init(void)
> clk_put(clk);
> timer_probe();
> }
> +
> +struct clk *clk_get_parent(struct clk *clk)
> +{
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(clk_get_parent);
>
>
> Naive implementation fails runtime with ENOENT in
> devm_clk_get("10130000.mmc", "source") in spite of clock definition in .dts.
>
> I traced the problem to CONFIG_COMMON_CLK not being defined for RALINK.
> It cannot be enabled because it will lead to multiple definition of
> several clock-related functions (e.g.: `clk_get_rate`).
> I found completely disabling clock handling in mtk_sd.c leads to a (for
> me) fully working SD card.
That's probably because the boot FW already enables the clocks as needed...
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 7726dcf48f2c..464f64bea7c6 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -730,18 +730,22 @@ static void msdc_set_timeout(struct msdc_host
> *host, u32 ns, u32 clks)
>
> static void msdc_gate_clock(struct msdc_host *host)
> {
> +#ifdef CONFIG_COMMON_CLK
> 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);
> +#endif
> }
>
> static void msdc_ungate_clock(struct msdc_host *host)
> {
> +#ifdef CONFIG_COMMON_CLK
> 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);
> +#endif
> while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
> cpu_relax();
> }
> @@ -2211,6 +2215,7 @@ static int msdc_drv_probe(struct platform_device
> *pdev)
> if (ret)
> goto host_free;
>
> +#ifdef CONFIG_COMMON_CLK
> host->src_clk = devm_clk_get(&pdev->dev, "source");
> if (IS_ERR(host->src_clk)) {
> ret = PTR_ERR(host->src_clk);
> @@ -2230,6 +2235,12 @@ static int msdc_drv_probe(struct platform_device
> *pdev)
> host->src_clk_cg = devm_clk_get(&pdev->dev, "source_cg");
> if (IS_ERR(host->src_clk_cg))
> host->src_clk_cg = NULL;
> +#else
> + host->src_clk = NULL;
> + host->h_clk = NULL;
> + host->bus_clk = NULL;
> + host->src_clk_cg = NULL;
> +#endif
>
> host->irq = platform_get_irq(pdev, 0);
> if (host->irq < 0) {
>
>
> ... but I'm unsure this hack-and-slash approach is the Right Thing to do ;)
>
I think the correct approach would be to write a clock driver which supports the
common clock framework.
The arch/mips/ralink/clk.c basically overwrites any calls to this so that things
somehow work.
Regards,
Matthias
> As said: this works for me, but I would like to fix it properly and have
> the fix sent upstream together with my SoM defconfig.
>
> Any hint welcome
> Regards
> Mauro Condarelli
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Enabling MMC on MT7628 SoC
2020-02-11 16:15 ` Matthias Brugger
@ 2020-02-12 16:27 ` Mauro Condarelli
0 siblings, 0 replies; 3+ messages in thread
From: Mauro Condarelli @ 2020-02-12 16:27 UTC (permalink / raw)
To: Matthias Brugger, linux-mediatek, John Crispin, Ralf Baechle,
Paul Burton, linux-mips
Thanks Matthias,
On 2/11/20 5:15 PM, Matthias Brugger wrote:
> [Adding MIPS people to the loop]
>
> On 01/02/2020 17:06, Mauro Condarelli wrote:
>> Hi,
>> I'm trying to enable MMC/SD access on a VoCore2 SOM (based on MT7628)
>> using mtk_sd driver.
>>
>> Just enabling mtk_sd will bomb wit undefined function `clk_get_parent`;
>> this can be trivially cured with:
>>
>> diff --git a/arch/mips/ralink/clk.c b/arch/mips/ralink/clk.c
>> index 2f9d5acb38ea..930c2776f6fd 100644
>> --- a/arch/mips/ralink/clk.c
>> +++ b/arch/mips/ralink/clk.c
>> @@ -85,3 +85,9 @@ void __init plat_time_init(void)
>> clk_put(clk);
>> timer_probe();
>> }
>> +
>> +struct clk *clk_get_parent(struct clk *clk)
>> +{
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(clk_get_parent);
>>
>>
>> Naive implementation fails runtime with ENOENT in
>> devm_clk_get("10130000.mmc", "source") in spite of clock definition in .dts.
>>
>> I traced the problem to CONFIG_COMMON_CLK not being defined for RALINK.
>> It cannot be enabled because it will lead to multiple definition of
>> several clock-related functions (e.g.: `clk_get_rate`).
>> I found completely disabling clock handling in mtk_sd.c leads to a (for
>> me) fully working SD card.
> That's probably because the boot FW already enables the clocks as needed...
>
>> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
>> index 7726dcf48f2c..464f64bea7c6 100644
>> --- a/drivers/mmc/host/mtk-sd.c
>> +++ b/drivers/mmc/host/mtk-sd.c
>> @@ -730,18 +730,22 @@ static void msdc_set_timeout(struct msdc_host
>> *host, u32 ns, u32 clks)
>>
>> static void msdc_gate_clock(struct msdc_host *host)
>> {
>> +#ifdef CONFIG_COMMON_CLK
>> 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);
>> +#endif
>> }
>>
>> static void msdc_ungate_clock(struct msdc_host *host)
>> {
>> +#ifdef CONFIG_COMMON_CLK
>> 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);
>> +#endif
>> while (!(readl(host->base + MSDC_CFG) & MSDC_CFG_CKSTB))
>> cpu_relax();
>> }
>> @@ -2211,6 +2215,7 @@ static int msdc_drv_probe(struct platform_device
>> *pdev)
>> if (ret)
>> goto host_free;
>>
>> +#ifdef CONFIG_COMMON_CLK
>> host->src_clk = devm_clk_get(&pdev->dev, "source");
>> if (IS_ERR(host->src_clk)) {
>> ret = PTR_ERR(host->src_clk);
>> @@ -2230,6 +2235,12 @@ static int msdc_drv_probe(struct platform_device
>> *pdev)
>> host->src_clk_cg = devm_clk_get(&pdev->dev, "source_cg");
>> if (IS_ERR(host->src_clk_cg))
>> host->src_clk_cg = NULL;
>> +#else
>> + host->src_clk = NULL;
>> + host->h_clk = NULL;
>> + host->bus_clk = NULL;
>> + host->src_clk_cg = NULL;
>> +#endif
>>
>> host->irq = platform_get_irq(pdev, 0);
>> if (host->irq < 0) {
>>
>>
>> ... but I'm unsure this hack-and-slash approach is the Right Thing to do ;)
>>
> I think the correct approach would be to write a clock driver which supports the
> common clock framework.
I'm afraid writing such a driver from scratch is over my time
allowance (and, most likely, also above my technical skills).
I can't, thus, volunteer for the task, but I'm surely available to help,
both with coding, reviewing and testing, if deemed useful.
Let me know if I can help somehow.
> The arch/mips/ralink/clk.c basically overwrites any calls to this so that things
> somehow work.
I've seen this is an almost-empty shell, but I have only vague
idea about what should actually be needed, sorry.
> Regards,
> Matthias
>
>> As said: this works for me, but I would like to fix it properly and have
>> the fix sent upstream together with my SoM defconfig.
I am cooperating with U-Boot folks to port it to this board (and,
apparently, with some success); I will check if some code can
be ported from there as MMC/SD seems to be working.
>> Any hint welcome
>> Regards
>> Mauro Condarelli
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Best Regards
Mauro Condarelli
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-12 16:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-01 16:06 Enabling MMC on MT7628 SoC Mauro Condarelli
2020-02-11 16:15 ` Matthias Brugger
2020-02-12 16:27 ` Mauro Condarelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).