Problems with 2.6.29-rc8 and SDRAM on LDP

Problems with 2.6.29-rc8 and SDRAM on LDP

[Peter Barada]

I'm basing a port of 2.6.29-rc8 (khilmans's PM branch) off of the LDP
configuration, and in arch/arm/mach-omap2/board-ldp, the call to
init_common_hw passes in four NULLs which cause the kernel to oops on
startup in omap2_sdrc_get_params, most likely because there is no
sdrc_params for it to sift through.

I tried to use the mt46j32m32lfc_sdrc_params(as in omap3beagle.c), but
that dies after the message "Reprogramming the SDRC" with a unhandled
fault, but no traceback.  I think this may be due to the OMAP2520LV SOM
board I'm using has a MT29C2G24MAKLAJG-6 IT instead of the MT46J32M32LFC
part, and SDRAM is all mucked up.

I assume that I have to create a sdram-mt29c262maklajg.h file that
contains the sdrc_params to use, but can anyone explain the differences
between the 5 entries in the array it would contain?  I know they are
tied to OPP levels, but "rate" seems to go down as the OPP level goes
up.  This doesn't make sense to me as I'd expect the rate to go up with
OPP level.  What am I missing?

-- 
Peter Barada <peterb@logicpd.com>

Re: Problems with 2.6.29-rc8 and SDRAM on LDP

[Paul Walmsley]

Hello Peter,

On Sun, 22 Mar 2009, Peter Barada wrote:

> I'm basing a port of 2.6.29-rc8 (khilmans's PM branch) off of the LDP
> configuration, and in arch/arm/mach-omap2/board-ldp, the call to
> init_common_hw passes in four NULLs which cause the kernel to oops on
> startup in omap2_sdrc_get_params, most likely because there is no
> sdrc_params for it to sift through.

Indeed; thanks for the report, patch at the bottom.  Could you give it a 
try?

> I tried to use the mt46j32m32lfc_sdrc_params(as in omap3beagle.c), but
> that dies after the message "Reprogramming the SDRC" with a unhandled
> fault, but no traceback.  I think this may be due to the OMAP2520LV SOM
> board I'm using has a MT29C2G24MAKLAJG-6 IT instead of the MT46J32M32LFC
> part, and SDRAM is all mucked up.

Looking this part up on the web search engines, one finds

  http://www.micron.com/products/partdetail?part=MT29C2G24MAKLAJG-6%20IT

which in turn provides the datasheet

  http://download.micron.com/pdf/datasheets/dram/mobile/1gb_ddr_mobile_sdram_t48m.pdf

This datasheet is the same datasheet used for the MT46J32M32LF part, so 
the existing mt46h32m32lf6_sdrc_params structure should work for the 
MT29C2G24MAKLAJG-6.  I'm at a loss to explain why it didn't work for you.

> I assume that I have to create a sdram-mt29c262maklajg.h file that
> contains the sdrc_params to use, but can anyone explain the differences
> between the 5 entries in the array it would contain?  I know they are
> tied to OPP levels, but "rate" seems to go down as the OPP level goes
> up.  This doesn't make sense to me as I'd expect the rate to go up with
> OPP level.  What am I missing?

It's just a coincidence that there are five array members.  These are 
unrelated to OPPs.  For most sane use cases there would only need to be 
two array entries; one for full speed and one for half speed; but some 
early TI bootloaders programmed DPLL3 to a slightly derated speed, hence 
the extra two unusual rates in these files.

The register settings were generated by a program here, 
calc-sdrc-params.c, that takes datasheet parameters and computes the SDRC 
register settings.  I'm happy to send it to anyone who asks me via E-mail 
until I have the chance to clean it up for public posting.  We may also 
ultimately compute these at runtime as board-rx51*.c does.


regards

- Paul


From: Paul Walmsley <paul@pwsan.com>
Date: Mon, 23 Mar 2009 00:33:28 -0600
Subject: [PATCH] SDRC: prevent null pointer dereference if sdrc_init_params is null

omap2_sdrc_get_params() should check to see if a board-*.c file has
called omap2_init_common_hw() with a null pointer for the
omap_sdrc_params argument, rather than attempting to dereference it.
Otherwise, boot will fail after the "Reprogramming SDRC" boot message.
Problem found by Peter Barada <peterb@logicpd.com>.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Peter Barada <peterb@logicpd.com>
---
 arch/arm/mach-omap2/sdrc.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/sdrc.c b/arch/arm/mach-omap2/sdrc.c
