diff mbox

[U-Boot] ns16550: allow UART address to be set dynamically

Message ID 1355354590-10023-1-git-send-email-swarren@wwwdotorg.org
State Rejected
Delegated to: Wolfgang Denk
Headers show

Commit Message

Stephen Warren Dec. 12, 2012, 11:23 p.m. UTC
From: Stephen Warren <swarren@nvidia.com>

A single U-Boot binary may support multiple very similar boards. These
boards may use different UARTs for the main debug console. Hence, it is
impossible to #define CONFIG_SYS_NS16550_COM1 to some static UART
address, since the true value may only be determined at run-time, after
identifying the actual hardware. Provide an API for boards to call to
set the actual address of the UART, e.g. from spl_board_init() or
board_early_init_f().

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
Note: I have a Tegra patch that will depend on this functionality. I'd
like to see the patch applied through the Tegra tree if possible, or if
not, quickly pushed into u-boot/master or u-boot/next so Tom Warren can
base a Tegra branch on top of it easily without delay.

 drivers/serial/serial_ns16550.c |    5 +++++
 include/ns16550.h               |    1 +
 2 files changed, 6 insertions(+)

Comments

Simon Glass Dec. 12, 2012, 11:38 p.m. UTC | #1
Hi Stephen,

On Wed, Dec 12, 2012 at 3:23 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> A single U-Boot binary may support multiple very similar boards. These
> boards may use different UARTs for the main debug console. Hence, it is
> impossible to #define CONFIG_SYS_NS16550_COM1 to some static UART
> address, since the true value may only be determined at run-time, after
> identifying the actual hardware. Provide an API for boards to call to
> set the actual address of the UART, e.g. from spl_board_init() or
> board_early_init_f().
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

This seems reasonable in the interim while we are hard-coding things
but needing more flexibility. How do you plan to configure the actual
address - is it with the ODM data or FDT?

One question though - is it not possible to select the correct port
number using environment (say) rather than changing the address of an
existing port? After all, I think we can assume that all available
ports are in the array. Or can we?

Regards,
Simon

> ---
> Note: I have a Tegra patch that will depend on this functionality. I'd
> like to see the patch applied through the Tegra tree if possible, or if
> not, quickly pushed into u-boot/master or u-boot/next so Tom Warren can
> base a Tegra branch on top of it easily without delay.
>
>  drivers/serial/serial_ns16550.c |    5 +++++
>  include/ns16550.h               |    1 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c
> index fc01a3c..fc8253b 100644
> --- a/drivers/serial/serial_ns16550.c
> +++ b/drivers/serial/serial_ns16550.c
> @@ -166,6 +166,11 @@ static int calc_divisor (NS16550_t port)
>                 (MODE_X_DIV * gd->baudrate);
>  }
>
> +void NS16550_set_dynamic_address(int port, NS16550_t com_port)
> +{
> +       PORT = com_port;
> +}
> +
>  void
>  _serial_putc(const char c,const int port)
>  {
> diff --git a/include/ns16550.h b/include/ns16550.h
> index 51cb5b4..6d7483f 100644
> --- a/include/ns16550.h
> +++ b/include/ns16550.h
> @@ -171,6 +171,7 @@ typedef struct NS16550 *NS16550_t;
>  /* useful defaults for LCR */
>  #define UART_LCR_8N1   0x03
>
> +void NS16550_set_dynamic_address(int port, NS16550_t com_port);
>  void NS16550_init(NS16550_t com_port, int baud_divisor);
>  void NS16550_putc(NS16550_t com_port, char c);
>  char NS16550_getc(NS16550_t com_port);
> --
> 1.7.10.4
>
Stephen Warren Dec. 12, 2012, 11:52 p.m. UTC | #2
On 12/12/2012 04:38 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Wed, Dec 12, 2012 at 3:23 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> A single U-Boot binary may support multiple very similar boards. These
>> boards may use different UARTs for the main debug console. Hence, it is
>> impossible to #define CONFIG_SYS_NS16550_COM1 to some static UART
>> address, since the true value may only be determined at run-time, after
>> identifying the actual hardware. Provide an API for boards to call to
>> set the actual address of the UART, e.g. from spl_board_init() or
>> board_early_init_f().
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> This seems reasonable in the interim while we are hard-coding things
> but needing more flexibility. How do you plan to configure the actual
> address - is it with the ODM data or FDT?

I intend to use the ODMDATA. This already includes a field that
specifies which UART to use. I'm working on some patches (to
BCT-generation tools and U-Boot) that define an ODMDATA2 value, which
will indicate the complete pinmux configuration required for the UART,
so everything can be self-contained. I'm fairly close to publishing
these patches.

> One question though - is it not possible to select the correct port
> number using environment (say) rather than changing the address of an
> existing port? After all, I think we can assume that all available
> ports are in the array. Or can we?

Right now, we only define one of CONFIG_SYS_NS16550_COMn (n==1..6). I
suppose we could define 5 of these, to represent the 5 different UARTs
on Tegra, so that all ports are always available, irrespective of what,
if anything, they're pinmuxed out to and hooked up to in HW.

The question would then become: how to tell U-Boot which to use? The
answer might be to put "eserial0" or "eserial1", etc. into the stdin
environment variable rather than plain "serial". However, the question
then becomes: what do we put into the default environment? The default
environment would then need to vary depending on which board you were
running on (since the serial port number might be different), and I
really want "env default -f -a" to leave the user with a working system.
How would that work?
Simon Glass Dec. 13, 2012, 12:38 a.m. UTC | #3
Hi Stephen,

On Wed, Dec 12, 2012 at 3:52 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 12/12/2012 04:38 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Wed, Dec 12, 2012 at 3:23 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> A single U-Boot binary may support multiple very similar boards. These
>>> boards may use different UARTs for the main debug console. Hence, it is
>>> impossible to #define CONFIG_SYS_NS16550_COM1 to some static UART
>>> address, since the true value may only be determined at run-time, after
>>> identifying the actual hardware. Provide an API for boards to call to
>>> set the actual address of the UART, e.g. from spl_board_init() or
>>> board_early_init_f().
>>>
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>
>> This seems reasonable in the interim while we are hard-coding things
>> but needing more flexibility. How do you plan to configure the actual
>> address - is it with the ODM data or FDT?
>
> I intend to use the ODMDATA. This already includes a field that
> specifies which UART to use. I'm working on some patches (to
> BCT-generation tools and U-Boot) that define an ODMDATA2 value, which
> will indicate the complete pinmux configuration required for the UART,
> so everything can be self-contained. I'm fairly close to publishing
> these patches.

Yes actually I remember you mentioning that before, sounds good.

>
>> One question though - is it not possible to select the correct port
>> number using environment (say) rather than changing the address of an
>> existing port? After all, I think we can assume that all available
>> ports are in the array. Or can we?
>
> Right now, we only define one of CONFIG_SYS_NS16550_COMn (n==1..6). I
> suppose we could define 5 of these, to represent the 5 different UARTs
> on Tegra, so that all ports are always available, irrespective of what,
> if anything, they're pinmuxed out to and hooked up to in HW.
>
> The question would then become: how to tell U-Boot which to use? The
> answer might be to put "eserial0" or "eserial1", etc. into the stdin
> environment variable rather than plain "serial". However, the question
> then becomes: what do we put into the default environment? The default
> environment would then need to vary depending on which board you were
> running on (since the serial port number might be different), and I
> really want "env default -f -a" to leave the user with a working system.
> How would that work?

Well I suppose U-Boot could have plain "serial" and its meaning would
be determined by the board at init. I don't think we have a way of
doing that.

But looking forward if we use the FDT to specify the console (as we
did in the Chromium Tegra tree) then it becomes a case of selecting
between available ports using /alias/console, rather than changing
port 0 to point to the selected port.

I don't see this as a big deal, particularly while we still have
everything in serial so hard-coded. Perhaps we should run with what
you have hear until the device model stuff lands, and then a new
solution will present itself.

Regards,
Simon
Wolfgang Denk Dec. 13, 2012, 10:27 a.m. UTC | #4
Dear Stephen Warren,

In message <1355354590-10023-1-git-send-email-swarren@wwwdotorg.org> you wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> A single U-Boot binary may support multiple very similar boards. These
> boards may use different UARTs for the main debug console. Hence, it is
> impossible to #define CONFIG_SYS_NS16550_COM1 to some static UART
> address, since the true value may only be determined at run-time, after
> identifying the actual hardware. Provide an API for boards to call to
> set the actual address of the UART, e.g. from spl_board_init() or
> board_early_init_f().
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

As is, this is just adding dead code.

Where would the device addresses come from - out of the device tree?

Best regards,

Wolfgang Denk
Wolfgang Denk Dec. 13, 2012, 10:29 a.m. UTC | #5
Dear Stephen Warren,

In message <50C918A5.6090207@wwwdotorg.org> you wrote:
>
> > This seems reasonable in the interim while we are hard-coding things
> > but needing more flexibility. How do you plan to configure the actual
> > address - is it with the ODM data or FDT?
> 
> I intend to use the ODMDATA. This already includes a field that
> specifies which UART to use. I'm working on some patches (to
> BCT-generation tools and U-Boot) that define an ODMDATA2 value, which
> will indicate the complete pinmux configuration required for the UART,
> so everything can be self-contained. I'm fairly close to publishing
> these patches.

Arghh... Do we really, really have to invent yet another way to pass
hardware configuration information?  Especially one totally
incompatible to any other system?

I think I will be objecting against such an approach.  Please use the
device tree instead.

Best regards,

Wolfgang Denk
Tom Rini Dec. 13, 2012, 1:11 p.m. UTC | #6
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/13/12 05:27, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message
> <1355354590-10023-1-git-send-email-swarren@wwwdotorg.org> you
> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>> 
>> A single U-Boot binary may support multiple very similar boards.
>> These boards may use different UARTs for the main debug console.
>> Hence, it is impossible to #define CONFIG_SYS_NS16550_COM1 to
>> some static UART address, since the true value may only be
>> determined at run-time, after identifying the actual hardware.
>> Provide an API for boards to call to set the actual address of
>> the UART, e.g. from spl_board_init() or board_early_init_f().
>> 
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> As is, this is just adding dead code.
> 
> Where would the device addresses come from - out of the device
> tree?

