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