Extend LPC32xx functionality and add LPC32xx-based work_92015 board

Signed-off-by Albert ARIBAUD (3ADEV) <[email protected]>

---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3:
- move regs and functions in private struct

Changes in v2:
- added MUX setting for SSP0

 arch/arm/cpu/arm926ejs/lpc32xx/devices.c      |  14 +++
 arch/arm/include/asm/arch-lpc32xx/clk.h       |   3 +
 arch/arm/include/asm/arch-lpc32xx/sys_proto.h |   1 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/lpc32xx_ssp.c                     | 144 ++++++++++++++++++++++++++
 5 files changed, 163 insertions(+)
 create mode 100644 drivers/spi/lpc32xx_ssp.c

arch/arm/cpu/arm926ejs/lpc32xx/devices.c +14 -0

index a407098..5a453e3 100644
@@ -8,11 +8,13 @@
 #include <asm/arch/cpu.h>
 #include <asm/arch/clk.h>
 #include <asm/arch/uart.h>
+#include <asm/arch/mux.h>
 #include <asm/io.h>
 #include <dm.h>
 
 static struct clk_pm_regs    *clk  = (struct clk_pm_regs *)CLK_PM_BASE;
 static struct uart_ctrl_regs *ctrl = (struct uart_ctrl_regs *)UART_CTRL_BASE;
