Skip to contentSkip to conversation

mmc: atmel_sdhci: Convert to the driver model support

PATCH Superseded Wenyou Yang 4 participants 5 messages Jul 20, 2016 – Jul 25, 2016

Subsystems (2)

v7 of 11 Superseded Latest is v11 →

lore
Share & tools
lore
#0 lore raw

PATCH v7 1/1 mmc: atmel_sdhci: Convert to the driver model support

Convert the driver to the driver model while retaining the existing legacy code. This allows the driver to support boards that have converted to driver model as well as those that have not.

---

Changes in v7:
- Add support for using driver model for block devices and MMC operations.
- Change clk_client.h -> clk.h to adapt to clk API conversion.

Changes in v6:
- Remove unnecessary white space.
- Use sdhci_read(), instead of readl().
- Remove the local variables min_clk.

Changes in v5:
- Add Reviewed-by tag.

Changes in v4:
- Update the clk API based on [PATCH] clk: convert API to match reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
- Remove check on dev_get_parent() return.
- Fixed the return value, such as -ENODEV->-EINVAL.

Changes in v3:
- Remove the redundant log print.

Changes in v2:
- Add clock support, include enabling peripheral clock and generated clock.
- Retain the existing legacy code to support boards which have not converted to driver model.

 drivers/mmc/Kconfig       |  10 ++++
 drivers/mmc/atmel_sdhci.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
 include/sdhci.h           |   2 +
 3 files changed, 137 insertions(+)
drivers/mmc/Kconfig +10 -0
index 79cf18f..49b325e 100644
@@ -34,6 +34,16 @@ config MSM_SDHCI
           SD 3.0 specifications. Both SD and eMMC devices are supported.
 	  Card-detect gpios are not supported.
 
+config ATMEL_SDHCI
+	bool "Atmel SDHCI controller support"
+	depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
+	help
+	  This enables support for the Atmel SDHCI controller, which supports
+	  the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
+	  Memory Card Specification V3.0, and the SDIO V3.0 specification.
+	  It is compliant with the SD Host Controller Standard V3.0
+	  specification.
+
 config ROCKCHIP_DWMMC
 	bool "Rockchip SD/MMC controller support"
 	depends on DM_MMC && OF_CONTROL
drivers/mmc/atmel_sdhci.c +125 -0

140 lines not on this page · raw message

include/sdhci.h +2 -0
index c4d3b55..fdc0032 100644
@@ -166,6 +166,8 @@
 #define  SDHCI_CAN_64BIT	0x10000000
 
 #define SDHCI_CAPABILITIES_1	0x44
+#define  SDHCI_CLOCK_MUL_MASK	0x00FF0000
+#define  SDHCI_CLOCK_MUL_SHIFT	16
 
 #define SDHCI_MAX_CURRENT	0x48
 
Signature · 1 line
2.7.4
#1 lore raw

PATCH v7 1/1 mmc: atmel_sdhci: Convert to the driver model support

Hi Wenyuo,

Quoted 66 lines · Wenyou Yang

Convert the driver to the driver model while retaining the existing legacy code. This allows the driver to support boards that have converted to driver model as well as those that have not.

---

Changes in v7:
- Add support for using driver model for block devices and MMC operations.
- Change clk_client.h -> clk.h to adapt to clk API conversion.

Changes in v6:
- Remove unnecessary white space.
- Use sdhci_read(), instead of readl().
- Remove the local variables min_clk.

Changes in v5:
- Add Reviewed-by tag.

Changes in v4:
- Update the clk API based on [PATCH] clk: convert API to match reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
- Remove check on dev_get_parent() return.
- Fixed the return value, such as -ENODEV->-EINVAL.

Changes in v3:
- Remove the redundant log print.

Changes in v2:
- Add clock support, include enabling peripheral clock and generated clock.
- Retain the existing legacy code to support boards which have not converted to driver model.

 drivers/mmc/Kconfig       |  10 ++++
 drivers/mmc/atmel_sdhci.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
 include/sdhci.h           |   2 +
 3 files changed, 137 insertions(+)
drivers/mmc/Kconfig +10 -0
index 79cf18f..49b325e 100644
@@ -34,6 +34,16 @@ config MSM_SDHCI
           SD 3.0 specifications. Both SD and eMMC devices are supported.
 	  Card-detect gpios are not supported.
 
