[PATCH 1/2] mtd: nand: raw: denali: Assert reset before deassert

Always put the controller in reset, then take it out of reset. This is to make sure controller always in reset state in both SPL and proper Uboot.
This is preparation for the next patch to poll for reset completion (rst_comp) bit after reset.
Signed-off-by: Radu Bacrau radu.bacrau@intel.com Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com --- drivers/mtd/nand/raw/denali_dt.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index 2728e8098faa..75ad15b0758c 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -148,6 +148,8 @@ static int denali_dt_probe(struct udevice *dev) if (ret) { dev_warn(dev, "Can't get reset: %d\n", ret); } else { + reset_assert_bulk(&resets); + udelay(2); reset_deassert_bulk(&resets);
/*

Fixed delay 200us is not working in certain platforms. Change to poll for reset completion status to have more reliable reset process.
Controller will set the rst_comp bit in intr_status register after controller has completed its reset and initialization process.
Signed-off-by: Radu Bacrau radu.bacrau@intel.com Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com --- drivers/mtd/nand/raw/denali.c | 11 +++++++++++ drivers/mtd/nand/raw/denali.h | 1 + drivers/mtd/nand/raw/denali_dt.c | 10 +++++++--- 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c index 15e90291de09..ab91db85467d 100644 --- a/drivers/mtd/nand/raw/denali.c +++ b/drivers/mtd/nand/raw/denali.c @@ -1220,6 +1220,17 @@ static int denali_multidev_fixup(struct denali_nand_info *denali) return 0; }
+int denali_wait_reset_complete(struct denali_nand_info *denali) +{ + u32 irq_status; + + irq_status = denali_wait_for_irq(denali, INTR__RST_COMP); + if (!(irq_status & INTR__RST_COMP)) + return -EIO; + + return 0; +} + int denali_init(struct denali_nand_info *denali) { struct nand_chip *chip = &denali->nand; diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h index 019deda094e5..6cd02b2e26ee 100644 --- a/drivers/mtd/nand/raw/denali.h +++ b/drivers/mtd/nand/raw/denali.h @@ -321,6 +321,7 @@ struct denali_nand_info { #define DENALI_CAP_DMA_64BIT BIT(1)
int denali_calc_ecc_bytes(int step_size, int strength); +int denali_wait_reset_complete(struct denali_nand_info *denali); int denali_init(struct denali_nand_info *denali);
#endif /* __DENALI_H__ */ diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index 75ad15b0758c..8a6950f8a39f 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -154,10 +154,14 @@ static int denali_dt_probe(struct udevice *dev)
/* * When the reset is deasserted, the initialization sequence is - * kicked (bootstrap process). The driver must wait until it is - * finished. Otherwise, it will result in unpredictable behavior. + * kicked. The driver must wait until it is finished. Otherwise, + * it will result in unpredictable behavior. */ - udelay(200); + ret = denali_wait_reset_complete(denali); + if (ret) { + dev_err(denali->dev, "reset not completed.\n"); + return ret; + } }
return denali_init(denali);

