[flashrom] [7/7, RFC] Unify usbdev_status and pcidev_status into dev_status.

[Kyösti Mälkki]

Mostly looked good to me, I did not rebase or compile.
Find a few comments inlined below.

Kyösti


On Sat, 2012-03-03 at 19:11 +0000, Stefan Tauner wrote:


> diff --git a/programmer.h b/programmer.h
> index 9f90fe5..b52727e 100644
> --- a/programmer.h
> +++ b/programmer.h
> @@ -93,15 +93,22 @@ enum programmer_type {
>  	OTHER,
>  };
>  
> +struct dev_status {
> +	uint16_t vendor_id;
> +	uint16_t device_id;
> +	const enum test_state status;
> +	const char *vendor_name;
> +	const char *device_name;
> +};

I find the name "dev_status" odd, these are the arrays for listing
and probing the VID/DID too, and the "status" is just documentation.


>  struct programmer_entry {
>  	const char *vendor;
>  	const char *name;
>  	enum programmer_type type;
>  	union {
> -		const struct pcidev_status *const pci;
> -		const struct usbdev_status *const usb;
> +		const struct dev_status *const status;
>  		const char * const note;
> -	} devices;
> +	} devs;

I don't know why the "devices->devs" rename is here. Maybe add comment
that "type" is the selector for the union. It might be confused for
some internal/nic/sata enumeration.

>  /* print.c */
>  #if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_NICINTEL+CONFIG_NICINTEL_SPI+CONFIG_OGP_SPI+CONFIG_SATAMV >= 1
> -void print_supported_pcidevs(const struct pcidev_status *devs);
> +void print_supported_devs(const struct dev_status *devs);
>  #endif

That #if does not look nice.