Matthew Wilcox wrote:
> On Tue, Jun 16, 2009 at 04:26:58PM +0900, Kenji Kaneshige wrote:
>> Remove __msi_set_enable() because it's used only by msi_set_enable().
>>
>> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>
>
> That's certainly one option. How about this one though?
Your patch looks better than mine, because it eliminates
pci_find_capability() calls.
Reviewed-by: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>
Thanks,
Kenji Kaneshige
>
> ----
>
> We have the 'pos' of the MSI capability at all locations which call
> msi_set_enable(), so pass it to msi_set_enable() instead of making it
> find the capability every time.
>
> Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxxxx>
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 3627732..10ad6e5 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -75,22 +75,17 @@ void arch_teardown_msi_irqs(struct pci_dev *dev)
> }
> #endif
>
> -static void __msi_set_enable(struct pci_dev *dev, int pos, int enable)
> +static void msi_set_enable(struct pci_dev *dev, int pos, int enable)
> {
> u16 control;
>
> - if (pos) {
> - pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
> - control &= ~PCI_MSI_FLAGS_ENABLE;
> - if (enable)
> - control |= PCI_MSI_FLAGS_ENABLE;
> - pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
> - }
> -}
> + BUG_ON(!pos);
>
> -static void msi_set_enable(struct pci_dev *dev, int enable)
> -{
> - __msi_set_enable(dev, pci_find_capability(dev, PCI_CAP_ID_MSI), enable);
> + pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
> + control &= ~PCI_MSI_FLAGS_ENABLE;
> + if (enable)
> + control |= PCI_MSI_FLAGS_ENABLE;
> + pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
> }
>
> static void msix_set_enable(struct pci_dev *dev, int enable)
> @@ -303,7 +298,7 @@ static void __pci_restore_msi_state(struct pci_dev *dev)
> pos = entry->msi_attrib.pos;
>
> pci_intx_for_msi(dev, 0);
> - msi_set_enable(dev, 0);
> + msi_set_enable(dev, pos, 0);
> write_msi_msg(dev->irq, &entry->msg);
>
> pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
> @@ -365,9 +360,9 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
> u16 control;
> unsigned mask;
>
> - msi_set_enable(dev, 0); /* Ensure msi is disabled as I set it up */
> -
> pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
> + msi_set_enable(dev, pos, 0); /* Disable MSI during set up */
> +
> pci_read_config_word(dev, msi_control_reg(pos), &control);
> /* MSI Entry Initialization */
> entry = alloc_msi_entry(dev);
> @@ -399,7 +394,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
>
> /* Set MSI enabled bits */
> pci_intx_for_msi(dev, 0);
> - msi_set_enable(dev, 1);
> + msi_set_enable(dev, pos, 1);
> dev->msi_enabled = 1;
>
> dev->irq = entry->irq;
> @@ -596,17 +591,20 @@ void pci_msi_shutdown(struct pci_dev *dev)
> struct msi_desc *desc;
> u32 mask;
> u16 ctrl;
> + unsigned pos;
>
> if (!pci_msi_enable || !dev || !dev->msi_enabled)
> return;
>
> - msi_set_enable(dev, 0);
> + BUG_ON(list_empty(&dev->msi_list));
> + desc = list_first_entry(&dev->msi_list, struct msi_desc, list);
> + pos = desc->msi_attrib.pos;
> +
> + msi_set_enable(dev, pos, 0);
> pci_intx_for_msi(dev, 1);
> dev->msi_enabled = 0;
>
> - BUG_ON(list_empty(&dev->msi_list));
> - desc = list_first_entry(&dev->msi_list, struct msi_desc, list);
> - pci_read_config_word(dev, desc->msi_attrib.pos + PCI_MSI_FLAGS, &ctrl);
> + pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &ctrl);
> mask = msi_capable_mask(ctrl);
> msi_mask_irq(desc, mask, ~mask);
>
>
--
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