+config ATMEL_SDHCI
+	bool "Atmel SDHCI controller support"
+	depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
+	help
+	  This enables support for the Atmel SDHCI controller, which supports
+	  the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
+	  Memory Card Specification V3.0, and the SDIO V3.0 specification.
+	  It is compliant with the SD Host Controller Standard V3.0
+	  specification.
+
 config ROCKCHIP_DWMMC
 	bool "Rockchip SD/MMC controller support"
 	depends on DM_MMC && OF_CONTROL
drivers/mmc/atmel_sdhci.c +0 -0
index 24b68b6..977f6ef 100644

drivers/mmc/atmel_sdhci.c line 10

Quoted 5 lines
drivers/mmc/atmel_sdhci.c +2 -0
@@ -6,12 +6,15 @@
  */
 
 #include <common.h>
+#include <clk.h>
+#include <dm.h>

If it doesn't use DM_MMC, then doesn't it need to include dm.h?

Quoted 34 lines
drivers/mmc/atmel_sdhci.c +21 -0
 #include <malloc.h>
 #include <sdhci.h>
 #include <asm/arch/clk.h>
 
 #define ATMEL_SDHC_MIN_FREQ	400000
 
+#ifndef CONFIG_DM_MMC
 int atmel_sdhci_init(void *regbase, u32 id)
 {
 	struct sdhci_host *host;
@@ -38,3 +41,125 @@ int atmel_sdhci_init(void *regbase, u32 id)
 
 	return 0;
 }
+
+#else
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct atmel_sdhci_plat {
+	struct mmc_config cfg;
+	struct mmc mmc;
+};
+
+static int atmel_sdhci_probe(struct udevice *dev)
+{
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
+	struct sdhci_host *host = dev_get_priv(dev);
+	u32 max_clk;
+	u32 caps, caps_1;
+	u32 clk_base, clk_mul;
+	u32 gck_rate;
+	struct udevice *dev_clk;

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id) · line 73

Quoted 10 lines
drivers/mmc/atmel_sdhci.c +10 -0
+	struct clk clk;
+	int periph, ret;
+
+	ret = clk_get_by_index(dev, 0, &clk);
+	if (ret)
+		return ret;
+
+	periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
+	if (periph < 0)
+		return -EINVAL;

periph is just used to pass to clk.id. it looks meaningless. As i already mentioned, you can make more cleaner than now.

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id) · line 85

Quoted 12 lines
drivers/mmc/atmel_sdhci.c +12 -0
+
+	dev_clk = dev_get_parent(clk.dev);
+	ret = clk_request(dev_clk, &clk);
+	if (ret)
+		return ret;
+
+	clk.id = periph;
+	ret = clk_enable(&clk);
+	if (ret)
+		return ret;
+
+	host->name = (char *)dev->name;

Needs the type casting?

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id) · line 98

Quoted 13 lines
drivers/mmc/atmel_sdhci.c +13 -0
+	host->ioaddr = (void *)dev_get_addr(dev);
+
+	host->quirks = 0;
+	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
+
+	host->bus_width	= fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+					 "bus-width", 4);
+
+	caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+	clk_base = (caps & SDHCI_CLOCK_V3_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
+	caps_1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+	clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
+	gck_rate = clk_base * 1000000 * (clk_mul + 1);

gck_rate can be over u32 boundary, doesn't?

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id) · line 102

Quoted 4 lines
drivers/mmc/atmel_sdhci.c +4 -0
+
+	ret = clk_get_by_index(dev, 1, &clk);
+	if (ret)
+		return ret;

It can reuse the above clock get sequence.

If reuse this sequence, lines can be reduced.

My suggestion is

1. Make atmel_sdhci_get_clk(..., int index), it should be returned "struct clk" type.

2. Then the below sequence should be included in function,

atmel_sdhci_get_clk(..., int index) {

	clk_get_by_index(dev, index, &clk);
	...

	return clk;
}

How about?

Best Regards,
Jaehoon Chung

