[PATCH 0/5] i2c: mux: pca954x: Add interrupt controller support

[PATCH 0/5] i2c: mux: pca954x: Add interrupt controller support

[Phil Reid]

Various muxes can aggregate multiple interrupts from each i2c bus.
All of the muxes with interrupt support combine the active low irq lines
using an internal 'and' function and generate a combined active low
output. The muxes do provide the ability to read a control register to
determine which irq is active. By making the mux an irq controller isr
latenct can potentially be reduced by reading the status register and 
then only calling the registered isr on that bus segment.

In addition an additional enable mask is added to work around devices
that assert irq immediately before being setup buy disabling the irq
from the mux until all devices are registered.

Phil Reid (5):
  i2c: mux: pca954x: Add missing pca9542 definition to chip_desc
  dt: bindings: i2c-mux-pca954x: Add documentation for interrupt
    controller
  i2c: mux: pca954x: Add interrupt controller support
  dt: bindings: i2c-mux-pca954x: Add documentation for
    i2c-mux-irq-mask-en
  i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs

 .../devicetree/bindings/i2c/i2c-mux-pca954x.txt    |  17 ++-
 drivers/i2c/muxes/i2c-mux-pca954x.c                | 140 ++++++++++++++++++++-
 2 files changed, 155 insertions(+), 2 deletions(-)

-- 
1.8.3.1

[PATCH 1/5] i2c: mux: pca954x: Add missing pca9542 definition to chip_desc

[Phil Reid]

The spec for the pca954x was missing. This chip is the same as the pca9540
except that it has interrupt lines. While the i2c_device_id table mapped
the pca9542 to the pca9540 definition the compatible table did not. In
preparation for irq support add the pca9542 definition.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8bc3d36..981d145 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -83,6 +83,11 @@ struct pca954x {
 		.enable = 0x4,
 		.muxtype = pca954x_ismux,
 	},
