[PATCH] MCP55 azalia driver

[Stefan Reinauer]

A while back I sent a fix for the MCP55 azalia driver which prevented
coreboot from hanging and at the same time allowed per-board Azalia
VERBs. The attached patch implements per-board VERBs for all MCP55
boards, and as I don't have the correct VERBs for all boards, uses the
default MCP55 VERB for all of them.

Stefan

[Peter Stuge]

Nice! But..

Stefan Reinauer wrote:
> +++ src/devices/oprom/x86.c	(working copy)
> @@ -45,6 +45,43 @@
>  void (*realmode_interrupt)(u32 intno, u32 eax, u32 ebx, u32 ecx, u32 edx, 
>    	u32 esi, u32 edi) __attribute__((regparm(0))) = (void *)&__realmode_interrupt;
>
> +#define DEBUG_REALMODE
> +#ifdef DEBUG_REALMODE
> +static void
> +dump(u8 * addr, u32 len)

Oops..

..but more importantly:
> +++ src/southbridge/nvidia/mcp55/Makefile.inc	(working copy)
> @@ -1,5 +1,5 @@
>  driver-y += mcp55.o
> -driver-y += mcp55_aza.o
> +driver-y += mcp55_azalia.o

This is so that the driver is built for all boards which

select SOUTHBRIDGE_NVIDIA_MCP55

in their Kconfig.
> +++ src/mainboard/supermicro/h8dmr/mainboard.c	(working copy)
> @@ -3,11 +3,11 @@
> Copyright (C) 2007 AMD
> Written by Yinghai Lu  for AMD.
> Copyright (C) 2010 coresystems GmbH
> 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
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> the Free Software Foundation; version 2 of the License.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
>
> @@ -24,8 +24,21 @@
>  #include <device/pci.h>
>  #include <device/pci_ids.h>
>  #include <device/pci_ops.h>
> +#include "hda_verb.h"
>  #include "chip.h"
>
> +static void verb_setup(void)
> +{
> cim_verb_data = mainboard_cim_verb_data;
> cim_verb_data_size = sizeof(mainboard_cim_verb_data);
>
> +}
>
> +static void mainboard_enable(device_t dev) 
> +{
> verb_setup();
>
> +}
>
> struct chip_operations mainboard_ops = {
>    CHIP_NAME("Supermicro H8DMR Mainboard")
> .enable_dev = mainboard_enable,
>
> };

So is there absolutely no way for us to avoid that this code is
duplicated for *every* board with such a select?

It seems wrong to automatically add a driver during build, and then
need to call it manually in boards where it was automatically built!

I think this is pretty important. It seems also related to the VSA
situation for the Geode that I worked around in r5581.

//Peter

[Stefan Reinauer]

On 6/1/10 8:02 AM, Peter Stuge wrote:
> So is there absolutely no way for us to avoid that this code is
> duplicated for *every* board with such a select?
>
> It seems wrong to automatically add a driver during build, and then
> need to call it manually in boards where it was automatically built!

That's actually not what happens. You only need to call it for those
boards that need to load a HDA "VERB table". Many boards should be able
to work without loading a VERB. Unfortuntately I could not test this on
all MCP55 boards, so hence the "safe way".

Providing the HDA VERB table is a mainboard specific thing. So it
belongs into the mainboard.

We could add some magic to load the VERB upon CONFIG_LOAD_VERB. I
considered this back when I implemented the algorithm, but it would even
more mean jumping between components with the code and making it harder
to understand.
> I think this is pretty important. It seems also related to the VSA
> situation for the Geode that I worked around in r5581.

In which way?

Stefan