[U-Boot] [PATCH v4] mmc: atmel_sdhci: Convert to the driver model support

Yang, Wenyou Wenyou.Yang at atmel.com
Mon Jun 27 04:52:38 CEST 2016


Hi Simon,

> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: 2016年6月26日 10:53
> To: Yang, Wenyou <Wenyou.Yang at atmel.com>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Andreas Bießmann
> <andreas at biessmann.org>
> Subject: Re: [PATCH v4] mmc: atmel_sdhci: Convert to the driver model support
> 
> Hi Wenyou,
> 
> On 19 June 2016 at 20:09, Wenyou Yang <wenyou.yang at atmel.com> 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>
> > ---
> >
> > 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 | 99
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 109 insertions(+)
> >
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> Please see question below.
> 
> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index
> > c80efc3..518c624 100644
> > --- a/drivers/mmc/Kconfig
> > +++ b/drivers/mmc/Kconfig
> > @@ -25,6 +25,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 && 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..a68d192 100644
> > --- a/drivers/mmc/atmel_sdhci.c
> > +++ b/drivers/mmc/atmel_sdhci.c
> > @@ -6,12 +6,16 @@
> >   */
> >
> >  #include <common.h>
> > +#include <clk_client.h>
> > +#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 +42,98 @@ int atmel_sdhci_init(void *regbase, u32 id)
> >
> >         return 0;
> >  }
> > +
> > +#else
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +static int atmel_sdhci_probe(struct udevice *dev) {
> > +       struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> > +       struct sdhci_host *host = dev_get_priv(dev);
> > +       u32 max_clk, min_clk = ATMEL_SDHC_MIN_FREQ;
> > +       u32 caps0, caps1;
> > +       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;
> > +
> > +       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;
> > +       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);
> > +
> > +       caps0 = readl(host->ioaddr + SDHCI_CAPABILITIES);
> > +       caps1 = readl(host->ioaddr + SDHCI_CAPABILITIES_1);
> > +       clk_base = (caps0 & SDHCI_CLOCK_V3_BASE_MASK) >>
> SDHCI_CLOCK_BASE_SHIFT;
> > +       clk_mul = (caps1 & SDHCI_CLOCK_MUL_MASK) >>
> SDHCI_CLOCK_MUL_SHIFT;
> > +       gck_rate = clk_base * 1000000 * (clk_mul + 1);
> > +
> > +       ret = clk_get_by_index(dev, 1, &clk);
> > +       if (ret)
> > +               return ret;
> > +
> > +       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;
> > +
> > +       add_sdhci(host, max_clk, min_clk);
> > +
> > +       upriv->mmc = host->mmc;
> > +
> > +       clk_free(&clk);
> 
> Should that be in a remove() method?

For AT91 clock, there is no special operation needed on free the clock,
and it  also doesn't provide the .free method, indeed, this statement can be removed.

I don't think it is necessary to move this line in a remove() method. 

> 
> > +
> > +       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,
> > +       .probe          = atmel_sdhci_probe,
> > +       .priv_auto_alloc_size = sizeof(struct sdhci_host), };
> > +
> > +#endif
> > --
> > 2.7.4
> >
> 
> Regards,
> Simon


Best Regards,
Wenyou Yang


More information about the U-Boot mailing list