On Mon, Jul 07, 2008 at 06:04:18AM -0600, Matthew Wilcox wrote: > On Mon, Jul 07, 2008 at 01:48:32PM +1000, Michael Ellerman wrote: > > Yeah seriously :) The _ is part of it, but MSI_ATTRIB is uglier than > > PCI_CAP_ID_MSI exactly because it's not PCI_CAP_ID_MSI, which exists and > > is well defined and is used in the rest of the code. > > Here's an improvement over both the status quo and my patch -- simply > use a single bit called is_msix. I prefer the "is_msix" for readability though I'm not particularly fond of bit fields (in any form). ... > @@ -589,12 +558,13 @@ void pci_msi_shutdown(struct pci_dev* dev) > u32 mask = entry->msi_attrib.maskbits_mask; > msi_set_mask_bits(dev->irq, mask, ~mask); > } > - if (!entry->dev || entry->msi_attrib.type != PCI_CAP_ID_MSI) > + if (!entry->dev || entry->msi_attrib.is_msix) > return; This is why I don't like bit fields. "uninitialized" (3rd state) doesn't exist. Is there something else in place to catch that state? (It's clearly a bug if someone did that and maybe I'm being too paranoid.) hth, grant -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html