Re: [PATCH 1/4] PCI MSI: Store the number of messages in themsi_desc

[Grant Grundler]

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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/