Quoted 77 lines
drivers/mmc/atmel_sdhci.c +63 -0
+
+	periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
+	if (periph < 0)
+		return -EINVAL;
+
+	dev_clk = dev_get_parent(clk.dev);
+	ret = clk_request(dev_clk, &clk);
+	if (ret)
+		return ret;
+
+	clk.id = periph;
+	ret = clk_set_rate(&clk, gck_rate);
+	if (ret)
+		return ret;
+
+	max_clk = clk_get_rate(&clk);
+	if (!max_clk)
+		return -EINVAL;
+
+	ret = sdhci_setup_cfg(&plat->cfg, dev->name, host->bus_width,
+			      caps, max_clk, ATMEL_SDHC_MIN_FREQ,
+			      host->version, host->quirks, 0);
+	if (ret)
+		return ret;
+
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
+	host->mmc->priv = host;
+	upriv->mmc = host->mmc;
+
+	clk_free(&clk);
+
+	return sdhci_probe(dev);
+}
+
+static int atmel_sdhci_bind(struct udevice *dev)
+{
+	struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
+	int ret;
+
+	ret = sdhci_bind(dev, &plat->mmc, &plat->cfg);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct udevice_id atmel_sdhci_ids[] = {
+	{ .compatible = "atmel,sama5d2-sdhci" },
+	{ }
+};
+
+U_BOOT_DRIVER(atmel_sdhci_drv) = {
+	.name		= "atmel_sdhci",
+	.id		= UCLASS_MMC,
+	.of_match	= atmel_sdhci_ids,
+	.ops		= &sdhci_ops,
+	.bind		= atmel_sdhci_bind,
+	.probe		= atmel_sdhci_probe,
+	.priv_auto_alloc_size = sizeof(struct sdhci_host),
+	.platdata_auto_alloc_size = sizeof(struct atmel_sdhci_plat),
+};
+#endif
include/sdhci.h +2 -0
index c4d3b55..fdc0032 100644
@@ -166,6 +166,8 @@
 #define  SDHCI_CAN_64BIT	0x10000000
 
 #define SDHCI_CAPABILITIES_1	0x44
+#define  SDHCI_CLOCK_MUL_MASK	0x00FF0000
+#define  SDHCI_CLOCK_MUL_SHIFT	16
 
 #define SDHCI_MAX_CURRENT	0x48
 
#2 lore raw

PATCH v7 1/1 mmc: atmel_sdhci: Convert to the driver model support

Hi,

Quoted 71 lines · Jaehoon Chung

Hi Wenyuo,

Quoted 68 lines · Wenyou Yang

Convert the driver to the driver model while retaining the existing legacy code. This allows the driver to support boards that have converted to driver model as well as those that have not.

---

Changes in v7:
- Add support for using driver model for block devices and MMC operations.
- Change clk_client.h -> clk.h to adapt to clk API conversion.

Changes in v6:
- Remove unnecessary white space.
- Use sdhci_read(), instead of readl().
- Remove the local variables min_clk.

Changes in v5:
- Add Reviewed-by tag.

Changes in v4:
- Update the clk API based on [PATCH] clk: convert API to match reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
- Remove check on dev_get_parent() return.
- Fixed the return value, such as -ENODEV->-EINVAL.

Changes in v3:
- Remove the redundant log print.

Changes in v2:
- Add clock support, include enabling peripheral clock and generated clock.
- Retain the existing legacy code to support boards which have not converted to driver model.

 drivers/mmc/Kconfig       |  10 ++++
 drivers/mmc/atmel_sdhci.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
 include/sdhci.h           |   2 +
 3 files changed, 137 insertions(+)
drivers/mmc/Kconfig +10 -0
index 79cf18f..49b325e 100644
@@ -34,6 +34,16 @@ config MSM_SDHCI
           SD 3.0 specifications. Both SD and eMMC devices are supported.
        Card-detect gpios are not supported.
 
+config ATMEL_SDHCI
+     bool "Atmel SDHCI controller support"
+     depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
+     help
+       This enables support for the Atmel SDHCI controller, which supports
+       the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
+       Memory Card Specification V3.0, and the SDIO V3.0 specification.
+       It is compliant with the SD Host Controller Standard V3.0
+       specification.
+
 config ROCKCHIP_DWMMC
      bool "Rockchip SD/MMC controller support"
      depends on DM_MMC && OF_CONTROL
drivers/mmc/atmel_sdhci.c +0 -0
index 24b68b6..977f6ef 100644
@@ -6,12 +6,15 @@
  */
 
Quoted 5 lines

drivers/mmc/atmel_sdhci.c line 10

Quoted 3 lines
drivers/mmc/atmel_sdhci.c +2 -0
 #include <common.h>
+#include <clk.h>
+#include <dm.h>

If it doesn't use DM_MMC, then doesn't it need to include dm.h?

Please don't put #idefs around header files, particularly this one. It just clutters the code. We can put logic in header files if needed. Eventually most files will include dm.h.

Quoted 6 lines

drivers/mmc/atmel_sdhci.c line 15

Quoted 6 lines
drivers/mmc/atmel_sdhci.c +0 -0
 #include <malloc.h>
 #include <sdhci.h>
 #include <asm/arch/clk.h>
 
 #define ATMEL_SDHC_MIN_FREQ  400000

Regards,
Simon

#3 lore raw

PATCH v7 1/1 mmc: atmel_sdhci: Convert to the driver model support

Quoted 80 lines · Simon Glass

Hi,

Quoted 71 lines · Jaehoon Chung

Hi Wenyuo,

Quoted 68 lines · Wenyou Yang

Convert the driver to the driver model while retaining the existing legacy code. This allows the driver to support boards that have converted to driver model as well as those that have not.

---

Changes in v7:
- Add support for using driver model for block devices and MMC operations.
- Change clk_client.h -> clk.h to adapt to clk API conversion.

Changes in v6:
- Remove unnecessary white space.
- Use sdhci_read(), instead of readl().
- Remove the local variables min_clk.

Changes in v5:
- Add Reviewed-by tag.

Changes in v4:
- Update the clk API based on [PATCH] clk: convert API to match reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
- Remove check on dev_get_parent() return.
- Fixed the return value, such as -ENODEV->-EINVAL.

Changes in v3:
- Remove the redundant log print.

Changes in v2:
- Add clock support, include enabling peripheral clock and generated clock.
- Retain the existing legacy code to support boards which have not converted to driver model.

 drivers/mmc/Kconfig       |  10 ++++
 drivers/mmc/atmel_sdhci.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++
 include/sdhci.h           |   2 +
 3 files changed, 137 insertions(+)
drivers/mmc/Kconfig +10 -0
index 79cf18f..49b325e 100644
@@ -34,6 +34,16 @@ config MSM_SDHCI
           SD 3.0 specifications. Both SD and eMMC devices are supported.
        Card-detect gpios are not supported.
 
+config ATMEL_SDHCI
+     bool "Atmel SDHCI controller support"
+     depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
+     help
+       This enables support for the Atmel SDHCI controller, which supports
+       the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
+       Memory Card Specification V3.0, and the SDIO V3.0 specification.
+       It is compliant with the SD Host Controller Standard V3.0
+       specification.
+
 config ROCKCHIP_DWMMC
      bool "Rockchip SD/MMC controller support"
      depends on DM_MMC && OF_CONTROL
drivers/mmc/atmel_sdhci.c +0 -0
index 24b68b6..977f6ef 100644
@@ -6,12 +6,15 @@
  */
 
Quoted 5 lines

drivers/mmc/atmel_sdhci.c line 10

Quoted 3 lines
drivers/mmc/atmel_sdhci.c +2 -0
 #include <common.h>
+#include <clk.h>
+#include <dm.h>

If it doesn't use DM_MMC, then doesn't it need to include dm.h?

Quoted 3 lines

Please don't put #idefs around header files, particularly this one. It just clutters the code. We can put logic in header files if needed. Eventually most files will include dm.h.

Agreed! Thanks for explanation.

Best Regards,
Jaehoon Chung

Quoted 13 lines
Quoted 7 lines

drivers/mmc/atmel_sdhci.c line 15

Quoted 7 lines
drivers/mmc/atmel_sdhci.c +0 -0
 #include <malloc.h>
 #include <sdhci.h>
 #include <asm/arch/clk.h>
 
 #define ATMEL_SDHC_MIN_FREQ  400000

Regards,
Simon

#4 lore raw

PATCH v7 1/1 mmc: atmel_sdhci: Convert to the driver model support

In reply to Jaehoon Chung

Hi Jaehoon,

Quoted 124 lines

-----Original Message-----
From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
Sent: 2016?7?21? 12:17
To: Wenyou Yang <[email protected]>; U-Boot Mailing List <u- boot at lists.denx.de>
Cc: Pantelis Antoniou <[email protected]>; Andreas Bie?mann <[email protected]>; Simon Glass <[email protected]>
Subject: Re: [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support

Hi Wenyuo,

Quoted 29 lines · Wenyou Yang

Convert the driver to the driver model while retaining the existing legacy code. This allows the driver to support boards that have converted to driver model as well as those that have not.

---

Changes in v7:
- Add support for using driver model for block devices and MMC operations.
- Change clk_client.h -> clk.h to adapt to clk API conversion.

Changes in v6:
- Remove unnecessary white space.
- Use sdhci_read(), instead of readl().
- Remove the local variables min_clk.

Changes in v5:
- Add Reviewed-by tag.

Changes in v4:
- Update the clk API based on [PATCH] clk: convert API to match reset/mailbox fstyle (http://patchwork.ozlabs.org/patch/625342/).
- Remove check on dev_get_parent() return.
- Fixed the return value, such as -ENODEV->-EINVAL.

Changes in v3:
- Remove the redundant log print.

Quoted 8 lines

Changes in v2:
- Add clock support, include enabling peripheral clock and generated clock.
- Retain the existing legacy code to support boards which have not converted to driver model.

 drivers/mmc/Kconfig       |  10 ++++
 drivers/mmc/atmel_sdhci.c | 125

++++++++++++++++++++++++++++++++++++++++++++++

Quoted 29 lines
 include/sdhci.h           |   2 +
 3 files changed, 137 insertions(+)
drivers/mmc/Kconfig index renamed from drivers/mmc/Kconfig +0 -0

79cf18f..49b325e 100644

drivers/mmc/Kconfig +10 -0
@@ -34,6 +34,16 @@ config MSM_SDHCI
           SD 3.0 specifications. Both SD and eMMC devices are supported.
 	  Card-detect gpios are not supported.
 
+config ATMEL_SDHCI
+	bool "Atmel SDHCI controller support"
+	depends on DM_MMC && BLK && DM_MMC_OPS && ARCH_AT91
+	help
+	  This enables support for the Atmel SDHCI controller, which supports
+	  the embedded MultiMedia Card (e.MMC) Specification V4.51, the SD
+	  Memory Card Specification V3.0, and the SDIO V3.0 specification.
+	  It is compliant with the SD Host Controller Standard V3.0
+	  specification.
+
 config ROCKCHIP_DWMMC
 	bool "Rockchip SD/MMC controller support"
 	depends on DM_MMC && OF_CONTROL
drivers/mmc/atmel_sdhci.c +0 -0
index 24b68b6..977f6ef 100644

drivers/mmc/atmel_sdhci.c line 10

Quoted 5 lines
drivers/mmc/atmel_sdhci.c +2 -0
@@ -6,12 +6,15 @@
  */
 
 #include <common.h>
+#include <clk.h>
+#include <dm.h>

If it doesn't use DM_MMC, then doesn't it need to include dm.h?

Quoted 37 lines
drivers/mmc/atmel_sdhci.c +25 -0
 #include <malloc.h>
 #include <sdhci.h>
 #include <asm/arch/clk.h>
 
 #define ATMEL_SDHC_MIN_FREQ	400000
 
+#ifndef CONFIG_DM_MMC
 int atmel_sdhci_init(void *regbase, u32 id)  {
 	struct sdhci_host *host;
@@ -38,3 +41,125 @@ int atmel_sdhci_init(void *regbase, u32 id)
 
 	return 0;
 }
+
+#else
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct atmel_sdhci_plat {
+	struct mmc_config cfg;
+	struct mmc mmc;
+};
+
+static int atmel_sdhci_probe(struct udevice *dev) {
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
+	struct sdhci_host *host = dev_get_priv(dev);
+	u32 max_clk;
+	u32 caps, caps_1;
+	u32 clk_base, clk_mul;
+	u32 gck_rate;
+	struct udevice *dev_clk;
+	struct clk clk;
+	int periph, ret;
+
+	ret = clk_get_by_index(dev, 0, &clk);
+	if (ret)
Quoted 8 lines

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id) · line 73

Quoted 5 lines
drivers/mmc/atmel_sdhci.c +5 -0
+		return ret;
+
+	periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
+	if (periph < 0)
+		return -EINVAL;

periph is just used to pass to clk.id. it looks meaningless. As i already mentioned, you can make more cleaner than now.

There is a litter difference from usual clock structure for Atmel clock driver.

The periph value is got from a child node, then is assigned to its parent clock node.

Quoted 15 lines

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id) · line 85

Quoted 13 lines
drivers/mmc/atmel_sdhci.c +12 -0
+
+	dev_clk = dev_get_parent(clk.dev);
+	ret = clk_request(dev_clk, &clk);
+	if (ret)
+		return ret;
+
+	clk.id = periph;
+	ret = clk_enable(&clk);
+	if (ret)
+		return ret;
+
+	host->name = (char *)dev->name;

Needs the type casting?

Accepted.

Quoted 12 lines

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id)

Quoted 11 lines
drivers/mmc/atmel_sdhci.c +10 -0
+	host->ioaddr = (void *)dev_get_addr(dev);
+
+	host->quirks = 0;
+	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
+
+	host->bus_width	= fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+					 "bus-width", 4);
+
+	caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+	clk_base = (caps & SDHCI_CLOCK_V3_BASE_MASK) >>

