From: Jonathan Larmour <jifl@jifvik.org>
To: Bart Veer <bartv@ecoscentric.com>
Cc: john@dallaway.org.uk, ecos-maintainers@ecos.sourceware.org
Subject: Re: Flash subsystem update
Date: Fri, 20 Feb 2009 21:22:00 -0000 [thread overview]
Message-ID: <499F1EC0.9030802@jifvik.org> (raw)
In-Reply-To: <pnab8hgtew.fsf@delenn.bartv.net>
Bart Veer wrote:
>>>>>>"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.
Hardly usably in flashv2-branch.
> >> 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.
Days, not years like the difference to 2.0. It was still the lead-up to
3.0. Otherwise we subvert the point of allowing ongoing development in a
CVS repository between stable releases.
> >> 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.
Bear in mind that one of the specific things we talked about in 3.0 was
that, being a point zero release, now was the time to get APIs right, and
far more extensive changes were initially proposed. Certainly I don't
think many eCos 2.0 apps will be compatible with 3.0.
Incidentally, I raised all this with you in mails in 2004 and 2005. Any
compatibility issues were avoidable back then...
> 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.
It's not just their code. We can't simplify some of our code either. We
could otherwise dispense with the CYG_FLASH_ERR_NOT_INIT checks and
associated static(s) for example, because eCosPro users (which seems to be
your main concern) could be affected.
> 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.
I guess it could be CYG_FAIL instead, and/or return an error code.
> (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.
This is of course another flaw of cyg_flash_init.
> 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.)
Indeed, an error would be returned from get_info if the device was never
successfully initialised.
> 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.
Sigh. In the interests of moving forwards, I am going to somewhat roll
back my changes. But I will retain the set_*printf functions, and add a
note in the documentation to indicate that an argument to cyg_flash_init
other than NULL is unsupported, and instead cyg_flash_set_global_printf
should always be used. In practice I will leave cyg_flash_init to still
accept and act on its argument so existing users are unaffected. This will
allow us to deprecate and then remove the function more easily at a later
date (a macro can check the argument against NULL, and if NULL is used
then constant folding will eliminate overhead).
Patch forthcoming on ecos-patches...
Jifl
--
------["The best things in life aren't things."]------ Opinions==mine
next prev parent reply other threads:[~2009-02-20 21:22 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-11 9:07 eCos 3.0 beta 1 punch list #2 John Dallaway
2009-02-13 19:56 ` Bart Veer
2009-02-17 1:29 ` Jonathan Larmour
2009-02-17 9:18 ` Jonathan Larmour
2009-02-17 9:34 ` Flash subsystem update [ was Re: eCos 3.0 beta 1 punch list #2 ] John Dallaway
2009-02-17 22:01 ` Flash subsystem update John Dallaway
2009-02-18 12:47 ` Bart Veer
2009-02-19 0:08 ` Jonathan Larmour
2009-02-19 11:50 ` Bart Veer
2009-02-19 14:40 ` John Dallaway
2009-02-19 15:13 ` Bart Veer
2009-02-19 20:53 ` Jonathan Larmour
2009-02-20 9:16 ` John Dallaway
2009-02-19 20:31 ` Jonathan Larmour
2009-02-20 12:55 ` Bart Veer
2009-02-20 21:22 ` Jonathan Larmour [this message]
2009-02-17 21:07 ` eCos 3.0 beta 1 punch list #2 Bart Veer
2009-02-17 23:10 ` Jonathan Larmour
2009-02-19 0:48 ` Jonathan Larmour
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=499F1EC0.9030802@jifvik.org \
--to=jifl@jifvik.org \
--cc=bartv@ecoscentric.com \
--cc=ecos-maintainers@ecos.sourceware.org \
--cc=john@dallaway.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).