On Wed, 27 Jul 2011 18:20:19 +0200
Mattias Mattsson <[email protected]> wrote:

> Index: 82802ab.c
> ===================================================================
> --- 82802ab.c (revision 1396)
> +++ 82802ab.c (working copy)
> @@ -208,3 +208,56 @@
>  
>       return 0;
>  }
> +
> +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 :(

> +     int i;
> +
> +     /* Wait if chip is busy */
> +     wait_82802ab(flash);
> +
> +     /* Read identifier codes */
> +     chip_writeb(0x90, bios);
> +
> +     /* Read master lock-bit */
> +     mcfg = chip_readb(bios + 0x3);
> +     msg_cdbg("master lock is ");
> +     if (mcfg) {
> +             msg_cdbg("locked!\n");
> +     } else {
> +             msg_cdbg("unlocked!\n");
> +             can_unlock = 1;
> +     }
> +
> +     /* Read block lock-bits, 8 * 8 KiB + 15 * 64 KiB */

although correct, we don't use the IEC prefixes nowhere else...

> +     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?

> +             bcfg = chip_readb(bios + i + 2); // read block lock config

/* */?
not that i find this convention necessary...

> +             msg_cdbg("block lock at %06x is %slocked!\n", i, bcfg ? "" : 
> "un");
> +             if (bcfg) {
> +                     need_unlock = 1;
> +             }
> +     }
> +
> +     /* Reset chip */
> +     chip_writeb(0xFF, bios);
> +
> +     /* Unlock: clear block lock-bits, if needed */
> +     if (can_unlock && need_unlock) {
> +             msg_cdbg("Unlock: ");
> +             chip_writeb(0x60, bios);
> +             chip_writeb(0xD0, bios);
> +             chip_writeb(0xFF, bios);
> +             wait_82802ab(flash);
> +             msg_cdbg("Done!\n");
> +     }
> +
> +     /* Error: master locked or a block is locked */
> +     if (!can_unlock && need_unlock) {
> +             msg_cerr("At least one block is locked and lockdown is 
> active!\n");
> +             return -1;
> +     }
> +
> +     return 0;
> +}

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

> 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?

> +             .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.

> +             .total_size     = 1024,
> +             .page_size      = 64 * 1024,
> +             .tested         = TEST_OK_PREW,
> +             .probe          = probe_82802ab,
> +             .probe_timing   = TIMING_ZERO,
> +             .block_erasers  =
> +             {
> +                     {
> +                             .eraseblocks = {
> +                                     {8 * 1024, 8},
> +                                     {64 * 1024, 15}
> +                              },
> +                             .block_erase = erase_block_82802ab,
> +                     }, {
> +                             .eraseblocks = { {1024 * 1024, 1} },
> +                             .block_erase = erase_sector_49lfxxxc,
> +                     }
> +             },
> +             .unlock         = unlock_lh28f008bjt,
> +             .write          = write_82802ab,
> +             .read           = read_memmapped,
> +             .voltage        = {2700, 3600},
> +     },
> +
> +     {
> +             .vendor         = "Sharp",
>               .name           = "LHF00L04",
>               .bustype        = BUS_FWH, /* A/A Mux */
>               .manufacture_id = SHARP_ID,
> Index: chipdrivers.h
> ===================================================================
> --- chipdrivers.h     (revision 1396)
> +++ chipdrivers.h     (working copy)
> @@ -83,6 +83,7 @@
>  void print_status_82802ab(uint8_t status);
>  int unlock_82802ab(struct flashchip *flash);
>  int unlock_28f004s5(struct flashchip *flash);
> +int unlock_lh28f008bjt(struct flashchip *flash);
>  
>  /* jedec.c */
>  uint8_t oddparity(uint8_t val);


-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner

_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to