Re: Flash subsystem update

[Bart Veer <bartv at ecoscentric dot com>]

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

    Jifl> Then how can you retain the semantics of setting the printf
    Jifl> function in cyg_flash_init if the API for doing so is
    Jifl> deprecated? What is the point of calling a new API official,
    Jifl> telling people to start coding to it, and deprecating it
    Jifl> immediately?

It is not a new API. It has been around for some years now. If it was
a completely new API I would not worry about it.

    >> It has also broken API compatibility for every application that
    >> has used the V2 flash API since that was merged into the
    >> anoncvs trunk.

    Jifl> That is measured in days, and the sole object of the import
    Jifl> was in order to create eCos 3.0. Calling that tiny window
    Jifl> significant for the purposes of backward compatibility
    Jifl> beggars belief. Are we to be compatible with all
    Jifl> intermediate states of the repository?

Days? The flash V2 branch was merged into the trunk over three months
ago, back in November. That started the discussions which led to this
flame war.    
    
    >> It has also broken API compatibility for every eCosPro release
    >> for the last four years or so.

    Jifl> I'm only wearing a maintainer hat here. Please do the same.

If the change really gained us anything I would not object. I do not
believe it gains us anything. It does cause an incompatibility problem
for every eCosPro customer over the last 4+ years who wants to upgrade
to a 3.0 sourcebase, as well as some unknown number of anoncvs users.

Now, much of the 3.0 work to date has been done on eCosCentric company
time. Some of the remaining 3.0 work also depends on eCosCentric
resources, e.g. access to the company's test farm. I think the company
can quite reasonably feel narked at an unnecessary last-minute
incompatible change that is going to make life more difficult for some
of its customers. I do not expect that to affect the 3.0 effort, this
is not intended as any kind of threat. However, I do believe it will
make it even more difficult in future to get any company resources
allocated to anoncvs work. That is a valid concern from a maintainer's
perspective.

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

    >> No, you still don't understand. My original proposal NEVER breaks
    >> compatibility. Not now, because it involved zero changes to the API.
    >> Not in future, because there will always be a cyg_flash_init() with
    >> the exact same interface as it does now. It will become deprecated but
    >> it will still exist.

    Jifl> Deprecating an API function means requiring API users to
    Jifl> stop using it. Are you requiring them to change their
    Jifl> application, freshly written to this new API, or not?

No. The correct solution, as I have said before is:

a) in flash.h, mark the function as 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")));

b) in flash.c, leave in place an implementation along the lines of:

    __externC int
    cyg_flash_init(cyg_flash_printf* pf)
    {
        cyg_flash_set_global_printf(pf);
	return 0;
    }

(cyg_flash_set_global_printf() and cyg_flash_set_printf() can be added
either now or at the point that cyg_flash_init() is marked deprecated.
There is little point in adding it now. The harm was that it involved
yet another change before the release branch could be cut, but that
has been superseded by events.)

Once cyg_flash_init() has been marked as deprecated, application
developers would get a new compiler warning. They can ignore the
warning and everything would continue to work. There is no
incompatibility.

Or they can investigate further. When they read the documentation they
would discover that flash subsystem initialization now happens
differently from before, and earlier. They may choose to remove the
cyg_flash_init() call completely. Or they may choose to replace it
with a call to cyg_flash_set_global_printf(). Or they may choose some
other action. Some users may realize that having the flash initialized
earlier means that they can now clean up and simplify some other bits
of their code. If they are using strange hardware they may realize
that having the flash initialized earlier could cause problems, and
that it would be a good idea to move some init code from the
application to the platform HAL.

The key point is that users get all the information they need to make
an intelligent decision on how to proceed. This contrasts with your
approach of defining cyg_flash_init() to be a no-op. That means that
application code will continue to have a cyg_flash_init() invocation
indefinitely, with no warnings and no indications of any kind that
this no longer has anything to do with flash initialization.

(Digression: there is actually a complication here. For full
compatibility cyg_flash_init() should not return 0, it should return
an error code indicating whether or not the flash subsystem has been
fully initialized. That "fully initialized" is not clearly defined.
For example, if a board has both NOR flash and dataflash, and the NOR
flash initializes correctly, but the dataflash fails to initialize
because it is not properly plugged into the socket, should
cyg_flash_init() return 0 or an error code? I don't know yet what the
correct answer is. My preferred solution would be for a deprecated
cyg_flash_init() to always return 0, but for the documentation to
describe this problem and suggest that application code uses the
get_info calls to check what actually happened during initialization.)


Unfortunately I am away this weekend and don't know yet whether I'll
have internet access. So I may not to be able to respond to any
further postings until late Monday or possibly Tuesday.

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