gpio: pcie-idio-24: Fix IRQ handling
Subsystems (2)
cover gpio: pcie-idio-24: Fix IRQ handling
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(-)
Signature · 1 line
2.25.1
PATCH 1/3 gpio: pcie-idio-24: Fix irq mask when masking
Fix the bitwise operation to remove only the corresponding bit from the mask.
--- drivers/gpio/gpio-pcie-idio-24.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpio-pcie-idio-24.c +1 -1
@@ -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) {
Signature · 1 line
2.25.1
Re: [PATCH 1/3] gpio: pcie-idio-24: Fix irq mask when masking
Quoted 5 lines · Arnaud de Turckheim
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]>
Quoted 20 lines
--- drivers/gpio/gpio-pcie-idio-24.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/gpio/gpio-pcie-idio-24.c +1
-1@@ -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) {Signature · 1 line
2.25.1
PATCH 2/3 gpio: pcie-idio-24: Fix IRQ Enable Register value
In reply to Arnaud de TurckheimThis fixes the COS Enable Register value for enabling/disabling the corresponding IRQs bank.
--- drivers/gpio/gpio-pcie-idio-24.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpio/gpio-pcie-idio-24.c +4 -4
@@ -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) {
Signature · 1 line
2.25.1
Re: [PATCH 2/3] gpio: pcie-idio-24: Fix IRQ Enable Register value
Quoted 5 lines · Arnaud de Turckheim
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]>
Quoted 42 lines
--- drivers/gpio/gpio-pcie-idio-24.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)drivers/gpio/gpio-pcie-idio-24.c +4
-4@@ -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) {Signature · 1 line
2.25.1
PATCH 3/3 gpio: pcie-idio-24: Enable PEX8311 interrupts
In reply to Arnaud de TurckheimThis enables the PEX8311 internal PCI wire interrupt and the PEX8311 local interrupt input so the local interrupts are forwarded to the PCI.
--- drivers/gpio/gpio-pcie-idio-24.c | 52 +++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-)
drivers/gpio/gpio-pcie-idio-24.c +51 -1
@@ -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) {
Signature · 1 line
2.25.1
Re: [PATCH 3/3] gpio: pcie-idio-24: Enable PEX8311 interrupts
Quoted 5 lines · Arnaud de Turckheim
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]>
Quoted 103 lines
--- drivers/gpio/gpio-pcie-idio-24.c | 52 +++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-)drivers/gpio/gpio-pcie-idio-24.c +51
-1@@ -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) {Signature · 1 line
2.25.1
Quoted 17 lines · Arnaud de Turckheim
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.
Quoted 11 lines
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 interruptsdrivers/gpio/gpio-pcie-idio-24.c | 62 ++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-)Signature · 1 line
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
Quoted 23 lines · Arnaud de Turckheim
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
Quoted 6 lines
drivers/gpio/gpio-pcie-idio-24.c | 62 ++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-)--
2.25.1
Series queued for fixes, thanks!
Bartosz