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 <[EMAIL PROTECTED]>
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
