Patchwork amd sb800 wrapper compile warning fix

login
register
about
Submitter She, Kerry
Date 2011-05-30 05:24:03
Message ID <F53A09371FB723428826B442A4B20A41063CC51A@sbjgexmb1.amd.com>
Download mbox | patch
Permalink /patch/3011/
State New
Headers show

Comments

She, Kerry - 2011-05-30 05:24:03
Hello,

 

This patch fix a AMD sb800 wrapper compile warning:

src/southbridge/amd/cimx_wrapper/sb800/late

call clear_ioapic but not include the prototype declare header file.

and remove some trivial blanks. 

 

Signed-off-by Kerry She <Kerry.she@amd.com> 

 

Regards, 
Kerry She < kerry.she@amd.com>
Tel:  86-10-6280-1415 
Mobile:  86 - 152 1018 2083
Marc Jones - 2011-05-31 23:17:23
On Sun, May 29, 2011 at 11:24 PM, She, Kerry <Kerry.She@amd.com> wrote:
> Hello,
>
>
>
> This patch fix a AMD sb800 wrapper compile warning:
>
> src/southbridge/amd/cimx_wrapper/sb800/late
>
> call clear_ioapic but not include the prototype declare header file.
>
> and remove some trivial blanks.
>
>
>
> Signed-off-by Kerry She <Kerry.she@amd.com>
>

Acked-by: Marc Jones <marcj303@gmail.com>
Peter Stuge - 2011-05-31 23:29:07
She, Kerry wrote:
> This patch fix a AMD sb800 wrapper compile warning:
> 
> src/southbridge/amd/cimx_wrapper/sb800/late
> 
> call clear_ioapic but not include the prototype declare header file.
> 
> and remove some trivial blanks. 

I think it is very important to make whitespace changes in a patch of
it's own, so that there are no other changes. Please also consider if
changing the license text in the source code is neccessary. It may be
better to keep unneccessary whitespace in the license texts, rather
than having a commit which changes the license.

Anyway, isolating whitespace changes in separate commits allows to
work more efficiently with the commit history in the future, which is
sooner or later always neccessary.

Please also create one patch for each logical change, with the
respective commit messages. Having multiple patches makes the review
process much easier, and also helps significantly when working with
the commit history in the future.


Thanks

//Peter
She, Kerry - 2011-06-01 02:07:12
> -----Original Message-----
> From: coreboot-bounces@coreboot.org
[mailto:coreboot-bounces@coreboot.org]
> On Behalf Of Peter Stuge
> Sent: Wednesday, June 01, 2011 7:29 AM
> To: coreboot@coreboot.org
> Subject: Re: [coreboot] amd sb800 wrapper compile warning fix
> 
> She, Kerry wrote:
> > This patch fix a AMD sb800 wrapper compile warning:
> >
> > src/southbridge/amd/cimx_wrapper/sb800/late
> >
> > call clear_ioapic but not include the prototype declare header file.
> >
> > and remove some trivial blanks.
> 
> I think it is very important to make whitespace changes in a patch of
> it's own, so that there are no other changes. Please also consider if
> changing the license text in the source code is neccessary. It may be
> better to keep unneccessary whitespace in the license texts, rather
> than having a commit which changes the license.
Removing the whitespace in the license text is not my intention.
I just use the script to remove all the whitespace when I found there is
some blank at the end of line in certain file.
 
> Anyway, isolating whitespace changes in separate commits allows to
> work more efficiently with the commit history in the future, which is
> sooner or later always neccessary.
Ok
 
> Please also create one patch for each logical change, with the
> respective commit messages. Having multiple patches makes the review
> process much easier, and also helps significantly when working with
> the commit history in the future.
Thanks for your advice

--
Kerry
Peter Stuge - 2011-06-01 02:33:41
She, Kerry wrote:
> > I think it is very important to make whitespace changes in a patch of
> > it's own,
..
> Removing the whitespace in the license text is not my intention.

Understood. Changing the license text is certainly not forbidden,
especially since you work with the copyright holder, and sometimes
the license text *does* change, but maybe it is preferable to skip
whitespace fixes. It's also possible that I am just being too
careful about this.


> I just use the script to remove all the whitespace when I found there
> is some blank at the end of line in certain file.

This is a good idea! Maybe it will be possible to run the script
before sending future patches, so that no extra commits for fixes
are neccessary.

If you are using git for version control then it can also be
configured to colour code trailing whitespace when viewing changes
using git diff:

git config --global core.whitespace trailing-space,space-before-tab,-cr-at-eol
git config --global color.diff auto


