From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Thu, 11 Apr 2013 10:00:27 +0200
Subject: [U-Boot] [PATCH] imx: Add titanium board support (i.MX6 based)
In-Reply-To: <20130410121353.A843820146E@gemini.denx.de>
References: <1365578252-13264-1-git-send-email-sr@denx.de>
	<20130410121353.A843820146E@gemini.denx.de>
Message-ID: <51666D9B.2050301@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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