[U-Boot] [RESEND PATCH v3 1/2] mmc: rockchip_sdhci: add clock init for mmc

[Stefan Herbrechtsmeier]

Hi,

Am 02.01.2017 um 02:29 schrieb Jaehoon Chung:
> Hi Stefan,
[snip]

>>> Of_course, it needs to consider the base clock broken case.
>> The whole discussion is about the base clock broken case. Otherwise the host->max_clk is extracted from the SDHCI_CAPABILITIES.
>> The linux kernel use a callback to request the host->max_clk from the driver in the base clock broken case. The current u-boot implementation only supports the host->max_clk but call it unfortunately mmc->cfg->f_max which could be mistaken as mmc->f_max from the kernel which represents max-frequency.
>>
> Linux kernel has two options. One is get_max_clock(), the other is "using 'max-frequency' property".
The 'max-frequency' is read in host.c into f_max and used to limit the 
maximum clock in mmc_set_clock() in core.c.

In sdhci.c the f_max is set to max_clk if f_max is zero or higher then 
max_clk.

The whole sdhci.c use only the max_clk to calculate the divider and 
multiplier for the requested clock.

Most sdhci-*.c drivers use a preset clock rate or set a fixed clock rate.

Only  the  sdhci-bcm-kona.c and sdhci-st.c use the f_max for 
clk_set_rate(). They should instead use the 'clock-frequency' or 
'assigned-clock-rates'. Especially the last assume three specific clock 
rates and set the clock to a minimum of 50 MHz even for a lower 
'max-frequency'.

You always need to values. One to set the clock and one to limit the 
max-frequency independent of the base clock.

>>>>> Kever's patch is not problem.
>>>> The problem is that the patch "init the clock rate to max-frequency" and this is wrong and differs from the kernel which use the assigned-clock-rates. What happens if somebody sets the max-frequency to 400000? Does the clock controller supports such a low frequency? What happens if the clock controller use a different clock as requested and the mmc framework assume the requested clock rate?
>>> Agreed this point, It needs to implement the clk_set_rate() in rockchip_sdhci.c. with value passed by set_ios().
>> Does we speak about the sdhci or dw_mmc controller? The sdhci don't change the host->max_clk and don't need the clk_set_rate(). It have its own divider and optional multiplier and doesn't change the base clock.
>>
> sdhci and dwmmc controller are using the clk_set_rate() in each SoC's drivers.
Where can I find this code? Even in u-boot some drivers use the preset 
clock values.

> Since driver's divider has a limitation. So i think it needs to use the "clk_set_rate()".
Do you plan to change all Linux drivers?

>
>>>> The mmc drivers shouldn't use the max-frequency to request a clock rate. It should only request the current clock rate or set a default clock rate independent of the max-frequency.
>> Back to this patch. It should use the CONFIG_ROCKCHIP_SDHCI_MAX_FREQ in the clk_set_rate() or use the default rate and only request it with clk_get_rate().
>>
>> Maybe my last patch could be generalized and the max-frequency support could be moved inside the sdhci driver.
> I don't want to use CONFIG_ROCKCHIP_SDHCI_MAX_FREQ...i will remove the all CONFIG_xxxx for using value.
But then you need to encode this specific values inside the device tree 
and don't misuse the 'max-frequency' property. You need to support a 
fixed clock frequency for sdhci controller with a lower 'max-frequency' 
for the external mmc interface.

Regards
   Stefan