[LinuxBIOS] r369 - in LinuxBIOSv3/southbridge: amd/cs5536 intel/i82371eb

[Uwe Hermann]

On Wed, Jun 27, 2007 at 03:59:18PM -0700, ron minnich wrote:
> >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.
> 
> I like the comment above the code block. I did change the constant to the 
> name.

Yep, that's ok, IMHO. I wouldn't drop the comment.


> >> +#define RTC_CENTURY 0x32
> >> +#define RTC_DOMA     0x3D
> >> +#define RTC_MONA     0x3E
> >
> >Perhaps these should go into some .h?
> 
> but where?

Good question. Are these Geode-specific values or generic RTC values?


> >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're trying. But marc did such a great job with post that I don't
> want to lose it.

Sure, but for consistency I propose two rules for POST codes:

 - All POST codes are emitted _only_ by post_code(), no other functions,
   macros, outb's, or whatever.

 - All POST codes are #defines in post_code.h, every #define is called
   POST_FOO (replace FOO with something which makes sense).

I.e., the only allowed form is post_code(POST_FOO). In addition we'll
have think more about which numbers we use in which situations, but
as a first step I think the above rules will improve the situation a lot.


> This LX port is going to go in several stages. For now, we want code
> in the repo that builds. My laptop disk is dying and I want it in the
> repo and off my disk .... there's a lot of urgency.

Send it as patch to the list and it's archived for eternity :)


> Index: southbridge/amd/cs5536/cs5536.c
> ===================================================================
> --- southbridge/amd/cs5536/cs5536.c	(revision 390)
> +++ southbridge/amd/cs5536/cs5536.c	(working copy)
> @@ -117,21 +117,21 @@
>  	/*      PM_WKXD */
>  	/*      Make sure bits[3:0]=0000b to clear the */
>  	/*      saved Sx state */
> -	port = (PMS_IO_BASE + 0x034);
> +	port = (PMS_IO_BASE + PM_WKXD);
>  	val = 0x0A0;		/*  5ms */
>  	outl(val, port);
>  
>  	/*      PM_WKD */
> -	port = (PMS_IO_BASE + 0x030);
> +	port = (PMS_IO_BASE + PM_WKD);
>  	outl(val, port);
>  
>  	/*      PM_SED */
> -	port = (PMS_IO_BASE + 0x014);
> +	port = (PMS_IO_BASE + PM_SED);
>  	val = 0x04601;		/*  5ms, # of 3.57954MHz clock edges */
>  	outl(val, port);
>  
>  	/*      PM_SIDD */
> -	port = (PMS_IO_BASE + 0x020);
> +	port = (PMS_IO_BASE + PM_SIDD);
>  	val = 0x08C02;		/*  10ms, # of 3.57954MHz clock edges */
>  	outl(val, port);
>  }
> @@ -519,7 +519,7 @@
>  	post_code(P80_CHIPSET_INIT);
>  	dev = dev_find_device(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CS5536_ISA, 0);
>  	if (! dev) {
> -		printk(BIOS_ERR, "%s: Could not find the south bridge!\n", __FUNCTION));
> +		printk(BIOS_ERR, "%s: Could not find the south bridge!\n", __FUNCTION__);
>  		return;
>  	}
>  	sb = (struct southbridge_amd_cs5536_config *)dev->device_configuration;
> @@ -540,8 +540,10 @@
>  	outl(GPIOL_2_SET, GPIO_IO_BASE + GPIOL_INPUT_ENABLE);
>  	outl(GPIOL_2_SET, GPIO_IO_BASE + GPIOL_IN_AUX1_SELECT);
>  
> -	/*      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. */

Can someone explain this, I'm slightly confused. "Can be done in HD
rom"? Huh?


> +	/* 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.
> +	  */

The last three lines of the comment are indented with one space too
much, but other than that this patch is fine:

Acked-by: Uwe Hermann <uwe at hermann-uwe.de>


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070628/ad0d73f1/attachment.sig>