This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [flashv2 merge] io/flash


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  <munz@speag.ch>
> -	    Andrew Lunn  <andrew.lunn@ascom.ch>
>  
> -	* 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
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  <jifl@eCosCentric.com>
+
+	* 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  <nickg@ecoscentric.com>
 
 	* 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);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]