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

[PATCH v6 0/3] 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.

Changes from v5:
- p3 Added Peter's Ack.
- Drop p4/5

Changes from v4:
- p4: Change definition of irq_mask_enable to an array.
- p4: Removed acks due to change requested by Peter
- p5: Parse array of enables. Currently only supports 1 chip
      But dt specification will allow expansion to handle
      multple irq consume chips to be registered on a bus segment
- p5: Fix up logic related to enabling and disable irq's.
      Use a flag to indicate when irq has been enabled.

Changes from v3:
- p3: Add spin lock to irq mask / unmask.
- p4: Add Rob's ack.

Changes from v2:
- p1: Added Acked-by
- p5: fixup 2 typos

Changes from v1:
- Update for new ACPI table
- Fix typo in documentation
- Fix typo in function names
- Fix typo in irq name
- Added spaces around '+' / '='
- Change goto label names
- Change property name from i2c-mux-irq-mask-en to nxp,irq-mask-enable
- Change variable name irq_mask_en to irq_mask_enable
- Add commentt about irq_mask_enable
- Added Acked-By's
Phil Reid (3):
  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

 .../devicetree/bindings/i2c/i2c-mux-pca954x.txt    |  14 +-
 drivers/i2c/muxes/i2c-mux-pca954x.c                | 150 ++++++++++++++++++++-
 2 files changed, 159 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[PATCH v6 1/3] 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.

Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index dd18b9c..bbf088e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -84,6 +84,11 @@ struct pca954x {
 		.enable = 0x4,
 		.muxtype = pca954x_ismux,
 	},
+	[pca_9542] = {
+		.nchans = 2,
+		.enable = 0x4,
+		.muxtype = pca954x_ismux,
+	},
 	[pca_9543] = {
 		.nchans = 2,
 		.muxtype = pca954x_isswi,
@@ -110,7 +115,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 },
@@ -124,7 +129,7 @@ struct pca954x {
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id pca954x_acpi_ids[] = {
 	{ .id = "PCA9540", .driver_data = pca_9540 },
-	{ .id = "PCA9542", .driver_data = pca_9540 },
+	{ .id = "PCA9542", .driver_data = pca_9542 },
 	{ .id = "PCA9543", .driver_data = pca_9543 },
 	{ .id = "PCA9544", .driver_data = pca_9544 },
 	{ .id = "PCA9545", .driver_data = pca_9545 },
-- 
1.8.3.1

--
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

[PATCH v6 2/3] 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.

Acked-by: Peter Rosin <peda@axentia.se>
Acked-by: Rob Herring <robh@kernel.org>
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..aa09704 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 an 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 v6 3/3] 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
latency 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.

Acked-by: Peter Rosin <peda@axentia.se>
Signed-off-by: Phil Reid <preid@electromag.com.au>
---
 drivers/i2c/muxes/i2c-mux-pca954x.c | 141 +++++++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index bbf088e..f55da88 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -41,14 +41,20 @@
 #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>
+#include <linux/spinlock.h>
 
 #define PCA954X_MAX_NCHANS 8
 
+#define PCA954X_IRQ_OFFSET 4
+
 enum pca_type {
 	pca_9540,
 	pca_9542,
@@ -63,6 +69,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
@@ -75,6 +82,10 @@ struct pca954x {
 	u8 last_chan;		/* last register value */
 	u8 deselect;
 	struct i2c_client *client;
+
+	struct irq_domain *irq;
+	unsigned int irq_mask;
+	spinlock_t lock;
 };
 
 /* Provide specs for the PCA954x types we know about */
@@ -87,19 +98,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] = {
@@ -222,6 +237,114 @@ 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;
+	unsigned long flags;
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	data->irq_mask &= ~BIT(pos);
+	if (!data->irq_mask)
+		disable_irq(data->client->irq);
+
+	spin_unlock_irqrestore(&data->lock, flags);
+}
+
+static void pca954x_irq_unmask(struct irq_data *idata)
+{
+	struct pca954x *data = irq_data_get_irq_chip_data(idata);
+	unsigned int pos = idata->hwirq;
+	unsigned long flags;
+
+	spin_lock_irqsave(&data->lock, flags);
+
+	if (!data->irq_mask)
+		enable_irq(data->client->irq);
+	data->irq_mask |= BIT(pos);
+
+	spin_unlock_irqrestore(&data->lock, flags);
+}
+
+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 pca954x_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;
+
+	spin_lock_init(&data->lock);
+
+	data->irq = irq_domain_add_linear(client->dev.of_node,
+					  data->chip->nchans,
+					  &irq_domain_simple_ops, data);
+	if (!data->irq)
+		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,
+					"pca954x", 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);
+
+	return err;
+}
+
 /*
  * I2C init/probing/exit functions
  */
