Re: Problems with 2.6.29-rc8 and SDRAM on LDP

[Peter Barada <peterb@logicpd.com>]

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>