[U-Boot] socfpga: initialize MMC

Pavel Machek pavel at denx.de
Mon Jul 21 12:20:04 CEST 2014


Hi!

> > +++ b/arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h
> > @@ -14,5 +14,8 @@
> >  #define SOCFPGA_CLKMGR_ADDRESS 0xffd04000
> >  #define SOCFPGA_RSTMGR_ADDRESS 0xffd05000
> >  #define SOCFPGA_SYSMGR_ADDRESS 0xffd08000
> > +#define SOCFPGA_SDMMC_ADDRESS 0xff704000
> 
> Please keep list sorted.

It was sorted by address before, so I'll keep it like that.

> > --- a/drivers/mmc/Makefile
> > +++ b/drivers/mmc/Makefile
> > @@ -28,6 +28,7 @@ obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o
> >  obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
> >  obj-$(CONFIG_DWMMC) += dw_mmc.o
> >  obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
> > +obj-$(CONFIG_ALTERA_DWMMC) += altera_dw_mmc.o
> >  obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
> >  obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
> >  obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o
> 
> Ditto.  This should be a sorted list.

This was not sorted and I will not need to modify it in next patch
version, so I'll leave this unfixed.

> > +static char *ALTERA_NAME = "ALTERA DWMMC";
> 
> Please use lower case variable names only.

Actually, it turns out we don't need the variable :-).

> > +int altera_dwmmc_init(u32 regbase, int bus_width, int index)
> > +{
> > +	struct dwmci_host *host = NULL;
> > +	host = malloc(sizeof(struct dwmci_host));
> 
> Please separate declartions and code by a blank line.

Ok.

> Hm, which sense does it make to initialize host as NULL, just to assign
> a different value in the next step?
> 
> Please fix.

Fixed.

> > +	host->name = ALTERA_NAME;
> > +	host->ioaddr = (void *)regbase;
> > +	host->buswidth = bus_width;
> > +	host->clksel = altera_dwmci_clksel;
> > +	host->dev_index = index;
> > +	host->bus_hz = 400000;
> > +	host->fifoth_val = MSIZE(0x2) |
> > +		RX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2 - 1) |
> > +		TX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2);
> > +
> > +	add_dwmci(host, host->bus_hz, host->bus_hz);
> 
> Is there a free(host) anywhere?

Dynamic allocation does not make much sense here, agreed. But as it is
existing code, and there are bigger issues around, I'd prefer to leave
it to someone else to clean it up...

Thanks,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


More information about the U-Boot mailing list