public inbox for ecos-discuss@sourceware.org
 help / color / mirror / Atom feed
* [ECOS] blib_get_block() uses blocks instead of bytes? flashiodev_bread() uses bytes instead of blocks?
@ 2005-11-25 23:21 Garth Zeglin
  2005-11-26 17:21 ` Andrew Lunn
  0 siblings, 1 reply; 3+ messages in thread
From: Garth Zeglin @ 2005-11-25 23:21 UTC (permalink / raw)
  To: ecos-discuss

Dear ecos-discuss,

In summary: 
  It appears that blib_get_block() is calling
cyg_io_bread()
  with a length argument of 1, intending to read one
block; however
  cyg_io_bread expects a length parameter in bytes.

  It appears that flashiodev_bread() is treating the
'pos' parameter
  in units of bytes, but that it is supplied in units
of blocks.

Am I correct that this is a bug?  I'm asking here
first in case there
is something obvious I am missing.  I am using the
current public CVS
sources.  It's not clear what is the definition of
cyg_io_bread(), the
only documentation I found was on the ecoscentric web
site:
http://www.ecoscentric.com/ecospro/doc/html/ecospro-ref/diskio-usage.html

My trivial fix for
packages/io/flash/current/src/flashiodev.c

*** flashiodev.c        Fri Nov 25 23:44:23 2005
--- flashiodev.c.~1.8.~ Thu Apr 29 08:37:56 2004
***************
*** 122,123 ****
-       pos *= 512;
- 
--- 121 ----


My trivial fix for
packages/services/blib/current/src/blib.c

*** blib.c      Fri Nov 25 23:47:30 2005
--- blib.c.~1.3.~       Wed Aug  3 22:47:51 2005
***************
*** 360 ****
!         len = 512;
--- 360 ----
!         len = 1;


My full story, in case anyone searches for this later:

I'm new to eCos and as a test I set out to mount a
FAT12 filesystem
contained within a synthetic flash image.  I'm using
the Linux
synthetic target and synthetic flash device.  I
observed that the
mount failed because only one byte of the boot record
was being read.

Here is the call chain:
  mount ("/dev/flash1", "/", "fatfs" )
    ....
      fatfs_init()                                    
          (fatfs_supp.c)
        read_boot_record()
          disk_read ( *len == 90 bytes, pos == 0 )
            cyg_blib_read() (*len == 90, bnum == 0,
pos == 0)     (blib.c)
              blib_get_block ( num == 0 )
                 calls bl->bread_fn ( *len == 1,
block->num == 0)
                ....
                  flashiodev_bread( len == 1, pos == 0
)         (flashiodev.c)

The result I observe is that only one byte is read by
flashiodev,
instead of the 90 bytes requested by
read_boot_record().  Adding the
two patches above fixed this, and the file system
mounts and the root
directory can be read.

Other notes: 

I used /sbin/mkdosfs to create the filesystem on the
synth.flash
file. It begins directly with a FBR, there is no MBR
in the file.
It is only 1M so it ends up as FAT12.

I configured CYGNUM_IO_FLASH_BLOCK_OFFSET_1 to zero;
the default
configuration size for the synthetic flash is 1024k,
and the default
block offset puts the start of the block device past
the end.

    Thanks,

    Garth Zeglin



__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com 

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [ECOS] blib_get_block() uses blocks instead of bytes? flashiodev_bread() uses bytes instead of blocks?
  2005-11-25 23:21 [ECOS] blib_get_block() uses blocks instead of bytes? flashiodev_bread() uses bytes instead of blocks? Garth Zeglin
@ 2005-11-26 17:21 ` Andrew Lunn
  2005-11-28 10:47   ` [ECOS] MMC Driver? (was Re: [ECOS] blib_get_block() uses blocks instead of bytes...) Garth Zeglin
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Lunn @ 2005-11-26 17:21 UTC (permalink / raw)
  To: Garth Zeglin; +Cc: ecos-discuss

On Fri, Nov 25, 2005 at 03:20:58PM -0800, Garth Zeglin wrote:
> Dear ecos-discuss,
> 
> In summary: 
>   It appears that blib_get_block() is calling
> cyg_io_bread()
>   with a length argument of 1, intending to read one
> block; however
>   cyg_io_bread expects a length parameter in bytes.
> 
>   It appears that flashiodev_bread() is treating the
> 'pos' parameter
>   in units of bytes, but that it is supplied in units
> of blocks.
> 
> Am I correct that this is a bug?  I'm asking here
> first in case there
> is something obvious I am missing. 

There is something none-obvious you are missing :-(

It is a bug, but a different bug to what you have found.

You are the first to try to put a fatfs on top of flash. Hence you
found the bug. Normally fatfs is put on top of a disk. The disk
bread/bwrite functions do work in blocks. The length is the number of
blocks and the pos is in units of blocks.

The flash block driver however is using bytes!

As you said, it is not well documented and the documentation you
pointed at also is not consistent with the disk implementation. It
says len is in bytes, but disk_bread() uses len in blocks.

In my opinion, the pos should be in blocks. If it was in bytes we are
going to have problems with big disks, since pos is only 32bits
long. len could be either, but it is easier to fix the documentation
than change the code and do lots of tests.

This however does not solve your problem. First off, i hope you
realise your fatfs can only be read only! 

The flash bwrite function does not erase the page before writting, so
writes are going to fail if the flash contains something. Also, fatfs
assumes blocks are 512 bytes in size. If you flash device has bigger
than 512 byte pages, doing a page erase is going to corrupt your
filesystem. In order to make a fatfs writable on flash you will need
to insert another layer. This would have to handle the erases and
differences between block sizes and page sizes. blib actually already
allows this. If you call cyg_blib_create() instead of
cyg_blib_io_create() you can specify the read and write functions.

The eCos maintainers need to have a discussion about what a block read
and write should do and if we want to modify the flash code to
implement it.

        Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [ECOS] MMC Driver? (was Re: [ECOS] blib_get_block() uses blocks instead of bytes...)
  2005-11-26 17:21 ` Andrew Lunn
