[U-Boot] [PATCH v2] socfpga/dwmmc: Adding DesignWare MMC driver support for SOCFPGA

[Wolfgang Denk <wd@denx.de>]

Dear Chin Liang See,

PLease fix your address list.  There is no such address as
"Andy at denx.de".

In message <1387386987-3581-1-git-send-email-clsee@altera.com> you wrote:
> To add the DesignWare MMC driver support for Altera SOCFPGA. It
> required information such as clocks and bus width from platform
> specific files (SOCFPGA handoff files)
...
> +#define CLKMGR_PERPLLGRP_EN_REG		(SOCFPGA_CLKMGR_ADDRESS + 0xA0)
...
> +#define SYSMGR_SDMMCGRP_CTRL_REG	(SOCFPGA_SYSMGR_ADDRESS + 0x108)

This looks as if you were trying to access device rtegisters through a
base address plus offset notation?

We do not allow this in U-Boot, as the compiler then has no chance to
check if you are using the correct data types, i. e. it cannot warn
you for example when you access a 32 bit register for a 16 bit data
type.

Please use C structs and proper I/O accessors instead.

> +	/* Disable SDMMC clock. */
> +	en = readl(CLKMGR_PERPLLGRP_EN_REG);
> +	en &= ~CLKMGR_SDMMC_CLK_ENABLE;
> +	writel(en, CLKMGR_PERPLLGRP_EN_REG);

Please fix such code.  Use proper I/O accessors.  This could (and
should) be written as:

	clrbits_le32(clkmgr->perpllgrp_en, CLKMGR_SDMMC_CLK_ENABLE);

[or similar struct member name].

> +	/* Enable SDMMC clock */
> +	en = readl(CLKMGR_PERPLLGRP_EN_REG);
> +	en |= CLKMGR_SDMMC_CLK_ENABLE;
> +	writel(en, CLKMGR_PERPLLGRP_EN_REG);

Ditto here, etc.

Please check all your device accesses.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
That was the thing about deserts. They had their  own  gravity.  They
sucked you into the centre.           - Terry Pratchett, _Small Gods_