Re: [PATCH v5] gpio: pcf857x: Add OF support
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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, due to being a bit overloaded my patch queue is overloaded... Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
On Wed, Aug 28, 2013 at 1:58 PM, Laurent Pinchart laurent.pinch...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
> 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. signature.asc Description: Digital signature
Re: [PATCH v5] gpio: pcf857x: Add OF support
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 > > >> --- > > >> > > >> .../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 ", 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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 > >> --- > >> > >> .../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 ",", 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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 --- .../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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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
> ---
> .../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 000..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
> . 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 = <>;
> + 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
> #include
>
[PATCH v5] gpio: pcf857x: Add OF support
Add DT bindings for the pcf857x-compatible chips and parse the device
tree node in the driver.
Signed-off-by: Laurent Pinchart
---
.../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 000..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 . 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 = <>;
+ 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
#include
#include
+#include
+#include
#include
#include
#include
@@ -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 =
[PATCH v5] gpio: pcf857x: Add OF support
Add DT bindings for the pcf857x-compatible chips and parse the device
tree node in the driver.
Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@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 000..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 },
+ {
Re: [PATCH v5] gpio: pcf857x: Add OF support
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+rene...@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 000..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
Re: [PATCH v5] gpio: pcf857x: Add OF support
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+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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+rene...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5] gpio: pcf857x: Add OF support
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. signature.asc Description: Digital signature