@@ -286,6 +409,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 = pca954x_irq_setup(muxc);
+	if (ret)
+		goto fail_del_adapters;
+
 	/* Now create an adapter for each channel */
 	for (num = 0; num < data->chip->nchans; num++) {
 		bool idle_disconnect_pd = false;
@@ -311,7 +438,7 @@ static int pca954x_probe(struct i2c_client *client,
 			dev_err(&client->dev,
 				"failed to register multiplexed adapter"
 				" %d as bus %d\n", num, force);
-			goto virt_reg_failed;
+			goto fail_del_adapters;
 		}
 	}
 
@@ -322,7 +449,7 @@ static int pca954x_probe(struct i2c_client *client,
 
 	return 0;
 
-virt_reg_failed:
+fail_del_adapters:
 	i2c_mux_del_adapters(muxc);
 	return ret;
 }
@@ -330,6 +457,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

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

[Peter Rosin]

On 2017-01-25 02:31, 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
> latency 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.
> 
> Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
>  drivers/i2c/muxes/i2c-mux-pca954x.c | 141 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 139 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index bbf088e..f55da88 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -41,14 +41,20 @@
>  #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>
> +#include <linux/spinlock.h>
>  
>  #define PCA954X_MAX_NCHANS 8
>  
> +#define PCA954X_IRQ_OFFSET 4
> +
>  enum pca_type {
>  	pca_9540,
>  	pca_9542,
> @@ -63,6 +69,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
> @@ -75,6 +82,10 @@ struct pca954x {
>  	u8 last_chan;		/* last register value */
>  	u8 deselect;
>  	struct i2c_client *client;
> +
> +	struct irq_domain *irq;
> +	unsigned int irq_mask;
> +	spinlock_t lock;
>  };
>  
>  /* Provide specs for the PCA954x types we know about */
> @@ -87,19 +98,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] = {
> @@ -222,6 +237,114 @@ 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;

See below.

> +
> +	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;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	data->irq_mask &= ~BIT(pos);
> +	if (!data->irq_mask)
> +		disable_irq(data->client->irq);
> +
> +	spin_unlock_irqrestore(&data->lock, flags);
> +}
> +
> +static void pca954x_irq_unmask(struct irq_data *idata)
> +{
> +	struct pca954x *data = irq_data_get_irq_chip_data(idata);
> +	unsigned int pos = idata->hwirq;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&data->lock, flags);
> +
> +	if (!data->irq_mask)
> +		enable_irq(data->client->irq);
> +	data->irq_mask |= BIT(pos);
> +
> +	spin_unlock_irqrestore(&data->lock, flags);
> +}
> +
> +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 pca954x_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;
> +
> +	spin_lock_init(&data->lock);
> +
> +	data->irq = irq_domain_add_linear(client->dev.of_node,
> +					  data->chip->nchans,
> +					  &irq_domain_simple_ops, data);
> +	if (!data->irq)
> +		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,
> +					"pca954x", 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);
> +
> +	return err;
> +}
> +
>  /*
>   * I2C init/probing/exit functions
>   */
> @@ -286,6 +409,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 = pca954x_irq_setup(muxc);
> +	if (ret)
> +		goto fail_del_adapters;
> +
>  	/* Now create an adapter for each channel */
>  	for (num = 0; num < data->chip->nchans; num++) {
>  		bool idle_disconnect_pd = false;
> @@ -311,7 +438,7 @@ static int pca954x_probe(struct i2c_client *client,
>  			dev_err(&client->dev,
>  				"failed to register multiplexed adapter"
>  				" %d as bus %d\n", num, force);
> -			goto virt_reg_failed;
> +			goto fail_del_adapters;
>  		}
>  	}
>  
> @@ -322,7 +449,7 @@ static int pca954x_probe(struct i2c_client *client,
>  
>  	return 0;
>  
> -virt_reg_failed:
> +fail_del_adapters:
>  	i2c_mux_del_adapters(muxc);
>  	return ret;
>  }
> @@ -330,6 +457,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;
> 

