mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/3] GPMI NAND xload for i.MX6
@ 2021-01-20 12:51 Andrej Picej
  2021-01-20 12:51 ` [PATCH 1/3] ARM: i.MX: move BCB structures to header file Andrej Picej
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Andrej Picej @ 2021-01-20 12:51 UTC (permalink / raw)
  To: barebox

Hi all,

these patches implement GPMI NAND xload mechanism for i.MX6 SoCs. The xload from
GPMI NAND is needed in case board wants to boot from internal SRAM (limited in
size) and then load remaining barebox image from NAND.

The usual use-case for xload mechanism is board DRAM size auto-detection and
runtime configuration without the need for separate DCD tables for each SoM
variant.

In our case patches were required to support DRAM size auto-detection on
Phytec's phycard-imx6 SoM with 1GiB and 2GiB DRAM variants. Thus we decided to
submit example implementation on the mentioned board as RFC patch that will
follow these patch series.

Nonetheless, the majority of work was already provided by Sascha Hauer, but his
patches were in WIP stage and not found in the mainline. We added missing parts
for detecting NAND's page and oob size necessary for initial FCB page
reading, etc.

We hope patches spawn ideas for future implementations and work!

Best regards,
Andrej Picej

Andrej Picej (1):
  ARM: i.MX: xload-gpmi-nand: apply errata 007117

Primoz Fiser (1):
  ARM: i.MX: move BCB structures to header file

Sascha Hauer (1):
  ARM: i.MX: implement GPMI NAND xload

 arch/arm/mach-imx/Makefile                    |    2 +-
 arch/arm/mach-imx/include/mach/imx-nand-bcb.h |   80 ++
 arch/arm/mach-imx/include/mach/xload.h        |    1 +
 arch/arm/mach-imx/xload-gpmi-nand.c           | 1225 +++++++++++++++++
 common/imx-bbu-nand-fcb.c                     |   69 +-
 5 files changed, 1308 insertions(+), 69 deletions(-)
 create mode 100644 arch/arm/mach-imx/include/mach/imx-nand-bcb.h
 create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c

-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] ARM: i.MX: move BCB structures to header file
  2021-01-20 12:51 [PATCH 0/3] GPMI NAND xload for i.MX6 Andrej Picej
@ 2021-01-20 12:51 ` Andrej Picej
  2021-01-20 12:51 ` [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload Andrej Picej
  2021-01-20 12:51 ` [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117 Andrej Picej
  2 siblings, 0 replies; 17+ messages in thread
From: Andrej Picej @ 2021-01-20 12:51 UTC (permalink / raw)
  To: barebox

From: Primoz Fiser <primoz.fiser@norik.com>

Move i.MX BCB related structures to header file so they can be used by
others.

Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
Signed-off-by: Andrej Picej <andrej.picej@norik.com>
---
 arch/arm/mach-imx/include/mach/imx-nand-bcb.h | 80 +++++++++++++++++++
 common/imx-bbu-nand-fcb.c                     | 69 +---------------
 2 files changed, 81 insertions(+), 68 deletions(-)
 create mode 100644 arch/arm/mach-imx/include/mach/imx-nand-bcb.h

diff --git a/arch/arm/mach-imx/include/mach/imx-nand-bcb.h b/arch/arm/mach-imx/include/mach/imx-nand-bcb.h
new file mode 100644
index 000000000..b60205bd5
--- /dev/null
+++ b/arch/arm/mach-imx/include/mach/imx-nand-bcb.h
@@ -0,0 +1,80 @@
+#ifndef __MACH_IMX_NAND_BCB_H
+#define __MACH_IMX_NAND_BCB_H
+
+#define	FCB_FINGERPRINT		0x20424346	/* 'FCB' */
+#define	FCB_VERSION_1		0x01000000
+#define	FCB_FINGERPRINT_OFF	0x4		/* FCB fingerprint offset*/
+
+#define	DBBT_FINGERPRINT	0x54424244	/* 'DBBT' */
+#define	DBBT_VERSION_1		0x01000000
+#define	DBBT_FINGERPRINT_OFF	0x4		/* DBBT fingerprint offset*/
+
+struct dbbt_block {
+	uint32_t Checksum;
+	uint32_t FingerPrint;
+	uint32_t Version;
+	uint32_t numberBB; /* reserved on i.MX6 */
+	uint32_t DBBTNumOfPages;
+};
+
+struct fcb_block {
+	uint32_t Checksum;		/* First fingerprint in first byte */
+	uint32_t FingerPrint;		/* 2nd fingerprint at byte 4 */
+	uint32_t Version;		/* 3rd fingerprint at byte 8 */
+	uint8_t DataSetup;
+	uint8_t DataHold;
+	uint8_t AddressSetup;
+	uint8_t DSAMPLE_TIME;
+	/* These are for application use only and not for ROM. */
+	uint8_t NandTimingState;
+	uint8_t REA;
+	uint8_t RLOH;
+	uint8_t RHOH;
+	uint32_t PageDataSize;		/* 2048 for 2K pages, 4096 for 4K pages */
+	uint32_t TotalPageSize;		/* 2112 for 2K pages, 4314 for 4K pages */
+	uint32_t SectorsPerBlock;	/* Number of 2K sections per block */
+	uint32_t NumberOfNANDs;		/* Total Number of NANDs - not used by ROM */
+	uint32_t TotalInternalDie;	/* Number of separate chips in this NAND */
+	uint32_t CellType;		/* MLC or SLC */
+	uint32_t EccBlockNEccType;	/* Type of ECC, can be one of BCH-0-20 */
+	uint32_t EccBlock0Size;		/* Number of bytes for Block0 - BCH */
+	uint32_t EccBlockNSize;		/* Block size in bytes for all blocks other than Block0 - BCH */
+	uint32_t EccBlock0EccType;	/* Ecc level for Block 0 - BCH */
+	uint32_t MetadataBytes;		/* Metadata size - BCH */
+	uint32_t NumEccBlocksPerPage;	/* Number of blocks per page for ROM use - BCH */
+	uint32_t EccBlockNEccLevelSDK;	/* Type of ECC, can be one of BCH-0-20 */
+	uint32_t EccBlock0SizeSDK;	/* Number of bytes for Block0 - BCH */
+	uint32_t EccBlockNSizeSDK;	/* Block size in bytes for all blocks other than Block0 - BCH */
+	uint32_t EccBlock0EccLevelSDK;	/* Ecc level for Block 0 - BCH */
+	uint32_t NumEccBlocksPerPageSDK;/* Number of blocks per page for SDK use - BCH */
+	uint32_t MetadataBytesSDK;	/* Metadata size - BCH */
+	uint32_t EraseThreshold;	/* To set into BCH_MODE register */
+	uint32_t BootPatch;		/* 0 for normal boot and 1 to load patch starting next to FCB */
+	uint32_t PatchSectors;		/* Size of patch in sectors */
+	uint32_t Firmware1_startingPage;/* Firmware image starts on this sector */
+	uint32_t Firmware2_startingPage;/* Secondary FW Image starting Sector */
+	uint32_t PagesInFirmware1;	/* Number of sectors in firmware image */
+	uint32_t PagesInFirmware2;	/* Number of sector in secondary FW image */
+	uint32_t DBBTSearchAreaStartAddress; /* Page address where dbbt search area begins */
+	uint32_t BadBlockMarkerByte;	/* Byte in page data that have manufacturer marked bad block marker, */
+					/* this will be swapped with metadata[0] to complete page data. */
+	uint32_t BadBlockMarkerStartBit;/* For BCH ECC sizes other than 8 and 16 the bad block marker does not */
+					/* start at 0th bit of BadBlockMarkerByte. This field is used to get to */
+					/* the start bit of bad block marker byte with in BadBlockMarkerByte */
+	uint32_t BBMarkerPhysicalOffset;/* FCB value that gives byte offset for bad block marker on physical NAND page */
+	uint32_t BCHType;
+
+	uint32_t TMTiming2_ReadLatency;
+	uint32_t TMTiming2_PreambleDelay;
+	uint32_t TMTiming2_CEDelay;
+	uint32_t TMTiming2_PostambleDelay;
+	uint32_t TMTiming2_CmdAddPause;
+	uint32_t TMTiming2_DataPause;
+	uint32_t TMSpeed;
+	uint32_t TMTiming1_BusyTimeout;
+
+	uint32_t DISBBM;	/* the flag to enable (1)/disable(0) bi swap */
+	uint32_t BBMarkerPhysicalOffsetInSpareData; /* The swap position of main area in spare area */
+};
+
+#endif /* __MACH_IMX_NAND_BCB_H */
diff --git a/common/imx-bbu-nand-fcb.c b/common/imx-bbu-nand-fcb.c
index f8c457805..a08a1d834 100644
--- a/common/imx-bbu-nand-fcb.c
+++ b/common/imx-bbu-nand-fcb.c
@@ -25,6 +25,7 @@
 #include <crc.h>
 #include <mach/generic.h>
 #include <mtd/mtd-peb.h>
+#include <mach/imx-nand-bcb.h>
 
 #ifdef CONFIG_ARCH_IMX6
 #include <mach/imx6.h>
@@ -39,74 +40,6 @@ static inline int fcb_is_bch_encoded(void)
 }
 #endif
 
