[PATCH v5] gpio: pcf857x: Add OF support

[PATCH v5] gpio: pcf857x: Add OF support

[Laurent Pinchart]

Add DT bindings for the pcf857x-compatible chips and parse the device
tree node in the driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 71 ++++++++++++++++++++++
 drivers/gpio/gpio-pcf857x.c                        | 44 +++++++++++---
 2 files changed, 107 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt

Changes since v4:

- Don't try to get ngpio from of_device_id data, we already get it from
  i2c_device_id

Changes since v3:

- Get rid of the #ifdef CONFIG_OF in the probe function
- Give DT node priority over platform data

Changes since v2:

- Replace mention about interrupts software configuration in DT bindings
  documentation with an explanation of the hardware configuration.

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
new file mode 100644
index 0000000..d261391
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
@@ -0,0 +1,71 @@
+* PCF857x-compatible I/O expanders
+
+The PCF857x-compatible chips have "quasi-bidirectional" I/O pins that can be
+driven high by a pull-up current source or driven low to ground. This combines
+the direction and output level into a single bit per pin, which can't be read
+back. We can't actually know at initialization time whether a pin is configured
+(a) as output and driving the signal low/high, or (b) as input and reporting a
+low/high value, without knowing the last value written since the chip came out
+of reset (if any). The only reliable solution for setting up pin direction is
+thus to do it explicitly.
+
+Required Properties:
+
+  - compatible: should be one of the following.
+    - "maxim,max7328": For the Maxim MAX7378
+    - "maxim,max7329": For the Maxim MAX7329
+    - "nxp,pca8574": For the NXP PCA8574
+    - "nxp,pca8575": For the NXP PCA8575
+    - "nxp,pca9670": For the NXP PCA9670
+    - "nxp,pca9671": For the NXP PCA9671
+    - "nxp,pca9672": For the NXP PCA9672
+    - "nxp,pca9673": For the NXP PCA9673
+    - "nxp,pca9674": For the NXP PCA9674
+    - "nxp,pca9675": For the NXP PCA9675
+    - "nxp,pcf8574": For the NXP PCF8574
+    - "nxp,pcf8574a": For the NXP PCF8574A
+    - "nxp,pcf8575": For the NXP PCF8575
+    - "ti,tca9554": For the TI TCA9554
+
+  - reg: I2C slave address.
+
+  - gpio-controller: Marks the device node as a gpio controller.
+  - #gpio-cells: Should be 2. The first cell is the GPIO number and the second
+    cell specifies GPIO flags, as defined in <dt-bindings/gpio/gpio.h>. Only the
+    GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
+
+Optional Properties:
+
+  - pins-initial-state: Bitmask that specifies the initial state of each pin.
+  When a bit is set to zero, the corresponding pin will be initialized to the
+  input (pulled-up) state. When the  bit is set to one, the pin will be
+  initialized the the low-level output state. If the property is not specified
+  all pins will be initialized to the input state.
+
+  The I/O expander can detect input state changes, and thus optionally act as
+  an interrupt controller. When the expander interrupt pin is connected all the
+  following properties must be set. For more information please see the
+  interrupt controller device tree bindings documentation available at
+  Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
+
+  - interrupt-controller: Identifies the node as an interrupt controller.
+  - #interrupt-cells: Number of cells to encode an interrupt source, shall be 2.
+  - interrupt-parent: phandle of the parent interrupt controller.
+  - interrupts: Interrupt specifier for the controllers interrupt.
+
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example: PCF8575 I/O expander node
+
+	pcf8575: gpio@20 {
+		compatible = "nxp,pcf8575";
+		reg = <0x20>;
+		interrupt-parent = <&irqpin2>;
+		interrupts = <3 0>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+	};
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 9e61bb0..864dd8c 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -26,6 +26,8 @@
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
@@ -50,6 +52,27 @@ static const struct i2c_device_id pcf857x_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, pcf857x_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id pcf857x_of_table[] = {
+	{ .compatible = "nxp,pcf8574" },
+	{ .compatible = "nxp,pcf8574a" },
+	{ .compatible = "nxp,pca8574" },
+	{ .compatible = "nxp,pca9670" },
+	{ .compatible = "nxp,pca9672" },
+	{ .compatible = "nxp,pca9674" },
+	{ .compatible = "nxp,pcf8575" },
+	{ .compatible = "nxp,pca8575" },
+	{ .compatible = "nxp,pca9671" },
+	{ .compatible = "nxp,pca9673" },
+	{ .compatible = "nxp,pca9675" },
+	{ .compatible = "maxim,max7328" },
+	{ .compatible = "maxim,max7329" },
+	{ .compatible = "ti,tca9554" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pcf857x_of_table);
+#endif
+
 /*
  * The pcf857x, pca857x, and pca967x chips only expose one read and one
  * write register.  Writing a "one" bit (to match the reset state) lets
@@ -257,14 +280,18 @@ fail:
 static int pcf857x_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
-	struct pcf857x_platform_data	*pdata;
+	struct pcf857x_platform_data	*pdata = dev_get_platdata(&client->dev);
+	struct device_node		*np = client->dev.of_node;
 	struct pcf857x			*gpio;
+	unsigned int			n_latch = 0;
 	int				status;
 
-	pdata = dev_get_platdata(&client->dev);
-	if (!pdata) {
+	if (IS_ENABLED(CONFIG_OF) && np)
+		of_property_read_u32(np, "pins-initial-state", &n_latch);
+	else if (pdata)
+		n_latch = pdata->n_latch;
+	else
 		dev_dbg(&client->dev, "no platform data\n");
-	}
 
 	/* Allocate, initialize, and register this gpio_chip. */
 	gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
@@ -357,11 +384,11 @@ static int pcf857x_probe(struct i2c_client *client,
 	 * may cause transient glitching since it can't know the last value
 	 * written (some pins may need to be driven low).
 	 *
-	 * Using pdata->n_latch avoids that trouble.  When left initialized
-	 * to zero, our software copy of the "latch" then matches the chip's
-	 * all-ones reset state.  Otherwise it flags pins to be driven low.
+	 * Using n_latch avoids that trouble.  When left initialized to zero,
+	 * our software copy of the "latch" then matches the chip's all-ones
+	 * reset state.  Otherwise it flags pins to be driven low.
 	 */
-	gpio->out = pdata ? ~pdata->n_latch : ~0;
+	gpio->out = ~n_latch;
 	gpio->status = gpio->out;
 
 	status = gpiochip_add(&gpio->chip);
@@ -423,6 +450,7 @@ static struct i2c_driver pcf857x_driver = {
 	.driver = {
 		.name	= "pcf857x",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(pcf857x_of_table),
 	},
 	.probe	= pcf857x_probe,
 	.remove	= pcf857x_remove,
-- 
Regards,

Laurent Pinchart

Re: [PATCH v5] gpio: pcf857x: Add OF support

[Tomasz Figa]

Hi Laurent,

On Tuesday 27 of August 2013 10:02:39 Laurent Pinchart wrote:
> Add DT bindings for the pcf857x-compatible chips and parse the device
> tree node in the driver.
> 
> Signed-off-by: Laurent Pinchart
> <laurent.pinchart+renesas@ideasonboard.com> ---
>  .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 71
> ++++++++++++++++++++++ drivers/gpio/gpio-pcf857x.c                     
>   | 44 +++++++++++--- 2 files changed, 107 insertions(+), 8
> deletions(-)
>  create mode 100644
> Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> 
> Changes since v4:
> 
> - Don't try to get ngpio from of_device_id data, we already get it from
>   i2c_device_id

Hmm, I'm not sure how this is supposed to work.

How does the I2C core resolve OF compatible name to particular entry in 
id_table? I believe it simply passes NULL as the second argument of 
.probe() if the device is instantiated based on OF compatible string and 
not one in the legacy ID table.

> 
> Changes since v3:
> 
> - Get rid of the #ifdef CONFIG_OF in the probe function
> - Give DT node priority over platform data
> 
> Changes since v2:
> 
> - Replace mention about interrupts software configuration in DT bindings
> documentation with an explanation of the hardware configuration.
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt new file mode
> 100644
> index 0000000..d261391
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> @@ -0,0 +1,71 @@
> +* PCF857x-compatible I/O expanders
> +
> +The PCF857x-compatible chips have "quasi-bidirectional" I/O pins that
> can be +driven high by a pull-up current source or driven low to
> ground. This combines +the direction and output level into a single bit
> per pin, which can't be read +back. We can't actually know at
> initialization time whether a pin is configured +(a) as output and
> driving the signal low/high, or (b) as input and reporting a +low/high
> value, without knowing the last value written since the chip came out
> +of reset (if any). The only reliable solution for setting up pin
> direction is +thus to do it explicitly.
> +
> +Required Properties:
> +
> +  - compatible: should be one of the following.
> +    - "maxim,max7328": For the Maxim MAX7378
> +    - "maxim,max7329": For the Maxim MAX7329
> +    - "nxp,pca8574": For the NXP PCA8574
> +    - "nxp,pca8575": For the NXP PCA8575
> +    - "nxp,pca9670": For the NXP PCA9670
> +    - "nxp,pca9671": For the NXP PCA9671
> +    - "nxp,pca9672": For the NXP PCA9672
> +    - "nxp,pca9673": For the NXP PCA9673
> +    - "nxp,pca9674": For the NXP PCA9674
> +    - "nxp,pca9675": For the NXP PCA9675
> +    - "nxp,pcf8574": For the NXP PCF8574
> +    - "nxp,pcf8574a": For the NXP PCF8574A
> +    - "nxp,pcf8575": For the NXP PCF8575
> +    - "ti,tca9554": For the TI TCA9554
> +
> +  - reg: I2C slave address.
> +
> +  - gpio-controller: Marks the device node as a gpio controller.
> +  - #gpio-cells: Should be 2. The first cell is the GPIO number and the
> second +    cell specifies GPIO flags, as defined in
> <dt-bindings/gpio/gpio.h>. Only the +    GPIO_ACTIVE_HIGH and
> GPIO_ACTIVE_LOW flags are supported. +
> +Optional Properties:
> +
> +  - pins-initial-state: Bitmask that specifies the initial state of
> each pin. +  When a bit is set to zero, the corresponding pin will be
> initialized to the +  input (pulled-up) state. When the  bit is set to
> one, the pin will be +  initialized the the low-level output state. If
> the property is not specified +  all pins will be initialized to the
> input state.
> +
> +  The I/O expander can detect input state changes, and thus optionally
> act as +  an interrupt controller. When the expander interrupt pin is
> connected all the +  following properties must be set. For more
> information please see the +  interrupt controller device tree bindings
> documentation available at + 
> Documentation/devicetree/bindings/interrupt-controller/interrupts.txt.
> +
> +  - interrupt-controller: Identifies the node as an interrupt
> controller. +  - #interrupt-cells: Number of cells to encode an
> interrupt source, shall be 2. +  - interrupt-parent: phandle of the
> parent interrupt controller. +  - interrupts: Interrupt specifier for
> the controllers interrupt. +
> +
> +Please refer to gpio.txt in this directory for details of the common
> GPIO +bindings used by client devices.
> +
> +Example: PCF8575 I/O expander node
> +
> +	pcf8575: gpio@20 {
> +		compatible = "nxp,pcf8575";
> +		reg = <0x20>;
> +		interrupt-parent = <&irqpin2>;
> +		interrupts = <3 0>;
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +	};
> diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
> index 9e61bb0..864dd8c 100644
> --- a/drivers/gpio/gpio-pcf857x.c
> +++ b/drivers/gpio/gpio-pcf857x.c
> @@ -26,6 +26,8 @@
>  #include <linux/irqdomain.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/workqueue.h>
> @@ -50,6 +52,27 @@ static const struct i2c_device_id pcf857x_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, pcf857x_id);
> 
> +#ifdef CONFIG_OF
> +static const struct of_device_id pcf857x_of_table[] = {
> +	{ .compatible = "nxp,pcf8574" },
> +	{ .compatible = "nxp,pcf8574a" },
> +	{ .compatible = "nxp,pca8574" },
> +	{ .compatible = "nxp,pca9670" },
> +	{ .compatible = "nxp,pca9672" },
> +	{ .compatible = "nxp,pca9674" },
> +	{ .compatible = "nxp,pcf8575" },
> +	{ .compatible = "nxp,pca8575" },
> +	{ .compatible = "nxp,pca9671" },
> +	{ .compatible = "nxp,pca9673" },
> +	{ .compatible = "nxp,pca9675" },
> +	{ .compatible = "maxim,max7328" },
> +	{ .compatible = "maxim,max7329" },
> +	{ .compatible = "ti,tca9554" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, pcf857x_of_table);
> +#endif
> +
>  /*
>   * The pcf857x, pca857x, and pca967x chips only expose one read and one
> * write register.  Writing a "one" bit (to match the reset state) lets
> @@ -257,14 +280,18 @@ fail:
>  static int pcf857x_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> -	struct pcf857x_platform_data	*pdata;
> +	struct pcf857x_platform_data	*pdata = dev_get_platdata(&client-
>dev);
> +	struct device_node		*np = client->dev.of_node;
>  	struct pcf857x			*gpio;
> +	unsigned int			n_latch = 0;
>  	int				status;
> 
> -	pdata = dev_get_platdata(&client->dev);
> -	if (!pdata) {
> +	if (IS_ENABLED(CONFIG_OF) && np)
> +		of_property_read_u32(np, "pins-initial-state", &n_latch);

I believe the convention is that platform data should be favoured, as you 
can pass it even if DT is present, using auxdata table.

So this should rather be:

	if (pdata)
		n_latch = pdata->n_latch;
	else if (IS_ENABLED(CONFIG_OF) && np)
		of_property_read_u32(np, "pins-initial-state", &n_latch);
	else
		dev_dbg(&client->dev, "no platform data\n");

> +	else if (pdata)
> +		n_latch = pdata->n_latch;
> +	else
>  		dev_dbg(&client->dev, "no platform data\n");
> -	}

Other than that, the patch looks good.

Best regards,
Tomasz

Re: [PATCH v5] gpio: pcf857x: Add OF support

[Archit Taneja]

Hi,

On Tuesday 27 August 2013 01:44 PM, Tomasz Figa wrote:
> Hi Laurent,
>
> On Tuesday 27 of August 2013 10:02:39 Laurent Pinchart wrote:
>> Add DT bindings for the pcf857x-compatible chips and parse the device
>> tree node in the driver.
>>
>> Signed-off-by: Laurent Pinchart
>> <laurent.pinchart+renesas@ideasonboard.com> ---
>>   .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 71
>> ++++++++++++++++++++++ drivers/gpio/gpio-pcf857x.c
>>    | 44 +++++++++++--- 2 files changed, 107 insertions(+), 8
>> deletions(-)
>>   create mode 100644
>> Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
>>
>> Changes since v4:
>>
>> - Don't try to get ngpio from of_device_id data, we already get it from
>>    i2c_device_id
>
> Hmm, I'm not sure how this is supposed to work.
>
> How does the I2C core resolve OF compatible name to particular entry in
> id_table? I believe it simply passes NULL as the second argument of
> .probe() if the device is instantiated based on OF compatible string and
> not one in the legacy ID table.

It doesn't pass the second argument as NULL. If you look at 
i2c_device_probe() in drivers/i2c/i2c-core.c, the second argument to 
probe is passed as: i2c_match_id(driver->id_table, client)

This will extract the i2c_device_id pointer from the id_table.

Archit

Re: [PATCH v5] gpio: pcf857x: Add OF support

[Tomasz Figa]

On Tuesday 27 of August 2013 14:00:24 Archit Taneja wrote:
> Hi,
> 
> On Tuesday 27 August 2013 01:44 PM, Tomasz Figa wrote:
> > Hi Laurent,
> > 
> > On Tuesday 27 of August 2013 10:02:39 Laurent Pinchart wrote:
> >> Add DT bindings for the pcf857x-compatible chips and parse the device
> >> tree node in the driver.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> <laurent.pinchart+renesas@ideasonboard.com> ---
> >> 
> >>   .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 71
> >> 
> >> ++++++++++++++++++++++ drivers/gpio/gpio-pcf857x.c
> >> 
> >>    | 44 +++++++++++--- 2 files changed, 107 insertions(+), 8
> >> 
> >> deletions(-)
> >> 
> >>   create mode 100644
> >> 
> >> Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> >> 
> >> Changes since v4:
> >> 
> >> - Don't try to get ngpio from of_device_id data, we already get it
> >> from
> >> 
> >>    i2c_device_id
> > 
> > Hmm, I'm not sure how this is supposed to work.
> > 
> > How does the I2C core resolve OF compatible name to particular entry in
> > id_table? I believe it simply passes NULL as the second argument of
> > .probe() if the device is instantiated based on OF compatible string
> > and
> > not one in the legacy ID table.
> 
> It doesn't pass the second argument as NULL. If you look at
> i2c_device_probe() in drivers/i2c/i2c-core.c, the second argument to
> probe is passed as: i2c_match_id(driver->id_table, client)
> 
> This will extract the i2c_device_id pointer from the id_table.

Yes, there is a chance that it will not return NULL, but I think that 
relying on this is rather flawed.

If you look at the whole code path, you can see that it's only a 
coincidence that this works. See the execution flow:
 - I2C adapter driver calls of_i2c_register_devices(),
 - of_i2c_register_devices() calls of_modalias_node() for every device on 
this bus,
 - of_modalias_node() stores the second substring of compatible string 
separated by a comma, if there is one or the whole compatible otherwise to 
the output buffer (type field of i2c_board_info struct, as passed by 
of_i2c_register_devices()),
 - of_i2c_register_devices() then calls i2c_new_device() with the resulting 
info struct,
 - i2c_new_device() takes info->type and copies its contents to client-
>name,
 - then a bit later, I2C core calls i2c_match_id(), which does matching of 
client->name against id_table of the driver and the resulting i2c_device_id 
entry (or NULL) is then passed to driver's .probe() callback.

So if it happens that compatible is not equal to "<vendor>,<ID from legacy 
I2C table>", then the matching will fail and NULL will be passed.

[CCing Wolfram and Grant, as they should now more about this behavior and 
whether it's intentional or no]

Best regards,
Tomasz

Re: [PATCH v5] gpio: pcf857x: Add OF support

[Laurent Pinchart]

Hi Tomasz,

On Tuesday 27 August 2013 13:55:00 Tomasz Figa wrote:
> On Tuesday 27 of August 2013 14:00:24 Archit Taneja wrote:
> > On Tuesday 27 August 2013 01:44 PM, Tomasz Figa wrote:
> > > On Tuesday 27 of August 2013 10:02:39 Laurent Pinchart wrote:
> > >> Add DT bindings for the pcf857x-compatible chips and parse the device
> > >> tree node in the driver.
> > >> 
> > >> Signed-off-by: Laurent Pinchart
> > >> <laurent.pinchart+renesas@ideasonboard.com> ---
> > >> 
> > >>   .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 71 +++++++++++++
> > >>   drivers/gpio/gpio-pcf857x.c                        | 44 ++++++++++---
> > >>   2 files changed, 107 insertions(+), 8 deletions(-)
> > >>   create mode 100644
> > >> Documentation/devicetree/bindings/gpio/gpio-pcf857x.txt
> > >> 
> > >> Changes since v4:
> > >> 
> > >> - Don't try to get ngpio from of_device_id data, we already get it
> > >>   from i2c_device_id
> > > 
> > > Hmm, I'm not sure how this is supposed to work.
> > > 
> > > How does the I2C core resolve OF compatible name to particular entry in
> > > id_table? I believe it simply passes NULL as the second argument of
> > > .probe() if the device is instantiated based on OF compatible string
> > > and not one in the legacy ID table.
> > 
> > It doesn't pass the second argument as NULL. If you look at
> > i2c_device_probe() in drivers/i2c/i2c-core.c, the second argument to
> > probe is passed as: i2c_match_id(driver->id_table, client)
> > 
> > This will extract the i2c_device_id pointer from the id_table.
> 
> Yes, there is a chance that it will not return NULL, but I think that
> relying on this is rather flawed.
> 
> If you look at the whole code path, you can see that it's only a
> coincidence that this works. See the execution flow:
>  - I2C adapter driver calls of_i2c_register_devices(),
>  - of_i2c_register_devices() calls of_modalias_node() for every device on
> this bus,
>  - of_modalias_node() stores the second substring of compatible string
> separated by a comma, if there is one or the whole compatible otherwise to
> the output buffer (type field of i2c_board_info struct, as passed by
> of_i2c_register_devices()),
>  - of_i2c_register_devices() then calls i2c_new_device() with the resulting
> info struct,
>  - i2c_new_device() takes info->type and copies its contents to client->name
>  - then a bit later, I2C core calls i2c_match_id(), which does matching of
> client->name against id_table of the driver and the resulting i2c_device_id
> entry (or NULL) is then passed to driver's .probe() callback.
> 
> So if it happens that compatible is not equal to "<vendor>,<ID from legacy
> I2C table>", then the matching will fail and NULL will be passed.

The driver should support the same chip models reardless of whether it's used 
with or without DT. If an entry in the OF table has no corresponding entry in 
the I2C table I would consider that as a driver bug. It would be caught early, 
as the driver would crash at probe time, so it will likely not make it to 
mainline (unless we merge untested code, but that's another issue :-)).

> [CCing Wolfram and Grant, as they should now more about this behavior and
> whether it's intentional or no]

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5] gpio: pcf857x: Add OF support

[Wolfram Sang]

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


> The driver should support the same chip models reardless of whether it's used 
> with or without DT. If an entry in the OF table has no corresponding entry in 
> the I2C table I would consider that as a driver bug.

Linus Walleij posted a patch to support DT only probing, but too many
side-effects showed up. Some were fixable (probe regressions) and some
need more work, if feasible at all. Most prominent is that runtime
instantiation of i2c slaves currently needs an entry in the i2c table.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v5] gpio: pcf857x: Add OF support

[Laurent Pinchart]

On Tuesday 27 August 2013 19:18:55 Wolfram Sang wrote:
> > The driver should support the same chip models reardless of whether it's
> > used with or without DT. If an entry in the OF table has no corresponding
> > entry in the I2C table I would consider that as a driver bug.
> 
> Linus Walleij posted a patch to support DT only probing, but too many
> side-effects showed up. Some were fixable (probe regressions) and some
> need more work, if feasible at all. Most prominent is that runtime
> instantiation of i2c slaves currently needs an entry in the i2c table.

Linus, I'd like to get this in v3.12 if it's not too late. Could you please 
provide your input ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5] gpio: pcf857x: Add OF support

[Linus Walleij]

On Wed, Aug 28, 2013 at 1:58 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tuesday 27 August 2013 19:18:55 Wolfram Sang wrote:
>> > The driver should support the same chip models reardless of whether it's
>> > used with or without DT. If an entry in the OF table has no corresponding
>> > entry in the I2C table I would consider that as a driver bug.
>>
>> Linus Walleij posted a patch to support DT only probing, but too many
>> side-effects showed up. Some were fixable (probe regressions) and some
>> need more work, if feasible at all. Most prominent is that runtime
>> instantiation of i2c slaves currently needs an entry in the i2c table.
>
> Linus, I'd like to get this in v3.12 if it's not too late. Could you please
> provide your input ?

Provided some input on the v4 version, due to being a bit overloaded
my patch queue is overloaded...

Yours,
Linus Walleij

Re: [PATCH v5] gpio: pcf857x: Add OF support

[Laurent Pinchart]

Hi Linus,

On Thursday 29 August 2013 20:24:32 Linus Walleij wrote:
> On Wed, Aug 28, 2013 at 1:58 PM, Laurent Pinchart wrote:
> > On Tuesday 27 August 2013 19:18:55 Wolfram Sang wrote:
> >> > The driver should support the same chip models reardless of whether
> >> > it's used with or without DT. If an entry in the OF table has no
> >> > corresponding entry in the I2C table I would consider that as a driver
> >> > bug.
> >> 
> >> Linus Walleij posted a patch to support DT only probing, but too many
> >> side-effects showed up. Some were fixable (probe regressions) and some
> >> need more work, if feasible at all. Most prominent is that runtime
> >> instantiation of i2c slaves currently needs an entry in the i2c table.
> > 
> > Linus, I'd like to get this in v3.12 if it's not too late. Could you
> > please provide your input ?
> 
> Provided some input on the v4 version,

Do you mean v4 of this patch ? I can't find your reply.

> due to being a bit overloaded my patch queue is overloaded...

No worries. We can delay this one until v3.13.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5] gpio: pcf857x: Add OF support

[Laurent Pinchart]

On Friday 30 August 2013 01:44:50 Laurent Pinchart wrote:
> On Thursday 29 August 2013 20:24:32 Linus Walleij wrote:
> > On Wed, Aug 28, 2013 at 1:58 PM, Laurent Pinchart wrote:
> > > On Tuesday 27 August 2013 19:18:55 Wolfram Sang wrote:
> > >> > The driver should support the same chip models reardless of whether
> > >> > it's used with or without DT. If an entry in the OF table has no
> > >> > corresponding entry in the I2C table I would consider that as a
> > >> > driver
> > >> > bug.
> > >> 
> > >> Linus Walleij posted a patch to support DT only probing, but too many
> > >> side-effects showed up. Some were fixable (probe regressions) and some
> > >> need more work, if feasible at all. Most prominent is that runtime
> > >> instantiation of i2c slaves currently needs an entry in the i2c table.
> > > 
> > > Linus, I'd like to get this in v3.12 if it's not too late. Could you
> > > please provide your input ?
> > 
> > Provided some input on the v4 version,
> 
> Do you mean v4 of this patch ? I can't find your reply.

Scratch that, I've found it. Sorry for the noise.

> > due to being a bit overloaded my patch queue is overloaded...
> 
> No worries. We can delay this one until v3.13.
-- 
Regards,

Laurent Pinchart