diff mbox

[v2] gpio-pca953x: Support NXP PCAL9555A with Agile I/O

Message ID 1436298146-26451-1-git-send-email-clemens.gruber@pqgruber.com
State Superseded
Headers show

Commit Message

Clemens Gruber July 7, 2015, 7:42 p.m. UTC
This patch adds support for the NXP PCAL9555A GPIO expander's extra
features, called Agile I/O:
Input latching, interrupt masks and open-drain output stages can be
configured via 3 optional device tree properties.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Alexandre Courbot <gnurou@gmail.com>
Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com>
---

Changes from v1:
- The mapping between bits and pins was counterintuitive. Corrected!
- Improved documentation for new properties and example comments

---
 .../devicetree/bindings/gpio/gpio-pca953x.txt      | 24 +++++++-
 drivers/gpio/gpio-pca953x.c                        | 68 ++++++++++++++++++++++
 2 files changed, 91 insertions(+), 1 deletion(-)

Comments

Clemens Gruber July 12, 2015, 10:51 p.m. UTC | #1
Hi,

I noticed the following issue after 100001 interrupts occured on the
pca-953x interrupt-controller driver used with a PCAL9555A chip. Its
INT output is connected to a GPIO line of a Freescale i.MX6Q SoC.

irq 47: nobody cared (try booting with the "irqpoll" option)
[  508.320093] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.2.0-rc1-00197-g3b9efb0 #92
[  508.327669] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[  508.334237] [<80016cac>] (unwind_backtrace) from
[<80013494>](show_stack+0x10/0x14)
[  508.342006] [<80013494>] (show_stack) from
[<80534e88>](dump_stack+0x84/0xc4)
[  508.349251] [<80534e88>] (dump_stack) from
[<800816cc>](__report_bad_irq+0x28/0xc4)
[  508.357008] [<800816cc>] (__report_bad_irq) from
[<80081c68>](note_interrupt+0x264/0x2b4)
[  508.365288] [<80081c68>] (note_interrupt) from
[<8007f338>](handle_irq_event_percpu+0xd0/0x138)
[  508.374088] [<8007f338>] (handle_irq_event_percpu) from
[<8007f3e0>](handle_irq_event+0x40/0x64)
[  508.382973] [<8007f3e0>] (handle_irq_event) from
[<800823dc>](handle_level_irq+0xc8/0x148)
[  508.391338] [<800823dc>] (handle_level_irq) from
[<8007e928>](generic_handle_irq+0x2c/0x3c)
[  508.399799] [<8007e928>] (generic_handle_irq) from
[<8029ea20>](mxc_gpio_irq_handler+0x38/0xf8)
[  508.407283] pca953x 1-0020: failed reading register
[  508.413479] [<8029ea20>] (mxc_gpio_irq_handler) from
[<8029eb68>](mx3_gpio_irq_handler+0x88/0xe8)
[  508.422451] [<8029eb68>] (mx3_gpio_irq_handler) from
[<8007e928>](generic_handle_irq+0x2c/0x3c)
[  508.431250] [<8007e928>] (generic_handle_irq) from
[<8007ec20>](__handle_domain_irq+0x7c/0xec)
[  508.439962] [<8007ec20>] (__handle_domain_irq) from
[<800094ac>](gic_handle_irq+0x24/0x5c)
[  508.448327] [<800094ac>] (gic_handle_irq) from
[<80014084>](__irq_svc+0x44/0x7c)
[  508.455817] Exception stack(0x80733f20 to 0x80733f68)
[  508.460882] 3f20: 00000001 00000001 00000000 80737a30 00000000 bf7a0850
c5ed8476 00000076
[  508.469071] 3f40: c5c85253 00000076 00000000 807345f0 00000000 80733f68
80071d40 8039b9bc
[  508.477254] 3f60: 200c0013 ffffffff
[  508.480763] [<80014084>] (__irq_svc) from [<8039b9bc>]
(cpuidle_enter_state+0x12c/0x264)
[  508.488872] [<8039b9bc>] (cpuidle_enter_state) from [<8006cbe4>]
(cpu_startup_entry+0x194/0x284)
[  508.497678] [<8006cbe4>] (cpu_startup_entry) from [<806f5c94>]
(start_kernel+0x3a4/0x3c4)
[  508.505862] handlers:
[  508.508148] [<8007f404>] irq_default_primary_handler threaded [<8029f2f0>]
pca953x_irq_handler
[  508.516812] Disabling IRQ #47

Did any of you experience this type of issue before?

