* [PATCH 0/2] spi: pca2xx: Prepare for and enable MSI support @ 2017-01-16 9:05 Jan Kiszka 2017-01-16 9:05 ` [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts Jan Kiszka 2017-01-16 9:06 ` [PATCH 2/2] spi: pca2xx-pci: Allow MSI Jan Kiszka 0 siblings, 2 replies; 9+ messages in thread From: Jan Kiszka @ 2017-01-16 9:05 UTC (permalink / raw) To: linux-arm-kernel Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, linux-kernel, Andy Shevchenko, Mika Westerberg, Jarkko Nikula, Sascha Weisenberger This enhances the pca2xx driver's interrupt handler with support for edge-triggered interrupts and makes use of that for PCI-hosted variants, like the Intel Quark SoC. Jan Jan Kiszka (2): spi: pxa2xx: Prepare for edge-triggered interrupts spi: pca2xx-pci: Allow MSI drivers/spi/spi-pxa2xx-pci.c | 14 +++++++++++-- drivers/spi/spi-pxa2xx.c | 50 +++++++++++++++++++++++++------------------- 2 files changed, 40 insertions(+), 24 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts 2017-01-16 9:05 [PATCH 0/2] spi: pca2xx: Prepare for and enable MSI support Jan Kiszka @ 2017-01-16 9:05 ` Jan Kiszka 2017-01-16 9:24 ` Andy Shevchenko 2017-01-16 9:06 ` [PATCH 2/2] spi: pca2xx-pci: Allow MSI Jan Kiszka 1 sibling, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2017-01-16 9:05 UTC (permalink / raw) To: linux-arm-kernel Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, linux-kernel, Andy Shevchenko, Mika Westerberg, Jarkko Nikula, Sascha Weisenberger When using the a device with edge-triggered interrupts, such as MSIs, the interrupt handler has to ensure that there is a point in time during its execution where all interrupts sources are silent so that a new event can trigger a new interrupt again. This is achieved here by looping over SSSR evaluation. We need to take into account that SSCR1 may be changed by the transfer handler, thus we need to redo the mask calculation, at least regarding the volatile interrupt enable bit (TIE). Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- drivers/spi/spi-pxa2xx.c | 50 +++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c index dd7b5b4..24bf549 100644 --- a/drivers/spi/spi-pxa2xx.c +++ b/drivers/spi/spi-pxa2xx.c @@ -737,6 +737,7 @@ static irqreturn_t ssp_int(int irq, void *dev_id) struct driver_data *drv_data = dev_id; u32 sccr1_reg; u32 mask = drv_data->mask_sr; + irqreturn_t ret = IRQ_NONE; u32 status; /* @@ -760,37 +761,42 @@ static irqreturn_t ssp_int(int irq, void *dev_id) sccr1_reg = pxa2xx_spi_read(drv_data, SSCR1); - /* Ignore possible writes if we don't need to write */ - if (!(sccr1_reg & SSCR1_TIE)) - mask &= ~SSSR_TFS; - /* Ignore RX timeout interrupt if it is disabled */ if (!(sccr1_reg & SSCR1_TINTE)) mask &= ~SSSR_TINT; - if (!(status & mask)) - return IRQ_NONE; + while (1) { + /* Ignore possible writes if we don't need to write */ + if (!(sccr1_reg & SSCR1_TIE)) + mask &= ~SSSR_TFS; - if (!drv_data->master->cur_msg) { + if (!(status & mask)) + return ret; - pxa2xx_spi_write(drv_data, SSCR0, - pxa2xx_spi_read(drv_data, SSCR0) - & ~SSCR0_SSE); - pxa2xx_spi_write(drv_data, SSCR1, - pxa2xx_spi_read(drv_data, SSCR1) - & ~drv_data->int_cr1); - if (!pxa25x_ssp_comp(drv_data)) - pxa2xx_spi_write(drv_data, SSTO, 0); - write_SSSR_CS(drv_data, drv_data->clear_sr); + if (!drv_data->master->cur_msg) { - dev_err(&drv_data->pdev->dev, - "bad message state in interrupt handler\n"); + pxa2xx_spi_write(drv_data, SSCR0, + pxa2xx_spi_read(drv_data, SSCR0) + & ~SSCR0_SSE); + pxa2xx_spi_write(drv_data, SSCR1, + pxa2xx_spi_read(drv_data, SSCR1) + & ~drv_data->int_cr1); + if (!pxa25x_ssp_comp(drv_data)) + pxa2xx_spi_write(drv_data, SSTO, 0); + write_SSSR_CS(drv_data, drv_data->clear_sr); - /* Never fail */ - return IRQ_HANDLED; - } + dev_err(&drv_data->pdev->dev, + "bad message state in interrupt handler\n"); - return drv_data->transfer_handler(drv_data); + /* Never fail */ + return IRQ_HANDLED; + } + + ret |= drv_data->transfer_handler(drv_data); + + status = pxa2xx_spi_read(drv_data, SSSR); + sccr1_reg = pxa2xx_spi_read(drv_data, SSCR1); + } } /* -- 2.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts 2017-01-16 9:05 ` [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts Jan Kiszka @ 2017-01-16 9:24 ` Andy Shevchenko 2017-01-16 11:18 ` Jan Kiszka 0 siblings, 1 reply; 9+ messages in thread From: Andy Shevchenko @ 2017-01-16 9:24 UTC (permalink / raw) To: Jan Kiszka, linux-arm-kernel Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, linux-kernel, Mika Westerberg, Jarkko Nikula, Sascha Weisenberger On Mon, 2017-01-16 at 10:05 +0100, Jan Kiszka wrote: > When using the a device with edge-triggered interrupts, such as MSIs, > the interrupt handler has to ensure that there is a point in time > during > its execution where all interrupts sources are silent so that a new > event can trigger a new interrupt again. > > This is achieved here by looping over SSSR evaluation. We need to take > into account that SSCR1 may be changed by the transfer handler, thus > we > need to redo the mask calculation, at least regarding the volatile > interrupt enable bit (TIE). Could you split this to two patches, one just move the code under question to a helper function (no functional change), the other does what you state in commit message here? > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > drivers/spi/spi-pxa2xx.c | 50 +++++++++++++++++++++++++++---------- > ----------- > 1 file changed, 28 insertions(+), 22 deletions(-) > > diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c > index dd7b5b4..24bf549 100644 > --- a/drivers/spi/spi-pxa2xx.c > +++ b/drivers/spi/spi-pxa2xx.c > @@ -737,6 +737,7 @@ static irqreturn_t ssp_int(int irq, void *dev_id) > struct driver_data *drv_data = dev_id; > u32 sccr1_reg; > u32 mask = drv_data->mask_sr; > + irqreturn_t ret = IRQ_NONE; > u32 status; > > /* > @@ -760,37 +761,42 @@ static irqreturn_t ssp_int(int irq, void > *dev_id) > > sccr1_reg = pxa2xx_spi_read(drv_data, SSCR1); > > - /* Ignore possible writes if we don't need to write */ > - if (!(sccr1_reg & SSCR1_TIE)) > - mask &= ~SSSR_TFS; > - > /* Ignore RX timeout interrupt if it is disabled */ > if (!(sccr1_reg & SSCR1_TINTE)) > mask &= ~SSSR_TINT; > > - if (!(status & mask)) > - return IRQ_NONE; > + while (1) { > + /* Ignore possible writes if we don't need to write > */ > + if (!(sccr1_reg & SSCR1_TIE)) > + mask &= ~SSSR_TFS; > > - if (!drv_data->master->cur_msg) { > + if (!(status & mask)) > + return ret; > > - pxa2xx_spi_write(drv_data, SSCR0, > - pxa2xx_spi_read(drv_data, SSCR0) > - & ~SSCR0_SSE); > - pxa2xx_spi_write(drv_data, SSCR1, > - pxa2xx_spi_read(drv_data, SSCR1) > - & ~drv_data->int_cr1); > - if (!pxa25x_ssp_comp(drv_data)) > - pxa2xx_spi_write(drv_data, SSTO, 0); > - write_SSSR_CS(drv_data, drv_data->clear_sr); > + if (!drv_data->master->cur_msg) { > > - dev_err(&drv_data->pdev->dev, > - "bad message state in interrupt handler\n"); > + pxa2xx_spi_write(drv_data, SSCR0, > + pxa2xx_spi_read(drv_data, > SSCR0) > + & ~SSCR0_SSE); > + pxa2xx_spi_write(drv_data, SSCR1, > + pxa2xx_spi_read(drv_data, > SSCR1) > + & ~drv_data->int_cr1); > + if (!pxa25x_ssp_comp(drv_data)) > + pxa2xx_spi_write(drv_data, SSTO, 0); > + write_SSSR_CS(drv_data, drv_data->clear_sr); > > - /* Never fail */ > - return IRQ_HANDLED; > - } > + dev_err(&drv_data->pdev->dev, > + "bad message state in interrupt > handler\n"); > > - return drv_data->transfer_handler(drv_data); > + /* Never fail */ > + return IRQ_HANDLED; > + } > + > + ret |= drv_data->transfer_handler(drv_data); > + > + status = pxa2xx_spi_read(drv_data, SSSR); > + sccr1_reg = pxa2xx_spi_read(drv_data, SSCR1); > + } > } > > /* -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts 2017-01-16 9:24 ` Andy Shevchenko @ 2017-01-16 11:18 ` Jan Kiszka 2017-01-16 17:53 ` Andy Shevchenko 0 siblings, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2017-01-16 11:18 UTC (permalink / raw) To: Andy Shevchenko, linux-arm-kernel Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, linux-kernel, Mika Westerberg, Jarkko Nikula, Sascha Weisenberger On 2017-01-16 10:24, Andy Shevchenko wrote: > On Mon, 2017-01-16 at 10:05 +0100, Jan Kiszka wrote: >> When using the a device with edge-triggered interrupts, such as MSIs, >> the interrupt handler has to ensure that there is a point in time >> during >> its execution where all interrupts sources are silent so that a new >> event can trigger a new interrupt again. >> >> This is achieved here by looping over SSSR evaluation. We need to take >> into account that SSCR1 may be changed by the transfer handler, thus >> we >> need to redo the mask calculation, at least regarding the volatile >> interrupt enable bit (TIE). > > Could you split this to two patches, one just move the code under > question to a helper function (no functional change), the other does > what you state in commit message here? IMHO, factoring out some helper called from the loop in ssp_int won't be a natural split due to the large number of local variables being shared here. But maybe I'm not seeing the design you have in mind, so please propose a useful helper function signature. Thanks, Jan > >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> drivers/spi/spi-pxa2xx.c | 50 +++++++++++++++++++++++++++---------- >> ----------- >> 1 file changed, 28 insertions(+), 22 deletions(-) >> >> diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c >> index dd7b5b4..24bf549 100644 >> --- a/drivers/spi/spi-pxa2xx.c >> +++ b/drivers/spi/spi-pxa2xx.c >> @@ -737,6 +737,7 @@ static irqreturn_t ssp_int(int irq, void *dev_id) >> struct driver_data *drv_data = dev_id; >> u32 sccr1_reg; >> u32 mask = drv_data->mask_sr; >> + irqreturn_t ret = IRQ_NONE; >> u32 status; >> >> /* >> @@ -760,37 +761,42 @@ static irqreturn_t ssp_int(int irq, void >> *dev_id) >> >> sccr1_reg = pxa2xx_spi_read(drv_data, SSCR1); >> >> - /* Ignore possible writes if we don't need to write */ >> - if (!(sccr1_reg & SSCR1_TIE)) >> - mask &= ~SSSR_TFS; >> - >> /* Ignore RX timeout interrupt if it is disabled */ >> if (!(sccr1_reg & SSCR1_TINTE)) >> mask &= ~SSSR_TINT; >> >> - if (!(status & mask)) >> - return IRQ_NONE; >> + while (1) { >> + /* Ignore possible writes if we don't need to write >> */ >> + if (!(sccr1_reg & SSCR1_TIE)) >> + mask &= ~SSSR_TFS; >> >> - if (!drv_data->master->cur_msg) { >> + if (!(status & mask)) >> + return ret; >> >> - pxa2xx_spi_write(drv_data, SSCR0, >> - pxa2xx_spi_read(drv_data, SSCR0) >> - & ~SSCR0_SSE); >> - pxa2xx_spi_write(drv_data, SSCR1, >> - pxa2xx_spi_read(drv_data, SSCR1) >> - & ~drv_data->int_cr1); >> - if (!pxa25x_ssp_comp(drv_data)) >> - pxa2xx_spi_write(drv_data, SSTO, 0); >> - write_SSSR_CS(drv_data, drv_data->clear_sr); >> + if (!drv_data->master->cur_msg) { >> >> - dev_err(&drv_data->pdev->dev, >> - "bad message state in interrupt handler\n"); >> + pxa2xx_spi_write(drv_data, SSCR0, >> + pxa2xx_spi_read(drv_data, >> SSCR0) >> + & ~SSCR0_SSE); >> + pxa2xx_spi_write(drv_data, SSCR1, >> + pxa2xx_spi_read(drv_data, >> SSCR1) >> + & ~drv_data->int_cr1); >> + if (!pxa25x_ssp_comp(drv_data)) >> + pxa2xx_spi_write(drv_data, SSTO, 0); >> + write_SSSR_CS(drv_data, drv_data->clear_sr); >> >> - /* Never fail */ >> - return IRQ_HANDLED; >> - } >> + dev_err(&drv_data->pdev->dev, >> + "bad message state in interrupt >> handler\n"); >> >> - return drv_data->transfer_handler(drv_data); >> + /* Never fail */ >> + return IRQ_HANDLED; >> + } >> + >> + ret |= drv_data->transfer_handler(drv_data); >> + >> + status = pxa2xx_spi_read(drv_data, SSSR); >> + sccr1_reg = pxa2xx_spi_read(drv_data, SSCR1); >> + } >> } >> >> /* > -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts 2017-01-16 11:18 ` Jan Kiszka @ 2017-01-16 17:53 ` Andy Shevchenko 2017-01-16 18:19 ` Jan Kiszka 0 siblings, 1 reply; 9+ messages in thread From: Andy Shevchenko @ 2017-01-16 17:53 UTC (permalink / raw) To: Jan Kiszka Cc: Andy Shevchenko, linux-arm Mailing List, Daniel Mack, Haojian Zhuang, Robert Jarzmik, linux-kernel, Mika Westerberg, Jarkko Nikula, Sascha Weisenberger On Mon, Jan 16, 2017 at 1:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2017-01-16 10:24, Andy Shevchenko wrote: >> On Mon, 2017-01-16 at 10:05 +0100, Jan Kiszka wrote: >>> When using the a device with edge-triggered interrupts, such as MSIs, >>> the interrupt handler has to ensure that there is a point in time >>> during >>> its execution where all interrupts sources are silent so that a new >>> event can trigger a new interrupt again. >>> >>> This is achieved here by looping over SSSR evaluation. We need to take >>> into account that SSCR1 may be changed by the transfer handler, thus >>> we >>> need to redo the mask calculation, at least regarding the volatile >>> interrupt enable bit (TIE). >> >> Could you split this to two patches, one just move the code under >> question to a helper function (no functional change), the other does >> what you state in commit message here? > > IMHO, factoring out some helper called from the loop in ssp_int won't be > a natural split due to the large number of local variables being shared > here. But maybe I'm not seeing the design you have in mind, so please > propose a useful helper function signature. At least everything starting from if (!...) {} can be a helper with only one parameter. Something like: static int handle_bad_msg(struct driver_data *drv_data) { if (...) return 0; ...handle it... return 1; } Let's start from above. P.S. Btw, you totally missed SPI list/maintainers. And you are using wrong Jarkko's address. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts 2017-01-16 17:53 ` Andy Shevchenko @ 2017-01-16 18:19 ` Jan Kiszka 0 siblings, 0 replies; 9+ messages in thread From: Jan Kiszka @ 2017-01-16 18:19 UTC (permalink / raw) To: Andy Shevchenko Cc: Andy Shevchenko, linux-arm Mailing List, Daniel Mack, Haojian Zhuang, Robert Jarzmik, linux-kernel, Mika Westerberg, Jarkko Nikula, Sascha Weisenberger On 2017-01-16 18:53, Andy Shevchenko wrote: > On Mon, Jan 16, 2017 at 1:18 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> On 2017-01-16 10:24, Andy Shevchenko wrote: >>> On Mon, 2017-01-16 at 10:05 +0100, Jan Kiszka wrote: >>>> When using the a device with edge-triggered interrupts, such as MSIs, >>>> the interrupt handler has to ensure that there is a point in time >>>> during >>>> its execution where all interrupts sources are silent so that a new >>>> event can trigger a new interrupt again. >>>> >>>> This is achieved here by looping over SSSR evaluation. We need to take >>>> into account that SSCR1 may be changed by the transfer handler, thus >>>> we >>>> need to redo the mask calculation, at least regarding the volatile >>>> interrupt enable bit (TIE). >>> >>> Could you split this to two patches, one just move the code under >>> question to a helper function (no functional change), the other does >>> what you state in commit message here? >> >> IMHO, factoring out some helper called from the loop in ssp_int won't be >> a natural split due to the large number of local variables being shared >> here. But maybe I'm not seeing the design you have in mind, so please >> propose a useful helper function signature. > > At least everything starting from if (!...) {} can be a helper with > only one parameter. Something like: > > static int handle_bad_msg(struct driver_data *drv_data) > { > if (...) > return 0; > > ...handle it... > return 1; > } > > Let's start from above. OK, but I'll factor out only the handling block, ie. after the if. That's more consistent. > > P.S. Btw, you totally missed SPI list/maintainers. And you are using > wrong Jarkko's address. Data-mined this from the list, both typical target group as well as Jarkko's address that he tends to use. Will adjust. Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] spi: pca2xx-pci: Allow MSI 2017-01-16 9:05 [PATCH 0/2] spi: pca2xx: Prepare for and enable MSI support Jan Kiszka 2017-01-16 9:05 ` [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts Jan Kiszka @ 2017-01-16 9:06 ` Jan Kiszka 2017-01-16 9:29 ` Andy Shevchenko 1 sibling, 1 reply; 9+ messages in thread From: Jan Kiszka @ 2017-01-16 9:06 UTC (permalink / raw) To: linux-arm-kernel Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, linux-kernel, Andy Shevchenko, Mika Westerberg, Jarkko Nikula, Sascha Weisenberger 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(-) 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[] = { -- 2.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] spi: pca2xx-pci: Allow MSI 2017-01-16 9:06 ` [PATCH 2/2] spi: pca2xx-pci: Allow MSI Jan Kiszka @ 2017-01-16 9:29 ` Andy Shevchenko 2017-01-16 17:33 ` Jan Kiszka 0 siblings, 1 reply; 9+ messages in thread From: Andy Shevchenko @ 2017-01-16 9:29 UTC (permalink / raw) To: Jan Kiszka, linux-arm-kernel Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, linux-kernel, Mika Westerberg, Jarkko Nikula, Sascha Weisenberger 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(). -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] spi: pca2xx-pci: Allow MSI 2017-01-16 9:29 ` Andy Shevchenko @ 2017-01-16 17:33 ` Jan Kiszka 0 siblings, 0 replies; 9+ messages in thread From: Jan Kiszka @ 2017-01-16 17:33 UTC (permalink / raw) To: Andy Shevchenko, linux-arm-kernel Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, linux-kernel, Mika Westerberg, Jarkko Nikula, Sascha Weisenberger 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 -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-01-16 18:20 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-16 9:05 [PATCH 0/2] spi: pca2xx: Prepare for and enable MSI support Jan Kiszka 2017-01-16 9:05 ` [PATCH 1/2] spi: pxa2xx: Prepare for edge-triggered interrupts Jan Kiszka 2017-01-16 9:24 ` Andy Shevchenko 2017-01-16 11:18 ` Jan Kiszka 2017-01-16 17:53 ` Andy Shevchenko 2017-01-16 18:19 ` Jan Kiszka 2017-01-16 9:06 ` [PATCH 2/2] spi: pca2xx-pci: Allow MSI Jan Kiszka 2017-01-16 9:29 ` Andy Shevchenko 2017-01-16 17:33 ` Jan Kiszka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).