On 04/29/2016 11:42 AM, Simon Glass wrote:
Hi Stephen,

On 29 April 2016 at 10:53, Simon Glass <s...@chromium.org> wrote:
Hi Stephen,

On 29 April 2016 at 10:27, Stephen Warren <swar...@wwwdotorg.org> wrote:
On 04/29/2016 08:02 AM, Simon Glass wrote:

Hi Stephen,

On 27 April 2016 at 10:13, Stephen Warren <swar...@wwwdotorg.org> wrote:

On 04/27/2016 08:50 AM, Simon Glass wrote:


Hi Stephen,

On 25 April 2016 at 13:25, Stephen Warren <swar...@wwwdotorg.org> wrote:


On 04/23/2016 11:14 AM, Simon Glass wrote:



Hi Stephen,

On 19 April 2016 at 14:59, Stephen Warren <swar...@wwwdotorg.org>
wrote:



From: Stephen Warren <swar...@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 <swar...@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.

So, just to be clear: TomW can apply this (once I post V2) even if you're not giving an ack?

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.

That's certainly not what it means right now, and my intention in this series was not to address any short-comings in the current meanings of Kconfig.

No matter what, if we do drive all this core SoC code from DT eventually, I don't imagine tegra_get_chip() will be the way code gets dispatched between multiple chips in the future, so removing it now shouldn't hurt any later conversion to runtime multi-chip support.

In practice in a DT-driven world, I'd expect:

- The code in enable_scu() to be part of some Tegra20-specific "SoC driver" or similar, and hence not need any runtime support restored.

- The code in cache.c to be part of some specific cache drivers, which would only be instantiated on Tegra20, and hence not need any runtime support restored.

- The code in cpu.c to be moved into a clock driver, and base on some per-SoC table in the clock driver, and hence not need any runtime special-casing.

As such, I don't see reverting any of this patch being reverted later, and I don't believe leaving in these runtime calls would make it any easier to refactor the code into drivers later.

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.

I'm just being practical. Please note in particular the comment about basing such decisions on DT; that and refactoring the code into DT-instantiated drivers would leave it so different that the tiny number of current runtime switches doesn't help anything.

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.

I don't have a plan for anything besides the clock and reset drivers at the moment. I /think/ that most of the rest of the code in arch/arm/mach-tegra simply won't be needed on Tegra186. We'll see; I only have UART and GPIO working right now, well and a very hacked-up MMC to avoid clock/reset access. There is a large amount of work left that I haven't started looking at yet.

Also, the model that seems to be emerging is that SPL detects the
platform and passes the correct DT to U-Boot proper.

There is no SPL on Tegra210 or later. It would be a pity to have to introduce one just to do runtime DT selection when there's no need to have runtime DT selection.

Specifically I think the end state should be:
- Most SoC code is in drivers using driver model

That is fine in many cases. There's plenty of low-level bootstrap code where I'm not sure that it's worth it. Equally, given that you have stated DM==DT, I worry about the implications of that statement if universally applied, rather than selectively/pragmatically applied.

- It is possible to build most of the tegra code without building each
SoC (just by having a 'tegra' board which enables all drivers)

I'm not convinced this is worth it universally.

For the purposes of actually running a binary on a device, I believe there is zero need for this.

For compile coverage it might be nice, especially for driver code, although running buildman on one Tegra board for each SoC generation doesn't take much time.

Even for drivers that solely use generic (non-SoC-version-specific) APIs, this would be difficult in some cases. This plays back into my earlier comments elsewhere about structs-vs-defines for register address calculations. Register layouts of some devices vary incompatibly between Tegra SoC generations. With a define-based driver, this can all be handled at run-time. With a struct-based driver, one must compile the code once per register layout. Rather than invest in making a "compile test" build that does this, which would require various build system work, it's much simpler to just compile the driver multiple times as part of a set of board builds, which already works today. The result is still compiling the code n times, so there wouldn't be much time saving wrapping everything into one build.

- If someone wanted to support multiple SoCs it would not be
impossible to do so (even if mainline doesn't make it)

I don't think it's a good idea to support features that mainline doesn't use. They'll bit-rot and not work anyway. I also can't see any use-case for such a build.

- New SoCs are supported mostly by adding new drivers (although there
is always of course some core/start-up code)

Yes, it certainly makes sense to use drivers for functionality which other pieces of the code must interact with in a generic way. For example, calls from I2C/MMC drivers to clock/reset APIs.

I expect some debate over what code is core/start-up code though. For example, static pinmux setup fits that category for me, since there's no generic code that needs to interface with it.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to