[PATCH] mmc: mtk-sd: Set src_clk rate/parent for accurate clock rate

[PATCH] mmc: mtk-sd: Set src_clk rate/parent for accurate clock rate

[AngeloGioacchino Del Regno]

The MediaTek MSDC controller has an internal divider for the input
(source) clock, but that may not be enough: as there's no multiplier
it is impossible to achieve certain clock rates depending on the
source clock rate.

This is especially seen with the SDR104 mode, where a clock source
typically of 200MHz or 400MHz will make us able to achieve 200MHz
(depending on the SoC's MSDCPLL, this will be more likely ~199MHz)
instead of the optimal 208MHz.

In order to solve this issue and achieve an accurate clock rate
for all modes, call clk_set_rate() on the source clock, so that
the clock framework will either change the PLL's rate or, more
likely, will switch the clock parent to the "best" one.

Since some platforms share one MSDCPLL for multiple MMC/SD "MSDC"
controllers, making sure that shared PLLs won't get their rate
changed (and devices over/underclocked) is obviously done in
the SoC-specific clock drivers, starting with commit
f235f6ae59e5 ("clk: mediatek: Remove CLK_SET_PARENT from all MSDC core clocks").

As an example, on MT8195, an accurate frequency will be achieved
by reparenting of the source clock from msdcpll div2 to univpll
div6-div2, giving out exactly 208000000Hz.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/mmc/host/mtk-sd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 8ce864169986..e0217783d22d 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -886,6 +886,13 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 		return;
 	}
 
+	/*
+	 * Select the best clock rate for src_clk: this is done in order
+	 * to save power and/or achieve an accurate rate for DDR52/SDR104.
+	 */
+	clk_set_rate(host->src_clk, hz);
+	host->src_clk_freq = clk_get_rate(host->src_clk);
+
 	flags = readl(host->base + MSDC_INTEN);
 	sdr_clr_bits(host->base + MSDC_INTEN, flags);
 	if (host->dev_comp->clk_div_bits == 8)
-- 
2.40.1

Re: [PATCH] mmc: mtk-sd: Set src_clk rate/parent for accurate clock rate

[Wenbin Mei (梅文彬)]

On Thu, 2023-06-15 at 09:47 +0200, AngeloGioacchino Del Regno wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  The MediaTek MSDC controller has an internal divider for the input
> (source) clock, but that may not be enough: as there's no multiplier
> it is impossible to achieve certain clock rates depending on the
> source clock rate.
> 
> This is especially seen with the SDR104 mode, where a clock source
> typically of 200MHz or 400MHz will make us able to achieve 200MHz
> (depending on the SoC's MSDCPLL, this will be more likely ~199MHz)
> instead of the optimal 208MHz.
> 
> In order to solve this issue and achieve an accurate clock rate
> for all modes, call clk_set_rate() on the source clock, so that
> the clock framework will either change the PLL's rate or, more
> likely, will switch the clock parent to the "best" one.
> 
> Since some platforms share one MSDCPLL for multiple MMC/SD "MSDC"
> controllers, making sure that shared PLLs won't get their rate
> changed (and devices over/underclocked) is obviously done in
> the SoC-specific clock drivers, starting with commit
> f235f6ae59e5 ("clk: mediatek: Remove CLK_SET_PARENT from all MSDC
> core clocks").
> 
> As an example, on MT8195, an accurate frequency will be achieved
> by reparenting of the source clock from msdcpll div2 to univpll
> div6-div2, giving out exactly 208000000Hz.
> 
Hi AngeloGioacchino,

We don't change the clock parent in kernel stage, which is fixed in
bootloader stage.
As we know, the clock has some jitter, so we can not set the clock to
max value(200M or 208M), the current setting is what we expected.

Begards,
Wenbin

> Signed-off-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  drivers/mmc/host/mtk-sd.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 8ce864169986..e0217783d22d 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -886,6 +886,13 @@ static void msdc_set_mclk(struct msdc_host
> *host, unsigned char timing, u32 hz)
>  		return;
>  	}
>  
> +	/*
> +	 * Select the best clock rate for src_clk: this is done in
> order
> +	 * to save power and/or achieve an accurate rate for
> DDR52/SDR104.
> +	 */
> +	clk_set_rate(host->src_clk, hz);
> +	host->src_clk_freq = clk_get_rate(host->src_clk);
> +
>  	flags = readl(host->base + MSDC_INTEN);
>  	sdr_clr_bits(host->base + MSDC_INTEN, flags);
>  	if (host->dev_comp->clk_div_bits == 8)
> -- 
> 2.40.1