Re: [RFC PATCH 09/11] PCI/MSI: refactor PCI MSI driver

From: Yijing Wang
Date: Wed Aug 20 2014 - 02:36:37 EST

Next message: Lars-Peter Clausen: "Re: [PATCH] staging:iio: moved platform_data into include/linux/iio"
Previous message: Ley Foon Tan: "[PATCH] net: stmmac: add fix_mac_speed support for socfpga"
In reply to: Bharat.Bhushan@xxxxxxxxxxxxx: "RE: [RFC PATCH 09/11] PCI/MSI: refactor PCI MSI driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

>> @@ -1025,21 +1059,52 @@ int pci_msi_enabled(void)
>> }
>> EXPORT_SYMBOL(pci_msi_enabled);
>>
>> -void pci_msi_init_pci_dev(struct pci_dev *dev)
>> +static struct msi_ops pci_msi = {
>> + .msi_set_enable = msi_set_enable,
>> + .msi_setup_entry = msi_setup_entry,
>> + .msix_setup_entries = msix_setup_entries,
>> + .msi_mask_irq = default_msi_mask_irq,
>> + .msix_mask_irq = default_msix_mask_irq,
>> + .msi_read_message = __read_msi_msg,
>> + .msi_write_message = __write_msi_msg,
>> + .msi_set_intx = pci_intx_for_msi,
>> +};
>
> Ahh, want to be sure I am understanding this correctly. So if I have a non-pci driver "xyz" which wants to use separate ops then I need to have a all these functions in that driver. Something like driver/xyz/msi.c

Yes, because different MSI device has different MSI hardware registers, so every MSI type should provide its own msi_ops, or its own msi_driver in my new proposal.

