Hi Wolfgang,
On 10.04.2013 14:13, Wolfgang Denk wrote:
+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.
Okay.
You return "status" but where do you actually set it? And where do you test for errors of imx_iomux_v3_setup_multiple_pads() ?
With only one pad-IF to configure I can use imx_iomux_v3_setup_pad() which does not return an error:
int imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad) { u32 mux_ctrl_ofs = (pad & MUX_CTRL_OFS_MASK) >> MUX_CTRL_OFS_SHIFT; u32 mux_mode = (pad & MUX_MODE_MASK) >> MUX_MODE_SHIFT; u32 sel_input_ofs = (pad & MUX_SEL_INPUT_OFS_MASK) >> MUX_SEL_INPUT_OFS_SHIFT; u32 sel_input = (pad & MUX_SEL_INPUT_MASK) >> MUX_SEL_INPUT_SHIFT; u32 pad_ctrl_ofs = (pad & MUX_PAD_CTRL_OFS_MASK) >> MUX_PAD_CTRL_OFS_SHIFT; u32 pad_ctrl = (pad & MUX_PAD_CTRL_MASK) >> MUX_PAD_CTRL_SHIFT;
if (mux_ctrl_ofs) __raw_writel(mux_mode, base + mux_ctrl_ofs);
if (sel_input_ofs) __raw_writel(sel_input, base + sel_input_ofs);
if (!(pad_ctrl & NO_PAD_CTRL) && pad_ctrl_ofs) __raw_writel(pad_ctrl, base + pad_ctrl_ofs);
return 0; }
So we can change this function to void:
void imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad)
And this makes returning an error in the multiple version also useless. I can change it to void as well.
Guess that should be fixed globally (and for some other boards as well).
Yes, I'll look into this. Thanks for the review.
Best regards, Stefan