On Thu, Jun 14, 2007 at 05:38:13PM +0200, Stefan Reinauer wrote:
- Uwe Hermann uwe@hermann-uwe.de [070606 23:34]:
code and had several other problems, e.g. it enabled write access to the ROM (why?)
for flash updates so flashrom does not have to do it.
OK, then I'll drop this. That's the job of flashrom.
- /* Decode 0x000E0000-0x000FFFFF (128 KB), not just 64 KB. */
- reg8 = pci_read_config8(dev, ROM_AT_LOGIC_CONTROL_REG);
- reg8 |= LOWER_ROM_ADDRESS_RANGE;
- pci_write_config8(dev, ROM_AT_LOGIC_CONTROL_REG, reg8);
I'd drop that and rather put ram there. LinuxBIOS does not use the <1M space.
OK, I'll test on hardware whether this has any consequences, but I guess not, so I'll drop it.
- /* TODO: Make ROM writable? As config option maybe? */
If support is in flashrom, we should not, i guess.
Yep.
Acked-by: Stefan Reinauer stepan@coresystems.de
please fix above issues before or after the commit.
One more issue / poll:
What looks better and is more readable/understandable/efficient/short?
a) if (conf->ide1_enable) { reg8 |= SECONDARY_IDE_ENABLE; } else { reg8 &= ~(SECONDARY_IDE_ENABLE); }
b)
reg8 = (conf->ide1_enable ? (reg8 | SECONDARY_IDE_ENABLE) : (reg8 & ~(SECONDARY_IDE_ENABLE)));
c)
Some even more elegant solution?
Well, a) is probably easier to understand, but b) is 3 lines shorter, and the 'foo ? bar : baz' idiom should be pretty well-known, I guess.
Opinions?
Uwe.