On Fri, Sep 02, 2016 at 09:42:39AM +0100, Marc Zyngier wrote: > Hi Thomas, > > I haven't checked things is details (mostly because the resulting diff > is impossible to read), but one thing caught my eye: I agree, it's too bad the diff is so hard to read. > On 01/09/16 16:38, 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> > > --- > > drivers/pci/host/pci-aardvark.c | 172 ++++++++++++++++------------------------ > > 1 file changed, 67 insertions(+), 105 deletions(-) > > > > diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c > > index ef9893f..0675cfa 100644 > > --- a/drivers/pci/host/pci-aardvark.c > > +++ b/drivers/pci/host/pci-aardvark.c > > [...] > > > -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; > > You used not to support MSIX... > > [...] > > > @@ -677,30 +650,25 @@ 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; > > > > - msi_irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-msi", > > - dev_name(dev)); > > - if (!msi_irq_chip->name) > > - return -ENOMEM; > > + mutex_init(&pcie->msi_used_lock); > > > > - 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 = &pcie->msi_bottom_irq_chip; > > > > - msi = &pcie->msi; > > + 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->setup_irq = advk_pcie_setup_msi_irq; > > - msi->teardown_irq = advk_pcie_teardown_msi_irq; > > - msi->of_node = node; > > + msi_ic = &pcie->msi_irq_chip; > > + msi_ic->name = "advk-MSI"; > > > > - 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_FLAG_PCI_MSIX; > > And yet you now advertize MSIX support. Was this an oversight in the > original code? If this is a fix, it'd be nice to have two patches: 1) make the fix 2) convert to new API -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html