> Thanks for your advice

Thank you for your continued work contributing to coreboot!


//Peter

Patch

Index: src/southbridge/amd/cimx_wrapper/sb800/cfg.h
===================================================================
--- src/southbridge/amd/cimx_wrapper/sb800/cfg.h	(revision 6612)
+++ src/southbridge/amd/cimx_wrapper/sb800/cfg.h	(working copy)
@@ -100,7 +100,7 @@ 
  * @breif INCHIP Sata Controller
  */
 #ifndef SATA_CONTROLLER
-  #define SATA_CONTROLLER		ENABLED
+  #define SATA_CONTROLLER		CIMX_OPTION_ENABLED
 #endif
 
 /**
@@ -202,7 +202,7 @@ 
  * @def GPP_CONTROLLER
  */
 #ifndef GPP_CONTROLLER
-  #define GPP_CONTROLLER		ENABLED
+  #define GPP_CONTROLLER		CIMX_OPTION_ENABLED
 #endif
 
 /**
Index: src/southbridge/amd/cimx_wrapper/sb800/late.c
===================================================================
--- src/southbridge/amd/cimx_wrapper/sb800/late.c	(revision 6612)
+++ src/southbridge/amd/cimx_wrapper/sb800/late.c	(working copy)
@@ -21,6 +21,7 @@ 
 #include <device/device.h>	/* device_t */
 #include <device/pci.h>		/* device_operations */
 #include <device/pci_ids.h>
+#include <arch/ioapic.h>
 #include <device/smbus.h>	/* smbus_bus_operations */
 #include <console/console.h>	/* printk */
 #include "lpc.h"		/* lpc_read_resources */
