[LinuxBIOS] [PATCH] v3: 8254_pit.patch
Uwe Hermann
uwe at hermann-uwe.de
Wed Jul 4 00:04:59 CEST 2007
On Tue, Jul 03, 2007 at 10:53:23AM -0600, Marc Jones wrote:
> Index: LinuxBIOSv3/arch/x86/geodelx/geodelx.c
> ===================================================================
> --- LinuxBIOSv3.orig/arch/x86/geodelx/geodelx.c 2007-07-02 10:23:52.000000000 -0600
> +++ LinuxBIOSv3/arch/x86/geodelx/geodelx.c 2007-07-03 09:37:11.000000000 -0600
> @@ -30,6 +30,7 @@
> #include <io.h>
> #include <amd_geodelx.h>
> #include <spd.h>
> +#include <8254_pit.h>
>
> /* all these functions used to be in a lot of fiddly little files. To
> * make it easier to find functions, we are merging them here. This
> @@ -41,8 +42,6 @@
>
> /**
> * Starts Timer 1 for port 61 use.
> - * 0x43 is PIT command/control.
> - * 0x41 is PIT counter 1.
> *
> * The command 0x56 means write counter 1 lower 8 bits in next IO,
> * set the counter mode to square wave generator (count down to 0
> @@ -58,8 +57,8 @@
> */
> void start_timer1(void)
> {
> - outb(0x56, 0x43);
> - outb(0x12, 0x41);
> + outb(0x56, I82C54_CONTROL_WORD_REGISTER);
> + outb(0x12, I82C54_COUNTER1);
Yep, but the 0x56 and 0x12 should be #defines (or have comments), too.
> }
>
> /**
> Index: LinuxBIOSv3/arch/x86/speaker.c
> ===================================================================
> --- LinuxBIOSv3.orig/arch/x86/speaker.c 2007-07-02 10:23:37.000000000 -0600
> +++ LinuxBIOSv3/arch/x86/speaker.c 2007-07-03 09:34:08.000000000 -0600
> @@ -2,6 +2,7 @@
> * This file is part of the LinuxBIOS project.
> *
> * Copyright (C) 2007 Uwe Hermann <uwe at hermann-uwe.de>
> + * Copyright (C) 2007 Advanced Micro Devices, Inc.
Nope, please drop that this time. There's no new code, just shuffling of
existing code in this patch.
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -18,23 +19,9 @@
> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> -/*
> - * Datasheet:
> - * - Name: 82C54 CHMOS Programmable Interval Timer
> - * - PDF: http://www.intel.com/design/archives/periphrl/docs/23124406.pdf
> - * - Order number: 231244-006
> - */
Please let's keep this here _and_ in 8254_pit.h, it's useful in both
files, IMO.
> -
> #include <io.h>
> #include <lib.h>
> -
> -#define I82C54_CONTROL_WORD_REGISTER 0x43 /* Write-only. */
> -
> -#define I82C54_COUNTER0 0x40
> -#define I82C54_COUNTER1 0x41
> -#define I82C54_COUNTER2 0x42
> -
> -#define PC_SPEAKER_PORT 0x61
The PC_SPEAKER_PORT belongs in this file, I think. It's not PIT related,
but speaker related.
> +#include <8254_pit.h>
>
> /**
> * Use the PC speaker to create a tone/sound of the specified frequency.
> Index: LinuxBIOSv3/include/arch/x86/8254_pit.h
Hm, maybe we should make this include/arch/x86/legacy.h and stick other
#defines in there, too. We should avoid too many small almost-empty
files and there will be other "legacy" devices which need #defines.
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ LinuxBIOSv3/include/arch/x86/8254_pit.h 2007-07-03 09:34:04.000000000 -0600
> @@ -0,0 +1,36 @@
> + * Copyright (C) 2007 Advanced Micro Devices, Inc.
See above, drop this please.
Otherwise patch is ok.
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/20070704/f5404bf4/attachment.sig>
More information about the coreboot
mailing list