Yes, I love top posting ;)
I agree to these comments and I think we should get this fixed. I am
going to check the patch in as is, but please don't forget to send a fix
later if possible.
Stefan
* Peter Stuge <[EMAIL PROTECTED]> [070504 03:50]:
> On Thu, May 03, 2007 at 12:15:57PM -0600, Marc Jones wrote:
> > Index: LinuxBIOSv2/src/include/device/pci_ids.h
> > ===================================================================
> > --- LinuxBIOSv2.orig/src/include/device/pci_ids.h 2007-05-02
> > 15:35:45.000000000 -0600
> > +++ LinuxBIOSv2/src/include/device/pci_ids.h 2007-05-02
> > 15:36:07.000000000 -0600
> > @@ -452,12 +452,13 @@
> > #define PCI_DEVICE_ID_AMD_AES 0x2082
> > #define PCI_DEVICE_ID_AMD_CS5536_ISA 0x2090
> > #define PCI_DEVICE_ID_AMD_CS5536_FLASH 0x2091
> > -#define PCI_DEVICE_ID_AMD_CS5536_IDE 0x2092
> > +#define PCI_DEVICE_ID_AMD_CS5536_IDE_A0 0x2092
> > #define PCI_DEVICE_ID_AMD_CS5536_AUDIO 0x2093
> > #define PCI_DEVICE_ID_AMD_CS5536_OHCI 0x2094
> > #define PCI_DEVICE_ID_AMD_CS5536_EHCI 0x2095
> > #define PCI_DEVICE_ID_AMD_CS5536_UDC 0x2096
> > #define PCI_DEVICE_ID_AMD_CS5536_OTG 0x2097
> > +#define PCI_DEVICE_ID_AMD_CS5536_IDE 0x209A
>
> I would like this to be more future proof, e.g. with _CS5536_A0_IDE
> and _CS5536_B3_IDE. (assuming B3 is the first rev with the new ID)
>
> Otherwise, the next time the PCI ID is bumped, a new build of old
> working code will break at runtime. That's unneccessary. Better it
> breaks at compile time or not at all..
>
>
> > +static void pmChipsetInit(void) {
> ..
>
> > + /* PM_SED*/
> > + port = (PMS_IO_BASE + 0x014);
> > +/* mov eax, 0x057642 ; 100ms, works*/
> > + val = 0x04601 ; /* 5ms*/
> > + outl(val, port);
>
> An assembly comment lost in C code. Let's help it find it's way back
> home. :)
>
> These comments are a bit confusing, maybe just because I don't have
> the data book at hand?
>
> "100ms, works" but let's use 5ms instead?
>
> It would be nice to have a better description of the reference
> values here.
>
>
> > + outb( P80_CHIPSET_INIT, 0x80);
>
> What was the resolution of the POST code output discussion?
>
> I would prefer if post_code() was used throughout so smart things
> could be added to that function later.
>
>
> > + /* we hope NEVER to be in linuxbios when S3 resumes
> > + if (! IsS3Resume()) */
>
> "hope" ? At the very least expand on the problem in the comment.
>
>
> > Index: LinuxBIOSv2/src/southbridge/amd/cs5536/cs5536_early_setup.c
> > ===================================================================
> > --- LinuxBIOSv2.orig/src/southbridge/amd/cs5536/cs5536_early_setup.c
> > 2007-05-02 15:35:45.000000000 -0600
> > +++ LinuxBIOSv2/src/southbridge/amd/cs5536/cs5536_early_setup.c
> > 2007-05-02 15:36:07.000000000 -0600
>
> [..]
>
> > -static void cs5536_setup_power_bottun(void)
> > +static void cs5536_setup_power_button(void)
>
> [..]
>
> > - ; setup GPIO24, it is the external signal for 5536 vsb_work_aux
> > + /* setup GPIO24, it is the external signal for 5536 vsb_work_aux
> > ; which controls all voltage rails except Vstandby & Vmem.
>
> Could this be any GPIO ball or is GPIO24 muxed with another function
> and GPIO24 just serves as a reference here?
>
> If any GPIO - it would be nice to make this an option.
>
> If muxed, is there a more relevant signal name that could be used
> instead of GPIO24?
>
>
> //Peter
>
> --
> linuxbios mailing list
> [email protected]
> http://www.linuxbios.org/mailman/listinfo/linuxbios
>
--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: [EMAIL PROTECTED] • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
--
linuxbios mailing list
[email protected]
http://www.linuxbios.org/mailman/listinfo/linuxbios
