On 13/08/2019 21:31, Eddie James wrote: > Add support for the Aspeed SD host controller engine.
It looks correct and simple enough. Some comments below. > > Signed-off-by: Eddie James <[email protected]> > --- > drivers/mmc/Kconfig | 11 +++++++ > drivers/mmc/Makefile | 1 + > drivers/mmc/aspeed_sdhci.c | 78 > ++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 90 insertions(+) > create mode 100644 drivers/mmc/aspeed_sdhci.c > > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig > index c6812f6..536f66a 100644 > --- a/drivers/mmc/Kconfig > +++ b/drivers/mmc/Kconfig > @@ -421,6 +421,17 @@ config SPL_MMC_SDHCI_ADMA > This enables support for the ADMA (Advanced DMA) defined > in the SD Host Controller Standard Specification Version 3.00 in SPL. > > +config MMC_SDHCI_ASPEED > + bool "Aspeed SDHCI controller" > + depends on ARCH_ASPEED > + depends on DM_MMC > + depends on MMC_SDHCI > + help > + Enables support for the Aspeed SDHCI 2.0 controller present on Aspeed > + SoCs. This device is compatible with SD 3.0 and/or MMC 4.3 > + specifications. On the AST2600, the device is also compatible with > + MMC 5.1 and eMMC 3.0. > + > config MMC_SDHCI_ATMEL > bool "Atmel SDHCI controller support" > depends on ARCH_AT91 > diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile > index 6cc018b..5594195 100644 > --- a/drivers/mmc/Makefile > +++ b/drivers/mmc/Makefile > @@ -46,6 +46,7 @@ obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o > > # SDHCI > obj-$(CONFIG_MMC_SDHCI) += sdhci.o > +obj-$(CONFIG_MMC_SDHCI_ASPEED) += aspeed_sdhci.o > obj-$(CONFIG_MMC_SDHCI_ATMEL) += atmel_sdhci.o > obj-$(CONFIG_MMC_SDHCI_BCM2835) += bcm2835_sdhci.o > obj-$(CONFIG_MMC_SDHCI_BCMSTB) += bcmstb_sdhci.o > diff --git a/drivers/mmc/aspeed_sdhci.c b/drivers/mmc/aspeed_sdhci.c > new file mode 100644 > index 0000000..c292c42 > --- /dev/null > +++ b/drivers/mmc/aspeed_sdhci.c > @@ -0,0 +1,78 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2019 IBM Corp. > + * Eddie James <[email protected]> > + */ > + > +#include <common.h> > +#include <clk.h> > +#include <dm.h> > +#include <malloc.h> > +#include <sdhci.h> > + > +struct aspeed_sdhci_plat { > + struct mmc_config cfg; > + struct mmc mmc; > +}; > + > +static int aspeed_sdhci_probe(struct udevice *dev) > +{ > + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); > + struct aspeed_sdhci_plat *plat = dev_get_platdata(dev); > + struct sdhci_host *host = dev_get_priv(dev); > + u32 max_clk; > + struct clk clk; > + int ret; > + > + ret = clk_get_by_index(dev, 0, &clk); > + if (ret) > + return ret; > + > + ret = clk_enable(&clk); > + if (ret) > + return ret; Don't we need to release the clock below in case of error ? > + host->name = dev->name; > + host->ioaddr = (void *)devfdt_get_addr(dev); > + > + max_clk = clk_get_rate(&clk); > + if (!max_clk) may be use IS_ERR_VALUE(max_clk) and return max_clk > + return -EINVAL; > + > + host->max_clk = max_clk; > + host->mmc = &plat->mmc; > + host->mmc->dev = dev; > + host->mmc->priv = host; > + upriv->mmc = host->mmc; > + > + ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0); > + if (ret) > + return ret; > + > + return sdhci_probe(dev); > +} > + > +static int aspeed_sdhci_bind(struct udevice *dev) > +{ > + struct aspeed_sdhci_plat *plat = dev_get_platdata(dev); > + > + return sdhci_bind(dev, &plat->mmc, &plat->cfg); > +} > + > +static const struct udevice_id aspeed_sdhci_ids[] = { > + { .compatible = "aspeed,ast2400-sdhci" }, > + { .compatible = "aspeed,ast2500-sdhci" }, > + { .compatible = "aspeed,ast2600-sdhci" }, > + { } > +}; > + > +U_BOOT_DRIVER(aspeed_sdhci_drv) = { > + .name = "aspeed_sdhci", > + .id = UCLASS_MMC, > + .of_match = aspeed_sdhci_ids, > + .ops = &sdhci_ops, > + .bind = aspeed_sdhci_bind, > + .probe = aspeed_sdhci_probe, > + .priv_auto_alloc_size = sizeof(struct sdhci_host), > + .platdata_auto_alloc_size = sizeof(struct aspeed_sdhci_plat), > +}; > _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
