Re: [coreboot] [v2] r4874 - in trunk/coreboot-v2: . src/arch/i386/boot src/arch/i386/lib src/console src/devices src/include/device src/northbridge/intel/i440bx src/southbridge/intel/i82371eb util/romcc util/sconfig util/x86emu

[Stefan Reinauer]

[e-mail] wrote:
> Modified: trunk/coreboot-v2/src/console/vsprintf.c
>
> --- trunk/coreboot-v2/src/console/vsprintf.c	2009-10-27 16:24:22 UTC (rev 4873)
> +++ trunk/coreboot-v2/src/console/vsprintf.c	2009-10-27 21:49:33 UTC (rev 4874)
> @@ -48,6 +48,8 @@
>    return i;
>  }
>
> +int sprintf(char *buf, const char *fmt, ...);
>
> int sprintf(char *buf, const char *fmt, ...)
>  {
>    va_list args;

What's the reason for not just including "string.h" in this file?

Putting the prototype right in front of the function just shuts up the
warning. That warning's intention is to tell us that a caller of the
function might assume a wrong prototype implicitly.

I think if stuff is called externally, we should have their prototypes
in header files. Otherwise they should be defined static.
> Modified: trunk/coreboot-v2/src/devices/device.c
>
> --- trunk/coreboot-v2/src/devices/device.c	2009-10-27 16:24:22 UTC (rev 4873)
> +++ trunk/coreboot-v2/src/devices/device.c	2009-10-27 21:49:33 UTC (rev 4874)
> @@ -671,8 +671,11 @@
>  device_t vga_pri = 0;
>  static void set_vga_bridge_bits(void)
>  {
> -#warning "FIXME modify set_vga_bridge so it is less pci centric!"
> -#warning "This function knows too much about PCI stuff, it should be just a iterator/visitor."
>
> /*
> * FIXME: Modify set_vga_bridge so it is less PCI centric!
> * This function knows too much about PCI stuff, it should be just
> * an iterator/visitor.
> */
>
> /* FIXME: Handle the VGA palette snooping. */
>  struct device *dev, *vga, *vga_onboard, *vga_first, *vga_last;

Why was that warning removed?
> Modified: trunk/coreboot-v2/src/devices/pciexp_device.c
>
> --- trunk/coreboot-v2/src/devices/pciexp_device.c	2009-10-27 16:24:22 UTC (rev 4873)
> +++ trunk/coreboot-v2/src/devices/pciexp_device.c	2009-10-27 21:49:33 UTC (rev 4874)
> @@ -34,8 +34,8 @@
>    	/* error... */
>    	return;
>    }
> printk_debug("PCIe: tuning %s\n", dev_path(dev));
>
> -#warning "IMPLEMENT PCI EXPRESS TUNING"
> // printk_debug("PCIe: tuning %s\n", dev_path(dev));
> /* TODO: Implement PCI Express tuning. */
>
> }
>
> unsigned int pciexp_scan_bus(struct bus *bus,

What kind of tuning should be done here?

Stefan