[LinuxBIOS] [PATCH] Reimplementation/fixing of CS5530/CS5530A southbridge code

[Uwe Hermann]

On Thu, Jun 14, 2007 at 05:38:13PM +0200, Stefan Reinauer wrote:
> * Uwe Hermann <uwe at 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 at 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.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070615/57e4e7a9/attachment.sig>