[flashrom] [PATCH 03/12] add macros and pretty printing for HSFS and HSFC

[Stefan Tauner]

On Wed, 08 Jun 2011 23:48:54 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:

> Am 08.06.2011 04:55 schrieb Stefan Tauner:
> > Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> > ---
> >  ichspi.c |   74 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
> >  1 files changed, 62 insertions(+), 12 deletions(-)
> >
> > diff --git a/ichspi.c b/ichspi.c
> > index 7e4a363..12727d1 100644
> > --- a/ichspi.c
> > +++ b/ichspi.c
> > @@ -43,6 +43,37 @@
> >  #include "spi.h"
> >  
> >  /* ICH9 controller register definition */
> > +#define ICH9_REG_HSFS		0x04	/* 16 Bits Hardware Sequencing Flash Status */
> > +#define HSFS_FDONE_OFF		0	/* 0: Flash Cycle Done */
> > +#define HSFS_FDONE		(0x1 << HSFS_FDONE_OFF)
> > +#define HSFS_FCERR_OFF		1	/* 1: Flash Cycle Error */
> > +#define HSFS_FCERR		(0x1 << HSFS_FCERR_OFF)
> > +#define HSFS_AEL_OFF		2	/* 2: Access Error Log */
> > +#define HSFS_AEL		(0x1 << HSFS_AEL_OFF)
> > +#define HSFS_BERASE_OFF		3	/* 3-4: Block/Sector Erase Size */
> > +#define HSFS_BERASE		(0x1 << HSFS_BERASE_OFF)
> >   
> 
> Code change! Please explain/fix.

good catch!
should have been (and i thought i have seen+fixed it already :/ ):
#define HSFS_BERASE_OFF		3	/* 3-4: Block/Sector Erase Size */
#define HSFS_BERASE		(0x3 << HSFS_BERASE_OFF)
thank you

> > @@ -1204,14 +1257,15 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
> >  	case SPI_CONTROLLER_ICH9:
> >  		tmp2 = mmio_readw(ich_spibar + 4);
> >  		msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2);
> > -		msg_pdbg("FLOCKDN %i, ", (tmp2 >> 15 & 1));
> > -		msg_pdbg("FDV %i, ", (tmp2 >> 14) & 1);
> > -		msg_pdbg("FDOPSS %i, ", (tmp2 >> 13) & 1);
> > -		msg_pdbg("SCIP %i, ", (tmp2 >> 5) & 1);
> > -		msg_pdbg("BERASE %i, ", (tmp2 >> 3) & 3);
> > -		msg_pdbg("AEL %i, ", (tmp2 >> 2) & 1);
> > -		msg_pdbg("FCERR %i, ", (tmp2 >> 1) & 1);
> > -		msg_pdbg("FDONE %i\n", (tmp2 >> 0) & 1);
> > +		prettyprint_ich9_reg_hsfs(tmp2);
> > +		if (tmp2 & (1 << 15)) {
> > +			msg_pinfo("WARNING: SPI Configuration Lockdown activated.\n");
> > +			ichspi_lock = 1;
> > +		}
> > +
> > +		tmp2 = mmio_readw(ich_spibar + 6);
> > +		msg_pdbg("0x06: 0x%04x (HSFC)\n", tmp2);
> > +		prettyprint_ich9_reg_hsfc(tmp2);
> >  
> >  		tmp = mmio_readl(ich_spibar + 0x50);
> >  		msg_pdbg("0x50: 0x%08x (FRAP)\n", tmp);
> > @@ -1258,10 +1312,6 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
> >  			     ichspi_bbar);
> >  		msg_pdbg("0xB0: 0x%08x (FDOC)\n",
> >  			     mmio_readl(ich_spibar + 0xB0));
> > -		if (tmp2 & (1 << 15)) {
> > -			msg_pinfo("WARNING: SPI Configuration Lockdown activated.\n");
> > -			ichspi_lock = 1;
> > -		}
> >   
> 
> Was it intentional to move that code a few lines up?

jup, because it does not make sense to postpone it - it belongs to the
HSFS output. if -V is not used it will be at the end anyway because
there is no other output but warnings. if the output is verbose then no
user will notice the line as it is anyway imho. i need ichspi_lock==1
earlier to register the hw sequencing programmer instead of the normal
ich9 programmer (see hwseq patch), but we have not decided if we want
to do it that way. even if so we could introduce a second if just for
the warning and output it at the end.
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner