mbox series

[0/3] gpio: pcie-idio-24: Fix IRQ handling

Message ID 20201104152455.40627-1-quarium@gmail.com
Headers show
Series gpio: pcie-idio-24: Fix IRQ handling | expand

Message

Arnaud de Turckheim Nov. 4, 2020, 3:24 p.m. UTC 
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(-)

Comments

William Breathitt Gray Nov. 4, 2020, 3:45 p.m. UTC | #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
William Breathitt Gray Nov. 4, 2020, 3:47 p.m. UTC | #2 
On Wed, Nov 04, 2020 at 04:24:53PM +0100, Arnaud de Turckheim wrote:
> 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 <quarium@gmail.com>


Reviewed-by: William Breathitt Gray <vilhelm.gray@gmail.com>


> ---

>  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

>
William Breathitt Gray Nov. 4, 2020, 3:47 p.m. UTC | #3 
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 <quarium@gmail.com>


Reviewed-by: William Breathitt Gray <vilhelm.gray@gmail.com>


> ---

>  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

>
Bartosz Golaszewski Nov. 6, 2020, 2:20 p.m. UTC | #4 
On Wed, Nov 4, 2020 at 4:25 PM Arnaud de Turckheim <quarium@gmail.com> 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