@@ -328,13 +329,13 @@ 
 	switch (dev->path.pci.devfn) {
 	case (0x11 << 3) | 0: /* 0:11.0  SATA */
 		if (dev->enabled) {
-  			sb_config->SATAMODE.SataMode.SataController = ENABLED;
+  			sb_config->SATAMODE.SataMode.SataController = CIMX_OPTION_ENABLED;
 			if (1 == sb_chip->boot_switch_sata_ide)
 				sb_config->SATAMODE.SataMode.SataIdeCombMdPriSecOpt = 0; //0 -IDE as primary.
 			else if (0 == sb_chip->boot_switch_sata_ide)
 				sb_config->SATAMODE.SataMode.SataIdeCombMdPriSecOpt = 1; //1 -IDE as secondary.
 		} else {
-  			sb_config->SATAMODE.SataMode.SataController = DISABLED;
+  			sb_config->SATAMODE.SataMode.SataController = CIMX_OPTION_DISABLED;
 		}
 
 		sataInitBeforePciEnum(sb_config); // Init SATA class code and PHY
@@ -352,18 +353,17 @@ 
 
 	case (0x14 << 3) | 0: /* 0:14:0 SMBUS */
         {
-	    u8 byte;
 	    u32 ioapic_base;
 
 	    printk(BIOS_INFO, "sm_init().\n");
-	    ioapic_base = 0xFEC00000;
+	    ioapic_base = IO_APIC_ADDR;
 	    clear_ioapic(ioapic_base);
 	    /* I/O APIC IDs are normally limited to 4-bits. Enforce this limit. */
 	    #if (CONFIG_APIC_ID_OFFSET == 0 && CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS < 16)
 	    /* Assign the ioapic ID the next available number after the processor core local APIC IDs */
 	    setup_ioapic(ioapic_base, CONFIG_MAX_CPUS * CONFIG_MAX_PHYSICAL_CPUS);
 	    #elif (CONFIG_APIC_ID_OFFSET > 0)
-	    /* Assign the ioapic ID the value 0. Processor APIC IDs follow. */ 
+	    /* Assign the ioapic ID the value 0. Processor APIC IDs follow. */
 	    setup_ioapic(ioapic_base, 0);
 	    #else
 	    #error "The processor APIC IDs must be lifted to make room for the I/O APIC ID"
@@ -374,9 +374,9 @@ 
 
 	case (0x14 << 3) | 1: /* 0:14:1 IDE */
 		if (dev->enabled) {
-			sb_config->SATAMODE.SataMode.SataIdeCombinedMode = ENABLED;
+			sb_config->SATAMODE.SataMode.SataIdeCombinedMode = CIMX_OPTION_ENABLED;
 		} else {
-  			sb_config->SATAMODE.SataMode.SataIdeCombinedMode = DISABLED;
+  			sb_config->SATAMODE.SataMode.SataIdeCombinedMode = CIMX_OPTION_DISABLED;
 		}
 		sataInitBeforePciEnum(sb_config); // Init SATA class code and PHY
 		break;
Index: src/vendorcode/amd/cimx/sb800/SBTYPE.h
===================================================================
--- src/vendorcode/amd/cimx/sb800/SBTYPE.h	(revision 6612)
+++ src/vendorcode/amd/cimx/sb800/SBTYPE.h	(working copy)
@@ -17,7 +17,7 @@ 
  *
  * Copyright (c) 2011, Advanced Micro Devices, Inc.
  * All rights reserved.
- * 
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *     * Redistributions of source code must retain the above copyright
@@ -25,10 +25,10 @@ 
  *     * Redistributions in binary form must reproduce the above copyright
  *       notice, this list of conditions and the following disclaimer in the
  *       documentation and/or other materials provided with the distribution.
- *     * Neither the name of Advanced Micro Devices, Inc. nor the names of 
- *       its contributors may be used to endorse or promote products derived 
+ *     * Neither the name of Advanced Micro Devices, Inc. nor the names of
+ *       its contributors may be used to endorse or promote products derived
  *       from this software without specific prior written permission.
- * 
+ *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
  * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
  * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
@@ -39,7 +39,7 @@ 
  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- * 
+ *
  * ***************************************************************************
  *
  */
@@ -1093,13 +1093,13 @@ 
 #pragma pack (pop)
 
 /**
- * DISABLED - Define disable in module
+ * CIMX_OPTION_DISABLED - Define disable in module
  */
-#define DISABLED   0
+#define CIMX_OPTION_DISABLED   0
 /**
- * ENABLED - Define enable in module
+ * CIMX_OPTION_ENABLED - Define enable in module
  */
-#define ENABLED    1
+#define CIMX_OPTION_ENABLED    1
 
 // mov al, code
 // out 80h, al
Index: src/vendorcode/amd/cimx/sb800/OEM.h
===================================================================
--- src/vendorcode/amd/cimx/sb800/OEM.h	(revision 6612)
+++ src/vendorcode/amd/cimx/sb800/OEM.h	(working copy)
@@ -3,7 +3,7 @@ 
  *
  * Copyright (c) 2011, Advanced Micro Devices, Inc.
  * All rights reserved.
- * 
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *     * Redistributions of source code must retain the above copyright
@@ -11,10 +11,10 @@ 
  *     * Redistributions in binary form must reproduce the above copyright
  *       notice, this list of conditions and the following disclaimer in the
  *       documentation and/or other materials provided with the distribution.
- *     * Neither the name of Advanced Micro Devices, Inc. nor the names of 
- *       its contributors may be used to endorse or promote products derived 
+ *     * Neither the name of Advanced Micro Devices, Inc. nor the names of
+ *       its contributors may be used to endorse or promote products derived
  *       from this software without specific prior written permission.
- * 
+ *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
  * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
  * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
@@ -25,7 +25,7 @@ 
  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- * 
+ *
  * ***************************************************************************
  *
  */
Index: src/vendorcode/amd/cimx/sb800/SATA.c
===================================================================
--- src/vendorcode/amd/cimx/sb800/SATA.c	(revision 6612)
+++ src/vendorcode/amd/cimx/sb800/SATA.c	(working copy)
@@ -17,7 +17,7 @@ 
  *
  * Copyright (c) 2011, Advanced Micro Devices, Inc.
  * All rights reserved.
- * 
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *     * Redistributions of source code must retain the above copyright
@@ -25,10 +25,10 @@ 
  *     * Redistributions in binary form must reproduce the above copyright
  *       notice, this list of conditions and the following disclaimer in the
  *       documentation and/or other materials provided with the distribution.
- *     * Neither the name of Advanced Micro Devices, Inc. nor the names of 
- *       its contributors may be used to endorse or promote products derived 
+ *     * Neither the name of Advanced Micro Devices, Inc. nor the names of
+ *       its contributors may be used to endorse or promote products derived
  *       from this software without specific prior written permission.
- * 
+ *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
  * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
  * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
@@ -39,11 +39,11 @@ 
  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- * 
+ *
  * ***************************************************************************
  *
  */
- 
+
 #include "SBPLATFORM.h"
 #include "cbtypes.h"
 
@@ -317,7 +317,7 @@ 
   }
   if ( ((pConfig->SataClass) == AHCI_MODE) || ((pConfig->SataClass) == IDE_TO_AHCI_MODE) ||
     ((pConfig->SataClass) == AHCI_MODE_4394) || ((pConfig->SataClass) == IDE_TO_AHCI_MODE_4394) ) {
-    if ( pConfig->BuildParameters.SataAHCISsid != NULL ) {    
+    if ( pConfig->BuildParameters.SataAHCISsid != NULL ) {
       ddTempVar = pConfig->BuildParameters.SataAHCISsid;
     }
   }
@@ -470,7 +470,7 @@ 
 
   if (((pConfig->SataClass) != NATIVE_IDE_MODE) && ((pConfig->SataClass) != LEGACY_IDE_MODE)) {
     // RIAD or AHCI
-    if ((pConfig->SATAMODE.SataMode.SataIdeCombinedMode) == DISABLED) {
+    if ((pConfig->SATAMODE.SataMode.SataIdeCombinedMode) == CIMX_OPTION_DISABLED) {
       RWMEM ((ddBar5 + SB_SATA_BAR5_REG00), AccWidthUint8 | S3_SAVE, ~(BIT2 + BIT1 + BIT0), BIT2 + BIT0);
       RWMEM ((ddBar5 + SB_SATA_BAR5_REG0C), AccWidthUint8 | S3_SAVE, 0xC0, 0x3F);
       // RPR 8.10 Disabling CCC (Command Completion Coalescing) support.
@@ -631,7 +631,7 @@ 
   //Enable write access to pci header, pm capabilities
   RWPCI (((SATA_BUS_DEV_FUN << 16) + SB_SATA_REG40), AccWidthUint8 | S3_SAVE, 0xff, BIT0);
 
-//  if ((pConfig->SATAMODE.SataMode.SataIdeCombinedMode) == DISABLED) {
+//  if ((pConfig->SATAMODE.SataMode.SataIdeCombinedMode) == CIMX_OPTION_DISABLED) {
   RWPCI (((SATA_BUS_DEV_FUN << 16) + SB_SATA_REG40 + 1), AccWidthUint8 | S3_SAVE, ~BIT7, BIT7);
 //  }
   sataBar5setting (pConfig, &ddBar5);
Index: src/vendorcode/amd/cimx/sb800/EC.c
===================================================================
--- src/vendorcode/amd/cimx/sb800/EC.c	(revision 6612)
+++ src/vendorcode/amd/cimx/sb800/EC.c	(working copy)
@@ -17,7 +17,7 @@ 
  *
  * Copyright (c) 2011, Advanced Micro Devices, Inc.
  * All rights reserved.
- * 
+ *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *     * Redistributions of source code must retain the above copyright
@@ -25,10 +25,10 @@ 
  *     * Redistributions in binary form must reproduce the above copyright
  *       notice, this list of conditions and the following disclaimer in the
  *       documentation and/or other materials provided with the distribution.
- *     * Neither the name of Advanced Micro Devices, Inc. nor the names of 
- *       its contributors may be used to endorse or promote products derived 
+ *     * Neither the name of Advanced Micro Devices, Inc. nor the names of
+ *       its contributors may be used to endorse or promote products derived
  *       from this software without specific prior written permission.
- * 
+ *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
  * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
  * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
@@ -39,7 +39,7 @@ 
  * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- * 
+ *
  * ***************************************************************************
  *
  */
@@ -71,7 +71,7 @@ 
   RWEC8 (0x61, 0x00, (MailBoxPort & 0xFF));  //set LSB of Mailbox port
   RWEC8 (0x30, 0x00, 0x01);               //;Enable Mailbox Registers Interface, bit0=1
 
-  if ( pConfig->BuildParameters.EcKbd == ENABLED) {
+  if ( pConfig->BuildParameters.EcKbd == CIMX_OPTION_ENABLED) {
     //Enable KBRST#, IRQ1 & IRQ12, GateA20 Function signal from IMC
     RWMEM (ACPI_MMIO_BASE + PMIO_BASE + SB_PMIOA_REGD6, AccWidthUint8, ~BIT8, BIT0 + BIT1 + BIT2 + BIT3);
 
@@ -83,7 +83,7 @@ 
     RWEC8 (0x30, 0x00, 0x01);
   }
 
-  if ( pConfig->BuildParameters.EcChannel0 == ENABLED) {
+  if ( pConfig->BuildParameters.EcChannel0 == CIMX_OPTION_ENABLED) {
     //Logical device 0x03
     RWEC8 (0x07, 0x00, 0x03);
     RWEC8 (0x60, 0x00, 0x00);