Re: [U-Boot] [PATCH 02/12] SPL: NAND: add support for mxs nand

2014-05-05 Thread Tim Harvey
On Fri, May 2, 2014 at 1:56 PM, Scott Wood scottw...@freescale.com wrote:

Hi Scott,

 On Mon, 2014-04-28 at 13:17 -0700, Tim Harvey wrote:
 +static void mxs_nand_command(struct mtd_info *mtd, unsigned int command,
 +  int column, int page_addr)
 +{
 + register struct nand_chip *chip = mtd-priv;
 + int ctrl = NAND_NCE | NAND_CTRL_CLE | NAND_CTRL_CHANGE;
 + u32 timeo, time_start;
 +
 + /* write out the command to the device */
 + chip-cmd_ctrl(mtd, command, ctrl);
 +
 + /* Address cycle, when necessary */
 + ctrl = NAND_NCE | NAND_CTRL_ALE | NAND_CTRL_CHANGE;
 + /* Serially input address */
 + if (column != -1) {
 + ctrl = NAND_CTRL_ALE | NAND_CTRL_CHANGE;

 What happened to NAND_NCE?

 It looks like the cmd_ctrl in nand_mxs.c doesn't care about NAND_MCE at
 all, so there's no functional impact, but either use it consistently or
 don't use it at all.

 + chip-cmd_ctrl(mtd, column, ctrl);
 + ctrl = ~NAND_CTRL_CHANGE;
 + chip-cmd_ctrl(mtd, column  8, ctrl);
 + }

 Why not pass the ctrl flags directly to cmd_ctrl as you do in most of
 the rest of the function?  nand_mxs.c doesn't even look at NAND_CTRL_CHANGE.

I'll simplify using only what nand_mxs.c pays attention to (NAND_ALE,
NAND_CLE), remove the ctrl var and repost


 +int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf)
 +{
 + register struct nand_chip *chip;

 Why register?

cut-n-paste code from nand_base.c - will remove


 + if (!(page % nand_page_per_block)) {
 + /*
 +  * Yes, new block. See if this block is good. If not,
 +  * loop until we find a good block.
 +  */
 + while (is_badblock(mtd, offs, 1)) {
 + page = page + nand_page_per_block;
 + /* Check i we've reached the end of flash. */
 + if (page = mtd.size  chip-page_shift)
 + return -1;

 Why -1 rather than a real error value?

probably from the example I used from drivers/mtd/nand/mxc_nand_spl.c.
The calling functions don't appear to do any error checking, but I'll
replace it with -ENOMEM and -ENODEV if mxs_nand_init() fails.

Thanks for the review!

Tim


 -Scott


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 02/12] SPL: NAND: add support for mxs nand

2014-05-02 Thread Scott Wood
On Mon, 2014-04-28 at 13:17 -0700, Tim Harvey wrote:
 +static void mxs_nand_command(struct mtd_info *mtd, unsigned int command,
 +  int column, int page_addr)
 +{
 + register struct nand_chip *chip = mtd-priv;
 + int ctrl = NAND_NCE | NAND_CTRL_CLE | NAND_CTRL_CHANGE;
 + u32 timeo, time_start;
 +
 + /* write out the command to the device */
 + chip-cmd_ctrl(mtd, command, ctrl);
 +
 + /* Address cycle, when necessary */
 + ctrl = NAND_NCE | NAND_CTRL_ALE | NAND_CTRL_CHANGE;
 + /* Serially input address */
 + if (column != -1) {
 + ctrl = NAND_CTRL_ALE | NAND_CTRL_CHANGE;

What happened to NAND_NCE?

It looks like the cmd_ctrl in nand_mxs.c doesn't care about NAND_MCE at
all, so there's no functional impact, but either use it consistently or
don't use it at all.

 + chip-cmd_ctrl(mtd, column, ctrl);
 + ctrl = ~NAND_CTRL_CHANGE;
 + chip-cmd_ctrl(mtd, column  8, ctrl);
 + }

Why not pass the ctrl flags directly to cmd_ctrl as you do in most of
the rest of the function?  nand_mxs.c doesn't even look at NAND_CTRL_CHANGE.

 +int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf)
 +{
 + register struct nand_chip *chip;

Why register?

 + if (!(page % nand_page_per_block)) {
 + /*
 +  * Yes, new block. See if this block is good. If not,
 +  * loop until we find a good block.
 +  */
 + while (is_badblock(mtd, offs, 1)) {
 + page = page + nand_page_per_block;
 + /* Check i we've reached the end of flash. */
 + if (page = mtd.size  chip-page_shift)
 + return -1;

Why -1 rather than a real error value?

-Scott


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 02/12] SPL: NAND: add support for mxs nand

2014-04-28 Thread Tim Harvey
This utilizes existing mxs_nand support layer to provide a method to load an
image off nand for SPL. The flash device will be detected in order to support
multiple flash devices instead of having layout hard coded at build time.

Cc: Scott Wood scottw...@freescale.com
Signed-off-by: Tim Harvey thar...@gateworks.com
---
v2:
- remove dependence on mtdpart, mtdcore, nand_util, nand_ecc, nand_base
  and nand_bbt to bring SPL down in size. This reduced codesize by about 32k
  where now mxs_spl_nand is about 12k total.
---
 drivers/mtd/nand/Makefile   |   1 +
 drivers/mtd/nand/mxs_nand_spl.c | 239 
 2 files changed, 240 insertions(+)
 create mode 100644 drivers/mtd/nand/mxs_nand_spl.c

diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 02b149c..de5b461 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -65,5 +65,6 @@ else  # minimal SPL drivers
 obj-$(CONFIG_NAND_FSL_ELBC) += fsl_elbc_spl.o
 obj-$(CONFIG_NAND_FSL_IFC) += fsl_ifc_spl.o
 obj-$(CONFIG_NAND_MXC) += mxc_nand_spl.o
+obj-$(CONFIG_NAND_MXS) += mxs_nand_spl.o mxs_nand.o
 
 endif # drivers
diff --git a/drivers/mtd/nand/mxs_nand_spl.c b/drivers/mtd/nand/mxs_nand_spl.c
new file mode 100644
index 000..82491a4
--- /dev/null
+++ b/drivers/mtd/nand/mxs_nand_spl.c
@@ -0,0 +1,239 @@
+/*
+ * Copyright (C) 2014 Gateworks Corporation
+ * Author: Tim Harvey thar...@gateworks.com
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+#include common.h
+#include nand.h
+#include malloc.h
+
+static nand_info_t mtd;
+static struct nand_chip nand_chip;
+
+static void mxs_nand_command(struct mtd_info *mtd, unsigned int command,
+int column, int page_addr)
+{
+   register struct nand_chip *chip = mtd-priv;
+   int ctrl = NAND_NCE | NAND_CTRL_CLE | NAND_CTRL_CHANGE;
+   u32 timeo, time_start;
+
+   /* write out the command to the device */
+   chip-cmd_ctrl(mtd, command, ctrl);
+
+   /* Address cycle, when necessary */
+   ctrl = NAND_NCE | NAND_CTRL_ALE | NAND_CTRL_CHANGE;
+   /* Serially input address */
+   if (column != -1) {
+   ctrl = NAND_CTRL_ALE | NAND_CTRL_CHANGE;
+   chip-cmd_ctrl(mtd, column, ctrl);
+   ctrl = ~NAND_CTRL_CHANGE;
+   chip-cmd_ctrl(mtd, column  8, ctrl);
+   }
+   if (page_addr != -1) {
+   chip-cmd_ctrl(mtd, page_addr, ctrl);
+   chip-cmd_ctrl(mtd, page_addr  8, NAND_NCE | NAND_ALE);
+   /* One more address cycle for devices  128MiB */
+   if (chip-chipsize  (128  20))
+   chip-cmd_ctrl(mtd, page_addr  16,
+  NAND_NCE | NAND_ALE);
+   }
+   chip-cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
+
+   if (command == NAND_CMD_READ0) {
+   chip-cmd_ctrl(mtd, NAND_CMD_READSTART,
+  NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
+   chip-cmd_ctrl(mtd, NAND_CMD_NONE,
+  NAND_NCE | NAND_CTRL_CHANGE);
+   }
+
+   /* wait for nand ready */
+   ndelay(100);
+   timeo = (CONFIG_SYS_HZ * 20) / 1000;
+   time_start = get_timer(0);
+   while (get_timer(time_start)  timeo) {
+   if (chip-dev_ready(mtd))
+   break;
+   }
+}
+
+static int mxs_flash_ident(struct mtd_info *mtd)
+{
+   register struct nand_chip *chip = mtd-priv;
+   int i;
+   u8 mfg_id, dev_id;
+   u8 id_data[8];
+   struct nand_onfi_params *p = chip-onfi_params;
+
+   /* Reset the chip */
+   chip-cmdfunc(mtd, NAND_CMD_RESET, -1, -1);
+
+   /* Send the command for reading device ID */
+   chip-cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);
+
+   /* Read manufacturer and device IDs */
+   mfg_id = chip-read_byte(mtd);
+   dev_id = chip-read_byte(mtd);
+
+   /* Try again to make sure */
+   chip-cmdfunc(mtd, NAND_CMD_READID, 0x00, -1);
+   for (i = 0; i  8; i++)
+   id_data[i] = chip-read_byte(mtd);
+   if (id_data[0] != mfg_id || id_data[1] != dev_id) {
+   printf(second ID read did not match);
+   return -1;
+   }
+   debug(0x%02x:0x%02x , mfg_id, dev_id);
+
+   /* read ONFI */
+   chip-onfi_version = 0;
+   chip-cmdfunc(mtd, NAND_CMD_READID, 0x20, -1);
+   if (chip-read_byte(mtd) != 'O' || chip-read_byte(mtd) != 'N' ||
+   chip-read_byte(mtd) != 'F' || chip-read_byte(mtd) != 'I') {
+   return -2;
+   }
+
+   /* we have ONFI, probe it */
+   chip-cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
+   chip-read_buf(mtd, (uint8_t *)p, sizeof(*p));
+   mtd-name = p-model;
+   mtd-writesize = le32_to_cpu(p-byte_per_page);
+   mtd-erasesize = le32_to_cpu(p-pages_per_block) * mtd-writesize;
+   mtd-oobsize = le16_to_cpu(p-spare_bytes_per_page);
+   chip-chipsize =