Make nand block functions use provided byte/word helpers.

PATCH In review Jonathan McDowell 2 participants 5 messages Feb 28, 2006 – Mar 1, 2006

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Jonathan McDowell

author

2006-02-28 20:59 UTC #0 lore raw

PATCH 1/1 Make nand block functions use provided byte/word helpers.

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]>

----------

drivers/mtd/nand/nand_base.c renamed from drivers/mtd/nand/nand_base.c.orig +6 -6

@@ -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;

----------

Signature · 2 lines

 [   There are always at least two ways to program the same thing.    ]
 [ http://www.blackcatnetworks.co.uk/ - IPv6 enabled ADSL/dialup/colo ]

Ben Dooks

2006-02-28 22:15 UTC #1 lore raw

Quoted 12 lines · Jonathan McDowell

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!

Signature · 3 lines

Ben ([email protected], http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Jonathan McDowell

author

2006-02-28 22:26 UTC #2 lore raw

Quoted 12 lines · Ben Dooks

Quoted 10 lines · Jonathan McDowell
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.

Quoted 3 lines

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.

Signature · 3 lines

noodles is fat
This .sig brought to you by the letter M and the number  5
Product of the Republic of HuggieTag

Ben Dooks

2006-03-01 11:50 UTC #3 lore raw

Quoted 17 lines · Jonathan McDowell

Quoted 12 lines · Ben Dooks
Quoted 10 lines · Jonathan McDowell
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.
Quoted 3 lines
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!

Quoted 7 lines

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.

Signature · 3 lines

Ben ([email protected], http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Jonathan McDowell

author

2006-03-01 12:29 UTC #4 lore raw

Quoted 23 lines · Ben Dooks

Quoted 22 lines · Jonathan McDowell
Quoted 13 lines · Ben Dooks
Quoted 11 lines · Jonathan McDowell
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.
Quoted 3 lines
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.

Quoted 6 lines

Quoted 3 lines
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.

Quoted 1 line

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.

Quoted 5 lines

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?

Signature · 3 lines

Web [            noodles is almost too good to be true             ]
site: http:// [                                          ]       Made by
www.earth.li/~noodles/  [                      ]         HuggieTag 0.0.23