[flashrom] [PATCH] board_enable selfcheck

[Carl-Daniel Hailfinger]

Am 01.06.2014 02:27 schrieb Carl-Daniel Hailfinger:
> Found this bitrotting away on my disk. Rebased from r1645, last time all
> snippets compiled was r1539. If you kill the cli_classic.c hunk, it
> should compile. That hunk was in the patch to make sure the msg_gspew()
> calls in selfcheck() aren't no-ops by default. Hm. This seems to be a
> bug still in HEAD.
>
> Not for direct merge, there are other random changes in there as well.
> Stefan, you already wrote a patch doing a board enable array selfcheck,
> and AFAICS yours checks some more bits.
> AFAICS the most interesting point in this patch are the verbose
> messages/comments.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

selfcheck_board_enables() needs an additional bugfix to compile. See below.
Sorry.


> Index: flashrom-boardenable_selfcheck/cli_classic.c
> ===================================================================
> --- flashrom-boardenable_selfcheck/cli_classic.c	(Revision 1806)
> +++ flashrom-boardenable_selfcheck/cli_classic.c	(Arbeitskopie)
> @@ -139,8 +139,10 @@
>  	print_version();
>  	print_banner();
>  
> +	verbose = 2;
>  	if (selfcheck())
>  		exit(1);
> +	verbose = 0;
>  
>  	setbuf(stdout, NULL);
>  	/* FIXME: Delay all operation_specified checks until after command
> Index: flashrom-boardenable_selfcheck/flashrom.c
> ===================================================================
> --- flashrom-boardenable_selfcheck/flashrom.c	(Revision 1806)
> +++ flashrom-boardenable_selfcheck/flashrom.c	(Arbeitskopie)
> @@ -1303,6 +1303,10 @@
>  				eraser.eraseblocks[i].size;
>  		}
>  		/* Empty eraseblock definition with erase function.  */
> +		/* FIXME: This message will never trigger because spew and debug
> +		 * messages are not printed at this stage before debug variable
> +		 * initialization.
> +		 */
>  		if (!done && eraser.block_erase)
>  			msg_gspew("Strange: Empty eraseblock definition with "
>  				  "non-empty erase function. Not an error.\n");
> @@ -1768,7 +1772,11 @@
>  				ret = 1;
>  			}
>  		}
> +		ret = 1;
>  	}
> +	if (selfcheck_boardenables()) {
> +		msg_gerr("Board enable self check failed!\n");
> +	}
>  
>  	/* TODO: implement similar sanity checks for other arrays where deemed necessary. */
>  	return ret;
> Index: flashrom-boardenable_selfcheck/programmer.h
> ===================================================================
> --- flashrom-boardenable_selfcheck/programmer.h	(Revision 1806)
> +++ flashrom-boardenable_selfcheck/programmer.h	(Arbeitskopie)
> @@ -263,6 +263,7 @@
>  uint8_t sio_read(uint16_t port, uint8_t reg);
>  void sio_write(uint16_t port, uint8_t reg, uint8_t data);
>  void sio_mask(uint16_t port, uint8_t reg, uint8_t data, uint8_t mask);
> +int selfcheck_boardenables(void);
>  void board_handle_before_superio(void);
>  void board_handle_before_laptop(void);
>  int board_flash_enable(const char *vendor, const char *model, const char *cb_vendor, const char *cb_model);
> Index: flashrom-boardenable_selfcheck/board_enable.c
> ===================================================================
> --- flashrom-boardenable_selfcheck/board_enable.c	(Revision 1806)
> +++ flashrom-boardenable_selfcheck/board_enable.c	(Arbeitskopie)
> @@ -2532,9 +2532,72 @@
>  	return NULL;
>  }
>  
> +int selfcheck_boardenables(void)
> +{
> +	struct board_pciid_enable *board = board_pciid_enables;

Sorry. This should have been
struct board_match *board = board_matches;


> +	int ret = 0;
> +	int i = 0;
> +
> +	for (; board->vendor_name; board++) {
> +		if (!board->first_vendor) {
> +			/* This would only be valid for boards which don't have
> +			 * any onboard PCI devices at all.
> +			 */
> +			msg_gerr("Zero first vendor in position %i "
> +				 "of the board enable table\n", i);
> +			ret = 1;
> +		}
> +		if (!board->first_device) {
> +			/* This would only be valid for boards which don't have
> +			 * any onboard PCI devices at all.
> +			 */
> +			msg_gerr("Zero first device in position %i "
> +				 "of the board enable table\n", i);
> +			ret = 1;
> +		}
> +		if (!board->board_name) {
> +			/* A nameless board in the list is a maintenance
> +			 * nightmare.
> +			 */
> +			msg_gerr("Missing board name in position %i "
> +				 "of the board enable table\n", i);
> +			ret = 1;
> +		}
> +		if (!board->first_card_vendor && !board->dmi_pattern) {
> +			msg_gerr("Zero first subsystem vendor and no DMI "
> +				 "pattern in position %i of the board enable "
> +				 "table\n", i);
> +			ret = 1;
> +		}
> +		if (!board->first_card_device && !board->dmi_pattern) {
> +			msg_gerr("Zero first subsystem device and no DMI "
> +				 "pattern in position %i of the board enable "
> +				 "table\n", i);
> +			ret = 1;
> +		}
> +		if (!board->first_card_vendor && board->dmi_pattern) {
> +			/* Such boards are usually really old. No error. */
> +			msg_gspew("Zero first subsystem vendor but with DMI pattern in position %i "
> +				  "of the board enable table\n", i);
> +		}
> +		if (!board->first_card_device && board->dmi_pattern) {
> +			/* Such boards are usually really old. No error. */
> +			msg_gspew("Zero first subsystem device but with DMI pattern in position %i "
> +				  "of the board enable table\n", i);
> +		}
> +
> +		i++;
> +	}
> +	
> +	return ret;
> +}
> +
>  /*
>   * Match boards on PCI IDs and subsystem IDs.
>   * Second set of IDs can be either main+subsystem IDs, main IDs or no IDs.
> + *
> + * Logic of this function: First PCI dev has to match ven/dev/subven/subdev.
> + * The second PCI dev is completely optional (bad idea).
>   */
>  const static struct board_match *board_match_pci_ids(enum board_match_phase phase)
>  {
>

Regards,
Carl-Daniel

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