Board specific knowledge.  I'd be tempted to add UART3 (iirc) into the
am335x_evm default build so that we can support the Industrial DevKit
variant out of the box, rather than needing one of the other _uartN
builds.  We can tell which board we're on at run-time already.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQydQbAAoJENk4IS6UOR1WLpsP/3TsidcGHLMQoqyktG/qtzFr
LmIT7wfNomLsl7xmTrO0B4GwvpsH6OucW90z6HrL0qvH3IhZ2FohUcyWwWNNo2KZ
1gEFSMPbwZt3htrFE/fhHT8n+Fo/eq2hY32WmxnWV8XS46+FL348FHNxEeIiQN1p
1MwxmJmEGBqAtBdC7t2JIoHsQqd+txDs6R5xpm8f2S2zenJFkbp45FwDeQrn4Bu/
XVagwL4R/L21bPt/I90RdkRe5lt7ukQwoG1+HgaEjoCdiCol9p6bjBwWll+NXb9/
ouk+7rYEncjxn+/W9XB7ojeBwOMxQbreg4JJFikn41g5XOkLIe+l0n2/j1jWVSOO
u3ORXxOr1icMRY9BgUkLuKlhtONQX5IPz8t5F4N8tyhsGFSxs6kuX2NKo+Oy25B5
cidh43exx8VkHqInsq7ZFlll/Xdk7PD16iY7qoZh8BE6KzdbchBeZX2bCn3NiOIS
RtfVXrng58PaetHyzjsfcu1HDaaGez8vztabVUF4PECQmnV7hz2Vw25HqoK9un/L
Snl9uPNBELKA7DesPRMx0LaGwDkx4UBecvX2nWm+krkihvbdalmnawNbIWv9WNSt
OG3w+r/Ka68t2vFVTbIBVK7IVeDe/dISLpQ7R/MiVIzJDnD9EFJUQpaKxWp54PKF
ZV4bb59FxRxBpkO5eA1L
=QfiQ
-----END PGP SIGNATURE-----
Wolfgang Denk Dec. 13, 2012, 2:22 p.m. UTC | #7
Dear Tom Rini,

In message <50C9D41B.7010800@ti.com> you wrote:
>
> > Where would the device addresses come from - out of the device
> > tree?
> 
> Board specific knowledge.  I'd be tempted to add UART3 (iirc) into the
> am335x_evm default build so that we can support the Industrial DevKit
> variant out of the box, rather than needing one of the other _uartN
> builds.  We can tell which board we're on at run-time already.

I'm afraid this doesn't scale.  You are opening a can of worms here.
The UART port may seem simple enough, but you set a precedent.  Next
comes some netowrk interface configuration, then if we have a RTC,
followed by LCD properties, and so on.

We have two ways to configure hardware properties:
- static configuration in the board config file
- dynamic configuration through the device tree

Please do not start adding other methods.

Best regards,

Wolfgang Denk
Stephen Warren Dec. 13, 2012, 6:17 p.m. UTC | #8
On 12/13/2012 03:29 AM, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message <50C918A5.6090207@wwwdotorg.org> you wrote:
>>
>>> This seems reasonable in the interim while we are hard-coding things
>>> but needing more flexibility. How do you plan to configure the actual
>>> address - is it with the ODM data or FDT?
>>
>> I intend to use the ODMDATA. This already includes a field that
>> specifies which UART to use. I'm working on some patches (to
>> BCT-generation tools and U-Boot) that define an ODMDATA2 value, which
>> will indicate the complete pinmux configuration required for the UART,
>> so everything can be self-contained. I'm fairly close to publishing
>> these patches.
> 
> Arghh... Do we really, really have to invent yet another way to pass
> hardware configuration information?  Especially one totally
> incompatible to any other system?

This is a special case for the console UART. The idea is to get that up
and running well before device tree is parsed in any way. For example,
Tegra's SPL doesn't touch the device tree in any way (or even know one
exists) but does want to print (possibly error) messages in a generic
fashion. Similarly, many problems could occur before the device tree is
parsed (e.g. the user forgets to provide one...), and having
specifically the console UART set up before that allows those errors to
be reported, rather than requiring a JTAG or similar debugger.

My intent is that ODMDATA will definitely only be used for the console
UART, and will NOT be used for anything else like LCD, RTC, ... Those
other devices will certainly be configured via device tree.
Wolfgang Denk Dec. 13, 2012, 8:36 p.m. UTC | #9
Dear Stephen Warren,

In message <50CA1BB8.4000704@wwwdotorg.org> you wrote:
>
> > Arghh... Do we really, really have to invent yet another way to pass
> > hardware configuration information?  Especially one totally
> > incompatible to any other system?
> 
> This is a special case for the console UART. The idea is to get that up
> and running well before device tree is parsed in any way. For example,
> Tegra's SPL doesn't touch the device tree in any way (or even know one
> exists) but does want to print (possibly error) messages in a generic
> fashion. Similarly, many problems could occur before the device tree is
> parsed (e.g. the user forgets to provide one...), and having
> specifically the console UART set up before that allows those errors to
> be reported, rather than requiring a JTAG or similar debugger.
> 
> My intent is that ODMDATA will definitely only be used for the console
> UART, and will NOT be used for anything else like LCD, RTC, ... Those
> other devices will certainly be configured via device tree.

We've been there before, you know.

OK - what is the scope of visibility of such code?  Will it be
strictly board specific only?  Or SoC specific? Arch? Global?

Best regards,

Wolfgang Denk
Stephen Warren Dec. 13, 2012, 8:45 p.m. UTC | #10
On 12/13/2012 01:36 PM, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message <50CA1BB8.4000704@wwwdotorg.org> you wrote:
>>
>>> Arghh... Do we really, really have to invent yet another way to pass
>>> hardware configuration information?  Especially one totally
>>> incompatible to any other system?
>>
>> This is a special case for the console UART. The idea is to get that up
>> and running well before device tree is parsed in any way. For example,
>> Tegra's SPL doesn't touch the device tree in any way (or even know one
>> exists) but does want to print (possibly error) messages in a generic
>> fashion. Similarly, many problems could occur before the device tree is
>> parsed (e.g. the user forgets to provide one...), and having
>> specifically the console UART set up before that allows those errors to
>> be reported, rather than requiring a JTAG or similar debugger.
>>
>> My intent is that ODMDATA will definitely only be used for the console
>> UART, and will NOT be used for anything else like LCD, RTC, ... Those
>> other devices will certainly be configured via device tree.
> 
> We've been there before, you know.

I'm not quite sure what the implication is here.

> OK - what is the scope of visibility of such code?  Will it be
> strictly board specific only?  Or SoC specific? Arch? Global?

It's partially SoC-specific, partially global.

Note that by "all" and "global" here, I'm talking relative to all Tegra
SoCs, not about anything non-Tegra. "SoC-specific" means different for
Tegra20, Tegra30, Tegra114, etc.

In every Tegra SoC, the boot ROM reads a BCT (Boot Configuration Table)
at boot. The BCT contains e.g. SDRAM controller configuration and other
low-level boot information. The BCT is stored within the boot flash. The
ODMDATA fields are stored within the BCT. The offset of the ODMDATA
within the BCT is SoC-specific since the BCT structure is SoC-specific.

The ODMDATA for all Tegra SoCs includes fields that define (a) which
UART to use (so far, identical across all SoCs) (b) the pinmux
configuration to use, and other information (mainly SDRAM size at the
moment). Since the pinmux HW is SoC-specific, so is the exact format of
the UART pinmux configuration data.

For more details on Tegra BCTs, you may refer to:
ftp://download.nvidia.com/tegra-public-appnotes/index.html
http://nv-tegra.nvidia.com/gitweb/?p=tools/cbootimage.git;a=summary

Note that in the latter case, I haven't pushed out the patches which
document the UART pinmux fields yet, but will very soon; most likely as
soon as we've resolved this conversation.
Tom Rini Dec. 13, 2012, 8:53 p.m. UTC | #11
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/13/12 15:45, Stephen Warren wrote:
> On 12/13/2012 01:36 PM, Wolfgang Denk wrote:
>> Dear Stephen Warren,
>> 
>> In message <50CA1BB8.4000704@wwwdotorg.org> you wrote:
>>> 
>>>> Arghh... Do we really, really have to invent yet another way
>>>> to pass hardware configuration information?  Especially one
>>>> totally incompatible to any other system?
>>> 
>>> This is a special case for the console UART. The idea is to get
>>> that up and running well before device tree is parsed in any
>>> way. For example, Tegra's SPL doesn't touch the device tree in
>>> any way (or even know one exists) but does want to print
>>> (possibly error) messages in a generic fashion. Similarly, many
>>> problems could occur before the device tree is parsed (e.g. the
>>> user forgets to provide one...), and having specifically the
>>> console UART set up before that allows those errors to be
>>> reported, rather than requiring a JTAG or similar debugger.
>>> 
>>> My intent is that ODMDATA will definitely only be used for the
>>> console UART, and will NOT be used for anything else like LCD,
>>> RTC, ... Those other devices will certainly be configured via
>>> device tree.
>> 
>> We've been there before, you know.
> 
> I'm not quite sure what the implication is here.
> 
>> OK - what is the scope of visibility of such code?  Will it be 
>> strictly board specific only?  Or SoC specific? Arch? Global?
> 
> It's partially SoC-specific, partially global.

Right.  I see what Wolfgang was saying before, and I get it now.  This
is not how we want to open the can of worms for "lets do dynamic
locations of stuff".  We should start with being able to parse (some
form of the normal) device tree, and be able to say "I now know I have
a UART $HERE and $THERE".  And yes, that's too late for initial
console being in the right spot, at first, probably.  But now we've
moved in the direction of being able to dynamically assign things.
And from there we can move on and say "On ${SoC} we get a device tree
(that we can't quite parse as we don't have enough resources) AND
$some-data (OMDATA or an abbreviated device tree or $whatever), lets
translate that into something we can make use of very early rather
than a hard-coded initial console location"

