On Friday 05 November 2010 08:48:43 Hidetoshi Seto wrote: > (2010/11/04 15:15), Sheng Yang wrote: > > Then we can use it instead of magic number 1. > > > > Cc: Matthew Wilcox <[email protected]> > > Cc: Jesse Barnes <[email protected]> > > Cc: [email protected] > > Signed-off-by: Sheng Yang <[email protected]> > > --- > > > > drivers/pci/msi.c | 4 ++-- > > include/linux/pci_regs.h | 1 + > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > > index 69b7be3..673e7dc 100644 > > --- a/drivers/pci/msi.c > > +++ b/drivers/pci/msi.c > > @@ -158,7 +158,7 @@ static u32 __msix_mask_irq(struct msi_desc *desc, u32 > > flag) > > > > u32 mask_bits = desc->masked; > > unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE + > > > > PCI_MSIX_ENTRY_VECTOR_CTRL; > > > > - mask_bits &= ~1; > > + mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT; > > > > mask_bits |= flag; > > writel(mask_bits, desc->mask_base + offset); > > > > @@ -185,7 +185,7 @@ static void msi_set_mask_bit(unsigned irq, u32 flag) > > > > void mask_msi_irq(unsigned int irq) > > { > > > > - msi_set_mask_bit(irq, 1); > > + msi_set_mask_bit(irq, PCI_MSIX_ENTRY_CTRL_MASKBIT); > > > > } > > > > void unmask_msi_irq(unsigned int irq) > > This hunk is not good, because the function msi_set_mask_bit() is > not only for MSI-X but also for MSI, so the number 0/1 here works > as like as false/true: > > static void msi_set_mask_bit(struct irq_data *data, u32 flag) > { > struct msi_desc *desc = irq_data_get_msi(data); > > if (desc->msi_attrib.is_msix) { > msix_mask_irq(desc, flag); > readl(desc->mask_base); /* Flush write to device */ > } else { > unsigned offset = data->irq - desc->dev->irq; > msi_mask_irq(desc, 1 << offset, flag << offset); > } > } > > > diff --git a/include/linux/pci_regs.h b/include/linux/pci_regs.h > > index acfc224..ff51632 100644 > > --- a/include/linux/pci_regs.h > > +++ b/include/linux/pci_regs.h > > @@ -313,6 +313,7 @@ > > > > #define PCI_MSIX_ENTRY_UPPER_ADDR 4 > > #define PCI_MSIX_ENTRY_DATA 8 > > #define PCI_MSIX_ENTRY_VECTOR_CTRL 12 > > > > +#define PCI_MSIX_ENTRY_CTRL_MASKBIT 1 > > > > /* CompactPCI Hotswap Register */ > > So I recommend you to have a patch with the above hunk for header > plus a change like: > > - mask_bits &= ~1; > - mask_bits |= flag; > + mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT; > + mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT & !!flag; > > Anyway thank you very much for doing this work.
Good suggestion. Would update it. > > Reviewed-by: Hidetoshi Seto <[email protected]> -- regards Yang, Sheng > > Thanks, > H.Seto -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