This patch needs this obvious fixup discovered by the kbuild bot

Cheers,
peda

--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -242,7 +242,7 @@ static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
 {
        struct pca954x *data = dev_id;
        unsigned int child_irq;
-       int ret, i, handled;
+       int ret, i, handled = 0;

        ret = i2c_smbus_read_byte(data->client);
        if (ret < 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 v6 0/3] i2c: mux: pca954x: Add interrupt controller support

[Peter Rosin]

Hi Wolfram!

I had prepared a branch for you to pull but then the kbuild
bot finally found this series and complained about the missing
initializer for the 'handled' variable. I amended the previously
posted fixup and was just about to send you the pull request when
you started to add patches to i2c/for-next. What is now left of what
I originally had in i2c-mux/for-next is this series. However, since
I told Phil that I amended the fixup I do not think he will resend
and I think you might therefore be waiting for a resend which will
probably not happen (anytime soon). I'm going to short out the wait
and just send a reworked pull request. See below.

A few thing to take away from this. Phil didn't post the series to
LKML, and the kbuild bots didn't find the series until I added it to
my i2c-mux tree. Fengguang indicated that the kbuild bot will now
(or soon) start to track the i2c list, which might be good to know.

I also think we need to come up with something that prevents us from
doing work twice. I think the main problem is at my end, for not
being clear enough about my intentions with an i2c-mux related
submission. So, from now on I think I'm just going to change my
acks to some message saying that the patch(es) have been added
to i2c-mux/for-next (in case the submission is clear-cut i2c-mux
and not at all about i2c-the-rest). If I just ack a submission I'll
expect you to pick it up. Ok?

The question then becomes at approximately which point you'll need
a pull request? Or should you perhaps be pulling in my tree
on a more continuous level so that everything i2c-related is
available in one tree (i.e. your tree)?

One thing that happened last time you pulled from the i2c-mux tree
was that you added your sign-offs, and that makes the i2c-mux tree
into a kind of 2nd rate tree that is not useful for much more than
feeding patches upstream. I have to force-push after each of your
pulls. I think (at least some) other 2nd level maintainers do not
"suffer" from this fate? Anyway, do you really need to add that
sign-off when you pull? It's not a big thing, but all things being
equal, I'd prefer the commits to stay as-is...

Cheers,
peda

The following changes since commit 7a308bb3016f57e5be11a677d15b821536419d36:

  Linux 4.10-rc5 (2017-01-22 12:54:15 -0800)

are available in the git repository at:

  https://github.com/peda-r/i2c-mux.git i2c-mux/for-next

for you to fetch changes up to f2114795f721bd5028284ddf84b150798a9b7a73:

  i2c: mux: pca954x: Add interrupt controller support (2017-02-10 08:23:51 +0100)

----------------------------------------------------------------
Phil Reid (3):
      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

 Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt |  14 +++++-
 drivers/i2c/muxes/i2c-mux-pca954x.c                       | 150 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 159 insertions(+), 5 deletions(-)


On 2017-01-25 02:31, 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
> latenct can potentially be reduced by reading the status register and 
> then only calling the registered isr on that bus segment.
> 
> Changes from v5:
> - p3 Added Peter's Ack.
> - Drop p4/5
> 
> Changes from v4:
> - p4: Change definition of irq_mask_enable to an array.
> - p4: Removed acks due to change requested by Peter
> - p5: Parse array of enables. Currently only supports 1 chip
>       But dt specification will allow expansion to handle
>       multple irq consume chips to be registered on a bus segment
> - p5: Fix up logic related to enabling and disable irq's.
>       Use a flag to indicate when irq has been enabled.
> 
> Changes from v3:
> - p3: Add spin lock to irq mask / unmask.
> - p4: Add Rob's ack.
> 
> Changes from v2:
> - p1: Added Acked-by
> - p5: fixup 2 typos
> 
> Changes from v1:
> - Update for new ACPI table
> - Fix typo in documentation
> - Fix typo in function names
> - Fix typo in irq name
> - Added spaces around '+' / '='
> - Change goto label names
> - Change property name from i2c-mux-irq-mask-en to nxp,irq-mask-enable
> - Change variable name irq_mask_en to irq_mask_enable
> - Add commentt about irq_mask_enable
> - Added Acked-By's
> Phil Reid (3):
>   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
> 
>  .../devicetree/bindings/i2c/i2c-mux-pca954x.txt    |  14 +-
>  drivers/i2c/muxes/i2c-mux-pca954x.c                | 150 ++++++++++++++++++++-
>  2 files changed, 159 insertions(+), 5 deletions(-)
> 

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2871 bytes --]