+	[pca_9542] = {
+		.nchans = 2,
+		.enable = 0x4,
+		.muxtype = pca954x_ismux,
+	},
 	[pca_9543] = {
 		.nchans = 2,
 		.muxtype = pca954x_isswi,
@@ -109,7 +114,7 @@ struct pca954x {
 
 static const struct i2c_device_id pca954x_id[] = {
 	{ "pca9540", pca_9540 },
-	{ "pca9542", pca_9540 },
+	{ "pca9542", pca_9542 },
 	{ "pca9543", pca_9543 },
 	{ "pca9544", pca_9544 },
 	{ "pca9545", pca_9545 },
-- 
1.8.3.1

[PATCH 2/5] dt: bindings: i2c-mux-pca954x: Add documentation for interrupt controller

[Phil Reid]

Various muxes can aggregate multiple irq lines and provide a control
register to determine the active line. Add bindings for interrupt
controller support.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
index cf53d5f..9f7c275 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
@@ -19,7 +19,14 @@ Optional Properties:
   - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
     children in idle state. This is necessary for example, if there are several
     multiplexers on the bus and the devices behind them use same I2C addresses.
-
+  - interrupt-parent: Phandle for the interrupt controller that services
+    interrupts for this device.
+  - interrupts: Interrupt mapping for IRQ.
+  - interrupt-controller: Marks the device node as a interrupt controller.
+  - #interrupt-cells : Should be two.
+    - first cell is the pin number
+    - second cell is used to specify flags.
+    See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
 
 Example:
 
@@ -29,6 +36,11 @@ Example:
 		#size-cells = <0>;
 		reg = <0x74>;
 
+		interrupt-parent = <&ipic>;
+		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-controller;
+		#interrupt-cells=<2>;
+
 		i2c@2 {
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
1.8.3.1

[PATCH 3/5] i2c: mux: pca954x: Add interrupt controller support

[Phil Reid]

Various muxes can aggregate multiple interrupts from each i2c bus.
All of the muxes with interrupt support combine the active low irq lines
using an internal 'and' function and generate a combined active low
output. The muxes do provide the ability to read a control register to
determine which irq is active. By making the mux an irq controller isr
can potentially be reduced by reading the status register and then only
calling the registered isr on that bus segment.

As there is no irq masking on the mux irq are disabled until irq_unmask is
called at least once.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 126 ++++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 981d145..f2dba7c 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -40,14 +40,19 @@
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
 #include <linux/i2c/pca954x.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_irq.h>
 #include <linux/pm.h>
 #include <linux/slab.h>
 
 #define PCA954X_MAX_NCHANS 8
 
+#define PCA954X_IRQ_OFFSET 4
+
 enum pca_type {
 	pca_9540,
 	pca_9542,
@@ -62,6 +67,7 @@ enum pca_type {
 struct chip_desc {
 	u8 nchans;
 	u8 enable;	/* used for muxes only */
+	u8 has_irq;
 	enum muxtype {
 		pca954x_ismux = 0,
 		pca954x_isswi
@@ -74,6 +80,9 @@ struct pca954x {
 	u8 last_chan;		/* last register value */
 	u8 deselect;
 	struct i2c_client *client;
+
+	struct irq_domain *irq;
+	unsigned int irq_mask;
 };
 
 /* Provide specs for the PCA954x types we know about */
@@ -86,19 +95,23 @@ struct pca954x {
 	[pca_9542] = {
 		.nchans = 2,
 		.enable = 0x4,
+		.has_irq = 1,
 		.muxtype = pca954x_ismux,
 	},
 	[pca_9543] = {
 		.nchans = 2,
+		.has_irq = 1,
 		.muxtype = pca954x_isswi,
 	},
 	[pca_9544] = {
 		.nchans = 4,
 		.enable = 0x4,
+		.has_irq = 1,
 		.muxtype = pca954x_ismux,
 	},
 	[pca_9545] = {
 		.nchans = 4,
+		.has_irq = 1,
 		.muxtype = pca954x_isswi,
 	},
 	[pca_9547] = {
@@ -203,6 +216,104 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
 	return pca954x_reg_write(muxc->parent, client, data->last_chan);
 }
 
+static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
+{
+	struct pca954x *data = dev_id;
+	unsigned int child_irq;
+	int ret, i, handled;
+
+	ret = i2c_smbus_read_byte(data->client);
+	if (ret < 0)
+		return IRQ_NONE;
+
+	for (i = 0; i < data->chip->nchans; i++) {
+		if (ret & BIT(PCA954X_IRQ_OFFSET+i)) {
+			child_irq = irq_linear_revmap(data->irq, i);
+			handle_nested_irq(child_irq);
+			handled++;
+		}
+	}
+	return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static void pca954x_irq_mask(struct irq_data *idata)
+{
+	struct pca954x *data = irq_data_get_irq_chip_data(idata);
+	unsigned int pos = idata->hwirq;
+
+	data->irq_mask &= ~BIT(pos);
+	if (!data->irq_mask)
+		disable_irq(data->client->irq);
+}
+
+static void pca954x_irq_unmask(struct irq_data *idata)
+{
+	struct pca954x *data = irq_data_get_irq_chip_data(idata);
+	unsigned int pos = idata->hwirq;
+
+	if (!data->irq_mask)
+		enable_irq(data->client->irq);
+	data->irq_mask |= BIT(pos);
+}
+
+static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
+{
+	if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
+		return -EINVAL;
+	return 0;
+}
+
+static struct irq_chip pca954x_irq_chip = {
+	.name = "i2c-mux-pca954x",
+	.irq_mask = pca954x_irq_mask,
+	.irq_unmask = pca954x_irq_unmask,
+	.irq_set_type = pca954x_irq_set_type,
+};
+
+static int pca953x_irq_setup(struct i2c_mux_core *muxc)
+{
+	struct pca954x *data = i2c_mux_priv(muxc);
+	struct i2c_client *client = data->client;
+	int c, err, irq;
+
+	if (!data->chip->has_irq || client->irq <= 0)
+		return 0;
+
+	data->irq = irq_domain_add_linear(client->dev.of_node,
+					  data->chip->nchans,
+					  &irq_domain_simple_ops, data);
+	if (!data->irq)	{
+		err = -ENODEV;
+		goto err_irq_domain;
+	}
+
+	for (c = 0; c < data->chip->nchans; c++) {
+		irq = irq_create_mapping(data->irq, c);
+		irq_set_chip_data(irq, data);
+		irq_set_chip_and_handler(irq, &pca954x_irq_chip,
+			handle_simple_irq);
+	}
+
+	err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
+					pca954x_irq_handler,
+					IRQF_ONESHOT | IRQF_SHARED,
+					"pca953x", data);
+	if (err)
+		goto err_req_irq;
+
+	disable_irq(data->client->irq);
+
+	return 0;
+err_req_irq:
+	for (c = 0; c < data->chip->nchans; c++) {
+		irq = irq_find_mapping(data->irq, c);
+		irq_dispose_mapping(irq);
+	}
+	irq_domain_remove(data->irq);
+err_irq_domain:
+	return err;
+}
+
 /*
  * I2C init/probing/exit functions
  */
@@ -258,6 +369,10 @@ static int pca954x_probe(struct i2c_client *client,
 	idle_disconnect_dt = of_node &&
 		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
 
+	ret = pca953x_irq_setup(muxc);
+	if (ret)
+		goto irq_setup_failed;
+
 	/* Now create an adapter for each channel */
 	for (num = 0; num < data->chip->nchans; num++) {
 		bool idle_disconnect_pd = false;
@@ -294,6 +409,7 @@ static int pca954x_probe(struct i2c_client *client,
 
 	return 0;
 
+irq_setup_failed:
 virt_reg_failed:
 	i2c_mux_del_adapters(muxc);
 	return ret;
@@ -302,6 +418,16 @@ static int pca954x_probe(struct i2c_client *client,
 static int pca954x_remove(struct i2c_client *client)
 {
 	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
+	struct pca954x *data = i2c_mux_priv(muxc);
+	int c, irq;
+
+	if (data->irq) {
+		for (c = 0; c < data->chip->nchans; c++) {
+			irq = irq_find_mapping(data->irq, c);
+			irq_dispose_mapping(irq);
+		}
+		irq_domain_remove(data->irq);
+	}
 
 	i2c_mux_del_adapters(muxc);
 	return 0;
-- 
1.8.3.1

[PATCH 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for i2c-mux-irq-mask-en

[Phil Reid]

Unfortunately some hardware device will assert their irq line immediately
on power on and provide no mechanism to mask the irq. As the i2c muxes
provide no method to mask irq line this provides a work around by keeping
the parent irq masked until enough device drivers have loaded to service
all pending interrupts.

For example the the ltc1760 assert its SMBALERT irq immediately on power
on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
device is registered irq are enabled and fire continuously as the second
device driver has not yet loaded. Setting this parameter to 0x3 while
delay the irq being enabled until both devices are ready.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
index 9f7c275..d3e63dc 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
@@ -19,6 +19,8 @@ Optional Properties:
   - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
     children in idle state. This is necessary for example, if there are several
     multiplexers on the bus and the devices behind them use same I2C addresses.
+  - i2c-mux-irq-mask-en: BitMask; Defines a mask for which irq lines need to be
+    unmasked before the parent irq line in enabled.
   - interrupt-parent: Phandle for the interrupt controller that services
     interrupts for this device.
   - interrupts: Interrupt mapping for IRQ.
@@ -36,6 +38,7 @@ Example:
 		#size-cells = <0>;
 		reg = <0x74>;
 
+		i2c-mux-irq-mask-en = <0x3>;
 		interrupt-parent = <&ipic>;
 		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
 		interrupt-controller;
-- 
1.8.3.1

[PATCH 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs

[Phil Reid]

Unfortunately some hardware device will assert their irq line immediately
on power on and provide no mechanism to mask the irq. As the i2c muxes
provide no method to mask irq line this provides a work around by keeping
the parent irq masked until enough device drivers have loaded to service
all pending interrupts.

For example the the ltc1760 assert its SMBALERT irq immediately on power
on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
device is registered irq are enabled and fire continuously as the second
device driver has not yet loaded. Setting this parameter to 0x3 while
delay the irq being enabled until both devices are ready.

Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index f2dba7c..16269e7 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -83,6 +83,7 @@ struct pca954x {
 
 	struct irq_domain *irq;
 	unsigned int irq_mask;
+	unsigned int irq_mask_en;
 };
 
 /* Provide specs for the PCA954x types we know about */
@@ -251,9 +252,12 @@ static void pca954x_irq_unmask(struct irq_data *idata)
 	struct pca954x *data = irq_data_get_irq_chip_data(idata);
 	unsigned int pos = idata->hwirq;
 
-	if (!data->irq_mask)
+	if (!data->irq_mask_en && !data->irq_mask)
 		enable_irq(data->client->irq);
 	data->irq_mask |= BIT(pos);
+	if (data->irq_mask_en &&
+		(data->irq_mask & data->irq_mask) == data->irq_mask_en)
+		enable_irq(data->client->irq);
 }
 
 static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
@@ -369,6 +373,9 @@ static int pca954x_probe(struct i2c_client *client,
 	idle_disconnect_dt = of_node &&
 		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
 
+	of_property_read_u32(of_node, "i2c-mux-irq-mask-en",
+			     &data->irq_mask_en);
+
 	ret = pca953x_irq_setup(muxc);
 	if (ret)
 		goto irq_setup_failed;
-- 
1.8.3.1

Re: [PATCH 1/5] i2c: mux: pca954x: Add missing pca9542 definition to chip_desc

[Peter Rosin]

On 2017-01-04 10:29, Phil Reid wrote:
> The spec for the pca954x was missing. This chip is the same as the pca9540
> except that it has interrupt lines. While the i2c_device_id table mapped
> the pca9542 to the pca9540 definition the compatible table did not. In
> preparation for irq support add the pca9542 definition.

A new ACPI table was added behind your back, which needs the same
treatment as the i2c table.

With that fixed,
Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

Cheers,
peda

> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
>  drivers/i2c/muxes/i2c-mux-pca954x.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 8bc3d36..981d145 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -83,6 +83,11 @@ struct pca954x {
>  		.enable = 0x4,
>  		.muxtype = pca954x_ismux,
>  	},
> +	[pca_9542] = {
> +		.nchans = 2,
> +		.enable = 0x4,
> +		.muxtype = pca954x_ismux,
> +	},
>  	[pca_9543] = {
>  		.nchans = 2,
>  		.muxtype = pca954x_isswi,
> @@ -109,7 +114,7 @@ struct pca954x {
>  
>  static const struct i2c_device_id pca954x_id[] = {
>  	{ "pca9540", pca_9540 },
> -	{ "pca9542", pca_9540 },
> +	{ "pca9542", pca_9542 },
>  	{ "pca9543", pca_9543 },
>  	{ "pca9544", pca_9544 },
>  	{ "pca9545", pca_9545 },
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] dt: bindings: i2c-mux-pca954x: Add documentation for interrupt controller

[Peter Rosin]

On 2017-01-04 10:29, Phil Reid wrote:
> Various muxes can aggregate multiple irq lines and provide a control
> register to determine the active line. Add bindings for interrupt
> controller support.
> 

I'm no irq expert, but looks good (superficially).

With the below nitpick,
Acked-by: Peter Rosin <peda@axentia.se>

Cheers,
peda

> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> index cf53d5f..9f7c275 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> @@ -19,7 +19,14 @@ Optional Properties:
>    - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
>      children in idle state. This is necessary for example, if there are several
>      multiplexers on the bus and the devices behind them use same I2C addresses.
> -
> +  - interrupt-parent: Phandle for the interrupt controller that services
> +    interrupts for this device.
> +  - interrupts: Interrupt mapping for IRQ.
> +  - interrupt-controller: Marks the device node as a interrupt controller.

as an interrupt

> +  - #interrupt-cells : Should be two.
> +    - first cell is the pin number
> +    - second cell is used to specify flags.
> +    See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>  
>  Example:
>  
> @@ -29,6 +36,11 @@ Example:
>  		#size-cells = <0>;
>  		reg = <0x74>;
>  
> +		interrupt-parent = <&ipic>;
> +		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-controller;
> +		#interrupt-cells=<2>;
> +
>  		i2c@2 {
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> 

Re: [PATCH 3/5] i2c: mux: pca954x: Add interrupt controller support

[Peter Rosin]

On 2017-01-04 10:29, Phil Reid wrote:
> Various muxes can aggregate multiple interrupts from each i2c bus.
> All of the muxes with interrupt support combine the active low irq lines
> using an internal 'and' function and generate a combined active low
> output. The muxes do provide the ability to read a control register to
> determine which irq is active. By making the mux an irq controller isr
> can potentially be reduced by reading the status register and then only
> calling the registered isr on that bus segment.
> 
> As there is no irq masking on the mux irq are disabled until irq_unmask is
> called at least once.

Irqs are not my strong point, I would prefer if someone else also had
a look. And there are some comments below...

Cheers,
peda

> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
>  drivers/i2c/muxes/i2c-mux-pca954x.c | 126 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 126 insertions(+)
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 981d145..f2dba7c 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -40,14 +40,19 @@
>  #include <linux/i2c.h>
>  #include <linux/i2c-mux.h>
>  #include <linux/i2c/pca954x.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/of_irq.h>
>  #include <linux/pm.h>
>  #include <linux/slab.h>
>  
>  #define PCA954X_MAX_NCHANS 8
>  
> +#define PCA954X_IRQ_OFFSET 4
> +
>  enum pca_type {
>  	pca_9540,
>  	pca_9542,
> @@ -62,6 +67,7 @@ enum pca_type {
>  struct chip_desc {
>  	u8 nchans;
>  	u8 enable;	/* used for muxes only */
> +	u8 has_irq;
>  	enum muxtype {
>  		pca954x_ismux = 0,
>  		pca954x_isswi
> @@ -74,6 +80,9 @@ struct pca954x {
>  	u8 last_chan;		/* last register value */
>  	u8 deselect;
>  	struct i2c_client *client;
> +
> +	struct irq_domain *irq;
> +	unsigned int irq_mask;
>  };
>  
>  /* Provide specs for the PCA954x types we know about */
> @@ -86,19 +95,23 @@ struct pca954x {
>  	[pca_9542] = {
>  		.nchans = 2,
>  		.enable = 0x4,
> +		.has_irq = 1,
>  		.muxtype = pca954x_ismux,
>  	},
>  	[pca_9543] = {
>  		.nchans = 2,
> +		.has_irq = 1,
>  		.muxtype = pca954x_isswi,
>  	},
>  	[pca_9544] = {
>  		.nchans = 4,
>  		.enable = 0x4,
> +		.has_irq = 1,
>  		.muxtype = pca954x_ismux,
>  	},
>  	[pca_9545] = {
>  		.nchans = 4,
> +		.has_irq = 1,
>  		.muxtype = pca954x_isswi,
>  	},
>  	[pca_9547] = {
> @@ -203,6 +216,104 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
>  	return pca954x_reg_write(muxc->parent, client, data->last_chan);
>  }
>  
> +static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
> +{
> +	struct pca954x *data = dev_id;
> +	unsigned int child_irq;
> +	int ret, i, handled;
> +
> +	ret = i2c_smbus_read_byte(data->client);
> +	if (ret < 0)
> +		return IRQ_NONE;
> +
> +	for (i = 0; i < data->chip->nchans; i++) {
> +		if (ret & BIT(PCA954X_IRQ_OFFSET+i)) {

Spaces around the +

> +			child_irq = irq_linear_revmap(data->irq, i);
> +			handle_nested_irq(child_irq);
> +			handled++;
> +		}
> +	}
> +	return handled ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> +static void pca954x_irq_mask(struct irq_data *idata)
> +{
> +	struct pca954x *data = irq_data_get_irq_chip_data(idata);
> +	unsigned int pos = idata->hwirq;
> +
> +	data->irq_mask &= ~BIT(pos);
> +	if (!data->irq_mask)
> +		disable_irq(data->client->irq);
> +}
> +
> +static void pca954x_irq_unmask(struct irq_data *idata)
> +{
> +	struct pca954x *data = irq_data_get_irq_chip_data(idata);
> +	unsigned int pos = idata->hwirq;
> +
> +	if (!data->irq_mask)
> +		enable_irq(data->client->irq);
> +	data->irq_mask |= BIT(pos);
> +}
> +
> +static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
> +{
> +	if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +static struct irq_chip pca954x_irq_chip = {
> +	.name = "i2c-mux-pca954x",
> +	.irq_mask = pca954x_irq_mask,
> +	.irq_unmask = pca954x_irq_unmask,
> +	.irq_set_type = pca954x_irq_set_type,
> +};
> +
> +static int pca953x_irq_setup(struct i2c_mux_core *muxc)
> +{
> +	struct pca954x *data = i2c_mux_priv(muxc);
> +	struct i2c_client *client = data->client;
> +	int c, err, irq;
> +
> +	if (!data->chip->has_irq || client->irq <= 0)
> +		return 0;
> +
> +	data->irq = irq_domain_add_linear(client->dev.of_node,
> +					  data->chip->nchans,
> +					  &irq_domain_simple_ops, data);
> +	if (!data->irq)	{
> +		err = -ENODEV;
> +		goto err_irq_domain;

This label is not needed, just return -ENODEV

> +	}
> +
> +	for (c = 0; c < data->chip->nchans; c++) {
> +		irq = irq_create_mapping(data->irq, c);
> +		irq_set_chip_data(irq, data);
> +		irq_set_chip_and_handler(irq, &pca954x_irq_chip,
> +			handle_simple_irq);
> +	}
> +
> +	err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
> +					pca954x_irq_handler,
> +					IRQF_ONESHOT | IRQF_SHARED,
> +					"pca953x", data);

"pca954x"

> +	if (err)
> +		goto err_req_irq;
> +
> +	disable_irq(data->client->irq);
> +
> +	return 0;
> +err_req_irq:
> +	for (c = 0; c < data->chip->nchans; c++) {
> +		irq = irq_find_mapping(data->irq, c);
> +		irq_dispose_mapping(irq);
> +	}
> +	irq_domain_remove(data->irq);
> +err_irq_domain:
> +	return err;
> +}
> +
>  /*
>   * I2C init/probing/exit functions
>   */
> @@ -258,6 +369,10 @@ static int pca954x_probe(struct i2c_client *client,
>  	idle_disconnect_dt = of_node &&
>  		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>  
> +	ret = pca953x_irq_setup(muxc);
> +	if (ret)
> +		goto irq_setup_failed;
> +
>  	/* Now create an adapter for each channel */
>  	for (num = 0; num < data->chip->nchans; num++) {
>  		bool idle_disconnect_pd = false;
> @@ -294,6 +409,7 @@ static int pca954x_probe(struct i2c_client *client,
>  
>  	return 0;
>  
> +irq_setup_failed:
>  virt_reg_failed:

Rename the virt_reg_failed label to name what is reversed instead of what
happened to fail. E.g. fail_del_adapters, or something.

>  	i2c_mux_del_adapters(muxc);
>  	return ret;
> @@ -302,6 +418,16 @@ static int pca954x_probe(struct i2c_client *client,
>  static int pca954x_remove(struct i2c_client *client)
>  {
>  	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
> +	struct pca954x *data = i2c_mux_priv(muxc);
> +	int c, irq;
> +
> +	if (data->irq) {
> +		for (c = 0; c < data->chip->nchans; c++) {
> +			irq = irq_find_mapping(data->irq, c);
> +			irq_dispose_mapping(irq);
> +		}
> +		irq_domain_remove(data->irq);
> +	}
>  
>  	i2c_mux_del_adapters(muxc);
>  	return 0;
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] dt: bindings: i2c-mux-pca954x: Add documentation for i2c-mux-irq-mask-en

[Peter Rosin]

On 2017-01-04 10:29, Phil Reid wrote:
> Unfortunately some hardware device will assert their irq line immediately
> on power on and provide no mechanism to mask the irq. As the i2c muxes
> provide no method to mask irq line this provides a work around by keeping
> the parent irq masked until enough device drivers have loaded to service
> all pending interrupts.
> 
> For example the the ltc1760 assert its SMBALERT irq immediately on power
> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
> device is registered irq are enabled and fire continuously as the second
> device driver has not yet loaded. Setting this parameter to 0x3 while
> delay the irq being enabled until both devices are ready.
> 
> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> index 9f7c275..d3e63dc 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> @@ -19,6 +19,8 @@ Optional Properties:
>    - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
>      children in idle state. This is necessary for example, if there are several
>      multiplexers on the bus and the devices behind them use same I2C addresses.
> +  - i2c-mux-irq-mask-en: BitMask; Defines a mask for which irq lines need to be
> +    unmasked before the parent irq line in enabled.

The prefix should be "nxp," instead of "i2c-mux-", and I don't think you should
abbreviate enable. So, nxp,irq-mask-enable.

With that fixed,
Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

Cheers,
peda

>    - interrupt-parent: Phandle for the interrupt controller that services
>      interrupts for this device.
>    - interrupts: Interrupt mapping for IRQ.
> @@ -36,6 +38,7 @@ Example:
>  		#size-cells = <0>;
>  		reg = <0x74>;
>  
> +		i2c-mux-irq-mask-en = <0x3>;
>  		interrupt-parent = <&ipic>;
>  		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
>  		interrupt-controller;
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs

[Peter Rosin]

On 2017-01-04 10:29, Phil Reid wrote:
> Unfortunately some hardware device will assert their irq line immediately
> on power on and provide no mechanism to mask the irq. As the i2c muxes
> provide no method to mask irq line this provides a work around by keeping
> the parent irq masked until enough device drivers have loaded to service
> all pending interrupts.
> 
> For example the the ltc1760 assert its SMBALERT irq immediately on power
> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
> device is registered irq are enabled and fire continuously as the second
> device driver has not yet loaded. Setting this parameter to 0x3 while
> delay the irq being enabled until both devices are ready.

I think this also needs a comment in the code, including a description
of the limitations. If the interrupt is shared between two devices on
the same bus, you would have the exact same problem and this workaround
would be no good...

Overall, this series fixes the issues I had with the patch from half
a year ago or so. Thanks!

With nitpicks fixed,
Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

Cheers,
peda

> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
>  drivers/i2c/muxes/i2c-mux-pca954x.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index f2dba7c..16269e7 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -83,6 +83,7 @@ struct pca954x {
>  
>  	struct irq_domain *irq;
>  	unsigned int irq_mask;
> +	unsigned int irq_mask_en;

Spell it out: irq_mask_enable

>  };
>  
>  /* Provide specs for the PCA954x types we know about */
> @@ -251,9 +252,12 @@ static void pca954x_irq_unmask(struct irq_data *idata)
>  	struct pca954x *data = irq_data_get_irq_chip_data(idata);
>  	unsigned int pos = idata->hwirq;
>  
> -	if (!data->irq_mask)
> +	if (!data->irq_mask_en && !data->irq_mask)
>  		enable_irq(data->client->irq);
>  	data->irq_mask |= BIT(pos);
> +	if (data->irq_mask_en &&
> +		(data->irq_mask & data->irq_mask) == data->irq_mask_en)
> +		enable_irq(data->client->irq);
>  }
>  
>  static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
> @@ -369,6 +373,9 @@ static int pca954x_probe(struct i2c_client *client,
>  	idle_disconnect_dt = of_node &&
>  		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>  
> +	of_property_read_u32(of_node, "i2c-mux-irq-mask-en",

"nxp,irq-mask-enable"

> +			     &data->irq_mask_en);
> +
>  	ret = pca953x_irq_setup(muxc);
>  	if (ret)
>  		goto irq_setup_failed;
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] i2c: mux: pca954x: Add interrupt controller support

[Peter Rosin]

On 2017-01-04 15:13, Peter Rosin wrote:
> On 2017-01-04 10:29, Phil Reid wrote:
>> Various muxes can aggregate multiple interrupts from each i2c bus.
>> All of the muxes with interrupt support combine the active low irq lines
>> using an internal 'and' function and generate a combined active low
>> output. The muxes do provide the ability to read a control register to
>> determine which irq is active. By making the mux an irq controller isr
>> can potentially be reduced by reading the status register and then only
>> calling the registered isr on that bus segment.
>>
>> As there is no irq masking on the mux irq are disabled until irq_unmask is
>> called at least once.
> 
> Irqs are not my strong point, I would prefer if someone else also had
> a look. And there are some comments below...
> 
> Cheers,
> peda
> 
>> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
>> ---
>>  drivers/i2c/muxes/i2c-mux-pca954x.c | 126 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 126 insertions(+)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> index 981d145..f2dba7c 100644
>> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
>> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> @@ -40,14 +40,19 @@
>>  #include <linux/i2c.h>
>>  #include <linux/i2c-mux.h>
>>  #include <linux/i2c/pca954x.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>> +#include <linux/of_irq.h>
>>  #include <linux/pm.h>
>>  #include <linux/slab.h>
>>  
>>  #define PCA954X_MAX_NCHANS 8
>>  
>> +#define PCA954X_IRQ_OFFSET 4
>> +
>>  enum pca_type {
>>  	pca_9540,
>>  	pca_9542,
>> @@ -62,6 +67,7 @@ enum pca_type {
>>  struct chip_desc {
>>  	u8 nchans;
>>  	u8 enable;	/* used for muxes only */
>> +	u8 has_irq;
>>  	enum muxtype {
>>  		pca954x_ismux = 0,
>>  		pca954x_isswi
>> @@ -74,6 +80,9 @@ struct pca954x {
>>  	u8 last_chan;		/* last register value */
>>  	u8 deselect;
>>  	struct i2c_client *client;
>> +
>> +	struct irq_domain *irq;
>> +	unsigned int irq_mask;
>>  };
>>  
>>  /* Provide specs for the PCA954x types we know about */
>> @@ -86,19 +95,23 @@ struct pca954x {
>>  	[pca_9542] = {
>>  		.nchans = 2,
>>  		.enable = 0x4,
>> +		.has_irq = 1,
>>  		.muxtype = pca954x_ismux,
>>  	},
>>  	[pca_9543] = {
>>  		.nchans = 2,
>> +		.has_irq = 1,
>>  		.muxtype = pca954x_isswi,
>>  	},
>>  	[pca_9544] = {
>>  		.nchans = 4,
>>  		.enable = 0x4,
>> +		.has_irq = 1,
>>  		.muxtype = pca954x_ismux,
>>  	},
>>  	[pca_9545] = {
>>  		.nchans = 4,
>> +		.has_irq = 1,
>>  		.muxtype = pca954x_isswi,
>>  	},
>>  	[pca_9547] = {
>> @@ -203,6 +216,104 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
>>  	return pca954x_reg_write(muxc->parent, client, data->last_chan);
>>  }
>>  
>> +static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
>> +{
>> +	struct pca954x *data = dev_id;
>> +	unsigned int child_irq;
>> +	int ret, i, handled;
>> +
>> +	ret = i2c_smbus_read_byte(data->client);
>> +	if (ret < 0)
>> +		return IRQ_NONE;
>> +
>> +	for (i = 0; i < data->chip->nchans; i++) {
>> +		if (ret & BIT(PCA954X_IRQ_OFFSET+i)) {
> 
> Spaces around the +
> 
>> +			child_irq = irq_linear_revmap(data->irq, i);
>> +			handle_nested_irq(child_irq);
>> +			handled++;
>> +		}
>> +	}
>> +	return handled ? IRQ_HANDLED : IRQ_NONE;
>> +}
>> +
>> +static void pca954x_irq_mask(struct irq_data *idata)
>> +{
>> +	struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> +	unsigned int pos = idata->hwirq;
>> +
>> +	data->irq_mask &= ~BIT(pos);
>> +	if (!data->irq_mask)
>> +		disable_irq(data->client->irq);
>> +}
>> +
>> +static void pca954x_irq_unmask(struct irq_data *idata)
>> +{
>> +	struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> +	unsigned int pos = idata->hwirq;
>> +
>> +	if (!data->irq_mask)
>> +		enable_irq(data->client->irq);
>> +	data->irq_mask |= BIT(pos);
>> +}
>> +
>> +static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
>> +{
>> +	if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
>> +		return -EINVAL;
>> +	return 0;
>> +}
>> +
>> +static struct irq_chip pca954x_irq_chip = {
>> +	.name = "i2c-mux-pca954x",
>> +	.irq_mask = pca954x_irq_mask,
>> +	.irq_unmask = pca954x_irq_unmask,
>> +	.irq_set_type = pca954x_irq_set_type,
>> +};
>> +
>> +static int pca953x_irq_setup(struct i2c_mux_core *muxc)

Ooop,
pca954x_irq_setup

>> +{
>> +	struct pca954x *data = i2c_mux_priv(muxc);
>> +	struct i2c_client *client = data->client;
>> +	int c, err, irq;
>> +
>> +	if (!data->chip->has_irq || client->irq <= 0)
>> +		return 0;
>> +
>> +	data->irq = irq_domain_add_linear(client->dev.of_node,
>> +					  data->chip->nchans,
>> +					  &irq_domain_simple_ops, data);
>> +	if (!data->irq)	{
>> +		err = -ENODEV;
>> +		goto err_irq_domain;
> 
> This label is not needed, just return -ENODEV
> 
>> +	}
>> +
>> +	for (c = 0; c < data->chip->nchans; c++) {
>> +		irq = irq_create_mapping(data->irq, c);
>> +		irq_set_chip_data(irq, data);
>> +		irq_set_chip_and_handler(irq, &pca954x_irq_chip,
>> +			handle_simple_irq);
>> +	}
>> +
>> +	err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
>> +					pca954x_irq_handler,
>> +					IRQF_ONESHOT | IRQF_SHARED,
>> +					"pca953x", data);
> 
> "pca954x"
> 
>> +	if (err)
>> +		goto err_req_irq;
>> +
>> +	disable_irq(data->client->irq);
>> +
>> +	return 0;
>> +err_req_irq:
>> +	for (c = 0; c < data->chip->nchans; c++) {
>> +		irq = irq_find_mapping(data->irq, c);
>> +		irq_dispose_mapping(irq);
>> +	}
>> +	irq_domain_remove(data->irq);
>> +err_irq_domain:
>> +	return err;
>> +}
>> +
>>  /*
>>   * I2C init/probing/exit functions
>>   */
>> @@ -258,6 +369,10 @@ static int pca954x_probe(struct i2c_client *client,
>>  	idle_disconnect_dt = of_node &&
>>  		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>>  
>> +	ret = pca953x_irq_setup(muxc);
>> +	if (ret)
>> +		goto irq_setup_failed;
>> +
>>  	/* Now create an adapter for each channel */
>>  	for (num = 0; num < data->chip->nchans; num++) {
>>  		bool idle_disconnect_pd = false;
>> @@ -294,6 +409,7 @@ static int pca954x_probe(struct i2c_client *client,
>>  
>>  	return 0;
>>  
>> +irq_setup_failed:
>>  virt_reg_failed:
> 
> Rename the virt_reg_failed label to name what is reversed instead of what
> happened to fail. E.g. fail_del_adapters, or something.
> 
>>  	i2c_mux_del_adapters(muxc);
>>  	return ret;
>> @@ -302,6 +418,16 @@ static int pca954x_probe(struct i2c_client *client,
>>  static int pca954x_remove(struct i2c_client *client)
>>  {
>>  	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>> +	struct pca954x *data = i2c_mux_priv(muxc);
>> +	int c, irq;
>> +
>> +	if (data->irq) {
>> +		for (c = 0; c < data->chip->nchans; c++) {
>> +			irq = irq_find_mapping(data->irq, c);
>> +			irq_dispose_mapping(irq);
>> +		}
>> +		irq_domain_remove(data->irq);
>> +	}
>>  
>>  	i2c_mux_del_adapters(muxc);
>>  	return 0;
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] dt: bindings: i2c-mux-pca954x: Add documentation for interrupt controller

[Rob Herring]

On Wed, Jan 04, 2017 at 05:29:54PM +0800, Phil Reid wrote:
> Various muxes can aggregate multiple irq lines and provide a control
> register to determine the active line. Add bindings for interrupt
> controller support.
> 
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> index cf53d5f..9f7c275 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> @@ -19,7 +19,14 @@ Optional Properties:
>    - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
>      children in idle state. This is necessary for example, if there are several
>      multiplexers on the bus and the devices behind them use same I2C addresses.
> -
> +  - interrupt-parent: Phandle for the interrupt controller that services
> +    interrupts for this device.
> +  - interrupts: Interrupt mapping for IRQ.
> +  - interrupt-controller: Marks the device node as a interrupt controller.
> +  - #interrupt-cells : Should be two.
> +    - first cell is the pin number
> +    - second cell is used to specify flags.
> +    See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>  
>  Example:
>  
> @@ -29,6 +36,11 @@ Example:
>  		#size-cells = <0>;
>  		reg = <0x74>;
>  
> +		interrupt-parent = <&ipic>;
> +		interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> +		interrupt-controller;
> +		#interrupt-cells=<2>;

Needs spaces around the '='. With that,

Acked-by: Rob Herring <robh@kernel.org>

> +
>  		i2c@2 {
>  			#address-cells = <1>;
>  			#size-cells = <0>;
> -- 
> 1.8.3.1
> 

Re: [PATCH 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs

[Phil Reid]

On 4/01/2017 22:21, Peter Rosin wrote:
> On 2017-01-04 10:29, Phil Reid wrote:
>> Unfortunately some hardware device will assert their irq line immediately
>> on power on and provide no mechanism to mask the irq. As the i2c muxes
>> provide no method to mask irq line this provides a work around by keeping
>> the parent irq masked until enough device drivers have loaded to service
>> all pending interrupts.
>>
>> For example the the ltc1760 assert its SMBALERT irq immediately on power
>> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
>> device is registered irq are enabled and fire continuously as the second
>> device driver has not yet loaded. Setting this parameter to 0x3 while
>> delay the irq being enabled until both devices are ready.
>
> I think this also needs a comment in the code, including a description
> of the limitations. If the interrupt is shared between two devices on
> the same bus, you would have the exact same problem and this workaround
> would be no good...
>
> Overall, this series fixes the issues I had with the patch from half
> a year ago or so. Thanks!
>
> With nitpicks fixed,
> Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
>
G'day Pater,

Thanks for the review. v2 on it's way.


-- 
Regards
Phil Reid

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html