-struct dbbt_block {
-	uint32_t Checksum;
-	uint32_t FingerPrint;
-	uint32_t Version;
-	uint32_t numberBB; /* reserved on i.MX6 */
-	uint32_t DBBTNumOfPages;
-};
-
-struct fcb_block {
-	uint32_t Checksum;		/* First fingerprint in first byte */
-	uint32_t FingerPrint;		/* 2nd fingerprint at byte 4 */
-	uint32_t Version;		/* 3rd fingerprint at byte 8 */
-	uint8_t DataSetup;
-	uint8_t DataHold;
-	uint8_t AddressSetup;
-	uint8_t DSAMPLE_TIME;
-	/* These are for application use only and not for ROM. */
-	uint8_t NandTimingState;
-	uint8_t REA;
-	uint8_t RLOH;
-	uint8_t RHOH;
-	uint32_t PageDataSize;		/* 2048 for 2K pages, 4096 for 4K pages */
-	uint32_t TotalPageSize;		/* 2112 for 2K pages, 4314 for 4K pages */
-	uint32_t SectorsPerBlock;	/* Number of 2K sections per block */
-	uint32_t NumberOfNANDs;		/* Total Number of NANDs - not used by ROM */
-	uint32_t TotalInternalDie;	/* Number of separate chips in this NAND */
-	uint32_t CellType;		/* MLC or SLC */
-	uint32_t EccBlockNEccType;	/* Type of ECC, can be one of BCH-0-20 */
-	uint32_t EccBlock0Size;		/* Number of bytes for Block0 - BCH */
-	uint32_t EccBlockNSize;		/* Block size in bytes for all blocks other than Block0 - BCH */
-	uint32_t EccBlock0EccType;	/* Ecc level for Block 0 - BCH */
-	uint32_t MetadataBytes;		/* Metadata size - BCH */
-	uint32_t NumEccBlocksPerPage;	/* Number of blocks per page for ROM use - BCH */
-	uint32_t EccBlockNEccLevelSDK;	/* Type of ECC, can be one of BCH-0-20 */
-	uint32_t EccBlock0SizeSDK;	/* Number of bytes for Block0 - BCH */
-	uint32_t EccBlockNSizeSDK;	/* Block size in bytes for all blocks other than Block0 - BCH */
-	uint32_t EccBlock0EccLevelSDK;	/* Ecc level for Block 0 - BCH */
-	uint32_t NumEccBlocksPerPageSDK;/* Number of blocks per page for SDK use - BCH */
-	uint32_t MetadataBytesSDK;	/* Metadata size - BCH */
-	uint32_t EraseThreshold;	/* To set into BCH_MODE register */
-	uint32_t BootPatch;		/* 0 for normal boot and 1 to load patch starting next to FCB */
-	uint32_t PatchSectors;		/* Size of patch in sectors */
-	uint32_t Firmware1_startingPage;/* Firmware image starts on this sector */
-	uint32_t Firmware2_startingPage;/* Secondary FW Image starting Sector */
-	uint32_t PagesInFirmware1;	/* Number of sectors in firmware image */
-	uint32_t PagesInFirmware2;	/* Number of sector in secondary FW image */
-	uint32_t DBBTSearchAreaStartAddress; /* Page address where dbbt search area begins */
-	uint32_t BadBlockMarkerByte;	/* Byte in page data that have manufacturer marked bad block marker, */
-					/* this will be swapped with metadata[0] to complete page data. */
-	uint32_t BadBlockMarkerStartBit;/* For BCH ECC sizes other than 8 and 16 the bad block marker does not */
-					/* start at 0th bit of BadBlockMarkerByte. This field is used to get to */
-					/* the start bit of bad block marker byte with in BadBlockMarkerByte */
-	uint32_t BBMarkerPhysicalOffset;/* FCB value that gives byte offset for bad block marker on physical NAND page */
-	uint32_t BCHType;
-
-	uint32_t TMTiming2_ReadLatency;
-	uint32_t TMTiming2_PreambleDelay;
-	uint32_t TMTiming2_CEDelay;
-	uint32_t TMTiming2_PostambleDelay;
-	uint32_t TMTiming2_CmdAddPause;
-	uint32_t TMTiming2_DataPause;
-	uint32_t TMSpeed;
-	uint32_t TMTiming1_BusyTimeout;
-
-	uint32_t DISBBM;	/* the flag to enable (1)/disable(0) bi swap */
-	uint32_t BBMarkerPhysicalOffsetInSpareData; /* The swap position of main area in spare area */
-};
-
 struct imx_nand_fcb_bbu_handler {
 	struct bbu_handler handler;
 
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
  2021-01-20 12:51 [PATCH 0/3] GPMI NAND xload for i.MX6 Andrej Picej
  2021-01-20 12:51 ` [PATCH 1/3] ARM: i.MX: move BCB structures to header file Andrej Picej
@ 2021-01-20 12:51 ` Andrej Picej
  2021-01-20 21:45   ` Roland Hieber
  2021-01-21  9:01   ` Sascha Hauer
  2021-01-20 12:51 ` [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117 Andrej Picej
  2 siblings, 2 replies; 17+ messages in thread
From: Andrej Picej @ 2021-01-20 12:51 UTC (permalink / raw)
  To: barebox

From: Sascha Hauer <s.hauer@pengutronix.de>

Commit is based on initial Sascha Hauer's work. It implements PBL xload
mechanism to load image from GPMI NAND flash.

Additional work was done, so that the NAND's size, page size and OOB's
size are autodetected and not hardcoded. Detection method follows the
same methods as used in NAND driver, meaning NAND ONFI support is probed
and if NAND supports ONFI, NAND memory organization is read from ONFI
parameter page otherwise "READ ID" is used.

Currently only implemented for i.MX6 familly of SoCs.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
Signed-off-by: Andrej Picej <andrej.picej@norik.com>
---
 arch/arm/mach-imx/Makefile             |    2 +-
 arch/arm/mach-imx/include/mach/xload.h |    1 +
 arch/arm/mach-imx/xload-gpmi-nand.c    | 1163 ++++++++++++++++++++++++
 3 files changed, 1165 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c

diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index e45f758e9..d94c846a1 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -26,4 +26,4 @@ obj-$(CONFIG_BAREBOX_UPDATE) += imx-bbu-internal.o
 obj-$(CONFIG_BAREBOX_UPDATE_IMX_EXTERNAL_NAND) += imx-bbu-external-nand.o
 obj-$(CONFIG_RESET_IMX_SRC) += src.o
 lwl-y += cpu_init.o
-pbl-y += xload-spi.o xload-common.o xload-imx-nand.o
+pbl-y += xload-spi.o xload-common.o xload-imx-nand.o xload-gpmi-nand.o
diff --git a/arch/arm/mach-imx/include/mach/xload.h b/arch/arm/mach-imx/include/mach/xload.h
index 94b2f3761..7187787f5 100644
--- a/arch/arm/mach-imx/include/mach/xload.h
+++ b/arch/arm/mach-imx/include/mach/xload.h
@@ -5,6 +5,7 @@ int imx53_nand_start_image(void);
 int imx6_spi_load_image(int instance, unsigned int flash_offset, void *buf, int len);
 int imx6_spi_start_image(int instance);
 int imx6_esdhc_start_image(int instance);
+int imx6_nand_start_image(void);
 int imx7_esdhc_start_image(int instance);
 int imx8m_esdhc_load_image(int instance, bool start);
 int imx8mp_esdhc_load_image(int instance, bool start);
diff --git a/arch/arm/mach-imx/xload-gpmi-nand.c b/arch/arm/mach-imx/xload-gpmi-nand.c
new file mode 100644
index 000000000..04f799604
--- /dev/null
+++ b/arch/arm/mach-imx/xload-gpmi-nand.c
@@ -0,0 +1,1163 @@
+/*
+ * This program 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; version 2.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#define pr_fmt(fmt) "xload-gpmi-nand: " fmt
+
+#include <common.h>
+#include <stmp-device.h>
+#include <asm-generic/io.h>
+#include <linux/sizes.h>
+#include <linux/mtd/nand.h>
+#include <asm/cache.h>
+#include <mach/xload.h>
+#include <mach/imx-nand-bcb.h>
+#include <linux/mtd/rawnand.h>
+
+/*
+ * MXS DMA hardware command.
+ *
+ * This structure describes the in-memory layout of an entire DMA command,
+ * including space for the maximum number of PIO accesses. See the appropriate
+ * reference manual for a detailed description of what these fields mean to the
+ * DMA hardware.
+ */
+#define	DMACMD_COMMAND_DMA_WRITE	0x1
+#define	DMACMD_COMMAND_DMA_READ		0x2
+#define	DMACMD_COMMAND_DMA_SENSE	0x3
+#define	DMACMD_CHAIN			(1 << 2)
+#define	DMACMD_IRQ			(1 << 3)
+#define	DMACMD_NAND_LOCK		(1 << 4)
+#define	DMACMD_NAND_WAIT_4_READY	(1 << 5)
+#define	DMACMD_DEC_SEM			(1 << 6)
+#define	DMACMD_WAIT4END			(1 << 7)
+#define	DMACMD_HALT_ON_TERMINATE	(1 << 8)
+#define	DMACMD_TERMINATE_FLUSH		(1 << 9)
+#define	DMACMD_PIO_WORDS(words)		((words) << 12)
+#define	DMACMD_XFER_COUNT(x)		((x) << 16)
+
+struct mxs_dma_cmd {
+	unsigned long		next;
+	unsigned long		data;
+	unsigned long		address;
+#define	APBH_DMA_PIO_WORDS	6
+	unsigned long		pio_words[APBH_DMA_PIO_WORDS];
+};
+
+enum mxs_dma_id {
+	IMX23_DMA,
+	IMX28_DMA,
+};
+
+struct apbh_dma {
+	void __iomem *regs;
+	enum mxs_dma_id id;
+};
+
+struct mxs_dma_chan {
+	unsigned int flags;
+	int channel;
+	struct apbh_dma *apbh;
+};
+
+#define	HW_APBHX_CTRL0				0x000
+#define	BM_APBH_CTRL0_APB_BURST8_EN		(1 << 29)
+#define	BM_APBH_CTRL0_APB_BURST_EN		(1 << 28)
+#define	BP_APBH_CTRL0_CLKGATE_CHANNEL		8
+#define	BP_APBH_CTRL0_RESET_CHANNEL		16
+#define	HW_APBHX_CTRL1				0x010
+#define	BP_APBHX_CTRL1_CH_CMDCMPLT_IRQ_EN	16
+#define	HW_APBHX_CTRL2				0x020
+#define	HW_APBHX_CHANNEL_CTRL			0x030
+#define	BP_APBHX_CHANNEL_CTRL_RESET_CHANNEL	16
+#define	BP_APBHX_VERSION_MAJOR			24
+#define	HW_APBHX_CHn_NXTCMDAR_MX23(n)		(0x050 + (n) * 0x70)
+#define	HW_APBHX_CHn_NXTCMDAR_MX28(n)		(0x110 + (n) * 0x70)
+#define	HW_APBHX_CHn_SEMA_MX23(n)		(0x080 + (n) * 0x70)
+#define	HW_APBHX_CHn_SEMA_MX28(n)		(0x140 + (n) * 0x70)
+#define	NAND_ONFI_CRC_BASE			0x4f4e
+
+#define apbh_dma_is_imx23(aphb) ((apbh)->id == IMX23_DMA)
+
+/* udelay() is not available in PBL, need to improvise */
+static void __udelay(int us)
+{
+	volatile int i;
+
+	for (i = 0; i < us * 4; i++);
+}
+
+/*
+ * Enable a DMA channel.
+ *
+ * If the given channel has any DMA descriptors on its active list, this
+ * function causes the DMA hardware to begin processing them.
+ *
+ * This function marks the DMA channel as "busy," whether or not there are any
+ * descriptors to process.
+ */
+static int mxs_dma_enable(struct mxs_dma_chan *pchan,
+	struct mxs_dma_cmd *pdesc)
+{
+	struct apbh_dma *apbh = pchan->apbh;
+	int channel_bit;
+	int channel = pchan->channel;
+
+	if (apbh_dma_is_imx23(apbh)) {
+		writel((uint32_t)pdesc,
+			apbh->regs + HW_APBHX_CHn_NXTCMDAR_MX23(channel));
+		writel(1, apbh->regs + HW_APBHX_CHn_SEMA_MX23(channel));
+		channel_bit = channel + BP_APBH_CTRL0_CLKGATE_CHANNEL;
+	} else {
+		writel((uint32_t)pdesc,
+			apbh->regs + HW_APBHX_CHn_NXTCMDAR_MX28(channel));
+		writel(1, apbh->regs + HW_APBHX_CHn_SEMA_MX28(channel));
+		channel_bit = channel;
+	}
+
+	writel(1 << channel_bit, apbh->regs +
+		HW_APBHX_CTRL0 + STMP_OFFSET_REG_CLR);
+
+	return 0;
+}
+
+/*
+ * Resets the DMA channel hardware.
+ */
+static int mxs_dma_reset(struct mxs_dma_chan *pchan)
+{
+	struct apbh_dma *apbh = pchan->apbh;
+
+	if (apbh_dma_is_imx23(apbh))
+		writel(1 << (pchan->channel + BP_APBH_CTRL0_RESET_CHANNEL),
+			apbh->regs + HW_APBHX_CTRL0 + STMP_OFFSET_REG_SET);
+	else
+		writel(1 << (pchan->channel +
+			BP_APBHX_CHANNEL_CTRL_RESET_CHANNEL),
+			apbh->regs + HW_APBHX_CHANNEL_CTRL +
+			STMP_OFFSET_REG_SET);
+
+	return 0;
+}
+
+static int mxs_dma_wait_complete(struct mxs_dma_chan *pchan)
+{
+	struct apbh_dma *apbh = pchan->apbh;
+	int timeout = 1000000;
+
+	while (1) {
+		if (readl(apbh->regs + HW_APBHX_CTRL1) & (1 << pchan->channel))
+			return 0;
+
+		if (!timeout--)
+			return -ETIMEDOUT;
+	}
+}
+
+/*
+ * Execute the DMA channel
+ */
+static int mxs_dma_run(struct mxs_dma_chan *pchan, struct mxs_dma_cmd *pdesc,
+		int num)
+{
+	struct apbh_dma *apbh = pchan->apbh;
+	int i, ret;
+
+	/* chain descriptors */
+	for (i = 0; i < num - 1; i++) {
+		pdesc[i].next = (uint32_t)(&pdesc[i + 1]);
+		pdesc[i].data |= DMACMD_CHAIN;
+	}
+
+	writel(1 << (pchan->channel + BP_APBHX_CTRL1_CH_CMDCMPLT_IRQ_EN),
+		apbh->regs + HW_APBHX_CTRL1 + STMP_OFFSET_REG_SET);
+
+	ret = mxs_dma_enable(pchan, pdesc);
+	if (ret) {
+		pr_err("%s: Failed to enable dma channel: %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	ret = mxs_dma_wait_complete(pchan);
+	if (ret) {
+		pr_err("%s: Failed to wait for completion: %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	/* Shut the DMA channel down. */
+	writel(1 << pchan->channel, apbh->regs +
+		HW_APBHX_CTRL1 + STMP_OFFSET_REG_CLR);
+	writel(1 << pchan->channel, apbh->regs +
+		HW_APBHX_CTRL2 + STMP_OFFSET_REG_CLR);
+
+	mxs_dma_reset(pchan);
+
+	writel(1 << (pchan->channel + BP_APBHX_CTRL1_CH_CMDCMPLT_IRQ_EN),
+		apbh->regs + HW_APBHX_CTRL1 + STMP_OFFSET_REG_CLR);
+
+	return 0;
+}
+
+/* ----------------------------- NAND driver part -------------------------- */
+
+#define	GPMI_CTRL0					0x00000000
+#define	GPMI_CTRL0_RUN					(1 << 29)
+#define	GPMI_CTRL0_DEV_IRQ_EN				(1 << 28)
+#define	GPMI_CTRL0_UDMA					(1 << 26)
+#define	GPMI_CTRL0_COMMAND_MODE_MASK			(0x3 << 24)
+#define	GPMI_CTRL0_COMMAND_MODE_OFFSET			24
+#define	GPMI_CTRL0_COMMAND_MODE_WRITE			(0x0 << 24)
+#define	GPMI_CTRL0_COMMAND_MODE_READ			(0x1 << 24)
+#define	GPMI_CTRL0_COMMAND_MODE_READ_AND_COMPARE	(0x2 << 24)
+#define	GPMI_CTRL0_COMMAND_MODE_WAIT_FOR_READY		(0x3 << 24)
+#define	GPMI_CTRL0_WORD_LENGTH				(1 << 23)
+#define	GPMI_CTRL0_CS(cs)				((cs) << 20)
+#define	GPMI_CTRL0_ADDRESS_MASK				(0x7 << 17)
+#define	GPMI_CTRL0_ADDRESS_OFFSET			17
+#define	GPMI_CTRL0_ADDRESS_NAND_DATA			(0x0 << 17)
+#define	GPMI_CTRL0_ADDRESS_NAND_CLE			(0x1 << 17)
+#define	GPMI_CTRL0_ADDRESS_NAND_ALE			(0x2 << 17)
+#define	GPMI_CTRL0_ADDRESS_INCREMENT			(1 << 16)
+#define	GPMI_CTRL0_XFER_COUNT_MASK			0xffff
+#define	GPMI_CTRL0_XFER_COUNT_OFFSET			0
+
+#define	GPMI_ECCCTRL_ECC_CMD_DECODE			(0x0 << 13)
+#define	GPMI_ECCCTRL_ENABLE_ECC				(1 << 12)
+#define	GPMI_ECCCTRL_BUFFER_MASK_BCH_PAGE		0x1ff
+
+#define	BCH_CTRL					0x00000000
+#define	BCH_CTRL_COMPLETE_IRQ				(1 << 0)
+
+#define	MXS_NAND_DMA_DESCRIPTOR_COUNT			6
+#define	MXS_NAND_CHUNK_DATA_CHUNK_SIZE			512
+#define	MXS_NAND_METADATA_SIZE				10
+#define	MXS_NAND_COMMAND_BUFFER_SIZE			128
+
+struct mxs_nand_info {
+	void __iomem *io_base;
+	void __iomem *bch_base;
+	struct mxs_dma_chan *dma_channel;
+	int cs;
+	uint8_t *cmd_buf;
+	struct mxs_dma_cmd *desc;
+	struct fcb_block fcb;
+	int dbbt_num_entries;
+	u32 *dbbt;
+	struct nand_memory_organization organization;
+	unsigned long nand_size;
+};
+
+static uint32_t mxs_nand_aux_status_offset(void)
+{
+	return (MXS_NAND_METADATA_SIZE + 0x3) & ~0x3;
+}
+
+static int mxs_nand_read_page(struct mxs_nand_info *info, int writesize,
+		int oobsize, int pagenum, void *databuf, int raw)
+{
+	void __iomem *bch_regs = info->bch_base;
+	unsigned column = 0;
+	struct mxs_dma_cmd *d;
+	int cmd_queue_len;
+	u8 *cmd_buf;
+	int ret;
+	uint8_t	*status;
+	int i;
+	int timeout;
+	int descnum = 0;
+	int max_pagenum = info->nand_size /
+		info->organization.pagesize;
+
+	memset(info->desc, 0,
+		sizeof(*info->desc) * MXS_NAND_DMA_DESCRIPTOR_COUNT);
+
+	/* Compile DMA descriptor - read0 */
+	cmd_buf = info->cmd_buf;
+	cmd_queue_len = 0;
+	d = &info->desc[descnum++];
+	d->address = (dma_addr_t)(cmd_buf);
+	cmd_buf[cmd_queue_len++] = NAND_CMD_READ0;
+	cmd_buf[cmd_queue_len++] = column;
+	cmd_buf[cmd_queue_len++] = column >> 8;
+	cmd_buf[cmd_queue_len++] = pagenum;
+	cmd_buf[cmd_queue_len++] = pagenum >> 8;
+
+	if ((max_pagenum - 1) >= SZ_64K)
+		cmd_buf[cmd_queue_len++] = pagenum >> 16;
+
+	d->data = DMACMD_COMMAND_DMA_READ |
+		DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(cmd_queue_len);
+
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WRITE |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_CLE |
+		GPMI_CTRL0_ADDRESS_INCREMENT |
+		cmd_queue_len;
+
+	/* Compile DMA descriptor - readstart */
+	cmd_buf = &info->cmd_buf[8];
+	cmd_queue_len = 0;
+	d = &info->desc[descnum++];
+	d->address = (dma_addr_t)(cmd_buf);
+
+	cmd_buf[cmd_queue_len++] = NAND_CMD_READSTART;
+
+	d->data = DMACMD_COMMAND_DMA_READ |
+		DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(cmd_queue_len);
+
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WRITE |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_CLE |
+		GPMI_CTRL0_ADDRESS_INCREMENT |
+		cmd_queue_len;
+
+	/* Compile DMA descriptor - wait for ready. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_CHAIN |
+		DMACMD_NAND_WAIT_4_READY |
+		DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(2);
+
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WAIT_FOR_READY |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_DATA;
+
+	if (raw) {
+		/* Compile DMA descriptor - read. */
+		d = &info->desc[descnum++];
+		d->data = DMACMD_WAIT4END |
+			DMACMD_PIO_WORDS(1) |
+			DMACMD_XFER_COUNT(writesize + oobsize) |
+			DMACMD_COMMAND_DMA_WRITE;
+		d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_READ |
+			GPMI_CTRL0_WORD_LENGTH |
+			GPMI_CTRL0_CS(info->cs) |
+			GPMI_CTRL0_ADDRESS_NAND_DATA |
+			(writesize + oobsize);
+		d->address = (dma_addr_t)databuf;
+	} else {
+		/* Compile DMA descriptor - enable the BCH block and read. */
+		d = &info->desc[descnum++];
+		d->data = DMACMD_WAIT4END | DMACMD_PIO_WORDS(6);
+		d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_READ |
+			GPMI_CTRL0_WORD_LENGTH |
+			GPMI_CTRL0_CS(info->cs) |
+			GPMI_CTRL0_ADDRESS_NAND_DATA |
+			(writesize + oobsize);
+		d->pio_words[1] = 0;
+		d->pio_words[2] = GPMI_ECCCTRL_ENABLE_ECC |
+			GPMI_ECCCTRL_ECC_CMD_DECODE |
+			GPMI_ECCCTRL_BUFFER_MASK_BCH_PAGE;
+		d->pio_words[3] = writesize + oobsize;
+		d->pio_words[4] = (dma_addr_t)databuf;
+		d->pio_words[5] = (dma_addr_t)(databuf + writesize);
+
+		/* Compile DMA descriptor - disable the BCH block. */
+		d = &info->desc[descnum++];
+		d->data = DMACMD_NAND_WAIT_4_READY |
+			DMACMD_WAIT4END |
+			DMACMD_PIO_WORDS(3);
+		d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WAIT_FOR_READY |
+			GPMI_CTRL0_WORD_LENGTH |
+			GPMI_CTRL0_CS(info->cs) |
+			GPMI_CTRL0_ADDRESS_NAND_DATA |
+			(writesize + oobsize);
+	}
+
+	/* Compile DMA descriptor - de-assert the NAND lock and interrupt. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_IRQ | DMACMD_DEC_SEM;
+
+	/* Execute the DMA chain. */
+	ret = mxs_dma_run(info->dma_channel, info->desc, descnum);
+	if (ret) {
+		pr_err("DMA read error\n");
+		goto err;
+	}
+
+	if (raw)
+		return 0;
+
+	timeout = 1000000;
+
+	while (1) {
+		if (!timeout--) {
+			ret = -ETIMEDOUT;
+			goto err;
+		}
+
+		if (readl(bch_regs + BCH_CTRL) & BCH_CTRL_COMPLETE_IRQ)
+			break;
+	}
+
+	writel(BCH_CTRL_COMPLETE_IRQ,
+		bch_regs + BCH_CTRL + STMP_OFFSET_REG_CLR);
+
+	/* Loop over status bytes, accumulating ECC status. */
+	status = databuf + writesize + mxs_nand_aux_status_offset();
+	for (i = 0; i < writesize / MXS_NAND_CHUNK_DATA_CHUNK_SIZE; i++) {
+		if (status[i] == 0xfe) {
+			ret = -EBADMSG;
+			goto err;
+		}
+	}
+
+	ret = 0;
+err:
+	return ret;
+}
+
+static int mxs_nand_get_read_status(struct mxs_nand_info *info, void *databuf)
+{
+	int ret;
+	u8 *cmd_buf;
+	struct mxs_dma_cmd *d;
+	int descnum = 0;
+	int cmd_queue_len;
+
+	memset(info->desc, 0,
+		sizeof(*info->desc) * MXS_NAND_DMA_DESCRIPTOR_COUNT);
+
+	/* Compile DMA descriptor - READ STATUS */
+	cmd_buf = info->cmd_buf;
+	cmd_queue_len = 0;
+	d = &info->desc[descnum++];
+	d->address = (dma_addr_t)(cmd_buf);
+	cmd_buf[cmd_queue_len++] = NAND_CMD_STATUS;
+
+	d->data = DMACMD_COMMAND_DMA_READ |
+		DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(cmd_queue_len);
+
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WRITE |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_CLE |
+		GPMI_CTRL0_ADDRESS_INCREMENT |
+		cmd_queue_len;
+
+	/* Compile DMA descriptor - read. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(1) |
+		DMACMD_COMMAND_DMA_WRITE;
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_READ |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_DATA |
+		(1);
+	d->address = (dma_addr_t)databuf;
+
+	/* Compile DMA descriptor - de-assert the NAND lock and interrupt. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_IRQ | DMACMD_DEC_SEM;
+
+	/* Execute the DMA chain. */
+	ret = mxs_dma_run(info->dma_channel, info->desc, descnum);
+	if (ret) {
+		pr_err("DMA read error\n");
+		return ret;
+	}
+
+	return ret;
+}
+
+static int mxs_nand_reset(struct mxs_nand_info *info, void *databuf)
+{
+	int ret, i;
+	u8 *cmd_buf;
+	struct mxs_dma_cmd *d;
+	int descnum = 0;
+	int cmd_queue_len;
+	u8 read_status;
+
+	memset(info->desc, 0,
+		sizeof(*info->desc) * MXS_NAND_DMA_DESCRIPTOR_COUNT);
+
+	/* Compile DMA descriptor - RESET */
+	cmd_buf = info->cmd_buf;
+	cmd_queue_len = 0;
+	d = &info->desc[descnum++];
+	d->address = (dma_addr_t)(cmd_buf);
+	cmd_buf[cmd_queue_len++] = NAND_CMD_RESET;
+
+	d->data = DMACMD_COMMAND_DMA_READ |
+		DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(cmd_queue_len);
+
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WRITE |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_CLE |
+		GPMI_CTRL0_ADDRESS_INCREMENT |
+		cmd_queue_len;
+
+	/* Compile DMA descriptor - de-assert the NAND lock and interrupt. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_IRQ | DMACMD_DEC_SEM;
+
+	/* Execute the DMA chain. */
+	ret = mxs_dma_run(info->dma_channel, info->desc, descnum);
+	if (ret) {
+		pr_err("DMA read error\n");
+		return ret;
+	}
+
+	/* Wait for NAND to wake up */
+	for (i = 0; i < 10; i++) {
+		__udelay(50000);
+		ret = mxs_nand_get_read_status(info, databuf);
+		if (ret)
+			return ret;
+		memcpy(&read_status, databuf, 1);
+		if (read_status == 0xe0)
+			return 0;
+	}
+
+	pr_warn("NAND Reset failed\n");
+	return -1;
+}
+
+/* function taken from nand_onfi.c */
+static u16 onfi_crc16(u16 crc, u8 const *p, size_t len)
+{
+	int i;
+
+	while (len--) {
+		crc ^= *p++ << 8;
+		for (i = 0; i < 8; i++)
+			crc = (crc << 1) ^ ((crc & 0x8000) ? 0x8005 : 0);
+	}
+	return crc;
+}
+
+static int mxs_nand_get_onfi(struct mxs_nand_info *info, void *databuf)
+{
+	int ret;
+	u8 *cmd_buf;
+	u16 crc;
+	struct mxs_dma_cmd *d;
+	int descnum = 0;
+	int cmd_queue_len;
+	struct nand_onfi_params nand_params;
+
+	memset(info->desc, 0,
+		sizeof(*info->desc) * MXS_NAND_DMA_DESCRIPTOR_COUNT);
+
+	/* Compile DMA descriptor - READ PARAMETER PAGE */
+	cmd_buf = info->cmd_buf;
+	cmd_queue_len = 0;
+	d = &info->desc[descnum++];
+	d->address = (dma_addr_t)(cmd_buf);
+	cmd_buf[cmd_queue_len++] = NAND_CMD_PARAM;
+	cmd_buf[cmd_queue_len++] = 0x00;
+
+	d->data = DMACMD_COMMAND_DMA_READ |
+		DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(cmd_queue_len);
+
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WRITE |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_CLE |
+		GPMI_CTRL0_ADDRESS_INCREMENT |
+		cmd_queue_len;
+
+	/* Compile DMA descriptor - wait for ready. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_CHAIN |
+		DMACMD_NAND_WAIT_4_READY |
+		DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(2);
+
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WAIT_FOR_READY |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_DATA;
+
+	/* Compile DMA descriptor - read. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(sizeof(struct nand_onfi_params)) |
+		DMACMD_COMMAND_DMA_WRITE;
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_READ |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_DATA |
+		(sizeof(struct nand_onfi_params));
+	d->address = (dma_addr_t)databuf;
+
+	/* Compile DMA descriptor - de-assert the NAND lock and interrupt. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_IRQ | DMACMD_DEC_SEM;
+
+	/* Execute the DMA chain. */
+	ret = mxs_dma_run(info->dma_channel, info->desc, descnum);
+	if (ret) {
+		pr_err("DMA read error\n");
+		return ret;
+	}
+
+	memcpy(&nand_params, databuf, sizeof(struct nand_onfi_params));
+
+	crc = onfi_crc16(NAND_ONFI_CRC_BASE, (u8 *)&nand_params, 254);
+	pr_debug("ONFI CRC: 0x%x, CALC. CRC 0x%x\n", nand_params.crc, crc);
+	if (crc != le16_to_cpu(nand_params.crc)) {
+		pr_debug("ONFI CRC mismatch!\n");
+		ret = -EUCLEAN;
+		return ret;
+	}
+
+	/* Fill the NAND organization struct with data */
+	info->organization.bits_per_cell = nand_params.bits_per_cell;
+	info->organization.pagesize = le32_to_cpu(nand_params.byte_per_page);
+	info->organization.oobsize =
+		le16_to_cpu(nand_params.spare_bytes_per_page);
+	info->organization.pages_per_eraseblock =
+		le32_to_cpu(nand_params.pages_per_block);
+	info->organization.eraseblocks_per_lun =
+		le32_to_cpu(nand_params.blocks_per_lun);
+	info->organization.max_bad_eraseblocks_per_lun =
+		le16_to_cpu(nand_params.bb_per_lun);
+	info->organization.luns_per_target = nand_params.lun_count;
+	info->nand_size = info->organization.pagesize *
+		info->organization.pages_per_eraseblock *
+		info->organization.eraseblocks_per_lun *
+		info->organization.luns_per_target;
+
+	return ret;
+}
+
+static int mxs_nand_check_onfi(struct mxs_nand_info *info, void *databuf)
+{
+	int ret;
+	u8 *cmd_buf;
+	struct mxs_dma_cmd *d;
+	int descnum = 0;
+	int cmd_queue_len;
+
+	struct onfi_header {
+		u8 byte0;
+		u8 byte1;
+		u8 byte2;
+		u8 byte3;
+	} onfi_head;
+
+	memset(info->desc, 0,
+		sizeof(*info->desc) * MXS_NAND_DMA_DESCRIPTOR_COUNT);
+
+	/* Compile DMA descriptor - READID + 0x20 (ADDR) */
+	cmd_buf = info->cmd_buf;
+	cmd_queue_len = 0;
+	d = &info->desc[descnum++];
+	d->address = (dma_addr_t)(cmd_buf);
+	cmd_buf[cmd_queue_len++] = NAND_CMD_READID;
+	cmd_buf[cmd_queue_len++] = 0x20;
+
+	d->data = DMACMD_COMMAND_DMA_READ |
+		DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(cmd_queue_len);
+
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WRITE |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_CLE |
+		GPMI_CTRL0_ADDRESS_INCREMENT |
+		cmd_queue_len;
+
+	/* Compile DMA descriptor - read. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(sizeof(struct onfi_header)) |
+		DMACMD_COMMAND_DMA_WRITE;
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_READ |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_DATA |
+		(sizeof(struct onfi_header));
+	d->address = (dma_addr_t)databuf;
+
+	/* Compile DMA descriptor - de-assert the NAND lock and interrupt. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_IRQ | DMACMD_DEC_SEM;
+
+	/* Execute the DMA chain. */
+	ret = mxs_dma_run(info->dma_channel, info->desc, descnum);
+	if (ret) {
+		pr_err("DMA read error\n");
+		return ret;
+	}
+
+	memcpy(&onfi_head, databuf, sizeof(struct onfi_header));
+
+	pr_debug("ONFI Byte0: 0x%x\n", onfi_head.byte0);
+	pr_debug("ONFI Byte1: 0x%x\n", onfi_head.byte1);
+	pr_debug("ONFI Byte2: 0x%x\n", onfi_head.byte2);
+	pr_debug("ONFI Byte3: 0x%x\n", onfi_head.byte3);
+
+	/* check if returned values correspond to ascii characters "ONFI" */
+	if (onfi_head.byte0 != 0x4f || onfi_head.byte1 != 0x4e ||
+		onfi_head.byte2 != 0x46 || onfi_head.byte3 != 0x49) {
+		pr_warn("ONFI not supported\n");
+		return 1;
+	}
+
+	return 0;
+}
+
+static int mxs_nand_get_readid(struct mxs_nand_info *info, void *databuf)
+{
+	int ret;
+	u8 *cmd_buf;
+	struct mxs_dma_cmd *d;
+	int descnum = 0;
+	int cmd_queue_len;
+
+	struct readid_data {
+		u8 byte0;
+		u8 byte1;
+		u8 byte2;
+		u8 byte3;
+		u8 byte4;
+	} id_data;
+
+	memset(info->desc, 0,
+		sizeof(*info->desc) * MXS_NAND_DMA_DESCRIPTOR_COUNT);
+
+	/* Compile DMA descriptor - READID */
+	cmd_buf = info->cmd_buf;
+	cmd_queue_len = 0;
+	d = &info->desc[descnum++];
+	d->address = (dma_addr_t)(cmd_buf);
+	cmd_buf[cmd_queue_len++] = NAND_CMD_READID;
+	cmd_buf[cmd_queue_len++] = 0x00;
+
+	d->data = DMACMD_COMMAND_DMA_READ |
+		DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(cmd_queue_len);
+
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_WRITE |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_CLE |
+		GPMI_CTRL0_ADDRESS_INCREMENT |
+		cmd_queue_len;
+
+	/* Compile DMA descriptor - read. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_WAIT4END |
+		DMACMD_PIO_WORDS(1) |
+		DMACMD_XFER_COUNT(sizeof(struct readid_data)) |
+		DMACMD_COMMAND_DMA_WRITE;
+	d->pio_words[0] = GPMI_CTRL0_COMMAND_MODE_READ |
+		GPMI_CTRL0_WORD_LENGTH |
+		GPMI_CTRL0_CS(info->cs) |
+		GPMI_CTRL0_ADDRESS_NAND_DATA |
+		(sizeof(struct readid_data));
+	d->address = (dma_addr_t)databuf;
+
+	/* Compile DMA descriptor - de-assert the NAND lock and interrupt. */
+	d = &info->desc[descnum++];
+	d->data = DMACMD_IRQ | DMACMD_DEC_SEM;
+
+	/* Execute the DMA chain. */
+	ret = mxs_dma_run(info->dma_channel, info->desc, descnum);
+	if (ret) {
+		pr_err("DMA read error\n");
+		return ret;
+	}
+
+	memcpy(&id_data, databuf, sizeof(struct readid_data));
+
+	pr_debug("NAND Byte0: 0x%x\n", id_data.byte0);
+	pr_debug("NAND Byte1: 0x%x\n", id_data.byte1);
+	pr_debug("NAND Byte2: 0x%x\n", id_data.byte2);
+	pr_debug("NAND Byte3: 0x%x\n", id_data.byte3);
+	pr_debug("NAND Byte4: 0x%x\n", id_data.byte4);
+
+	if (id_data.byte0 == 0xff || id_data.byte1 == 0xff ||
+		id_data.byte2 == 0xff || id_data.byte3 == 0xff ||
+		id_data.byte4 == 0xff) {
+		pr_err("\"READ ID\" returned 0xff, possible error!\n");
+		return -EOVERFLOW;
+	}
+
+	/* Fill the NAND organization struct with data */
+	info->organization.bits_per_cell =
+		(1 << ((id_data.byte2 >> 2) & 0x3)) * 2;
+	info->organization.pagesize =
+		(1 << (id_data.byte3 & 0x3)) * SZ_1K;
+	info->organization.oobsize = id_data.byte3 & 0x4 ?
+		info->organization.pagesize / 512 * 16 :
+		info->organization.pagesize / 512 * 8;
+	info->organization.pages_per_eraseblock =
+		(1 << ((id_data.byte3 >> 4) & 0x3)) * SZ_64K /
+		info->organization.pagesize;
+	info->organization.planes_per_lun =
+		1 << ((id_data.byte4 >> 2) & 0x3);
+	info->nand_size = info->organization.planes_per_lun *
+		(1 << ((id_data.byte4 >> 4) & 0x7)) * SZ_8M;
+	info->organization.eraseblocks_per_lun = info->nand_size /
+		(info->organization.pages_per_eraseblock *
+		info->organization.pagesize);
+
+	return ret;
+}
+
+static int mxs_nand_get_info(struct mxs_nand_info *info, void *databuf)
+{
+	int ret, i;
+
+	ret = mxs_nand_check_onfi(info, databuf);
+	if (ret) {
+		if (ret != 1)
+			return ret;
+		pr_warn("ONFI not supported, try \"READ ID\"...\n");
+	} else {
+		/*
+		 * Some NAND's don't return the correct data the first time
+		 * "READ PARAMETER PAGE" is returned. Execute the command
+		 * multimple times
+		 */
+		for (i = 0; i < 3; i++) {
+			/*
+			 * Some NAND's need to be reset before "READ PARAMETER
+			 * PAGE" can be successfully executed.
+			 */
+			ret = mxs_nand_reset(info, databuf);
+			if (ret)
+				return ret;
+			ret = mxs_nand_get_onfi(info, databuf);
+			if (ret)
+				pr_err("ONFI error: %d\n", ret);
+			else
+				break;
+		}
+		if (!ret)
+			goto succ;
+	}
+
+	/*
+	 * If ONFI is not supported or if it fails try to get NAND's info from
+	 * "READ ID" command.
+	 */
+	pr_debug("Trying \"READ ID\" command...\n");
+	ret = mxs_nand_get_readid(info, databuf);
+	if (ret) {
+		if (ret != -EOVERFLOW)
+			return ret;
+
+		/*
+		 * If NAND's "READ ID" returns bad values, try to set them to
+		 * default (most common NAND memory org.) and continue.
+		 */
+		pr_warn("NANDs READ ID command returned bad values" \
+			" set them to default and try to continue!\n");
+		info->organization.pagesize = 2048;
+		info->organization.oobsize = 64;
+		info->nand_size = SZ_1G;
+	}
+succ:
+	pr_debug("NAND page_size: %d\n", info->organization.pagesize);
+	pr_debug("NAND block_size: %d\n",
+		info->organization.pages_per_eraseblock
+		* info->organization.pagesize);
+	pr_debug("NAND oob_size: %d\n", info->organization.oobsize);
+	pr_debug("NAND nand_size: %lu\n", info->nand_size);
+	pr_debug("NAND bits_per_cell: %d\n", info->organization.bits_per_cell);
+	pr_debug("NAND planes_per_lun: %d\n",
+		info->organization.planes_per_lun);
+	pr_debug("NAND luns_per_target: %d\n",
+		info->organization.luns_per_target);
+	pr_debug("NAND eraseblocks_per_lun: %d\n",
+		info->organization.eraseblocks_per_lun);
+	pr_debug("NAND ntargets: %d\n", info->organization.ntargets);
+
+
+	return 0;
+}
+
+/* ---------------------------- BCB handling part -------------------------- */
+
+static uint32_t calc_chksum(void *buf, size_t size)
+{
+	u32 chksum = 0;
+	u8 *bp = buf;
+	size_t i;
+
+	for (i = 0; i < size; i++)
+		chksum += bp[i];
+
+	return ~chksum;
+}
+
+static int get_fcb(struct mxs_nand_info *info, void *databuf)
+{
+	int i, pagenum, ret;
+	uint32_t checksum;
+	struct fcb_block *fcb = &info->fcb;
+
+	/* First page read fails, this shouldn't be necessary */
+	mxs_nand_read_page(info, info->organization.pagesize,
+		info->organization.oobsize, 0, databuf, 1);
+
+	for (i = 0; i < 4; i++) {
+		pagenum = 64 * i;
+
+		ret = mxs_nand_read_page(info, info->organization.pagesize,
+			info->organization.oobsize, pagenum, databuf, 1);
+		if (ret)
+			continue;
+
+		memcpy(fcb, databuf + mxs_nand_aux_status_offset(),
+			sizeof(*fcb));
+
+		if (fcb->FingerPrint != FCB_FINGERPRINT) {
+			pr_err("No FCB found on page %d\n", pagenum);
+			continue;
+		}
+
+		checksum = calc_chksum((void *)fcb +
+			sizeof(uint32_t), sizeof(*fcb) - sizeof(uint32_t));
+
+		if (checksum != fcb->Checksum) {
+			pr_err("FCB on page %d has invalid checksum. " \
+				"Expected: 0x%08x, calculated: 0x%08x",
+				pagenum, fcb->Checksum, checksum);
+			continue;
+		}
+
+		pr_debug("Found FCB:\n");
+		pr_debug("PageDataSize:     0x%08x\n", fcb->PageDataSize);
+		pr_debug("TotalPageSize:    0x%08x\n", fcb->TotalPageSize);
+		pr_debug("SectorsPerBlock:  0x%08x\n", fcb->SectorsPerBlock);
+		pr_debug("FW1_startingPage: 0x%08x\n",
+			fcb->Firmware1_startingPage);
+		pr_debug("PagesInFW1:       0x%08x\n", fcb->PagesInFirmware1);
+		pr_debug("FW2_startingPage: 0x%08x\n",
+			fcb->Firmware2_startingPage);
+		pr_debug("PagesInFW2:       0x%08x\n", fcb->PagesInFirmware2);
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int get_dbbt(struct mxs_nand_info *info, void *databuf)
+{
+	int i, ret;
+	int page;
+	int startpage = info->fcb.DBBTSearchAreaStartAddress;
+	struct dbbt_block dbbt;
+
+	for (i = 0; i < 4; i++) {
+		page = startpage + i * 64;
+
+		ret = mxs_nand_read_page(info, info->organization.pagesize,
+			info->organization.oobsize, page, databuf, 0);
+		if (ret)
+			continue;
+
+		memcpy(&dbbt, databuf, sizeof(struct dbbt_block));
+
+		if (*(u32 *)(databuf + sizeof(u32)) != DBBT_FINGERPRINT)
+			continue;
+
+		/* Version check */
+		if (be32_to_cpup(databuf + 2 * sizeof(u32)) < 1)
+			return -ENOENT;
+
+		ret = mxs_nand_read_page(info, info->organization.pagesize,
+			info->organization.oobsize, page + 4, databuf, 0);
+		if (ret)
+			continue;
+
+		info->dbbt_num_entries = *(u32 *)(databuf + sizeof(u32));
+
+		pr_debug("Found DBBT with %d entries\n",
+			info->dbbt_num_entries);
+		pr_debug("Checksum = 0x%08x\n", dbbt.Checksum);
+		pr_debug("FingerPrint = 0x%08x\n", dbbt.FingerPrint);
+		pr_debug("Version = 0x%08x\n", dbbt.Version);
+		pr_debug("numberBB = 0x%08x\n", dbbt.numberBB);
+		pr_debug("DBBTNumOfPages= 0x%08x\n", dbbt.DBBTNumOfPages);
+
+		for (i = 0; i < info->dbbt_num_entries; i++)
+			pr_debug("badblock %d at block %d\n", i,
+				*(u32 *)(databuf + (2 + i) * sizeof(u32)));
+
+		info->dbbt = databuf + 2 * sizeof(u32);
+
+		return 0;
+	}
+
+	return -ENOENT;
+}
+
+static int block_is_bad(struct mxs_nand_info *info, int blocknum)
+{
+	int i;
+	u32 *dbbt = info->dbbt;
+
+	if (!dbbt)
+		return 0;
+
+	for (i = 0; i < info->dbbt_num_entries; i++) {
+		if (dbbt[i] == blocknum)
+			return 1;
+	}
+
+	return 0;
+}
+
+static int read_firmware(struct mxs_nand_info *info, int startpage,
+	void *dest, int len)
+{
+	int curpage = startpage;
+	struct fcb_block *fcb = &info->fcb;
+	int pagesperblock = fcb->SectorsPerBlock;
+	int numpages = (len / fcb->PageDataSize) + 1;
+	int ret;
+	int pagesize = fcb->PageDataSize;
+	int oobsize = fcb->TotalPageSize - pagesize;
+
+	pr_debug("Reading %d pages starting from page %d\n",
+		numpages, startpage);
+
+	if (block_is_bad(info, curpage / pagesperblock))
+		curpage = ALIGN_DOWN(curpage + pagesperblock, pagesperblock);
+
+	while (numpages) {
+		if (!(curpage & (pagesperblock - 1))) {
+			/* Check for bad blocks on each block boundary */
+			if (block_is_bad(info, curpage / pagesperblock)) {
+				pr_debug("Skipping bad block at page %d\n",
+					curpage);
+				curpage += pagesperblock;
+				continue;
+			}
+		}
+
+		ret = mxs_nand_read_page(info, pagesize, oobsize,
+			curpage, dest, 0);
+		if (ret) {
+			pr_debug("Failed to read page %d\n", curpage);
+			return ret;
+		}
+
+		*((u8 *)dest + fcb->BadBlockMarkerByte) =
+			*(u8 *)(dest + pagesize);
+
+		numpages--;
+		dest += pagesize;
+		curpage++;
+	}
+
+	return 0;
+}
+
+static int __maybe_unused imx6_nand_load_image(void *cmdbuf, void *descs,
+	void *databuf, void *dest, int len)
+{
+	struct mxs_nand_info info = {
+		.io_base = (void *)0x00112000,
+		.bch_base = (void *)0x00114000,
+	};
+	struct apbh_dma apbh = {
+		.id = IMX28_DMA,
+		.regs = (void *)0x00110000,
+	};
+	struct mxs_dma_chan pchan = {
+		.channel = 0, /* MXS: MXS_DMA_CHANNEL_AHB_APBH_GPMI0 */
+		.apbh = &apbh,
+	};
+	int ret;
+	struct fcb_block *fcb;
+
+	info.dma_channel = &pchan;
+
+	pr_debug("cmdbuf: 0x%p descs: 0x%p databuf: 0x%p dest: 0x%p\n",
+			cmdbuf, descs, databuf, dest);
+
+	/* Command buffers */
+	info.cmd_buf = cmdbuf;
+	info.desc = descs;
+
+	ret = mxs_nand_get_info(&info, databuf);
+	if (ret)
+		return ret;
+
+	ret = get_fcb(&info, databuf);
+	if (ret)
+		return ret;
+
+	fcb = &info.fcb;
+
+	get_dbbt(&info, databuf);
+
+	ret = read_firmware(&info, fcb->Firmware1_startingPage, dest, len);
+	if (ret) {
+		pr_err("Failed to read firmware1, trying firmware2\n");
+		ret = read_firmware(&info, fcb->Firmware2_startingPage,
+			dest, len);
+		if (ret) {
+			pr_err("Failed to also read firmware2\n");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+int imx6_nand_start_image(void)
+{
+	int ret;
+	void *sdram = (void *)0x10000000;
+	void __noreturn (*bb)(void);
+	void *cmdbuf, *databuf, *descs;
+
+	cmdbuf = sdram;
+	descs = sdram + MXS_NAND_COMMAND_BUFFER_SIZE;
+	databuf = descs +
+		sizeof(struct mxs_dma_cmd) * MXS_NAND_DMA_DESCRIPTOR_COUNT;
+	bb = (void *)PAGE_ALIGN((unsigned long)databuf + SZ_8K);
+
+	ret = imx6_nand_load_image(cmdbuf, descs, databuf,
+		bb, imx_image_size());
+	if (ret) {
+		pr_err("Loading image failed: %d\n", ret);
+		return ret;
+	}
+
+	pr_debug("Starting barebox image at 0x%p\n", bb);
+
+	arm_early_mmu_cache_invalidate();
+	barrier();
+
+	bb();
+}
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117
  2021-01-20 12:51 [PATCH 0/3] GPMI NAND xload for i.MX6 Andrej Picej
  2021-01-20 12:51 ` [PATCH 1/3] ARM: i.MX: move BCB structures to header file Andrej Picej
  2021-01-20 12:51 ` [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload Andrej Picej
@ 2021-01-20 12:51 ` Andrej Picej
  2021-01-21  9:11   ` Sascha Hauer
  2 siblings, 1 reply; 17+ messages in thread
From: Andrej Picej @ 2021-01-20 12:51 UTC (permalink / raw)
  To: barebox

During raw NAND booting, GPMI/BCH clock generation might fail due to
improper clock gating conditions and consequently booting from NAND will
fail. This is caused by silicon errata ERR007117. Apply errata fix
workaround before GPMI NAND xload to prevent this from occurring.

Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
Signed-off-by: Andrej Picej <andrej.picej@norik.com>
---
 arch/arm/mach-imx/xload-gpmi-nand.c | 62 +++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/arch/arm/mach-imx/xload-gpmi-nand.c b/arch/arm/mach-imx/xload-gpmi-nand.c
index 04f799604..4be6d1890 100644
--- a/arch/arm/mach-imx/xload-gpmi-nand.c
+++ b/arch/arm/mach-imx/xload-gpmi-nand.c
@@ -20,6 +20,8 @@
 #include <mach/xload.h>
 #include <mach/imx-nand-bcb.h>
 #include <linux/mtd/rawnand.h>
+#include <mach/imx6-regs.h>
+#include <mach/clock-imx6.h>
 
 /*
  * MXS DMA hardware command.
@@ -256,6 +258,63 @@ struct mxs_nand_info {
 	unsigned long nand_size;
 };
 
+/**
+ * It was discovered that xloading barebox from NAND sometimes fails. Observed
+ * behaviour is similar to silicon errata ERR007117 for i.MX6.
+ *
+ * ERR007117 description:
+ * For raw NAND boot, ROM switches the source of enfc_clk_root from PLL2_PFD2
+ * to PLL3. The root clock is required to be gated before switching the source
+ * clock. If the root clock is not gated, clock glitches might be passed to the
+ * divider that follows the clock mux, and the divider might behave
+ * unpredictably. This can cause the clock generation to fail and the chip will
+ * not boot successfully.
+ *
+ * Workaround solution for this errata:
+ * 1) gate all GPMI/BCH related clocks (CG15, G14, CG13, CG12 and CG6)
+ * 2) reconfigure clocks
+ * 3) ungate all GPMI/BCH related clocks
+ *
+ */
+static inline void imx6_errata_007117_enable(void)
+{
+	u32 reg;
+
+	/* Gate (disable) the GPMI/BCH clocks in CCM_CCGR4 */
+	reg = readl(MXC_CCM_CCGR4);
+	reg &= ~(0xFF003000);
+	writel(reg, MXC_CCM_CCGR4);
+
+	/**
+	 * Gate (disable) the enfc_clk_root before changing the enfc_clk_root
+	 * source or dividers by clearing CCM_CCGR2[CG7] to 2'b00. This
+	 * disables the iomux_ipt_clk_io_clk.
+	 */
+	reg = readl(MXC_CCM_CCGR2);
+	reg &= ~(0x3 << 14);
+	writel(reg, MXC_CCM_CCGR2);
+
+	/* Configure CCM_CS2CDR for the new clock source configuration */
+	reg = readl(MXC_CCM_CS2CDR);
+	reg &= ~(0x7FF0000);
+	writel(reg, MXC_CCM_CS2CDR);
+	reg |= 0xF0000;
+	writel(reg, MXC_CCM_CS2CDR);
+
+	/**
+	 * Enable enfc_clk_root by setting CCM_CCGR2[CG7] to 2'b11. This
+	 * enables the iomux_ipt_clk_io_clk.
+	 */
+	reg = readl(MXC_CCM_CCGR2);
+	reg |= 0x3 << 14;
+	writel(reg, MXC_CCM_CCGR2);
+
+	/* Ungate (enable) the GPMI/BCH clocks in CCM_CCGR4 */
+	reg = readl(MXC_CCM_CCGR4);
+	reg |= 0xFF003000;
+	writel(reg, MXC_CCM_CCGR4);
+}
+
 static uint32_t mxs_nand_aux_status_offset(void)
 {
 	return (MXS_NAND_METADATA_SIZE + 0x3) & ~0x3;
@@ -1147,6 +1206,9 @@ int imx6_nand_start_image(void)
 		sizeof(struct mxs_dma_cmd) * MXS_NAND_DMA_DESCRIPTOR_COUNT;
 	bb = (void *)PAGE_ALIGN((unsigned long)databuf + SZ_8K);
 
+	/* Apply ERR007117 workaround */
+	imx6_errata_007117_enable();
+
 	ret = imx6_nand_load_image(cmdbuf, descs, databuf,
 		bb, imx_image_size());
 	if (ret) {
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
  2021-01-20 12:51 ` [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload Andrej Picej
@ 2021-01-20 21:45   ` Roland Hieber
  2021-01-21  9:01   ` Sascha Hauer
  1 sibling, 0 replies; 17+ messages in thread
From: Roland Hieber @ 2021-01-20 21:45 UTC (permalink / raw)
  Cc: barebox

On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Commit is based on initial Sascha Hauer's work. It implements PBL xload
> mechanism to load image from GPMI NAND flash.
> 
> Additional work was done, so that the NAND's size, page size and OOB's
> size are autodetected and not hardcoded. Detection method follows the
> same methods as used in NAND driver, meaning NAND ONFI support is probed
> and if NAND supports ONFI, NAND memory organization is read from ONFI
> parameter page otherwise "READ ID" is used.
> 
> Currently only implemented for i.MX6 familly of SoCs.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> Signed-off-by: Andrej Picej <andrej.picej@norik.com>

I did not look at the code, but I appreciate how those names line up
nicely :-)

 - Roland

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
  2021-01-20 12:51 ` [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload Andrej Picej
  2021-01-20 21:45   ` Roland Hieber
@ 2021-01-21  9:01   ` Sascha Hauer
  2021-01-21 10:28     ` Andrej Picej
  1 sibling, 1 reply; 17+ messages in thread
From: Sascha Hauer @ 2021-01-21  9:01 UTC (permalink / raw)
  To: Andrej Picej; +Cc: barebox

Hi Andrej,

Some comments inline.

On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Commit is based on initial Sascha Hauer's work. It implements PBL xload
> mechanism to load image from GPMI NAND flash.
> 
> Additional work was done, so that the NAND's size, page size and OOB's
> size are autodetected and not hardcoded. Detection method follows the
> same methods as used in NAND driver, meaning NAND ONFI support is probed
> and if NAND supports ONFI, NAND memory organization is read from ONFI
> parameter page otherwise "READ ID" is used.
> 
> Currently only implemented for i.MX6 familly of SoCs.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> ---
>  arch/arm/mach-imx/Makefile             |    2 +-
>  arch/arm/mach-imx/include/mach/xload.h |    1 +
>  arch/arm/mach-imx/xload-gpmi-nand.c    | 1163 ++++++++++++++++++++++++
>  3 files changed, 1165 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c
> 
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index e45f758e9..d94c846a1 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> +static int mxs_nand_get_info(struct mxs_nand_info *info, void *databuf)
> +{
> +	int ret, i;
> +
> +	ret = mxs_nand_check_onfi(info, databuf);
> +	if (ret) {
> +		if (ret != 1)
> +			return ret;
> +		pr_warn("ONFI not supported, try \"READ ID\"...\n");

You already printed a "ONFI not supported\n" message. Printing it once
is enough. Also this message appears with every non-ONFI nand, right? In
that case it should rather be pr_info()

> +	/*
> +	 * If ONFI is not supported or if it fails try to get NAND's info from
> +	 * "READ ID" command.
> +	 */
> +	pr_debug("Trying \"READ ID\" command...\n");
> +	ret = mxs_nand_get_readid(info, databuf);
> +	if (ret) {
> +		if (ret != -EOVERFLOW)
> +			return ret;
> +
> +		/*
> +		 * If NAND's "READ ID" returns bad values, try to set them to
> +		 * default (most common NAND memory org.) and continue.
> +		 */
> +		pr_warn("NANDs READ ID command returned bad values" \
> +			" set them to default and try to continue!\n");
> +		info->organization.pagesize = 2048;
> +		info->organization.oobsize = 64;
> +		info->nand_size = SZ_1G;

Is this worth it? READ ID is the most basic command, when this doesn't
work I don't think there's a point in continuing.

> +	}
> +succ:
> +	pr_debug("NAND page_size: %d\n", info->organization.pagesize);
> +	pr_debug("NAND block_size: %d\n",
> +		info->organization.pages_per_eraseblock
> +		* info->organization.pagesize);
> +	pr_debug("NAND oob_size: %d\n", info->organization.oobsize);
> +	pr_debug("NAND nand_size: %lu\n", info->nand_size);
> +	pr_debug("NAND bits_per_cell: %d\n", info->organization.bits_per_cell);
> +	pr_debug("NAND planes_per_lun: %d\n",
> +		info->organization.planes_per_lun);
> +	pr_debug("NAND luns_per_target: %d\n",
> +		info->organization.luns_per_target);
> +	pr_debug("NAND eraseblocks_per_lun: %d\n",
> +		info->organization.eraseblocks_per_lun);
> +	pr_debug("NAND ntargets: %d\n", info->organization.ntargets);
> +
> +
> +	return 0;
> +}
> +
> +/* ---------------------------- BCB handling part -------------------------- */
> +
> +static uint32_t calc_chksum(void *buf, size_t size)
> +{
> +	u32 chksum = 0;
> +	u8 *bp = buf;
> +	size_t i;
> +
> +	for (i = 0; i < size; i++)
> +		chksum += bp[i];
> +
> +	return ~chksum;
> +}
> +
> +static int get_fcb(struct mxs_nand_info *info, void *databuf)
> +{
> +	int i, pagenum, ret;
> +	uint32_t checksum;
> +	struct fcb_block *fcb = &info->fcb;
> +
> +	/* First page read fails, this shouldn't be necessary */
> +	mxs_nand_read_page(info, info->organization.pagesize,
> +		info->organization.oobsize, 0, databuf, 1);
> +
> +	for (i = 0; i < 4; i++) {
> +		pagenum = 64 * i;

I haven't looked at the documentation, but the '64' should probably be pages
per block, so better info->organization.pages_per_eraseblock?

> +
> +		ret = mxs_nand_read_page(info, info->organization.pagesize,
> +			info->organization.oobsize, pagenum, databuf, 1);
> +		if (ret)
> +			continue;
> +
> +		memcpy(fcb, databuf + mxs_nand_aux_status_offset(),
> +			sizeof(*fcb));
> +
> +		if (fcb->FingerPrint != FCB_FINGERPRINT) {
> +			pr_err("No FCB found on page %d\n", pagenum);
> +			continue;
> +		}
> +
> +		checksum = calc_chksum((void *)fcb +
> +			sizeof(uint32_t), sizeof(*fcb) - sizeof(uint32_t));
> +
> +		if (checksum != fcb->Checksum) {
> +			pr_err("FCB on page %d has invalid checksum. " \
> +				"Expected: 0x%08x, calculated: 0x%08x",
> +				pagenum, fcb->Checksum, checksum);
> +			continue;
> +		}
> +
> +		pr_debug("Found FCB:\n");
> +		pr_debug("PageDataSize:     0x%08x\n", fcb->PageDataSize);
> +		pr_debug("TotalPageSize:    0x%08x\n", fcb->TotalPageSize);
> +		pr_debug("SectorsPerBlock:  0x%08x\n", fcb->SectorsPerBlock);
> +		pr_debug("FW1_startingPage: 0x%08x\n",
> +			fcb->Firmware1_startingPage);
> +		pr_debug("PagesInFW1:       0x%08x\n", fcb->PagesInFirmware1);
> +		pr_debug("FW2_startingPage: 0x%08x\n",
> +			fcb->Firmware2_startingPage);
> +		pr_debug("PagesInFW2:       0x%08x\n", fcb->PagesInFirmware2);
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int get_dbbt(struct mxs_nand_info *info, void *databuf)
> +{
> +	int i, ret;
> +	int page;
> +	int startpage = info->fcb.DBBTSearchAreaStartAddress;
> +	struct dbbt_block dbbt;
> +
> +	for (i = 0; i < 4; i++) {
> +		page = startpage + i * 64;

Same here.

Regards,
 Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117
  2021-01-20 12:51 ` [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117 Andrej Picej
@ 2021-01-21  9:11   ` Sascha Hauer
  2021-01-21 12:23     ` Andrej Picej
  0 siblings, 1 reply; 17+ messages in thread
From: Sascha Hauer @ 2021-01-21  9:11 UTC (permalink / raw)
  To: Andrej Picej; +Cc: barebox

On Wed, Jan 20, 2021 at 01:51:07PM +0100, Andrej Picej wrote:
> During raw NAND booting, GPMI/BCH clock generation might fail due to
> improper clock gating conditions and consequently booting from NAND will
> fail. This is caused by silicon errata ERR007117. Apply errata fix
> workaround before GPMI NAND xload to prevent this from occurring.
> 
> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> ---
>  arch/arm/mach-imx/xload-gpmi-nand.c | 62 +++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/xload-gpmi-nand.c b/arch/arm/mach-imx/xload-gpmi-nand.c
> index 04f799604..4be6d1890 100644
> --- a/arch/arm/mach-imx/xload-gpmi-nand.c
> +++ b/arch/arm/mach-imx/xload-gpmi-nand.c
> @@ -20,6 +20,8 @@
>  #include <mach/xload.h>
>  #include <mach/imx-nand-bcb.h>
>  #include <linux/mtd/rawnand.h>
> +#include <mach/imx6-regs.h>
> +#include <mach/clock-imx6.h>
>  
>  /*
>   * MXS DMA hardware command.
> @@ -256,6 +258,63 @@ struct mxs_nand_info {
>  	unsigned long nand_size;
>  };
>  
> +/**
> + * It was discovered that xloading barebox from NAND sometimes fails. Observed
> + * behaviour is similar to silicon errata ERR007117 for i.MX6.

Have you really seen this behaviour? I wonder because the ROM has
already loaded the initial code that is just running from NAND, so it
surprises me that we still have to apply clock fixes.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
  2021-01-21  9:01   ` Sascha Hauer
@ 2021-01-21 10:28     ` Andrej Picej
  2021-01-25  9:15       ` Sascha Hauer
  0 siblings, 1 reply; 17+ messages in thread
From: Andrej Picej @ 2021-01-21 10:28 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

thanks for your comments.

Responses inline.

On 21. 01. 21 10:01, Sascha Hauer wrote:
> Hi Andrej,
> 
> Some comments inline.
> 
> On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>
>> Commit is based on initial Sascha Hauer's work. It implements PBL xload
>> mechanism to load image from GPMI NAND flash.
>>
>> Additional work was done, so that the NAND's size, page size and OOB's
>> size are autodetected and not hardcoded. Detection method follows the
>> same methods as used in NAND driver, meaning NAND ONFI support is probed
>> and if NAND supports ONFI, NAND memory organization is read from ONFI
>> parameter page otherwise "READ ID" is used.
>>
>> Currently only implemented for i.MX6 familly of SoCs.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>> ---
>>   arch/arm/mach-imx/Makefile             |    2 +-
>>   arch/arm/mach-imx/include/mach/xload.h |    1 +
>>   arch/arm/mach-imx/xload-gpmi-nand.c    | 1163 ++++++++++++++++++++++++
>>   3 files changed, 1165 insertions(+), 1 deletion(-)
>>   create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c
>>
>> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
>> index e45f758e9..d94c846a1 100644
>> --- a/arch/arm/mach-imx/Makefile
>> +++ b/arch/arm/mach-imx/Makefile
>> +static int mxs_nand_get_info(struct mxs_nand_info *info, void *databuf)
>> +{
>> +	int ret, i;
>> +
>> +	ret = mxs_nand_check_onfi(info, databuf);
>> +	if (ret) {
>> +		if (ret != 1)
>> +			return ret;
>> +		pr_warn("ONFI not supported, try \"READ ID\"...\n");
> 
> You already printed a "ONFI not supported\n" message. Printing it once
> is enough. Also this message appears with every non-ONFI nand, right? In
> that case it should rather be pr_info()

OK, I agree, will fix it in v2.

> 
>> +	/*
>> +	 * If ONFI is not supported or if it fails try to get NAND's info from
>> +	 * "READ ID" command.
>> +	 */
>> +	pr_debug("Trying \"READ ID\" command...\n");
>> +	ret = mxs_nand_get_readid(info, databuf);
>> +	if (ret) {
>> +		if (ret != -EOVERFLOW)
>> +			return ret;
>> +
>> +		/*
>> +		 * If NAND's "READ ID" returns bad values, try to set them to
>> +		 * default (most common NAND memory org.) and continue.
>> +		 */
>> +		pr_warn("NANDs READ ID command returned bad values" \
>> +			" set them to default and try to continue!\n");
>> +		info->organization.pagesize = 2048;
>> +		info->organization.oobsize = 64;
>> +		info->nand_size = SZ_1G;
> 
> Is this worth it? READ ID is the most basic command, when this doesn't
> work I don't think there's a point in continuing.

We had a case with Samsung K9K8G08U0E when sometimes (reasons not known) 
the 5th byte returned 0xff instead of correct values, all other returned 
values were correct. In this case the device booted successfully because 
of this hook. Maybe a better solution would be to check only the 5th 
byte for 0xff value (5th byte is not supported from all NAND vendors), 
if this is the case set the NAND size to 1GB.
Would that make more sense?

> 
>> +	}
>> +succ:
>> +	pr_debug("NAND page_size: %d\n", info->organization.pagesize);
>> +	pr_debug("NAND block_size: %d\n",
>> +		info->organization.pages_per_eraseblock
>> +		* info->organization.pagesize);
>> +	pr_debug("NAND oob_size: %d\n", info->organization.oobsize);
>> +	pr_debug("NAND nand_size: %lu\n", info->nand_size);
>> +	pr_debug("NAND bits_per_cell: %d\n", info->organization.bits_per_cell);
>> +	pr_debug("NAND planes_per_lun: %d\n",
>> +		info->organization.planes_per_lun);
>> +	pr_debug("NAND luns_per_target: %d\n",
>> +		info->organization.luns_per_target);
>> +	pr_debug("NAND eraseblocks_per_lun: %d\n",
>> +		info->organization.eraseblocks_per_lun);
>> +	pr_debug("NAND ntargets: %d\n", info->organization.ntargets);
>> +
>> +
>> +	return 0;
>> +}
>> +
>> +/* ---------------------------- BCB handling part -------------------------- */
>> +
>> +static uint32_t calc_chksum(void *buf, size_t size)
>> +{
>> +	u32 chksum = 0;
>> +	u8 *bp = buf;
>> +	size_t i;
>> +
>> +	for (i = 0; i < size; i++)
>> +		chksum += bp[i];
>> +
>> +	return ~chksum;
>> +}
>> +
>> +static int get_fcb(struct mxs_nand_info *info, void *databuf)
>> +{
>> +	int i, pagenum, ret;
>> +	uint32_t checksum;
>> +	struct fcb_block *fcb = &info->fcb;
>> +
>> +	/* First page read fails, this shouldn't be necessary */
>> +	mxs_nand_read_page(info, info->organization.pagesize,
>> +		info->organization.oobsize, 0, databuf, 1);
>> +
>> +	for (i = 0; i < 4; i++) {
>> +		pagenum = 64 * i;
> 
> I haven't looked at the documentation, but the '64' should probably be pages
> per block, so better info->organization.pages_per_eraseblock.
Yes, I agree, will fix it in v2.

> 
>> +
>> +		ret = mxs_nand_read_page(info, info->organization.pagesize,
>> +			info->organization.oobsize, pagenum, databuf, 1);
>> +		if (ret)
>> +			continue;
>> +
>> +		memcpy(fcb, databuf + mxs_nand_aux_status_offset(),
>> +			sizeof(*fcb));
>> +
>> +		if (fcb->FingerPrint != FCB_FINGERPRINT) {
>> +			pr_err("No FCB found on page %d\n", pagenum);
>> +			continue;
>> +		}
>> +
>> +		checksum = calc_chksum((void *)fcb +
>> +			sizeof(uint32_t), sizeof(*fcb) - sizeof(uint32_t));
>> +
>> +		if (checksum != fcb->Checksum) {
>> +			pr_err("FCB on page %d has invalid checksum. " \
>> +				"Expected: 0x%08x, calculated: 0x%08x",
>> +				pagenum, fcb->Checksum, checksum);
>> +			continue;
>> +		}
>> +
>> +		pr_debug("Found FCB:\n");
>> +		pr_debug("PageDataSize:     0x%08x\n", fcb->PageDataSize);
>> +		pr_debug("TotalPageSize:    0x%08x\n", fcb->TotalPageSize);
>> +		pr_debug("SectorsPerBlock:  0x%08x\n", fcb->SectorsPerBlock);
>> +		pr_debug("FW1_startingPage: 0x%08x\n",
>> +			fcb->Firmware1_startingPage);
>> +		pr_debug("PagesInFW1:       0x%08x\n", fcb->PagesInFirmware1);
>> +		pr_debug("FW2_startingPage: 0x%08x\n",
>> +			fcb->Firmware2_startingPage);
>> +		pr_debug("PagesInFW2:       0x%08x\n", fcb->PagesInFirmware2);
>> +
>> +		return 0;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int get_dbbt(struct mxs_nand_info *info, void *databuf)
>> +{
>> +	int i, ret;
>> +	int page;
>> +	int startpage = info->fcb.DBBTSearchAreaStartAddress;
>> +	struct dbbt_block dbbt;
>> +
>> +	for (i = 0; i < 4; i++) {
>> +		page = startpage + i * 64;
> 
> Same here.

Again, will fix it in v2.

> 
> Regards,
>   Sascha
> 

Thank you for your comments and review.

Best regards,
Andrej Picej

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117
  2021-01-21  9:11   ` Sascha Hauer
@ 2021-01-21 12:23     ` Andrej Picej
  2021-01-22  8:12       ` Sascha Hauer
  0 siblings, 1 reply; 17+ messages in thread
From: Andrej Picej @ 2021-01-21 12:23 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

Hi Sascha,

yes, unfortunately we have. We applied this just because we experienced 
hangs in device booting tests. After this patch the NAND booting 
problems went away. The booting failed during DMA page reading.
I just tested this again and after excluding the changes in this patch 
the device fails at executing DMA commands. The boot fails in 
approximately one out of 10 boots.

Best regards,
Andrej

On 21. 01. 21 10:11, Sascha Hauer wrote:
> On Wed, Jan 20, 2021 at 01:51:07PM +0100, Andrej Picej wrote:
>> During raw NAND booting, GPMI/BCH clock generation might fail due to
>> improper clock gating conditions and consequently booting from NAND will
>> fail. This is caused by silicon errata ERR007117. Apply errata fix
>> workaround before GPMI NAND xload to prevent this from occurring.
>>
>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>> ---
>>   arch/arm/mach-imx/xload-gpmi-nand.c | 62 +++++++++++++++++++++++++++++
>>   1 file changed, 62 insertions(+)
>>
>> diff --git a/arch/arm/mach-imx/xload-gpmi-nand.c b/arch/arm/mach-imx/xload-gpmi-nand.c
>> index 04f799604..4be6d1890 100644
>> --- a/arch/arm/mach-imx/xload-gpmi-nand.c
>> +++ b/arch/arm/mach-imx/xload-gpmi-nand.c
>> @@ -20,6 +20,8 @@
>>   #include <mach/xload.h>
>>   #include <mach/imx-nand-bcb.h>
>>   #include <linux/mtd/rawnand.h>
>> +#include <mach/imx6-regs.h>
>> +#include <mach/clock-imx6.h>
>>   
>>   /*
>>    * MXS DMA hardware command.
>> @@ -256,6 +258,63 @@ struct mxs_nand_info {
>>   	unsigned long nand_size;
>>   };
>>   
>> +/**
>> + * It was discovered that xloading barebox from NAND sometimes fails. Observed
>> + * behaviour is similar to silicon errata ERR007117 for i.MX6.
> 
> Have you really seen this behaviour? I wonder because the ROM has
> already loaded the initial code that is just running from NAND, so it
> surprises me that we still have to apply clock fixes.
> 
> Sascha
> 

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117
  2021-01-21 12:23     ` Andrej Picej
@ 2021-01-22  8:12       ` Sascha Hauer
  2021-01-22  9:47         ` Andrej Picej
  0 siblings, 1 reply; 17+ messages in thread
From: Sascha Hauer @ 2021-01-22  8:12 UTC (permalink / raw)
  To: Andrej Picej; +Cc: barebox

On Thu, Jan 21, 2021 at 01:23:16PM +0100, Andrej Picej wrote:
> Hi Sascha,
> 
> yes, unfortunately we have. We applied this just because we experienced
> hangs in device booting tests. After this patch the NAND booting problems
> went away. The booting failed during DMA page reading.
> I just tested this again and after excluding the changes in this patch the
> device fails at executing DMA commands. The boot fails in approximately one
> out of 10 boots.

Where exactly did it hang? In the xloader or later in the real barebox
driver?
You probably mean that it hangs in the xloader. Sorry to ask, but I'm
still trying to understand. That would mean the ROM leaves the GPMI in
an unusable state and I really wonder how this can happen.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117
  2021-01-22  8:12       ` Sascha Hauer
@ 2021-01-22  9:47         ` Andrej Picej
  2021-01-25  8:57           ` Sascha Hauer
  0 siblings, 1 reply; 17+ messages in thread
From: Andrej Picej @ 2021-01-22  9:47 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox



On 22. 01. 21 09:12, Sascha Hauer wrote:
> On Thu, Jan 21, 2021 at 01:23:16PM +0100, Andrej Picej wrote:
>> Hi Sascha,
>>
>> yes, unfortunately we have. We applied this just because we experienced
>> hangs in device booting tests. After this patch the NAND booting problems
>> went away. The booting failed during DMA page reading.
>> I just tested this again and after excluding the changes in this patch the
>> device fails at executing DMA commands. The boot fails in approximately one
>> out of 10 boots.
> 
> Where exactly did it hang? In the xloader or later in the real barebox
> driver?
> You probably mean that it hangs in the xloader. Sorry to ask, but I'm
> still trying to understand. That would mean the ROM leaves the GPMI in
> an unusable state and I really wonder how this can happen.
> 
> Sascha
> 

Sorry for not being clear enough the first time. You are correct, it 
hangs in xloader.

Maybe some additional information that may be relevant. When we boot 
barebox without PBL support barebox boots without problem and NAND 
behaves normal, so i guess the ROM sets the GPMI in usable state.
Just a thought: could it be that the problem is in xloader and we 
somehow repair or reset the GPMI with this errata fix, so it works with 
xloader?

Andrej

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117
  2021-01-22  9:47         ` Andrej Picej
@ 2021-01-25  8:57           ` Sascha Hauer
  0 siblings, 0 replies; 17+ messages in thread
From: Sascha Hauer @ 2021-01-25  8:57 UTC (permalink / raw)
  To: Andrej Picej; +Cc: barebox

On Fri, Jan 22, 2021 at 10:47:09AM +0100, Andrej Picej wrote:
> 
> 
> On 22. 01. 21 09:12, Sascha Hauer wrote:
> > On Thu, Jan 21, 2021 at 01:23:16PM +0100, Andrej Picej wrote:
> > > Hi Sascha,
> > > 
> > > yes, unfortunately we have. We applied this just because we experienced
> > > hangs in device booting tests. After this patch the NAND booting problems
> > > went away. The booting failed during DMA page reading.
> > > I just tested this again and after excluding the changes in this patch the
> > > device fails at executing DMA commands. The boot fails in approximately one
> > > out of 10 boots.
> > 
> > Where exactly did it hang? In the xloader or later in the real barebox
> > driver?
> > You probably mean that it hangs in the xloader. Sorry to ask, but I'm
> > still trying to understand. That would mean the ROM leaves the GPMI in
> > an unusable state and I really wonder how this can happen.
> > 
> > Sascha
> > 
> 
> Sorry for not being clear enough the first time. You are correct, it hangs
> in xloader.
> 
> Maybe some additional information that may be relevant. When we boot barebox
> without PBL support barebox boots without problem and NAND behaves normal,
> so i guess the ROM sets the GPMI in usable state.
> Just a thought: could it be that the problem is in xloader and we somehow
> repair or reset the GPMI with this errata fix, so it works with xloader?

It could be that the ROM changes parts of the clock tree after having
loaded the image from NAND and before it jumps to the payload. It might
be that we would have the same problem in the real barebox driver if we
wouldn't reconfigure the clocks anyway in the clock driver.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
  2021-01-21 10:28     ` Andrej Picej
@ 2021-01-25  9:15       ` Sascha Hauer
  2021-01-25  9:43         ` Andrej Picej
  0 siblings, 1 reply; 17+ messages in thread
From: Sascha Hauer @ 2021-01-25  9:15 UTC (permalink / raw)
  To: Andrej Picej; +Cc: barebox

On Thu, Jan 21, 2021 at 11:28:39AM +0100, Andrej Picej wrote:
> Hi Sascha,
> 
> thanks for your comments.
> 
> Responses inline.
> 
> On 21. 01. 21 10:01, Sascha Hauer wrote:
> > Hi Andrej,
> > 
> > Some comments inline.
> > 
> > On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
> > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > 
> > > Commit is based on initial Sascha Hauer's work. It implements PBL xload
> > > mechanism to load image from GPMI NAND flash.
> > > 
> > > Additional work was done, so that the NAND's size, page size and OOB's
> > > size are autodetected and not hardcoded. Detection method follows the
> > > same methods as used in NAND driver, meaning NAND ONFI support is probed
> > > and if NAND supports ONFI, NAND memory organization is read from ONFI
> > > parameter page otherwise "READ ID" is used.
> > > 
> > > Currently only implemented for i.MX6 familly of SoCs.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> > > Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> > > ---
> > >   arch/arm/mach-imx/Makefile             |    2 +-
> > >   arch/arm/mach-imx/include/mach/xload.h |    1 +
> > >   arch/arm/mach-imx/xload-gpmi-nand.c    | 1163 ++++++++++++++++++++++++
> > >   3 files changed, 1165 insertions(+), 1 deletion(-)
> > >   create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c
> > > 
> > > diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> > > index e45f758e9..d94c846a1 100644
> > > --- a/arch/arm/mach-imx/Makefile
> > > +++ b/arch/arm/mach-imx/Makefile
> > > +static int mxs_nand_get_info(struct mxs_nand_info *info, void *databuf)
> > > +{
> > > +	int ret, i;
> > > +
> > > +	ret = mxs_nand_check_onfi(info, databuf);
> > > +	if (ret) {
> > > +		if (ret != 1)
> > > +			return ret;
> > > +		pr_warn("ONFI not supported, try \"READ ID\"...\n");
> > 
> > You already printed a "ONFI not supported\n" message. Printing it once
> > is enough. Also this message appears with every non-ONFI nand, right? In
> > that case it should rather be pr_info()
> 
> OK, I agree, will fix it in v2.
> 
> > 
> > > +	/*
> > > +	 * If ONFI is not supported or if it fails try to get NAND's info from
> > > +	 * "READ ID" command.
> > > +	 */
> > > +	pr_debug("Trying \"READ ID\" command...\n");
> > > +	ret = mxs_nand_get_readid(info, databuf);
> > > +	if (ret) {
> > > +		if (ret != -EOVERFLOW)
> > > +			return ret;
> > > +
> > > +		/*
> > > +		 * If NAND's "READ ID" returns bad values, try to set them to
> > > +		 * default (most common NAND memory org.) and continue.
> > > +		 */
> > > +		pr_warn("NANDs READ ID command returned bad values" \
> > > +			" set them to default and try to continue!\n");
> > > +		info->organization.pagesize = 2048;
> > > +		info->organization.oobsize = 64;
> > > +		info->nand_size = SZ_1G;
> > 
> > Is this worth it? READ ID is the most basic command, when this doesn't
> > work I don't think there's a point in continuing.
> 
> We had a case with Samsung K9K8G08U0E when sometimes (reasons not known) the
> 5th byte returned 0xff instead of correct values, all other returned values
> were correct. In this case the device booted successfully because of this
> hook. Maybe a better solution would be to check only the 5th byte for 0xff
> value (5th byte is not supported from all NAND vendors), if this is the case
> set the NAND size to 1GB.
> Would that make more sense?

The best would be to track the issue down and to fix it ;)
It's not very nice to assume it in case of read id failures it's exactly
that NAND type you have troubles with.
Does it help to reset the NAND chip before reading the ID?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
  2021-01-25  9:15       ` Sascha Hauer
@ 2021-01-25  9:43         ` Andrej Picej
  2021-01-26 11:40           ` Andrej Picej
  0 siblings, 1 reply; 17+ messages in thread
From: Andrej Picej @ 2021-01-25  9:43 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 25. 01. 21 10:15, Sascha Hauer wrote:
> On Thu, Jan 21, 2021 at 11:28:39AM +0100, Andrej Picej wrote:
>> Hi Sascha,
>>
>> thanks for your comments.
>>
>> Responses inline.
>>
>> On 21. 01. 21 10:01, Sascha Hauer wrote:
>>> Hi Andrej,
>>>
>>> Some comments inline.
>>>
>>> On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
>>>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>>>
>>>> Commit is based on initial Sascha Hauer's work. It implements PBL xload
>>>> mechanism to load image from GPMI NAND flash.
>>>>
>>>> Additional work was done, so that the NAND's size, page size and OOB's
>>>> size are autodetected and not hardcoded. Detection method follows the
>>>> same methods as used in NAND driver, meaning NAND ONFI support is probed
>>>> and if NAND supports ONFI, NAND memory organization is read from ONFI
>>>> parameter page otherwise "READ ID" is used.
>>>>
>>>> Currently only implemented for i.MX6 familly of SoCs.
>>>>
>>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>>> ---
>>>>    arch/arm/mach-imx/Makefile             |    2 +-
>>>>    arch/arm/mach-imx/include/mach/xload.h |    1 +
>>>>    arch/arm/mach-imx/xload-gpmi-nand.c    | 1163 ++++++++++++++++++++++++
>>>>    3 files changed, 1165 insertions(+), 1 deletion(-)
>>>>    create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c
>>>>
>>>> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
>>>> index e45f758e9..d94c846a1 100644
>>>> --- a/arch/arm/mach-imx/Makefile
>>>> +++ b/arch/arm/mach-imx/Makefile
>>>> +static int mxs_nand_get_info(struct mxs_nand_info *info, void *databuf)
>>>> +{
>>>> +	int ret, i;
>>>> +
>>>> +	ret = mxs_nand_check_onfi(info, databuf);
>>>> +	if (ret) {
>>>> +		if (ret != 1)
>>>> +			return ret;
>>>> +		pr_warn("ONFI not supported, try \"READ ID\"...\n");
>>>
>>> You already printed a "ONFI not supported\n" message. Printing it once
>>> is enough. Also this message appears with every non-ONFI nand, right? In
>>> that case it should rather be pr_info()
>>
>> OK, I agree, will fix it in v2.
>>
>>>
>>>> +	/*
>>>> +	 * If ONFI is not supported or if it fails try to get NAND's info from
>>>> +	 * "READ ID" command.
>>>> +	 */
>>>> +	pr_debug("Trying \"READ ID\" command...\n");
>>>> +	ret = mxs_nand_get_readid(info, databuf);
>>>> +	if (ret) {
>>>> +		if (ret != -EOVERFLOW)
>>>> +			return ret;
>>>> +
>>>> +		/*
>>>> +		 * If NAND's "READ ID" returns bad values, try to set them to
>>>> +		 * default (most common NAND memory org.) and continue.
>>>> +		 */
>>>> +		pr_warn("NANDs READ ID command returned bad values" \
>>>> +			" set them to default and try to continue!\n");
>>>> +		info->organization.pagesize = 2048;
>>>> +		info->organization.oobsize = 64;
>>>> +		info->nand_size = SZ_1G;
>>>
>>> Is this worth it? READ ID is the most basic command, when this doesn't
>>> work I don't think there's a point in continuing.
>>
>> We had a case with Samsung K9K8G08U0E when sometimes (reasons not known) the
>> 5th byte returned 0xff instead of correct values, all other returned values
>> were correct. In this case the device booted successfully because of this
>> hook. Maybe a better solution would be to check only the 5th byte for 0xff
>> value (5th byte is not supported from all NAND vendors), if this is the case
>> set the NAND size to 1GB.
>> Would that make more sense?
> 
> The best would be to track the issue down and to fix it ;)
> It's not very nice to assume it in case of read id failures it's exactly
> that NAND type you have troubles with.
> Does it help to reset the NAND chip before reading the ID?
> 
> Sascha
> 

I totally agree with fixing the issue and will look into it a bit more.
I will reset the NAND before reading the ID in the same way as it is 
done before reading parameter page. I will leave the testing on over 
night (reading the ID failed in about 1 out of 500 boots) and report the 
results tomorrow.

BR,
Andrej

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
  2021-01-25  9:43         ` Andrej Picej
@ 2021-01-26 11:40           ` Andrej Picej
  2021-01-26 12:08             ` Sascha Hauer
  0 siblings, 1 reply; 17+ messages in thread
From: Andrej Picej @ 2021-01-26 11:40 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox


On 25. 01. 21 10:43, Andrej Picej wrote:
> On 25. 01. 21 10:15, Sascha Hauer wrote:
>> On Thu, Jan 21, 2021 at 11:28:39AM +0100, Andrej Picej wrote:
>>> Hi Sascha,
>>>
>>> thanks for your comments.
>>>
>>> Responses inline.
>>>
>>> On 21. 01. 21 10:01, Sascha Hauer wrote:
>>>> Hi Andrej,
>>>>
>>>> Some comments inline.
>>>>
>>>> On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
>>>>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>>>>
>>>>> Commit is based on initial Sascha Hauer's work. It implements PBL 
>>>>> xload
>>>>> mechanism to load image from GPMI NAND flash.
>>>>>
>>>>> Additional work was done, so that the NAND's size, page size and OOB's
>>>>> size are autodetected and not hardcoded. Detection method follows the
>>>>> same methods as used in NAND driver, meaning NAND ONFI support is 
>>>>> probed
>>>>> and if NAND supports ONFI, NAND memory organization is read from ONFI
>>>>> parameter page otherwise "READ ID" is used.
>>>>>
>>>>> Currently only implemented for i.MX6 familly of SoCs.
>>>>>
>>>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>>>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>>>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>>>> ---
>>>>>    arch/arm/mach-imx/Makefile             |    2 +-
>>>>>    arch/arm/mach-imx/include/mach/xload.h |    1 +
>>>>>    arch/arm/mach-imx/xload-gpmi-nand.c    | 1163 
>>>>> ++++++++++++++++++++++++
>>>>>    3 files changed, 1165 insertions(+), 1 deletion(-)
>>>>>    create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c
>>>>>
>>>>> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
>>>>> index e45f758e9..d94c846a1 100644
>>>>> --- a/arch/arm/mach-imx/Makefile
>>>>> +++ b/arch/arm/mach-imx/Makefile
>>>>> +static int mxs_nand_get_info(struct mxs_nand_info *info, void 
>>>>> *databuf)
>>>>> +{
>>>>> +    int ret, i;
>>>>> +
>>>>> +    ret = mxs_nand_check_onfi(info, databuf);
>>>>> +    if (ret) {
>>>>> +        if (ret != 1)
>>>>> +            return ret;
>>>>> +        pr_warn("ONFI not supported, try \"READ ID\"...\n");
>>>>
>>>> You already printed a "ONFI not supported\n" message. Printing it once
>>>> is enough. Also this message appears with every non-ONFI nand, 
>>>> right? In
>>>> that case it should rather be pr_info()
>>>
>>> OK, I agree, will fix it in v2.
>>>
>>>>
>>>>> +    /*
>>>>> +     * If ONFI is not supported or if it fails try to get NAND's 
>>>>> info from
>>>>> +     * "READ ID" command.
>>>>> +     */
>>>>> +    pr_debug("Trying \"READ ID\" command...\n");
>>>>> +    ret = mxs_nand_get_readid(info, databuf);
>>>>> +    if (ret) {
>>>>> +        if (ret != -EOVERFLOW)
>>>>> +            return ret;
>>>>> +
>>>>> +        /*
>>>>> +         * If NAND's "READ ID" returns bad values, try to set them to
>>>>> +         * default (most common NAND memory org.) and continue.
>>>>> +         */
>>>>> +        pr_warn("NANDs READ ID command returned bad values" \
>>>>> +            " set them to default and try to continue!\n");
>>>>> +        info->organization.pagesize = 2048;
>>>>> +        info->organization.oobsize = 64;
>>>>> +        info->nand_size = SZ_1G;
>>>>
>>>> Is this worth it? READ ID is the most basic command, when this doesn't
>>>> work I don't think there's a point in continuing.
>>>
>>> We had a case with Samsung K9K8G08U0E when sometimes (reasons not 
>>> known) the
>>> 5th byte returned 0xff instead of correct values, all other returned 
>>> values
>>> were correct. In this case the device booted successfully because of 
>>> this
>>> hook. Maybe a better solution would be to check only the 5th byte for 
>>> 0xff
>>> value (5th byte is not supported from all NAND vendors), if this is 
>>> the case
>>> set the NAND size to 1GB.
>>> Would that make more sense?
>>
>> The best would be to track the issue down and to fix it ;)
>> It's not very nice to assume it in case of read id failures it's exactly
>> that NAND type you have troubles with.
>> Does it help to reset the NAND chip before reading the ID?
>>
>> Sascha
>>
> 
> I totally agree with fixing the issue and will look into it a bit more.
> I will reset the NAND before reading the ID in the same way as it is 
> done before reading parameter page. I will leave the testing on over 
> night (reading the ID failed in about 1 out of 500 boots) and report the 
> results tomorrow.
> 

I guess resetting fixed this problem, I got 50.000 boots without any 
problems. Will fix this in v2 :).

Should I now completely abandon setting the NANDs memory to default 
values and just return error on failure?
Something like this:

> 	/*
> 	 * If ONFI is not supported or if it fails try to get NAND's info from
> 	 * "READ ID" command.
> 	 */
> 	ret = mxs_nand_reset(info, databuf);
> 	if (ret)
> 		return ret;
> 	pr_debug("Trying \"READ ID\" command...\n");
> 	ret = mxs_nand_get_readid(info, databuf);
> 	if (ret) {
> 		pr_err("xloader supports only ONFI and generic \"READ ID\" " \
> 			"supported NANDs\n");
> 		return -1;
> 	}

I'm asking this because NANDs ID command is manufacturer specific, some 
manufacturers follow generic values and other unfortunately don't. In 
barebox NAND driver decoding is done based on manufacturer. I guess we 
could do the same here but I don't want to expand the PBL part too much 
just for reading FCB, where all NAND's memory parameters are decoded from.

BR,
Andrej

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
  2021-01-26 11:40           ` Andrej Picej
@ 2021-01-26 12:08             ` Sascha Hauer
  2021-01-26 12:18               ` Andrej Picej
  0 siblings, 1 reply; 17+ messages in thread
From: Sascha Hauer @ 2021-01-26 12:08 UTC (permalink / raw)
  To: Andrej Picej; +Cc: barebox

On Tue, Jan 26, 2021 at 12:40:17PM +0100, Andrej Picej wrote:
> 
> On 25. 01. 21 10:43, Andrej Picej wrote:
> > On 25. 01. 21 10:15, Sascha Hauer wrote:
> > > On Thu, Jan 21, 2021 at 11:28:39AM +0100, Andrej Picej wrote:
> > > > Hi Sascha,
> > > > 
> > > > thanks for your comments.
> > > > 
> > > > Responses inline.
> > > > 
> > > > On 21. 01. 21 10:01, Sascha Hauer wrote:
> > > > > Hi Andrej,
> > > > > 
> > > > > Some comments inline.
> > > > > 
> > > > > On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
> > > > > > From: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > > 
> > > > > > Commit is based on initial Sascha Hauer's work. It
> > > > > > implements PBL xload
> > > > > > mechanism to load image from GPMI NAND flash.
> > > > > > 
> > > > > > Additional work was done, so that the NAND's size, page size and OOB's
> > > > > > size are autodetected and not hardcoded. Detection method follows the
> > > > > > same methods as used in NAND driver, meaning NAND ONFI
> > > > > > support is probed
> > > > > > and if NAND supports ONFI, NAND memory organization is read from ONFI
> > > > > > parameter page otherwise "READ ID" is used.
> > > > > > 
> > > > > > Currently only implemented for i.MX6 familly of SoCs.
> > > > > > 
> > > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > > Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
> > > > > > Signed-off-by: Andrej Picej <andrej.picej@norik.com>
> > > > > > ---
> > > > > >    arch/arm/mach-imx/Makefile             |    2 +-
> > > > > >    arch/arm/mach-imx/include/mach/xload.h |    1 +
> > > > > >    arch/arm/mach-imx/xload-gpmi-nand.c    | 1163
> > > > > > ++++++++++++++++++++++++
> > > > > >    3 files changed, 1165 insertions(+), 1 deletion(-)
> > > > > >    create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c
> > > > > > 
> > > > > > diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> > > > > > index e45f758e9..d94c846a1 100644
> > > > > > --- a/arch/arm/mach-imx/Makefile
> > > > > > +++ b/arch/arm/mach-imx/Makefile
> > > > > > +static int mxs_nand_get_info(struct mxs_nand_info
> > > > > > *info, void *databuf)
> > > > > > +{
> > > > > > +    int ret, i;
> > > > > > +
> > > > > > +    ret = mxs_nand_check_onfi(info, databuf);
> > > > > > +    if (ret) {
> > > > > > +        if (ret != 1)
> > > > > > +            return ret;
> > > > > > +        pr_warn("ONFI not supported, try \"READ ID\"...\n");
> > > > > 
> > > > > You already printed a "ONFI not supported\n" message. Printing it once
> > > > > is enough. Also this message appears with every non-ONFI
> > > > > nand, right? In
> > > > > that case it should rather be pr_info()
> > > > 
> > > > OK, I agree, will fix it in v2.
> > > > 
> > > > > 
> > > > > > +    /*
> > > > > > +     * If ONFI is not supported or if it fails try to
> > > > > > get NAND's info from
> > > > > > +     * "READ ID" command.
> > > > > > +     */
> > > > > > +    pr_debug("Trying \"READ ID\" command...\n");
> > > > > > +    ret = mxs_nand_get_readid(info, databuf);
> > > > > > +    if (ret) {
> > > > > > +        if (ret != -EOVERFLOW)
> > > > > > +            return ret;
> > > > > > +
> > > > > > +        /*
> > > > > > +         * If NAND's "READ ID" returns bad values, try to set them to
> > > > > > +         * default (most common NAND memory org.) and continue.
> > > > > > +         */
> > > > > > +        pr_warn("NANDs READ ID command returned bad values" \
> > > > > > +            " set them to default and try to continue!\n");
> > > > > > +        info->organization.pagesize = 2048;
> > > > > > +        info->organization.oobsize = 64;
> > > > > > +        info->nand_size = SZ_1G;
> > > > > 
> > > > > Is this worth it? READ ID is the most basic command, when this doesn't
> > > > > work I don't think there's a point in continuing.
> > > > 
> > > > We had a case with Samsung K9K8G08U0E when sometimes (reasons
> > > > not known) the
> > > > 5th byte returned 0xff instead of correct values, all other
> > > > returned values
> > > > were correct. In this case the device booted successfully
> > > > because of this
> > > > hook. Maybe a better solution would be to check only the 5th
> > > > byte for 0xff
> > > > value (5th byte is not supported from all NAND vendors), if this
> > > > is the case
> > > > set the NAND size to 1GB.
> > > > Would that make more sense?
> > > 
> > > The best would be to track the issue down and to fix it ;)
> > > It's not very nice to assume it in case of read id failures it's exactly
> > > that NAND type you have troubles with.
> > > Does it help to reset the NAND chip before reading the ID?
> > > 
> > > Sascha
> > > 
> > 
> > I totally agree with fixing the issue and will look into it a bit more.
> > I will reset the NAND before reading the ID in the same way as it is
> > done before reading parameter page. I will leave the testing on over
> > night (reading the ID failed in about 1 out of 500 boots) and report the
> > results tomorrow.
> > 
> 
> I guess resetting fixed this problem, I got 50.000 boots without any
> problems. Will fix this in v2 :).
> 
> Should I now completely abandon setting the NANDs memory to default values
> and just return error on failure?

Yes please.

> Something like this:
> 
> > 	/*
> > 	 * If ONFI is not supported or if it fails try to get NAND's info from
> > 	 * "READ ID" command.
> > 	 */
> > 	ret = mxs_nand_reset(info, databuf);
> > 	if (ret)
> > 		return ret;
> > 	pr_debug("Trying \"READ ID\" command...\n");
> > 	ret = mxs_nand_get_readid(info, databuf);
> > 	if (ret) {
> > 		pr_err("xloader supports only ONFI and generic \"READ ID\" " \
> > 			"supported NANDs\n");
> > 		return -1;
> > 	}
> 
> I'm asking this because NANDs ID command is manufacturer specific, some
> manufacturers follow generic values and other unfortunately don't. In
> barebox NAND driver decoding is done based on manufacturer. I guess we could
> do the same here but I don't want to expand the PBL part too much just for
> reading FCB, where all NAND's memory parameters are decoded from.

We can do that once we have to later. Decoding the ID into page/block
sizes should also be done generically and not part of the i.MX6 NAND
loader.  Maybe we could also utilize parts of the NAND layer instead of
adding new code.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload
  2021-01-26 12:08             ` Sascha Hauer
@ 2021-01-26 12:18               ` Andrej Picej
  0 siblings, 0 replies; 17+ messages in thread
From: Andrej Picej @ 2021-01-26 12:18 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 26. 01. 21 13:08, Sascha Hauer wrote:
> On Tue, Jan 26, 2021 at 12:40:17PM +0100, Andrej Picej wrote:
>>
>> On 25. 01. 21 10:43, Andrej Picej wrote:
>>> On 25. 01. 21 10:15, Sascha Hauer wrote:
>>>> On Thu, Jan 21, 2021 at 11:28:39AM +0100, Andrej Picej wrote:
>>>>> Hi Sascha,
>>>>>
>>>>> thanks for your comments.
>>>>>
>>>>> Responses inline.
>>>>>
>>>>> On 21. 01. 21 10:01, Sascha Hauer wrote:
>>>>>> Hi Andrej,
>>>>>>
>>>>>> Some comments inline.
>>>>>>
>>>>>> On Wed, Jan 20, 2021 at 01:51:06PM +0100, Andrej Picej wrote:
>>>>>>> From: Sascha Hauer <s.hauer@pengutronix.de>
>>>>>>>
>>>>>>> Commit is based on initial Sascha Hauer's work. It
>>>>>>> implements PBL xload
>>>>>>> mechanism to load image from GPMI NAND flash.
>>>>>>>
>>>>>>> Additional work was done, so that the NAND's size, page size and OOB's
>>>>>>> size are autodetected and not hardcoded. Detection method follows the
>>>>>>> same methods as used in NAND driver, meaning NAND ONFI
>>>>>>> support is probed
>>>>>>> and if NAND supports ONFI, NAND memory organization is read from ONFI
>>>>>>> parameter page otherwise "READ ID" is used.
>>>>>>>
>>>>>>> Currently only implemented for i.MX6 familly of SoCs.
>>>>>>>
>>>>>>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>>>>>>> Signed-off-by: Primoz Fiser <primoz.fiser@norik.com>
>>>>>>> Signed-off-by: Andrej Picej <andrej.picej@norik.com>
>>>>>>> ---
>>>>>>>     arch/arm/mach-imx/Makefile             |    2 +-
>>>>>>>     arch/arm/mach-imx/include/mach/xload.h |    1 +
>>>>>>>     arch/arm/mach-imx/xload-gpmi-nand.c    | 1163
>>>>>>> ++++++++++++++++++++++++
>>>>>>>     3 files changed, 1165 insertions(+), 1 deletion(-)
>>>>>>>     create mode 100644 arch/arm/mach-imx/xload-gpmi-nand.c
>>>>>>>
>>>>>>> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
>>>>>>> index e45f758e9..d94c846a1 100644
>>>>>>> --- a/arch/arm/mach-imx/Makefile
>>>>>>> +++ b/arch/arm/mach-imx/Makefile
>>>>>>> +static int mxs_nand_get_info(struct mxs_nand_info
>>>>>>> *info, void *databuf)
>>>>>>> +{
>>>>>>> +    int ret, i;
>>>>>>> +
>>>>>>> +    ret = mxs_nand_check_onfi(info, databuf);
>>>>>>> +    if (ret) {
>>>>>>> +        if (ret != 1)
>>>>>>> +            return ret;
>>>>>>> +        pr_warn("ONFI not supported, try \"READ ID\"...\n");
>>>>>>
>>>>>> You already printed a "ONFI not supported\n" message. Printing it once
>>>>>> is enough. Also this message appears with every non-ONFI
>>>>>> nand, right? In
>>>>>> that case it should rather be pr_info()
>>>>>
>>>>> OK, I agree, will fix it in v2.
>>>>>
>>>>>>
>>>>>>> +    /*
>>>>>>> +     * If ONFI is not supported or if it fails try to
>>>>>>> get NAND's info from
>>>>>>> +     * "READ ID" command.
>>>>>>> +     */
>>>>>>> +    pr_debug("Trying \"READ ID\" command...\n");
>>>>>>> +    ret = mxs_nand_get_readid(info, databuf);
>>>>>>> +    if (ret) {
>>>>>>> +        if (ret != -EOVERFLOW)
>>>>>>> +            return ret;
>>>>>>> +
>>>>>>> +        /*
>>>>>>> +         * If NAND's "READ ID" returns bad values, try to set them to
>>>>>>> +         * default (most common NAND memory org.) and continue.
>>>>>>> +         */
>>>>>>> +        pr_warn("NANDs READ ID command returned bad values" \
>>>>>>> +            " set them to default and try to continue!\n");
>>>>>>> +        info->organization.pagesize = 2048;
>>>>>>> +        info->organization.oobsize = 64;
>>>>>>> +        info->nand_size = SZ_1G;
>>>>>>
>>>>>> Is this worth it? READ ID is the most basic command, when this doesn't
>>>>>> work I don't think there's a point in continuing.
>>>>>
>>>>> We had a case with Samsung K9K8G08U0E when sometimes (reasons
>>>>> not known) the
>>>>> 5th byte returned 0xff instead of correct values, all other
>>>>> returned values
>>>>> were correct. In this case the device booted successfully
>>>>> because of this
>>>>> hook. Maybe a better solution would be to check only the 5th
>>>>> byte for 0xff
>>>>> value (5th byte is not supported from all NAND vendors), if this
>>>>> is the case
>>>>> set the NAND size to 1GB.
>>>>> Would that make more sense?
>>>>
>>>> The best would be to track the issue down and to fix it ;)
>>>> It's not very nice to assume it in case of read id failures it's exactly
>>>> that NAND type you have troubles with.
>>>> Does it help to reset the NAND chip before reading the ID?
>>>>
>>>> Sascha
>>>>
>>>
>>> I totally agree with fixing the issue and will look into it a bit more.
>>> I will reset the NAND before reading the ID in the same way as it is
>>> done before reading parameter page. I will leave the testing on over
>>> night (reading the ID failed in about 1 out of 500 boots) and report the
>>> results tomorrow.
>>>
>>
>> I guess resetting fixed this problem, I got 50.000 boots without any
>> problems. Will fix this in v2 :).
>>
>> Should I now completely abandon setting the NANDs memory to default values
>> and just return error on failure?
> 
> Yes please.
> 
>> Something like this:
>>
>>> 	/*
>>> 	 * If ONFI is not supported or if it fails try to get NAND's info from
>>> 	 * "READ ID" command.
>>> 	 */
>>> 	ret = mxs_nand_reset(info, databuf);
>>> 	if (ret)
>>> 		return ret;
>>> 	pr_debug("Trying \"READ ID\" command...\n");
>>> 	ret = mxs_nand_get_readid(info, databuf);
>>> 	if (ret) {
>>> 		pr_err("xloader supports only ONFI and generic \"READ ID\" " \
>>> 			"supported NANDs\n");
>>> 		return -1;
>>> 	}
>>
>> I'm asking this because NANDs ID command is manufacturer specific, some
>> manufacturers follow generic values and other unfortunately don't. In
>> barebox NAND driver decoding is done based on manufacturer. I guess we could
>> do the same here but I don't want to expand the PBL part too much just for
>> reading FCB, where all NAND's memory parameters are decoded from.
> 
> We can do that once we have to later. Decoding the ID into page/block
> sizes should also be done generically and not part of the i.MX6 NAND
> loader.  Maybe we could also utilize parts of the NAND layer instead of
> adding new code.
> 
Ok, will prepare v2 and submit it when done.
Thank you for your kind comments and guidance so far.

BR,
Andrej


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-01-26 12:18 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 12:51 [PATCH 0/3] GPMI NAND xload for i.MX6 Andrej Picej
2021-01-20 12:51 ` [PATCH 1/3] ARM: i.MX: move BCB structures to header file Andrej Picej
2021-01-20 12:51 ` [PATCH 2/3] ARM: i.MX: implement GPMI NAND xload Andrej Picej
2021-01-20 21:45   ` Roland Hieber
2021-01-21  9:01   ` Sascha Hauer
2021-01-21 10:28     ` Andrej Picej
2021-01-25  9:15       ` Sascha Hauer
2021-01-25  9:43         ` Andrej Picej
2021-01-26 11:40           ` Andrej Picej
2021-01-26 12:08             ` Sascha Hauer
2021-01-26 12:18               ` Andrej Picej
2021-01-20 12:51 ` [PATCH 3/3] ARM: i.MX: xload-gpmi-nand: apply errata 007117 Andrej Picej
2021-01-21  9:11   ` Sascha Hauer
2021-01-21 12:23     ` Andrej Picej
2021-01-22  8:12       ` Sascha Hauer
2021-01-22  9:47         ` Andrej Picej
2021-01-25  8:57           ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox