Ubuntu
x-loader package

x-loader-omap: identify_xm_ddr() in support_micron_and_numonyx_memory.patch doesn't return a value on all code paths

Bug #652850 reported by Peter Maydell
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
x-loader (Ubuntu)
New
Undecided
Unassigned

Bug Description

Package: x-loader
Version: 1.4.4git20100713-1ubuntu2

The patch debian/patches/support_micron_and_numonyx_memory.patch (introduced to fix LP: #628243) includes the new function int identify_xm_ddr() which ends like this:

        nand_readid(&mfr, &id);
        if (mfr == 0)
                return MICRON_DDR;
        if ((mfr == 0x20) && (id == 0xba))
                return NUMONYX_MCP;
}

If the mfr read from the NAND is not recognised as either Micron or Numonyx we just fall off the end of the function without returning any value, so we might randomly try either (but probably will guess Numonyx given the structure of the calling function).

Found by code inspection while looking for a different problem.

Revision history for this message
Peter Maydell (pmaydell) wrote :

Also, we ought to put the #defines from drivers/k9f1g08r0a.c in a header file and use them rather than magic numbers:
#define MT29F1G_MFR 0x2c /* Micron */
#define MT29F1G_MFR2 0x20 /* numonyx */
#define MT29F1G_ID 0xa1 /* x8, 1GiB */
#define MT29F2G_ID 0xba /* x16, 2GiB */
#define MT29F4G_ID 0xbc /* x16, 4GiB */

And why does identify_xm_ddr() want to see 0 but k9f1g08r0a.c think micron is 0x2c ? That looks a bit suspicious.

(Code cleanup, if we care: k9f1g08r0a.c:nand_chip() should use the new nand_readid() function.)

Revision history for this message
Steve Sakoman (steve-sakoman) wrote :

> And why does identify_xm_ddr() want to see 0 but k9f1g08r0a.c think micron is 0x2c ? That looks a bit suspicious.

The Micron POP used on the Beagleboard xM has no NAND, hence it returns 0. Yeah, this is certainly not obvious from the code :-)

This code is far from bullet-proof, but that could be said of x-load in general!

I'll put a fix patch in my repository and also ping the Beagleboard folks about it.

X-load could use lots of clean-up, but like many others, I tend to think the energy would be better spent in finding a replacement.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.