public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems
@ 2012-12-05 20:48 Wolfram Sang
  2012-12-05 22:28 ` Otavio Salvador
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Wolfram Sang @ 2012-12-05 20:48 UTC (permalink / raw)
  To: u-boot

It could happen (1 out of 100 times) that NAND did not start up correctly after
warm rebooting, so we end up with various failures or DMA timed out due to a
stalled BCH. When resetting BCH together with GPMI, the issue could not be
observed anymore (after 10000+ reboots). We probably need the consistent state
already before sending commands to NAND. This behaviour was observed in barebox
and kernel, so I assume it affects U-Boot as well. I chose to keep the extra
reset for BCH when changing the flash layout to be on the safe side.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---

Only compile tested. Test case was to repeatedly reboot into a simple userspace
on a UBI volume. Either the bootloader failed to find its data or the kernel
could not mount the UBI volume once in a while. (Yes, the kernel could not
mount the UBI although it was itself loaded correctly from NAND. So, it is some
set-up issue.) This was nasty to debug, so I thought I let you know...

 drivers/mtd/nand/mxs_nand.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
index 4701be8..e38e151 100644
--- a/drivers/mtd/nand/mxs_nand.c
+++ b/drivers/mtd/nand/mxs_nand.c
@@ -1058,6 +1058,8 @@ int mxs_nand_init(struct mxs_nand_info *info)
 {
 	struct mxs_gpmi_regs *gpmi_regs =
 		(struct mxs_gpmi_regs *)MXS_GPMI_BASE;
+	struct mxs_bch_regs *bch_regs =
+		(struct mxs_bch_regs *)MXS_BCH_BASE;
 	int i = 0, j;
 
 	info->desc = malloc(sizeof(struct mxs_dma_desc *) *
@@ -1081,6 +1083,7 @@ int mxs_nand_init(struct mxs_nand_info *info)
 
 	/* Reset the GPMI block. */
 	mxs_reset_block(&gpmi_regs->hw_gpmi_ctrl0_reg);
+	mxs_reset_block(&bch_regs->hw_bch_ctrl_reg);
 
 	/*
 	 * Choose NAND mode, set IRQ polarity, disable write protection and
-- 
1.7.10.4

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

* [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems
  2012-12-05 20:48 [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems Wolfram Sang
@ 2012-12-05 22:28 ` Otavio Salvador
  2012-12-05 23:35 ` Fabio Estevam
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Otavio Salvador @ 2012-12-05 22:28 UTC (permalink / raw)
  To: u-boot

On Wed, Dec 5, 2012 at 6:48 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:

> It could happen (1 out of 100 times) that NAND did not start up correctly
> after
> warm rebooting, so we end up with various failures or DMA timed out due to
> a
> stalled BCH. When resetting BCH together with GPMI, the issue could not be
> observed anymore (after 10000+ reboots). We probably need the consistent
> state
> already before sending commands to NAND. This behaviour was observed in
> barebox
> and kernel, so I assume it affects U-Boot as well. I chose to keep the
> extra
> reset for BCH when changing the flash layout to be on the safe side.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---
>
> Only compile tested. Test case was to repeatedly reboot into a simple
> userspace
> on a UBI volume. Either the bootloader failed to find its data or the
> kernel
> could not mount the UBI volume once in a while. (Yes, the kernel could not
> mount the UBI although it was itself loaded correctly from NAND. So, it is
> some
> set-up issue.) This was nasty to debug, so I thought I let you know...
>
>  drivers/mtd/nand/mxs_nand.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index 4701be8..e38e151 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -1058,6 +1058,8 @@ int mxs_nand_init(struct mxs_nand_info *info)
>  {
>         struct mxs_gpmi_regs *gpmi_regs =
>                 (struct mxs_gpmi_regs *)MXS_GPMI_BASE;
> +       struct mxs_bch_regs *bch_regs =
> +               (struct mxs_bch_regs *)MXS_BCH_BASE;
>         int i = 0, j;
>
>         info->desc = malloc(sizeof(struct mxs_dma_desc *) *
> @@ -1081,6 +1083,7 @@ int mxs_nand_init(struct mxs_nand_info *info)
>
>         /* Reset the GPMI block. */
>         mxs_reset_block(&gpmi_regs->hw_gpmi_ctrl0_reg);
> +       mxs_reset_block(&bch_regs->hw_bch_ctrl_reg);
>

A comment here why this is need would be nice.


>         /*
>          * Choose NAND mode, set IRQ polarity, disable write protection and
> --
> 1.7.10.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>



-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

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

* [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems
  2012-12-05 20:48 [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems Wolfram Sang
  2012-12-05 22:28 ` Otavio Salvador
@ 2012-12-05 23:35 ` Fabio Estevam
  2012-12-05 23:39   ` Wolfram Sang
  2012-12-11  2:32 ` Marek Vasut
  2012-12-11 23:23 ` Scott Wood
  3 siblings, 1 reply; 7+ messages in thread
From: Fabio Estevam @ 2012-12-05 23:35 UTC (permalink / raw)
  To: u-boot

Hi Wolfram,

On Wed, Dec 5, 2012 at 6:48 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:

> diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> index 4701be8..e38e151 100644
> --- a/drivers/mtd/nand/mxs_nand.c
> +++ b/drivers/mtd/nand/mxs_nand.c
> @@ -1058,6 +1058,8 @@ int mxs_nand_init(struct mxs_nand_info *info)
>  {
>         struct mxs_gpmi_regs *gpmi_regs =
>                 (struct mxs_gpmi_regs *)MXS_GPMI_BASE;
> +       struct mxs_bch_regs *bch_regs =
> +               (struct mxs_bch_regs *)MXS_BCH_BASE;
>         int i = 0, j;
>
>         info->desc = malloc(sizeof(struct mxs_dma_desc *) *
> @@ -1081,6 +1083,7 @@ int mxs_nand_init(struct mxs_nand_info *info)
>
>         /* Reset the GPMI block. */
>         mxs_reset_block(&gpmi_regs->hw_gpmi_ctrl0_reg);
> +       mxs_reset_block(&bch_regs->hw_bch_ctrl_reg);

In your kernel patch you only do the reset for mx23, but here you do
it for mx28.

Which one is correct?

Regards,

Fabio Estevam

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

* [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems
  2012-12-05 23:35 ` Fabio Estevam
@ 2012-12-05 23:39   ` Wolfram Sang
  2012-12-06 10:11     ` Fabio Estevam
  0 siblings, 1 reply; 7+ messages in thread
From: Wolfram Sang @ 2012-12-05 23:39 UTC (permalink / raw)
  To: u-boot

On Wed, Dec 05, 2012 at 09:35:26PM -0200, Fabio Estevam wrote:
> Hi Wolfram,
> 
> On Wed, Dec 5, 2012 at 6:48 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> 
> > diff --git a/drivers/mtd/nand/mxs_nand.c b/drivers/mtd/nand/mxs_nand.c
> > index 4701be8..e38e151 100644
> > --- a/drivers/mtd/nand/mxs_nand.c
> > +++ b/drivers/mtd/nand/mxs_nand.c
> > @@ -1058,6 +1058,8 @@ int mxs_nand_init(struct mxs_nand_info *info)
> >  {
> >         struct mxs_gpmi_regs *gpmi_regs =
> >                 (struct mxs_gpmi_regs *)MXS_GPMI_BASE;
> > +       struct mxs_bch_regs *bch_regs =
> > +               (struct mxs_bch_regs *)MXS_BCH_BASE;
> >         int i = 0, j;
> >
> >         info->desc = malloc(sizeof(struct mxs_dma_desc *) *
> > @@ -1081,6 +1083,7 @@ int mxs_nand_init(struct mxs_nand_info *info)
> >
> >         /* Reset the GPMI block. */
> >         mxs_reset_block(&gpmi_regs->hw_gpmi_ctrl0_reg);
> > +       mxs_reset_block(&bch_regs->hw_bch_ctrl_reg);
> 
> In your kernel patch you only do the reset for mx23, but here you do
> it for mx28.
> 
> Which one is correct?

Both patches are correct. Check the kernel code again, please,
especially the function arguments.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121206/1b443d87/attachment.pgp>

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

* [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems
  2012-12-05 23:39   ` Wolfram Sang
@ 2012-12-06 10:11     ` Fabio Estevam
  0 siblings, 0 replies; 7+ messages in thread
From: Fabio Estevam @ 2012-12-06 10:11 UTC (permalink / raw)
  To: u-boot

On Wed, Dec 5, 2012 at 9:39 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:

> Both patches are correct. Check the kernel code again, please,
> especially the function arguments.

Ok, all is clear after reading the comments of the kernel
gpmi_reset_block function.

Regards,

Fabio Estevam

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

* [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems
  2012-12-05 20:48 [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems Wolfram Sang
  2012-12-05 22:28 ` Otavio Salvador
  2012-12-05 23:35 ` Fabio Estevam
@ 2012-12-11  2:32 ` Marek Vasut
  2012-12-11 23:23 ` Scott Wood
  3 siblings, 0 replies; 7+ messages in thread
From: Marek Vasut @ 2012-12-11  2:32 UTC (permalink / raw)
  To: u-boot

Dear Wolfram Sang,

> It could happen (1 out of 100 times) that NAND did not start up correctly
> after warm rebooting, so we end up with various failures or DMA timed out
> due to a stalled BCH. When resetting BCH together with GPMI, the issue
> could not be observed anymore (after 10000+ reboots). We probably need the
> consistent state already before sending commands to NAND. This behaviour
> was observed in barebox and kernel, so I assume it affects U-Boot as well.
> I chose to keep the extra reset for BCH when changing the flash layout to
> be on the safe side.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
[...]

Good, thanks. Ccing Scott to pick this up.

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

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

* [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems
  2012-12-05 20:48 [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems Wolfram Sang
                   ` (2 preceding siblings ...)
  2012-12-11  2:32 ` Marek Vasut
@ 2012-12-11 23:23 ` Scott Wood
  3 siblings, 0 replies; 7+ messages in thread
From: Scott Wood @ 2012-12-11 23:23 UTC (permalink / raw)
  To: u-boot

On 12/05/2012 02:48:47 PM, Wolfram Sang wrote:
> It could happen (1 out of 100 times) that NAND did not start up  
> correctly after
> warm rebooting, so we end up with various failures or DMA timed out  
> due to a
> stalled BCH. When resetting BCH together with GPMI, the issue could  
> not be
> observed anymore (after 10000+ reboots). We probably need the  
> consistent state
> already before sending commands to NAND. This behaviour was observed  
> in barebox
> and kernel, so I assume it affects U-Boot as well. I chose to keep  
> the extra
> reset for BCH when changing the flash layout to be on the safe side.
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> ---

Applied to u-boot-nand-flash

-Scott

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

end of thread, other threads:[~2012-12-11 23:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-05 20:48 [U-Boot] [PATCH] mtd: nand: mxs: reset BCH earlier, too, to avoid NAND startup problems Wolfram Sang
2012-12-05 22:28 ` Otavio Salvador
2012-12-05 23:35 ` Fabio Estevam
2012-12-05 23:39   ` Wolfram Sang
2012-12-06 10:11     ` Fabio Estevam
2012-12-11  2:32 ` Marek Vasut
2012-12-11 23:23 ` Scott Wood

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