This patch set fixes the irq_mask/irq_unmask functions and enables the
PLX PEX8311 local interrupts.
With the current version of the driver, gpiomon (from libgpiod) or a
poll(2) on the sysfs value are not working. According to
/proc/interrupts, there is no interruption triggered.
The main issue is that the local interrupts are not forwarded by the
PEX8311 to the PCI.
There is also two bugs:
- The IRQ mask is not correctly updated when unmasking an
interruption.
- The COS Enable Register value is not correctly updated when
masking/unmasking an interruption.
It seems this problems exist since the initial commit.
Arnaud de Turckheim (3):
gpio: pcie-idio-24: Fix irq mask when masking
gpio: pcie-idio-24: Fix IRQ Enable Register value
gpio: pcie-idio-24: Enable PEX8311 interrupts
drivers/gpio/gpio-pcie-idio-24.c | 62 ++++++++++++++++++++++++++++----
1 file changed, 56 insertions(+), 6 deletions(-)
--
2.25.1
This enables the PEX8311 internal PCI wire interrupt and the PEX8311
local interrupt input so the local interrupts are forwarded to the PCI.
Fixes: 585562046628 ("gpio: Add GPIO support for the ACCES PCIe-IDIO-24 family")
Signed-off-by: Arnaud de Turckheim <[email protected]>
---
drivers/gpio/gpio-pcie-idio-24.c | 52 +++++++++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pcie-idio-24.c b/drivers/gpio/gpio-pcie-idio-24.c
index a61de14d09b6..2a07fd96707e 100644
--- a/drivers/gpio/gpio-pcie-idio-24.c
+++ b/drivers/gpio/gpio-pcie-idio-24.c
@@ -28,6 +28,47 @@
#include <linux/spinlock.h>
#include <linux/types.h>
+/*
+ * PLX PEX8311 PCI LCS_INTCSR Interrupt Control/Status
+ *
+ * Bit: Description
+ * 0: Enable Interrupt Sources (Bit 0)
+ * 1: Enable Interrupt Sources (Bit 1)
+ * 2: Generate Internal PCI Bus Internal SERR# Interrupt
+ * 3: Mailbox Interrupt Enable
+ * 4: Power Management Interrupt Enable
+ * 5: Power Management Interrupt
+ * 6: Slave Read Local Data Parity Check Error Enable
+ * 7: Slave Read Local Data Parity Check Error Status
+ * 8: Internal PCI Wire Interrupt Enable
+ * 9: PCI Express Doorbell Interrupt Enable
+ * 10: PCI Abort Interrupt Enable
+ * 11: Local Interrupt Input Enable
+ * 12: Retry Abort Enable
+ * 13: PCI Express Doorbell Interrupt Active
+ * 14: PCI Abort Interrupt Active
+ * 15: Local Interrupt Input Active
+ * 16: Local Interrupt Output Enable
+ * 17: Local Doorbell Interrupt Enable
+ * 18: DMA Channel 0 Interrupt Enable
+ * 19: DMA Channel 1 Interrupt Enable
+ * 20: Local Doorbell Interrupt Active
+ * 21: DMA Channel 0 Interrupt Active
+ * 22: DMA Channel 1 Interrupt Active
+ * 23: Built-In Self-Test (BIST) Interrupt Active
+ * 24: Direct Master was the Bus Master during a Master or Target Abort
+ * 25: DMA Channel 0 was the Bus Master during a Master or Target Abort
+ * 26: DMA Channel 1 was the Bus Master during a Master or Target Abort
+ * 27: Target Abort after internal 256 consecutive Master Retrys
+ * 28: PCI Bus wrote data to LCS_MBOX0
+ * 29: PCI Bus wrote data to LCS_MBOX1
+ * 30: PCI Bus wrote data to LCS_MBOX2
+ * 31: PCI Bus wrote data to LCS_MBOX3
+ */
+#define PLX_PEX8311_PCI_LCS_INTCSR 0x68
+#define INTCSR_INTERNAL_PCI_WIRE BIT(8)
+#define INTCSR_LOCAL_INPUT BIT(11)
+
/**
* struct idio_24_gpio_reg - GPIO device registers structure
* @out0_7: Read: FET Outputs 0-7
@@ -92,6 +133,7 @@ struct idio_24_gpio_reg {
struct idio_24_gpio {
struct gpio_chip chip;
raw_spinlock_t lock;
+ __u8 __iomem *plx;
struct idio_24_gpio_reg __iomem *reg;
unsigned long irq_mask;
};
@@ -455,6 +497,7 @@ static int idio_24_probe(struct pci_dev *pdev, const struct pci_device_id *id)
struct device *const dev = &pdev->dev;
struct idio_24_gpio *idio24gpio;
int err;
+ const size_t pci_plx_bar_index = 1;
const size_t pci_bar_index = 2;
const char *const name = pci_name(pdev);
struct gpio_irq_chip *girq;
@@ -469,12 +512,13 @@ static int idio_24_probe(struct pci_dev *pdev, const struct pci_device_id *id)
return err;
}
- err = pcim_iomap_regions(pdev, BIT(pci_bar_index), name);
+ err = pcim_iomap_regions(pdev, BIT(pci_plx_bar_index) | BIT(pci_bar_index), name);
if (err) {
dev_err(dev, "Unable to map PCI I/O addresses (%d)\n", err);
return err;
}
+ idio24gpio->plx = pcim_iomap_table(pdev)[pci_plx_bar_index];
idio24gpio->reg = pcim_iomap_table(pdev)[pci_bar_index];
idio24gpio->chip.label = name;
@@ -504,6 +548,12 @@ static int idio_24_probe(struct pci_dev *pdev, const struct pci_device_id *id)
/* Software board reset */
iowrite8(0, &idio24gpio->reg->soft_reset);
+ /*
+ * enable PLX PEX8311 internal PCI wire interrupt and local interrupt
+ * input
+ */
+ iowrite8((INTCSR_INTERNAL_PCI_WIRE | INTCSR_LOCAL_INPUT) >> 8,
+ idio24gpio->plx + PLX_PEX8311_PCI_LCS_INTCSR + 1);
err = devm_gpiochip_add_data(dev, &idio24gpio->chip, idio24gpio);
if (err) {
--
2.25.1
This fixes the COS Enable Register value for enabling/disabling the
corresponding IRQs bank.
Fixes: 585562046628 ("gpio: Add GPIO support for the ACCES PCIe-IDIO-24 family")
Signed-off-by: Arnaud de Turckheim <[email protected]>
---
drivers/gpio/gpio-pcie-idio-24.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-pcie-idio-24.c b/drivers/gpio/gpio-pcie-idio-24.c
index 5ea517416366..a61de14d09b6 100644
--- a/drivers/gpio/gpio-pcie-idio-24.c
+++ b/drivers/gpio/gpio-pcie-idio-24.c
@@ -334,13 +334,13 @@ static void idio_24_irq_mask(struct irq_data *data)
unsigned long flags;
const unsigned long bit_offset = irqd_to_hwirq(data) - 24;
unsigned char new_irq_mask;
- const unsigned long bank_offset = bit_offset/8 * 8;
+ const unsigned long bank_offset = bit_offset / 8;
unsigned char cos_enable_state;
raw_spin_lock_irqsave(&idio24gpio->lock, flags);
idio24gpio->irq_mask &= ~BIT(bit_offset);
- new_irq_mask = idio24gpio->irq_mask >> bank_offset;
+ new_irq_mask = idio24gpio->irq_mask >> bank_offset * 8;
if (!new_irq_mask) {
cos_enable_state = ioread8(&idio24gpio->reg->cos_enable);
@@ -363,12 +363,12 @@ static void idio_24_irq_unmask(struct irq_data *data)
unsigned long flags;
unsigned char prev_irq_mask;
const unsigned long bit_offset = irqd_to_hwirq(data) - 24;
- const unsigned long bank_offset = bit_offset/8 * 8;
+ const unsigned long bank_offset = bit_offset / 8;
unsigned char cos_enable_state;
raw_spin_lock_irqsave(&idio24gpio->lock, flags);
- prev_irq_mask = idio24gpio->irq_mask >> bank_offset;
+ prev_irq_mask = idio24gpio->irq_mask >> bank_offset * 8;
idio24gpio->irq_mask |= BIT(bit_offset);
if (!prev_irq_mask) {
--
2.25.1
On Wed, Nov 04, 2020 at 04:24:52PM +0100, Arnaud de Turckheim wrote:
> This patch set fixes the irq_mask/irq_unmask functions and enables the
> PLX PEX8311 local interrupts.
>
> With the current version of the driver, gpiomon (from libgpiod) or a
> poll(2) on the sysfs value are not working. According to
> /proc/interrupts, there is no interruption triggered.
> The main issue is that the local interrupts are not forwarded by the
> PEX8311 to the PCI.
>
> There is also two bugs:
> - The IRQ mask is not correctly updated when unmasking an
> interruption.
> - The COS Enable Register value is not correctly updated when
> masking/unmasking an interruption.
>
> It seems this problems exist since the initial commit.
>
> Arnaud de Turckheim (3):
> gpio: pcie-idio-24: Fix irq mask when masking
> gpio: pcie-idio-24: Fix IRQ Enable Register value
> gpio: pcie-idio-24: Enable PEX8311 interrupts
>
> drivers/gpio/gpio-pcie-idio-24.c | 62 ++++++++++++++++++++++++++++----
> 1 file changed, 56 insertions(+), 6 deletions(-)
>
> --
> 2.25.1
>
This patchset looks good to me and fixes several fundamental bugs. I'm
going to reply to each patch individually to give my Reviewed-by tags
for the benefit of those picking up the patches.
Thanks!
William Breathitt Gray
On Wed, Nov 04, 2020 at 04:24:55PM +0100, Arnaud de Turckheim wrote:
> This enables the PEX8311 internal PCI wire interrupt and the PEX8311
> local interrupt input so the local interrupts are forwarded to the PCI.
>
> Fixes: 585562046628 ("gpio: Add GPIO support for the ACCES PCIe-IDIO-24 family")
> Signed-off-by: Arnaud de Turckheim <[email protected]>
Reviewed-by: William Breathitt Gray <[email protected]>
> ---
> drivers/gpio/gpio-pcie-idio-24.c | 52 +++++++++++++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-pcie-idio-24.c b/drivers/gpio/gpio-pcie-idio-24.c
> index a61de14d09b6..2a07fd96707e 100644
> --- a/drivers/gpio/gpio-pcie-idio-24.c
> +++ b/drivers/gpio/gpio-pcie-idio-24.c
> @@ -28,6 +28,47 @@
> #include <linux/spinlock.h>
> #include <linux/types.h>
>
> +/*
> + * PLX PEX8311 PCI LCS_INTCSR Interrupt Control/Status
> + *
> + * Bit: Description
> + * 0: Enable Interrupt Sources (Bit 0)
> + * 1: Enable Interrupt Sources (Bit 1)
> + * 2: Generate Internal PCI Bus Internal SERR# Interrupt
> + * 3: Mailbox Interrupt Enable
> + * 4: Power Management Interrupt Enable
> + * 5: Power Management Interrupt
> + * 6: Slave Read Local Data Parity Check Error Enable
> + * 7: Slave Read Local Data Parity Check Error Status
> + * 8: Internal PCI Wire Interrupt Enable
> + * 9: PCI Express Doorbell Interrupt Enable
> + * 10: PCI Abort Interrupt Enable
> + * 11: Local Interrupt Input Enable
> + * 12: Retry Abort Enable
> + * 13: PCI Express Doorbell Interrupt Active
> + * 14: PCI Abort Interrupt Active
> + * 15: Local Interrupt Input Active
> + * 16: Local Interrupt Output Enable
> + * 17: Local Doorbell Interrupt Enable
> + * 18: DMA Channel 0 Interrupt Enable
> + * 19: DMA Channel 1 Interrupt Enable
> + * 20: Local Doorbell Interrupt Active
> + * 21: DMA Channel 0 Interrupt Active
> + * 22: DMA Channel 1 Interrupt Active
> + * 23: Built-In Self-Test (BIST) Interrupt Active
> + * 24: Direct Master was the Bus Master during a Master or Target Abort
> + * 25: DMA Channel 0 was the Bus Master during a Master or Target Abort
> + * 26: DMA Channel 1 was the Bus Master during a Master or Target Abort
> + * 27: Target Abort after internal 256 consecutive Master Retrys
> + * 28: PCI Bus wrote data to LCS_MBOX0
> + * 29: PCI Bus wrote data to LCS_MBOX1
> + * 30: PCI Bus wrote data to LCS_MBOX2
> + * 31: PCI Bus wrote data to LCS_MBOX3
> + */
> +#define PLX_PEX8311_PCI_LCS_INTCSR 0x68
> +#define INTCSR_INTERNAL_PCI_WIRE BIT(8)
> +#define INTCSR_LOCAL_INPUT BIT(11)
> +
> /**
> * struct idio_24_gpio_reg - GPIO device registers structure
> * @out0_7: Read: FET Outputs 0-7
> @@ -92,6 +133,7 @@ struct idio_24_gpio_reg {
> struct idio_24_gpio {
> struct gpio_chip chip;
> raw_spinlock_t lock;
> + __u8 __iomem *plx;
> struct idio_24_gpio_reg __iomem *reg;
> unsigned long irq_mask;
> };
> @@ -455,6 +497,7 @@ static int idio_24_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> struct device *const dev = &pdev->dev;
> struct idio_24_gpio *idio24gpio;
> int err;
> + const size_t pci_plx_bar_index = 1;
> const size_t pci_bar_index = 2;
> const char *const name = pci_name(pdev);
> struct gpio_irq_chip *girq;
> @@ -469,12 +512,13 @@ static int idio_24_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> return err;
> }
>
> - err = pcim_iomap_regions(pdev, BIT(pci_bar_index), name);
> + err = pcim_iomap_regions(pdev, BIT(pci_plx_bar_index) | BIT(pci_bar_index), name);
> if (err) {
> dev_err(dev, "Unable to map PCI I/O addresses (%d)\n", err);
> return err;
> }
>
> + idio24gpio->plx = pcim_iomap_table(pdev)[pci_plx_bar_index];
> idio24gpio->reg = pcim_iomap_table(pdev)[pci_bar_index];
>
> idio24gpio->chip.label = name;
> @@ -504,6 +548,12 @@ static int idio_24_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>
> /* Software board reset */
> iowrite8(0, &idio24gpio->reg->soft_reset);
> + /*
> + * enable PLX PEX8311 internal PCI wire interrupt and local interrupt
> + * input
> + */
> + iowrite8((INTCSR_INTERNAL_PCI_WIRE | INTCSR_LOCAL_INPUT) >> 8,
> + idio24gpio->plx + PLX_PEX8311_PCI_LCS_INTCSR + 1);
>
> err = devm_gpiochip_add_data(dev, &idio24gpio->chip, idio24gpio);
> if (err) {
> --
> 2.25.1
>
Fix the bitwise operation to remove only the corresponding bit from the
mask.
Fixes: 585562046628 ("gpio: Add GPIO support for the ACCES PCIe-IDIO-24 family")
Signed-off-by: Arnaud de Turckheim <[email protected]>
---
drivers/gpio/gpio-pcie-idio-24.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-pcie-idio-24.c b/drivers/gpio/gpio-pcie-idio-24.c
index a68941d19ac6..5ea517416366 100644
--- a/drivers/gpio/gpio-pcie-idio-24.c
+++ b/drivers/gpio/gpio-pcie-idio-24.c
@@ -339,7 +339,7 @@ static void idio_24_irq_mask(struct irq_data *data)
raw_spin_lock_irqsave(&idio24gpio->lock, flags);
- idio24gpio->irq_mask &= BIT(bit_offset);
+ idio24gpio->irq_mask &= ~BIT(bit_offset);
new_irq_mask = idio24gpio->irq_mask >> bank_offset;
if (!new_irq_mask) {
--
2.25.1
On Wed, Nov 04, 2020 at 04:24:54PM +0100, Arnaud de Turckheim wrote:
> This fixes the COS Enable Register value for enabling/disabling the
> corresponding IRQs bank.
>
> Fixes: 585562046628 ("gpio: Add GPIO support for the ACCES PCIe-IDIO-24 family")
> Signed-off-by: Arnaud de Turckheim <[email protected]>
Reviewed-by: William Breathitt Gray <[email protected]>
> ---
> drivers/gpio/gpio-pcie-idio-24.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pcie-idio-24.c b/drivers/gpio/gpio-pcie-idio-24.c
> index 5ea517416366..a61de14d09b6 100644
> --- a/drivers/gpio/gpio-pcie-idio-24.c
> +++ b/drivers/gpio/gpio-pcie-idio-24.c
> @@ -334,13 +334,13 @@ static void idio_24_irq_mask(struct irq_data *data)
> unsigned long flags;
> const unsigned long bit_offset = irqd_to_hwirq(data) - 24;
> unsigned char new_irq_mask;
> - const unsigned long bank_offset = bit_offset/8 * 8;
> + const unsigned long bank_offset = bit_offset / 8;
> unsigned char cos_enable_state;
>
> raw_spin_lock_irqsave(&idio24gpio->lock, flags);
>
> idio24gpio->irq_mask &= ~BIT(bit_offset);
> - new_irq_mask = idio24gpio->irq_mask >> bank_offset;
> + new_irq_mask = idio24gpio->irq_mask >> bank_offset * 8;
>
> if (!new_irq_mask) {
> cos_enable_state = ioread8(&idio24gpio->reg->cos_enable);
> @@ -363,12 +363,12 @@ static void idio_24_irq_unmask(struct irq_data *data)
> unsigned long flags;
> unsigned char prev_irq_mask;
> const unsigned long bit_offset = irqd_to_hwirq(data) - 24;
> - const unsigned long bank_offset = bit_offset/8 * 8;
> + const unsigned long bank_offset = bit_offset / 8;
> unsigned char cos_enable_state;
>
> raw_spin_lock_irqsave(&idio24gpio->lock, flags);
>
> - prev_irq_mask = idio24gpio->irq_mask >> bank_offset;
> + prev_irq_mask = idio24gpio->irq_mask >> bank_offset * 8;
> idio24gpio->irq_mask |= BIT(bit_offset);
>
> if (!prev_irq_mask) {
> --
> 2.25.1
>
On Wed, Nov 4, 2020 at 4:25 PM Arnaud de Turckheim <[email protected]> wrote:
>
> This patch set fixes the irq_mask/irq_unmask functions and enables the
> PLX PEX8311 local interrupts.
>
> With the current version of the driver, gpiomon (from libgpiod) or a
> poll(2) on the sysfs value are not working. According to
> /proc/interrupts, there is no interruption triggered.
> The main issue is that the local interrupts are not forwarded by the
> PEX8311 to the PCI.
>
> There is also two bugs:
> - The IRQ mask is not correctly updated when unmasking an
> interruption.
> - The COS Enable Register value is not correctly updated when
> masking/unmasking an interruption.
>
> It seems this problems exist since the initial commit.
>
> Arnaud de Turckheim (3):
> gpio: pcie-idio-24: Fix irq mask when masking
> gpio: pcie-idio-24: Fix IRQ Enable Register value
> gpio: pcie-idio-24: Enable PEX8311 interrupts
>
> drivers/gpio/gpio-pcie-idio-24.c | 62 ++++++++++++++++++++++++++++----
> 1 file changed, 56 insertions(+), 6 deletions(-)
>
> --
> 2.25.1
>
Series queued for fixes, thanks!
Bartosz