[coreboot] [PATCH] flashrom: Rewrite SB600 chipset enable function

[Carl-Daniel Hailfinger]

On 05.05.2009 18:17, Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
>   
>> +		prot = pci_read_long(dev, reg);
>> +		if (prot & 0x3)
>> +			printf("SB600 %s %s protected from %u to %u\n",
>> +				(prot & 0x1) ? "write" : "",
>> +				(prot & 0x2) ? "read" : "",
>> +				(prot & 0xfffffc00),
>> +				(prot & 0xfffffc00) + ((prot & 0x3ff) << 8));
>> +		prot &= 0xfffffffc;
>> +		pci_write_byte(dev, reg, prot);
>>     
>
> I'd like to avoid writing at all if the region is unprotected. Please
> just if (0 == (prot & 0x3)) continue; at the top.

Done.

> Maybe add a message
> saying that the region is unprotected.
>   

An empty region with zero address and no protection flags probably does
not need printing IMHO.


> And I think these should be printf_debug() or such.
>   

I made the first printf printf_debug and the second one (failure)
unconditional printf. Not telling the user about a failed unprotect is bad.


>> +			printf("SB600 still %s %s protected from %u to %u\n",
>> +				(prot & 0x1) ? "write" : "",
>> +				(prot & 0x2) ? "read" : "",
>>     
>
> Output looks a little nicer if the space is moved down to the
> trigraphs.
>
> ..still%s%s protected ..
> " write" : "",
> " read" : ""
>
>
> Fix this and it's
>   

Done.

> Acked-by: Peter Stuge <peter at stuge.se>
>   

Thanks, committed in r463.


Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/