| Message ID | 4061bba80ecc632b219b56d8a8cd25d62658d48a.1484557560.git.jan.kiszka@siemens.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2017-01-16 at 10:06 +0100, Jan Kiszka wrote: > Now that the core is ready for edge-triggered interrupts, we can > safely > allow the PCI versions that provide this to enable the feature and, > thus, have less shared interrupts. > My comments below. > - if (IS_ERR(ssp->clk)) > + if (IS_ERR(ssp->clk)) > return PTR_ERR(ssp->clk); This doesn't belong to the patch. > + pci_set_master(dev); > + > + ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES); > + if (ret < 0) { > + clk_unregister(ssp->clk); > + return ret; > + } > + ssp->irq = pci_irq_vector(dev, 0); > + This looks good, though I would put it closer to the initial place of ssp->irq assignment, i.e. before clock registering. > + pci_free_irq_vectors(dev); > + pci_free_irq_vectors(dev); You know my answer, right? So, please be sure that we are using pcim_alloc_irq_vectors(). Yes, I know there is (was?) no such API, needs to be created. Currently this might make a mess on ->remove().
On 2017-01-16 10:29, Andy Shevchenko wrote: > On Mon, 2017-01-16 at 10:06 +0100, Jan Kiszka wrote: >> Now that the core is ready for edge-triggered interrupts, we can >> safely >> allow the PCI versions that provide this to enable the feature and, >> thus, have less shared interrupts. >> > > My comments below. > >> - if (IS_ERR(ssp->clk)) >> + if (IS_ERR(ssp->clk)) >> return PTR_ERR(ssp->clk); > > This doesn't belong to the patch. > >> + pci_set_master(dev); >> + >> + ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES); >> + if (ret < 0) { >> + clk_unregister(ssp->clk); >> + return ret; >> + } >> + ssp->irq = pci_irq_vector(dev, 0); >> + > > This looks good, though I would put it closer to the initial place of > ssp->irq assignment, i.e. before clock registering. > >> + pci_free_irq_vectors(dev); >> + pci_free_irq_vectors(dev); > > You know my answer, right? So, please be sure that we are using > pcim_alloc_irq_vectors(). > > Yes, I know there is (was?) no such API, needs to be created. Currently > this might make a mess on ->remove(). > FWIW, I've an updated version of this patch already, addressing the remarks. Just waiting for a reply on the other patch now. Jan
diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c index 58d2d48..e7e5b08 100644 --- a/drivers/spi/spi-pxa2xx-pci.c +++ b/drivers/spi/spi-pxa2xx-pci.c @@ -203,16 +203,24 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev, ssp = &spi_pdata.ssp; ssp->phys_base = pci_resource_start(dev, 0); ssp->mmio_base = pcim_iomap_table(dev)[0]; - ssp->irq = dev->irq; ssp->port_id = (c->port_id >= 0) ? c->port_id : dev->devfn; ssp->type = c->type; snprintf(buf, sizeof(buf), "pxa2xx-spi.%d", ssp->port_id); ssp->clk = clk_register_fixed_rate(&dev->dev, buf , NULL, 0, c->max_clk_rate); - if (IS_ERR(ssp->clk)) + if (IS_ERR(ssp->clk)) return PTR_ERR(ssp->clk); + pci_set_master(dev); + + ret = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES); + if (ret < 0) { + clk_unregister(ssp->clk); + return ret; + } + ssp->irq = pci_irq_vector(dev, 0); + memset(&pi, 0, sizeof(pi)); pi.fwnode = dev->dev.fwnode; pi.parent = &dev->dev; @@ -224,6 +232,7 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev, pdev = platform_device_register_full(&pi); if (IS_ERR(pdev)) { clk_unregister(ssp->clk); + pci_free_irq_vectors(dev); return PTR_ERR(pdev); } @@ -241,6 +250,7 @@ static void pxa2xx_spi_pci_remove(struct pci_dev *dev) platform_device_unregister(pdev); clk_unregister(spi_pdata->ssp.clk); + pci_free_irq_vectors(dev); } static const struct pci_device_id pxa2xx_spi_pci_devices[] = {
Now that the core is ready for edge-triggered interrupts, we can safely allow the PCI versions that provide this to enable the feature and, thus, have less shared interrupts. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- drivers/spi/spi-pxa2xx-pci.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)