Hi LYH, Ron and others,
I've been looking at the recent changes that went into the tree, and I don't like the fact that a lot of code is duplicated. Is there some way to unify this?
--- raminit.c 2003-08-28 14:43:39.000000000 +0200 +++ raminit.1.c 2003-08-28 06:42:50.000000000 +0200 [..] - PCI_ADDR(0, 0x18, 1, 0xBC), 0x00000048, 0x00ffff00, + PCI_ADDR(0, 0x18, 1, 0xBC), 0x00000048, 0x00ffff20,
This changes the link id where the southbridge is connected. We can get this information out of the configuration file since we are able to describe a map of all CHT components. On most machines the southbridge is probably connected to LDT0, but having two files for this one setting seems inappropriate. Ron, what is the easiest way of getting this information from the config file?
LYH: To get this unified again, I added a resourcemap.c file to the s2885 directory that contains the resource map of the 2885. In auto.c this is used instead of the default.
- PCI_ADDR(0, 0x18, 1, 0xC4), 0xFE000FC8, 0x01fff000, + PCI_ADDR(0, 0x18, 1, 0xC4), 0xFE000FC8, 0x01fff020,
same as above.
- PCI_ADDR(0, 0x18, 1, 0xE0), 0x0000FC88, 0xff000003, - PCI_ADDR(0, 0x18, 1, 0xE4), 0x0000FC88, 0x00000000, + PCI_ADDR(0, 0x18, 1, 0xE0), 0x0000FC88, 0x03000203, + PCI_ADDR(0, 0x18, 1, 0xE4), 0x0000FC88, 0x05040003,
These change the bus numbers and read/write enables. How can we write these more general?
+/* //BY LYH add IOMMU 64M APERTURE PCI_ADDR(0, 0x18, 3, 0x94), 0xffff8000, 0x00000f70, PCI_ADDR(0, 0x18, 3, 0x90), 0xffffff80, 0x00000002, PCI_ADDR(0, 0x18, 3, 0x98), 0x0000000f, 0x00068300,
//BY LYH END +*/
I added a configuration variable ENABLE_IOMMU and set it to 1 for all existing boards except the s2885. It would be nice to have the size of the aperture also selectable via the configuration file, or even better during firmware run time.
I am scared that changes for special motherboards will contaminate the source tree and make it hard for anyone to see what is actually going on. If this happens in a source tree for a special motherboard, it does not really matter, but I think it is a good goal to keep the generic code clean and generic and make it even more generic in the places where it's not sufficient.
I'll look into coherent_ht.1.c and see if this can be done more general as well.
I commited the attached patch. Please comment.
Stefan
we are clearly going down the wrong path with the motherboard-specific changes. I mainly committed that so we could get a handle on what we really need.
Yes, this type of thing should be driven from config files. I am pausing on changes because I want to get a better idea what Eric is up to in this area.
I'll look some more at this however.
ron
Hi again,
the coherent_ht.c changes are all to LYH's hardcodes to coherent_ht_finalize(). These are there to setup link speed and width on the cht nodes.
I remember tomz from LNXI was working on that on a higher level at pci setup. Is that code complete by now? Are all bridges set up full speed?
Then we could just drop this code, which i would favour. Otherwise it should walk into the motherboard specific code.
As far as I can see this code is noop anyways since speed changes need a reset or LDTSTOP_L which is not asserted here. LYH, any comments?
--- coherent_ht.c 2003-08-14 11:11:10.000000000 +0200 +++ coherent_ht.1.c 2003-08-28 06:42:50.000000000 +0200 [..] - PCI_ADDR(0, 0x18, 0, 0xc4), 0x88ff9c05, 0x770000d0, + PCI_ADDR(0, 0x18, 0, 0xc4), 0x88ff9c05, 0x11000020, [..] - PCI_ADDR(0, 0x18, 0, 0xc8), 0xfffff0ff, 0x00000000, + PCI_ADDR(0, 0x18, 0, 0xc8), 0xfffff0ff, 0x00000400, [..] - PCI_ADDR(0, 0x18, 0, 0x94), 0xff0000ff, 0x00ff0000, + PCI_ADDR(0, 0x18, 0, 0x94), 0xff0000ff, 0x00050000, [..] - PCI_ADDR(0, 0x18, 0, 0xd4), 0xff0000ff, 0x00000000, + PCI_ADDR(0, 0x18, 0, 0xd4), 0xff0000ff, 0x00030000,
Stefan
thanks stefan for cleaning this up, this is a step in the right direction.
I think we need to drive this kind of thing out of the config tool, I'm still trying to get my brain wrapped around the whole thing.
ron
* Stefan Reinauer stepan@suse.de [030828 16:39]:
Then we could just drop this code, which i would favour. Otherwise it should walk into the motherboard specific code.
As far as I can see this code is noop anyways since speed changes need a reset or LDTSTOP_L which is not asserted here. LYH, any comments?
Plus, the code is only touching node 0. This should be done for all nodes, if it is done...
Stefan
* Stefan Reinauer stepan@suse.de [030828 16:51]:
- Stefan Reinauer stepan@suse.de [030828 16:39]:
Then we could just drop this code, which i would favour. Otherwise it should walk into the motherboard specific code.
As far as I can see this code is noop anyways since speed changes need a reset or LDTSTOP_L which is not asserted here. LYH, any comments?
Plus, the code is only touching node 0. This should be done for all nodes, if it is done...
I should read before writing. It does touch both CPUs. Still, it does not check whether both CPUs are actually there.
Stefan
* Stefan Reinauer stepan@suse.de [030828 16:39]:
the coherent_ht.c changes are all to LYH's hardcodes to coherent_ht_finalize(). These are there to setup link speed and width on the cht nodes.
Ok, I changed this bit also, getting rid of northbridge/amd/amdk8/coherent_ht.1.c
M mainboard/amd/quartet/auto.c M mainboard/amd/solo/auto.c M mainboard/arima/hdama/auto.c M mainboard/tyan/s2880/auto.c A mainboard/tyan/s2880/hypertransport.c M mainboard/tyan/s2882/auto.c A mainboard/tyan/s2882/hypertransport.c M mainboard/tyan/s2885/auto.c A mainboard/tyan/s2885/hypertransport.c R northbridge/amd/amdk8/coherent_ht.1.c M northbridge/amd/amdk8/coherent_ht.c
All tyan motherboards load the hardcodes from their special hypertransport.c now, so tyan should be happy. All other mainboards only have a dummy function there to satisfy romcc since I don't think that this code is actually needed.
Stefan
-
ron minnich -
Stefan Reinauer