[LinuxBIOS] r394 - in LinuxBIOSv3: mainboard mainboard/adl/msm800sev mainboard/amd/norwich mainboard/artecgroup mainboard/artecgroup/dbe61 mainboard/emulation/qemu-x86 util/dtc

[Uwe Hermann]

On Thu, Jun 28, 2007 at 06:21:38PM +0200, svn at openbios.org wrote:
>       mainboard-vendor = "AMD";
>       mainboard-part-number = "Norwich";

Wouldn't

        mainboard-name = "Norwich";

make more sense? It's not really a part number, it's a name.


>  }
> 
>  statictree.h will have:
>  extern const char *mainboard_vendor, *mainboard_part_number;
> 
>  and statictree.c will have:
>  const char *mainboard_vendor = "AMD";
>  const char *mainboard_part_number = "Norwich";
> 
>  It is an error to NOT have the vendor and part number in the top
>  level dts.

Great! Needs to be documentated somewhere, though.


> Modified: LinuxBIOSv3/mainboard/Kconfig
> ===================================================================
> --- LinuxBIOSv3/mainboard/Kconfig	2007-06-28 15:19:30 UTC (rev 393)
> +++ LinuxBIOSv3/mainboard/Kconfig	2007-06-28 16:21:38 UTC (rev 394)
> @@ -38,6 +38,12 @@
>  	  Select this option for various systems from 
>  	  Advanced Micro Devices, Inc
>  
> +config VENDOR_ARTECGROUP
> +	bool "Artec Group"
> +	help
> +	  Select this option for various systems from 
> +	  the Artec Group

