[PATCH V2 1/3] mtd: rawnand: denali-spl: Add missing hardware init on SoCFPGA

On Altera SoCFPGA, upon either cold-boot or power-on reset, the Denali NAND IP is initialized by the BootROM ; upon warm-reset, the Denali NAND IP is NOT initialized by BootROM. In fact, upon warm-reset, the SoCFPGA BootROM checks whether the SPL image in on-chip RAM is valid and if so, completely skips re-loading the SPL from the boot media.
This does sometimes leads to problems where the software left the boot media in inconsistent state before warm-reset, and because the BootROM does not reset the boot media, the boot media is left in this inconsistent state, often until another component attempts to access the boot media and fails with an difficult to debug failure. To mitigate this problem, the SPL on Altera SoCFPGA always resets all the IPs on the SoC early on boot.
This results in a couple of register values, pre-programmed by the BootROM, to be lost during this reset. To restore correct operation of the IP on SoCFPGA, these values must be programmed back into the controller by the driver. Note that on other SoCs which do not use the HW-controlled bootstrap, more registers may have to be programmed.
This also aligns the SPL behavior with the full Denali NAND driver, which sets these values in denali_hw_init().
Signed-off-by: Marek Vasut marex@denx.de Cc: Masahiro Yamada yamada.masahiro@socionext.com --- V2: Reword the commit message --- drivers/mtd/nand/raw/denali_spl.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/mtd/nand/raw/denali_spl.c b/drivers/mtd/nand/raw/denali_spl.c index dbaba3cab2..b8b29812aa 100644 --- a/drivers/mtd/nand/raw/denali_spl.c +++ b/drivers/mtd/nand/raw/denali_spl.c @@ -173,6 +173,13 @@ void nand_init(void) page_size = readl(denali_flash_reg + DEVICE_MAIN_AREA_SIZE); oob_size = readl(denali_flash_reg + DEVICE_SPARE_AREA_SIZE); pages_per_block = readl(denali_flash_reg + PAGES_PER_BLOCK); + + /* Do as denali_hw_init() does. */ + writel(CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES, + denali_flash_reg + SPARE_AREA_SKIP_BYTES); + writel(0x0F, denali_flash_reg + RB_PIN_ENABLED); + writel(CHIP_EN_DONT_CARE__FLAG, denali_flash_reg + CHIP_ENABLE_DONT_CARE); + writel(0xffff, denali_flash_reg + SPARE_AREA_MARKER); }
int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)

