[U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc

Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by: Kever Yang kever.yang@rock-chips.com ---
Changes in v2: - using the return value
drivers/mmc/rockchip_sdhci.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index c56e1a3..96049f3 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -12,6 +12,7 @@ #include <libfdt.h> #include <malloc.h> #include <sdhci.h> +#include <clk.h>
/* 400KHz is max freq for card ID etc. Use that as min */ #define EMMC_MIN_FREQ 400000 @@ -33,6 +34,16 @@ static int arasan_sdhci_probe(struct udevice *dev) struct rockchip_sdhc *prv = dev_get_priv(dev); struct sdhci_host *host = &prv->host; int ret; + struct clk clk; + + ret = clk_get_by_index(dev, 0, &clk); + if (!ret) { + ret = clk_set_rate(&clk, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ); + if (IS_ERR_VALUE(ret)) + printf("%s clk set rate fail!\n", __func__); + } else { + printf("%s fail to get clk\n", __func__); + }
host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;

Hi Kever,
On 12/27/2016 10:09 PM, Kever Yang wrote:
Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
Changes in v2:
- using the return value
drivers/mmc/rockchip_sdhci.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index c56e1a3..96049f3 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -12,6 +12,7 @@ #include <libfdt.h> #include <malloc.h> #include <sdhci.h> +#include <clk.h>
/* 400KHz is max freq for card ID etc. Use that as min */ #define EMMC_MIN_FREQ 400000 @@ -33,6 +34,16 @@ static int arasan_sdhci_probe(struct udevice *dev) struct rockchip_sdhc *prv = dev_get_priv(dev); struct sdhci_host *host = &prv->host; int ret;
- struct clk clk;
- ret = clk_get_by_index(dev, 0, &clk);
- if (!ret) {
ret = clk_set_rate(&clk, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ);
How about getting clock value from dt?
Best Regards, Jaehoon Chung
if (IS_ERR_VALUE(ret))
printf("%s clk set rate fail!\n", __func__);
} else {
printf("%s fail to get clk\n", __func__);
}
host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;

Hi Jaehoon,
On 12/28/2016 06:08 AM, Jaehoon Chung wrote:
Hi Kever,
On 12/27/2016 10:09 PM, Kever Yang wrote:
Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
Changes in v2:
using the return value
drivers/mmc/rockchip_sdhci.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index c56e1a3..96049f3 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -12,6 +12,7 @@ #include <libfdt.h> #include <malloc.h> #include <sdhci.h> +#include <clk.h>
/* 400KHz is max freq for card ID etc. Use that as min */ #define EMMC_MIN_FREQ 400000 @@ -33,6 +34,16 @@ static int arasan_sdhci_probe(struct udevice *dev) struct rockchip_sdhc *prv = dev_get_priv(dev); struct sdhci_host *host = &prv->host; int ret;
- struct clk clk;
- ret = clk_get_by_index(dev, 0, &clk);
- if (!ret) {
ret = clk_set_rate(&clk, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ);
How about getting clock value from dt?
SDHCI dts node is like below: sdhci: sdhci@fe330000 { u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1"; reg = <0x0 0xfe330000 0x0 0x10000>; interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; assigned-clocks = <&cru SCLK_EMMC>; assigned-clock-rates = <200000000>; clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>; clock-names = "clk_xin", "clk_ahb"; phys = <&emmc_phy>; phy-names = "phy_arasan"; status = "disabled"; };
There is an assigned-clock-rates, which is parsed by clock driver in kernel, but the U-Boot clk driver do not have this feature, is it OK for us to parse it in sdhci driver?
Thanks, - Kever
Best Regards, Jaehoon Chung
if (IS_ERR_VALUE(ret))
printf("%s clk set rate fail!\n", __func__);
} else {
printf("%s fail to get clk\n", __func__);
}
host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;

Hi Kever,
On 12/28/2016 10:12 AM, Kever Yang wrote:
Hi Jaehoon,
On 12/28/2016 06:08 AM, Jaehoon Chung wrote:
Hi Kever,
On 12/27/2016 10:09 PM, Kever Yang wrote:
Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
Changes in v2:
using the return value
drivers/mmc/rockchip_sdhci.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index c56e1a3..96049f3 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -12,6 +12,7 @@ #include <libfdt.h> #include <malloc.h> #include <sdhci.h> +#include <clk.h> /* 400KHz is max freq for card ID etc. Use that as min */ #define EMMC_MIN_FREQ 400000 @@ -33,6 +34,16 @@ static int arasan_sdhci_probe(struct udevice *dev) struct rockchip_sdhc *prv = dev_get_priv(dev); struct sdhci_host *host = &prv->host; int ret;
- struct clk clk;
- ret = clk_get_by_index(dev, 0, &clk);
- if (!ret) {
ret = clk_set_rate(&clk, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ);
How about getting clock value from dt?
SDHCI dts node is like below: sdhci: sdhci@fe330000 { u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1"; reg = <0x0 0xfe330000 0x0 0x10000>; interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; assigned-clocks = <&cru SCLK_EMMC>; assigned-clock-rates = <200000000>; clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>; clock-names = "clk_xin", "clk_ahb"; phys = <&emmc_phy>; phy-names = "phy_arasan"; status = "disabled"; };
There is an assigned-clock-rates, which is parsed by clock driver in kernel, but the U-Boot clk driver do not have this feature, is it OK for us to parse it in sdhci driver?
Yes, Maybe it's possible to parse it in sdhci driver. (assigned-clock-rate might be for assigning to initial source clock value.) In Linux kernel, there is also similar property like "max-frequency". MMC can have the difference maximum frequency for each SoC.
I'm considering that implement the generic mmc parsing function. Because some property can be used commonly.
If you put the parser in sdhci driver, i will rework it into generic parsing function in future. How about? :)
Best Regards, Jaehoon Chung
Thanks,
- Kever
Best Regards, Jaehoon Chung
if (IS_ERR_VALUE(ret))
printf("%s clk set rate fail!\n", __func__);
- } else {
printf("%s fail to get clk\n", __func__);
- } host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;

Hi Jaehoon,
On 12/28/2016 09:34 AM, Jaehoon Chung wrote:
Hi Kever,
On 12/28/2016 10:12 AM, Kever Yang wrote:
Hi Jaehoon,
On 12/28/2016 06:08 AM, Jaehoon Chung wrote:
Hi Kever,
On 12/27/2016 10:09 PM, Kever Yang wrote:
Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
Changes in v2:
using the return value
drivers/mmc/rockchip_sdhci.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index c56e1a3..96049f3 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -12,6 +12,7 @@ #include <libfdt.h> #include <malloc.h> #include <sdhci.h> +#include <clk.h> /* 400KHz is max freq for card ID etc. Use that as min */ #define EMMC_MIN_FREQ 400000 @@ -33,6 +34,16 @@ static int arasan_sdhci_probe(struct udevice *dev) struct rockchip_sdhc *prv = dev_get_priv(dev); struct sdhci_host *host = &prv->host; int ret;
- struct clk clk;
- ret = clk_get_by_index(dev, 0, &clk);
- if (!ret) {
ret = clk_set_rate(&clk, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ);
How about getting clock value from dt?
SDHCI dts node is like below: sdhci: sdhci@fe330000 { u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1"; reg = <0x0 0xfe330000 0x0 0x10000>; interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; assigned-clocks = <&cru SCLK_EMMC>; assigned-clock-rates = <200000000>; clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>; clock-names = "clk_xin", "clk_ahb"; phys = <&emmc_phy>; phy-names = "phy_arasan"; status = "disabled"; };
There is an assigned-clock-rates, which is parsed by clock driver in kernel, but the U-Boot clk driver do not have this feature, is it OK for us to parse it in sdhci driver?
Yes, Maybe it's possible to parse it in sdhci driver. (assigned-clock-rate might be for assigning to initial source clock value.) In Linux kernel, there is also similar property like "max-frequency". MMC can have the difference maximum frequency for each SoC.
I'm considering that implement the generic mmc parsing function. Because some property can be used commonly.
If you put the parser in sdhci driver, i will rework it into generic parsing function in future. How about? :)
Look into another sdmmc dts node(dwmmc), I can found clk-freq-min-max, I think this is more generic for add the SDHCI driver, right? sdmmc: dwmmc@fe320000 { compatible = "rockchip,rk3399-dw-mshc", "rockchip,rk3288-dw-mshc"; reg = <0x0 0xfe320000 0x0 0x4000>; interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>; clock-freq-min-max = <400000 150000000>; clocks = <&cru SCLK_SDMMC>, <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC_DRV>, <&cru SCLK_SDMMC_SAMPLE>; clock-names = "ciu", "biu", "ciu-drive", "ciu-sample"; pinctrl-names = "default"; pinctrl-0 = <&sdmmc_clk>; fifo-depth = <0x100>; status = "disabled"; };
The U-Boot dwmmc driver will set the clock via system clock API, but SDHCI driver do not do that, that's why I have to init the clock in rockchip_sdhci.
I hope everything are working like kernel, you also working on kernel mmc driver, right?
Thanks, - Kever
Best Regards, Jaehoon Chung
Thanks,
- Kever
Best Regards, Jaehoon Chung
if (IS_ERR_VALUE(ret))
printf("%s clk set rate fail!\n", __func__);
- } else {
printf("%s fail to get clk\n", __func__);
- } host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;

On 12/28/2016 10:45 AM, Kever Yang wrote:
Hi Jaehoon,
On 12/28/2016 09:34 AM, Jaehoon Chung wrote:
Hi Kever,
On 12/28/2016 10:12 AM, Kever Yang wrote:
Hi Jaehoon,
On 12/28/2016 06:08 AM, Jaehoon Chung wrote:
Hi Kever,
On 12/27/2016 10:09 PM, Kever Yang wrote:
Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
Changes in v2:
using the return value
drivers/mmc/rockchip_sdhci.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c index c56e1a3..96049f3 100644 --- a/drivers/mmc/rockchip_sdhci.c +++ b/drivers/mmc/rockchip_sdhci.c @@ -12,6 +12,7 @@ #include <libfdt.h> #include <malloc.h> #include <sdhci.h> +#include <clk.h> /* 400KHz is max freq for card ID etc. Use that as min */ #define EMMC_MIN_FREQ 400000 @@ -33,6 +34,16 @@ static int arasan_sdhci_probe(struct udevice *dev) struct rockchip_sdhc *prv = dev_get_priv(dev); struct sdhci_host *host = &prv->host; int ret;
- struct clk clk;
- ret = clk_get_by_index(dev, 0, &clk);
- if (!ret) {
ret = clk_set_rate(&clk, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ);
How about getting clock value from dt?
SDHCI dts node is like below: sdhci: sdhci@fe330000 { u-boot,dm-pre-reloc; compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1"; reg = <0x0 0xfe330000 0x0 0x10000>; interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>; assigned-clocks = <&cru SCLK_EMMC>; assigned-clock-rates = <200000000>; clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>; clock-names = "clk_xin", "clk_ahb"; phys = <&emmc_phy>; phy-names = "phy_arasan"; status = "disabled"; };
There is an assigned-clock-rates, which is parsed by clock driver in kernel, but the U-Boot clk driver do not have this feature, is it OK for us to parse it in sdhci driver?
Yes, Maybe it's possible to parse it in sdhci driver. (assigned-clock-rate might be for assigning to initial source clock value.) In Linux kernel, there is also similar property like "max-frequency". MMC can have the difference maximum frequency for each SoC.
I'm considering that implement the generic mmc parsing function. Because some property can be used commonly.
If you put the parser in sdhci driver, i will rework it into generic parsing function in future. How about? :)
Look into another sdmmc dts node(dwmmc), I can found clk-freq-min-max, I think this is more generic for add the SDHCI driver, right?
Yes, likes them.
sdmmc: dwmmc@fe320000 { compatible = "rockchip,rk3399-dw-mshc", "rockchip,rk3288-dw-mshc"; reg = <0x0 0xfe320000 0x0 0x4000>; interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>; clock-freq-min-max = <400000 150000000>; clocks = <&cru SCLK_SDMMC>, <&cru HCLK_SDMMC>, <&cru SCLK_SDMMC_DRV>, <&cru SCLK_SDMMC_SAMPLE>; clock-names = "ciu", "biu", "ciu-drive", "ciu-sample"; pinctrl-names = "default"; pinctrl-0 = <&sdmmc_clk>; fifo-depth = <0x100>; status = "disabled"; };
The U-Boot dwmmc driver will set the clock via system clock API, but SDHCI driver do not do that, that's why I have to init the clock in rockchip_sdhci.
Now, I think that it's enough to put the parsing code in your rockchip_sdhci.c, isn't?
arasan_sdhci_probe() { .. parsing "max-frequency" clk_get_by_index(); clk_set_rate(); ..
}
Is it working?
I hope everything are working like kernel, you also working on kernel mmc driver, right?
Yes, i'm working on also kernel side. I also hope so. :)
Best Regards, Jaehoon Chung
Thanks,
- Kever
Best Regards, Jaehoon Chung
Thanks,
- Kever
Best Regards, Jaehoon Chung
if (IS_ERR_VALUE(ret))
printf("%s clk set rate fail!\n", __func__);
- } else {
printf("%s fail to get clk\n", __func__);
- } host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
participants (2)
-
Jaehoon Chung
-
Kever Yang