On Thu, 31 Mar 2011 08:35:38 +0200 Carl-Daniel Hailfinger <[email protected]> wrote:
> Am 15.03.2011 16:29 schrieb Stefan Tauner: > > Signed-off-by: Stefan Tauner<[email protected]> > > --- > > chipdrivers.h | 3 +-- > > flashchips.c | 6 ++++-- > > spi25.c | 10 +--------- > > 3 files changed, 6 insertions(+), 13 deletions(-) > > > > diff --git a/chipdrivers.h b/chipdrivers.h > > index c01ab7a..dc46fe1 100644 > > --- a/chipdrivers.h > > +++ b/chipdrivers.h > > @@ -45,13 +45,12 @@ int spi_chip_read(struct flashchip *flash, > > uint8_t *buf, int start, int len); uint8_t > > spi_read_status_register(void); int > > spi_prettyprint_status_register_at25df(struct flashchip *flash); > > int spi_prettyprint_status_register_at25df_sec(struct flashchip > > *flash); -int spi_prettyprint_status_register_at25f(struct > > flashchip *flash); +int > > spi_prettyprint_status_register_at25f512b(struct flashchip *flash); > > int spi_prettyprint_status_register_at25fs010(struct flashchip > > *flash); int spi_prettyprint_status_register_at25fs040(struct > > flashchip *flash); int spi_disable_blockprotect(struct flashchip > > *flash); int spi_disable_blockprotect_at25df(struct flashchip > > *flash); int spi_disable_blockprotect_at25df_sec(struct flashchip > > *flash); -int spi_disable_blockprotect_at25f(struct flashchip > > *flash); int spi_disable_blockprotect_at25fs010(struct flashchip > > *flash); int spi_disable_blockprotect_at25fs040(struct flashchip > > *flash); int spi_byte_program(int addr, uint8_t databyte); diff > > --git a/flashchips.c b/flashchips.c index 753a094..29a4da0 100644 > > --- a/flashchips.c +++ b/flashchips.c > > @@ -1612,8 +1612,10 @@ struct flashchip flashchips[] = { > > .block_erase = spi_block_erase_c7, > > } > > }, > > - .printlock = > > spi_prettyprint_status_register_at25f, > > - .unlock = > > spi_disable_blockprotect_at25f, > > + .printlock = > > spi_prettyprint_status_register_at25f512b, > > + /* spi_disable_blockprotect_at25df is not really > > the right way to do > > + * this, but the side effects of said function > > work here as well. */ > > For disabling block protection of SPI chips we have quite a few > functions where the side effects work just fine, but the comments > inside the function are not correct. Not sure if we have to list this > in a comment here or rather at the top of this unlock function > because it is used for multiple chips. > I see you just wanted to avoid the existing wrapper function and that > sort of makes sense... I'm undecided here. yes the purpose is to remove the wrapper and comment it in a conspicuous place. commenting it _this way_ in the non-wrapped function (spi_disable_blockprotect_at25df) of course does not make sense. adding a comment to the called, shared function mentioning all calling functions _additionally_ to the comment above would be the best think imho. redundancy is not always bad. > > + .unlock = > > spi_disable_blockprotect_at25df, .write = > > spi_chip_write_256, .read = spi_chip_read, > > }, > > diff --git a/spi25.c b/spi25.c > > index c774032..5d73411 100644 > > --- a/spi25.c > > +++ b/spi25.c > > @@ -394,7 +394,7 @@ int > > spi_prettyprint_status_register_at25df_sec(struct flashchip *flash) > > return spi_prettyprint_status_register_at25df(flash); } > > > > -int spi_prettyprint_status_register_at25f(struct flashchip *flash) > > +int spi_prettyprint_status_register_at25f512b(struct flashchip > > *flash) > > _at25f was originally intended as generic version usable by more > AT25* chips. I have a conflicting patch for this region, will repost > it so we can discuss how to merge them. all those pending patches are a pain. i know you know that. *shrug* -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
