From: Jan Glauber <jan.glauber@caviumnetworks.com>
To: Sergei Temerkhanov <s.temerkhanov@gmail.com>
Cc: Borislav Petkov <bp@suse.de>,
linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
Thomas Gleixner <tglx@linutronix.de>
Subject: edac: thunderx: Replace pci_alloc_msix_exact
Date: Thu, 18 May 2017 11:15:46 +0200 [thread overview]
Message-ID: <20170518091546.GA25620@hc> (raw)
On Thu, May 18, 2017 at 02:52:54AM +0300, Sergei Temerkhanov wrote:
> On Wed, May 17, 2017 at 8:23 PM, Jan Glauber
> <jan.glauber@caviumnetworks.com> wrote:
> > On Wed, May 17, 2017 at 06:35:05PM +0300, Sergei Temerkhanov wrote:
> >> CIL...
> >>
> >> On Tue, May 16, 2017 at 12:54 PM, Jan Glauber <jglauber@cavium.com> wrote:
> >> > Replace the deprecated pci_alloc_msix_exact() with
> >> > pci_alloc_irq_vectors().
> >> >
> >> > Avoid the container_of usage in the interrupt handler
> >> > by simply passing the required struct as data to the interrupt
> >> > handler.
> >> >
> >> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> >> > ---
> >> > drivers/edac/thunderx_edac.c | 91 ++++++++++++++------------------------------
> >> > 1 file changed, 28 insertions(+), 63 deletions(-)
> >> >
> >> > diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
> >> > index 86d585c..da12804 100644
> >> > --- a/drivers/edac/thunderx_edac.c
> >> > +++ b/drivers/edac/thunderx_edac.c
> >> > @@ -183,7 +183,6 @@ struct lmc_err_ctx {
> >> > struct thunderx_lmc {
> >> > void __iomem *regs;
> >> > struct pci_dev *pdev;
> >> > - struct msix_entry msix_ent;
> >> >
> >> > atomic_t ecc_int;
> >> >
> >> > @@ -738,18 +737,17 @@ static int thunderx_lmc_probe(struct pci_dev *pdev,
> >> > mci->scrub_mode = SCRUB_NONE;
> >> >
> >> > lmc->pdev = pdev;
> >> > - lmc->msix_ent.entry = 0;
> >> >
> >> > lmc->ring_head = 0;
> >> > lmc->ring_tail = 0;
> >> >
> >> > - ret = pci_enable_msix_exact(pdev, &lmc->msix_ent, 1);
> >> > - if (ret) {
> >> > + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
> >> > + if (ret < 0) {
> >> > dev_err(&pdev->dev, "Cannot enable interrupt: %d\n", ret);
> >> > goto err_free;
> >> > }
> >> >
> >> > - ret = devm_request_threaded_irq(&pdev->dev, lmc->msix_ent.vector,
> >> > + ret = devm_request_threaded_irq(&pdev->dev, pci_irq_vector(pdev, 0),
> >> > thunderx_lmc_err_isr,
> >> > thunderx_lmc_threaded_isr, 0,
> >> > "[EDAC] ThunderX LMC", mci);
> >> > @@ -1079,7 +1077,6 @@ struct thunderx_ocx {
> >> > struct edac_device_ctl_info *edac_dev;
> >> >
> >> > struct dentry *debugfs;
> >> > - struct msix_entry msix_ent[OCX_INTS];
> >> >
> >> > struct ocx_com_err_ctx com_err_ctx[RING_ENTRIES];
> >> > struct ocx_link_err_ctx link_err_ctx[RING_ENTRIES];
> >> > @@ -1095,12 +1092,9 @@ struct thunderx_ocx {
> >> > #define OCX_OTHER_SIZE (50 * ARRAY_SIZE(ocx_com_link_errors))
> >> >
> >> > /* This handler is threaded */
> >> > -static irqreturn_t thunderx_ocx_com_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_ocx_com_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_ocx *ocx = container_of(msix, struct thunderx_ocx,
> >> > - msix_ent[msix->entry]);
> >> > -
> >> > + struct thunderx_ocx *ocx = (struct thunderx_ocx *) dev_id;
> >> > int lane;
> >> > unsigned long head = ring_pos(ocx->com_ring_head,
> >> > ARRAY_SIZE(ocx->com_err_ctx));
> >> > @@ -1124,12 +1118,9 @@ static irqreturn_t thunderx_ocx_com_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_ocx_com_threaded_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_ocx_com_threaded_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_ocx *ocx = container_of(msix, struct thunderx_ocx,
> >> > - msix_ent[msix->entry]);
> >> > -
> >> > + struct thunderx_ocx *ocx = (struct thunderx_ocx *) dev_id;
> >> > irqreturn_t ret = IRQ_NONE;
> >> >
> >> > unsigned long tail;
> >> > @@ -1188,16 +1179,14 @@ static irqreturn_t thunderx_ocx_com_threaded_isr(int irq, void *irq_id)
> >> > return ret;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_ocx_lnk_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_ocx_lnk_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_ocx *ocx = container_of(msix, struct thunderx_ocx,
> >> > - msix_ent[msix->entry]);
> >> > + struct thunderx_ocx *ocx = (struct thunderx_ocx *) dev_id;
> >> > unsigned long head = ring_pos(ocx->link_ring_head,
> >> > ARRAY_SIZE(ocx->link_err_ctx));
> >> > struct ocx_link_err_ctx *ctx = &ocx->link_err_ctx[head];
> >> >
> >> > - ctx->link = msix->entry;
> >> > + ctx->link = irq - pci_irq_vector(ocx->pdev, 0);
> >>
> >> This assumes MSIX vectors are allocated sequentially, as far as I can
> >> tell. Is this behavior guaranteed?
> >
> > From looking at the implementation in pci_irq_vector I would say it is.
>
> What if the implementation changes?
Maybe Thomas can comment if this assumption can be made or should be
avoided...
> >
> >> As a precaution, a check for 0 <= ctx->link <= 2 might need to be added here.
> >>
> >> > ctx->reg_com_link_int = readq(ocx->regs + OCX_COM_LINKX_INT(ctx->link));
> >> >
> >> > writeq(ctx->reg_com_link_int, ocx->regs + OCX_COM_LINKX_INT(ctx->link));
> >> > @@ -1207,11 +1196,9 @@ static irqreturn_t thunderx_ocx_lnk_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_ocx_lnk_threaded_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_ocx_lnk_threaded_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_ocx *ocx = container_of(msix, struct thunderx_ocx,
> >> > - msix_ent[msix->entry]);
> >> > + struct thunderx_ocx *ocx = (struct thunderx_ocx *) dev_id;
> >> > irqreturn_t ret = IRQ_NONE;
> >> > unsigned long tail;
> >> > struct ocx_link_err_ctx *ctx;
> >> > @@ -1410,20 +1397,15 @@ static int thunderx_ocx_probe(struct pci_dev *pdev,
> >> >
> >> > ocx->pdev = pdev;
> >> >
> >> > - for (i = 0; i < OCX_INTS; i++) {
> >> > - ocx->msix_ent[i].entry = i;
> >> > - ocx->msix_ent[i].vector = 0;
> >> > - }
> >> > -
> >> > - ret = pci_enable_msix_exact(pdev, ocx->msix_ent, OCX_INTS);
> >> > - if (ret) {
> >> > + ret = pci_alloc_irq_vectors(pdev, 1, OCX_INTS, PCI_IRQ_MSIX);
> >> > + if (ret < 0) {
> >> > dev_err(&pdev->dev, "Cannot enable interrupt: %d\n", ret);
> >> > goto err_free;
> >> > }
> >>
> >> What happens if less than OCX_INTS vectors are allocated?
> >>
> >
> > The same behaviour as before, it is ignored.
>
> No, pci_enable_msix_exact() fails when it cannot enable the specified
> number of interrupts.
I agree, I misread that (maybe that is the reason for the added _exact).
> This can be achieved with pci_alloc_irq_vectors(pdev, OCX_INTS,
> OCX_INTS, PCI_IRQ_MSIX)
Yes. Thanks for spotting this.
> >
> > Regards,
> > Jan
> >
> >> >
> >> > for (i = 0; i < OCX_INTS; i++) {
> >> > ret = devm_request_threaded_irq(&pdev->dev,
> >> > - ocx->msix_ent[i].vector,
> >> > + pci_irq_vector(pdev, i),
> >> > (i == 3) ?
> >> > thunderx_ocx_com_isr :
> >> > thunderx_ocx_lnk_isr,
> >> > @@ -1431,7 +1413,7 @@ static int thunderx_ocx_probe(struct pci_dev *pdev,
> >> > thunderx_ocx_com_threaded_isr :
> >> > thunderx_ocx_lnk_threaded_isr,
> >> > 0, "[EDAC] ThunderX OCX",
> >> > - &ocx->msix_ent[i]);
> >> > + ocx);
> >> > if (ret)
> >> > goto err_free;
> >> > }
> >> > @@ -1773,19 +1755,14 @@ struct thunderx_l2c {
> >> >
> >> > int index;
> >> >
> >> > - struct msix_entry msix_ent;
> >> > -
> >> > struct l2c_err_ctx err_ctx[RING_ENTRIES];
> >> > unsigned long ring_head;
> >> > unsigned long ring_tail;
> >> > };
> >> >
> >> > -static irqreturn_t thunderx_l2c_tad_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_l2c_tad_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_l2c *tad = container_of(msix, struct thunderx_l2c,
> >> > - msix_ent);
> >> > -
> >> > + struct thunderx_l2c *tad = (struct thunderx_l2c *) dev_id;
> >> > unsigned long head = ring_pos(tad->ring_head, ARRAY_SIZE(tad->err_ctx));
> >> > struct l2c_err_ctx *ctx = &tad->err_ctx[head];
> >> >
> >> > @@ -1812,12 +1789,9 @@ static irqreturn_t thunderx_l2c_tad_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_l2c_cbc_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_l2c_cbc_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_l2c *cbc = container_of(msix, struct thunderx_l2c,
> >> > - msix_ent);
> >> > -
> >> > + struct thunderx_l2c *cbc = (struct thunderx_l2c *) dev_id;
> >> > unsigned long head = ring_pos(cbc->ring_head, ARRAY_SIZE(cbc->err_ctx));
> >> > struct l2c_err_ctx *ctx = &cbc->err_ctx[head];
> >> >
> >> > @@ -1841,12 +1815,9 @@ static irqreturn_t thunderx_l2c_cbc_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_l2c_mci_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_l2c_mci_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_l2c *mci = container_of(msix, struct thunderx_l2c,
> >> > - msix_ent);
> >> > -
> >> > + struct thunderx_l2c *mci = (struct thunderx_l2c *) dev_id;
> >> > unsigned long head = ring_pos(mci->ring_head, ARRAY_SIZE(mci->err_ctx));
> >> > struct l2c_err_ctx *ctx = &mci->err_ctx[head];
> >> >
> >> > @@ -1862,12 +1833,9 @@ static irqreturn_t thunderx_l2c_mci_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_l2c_threaded_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_l2c_threaded_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_l2c *l2c = container_of(msix, struct thunderx_l2c,
> >> > - msix_ent);
> >> > -
> >> > + struct thunderx_l2c *l2c = (struct thunderx_l2c *) dev_id;
> >> > unsigned long tail = ring_pos(l2c->ring_tail, ARRAY_SIZE(l2c->err_ctx));
> >> > struct l2c_err_ctx *ctx = &l2c->err_ctx[tail];
> >> > irqreturn_t ret = IRQ_NONE;
> >> > @@ -2049,20 +2017,17 @@ static int thunderx_l2c_probe(struct pci_dev *pdev,
> >> > l2c->ring_head = 0;
> >> > l2c->ring_tail = 0;
> >> >
> >> > - l2c->msix_ent.entry = 0;
> >> > - l2c->msix_ent.vector = 0;
> >> > -
> >> > - ret = pci_enable_msix_exact(pdev, &l2c->msix_ent, 1);
> >> > - if (ret) {
> >> > + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
> >> > + if (ret < 0) {
> >> > dev_err(&pdev->dev, "Cannot enable interrupt: %d\n", ret);
> >> > goto err_free;
> >> > }
> >> >
> >> > - ret = devm_request_threaded_irq(&pdev->dev, l2c->msix_ent.vector,
> >> > + ret = devm_request_threaded_irq(&pdev->dev, pci_irq_vector(pdev, 0),
> >> > thunderx_l2c_isr,
> >> > thunderx_l2c_threaded_isr,
> >> > 0, "[EDAC] ThunderX L2C",
> >> > - &l2c->msix_ent);
> >> > + l2c);
> >> > if (ret)
> >> > goto err_free;
> >> >
> >> > --
> >> > 1.9.1
> >> >
> >>
> >> Regards,
> >> Sergey
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Jan Glauber <jan.glauber@caviumnetworks.com>
To: Sergei Temerkhanov <s.temerkhanov@gmail.com>
Cc: Borislav Petkov <bp@suse.de>,
linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH] edac: thunderx: Replace pci_alloc_msix_exact
Date: Thu, 18 May 2017 11:15:46 +0200 [thread overview]
Message-ID: <20170518091546.GA25620@hc> (raw)
In-Reply-To: <CAPEA6dbT=yAaW7097_spBd919L12EjdY0kWkaW6Ajd8J9e1vhw@mail.gmail.com>
On Thu, May 18, 2017 at 02:52:54AM +0300, Sergei Temerkhanov wrote:
> On Wed, May 17, 2017 at 8:23 PM, Jan Glauber
> <jan.glauber@caviumnetworks.com> wrote:
> > On Wed, May 17, 2017 at 06:35:05PM +0300, Sergei Temerkhanov wrote:
> >> CIL...
> >>
> >> On Tue, May 16, 2017 at 12:54 PM, Jan Glauber <jglauber@cavium.com> wrote:
> >> > Replace the deprecated pci_alloc_msix_exact() with
> >> > pci_alloc_irq_vectors().
> >> >
> >> > Avoid the container_of usage in the interrupt handler
> >> > by simply passing the required struct as data to the interrupt
> >> > handler.
> >> >
> >> > Signed-off-by: Jan Glauber <jglauber@cavium.com>
> >> > ---
> >> > drivers/edac/thunderx_edac.c | 91 ++++++++++++++------------------------------
> >> > 1 file changed, 28 insertions(+), 63 deletions(-)
> >> >
> >> > diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
> >> > index 86d585c..da12804 100644
> >> > --- a/drivers/edac/thunderx_edac.c
> >> > +++ b/drivers/edac/thunderx_edac.c
> >> > @@ -183,7 +183,6 @@ struct lmc_err_ctx {
> >> > struct thunderx_lmc {
> >> > void __iomem *regs;
> >> > struct pci_dev *pdev;
> >> > - struct msix_entry msix_ent;
> >> >
> >> > atomic_t ecc_int;
> >> >
> >> > @@ -738,18 +737,17 @@ static int thunderx_lmc_probe(struct pci_dev *pdev,
> >> > mci->scrub_mode = SCRUB_NONE;
> >> >
> >> > lmc->pdev = pdev;
> >> > - lmc->msix_ent.entry = 0;
> >> >
> >> > lmc->ring_head = 0;
> >> > lmc->ring_tail = 0;
> >> >
> >> > - ret = pci_enable_msix_exact(pdev, &lmc->msix_ent, 1);
> >> > - if (ret) {
> >> > + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
> >> > + if (ret < 0) {
> >> > dev_err(&pdev->dev, "Cannot enable interrupt: %d\n", ret);
> >> > goto err_free;
> >> > }
> >> >
> >> > - ret = devm_request_threaded_irq(&pdev->dev, lmc->msix_ent.vector,
> >> > + ret = devm_request_threaded_irq(&pdev->dev, pci_irq_vector(pdev, 0),
> >> > thunderx_lmc_err_isr,
> >> > thunderx_lmc_threaded_isr, 0,
> >> > "[EDAC] ThunderX LMC", mci);
> >> > @@ -1079,7 +1077,6 @@ struct thunderx_ocx {
> >> > struct edac_device_ctl_info *edac_dev;
> >> >
> >> > struct dentry *debugfs;
> >> > - struct msix_entry msix_ent[OCX_INTS];
> >> >
> >> > struct ocx_com_err_ctx com_err_ctx[RING_ENTRIES];
> >> > struct ocx_link_err_ctx link_err_ctx[RING_ENTRIES];
> >> > @@ -1095,12 +1092,9 @@ struct thunderx_ocx {
> >> > #define OCX_OTHER_SIZE (50 * ARRAY_SIZE(ocx_com_link_errors))
> >> >
> >> > /* This handler is threaded */
> >> > -static irqreturn_t thunderx_ocx_com_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_ocx_com_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_ocx *ocx = container_of(msix, struct thunderx_ocx,
> >> > - msix_ent[msix->entry]);
> >> > -
> >> > + struct thunderx_ocx *ocx = (struct thunderx_ocx *) dev_id;
> >> > int lane;
> >> > unsigned long head = ring_pos(ocx->com_ring_head,
> >> > ARRAY_SIZE(ocx->com_err_ctx));
> >> > @@ -1124,12 +1118,9 @@ static irqreturn_t thunderx_ocx_com_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_ocx_com_threaded_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_ocx_com_threaded_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_ocx *ocx = container_of(msix, struct thunderx_ocx,
> >> > - msix_ent[msix->entry]);
> >> > -
> >> > + struct thunderx_ocx *ocx = (struct thunderx_ocx *) dev_id;
> >> > irqreturn_t ret = IRQ_NONE;
> >> >
> >> > unsigned long tail;
> >> > @@ -1188,16 +1179,14 @@ static irqreturn_t thunderx_ocx_com_threaded_isr(int irq, void *irq_id)
> >> > return ret;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_ocx_lnk_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_ocx_lnk_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_ocx *ocx = container_of(msix, struct thunderx_ocx,
> >> > - msix_ent[msix->entry]);
> >> > + struct thunderx_ocx *ocx = (struct thunderx_ocx *) dev_id;
> >> > unsigned long head = ring_pos(ocx->link_ring_head,
> >> > ARRAY_SIZE(ocx->link_err_ctx));
> >> > struct ocx_link_err_ctx *ctx = &ocx->link_err_ctx[head];
> >> >
> >> > - ctx->link = msix->entry;
> >> > + ctx->link = irq - pci_irq_vector(ocx->pdev, 0);
> >>
> >> This assumes MSIX vectors are allocated sequentially, as far as I can
> >> tell. Is this behavior guaranteed?
> >
> > From looking at the implementation in pci_irq_vector I would say it is.
>
> What if the implementation changes?
Maybe Thomas can comment if this assumption can be made or should be
avoided...
> >
> >> As a precaution, a check for 0 <= ctx->link <= 2 might need to be added here.
> >>
> >> > ctx->reg_com_link_int = readq(ocx->regs + OCX_COM_LINKX_INT(ctx->link));
> >> >
> >> > writeq(ctx->reg_com_link_int, ocx->regs + OCX_COM_LINKX_INT(ctx->link));
> >> > @@ -1207,11 +1196,9 @@ static irqreturn_t thunderx_ocx_lnk_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_ocx_lnk_threaded_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_ocx_lnk_threaded_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_ocx *ocx = container_of(msix, struct thunderx_ocx,
> >> > - msix_ent[msix->entry]);
> >> > + struct thunderx_ocx *ocx = (struct thunderx_ocx *) dev_id;
> >> > irqreturn_t ret = IRQ_NONE;
> >> > unsigned long tail;
> >> > struct ocx_link_err_ctx *ctx;
> >> > @@ -1410,20 +1397,15 @@ static int thunderx_ocx_probe(struct pci_dev *pdev,
> >> >
> >> > ocx->pdev = pdev;
> >> >
> >> > - for (i = 0; i < OCX_INTS; i++) {
> >> > - ocx->msix_ent[i].entry = i;
> >> > - ocx->msix_ent[i].vector = 0;
> >> > - }
> >> > -
> >> > - ret = pci_enable_msix_exact(pdev, ocx->msix_ent, OCX_INTS);
> >> > - if (ret) {
> >> > + ret = pci_alloc_irq_vectors(pdev, 1, OCX_INTS, PCI_IRQ_MSIX);
> >> > + if (ret < 0) {
> >> > dev_err(&pdev->dev, "Cannot enable interrupt: %d\n", ret);
> >> > goto err_free;
> >> > }
> >>
> >> What happens if less than OCX_INTS vectors are allocated?
> >>
> >
> > The same behaviour as before, it is ignored.
>
> No, pci_enable_msix_exact() fails when it cannot enable the specified
> number of interrupts.
I agree, I misread that (maybe that is the reason for the added _exact).
> This can be achieved with pci_alloc_irq_vectors(pdev, OCX_INTS,
> OCX_INTS, PCI_IRQ_MSIX)
Yes. Thanks for spotting this.
> >
> > Regards,
> > Jan
> >
> >> >
> >> > for (i = 0; i < OCX_INTS; i++) {
> >> > ret = devm_request_threaded_irq(&pdev->dev,
> >> > - ocx->msix_ent[i].vector,
> >> > + pci_irq_vector(pdev, i),
> >> > (i == 3) ?
> >> > thunderx_ocx_com_isr :
> >> > thunderx_ocx_lnk_isr,
> >> > @@ -1431,7 +1413,7 @@ static int thunderx_ocx_probe(struct pci_dev *pdev,
> >> > thunderx_ocx_com_threaded_isr :
> >> > thunderx_ocx_lnk_threaded_isr,
> >> > 0, "[EDAC] ThunderX OCX",
> >> > - &ocx->msix_ent[i]);
> >> > + ocx);
> >> > if (ret)
> >> > goto err_free;
> >> > }
> >> > @@ -1773,19 +1755,14 @@ struct thunderx_l2c {
> >> >
> >> > int index;
> >> >
> >> > - struct msix_entry msix_ent;
> >> > -
> >> > struct l2c_err_ctx err_ctx[RING_ENTRIES];
> >> > unsigned long ring_head;
> >> > unsigned long ring_tail;
> >> > };
> >> >
> >> > -static irqreturn_t thunderx_l2c_tad_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_l2c_tad_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_l2c *tad = container_of(msix, struct thunderx_l2c,
> >> > - msix_ent);
> >> > -
> >> > + struct thunderx_l2c *tad = (struct thunderx_l2c *) dev_id;
> >> > unsigned long head = ring_pos(tad->ring_head, ARRAY_SIZE(tad->err_ctx));
> >> > struct l2c_err_ctx *ctx = &tad->err_ctx[head];
> >> >
> >> > @@ -1812,12 +1789,9 @@ static irqreturn_t thunderx_l2c_tad_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_l2c_cbc_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_l2c_cbc_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_l2c *cbc = container_of(msix, struct thunderx_l2c,
> >> > - msix_ent);
> >> > -
> >> > + struct thunderx_l2c *cbc = (struct thunderx_l2c *) dev_id;
> >> > unsigned long head = ring_pos(cbc->ring_head, ARRAY_SIZE(cbc->err_ctx));
> >> > struct l2c_err_ctx *ctx = &cbc->err_ctx[head];
> >> >
> >> > @@ -1841,12 +1815,9 @@ static irqreturn_t thunderx_l2c_cbc_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_l2c_mci_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_l2c_mci_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_l2c *mci = container_of(msix, struct thunderx_l2c,
> >> > - msix_ent);
> >> > -
> >> > + struct thunderx_l2c *mci = (struct thunderx_l2c *) dev_id;
> >> > unsigned long head = ring_pos(mci->ring_head, ARRAY_SIZE(mci->err_ctx));
> >> > struct l2c_err_ctx *ctx = &mci->err_ctx[head];
> >> >
> >> > @@ -1862,12 +1833,9 @@ static irqreturn_t thunderx_l2c_mci_isr(int irq, void *irq_id)
> >> > return IRQ_WAKE_THREAD;
> >> > }
> >> >
> >> > -static irqreturn_t thunderx_l2c_threaded_isr(int irq, void *irq_id)
> >> > +static irqreturn_t thunderx_l2c_threaded_isr(int irq, void *dev_id)
> >> > {
> >> > - struct msix_entry *msix = irq_id;
> >> > - struct thunderx_l2c *l2c = container_of(msix, struct thunderx_l2c,
> >> > - msix_ent);
> >> > -
> >> > + struct thunderx_l2c *l2c = (struct thunderx_l2c *) dev_id;
> >> > unsigned long tail = ring_pos(l2c->ring_tail, ARRAY_SIZE(l2c->err_ctx));
> >> > struct l2c_err_ctx *ctx = &l2c->err_ctx[tail];
> >> > irqreturn_t ret = IRQ_NONE;
> >> > @@ -2049,20 +2017,17 @@ static int thunderx_l2c_probe(struct pci_dev *pdev,
> >> > l2c->ring_head = 0;
> >> > l2c->ring_tail = 0;
> >> >
> >> > - l2c->msix_ent.entry = 0;
> >> > - l2c->msix_ent.vector = 0;
> >> > -
> >> > - ret = pci_enable_msix_exact(pdev, &l2c->msix_ent, 1);
> >> > - if (ret) {
> >> > + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSIX);
> >> > + if (ret < 0) {
> >> > dev_err(&pdev->dev, "Cannot enable interrupt: %d\n", ret);
> >> > goto err_free;
> >> > }
> >> >
> >> > - ret = devm_request_threaded_irq(&pdev->dev, l2c->msix_ent.vector,
> >> > + ret = devm_request_threaded_irq(&pdev->dev, pci_irq_vector(pdev, 0),
> >> > thunderx_l2c_isr,
> >> > thunderx_l2c_threaded_isr,
> >> > 0, "[EDAC] ThunderX L2C",
> >> > - &l2c->msix_ent);
> >> > + l2c);
> >> > if (ret)
> >> > goto err_free;
> >> >
> >> > --
> >> > 1.9.1
> >> >
> >>
> >> Regards,
> >> Sergey
next reply other threads:[~2017-05-18 9:15 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-18 9:15 Jan Glauber [this message]
2017-05-18 9:15 ` [PATCH] edac: thunderx: Replace pci_alloc_msix_exact Jan Glauber
-- strict thread matches above, loose matches on Subject: below --
2017-05-17 23:52 Sergey Temerkhanov
2017-05-17 23:52 ` [PATCH] " Sergei Temerkhanov
2017-05-17 17:23 Jan Glauber
2017-05-17 17:23 ` [PATCH] " Jan Glauber
2017-05-17 15:35 Sergey Temerkhanov
2017-05-17 15:35 ` [PATCH] " Sergei Temerkhanov
2017-05-16 9:54 Jan Glauber
2017-05-16 9:54 ` [PATCH] " Jan Glauber
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170518091546.GA25620@hc \
--to=jan.glauber@caviumnetworks.com \
--cc=bp@suse.de \
--cc=linux-edac@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=s.temerkhanov@gmail.com \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.