On Mon, Jun 29, 2020 at 7:12 PM Ley Foon Tan ley.foon.tan@intel.com wrote:
Fixed delay 200us is not working in certain platforms. Change to poll for reset completion status to have more reliable reset process.
Controller will set the rst_comp bit in intr_status register after controller has completed its reset and initialization process.
Signed-off-by: Radu Bacrau radu.bacrau@intel.com Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/mtd/nand/raw/denali.c | 11 +++++++++++ drivers/mtd/nand/raw/denali.h | 1 + drivers/mtd/nand/raw/denali_dt.c | 10 +++++++--- 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c index 15e90291de09..ab91db85467d 100644 --- a/drivers/mtd/nand/raw/denali.c +++ b/drivers/mtd/nand/raw/denali.c @@ -1220,6 +1220,17 @@ static int denali_multidev_fixup(struct denali_nand_info *denali) return 0; }
+int denali_wait_reset_complete(struct denali_nand_info *denali) +{
u32 irq_status;
irq_status = denali_wait_for_irq(denali, INTR__RST_COMP);
if (!(irq_status & INTR__RST_COMP))
return -EIO;
return 0;
+}
int denali_init(struct denali_nand_info *denali) { struct nand_chip *chip = &denali->nand; diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h index 019deda094e5..6cd02b2e26ee 100644 --- a/drivers/mtd/nand/raw/denali.h +++ b/drivers/mtd/nand/raw/denali.h @@ -321,6 +321,7 @@ struct denali_nand_info { #define DENALI_CAP_DMA_64BIT BIT(1)
int denali_calc_ecc_bytes(int step_size, int strength); +int denali_wait_reset_complete(struct denali_nand_info *denali); int denali_init(struct denali_nand_info *denali);
#endif /* __DENALI_H__ */ diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index 75ad15b0758c..8a6950f8a39f 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -154,10 +154,14 @@ static int denali_dt_probe(struct udevice *dev)
/* * When the reset is deasserted, the initialization sequence is
* kicked (bootstrap process). The driver must wait until it is
* finished. Otherwise, it will result in unpredictable behavior.
* kicked. The driver must wait until it is finished. Otherwise,
* it will result in unpredictable behavior.
What is your motivation for this hunk of change? (removal of "bootstrap process")
What was wrong with the current comment block?
The term "bootstrap" appears in the Cadence (Denali) Flash Controller User Guide.
*/
udelay(200);
ret = denali_wait_reset_complete(denali);
if (ret) {
dev_err(denali->dev, "reset not completed.\n");
return ret;
} } return denali_init(denali);
-- 2.19.0
I tested this patch on some of my socionext SoC boards, and I did not see regression.
Tested-by: Masahiro Yamada yamada.masahiro@socionext.com
But, please note this code will diverge from the Linux code.

-----Original Message----- From: Masahiro Yamada masahiroy@kernel.org Sent: Friday, July 10, 2020 12:27 AM To: Tan, Ley Foon ley.foon.tan@intel.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Simon Glass sjg@chromium.org; See, Chin Liang chin.liang.see@intel.com; Bacrau, Radu radu.bacrau@intel.com; Marek Vasut marex@denx.de Subject: Re: [PATCH 2/2] mtd: nand: raw: denali: Wait for reset completion status
On Mon, Jun 29, 2020 at 7:12 PM Ley Foon Tan ley.foon.tan@intel.com wrote:
Fixed delay 200us is not working in certain platforms. Change to poll for reset completion status to have more reliable reset process.
Controller will set the rst_comp bit in intr_status register after controller has completed its reset and initialization process.
Signed-off-by: Radu Bacrau radu.bacrau@intel.com Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/mtd/nand/raw/denali.c | 11 +++++++++++ drivers/mtd/nand/raw/denali.h | 1 + drivers/mtd/nand/raw/denali_dt.c | 10 +++++++--- 3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c index 15e90291de09..ab91db85467d 100644 --- a/drivers/mtd/nand/raw/denali.c +++ b/drivers/mtd/nand/raw/denali.c @@ -1220,6 +1220,17 @@ static int denali_multidev_fixup(struct
denali_nand_info *denali)
return 0;
}
+int denali_wait_reset_complete(struct denali_nand_info *denali) {
u32 irq_status;
irq_status = denali_wait_for_irq(denali, INTR__RST_COMP);
if (!(irq_status & INTR__RST_COMP))
return -EIO;
return 0;
+}
int denali_init(struct denali_nand_info *denali) { struct nand_chip *chip = &denali->nand; diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h index 019deda094e5..6cd02b2e26ee 100644 --- a/drivers/mtd/nand/raw/denali.h +++ b/drivers/mtd/nand/raw/denali.h @@ -321,6 +321,7 @@ struct denali_nand_info { #define DENALI_CAP_DMA_64BIT BIT(1)
int denali_calc_ecc_bytes(int step_size, int strength); +int denali_wait_reset_complete(struct denali_nand_info *denali); int denali_init(struct denali_nand_info *denali);
#endif /* __DENALI_H__ */ diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index 75ad15b0758c..8a6950f8a39f 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -154,10 +154,14 @@ static int denali_dt_probe(struct udevice *dev)
/* * When the reset is deasserted, the initialization sequence is
* kicked (bootstrap process). The driver must wait until it is
* finished. Otherwise, it will result in unpredictable behavior.
* kicked. The driver must wait until it is finished. Otherwise,
* it will result in unpredictable behavior.
What is your motivation for this hunk of change? (removal of "bootstrap process")
What was wrong with the current comment block?
The term "bootstrap" appears in the Cadence (Denali) Flash Controller User Guide.
No special reason. Will send v2 patch to revert back "bootstrap process" in comment.
Thanks.
Regards Ley Foon

On Mon, Jun 29, 2020 at 7:11 PM Ley Foon Tan ley.foon.tan@intel.com wrote:
Always put the controller in reset, then take it out of reset. This is to make sure controller always in reset state in both SPL and proper Uboot.
This is preparation for the next patch to poll for reset completion (rst_comp) bit after reset.
Signed-off-by: Radu Bacrau radu.bacrau@intel.com Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/mtd/nand/raw/denali_dt.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index 2728e8098faa..75ad15b0758c 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -148,6 +148,8 @@ static int denali_dt_probe(struct udevice *dev) if (ret) { dev_warn(dev, "Can't get reset: %d\n", ret); } else {
reset_assert_bulk(&resets);
udelay(2); reset_deassert_bulk(&resets); /*
Tested-by: Masahiro Yamada yamada.masahiro@socionext.com
participants (3)
-
Ley Foon Tan
-
Masahiro Yamada
-
Tan, Ley Foon