[U-Boot] [PATCH] imx: Add titanium board support (i.MX6 based)

Wolfgang Denk
2013-04-10T12:13:53Z

Dear Stefan Roese,

In message <1365578252-13264-1-git-send-email-sr@denx.de> you wrote:
> Titanium is a i.MX6 based board from ProjectionDesign / Barco. This
> patch adds support for this board with the newly introduced NAND
> support for i.MX6.

having a second look...

> +int board_mmc_init(bd_t *bis)
> +{
> + s32 status = 0;
> + u32 index = 0;
> +
> + usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK);
> +
> + for (index = 0; index < CONFIG_SYS_FSL_USDHC_NUM; ++index) {
> + switch (index) {
> + case 0:
> + imx_iomux_v3_setup_multiple_pads(
> + usdhc3_pads, ARRAY_SIZE(usdhc3_pads));
> + break;
> + default:
> + printf("Warning: you configured more USDHC controllers"
> + "(%d) then supported by the board (%d)\n",
> + index + 1, CONFIG_SYS_FSL_USDHC_NUM);
> + return status;
> + }
> +
> + status |= fsl_esdhc_initialize(bis, &usdhc_cfg[index]);
> + }
> +
> + return status;
> +}

CONFIG_SYS_FSL_USDHC_NUM is a #defined constant, so the test can be
done at compile time. Doing this at run time makes no sense at all.

Drop the whole loop here, it is not needed for a single interface. It
is just a waste of code that obfuscates what you are doing.

You return "status" but where do you actually set it? And where do
you test for errors of imx_iomux_v3_setup_multiple_pads() ?

Guess that should be fixed globally (and for some other boards as
well).

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
The White Rabbit put on his spectacles. "Where shall I begin, please
your Majesty ?" he asked.
"Begin at the beginning,", the King said, very gravely, "and go on
till you come to the end: then stop." -- Lewis Carroll