[coreboot] [PATCH] flashrom - board enable - reconstruct table.

[Carl-Daniel Hailfinger]

On 20.04.2009 12:12, Luc Verhaegen wrote:
> Flashrom: board_enables: reconstruct table.
>
> This patch restores the pciid based board matching table. It makes this
> table readable and hackable again, and the only disadvantage is that the
> right margin is way beyond the rather dogmatic 80. All 0x0000 pci ids have
> been string replaced by 0 to more easily spot missing ids, and extra
> comments have been added to explain how the various entries are used.
>
> Signed-Off-By: Luc Verhaegen <libv at skynet.be>
>   

The extra comments are good, but "coreboot bios" in such a text really
hurts. Please change the word BIOS to all-caps and don't refer to
coreboot as a BIOS. Thanks.
The documentation part can be committed after one more iteration.

The rest of the patch makes board entries unreadable.
In the past few weeks, we had quite a few people in #coreboot who wanted
to add support for their boards. The presence of struct member names
helped immensely to explain how to add board enables. Some even figured
it out from similar entries. Lowering the barrier for possible future
developers is very important.

Since IIRC you said some time ago that the multiline layout hinders your
workflow, feel free to submit any new board enables in the single-line
layout. I'll fix them up.

Regards,
Carl-Daniel

> Index: board_enable.c
> ===================================================================
> --- board_enable.c	(revision 4132)
> +++ board_enable.c	(working copy)
> @@ -618,9 +618,18 @@
>   * the basis of subsystem/card IDs. As not every vendor handles
>   * subsystem/card IDs in a sane manner.
>   *
> - * Keep the second set NULLed if it should be ignored.
> + * Keep the second set NULLed if it should be ignored. Keep the subsystem IDs
> + * NULLed if they don't identify the board fully. But please take care to
> + * provide an as complete set of pci id as possible; autodetection is preferred
> + * behaviour and we would like to make sure that matches are unique.
>   *
> - * Keep the subsystem IDs NULLed if they don't identify the board fully.
> + * The coreboot ids are used two fold. When a coreboot bios is installed, it
> + * uniquely matches the coreboot board identification string. When a legacy
> + * bios is installed and when autodetection is not possible, these ids can be
> + * used to identify the board through a command line argument.
> + *
> + * When a board is identified through its coreboot ids (in both cases), the
> + * main pci ids will still be matched as a safe-guard.
>   */
>  struct board_pciid_enable {
>  	/* Any device, but make it sensible, like the ISA bridge. */
> @@ -629,7 +638,7 @@
>  	uint16_t first_card_vendor;
>  	uint16_t first_card_device;
>  
> -	/* Any device, but make it sensible, like 
> +	/* Any device, but make it sensible, like
>  	 * the host bridge. May be NULL.
>  	 */
>  	uint16_t second_vendor;
> @@ -646,425 +655,38 @@
>  };
>  
>  struct board_pciid_enable board_pciid_enables[] = {
> -	{
> -		.first_vendor		= 0x1106,
> -		.first_device		= 0x0571,
> -		.first_card_vendor	= 0x1462,
> -		.first_card_device	= 0x7120,
> -		.second_vendor		= 0x0000,
> -		.second_device		= 0x0000,
> -		.second_card_vendor	= 0x0000,
> -		.second_card_device	= 0x0000,
> -		.lb_vendor		= "msi",
> -		.lb_part		= "kt4v",
> -		.name			= "MSI KT4V",
> -		.enable			= board_msi_kt4v,
> -	},

-- 
http://www.hailfinger.org/