mmc: rockchip_sdhci: add clock init for mmc

PATCH RESEND Superseded Kever Yang 2 participants 6 messages Dec 27, 2016 – Dec 28, 2016

Subsystems (2)

MMC, SD and SDIO Arm Rockchip

lore

Share & tools

lore

Kever Yang

author

2016-12-27 13:09 UTC #0 lore raw

PATCH RESEND v2 1/1 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 <[email protected]>

---

Changes in v2:
- using the return value

 drivers/mmc/rockchip_sdhci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/mmc/rockchip_sdhci.c +11 -0

index c56e1a3..96049f3 100644
@@ -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;

Signature · 1 line

1.9.1

Jaehoon Chung

2016-12-27 22:08 UTC #1 lore raw

PATCH RESEND v2 1/1 mmc: rockchip_sdhci: add clock init for mmc

Hi Kever,

Quoted 25 lines · Kever Yang

Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by Kever Yang <[email protected]>
---
Changes in v2:
- using the return value
 drivers/mmc/rockchip_sdhci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
drivers/mmc/rockchip_sdhci.c +1 -0
index c56e1a3..96049f3 100644
@@ -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

drivers/mmc/rockchip_sdhci.c static int arasan_sdhci_probe(struct udevice *dev) · line 41

Quoted 8 lines

drivers/mmc/rockchip_sdhci.c +5 -0

@@ -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

Quoted 9 lines

drivers/mmc/rockchip_sdhci.c +5 -0

+		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;

Kever Yang

author

2016-12-28 01:12 UTC #2 lore raw

PATCH RESEND v2 1/1 mmc: rockchip_sdhci: add clock init for mmc

Hi Jaehoon,

Quoted 25 lines · Jaehoon Chung

Hi Kever,
Quoted 22 lines · Kever Yang
Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by Kever Yang <[email protected]>
---
Changes in v2:
- using the return value
  drivers/mmc/rockchip_sdhci.c | 11 +++++++++++
  1 file changed, 11 insertions(+)
drivers/mmc/rockchip_sdhci.c +1 -0
index c56e1a3..96049f3 100644
@@ -12,6 +12,7 @@
  #include <libfdt.h>
  #include <malloc.h>
  #include <sdhci.h>
+#include <clk.h>
 

Quoted 12 lines

drivers/mmc/rockchip_sdhci.c static int arasan_sdhci_probe(struct udevice *dev) · line 41

Quoted 11 lines

drivers/mmc/rockchip_sdhci.c +5 -0

  /* 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

Quoted 16 lines

Best Regards,
Jaehoon Chung

Quoted 9 lines

drivers/mmc/rockchip_sdhci.c +5 -0

+		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;

Jaehoon Chung

2016-12-28 01:34 UTC #3 lore raw

PATCH RESEND v2 1/1 mmc: rockchip_sdhci: add clock init for mmc

Hi Kever,

Quoted 55 lines · Kever Yang

Hi Jaehoon,

Quoted 28 lines · Jaehoon Chung

Hi Kever,
Quoted 25 lines · Kever Yang
Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by Kever Yang <[email protected]>
---
Changes in v2:
- using the return value
  drivers/mmc/rockchip_sdhci.c | 11 +++++++++++
  1 file changed, 11 insertions(+)
drivers/mmc/rockchip_sdhci.c +1 -0
index c56e1a3..96049f3 100644
@@ -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);

Quoted 8 lines

drivers/mmc/rockchip_sdhci.c static int arasan_sdhci_probe(struct udevice *dev) · line 41
Quoted 7 lines
drivers/mmc/rockchip_sdhci.c +5 -0
      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";
        };

Quoted 2 lines

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

Quoted 20 lines

Thanks,
- Kever

Quoted 14 lines

Best Regards,
Jaehoon Chung

Quoted 7 lines

drivers/mmc/rockchip_sdhci.c +5 -0

+        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;

Kever Yang

author

2016-12-28 01:45 UTC #4 lore raw

PATCH RESEND v2 1/1 mmc: rockchip_sdhci: add clock init for mmc

Hi Jaehoon,

Quoted 64 lines · Jaehoon Chung

Hi Kever,
Quoted 54 lines · Kever Yang
Hi Jaehoon,
Quoted 28 lines · Jaehoon Chung
Hi Kever,
Quoted 25 lines · Kever Yang
Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by Kever Yang <[email protected]>
---
Changes in v2:
- using the return value
   drivers/mmc/rockchip_sdhci.c | 11 +++++++++++
   1 file changed, 11 insertions(+)
drivers/mmc/rockchip_sdhci.c +1 -0
index c56e1a3..96049f3 100644
@@ -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);
Quoted 8 lines
drivers/mmc/rockchip_sdhci.c static int arasan_sdhci_probe(struct udevice *dev) · line 41
Quoted 7 lines
drivers/mmc/rockchip_sdhci.c +5 -0
       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";
         };
Quoted 2 lines
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.

Quoted 5 lines

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

Quoted 23 lines

Best Regards,
Jaehoon Chung
Quoted 16 lines
Thanks,
- Kever
Quoted 12 lines
Best Regards,
Jaehoon Chung
Quoted 7 lines
drivers/mmc/rockchip_sdhci.c +5 -0
+        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;
 

Jaehoon Chung

2016-12-28 02:03 UTC #5 lore raw

PATCH RESEND v2 1/1 mmc: rockchip_sdhci: add clock init for mmc

Quoted 67 lines · Kever Yang

Hi Jaehoon,
Quoted 64 lines · Jaehoon Chung
Hi Kever,
Quoted 54 lines · Kever Yang
Hi Jaehoon,
Quoted 28 lines · Jaehoon Chung
Hi Kever,
Quoted 25 lines · Kever Yang
Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver api.
Signed-off-by Kever Yang <[email protected]>
---
Changes in v2:
- using the return value
   drivers/mmc/rockchip_sdhci.c | 11 +++++++++++
   1 file changed, 11 insertions(+)
drivers/mmc/rockchip_sdhci.c +1 -0
index c56e1a3..96049f3 100644
@@ -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);
Quoted 8 lines
drivers/mmc/rockchip_sdhci.c static int arasan_sdhci_probe(struct udevice *dev) · line 41
Quoted 7 lines
drivers/mmc/rockchip_sdhci.c +5 -0
       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";
         };
Quoted 2 lines
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.

Quoted 7 lines

Quoted 5 lines
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.

Quoted 15 lines

        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";
        };

Quoted 2 lines

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?

Quoted 2 lines

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

Quoted 32 lines

Thanks,
- Kever
Quoted 23 lines
Best Regards,
Jaehoon Chung
Quoted 16 lines
Thanks,
- Kever
Quoted 12 lines
Best Regards,
Jaehoon Chung
Quoted 7 lines
drivers/mmc/rockchip_sdhci.c +5 -0
+        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;
 