This is the mail archive of the
ecos-discuss@sourceware.org
mailing list for the eCos project.
Re: Re: [PATCH] redboot - add exec option to set board revision
- From: Gary Thomas <gary at mlbassoc dot com>
- To: Jose Vasconcellos <jvasco at verizon dot net>
- Cc: eCos Discussion <ecos-discuss at ecos dot sourceware dot org>
- Date: Thu, 13 May 2010 11:15:11 -0600
- Subject: Re: [ECOS] Re: [PATCH] redboot - add exec option to set board revision
- References: <AANLkTinUF21Am97V_-u8H3WhXUcJBREkW1SmjBURQvv9@mail.gmail.com> <4BEC1102.9010906@mlbassoc.com> <AANLkTinp-6tidUnqxg_vXie5Kqw5onA_Es7oEnh5DEMg@mail.gmail.com> <4BEC2132.6060603@mlbassoc.com> <AANLkTilH7tnHKVptetNQAu7tdfgRIDQ3ztGqkxuTAXdF@mail.gmail.com>
On 05/13/2010 11:06 AM, Jose Vasconcellos wrote:
On Thu, May 13, 2010 at 10:56 AM, Gary Thomas<gary@mlbassoc.com> wrote:
Please keep your replies on the list so that all may benefit.
On 05/13/2010 09:33 AM, Jose Vasconcellos wrote:
This allows setting the ATAG_REVISION so it gets passed to
the kernel. This is useful for systems that share the same id
but have some small hardware differences between board
revisions.
On Thu, May 13, 2010 at 9:47 AM, Gary Thomas<gary@mlbassoc.com> wrote:
On 05/13/2010 08:02 AM, Jose Vasconcellos wrote:
This patch adds an exec option (-v) to set the board revision
via the ATAG_REVISION tag.
What's it used for? (Why do you need this tag)
I think it would be better in this case to have this provided
automatically. Whatever value you would be passing seems to
be specific to your target, so just define it as a platform
defined value and make the RedBoot exec code pass it.
Define it wherever you've defined HAL_PLATFORM_MACHINE_TYPE, e.g.
#define HAL_PLATFORM_REVISION 123
Then in the code
#if defined(HAL_PLATFORM_REVISION)
...
#endif
Otherwise, you pollute the command with options which do nothing
on most platforms and will just be confusing to users.
--
------------------------------------------------------------
Gary Thomas | Consulting for the
MLB Associates | Embedded world
------------------------------------------------------------
It would be better to autodetect the hardware revision and pass that
when loading the kernel. It would have to be something that's detected
in something like plf_hardware_init. I'm not sure how to pass that info
to do_exec.
Similarly to what I said before, just make 'HAL_PLATFORM_REVISION'
be defined as a function that returns the appropriate value, or even
the name of the variable that holds it - your choice.
Note that my proposed change also fixed another bug. The opts array
is declared with a size of 7 but it should be 8 if the option
CYGHWR_REDBOOT_LINUX_EXEC_X_SWITCH is used; otherwise
it corrupts the stack.
Noted, thanks
--- a/packages/hal/arm/arch/current/src/redboot_linux_exec.c
+++ b/packages/hal/arm/arch/current/src/redboot_linux_exec.c
@@ -313,7 +313,7 @@
bool ramdisk_addr_set, ramdisk_size_set;
unsigned long base_addr, length;
unsigned long ramdisk_addr, ramdisk_size;
- struct option_info opts[7];
+ struct option_info opts[8];
char line[8];
char *cmd_line;
struct tag *params = (struct tag *)CYGHWR_REDBOOT_ARM_LINUX_TAGS_ADDRESS;
--
Please keep your replies on the mailing list(s) so that all
may benefit. Private support is available under contract
from various agents, including MLB Associates. Private
email to me without a contract will be ignored.
------------------------------------------------------------
Gary Thomas | Consulting for the
MLB Associates | Embedded world
------------------------------------------------------------
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss