[I think this was only sent to me, not the list, so I'm forwarding...] ----- Forwarded message from Marc Jones <[EMAIL PROTECTED]> -----
From: Marc Jones <[EMAIL PROTECTED]>
To: Uwe Hermann <[EMAIL PROTECTED]>
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 <[EMAIL PROTECTED]>
>
>
>Uwe.
>
>
----- End forwarded message -----
Uwe.
--
http://www.hermann-uwe.de | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
signature.asc
Description: Digital signature
-- linuxbios mailing list [email protected] http://www.linuxbios.org/mailman/listinfo/linuxbios