In my patch [1] from last week, I added an option to unmask the interrupts on
the PCAL9555A where they are masked by default. But when I do and if I then
try to trigger as many as possible, they seem not to be handled correctly by
the driver.

In the devicetree I specify:

gpioexp_stat_a: pcal9555a@20 {
   compatible = "nxp,pcal9555a";
   reg = <0x20>;
   gpio-controller;
   #gpio-cells = <2>;
   interrupt-controller;
   #interrupt-cells = <2>;
   interrupt-parent = <&gpio1>;
   interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
   nxp,intr-mask = <0x0000>; /* Do not mask the interrupts */
};

Both the PCA9555 [2] and the PCAL9555A [3] clear the interrupt if we read from
the input port register.
So apart from the PCAL9555A having interrupt mask registers which are set to 1
upon power-on and then cleared by the changes introduced in my patch, it should
not behave differently than the PCA9555.

As the PCAL9555A also has interrupt status registers it would be possible to use
them to find out which line triggered the interrupt, instead of relying on the
previously read input values (as it is done now in pca953x_irq_pending).
But the latter should work both on the PCA9555 and on the PCAL9555A, right?

Am I missing something?

Regards,
Clemens

[1]: https://patchwork.ozlabs.org/patch/492586/
[2]: http://www.nxp.com/documents/data_sheet/PCA9555.pdf
[3]: http://www.nxp.com/documents/data_sheet/PCAL9555A.pdf
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij July 16, 2015, 12:41 p.m. UTC | #2
On Tue, Jul 7, 2015 at 9:42 PM, Clemens Gruber
<clemens.gruber@pqgruber.com> wrote:

> +Optional properties for chips with Agile I/O:
> +- nxp,input-latch: Enable input latch, one bit per pin
> +- nxp,intr-mask: Unmask interrupts by clearing the mask bits per pin
> +- nxp,open-drain: Configure outputs as open-drain, one bit per bank

Same comments as on v1.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij July 16, 2015, 1:37 p.m. UTC | #3
On Mon, Jul 13, 2015 at 12:51 AM, Clemens Gruber
<clemens.gruber@pqgruber.com> wrote:

