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;
- } 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.