[flashrom] flashrom on PowerPC (002-lh28f008bjt.patch)

Uwe Hermann uwe at hermann-uwe.de
Fri Aug 26 00:51:26 CEST 2011


Hi,

this is committed as r1420 with some of the changes mentioned.

On Sun, Aug 14, 2011 at 04:12:02AM +0200, Stefan Tauner wrote:
> > +int unlock_lh28f008bjt(struct flashchip *flash)
> > +{
> > +	chipaddr bios = flash->virtual_memory;
> > +	uint8_t mcfg, bcfg, need_unlock = 0, can_unlock = 0;
> 
> mixing initialized and declared-only variables is :(

Fixed.

 
> > +	/* Read block lock-bits, 8 * 8 KiB + 15 * 64 KiB */
> 
> although correct, we don't use the IEC prefixes nowhere else...

Changed.

 
> > +	for (i = 0; i < flash->total_size * 1024; i+= (i >= (64 * 1024) ? 64 * 1024 : 8 * 1024)) {
> 
> hm... line limit. looks very odd at first sight. maybe a while loop
> would be better?

Wrapped the line, other changes maybe in another patch or so.


> > +		bcfg = chip_readb(bios + i + 2); // read block lock config
> 
> /* */?
> not that i find this convention necessary...

Fixed.

 
> you start messages lower-case and end them with an exclamation mark very
> often. we usually do this for errors (or warnings) only.

Same as in some other function in that file, we should probably unify
the style in a global manner at some point. Left unchanged for now.

 
> > Index: flashchips.c
> > ===================================================================
> > --- flashchips.c	(revision 1396)
> > +++ flashchips.c	(working copy)
> > @@ -5379,6 +5379,36 @@
> >  
> >  	{
> >  		.vendor		= "Sharp",
> > +		.name		= "LH28F008BJT-BTLZ1",
> 
> why the -BTLZ1 suffix?
> i have just looked briefly at the LH28F008BJT-BTLZ1 and
> LH28F008BJT-BTLZC datasheets, but they seem to be almost identical?

Yup, that's because the BTLZC is basically the ROHS variant of the
BTLZ1. However, there are also LH28F008BJT-TTLZ2 and LH28F008BJT-TTLZB,
which are different chips (different ID, top boot block instead of
bottom boot block), where TTLZB is the ROHS version of the TTLZ2.

> 
> > +		.bustype	= BUS_PARALLEL,
> > +		.manufacture_id	= SHARP_ID,
> > +		.model_id	= SHARP_LH28F008BJxxPB,
> 
> i am not sure about those id names either, but they are not as
> important because they are not visible to the user.

Yeah, should probably be fixed too (different patch), I know of the
following four "LH28F008BJT" chips, see above:

LH28F008BJT-BTLZ1 / LH28F008BJT-BTLZC
LH28F008BJT-TTLZ2 / LH28F008BJT-TTLZB

SHARP_LH28F008BJT_BTLZx or similar might make sense.


Uwe. 
-- 
http://hermann-uwe.de     | http://sigrok.org
http://randomprojects.org | http://unmaintained-free-software.org




More information about the flashrom mailing list