[U-Boot] [PATCH 38/60] ARM: tegra: remove tegra_get_chip()

[Simon Glass]

Hi Stephen,

On 29 April 2016 at 10:53, Simon Glass <sjg at chromium.org> wrote:
> Hi Stephen,
>
> On 29 April 2016 at 10:27, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 04/29/2016 08:02 AM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 27 April 2016 at 10:13, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>
>>>> On 04/27/2016 08:50 AM, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Hi Stephen,
>>>>>
>>>>> On 25 April 2016 at 13:25, Stephen Warren <swarren at wwwdotorg.org> wrote:
>>>>>>
>>>>>>
>>>>>> On 04/23/2016 11:14 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>
>>>>>>>>
>>>>>>>> U-Boot is compiled for a single board, which in turn uses a specific
>>>>>>>> SoC.
>>>>>>>> There's no need to make runtime decisions based on SoC ID. While
>>>>>>>> there's
>>>>>>>> certainly an argument for making the code support different SoCs at
>>>>>>>> run-time, the Tegra code is so far from that possible ideal that the
>>>>>>>> existing runtime code is an anomaly. If this changes in the future,
>>>>>>>> all
>>>>>>>> runtime decisions should likely be based on DT anyway.
>>>>>>>>
>>>>>>>> Signed-off-by: Stephen Warren <swarren at nvidia.com>
>>>>>>>> ---
>>>>>>>>     arch/arm/mach-tegra/ap.c               | 106
>>>>>>>> ++++++++++-----------------------
>>>>>>>>     arch/arm/mach-tegra/cache.c            |  20 +++----
>>>>>>>>     arch/arm/mach-tegra/cpu.c              |  16 ++---
>>>>>>>>     arch/arm/mach-tegra/cpu.h              |   6 --
>>>>>>>>     arch/arm/mach-tegra/tegra20/warmboot.c |  20 ++-----
>>>>>>>>     5 files changed, 51 insertions(+), 117 deletions(-)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> What exactly is missing to prevent multi-arch support?
>>>>>>
>>>>>>
>>>>>> In a word: everything:-)
>>>>>>
>>>>>> Pretty much all decisions in core architecture code, core Tegra code,
>>>>>> drivers, and even board files are currently made at compile time. For
>>>>>> example, consider drivers where the register layouts are different
>>>>>> between
>>>>>> different SoCs; not just new fields added, but existing fields moved to
>>>>>> different offsets. Right now, we handle this by changing the register
>>>>>> struct
>>>>>> definition at compile time. To support multiple chips, we'd have to
>>>>>> either
>>>>>> (a) link in n copies of the driver, one per register layout, or (b)
>>>>>> rework
>>>>>> the driver to use #defines and runtime calculations for register
>>>>>> offsets,
>>>>>> like the Linux kernel drivers do. Tegra USB is one example. The pinmux
>>>>>> and
>>>>>> clock drivers have a significantly different sets of
>>>>>> pins/clocks/resets/...
>>>>>> per SoC, and enums/tables describing those sets are currently
>>>>>> configured at
>>>>>> compile time. Some PMIC constants (e.g. vdd_cpu voltage) are configured
>>>>>> at
>>>>>> compile-time, and even differ per board.
>>>>>
>>>>>
>>>>> I wonder how far we would get by converting clock, pinctrl, reset to
>>>>> driver model drivers?
>>>>
>>>>
>>>> Well, I expect we'll find out soon. The next SoC has radically different
>>>> clock/reset mechanisms, so we'll need to switch to standardized APIs for
>>>> clock/reset on Tegra to isolate drivers from those differences, and I
>>>> imagine that conversion would also involve conversion to DM since any
>>>> standard APIs probably assume use of DM. I haven't investigated this in
>>>> detail yet though.
>>>
>>>
>>> That sounds like a good move.
>>
>>
>> I'm not sure if you still object to this patch for now, or would be happy
>> giving it an ack?

And just to be clear, I'm always keen to move things forward and I can
see you have put a lot of work into this series. It would be easier to
just apply it as is. What I am looking for is how we might get closer
to the goal, perhaps after this series. This series has exposed this
which is why I have brought it up.

>
> Sorry, I still feel this is going in the wrong
> direction...CONFIG_TEGRA124 should mean that the code supports it,
> rather than the code exclusively supports it.
>
> In your commit message you say:
>
> "While there's
> certainly an argument for making the code support different SoCs at
> run-time, the Tegra code is so far from that possible ideal that the
> existing runtime code is an anomaly. If this changes in the future, all
> runtime decisions should likely be based on DT anyway."
>
> Or even build time.... That statement  seems like saying that
> everything is so terrible that we might as well give up.
>
> What's your plan to move code into drivers? I am happy to help move
> things over and get towards the goal rather than further away. But
> really that should happen first.
>
> Also, the model that seems to be emerging is that SPL detects the
> platform and passes the correct DT to U-Boot proper.
>
> Specifically I think the end state should be:
> - Most SoC code is in drivers using driver model
> - It is possible to build most of the tegra code without building each
> SoC (just by having a 'tegra' board which enables all drivers)
> - If someone wanted to support multiple SoCs it would not be
> impossible to do so (even if mainline doesn't make it)
> - New SoCs are supported mostly by adding new drivers (although there
> is always of course some core/start-up code)
>
> Do you agree with that?
>
> Regards,
> Simon

Regards,
Simon