| Submitter | Rudolf Marek |
|---|---|
| Date | 2010-04-20 21:47:25 |
| Message ID | <4BCE20ED.205@assembler.cz> |
| Download | mbox | patch |
| Permalink | /patch/1248/ |
| State | Superseded |
| Headers | show |
Comments
On 4/20/10 11:47 PM, Rudolf Marek wrote: Does it work better like this? > /* Power management controller */ > dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, > PCI_DEVICE_ID_VIA_VT8237R_LPC), 0); > > if (dev == PCI_DEV_INVALID) { > /* Power management controller */ > dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, > PCI_DEVICE_ID_VIA_VT8237S_LPC), 0); } // close scope early > > if (dev == PCI_DEV_INVALID) > return; > pci_write_config8(dev, 0x41, 0x7f); If not, we should make a test case and send it to eric. .... > +static void bootblock_southbridge_init(void) { > functions should have their opening bracket on the next line > + > + /* ROM decode last 8MB FF800000 - FFFFFFFF on VT8237S/VT8237A */ > + /* ROM decode last 4MB FFC00000 - FFFFFFFF on VT8237R */ > + > + /* get SB on 0:11.0, because pci_locate_device construct similar in > + enable_rom_decode() did not worked for some reason */ > + pci_write_config8(PCI_DEV(0,0x11,0), 0x41, 0x7f); > +} > Is the device always on 0, 0x11, 0? If not, we should add a more explicit comment saying TODO and explaining why the broken code was checked in.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 > If not, we should make a test case and send it to eric. It does not. I tried what you suggested. What now? > > > .... >> +static void bootblock_southbridge_init(void) { >> > functions should have their opening bracket on the next line Aha ok will fix it for a final patch. >> + >> + /* ROM decode last 8MB FF800000 - FFFFFFFF on VT8237S/VT8237A */ >> + /* ROM decode last 4MB FFC00000 - FFFFFFFF on VT8237R */ >> + >> + /* get SB on 0:11.0, because pci_locate_device construct similar in >> + enable_rom_decode() did not worked for some reason */ >> + pci_write_config8(PCI_DEV(0,0x11,0), 0x41, 0x7f); >> +} >> > Is the device always on 0, 0x11, 0? If not, we should add a more > explicit comment saying TODO and explaining why the broken code was > checked in. Yes it it should although VT8237S has programmable SB IDSEL never seen any VT8237 on something else than 11.0, and I did not found any straps for it just a sw reg. Rudolf -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkvOJt8ACgkQ3J9wPJqZRNVA4QCgpApwVfH4rnmlbsuiVkgCKbMW ijMAn0K26Fuax7CWJJ6J0HCjcE5Ua7S8 =DzHk -----END PGP SIGNATURE-----
On 4/20/10 11:47 PM, Rudolf Marek wrote: > Hello, > > Attached patch adds support for tinybootblock on VT8237* to decode > whole flash > independent of strapping, making larger flashes work again (as it was in > pre-tiny bootblock era) > > Signed-off-by: Rudolf Marek <r.marek@assembler.cz> > > I had a strange problem: > > #include <arch/io.h> > #include <arch/romcc_io.h> > #include <device/pci_ids.h> > > static void bootblock_southbridge_init(void) { > > device_t dev; > > /* Power management controller */ > dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, > PCI_DEVICE_ID_VIA_VT8237R_LPC), 0); > > if (dev == PCI_DEV_INVALID) { > /* Power management controller */ > dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, > PCI_DEVICE_ID_VIA_VT8237S_LPC), 0); > > if (dev == PCI_DEV_INVALID) > return; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ And you are sure that dev is not PCI_DEV_INVALID at this point? > } > pci_write_config8(dev, 0x41, 0x7f); > }
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Following code does work:
static void bootblock_southbridge_init(void) {
/* ROM decode last 8MB FF800000 - FFFFFFFF on VT8237S/VT8237A */
/* ROM decode last 4MB FFC00000 - FFFFFFFF on VT8237R */
device_t dev;
/* Power management controller */
//dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA,
// PCI_DEVICE_ID_VIA_VT8237R_LPC), 0);
//if (dev == PCI_DEV_INVALID) {
/* Power management controller */
dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA,
PCI_DEVICE_ID_VIA_VT8237S_LPC), 0);
//}
if (dev == PCI_DEV_INVALID)
return;
pci_write_config8(dev, 0x41, 0x7f);
}
If one uncomment the commented out parts. It stops working. Nothing is changed
if the if (dev == PCI_DEV_INVALID) return; is moved to the if or not.
Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iEYEARECAAYFAkvQuesACgkQ3J9wPJqZRNUikwCglEm7Upbqg8SlTNnyUaRoKObi
IKgAoNaottXQGBnJaYsCRFRgTzkafhuV
=/HGI
-----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi again, dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VT8237R_LPC), 0); pci_write_config8(PCI_DEV(0,0x11,0), 0x41, 0x7f); This code won't make it either. So I suspect it is not good idea to walk whole PCI bus so early. Maybe HT needs to be setup? Thanks, Rudolf -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkvQvfoACgkQ3J9wPJqZRNV/vACfUDVMP5rnCPFbYgXe8eNdGq8c AmoAoJo7TzcKesMm26tXGY++crtZJm2l =m4HL -----END PGP SIGNATURE-----
> Hi again, > > dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, > PCI_DEVICE_ID_VIA_VT8237R_LPC), 0); > > pci_write_config8(PCI_DEV(0,0x11,0), 0x41, 0x7f); > > This code won't make it either. So I suspect it is not good idea to walk whole > PCI bus so early. Maybe HT needs to be setup? You could use pci_locate_device_on_bus() with bus 0 to test that theory. Thanks, Myles
Patch
Index: src/southbridge/via/vt8237r/bootblock.c =================================================================== --- src/southbridge/via/vt8237r/bootblock.c (revision 0) +++ src/southbridge/via/vt8237r/bootblock.c (revision 0) @@ -0,0 +1,32 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2010 Rudolf Marek <r.marek@assembler.cz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <arch/io.h> +#include <arch/romcc_io.h> +#include <device/pci_ids.h> + +static void bootblock_southbridge_init(void) { + + /* ROM decode last 8MB FF800000 - FFFFFFFF on VT8237S/VT8237A */ + /* ROM decode last 4MB FFC00000 - FFFFFFFF on VT8237R */ + + /* get SB on 0:11.0, because pci_locate_device construct similar in + enable_rom_decode() did not worked for some reason */ + pci_write_config8(PCI_DEV(0,0x11,0), 0x41, 0x7f); +} Index: src/southbridge/via/vt8237r/Kconfig =================================================================== --- src/southbridge/via/vt8237r/Kconfig (revision 5467) +++ src/southbridge/via/vt8237r/Kconfig (working copy) @@ -25,3 +25,8 @@ bool default n depends on SOUTHBRIDGE_VIA_VT8237R + +config BOOTBLOCK_SOUTHBRIDGE_INIT + string + default "southbridge/via/vt8237r/bootblock.c" + depends on SOUTHBRIDGE_VIA_VT8237R
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hello, Attached patch adds support for tinybootblock on VT8237* to decode whole flash independent of strapping, making larger flashes work again (as it was in pre-tiny bootblock era) Signed-off-by: Rudolf Marek <r.marek@assembler.cz> I had a strange problem: #include <arch/io.h> #include <arch/romcc_io.h> #include <device/pci_ids.h> static void bootblock_southbridge_init(void) { device_t dev; /* Power management controller */ dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VT8237R_LPC), 0); if (dev == PCI_DEV_INVALID) { /* Power management controller */ dev = pci_locate_device(PCI_ID(PCI_VENDOR_ID_VIA, PCI_DEVICE_ID_VIA_VT8237S_LPC), 0); if (dev == PCI_DEV_INVALID) return; } pci_write_config8(dev, 0x41, 0x7f); } did not reach the pci write for some reason. This code is a copy from enable_rom_decode() so it should work. Thanks, Rudolf -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkvOIO0ACgkQ3J9wPJqZRNVqFwCgz6+rJ6QmDVoFLHjZ3MS668jY uW4An2kWok6aukzhZGC1Fue5yJkN0Eef =LGC8 -----END PGP SIGNATURE-----