Re: [U-Boot] [PATCH v3 3/4] mtd: nand: add Freescale vf610_nfc driver
Hi Stefan,
patch is landed on my desk as part of i.MX. I will have some minor
points. Is thi
On 18/08/2014 18:26, Stefan Agner wrote:
This adds initial support for Freescale NFC (NAND Flash Controller)
found in ARM Vybrid SoC's, Power Architecture MPC5125 and others.
The driver is called vf610_nfc since this is the first supported
and tested hardware platform supported by the driver.
Signed-off-by: Stefan Agner ste...@agner.ch
---
[snip]
+struct vf610_nfc {
+ struct mtd_info *mtd;
+ struct nand_chip chip;
+/* struct device *dev;*/
Do not add dead code. Check this globally.
+ void __iomem *regs;
+ uint column;
+ intspareonly;
+ intpage;
+ /* Status and ID are in alternate locations. */
+ intalt_buf;
+#define ALT_BUF_ID 1
+#define ALT_BUF_STAT 2
+ struct clk*clk;
+};
+
+#define mtd_to_nfc(_mtd) (struct vf610_nfc *)((struct nand_chip
*)_mtd-priv)-priv;
+
+static u8 bbt_pattern[] = {'B', 'b', 't', '0' };
+static u8 mirror_pattern[] = {'1', 't', 'b', 'B' };
+
+static struct nand_bbt_descr bbt_main_descr = {
+ .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
+NAND_BBT_2BIT | NAND_BBT_VERSION,
+ .offs = 11,
+ .len = 4,
+ .veroffs = 15,
+ .maxblocks = 4,
+ .pattern = bbt_pattern,
+};
+
+static struct nand_bbt_descr bbt_mirror_descr = {
+ .options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE |
+NAND_BBT_2BIT | NAND_BBT_VERSION,
+ .offs = 11,
+ .len = 4,
+ .veroffs = 15,
+ .maxblocks = 4,
+ .pattern = mirror_pattern,
+};
+
+static struct nand_ecclayout vf610_nfc_ecc45 = {
+ .eccbytes = 45,
+ .eccpos = {19, 20, 21, 22, 23,
+24, 25, 26, 27, 28, 29, 30, 31,
+32, 33, 34, 35, 36, 37, 38, 39,
+40, 41, 42, 43, 44, 45, 46, 47,
+48, 49, 50, 51, 52, 53, 54, 55,
+56, 57, 58, 59, 60, 61, 62, 63},
+ .oobfree = {
+ {.offset = 8,
+ .length = 11} }
+};
+
+static inline u32 vf610_nfc_read(struct mtd_info *mtd, uint reg)
+{
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+ return readl(nfc-regs + reg);
+}
+
+static inline void vf610_nfc_write(struct mtd_info *mtd, uint reg, u32 val)
+{
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+
+ writel(val, nfc-regs + reg);
+}
+
+static inline void vf610_nfc_set(struct mtd_info *mtd, uint reg, u32 bits)
+{
+ vf610_nfc_write(mtd, reg, vf610_nfc_read(mtd, reg) | bits);
+}
+
+static inline void vf610_nfc_clear(struct mtd_info *mtd, uint reg, u32 bits)
+{
+ vf610_nfc_write(mtd, reg, vf610_nfc_read(mtd, reg) ~bits);
+}
+
+static inline void vf610_nfc_set_field(struct mtd_info *mtd, u32 reg,
+u32 mask, u32 shift, u32 val)
+{
+ vf610_nfc_write(mtd, reg,
+ (vf610_nfc_read(mtd, reg) (~mask)) | val shift);
+}
+
+/* Clear flags for upcoming command */
+static inline void vf610_nfc_clear_status(void __iomem *regbase)
+{
+ void __iomem *reg = regbase + NFC_IRQ_STATUS;
+ u32 tmp = __raw_readl(reg);
+ tmp |= CMD_DONE_CLEAR_BIT | IDLE_CLEAR_BIT;
+ __raw_writel(tmp, reg);
+}
+
+/* Wait for complete operation */
+static inline void vf610_nfc_done(struct mtd_info *mtd)
+{
+ struct vf610_nfc *nfc = mtd_to_nfc(mtd);
+ uint start;
+
+ /*
+ * Barrier is needed after this write. This write need
+ * to be done before reading the next register the first
+ * time.
+ * vf610_nfc_set implicates such a barrier by using writel
+ * to write to the register.
+ */
+ vf610_nfc_set(mtd, NFC_FLASH_CMD2, START_BIT);
+
+ start = get_timer(0);
+
+ while (!(vf610_nfc_read(mtd, NFC_IRQ_STATUS) IDLE_IRQ_BIT)) {
+ if (get_timer(start) NFC_TIMEOUT) {
+ printf(Timeout while waiting for !BUSY.\n);
+ return;
+ }
+ }
+ vf610_nfc_clear_status(nfc-regs);
+}
+
+static u8 vf610_nfc_get_id(struct mtd_info *mtd, int col)
+{
+ u32 flash_id;
+
+ if (col 4) {
+ flash_id = vf610_nfc_read(mtd, NFC_FLASH_STATUS1);
+ return (flash_id (3-col)*8) 0xff;
+ } else {
+ flash_id = vf610_nfc_read(mtd, NFC_FLASH_STATUS2);
+ return flash_id 24;
+ }
+}
+
+static u8 vf610_nfc_get_status(struct mtd_info *mtd)
+{
+ return vf610_nfc_read(mtd, NFC_FLASH_STATUS2) STATUS_BYTE1_MASK;
+}
+
+/* Single command */
+static void vf610_nfc_send_command(void __iomem *regbase, u32 cmd_byte1,
+u32 cmd_code)
+{
+ void __iomem *reg = regbase + NFC_FLASH_CMD2;
+ u32 tmp;
+ vf610_nfc_clear_status(regbase);
+
+ tmp = __raw_readl(reg);
+ tmp =
Re: [U-Boot] [PATCH v3 3/4] mtd: nand: add Freescale vf610_nfc driver
On 21/08/2014 23:15, Bill Pringlemeir wrote:
On 14 Aug 2014, ste...@agner.ch wrote:
This adds initial support for Freescale NFC (NAND Flash
Controller) found in ARM Vybrid SoC's, Power Architecture MPC5125
and others. However, this driver is only tested on Vybrid.
On Wed, 2014-08-13 at 22:32, Scott Wood wrote:
raw_writel() is itself something that should only be used for
hand-optimized sections. For non-performance-critical code you
should use normal writel() so that you don't need to worry about
manually adding I/O barriers.
Am 2014-08-14 23:12, schrieb Bill Pringlemeir:
[regarding memcpy() in the driver]
Maybe a comment is fine? It seems the Vybrid is safe for
different access sizes, but it is possible that some other CPU
might not be able to access this memory via 32/16/8 bit accesses
and 'memcpy()' may not be appropriate. It seems that 'natural'
size of the NFC controller itself is 32bits and the CPU interface
does lane masking. Ie, boot mode documentation talks about
remapping 'sram_physical_addr[13:3] =
{cpu_addr[11:3],cpu_addr[13:12]}' saying that bits 2,1 are not used
(hopefully one based numbers). This is just my guess...
On 18 Aug 2014, ste...@agner.ch wrote:
What assumptions do you make how memcpy accesses memory? This latest
patch now uses the optimized versions from the kernel... Maybe they
even try to access 64-bit width (the NIC interconnect supports
64-bit access)
[snip]
Am 2014-08-18 18:38, schrieb Bill Pringlemeir:
My only point is that the SRAM buffers use the same interface as the
main Nand controller register banks. So using 'readl/writel' for the
register, but not the SRAM buffers seems inconsistent.
So to address this inconsistency, I was thinking that we should at
least have a comment?
On 19 Aug 2014, ste...@agner.ch wrote:
IMHO, we just treat this as if its memory and I guess this is fine for
a buffer. memcpy knows how to copy data, and takes care if the
architecture needs aligned access when reading 32-bit width, or
similar requirements. We do not know whether memcpy really uses 32-bit
accesses, hence this comment might even be wrong. In a short test, I
could also access the buffer in byte/word length (tested using
md.b/md.w).
Also, I assume this just works for a different architecture too. If
not, the one using this driver the first time on a different
architecture would see this pretty quickly I guess :-)
[snip]
In our case, a barrier just after the memcpy would be sufficient.
I would suggest you make a 'vf610_nfc_memcpy()' [or even from/to
variants if you are pendantic] which can be a wrapper function of just
'memcpy'. Just the like the readl/writel wrappers this will collect the
BUS accesses into one place. So they are documented for people porting
the code. Trying to accommodate some future insane hardware hookup seems
futile beyond this?
Otherwise, I will add an 'Ack' or 'Reviewed-By' from me if you like. I
am sorry, I don't know what if anything is appropriate.
Both are appropraite. IMHO you are an author and you checked the code
after Stefan's porting: ACK should be the best choice,.
Best regards,
Stefano Babic
--
=
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 3/4] mtd: nand: add Freescale vf610_nfc driver
Hi Scott, Anything missing from your side in this version of the patch? Patch 1 and 2 of this patch set recently got merged. -- Stefan Am 2014-08-18 18:26, schrieb Stefan Agner: This adds initial support for Freescale NFC (NAND Flash Controller) found in ARM Vybrid SoC's, Power Architecture MPC5125 and others. The driver is called vf610_nfc since this is the first supported and tested hardware platform supported by the driver. Signed-off-by: Stefan Agner ste...@agner.ch --- drivers/mtd/nand/Makefile| 1 + drivers/mtd/nand/vf610_nfc.c | 714 +++ 2 files changed, 715 insertions(+) create mode 100644 drivers/mtd/nand/vf610_nfc.c diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index bf1312a..eef86d1 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_NAND_KB9202) += kb9202_nand.o obj-$(CONFIG_NAND_KIRKWOOD) += kirkwood_nand.o obj-$(CONFIG_NAND_KMETER1) += kmeter1_nand.o obj-$(CONFIG_NAND_MPC5121_NFC) += mpc5121_nfc.o +obj-$(CONFIG_NAND_VF610_NFC) += vf610_nfc.o obj-$(CONFIG_NAND_MXC) += mxc_nand.o obj-$(CONFIG_NAND_MXS) += mxs_nand.o obj-$(CONFIG_NAND_NDFC) += ndfc.o diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c new file mode 100644 index 000..32203fd --- /dev/null +++ b/drivers/mtd/nand/vf610_nfc.c @@ -0,0 +1,714 @@ +/* + * Copyright 2009-2014 Freescale Semiconductor, Inc. and others + * + * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver. + * Ported to U-Boot by Stefan Agner + * Based on RFC driver posted on Kernel Mailing list by Bill Pringlemeir + * Jason ported to M54418TWR and MVFA5. + * Authors: Stefan Agner stefan.ag...@toradex.com + * Bill Pringlemeir bpringlem...@nbsps.com + * Shaohui Xie b21...@freescale.com + * Jason Jin jason@freescale.com + * + * Based on original driver mpc5121_nfc.c. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * Limitations: + * - Untested on MPC5125 and M54418. + * - DMA not used. + * - 2K pages or less. + * - Only 2K page w. 64+OOB and hardware ECC. + */ + +#include common.h +#include malloc.h + +#include linux/mtd/mtd.h +#include linux/mtd/nand.h +#include linux/mtd/partitions.h + +#include nand.h +#include errno.h +#include asm/io.h +#include asm/processor.h + +/* Register Offsets */ +#define NFC_FLASH_CMD1 0x3F00 +#define NFC_FLASH_CMD2 0x3F04 +#define NFC_COL_ADDR 0x3F08 +#define NFC_ROW_ADDR 0x3F0c +#define NFC_ROW_ADDR_INC 0x3F14 +#define NFC_FLASH_STATUS10x3F18 +#define NFC_FLASH_STATUS20x3F1c +#define NFC_CACHE_SWAP 0x3F28 +#define NFC_SECTOR_SIZE 0x3F2c +#define NFC_FLASH_CONFIG 0x3F30 +#define NFC_IRQ_STATUS 0x3F38 + +/* Addresses for NFC MAIN RAM BUFFER areas */ +#define NFC_MAIN_AREA(n) ((n) * 0x1000) + +#define PAGE_2K 0x0800 +#define OOB_64 0x0040 + +/* + * NFC_CMD2[CODE] values. See section: + * - 31.4.7 Flash Command Code Description, Vybrid manual + * - 23.8.6 Flash Command Sequencer, MPC5125 manual + * + * Briefly these are bitmasks of controller cycles. + */ +#define READ_PAGE_CMD_CODE 0x7EE0 +#define PROGRAM_PAGE_CMD_CODE0x7FC0 +#define ERASE_CMD_CODE 0x4EC0 +#define READ_ID_CMD_CODE 0x4804 +#define RESET_CMD_CODE 0x4040 +#define STATUS_READ_CMD_CODE 0x4068 + +/* NFC ECC mode define */ +#define ECC_BYPASS 0 +#define ECC_45_BYTE 6 + +/*** Register Mask and bit definitions */ + +/* NFC_FLASH_CMD1 Field */ +#define CMD_BYTE2_MASK 0xFF00 +#define CMD_BYTE2_SHIFT 24 + +/* NFC_FLASH_CM2 Field */ +#define CMD_BYTE1_MASK 0xFF00 +#define CMD_BYTE1_SHIFT 24 +#define CMD_CODE_MASK0x0000 +#define CMD_CODE_SHIFT 8 +#define BUFNO_MASK 0x0006 +#define BUFNO_SHIFT 1 +#define START_BIT(10) + +/* NFC_COL_ADDR Field */ +#define COL_ADDR_MASK0x +#define COL_ADDR_SHIFT 0 + +/* NFC_ROW_ADDR Field */ +#define ROW_ADDR_MASK0x00FF +#define ROW_ADDR_SHIFT 0
[U-Boot] [PATCH v3 3/4] mtd: nand: add Freescale vf610_nfc driver
On 14 Aug 2014, ste...@agner.ch wrote:
This adds initial support for Freescale NFC (NAND Flash
Controller) found in ARM Vybrid SoC's, Power Architecture MPC5125
and others. However, this driver is only tested on Vybrid.
On Wed, 2014-08-13 at 22:32, Scott Wood wrote:
raw_writel() is itself something that should only be used for
hand-optimized sections. For non-performance-critical code you
should use normal writel() so that you don't need to worry about
manually adding I/O barriers.
Am 2014-08-14 23:12, schrieb Bill Pringlemeir:
[regarding memcpy() in the driver]
Maybe a comment is fine? It seems the Vybrid is safe for
different access sizes, but it is possible that some other CPU
might not be able to access this memory via 32/16/8 bit accesses
and 'memcpy()' may not be appropriate. It seems that 'natural'
size of the NFC controller itself is 32bits and the CPU interface
does lane masking. Ie, boot mode documentation talks about
remapping 'sram_physical_addr[13:3] =
{cpu_addr[11:3],cpu_addr[13:12]}' saying that bits 2,1 are not used
(hopefully one based numbers). This is just my guess...
On 18 Aug 2014, ste...@agner.ch wrote:
What assumptions do you make how memcpy accesses memory? This latest
patch now uses the optimized versions from the kernel... Maybe they
even try to access 64-bit width (the NIC interconnect supports
64-bit access)
[snip]
Am 2014-08-18 18:38, schrieb Bill Pringlemeir:
My only point is that the SRAM buffers use the same interface as the
main Nand controller register banks. So using 'readl/writel' for the
register, but not the SRAM buffers seems inconsistent.
So to address this inconsistency, I was thinking that we should at
least have a comment?
On 19 Aug 2014, ste...@agner.ch wrote:
IMHO, we just treat this as if its memory and I guess this is fine for
a buffer. memcpy knows how to copy data, and takes care if the
architecture needs aligned access when reading 32-bit width, or
similar requirements. We do not know whether memcpy really uses 32-bit
accesses, hence this comment might even be wrong. In a short test, I
could also access the buffer in byte/word length (tested using
md.b/md.w).
Also, I assume this just works for a different architecture too. If
not, the one using this driver the first time on a different
architecture would see this pretty quickly I guess :-)
[snip]
In our case, a barrier just after the memcpy would be sufficient.
I would suggest you make a 'vf610_nfc_memcpy()' [or even from/to
variants if you are pendantic] which can be a wrapper function of just
'memcpy'. Just the like the readl/writel wrappers this will collect the
BUS accesses into one place. So they are documented for people porting
the code. Trying to accommodate some future insane hardware hookup seems
futile beyond this?
Otherwise, I will add an 'Ack' or 'Reviewed-By' from me if you like. I
am sorry, I don't know what if anything is appropriate.
Thanks,
Bill Pringlemeir.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3 3/4] mtd: nand: add Freescale vf610_nfc driver
This adds initial support for Freescale NFC (NAND Flash Controller) found in ARM Vybrid SoC's, Power Architecture MPC5125 and others. The driver is called vf610_nfc since this is the first supported and tested hardware platform supported by the driver. Signed-off-by: Stefan Agner ste...@agner.ch --- drivers/mtd/nand/Makefile| 1 + drivers/mtd/nand/vf610_nfc.c | 714 +++ 2 files changed, 715 insertions(+) create mode 100644 drivers/mtd/nand/vf610_nfc.c diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index bf1312a..eef86d1 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_NAND_KB9202) += kb9202_nand.o obj-$(CONFIG_NAND_KIRKWOOD) += kirkwood_nand.o obj-$(CONFIG_NAND_KMETER1) += kmeter1_nand.o obj-$(CONFIG_NAND_MPC5121_NFC) += mpc5121_nfc.o +obj-$(CONFIG_NAND_VF610_NFC) += vf610_nfc.o obj-$(CONFIG_NAND_MXC) += mxc_nand.o obj-$(CONFIG_NAND_MXS) += mxs_nand.o obj-$(CONFIG_NAND_NDFC) += ndfc.o diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c new file mode 100644 index 000..32203fd --- /dev/null +++ b/drivers/mtd/nand/vf610_nfc.c @@ -0,0 +1,714 @@ +/* + * Copyright 2009-2014 Freescale Semiconductor, Inc. and others + * + * Description: MPC5125, VF610, MCF54418 and Kinetis K70 Nand driver. + * Ported to U-Boot by Stefan Agner + * Based on RFC driver posted on Kernel Mailing list by Bill Pringlemeir + * Jason ported to M54418TWR and MVFA5. + * Authors: Stefan Agner stefan.ag...@toradex.com + * Bill Pringlemeir bpringlem...@nbsps.com + * Shaohui Xie b21...@freescale.com + * Jason Jin jason@freescale.com + * + * Based on original driver mpc5121_nfc.c. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * Limitations: + * - Untested on MPC5125 and M54418. + * - DMA not used. + * - 2K pages or less. + * - Only 2K page w. 64+OOB and hardware ECC. + */ + +#include common.h +#include malloc.h + +#include linux/mtd/mtd.h +#include linux/mtd/nand.h +#include linux/mtd/partitions.h + +#include nand.h +#include errno.h +#include asm/io.h +#include asm/processor.h + +/* Register Offsets */ +#define NFC_FLASH_CMD1 0x3F00 +#define NFC_FLASH_CMD2 0x3F04 +#define NFC_COL_ADDR 0x3F08 +#define NFC_ROW_ADDR 0x3F0c +#define NFC_ROW_ADDR_INC 0x3F14 +#define NFC_FLASH_STATUS1 0x3F18 +#define NFC_FLASH_STATUS2 0x3F1c +#define NFC_CACHE_SWAP 0x3F28 +#define NFC_SECTOR_SIZE0x3F2c +#define NFC_FLASH_CONFIG 0x3F30 +#define NFC_IRQ_STATUS 0x3F38 + +/* Addresses for NFC MAIN RAM BUFFER areas */ +#define NFC_MAIN_AREA(n) ((n) * 0x1000) + +#define PAGE_2K0x0800 +#define OOB_64 0x0040 + +/* + * NFC_CMD2[CODE] values. See section: + * - 31.4.7 Flash Command Code Description, Vybrid manual + * - 23.8.6 Flash Command Sequencer, MPC5125 manual + * + * Briefly these are bitmasks of controller cycles. + */ +#define READ_PAGE_CMD_CODE 0x7EE0 +#define PROGRAM_PAGE_CMD_CODE 0x7FC0 +#define ERASE_CMD_CODE 0x4EC0 +#define READ_ID_CMD_CODE 0x4804 +#define RESET_CMD_CODE 0x4040 +#define STATUS_READ_CMD_CODE 0x4068 + +/* NFC ECC mode define */ +#define ECC_BYPASS 0 +#define ECC_45_BYTE6 + +/*** Register Mask and bit definitions */ + +/* NFC_FLASH_CMD1 Field */ +#define CMD_BYTE2_MASK 0xFF00 +#define CMD_BYTE2_SHIFT24 + +/* NFC_FLASH_CM2 Field */ +#define CMD_BYTE1_MASK 0xFF00 +#define CMD_BYTE1_SHIFT24 +#define CMD_CODE_MASK 0x0000 +#define CMD_CODE_SHIFT 8 +#define BUFNO_MASK 0x0006 +#define BUFNO_SHIFT1 +#define START_BIT (10) + +/* NFC_COL_ADDR Field */ +#define COL_ADDR_MASK 0x +#define COL_ADDR_SHIFT 0 + +/* NFC_ROW_ADDR Field */ +#define ROW_ADDR_MASK 0x00FF +#define ROW_ADDR_SHIFT 0 +#define ROW_ADDR_CHIP_SEL_RB_MASK 0xF000 +#define ROW_ADDR_CHIP_SEL_RB_SHIFT 28 +#define ROW_ADDR_CHIP_SEL_MASK 0x0F00 +#define ROW_ADDR_CHIP_SEL_SHIFT24 + +/* NFC_FLASH_STATUS2 Field */ +#define STATUS_BYTE1_MASK 0x00FF + +/* NFC_FLASH_CONFIG Field */ +#define
