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. :/ 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 -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
>From bf4f3ea5233b8670457803a1b3f765e3c2677408 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..7181371 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) +void ich_set_bbar(uint32_t min_addr) { - minaddr &= BBAR_MASK; + void *bbar_addr; 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_addr = ich_spibar + 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_addr = ich_spibar + ICH9_REG_BBAR; break; default: msg_perr("Unknown chipset for BBAR setting!\n"); - break; + return; + } + + ichspi_bbar = mmio_readl(bbar_addr) & ~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, bbar_addr); + ichspi_bbar = mmio_readl(bbar_addr); + + /* 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
