Patchwork Kill spurious cbtable related board enable warning

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2009-07-27 17:39:36
Message ID <4A6DE658.6060200@gmx.net>
Download mbox | patch
Permalink /patch/76/
State Accepted
Commit r668
Headers show

Comments

Carl-Daniel Hailfinger - 2009-07-27 17:39:36
Boards with coreboot have a cbtable containing vendor and board name.
flashrom tries to match these with board enable entries in its database.
If no such board enable entry exists because the board doesn't need one,
flashrom complains. Silence that complaint.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Stefan Reinauer - 2009-07-27 18:00:41
On 7/27/09 7:39 PM, Carl-Daniel Hailfinger wrote:
> Index: flashrom-cbtable_boardenable_nowarn/board_enable.c
> ===================================================================
> --- flashrom-cbtable_boardenable_nowarn/board_enable.c	(Revision 667)
> +++ flashrom-cbtable_boardenable_nowarn/board_enable.c	(Arbeitskopie)
> @@ -1134,7 +1134,14 @@
>  	if (partmatch)
>  		return partmatch;
>  
> -	printf("\nUnknown vendor:board from coreboot table or -m option: %s:%s\n\n", vendor, part);
> +	if (partvendor_from_cbtable) {
> +		printf("vendor:board from coreboot table is %s:%s, apparently "
> +		       "no board specific flash chip write enable is needed.\n",
> +		       vendor, part);
> +	} else {
> +		printf("\nUnknown vendor:board from -m option: %s:%s\n\n",
> +		       vendor, part);
> +	}
>  	return NULL;
>  }
>   

Can we get a method to make the coreboot table case not output any odd
technical message at all?

Stefan
Carl-Daniel Hailfinger - 2009-07-27 18:27:20
On 27.07.2009 20:00, Stefan Reinauer wrote:
> On 7/27/09 7:39 PM, Carl-Daniel Hailfinger wrote:
>   
>> Index: flashrom-cbtable_boardenable_nowarn/board_enable.c
>> ===================================================================
>> --- flashrom-cbtable_boardenable_nowarn/board_enable.c	(Revision 667)
>> +++ flashrom-cbtable_boardenable_nowarn/board_enable.c	(Arbeitskopie)
>> @@ -1134,7 +1134,14 @@
>>  	if (partmatch)
>>  		return partmatch;
>>  
>> -	printf("\nUnknown vendor:board from coreboot table or -m option: %s:%s\n\n", vendor, part);
>> +	if (partvendor_from_cbtable) {
>> +		printf("vendor:board from coreboot table is %s:%s, apparently "
>> +		       "no board specific flash chip write enable is needed.\n",
>> +		       vendor, part);
>> +	} else {
>> +		printf("\nUnknown vendor:board from -m option: %s:%s\n\n",
>> +		       vendor, part);
>> +	}
>>  	return NULL;
>>  }
>>   
>>     
>
> Can we get a method to make the coreboot table case not output any odd
> technical message at all?
>   

Sure, I can remove the if part and transform the else part to
if (!partvendor_from_cbtable)

Would that be OK for you?

Regards,
Carl-Daniel
Stefan Reinauer - 2009-07-27 18:32:06
On 7/27/09 8:27 PM, Carl-Daniel Hailfinger wrote:
> On 27.07.2009 20:00, Stefan Reinauer wrote:
>   
>> On 7/27/09 7:39 PM, Carl-Daniel Hailfinger wrote:
>>   
>>     
>>> Index: flashrom-cbtable_boardenable_nowarn/board_enable.c
>>> ===================================================================
>>> --- flashrom-cbtable_boardenable_nowarn/board_enable.c	(Revision 667)
>>> +++ flashrom-cbtable_boardenable_nowarn/board_enable.c	(Arbeitskopie)
>>> @@ -1134,7 +1134,14 @@
>>>  	if (partmatch)
>>>  		return partmatch;
>>>  
>>> -	printf("\nUnknown vendor:board from coreboot table or -m option: %s:%s\n\n", vendor, part);
>>> +	if (partvendor_from_cbtable) {
>>> +		printf("vendor:board from coreboot table is %s:%s, apparently "
>>> +		       "no board specific flash chip write enable is needed.\n",
>>> +		       vendor, part);
>>> +	} else {
>>> +		printf("\nUnknown vendor:board from -m option: %s:%s\n\n",
>>> +		       vendor, part);
>>> +	}
>>>  	return NULL;
>>>  }
>>>   
>>>     
>>>       
>> Can we get a method to make the coreboot table case not output any odd
>> technical message at all?
>>   
>>     
> Sure, I can remove the if part and transform the else part to
> if (!partvendor_from_cbtable)
>
> Would that be OK for you?
I think that's better... the mainboard and vendor are printed before, so
in case something would go wrong, we'd still know from a report which
board it is..

Patch

Index: flashrom-cbtable_boardenable_nowarn/flash.h
===================================================================
--- flashrom-cbtable_boardenable_nowarn/flash.h	(Revision 667)
+++ flashrom-cbtable_boardenable_nowarn/flash.h	(Arbeitskopie)
@@ -402,6 +402,7 @@ 
 /* cbtable.c */
 int coreboot_init(void);
 extern char *lb_part, *lb_vendor;
+extern int partvendor_from_cbtable;
 
 /* spi.c */
 enum spi_controller {
Index: flashrom-cbtable_boardenable_nowarn/cbtable.c
===================================================================
--- flashrom-cbtable_boardenable_nowarn/cbtable.c	(Revision 667)
+++ flashrom-cbtable_boardenable_nowarn/cbtable.c	(Arbeitskopie)
@@ -31,6 +31,7 @@ 
 #include "coreboot_tables.h"
 
 char *lb_part = NULL, *lb_vendor = NULL;
+int partvendor_from_cbtable = 0;
 
 static unsigned long compute_checksum(void *addr, unsigned long length)
 {
@@ -150,6 +151,7 @@ 
 	if (lb_part) {
 		printf("Overwritten by command line, vendor ID: %s, part ID: %s.\n", lb_vendor, lb_part);
 	} else {
+		partvendor_from_cbtable = 1;
 		lb_part = strdup(part);
 		lb_vendor = strdup(vendor);
 	}
Index: flashrom-cbtable_boardenable_nowarn/board_enable.c
===================================================================
--- flashrom-cbtable_boardenable_nowarn/board_enable.c	(Revision 667)
+++ flashrom-cbtable_boardenable_nowarn/board_enable.c	(Arbeitskopie)
@@ -1134,7 +1134,14 @@ 
 	if (partmatch)
 		return partmatch;
 
-	printf("\nUnknown vendor:board from coreboot table or -m option: %s:%s\n\n", vendor, part);
+	if (partvendor_from_cbtable) {
+		printf("vendor:board from coreboot table is %s:%s, apparently "
+		       "no board specific flash chip write enable is needed.\n",
+		       vendor, part);
+	} else {
+		printf("\nUnknown vendor:board from -m option: %s:%s\n\n",
+		       vendor, part);
+	}
 	return NULL;
 }