In other words, we want to (re)start the conversation around lets get
device tree going.  Then we can deal with other things.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQykBWAAoJENk4IS6UOR1W0q4P/2P2yFc0FgPNfDdPU31DCVwI
mZPEzIY4xkuCAh0F2fGfzHjqYHtivru3py9Q7Os1Quse7BoWYea5HHH0/dbxCGGI
DWuqWUtle7XPFwfdTiMkf60wXcYzTKrg/1KXV259lvy3zxjhgttDMNMhucHEgWG4
E3wdfGqvQ6SuAYOxDAuSZrA6N7hCLCcCDSt2YMKWpIuP0iJiwCYLPloXgMfQd9h0
en+KY2TivavbFRsUoXzdmmDk7k0/7nP0TzOGn7TQVWetQ3x63C8Z8nD25QxiDDEP
onQq7p32UjXpbd1DVgy1F77n8afj6jKYWyRKCC8w2pp/PXx7W00qJRg/7KBGYAgx
VASpUpWb004WPIHLUykOee9cZneo0HlH1EUCIMkFLVLunABMxlMjLvdVQ04Ow4st
GbDRTKkTsG0pheGCTN50CyJMexv0wjDgaqS9PsaRtYNBliwXslg5lqFBm6u96/gR
+6nzkI9vHLGNnFOjqWVQeKt3XjQW+eByivDB778isMYohdEwOMCFz3uFSsK3AcGR
YxdM3CPNYzbMMZQ6SwYp4NZ+6XQd+ovIunuWECapRLRSIaI6pOKo3TBmOJOfbCD0
Pxf60seXUj4jsCO6e9oki5C/vwC1PiQ6uMxL2ucNpJN6sHdH0ivHRCGdjy866m0r
FGFVZlNtxmXhROcdBYTV
=N5NV
-----END PGP SIGNATURE-----
Stephen Warren Dec. 13, 2012, 9:07 p.m. UTC | #12
On 12/13/2012 01:53 PM, Tom Rini wrote:
> On 12/13/12 15:45, Stephen Warren wrote:
>> On 12/13/2012 01:36 PM, Wolfgang Denk wrote:
>>> Dear Stephen Warren,
>>> 
>>> In message <50CA1BB8.4000704@wwwdotorg.org> you wrote:
>>>> 
>>>>> Arghh... Do we really, really have to invent yet another
>>>>> way to pass hardware configuration information?  Especially
>>>>> one totally incompatible to any other system?
>>>> 
>>>> This is a special case for the console UART. The idea is to
>>>> get that up and running well before device tree is parsed in
>>>> any way. For example, Tegra's SPL doesn't touch the device
>>>> tree in any way (or even know one exists) but does want to
>>>> print (possibly error) messages in a generic fashion.
>>>> Similarly, many problems could occur before the device tree
>>>> is parsed (e.g. the user forgets to provide one...), and
>>>> having specifically the console UART set up before that
>>>> allows those errors to be reported, rather than requiring a
>>>> JTAG or similar debugger.
>>>> 
>>>> My intent is that ODMDATA will definitely only be used for
>>>> the console UART, and will NOT be used for anything else like
>>>> LCD, RTC, ... Those other devices will certainly be
>>>> configured via device tree.
>>> 
>>> We've been there before, you know.
> 
>> I'm not quite sure what the implication is here.
> 
>>> OK - what is the scope of visibility of such code?  Will it be
>>>  strictly board specific only?  Or SoC specific? Arch? Global?
> 
>> It's partially SoC-specific, partially global.
> 
> Right.  I see what Wolfgang was saying before, and I get it now.
> This is not how we want to open the can of worms for "lets do
> dynamic locations of stuff".  We should start with being able to
> parse (some form of the normal) device tree, and be able to say "I
> now know I have a UART $HERE and $THERE".

So if Tegra were to statically define the location of all 5 on-SoC
UARTs, by defining CONFIG_SYS_NS16550_COM*, and then use the ODMDATA
to select which UART to use for the console, rather than using the
ODMDATA to dynamically change the value that CONFIG_SYS_NS16550_COM1
sets up, would that remove the objection? I haven't look into coding
that up, but I imagine it could be made to work...

> And yes, that's too late for initial console being in the right
> spot, at first, probably.  But now we've moved in the direction of
> being able to dynamically assign things.

I'm not sure about "at first"; on Tegra, I don't imagine the SPL would
ever use device tree. The only HW- (board-) specific thing that's
relevant to it is the UART and UART-pinmux, since the SPL only exists
to boot the main A9 cores, and doesn't ever access any kind of storage
device.

> And from there we can move on and say "On ${SoC} we get a device
> tree (that we can't quite parse as we don't have enough resources)
> AND $some-data (OMDATA or an abbreviated device tree or $whatever),
> lets translate that into something we can make use of very early
> rather than a hard-coded initial console location"

It seems like you're saying that once we have dynamic serial port
assignment working based on DT, you'll be fine using ODMDATA to
initialize the early console, but not before then? If so, I'm having a
hard time understanding why enabling the DT-based support blocks using
ODMDATA, since the code would be pretty orthogonal.
Simon Glass Dec. 13, 2012, 9:51 p.m. UTC | #13
Hi Stephen,

On Thu, Dec 13, 2012 at 1:07 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 12/13/2012 01:53 PM, Tom Rini wrote:
>> On 12/13/12 15:45, Stephen Warren wrote:
>>> On 12/13/2012 01:36 PM, Wolfgang Denk wrote:
>>>> Dear Stephen Warren,
>>>>
>>>> In message <50CA1BB8.4000704@wwwdotorg.org> you wrote:
>>>>>
>>>>>> Arghh... Do we really, really have to invent yet another
>>>>>> way to pass hardware configuration information?  Especially
>>>>>> one totally incompatible to any other system?
>>>>>
>>>>> This is a special case for the console UART. The idea is to
>>>>> get that up and running well before device tree is parsed in
>>>>> any way. For example, Tegra's SPL doesn't touch the device
>>>>> tree in any way (or even know one exists) but does want to
>>>>> print (possibly error) messages in a generic fashion.
>>>>> Similarly, many problems could occur before the device tree
>>>>> is parsed (e.g. the user forgets to provide one...), and
>>>>> having specifically the console UART set up before that
>>>>> allows those errors to be reported, rather than requiring a
>>>>> JTAG or similar debugger.
>>>>>
>>>>> My intent is that ODMDATA will definitely only be used for
>>>>> the console UART, and will NOT be used for anything else like
>>>>> LCD, RTC, ... Those other devices will certainly be
>>>>> configured via device tree.
>>>>
>>>> We've been there before, you know.
>>
>>> I'm not quite sure what the implication is here.
>>
>>>> OK - what is the scope of visibility of such code?  Will it be
>>>>  strictly board specific only?  Or SoC specific? Arch? Global?
>>
>>> It's partially SoC-specific, partially global.
>>
>> Right.  I see what Wolfgang was saying before, and I get it now.
>> This is not how we want to open the can of worms for "lets do
>> dynamic locations of stuff".  We should start with being able to
>> parse (some form of the normal) device tree, and be able to say "I
>> now know I have a UART $HERE and $THERE".
>
> So if Tegra were to statically define the location of all 5 on-SoC
> UARTs, by defining CONFIG_SYS_NS16550_COM*, and then use the ODMDATA
> to select which UART to use for the console, rather than using the
> ODMDATA to dynamically change the value that CONFIG_SYS_NS16550_COM1
> sets up, would that remove the objection? I haven't look into coding
> that up, but I imagine it could be made to work...

Seems good to me.

>
>> And yes, that's too late for initial console being in the right
>> spot, at first, probably.  But now we've moved in the direction of
>> being able to dynamically assign things.
>
> I'm not sure about "at first"; on Tegra, I don't imagine the SPL would
> ever use device tree. The only HW- (board-) specific thing that's
> relevant to it is the UART and UART-pinmux, since the SPL only exists
> to boot the main A9 cores, and doesn't ever access any kind of storage
> device.

On Tegra, yes. On some chips, SPL accesses devices to read U-Boot.

In extremis we could use a very simple table (a C structure with a
couple of members) which is filled in by a tool from the device tree
as part of image creation, just to avoid the FDT overhead. However,
given that many of the SOCs I seem to be using have >100KB of SRAM,
it's not clear that we shouldn't just use FDT eventually.

>
>> And from there we can move on and say "On ${SoC} we get a device
>> tree (that we can't quite parse as we don't have enough resources)
>> AND $some-data (OMDATA or an abbreviated device tree or $whatever),
>> lets translate that into something we can make use of very early
>> rather than a hard-coded initial console location"
>
> It seems like you're saying that once we have dynamic serial port
> assignment working based on DT, you'll be fine using ODMDATA to
> initialize the early console, but not before then? If so, I'm having a
> hard time understanding why enabling the DT-based support blocks using
> ODMDATA, since the code would be pretty orthogonal.

Yes well dynamic console selection sounds find to me, ODMDATA or
otherwise. To me it is a Tegra feature that should be supported as
such. Perhaps we can allow the FDT console alias to specify "odmdata"
to mean that, and/or (as you suggest I think) set the console to
USE_ODMDATA, which then selects CONFIG_SYS_NS16550_COMx accordingly.

Regards,
Simon
Wolfgang Denk Dec. 13, 2012, 11:11 p.m. UTC | #14
Dear Stephen Warren,

In message <50CA3E7A.8020407@wwwdotorg.org> you wrote:
>
> >> My intent is that ODMDATA will definitely only be used for the console
> >> UART, and will NOT be used for anything else like LCD, RTC, ... Those
> >> other devices will certainly be configured via device tree.
> > 
> > We've been there before, you know.
> 
> I'm not quite sure what the implication is here.

What I mean is: there have been a number of times before when we
decided to do something more or less ugly because it appeared to be
the easiest / fastest / most simple approacht at that time,and we were
sure we would it need for this one special case only.  Then it gor
reused, and again, and it spread...

> > OK - what is the scope of visibility of such code?  Will it be
> > strictly board specific only?  Or SoC specific? Arch? Global?
> 
> It's partially SoC-specific, partially global.

Which exact parts would be global?

I am aware that the capability to set the UART is obviously part of
the global code.

But the actual implementation of such setting would be not global at
all, right?

> Note that by "all" and "global" here, I'm talking relative to all Tegra
> SoCs, not about anything non-Tegra. "SoC-specific" means different for
> Tegra20, Tegra30, Tegra114, etc.

OK.

> Note that in the latter case, I haven't pushed out the patches which
> document the UART pinmux fields yet, but will very soon; most likely as
> soon as we've resolved this conversation.

You guarantee that this all will remain strictly within Tegra specific
areas, only?  And only for the UART?

Best regards,

Wolfgang Denk
Stephen Warren Dec. 13, 2012, 11:26 p.m. UTC | #15
On 12/13/2012 04:11 PM, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message <50CA3E7A.8020407@wwwdotorg.org> you wrote:
>>
>>>> My intent is that ODMDATA will definitely only be used for the console
>>>> UART, and will NOT be used for anything else like LCD, RTC, ... Those
>>>> other devices will certainly be configured via device tree.
>>>
>>> We've been there before, you know.
>>
>> I'm not quite sure what the implication is here.
> 
> What I mean is: there have been a number of times before when we
> decided to do something more or less ugly because it appeared to be
> the easiest / fastest / most simple approacht at that time,and we were
> sure we would it need for this one special case only.  Then it gor
> reused, and again, and it spread...
> 
>>> OK - what is the scope of visibility of such code?  Will it be
>>> strictly board specific only?  Or SoC specific? Arch? Global?
>>
>> It's partially SoC-specific, partially global.
> 
> Which exact parts would be global?

I guess by global you meant for any SoC/CPU/... U-Boot supports, whereas
I was treating global as across all Tegras.

So given that, the only global part would be the NS16550 patch I sent
out already. Anything else would be contained entirely within the Tegra
common board file.

