[flashrom] [PATCH 3/8] ichspi: don't touch the nonexistent(?) BBAR register on ICH8
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Sat Sep 17 16:00:21 CEST 2011
Am 20.08.2011 12:39 schrieb Stefan Tauner:
> Until now we implicitly accessed it via the ICH9 offset. I think
> this is an evident sign that the naming of the spi controller types
> in ichspi.c (SPI_CONTROLLER_VIA, SPI_CONTROLLER_ICH7,
> SPI_CONTROLLER_ICH9) might not cut it and we should think about
> introducing a special ICH8 one, even if its struct is identical to
> the ICH9. having most of the ICH8 code path guarded/controlled by
> SPI_CONTROLLER_ICH7 or SPI_CONTROLLER_ICH9 (depending on which is
> more similar in one situation), is an open invitation to similar
> bugs because one easily forgets that ICH8 is very special.
>
> 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.
> diff --git a/ichspi.c b/ichspi.c
> index 343a0af..d51a6b2 100644
> --- a/ichspi.c
> +++ b/ichspi.c
> @@ -558,20 +558,19 @@ 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.
> */
> -static void ich_set_bbar(uint32_t min_addr)
> +static void ich_set_bbar(int ich_generation, uint32_t min_addr)
> {
> int bbar_off;
> - switch (spi_programmer->type) {
> - case SPI_CONTROLLER_ICH7:
> - case SPI_CONTROLLER_VIA:
> + switch (ich_generation) {
> + case 7:
> bbar_off = 0x50;
Would be nice to have a #define for that.
> break;
> - case SPI_CONTROLLER_ICH9:
> + case 8:
> + msg_perr("BBAR offset is unknown on ICH8!\n");
> + return;
> + default: /* Future version might behave the same */
> bbar_off = ICH9_REG_BBAR;
> break;
> - default:
> - msg_perr("Unknown chipset for BBAR setting!\n");
> - return;
> }
>
> ichspi_bbar = mmio_readl(ich_spibar + bbar_off) & ~BBAR_MASK;
> @@ -589,6 +588,8 @@ static void ich_set_bbar(uint32_t min_addr)
> */
> if (ichspi_bbar != min_addr)
> msg_perr("Setting BBAR failed!\n");
> + else
> + msg_pspew("Setting BBAR succeeded!\n");
Do we really need that message? It should be implicit from the absence
of "...failed", and msg_pspew is used almost never nowadays, so it
mostly bloats the binary without real benefit.
> }
>
> /* Read len bytes from the fdata/spid register into the data array.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the flashrom
mailing list