>
> Thanks
> -Bharat
>
>> +
>> +struct msi_irqs *alloc_msi_irqs(void *data, struct msi_ops *ops)
>> {
>> - INIT_LIST_HEAD(&dev->msi_list);
>> + struct msi_irqs *msi;
>> +
>> + msi = kzalloc(sizeof(struct msi_irqs), GFP_KERNEL);
>> + if (!msi)
>> + return NULL;
>>
>> + INIT_LIST_HEAD(&msi->msi_list);
>> + msi->data = data;
>> + msi->ops = ops;
>> + return msi;
>> +}
>> +
>> +void pci_msi_init_pci_dev(struct pci_dev *dev)
>> +{
>> /* Disable the msi hardware to avoid screaming interrupts
>> * during boot. This is the power on reset default so
>> * usually this should be a noop.
>> */
>> dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
>> - if (dev->msi_cap)
>> - msi_set_enable(dev, 0);
>> -
>> dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
>> - if (dev->msix_cap)
>> - msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
>> +
>> + if (dev->msi_cap || dev->msix_cap) {
>> + dev->msi = alloc_msi_irqs(dev, &pci_msi);
>> + if (!dev->msi)
>> + return;
>> +
>> + dev->msi->node = dev_to_node(&dev->dev);
>> + if (dev->msi_cap)
>> + msi_set_enable(dev->msi, 0, MSI_TYPE);
>> +
>> + if (dev->msix_cap)
>> + msi_set_enable(dev->msi, 0, MSIX_TYPE);
>> + }
>> }
>>
>> /**
>> @@ -1060,13 +1125,13 @@ int pci_enable_msi_range(struct pci_dev *dev, int
>> minvec, int maxvec)
>> int rc;
>> struct msi_desc *entry;
>>
>> - if (dev->current_state != PCI_D0)
>> + if (dev->current_state != PCI_D0 || !dev->msi)
>> return -EINVAL;
>>
>> - WARN_ON(!!dev->msi_enabled);
>> + WARN_ON(!!pci_dev_msi_enabled(dev, MSI_TYPE));
>>
>> /* Check whether driver already requested MSI-X irqs */
>> - if (dev->msix_enabled) {
>> + if (pci_dev_msi_enabled(dev, MSIX_TYPE)) {
>> dev_info(&dev->dev,
>> "can't enable MSI (MSI-X already enabled)\n");
>> return -EINVAL;
>> @@ -1095,7 +1160,7 @@ int pci_enable_msi_range(struct pci_dev *dev, int minvec,
>> int maxvec)
>> } while (rc);
>>
>> do {
>> - rc = msi_capability_init(dev, nvec);
>> + rc = msi_capability_init(dev->msi, nvec);
>> if (rc < 0) {
>> return rc;
>> } else if (rc > 0) {
>> @@ -1107,14 +1172,14 @@ int pci_enable_msi_range(struct pci_dev *dev, int
>> minvec, int maxvec)
>>
>> rc = populate_msi_sysfs(dev);
>> if (rc) {
>> - msi_set_enable(dev, 0);
>> - pci_intx_for_msi(dev, 1);
>> - dev->msi_enabled = 0;
>> - free_msi_irqs(dev);
>> + msi_set_enable(dev->msi, 0, MSI_TYPE);
>> + pci_intx_for_msi(dev->msi, 1);
>> + dev->msi->msi_enabled = 0;
>> + free_msi_irqs(dev->msi);
>> return rc;
>> }
>>
>> - entry = list_entry(dev->msi_list.next, struct msi_desc, list);
>> + entry = list_entry(dev->msi->msi_list.next, struct msi_desc, list);
>> dev->irq = entry->irq;
>> return nvec;
>> }
>> @@ -1158,3 +1223,5 @@ int pci_enable_msix_range(struct pci_dev *dev, struct
>> msix_entry *entries,
>> return nvec;
>> }
>> EXPORT_SYMBOL(pci_enable_msix_range);
>> +
>> +
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index 5a672d3..fc8f3e8 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -83,15 +83,15 @@ struct msi_desc {
>> * implemented as weak symbols so that they /can/ be overriden by
>> * architecture specific code if needed.
>> */
>> -int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
>> +int arch_setup_msi_irq(struct msi_irqs *msi, struct msi_desc *desc);
>> void arch_teardown_msi_irq(unsigned int irq);
>> -int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
>> -void arch_teardown_msi_irqs(struct pci_dev *dev);
>> -int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
>> -void arch_restore_msi_irqs(struct pci_dev *dev);
>> +int arch_setup_msi_irqs(struct msi_irqs *msi, int nvec, int type);
>> +void arch_teardown_msi_irqs(struct msi_irqs *msi);
>> +int arch_msi_check_device(struct msi_irqs *msi, int nvec, int type);
>> +void arch_restore_msi_irqs(struct msi_irqs *msi);
>>
>> -void default_teardown_msi_irqs(struct pci_dev *dev);
>> -void default_restore_msi_irqs(struct pci_dev *dev);
>> +void default_teardown_msi_irqs(struct msi_irqs *msi);
>> +void default_restore_msi_irqs(struct msi_irqs *msi);
>> u32 default_msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag);
>> u32 default_msix_mask_irq(struct msi_desc *desc, u32 flag);
>>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index c7bca1c..d7126fc 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -334,8 +334,6 @@ struct pci_dev {
>> unsigned int block_cfg_access:1; /* config space access is blocked */
>> unsigned int broken_parity_status:1; /* Device generates false positive
>> parity */
>> unsigned int irq_reroute_variant:2; /* device needs IRQ rerouting
>> variant */
>> - unsigned int msi_enabled:1;
>> - unsigned int msix_enabled:1;
>> unsigned int ari_enabled:1; /* ARI forwarding */
>> unsigned int is_managed:1;
>> unsigned int needs_freset:1; /* Dev requires fundamental reset */
>> @@ -358,7 +356,7 @@ struct pci_dev {
>> struct bin_attribute *res_attr[DEVICE_COUNT_RESOURCE]; /* sysfs file for
>> resources */
>> struct bin_attribute *res_attr_wc[DEVICE_COUNT_RESOURCE]; /* sysfs file
>> for WC mapping of resources */
>> #ifdef CONFIG_PCI_MSI
>> - struct list_head msi_list;
>> + struct msi_irqs *msi;
>> const struct attribute_group **msi_irq_groups;
>> #endif
>> struct pci_vpd *vpd;
>> @@ -510,11 +508,14 @@ static inline struct pci_dev *pci_upstream_bridge(struct
>> pci_dev *dev)
>> static inline bool pci_dev_msi_enabled(struct pci_dev *pci_dev, int type)
>> {
>> bool enabled = 0;
>> +
>> + if (!pci_dev->msi)
>> + return false;
>>
>> if (type & MSI_TYPE)
>> - enabled |= pci_dev->msi_enabled;
>> + enabled |= pci_dev->msi->msi_enabled;
>> if (type & MSIX_TYPE)
>> - enabled |= pci_dev->msix_enabled;
>> + enabled |= pci_dev->msi->msix_enabled;
>>
>> return enabled;
>> }
>> --
>> 1.7.1
>>
>> --
>> 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
>
> .
>

--
Thanks!
Yijing

--
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/

Next message: Lars-Peter Clausen: "Re: [PATCH] staging:iio: moved platform_data into include/linux/iio"
Previous message: Ley Foon Tan: "[PATCH] net: stmmac: add fix_mac_speed support for socfpga"
In reply to: Bharat.Bhushan@xxxxxxxxxxxxx: "RE: [RFC PATCH 09/11] PCI/MSI: refactor PCI MSI driver"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]