[ECOS] Re: [PATCH] redboot - add exec option to set board revision

[Jose Vasconcellos <jvasco at verizon dot net>]

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.

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.

--- 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;

--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss