Am 17.11.2012 20:09 schrieb Stefan Tauner: > usbdev_status was created for the ft2232 programmer. Its IDs are semantically > different because they indicate USB instead of PCI IDs, but apart from that > both > data structures are equal. This change makes life easier for everything > involved > in handling and printing the status of devices that is noted in that > structure. > > --- > It is still possible to distinguish between PCI and USB devices by using the > struct programmer's type field. It still seems a bit hacky, but i think we > are better > off with it anyway. If we really would want to distinguish between the > different > types of IDs i'd rater make the IDs a union of different ID types inside > struct > dev_status. > > Signed-off-by: Stefan Tauner <[email protected]>
Thanks a lot for this cleanup! It kills substantial amounts of duplicated code. Acked-by: Carl-Daniel Hailfinger <[email protected]> Apart from the "devs" vs. "devices" naming (which you already resolved in your other reply), there is only one minor nit: > diff --git a/programmer.h b/programmer.h > index 21fa707..90033a9 100644 > --- a/programmer.h > +++ b/programmer.h > @@ -243,12 +250,6 @@ int rpci_write_word(struct pci_dev *dev, int reg, > uint16_t data); > int rpci_write_long(struct pci_dev *dev, int reg, uint32_t data); > #endif > > -/* 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 > -/* Not needed for CONFIG_INTERNAL, but for all other PCI-based programmers. > */ > -void print_supported_pcidevs(const struct pcidev_status *devs); > -#endif > - > struct usbdev_status { > uint16_t vendor_id; > uint16_t device_id; > @@ -264,6 +265,12 @@ struct pcidev_status { > const char *device_name; > }; > > +/* 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 > +/* Not needed for CONFIG_INTERNAL, but for all other PCI-based programmers. > */ > +void print_supported_pcidevs(const struct pcidev_status *devs); > +#endif > + > #if CONFIG_INTERNAL == 1 > /* board_enable.c */ > int board_parse_parameter(const char *boardstring, const char **vendor, > const char **model); AFAICS prints_supported_pcidevs has been killed by this patch, no need to keep its declaration in a header. Regards, Carl-Daniel -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
