On Wed, Jun 27, 2007 at 09:14:27PM +0200, [EMAIL PROTECTED] wrote:
> +     /*      PM_WKXD */
> +     /*      Make sure bits[3:0]=0000b to clear the */
> +     /*      saved Sx state */
> +     port = (PMS_IO_BASE + 0x034);
> +     val = 0x0A0;            /*  5ms */
> +     outl(val, port);

This construct is all over the file. There is a #define PM_WKXD 0x034
so please change the code to use it and remove the comment.


> +#define RTC_CENTURY 0x32
> +#define RTC_DOMA     0x3D
> +#define RTC_MONA     0x3E

Perhaps these should go into some .h?


> +     /* COM1 */
> +     if (sb->com1_enable) {
> +             /* Set the address */
> +             switch (sb->com1_address) {
> +             case 0x3F8:
> +             case 0x3E8:
> +             case 0x2F8:
> +             case 0x2E8:

Maybe there should be a default: here to catch problems in the dts?






> +/** 
> + * Depending on settings in the config struct, enable COM1 or COM2 or both. 

> + * If the enable is NOT set, the UARTS are explicitly disabled, which is 
> required 
> + * if (e.g.) there is a superio attached that does COM1 or COM2.
> +  * @param southbridge config structure 
> +  */
> +static void enable_USB_port4(struct southbridge_amd_cs5536_config *sb)

The comment is obviously wrong for USB. :)


> +             /* ; EECP=50h, IST=01h, ASPC=1 */
> +             *(bar + HCCPARAMS) = 0x00005012;

Don't know if these exist as defines, but they should.


> +     /* Disable virtual PCI UDC and OTG headers */
> +     dev = dev_find_device(PCI_VENDOR_ID_AMD, 
> +                     PCI_DEVICE_ID_AMD_CS5536_UDC, 0);
> +     if (dev) {
> +             pci_write_config32(dev, 0x7C, 0xDEADBEEF);
> +     }
> +
> +     dev = dev_find_device(PCI_VENDOR_ID_AMD, 
> +                     PCI_DEVICE_ID_AMD_CS5536_OTG, 0);
> +     if (dev) {
> +             pci_write_config32(dev, 0x7C, 0xDEADBEEF);
> +     }

#define 0x7C ?


> +     post_code(P80_CHIPSET_INIT);

I would love to see the POST code mess unified before there are ten
boards in v3 all using their own slightly different code and define
style.


> +     /* we hope NEVER to be in linuxbios when S3 resumes
> +        if (! IsS3Resume()) */

This comment is still bad. Marc and/or Jordan and I talked about it
on IRC a while ago and it's not as bad as it sounds but the comment
should really change. I don't remember the details anymore. :\


> +     /*      Allow IO read and writes during a ATA DMA operation. */
> +     /*       This could be done in the HD rom but do it here for easier 
> debugging. */

long lines..


> +     msrnum = ATA_SB_GLD_MSR_ERR;
> +     msr = rdmsr(msrnum);
> +     msr.lo &= ~0x100;
> +     wrmsr(msrnum, msr);

#define 0x100 ?


> +#warning Add back in unwanted VPCI support
> +#if 0
> +     /* disable unwanted virtual PCI devices */
> +     for (i = 0; (i < MAX_UNWANTED_VPCI) && (0 != sb->unwanted_vpci[i]); 
> i++) {
> +             printk(BIOS_DEBUG, "Disabling VPCI device: 0x%08X\n",
> +                          sb->unwanted_vpci[i]);
> +             outl(sb->unwanted_vpci[i] + 0x7C, 0xCF8);

Here's 0x7c again. Same value? #define ?


> +             outl(0xDEADBEEF, 0xCFC);
> +     }
> +#endif

Should all of this code just be deleted?


Other than that this is becoming pretty beautiful code!! :)


//Peter

-- 
linuxbios mailing list
[email protected]
http://www.linuxbios.org/mailman/listinfo/linuxbios

Reply via email to