[U-Boot] [PATCH 55/60] i2c: tegra: move pinmux setup to board files

[Stephen Warren]

On 04/27/2016 10:58 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 27 April 2016 at 10:24, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 04/27/2016 09:12 AM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 19 April 2016 at 14:59, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>
>>>> From: Stephen Warren <swarren at nvidia.com>
>>>>
>>>> Remove funcmux calls from the Tegra I2C driver. Knowledge of pinmux
>>>> setup must come from either board files or DT; it should not be embedded
>>>> into board-agnostic driver code. The DT pinmux bindings do not allow
>>>> drivers to derive funcmux-style information, since the DT bindings are
>>>> pin-based whereas funcmux is controller-based, so there's no good way to
>>>> call the existing funcmux APIs from drivers. Converting drivers to use a
>>>> new (as yet non-existent in U-Boot) API that pulls pinmux information
>>>> from
>>>> DT isn't useful for Tegra, since Tegra's DT files don't contain any
>>>> per-device pinmux tables, so this would simply be extra code that has no
>>>> effect; doesn't actually set up the pinmux. We are left with moving the
>>>> pinmux setup functionality into board files. In theory the board files
>>>> could be converted later to use DT, but that would be a separate change.
>>>>
>>>> Signed-off-by: Stephen Warren <swarren at nvidia.com>
>>>> ---
>>>>    board/avionic-design/common/tamonten.c  |  5 +++++
>>>>    board/nvidia/seaboard/seaboard.c        |  3 +++
>>>>    board/nvidia/whistler/whistler.c        |  1 +
>>>>    board/toradex/colibri_t20/colibri_t20.c |  3 +++
>>>>    drivers/i2c/tegra_i2c.c                 | 19 -------------------
>>>>    5 files changed, 12 insertions(+), 19 deletions(-)
>>>
>>>
>>> This should use driver model, which handles pinmux automatically if
>>> you have a pinctrl driver.
>>
>>
>> Can you define "automatic"? I don't understand exactly which benefit you're
>> describing there.
>
> When you probe a device, its pinmux is set up automatically, so the
> explicit funcmux calls can go away.

So the device here would be the pin controller device itself, not 
individual I2C/SPI/SD/... devices.

That's because the Tegra HW does not support[1] dynamic or partial 
pinmux configuration. As such, we need to program the entire pinmux at 
once early during boot, not piece-by-piece as the individual U-Boot 
devices that use individual pins are probed. This is why for example the 
kernel DT contains a single pinmux table that the pin controller driver 
sets up at boot (with identical configuration to what U-Boot sets up, so 
it's a no-op), rather than splitting it into per-device chunks.

As such, there isn't any need for, say, an I2C device probe to call into 
pinmux at all; the pinmux would already have been set up entirely during 
early boot, and hence no I2C-specific actions would be needed later.

So I'm not sure what benefit conversion to DM pinctrl has here. Sure it 
means things get set up the same way as with any other pinctrl device or 
SoC, but this is early SoC-specific configuration, without any 
interaction with common or driver code besides being implemented via 
some standard core->board callbacks/hooks. It seems reasonable to just 
program the pinmux directly using SoC-specific APIs rather than having 
to add a layer of abstraction on top of it just so we can route it 
through DM. In other words, what's already done by this patch series.

Besides, I believe the programming happens before a DM pinctrl device 
would be ready, doesn't it, given it happens from 
tegra_board_early_init_f()? Or, would we be able to fully probe a DM 
device at that point? The UART console setup is even earlier, in SPL 
pre-T210, where I don't think we even have DM enabled.

[1] Yes, the HW registers can in practice be programmed bit-by-bit, 
simply because there are a number of registers and the SoC doesn't have 
a way to physically force SW to write to each of those registers. 
"Support" here refers to what the ASIC team will guarantee will work 
correctly without causing glitches or similar issues. There are a few 
limited exceptions, e.g. console UART muxing on its own has been at 
least partially thought out (although there are still conflicts in some 
cases on older chips), and IO controllers that may contain boot media 
are generally OK to mux on their own. However, for anything else, i.e. 
the majority of cases, the supported model is to program everything 
up-front in one go, and not change it later. Sticking to that general 
model in absolutely all cases removes special cases and simplifies the code.