> I am aware that the capability to set the UART is obviously part of
> the global code.

Right.

> But the actual implementation of such setting would be not global at
> all, right?

Right; it'd be part of that Tegra common board.c file, shared by all
Tegra boards, but should have zero impact outside any Tegra-specific code.

>> Note that by "all" and "global" here, I'm talking relative to all Tegra
>> SoCs, not about anything non-Tegra. "SoC-specific" means different for
>> Tegra20, Tegra30, Tegra114, etc.
> 
> OK.
> 
>> Note that in the latter case, I haven't pushed out the patches which
>> document the UART pinmux fields yet, but will very soon; most likely as
>> soon as we've resolved this conversation.
> 
> You guarantee that this all will remain strictly within Tegra specific
> areas, only?  And only for the UART?

Yes. I don't have any intention to use it for anything other than
console UART. I don't know of anyone else who wants to use it for
anything other than console UART. If anyone else tries to use it for
anything else, I'll give review comments not to, and direct them towards
device tree.

Of course, I can't predict the future, but that's just science, not my
trying to weasel out of a promise.
Tom Rini Dec. 14, 2012, 8:40 p.m. UTC | #16
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/13/12 16:51, Simon Glass wrote:

[snip]
>>> And from there we can move on and say "On ${SoC} we get a
>>> device tree (that we can't quite parse as we don't have enough
>>> resources) AND $some-data (OMDATA or an abbreviated device tree
>>> or $whatever), lets translate that into something we can make
>>> use of very early rather than a hard-coded initial console
>>> location"
>> 
>> It seems like you're saying that once we have dynamic serial
>> port assignment working based on DT, you'll be fine using ODMDATA
>> to initialize the early console, but not before then? If so, I'm
>> having a hard time understanding why enabling the DT-based
>> support blocks using ODMDATA, since the code would be pretty
>> orthogonal.
> 
> Yes well dynamic console selection sounds find to me, ODMDATA or 
> otherwise. To me it is a Tegra feature that should be supported as 
> such. Perhaps we can allow the FDT console alias to specify
> "odmdata" to mean that, and/or (as you suggest I think) set the
> console to USE_ODMDATA, which then selects CONFIG_SYS_NS16550_COMx
> accordingly.

There's two parts to it.  One part is that sure, Tegra and only Tegra
has ODMDATA.  But on am33xx if we poke the i2c eeprom (on platforms
that do the eeprom) we can then know ...  And I bet other SoCs have
other tricks for this or that.  So it's not just tegra that can tell
us the initial console is $here or $there if we just ...something.

The other part is, take a look at the Allwinner thread from a week or
so ago.  We really need to define how we want early board specific
data to come in because if we start saying we'll accept per-SoC
solutions we'll be drowning in them in short time.  We want to say
here's our preferred way to pass this information in.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQy47RAAoJENk4IS6UOR1W/wIP/1CyXg8ShiITZAS6/R52Aj89
X7mhTHWUg3m+BtEN8TGkI8foznHjpv5JK7Exlf8XgKuoH6idBYd/5eF6nRXGfePY
rQAad+hHeBYSBfvaP6GIaSTMm5x6KDMILExDkxue0mNcdkRL568ac0oR/HsVPM/N
d75PVHsK77dAIzm9DbT3m2zilHC6balbplG1LtnKx0+sKb/PGpfXT5GYCZUUc9es
R2Uzkyx+f25ZTlVRzdrh+h8SDhNzuACszJtqY11SxhLUZevvkja0jm6LykBsyJGH
wh1wydRRPN6LKWRVQgUAHBJBEebf6Olck+PPBBA3+LypN5kSuj+bKixoyNxTQHIf
E8qJb59rB7bnXLVb43AudcbUWe/PqdwZ8Yha3dmMrT/r8k0eXfNLBXlIP8BDXPis
4ssoH/DZXqv0+QvsmX+NPoF/3pBSy/Uc1g13w20xbdy12ovGEVgOWwioOPKgWgCR
b34CvOT7MR+8zMixCHP287ITyTrpY/K7gxo2rF8EQ+o6QW1JCphTJFpVqlTVWEAT
5vPMnt7y7w9WtImCxUpa7itMfeVOgnbv+2bB2Ipxj6VjOZcIvdqB405wN39bGYux
fzatCFurtbdaSQ7aFR1ZFRp51rjl/rC7QxODD0H84Ip7AbVO2qLPOTri2wwYfaPN
EXPCI+T8YtDbI2/RW92B
=DR6s
-----END PGP SIGNATURE-----
Simon Glass Dec. 14, 2012, 9:14 p.m. UTC | #17
Hi Tom,

On Fri, Dec 14, 2012 at 12:40 PM, Tom Rini <trini@ti.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 12/13/12 16:51, Simon Glass wrote:
>
> [snip]
>>>> And from there we can move on and say "On ${SoC} we get a
>>>> device tree (that we can't quite parse as we don't have enough
>>>> resources) AND $some-data (OMDATA or an abbreviated device tree
>>>> or $whatever), lets translate that into something we can make
>>>> use of very early rather than a hard-coded initial console
>>>> location"
>>>
>>> It seems like you're saying that once we have dynamic serial
>>> port assignment working based on DT, you'll be fine using ODMDATA
>>> to initialize the early console, but not before then? If so, I'm
>>> having a hard time understanding why enabling the DT-based
>>> support blocks using ODMDATA, since the code would be pretty
>>> orthogonal.
>>
>> Yes well dynamic console selection sounds find to me, ODMDATA or
>> otherwise. To me it is a Tegra feature that should be supported as
>> such. Perhaps we can allow the FDT console alias to specify
>> "odmdata" to mean that, and/or (as you suggest I think) set the
>> console to USE_ODMDATA, which then selects CONFIG_SYS_NS16550_COMx
>> accordingly.
>
> There's two parts to it.  One part is that sure, Tegra and only Tegra
> has ODMDATA.  But on am33xx if we poke the i2c eeprom (on platforms
> that do the eeprom) we can then know ...  And I bet other SoCs have
> other tricks for this or that.  So it's not just tegra that can tell
> us the initial console is $here or $there if we just ...something.
>
> The other part is, take a look at the Allwinner thread from a week or
> so ago.  We really need to define how we want early board specific
> data to come in because if we start saying we'll accept per-SoC
> solutions we'll be drowning in them in short time.  We want to say
> here's our preferred way to pass this information in.

Yes there is a general problem to be solved here. Assuming that the
problem is solved in U-Boot itself with device tree, then:

1. It would be nice to keep a single source for this information so
that SPL and U-Boot are consistent. Where we invent a new mechanism
for efficiency reasons it would be best if there was a 1-1 mapping
from device tree to this new mechanism.

2. It would be nice if we could write a simple tool which is generic
across architectures and configures an SPL given a device tree file.
I'm not sure if that is a problem worth solving or not.

3. From the SPL point of view, the less code required to get at the
information, the better.

For one possible solution, see:

arch/arm/include/asm/arch-exynos/spl.h

Basically it works (for a small number of parameters) by giving each a
letter, and listing the required parameters in the structure. An
external tool can then fairly easily fill in the values it needs from
the device tree, without worrying about the format being wrong, etc.

I'm not sure that it scales to all SOCs though.

Personally I would like to see the simplest option possible and avoid
inventing a new infrastructure just for SPL.

Regards,
Simon

>
> - --
> Tom
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://www.enigmail.net/
>
> iQIcBAEBAgAGBQJQy47RAAoJENk4IS6UOR1W/wIP/1CyXg8ShiITZAS6/R52Aj89
> X7mhTHWUg3m+BtEN8TGkI8foznHjpv5JK7Exlf8XgKuoH6idBYd/5eF6nRXGfePY
> rQAad+hHeBYSBfvaP6GIaSTMm5x6KDMILExDkxue0mNcdkRL568ac0oR/HsVPM/N
> d75PVHsK77dAIzm9DbT3m2zilHC6balbplG1LtnKx0+sKb/PGpfXT5GYCZUUc9es
> R2Uzkyx+f25ZTlVRzdrh+h8SDhNzuACszJtqY11SxhLUZevvkja0jm6LykBsyJGH
> wh1wydRRPN6LKWRVQgUAHBJBEebf6Olck+PPBBA3+LypN5kSuj+bKixoyNxTQHIf
> E8qJb59rB7bnXLVb43AudcbUWe/PqdwZ8Yha3dmMrT/r8k0eXfNLBXlIP8BDXPis
> 4ssoH/DZXqv0+QvsmX+NPoF/3pBSy/Uc1g13w20xbdy12ovGEVgOWwioOPKgWgCR
> b34CvOT7MR+8zMixCHP287ITyTrpY/K7gxo2rF8EQ+o6QW1JCphTJFpVqlTVWEAT
> 5vPMnt7y7w9WtImCxUpa7itMfeVOgnbv+2bB2Ipxj6VjOZcIvdqB405wN39bGYux
> fzatCFurtbdaSQ7aFR1ZFRp51rjl/rC7QxODD0H84Ip7AbVO2qLPOTri2wwYfaPN
> EXPCI+T8YtDbI2/RW92B
> =DR6s
> -----END PGP SIGNATURE-----
Stephen Warren Dec. 14, 2012, 9:52 p.m. UTC | #18
On 12/14/2012 01:40 PM, Tom Rini wrote:
> On 12/13/12 16:51, Simon Glass wrote:
> 
> [snip]
>>>> And from there we can move on and say "On ${SoC} we get a 
>>>> device tree (that we can't quite parse as we don't have
>>>> enough resources) AND $some-data (OMDATA or an abbreviated
>>>> device tree or $whatever), lets translate that into something
>>>> we can make use of very early rather than a hard-coded
>>>> initial console location"
>>> 
>>> It seems like you're saying that once we have dynamic serial 
>>> port assignment working based on DT, you'll be fine using
>>> ODMDATA to initialize the early console, but not before then?
>>> If so, I'm having a hard time understanding why enabling the
>>> DT-based support blocks using ODMDATA, since the code would be
>>> pretty orthogonal.
> 
>> Yes well dynamic console selection sounds find to me, ODMDATA or
>>  otherwise. To me it is a Tegra feature that should be supported
>> as such. Perhaps we can allow the FDT console alias to specify 
>> "odmdata" to mean that, and/or (as you suggest I think) set the 
>> console to USE_ODMDATA, which then selects
>> CONFIG_SYS_NS16550_COMx accordingly.
> 
> There's two parts to it.  One part is that sure, Tegra and only
> Tegra has ODMDATA.  But on am33xx if we poke the i2c eeprom (on
> platforms that do the eeprom) we can then know ...  And I bet other
> SoCs have other tricks for this or that.  So it's not just tegra
> that can tell us the initial console is $here or $there if we just
> ...something.

