Re: [coreboot] [PATCH] warning days - m57sli/mcp55

[Stefan Reinauer]

> This fixes a number of warnings when building m57sli (and other boards with mcp55).
>
> This patch is boot tested on m57sli.
>
> What appears to be a shortening of the delay in
>
> src/southbridge/nvidia/mcp55/mcp55_early_setup_car.c
>
> is functionally exactly the same as before; the delayx function takes a uint8_t
> argument, so the old value (1000 - 0x3E8) was truncated to 232 (0xE8).
>
> Signed-off-by: Ward Vandewege
> Index: src/southbridge/nvidia/mcp55/mcp55_early_setup_car.c
>
> --- src/southbridge/nvidia/mcp55/mcp55_early_setup_car.c	(revision 5411)
> +++ src/southbridge/nvidia/mcp55/mcp55_early_setup_car.c	(working copy)
> @@ -177,7 +177,7 @@
>    pci_write_config32(dev, 0xe4, dword);
>
> //	need to wait 100ms
> delayx(1000);
> delayx(232);
>
> }

it sounds a lot to do 0x8000 outb to wait 100us, but who knows...
I think it would be better to change the input type to something else
than uint8_t, supposedly "unsigned" as most other udelay functions.

Alternatively you could try if this works:

Index: src/southbridge/nvidia/mcp55/mcp55_early_setup_car.c
===================================================================
--- src/southbridge/nvidia/mcp55/mcp55_early_setup_car.c (revision 5411)
+++ src/southbridge/nvidia/mcp55/mcp55_early_setup_car.c (working copy)
@@ -131,15 +131,9 @@

}
-static void delayx(uint8_t value) {
-#if 1
- int i;
- for(i=0;i<0x8000;i++) {
- outb(value, 0x80);
- }
-#endif
-}

+#include "pc80/udelay_io.c"
+
static void mcp55_early_pcie_setup(unsigned busnx, unsigned devnx,
unsigned anactrl_io_base, unsigned pci_e_x)
{
uint32_t tgio_ctrl;
@@ -170,14 +164,14 @@
outl(tgio_ctrl, anactrl_io_base + 0xcc);

// wait 100us
- delayx(1);
+ udelay(100);

dword = pci_read_config32(dev, 0xe4);
dword &= ~(0x3f0); // enable
pci_write_config32(dev, 0xe4, dword);

// need to wait 100ms
- delayx(1000);
+ udelay(100 * 1000);
}

static void mcp55_early_setup(unsigned mcp55_num, unsigned *busn,
unsigned *devn, unsigned *io_base, unsigned *pci_e_x)
> Index: src/mainboard/gigabyte/m57sli/fanctl.c
>
> --- src/mainboard/gigabyte/m57sli/fanctl.c	(revision 5411)
> +++ src/mainboard/gigabyte/m57sli/fanctl.c	(working copy)
> @@ -71,6 +71,7 @@
>  /*
> Called from superio.c
>
> */
> +extern void init_ec(uint16_t base);
>  void init_ec(uint16_t base)
>  {
>    int i;

init_ec() is the API between the superio drivers and the mainboard
drivers...

If this is a single hack, it's fine as it is.. If we're going to have an
API here, we should create a src/include/superio.h or some such
> Index: src/northbridge/amd/amdk8/exit_from_self.c
>
> --- src/northbridge/amd/amdk8/exit_from_self.c	(revision 5411)
> +++ src/northbridge/amd/amdk8/exit_from_self.c	(working copy)
> @@ -17,6 +17,8 @@
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
>
> */
>
> +extern void exit_from_self(int controllers, const struct mem_controller *ctrl,
>    struct sys_info *sysinfo);
>
> void exit_from_self(int controllers, const struct mem_controller *ctrl,
>    	    struct sys_info *sysinfo)
>  {

since this is a C file that is included in exactly one file,
"raminit_f.c" you could as well just mark the function "static".

btw, for function prototypes the extern in not really needed. I keep
removing them from the tree, but if people think we should have them,
I'll try to be consistent and stop deleting them :-)

Stefan

Acked-by: Stefan Reinauer