On Mon, Oct 06, 2008 at 06:11:15PM -0600, Jordan Crouse wrote:
> [PATCH] coreboot: Don't loop forever waiting for HDA codecs
>
> We shouldn't assume the presence of a working HDA codec, so put in
> a reasonable timeout of 50usecs (timeout value borrowed from the kernel).
> This makes SimNow work, since apparently though the codec is
> present in Simnow, it is non functional.
>
> Signed-off-by: Jordan Crouse <[EMAIL PROTECTED]>
Acked-by: Uwe Hermann <[EMAIL PROTECTED]>
(assuming this is build-tested). But see also below...
> Index: coreboot-v2/src/southbridge/amd/sb600/sb600_hda.c
> ===================================================================
> --- coreboot-v2.orig/src/southbridge/amd/sb600/sb600_hda.c 2008-10-06
> 18:00:06.000000000 -0600
> +++ coreboot-v2/src/southbridge/amd/sb600/sb600_hda.c 2008-10-06
> 18:10:32.000000000 -0600
> @@ -160,6 +160,32 @@
> return sizeof(cim_verb_data) / sizeof(u32);
> }
>
I'd add some (Doxygen) comment, at least saying "Don't loop forever
waiting for HDA codecs" or something more elaborate...
> +static int wait_for_ready(u8 *base)
> +{
> + int timeout = 50;
A comment here why it's exactly 50 would be nice.
> + while(timeout--) {
^
space
> + u32 dword=readl(base + 0x68);
^^
spaces
> + if (!(dword & 1))
Make the hardcoded "1" into a nice, descriptive #define?
> + return 0;
> + udelay(1);
> + }
> +
> + return -1;
> +}
> +
> +static int wait_for_valid(u8 *base)
> +{
> + int timeout = 50;
> + while(timeout--) {
^
space
> + u32 dword = readl(base + 0x68);
> + if ((dword & 3) == 2)
Make the hardcoded "2" and "3" into nice, descriptive #defines?
Also the 0x68 register could be a #define.
> + return 0;
> + udelay(1);
> + }
> +
> + return 1;
> +}
> +
> static void codec_init(u8 * base, int addr)
> {
> u32 dword;
> @@ -168,16 +194,14 @@
> int i;
>
> /* 1 */
> - do {
> - dword = readl(base + 0x68);
> - } while (dword & 1);
> + if (wait_for_ready(base) == -1)
> + return;
>
> dword = (addr << 28) | 0x000f0000;
> writel(dword, base + 0x60);
>
> - do {
> - dword = readl(base + 0x68);
> - } while ((dword & 3) != 2);
> + if (wait_for_valid(base) == -1)
> + return;
>
> dword = readl(base + 0x64);
>
> @@ -193,15 +217,13 @@
> printk_debug("verb_size: %d\n", verb_size);
> /* 3 */
> for (i = 0; i < verb_size; i++) {
> - do {
> - dword = readl(base + 0x68);
> - } while (dword & 1);
> + if (wait_for_ready(base) == -1)
> + return;
>
> writel(verb[i], base + 0x60);
>
> - do {
> - dword = readl(base + 0x68);
> - } while ((dword & 3) != 2);
> + if (wait_for_valid(base) == -1)
> + return;
> }
> printk_debug("verb loaded!\n");
> }
Uwe.
--
http://www.hermann-uwe.de | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