> I noticed the following issue after 100001 interrupts occured on the
> pca-953x interrupt-controller driver used with a PCAL9555A chip. Its
> INT output is connected to a GPIO line of a Freescale i.MX6Q SoC.
>
> irq 47: nobody cared (try booting with the "irqpoll" option)
> [  508.320093] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.2.0-rc1-00197-g3b9efb0 #92
> [  508.327669] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [  508.334237] [<80016cac>] (unwind_backtrace) from
> [<80013494>](show_stack+0x10/0x14)
> [  508.342006] [<80013494>] (show_stack) from
> [<80534e88>](dump_stack+0x84/0xc4)
> [  508.349251] [<80534e88>] (dump_stack) from
> [<800816cc>](__report_bad_irq+0x28/0xc4)
> [  508.357008] [<800816cc>] (__report_bad_irq) from
> [<80081c68>](note_interrupt+0x264/0x2b4)
> [  508.365288] [<80081c68>] (note_interrupt) from
> [<8007f338>](handle_irq_event_percpu+0xd0/0x138)
> [  508.374088] [<8007f338>] (handle_irq_event_percpu) from
> [<8007f3e0>](handle_irq_event+0x40/0x64)
> [  508.382973] [<8007f3e0>] (handle_irq_event) from
> [<800823dc>](handle_level_irq+0xc8/0x148)
> [  508.391338] [<800823dc>] (handle_level_irq) from
> [<8007e928>](generic_handle_irq+0x2c/0x3c)
> [  508.399799] [<8007e928>] (generic_handle_irq) from
> [<8029ea20>](mxc_gpio_irq_handler+0x38/0xf8)
> [  508.407283] pca953x 1-0020: failed reading register
> [  508.413479] [<8029ea20>] (mxc_gpio_irq_handler) from
> [<8029eb68>](mx3_gpio_irq_handler+0x88/0xe8)
> [  508.422451] [<8029eb68>] (mx3_gpio_irq_handler) from
> [<8007e928>](generic_handle_irq+0x2c/0x3c)
> [  508.431250] [<8007e928>] (generic_handle_irq) from
> [<8007ec20>](__handle_domain_irq+0x7c/0xec)
> [  508.439962] [<8007ec20>] (__handle_domain_irq) from
> [<800094ac>](gic_handle_irq+0x24/0x5c)
> [  508.448327] [<800094ac>] (gic_handle_irq) from
> [<80014084>](__irq_svc+0x44/0x7c)
> [  508.455817] Exception stack(0x80733f20 to 0x80733f68)
> [  508.460882] 3f20: 00000001 00000001 00000000 80737a30 00000000 bf7a0850
> c5ed8476 00000076
> [  508.469071] 3f40: c5c85253 00000076 00000000 807345f0 00000000 80733f68
> 80071d40 8039b9bc
> [  508.477254] 3f60: 200c0013 ffffffff
> [  508.480763] [<80014084>] (__irq_svc) from [<8039b9bc>]
> (cpuidle_enter_state+0x12c/0x264)
> [  508.488872] [<8039b9bc>] (cpuidle_enter_state) from [<8006cbe4>]
> (cpu_startup_entry+0x194/0x284)
> [  508.497678] [<8006cbe4>] (cpu_startup_entry) from [<806f5c94>]
> (start_kernel+0x3a4/0x3c4)
> [  508.505862] handlers:
> [  508.508148] [<8007f404>] irq_default_primary_handler threaded [<8029f2f0>]
> pca953x_irq_handler
> [  508.516812] Disabling IRQ #47
>
> Did any of you experience this type of issue before?
>
> In my patch [1] from last week, I added an option to unmask the interrupts on
> the PCAL9555A where they are masked by default. But when I do and if I then
> try to trigger as many as possible, they seem not to be handled correctly by
> the driver.
>
> In the devicetree I specify:
>
> gpioexp_stat_a: pcal9555a@20 {
>    compatible = "nxp,pcal9555a";
>    reg = <0x20>;
>    gpio-controller;
>    #gpio-cells = <2>;
>    interrupt-controller;
>    #interrupt-cells = <2>;
>    interrupt-parent = <&gpio1>;
>    interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
>    nxp,intr-mask = <0x0000>; /* Do not mask the interrupts */
> };
>
> Both the PCA9555 [2] and the PCAL9555A [3] clear the interrupt if we read from
> the input port register.
> So apart from the PCAL9555A having interrupt mask registers which are set to 1
> upon power-on and then cleared by the changes introduced in my patch, it should
> not behave differently than the PCA9555.
>
> As the PCAL9555A also has interrupt status registers it would be possible to use
> them to find out which line triggered the interrupt, instead of relying on the
> previously read input values (as it is done now in pca953x_irq_pending).
> But the latter should work both on the PCA9555 and on the PCAL9555A, right?
>
> Am I missing something?
>
> Regards,
> Clemens
>
> [1]: https://patchwork.ozlabs.org/patch/492586/
> [2]: http://www.nxp.com/documents/data_sheet/PCA9555.pdf
> [3]: http://www.nxp.com/documents/data_sheet/PCAL9555A.pdf

You need to mail the driver maintainers and/or people that were working
on it recently. None of the maintainers have this hardware.

Use git log drivers/gpio-pca953x.c

It seems Grygorii has a patch for this (to my untrained eye) that is
merged for fixes, subject "gpio: pca953x: fix nested irqs rescheduling"

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grygorii Strashko July 16, 2015, 2:09 p.m. UTC | #4
Clemens,

On 07/16/2015 04:37 PM, Linus Walleij wrote:
> On Mon, Jul 13, 2015 at 12:51 AM, Clemens Gruber
> <clemens.gruber@pqgruber.com> wrote:
> 
>> I noticed the following issue after 100001 interrupts occured on the
>> pca-953x interrupt-controller driver used with a PCAL9555A chip. Its
>> INT output is connected to a GPIO line of a Freescale i.MX6Q SoC.
>>
>> irq 47: nobody cared (try booting with the "irqpoll" option)
>> [  508.320093] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
>> 4.2.0-rc1-00197-g3b9efb0 #92
>> [  508.327669] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
>> [  508.334237] [<80016cac>] (unwind_backtrace) from
>> [<80013494>](show_stack+0x10/0x14)
>> [  508.342006] [<80013494>] (show_stack) from
>> [<80534e88>](dump_stack+0x84/0xc4)
>> [  508.349251] [<80534e88>] (dump_stack) from
>> [<800816cc>](__report_bad_irq+0x28/0xc4)
>> [  508.357008] [<800816cc>] (__report_bad_irq) from
>> [<80081c68>](note_interrupt+0x264/0x2b4)
>> [  508.365288] [<80081c68>] (note_interrupt) from
>> [<8007f338>](handle_irq_event_percpu+0xd0/0x138)
>> [  508.374088] [<8007f338>] (handle_irq_event_percpu) from
>> [<8007f3e0>](handle_irq_event+0x40/0x64)
>> [  508.382973] [<8007f3e0>] (handle_irq_event) from
>> [<800823dc>](handle_level_irq+0xc8/0x148)
>> [  508.391338] [<800823dc>] (handle_level_irq) from
>> [<8007e928>](generic_handle_irq+0x2c/0x3c)
>> [  508.399799] [<8007e928>] (generic_handle_irq) from
>> [<8029ea20>](mxc_gpio_irq_handler+0x38/0xf8)
>> [  508.407283] pca953x 1-0020: failed reading register
>> [  508.413479] [<8029ea20>] (mxc_gpio_irq_handler) from

