Re: Flash subsystem update

[Jonathan Larmour <jifl@eCosCentric.com>]

Bart Veer wrote:
>>>>>>"Jifl" == Jonathan Larmour <jifl@eCosCentric.com> writes:
> 
>     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.

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

> 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.
[snip]
> 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.

But as you yourself said, the V2 flash branch is a development branch, and 
rightly so. There has been no release from it, nor has it been 
release-worthy. Anyone working against it faced a large struggle because 
it was interdependent on the rest of the tree which was ancient. And as we 
know from eCosCentric work, a lot of the redboot stuff was broken anyway, 
and we spent a lot of time fixing much of it, which has only gotten 
integrated as part of the merge to trunk.

I have no intention of retaining compatibility with an obsolete -cum- work 
in progress tree. What would be the point of that.

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

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

> It has also broken API compatibility for every eCosPro release
> for the last four years or so.

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

The flash v2 API has never been released and there are no backward 
compatibility implications. eCos 3.0 is what cements the API. What I am 
concerned about is forward compatibility. I would rather have the API 
completely straight, including removal of cyg_flash_init. But since you 
are resistant to that, I have compromised with something which at least 
allows it to be #define'd away to nothing.

Before now there hasn't _been_ a cyg_flash_init function to be compatible 
with. And I've ensured that the legacy API's flash_init() DTRT.

Jifl
-- 
*See us at Embedded World 2009, Nürnberg, Germany, 3-5 Mar, Stand 11-300*
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["Si fractum non sit, noli id reficere"]------       Opinions==mine