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

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to