[flashrom] [PATCH 3/3] Warn if running on laptops

[Carl-Daniel Hailfinger]

Missing signoff. Will comment anyway.

On 24.02.2010 14:28, Michael Karcher wrote:
> diff --git a/dmi.c b/dmi.c
> index 100767f..ad023e0 100644
> --- a/dmi.c
> +++ b/dmi.c
> @@ -96,6 +96,7 @@ static char *get_dmi_string(const char * string_name)
>  void dmi_init(void)
>  {
>  	int i;
> +	char * chassis_type;
>   

char *chassis_type;


>  	has_dmi_support = 1;
>  	for (i = 0; i < DMI_ID_INVALID; i++) {
>  		dmistrings[i] = get_dmi_string(dmidecode_names[i]);
> @@ -104,6 +105,14 @@ void dmi_init(void)
>  			break;
>  		}
>  	}
> +
> +	chassis_type = get_dmi_string("chassis-type");
> +	if (chassis_type && !strcmp(chassis_type, "Notebook"))
> +	{
> +		printf_debug("Laptop detected via DMI");
> +		is_laptop = 1;
> +	}
> +	free (chassis_type);
>   

Not sure about the most common coding style. free(object) or free
(object). Would be nice to have uniform usage.


>  }
>  
>  /**
> diff --git a/flash.h b/flash.h
> index ff2004c..0ad6212 100644
> --- a/flash.h
> +++ b/flash.h
> @@ -382,6 +382,7 @@ struct pci_dev *pci_card_find(uint16_t vendor, uint16_t device,
>  void get_io_perms(void);
>  void release_io_perms(void);
>  #if INTERNAL_SUPPORT == 1
> +extern int is_laptop;
>  void probe_superio(void);
>  int internal_init(void);
>  int internal_shutdown(void);
> diff --git a/internal.c b/internal.c
> index 12dae39..d557738 100644
> --- a/internal.c
> +++ b/internal.c
> @@ -110,6 +110,8 @@ void probe_superio(void)
>  #endif
>  }
>  
> +int is_laptop;
> +
>  int internal_init(void)
>  {
>  	int ret = 0;
> @@ -131,6 +133,16 @@ int internal_init(void)
>  	/* Probe for the SuperI/O chip and fill global struct superio. */
>  	probe_superio();
>  
> +	/* Warn if a laptop is detected */
> +	if (is_laptop)
> +		printf("========================================================================\n"
> +		       "WARNING! You seem to be running flashrom on a laptop.\n"
> +		       "Laptops, notebooks and netbooks are difficult to support and we recommend\n"
> +		       "to use the vendor flashing utility. The embedded controller (EC) in these\n"
> +		       "machines often interacts badly with flashing\n"
> +		       "See http://www.flashrom.org/Supported_hardware#Unsupported_Laptops.2FNotebooks.2FNetbooks\n"
>   

Should we point people to http://www.flashrom.org/Laptops instead?


> +		       "========================================================================\n");
> +
>  	/* try to enable it. Failure IS an option, since not all motherboards
>  	 * really need this to be done, etc., etc.
>  	 */
>   

Looks good otherwise. Fix or explain the issues above, and this is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Regards,
Carl-Daniel

-- 
"I do consider assignment statements and pointer variables to be among
computer science's most valuable treasures."
-- Donald E. Knuth