From: "Menon, Nishanth" <nm@ti.com>
To: Paul Walmsley <paul@pwsan.com>
Cc: linux-omap@vger.kernel.org, Tero.Kristo@nokia.com,
r-woodruff2@ti.com, b-cousson@ti.com, rnayak@ti.com,
c-sucur@ti.com
Subject: Re: [PATCH] OMAP3: SDRC: Comment out SDRC AC timing and MR changes in CORE DVFS SRAM code
Date: Wed, 02 Dec 2009 08:12:34 +0200 [thread overview]
Message-ID: <4B160552.6050108@ti.com> (raw)
In-Reply-To: <alpine.DEB.2.00.0912011832290.13408@utopia.booyaka.com>
Paul Walmsley said the following on 12/02/2009 03:33 AM:
> The code that reprograms the SDRC memory controller during CORE DVFS,
> mach-omap2/sram34xx.S:omap3_sram_configure_core_dpll(), does not
> ensure that all L3 initiators are prevented from accessing the SDRAM
> before modifying the SDRC AC timing and MR registers. This can cause
> memory to be corrupted or cause the SDRC to enter an unpredictable
> state. This patch comments out that code for now and adds a note
> explaining what is going on. Ideally it can be added back in once
> supporting code is present to ensure that other initiators aren't
> touching the SDRAM. At the very least, these registers should be
> reprogrammable during kernel init to deal with buggy bootloaders.
>
> This is a modification of a patch originally written by Rajendra Nayak
> <rnayak@ti.com> (the original is at http://patchwork.kernel.org/patch/51927/).
> Rather than removing the code completely, this patch just comments it out.
>
why not make this a #ifdef instead if we need it some other time, a #if
0 and it's intention might not be readable without doing a git annotate
in a few months time..
> Thanks to Benoît Cousson <b-cousson@ti.com> and Christophe Sucur
> <c-sucur@ti.com> for explaining the technical basis for this and for
> explaining what can be done to make this path work in future code.
> Thanks to Richard Woodruff <r-woodruff2@ti.com> for his comments.
>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Rajendra Nayak <rnayak@ti.com>
> Cc: Christophe Sucur <c-sucur@ti.com>
> Cc: Benoît Cousson <b-cousson@ti.com>
> Cc: Richard Woodruff <r-woodruff2@ti.com>
> ---
> arch/arm/mach-omap2/sram34xx.S | 18 ++++++++++++++++--
> 1 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
> index 82aa4a3..8fa8955 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -91,8 +91,18 @@
> * new SDRC_ACTIM_CTRL_B_1 register contents
> * new SDRC_MR_1 register value
> *
> - * If the param SDRC_RFR_CTRL_1 is 0, the parameters
> - * are not programmed into the SDRC CS1 registers
> + * If the param SDRC_RFR_CTRL_1 is 0, the parameters are not programmed into
> + * the SDRC CS1 registers
> + *
> + * NOTE: This code no longer attempts to program the SDRC AC timing and MR
> + * registers. This is because the code currently cannot ensure that all
> + * L3 initiators (e.g., sDMA, IVA, DSS DISPC, etc.) are not accessing the
> + * SDRAM when the registers are written. If the registers are changed while
> + * an initiator is accessing SDRAM, memory can be corrupted and/or the SDRC
> + * may enter an unpredictable state. The code to reprogram the registers,
> + * however, has been left in -- commented out in "#if 0" .. "#endif" blocks --
> + * since in the future, the intent is to re-enable this code in cases where we
> + * can ensure that no initiators are touching the SDRAM.
> */
> ENTRY(omap3_sram_configure_core_dpll)
> stmfd sp!, {r1-r12, lr} @ store regs to stack
> @@ -219,6 +229,7 @@ configure_sdrc:
> ldr r12, omap_sdrc_rfr_ctrl_0_val @ fetch value from SRAM
> ldr r11, omap3_sdrc_rfr_ctrl_0 @ fetch addr from SRAM
> str r12, [r11] @ store
> +#if 0
> ldr r12, omap_sdrc_actim_ctrl_a_0_val
> ldr r11, omap3_sdrc_actim_ctrl_a_0
> str r12, [r11]
> @@ -228,11 +239,13 @@ configure_sdrc:
> ldr r12, omap_sdrc_mr_0_val
> ldr r11, omap3_sdrc_mr_0
> str r12, [r11]
> +#endif
> ldr r12, omap_sdrc_rfr_ctrl_1_val
> cmp r12, #0 @ if SDRC_RFR_CTRL_1 is 0,
> beq skip_cs1_prog @ do not program cs1 params
> ldr r11, omap3_sdrc_rfr_ctrl_1
> str r12, [r11]
> +#if 0
> ldr r12, omap_sdrc_actim_ctrl_a_1_val
> ldr r11, omap3_sdrc_actim_ctrl_a_1
> str r12, [r11]
> @@ -242,6 +255,7 @@ configure_sdrc:
> ldr r12, omap_sdrc_mr_1_val
> ldr r11, omap3_sdrc_mr_1
> str r12, [r11]
> +#endif
> skip_cs1_prog:
> ldr r12, [r11] @ posted-write barrier for SDRC
> bx lr
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2009-12-02 6:12 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-10-06 13:20 [PATCH] OMAP3: DVFS: No sdrc AC timing changes during core dvfs Rajendra Nayak
2009-10-21 1:21 ` Paul Walmsley
2009-10-21 4:07 ` Woodruff, Richard
2009-10-22 8:58 ` Cousson, Benoit
2009-10-22 12:20 ` Woodruff, Richard
2009-11-24 14:35 ` Tero.Kristo
2009-12-02 1:33 ` [PATCH] OMAP3: SDRC: Comment out SDRC AC timing and MR changes in CORE DVFS SRAM code Paul Walmsley
2009-12-02 6:12 ` Menon, Nishanth [this message]
2009-12-02 15:56 ` Olof Johansson
2009-12-02 16:22 ` Paul Walmsley
2009-12-03 13:41 ` [PATCH v2] " Paul Walmsley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B160552.6050108@ti.com \
--to=nm@ti.com \
--cc=Tero.Kristo@nokia.com \
--cc=b-cousson@ti.com \
--cc=c-sucur@ti.com \
--cc=linux-omap@vger.kernel.org \
--cc=paul@pwsan.com \
--cc=r-woodruff2@ti.com \
--cc=rnayak@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.