From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9986 invoked by alias); 18 Nov 2008 02:32:52 -0000 Received: (qmail 9902 invoked by uid 22791); 18 Nov 2008 02:32:46 -0000 X-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from hagrid.ecoscentric.com (HELO mail.ecoscentric.com) (212.13.207.197) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 18 Nov 2008 02:31:56 +0000 Received: from localhost (hagrid.ecoscentric.com [127.0.0.1]) by mail.ecoscentric.com (Postfix) with ESMTP id 9E6EA3B4006F for ; Tue, 18 Nov 2008 02:31:53 +0000 (GMT) X-Virus-Scanned: amavisd-new at ecoscentric.com Received: from mail.ecoscentric.com ([127.0.0.1]) by localhost (hagrid.ecoscentric.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id vXptk62ttsKr; Tue, 18 Nov 2008 02:31:52 +0000 (GMT) Message-ID: <49222917.8020407@eCosCentric.com> Date: Tue, 18 Nov 2008 02:32:00 -0000 From: Jonathan Larmour User-Agent: Thunderbird 1.5.0.12 (X11/20070530) MIME-Version: 1.0 To: eCos Patches List Subject: Re: [flashv2 merge] io/flash References: <4922125E.9070308@eCosCentric.com> In-Reply-To: <4922125E.9070308@eCosCentric.com> X-Enigmail-Version: 0.94.4.0 OpenPGP: id=A5FB74E6 Content-Type: multipart/mixed; boundary="------------030801080700080406070104" X-Virus-Checked: Checked by ClamAV on sourceware.org Mailing-List: contact ecos-patches-help@ecos.sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: ecos-patches-owner@ecos.sourceware.org X-SW-Source: 2008-11/txt/msg00059.txt.bz2 This is a multi-part message in MIME format. --------------030801080700080406070104 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-length: 3343 Jonathan Larmour wrote: > diff -u -5 -p -r1.43 ChangeLog > --- packages/io/flash/current/ChangeLog 25 Feb 2006 14:07:43 -0000 1.43 > +++ packages/io/flash/current/ChangeLog 18 Nov 2008 00:54:12 -0000 > @@ -1,20 +1,314 @@ > -2006-02-21 Oliver Munz > - Andrew Lunn > > - * src/flash.c (flash_init): Allow repeat calls change the function > - used for printing. There are times you don't any output, eg you > - are downloading am image over the serial port. About this bit. This patch which had been in the trunk couldn't be merged directly. Instead I am applying the attached patch to replace it. This is worthy of separate comment as it seems to subvert the principle of having different printf functions per device - otherwise what's the point? Perhaps this patch shouldn't be applied? I did in fact raise this issue (naffness of single printf function at init time) way back when the Flash v2 API was under discussion. I'll raise it again here. Here's what I wrote about it: -=-=-=-=-=-- [Remove printf function argument entirely from cyg_flash_init, and then...] Why not use a global constructor and have a function to only set the printf function away from the default. That function could also interpret NULL to be to set to a dummy empty function (like src/flashiodev.c does) Could also then do away with all CYG_FLASH_ERR_NOT_INIT checks (or relegate to an assert as it really should never happen) It also similarly simplifies flashiodev.c as then it doesn't have to check for init as well. -=-=-=-=-=-=- To which Bart validly replied: -=-=-=-=-=-=- I raised it with Andrew back in July. Unfortunately it is not currently possible. The currently defined constructor priorities are not sufficiently flexible to cope with dataflash, where the flash subsystem cannot be initialized until after the SPI bus. It would be necessary to reorganize the defined priorities, which is not something to be tackled lightly. -=-=-=-=-=-=- Unfortunately discussion fizzled out. My bad. However we've been happily using JFFS2, which uses the flashiodev stuff. That inits things at CYG_INIT_IO priority (49000). Obviously it doesn't use the dataflash, but it has been used in configurations where the dataflash is part of what has to be initted. From what Bart says, this has been risky, and potentially has depended on link order. We haven't seen this in practice, but I can believe it given both the flashiodev init priority and the SPI init priority are both CYG_INIT_IO, so the order depends on the whim of the linker. However given our experience and absence of problems in practice, it would seem low-risk to be tackling priorities. At least, if Bart is right, then there's already a problem, so we can hardly do nothing. Therefore as a starting point for discussion, I would suggest a new priority CYG_INIT_IO_BUS at 48000, i.e. before CYG_INIT_IO. This way buses can be initialised before devices which use them. Should we do this? Before eCos 3.0 would be the last chance to be fiddling with the flash API. Jifl -- 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 --------------030801080700080406070104 Content-Type: text/x-patch; name="flashv2.changeprintf.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="flashv2.changeprintf.diff" Content-length: 1743 Index: ChangeLog =================================================================== RCS file: /cvs/ecos/ecos/packages/io/flash/current/ChangeLog,v retrieving revision 1.44 diff -u -5 -p -r1.44 ChangeLog --- ChangeLog 18 Nov 2008 00:56:04 -0000 1.44 +++ ChangeLog 18 Nov 2008 01:53:44 -0000 @@ -1,5 +1,12 @@ +2008-11-18 Jonathan Larmour + + * src/flash.c (cyg_flash_init): Allow repeated calls to + change the printf function for all devices. There are + times you don't any output, eg you are downloading an + image over the serial port. + 2008-08-29 Nick Garnett * src/flashiodev.c (flashiodev_init): Assign CYG_DEVTAB_STATUS_BLOCK to status field of device table entries. This causes devfs to call the right read/write Index: src/flash.c =================================================================== RCS file: /cvs/ecos/ecos/packages/io/flash/current/src/flash.c,v retrieving revision 1.28 diff -u -5 -p -r1.28 flash.c --- src/flash.c 18 Nov 2008 00:56:04 -0000 1.28 +++ src/flash.c 18 Nov 2008 01:53:44 -0000 @@ -221,14 +221,20 @@ __externC int cyg_flash_init(cyg_flash_printf *pf) { int err; struct cyg_flash_dev * dev; - if (init) return CYG_FLASH_ERR_OK; - CYG_ASSERT(&(cyg_flashdevtab[CYGHWR_IO_FLASH_DEVICE]) == &cyg_flashdevtab_end, "incorrect number of flash devices"); + if (init) { + // In case the printf function has changed. + for (dev = &cyg_flashdevtab[0]; dev != &cyg_flashdevtab_end; dev++) { + dev->pf = pf; + } + return CYG_FLASH_ERR_OK; + } + for (dev = &cyg_flashdevtab[0]; dev != &cyg_flashdevtab_end; dev++) { dev->pf = pf; LOCK_INIT(dev); err = dev->funs->flash_init(dev); --------------030801080700080406070104--