As per your log there is i2c error, which could be the reason of the issue
[  508.407283] pca953x 1-0020: failed reading register


>> [  508.516812] Disabling IRQ #47
>>
>> Did any of you experience this type of issue before?
>>
>> In my patch [1] from last week, I added an option to unmask the interrupts on
>> the PCAL9555A where they are masked by default. But when I do and if I then
>> try to trigger as many as possible, they seem not to be handled correctly by
>> the driver.
>>
>> In the devicetree I specify:
>>
>> gpioexp_stat_a: pcal9555a@20 {
>>     compatible = "nxp,pcal9555a";
>>     reg = <0x20>;
>>     gpio-controller;
>>     #gpio-cells = <2>;
>>     interrupt-controller;
>>     #interrupt-cells = <2>;
>>     interrupt-parent = <&gpio1>;
>>     interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
>>     nxp,intr-mask = <0x0000>; /* Do not mask the interrupts */
>> };
>>
>> Both the PCA9555 [2] and the PCAL9555A [3] clear the interrupt if we read from
>> the input port register.
>> So apart from the PCAL9555A having interrupt mask registers which are set to 1
>> upon power-on and then cleared by the changes introduced in my patch, it should
>> not behave differently than the PCA9555.
>>
>> As the PCAL9555A also has interrupt status registers it would be possible to use
>> them to find out which line triggered the interrupt, instead of relying on the
>> previously read input values (as it is done now in pca953x_irq_pending).
>> But the latter should work both on the PCA9555 and on the PCAL9555A, right?
>>
>> Am I missing something?
>>
> 
> You need to mail the driver maintainers and/or people that were working
> on it recently. None of the maintainers have this hardware.
> 
> Use git log drivers/gpio-pca953x.c
> 
> It seems Grygorii has a patch for this (to my untrained eye) that is
> merged for fixes, subject "gpio: pca953x: fix nested irqs rescheduling"

Not sure. I assume this issue is observed with above patch applied. Right?
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
index b9a42f2..6609906 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
@@ -11,6 +11,7 @@  Required properties:
 	nxp,pca9539
 	nxp,pca9554
 	nxp,pca9555
+	nxp,pcal9555a
 	nxp,pca9556
 	nxp,pca9557
 	nxp,pca9574
@@ -26,8 +27,15 @@  Required properties:
 	ti,tca6424
 	exar,xra1202
 
-Example:
+Supported chips with Agile I/O features:
+- nxp,pcal9555a
 
+Optional properties for chips with Agile I/O:
+- nxp,input-latch: Enable input latch, one bit per pin
+- nxp,intr-mask: Unmask interrupts by clearing the mask bits per pin
+- nxp,open-drain: Configure outputs as open-drain, one bit per bank
+
+Examples:
 
 	gpio@20 {
 		compatible = "nxp,pca9505";
@@ -37,3 +45,17 @@  Example:
 		interrupt-parent = <&gpio3>;
 		interrupts = <23 IRQ_TYPE_LEVEL_LOW>;
 	};
+
+	gpio@22 {
+		compatible = "nxp,pcal9555a";
+		reg = <0x22>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupt-parent = <&gpio2>;
+		interrupts = <20 IRQ_TYPE_EDGE_FALLING>;
+		nxp,input-latch = <0x1>; /* Latch the input state of I0.0 (bit [0]) */
+		nxp,intr-mask = <0x000f>; /* Mask interrupts of I0.3-0 (bits [3..0]) */
+		nxp,open-drain = <0x2>; /* Enable open-drain stage for output port 1 */
+	};
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d233eb3..b3191f7 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -3,6 +3,7 @@ 
  *
  *  Copyright (C) 2005 Ben Gardner <bgardner@wabtec.com>
  *  Copyright (C) 2007 Marvell International Ltd.
+ *  Copyright (C) 2015 Clemens Gruber <clemens.gruber@pqgruber.com>
  *
  *  Derived from drivers/i2c/chips/pca9539.c
  *
@@ -26,6 +27,9 @@ 
 #define PCA953X_OUTPUT		1
 #define PCA953X_INVERT		2
 #define PCA953X_DIRECTION	3