Hi Peter,

> initializer for the 'handled' variable. I amended the previously
> posted fixup and was just about to send you the pull request when
> you started to add patches to i2c/for-next.

Oh, sorry! I wasn't sure if we agreed that you send pull-requests for
v4.11 already or starting with v4.12. And since there seems to be no rc8
for 4.11 I decided to start pulling in.

> probably not happen (anytime soon). I'm going to short out the wait
> and just send a reworked pull request. See below.

Perfect!

> A few thing to take away from this. Phil didn't post the series to
> LKML, and the kbuild bots didn't find the series until I added it to
> my i2c-mux tree. Fengguang indicated that the kbuild bot will now
> (or soon) start to track the i2c list, which might be good to know.

Well, I asked Fengguang to remove the i2c list from that feature a
while ago. People send out RFCs, debug patches, etc... and for all those
build bot reports are just noise IMO.

Build testing plus sparse+smatch testing for the patches when I apply
them to my tree and then having my branches additionally checked by
build bot works well for me. I will happily share my super-simple
'ninja-check' script with you which runs various code checkers when
compiling. It is basically adding "W=1 C=1 CHECK='ninja-check'" to the
kernel build command-line.

I hope you are open for this workflow. But we can discuss, of course.

> submission. So, from now on I think I'm just going to change my
> acks to some message saying that the patch(es) have been added
> to i2c-mux/for-next (in case the submission is clear-cut i2c-mux
> and not at all about i2c-the-rest). If I just ack a submission I'll
> expect you to pick it up. Ok?

Perfect!

> The question then becomes at approximately which point you'll need
> a pull request? Or should you perhaps be pulling in my tree
> on a more continuous level so that everything i2c-related is
> available in one tree (i.e. your tree)?

I prefer pull requests a little bit. Then, I get a consistent state
approved by you and already checked by build bot. BTW is your tree in
linux-next as well?

> sign-off when you pull? It's not a big thing, but all things being
> equal, I'd prefer the commits to stay as-is...

Sure thing. It might have been done automatically, will check and fix my
scripts...

