[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