Xavi Drudis Ferran wrote:
1.- In order to get sb700_lpc_init in sb700_early_setup.c to work I've got to modifiy pci_locate_device in order to refrain from scanning some functions in pci bus 0.
Hm. Which fn?
Sorry, I should have said devices. I think it scans every function of every device, reg. 00 and I told it not to do so for any function of some devices (those with nothing attached in my board). I'll send the patch later.
I don't know if it makes sense. That's not neccessarily the case. It depends on if the SB700 can have a different BDF on different boards.
I'd say no, but what do I know?
but then I wonder whether my change is acceptable for other mainboards,
No, I don't think it is.
Ok, then I'll change it.
or I should somehow configure some exclusion parameter so that it can still scan all devices if needed for some other card.
This is also not so nice IMO.
And not all that easy, so discarding it.
In general nothing should be hard coded that is not hard coded in documentation. Ie. if BDF *can* change, then it can not be hard coded. However, the BDF should already be available from devicetree.cb, and hopefully that can be used also for early setup.
Ah, I'll see how to access de devicetree.cb, then. But I think it was hardcoded in docs. They said dev 0 func 0x14 IIRC. I'll check again.
Please send patches for all these types of changes that you have already done. Send many patches with one isolated change in every patch. The smaller each patch is, the easier it is to commit it and also review and comment on any things that should be improved.
Ok. I have to clean up the code a little and incorporate las weekend svn changes, and then I'll break the changes in patches and start sending them. Beware that they are not completely tested (my board does not boot yet).
Certainly. I for one think that the good fix is to make a single long data structure which gathers that CPU revision information in one place.
[...]
I like the idea of a data structure better. We're using C here so there is no reason to spread one piece of logical information out over multiple discrete variable types. A revision bitfield and an array of names doesn't make sense when there could be a structure with everything gathered in one place.
Some of the tricks that are currently being done with the bitfield are harder with a structure. There is an array that is basically a ruleset for msr init and its elements include bitmasks to decide which rule fires for which cpu. If we use structs I guess we need lists of alternative structs in those rules, some values for each field meaning don't care or some such. And I have to check if there is any such design in parts compiled with romcc (lists are hard there, I gather). I guess if make says to me it compiles a file with romcc this file will be compiled with romcc for anyone with any configuration, right?
Has people already used RB_C3 processors with coreboot?
I doubt it.
Ok, thanks.
I think the code just has not been developed yet. The existing code is in no way complete, and all your improvements are very much welcome.
I'll do what I can and when I can, I have little spare time, and it looks it'll take long... I'll try to do at least whatever I need for my setup and as much more as I can without bending over... But I'll leave some parts to do, and may very easily get the others wrong. Sometimes you have to decide whether you support cache scrubbing or cache flush on halt, or similar features that are a bit esoteric for me, so I guess I'll begin by supporting none.
This is very wise.
[...]
Certainly not, you've identified very important points where coreboot needs to be improved.
Fine, thanks, I feel a little more sane (not so much, just as much as before I started with coreboot...)
Thanks!
To you and the rest of the coreboot team.
I'll send more details/patches later in new threads.