[coreboot] r1009 - in coreboot-v3: mainboard/kontron/986lcd-m northbridge/intel/i945 southbridge/intel/i82801gx

[ron minnich]

On Wed, Nov 12, 2008 at 6:07 PM, Stefan Reinauer <stepan at coresystems.de> wrote:

> Absolutely right.

I'm fine with going back. Are there any objections to just building in
all bits of a chip, even if all the parts are not even used?
(oh! see below! I feel happy about things now :-)


> How so? Just having one for "include all code for that southbridge" is fine.

Are you sure? If so that's fine with me!


>> There's another problem. If you don't reference ide in the dts, then
>> the ide.c won't compile -- no config struct for it.
> Darn. We're back and re-implemented the v2 nastyness with a completely
> different set of tools. That's really a pity.

no, it's a totally different problem.

in v2, we defined the struct in chip.h and set settings in Config.lb.

In v3, dtc generates the struct. So you can't build ide.c without
referencing ide.dts. So we solved the 'we can't set default values in
chip.h' but created a different problem. Why is it always that way :-)

> I disagree. Nobody would ever want to include the code of half a
> southbridge.

as long as we are sure about that, there is no problem. See the patch
I sent. It sets it all back.

>
>> I definitely don't want to revisit this argument:
>> STAGE2_CHIPSET_SRC-$(CONFIG_INTEL_I82801GX) += srcfiles
>>
>> we've been there and decided not to. I don't wish to rehash.
>>
>>
> I don't understand what you're trying to say. We're doing
>
> ifeq ($(CONFIG_SOUTHBRIDGE_INTEL_I82371EB),y)
>
> STAGE2_CHIPSET_SRC += $(src)/southbridge/intel/i82371eb/i82371eb.c
> STAGE2_CHIPSET_SRC += $(src)/southbridge/intel/i82371eb/i82371eb_foo.c
> STAGE2_CHIPSET_SRC += $(src)/southbridge/intel/i82371eb/i82371eb_bar.c
> STAGE2_CHIPSET_SRC += $(src)/southbridge/intel/i82371eb/i82371eb_blubb.c
> endif

but that's different.  (to me).

>
> in southbridge/intel/i82371eb/Makefile. Why would that not be
> appropriate for the ich7?

Sure it's fine.


>> Remember, long ago, however, I was putting everything in one file
>> because of this kind of situation. The sense of the community was that
>> we should break it out.
>>
> I don't remember what this was about. pci drivers in v3?

it was about the fact that cs5536.c was one file that included all the
devices. People stated a preference for one file per device.

>> OK, I broke it out. Now, before we decide that breaking it out was
>> bad, let's make sure this time. :-)
>>
> While it makes sense to have one "driver" in one file to make the code
> readable, it sure does not make much sense to choose to have the ich7
> usb driver in the bios image but not the ich7 ehci driver.

That is *good* to hear. You just simplified our life tremendously.

So do we say this:

if you have a chip, such as the i82801gx, then all the files for that
chip get compiled in; and all the dts files in that
directory should be used in the mainboard dts.

Will that rule be ok?


>> What's interesting to me is how many times unchanged v2 code is found
>> unacceptable for v3 -- is anyone looking at v2? -- or we keep
>> revisiting decisions we made already.
>>
> It's so much easier to bitch about v2 and dream about v3 than to look
> and learn from v2 and not doing micro-optimizations in v3.  We've been
> endorsing this behavior, unfortunately.
>

uh oh. Did I do that too?

Actually, if we do make one set of decisions about build rules out of
this discussion, I am glad I stirred the bees nest with that commit :)

thanks

ron