SDHCI_CLOCK_BASE_SHIFT;

Quoted 6 lines

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id) · line 96

Quoted 2 lines
drivers/mmc/atmel_sdhci.c +2 -0
+	caps_1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+	clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >>

SDHCI_CLOCK_MUL_SHIFT;

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id) · line 98

Quoted 1 line
drivers/mmc/atmel_sdhci.c +1 -0
+	gck_rate = clk_base * 1000000 * (clk_mul + 1);

gck_rate can be over u32 boundary, doesn't?

Accepted.

Quoted 18 lines

drivers/mmc/atmel_sdhci.c int atmel_sdhci_init(void *regbase, u32 id) · line 102

Quoted 5 lines
drivers/mmc/atmel_sdhci.c +4 -0
+
+	ret = clk_get_by_index(dev, 1, &clk);
+	if (ret)
+		return ret;

It can reuse the above clock get sequence.

If reuse this sequence, lines can be reduced.

My suggestion is

1. Make atmel_sdhci_get_clk(..., int index), it should be returned "struct clk" type.

2. Then the below sequence should be included in function,

atmel_sdhci_get_clk(..., int index) {

Quoted 7 lines
	clk_get_by_index(dev, index, &clk);
	...

	return clk;
}

How about?

Good ideas, thanks a lot.