From: Masahiro Yamada yamada.masahiro@socionext.com
The "nand_x" and "ecc" clocks are currently optional. Make the core clock optional in the same way. This will allow platforms with no clock driver support to use this driver.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com Tested-by: Marek Vasut marex@denx.de # On SoCFPGA Arria V --- V2: No change --- drivers/mtd/nand/raw/denali_dt.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index 0ce81324b9..b1e14982c4 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -91,7 +91,7 @@ static int denali_dt_probe(struct udevice *dev) if (ret) ret = clk_get_by_index(dev, 0, &clk); if (ret) - return ret; + clk.dev = NULL;
ret = clk_get_by_name(dev, "nand_x", &clk_x); if (ret) @@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev) if (ret) clk_ecc.dev = NULL;
- ret = clk_enable(&clk); - if (ret) - return ret; + if (clk.dev) { + ret = clk_enable(&clk); + if (ret) + return ret; + }
if (clk_x.dev) { ret = clk_enable(&clk_x);

On Wed, Jan 22, 2020 at 4:03 AM Marek Vasut marex@denx.de wrote:
From: Masahiro Yamada yamada.masahiro@socionext.com
The "nand_x" and "ecc" clocks are currently optional. Make the core clock optional in the same way. This will allow platforms with no clock driver support to use this driver.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com Tested-by: Marek Vasut marex@denx.de # On SoCFPGA Arria V
Applied to u-boot-uniphier. Thanks.
V2: No change
drivers/mtd/nand/raw/denali_dt.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index 0ce81324b9..b1e14982c4 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -91,7 +91,7 @@ static int denali_dt_probe(struct udevice *dev) if (ret) ret = clk_get_by_index(dev, 0, &clk); if (ret)
return ret;
clk.dev = NULL; ret = clk_get_by_name(dev, "nand_x", &clk_x); if (ret)
@@ -101,9 +101,11 @@ static int denali_dt_probe(struct udevice *dev) if (ret) clk_ecc.dev = NULL;
ret = clk_enable(&clk);
if (ret)
return ret;
if (clk.dev) {
ret = clk_enable(&clk);
if (ret)
return ret;
} if (clk_x.dev) { ret = clk_enable(&clk_x);
-- 2.24.1

The register values programmed into the Denali NAND IP are not preserved when the IP reset signal is asserted. Depending on whether the Denali NAND IP implementation uses HW-assisted bootstrap or not, some of the register values are restored automatically after the IP is released from reset.
If HW-assisted bootstrap is used, only RB_PIN_ENABLED, CHIP_ENABLE_DONT_CARE, SPARE_AREA_SKIP_BYTES and SPARE_AREA_MARKER need to be restored after reset, this is the case on Altera SoCFPGA. Otherwise, many more registers must be restored. Since mainline Linux currently does not handle this correctly, do not reset the Denali NAND IP before booting Linux. This permits Linux to use the register values retained in the Denali NAND IP and thus correctly access the NAND.
Fixes: ed784ac3822b ("mtd: rawnand: denali: add reset handling") Signed-off-by: Marek Vasut marex@denx.de Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com --- V2: - Make resets into local variable - Update patch description --- drivers/mtd/nand/raw/denali.h | 2 -- drivers/mtd/nand/raw/denali_dt.c | 15 ++++----------- 2 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h index 63ae828768..019deda094 100644 --- a/drivers/mtd/nand/raw/denali.h +++ b/drivers/mtd/nand/raw/denali.h @@ -10,7 +10,6 @@ #include <linux/bitops.h> #include <linux/mtd/rawnand.h> #include <linux/types.h> -#include <reset.h>
#define DEVICE_RESET 0x0 #define DEVICE_RESET__BANK(bank) BIT(bank) @@ -316,7 +315,6 @@ struct denali_nand_info { void (*host_write)(struct denali_nand_info *denali, u32 addr, u32 data); void (*setup_dma)(struct denali_nand_info *denali, dma_addr_t dma_addr, int page, int write); - struct reset_ctl_bulk resets; };
#define DENALI_CAP_HW_ECC_FIXUP BIT(0) diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index b1e14982c4..91d0f20aae 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -9,6 +9,7 @@ #include <linux/io.h> #include <linux/ioport.h> #include <linux/printk.h> +#include <reset.h>
#include "denali.h"
@@ -63,6 +64,7 @@ static int denali_dt_probe(struct udevice *dev) struct denali_nand_info *denali = dev_get_priv(dev); const struct denali_dt_data *data; struct clk clk, clk_x, clk_ecc; + struct reset_ctl_bulk resets; struct resource res; int ret;
@@ -133,30 +135,21 @@ static int denali_dt_probe(struct udevice *dev) denali->clk_x_rate = 200000000; }
- ret = reset_get_bulk(dev, &denali->resets); + ret = reset_get_bulk(dev, &resets); if (ret) dev_warn(dev, "Can't get reset: %d\n", ret); else - reset_deassert_bulk(&denali->resets); + reset_deassert_bulk(&resets);
return denali_init(denali); }
-static int denali_dt_remove(struct udevice *dev) -{ - struct denali_nand_info *denali = dev_get_priv(dev); - - return reset_release_bulk(&denali->resets); -} - U_BOOT_DRIVER(denali_nand_dt) = { .name = "denali-nand-dt", .id = UCLASS_MISC, .of_match = denali_nand_dt_ids, .probe = denali_dt_probe, .priv_auto_alloc_size = sizeof(struct denali_nand_info), - .remove = denali_dt_remove, - .flags = DM_FLAG_OS_PREPARE, };
void board_nand_init(void)

