From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21689 invoked by alias); 20 Feb 2009 21:22:02 -0000 Received: (qmail 21677 invoked by uid 22791); 20 Feb 2009 21:22:01 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from virtual.bogons.net (HELO virtual.bogons.net) (193.178.223.136) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 20 Feb 2009 21:21:15 +0000 Received: from jifvik.dyndns.org (jifvik.dyndns.org [85.158.45.40]) by virtual.bogons.net (8.10.2+Sun/8.11.2) with ESMTP id n1KLL1429404; Fri, 20 Feb 2009 21:21:02 GMT Received: from [172.31.1.2] (unknown [172.31.1.2]) by jifvik.dyndns.org (Postfix) with ESMTP id 95E443FEB; Fri, 20 Feb 2009 21:21:05 +0000 (GMT) Message-ID: <499F1EC0.9030802@jifvik.org> Date: Fri, 20 Feb 2009 21:22:00 -0000 From: Jonathan Larmour User-Agent: Mozilla Thunderbird 1.0.8-1.1.fc3.4.legacy (X11/20060515) MIME-Version: 1.0 To: Bart Veer Cc: john@dallaway.org.uk, ecos-maintainers@ecos.sourceware.org Subject: Re: Flash subsystem update References: <4992951D.10004@dallaway.org.uk> <499A12DD.4060609@eCosCentric.com> <499A849D.6030809@dallaway.org.uk> <499B33AE.6050608@dallaway.org.uk> <499CA2FD.2000504@eCosCentric.com> <499DC199.2030206@eCosCentric.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Mailing-List: contact ecos-maintainers-help@ecos.sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: ecos-maintainers-owner@ecos.sourceware.org X-SW-Source: 2009-02/txt/msg00044.txt.bz2 Bart Veer wrote: >>>>>>"Jifl" == Jonathan Larmour 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