Re: [U-Boot] [PATCH 4/4] mtd: vf610_nfc: support subpage write
On 3 Apr 2015, ste...@agner.ch wrote: I will remove the page read on NAND_CMD_SEQIN, since we memcpy the full page anyway. I also just realized that the page read actually happens always and hence slows down even full page writes... Yes, I remove this in Linux (4.0) and it corrupted things when writing. I think your previous conclusion about we never use 'write caching' was wrong. This one is for writes, case NAND_CMD_SEQIN: /* Pre-read for partial writes. */ This one is for reads, case NAND_CMD_READ0: The interface between 'nand_base' and the MTD driver is hard to decipher. Does Scott (or anyone) know if there is any documentation on this? Stefan is completely correct that if a full page is being written, then the 'SEQIN' should not read a page. However, I only see 'column' being passed. How is 'SEQIN' and 'PAGEPROG' to detect if a full page is being written or not? The other way to handle things would to be to investigate the NFC_CFG[PAGE_CNT] and NFC_SECSZ to have the virtual pages support sub-pages. I think the OOB mapping would be non-standard in such cases. The buffer management in the driver is most simple in it's current form. The other versions that I found seemed to be buggy to me. However, the current driver doesn't use all of the NFC SRAM buffer space. Btw, the READ_OOB is very nice for Linux as well. It is a much faster mount of UBI/UbiFs as well. Fwiw, Bill Pringlemeir. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4] mtd: vf610_nfc: support subpage write
On Tue, 2015-04-07 at 09:48 -0400, Bill Pringlemeir wrote: On 3 Apr 2015, ste...@agner.ch wrote: I will remove the page read on NAND_CMD_SEQIN, since we memcpy the full page anyway. I also just realized that the page read actually happens always and hence slows down even full page writes... Yes, I remove this in Linux (4.0) and it corrupted things when writing. I think your previous conclusion about we never use 'write caching' was wrong. This one is for writes, case NAND_CMD_SEQIN: /* Pre-read for partial writes. */ This one is for reads, case NAND_CMD_READ0: The interface between 'nand_base' and the MTD driver is hard to decipher. Does Scott (or anyone) know if there is any documentation on this? It's an awkward interface for drivers that expose a higher-level programming model. Basically you have to behave as if nand_base could send commands directly to the chip. Stefan is completely correct that if a full page is being written, then the 'SEQIN' should not read a page. However, I only see 'column' being passed. How is 'SEQIN' and 'PAGEPROG' to detect if a full page is being written or not? At SEQIN time, you can't. You'll know based on how much data is written into the buffer. Or, you can skip this low-level interface and replace nand_write_page (which is what I wish we'd done in the eLBC/IFC drivers). The other way to handle things would to be to investigate the NFC_CFG[PAGE_CNT] and NFC_SECSZ to have the virtual pages support sub-pages. I think the OOB mapping would be non-standard in such cases. Wouldn't that mess up factory bad block markers? -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 4/4] mtd: vf610_nfc: support subpage write
Support subpage writes using a custom implementation of write_subpage.
The driver loads the page into SRAM buffer using NAND_CMD_READ0, when
the framework requests the NAND_CMD_SEQIN command. Then, the buffer is
updated by the custom write_subpage implementation. Upon write, the
controller calculates the hardware ECC across the whole page before
programming the page.
This method saves transferring the whole page over the bus to the NFC
IP, which would happen when using NAND_NO_SUBPAGE_WRITE.
Signed-off-by: Stefan Agner ste...@agner.ch
---
This implements the procedure as discussed on the ML:
http://lists.denx.de/pipermail/u-boot/2015-March/209567.html
http://lists.denx.de/pipermail/u-boot/2015-March/209671.html
The drivers mxc_nand and mpc5121_nfc implement a similar subpage write.
drivers/mtd/nand/vf610_nfc.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
index a8af00e..d552bed 100644
--- a/drivers/mtd/nand/vf610_nfc.c
+++ b/drivers/mtd/nand/vf610_nfc.c
@@ -566,6 +566,19 @@ static int vf610_nfc_write_page(struct mtd_info *mtd,
struct nand_chip *chip,
return 0;
}
+static int vf610_nfc_write_subpage(struct mtd_info *mtd, struct nand_chip
*chip,
+ uint32_t offset, uint32_t data_len,
+ const uint8_t *buf, int oob_required)
+{
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+ nfc-column = offset;
+ vf610_nfc_write_buf(mtd, buf, data_len);
+ if (oob_required)
+ vf610_nfc_write_buf(mtd, chip-oob_poi, mtd-oobsize);
+
+ return 0;
+}
+
struct vf610_nfc_config {
int hardware_ecc;
int width;
@@ -608,9 +621,6 @@ static int vf610_nfc_nand_init(int devnum, void __iomem
*addr)
vf610_nfc_clear(mtd, NFC_FLASH_CONFIG, CONFIG_16BIT);
}
- /* Disable subpage writes as we do not provide ecc-hwctl */
- chip-options |= NAND_NO_SUBPAGE_WRITE;
-
chip-dev_ready = vf610_nfc_dev_ready;
chip-cmdfunc = vf610_nfc_command;
chip-read_byte = vf610_nfc_read_byte;
@@ -669,6 +679,7 @@ static int vf610_nfc_nand_init(int devnum, void __iomem
*addr)
mtd-ecclayout = chip-ecc.layout;
chip-ecc.read_page = vf610_nfc_read_page;
chip-ecc.write_page = vf610_nfc_write_page;
+ chip-ecc.write_subpage = vf610_nfc_write_subpage;
chip-ecc.mode = NAND_ECC_HW;
chip-ecc.size = PAGE_2K;
--
2.3.5
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4] mtd: vf610_nfc: support subpage write
On 2015-04-03 22:36, Scott Wood wrote:
On Fri, 2015-04-03 at 20:40 +0200, Stefan Agner wrote:
Support subpage writes using a custom implementation of write_subpage.
The driver loads the page into SRAM buffer using NAND_CMD_READ0, when
the framework requests the NAND_CMD_SEQIN command. Then, the buffer is
updated by the custom write_subpage implementation. Upon write, the
controller calculates the hardware ECC across the whole page before
programming the page.
This method saves transferring the whole page over the bus to the NFC
IP, which would happen when using NAND_NO_SUBPAGE_WRITE.
Signed-off-by: Stefan Agner ste...@agner.ch
As previously discussed, please explain why subpage writes make sense
with this controller.
I thought the subpage callback is just about writing an arbitrary amount
of data (e.g. 32 bytes) into a page.
I quickly checked, when doing
# nand write ${loadaddr} 0x1 0x10
vf610_nfc_write_subpage gets actually called with a data_len of 16.
This is how I understand it:
Currently, subpage writes are supported by the MTD subsystem due to the
option NAND_NO_SUBPAGE_WRITE. Due to that option, nand_write_page in
nand_base.c calls write_page which copies always the whole page into the
SRAM buffer over the AHB bus to the NFC IP (vf610_nfc_write_page uses
memcpy uses mtd-writesize).
This patch supports it naively, which means that only the updated data
(according to offset and data_len parameter of write_subpage) get copied
over the AHB bus to the NFC IP (vf610_nfc_write_subpage uses memcpy to
only copy data_len). To have reasonable data for the rest of the page,
the page gets read back when calling NAND_CMD_SEQIN.
Since the whole page get programmed, this assumes that none of the page
has been programmed before (erased page). Hence, we essentially just
read 0xff. When I think about it now, reading the page back in SEQIN is
then probably just an expensive memset 0xff replacement.
I guess when having real subpages (e.g. 4x512bytes, each subpage with
its own ECC), I would have to make sure only the affected subpage gets
ECC'ed and written.
Even without having real subpages, if somebody only writes part of a
page, memset directly into SRAM memcpy the subpage data is
theoretically faster then memset the whole page in DDR RAM and memcpy
the whole page
Just checked what higher up the stack is actually happening:
nand_write_page anyway receives a whole page buffer as argument, which
is memset'ed with 0xff and the data to be written memcpy'ed.
I see, that this patch tries to improve something which anyway is taken
care of by the stack.
I will remove the page read on NAND_CMD_SEQIN, since we memcpy the full
page anyway. I also just realized that the page read actually happens
always and hence slows down even full page writes...
--
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 4/4] mtd: vf610_nfc: support subpage write
On Fri, 2015-04-03 at 20:40 +0200, Stefan Agner wrote: Support subpage writes using a custom implementation of write_subpage. The driver loads the page into SRAM buffer using NAND_CMD_READ0, when the framework requests the NAND_CMD_SEQIN command. Then, the buffer is updated by the custom write_subpage implementation. Upon write, the controller calculates the hardware ECC across the whole page before programming the page. This method saves transferring the whole page over the bus to the NFC IP, which would happen when using NAND_NO_SUBPAGE_WRITE. Signed-off-by: Stefan Agner ste...@agner.ch As previously discussed, please explain why subpage writes make sense with this controller. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