That's certainly true.

I personally view the method of retrieving this kind of information as
part of an SoC's boot architecture, or as part of a board's design. As
you have mentioned above, different SoCs/boards already have
mechanisms to represent/determine this information. These mechanisms
are already in-place and defined by the SoC or board designers.

> The other part is, take a look at the Allwinner thread from a week
> or so ago.  We really need to define how we want early board
> specific data to come in because if we start saying we'll accept
> per-SoC solutions we'll be drowning in them in short time.  We want
> to say here's our preferred way to pass this information in.

I don't understand why you think U-Boot is in a position to mandate
that the existing solutions that are already in place are incorrect,
and must be replaced with some alternative.
Stephen Warren Dec. 14, 2012, 10:03 p.m. UTC | #19
On 12/14/2012 02:14 PM, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, Dec 14, 2012 at 12:40 PM, Tom Rini <trini@ti.com> wrote: On
> 12/13/12 16:51, Simon Glass wrote:
> 
> [snip]
>>>>>> And from there we can move on and say "On ${SoC} we get
>>>>>> a device tree (that we can't quite parse as we don't have
>>>>>> enough resources) AND $some-data (OMDATA or an
>>>>>> abbreviated device tree or $whatever), lets translate
>>>>>> that into something we can make use of very early rather
>>>>>> than a hard-coded initial console location"
>>>>> 
>>>>> It seems like you're saying that once we have dynamic
>>>>> serial port assignment working based on DT, you'll be fine
>>>>> using ODMDATA to initialize the early console, but not
>>>>> before then? If so, I'm having a hard time understanding
>>>>> why enabling the DT-based support blocks using ODMDATA,
>>>>> since the code would be pretty orthogonal.
>>>> 
>>>> Yes well dynamic console selection sounds find to me, ODMDATA
>>>> or otherwise. To me it is a Tegra feature that should be
>>>> supported as such. Perhaps we can allow the FDT console alias
>>>> to specify "odmdata" to mean that, and/or (as you suggest I
>>>> think) set the console to USE_ODMDATA, which then selects
>>>> CONFIG_SYS_NS16550_COMx accordingly.
> 
> There's two parts to it.  One part is that sure, Tegra and only
> Tegra has ODMDATA.  But on am33xx if we poke the i2c eeprom (on
> platforms that do the eeprom) we can then know ...  And I bet other
> SoCs have other tricks for this or that.  So it's not just tegra
> that can tell us the initial console is $here or $there if we just
> ...something.
> 
> The other part is, take a look at the Allwinner thread from a week
> or so ago.  We really need to define how we want early board
> specific data to come in because if we start saying we'll accept
> per-SoC solutions we'll be drowning in them in short time.  We want
> to say here's our preferred way to pass this information in.
> 
>> Yes there is a general problem to be solved here. Assuming that
>> the problem is solved in U-Boot itself with device tree, then:
> 
>> 1. It would be nice to keep a single source for this information
>> so that SPL and U-Boot are consistent. Where we invent a new
>> mechanism for efficiency reasons it would be best if there was a
>> 1-1 mapping from device tree to this new mechanism.

Many (most, I assume) U-Boot builds don't use device tree at all
(yet?). I'm not sure we should tie any new mechanism for low-level
boot information into device tree, since that severely limits where it
can be used.

>> 2. It would be nice if we could write a simple tool which is
>> generic across architectures and configures an SPL given a device
>> tree file. I'm not sure if that is a problem worth solving or
>> not.

I'm not sure the information is generic enough to even represent in
device tree, or that it even makes sense to do so.

After all, what the Tegra ODMDATA2 fields are representing is pinmux
setup. Every piece of hardware does pinmux differently; at the very
least, the pin IDs and available mux selection options are different.
Some SoCs mux pins individually, some in groups. Sometimes more than
just mux options must be selected. Some SoCs don't need pinmux. Some
SoCs would allow the data to be embedded into some boot flash in a
SoC-defined manner, whereas other SoCs' boards might require reading
GPIOs, I2C EEPROMs, ... to get the information, etc. All this means
that the information required, and the format needed to represent it,
really is different in every case. The only way to avoid this would be
to retro-actively redesign every SoC and board to always have the same
requirements for what data needs to represent the UART selection
process, and implement in the same way. That's obviously impossible to
do, and so having any kind of remotely generic tool to handle the
representation of this data also seems quite impossible.

>> 3. From the SPL point of view, the less code required to get at
>> the information, the better.
> 
>> For one possible solution, see:
> 
>> arch/arm/include/asm/arch-exynos/spl.h

A certain amount of that information duplicates what's in the Tegra
BCT, which is essentially part of the HW itself, since it's handled by
the non-modifiable boot ROM code. I really don't think U-Boot should
be mandating some data structure that requires duplication of
information that's already present.
Simon Glass Dec. 14, 2012, 10:22 p.m. UTC | #20
Hi Stephen,

On Fri, Dec 14, 2012 at 2:03 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 12/14/2012 02:14 PM, Simon Glass wrote:
>> Hi Tom,
>>
>> On Fri, Dec 14, 2012 at 12:40 PM, Tom Rini <trini@ti.com> wrote: On
>> 12/13/12 16:51, Simon Glass wrote:
>>
>> [snip]
>>>>>>> And from there we can move on and say "On ${SoC} we get
>>>>>>> a device tree (that we can't quite parse as we don't have
>>>>>>> enough resources) AND $some-data (OMDATA or an
>>>>>>> abbreviated device tree or $whatever), lets translate
>>>>>>> that into something we can make use of very early rather
>>>>>>> than a hard-coded initial console location"
>>>>>>
>>>>>> It seems like you're saying that once we have dynamic
>>>>>> serial port assignment working based on DT, you'll be fine
>>>>>> using ODMDATA to initialize the early console, but not
>>>>>> before then? If so, I'm having a hard time understanding
>>>>>> why enabling the DT-based support blocks using ODMDATA,
>>>>>> since the code would be pretty orthogonal.
>>>>>
>>>>> Yes well dynamic console selection sounds find to me, ODMDATA
>>>>> or otherwise. To me it is a Tegra feature that should be
>>>>> supported as such. Perhaps we can allow the FDT console alias
>>>>> to specify "odmdata" to mean that, and/or (as you suggest I
>>>>> think) set the console to USE_ODMDATA, which then selects
>>>>> CONFIG_SYS_NS16550_COMx accordingly.
>>
>> There's two parts to it.  One part is that sure, Tegra and only
>> Tegra has ODMDATA.  But on am33xx if we poke the i2c eeprom (on
>> platforms that do the eeprom) we can then know ...  And I bet other
>> SoCs have other tricks for this or that.  So it's not just tegra
>> that can tell us the initial console is $here or $there if we just
>> ...something.
>>
>> The other part is, take a look at the Allwinner thread from a week
>> or so ago.  We really need to define how we want early board
>> specific data to come in because if we start saying we'll accept
>> per-SoC solutions we'll be drowning in them in short time.  We want
>> to say here's our preferred way to pass this information in.
>>
>>> Yes there is a general problem to be solved here. Assuming that
>>> the problem is solved in U-Boot itself with device tree, then:
>>
>>> 1. It would be nice to keep a single source for this information
>>> so that SPL and U-Boot are consistent. Where we invent a new
>>> mechanism for efficiency reasons it would be best if there was a
>>> 1-1 mapping from device tree to this new mechanism.
>
> Many (most, I assume) U-Boot builds don't use device tree at all
> (yet?). I'm not sure we should tie any new mechanism for low-level
> boot information into device tree, since that severely limits where it
> can be used.

Perhaps I can make the point another way. Assuming that the SOC in
question is ARM-based and has Linux support it either supports FDT now
or presumably will fairly soon. We found that some of the things we
want to know about at the low level are hardware properties that are
already sit in a node in the FDT. For example the memory controller
may have information about the memory type attached to it.

Given this, my suggestion is that this hardware information be kept in
one place (FDT) where possible, even if it unfortunately temporarily
needs to be translated into some simpler format as part of the SPL
build for efficiency reasons.

As to platforms that support FDT, that is true, only a few. Adding
basic support for a new board is very easy though.

But looking at your comments below, I worry that this might be a
sledgehammer to crack a nut. As I understand it, you really just have
a 32-word which selects the console UART and a few other things. It
seems like you solved that problem a few emails back.

>
>>> 2. It would be nice if we could write a simple tool which is
>>> generic across architectures and configures an SPL given a device
>>> tree file. I'm not sure if that is a problem worth solving or
>>> not.
>
> I'm not sure the information is generic enough to even represent in
> device tree, or that it even makes sense to do so.
>
> After all, what the Tegra ODMDATA2 fields are representing is pinmux
> setup. Every piece of hardware does pinmux differently; at the very
> least, the pin IDs and available mux selection options are different.
> Some SoCs mux pins individually, some in groups. Sometimes more than
> just mux options must be selected. Some SoCs don't need pinmux. Some
> SoCs would allow the data to be embedded into some boot flash in a
> SoC-defined manner, whereas other SoCs' boards might require reading
> GPIOs, I2C EEPROMs, ... to get the information, etc. All this means
> that the information required, and the format needed to represent it,
> really is different in every case. The only way to avoid this would be
> to retro-actively redesign every SoC and board to always have the same
> requirements for what data needs to represent the UART selection
> process, and implement in the same way. That's obviously impossible to
> do, and so having any kind of remotely generic tool to handle the
> representation of this data also seems quite impossible.
>
>>> 3. From the SPL point of view, the less code required to get at
>>> the information, the better.
>>
>>> For one possible solution, see:
>>
>>> arch/arm/include/asm/arch-exynos/spl.h
>
> A certain amount of that information duplicates what's in the Tegra
> BCT, which is essentially part of the HW itself, since it's handled by
> the non-modifiable boot ROM code. I really don't think U-Boot should
> be mandating some data structure that requires duplication of
> information that's already present.

No certainly not. But I think Tom is concerned that each SOC will do
it differently and we might end up with a mess. SPL configuration is a
slightly different problem to what you talk about here, but in a sense
the BCT is an SOC-specific binary config blob attached to SPL. Other
SOCs might do a similar thing but in their own format and in their own
tools. Is it possible to unify these at all? Perhaps not?

Regards,
Simon
Wolfgang Denk Dec. 14, 2012, 10:26 p.m. UTC | #21
Dear Tom Rini,

In message <50CB8ED1.7020503@ti.com> you wrote:
>
> The other part is, take a look at the Allwinner thread from a week or
> so ago.  We really need to define how we want early board specific
> data to come in because if we start saying we'll accept per-SoC
> solutions we'll be drowning in them in short time.  We want to say
> here's our preferred way to pass this information in.

ACK!

