Hi Thomas, On 23/12/16 09:20, Thomas Petazzoni wrote: > The MSI support introduced with the initial Aardvark driver was based > on the msi_controller structure and the of_pci_msi_chip_add() / > of_pci_find_msi_chip_by_node() API, which are being deprecated in > favor of the generic MSI support. > > Therefore, this commit updates the Aardvark driver to use the generic > MSI support. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx> > --- > Changes since v1: > - Rebased on top of v4.9 > - Removed the MSI_FLAG_PCI_MSIX flag since we don't support MSI-X for > the moment. > --- > drivers/pci/host/pci-aardvark.c | 172 +++++++++++++++------------------------- > 1 file changed, 66 insertions(+), 106 deletions(-) > > diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c > index 4fce494..3fdca5d 100644 > --- a/drivers/pci/host/pci-aardvark.c > +++ b/drivers/pci/host/pci-aardvark.c > @@ -200,10 +200,12 @@ struct advk_pcie { > struct list_head resources; > struct irq_domain *irq_domain; > struct irq_chip irq_chip; > - struct msi_controller msi; > struct irq_domain *msi_domain; > + struct irq_domain *msi_inner_domain; > + struct irq_chip msi_bottom_irq_chip; > struct irq_chip msi_irq_chip; > - DECLARE_BITMAP(msi_irq_in_use, MSI_IRQ_NUM); > + struct msi_domain_info msi_domain_info; > + DECLARE_BITMAP(msi_used, MSI_IRQ_NUM); > struct mutex msi_used_lock; > u16 msi_msg; > int root_bus_nr; > @@ -545,94 +547,64 @@ static struct pci_ops advk_pcie_ops = { > .write = advk_pcie_wr_conf, > }; > > -static int advk_pcie_alloc_msi(struct advk_pcie *pcie) > +static void advk_msi_irq_compose_msi_msg(struct irq_data *data, > + struct msi_msg *msg) > { > - int hwirq; > + struct advk_pcie *pcie = irq_data_get_irq_chip_data(data); > + phys_addr_t msi_msg = virt_to_phys(&pcie->msi_msg); > > - mutex_lock(&pcie->msi_used_lock); > - hwirq = find_first_zero_bit(pcie->msi_irq_in_use, MSI_IRQ_NUM); > - if (hwirq >= MSI_IRQ_NUM) > - hwirq = -ENOSPC; > - else > - set_bit(hwirq, pcie->msi_irq_in_use); > - mutex_unlock(&pcie->msi_used_lock); > - > - return hwirq; > + msg->address_lo = lower_32_bits(msi_msg); > + msg->address_hi = upper_32_bits(msi_msg); > + msg->data = data->irq; > } > > -static void advk_pcie_free_msi(struct advk_pcie *pcie, int hwirq) > +static int advk_msi_set_affinity(struct irq_data *irq_data, > + const struct cpumask *mask, bool force) > { > - struct device *dev = &pcie->pdev->dev; > - > - mutex_lock(&pcie->msi_used_lock); > - if (!test_bit(hwirq, pcie->msi_irq_in_use)) > - dev_err(dev, "trying to free unused MSI#%d\n", hwirq); > - else > - clear_bit(hwirq, pcie->msi_irq_in_use); > - mutex_unlock(&pcie->msi_used_lock); > + return -EINVAL; > } > > -static int advk_pcie_setup_msi_irq(struct msi_controller *chip, > - struct pci_dev *pdev, > - struct msi_desc *desc) > +static int advk_msi_irq_domain_alloc(struct irq_domain *domain, > + unsigned int virq, > + unsigned int nr_irqs, void *args) > { > - struct advk_pcie *pcie = pdev->bus->sysdata; > - struct msi_msg msg; > - int virq, hwirq; > - phys_addr_t msi_msg_phys; > - > - /* We support MSI, but not MSI-X */ > - if (desc->msi_attrib.is_msix) > - return -EINVAL; > - > - hwirq = advk_pcie_alloc_msi(pcie); > - if (hwirq < 0) > - return hwirq; > + struct advk_pcie *pcie = domain->host_data; > + int hwirq, i; > > - virq = irq_create_mapping(pcie->msi_domain, hwirq); > - if (!virq) { > - advk_pcie_free_msi(pcie, hwirq); > - return -EINVAL; > + mutex_lock(&pcie->msi_used_lock); > + hwirq = bitmap_find_next_zero_area(pcie->msi_used, MSI_IRQ_NUM, > + 0, nr_irqs, 0); > + if (hwirq >= MSI_IRQ_NUM) { > + mutex_unlock(&pcie->msi_used_lock); > + return -ENOSPC; > } > > - irq_set_msi_desc(virq, desc); > - > - msi_msg_phys = virt_to_phys(&pcie->msi_msg); > - > - msg.address_lo = lower_32_bits(msi_msg_phys); > - msg.address_hi = upper_32_bits(msi_msg_phys); > - msg.data = virq; > - > - pci_write_msi_msg(virq, &msg); > - > - return 0; > -} > + bitmap_set(pcie->msi_used, hwirq, nr_irqs); > + mutex_unlock(&pcie->msi_used_lock); > > -static void advk_pcie_teardown_msi_irq(struct msi_controller *chip, > - unsigned int irq) > -{ > - struct irq_data *d = irq_get_irq_data(irq); > - struct msi_desc *msi = irq_data_get_msi_desc(d); > - struct advk_pcie *pcie = msi_desc_to_pci_sysdata(msi); > - unsigned long hwirq = d->hwirq; > + for (i = 0; i < nr_irqs; i++) > + irq_domain_set_info(domain, virq + i, hwirq + i, > + &pcie->msi_bottom_irq_chip, > + domain->host_data, handle_simple_irq, > + NULL, NULL); Given that you do not support multi-MSI, this function should never be called with nr_irqs != 1. Which means you could loose that loop. > > - irq_dispose_mapping(irq); > - advk_pcie_free_msi(pcie, hwirq); > + return hwirq; > } > > -static int advk_pcie_msi_map(struct irq_domain *domain, > - unsigned int virq, irq_hw_number_t hw) > +static void advk_msi_irq_domain_free(struct irq_domain *domain, > + unsigned int virq, unsigned int nr_irqs) > { > + struct irq_data *d = irq_domain_get_irq_data(domain, virq); > struct advk_pcie *pcie = domain->host_data; > > - irq_set_chip_and_handler(virq, &pcie->msi_irq_chip, > - handle_simple_irq); > - > - return 0; > + mutex_lock(&pcie->msi_used_lock); > + bitmap_clear(pcie->msi_used, d->hwirq, nr_irqs); > + mutex_unlock(&pcie->msi_used_lock); > } > > -static const struct irq_domain_ops advk_pcie_msi_irq_ops = { > - .map = advk_pcie_msi_map, > +static const struct irq_domain_ops advk_msi_domain_ops = { > + .alloc = advk_msi_irq_domain_alloc, > + .free = advk_msi_irq_domain_free, > }; > > static void advk_pcie_irq_mask(struct irq_data *d) > @@ -680,30 +652,24 @@ static int advk_pcie_init_msi_irq_domain(struct advk_pcie *pcie) > { > struct device *dev = &pcie->pdev->dev; > struct device_node *node = dev->of_node; > - struct irq_chip *msi_irq_chip; > - struct msi_controller *msi; > + struct irq_chip *bottom_ic, *msi_ic; > + struct msi_domain_info *msi_di; > phys_addr_t msi_msg_phys; > - int ret; > > - msi_irq_chip = &pcie->msi_irq_chip; > + mutex_init(&pcie->msi_used_lock); > > - msi_irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-msi", > - dev_name(dev)); > - if (!msi_irq_chip->name) > - return -ENOMEM; > + bottom_ic = &pcie->msi_bottom_irq_chip; > > - msi_irq_chip->irq_enable = pci_msi_unmask_irq; > - msi_irq_chip->irq_disable = pci_msi_mask_irq; > - msi_irq_chip->irq_mask = pci_msi_mask_irq; > - msi_irq_chip->irq_unmask = pci_msi_unmask_irq; > + bottom_ic->name = "MSI"; > + bottom_ic->irq_compose_msi_msg = advk_msi_irq_compose_msi_msg; > + bottom_ic->irq_set_affinity = advk_msi_set_affinity; > > - msi = &pcie->msi; > + msi_ic = &pcie->msi_irq_chip; > + msi_ic->name = "advk-MSI"; > > - msi->setup_irq = advk_pcie_setup_msi_irq; > - msi->teardown_irq = advk_pcie_teardown_msi_irq; > - msi->of_node = node; > - > - mutex_init(&pcie->msi_used_lock); > + msi_di = &pcie->msi_domain_info; > + msi_di->flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS; > + msi_di->chip = msi_ic; > > msi_msg_phys = virt_to_phys(&pcie->msi_msg); > > @@ -712,16 +678,18 @@ static int advk_pcie_init_msi_irq_domain(struct advk_pcie *pcie) > advk_writel(pcie, upper_32_bits(msi_msg_phys), > PCIE_MSI_ADDR_HIGH_REG); > > - pcie->msi_domain = > + pcie->msi_inner_domain = > irq_domain_add_linear(NULL, MSI_IRQ_NUM, > - &advk_pcie_msi_irq_ops, pcie); > - if (!pcie->msi_domain) > + &advk_msi_domain_ops, pcie); > + if (!pcie->msi_inner_domain) > return -ENOMEM; > > - ret = of_pci_msi_chip_add(msi); > - if (ret < 0) { > - irq_domain_remove(pcie->msi_domain); > - return ret; > + pcie->msi_domain = > + pci_msi_create_irq_domain(of_node_to_fwnode(node), > + msi_di, pcie->msi_inner_domain); > + if (!pcie->msi_domain) { > + irq_domain_remove(pcie->msi_inner_domain); > + return -ENOMEM; > } > > return 0; > @@ -729,8 +697,8 @@ static int advk_pcie_init_msi_irq_domain(struct advk_pcie *pcie) > > static void advk_pcie_remove_msi_irq_domain(struct advk_pcie *pcie) > { > - of_pci_msi_chip_remove(&pcie->msi); > irq_domain_remove(pcie->msi_domain); > + irq_domain_remove(pcie->msi_inner_domain); > } > > static int advk_pcie_init_irq_domain(struct advk_pcie *pcie) > @@ -917,8 +885,6 @@ static int advk_pcie_probe(struct platform_device *pdev) > struct advk_pcie *pcie; > struct resource *res; > struct pci_bus *bus, *child; > - struct msi_controller *msi; > - struct device_node *msi_node; > int ret, irq; > > pcie = devm_kzalloc(dev, sizeof(struct advk_pcie), GFP_KERNEL); > @@ -962,14 +928,8 @@ static int advk_pcie_probe(struct platform_device *pdev) > return ret; > } > > - msi_node = of_parse_phandle(dev->of_node, "msi-parent", 0); > - if (msi_node) > - msi = of_pci_find_msi_chip_by_node(msi_node); > - else > - msi = NULL; > - > - bus = pci_scan_root_bus_msi(dev, 0, &advk_pcie_ops, > - pcie, &pcie->resources, &pcie->msi); > + bus = pci_scan_root_bus(dev, 0, &advk_pcie_ops, > + pcie, &pcie->resources); > if (!bus) { > advk_pcie_remove_msi_irq_domain(pcie); > advk_pcie_remove_irq_domain(pcie); > Thanks, M. -- Jazz is not dead. It just smells funny... -- 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