Hi.
I've been writing a NAND driver for the flash on the Amstrad E3. One of
the peculiarities of this device is that the write & read enable lines
are on a latch, rather than strobed by the act of reading/writing from
the data latch. As such I've got custom read_byte/write_byte functions
defined. However the nand_*_buf functions in drivers/mtd/nand/nand_base.c
are all appropriate, except for the fact they call readb/writeb
themselves, instead of using this->read_byte or this->write_byte. The
patch below changes them to use these functions, meaning a driver just
needs to define read_byte and write_byte functions and gains all the
nand_*_buf functions free.
Signed-off-by: Jonathan McDowell <[email protected]>
----------
--- linux-2.6.15/drivers/mtd/nand/nand_base.c.orig 2006-02-28 20:41:54.000000000 +0000
+++ linux-2.6.15/drivers/mtd/nand/nand_base.c 2006-02-28 20:46:44.000000000 +0000
@@ -302,7 +302,7 @@ static void nand_write_buf(struct mtd_in
struct nand_chip *this = mtd->priv;
for (i=0; i<len; i++)
- writeb(buf[i], this->IO_ADDR_W);
+ this->write_byte(mtd, buf[i]);
}
/**
@@ -319,7 +319,7 @@ static void nand_read_buf(struct mtd_inf
struct nand_chip *this = mtd->priv;
for (i=0; i<len; i++)
- buf[i] = readb(this->IO_ADDR_R);
+ buf[i] = this->read_byte(mtd);
}
/**
@@ -336,7 +336,7 @@ static int nand_verify_buf(struct mtd_in
struct nand_chip *this = mtd->priv;
for (i=0; i<len; i++)
- if (buf[i] != readb(this->IO_ADDR_R))
+ if (buf[i] != this->read_byte(mtd))
return -EFAULT;
return 0;
@@ -358,7 +358,7 @@ static void nand_write_buf16(struct mtd_
len >>= 1;
for (i=0; i<len; i++)
- writew(p[i], this->IO_ADDR_W);
+ this->write_word(mtd, p[i]);
}
@@ -378,7 +378,7 @@ static void nand_read_buf16(struct mtd_i
len >>= 1;
for (i=0; i<len; i++)
- p[i] = readw(this->IO_ADDR_R);
+ p[i] = this->read_word(mtd);
}
/**
@@ -397,7 +397,7 @@ static int nand_verify_buf16(struct mtd_
len >>= 1;
for (i=0; i<len; i++)
- if (p[i] != readw(this->IO_ADDR_R))
+ if (p[i] != this->read_word(mtd))
return -EFAULT;
return 0;
----------
J.
--
[ There are always at least two ways to program the same thing. ]
[ http://www.blackcatnetworks.co.uk/ - IPv6 enabled ADSL/dialup/colo ]
On Tue, Feb 28, 2006 at 08:59:03PM +0000, Jonathan McDowell wrote:
> Hi.
>
> I've been writing a NAND driver for the flash on the Amstrad E3. One of
> the peculiarities of this device is that the write & read enable lines
> are on a latch, rather than strobed by the act of reading/writing from
> the data latch. As such I've got custom read_byte/write_byte functions
> defined. However the nand_*_buf functions in drivers/mtd/nand/nand_base.c
> are all appropriate, except for the fact they call readb/writeb
> themselves, instead of using this->read_byte or this->write_byte. The
> patch below changes them to use these functions, meaning a driver just
> needs to define read_byte and write_byte functions and gains all the
> nand_*_buf functions free.
Why not make life easier on everyone else by over-riding the
functions for read/write buffer (etc) in the nand driver... less
intrusive into the core code!
--
Ben ([email protected], http://www.fluff.org/)
'a smiley only costs 4 bytes'
On Tue, Feb 28, 2006 at 10:12:43PM +0000, Ben Dooks wrote:
> On Tue, Feb 28, 2006 at 08:59:03PM +0000, Jonathan McDowell wrote:
> > I've been writing a NAND driver for the flash on the Amstrad E3. One of
> > the peculiarities of this device is that the write & read enable lines
> > are on a latch, rather than strobed by the act of reading/writing from
> > the data latch. As such I've got custom read_byte/write_byte functions
> > defined. However the nand_*_buf functions in drivers/mtd/nand/nand_base.c
> > are all appropriate, except for the fact they call readb/writeb
> > themselves, instead of using this->read_byte or this->write_byte. The
> > patch below changes them to use these functions, meaning a driver just
> > needs to define read_byte and write_byte functions and gains all the
> > nand_*_buf functions free.
>
> Why not make life easier on everyone else by over-riding the
> functions for read/write buffer (etc) in the nand driver... less
> intrusive into the core code!
If the patch is deemed too intrusive then that's what I'll do; I nearly
did so originally but when I caught myself cut and pasting the code from
nand_base I thought my patch was the cleaner way.
The patch shouldn't cause any extra work for anyone that I can see and I
don't think it's intrusive at all; I submitted it because I figured it
might save someone else some work down the line as well.
J.
--
noodles is fat
This .sig brought to you by the letter M and the number 5
Product of the Republic of HuggieTag
On Tue, Feb 28, 2006 at 10:25:59PM +0000, Jonathan McDowell wrote:
> On Tue, Feb 28, 2006 at 10:12:43PM +0000, Ben Dooks wrote:
> > On Tue, Feb 28, 2006 at 08:59:03PM +0000, Jonathan McDowell wrote:
> > > I've been writing a NAND driver for the flash on the Amstrad E3. One of
> > > the peculiarities of this device is that the write & read enable lines
> > > are on a latch, rather than strobed by the act of reading/writing from
> > > the data latch. As such I've got custom read_byte/write_byte functions
> > > defined. However the nand_*_buf functions in drivers/mtd/nand/nand_base.c
> > > are all appropriate, except for the fact they call readb/writeb
> > > themselves, instead of using this->read_byte or this->write_byte. The
> > > patch below changes them to use these functions, meaning a driver just
> > > needs to define read_byte and write_byte functions and gains all the
> > > nand_*_buf functions free.
> >
> > Why not make life easier on everyone else by over-riding the
> > functions for read/write buffer (etc) in the nand driver... less
> > intrusive into the core code!
>
> If the patch is deemed too intrusive then that's what I'll do; I nearly
> did so originally but when I caught myself cut and pasting the code from
> nand_base I thought my patch was the cleaner way.
>
> The patch shouldn't cause any extra work for anyone that I can see and I
> don't think it's intrusive at all; I submitted it because I figured it
> might save someone else some work down the line as well.
Well, a small iritation is that it adds work for the read and
write byte function, as a function call adds instructions to
the path. Also, do you want to check that it dosen't break
any existing drivers?
Secondly, you'll probably have a better piece of code for the
E3 driver if you did the block calls with only one setup for
the r/w enable lines per block, instead of checking them for
every byte done. You may even want to use readsb/writesb or
DMA to accelerate the operation.
--
Ben ([email protected], http://www.fluff.org/)
'a smiley only costs 4 bytes'
On Wed, Mar 01, 2006 at 11:50:20AM +0000, Ben Dooks wrote:
> On Tue, Feb 28, 2006 at 10:25:59PM +0000, Jonathan McDowell wrote:
> > On Tue, Feb 28, 2006 at 10:12:43PM +0000, Ben Dooks wrote:
> > > On Tue, Feb 28, 2006 at 08:59:03PM +0000, Jonathan McDowell wrote:
> > > > I've been writing a NAND driver for the flash on the Amstrad E3.
> > > > One of the peculiarities of this device is that the write & read
> > > > enable lines are on a latch, rather than strobed by the act of
> > > > reading/writing from the data latch. As such I've got custom
> > > > read_byte/write_byte functions defined. However the nand_*_buf
> > > > functions in drivers/mtd/nand/nand_base.c are all appropriate,
> > > > except for the fact they call readb/writeb themselves, instead
> > > > of using this->read_byte or this->write_byte. The patch below
> > > > changes them to use these functions, meaning a driver just needs
> > > > to define read_byte and write_byte functions and gains all the
> > > > nand_*_buf functions free.
> > >
> > > Why not make life easier on everyone else by over-riding the
> > > functions for read/write buffer (etc) in the nand driver... less
> > > intrusive into the core code!
> >
> > If the patch is deemed too intrusive then that's what I'll do; I
> > nearly did so originally but when I caught myself cut and pasting
> > the code from nand_base I thought my patch was the cleaner way.
> >
> > The patch shouldn't cause any extra work for anyone that I can see
> > and I don't think it's intrusive at all; I submitted it because I
> > figured it might save someone else some work down the line as well.
>
> Well, a small iritation is that it adds work for the read and write
> byte function, as a function call adds instructions to the path.
Well, the _buf functions rather than the read/write_byte functions, but
yes, there will be extra instructions in the path.
> Also, do you want to check that it dosen't break any existing drivers?
Hard to do without the hardware, but I did check out the existing
drivers (from mtd-snapshot-20060222 mtd/drivers/mtd/nand/):
at91_nand.c
Doesn't override any of the _buf functions, will end up being
hit by the extra instructions in the path.
au1550nd.c
Overrides the _buf functions, unaffected.
autcpu12.c
Doesn't override any of the _buf functions, will end up being
hit by the extra instructions in the path.
diskonchip.c
Overrides the _buf functions, unaffected.
edb7312.c
Doesn't override any of the _buf functions, will end up being
hit by the extra instructions in the path.
h1910.c
Doesn't override any of the _buf functions, will end up being
hit by the extra instructions in the path.
rtc_from4.c
Doesn't override any of the _buf functions, will end up being
hit by the extra instructions in the path.
s3c2410.c
Overrides the _buf functions, unaffected.
sharpsl.c
Doesn't override any of the _buf functions, will end up being
hit by the extra instructions in the path.
spia.c
Doesn't override any of the _buf functions, will end up being
hit by the extra instructions in the path.
toto.c
Doesn't override any of the _buf functions, will end up being
hit by the extra instructions in the path.
tx4925ndfmc.c
Overrides the _buf functions.
tx4938ndfmc.c
Overrides the _buf functions.
So 8 drivers that will have the extra instructions in the call path, 5
that already provide their own. Nothing overrides the
read_byte/write_byte functions that doesn't also override the _buf
functions that I can see, so there should be no change in behaviour.
> Secondly, you'll probably have a better piece of code for the E3
> driver if you did the block calls with only one setup for the r/w
> enable lines per block, instead of checking them for every byte done.
> You may even want to use readsb/writesb or DMA to accelerate the
> operation.
It's not a case of checking the lines; the problem is that all of NCE,
NWE and NRE are hooked up to a latch, rather than processor control
lines. My understanding is that NCE or NWE/NRE need pulled high between
writes/reads and the only way to do this is output to the control latch.
As such I don't think readsb/writesb or DMA are able to help me out?
J.
--
Web [ noodles is almost too good to be true ]
site: http:// [ ] Made by
http://www.earth.li/~noodles/ [ ] HuggieTag 0.0.23