index 7da6587..993fd25 100644
--- a/arch/arm/mach-omap2/sdrc.c
+++ b/arch/arm/mach-omap2/sdrc.c
@@ -56,6 +56,9 @@ struct omap_sdrc_params *omap2_sdrc_get_params(unsigned long r)
 {
 	struct omap_sdrc_params *sp;
 
+	if (!sdrc_init_params)
+		return NULL;
+
 	sp = sdrc_init_params;
 
 	while (sp->rate && sp->rate != r)
-- 
1.6.0.2.GIT

Re: Problems with 2.6.29-rc8 and SDRAM on LDP

[Paul Walmsley]

Hello Peter, 

On Mon, 23 Mar 2009, Paul Walmsley wrote:

> On Sun, 22 Mar 2009, Peter Barada wrote:
> 
> > I assume that I have to create a sdram-mt29c262maklajg.h file that
> > contains the sdrc_params to use, but can anyone explain the differences
> > between the 5 entries in the array it would contain?  I know they are
> > tied to OPP levels, but "rate" seems to go down as the OPP level goes
> > up.  This doesn't make sense to me as I'd expect the rate to go up with
> > OPP level.  What am I missing?
> 
> It's just a coincidence that there are five array members.  These are 
> unrelated to OPPs.

I should clarify this a bit.  It is the array _indices_ that are unrelated 
to the OPPs.  The SDRC rates are of course related to the CORE DVFS OPPs, 
in that CORE OPP3 corresponds to the 133MHz rate, and CORE OPP2 
corresponds to the 83MHz rate.


- Paul

Re: Problems with 2.6.29-rc8 and SDRAM on LDP

[Peter Barada]

On Mon, 2009-03-23 at 00:38 -0600, Paul Walmsley wrote:
> Hello Peter,
> 
> On Sun, 22 Mar 2009, Peter Barada wrote:
> 
> > I'm basing a port of 2.6.29-rc8 (khilmans's PM branch) off of the LDP
> > configuration, and in arch/arm/mach-omap2/board-ldp, the call to
> > init_common_hw passes in four NULLs which cause the kernel to oops on
> > startup in omap2_sdrc_get_params, most likely because there is no
> > sdrc_params for it to sift through.
> 
> Indeed; thanks for the report, patch at the bottom.  Could you give it a 
> try?

With your patch and a NULL as first paramter to init_common_hw(), works
for me on my OMAP3530_LV_SOM configuration (cloned from LDP).  Now to
turn on CONFIG_PM_SUSPEND and see if it will suspend/resume with a
minimal setup (only serial, ethernet), then I can start adding in the
other drivers.

> > I tried to use the mt46j32m32lfc_sdrc_params(as in omap3beagle.c), but
> > that dies after the message "Reprogramming the SDRC" with a unhandled
> > fault, but no traceback.  I think this may be due to the OMAP2520LV SOM
> > board I'm using has a MT29C2G24MAKLAJG-6 IT instead of the MT46J32M32LFC
> > part, and SDRAM is all mucked up.
> 
> Looking this part up on the web search engines, one finds
> 
>   http://www.micron.com/products/partdetail?part=MT29C2G24MAKLAJG-6%20IT
> 
> which in turn provides the datasheet
> 
>   http://download.micron.com/pdf/datasheets/dram/mobile/1gb_ddr_mobile_sdram_t48m.pdf
> 
> This datasheet is the same datasheet used for the MT46J32M32LF part, so 
> the existing mt46h32m32lf6_sdrc_params structure should work for the 
> MT29C2G24MAKLAJG-6.  I'm at a loss to explain why it didn't work for you.

> > I assume that I have to create a sdram-mt29c262maklajg.h file that
> > contains the sdrc_params to use, but can anyone explain the differences
> > between the 5 entries in the array it would contain?  I know they are
> > tied to OPP levels, but "rate" seems to go down as the OPP level goes
> > up.  This doesn't make sense to me as I'd expect the rate to go up with
> > OPP level.  What am I missing?
> 
> It's just a coincidence that there are five array members.  These are 
> unrelated to OPPs.  For most sane use cases there would only need to be 
> two array entries; one for full speed and one for half speed; but some 
> early TI bootloaders programmed DPLL3 to a slightly derated speed, hence 
> the extra two unusual rates in these files.

My bad, I was sifting through the code path from init_common_hw, and saw
the comments around omap3_mpu_rate_table in omap3-opp.h and jumped to
conclude they're connected...

> The register settings were generated by a program here, 
> calc-sdrc-params.c, that takes datasheet parameters and computes the SDRC 
> register settings.  I'm happy to send it to anyone who asks me via E-mail 
> until I have the chance to clean it up for public posting.  We may also 
> ultimately compute these at runtime as board-rx51*.c does.

Paul, can you send it along to me?  TIA...

> 
> regards
> 
> - Paul
> 
> 
> From: Paul Walmsley <paul@pwsan.com>
> Date: Mon, 23 Mar 2009 00:33:28 -0600
> Subject: [PATCH] SDRC: prevent null pointer dereference if sdrc_init_params is null
> 
> omap2_sdrc_get_params() should check to see if a board-*.c file has
> called omap2_init_common_hw() with a null pointer for the
> omap_sdrc_params argument, rather than attempting to dereference it.
> Otherwise, boot will fail after the "Reprogramming SDRC" boot message.
> Problem found by Peter Barada <peterb@logicpd.com>.
> 
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Peter Barada <peterb@logicpd.com>
> ---
>  arch/arm/mach-omap2/sdrc.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/sdrc.c b/arch/arm/mach-omap2/sdrc.c
> index 7da6587..993fd25 100644
> --- a/arch/arm/mach-omap2/sdrc.c
> +++ b/arch/arm/mach-omap2/sdrc.c
> @@ -56,6 +56,9 @@ struct omap_sdrc_params *omap2_sdrc_get_params(unsigned long r)
>  {
>  	struct omap_sdrc_params *sp;
>  
> +	if (!sdrc_init_params)
> +		return NULL;
> +
>  	sp = sdrc_init_params;
>  
>  	while (sp->rate && sp->rate != r)
-- 
Peter Barada <peterb@logicpd.com>