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 <[email protected]>
Acked-by: Carl-Daniel Hailfinger <[email protected]> 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/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
