Josh Green wrote:
> Hello, I found a couple compile problems with the
> drivers/mtd/maps/db1x00-flash.c MTD driver. I'm using linux-mips CVS
> from a few weeks back, corresponding to 2.6.11rc2. I noticed some
> recent CVS traffic in regards to this driver, but I didn't see them in
> cvsweb on the linux-mips site. My apologies if this is something that
> has already been reported.
Even if reported, it hasn't been fixed - I have similar problems.
> - Cast return value of ioremap to (void __iomem *) to get rid of warning
> concerning conversion of integer to pointer
This one needs further inspection, I'd say. Grepping through the sources, I
see that some platforms define ioremap to return 'void*' and some use 'void
__iomem*'. The same class inconsistencies exist for iounmap(), I think the
right thing is 'iounmap( void __iomem*)'.
I found a comment that the value returned from ioremap() is not to be used as
a virtual address, so it can't be used directly anyway, so a qualifier like
volatile is not even necessary. The qualifier becomes necessary inside the
functions that perform the actual IO like readb(), but everything outside
should not even attempt to look at this pointer.
Yes, on MIPS it can be used as virtual address AFAICT, some (broken?) code
might even do so. If that code then relies on the volatile qualifier it will
I went ahead and changed the functions in asm-mips/io.h, and my Au1100 board
still seems to work as before. Several other architectures need these fixes,
too, and several cases where the returnvalue of ioremap() or the parameter to
iounmap() is cast falsely/unnecessarily are also present.
Hacking on above stuff, I came across another thing that escapes me: inside
functions like writes*() and reads*(), the the buffer to write to or read is
taken as 'void*', then to be cast to 'volatile void*'. In the case of
writes*(), IMHO the pointer should be const. In neither cases does it make
sense to me to add the volatile to the pointer. What is the reason for this?
Disclaimer: I'm far from being a kernel expert, so if I'm talking crap
somebody please enlighten me. I just looked at the code and saw what to me
> - Setup DB1X00_BOTH_BANKS, DB1X00_BOOT_ONLY, and DB1X00_USER_ONLY
> defines in db1x00.h (used pb1550.h as an example)
I copied over the code from the db1x00.h of Linux 2.4 to achieve the same.
However, I didn't put this in any header because its use is limited to just
one file, AFAICT.
Hmm, I just looked a bit further, and now I wonder why there are so many
drivers for Au1xx0 based boards. pb1550-flash.c and db1550-flash.c are almost
similar, including the comment about the file they are based on (look at it,
seems like search&replace gone amok).
I haven't looked too far into it, but I really wonder if not at least
db1x00-flash.c and db1550-flash.c could be merged with pb1xxx-flash.c and
pb1550-flash.c, if not even all four could be merged into a single driver.
Point is that the main difference seem to be the memory layout of the flash,
all the rest seems generic enough.
> I can see the partitions in /dev/mtd now, but I have not thoroughly
> tested it yet to see if there are any other problems.
Can you tell me how you created /dev/mtd? My version (Debian/x86) of MAKEDEV
doesn't know these. Also, could you tell me how you configured your kernel? I
have never seen an MTD working, so I don't even know if what I'm doing is
supposed to work. :(