> 
> Cheers,
> peda
> 
> The following changes since commit 7a308bb3016f57e5be11a677d15b821536419d36:
> 
>   Linux 4.10-rc5 (2017-01-22 12:54:15 -0800)
> 
> are available in the git repository at:
> 
>   https://github.com/peda-r/i2c-mux.git i2c-mux/for-next
> 
> for you to fetch changes up to f2114795f721bd5028284ddf84b150798a9b7a73:
> 
>   i2c: mux: pca954x: Add interrupt controller support (2017-02-10 08:23:51 +0100)

And pulled! Thank you for the work!

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Peter Rosin]

On 2017-02-10 13:52, Wolfram Sang wrote:
> Hi Peter,
> 
>> initializer for the 'handled' variable. I amended the previously
>> posted fixup and was just about to send you the pull request when
>> you started to add patches to i2c/for-next.
> 
> Oh, sorry! I wasn't sure if we agreed that you send pull-requests for
> v4.11 already or starting with v4.12. And since there seems to be no rc8
> for 4.11 I decided to start pulling in.

FWIW, I think you did the right thing, and it was me that should have
been clearer from the start. No big thing, just a few patches.

> Build testing plus sparse+smatch testing for the patches when I apply
> them to my tree and then having my branches additionally checked by
> build bot works well for me. I will happily share my super-simple
> 'ninja-check' script with you which runs various code checkers when
> compiling. It is basically adding "W=1 C=1 CHECK='ninja-check'" to the
> kernel build command-line.
> 
> I hope you are open for this workflow. But we can discuss, of course.

No trouble at all, and I'll certainly have a look at the script, so
please send it my way. Thanks!

>> The question then becomes at approximately which point you'll need
>> a pull request? Or should you perhaps be pulling in my tree
>> on a more continuous level so that everything i2c-related is
>> available in one tree (i.e. your tree)?
> 
> I prefer pull requests a little bit. Then, I get a consistent state
> approved by you and already checked by build bot. BTW is your tree in
> linux-next as well?

Nope, it's not, but I intend to fix that for the next round. Ok, I think
we have a sane plan, people should be testing linux-next anyway...

Cheers,
Peter

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1671 bytes --]


> No trouble at all, and I'll certainly have a look at the script, so
> please send it my way. Thanks!

Some notes first:: sparse and smatch are probably the most useful for
kernel builds. spatch (=coccinelle) is nice, too, but needs quite a bit
more time for checking. cppcheck occasionally finds something that the
others don't, flawfinder not really. There are some false positives,
too, so you need to get a bit used to read the output. So, here you go:

=== ninja-check
#!/bin/sh -u
# wrapper to call various static checkers for kernel builds.
# Use: make C=1 CHECK='ninja-check' ...
# done by Wolfram Sang in 2012-14, version 20140514 - WTFPLv2

check_for()
{
	command -v $1 > /dev/null
	ret=$?
	[ $ret -eq 0 ] && echo "    $1" | tr a-z A-Z
	return $ret
}

# Get filename (last argument)
eval file_to_check=\${$#}

check_for sparse && sparse -Wsparse-all "$@"

check_for smatch && smatch --two-passes --project=kernel "$@" 1>&2

# Don't provide include-dirs since number of code paths increases drastically (#defines!) and '-f' checks all of them. Just suppress the warning.
check_for cppcheck && cppcheck -f -q --platform=unix64 --template=gcc --enable=all --language=c --suppress=missingInclude --suppress=clarifyCalculation --suppress=unmatchedSuppression --suppress=variableScope "$file_to_check"

check_for spatch && MODE=report scripts/coccicheck "$file_to_check" 1>&2

check_for flawfinder && flawfinder --minlevel=0 --quiet --dataonly --singleline "$file_to_check" 1>&2

# RATS mainly checks for dangerous functions. Not so useful for kernel analysis. flawfinder does string checking, too.
#check_for rats && rats --resultsonly -w 3 "$file_to_check" 1>&2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]