[U-Boot] [PATCH 0/7] tegra: Enhance funcmux to support I2C and MMC

[Simon Glass]

Hi Stephen,

On Mon, Jan 9, 2012 at 3:11 PM, Stephen Warren <swarren at nvidia.com> wrote:
> On 01/09/2012 03:53 PM, Simon Glass wrote:
>> This series expands funcmux_select() to support configs other than 0, and
>> to support options associated with a config.
>>
>> This permits introduction of I2C support using multiple config options.
>>
>> The options parameter is used by MMC to select standard (4-bit) or 8-bit
>> operation.
>
> The unification in this series basically seems fine.
>
> Why not consider bus width part of the "config" though, rather the
> complicating things with an extra parameter? As an example, for SDMMC4,
> you'd have say:
>
> 0: ATC + ATD 8 bit
> 1: ATB + GMA 4 bit
> 2: ATB + GMA + GME 8 bit
>
> ... and no option values.

I am thinking ahead a little to where we have more peripherals with
several options. If we imagine a situation where the SOC has 3
different pin configs each of which can be 1-bit, 4-bit or 8-bit, then
it is nice to have the options broken out separately.

Also, we can also use the options for something else, like Tegra 3's
drive strength and slew rate control (and perhaps other things I
understand even less).

>
> Also, we should probably define names for the config values, at least in
> the cases where 0 isn't the only option. Hard-coding 0 or 1 at the call
> sites isn't very meaningful.

I can certainly do that - it was in the back of my mind. But the only
thing I could think of was to create an enum with the pingroup
assignments, like:

enum {
    UART1_IRRX_IRTX   = 0,
    UART2_UAD             = 0,
...
};

Seems a bit ugly?

>
> Oh, and SDMMC4's option 0 isn't correct: I think pin group ATB should be
> removed, since it contains signals SDIO4_CLK and SDIO4_CMD which are
> also part of pin group ATC, when those pin groups are set to function
> SDIO4 at least. I didn't check to see if any other similar problems exist.

Yes, thanks for finding that, will fix once I hear back about the above.

Regards,
SImon

>
> --
> nvpublic