* [PATCH] edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-16 9:54 ` Jan Glauber
0 siblings, 0 replies; 10+ messages in thread
From: Jan Glauber @ 2017-05-16 9:54 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-edac, linux-kernel, Sergey Temerkhanov, Jan Glauber
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);
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;
}
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-16 9:54 ` Jan Glauber
0 siblings, 0 replies; 10+ messages in thread
From: Jan Glauber @ 2017-05-16 9:54 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-edac, linux-kernel, Sergey Temerkhanov, Jan Glauber
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);
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;
}
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;
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-17 15:35 ` Sergey Temerkhanov
0 siblings, 0 replies; 10+ messages in thread
From: Sergei Temerkhanov @ 2017-05-17 15:35 UTC (permalink / raw)
To: Jan Glauber; +Cc: Borislav Petkov, linux-edac, linux-kernel
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?
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?
>
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-17 15:35 ` Sergey Temerkhanov
0 siblings, 0 replies; 10+ messages in thread
From: Sergey Temerkhanov @ 2017-05-17 15:35 UTC (permalink / raw)
To: Jan Glauber; +Cc: Borislav Petkov, linux-edac, linux-kernel
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?
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?
>
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-17 17:23 ` Jan Glauber
0 siblings, 0 replies; 10+ messages in thread
From: Jan Glauber @ 2017-05-17 17:23 UTC (permalink / raw)
To: Sergei Temerkhanov; +Cc: Borislav Petkov, linux-edac, linux-kernel
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.
> 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.
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-17 17:23 ` Jan Glauber
0 siblings, 0 replies; 10+ messages in thread
From: Jan Glauber @ 2017-05-17 17:23 UTC (permalink / raw)
To: Sergei Temerkhanov; +Cc: Borislav Petkov, linux-edac, linux-kernel
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.
> 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.
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-17 23:52 ` Sergey Temerkhanov
0 siblings, 0 replies; 10+ messages in thread
From: Sergei Temerkhanov @ 2017-05-17 23:52 UTC (permalink / raw)
To: Jan Glauber; +Cc: Borislav Petkov, linux-edac, linux-kernel
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?
>
>> 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.
This can be achieved with pci_alloc_irq_vectors(pdev, OCX_INTS,
OCX_INTS, PCI_IRQ_MSIX)
>
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-17 23:52 ` Sergey Temerkhanov
0 siblings, 0 replies; 10+ messages in thread
From: Sergey Temerkhanov @ 2017-05-17 23:52 UTC (permalink / raw)
To: Jan Glauber; +Cc: Borislav Petkov, linux-edac, linux-kernel
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?
>
>> 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.
This can be achieved with pci_alloc_irq_vectors(pdev, OCX_INTS,
OCX_INTS, PCI_IRQ_MSIX)
>
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-18 9:15 ` Jan Glauber
0 siblings, 0 replies; 10+ messages in thread
From: Jan Glauber @ 2017-05-18 9:15 UTC (permalink / raw)
To: Sergei Temerkhanov
Cc: Borislav Petkov, linux-edac, linux-kernel, Thomas Gleixner
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* edac: thunderx: Replace pci_alloc_msix_exact
@ 2017-05-18 9:15 ` Jan Glauber
0 siblings, 0 replies; 10+ messages in thread
From: Jan Glauber @ 2017-05-18 9:15 UTC (permalink / raw)
To: Sergei Temerkhanov
Cc: Borislav Petkov, linux-edac, linux-kernel, Thomas Gleixner
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
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-05-18 9:16 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-16 9:54 [PATCH] edac: thunderx: Replace pci_alloc_msix_exact Jan Glauber
2017-05-16 9:54 ` Jan Glauber
2017-05-17 15:35 ` [PATCH] " Sergei Temerkhanov
2017-05-17 15:35 ` Sergey Temerkhanov
2017-05-17 17:23 ` [PATCH] " Jan Glauber
2017-05-17 17:23 ` Jan Glauber
2017-05-17 23:52 ` [PATCH] " Sergei Temerkhanov
2017-05-17 23:52 ` Sergey Temerkhanov
2017-05-18 9:15 ` [PATCH] " Jan Glauber
2017-05-18 9:15 ` Jan Glauber
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.