[U-Boot] [PATCH v7] mmc: atmel_sdhci: Convert to the driver model support
Jaehoon Chung
jh80.chung at samsung.com
Thu Jul 21 06:16:54 CEST 2016
Hi Wenyuo,
On 07/20/2016 07:04 PM, Wenyou Yang wrote:
> 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.
>
> Signed-off-by: Wenyou Yang <wenyou.yang at atmel.com>
> Reviewed-by: Simon Glass <sjg at chromium.org>
> ---
>
> 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(+)
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 79cf18f..49b325e 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -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
> diff --git a/drivers/mmc/atmel_sdhci.c b/drivers/mmc/atmel_sdhci.c
> index 24b68b6..977f6ef 100644
> --- a/drivers/mmc/atmel_sdhci.c
> +++ b/drivers/mmc/atmel_sdhci.c
> @@ -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?
> #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)
> + 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.
> +
> + 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?
> + 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?
> +
> + 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
> +
> + 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
> diff --git a/include/sdhci.h b/include/sdhci.h
> index c4d3b55..fdc0032 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -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
>
>
More information about the U-Boot
mailing list