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.