+#define PCAL95XXA_INPUT_LATCH	0x44
+#define PCAL95XXA_INTR_MASK	0x4A
+#define PCAL95XXA_OUTPUT_CONFIG	0x4F
 
 #define REG_ADDR_AI		0x80
 
@@ -40,6 +44,7 @@ 
 
 #define PCA_GPIO_MASK		0x00FF
 #define PCA_INT			0x0100
+#define PCA_AGILEIO		0x0200
 #define PCA953X_TYPE		0x1000
 #define PCA957X_TYPE		0x2000
 
@@ -53,6 +58,7 @@  static const struct i2c_device_id pca953x_id[] = {
 	{ "pca9539", 16 | PCA953X_TYPE | PCA_INT, },
 	{ "pca9554", 8  | PCA953X_TYPE | PCA_INT, },
 	{ "pca9555", 16 | PCA953X_TYPE | PCA_INT, },
+	{ "pcal9555a", 16 | PCA953X_TYPE | PCA_INT | PCA_AGILEIO, },
 	{ "pca9556", 8  | PCA953X_TYPE, },
 	{ "pca9557", 8  | PCA953X_TYPE, },
 	{ "pca9574", 8  | PCA957X_TYPE | PCA_INT, },
@@ -614,6 +620,53 @@  out:
 	return ret;
 }
 
+static int device_pcal95xxa_agileio_setup(struct pca953x_chip *chip,
+					  struct device_node *node)
+{
+	int ret;
+	u32 input_latch = 0;
+	u32 intr_mask = 0;
+	u32 open_drain = 0;
+
+	/* Input latch */
+	if (of_property_read_u32(node, "nxp,input-latch", &input_latch) == 0) {
+		if (input_latch > 0xFFFF)
+			return -EINVAL;
+
+		ret = i2c_smbus_write_word_data(chip->client,
+						PCAL95XXA_INPUT_LATCH,
+						(u16)input_latch);
+		if (ret)
+			return -EIO;
+	}
+
+	/* Interrupt mask */
+	if (of_property_read_u32(node, "nxp,intr-mask", &intr_mask) == 0) {
+		if (intr_mask > 0xFFFF)
+			return -EINVAL;
+
+		ret = i2c_smbus_write_word_data(chip->client,
+						PCAL95XXA_INTR_MASK,
+						(u16)intr_mask);
+		if (ret)
+			return -EIO;
+	}
+
+	/* Open-drain output stage per port (bank) */
+	if (of_property_read_u32(node, "nxp,open-drain", &open_drain) == 0) {
+		if (open_drain > 0x3)
+			return -EINVAL;
+
+		ret = i2c_smbus_write_byte_data(chip->client,
+						PCAL95XXA_OUTPUT_CONFIG,
+						(u8)open_drain);
+		if (ret)
+			return -EIO;
+	}
+
+	return 0;
+}
+
 static int device_pca957x_init(struct pca953x_chip *chip, u32 invert)
 {
 	int ret;
@@ -645,6 +698,7 @@  out:
 static int pca953x_probe(struct i2c_client *client,
 				   const struct i2c_device_id *id)
 {
+	struct device_node *node = client->dev.of_node;
 	struct pca953x_platform_data *pdata;
 	struct pca953x_chip *chip;
 	int irq_base = 0;
@@ -700,6 +754,19 @@  static int pca953x_probe(struct i2c_client *client,
 			dev_warn(&client->dev, "setup failed, %d\n", ret);
 	}
 
+	/* Configure Agile I/O features, if supported by the chip */
+	if ((id->driver_data & PCA_AGILEIO) && IS_ENABLED(CONFIG_OF) && node) {
+		/* Only expanders with 16-bit supported */
+		if (NBANK(chip) == 2) {
+			ret = device_pcal95xxa_agileio_setup(chip, node);
+			if (ret < 0)
+				dev_warn(&client->dev,
+					 "Agile I/O setup failed, %d\n", ret);
+		} else
+			dev_warn(&client->dev,
+				 "Agile I/O not supported on this chip\n");
+	}
+
 	i2c_set_clientdata(client, chip);
 	return 0;
 }
@@ -735,6 +802,7 @@  static const struct of_device_id pca953x_dt_ids[] = {
 	{ .compatible = "nxp,pca9539", },
 	{ .compatible = "nxp,pca9554", },
 	{ .compatible = "nxp,pca9555", },
+	{ .compatible = "nxp,pcal9555a", },
 	{ .compatible = "nxp,pca9556", },
 	{ .compatible = "nxp,pca9557", },
 	{ .compatible = "nxp,pca9574", },