And we already have a well-defined way to do this, which is the device
tree.  So any attempts to implement something different should be
reviewed very carefully.

Best regards,

Wolfgang Denk
Wolfgang Denk Dec. 14, 2012, 10:31 p.m. UTC | #22
Dear Stephen Warren,

In message <50CB9F9F.5010402@wwwdotorg.org> you wrote:
>
> I don't understand why you think U-Boot is in a position to mandate
> that the existing solutions that are already in place are incorrect,
> and must be replaced with some alternative.

There will always be times when common agreement on a new, superior
solution will enforce adaption of the existing implementations, or
they will break and drop out of mainline.

I don't claim this is such a case, but it could well be so.

Best regards,

Wolfgang Denk
Wolfgang Denk Dec. 14, 2012, 10:35 p.m. UTC | #23
Dear Stephen Warren,

In message <50CBA217.3070202@wwwdotorg.org> you wrote:
>
> Many (most, I assume) U-Boot builds don't use device tree at all
> (yet?). I'm not sure we should tie any new mechanism for low-level
> boot information into device tree, since that severely limits where it
> can be used.

We're talking about ways to pass hardware cosnfiguration information
to the boot loader.  From the software engineering point of view,
there should be just one implementation for this feature, which is
then used everywhere.  The de-factor satndard for this functionaity is
the device tree.  Which means that any other approaches either need
very strong reasons to exist, or should be adapted.

> I'm not sure the information is generic enough to even represent in
> device tree, or that it even makes sense to do so.

The DT is as good a place for such information as any other.

> A certain amount of that information duplicates what's in the Tegra
> BCT, which is essentially part of the HW itself, since it's handled by

There is more SoCs around than just Tegra, and a solution that fits
all is definitely better than everybody implementing hos own private
thingy.

Best regards,

Wolfgang Denk
Stephen Warren Dec. 14, 2012, 10:45 p.m. UTC | #24
On 12/14/2012 03:22 PM, Simon Glass wrote:
> Hi Stephen,
...
> Perhaps I can make the point another way. Assuming that the SOC in
> question is ARM-based and has Linux support it either supports FDT now
> or presumably will fairly soon.

Sure, but I'm *explicitly* avoiding relying on DT for this, because
there are plenty of things that happen before DT can or should be
touched that might warrant serial port output.

The kernel has exactly the same kind of feature; log messages can be
sent out through an SoC-specific earlyprintk mechanism. In the U-Boot
case though, I don't plan on replacing the serial port based on
information from DT, but simply setting it up much much earlier via a
simpler mechanism.
Graeme Russ Dec. 14, 2012, 11:16 p.m. UTC | #25
Hi Wolfgang,

On 15/12/12 09:26, Wolfgang Denk wrote:
> Dear Tom Rini,
> 
> In message <50CB8ED1.7020503@ti.com> you wrote:
>>
>> The other part is, take a look at the Allwinner thread from a week or
>> so ago.  We really need to define how we want early board specific
>> data to come in because if we start saying we'll accept per-SoC
>> solutions we'll be drowning in them in short time.  We want to say
>> here's our preferred way to pass this information in.
> 
> ACK!
> 
> And we already have a well-defined way to do this, which is the device
> tree.  So any attempts to implement something different should be
> reviewed very carefully.

I'm not sure I 100% get this, but from what I understand, the SoC (or maybe
even some EEPROM on a particular board family) may contain device
enumeration data in some vendor specific format (i.e. not in a device tree
compatible format).

The way I see it, there is no way that U-Boot can dictate to SoC and board
vendors and say that data must be stored in DT format. So shouldn't U-Boot
instead implement a board/SoC specific translation layer which converts
this custom data into DT format (maybe in SPL if possible)?

But the other problem is if this data includes console specific information
(UART configuration). We are left blind until the DT functions become
available. So maybe we need some small standard interface to allow the
console to be configured early. But we need to prevent this from being
abused (i.e. being used to do all kinds of hardware setting from the raw
data and bypassing DT)

Am I understanding this right?

Regards,

Graeme
Wolfgang Denk Dec. 15, 2012, 12:32 a.m. UTC | #26
Dear Graeme Russ,

In message <50CBB346.30208@gmail.com> you wrote:
> 
> > And we already have a well-defined way to do this, which is the device
> > tree.  So any attempts to implement something different should be
> > reviewed very carefully.
> 
> I'm not sure I 100% get this, but from what I understand, the SoC (or maybe
> even some EEPROM on a particular board family) may contain device
> enumeration data in some vendor specific format (i.e. not in a device tree
> compatible format).

Yes, this may, and will, happen.  And we will have to support it.  The
question is, how to do that.  I definitely do not want to see any
uncontrolled growth of more and more such board or SoC specific code.

> The way I see it, there is no way that U-Boot can dictate to SoC and board
> vendors and say that data must be stored in DT format. So shouldn't U-Boot

We cannot dictate, but we can encourage and discourage such decisions.
If we communicate a clear position, we may even prevent ugly things
from happening.

> instead implement a board/SoC specific translation layer which converts
> this custom data into DT format (maybe in SPL if possible)?

Do you want to see each board grow it's own code to do that?  Because
this is the extreme that could result from such a decision, and I
seriously dislike any such thought.  Which is why I'm questioning the
general approach when I see it first.

> But the other problem is if this data includes console specific information
> (UART configuration). We are left blind until the DT functions become
> available. So maybe we need some small standard interface to allow the
> console to be configured early. But we need to prevent this from being
> abused (i.e. being used to do all kinds of hardware setting from the raw
> data and bypassing DT)

Why do we have to support such dynamic hardware configuration for a
very basic thing as the console port at all?

If the hardware designers cannot fix their minds and use a fixed
console port, they should be willing to suffer fromthe penalty that
they will have to use board specific board configurations that match
the actual consoles settings.  Why should we bend and do ugly stuff?
Just because software is so much easier to change than hardware?
I'm not going to buy this argument.

Best regards,

Wolfgang Denk
Graeme Russ Dec. 15, 2012, 1:32 a.m. UTC | #27
Hi Wolfgang,

On 15/12/12 11:32, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <50CBB346.30208@gmail.com> you wrote:
>>
>>> And we already have a well-defined way to do this, which is the device
>>> tree.  So any attempts to implement something different should be
>>> reviewed very carefully.
>>
>> I'm not sure I 100% get this, but from what I understand, the SoC (or maybe
>> even some EEPROM on a particular board family) may contain device
>> enumeration data in some vendor specific format (i.e. not in a device tree
>> compatible format).
> 
> Yes, this may, and will, happen.  And we will have to support it.  The
> question is, how to do that.  I definitely do not want to see any
> uncontrolled growth of more and more such board or SoC specific code.
> 
>> The way I see it, there is no way that U-Boot can dictate to SoC and board
>> vendors and say that data must be stored in DT format. So shouldn't U-Boot
> 
> We cannot dictate, but we can encourage and discourage such decisions.
> If we communicate a clear position, we may even prevent ugly things
> from happening.

Understood, but in the end, board and SoC vendors will do what is most
expedient for them, and that may well be a raw binary data format buried in
some reserved ROM area (either on-time-writable or EEPROM)

>> instead implement a board/SoC specific translation layer which converts
>> this custom data into DT format (maybe in SPL if possible)?
> 
> Do you want to see each board grow it's own code to do that?  Because
> this is the extreme that could result from such a decision, and I
> seriously dislike any such thought.  Which is why I'm questioning the
> general approach when I see it first.

Well if the SoC or board (but more likely SoC) has already defined the data
structure, you a bit stuck. I fully agree that board developers that choose
U-Boot as their primary bootloader should be following U-Boot conventions.

>> But the other problem is if this data includes console specific information
>> (UART configuration). We are left blind until the DT functions become
>> available. So maybe we need some small standard interface to allow the
>> console to be configured early. But we need to prevent this from being
>> abused (i.e. being used to do all kinds of hardware setting from the raw
>> data and bypassing DT)
> 
> Why do we have to support such dynamic hardware configuration for a
> very basic thing as the console port at all?

You may not find as much in consumer devices (set-top-boxes, mobile phones,
tablets etc.) but you will in industrial devices.

I can give you an example - Remote Telemetry Units (RTUs). They usually
have a number of serial ports. The number of ports may vary based on the
sub-model. Some ports may be RS-232, some may be RS-485 or RS-422.
Depending on what additional devices you want to communicate with, you may
need to use the 'console/diag' port to connect to a real device. So what
you want to do is route console to another port (if available) or even
netconsole.

> If the hardware designers cannot fix their minds and use a fixed
> console port, they should be willing to suffer fromthe penalty that
> they will have to use board specific board configurations that match
> the actual consoles settings.  Why should we bend and do ugly stuff?
> Just because software is so much easier to change than hardware?
> I'm not going to buy this argument.

I do get your point of view. But I think a combination of storing the
dynamic console info in a DT format, the pre-console buffer and getting DT
available as early as possible can yield a 'non-cludgy' solution. For board
or SoC vendors who, for whatever reason, have implemented non-DT storage of
hardware enumeration data they will be stuck with the penalty of having to
translate that data into DT format before it can be parsed by U-Boot. Maybe
this could be done in SPL. Yes, it's a hack, but if it can't be worked
around, push it as low as possible and as far away from the U-Boot core as
possible

Regards,

Graeme
Wolfgang Denk Dec. 15, 2012, 7:30 a.m. UTC | #28
Dear Graeme Russ,

In message <50CBD313.60508@gmail.com> you wrote:
> 
> I can give you an example - Remote Telemetry Units (RTUs). They usually
> have a number of serial ports. The number of ports may vary based on the
> sub-model. Some ports may be RS-232, some may be RS-485 or RS-422.
> Depending on what additional devices you want to communicate with, you may
> need to use the 'console/diag' port to connect to a real device. So what
> you want to do is route console to another port (if available) or even
> netconsole.

Netconsole is always an option, and I think we also support switching
to other serial ports here and there (after relocation, that is).

But if you need console output before relocation (i. e. during
debugging), then I do not see why we cannot demand that the console
port is statically configured, and that you need corectly configured
images to have an early working console.

> I do get your point of view. But I think a combination of storing the
> dynamic console info in a DT format, the pre-console buffer and getting DT
> available as early as possible can yield a 'non-cludgy' solution. For board
> or SoC vendors who, for whatever reason, have implemented non-DT storage of
> hardware enumeration data they will be stuck with the penalty of having to
> translate that data into DT format before it can be parsed by U-Boot. Maybe
> this could be done in SPL. Yes, it's a hack, but if it can't be worked
> around, push it as low as possible and as far away from the U-Boot core as
> possible

I mostly agree here.  But I still fail to see why we havet os upport
this combination of early and dynamic - and only this is what causes
some issues.

Best regards,

