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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to