Quoted 72 lines

Best Regards,
Jaehoon Chung

Quoted 68 lines
drivers/mmc/atmel_sdhci.c +61 -0
+
+	periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
+	if (periph < 0)
+		return -EINVAL;
+
+	dev_clk = dev_get_parent(clk.dev);
+	ret = clk_request(dev_clk, &clk);
+	if (ret)
+		return ret;
+
+	clk.id = periph;
+	ret = clk_set_rate(&clk, gck_rate);
+	if (ret)
+		return ret;
+
+	max_clk = clk_get_rate(&clk);
+	if (!max_clk)
+		return -EINVAL;
+
+	ret = sdhci_setup_cfg(&plat->cfg, dev->name, host->bus_width,
+			      caps, max_clk, ATMEL_SDHC_MIN_FREQ,
+			      host->version, host->quirks, 0);
+	if (ret)
+		return ret;
+
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
+	host->mmc->priv = host;
+	upriv->mmc = host->mmc;
+
+	clk_free(&clk);
+
+	return sdhci_probe(dev);
+}
+
+static int atmel_sdhci_bind(struct udevice *dev) {
+	struct atmel_sdhci_plat *plat = dev_get_platdata(dev);
+	int ret;
+
+	ret = sdhci_bind(dev, &plat->mmc, &plat->cfg);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct udevice_id atmel_sdhci_ids[] = {
+	{ .compatible = "atmel,sama5d2-sdhci" },
+	{ }
+};
+
+U_BOOT_DRIVER(atmel_sdhci_drv) = {
+	.name		= "atmel_sdhci",
+	.id		= UCLASS_MMC,
+	.of_match	= atmel_sdhci_ids,
+	.ops		= &sdhci_ops,
+	.bind		= atmel_sdhci_bind,
+	.probe		= atmel_sdhci_probe,
+	.priv_auto_alloc_size = sizeof(struct sdhci_host),
+	.platdata_auto_alloc_size = sizeof(struct atmel_sdhci_plat), };
+#endif
include/sdhci.h index c4d3b55..fdc0032 renamed from include/sdhci.h +0 -0

100644

include/sdhci.h +0 -0
@@ -166,6 +166,8 @@
 #define  SDHCI_CAN_64BIT	0x10000000
 
Quoted 7 lines

include/sdhci.h line 173

Quoted 7 lines
include/sdhci.h +2 -0
 #define SDHCI_CAPABILITIES_1	0x44
+#define  SDHCI_CLOCK_MUL_MASK	0x00FF0000
+#define  SDHCI_CLOCK_MUL_SHIFT	16
 
 #define SDHCI_MAX_CURRENT	0x48
 

Best Regards,
Wenyou Yang