Wolfgang Denk
Graeme Russ Dec. 15, 2012, 9:53 a.m. UTC | #29
Hi Wolfgang,

On Dec 15, 2012 6:30 PM, "Wolfgang Denk" <wd@denx.de> wrote:
>
> Dear Graeme Russ,
>
> In message <50CBD313.60508@gmail.com> you wrote:
> >
> > I can give you an example - Remote Telemetry Units (RTUs). They usually
> > have a number of serial ports. The number of ports may vary based on the
> > sub-model. Some ports may be RS-232, some may be RS-485 or RS-422.
> > Depending on what additional devices you want to communicate with, you
may
> > need to use the 'console/diag' port to connect to a real device. So what
> > you want to do is route console to another port (if available) or even
> > netconsole.
>
> Netconsole is always an option, and I think we also support switching
> to other serial ports here and there (after relocation, that is).
>
> But if you need console output before relocation (i. e. during
> debugging), then I do not see why we cannot demand that the console
> port is statically configured, and that you need corectly configured
> images to have an early working console.

I have seen situations where console output by the bootloader messes up
attached serial devices hence effectively dropping the serial port count by
one. Pre-console buffer helps a lot (no console output until we know where
to send it to). But that kills early debug.

>
> > I do get your point of view. But I think a combination of storing the
> > dynamic console info in a DT format, the pre-console buffer and getting
DT
> > available as early as possible can yield a 'non-cludgy' solution. For
board
> > or SoC vendors who, for whatever reason, have implemented non-DT
storage of
> > hardware enumeration data they will be stuck with the penalty of having
to
> > translate that data into DT format before it can be parsed by U-Boot.
Maybe
> > this could be done in SPL. Yes, it's a hack, but if it can't be worked
> > around, push it as low as possible and as far away from the U-Boot core
as
> > possible
>
> I mostly agree here.  But I still fail to see why we havet os upport
> this combination of early and dynamic - and only this is what causes
> some issues.

The situations I have seen can be resolved by pre-console buffer and
console configured in env. If the hardware is playing up,  a factory reset
to default console (without using pre-console buffer) suffices (the device
is on the bench with nothing attached). But then we are back to the board
specific/pre-DT problem.

Regards,

Graeme
Tom Rini Dec. 17, 2012, 9:04 p.m. UTC | #30
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/14/12 17:26, Wolfgang Denk wrote:
> Dear Tom Rini,
> 
> In message <50CB8ED1.7020503@ti.com> you wrote:
>> 
>> The other part is, take a look at the Allwinner thread from a
>> week or so ago.  We really need to define how we want early board
>> specific data to come in because if we start saying we'll accept
>> per-SoC solutions we'll be drowning in them in short time.  We
>> want to say here's our preferred way to pass this information
>> in.
> 
> ACK!
> 
> And we already have a well-defined way to do this, which is the
> device tree.  So any attempts to implement something different
> should be reviewed very carefully.

Exactly.  We are not yet making as much use of the device tree as we
can/should, which is where at least part of the hurdle is.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQz4jNAAoJENk4IS6UOR1Wx+0P+wQuv3IKoCrhVCC4GXbCcGmZ
2XnE8dum/MnlpHBKETnvAqDmr8TV2JQc/34Efm6d+6A67OYDDcu7cD1wt+/aTFuw
uYVZkWMEeGncdzZaHYTev9NK/YVikMbYezxF+PravzXaIblGpZGWw/xgBljemnBL
smjugTtWNOz6vDu45g2+dFIg92jDxuP4rznJrOEkF6QG03Ll8tI1WBAK+s079vmk
gkeTsTQnlOMsEtK++K8kNPoz5EK6JpPfAlmJKhv8P/Msfs0TR428JPtv/G9zKJkw
2VBk6Ru82xn7l7ygKSQqwiMRWygbsYzb1AX3hab+KjmsuCU+2UUZGYQMqv2aqY4U
4Uw1Pw21Q3eQNmHt16AIyQqB9PF1Byw11TmkjZrrm78Xbr/euCc9FtZoU6yB5y/T
zpSSA/kgqoY0UTyHhMEpdDYXrmeScRBrs7MSHxTTA4gS92Ng0bmlPtkJZUG8iWTR
hqKCRJj+2ymZzDh5e5nUYH60C4b2R2sW2eGJDEMmmB2yoGI2Jj6O3x4PAy25wvd1
dK2p03rYVgeFIw+wFeC7FD3ssidUq+l8Z9fDm2o1jQWmH3bJlC3zbftq39DJcTW0
PsRCk/a46oqDJuhUU1tbs5osgw220+1up3Xw/WldJ65wtwizsByHCUQ0aS+Y8qzG
Fy29qoQxjn5u/C/bjfwn
=CQyA
-----END PGP SIGNATURE-----
Tom Rini Dec. 17, 2012, 9:09 p.m. UTC | #31
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/14/12 17:45, Stephen Warren wrote:
> On 12/14/2012 03:22 PM, Simon Glass wrote:
>> Hi Stephen,
> ...
>> Perhaps I can make the point another way. Assuming that the SOC
>> in question is ARM-based and has Linux support it either supports
>> FDT now or presumably will fairly soon.
> 
> Sure, but I'm *explicitly* avoiding relying on DT for this,
> because there are plenty of things that happen before DT can or
> should be touched that might warrant serial port output.

Also totally true and valid.  But what I'd like to see is:
parse_odmdata(void *ptr) {
  if (ptr->console)
    device_tree_register_uart(... fake it ...);
}

In other words, if we add the register a port call now, there's a
history of adding just one more thing, by someone else (say am335x and
our EVMs that differ on where primary UART is, and only need a little
different logic to say 'oh! this one.') and making a mess of things.
Once we deal with device trees in some manner, then we can just fake
their existence at this point and pass in the console information from
ODMDATA.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQz4n3AAoJENk4IS6UOR1WrqoP/0glYuxrrrn9hgZa4WOhKhCd
K14da+7svtAdZLEFNVzbBb67sMsK9f0BH3PiesIxDBU2wAj7CgAZMpkedyAhwPjY
/oX8ywaWouckSrtVe1k4FTT7yPJOW6WlljaOFypHvt5VWRfhKJ6kdlMlkAiPPMEY
ng8cE1YKcbZZD9RsxhMpW6+OGVN9iRcFDSDW3EvPpvQaIl5hp3LYcKnQvNi7I3kE
edGIdzakPjxtJTVv8KiPuIOhYtYlTUvgcaeLgqbm9g/57z01Cz+SrAAaHXR8qxOv
pwwR53owgh6Cb033QEClP9mDGrd9QOXs9mkZm/GxhTFI4bWS3qJXvh2I73NUVL3Y
OrN0XfoLX5RZzkk/oYUV46OsI6oxPOImH4KEDFm2ST/twni813DIEtPGcotrWZm4
oSKYxONOvofaRalcwH+P89Iq8zLWgjglWwNoOCgItWjhwvY75q/RCQPB2vOkSFtb
/Vt4Do5HeqfmR6XPHHvP0dUCZ2IS0/Xh7w+OO44HakDLESiWfnfr/dWSmTiiqBA+
wfWDMhE+pK/fnz3CG4eAOEUQ7Y7Uh3jGo42WpthpCuY0ZltnhyEHwmPJ6MuwAqbs
EDUXSbNnpLOC9iIRKnT6J0hkhONp/ip+1Mad2hsVluAMmqYBTdM7rKi/kuV0HdYl
LPh6xToy9xyUgkllZZbw
=nUfP
-----END PGP SIGNATURE-----
Tom Rini Dec. 17, 2012, 9:09 p.m. UTC | #32
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/14/12 17:22, Simon Glass wrote:

[snip]
> Perhaps I can make the point another way. Assuming that the SOC in 
> question is ARM-based and has Linux support it either supports FDT
> now or presumably will fairly soon. We found that some of the
> things we want to know about at the low level are hardware
> properties that are already sit in a node in the FDT. For example
> the memory controller may have information about the memory type
> attached to it.
> 
> Given this, my suggestion is that this hardware information be kept
> in one place (FDT) where possible, even if it unfortunately
> temporarily needs to be translated into some simpler format as part
> of the SPL build for efficiency reasons.

Exactly.  The fewer places we have to write down the details the better.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQz4n7AAoJENk4IS6UOR1WkQYP/RcnxKDaMLKdT9j+rQ+FEKtS
s0vl4v8OFRMetmVzPqeMndHMk4IIbMFOgW58tiYnVZm0iJNCZDr/WyIiU0XRLmkl
jWPl8qszw/9l5EkgOWOmPK4r9fBhv0QLGonyBMU3chUI6/uVH/fZXMm7m+f3rwS6
9JSiA+E+VAhSwFMgNeLyk4H85xyIDu3tH4q8hqWQ2c223sgTuZPD2/f3EUQ+KyrX
3Rk133/2LLeJHkXqwkJhYkhNgvvooHy/5SeUuv4LRsLDV3xqoo7mpJ1NREtinthN
PnoiJP3wTDUffSe38kk0VKjQ//xaYuM28PfrTpZQ0O82GPOlSko4M0rpekWpFdiP
Mvqf2LK01QqbxNF/dllwlv2AeU2hR2m2j8cSsT+Ugo1E9tLl/P+/ziXR2zMkbDe/
zJCzJmG3L9p7EtSzsReguUPNonsimxNlpDIhOBGdiob6W9jfKELIYHWoWI3O8tJr
GhGmujoo/zmEa6berP2hDB0qs+lHXU3fk57wc+aIeOXKfTadqRoWhqSy4FeYlDHX
yYyEOkOntmC61e3lJR01wmLUsPkHjLVRkfT3xR9ivTIhkAoQaSRJC1WoEPzlM7qU
VTwnKkE9jlDl7n2ASXKa0A+r2hBaagV8i6rJceR+2z4CVrPOl1tJUFARAmRbxZWV
WfxpfY0IMEbCpQ46Jhif
=3dA/
-----END PGP SIGNATURE-----
Stephen Warren Dec. 17, 2012, 10:24 p.m. UTC | #33
On 12/17/2012 02:09 PM, Tom Rini wrote:
> On 12/14/12 17:45, Stephen Warren wrote:
>> On 12/14/2012 03:22 PM, Simon Glass wrote:
>>> Hi Stephen,
>> ...
>>> Perhaps I can make the point another way. Assuming that the SOC
>>> in question is ARM-based and has Linux support it either supports
>>> FDT now or presumably will fairly soon.
> 
>> Sure, but I'm *explicitly* avoiding relying on DT for this,
>> because there are plenty of things that happen before DT can or
>> should be touched that might warrant serial port output.
> 
> Also totally true and valid.  But what I'd like to see is:
> parse_odmdata(void *ptr) {
>   if (ptr->console)
>     device_tree_register_uart(... fake it ...);
> }

