Re: Flash subsystem update

[Bart Veer <bartv at ecoscentric dot com>]

>>>>> "Jifl" == Jonathan Larmour <jifl@eCosCentric.com> writes:

    Jifl> Bart Veer wrote:
    >> 
    >> By far the most sensible thing to do right now is to leave
    >> things exactly as they are. The functionality change is of
    >> interest to very few if any users. It may be interesting to us
    >> in the long term if we want to statically initialize more
    >> subsystems, but that does not require anything to be done
    >> today. There are no significant API compatibility implications:
    >> all of the functions that exist today will continue to exist;
    >> one of them will become pretty much irrelevant and users will
    >> be warned that they can safely remove it and save a few bytes,
    >> but it will continue to exist.

    Jifl> I still think you haven't got the point of what is being
    Jifl> discussed. As I've mentioned a few times now, your concerns
    Jifl> are to do with initialising by C++ constructor. While I
    Jifl> think the risks of this change are rather theoretical, the
    Jifl> effects on setting the printf function are not, and that's
    Jifl> the source of API breakage when cyg_flash_init disappears in
    Jifl> future.

Uh, excuse me. If you look again at the email I sent last Friday, I
clearly discussed both the constructor priority issue and the API
issue. I concluded that keeping things exactly as they were did not
involve any API compatibility issues.
    
    Jifl> Since you aren't keen to work on 3.0 any more or discuss
    Jifl> further, I'm checking in a patch which updates
    Jifl> cyg_flash_init to remove its argument (I toyed with keeping
    Jifl> the argument and deprecating it but that seemed the worst of
    Jifl> all worlds by hiding the change for any existing API users).
    Jifl> And the main benefit of doing so is that later on we can do:
    Jifl> #define cyg_flash_init() CYG_EMPTY_STATEMENT and there's no
    Jifl> overhead, and no API breakage.

    Jifl> The patch adds the functions proposed by your good self on
    Jifl> 18th Nov and updates everything accordingly, including docs.
    Jifl> See ecos-patches.

This is exactly the wrong thing to do, as I figured out after 18 Nov
when I actually started making changes along these lines. As I did the
work I realized that the changes were both unnecessary and harmful.

The correct thing to do in a future release is to keep
cyg_flash_init() exactly as it was before your check-in, but marked
deprecated.

  __externC int cyg_flash_init( cyg_flash_printf *pf ) __attribute__ ((deprecated));

or even better, if gcc were to support it:

  __externC int cyg_flash_init( cyg_flash_printf *pf )
      __attribute__ ((deprecated ("explicit calls to cyg_flash_init() are no longer required")));

cyg_flash_init() would continue to exist indefinitely and would
continue to take a printf function pointer. There is no API breakage.
However people who bother to look at the compiler warnings would
figure out that they could save a few bytes of code.

Your change to cyg_flash_init() has broken API compatibility for every
flash-using application that has used the V2 flash branch since it was
created. It has also broken API compatibility for every application
that has used the V2 flash API since that was merged into the anoncvs
trunk. It has also broken API compatibility for every eCosPro release
for the last four years or so. And it does not gain us anything. That
is why I abandoned my work along these lines.

I want to see this change reverted. You can keep the
cyg_flash_set_printf() and cyg_flash_set_global_printf() functions if
you really want to. They don't add any significant value, but they are
also harmless. However cyg_flash_init() should go back the way it was.

Bart
    
-- 
Bart Veer                                   eCos Configuration Architect
eCosCentric Limited    The eCos experts      http://www.ecoscentric.com/
Barnwell House, Barnwell Drive, Cambridge, UK.      Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
Besuchen Sie uns vom 3.-5.03.09 auf der Embedded World 2009, Stand 11-300
Visit us at Embedded World 2009, Nürnberg, Germany, 3-5 Mar, Stand 11-300