Re: [coreboot] [v2] r4335 - in trunk/coreboot-v2: src/console src/cpu/amd/dualcore src/cpu/amd/model_fxx src/cpu/amd/quadcore src/cpu/intel/hyperthreading src/include/console src/include/pc80 src/mainboard/amd/dbm690t src/mainboard/amd/pistachio src/mainboard/amd/serengeti_cheetah src/mainboard/amd/serengeti_cheetah_fam10 src/mainboard/arima/hdama src/mainboard/asus/a8n_e src/mainboard/asus/a8v-e_se src/mainboard/asus/m2v-mx_se src/mainboard/broadcom/blast src/mainboard/dell/s1850 src/mainboard/gigabyte/ga_

[Luc Verhaegen]

On Wed, Jun 03, 2009 at 04:19:34PM +0200, [e-mail] wrote:
> Author: libv
> Date: 2009-06-03 16:19:33 +0200 (Wed, 03 Jun 2009)
> New Revision: 4335
>
> Modified:
> Log:
> Revert "CMOS: Add set_option and rework get_option."
>
> This reverts commit eb7bb49eb5b48c39baf7a256b7c74e23e3da5660.
>
> Stepan pointed out that "s" means string, which makes the following statement
> in this commit message invalid: "Since we either have reserved space (which
> we shouldn't do anything with in these two functions), an enum or a
> hexadecimal value, unsigned int seemed like the way to go."
>
> Signed-off-by: Luc Verhaegen 
> Acked-by: Luc Verhaegen

Not to minimise my own part in this mess, but; several people found it 
necessary to chime in on a brainfart of mine, but only Peter spent the 
time reviewing this patch. If people had spent some time reviewing this, 
then maybe the wrong assumption would've been caught in time. Instead, 
several people, some of whom should have intimate enough knowledge of 
the cmos handling, found it necessary to weigh in on a discussion where 
it was clear that the person who started it (me) wasn't intending on 
putting his weight behind any code in the very near future anyway. The 
code that was actually there did get ignored, and with one ACK in the 
pocket, it ended up getting committed, with this mess as a result.

This is where this whole acking system breaks down, and no tool, which 
will incur even more overhead and less willingness to review, will help, 
it will only make things worse.

And another thing: the option table building tool is some of the worst 
examples of C code that i have seen in quite a while. Whatever option 
handling stuff you (whoever you are) are thinking up now, know that it 
also needs to be implemented, and properly this time, and you better be 
prepared to do so by yourself.

Yes, i am quite miffed. I care about the code i produce and i spent 
quite some time on this patch, cleaning up several things that weren't 
ever given a single thought before, providing an actually working 
integer bit retrieval, trying hard to cover all angles, but apparently i 
still missed one. So what i got in return is 1 big commit, 1 build 
breakage with shortly after the fixing commit, then both getting quickly 
reverted, and i rather dislike this. I do not like to see halfarsed or 
broken stuff going out, especially not if i am myself responsible for 
having produced the crap in the first place.

Luc Verhaegen.