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

[Stephen Warren]

On 01/09/2012 04:36 PM, Simon Glass wrote:
> 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.

On Tegra20, the pin mux is controlled in groups, so you're mostly
picking which groups to use for the function, which then determines the
bus width. In other words, its often unlikely that you can pick bus
width as an independent option from the set of pins/groups used.

On Tegra30, the situation is about the same except that the mux function
is picked on a per-pin basis instead of in groups of pins, which takes
the same argument even further; for a 4-bit bus you'd simply remove 4
pins from the list of pins being used, so it doesn't make sense to refer
to 4-bit as an option of an 8-bit setup with some pins unused, because
the unused pins are set to some other mux function.

> 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).

There are far far too many options for that to be represented by a
single U32, or even a small number of them. When boards start needing to
set up drive strengths etc., we'll probably need individual API calls
for each config option, since each board's characterization will trigger
a combination of options that's extremely likely to be unique.

>> 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?

I don't agree. The options are just completely arbitrary IDs for a
particular set of pins/groups that are being used. Well, arbitrary
within the set of possibilities given the wiring of Tegra's pinmux HW
anyway. Hence, giving those IDs names based on which pins/groups are
being used makes sense to me.

-- 
nvpublic