[PATCH 0/2] Designware MSI fixes

[Lucas Stach]

Those two patches were part of my designware multi-msi series. I decided
to postpone that one a little bit until the treewide MSI chip patches from
Yijing Wang have landed, but those two patches are just too obvious
fixes/style corrections to let them wait any longer.

Both patches were tested by Jingoo Han as part of the earlier series, but
I had to modify them a bit for the rebase, so any review/testing much
appreciated. Patches are on top of the pci/host-designware branch.

Lucas Stach (2):
PCI: designware: remove bogus multiple MSI setup
PCI: designware: remove open-coded bitmap operations

drivers/pci/host/pcie-designware.c | 98 ++++++--------------------------------
1 file changed, 14 insertions(+), 84 deletions(-)

[Lucas Stach]

The setup_irq function is supposed to set up exactly one MSI
irq. Multiple IRQ setup is handled differently, to respect
the choices made by the upper layers.

Also only clear one MSI irq at a time, the PCI core will
call into this function multiple times if it has to tear
down more than one MSI irq.

Signed-off-by: Lucas Stach <***@pengutronix.de>
---
drivers/pci/host/pcie-designware.c | 47 ++++++--------------------------------
1 file changed, 7 insertions(+), 40 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 34e736601259..339296cbc823 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -313,54 +313,17 @@ no_valid_irq:
return -ENOSPC;
}

-static void clear_irq(unsigned int irq)
-{
- unsigned int pos, nvec;
- struct msi_desc *msi;
- struct pcie_port *pp;
- struct irq_data *data = irq_get_irq_data(irq);
-
- /* get the port structure */
- msi = irq_data_get_msi(data);
- pp = sys_to_pcie(msi->dev->bus->sysdata);
-
- /* undo what was done in assign_irq */
- pos = data->hwirq;
- nvec = 1 << msi->msi_attrib.multiple;
-
- clear_irq_range(pp, irq, nvec, pos);
-
- /* all irqs cleared; reset attributes */
- msi->irq = 0;
- msi->msi_attrib.multiple = 0;
-}
-
static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,
struct msi_desc *desc)
{
- int irq, pos, msgvec;
- u16 msg_ctr;
+ int irq, pos;
struct msi_msg msg;
struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);

- pci_read_config_word(pdev, desc->msi_attrib.pos+PCI_MSI_FLAGS,
- &msg_ctr);
- msgvec = (msg_ctr&PCI_MSI_FLAGS_QSIZE) >> 4;
- if (msgvec == 0)
- msgvec = (msg_ctr & PCI_MSI_FLAGS_QMASK) >> 1;
- if (msgvec > 5)
- msgvec = 0;
-
- irq = assign_irq((1 << msgvec), desc, &pos);
+ irq = assign_irq(1, desc, &pos);
if (irq < 0)
return irq;

- /*
- * write_msi_msg() will update PCI_MSI_FLAGS so there is
- * no need to explicitly call pci_write_config_word().
- */
- desc->msi_attrib.multiple = msgvec;
-
if (pp->ops->get_msi_addr)
msg.address_lo = pp->ops->get_msi_addr(pp);
else
@@ -379,7 +342,11 @@ static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,

static void dw_msi_teardown_irq(struct msi_chip *chip, unsigned int irq)
{
- clear_irq(irq);
+ struct irq_data *data = irq_get_irq_data(irq);
+ struct msi_desc *msi = irq_data_get_msi(data);
+ struct pcie_port *pp = sys_to_pcie(msi->dev->bus->sysdata);
+
+ clear_irq_range(pp, irq, 1, data->hwirq);
}

static struct msi_chip dw_pcie_msi_chip = {

[Lucas Stach]

Replace them by using the standard kernel bitmap ops.
No functional change, but makes the code a lot cleaner.

Signed-off-by: Lucas Stach <***@pengutronix.de>
---
drivers/pci/host/pcie-designware.c | 51 ++++++--------------------------------
1 file changed, 7 insertions(+), 44 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 339296cbc823..dfed00aa3ac0 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -196,30 +196,6 @@ void dw_pcie_msi_init(struct pcie_port *pp)
dw_pcie_wr_own_conf(pp, PCIE_MSI_ADDR_HI, 4, 0);
}

