[flashrom] [PATCH 6/8] ichspi: add prettyprinting for PR registers

[Stefan Tauner]

On Sat, 17 Sep 2011 16:13:50 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:

> Am 20.08.2011 12:39 schrieb Stefan Tauner:
> > Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> 
> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
> with comments.
> 
> > ---
> >  ichspi.c |   45 +++++++++++++++++++++++++++++++--------------
> >  1 files changed, 31 insertions(+), 14 deletions(-)
> >
> > diff --git a/ichspi.c b/ichspi.c
> > index 61c0825..c38fbfc 100644
> > --- a/ichspi.c
> > +++ b/ichspi.c
> > @@ -73,10 +73,8 @@
> >  #define ICH9_REG_FREG0		0x54	/* 32 Bytes Flash Region 0 */
> >  
> >  #define ICH9_REG_PR0		0x74	/* 32 Bytes Protected Range 0 */
> > -#define ICH9_REG_PR1		0x78	/* 32 Bytes Protected Range 1 */
> > -#define ICH9_REG_PR2		0x7c	/* 32 Bytes Protected Range 2 */
> > -#define ICH9_REG_PR3		0x80	/* 32 Bytes Protected Range 3 */
> > -#define ICH9_REG_PR4		0x84	/* 32 Bytes Protected Range 4 */
> > +#define PR_WP_OFF		31	/* 31: write protection enable */
> > +#define PR_RP_OFF		15	/* 15: read protection enable */
> >  
> >  #define ICH9_REG_SSFS		0x90	/* 08 Bits */
> >  #define SSFS_SCIP_OFF		0	/* SPI Cycle In Progress */
> > @@ -1482,6 +1480,33 @@ static void do_ich9_spi_frap(uint32_t frap, int i)
> >  
> >  }
> >  
> > +	/* In contrast to FRAP and the master section of the descriptor the bits
> > +	 * in the PR registers have an inverted meaning. The bits in FRAP
> > +	 * indicate read and write access _grant_. Here they indicate read
> > +	 * and write _protection_ respectively. If both bits are 0 the address
> > +	 * bits are ignored.
> > +	 */
> > +#define ICH_PR_PERMS(pr)	(((~((pr) >> PR_RP_OFF) & 1) << 0) | \
> > +				 ((~((pr) >> PR_WP_OFF) & 1) << 1))
> > +
> > +static void prettyprint_ich9_reg_pr(int i)
> > +{
> > +	static const char *const access_names[4] = {
> > +		"locked", "read-only", "write-only", "read-write"
> > +	};
> > +	uint8_t off = ICH9_REG_PR0 + (i * 4);
> > +	uint32_t pr = mmio_readl(ich_spibar + off);
> > +	int rwperms = ICH_PR_PERMS(pr);
> > +
> > +	msg_pdbg("0x%02X: 0x%08x (PR%u", off, pr, i);
> > +	if (rwperms != 0x3)
> > +		msg_pdbg(")\n0x%08x-0x%08x is %s\n", (ICH_FREG_BASE(pr) << 12),
> > +			 (ICH_FREG_LIMIT(pr) << 12) | 0x0fff,
> > +			 access_names[rwperms]);
> > +	else
> > +		msg_pdbg(", unused)\n");
> 
> dbg2 IMHO. And why do you need two lines of output per PR register?

dbg2 now.

to match the output of the FREG registers:
0x54: 0x00000000 (FREG0: Flash Descriptor)
0x00000000-0x00000fff is read-only
0x58: 0x07ff0500 (FREG1: BIOS)
0x00500000-0x007fffff is read-write
0x5C: 0x04ff0003 (FREG2: Management Engine)
0x00003000-0x004fffff is locked
0x60: 0x00020001 (FREG3: Gigabit Ethernet)
0x00001000-0x00002fff is read-write
0x64: 0x00000fff (FREG4: Platform Data)
Platform Data region is unused.
0x74: 0x9fff07d0 (PR0)
0x007d0000-0x01ffffff is read-only
0x78: 0x00000000 (PR1, unused)
0x7C: 0x00000000 (PR2, unused)
0x80: 0x00000000 (PR3, unused)
0x84: 0x00000000 (PR4, unused)

as you can see i even save one uninteresting line for each unused
protection just for you! ;)

> > +}
> > +
> >  static const struct spi_programmer spi_programmer_ich7 = {
> >  	.type = SPI_CONTROLLER_ICH7,
> >  	.max_data_read = 64,
> > @@ -1623,16 +1648,8 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb,
> >  		for(i = 0; i < 5; i++)
> >  			do_ich9_spi_frap(tmp, i);
> >  
> > -		msg_pdbg("0x74: 0x%08x (PR0)\n",
> > -			     mmio_readl(ich_spibar + ICH9_REG_PR0));
> > -		msg_pdbg("0x78: 0x%08x (PR1)\n",
> > -			     mmio_readl(ich_spibar + ICH9_REG_PR1));
> > -		msg_pdbg("0x7C: 0x%08x (PR2)\n",
> > -			     mmio_readl(ich_spibar + ICH9_REG_PR2));
> > -		msg_pdbg("0x80: 0x%08x (PR3)\n",
> > -			     mmio_readl(ich_spibar + ICH9_REG_PR3));
> > -		msg_pdbg("0x84: 0x%08x (PR4)\n",
> > -			     mmio_readl(ich_spibar + ICH9_REG_PR4));
> > +		for(i = 0; i < 5; i++)
> > +			prettyprint_ich9_reg_pr(i);
> 
> IIRC some chipset generations only support PR0-PR3. Besides that, it
> would be nice to have those prettyprinters for ICH7 as well if
> possible/applicable.

lets see.
ich7  has 3 of them starting at 60h
ich8  has 5 of them starting at 74h
ich9  has 5 of them starting at 74h
ich10 has 5 of them starting at 74h

that would be easy... BUT ich7's PR are very different:
 - they only have a write protection bit, not read protection
 - the address ranges are encoded in different bits of the registers.
 - ich8+ support 2 flash chips hence need 25 bits for addresses...

so yes it would be nice, but requires more work which i wont do right
now. committed with the verbosity change only in r1446
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner