Submitter | David Borg |
---|---|
Date | 2010-07-30 18:17:29 |
Message ID | <AANLkTimCz1S-OuvD3y4r5C0MLqgB3LQc4w1Bmm-e1p4a@mail.gmail.com> |
Download | mbox | patch |
Permalink | /patch/1704/ |
State | Superseded |
Headers | show |
Comments
On 30.07.2010 20:17, David Borg wrote: > the subsystem ID has been left blank as the PCI bridge has no subsystem ID. > > On 30 July 2010 19:42, David Borg <borg.db@gmail.com> wrote: > >> Tested and works. Thanks to Carl-Daniel Hailfinger for simplifying the >> chipset enable for this board! >> >> Signed-off-by: David Borg <borg.db@gmail.com> >> > Index: board_enable.c > =================================================================== > --- board_enable.c (revision 1127) > +++ board_enable.c (working copy) > @@ -352,7 +352,9 @@ > } > > /** > - * w83627: Enable MEMW# and set ROM size to max. > + * Enable MEMW# and set ROM size to max. > + * Supported chips: > + * W83L517D, W83697HF/F/HG, W83697SF/UF/UG > Is there a reason you removed w83627 from the list? > */ > static void w836xx_memw_enable(uint16_t port) > { > @@ -1715,6 +1717,7 @@ > {0x8086, 0x24D3, 0x1043, 0x80A6, 0x8086, 0x2578, 0x1043, 0x80F6, NULL, NULL, NULL, "ASUS", "P4C800-E Deluxe", 0, OK, intel_ich_gpio21_raise}, > {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, NULL, "ASUS", "P4P800-E Deluxe", 0, OK, intel_ich_gpio21_raise}, > {0x8086, 0x2570, 0x1043, 0x80A5, 0x105A, 0x24D3, 0x1043, 0x80A6, NULL, NULL, NULL, "ASUS", "P4SD-LA", 0, NT, intel_ich_gpio32_raise}, > + {0x1039, 0x0661, 0x1043, 0x8113, 0x1039, 0x0003, 0, 0, "^P4S800-MX$", NULL, NULL, "ASUS", "P4S800-MX", 512, OK, w836xx_memw_enable_2e}, > Second subsystem vendor has one space too few, and second subsystem device has one space too many. > {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "^P5A$", "asus", "p5a", "ASUS", "P5A", 0, OK, board_asus_p5a}, > {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", 0, OK, nvidia_mcp_gpio10_raise}, > {0x8086, 0x24dd, 0x1043, 0x80a6, 0x8086, 0x2570, 0x1043, 0x8157, NULL, NULL, NULL, "ASUS", "P5PE-VM", 0, OK, intel_ich_gpio21_raise}, > I'll leave the review to Michael or Uwe who know board enables really well. Apart from the cosmetic comments above, the board enable looks good IMHO. We should check if DMI matching is really needed or just excessive caution. Regards, Carl-Daniel
>> Index: board_enable.c >> =================================================================== >> --- board_enable.c (revision 1127) >> +++ board_enable.c (working copy) >> @@ -352,7 +352,9 @@ >> } >> >> /** >> - * w83627: Enable MEMW# and set ROM size to max. >> + * Enable MEMW# and set ROM size to max. >> + * Supported chips: >> + * W83L517D, W83697HF/F/HG, W83697SF/UF/UG >> > > Is there a reason you removed w83627 from the list? Yes. A cross check with the datasheets for the following shows that it is not correct: W83627DHG W83627DHG-P W83627DHG-PT W83627EEF W83627EEG W83627EHF W83627EHG W83627F W83627G W83627HF W83627HG W83627SF W83627THF W83627THG W83627UHG In addition the function was introduced at revision 490 (http://flashrom.org/trac/flashrom/changeset/490/trunk/board_enable.c), replacing existing W83697 SuperIO enables. Motherboards that used the function at that time where: ASUS A7V8X-MX SE and A7V400-MX, Tyan Tomcat K7M and EPoX EP-8K5A2 A7V8X-MX SE: W83697HF SuperIO (http://www.mail-archive.com/flashrom@flashrom.org/msg01316.html) A7V400-MX: W83697HF SuperIO (http://www.mail-archive.com/linuxbios@linuxbios.org/msg05809.html Used function W83697_rom_memw_enable, and more importantly, the following CPU-Z output http://forum.darkhardware.com/viewtopic.php?f=8&t=158540) Tomcat K7M: W83697HF SuperIO (http://tracker.coreboot.org/trac/coreboot/browser/trunk/util/flashrom/board_enable.c?rev=2732, http://www.macmall.com/p/product~dpno~686101~pdp.dcjggah?source=zwb12166 - under Chipset section) EP-8K5A2: W83697HF SuperIO (http://forums.amd.com/forum/messageview.cfm?catid=24&threadid=65550) The following source code documentation takes care of the other motherboards /** * Winbond W83697HF Super I/O + VIA VT8235 southbridge * * Suited for: * - MSI KT4V and KT4V-L: AMD K7 + VIA KT400 + VT8235 * - MSI KT4 Ultra: AMD K7 + VIA KT400 + VT8235 */ > > Second subsystem vendor has one space too few, and second subsystem > device has one space too many. > I'm guessing the combination of ssh + bash + nano isn't too whitespace friendly. > > I'll leave the review to Michael or Uwe who know board enables really > well. Apart from the cosmetic comments above, the board enable looks > good IMHO. > We should check if DMI matching is really needed or just excessive caution. > Ok. Thanks for the quick review! Regards, David Borg
Am Sonntag, den 01.08.2010, 01:31 +0200 schrieb Carl-Daniel Hailfinger: > > + {0x1039, 0x0661, 0x1043, 0x8113, 0x1039, 0x0003, 0, 0, "^P4S800-MX$", NULL, NULL, "ASUS", "P4S800-MX", 512, OK, w836xx_memw_enable_2e}, > > > I'll leave the review to Michael or Uwe who know board enables really > well. Apart from the cosmetic comments above, the board enable looks > good IMHO. > We should check if DMI matching is really needed or just excessive caution. If you just have one subsystem ID, DMI might be a good idea. I suggested not to use the IDE controller for the second subsystem ID, as it varies with AHCI/RAID/whatsoever modes, but this board is so old, it has nothing of them. I suggest to use the IDE PCI ID with subsystem as second device and drop the DMI. Regards, Michael Karcher
Patch
Index: board_enable.c =================================================================== --- board_enable.c (revision 1127) +++ board_enable.c (working copy) @@ -352,7 +352,9 @@ } /** - * w83627: Enable MEMW# and set ROM size to max. + * Enable MEMW# and set ROM size to max. + * Supported chips: + * W83L517D, W83697HF/F/HG, W83697SF/UF/UG */ static void w836xx_memw_enable(uint16_t port) { @@ -1715,6 +1717,7 @@ {0x8086, 0x24D3, 0x1043, 0x80A6, 0x8086, 0x2578, 0x1043, 0x80F6, NULL, NULL, NULL, "ASUS", "P4C800-E Deluxe", 0, OK, intel_ich_gpio21_raise}, {0x8086, 0x2570, 0x1043, 0x80F2, 0x105A, 0x3373, 0x1043, 0x80F5, NULL, NULL, NULL, "ASUS", "P4P800-E Deluxe", 0, OK, intel_ich_gpio21_raise}, {0x8086, 0x2570, 0x1043, 0x80A5, 0x105A, 0x24D3, 0x1043, 0x80A6, NULL, NULL, NULL, "ASUS", "P4SD-LA", 0, NT, intel_ich_gpio32_raise}, + {0x1039, 0x0661, 0x1043, 0x8113, 0x1039, 0x0003, 0, 0, "^P4S800-MX$", NULL, NULL, "ASUS", "P4S800-MX", 512, OK, w836xx_memw_enable_2e}, {0x10B9, 0x1541, 0, 0, 0x10B9, 0x1533, 0, 0, "^P5A$", "asus", "p5a", "ASUS", "P5A", 0, OK, board_asus_p5a}, {0x10DE, 0x0030, 0x1043, 0x818a, 0x8086, 0x100E, 0x1043, 0x80EE, NULL, NULL, NULL, "ASUS", "P5ND2-SLI Deluxe", 0, OK, nvidia_mcp_gpio10_raise}, {0x8086, 0x24dd, 0x1043, 0x80a6, 0x8086, 0x2570, 0x1043, 0x8157, NULL, NULL, NULL, "ASUS", "P5PE-VM", 0, OK, intel_ich_gpio21_raise},