[coreboot] patch: dbe62
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Fri Feb 29 15:13:43 CET 2008
On 29.02.2008 14:41, Peter Stuge wrote:
> On Thu, Feb 28, 2008 at 06:53:39PM -0800, ron minnich wrote:
>
>> On Thu, Feb 28, 2008 at 5:00 PM, Peter Stuge <peter at stuge.se> wrote:
>>
>>
>>> > + help
>>> > + This is the default mainboard name.
>>>
>>> What's a default name?
>>>
>> you'll have to ask the guy who wrote that Kconfig file. I'm just a
>> humble intern.
>>
>
> :) Change the help text to something better then.
>
"default name" is very misleading. Look at this snippet from the main
Makefile:
MAINBOARDDIR=$(shell echo $(CONFIG_MAINBOARD_NAME))
Maybe change the text to "This is the path of the directory for this
mainboard below mainboard/ ."
And once we have done that change, there is almost no reason not to
rename that config variable to something else and/or use a better
structure. I also believe this peculiarity is at fault for some of the
weirdness I saw when rebuilding with a changed .config without make
distclean in between.
>>> > +void mainboard_pre_payload(void)
>>> > +{
>>> > + geode_pre_payload();
>>> > + banner(BIOS_DEBUG, "mainboard_pre_payload: done");
>>> > +}
>>>
>>> Why do we need this mainboard code when it is only calling a
>>> function that can be determined using the dts?
>>>
>> Show me how. I don't know.
>>
>
> arch/x86/stage1.c currently calls mainboard_pre_payload() - for
> Geode LX the call has nothing to do with the mainboard and
> everything to do with the north.
>
> geode_pre_payload() is defined in geodelx/geodelxinit.c, so one easy
> way is to rename it to northbridge_amd_geodelx_pre_payload() and
> generate the call from data in the dts.
>
> Another way would be to add it to struct device_operations. I like
> that better actually.
>
Stage 2 phase 7?
>>> > +unsigned long write_pirq_routing_table(unsigned long addr)
>>>
>>> What an abomination. hint hint ;)
>>>
>>> Can this code really not live in northbridge/ ?
>>>
>> no, it's a south function, with mainboard-dependent bits. I'm happy
>> to see a way to make it generic, I just don't know how yet.
>>
>
> Is the _code_ really mainboard dependent? I would think it's device
> dependent, and the board only changes input values. Am I off?
>
Once you use stuff like ARRAY_SIZE, it is difficult to make the code
generic.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list