On Mon, 13 Jun 2011 22:09:35 +0200 Carl-Daniel Hailfinger <[email protected]> wrote:
> Am 13.06.2011 20:31 schrieb Stefan Tauner: > > On Mon, 13 Jun 2011 19:24:55 +0200 > > Carl-Daniel Hailfinger <[email protected]> wrote: > > > > > >> Am 08.06.2011 04:55 schrieb Stefan Tauner: > >> > >>> ~10 lines less, documenting better what the differences are (i.e. offset > >>> of BBAR only). > >>> > >>> Signed-off-by: Stefan Tauner <[email protected]> > >>> > >>> > >> Nice cleanup! > >> Acked-by: Carl-Daniel Hailfinger <[email protected]> > >> > > ahem! :) > > > > because of idwer's remark that "msg_pdbg("Reserved bits in BBAR not > > zero: 0x%04x"..." should get a \n (thx!), i reviewed that patch again > > and noticed that it is quite broken. > > > > in the original code the real bbar is read from the register and > > checked for reserved bits before we try to set the requested min > > address. i somehow missed that, sorry. :/ > > > > Ouch. I overlooked that during review. The patch looked so simple. famous last words ;) > > the new, attached patch has following changes: > > - check for reserved bits > > - output 8 hex characters instead of 4 (i.e. 32b instead of 16b) > > - add \n to the message above > > > > Since you're already changing the function prototype of ich_set_bbar, > can you make it static? > > And another point about the coding style: > Can you use int bbar_offs instead of void *bbar_addr? > That way, it is easier to check directly which physmap region the > accesses correspond to. like so? -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
>From 5e9fca87c7704889086a61625dc55f0a997bccbc Mon Sep 17 00:00:00 2001 From: Stefan Tauner <[email protected]> Date: Mon, 13 Jun 2011 20:28:47 +0200 Subject: [PATCH] simplify ich_set_bbar less code, documenting better what the differences are (i.e. offset of BBAR only). Signed-off-by: Stefan Tauner <[email protected]> --- ichspi.c | 48 +++++++++++++++++++++--------------------------- 1 files changed, 21 insertions(+), 27 deletions(-) diff --git a/ichspi.c b/ichspi.c index b4eb236..a02ad58 100644 --- a/ichspi.c +++ b/ichspi.c @@ -555,43 +555,37 @@ static int program_opcodes(OPCODES *op, int enable_undo) * Try to set BBAR (BIOS Base Address Register), but read back the value in case * it didn't stick. */ -void ich_set_bbar(uint32_t minaddr) +static void ich_set_bbar(uint32_t min_addr) { - minaddr &= BBAR_MASK; + int bbar_off; switch (spi_programmer->type) { case SPI_CONTROLLER_ICH7: case SPI_CONTROLLER_VIA: - ichspi_bbar = mmio_readl(ich_spibar + 0x50) & ~BBAR_MASK; - if (ichspi_bbar) - msg_pdbg("Reserved bits in BBAR not zero: 0x%04x", - ichspi_bbar); - ichspi_bbar |= minaddr; - rmmio_writel(ichspi_bbar, ich_spibar + 0x50); - ichspi_bbar = mmio_readl(ich_spibar + 0x50); - /* We don't have any option except complaining. And if the write - * failed, the restore will fail as well, so no problem there. - */ - if (ichspi_bbar != minaddr) - msg_perr("Setting BBAR failed!\n"); + bbar_off = 0x50; break; case SPI_CONTROLLER_ICH9: - ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR) & ~BBAR_MASK; - if (ichspi_bbar) - msg_pdbg("Reserved bits in BBAR not zero: 0x%04x", - ichspi_bbar); - ichspi_bbar |= minaddr; - rmmio_writel(ichspi_bbar, ich_spibar + ICH9_REG_BBAR); - ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR); - /* We don't have any option except complaining. And if the write - * failed, the restore will fail as well, so no problem there. - */ - if (ichspi_bbar != minaddr) - msg_perr("Setting BBAR failed!\n"); + bbar_off = ICH9_REG_BBAR; break; default: msg_perr("Unknown chipset for BBAR setting!\n"); - break; + return; + } + + ichspi_bbar = mmio_readl(ich_spibar + bbar_off) & ~BBAR_MASK; + if (ichspi_bbar) { + msg_pdbg("Reserved bits in BBAR not zero: 0x%08x\n", + ichspi_bbar); } + min_addr &= BBAR_MASK; + ichspi_bbar |= min_addr; + rmmio_writel(ichspi_bbar, ich_spibar + bbar_off); + ichspi_bbar = mmio_readl(ich_spibar + bbar_off); + + /* We don't have any option except complaining. And if the write + * failed, the restore will fail as well, so no problem there. + */ + if (ichspi_bbar != min_addr) + msg_perr("Setting BBAR failed!\n"); } /* This function generates OPCODES from or programs OPCODES to ICH according to -- 1.7.1
_______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