-static int find_valid_pos0(struct pcie_port *pp, int msgvec, int pos, int *pos0)
-{
- int flag = 1;
-
- do {
- pos = find_next_zero_bit(pp->msi_irq_in_use,
- MAX_MSI_IRQS, pos);
- /*if you have reached to the end then get out from here.*/
- if (pos == MAX_MSI_IRQS)
- return -ENOSPC;
- /*
- * Check if this position is at correct offset.nvec is always a
- * power of two. pos0 must be nvec bit aligned.
- */
- if (pos % msgvec)
- pos += msgvec - (pos % msgvec);
- else
- flag = 0;
- } while (flag);
-
- *pos0 = pos;
- return 0;
-}
-
static void dw_pcie_msi_clear_irq(struct pcie_port *pp, int irq)
{
unsigned int res, bit, val;
@@ -238,13 +214,14 @@ static void clear_irq_range(struct pcie_port *pp, unsigned int irq_base,

for (i = 0; i < nvec; i++) {
irq_set_msi_desc_off(irq_base, i, NULL);
- clear_bit(pos + i, pp->msi_irq_in_use);
/* Disable corresponding interrupt on MSI controller */
if (pp->ops->msi_clear_irq)
pp->ops->msi_clear_irq(pp, pos + i);
else
dw_pcie_msi_clear_irq(pp, pos + i);
}
+
+ bitmap_release_region(pp->msi_irq_in_use, pos, order_base_2(nvec));
}

static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
@@ -260,26 +237,13 @@ static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)

static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
{
- int irq, pos0, pos1, i;
+ int irq, pos0, i;
struct pcie_port *pp = sys_to_pcie(desc->dev->bus->sysdata);

- pos0 = find_first_zero_bit(pp->msi_irq_in_use,
- MAX_MSI_IRQS);
- if (pos0 % no_irqs) {
- if (find_valid_pos0(pp, no_irqs, pos0, &pos0))
- goto no_valid_irq;
- }
- if (no_irqs > 1) {
- pos1 = find_next_bit(pp->msi_irq_in_use,
- MAX_MSI_IRQS, pos0);
- /* there must be nvec number of consecutive free bits */
- while ((pos1 - pos0) < no_irqs) {
- if (find_valid_pos0(pp, no_irqs, pos1, &pos0))
- goto no_valid_irq;
- pos1 = find_next_bit(pp->msi_irq_in_use,
- MAX_MSI_IRQS, pos0);
- }
- }
+ pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS,
+ order_base_2(no_irqs));
+ if (pos0 < 0)
+ goto no_valid_irq;

irq = irq_find_mapping(pp->irq_domain, pos0);
if (!irq)
@@ -297,7 +261,6 @@ static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
clear_irq_range(pp, irq, i, pos0);
goto no_valid_irq;
}
- set_bit(pos0 + i, pp->msi_irq_in_use);
/*Enable corresponding interrupt in MSI interrupt controller */
if (pp->ops->msi_set_irq)
pp->ops->msi_set_irq(pp, pos0 + i);

[Pratyush Anand]

Post by Lucas Stach
Those two patches were part of my designware multi-msi series. I decided
to postpone that one a little bit until the treewide MSI chip patches from
Yijing Wang have landed, but those two patches are just too obvious
fixes/style corrections to let them wait any longer.
Both patches were tested by Jingoo Han as part of the earlier series, but
I had to modify them a bit for the rebase, so any review/testing much
appreciated. Patches are on top of the pci/host-designware branch.
PCI: designware: remove bogus multiple MSI setup
PCI: designware: remove open-coded bitmap operations

Looks cleaner to me.

Post by Lucas Stach
drivers/pci/host/pcie-designware.c | 98 ++++++--------------------------------
1 file changed, 14 insertions(+), 84 deletions(-)
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Jingoo Han]

Post by Pratyush Anand

Post by Lucas Stach
Those two patches were part of my designware multi-msi series. I decided
to postpone that one a little bit until the treewide MSI chip patches from
Yijing Wang have landed, but those two patches are just too obvious
fixes/style corrections to let them wait any longer.
Both patches were tested by Jingoo Han as part of the earlier series, but
I had to modify them a bit for the rebase, so any review/testing much
appreciated. Patches are on top of the pci/host-designware branch.
PCI: designware: remove bogus multiple MSI setup
PCI: designware: remove open-coded bitmap operations

Looks cleaner to me.

Acked-by: Jingoo Han <***@samsung.com>

Best regards,
Jingoo Han

Post by Pratyush Anand

Post by Lucas Stach
drivers/pci/host/pcie-designware.c | 98 ++++++--------------------------------
1 file changed, 14 insertions(+), 84 deletions(-)
--
2.1.0

[Bjorn Helgaas]

Post by Lucas Stach
Those two patches were part of my designware multi-msi series. I decided
to postpone that one a little bit until the treewide MSI chip patches from
Yijing Wang have landed, but those two patches are just too obvious
fixes/style corrections to let them wait any longer.
Both patches were tested by Jingoo Han as part of the earlier series, but
I had to modify them a bit for the rebase, so any review/testing much
appreciated. Patches are on top of the pci/host-designware branch.
PCI: designware: remove bogus multiple MSI setup
PCI: designware: remove open-coded bitmap operations
drivers/pci/host/pcie-designware.c | 98 ++++++--------------------------------
1 file changed, 14 insertions(+), 84 deletions(-)

Applied to pci/host-designware for v3.18, with Pratyush's reviewed-by and
Jingoo's ack. Thanks!