Cosmetic:
Should end with a full stop, as every other sentence, and please write
it into one line (it won't exceed 79 characters, I think).

 
> +choice
> +	prompt "Mainboard model"
> +	depends on VENDOR_ARTECGROUP
> +
> +config BOARD_ARTECGROUP_DBE61
> +	bool "dbe61"

Should be "DBE61" as it's the user-visible name in menuconfig.


> +config MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID
> +	hex
> +	default 0x1022
> +	depends BOARD_ARTECGROUP_DBE61
> +	help
> +	  Mainboard specific PCI subsystem vendor ID.
> +
> +config MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID
> +	hex
> +	default 0x2323
> +	depends BOARD_ARTECGROUP_DBE61
> +	help
> +	  Mainboard specific PCI subsystem device ID.

Are the subsystem IDs correct for this board or copy+pasted?


> +$(obj)/linuxbios.vpd:
> +	$(Q)printf "  BUILD   DUMMY VPD\n"
> +	$(Q)dd if=/dev/zero of=$(obj)/linuxbios.vpd bs=256 count=1 $(SILENT)
> +

Mainboard-specific? If not it should be in some common Makefile, not in
every single mainboard Makefile.


> Added: LinuxBIOSv3/mainboard/artecgroup/dbe61/cmos.layout
> ===================================================================
> --- LinuxBIOSv3/mainboard/artecgroup/dbe61/cmos.layout	                        (rev 0)
> +++ LinuxBIOSv3/mainboard/artecgroup/dbe61/cmos.layout	2007-06-28 16:21:38 UTC (rev 394)

Missing license header.


> +	msr.lo = 
> +			(6<<28) |		// cas_lat
> +			(10<<24)|		// ref2act
> +			(7<<20)|		// act2pre
> +			(3<<16)|		// pre2act
> +			(3<<12)|		// act2cmd
> +			(2<<8)|			// act2act
> +			(2<<6)|			// dplwr
> +			(2<<4)|			// dplrd
> +			(3);			// dal

Maybe make the hardcoded values defines which have the name of the
comments to the right?


> Added: LinuxBIOSv3/mainboard/artecgroup/dbe61/irq_tables.c
> ===================================================================
> --- LinuxBIOSv3/mainboard/artecgroup/dbe61/irq_tables.c	                        (rev 0)
> +++ LinuxBIOSv3/mainboard/artecgroup/dbe61/irq_tables.c	2007-06-28 16:21:38 UTC (rev 394)

Missing license header.


> @@ -0,0 +1,60 @@
> +/* This file was generated by getpir.c, do not modify! 
> +   (but if you do, please run checkpir on it to verify)
> + * Contains the IRQ Routing Table dumped directly from your memory, which BIOS sets up
> + *
> + * Documentation at : http://www.microsoft.com/hwdev/busbios/PCIIRQ.HTM
> +*/

Can we drop this? A link to the documentation should be in the wiki, not
every single file. As far as I understand the getpir output is not
reliable anyway, so we should drop the above note (also from getpir output).


> +
> +#include <arch/pirq_routing.h>
> +
> +#define ID_SLOT_PCI_NET		1			// ThinCan ethernet
> +#define ID_SLOT_PCI_RSVD1	2           // reserved entry 1
> +#define ID_SLOT_PCI_RSVD3	3           // reserved entry 2
> +#define ID_SLOT_PCI_RSVD2	4			// reserved entry 3
> +#define ID_EMBED_PCI		0xff		// onboard PCI device
> +
> +// CS5535 PCI INT[A-D] Interrupt Routing lines.
> +#define NO_CONNECT			0			// not used
> +#define CS_PCI_INTA			1			// PCI INTA
> +#define CS_PCI_INTB			2			// PCI INTB
> +#define CS_PCI_INTC			3			// PCI INTC
> +#define CS_PCI_INTD			4			// PCI INTD
> +
> +// IRQ bitmap reference line	FEDCBA9876543210
> +//								0000110000100000b
> +#define PCI_IRQ					0xc20	// PCI allowed IRQs here
> +
> +const struct irq_routing_table intel_irq_routing_table = 
> +{
> +	PIRQ_SIGNATURE,  /* u32 signature */
> +	PIRQ_VERSION,    /* u16 version   */
> +	32+16*6,		/* there can be total 2 devices on the bus */
              ^                                       ^
   One of them is wrong (2 or 6)


> +	/* special for the root. Emit the names for the mainboard vendor and part # */
> +	for_each_property(bi->dt, prop) {
> +		if (streq(prop->name, "mainboard-vendor")){
> +			found_mainboard_vendor = 1;
> +			fprintf(f, "const char *mainboard_vendor = \"%s\";\n", prop->val.val);
> +		}
> +		if (streq(prop->name, "mainboard-part-number")){
> +			found_mainboard_partnumber = 1;
> +			fprintf(f, "const char *mainboard_part_number = \"%s\";\n", prop->val.val);
> +		}
> +	}
> +
> +	if (! 	found_mainboard_vendor){
             ^^^
       Drop the spaces.


> +		die("There is no mainboard-vendor property in the root. Please add one."
> +			"(and make sure there is a mainboard-part-number property too");
> +	}
> +
> +	if (! 	found_mainboard_partnumber){

             Ditto


> +		die("There is no mainboard-part-number property in the root. "
> +			"Please add one."
> +			"(and make sure there is a mainboard-vendor property too");
> +	}
> +
> +
>  	/* emit the code, if any */
>  	if (code)
>  		fprintf(f, "%s\n", code);
> @@ -1323,6 +1350,7 @@
>  	fix_next(bi->dt);
>  	/* emit any includes that we need  -- TODO: ONLY ONCE PER TYPE*/
>  	fprintf(f, "#include <device/device.h>\n#include <device/pci.h>\n");
> +	fprintf(f, "extern const char *mainboard_vendor, *mainboard_part_number;\n");
>  	flatten_tree_emit_includes(bi->dt, &linuxbios_emitter, f, &strbuf, vi);
>  
>  	flatten_tree_emit_structdecls(bi->dt, &linuxbios_emitter, f, &strbuf, vi);


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/20070628/e083d955/attachment.sig>