Linux-MIPS Archive on lore.kernel.org
 help / color / Atom feed
From: Matthias Brugger <matthias.bgg@gmail.com>
To: Mauro Condarelli <mc5686@mclink.it>,
	linux-mediatek@lists.infradead.org,
	John Crispin <john@phrozen.org>,
	Ralf Baechle <ralf@linux-mips.org>,
	Paul Burton <paulburton@kernel.org>,
	linux-mips@vger.kernel.org
Subject: Re: Enabling MMC on MT7628 SoC
Date: Tue, 11 Feb 2020 17:15:44 +0100
Message-ID: <ae707c5d-3c3c-724d-1eba-adcb1db52eb9@gmail.com> (raw)
In-Reply-To: <e6c30f55-5f65-b165-4c5d-1d25a425e744@mclink.it>

[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
> 

       reply index

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <e6c30f55-5f65-b165-4c5d-1d25a425e744@mclink.it>
2020-02-11 16:15 ` Matthias Brugger [this message]
2020-02-12 16:27   ` Mauro Condarelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ae707c5d-3c3c-724d-1eba-adcb1db52eb9@gmail.com \
    --to=matthias.bgg@gmail.com \
    --cc=john@phrozen.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=mc5686@mclink.it \
    --cc=paulburton@kernel.org \
    --cc=ralf@linux-mips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-MIPS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mips/0 linux-mips/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mips linux-mips/ https://lore.kernel.org/linux-mips \
		linux-mips@vger.kernel.org
	public-inbox-index linux-mips

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-mips


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git