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

Stephen Warren
2016-04-27T17:16:02Z

On 04/27/2016 10:58 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 27 April 2016 at 10:24, Stephen Warren <swarren@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@wwwdotorg.org> wrote:
>>>>
>>>> From: Stephen Warren <swarren@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@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.