* [U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc
@ 2016-12-27 13:09 ` Kever Yang
2016-12-27 22:08 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: Kever Yang @ 2016-12-27 13:09 UTC (permalink / raw)
To: u-boot
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;
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc
2016-12-27 13:09 ` [U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc Kever Yang
@ 2016-12-27 22:08 ` Jaehoon Chung
2016-12-28 1:12 ` Kever Yang
0 siblings, 1 reply; 6+ messages in thread
From: Jaehoon Chung @ 2016-12-27 22:08 UTC (permalink / raw)
To: u-boot
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;
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc
2016-12-27 22:08 ` Jaehoon Chung
@ 2016-12-28 1:12 ` Kever Yang
2016-12-28 1:34 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: Kever Yang @ 2016-12-28 1:12 UTC (permalink / raw)
To: u-boot
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 at 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;
>>
>>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc
2016-12-28 1:12 ` Kever Yang
@ 2016-12-28 1:34 ` Jaehoon Chung
2016-12-28 1:45 ` Kever Yang
0 siblings, 1 reply; 6+ messages in thread
From: Jaehoon Chung @ 2016-12-28 1:34 UTC (permalink / raw)
To: u-boot
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 at 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;
>>>
>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc
2016-12-28 1:34 ` Jaehoon Chung
@ 2016-12-28 1:45 ` Kever Yang
2016-12-28 2:03 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: Kever Yang @ 2016-12-28 1:45 UTC (permalink / raw)
To: u-boot
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 at 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 at 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;
>>>>
>>>
>>>
>>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc
2016-12-28 1:45 ` Kever Yang
@ 2016-12-28 2:03 ` Jaehoon Chung
0 siblings, 0 replies; 6+ messages in thread
From: Jaehoon Chung @ 2016-12-28 2:03 UTC (permalink / raw)
To: u-boot
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 at 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 at 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;
>>>>>
>>>>
>>>>
>>>
>>
>>
>>
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-28 2:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20161227131000epcas2p346436031cb1af4ade4ff7b0374a2bd4b@epcas2p3.samsung.com>
2016-12-27 13:09 ` [U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc Kever Yang
2016-12-27 22:08 ` Jaehoon Chung
2016-12-28 1:12 ` Kever Yang
2016-12-28 1:34 ` Jaehoon Chung
2016-12-28 1:45 ` Kever Yang
2016-12-28 2:03 ` Jaehoon Chung
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.