[I think this was only sent to me, not the list, so I'm forwarding...]
----- Forwarded message from Marc Jones Marc.Jones@AMD.com -----
From: Marc Jones Marc.Jones@AMD.com To: Uwe Hermann uwe@hermann-uwe.de Subject: Re: [LinuxBIOS] r369 - in LinuxBIOSv3/southbridge: amd/cs5536 intel/i82371eb Date: Wed, 27 Jun 2007 23:21:22 -0600
Just a couple of comments.
Uwe Hermann wrote:
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?
They are not generic(legacy) and not Geode specific. How's that for an answer ;) These are fairly common extensions to modern the RTC.
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?
If there were a 5536 IDE ROM it could do the setup. Since there isn't I would remove the comment.
- /* 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@hermann-uwe.de
Uwe.
----- End forwarded message -----
Uwe.