@ 2005-11-28 10:47   ` Garth Zeglin
  0 siblings, 0 replies; 3+ messages in thread
From: Garth Zeglin @ 2005-11-28 10:47 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: ecos-discuss


Dear Andrew and ecos-discuss,

Thanks for the comments and warnings, that was helpful.

My immediate goal was read-only fatfs in FLASH as a test case for setting up a fatfs via a SD/MMC
card SPI mode driver.  I have not yet found an eCos MMC driver, although it seems there is one in
eCosPro.   Do you know of any open ones available?

I have found several open sources I could try adapting for my needs; the protocol doesn't look too
hard.  But a working one would save me some work.

   Thanks,

   Garth Zeglin

--- Andrew Lunn <andrew@lunn.ch> wrote:
> It is a bug, but a different bug to what you have found.
> 
> You are the first to try to put a fatfs on top of flash. Hence you
> found the bug. Normally fatfs is put on top of a disk. The disk
> bread/bwrite functions do work in blocks. The length is the number of
> blocks and the pos is in units of blocks.
> 
> The flash block driver however is using bytes!
> 
> As you said, it is not well documented and the documentation you
> pointed at also is not consistent with the disk implementation. It
> says len is in bytes, but disk_bread() uses len in blocks.
> 
> In my opinion, the pos should be in blocks. If it was in bytes we are
> going to have problems with big disks, since pos is only 32bits
> long. len could be either, but it is easier to fix the documentation
> than change the code and do lots of tests.
> 
> This however does not solve your problem. First off, i hope you
> realise your fatfs can only be read only! 
> 
> The flash bwrite function does not erase the page before writting, so
> writes are going to fail if the flash contains something. Also, fatfs
> assumes blocks are 512 bytes in size. If you flash device has bigger
> than 512 byte pages, doing a page erase is going to corrupt your
> filesystem. In order to make a fatfs writable on flash you will need
> to insert another layer. This would have to handle the erases and
> differences between block sizes and page sizes. blib actually already
> allows this. If you call cyg_blib_create() instead of
> cyg_blib_io_create() you can specify the read and write functions.
> 
> The eCos maintainers need to have a discussion about what a block read
> and write should do and if we want to modify the flash code to
> implement it.
> 
>         Andrew
> 



	
		
__________________________________ 
Yahoo! Mail - PC Magazine Editors' Choice 2005 
http://mail.yahoo.com

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2005-11-28 10:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-11-25 23:21 [ECOS] blib_get_block() uses blocks instead of bytes? flashiodev_bread() uses bytes instead of blocks? Garth Zeglin
2005-11-26 17:21 ` Andrew Lunn
2005-11-28 10:47   ` [ECOS] MMC Driver? (was Re: [ECOS] blib_get_block() uses blocks instead of bytes...) Garth Zeglin

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).