[PATCH] add option to override redboot abort script check

[Gary Thomas]

Jose Vasconcellos wrote:
> Gary Thomas wrote:
>> Andrew Lunn wrote:
>>> On Mon, Aug 25, 2008 at 10:02:37PM -0400, Jose Vasconcellos wrote:
>>>> This patch factors out the test for ^C that is used to bypass the  
>>>> start-up script.
>>>> By creating the function redboot_abort_script and declaring weak, 
>>>> it's  possible
>>>> to easily replace it for custom needs.
>>>>
>>>> In my case, there's no easily accessible serial port so get to the  
>>>> redboot prompt
>>>> telnet is used. However, if there's a boot script, there needs to be 
>>>> a  way for the
>>>> user to abort it in an easy manner. So, in my platform specific 
>>>> code, the
>>>> redboot_abort_script is replaced by code to check for to see button 
>>>> if a  button
>>>> is pressed.
>>>
>>> What i don't like about this is that it removes the normal ^C
>>> handling. For you that is probably O.K. however in other situations,
>>> it would be better to add additional mechanism, rather than replace
>>> mechanisms. So in the field you can use a jumper on a GPIO, but the
>>> developers desk which does have a serial port, maybe via a bed of
>>> nails, can continue to use ^C etc.
>>
>> Indeed, the initial discussion did not mention using weak functions
>> (although I suspected this).  I'm also not keen on a wholesale 
>> replacement.
>>
>>> Redboot makes a lot of use of HAL tables, eg for adding new commands,
>>> idle handlers, etc. What i would suggest is adding a new table for
>>> script abort functions. The ^C handler would be the first entry in the
>>> table. HALs or other packages could then add more entries, eg read a
>>> GPIO line etc. If any of these functions return true, the script is
>>> aborted. 
>>
>> Another possibility would be to use a HAL defined macro, in the
>> same spirit as HAL_MEM_REAL_REGION_TOP which is used in that same
>> file (main.c)
>>
> Thank you for your feedback.
> 
> The patch simply makes a function out of an internal loop; by adding a weak
> reference it may be overridden for special needs. The change involved is
> quite minimal, and it doesn't have any impact if not used. Andrew's and 
> Gary's
> suggestions may be more general but it's not clear that this is necessary.
> After all, redboot has been around a very long time and this hasn't been 
> brought
> out before.

Sorry, but your comment is antithetic to the eCos/RedBoot philosophy.
Even if this code *has* been around for years, unchanged, it behooves us
to make changes which are both flexible and follow the existing model.

The attached patch affects the same change you'd like, with the benefit of
retaining existing functionality (e.g. being able to connect and abort
via ^C) while giving your target platform the ability to handle the abort
button.  Simply define the macro HAL_REDBOOT_ABORT_FUNCTION in your HAL
to return true on abort and you're done.


-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: diffs
URL: <http://sourceware.org/pipermail/ecos-patches/attachments/20080826/1d2d8692/attachment.ksh>