Sorry, I'm having a hard time parsing:

> In other words, if we add the register a port call now, there's a

By "register a port call", do you mean the device_tree_register_uart()
you're proposing above, or the NS16550_set_dynamic_address() I'd
implemented in my patch?

Below, you appear to be pointing out problems with adding the "register
a port call now" which seems like you're arguing against your own example?

> history of adding just one more thing, by someone else (say am335x and
> our EVMs that differ on where primary UART is, and only need a little
> different logic to say 'oh! this one.') and making a mess of things.

> Once we deal with device trees in some manner, then we can just fake
> their existence at this point and pass in the console information from
> ODMDATA.

There are many ways besides device tree to enumerate hardware. For
example, consider PCI or USB (albeit USB isn't memory mapped). I don't
think we should tie any new U-Boot dynamic device registration API to
device tree, since that would seem to prevent (or imply against) usage
of that API with PCI for example.

Perhaps this is just bike-shedding over naming?

So, perhaps:

int device_register_uart(enum uart_type, u32 base_address)

So that all of the following cases could call that:

* DT parsing.
* PCI device enumeration.
* ODMDATA parsing.

(and where enum uart_type is some internal U-Boot identifier for the
UART drivers it supports)

Presumably the implementation would validate if the (uart_type,
base_address) combination had been seen already, and then do nothing.
that would allow for ODMDATA parsing to set up the UART, and then DT
parsing to avoid any special cases to skip handling DT nodes for serial
devices that were already registered.
Wolfgang Denk Dec. 17, 2012, 10:37 p.m. UTC | #34
Dear Stephen Warren,

In message <50CF9BAA.3050504@wwwdotorg.org> you wrote:
>
> There are many ways besides device tree to enumerate hardware. For
> example, consider PCI or USB (albeit USB isn't memory mapped). I don't

Yes, there are.  But your console port cannot be compred against
dynamically populated and scannable bus interfaces like USB or PCI,
and I think you are aware of that.

> think we should tie any new U-Boot dynamic device registration API to
> device tree, since that would seem to prevent (or imply against) usage
> of that API with PCI for example.

Not any dynamic device registration.  But here, it actually AIN'T
dynamic - it is fully static, just board dependent.

> Perhaps this is just bike-shedding over naming?

No.

> So that all of the following cases could call that:
> 
> * DT parsing.
> * PCI device enumeration.
> * ODMDATA parsing.

NAK.  This is totally wrong.  ODMDATA is just a different
representation of information that could as well be encoded in a DT.
PCI or USB bus scanning gives information which cannot be encoded in a
DT passed to U-Boot (at least not unless you dynamically generate that
DT using some other softeware performing such a bus scan).

Best regards,

Wolfgang Denk
Stephen Warren Dec. 17, 2012, 10:58 p.m. UTC | #35
On 12/17/2012 03:37 PM, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message <50CF9BAA.3050504@wwwdotorg.org> you wrote:
>>
>> There are many ways besides device tree to enumerate hardware. For
>> example, consider PCI or USB (albeit USB isn't memory mapped). I don't
> 
> Yes, there are.  But your console port cannot be compred against
> dynamically populated and scannable bus interfaces like USB or PCI,
> and I think you are aware of that.

I honestly don't know why you couldn't have a PCI-based console UART.

>> think we should tie any new U-Boot dynamic device registration API to
>> device tree, since that would seem to prevent (or imply against) usage
>> of that API with PCI for example.
> 
> Not any dynamic device registration.  But here, it actually AIN'T
> dynamic - it is fully static, just board dependent.

If you want to run the same U-Boot binary on multiple different boards,
then that does make the UART selection dynamic. There's no conceptual
difference between dynamic information coming from a DT passed to U-Boot
at runtime, SoC-defined enumeration/selection mechanisms such as Tegra's
ODMDATA, or scanning a PCI bus.

Or, is U-Boot going to ban addressing TI's case where the UART selection
is stored in an I2C EEPROM that can be read at run-time, since instead
during flashing that information could be extracted and hard-coded into
the board's device tree instead?
Wolfgang Denk Dec. 18, 2012, 6:39 a.m. UTC | #36
Dear Stephen,

In message <50CFA394.40901@wwwdotorg.org> you wrote:
>
> > Yes, there are.  But your console port cannot be compred against
> > dynamically populated and scannable bus interfaces like USB or PCI,
> > and I think you are aware of that.
> 
> I honestly don't know why you couldn't have a PCI-based console UART.

This is actually another question.

You cannot compare a statically configured UART port (where all
configuration information you need is the index into the table of
possible UARTs) to a dynamic bus scan where you cannot know in advance
whether you detect any devices at all, or how many, or which types.

You will have hard times using a PCI or USB based console port
for early console output because the majority of the PCI and USB
related code will only be runnable after relocation.

> > Not any dynamic device registration.  But here, it actually AIN'T
> > dynamic - it is fully static, just board dependent.
> 
> If you want to run the same U-Boot binary on multiple different boards,
> then that does make the UART selection dynamic. There's no conceptual
> difference between dynamic information coming from a DT passed to U-Boot
> at runtime, SoC-defined enumeration/selection mechanisms such as Tegra's
> ODMDATA, or scanning a PCI bus.

Maybe there is no conceptual difference - but only if you chose to
look at things at a relatively high abstraction level.

Implementation-wise the difference it between passing a single integer
variable (the UART index) and performing a dynamic bus scan, detecting
number and type of devices and selecting appropriate drivers.

> Or, is U-Boot going to ban addressing TI's case where the UART selection
> is stored in an I2C EEPROM that can be read at run-time, since instead
> during flashing that information could be extracted and hard-coded into
> the board's device tree instead?

Here again we are talking about a UART, which is statically confgured
per board, and we just need this specific piece of board information.

And yes, this piece of information should be encoded in the device
tree.  Note that the "hard-coded" you mantion has never been made a
requirement.  For simplicity of design it might make sense to actually
use the DT format for such information right away.  But as your own
example shows, not all chip / board vendors do that.  Simon and Tom
already commented on what to do in such situations.

It seems Simon, Tom and me mostly agree on what to do.


Best regards,

Wolfgang Denk
Stephen Warren Dec. 18, 2012, 4:37 p.m. UTC | #37
On 12/17/2012 11:39 PM, Wolfgang Denk wrote:
> Dear Stephen,
> 
> In message <50CFA394.40901@wwwdotorg.org> you wrote:
>>
>>> Yes, there are.  But your console port cannot be compred against
>>> dynamically populated and scannable bus interfaces like USB or PCI,
>>> and I think you are aware of that.
>>
>> I honestly don't know why you couldn't have a PCI-based console UART.
> 
> This is actually another question.
> 
> You cannot compare a statically configured UART port (where all
> configuration information you need is the index into the table of
> possible UARTs)

That's not the only piece of information that is required. At least on
Tegra (and I imagine on most SoCs with a pinmux) you need to fully
describe the UART-related pinmux programming, so that the UART signals
actually get routed out of the SoC.

> to a dynamic bus scan where you cannot know in advance
> whether you detect any devices at all, or how many, or which types.

A board could have a PCI UART soldered onto the board, and hence be
physically static and known ahead of time, yet still require (at least
part of) the PCI bus to be correctly probed and programmed.

> It seems Simon, Tom and me mostly agree on what to do.

To be honest, that's not remotely the impression I get.
Simon Glass Dec. 18, 2012, 7:15 p.m. UTC | #38
Hi Stephen,

On Tue, Dec 18, 2012 at 8:37 AM, Stephen Warren <swarren@wwwdotorg.org>wrote:

> On 12/17/2012 11:39 PM, Wolfgang Denk wrote:
> > Dear Stephen,
> >
> > In message <50CFA394.40901@wwwdotorg.org> you wrote:
> >>
> >>> Yes, there are.  But your console port cannot be compred against
> >>> dynamically populated and scannable bus interfaces like USB or PCI,
> >>> and I think you are aware of that.
> >>
> >> I honestly don't know why you couldn't have a PCI-based console UART.
> >
> > This is actually another question.
> >
> > You cannot compare a statically configured UART port (where all
> > configuration information you need is the index into the table of
> > possible UARTs)
>
> That's not the only piece of information that is required. At least on
> Tegra (and I imagine on most SoCs with a pinmux) you need to fully
> describe the UART-related pinmux programming, so that the UART signals
> actually get routed out of the SoC.
>

Is that information in the ODM data also? I thought ODM data was just a
32-bit word with a UART number in it. It seemed equivalent to the console
alias to me.


>
> > to a dynamic bus scan where you cannot know in advance
> > whether you detect any devices at all, or how many, or which types.
>
> A board could have a PCI UART soldered onto the board, and hence be
> physically static and known ahead of time, yet still require (at least
> part of) the PCI bus to be correctly probed and programmed.


> > It seems Simon, Tom and me mostly agree on what to do.
>
> To be honest, that's not remotely the impression I get.
>
>
Well anyway, to your problem, we did in fact have the Chromium tree working
with an FDT. This is available very early (before console_init_f()) so it
was no trouble to get the information from there. Two of the devices we
booting on had different console UARTs, and we just changed the alias
property to deal with that: serial = "/serial@xxx", or serial = "/serial@yyy".
We didn't use ODM data to select the console (perhaps because we didn't
know about this feature).

I believe with Tegra the ODM data is more useful, so there is an overlap
between some of the fields in ODM data (and perhaps BCT) and FDT. That's
why I suggested something like a setting in the FDT to say that the console
UART number comes from ODM data. I'm not sure how to specify that with an
alias.

Regards,
Simon
diff mbox

Patch

diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c
index fc01a3c..fc8253b 100644
--- a/drivers/serial/serial_ns16550.c
+++ b/drivers/serial/serial_ns16550.c
@@ -166,6 +166,11 @@  static int calc_divisor (NS16550_t port)
 		(MODE_X_DIV * gd->baudrate);
 }
 
+void NS16550_set_dynamic_address(int port, NS16550_t com_port)
+{
+	PORT = com_port;
+}
+
 void
 _serial_putc(const char c,const int port)
 {
diff --git a/include/ns16550.h b/include/ns16550.h
index 51cb5b4..6d7483f 100644
--- a/include/ns16550.h
+++ b/include/ns16550.h
@@ -171,6 +171,7 @@  typedef struct NS16550 *NS16550_t;
 /* useful defaults for LCR */
 #define UART_LCR_8N1	0x03
 
+void NS16550_set_dynamic_address(int port, NS16550_t com_port);
 void NS16550_init(NS16550_t com_port, int baud_divisor);
 void NS16550_putc(NS16550_t com_port, char c);
 char NS16550_getc(NS16550_t com_port);