On Wed, Jan 22, 2020 at 4:03 AM Marek Vasut marex@denx.de wrote:
The register values programmed into the Denali NAND IP are not preserved when the IP reset signal is asserted. Depending on whether the Denali NAND IP implementation uses HW-assisted bootstrap or not, some of the register values are restored automatically after the IP is released from reset.
If HW-assisted bootstrap is used, only RB_PIN_ENABLED, CHIP_ENABLE_DONT_CARE, SPARE_AREA_SKIP_BYTES and SPARE_AREA_MARKER need to be restored after reset, this is the case on Altera SoCFPGA. Otherwise, many more registers must be restored. Since mainline Linux currently does not handle this correctly, do not reset the Denali NAND IP before booting Linux. This permits Linux to use the register values retained in the Denali NAND IP and thus correctly access the NAND.
Fixes: ed784ac3822b ("mtd: rawnand: denali: add reset handling") Signed-off-by: Marek Vasut marex@denx.de Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
I talked to Marek off-list, and he acked me to reword the commit log like follows when applied.
------------------>8------------------ commit 9a230b0072504511454912a9e301f0beabc270c6 (HEAD -> uniphier, origin/uniphier) Author: Marek Vasut marex@denx.de Date: Tue Jan 21 20:03:11 2020 +0100
mtd: rawnand: denali: Do not reset the block before booting the kernel
The Denali NAND driver in mainline Linux currently cannot deassert the reset. The upcoming Linux 5.6 will support the reset controlling, and also set up SPARE_AREA_SKIP_BYTES correctly. So, the Denali driver in the future kernel will work without relying on any bootloader or firmware. However, we still need to take care of stable kernel versions for a while. U-boot should not assert the reset of this controller.
Fixes: ed784ac3822b ("mtd: rawnand: denali: add reset handling") Signed-off-by: Marek Vasut marex@denx.de [yamada.masahiro: reword the commit description] Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
------------------>8------------------
Applied to u-boot-uniphier. Thanks.
V2: - Make resets into local variable - Update patch description
drivers/mtd/nand/raw/denali.h | 2 -- drivers/mtd/nand/raw/denali_dt.c | 15 ++++----------- 2 files changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h index 63ae828768..019deda094 100644 --- a/drivers/mtd/nand/raw/denali.h +++ b/drivers/mtd/nand/raw/denali.h @@ -10,7 +10,6 @@ #include <linux/bitops.h> #include <linux/mtd/rawnand.h> #include <linux/types.h> -#include <reset.h>
#define DEVICE_RESET 0x0 #define DEVICE_RESET__BANK(bank) BIT(bank) @@ -316,7 +315,6 @@ struct denali_nand_info { void (*host_write)(struct denali_nand_info *denali, u32 addr, u32 data); void (*setup_dma)(struct denali_nand_info *denali, dma_addr_t dma_addr, int page, int write);
struct reset_ctl_bulk resets;
};
#define DENALI_CAP_HW_ECC_FIXUP BIT(0) diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index b1e14982c4..91d0f20aae 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -9,6 +9,7 @@ #include <linux/io.h> #include <linux/ioport.h> #include <linux/printk.h> +#include <reset.h>
#include "denali.h"
@@ -63,6 +64,7 @@ static int denali_dt_probe(struct udevice *dev) struct denali_nand_info *denali = dev_get_priv(dev); const struct denali_dt_data *data; struct clk clk, clk_x, clk_ecc;
struct reset_ctl_bulk resets; struct resource res; int ret;
@@ -133,30 +135,21 @@ static int denali_dt_probe(struct udevice *dev) denali->clk_x_rate = 200000000; }
ret = reset_get_bulk(dev, &denali->resets);
ret = reset_get_bulk(dev, &resets); if (ret) dev_warn(dev, "Can't get reset: %d\n", ret); else
reset_deassert_bulk(&denali->resets);
reset_deassert_bulk(&resets); return denali_init(denali);
}
-static int denali_dt_remove(struct udevice *dev) -{
struct denali_nand_info *denali = dev_get_priv(dev);
return reset_release_bulk(&denali->resets);
-}
U_BOOT_DRIVER(denali_nand_dt) = { .name = "denali-nand-dt", .id = UCLASS_MISC, .of_match = denali_nand_dt_ids, .probe = denali_dt_probe, .priv_auto_alloc_size = sizeof(struct denali_nand_info),
.remove = denali_dt_remove,
.flags = DM_FLAG_OS_PREPARE,
};
void board_nand_init(void)
2.24.1
-- Best Regards Masahiro Yamada

