| Message ID | 1386999720-23460-2-git-send-email-ynvich@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
On Saturday, December 14, 2013 at 06:41:57 AM, Sergei Ianovich wrote: > Erratum 71 of PXA270M Processor Family Specification Update > (April 19, 2010) explains that watchdog reset time is just > 8us insead of 10ms in EMTS. > > If SDRAM is not reset, it causes memory bus congestion and > the device hangs. > > We put SDRAM in selfresh mode before watchdog reset, removing > potential freezes. > > Signed-off-by: Sergei Ianovich <ynvich@gmail.com> > --- > arch/arm/cpu/pxa/pxa2xx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/cpu/pxa/pxa2xx.c b/arch/arm/cpu/pxa/pxa2xx.c > index c9a7d45..93ca2f0 100644 > --- a/arch/arm/cpu/pxa/pxa2xx.c > +++ b/arch/arm/cpu/pxa/pxa2xx.c > @@ -281,5 +281,5 @@ void reset_cpu(ulong ignored) > writel(tmp, OSMR3); > > for (;;) > - ; > + writel(MDREFR_SLFRSH, MDREFR); Do you need to write this register in an endless loop ? > } Best regards, Marek Vasut
On Sat, 2013-12-14 at 13:29 +0100, Marek Vasut wrote: > On Saturday, December 14, 2013 at 06:41:57 AM, Sergei Ianovich wrote: > > Erratum 71 of PXA270M Processor Family Specification Update > > (April 19, 2010) explains that watchdog reset time is just > > 8us insead of 10ms in EMTS. > > > > If SDRAM is not reset, it causes memory bus congestion and > > the device hangs. > > > > We put SDRAM in selfresh mode before watchdog reset, removing > > potential freezes. > > > > Signed-off-by: Sergei Ianovich <ynvich@gmail.com> > > --- > > arch/arm/cpu/pxa/pxa2xx.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm/cpu/pxa/pxa2xx.c b/arch/arm/cpu/pxa/pxa2xx.c > > index c9a7d45..93ca2f0 100644 > > --- a/arch/arm/cpu/pxa/pxa2xx.c > > +++ b/arch/arm/cpu/pxa/pxa2xx.c > > @@ -281,5 +281,5 @@ void reset_cpu(ulong ignored) > > writel(tmp, OSMR3); > > > > for (;;) > > - ; > > + writel(MDREFR_SLFRSH, MDREFR); > > Do you need to write this register in an endless loop ? I didn't think this way. We need to have at least 3, but up to 5 cycles to put SDRAM in SLFRFRSH. It depends on the current state of SDRAM. There is no way to know. It can probably work if we write just once. But if we have another thread doing something with SDRAM in between, we will still hang. I am not sure how likely is the situation, though.
On Saturday, December 14, 2013 at 04:31:31 PM, Sergei Ianovich wrote: > On Sat, 2013-12-14 at 13:29 +0100, Marek Vasut wrote: > > On Saturday, December 14, 2013 at 06:41:57 AM, Sergei Ianovich wrote: > > > Erratum 71 of PXA270M Processor Family Specification Update > > > (April 19, 2010) explains that watchdog reset time is just > > > 8us insead of 10ms in EMTS. > > > > > > If SDRAM is not reset, it causes memory bus congestion and > > > the device hangs. > > > > > > We put SDRAM in selfresh mode before watchdog reset, removing > > > potential freezes. > > > > > > Signed-off-by: Sergei Ianovich <ynvich@gmail.com> > > > --- > > > > > > arch/arm/cpu/pxa/pxa2xx.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/arch/arm/cpu/pxa/pxa2xx.c b/arch/arm/cpu/pxa/pxa2xx.c > > > index c9a7d45..93ca2f0 100644 > > > --- a/arch/arm/cpu/pxa/pxa2xx.c > > > +++ b/arch/arm/cpu/pxa/pxa2xx.c > > > @@ -281,5 +281,5 @@ void reset_cpu(ulong ignored) > > > > > > writel(tmp, OSMR3); > > > > > > for (;;) > > > > > > - ; > > > + writel(MDREFR_SLFRSH, MDREFR); > > > > Do you need to write this register in an endless loop ? > > I didn't think this way. We need to have at least 3, but up to 5 cycles > to put SDRAM in SLFRFRSH. It depends on the current state of SDRAM. > There is no way to know. OK, I seem to remember the uglinesses of the PXA DRAM controller, indeed :( > It can probably work if we write just once. But if we have another > thread doing something with SDRAM in between, we will still hang. U-Boot is single-threaded ;-) > I am not sure how likely is the situation, though. It cannot happen, really ;-) BUT (!) I understand your intention. If writing the MDREFR multiple times won't be a problem, I am _not_ opposed to this patch. So please only make sure that's not a problem and if it's not, I won't block this patch. Thank you very much for your efforts on the PXA front ! Best regards, Marek Vasut
On Sat, 2013-12-14 at 18:14 +0100, Marek Vasut wrote: > On Saturday, December 14, 2013 at 04:31:31 PM, Sergei Ianovich wrote: > > On Sat, 2013-12-14 at 13:29 +0100, Marek Vasut wrote: > > > Do you need to write this register in an endless loop ? > > > > I didn't think this way. We need to have at least 3, but up to 5 cycles > > to put SDRAM in SLFRFRSH. It depends on the current state of SDRAM. > > There is no way to know. > > OK, I seem to remember the uglinesses of the PXA DRAM controller, indeed :( > > > It can probably work if we write just once. But if we have another > > thread doing something with SDRAM in between, we will still hang. > > U-Boot is single-threaded ;-) > > > I am not sure how likely is the situation, though. > > It cannot happen, really ;-) > > BUT (!) I understand your intention. If writing the MDREFR multiple times won't > be a problem, I am _not_ opposed to this patch. So please only make sure that's > not a problem and if it's not, I won't block this patch. The relevant doc is [1, section 6.1.5.4]. Refresh rules are rather complex. However, clearing DRI and repeatedly writing to MDREFR should refresh. The refreshes should advance SDRAM state machine to "Self-refresh and Clock-stop". This is what we are trying to achieve. I've run close to 1000 reboot of patched linux kernel. This mean several billions writes to MDREFR. If a write can cause a problem, it should have already shown up. So I think it is not a problem. Nevertheless, I've put a patched U-Boot with a single write to MDREFR to test (reset every 2 sec). After several hours, it will be clear, if a single write works. Let's do it the right way. 1. http://www.marvell.com/application-processors/pxa-family/assets/pxa_27x_dev_man.pdf
On Sunday, December 15, 2013 at 12:51:44 AM, Sergei Ianovich wrote: > On Sat, 2013-12-14 at 18:14 +0100, Marek Vasut wrote: > > On Saturday, December 14, 2013 at 04:31:31 PM, Sergei Ianovich wrote: > > > On Sat, 2013-12-14 at 13:29 +0100, Marek Vasut wrote: > > > > Do you need to write this register in an endless loop ? > > > > > > I didn't think this way. We need to have at least 3, but up to 5 cycles > > > to put SDRAM in SLFRFRSH. It depends on the current state of SDRAM. > > > There is no way to know. > > > > OK, I seem to remember the uglinesses of the PXA DRAM controller, indeed > > :( > > > > > It can probably work if we write just once. But if we have another > > > thread doing something with SDRAM in between, we will still hang. > > > > U-Boot is single-threaded ;-) > > > > > I am not sure how likely is the situation, though. > > > > It cannot happen, really ;-) > > > > BUT (!) I understand your intention. If writing the MDREFR multiple times > > won't be a problem, I am _not_ opposed to this patch. So please only > > make sure that's not a problem and if it's not, I won't block this > > patch. > > The relevant doc is [1, section 6.1.5.4]. Refresh rules are rather > complex. However, clearing DRI and repeatedly writing to MDREFR should > refresh. The refreshes should advance SDRAM state machine to > "Self-refresh and Clock-stop". This is what we are trying to achieve. > > I've run close to 1000 reboot of patched linux kernel. This mean several > billions writes to MDREFR. If a write can cause a problem, it should > have already shown up. So I think it is not a problem. I saw this kernel patch, yes. > Nevertheless, I've put a patched U-Boot with a single write to MDREFR to > test (reset every 2 sec). After several hours, it will be clear, if a > single write works. Let's do it the right way. Thank you ! > 1. > http://www.marvell.com/application-processors/pxa-family/assets/pxa_27x_de > v_man.pdf Best regards, Marek Vasut
On Sun, 2013-12-15 at 06:17 +0100, Marek Vasut wrote: > On Sunday, December 15, 2013 at 12:51:44 AM, Sergei Ianovich wrote: > > Nevertheless, I've put a patched U-Boot with a single write to MDREFR to > > test (reset every 2 sec). After several hours, it will be clear, if a > > single write works. Let's do it the right way. > It works with a single write. I'll post v2 to the series. Thanks for constructive reviewing.
On Sunday, December 15, 2013 at 10:48:16 AM, Sergei Ianovich wrote: > On Sun, 2013-12-15 at 06:17 +0100, Marek Vasut wrote: > > On Sunday, December 15, 2013 at 12:51:44 AM, Sergei Ianovich wrote: > > > Nevertheless, I've put a patched U-Boot with a single write to MDREFR > > > to test (reset every 2 sec). After several hours, it will be clear, if > > > a single write works. Let's do it the right way. > > It works with a single write. I'll post v2 to the series. > > Thanks for constructive reviewing. No, thank _you_ for the efforts! Best regards, Marek Vasut
diff --git a/arch/arm/cpu/pxa/pxa2xx.c b/arch/arm/cpu/pxa/pxa2xx.c index c9a7d45..93ca2f0 100644 --- a/arch/arm/cpu/pxa/pxa2xx.c +++ b/arch/arm/cpu/pxa/pxa2xx.c @@ -281,5 +281,5 @@ void reset_cpu(ulong ignored) writel(tmp, OSMR3); for (;;) - ; + writel(MDREFR_SLFRSH, MDREFR); }
Erratum 71 of PXA270M Processor Family Specification Update (April 19, 2010) explains that watchdog reset time is just 8us insead of 10ms in EMTS. If SDRAM is not reset, it causes memory bus congestion and the device hangs. We put SDRAM in selfresh mode before watchdog reset, removing potential freezes. Signed-off-by: Sergei Ianovich <ynvich@gmail.com> --- arch/arm/cpu/pxa/pxa2xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)