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

Stefan Roese
2013-04-11T08:00:27Z

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