[flashrom] [PATCH 2/8] The AT25F512B is quite different from the other (yet unsupported) chips in the AT25F* familiy, so rename 512B-specific stuff.

Stefan Tauner stefan.tauner at student.tuwien.ac.at
Thu Mar 31 11:28:10 CEST 2011


On Thu, 31 Mar 2011 08:35:38 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:

> Am 15.03.2011 16:29 schrieb Stefan Tauner:
> > Signed-off-by: Stefan Tauner<stefan.tauner at student.tuwien.ac.at>
> > ---
> >   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




More information about the flashrom mailing list