Hi.
On Wed, Jan 22, 2020 at 4:03 AM Marek Vasut marex@denx.de wrote:
On Altera SoCFPGA, upon either cold-boot or power-on reset, the Denali NAND IP is initialized by the BootROM ; upon warm-reset, the Denali NAND IP is NOT initialized by BootROM. In fact, upon warm-reset, the SoCFPGA BootROM checks whether the SPL image in on-chip RAM is valid and if so, completely skips re-loading the SPL from the boot media.
This does sometimes leads to problems where the software left the boot media in inconsistent state before warm-reset, and because the BootROM does not reset the boot media, the boot media is left in this inconsistent state, often until another component attempts to access the boot media and fails with an difficult to debug failure. To mitigate this problem, the SPL on Altera SoCFPGA always resets all the IPs on the SoC early on boot.
This results in a couple of register values, pre-programmed by the BootROM, to be lost during this reset. To restore correct operation of the IP on SoCFPGA, these values must be programmed back into the controller by the driver. Note that on other SoCs which do not use the HW-controlled bootstrap, more registers may have to be programmed.
This also aligns the SPL behavior with the full Denali NAND driver, which sets these values in denali_hw_init().
Signed-off-by: Marek Vasut marex@denx.de Cc: Masahiro Yamada yamada.masahiro@socionext.com
Thanks for the update.
I will queue this series up, and send one more fix (add udelay after the reset deassert).
Thank you.
V2: Reword the commit message
drivers/mtd/nand/raw/denali_spl.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/mtd/nand/raw/denali_spl.c b/drivers/mtd/nand/raw/denali_spl.c index dbaba3cab2..b8b29812aa 100644 --- a/drivers/mtd/nand/raw/denali_spl.c +++ b/drivers/mtd/nand/raw/denali_spl.c @@ -173,6 +173,13 @@ void nand_init(void) page_size = readl(denali_flash_reg + DEVICE_MAIN_AREA_SIZE); oob_size = readl(denali_flash_reg + DEVICE_SPARE_AREA_SIZE); pages_per_block = readl(denali_flash_reg + PAGES_PER_BLOCK);
/* Do as denali_hw_init() does. */
writel(CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES,
denali_flash_reg + SPARE_AREA_SKIP_BYTES);
writel(0x0F, denali_flash_reg + RB_PIN_ENABLED);
writel(CHIP_EN_DONT_CARE__FLAG, denali_flash_reg + CHIP_ENABLE_DONT_CARE);
writel(0xffff, denali_flash_reg + SPARE_AREA_MARKER);
}
int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
2.24.1

On Wed, Jan 22, 2020 at 4:03 AM Marek Vasut marex@denx.de wrote:
On Altera SoCFPGA, upon either cold-boot or power-on reset, the Denali NAND IP is initialized by the BootROM ; upon warm-reset, the Denali NAND IP is NOT initialized by BootROM. In fact, upon warm-reset, the SoCFPGA BootROM checks whether the SPL image in on-chip RAM is valid and if so, completely skips re-loading the SPL from the boot media.
This does sometimes leads to problems where the software left the boot media in inconsistent state before warm-reset, and because the BootROM does not reset the boot media, the boot media is left in this inconsistent state, often until another component attempts to access the boot media and fails with an difficult to debug failure. To mitigate this problem, the SPL on Altera SoCFPGA always resets all the IPs on the SoC early on boot.
This results in a couple of register values, pre-programmed by the BootROM, to be lost during this reset. To restore correct operation of the IP on SoCFPGA, these values must be programmed back into the controller by the driver. Note that on other SoCs which do not use the HW-controlled bootstrap, more registers may have to be programmed.
This also aligns the SPL behavior with the full Denali NAND driver, which sets these values in denali_hw_init().
Signed-off-by: Marek Vasut marex@denx.de Cc: Masahiro Yamada yamada.masahiro@socionext.com
Applied to u-boot-uniphier. Thanks.
V2: Reword the commit message
drivers/mtd/nand/raw/denali_spl.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/mtd/nand/raw/denali_spl.c b/drivers/mtd/nand/raw/denali_spl.c index dbaba3cab2..b8b29812aa 100644 --- a/drivers/mtd/nand/raw/denali_spl.c +++ b/drivers/mtd/nand/raw/denali_spl.c @@ -173,6 +173,13 @@ void nand_init(void) page_size = readl(denali_flash_reg + DEVICE_MAIN_AREA_SIZE); oob_size = readl(denali_flash_reg + DEVICE_SPARE_AREA_SIZE); pages_per_block = readl(denali_flash_reg + PAGES_PER_BLOCK);
/* Do as denali_hw_init() does. */
writel(CONFIG_NAND_DENALI_SPARE_AREA_SKIP_BYTES,
denali_flash_reg + SPARE_AREA_SKIP_BYTES);
writel(0x0F, denali_flash_reg + RB_PIN_ENABLED);
writel(CHIP_EN_DONT_CARE__FLAG, denali_flash_reg + CHIP_ENABLE_DONT_CARE);
writel(0xffff, denali_flash_reg + SPARE_AREA_MARKER);
}
int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
2.24.1
participants (2)
-
Marek Vasut
-
Masahiro Yamada