From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hidetoshi Seto Subject: Re: [PATCH 2/5] PCI: Add mask bit definition for MSI-X table Date: Fri, 05 Nov 2010 09:48:43 +0900 Message-ID: <4CD3546B.60909@jp.fujitsu.com> References: <1288851321-3964-1-git-send-email-sheng@linux.intel.com> <1288851321-3964-3-git-send-email-sheng@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-2022-JP Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , "Michael S. Tsirkin" , kvm@vger.kernel.org, Matthew Wilcox , Jesse Barnes , linux-pci@vger.kernel.org To: Sheng Yang Return-path: Received: from fgwmail6.fujitsu.co.jp ([192.51.44.36]:57531 "EHLO fgwmail6.fujitsu.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751613Ab0KEAtE (ORCPT ); Thu, 4 Nov 2010 20:49:04 -0400 In-Reply-To: <1288851321-3964-3-git-send-email-sheng@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: (2010/11/04 15:15), Sheng Yang wrote: > Then we can use it instead of magic number 1. > > Cc: Matthew Wilcox > Cc: Jesse Barnes > Cc: linux-pci@vger.kernel.org > Signed-off-by: Sheng Yang > --- > 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. Reviewed-by: Hidetoshi Seto Thanks, H.Seto