+static struct mux_regs *mux = (struct mux_regs *)MUX_BASE;
 
 void lpc32xx_uart_init(unsigned int uart_id)
 {

@@ -66,3 +68,15 @@ void lpc32xx_i2c_init(unsigned int devnum)
 U_BOOT_DEVICE(lpc32xx_gpios) = {
        .name = "gpio_lpc32xx"
 };
+
+/* Mux for SCK0, MISO0, MOSI0. We do not use SSEL0. */
+
+#define P_MUX_SET_SSP0 0x1600
+
+void lpc32xx_ssp_init(void)
+{
+       /* Enable SSP0 interface */
+       writel(CLK_SSP0_ENABLE_CLOCK, &clk->ssp_ctrl);
+       /* Mux SSP0 pins */
+       writel(P_MUX_SET_SSP0, &mux->p_mux_set);
+}

arch/arm/include/asm/arch-lpc32xx/clk.h +3 -0

index 781ac07..2cb5703 100644
@@ -155,6 +155,9 @@ struct clk_pm_regs {
 #define CLK_NAND_MLC                   (1 << 1)
 #define CLK_NAND_MLC_INT               (1 << 5)
 
+/* SSP Clock Control Register bits */
+#define CLK_SSP0_ENABLE_CLOCK          (1 << 0)
+
 unsigned int get_sys_clk_rate(void);
 unsigned int get_hclk_pll_rate(void);
 unsigned int get_hclk_clk_div(void);

arch/arm/include/asm/arch-lpc32xx/sys_proto.h +1 -0

index a4a05d1..86d5ee9 100644
@@ -11,5 +11,6 @@ void lpc32xx_uart_init(unsigned int uart_id);
 void lpc32xx_mac_init(void);
 void lpc32xx_mlc_nand_init(void);
 void lpc32xx_i2c_init(unsigned int devnum);
+void lpc32xx_ssp_init(void);
 
 #endif /* _LPC32XX_SYS_PROTO_H */

drivers/spi/Makefile +1 -0

index edbd520..ce6f1cc 100644
@@ -32,6 +32,7 @@ obj-$(CONFIG_EXYNOS_SPI) += exynos_spi.o
 obj-$(CONFIG_FTSSP010_SPI) += ftssp010_spi.o
 obj-$(CONFIG_ICH_SPI) +=  ich.o
 obj-$(CONFIG_KIRKWOOD_SPI) += kirkwood_spi.o
+obj-$(CONFIG_LPC32XX_SSP) += lpc32xx_ssp.o
 obj-$(CONFIG_MPC52XX_SPI) += mpc52xx_spi.o
 obj-$(CONFIG_MPC8XXX_SPI) += mpc8xxx_spi.o
 obj-$(CONFIG_MXC_SPI) += mxc_spi.o

drivers/spi/lpc32xx_ssp.c new file +29 -0

new file mode 100644
index 0000000..0fc44a0
@@ -0,0 +1,144 @@
+/*
+ * LPC32xx SSP interface (SPI mode)
+ *
+ * (C) Copyright 2014  DENX Software Engineering GmbH
+ * Written-by: Albert ARIBAUD <[email protected]>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+#include <linux/compat.h>
+#include <asm/io.h>
+#include <malloc.h>
+#include <spi.h>
+#include <asm/arch/clk.h>
+
+/* SSP chip registers */
+struct ssp_regs {
+       u32 cr0;
+       u32 cr1;
+       u32 data;
+       u32 sr;
+       u32 cpsr;
+       u32 imsc;
+       u32 ris;
+       u32 mis;
+       u32 icr;
+       u32 dmacr;
+};

drivers/spi/lpc32xx_ssp.c new file +105 -1

+struct lpc32xx_spi_slave {
+       struct spi_slave slave;
+       struct ssp_regs *regs;
+};
+
+static inline struct lpc32xx_spi_slave *to_lpc32xx_spi_slave(
+       struct spi_slave *slave)
+{
+       return container_of(slave, struct lpc32xx_spi_slave, slave);
+}
+
+/* spi_init is called during boot when CONFIG_CMD_SPI is defined */
+void spi_init(void)
+{
+       /*
+        *  nothing to do: clocking was enabled in lpc32xx_ssp_enable()
+        * and configuration will be done in spi_setup_slave()
+       */
+}
+
+/* the following is called in sequence by do_spi_xfer() */
+
+struct spi_slave *spi_setup_slave(uint bus, uint cs, uint max_hz, uint mode)
+{
+       struct lpc32xx_spi_slave *lslave;
+
+       /* we only set up SSP0 for now, so ignore bus */
+
+       if (mode & SPI_3WIRE) {
+               error("3-wire mode not supported");
+               return NULL;
+       }
+
+       if (mode & SPI_SLAVE) {
+               error("slave mode not supported\n");
+               return NULL;
+       }
+
+       if (mode & SPI_PREAMBLE) {
+               error("preamble byte skipping not supported\n");
+               return NULL;
+       }
+
+       lslave = spi_alloc_slave(struct lpc32xx_spi_slave, bus, cs);
+       if (!lslave) {
+               printf("SPI_error: Fail to allocate lpc32xx_spi_slave\n");
+               return NULL;
+       }
+
+       lslave->regs = (struct ssp_regs *)SSP0_BASE;
+
+       /*
+        * 8 bit frame, SPI fmt, 500kbps -> clock divider is 26.
+        * Set SCR to 0 and CPSDVSR to 26.
+        */
+
+       writel(0x7, &lslave->regs->cr0); /* 8-bit chunks, SPI, 1 clk/bit */
+       writel(26, &lslave->regs->cpsr); /* SSP clock = HCLK/26 = 500kbps */
+       writel(0, &lslave->regs->imsc); /* do not raise any interrupts */
+       writel(0, &lslave->regs->icr); /* clear any pending interrupt */
+       writel(0, &lslave->regs->dmacr); /* do not do DMAs */
+       writel(SSP_CR1_SSP_ENABLE, &lslave->regs->cr1); /* enable SSP0 */
+       return &lslave->slave;
+}
+
+void spi_free_slave(struct spi_slave *slave)
+{
+       struct lpc32xx_spi_slave *lslave = to_lpc32xx_spi_slave(slave);
+
+       debug("(lpc32xx) spi_free_slave: 0x%08x\n", (u32)lslave);
+       free(lslave);
+}
+
+int spi_claim_bus(struct spi_slave *slave)
+{
+       /* only one bus and slave so far, always available */
+       return 0;
+}
+
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
+       const void *dout, void *din, unsigned long flags)
+{
+       struct lpc32xx_spi_slave *lslave = to_lpc32xx_spi_slave(slave);
+       int bytelen = bitlen >> 3;
+       int idx_out = 0;
+       int idx_in = 0;
+       int start_time;
+
+       start_time = get_timer(0);
+       while ((idx_out < bytelen) || (idx_in < bytelen)) {
+               int status = readl(&lslave->regs->sr);
+               if ((idx_out < bytelen) && (status & SSP_SR_TNF))
+                       writel(((u8 *)dout)[idx_out++], &lslave->regs->data);
+               if ((idx_in < bytelen) && (status & status & SSP_SR_RNE))
+                       ((u8 *)din)[idx_in++] = readl(&lslave->regs->data);
+               if (get_timer(start_time) >= CONFIG_LPC32XX_SSP_TIMEOUT)
+                       return -1;
+       }
+       return 0;
+}
+
+void spi_release_bus(struct spi_slave *slave)
+{
+       /* do nothing */
+}
--

2.1.0

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +4 -0

+	struct lpc32xx_oob oob;
+	unsigned int page, left;
+
+	/* if starting mid-block consider block good */

drivers/mtd/nand/lpc32xx_nand_mlc.c line 673

Quoted 3 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +3 -0

+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
+{
+	int block_good;

bool?

Quoted 4 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +4 -0

+	struct lpc32xx_oob oob;
+	unsigned int page, left;
+
+	/* if starting mid-block consider block good */

drivers/mtd/nand/lpc32xx_nand_mlc.c line 687

Quoted 10 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +10 -0

+	block_good = 1;
+
+	for (page = offs / BYTES_PER_PAGE,
+	     left = DIV_ROUND_UP(size, BYTES_PER_PAGE);
+	     left && (page < PAGES_PER_CHIP_MAX); page++) {
+		/* read inband and oob page data anyway */
+		int res = read_single_page(dst, page, &oob);
+		/* return error if a good block's page was not readable */
+		if ((res < 0) && block_good)
+			return -1;

Why even try to read it if it's been marked bad?

drivers/mtd/nand/lpc32xx_nand_mlc.c line 688

Quoted 1 line

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +1 -0

+		/* if first page in a block, update 'block good' status */

The non-SPL driver appears to be checking the last two pages in the block, not the first page.

Quoted 4 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +4 -0

+		if ((page % PAGES_PER_BLOCK) == 0) {
+			/* assume block is good */
+			block_good = 1;
+			/* if page could not be read, assume block is bad */

Review

Quoted 2 lines

(quoted diff) +2 -0

+			if (res < 0)
+				block_good = 0;

No. A block is to be skipped if and only if it has a bad block marker. ECC errors should not be silently ignored just because they happen on the first page of a block. If the writer of this data didn't skip the block, then skipping it when reading will result in unusable data regardless of the underlying ECC problem.

-Scott

Hi Scott,

Le Thu, 19 Mar 2015 16:39:42 -0500, Scott Wood
<[email protected]> a ?crit :

Quoted 7 lines · On Wed, 2015-03-18 at 10:04 +0100, Albert ARIBAUD (3ADEV) wrote:

drivers/mtd/nand/lpc32xx_nand_mlc.c line 673

Quoted 3 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +3 -0

+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
+{
+	int block_good;

bool?

Quoted 16 lines

Quoted 4 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +4 -0

+	struct lpc32xx_oob oob;
+	unsigned int page, left;
+
+	/* if starting mid-block consider block good */

drivers/mtd/nand/lpc32xx_nand_mlc.c line 687

Quoted 10 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +10 -0

+	block_good = 1;
+
+	for (page = offs / BYTES_PER_PAGE,
+	     left = DIV_ROUND_UP(size, BYTES_PER_PAGE);
+	     left && (page < PAGES_PER_CHIP_MAX); page++) {
+		/* read inband and oob page data anyway */
+		int res = read_single_page(dst, page, &oob);
+		/* return error if a good block's page was not readable */
+		if ((res < 0) && block_good)
+			return -1;

Why even try to read it if it's been marked bad?

Actually, at this point, we may not know yet if the block is good or bad, since we might be reading a page of it for the first time. Will fix, see below.

Quoted 4 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c line 688

Quoted 1 line

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +1 -0

+		/* if first page in a block, update 'block good' status */

The non-SPL driver appears to be checking the last two pages in the block, not the first page.

Thanks for pointing out this discrepancy.

I took the first page option here after checking the NAND's datasheet, which says that for factory bad block marking at least, while all pages in a bad block /should/ bear the bad block marker, only the first one is /guaranteed/ to have it. The driver might have inherited the 'last page' option from the previous NAND chip used, or from a mistake of my own.

BTW, is there a standard way to ask a NAND chip which page(s) in a bad block should be checked?

Quoted 4 lines

Quoted 4 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +4 -0

+		if ((page % PAGES_PER_BLOCK) == 0) {
+			/* assume block is good */
+			block_good = 1;
+			/* if page could not be read, assume block is bad */

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+			if (res < 0)
+				block_good = 0;

No. A block is to be skipped if and only if it has a bad block marker. ECC errors should not be silently ignored just because they happen on the first page of a block. If the writer of this data didn't skip the block, then skipping it when reading will result in unusable data regardless of the underlying ECC problem.

You're correct of course.

However, I do want the SPL chainloading to be as resilient as possible, and we have established that it will have to read some part of a bad block -- possibly resulting in a read failure -- before knowing that the block is bad, which means it may have to finally ignore the read failure.

I guess I could wait to declare the block good or bad until I could read at least one if its pages (actually, one of its pages' OOB data) properly, only bail out if the OOB says the block is supposed to be good.

Now, if none of the block's pages could be read, this still prevents us from knowing whether these read failures are catastrophic.

If the block was actually bad and skipped, then the resulting image might be intact, will pass the checksum test, and will run; if the block was actually good, SPL will detect it and not boot the corrupt image -- except if the completely unreadable good block was the first one, which holds the signature and checksum, in which case SPL will fall back to considering a raw, unsigned, unverifiable image, and will jump into corrupt code.

This may happen; but I think the probability of having a completely unreadable sector is very low, and the probability of this sector being the first one of the image is very low too [1].

Which leads me to two possible courses of action:

- consider the risk even though the probabilities are very low, thus consider any totally unreadable sector as good, and bail out in this case, or

- add an option to SPL that prevents common/spl/spl.c from falling back to running the image as raw if no signature is found, and consider any totally unreadable sector as bad and therefore ignore the read errors. Either the sector was actually good, and SPL will catch the image as corrupt or missing a signature, or the sector was actually bad, and the image will run as expected.

Quoted 40 lines

Hi Scott,

Le Thu, 19 Mar 2015 16:39:42 -0500, Scott Wood
<[email protected]> a ?crit :

Quoted 7 lines · On Wed, 2015-03-18 at 10:04 +0100, Albert ARIBAUD (3ADEV) wrote:

drivers/mtd/nand/lpc32xx_nand_mlc.c line 673

Quoted 3 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +3 -0

+int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
+{
+	int block_good;

bool?

Quoted 16 lines

Quoted 4 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +4 -0

+	struct lpc32xx_oob oob;
+	unsigned int page, left;
+
+	/* if starting mid-block consider block good */

drivers/mtd/nand/lpc32xx_nand_mlc.c line 687

Quoted 10 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +10 -0

+	block_good = 1;
+
+	for (page = offs / BYTES_PER_PAGE,
+	     left = DIV_ROUND_UP(size, BYTES_PER_PAGE);
+	     left && (page < PAGES_PER_CHIP_MAX); page++) {
+		/* read inband and oob page data anyway */
+		int res = read_single_page(dst, page, &oob);
+		/* return error if a good block's page was not readable */
+		if ((res < 0) && block_good)
+			return -1;

Why even try to read it if it's been marked bad?

Actually, at this point, we may not know yet if the block is good or bad, since we might be reading a page of it for the first time. Will fix, see below.

Quoted 4 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c line 688

Quoted 1 line

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +1 -0

+		/* if first page in a block, update 'block good' status */

The non-SPL driver appears to be checking the last two pages in the block, not the first page.

Thanks for pointing out this discrepancy.

Quoted 5 lines

I took the first page option here after checking the NAND's datasheet, which says that for factory bad block marking at least, while all pages in a bad block /should/ bear the bad block marker, only the first one is /guaranteed/ to have it. The driver might have inherited the 'last page' option from the previous NAND chip used, or from a mistake of my own.

Are there any plans for this controller to be used with different chips?

Quoted 2 lines

BTW, is there a standard way to ask a NAND chip which page(s) in a bad block should be checked?

Yes and no:
http://lists.infradead.org/pipermail/linux-mtd/2011-November/038419.html

Quoted 13 lines

Quoted 4 lines

Quoted 4 lines

drivers/mtd/nand/lpc32xx_nand_mlc.c new file +4 -0

+		if ((page % PAGES_PER_BLOCK) == 0) {
+			/* assume block is good */
+			block_good = 1;
+			/* if page could not be read, assume block is bad */

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+			if (res < 0)
+				block_good = 0;

No. A block is to be skipped if and only if it has a bad block marker. ECC errors should not be silently ignored just because they happen on the first page of a block. If the writer of this data didn't skip the block, then skipping it when reading will result in unusable data regardless of the underlying ECC problem.

Quoted 7 lines

You're correct of course.

However, I do want the SPL chainloading to be as resilient as possible, and we have established that it will have to read some part of a bad block -- possibly resulting in a read failure -- before knowing that the block is bad, which means it may have to finally ignore the read failure.

FWIW, the eLBC/IFC SPL functions have the same problem regarding halting on an ECC error before checking the bad block status. Instead it should return an error code, and let the caller halt after it's sure it's not marked bad.

Quoted 20 lines

I guess I could wait to declare the block good or bad until I could read at least one if its pages (actually, one of its pages' OOB data) properly, only bail out if the OOB says the block is supposed to be good.

Now, if none of the block's pages could be read, this still prevents us from knowing whether these read failures are catastrophic.

If the block was actually bad and skipped, then the resulting image might be intact, will pass the checksum test, and will run; if the block was actually good, SPL will detect it and not boot the corrupt image -- except if the completely unreadable good block was the first one, which holds the signature and checksum, in which case SPL will fall back to considering a raw, unsigned, unverifiable image, and will jump into corrupt code.

This may happen; but I think the probability of having a completely unreadable sector is very low, and the probability of this sector being the first one of the image is very low too [1].

Quoted 5 lines

Which leads me to two possible courses of action:

- consider the risk even though the probabilities are very low, thus consider any totally unreadable sector as good, and bail out in this case, or

I was thinking instead to distinguish between a hard failure where you got no data from the NAND (e.g. a timeout), versus an ECC failure. In the later case you can still look in the OOB for a bad block marker.

Quoted 6 lines

- add an option to SPL that prevents common/spl/spl.c from falling back to running the image as raw if no signature is found, and consider any totally unreadable sector as bad and therefore ignore the read errors. Either the sector was actually good, and SPL will catch the image as corrupt or missing a signature, or the sector was actually bad, and the image will run as expected.

...but this seems reasonable as well (it wouldn't work for eLBC/IFC since they need to support tiny SPLs that can only handle raw images).

If you want to do this, just put a comment in explaining why you're skipping in that situation.

-Scott

Bonjour Scott,

Le Fri, 20 Mar 2015 17:41:11 -0500, Scott Wood
<[email protected]> a ?crit :

Quoted 6 lines · Albert ARIBAUD

Quoted 2 lines

BTW, is there a standard way to ask a NAND chip which page(s) in a bad block should be checked?

Yes and no:
http://lists.infradead.org/pipermail/linux-mtd/2011-November/038419.html

... right.

How about this: the driver expects a driver-specific configuration option which tells it which page(s) in a block, and which byte in a page's OOB area, should be scanned for bad block markers, and "my" board provides a value for said option.

This leads me to a half-OT question: so those SPL, while too tiny to handle non-raw images, still do include the whole common/spl/spl.c

Quoted 2 lines

If you want to do this, just put a comment in explaining why you're skipping in that situation.

Ok -- I will go with the 'option' method then, and add a (lengthy, I'm afraid) comment in common/spl/spl.c explaining that skipping the raw image handling is required when chainloading from NAND and the NAND driver considers totally unreadable sectors bad, in order not to confuse a missing image header with a raw image.

Quoted 12 lines

Bonjour Scott,

Le Fri, 20 Mar 2015 17:41:11 -0500, Scott Wood
<[email protected]> a ?crit :

Quoted 6 lines · Albert ARIBAUD

Quoted 2 lines

BTW, is there a standard way to ask a NAND chip which page(s) in a bad block should be checked?

Yes and no:
http://lists.infradead.org/pipermail/linux-mtd/2011-November/038419.html

Quoted 6 lines

... right.

How about this: the driver expects a driver-specific configuration option which tells it which page(s) in a block, and which byte in a page's OOB area, should be scanned for bad block markers, and "my" board provides a value for said option.

It looks like the common NAND code will set NAND_BBT_SCANLASTPAGE/NAND_BBT_SCAN2NDPAGE automatically if it sees certain manufacturer IDs, so I don't think drivers should be setting them at all (and currently, none do).

That still leaves the question of what to do in SPL. For simplicity you could check every page as you do the normal read.

Quoted 2 lines

This leads me to a half-OT question: so those SPL, while too tiny to handle non-raw images, still do include the whole common/spl/spl.c

No, there's no room for that.

Quoted 8 lines

Quoted 2 lines

If you want to do this, just put a comment in explaining why you're skipping in that situation.

Ok -- I will go with the 'option' method then, and add a (lengthy, I'm afraid) comment in common/spl/spl.c explaining that skipping the raw image handling is required when chainloading from NAND and the NAND driver considers totally unreadable sectors bad, in order not to confuse a missing image header with a raw image.

Skipping raw wouldn't be needed for all NAND drivers, only those that aren't guaranteed to halt on a legitimate unrecoverable ECC error.

-Scott