[U-Boot] [PATCH v2] socfpga/dwmmc: Adding DesignWare MMC driver support for SOCFPGA
Wolfgang Denk
wd at denx.de
Thu Dec 26 09:38:10 CET 2013
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 at 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_
More information about the U-Boot
mailing list