2012-11-28 18:47:27

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V4 Resend 1/3] mfd: stmpe: Get rid of irq_invert_polarity

Since the very first patch, stmpe core driver is using irq_invert_polarity as
part of platform data. But, nobody is actually using it in kernel till now.

Also, this is not something part of hardware specs, but is included to cater
some board mistakes or quirks.

So, better get rid of it. This is earlier discussed here:

https://lkml.org/lkml/2012/11/27/636

Signed-off-by: Viresh Kumar <[email protected]>
---
This is actually V1 of this patch.

drivers/mfd/stmpe.c | 7 -------
include/linux/mfd/stmpe.h | 2 --
2 files changed, 9 deletions(-)

diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
index 6175aa4..34408b4 100644
--- a/drivers/mfd/stmpe.c
+++ b/drivers/mfd/stmpe.c
@@ -960,13 +960,6 @@ static int __devinit stmpe_chip_init(struct stmpe *stmpe)
else
icr |= STMPE_ICR_LSB_HIGH;
}
-
- if (stmpe->pdata->irq_invert_polarity) {
- if (id == STMPE801_ID)
- icr ^= STMPE801_REG_SYS_CTRL_INT_HI;
- else
- icr ^= STMPE_ICR_LSB_HIGH;
- }
}

if (stmpe->pdata->autosleep) {
diff --git a/include/linux/mfd/stmpe.h b/include/linux/mfd/stmpe.h
index 15dac79..383ac15 100644
--- a/include/linux/mfd/stmpe.h
+++ b/include/linux/mfd/stmpe.h
@@ -190,7 +190,6 @@ struct stmpe_ts_platform_data {
* @id: device id to distinguish between multiple STMPEs on the same board
* @blocks: bitmask of blocks to enable (use STMPE_BLOCK_*)
* @irq_trigger: IRQ trigger to use for the interrupt to the host
- * @irq_invert_polarity: IRQ line is connected with reversed polarity
* @autosleep: bool to enable/disable stmpe autosleep
* @autosleep_timeout: inactivity timeout in milliseconds for autosleep
* @irq_base: base IRQ number. %STMPE_NR_IRQS irqs will be used, or
@@ -207,7 +206,6 @@ struct stmpe_platform_data {
unsigned int blocks;
int irq_base;
unsigned int irq_trigger;
- bool irq_invert_polarity;
bool autosleep;
bool irq_over_gpio;
int irq_gpio;
--
1.7.12.rc2.18.g61b472e


2012-11-28 18:47:36

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V4 Resend 2/3] mfd: stmpe: Remove irq_trigger from platform data

STMPE can confige the way the device emits interrupts and till now this
information is passed as part of platform data.

It would actually be good to ask the interrupt controller driver what kind of
interrupt signal it expects for a given interrupt line. We can get the irq type
by calling: irqd_get_trigger_type() routine.

So, now we don't need to pass it via platform data. This is earlier discussed
here:

https://lkml.org/lkml/2012/11/26/711

Signed-off-by: Viresh Kumar <[email protected]>
---

@Linus/Shiraz: Can you please test this patch? You don't really need to test 1/3
and 3/3, but would be good if you do that too..

This is actually V1 of this patch.

arch/arm/mach-ux500/board-mop500-stuib.c | 1 -
drivers/mfd/stmpe.c | 7 ++++---
include/linux/mfd/stmpe.h | 2 --
3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-ux500/board-mop500-stuib.c b/arch/arm/mach-ux500/board-mop500-stuib.c
index 564f57d..0efcf97 100644
--- a/arch/arm/mach-ux500/board-mop500-stuib.c
+++ b/arch/arm/mach-ux500/board-mop500-stuib.c
@@ -57,7 +57,6 @@ static struct stmpe_keypad_platform_data stmpe1601_keypad_data = {
static struct stmpe_platform_data stmpe1601_data = {
.id = 1,
.blocks = STMPE_BLOCK_KEYPAD,
- .irq_trigger = IRQF_TRIGGER_FALLING,
.irq_base = MOP500_STMPE1601_IRQ(0),
.keypad = &stmpe1601_keypad_data,
.autosleep = true,
diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
index 34408b4..10819e6 100644
--- a/drivers/mfd/stmpe.c
+++ b/drivers/mfd/stmpe.c
@@ -914,7 +914,6 @@ static int __devinit stmpe_irq_init(struct stmpe *stmpe,

static int __devinit stmpe_chip_init(struct stmpe *stmpe)
{
- unsigned int irq_trigger = stmpe->pdata->irq_trigger;
int autosleep_timeout = stmpe->pdata->autosleep_timeout;
struct stmpe_variant_info *variant = stmpe->variant;
u8 icr = 0;
@@ -941,6 +940,9 @@ static int __devinit stmpe_chip_init(struct stmpe *stmpe)
return ret;

if (stmpe->irq >= 0) {
+ unsigned int irq_trigger =
+ irqd_get_trigger_type(irq_get_irq_data(stmpe->irq));
+
if (id == STMPE801_ID)
icr = STMPE801_REG_SYS_CTRL_INT_EN;
else
@@ -1118,8 +1120,7 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum)
return ret;

ret = devm_request_threaded_irq(ci->dev, stmpe->irq, NULL,
- stmpe_irq, pdata->irq_trigger | IRQF_ONESHOT,
- "stmpe", stmpe);
+ stmpe_irq, IRQF_ONESHOT, "stmpe", stmpe);
if (ret) {
dev_err(stmpe->dev, "failed to request IRQ: %d\n",
ret);
diff --git a/include/linux/mfd/stmpe.h b/include/linux/mfd/stmpe.h
index 383ac15..2519326 100644
--- a/include/linux/mfd/stmpe.h
+++ b/include/linux/mfd/stmpe.h
@@ -189,7 +189,6 @@ struct stmpe_ts_platform_data {
* struct stmpe_platform_data - STMPE platform data
* @id: device id to distinguish between multiple STMPEs on the same board
* @blocks: bitmask of blocks to enable (use STMPE_BLOCK_*)
- * @irq_trigger: IRQ trigger to use for the interrupt to the host
* @autosleep: bool to enable/disable stmpe autosleep
* @autosleep_timeout: inactivity timeout in milliseconds for autosleep
* @irq_base: base IRQ number. %STMPE_NR_IRQS irqs will be used, or
@@ -205,7 +204,6 @@ struct stmpe_platform_data {
int id;
unsigned int blocks;
int irq_base;
- unsigned int irq_trigger;
bool autosleep;
bool irq_over_gpio;
int irq_gpio;
--
1.7.12.rc2.18.g61b472e

2012-11-28 18:47:42

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V4 Resend 3/3] mfd: stmpe: Update DT support in stmpe driver

From: Vipul Kumar Samar <[email protected]>

This patch extends existing DT support for stmpe devices. This updates:
- DT support from stmpe SPI and I2C drivers
- missing header files in stmpe.c
- stmpe_of_probe() with pwm, rotator and new bindings.
- Bindings are updated in binding document.

Signed-off-by: Vipul Kumar Samar <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
V3->V4:
- Remove need to pass irq type and polarity from DT.
- Remove need to pass id from platform data.

Documentation/devicetree/bindings/mfd/stmpe.txt | 9 ++++++---
drivers/mfd/stmpe-i2c.c | 15 +++++++++++++++
drivers/mfd/stmpe-spi.c | 15 +++++++++++++++
drivers/mfd/stmpe.c | 22 ++++++++++++++++------
4 files changed, 52 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt
index 8f0aeda..d2d88d9 100644
--- a/Documentation/devicetree/bindings/mfd/stmpe.txt
+++ b/Documentation/devicetree/bindings/mfd/stmpe.txt
@@ -1,8 +1,11 @@
-* STMPE Multi-Functional Device
+* ST Microelectronics STMPE Multi-Functional Device
+
+STMPE is an MFD device which may expose following inbuilt devices: gpio, keypad,
+touchscreen, adc, pwm, rotator.

Required properties:
- - compatible : "st,stmpe[811|1601|2401|2403]"
- - reg : I2C address of the device
+ - compatible : "st,stmpe[610|801|811|1601|2401|2403]"
+ - reg : I2C/SPI address of the device

Optional properties:
- interrupts : The interrupt outputs from the controller
diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c
index c734dc3..003c49b 100644
--- a/drivers/mfd/stmpe-i2c.c
+++ b/drivers/mfd/stmpe-i2c.c
@@ -13,6 +13,7 @@
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/types.h>
#include "stmpe.h"

@@ -81,6 +82,19 @@ static const struct i2c_device_id stmpe_i2c_id[] = {
};
MODULE_DEVICE_TABLE(i2c, stmpe_id);

+#ifdef CONFIG_OF
+static const struct of_device_id stmpe_dt_ids[] = {
+ { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], },
+ { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], },
+ { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], },
+ { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], },
+ { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], },
+ { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], },
+ { }
+};
+MODULE_DEVICE_TABLE(of, stmpe_dt_ids);
+#endif
+
static struct i2c_driver stmpe_i2c_driver = {
.driver = {
.name = "stmpe-i2c",
@@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = {
#ifdef CONFIG_PM
.pm = &stmpe_dev_pm_ops,
#endif
+ .of_match_table = of_match_ptr(stmpe_dt_ids),
},
.probe = stmpe_i2c_probe,
.remove = __devexit_p(stmpe_i2c_remove),
diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c
index 9edfe86..1e2bff0 100644
--- a/drivers/mfd/stmpe-spi.c
+++ b/drivers/mfd/stmpe-spi.c
@@ -11,6 +11,7 @@
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/types.h>
#include "stmpe.h"

@@ -119,6 +120,19 @@ static const struct spi_device_id stmpe_spi_id[] = {
};
MODULE_DEVICE_TABLE(spi, stmpe_id);

+#ifdef CONFIG_OF
+static const struct of_device_id stmpe_dt_ids[] = {
+ { .compatible = "st,stmpe610", .data = (void *)STMPE610, },
+ { .compatible = "st,stmpe801", .data = (void *)STMPE801, },
+ { .compatible = "st,stmpe811", .data = (void *)STMPE811, },
+ { .compatible = "st,stmpe1601", .data = (void *)STMPE1601, },
+ { .compatible = "st,stmpe2401", .data = (void *)STMPE2401, },
+ { .compatible = "st,stmpe2403", .data = (void *)STMPE2403, },
+ { }
+};
+MODULE_DEVICE_TABLE(of, stmpe_dt_ids);
+#endif
+
static struct spi_driver stmpe_spi_driver = {
.driver = {
.name = "stmpe-spi",
@@ -126,6 +140,7 @@ static struct spi_driver stmpe_spi_driver = {
#ifdef CONFIG_PM
.pm = &stmpe_dev_pm_ops,
#endif
+ .of_match_table = of_match_ptr(stmpe_dt_ids),
},
.probe = stmpe_spi_probe,
.remove = __devexit_p(stmpe_spi_remove),
diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
index 10819e6..88cd8e5 100644
--- a/drivers/mfd/stmpe.c
+++ b/drivers/mfd/stmpe.c
@@ -7,12 +7,15 @@
* Author: Rabin Vincent <[email protected]> for ST-Ericsson
*/

+#include <linux/err.h>
#include <linux/gpio.h>
#include <linux/export.h>
#include <linux/kernel.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
#include <linux/pm.h>
#include <linux/slab.h>
#include <linux/mfd/core.h>
@@ -1021,6 +1024,12 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata,
{
struct device_node *child;

+ /*
+ * Distinct names of same cell-type within multiple instances of stmpe
+ * will be guaranteed by DT.
+ */
+ pdata->id = -1;
+
of_property_read_u32(np, "st,autosleep-timeout",
&pdata->autosleep_timeout);

@@ -1029,15 +1038,16 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata,
for_each_child_of_node(np, child) {
if (!strcmp(child->name, "stmpe_gpio")) {
pdata->blocks |= STMPE_BLOCK_GPIO;
- }
- if (!strcmp(child->name, "stmpe_keypad")) {
+ } else if (!strcmp(child->name, "stmpe_keypad")) {
pdata->blocks |= STMPE_BLOCK_KEYPAD;
- }
- if (!strcmp(child->name, "stmpe_touchscreen")) {
+ } else if (!strcmp(child->name, "stmpe_touchscreen")) {
pdata->blocks |= STMPE_BLOCK_TOUCHSCREEN;
- }
- if (!strcmp(child->name, "stmpe_adc")) {
+ } else if (!strcmp(child->name, "stmpe_adc")) {
pdata->blocks |= STMPE_BLOCK_ADC;
+ } else if (!strcmp(child->name, "stmpe_pwm")) {
+ pdata->blocks |= STMPE_BLOCK_PWM;
+ } else if (!strcmp(child->name, "stmpe_rotator")) {
+ pdata->blocks |= STMPE_BLOCK_ROTATOR;
}
}
}
--
1.7.12.rc2.18.g61b472e

2012-11-29 09:20:10

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH V4 Resend 1/3] mfd: stmpe: Get rid of irq_invert_polarity

On Thu, 29 Nov 2012, Viresh Kumar wrote:

> Since the very first patch, stmpe core driver is using irq_invert_polarity as
> part of platform data. But, nobody is actually using it in kernel till now.
>
> Also, this is not something part of hardware specs, but is included to cater
> some board mistakes or quirks.
>
> So, better get rid of it. This is earlier discussed here:
>
> https://lkml.org/lkml/2012/11/27/636
>
> Signed-off-by: Viresh Kumar <[email protected]>

Glad to see the back of it:

Acked-by: Lee Jones <[email protected]>

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2012-11-29 09:23:16

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH V4 Resend 3/3] mfd: stmpe: Update DT support in stmpe driver

On Thu, 29 Nov 2012, Viresh Kumar wrote:

> From: Vipul Kumar Samar <[email protected]>
>
> This patch extends existing DT support for stmpe devices. This updates:
> - DT support from stmpe SPI and I2C drivers
> - missing header files in stmpe.c
> - stmpe_of_probe() with pwm, rotator and new bindings.
> - Bindings are updated in binding document.
>
> Signed-off-by: Vipul Kumar Samar <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> V3->V4:
> - Remove need to pass irq type and polarity from DT.
> - Remove need to pass id from platform data.
>
> Documentation/devicetree/bindings/mfd/stmpe.txt | 9 ++++++---
> drivers/mfd/stmpe-i2c.c | 15 +++++++++++++++
> drivers/mfd/stmpe-spi.c | 15 +++++++++++++++
> drivers/mfd/stmpe.c | 22 ++++++++++++++++------
> 4 files changed, 52 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt
> index 8f0aeda..d2d88d9 100644
> --- a/Documentation/devicetree/bindings/mfd/stmpe.txt
> +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt
> @@ -1,8 +1,11 @@
> -* STMPE Multi-Functional Device
> +* ST Microelectronics STMPE Multi-Functional Device
> +
> +STMPE is an MFD device which may expose following inbuilt devices: gpio, keypad,
> +touchscreen, adc, pwm, rotator.
>
> Required properties:
> - - compatible : "st,stmpe[811|1601|2401|2403]"
> - - reg : I2C address of the device
> + - compatible : "st,stmpe[610|801|811|1601|2401|2403]"
> + - reg : I2C/SPI address of the device
>
> Optional properties:
> - interrupts : The interrupt outputs from the controller
> diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c
> index c734dc3..003c49b 100644
> --- a/drivers/mfd/stmpe-i2c.c
> +++ b/drivers/mfd/stmpe-i2c.c
> @@ -13,6 +13,7 @@
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/types.h>
> #include "stmpe.h"
>
> @@ -81,6 +82,19 @@ static const struct i2c_device_id stmpe_i2c_id[] = {
> };
> MODULE_DEVICE_TABLE(i2c, stmpe_id);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id stmpe_dt_ids[] = {
> + { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], },
> + { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], },
> + { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], },
> + { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], },
> + { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], },
> + { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, stmpe_dt_ids);
> +#endif
> +
> static struct i2c_driver stmpe_i2c_driver = {
> .driver = {
> .name = "stmpe-i2c",
> @@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = {
> #ifdef CONFIG_PM
> .pm = &stmpe_dev_pm_ops,
> #endif
> + .of_match_table = of_match_ptr(stmpe_dt_ids),
> },
> .probe = stmpe_i2c_probe,
> .remove = __devexit_p(stmpe_i2c_remove),
> diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c
> index 9edfe86..1e2bff0 100644
> --- a/drivers/mfd/stmpe-spi.c
> +++ b/drivers/mfd/stmpe-spi.c
> @@ -11,6 +11,7 @@
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/types.h>
> #include "stmpe.h"
>
> @@ -119,6 +120,19 @@ static const struct spi_device_id stmpe_spi_id[] = {
> };
> MODULE_DEVICE_TABLE(spi, stmpe_id);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id stmpe_dt_ids[] = {
> + { .compatible = "st,stmpe610", .data = (void *)STMPE610, },
> + { .compatible = "st,stmpe801", .data = (void *)STMPE801, },
> + { .compatible = "st,stmpe811", .data = (void *)STMPE811, },
> + { .compatible = "st,stmpe1601", .data = (void *)STMPE1601, },
> + { .compatible = "st,stmpe2401", .data = (void *)STMPE2401, },
> + { .compatible = "st,stmpe2403", .data = (void *)STMPE2403, },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, stmpe_dt_ids);
> +#endif
> +
> static struct spi_driver stmpe_spi_driver = {
> .driver = {
> .name = "stmpe-spi",
> @@ -126,6 +140,7 @@ static struct spi_driver stmpe_spi_driver = {
> #ifdef CONFIG_PM
> .pm = &stmpe_dev_pm_ops,
> #endif
> + .of_match_table = of_match_ptr(stmpe_dt_ids),
> },
> .probe = stmpe_spi_probe,
> .remove = __devexit_p(stmpe_spi_remove),
> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
> index 10819e6..88cd8e5 100644
> --- a/drivers/mfd/stmpe.c
> +++ b/drivers/mfd/stmpe.c
> @@ -7,12 +7,15 @@
> * Author: Rabin Vincent <[email protected]> for ST-Ericsson
> */
>
> +#include <linux/err.h>
> #include <linux/gpio.h>
> #include <linux/export.h>
> #include <linux/kernel.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> #include <linux/irqdomain.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> #include <linux/pm.h>
> #include <linux/slab.h>
> #include <linux/mfd/core.h>
> @@ -1021,6 +1024,12 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata,
> {
> struct device_node *child;
>
> + /*
> + * Distinct names of same cell-type within multiple instances of stmpe
> + * will be guaranteed by DT.
> + */
> + pdata->id = -1;

And what if we're not booting with DT?

> of_property_read_u32(np, "st,autosleep-timeout",
> &pdata->autosleep_timeout);
>
> @@ -1029,15 +1038,16 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata,
> for_each_child_of_node(np, child) {
> if (!strcmp(child->name, "stmpe_gpio")) {
> pdata->blocks |= STMPE_BLOCK_GPIO;
> - }
> - if (!strcmp(child->name, "stmpe_keypad")) {
> + } else if (!strcmp(child->name, "stmpe_keypad")) {
> pdata->blocks |= STMPE_BLOCK_KEYPAD;
> - }
> - if (!strcmp(child->name, "stmpe_touchscreen")) {
> + } else if (!strcmp(child->name, "stmpe_touchscreen")) {
> pdata->blocks |= STMPE_BLOCK_TOUCHSCREEN;
> - }
> - if (!strcmp(child->name, "stmpe_adc")) {
> + } else if (!strcmp(child->name, "stmpe_adc")) {
> pdata->blocks |= STMPE_BLOCK_ADC;
> + } else if (!strcmp(child->name, "stmpe_pwm")) {
> + pdata->blocks |= STMPE_BLOCK_PWM;
> + } else if (!strcmp(child->name, "stmpe_rotator")) {
> + pdata->blocks |= STMPE_BLOCK_ROTATOR;
> }
> }
> }
> --
> 1.7.12.rc2.18.g61b472e
>

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2012-11-29 09:24:58

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH V4 Resend 3/3] mfd: stmpe: Update DT support in stmpe driver

On Thu, 29 Nov 2012, Lee Jones wrote:

> On Thu, 29 Nov 2012, Viresh Kumar wrote:
>
> > From: Vipul Kumar Samar <[email protected]>
> >
> > This patch extends existing DT support for stmpe devices. This updates:
> > - DT support from stmpe SPI and I2C drivers
> > - missing header files in stmpe.c
> > - stmpe_of_probe() with pwm, rotator and new bindings.
> > - Bindings are updated in binding document.
> >
> > Signed-off-by: Vipul Kumar Samar <[email protected]>
> > Signed-off-by: Viresh Kumar <[email protected]>
> > ---
> > V3->V4:
> > - Remove need to pass irq type and polarity from DT.
> > - Remove need to pass id from platform data.
> >
> > Documentation/devicetree/bindings/mfd/stmpe.txt | 9 ++++++---
> > drivers/mfd/stmpe-i2c.c | 15 +++++++++++++++
> > drivers/mfd/stmpe-spi.c | 15 +++++++++++++++
> > drivers/mfd/stmpe.c | 22 ++++++++++++++++------
> > 4 files changed, 52 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt
> > index 8f0aeda..d2d88d9 100644
> > --- a/Documentation/devicetree/bindings/mfd/stmpe.txt
> > +++ b/Documentation/devicetree/bindings/mfd/stmpe.txt
> > @@ -1,8 +1,11 @@
> > -* STMPE Multi-Functional Device
> > +* ST Microelectronics STMPE Multi-Functional Device
> > +
> > +STMPE is an MFD device which may expose following inbuilt devices: gpio, keypad,
> > +touchscreen, adc, pwm, rotator.
> >
> > Required properties:
> > - - compatible : "st,stmpe[811|1601|2401|2403]"
> > - - reg : I2C address of the device
> > + - compatible : "st,stmpe[610|801|811|1601|2401|2403]"
> > + - reg : I2C/SPI address of the device
> >
> > Optional properties:
> > - interrupts : The interrupt outputs from the controller
> > diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c
> > index c734dc3..003c49b 100644
> > --- a/drivers/mfd/stmpe-i2c.c
> > +++ b/drivers/mfd/stmpe-i2c.c
> > @@ -13,6 +13,7 @@
> > #include <linux/interrupt.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > +#include <linux/of.h>
> > #include <linux/types.h>
> > #include "stmpe.h"
> >
> > @@ -81,6 +82,19 @@ static const struct i2c_device_id stmpe_i2c_id[] = {
> > };
> > MODULE_DEVICE_TABLE(i2c, stmpe_id);
> >
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id stmpe_dt_ids[] = {
> > + { .compatible = "st,stmpe610", .data = &stmpe_i2c_id[0], },
> > + { .compatible = "st,stmpe801", .data = &stmpe_i2c_id[1], },
> > + { .compatible = "st,stmpe811", .data = &stmpe_i2c_id[2], },
> > + { .compatible = "st,stmpe1601", .data = &stmpe_i2c_id[3], },
> > + { .compatible = "st,stmpe2401", .data = &stmpe_i2c_id[4], },
> > + { .compatible = "st,stmpe2403", .data = &stmpe_i2c_id[5], },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, stmpe_dt_ids);
> > +#endif
> > +
> > static struct i2c_driver stmpe_i2c_driver = {
> > .driver = {
> > .name = "stmpe-i2c",
> > @@ -88,6 +102,7 @@ static struct i2c_driver stmpe_i2c_driver = {
> > #ifdef CONFIG_PM
> > .pm = &stmpe_dev_pm_ops,
> > #endif
> > + .of_match_table = of_match_ptr(stmpe_dt_ids),
> > },
> > .probe = stmpe_i2c_probe,
> > .remove = __devexit_p(stmpe_i2c_remove),
> > diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c
> > index 9edfe86..1e2bff0 100644
> > --- a/drivers/mfd/stmpe-spi.c
> > +++ b/drivers/mfd/stmpe-spi.c
> > @@ -11,6 +11,7 @@
> > #include <linux/interrupt.h>
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > +#include <linux/of.h>
> > #include <linux/types.h>
> > #include "stmpe.h"
> >
> > @@ -119,6 +120,19 @@ static const struct spi_device_id stmpe_spi_id[] = {
> > };
> > MODULE_DEVICE_TABLE(spi, stmpe_id);
> >
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id stmpe_dt_ids[] = {
> > + { .compatible = "st,stmpe610", .data = (void *)STMPE610, },
> > + { .compatible = "st,stmpe801", .data = (void *)STMPE801, },
> > + { .compatible = "st,stmpe811", .data = (void *)STMPE811, },
> > + { .compatible = "st,stmpe1601", .data = (void *)STMPE1601, },
> > + { .compatible = "st,stmpe2401", .data = (void *)STMPE2401, },
> > + { .compatible = "st,stmpe2403", .data = (void *)STMPE2403, },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, stmpe_dt_ids);
> > +#endif
> > +
> > static struct spi_driver stmpe_spi_driver = {
> > .driver = {
> > .name = "stmpe-spi",
> > @@ -126,6 +140,7 @@ static struct spi_driver stmpe_spi_driver = {
> > #ifdef CONFIG_PM
> > .pm = &stmpe_dev_pm_ops,
> > #endif
> > + .of_match_table = of_match_ptr(stmpe_dt_ids),
> > },
> > .probe = stmpe_spi_probe,
> > .remove = __devexit_p(stmpe_spi_remove),
> > diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
> > index 10819e6..88cd8e5 100644
> > --- a/drivers/mfd/stmpe.c
> > +++ b/drivers/mfd/stmpe.c
> > @@ -7,12 +7,15 @@
> > * Author: Rabin Vincent <[email protected]> for ST-Ericsson
> > */
> >
> > +#include <linux/err.h>
> > #include <linux/gpio.h>
> > #include <linux/export.h>
> > #include <linux/kernel.h>
> > #include <linux/interrupt.h>
> > #include <linux/irq.h>
> > #include <linux/irqdomain.h>
> > +#include <linux/of.h>
> > +#include <linux/of_gpio.h>
> > #include <linux/pm.h>
> > #include <linux/slab.h>
> > #include <linux/mfd/core.h>
> > @@ -1021,6 +1024,12 @@ void __devinit stmpe_of_probe(struct stmpe_platform_data *pdata,
> > {
> > struct device_node *child;
> >
> > + /*
> > + * Distinct names of same cell-type within multiple instances of stmpe
> > + * will be guaranteed by DT.
> > + */
> > + pdata->id = -1;
>
> And what if we're not booting with DT?

Ah, we're in *_of_probe(), ignore this.

Instead:

Acked-by: Lee Jones <[email protected]>

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2012-11-29 09:27:09

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V4 Resend 3/3] mfd: stmpe: Update DT support in stmpe driver

On 29 November 2012 14:53, Lee Jones <[email protected]> wrote:
>> From: Vipul Kumar Samar <[email protected]>
>> + /*
>> + * Distinct names of same cell-type within multiple instances of stmpe
>> + * will be guaranteed by DT.
>> + */
>> + pdata->id = -1;
>
> And what if we're not booting with DT?

Then it is responsibility of code creating platform data for stmpe to fill
it appropriately. i.e. If there is only one instance of stmpe on board, then
better pass it as -1, else start counting.

2012-11-29 09:34:36

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH V4 Resend 2/3] mfd: stmpe: Remove irq_trigger from platform data

On Thu, 29 Nov 2012, Viresh Kumar wrote:

> STMPE can confige

configure?

> the way the device emits interrupts and till now this

until?

> information is passed as part of platform data.
>
> It would actually be good to ask the interrupt controller driver what kind of
> interrupt signal it expects for a given interrupt line. We can get the irq type
> by calling: irqd_get_trigger_type() routine.
>
> So, now we don't need to pass it via platform data. This is earlier discussed
> here:
>
> https://lkml.org/lkml/2012/11/26/711
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
>
> @Linus/Shiraz: Can you please test this patch? You don't really need to test 1/3
> and 3/3, but would be good if you do that too..
>
> This is actually V1 of this patch.
>
> arch/arm/mach-ux500/board-mop500-stuib.c | 1 -
> drivers/mfd/stmpe.c | 7 ++++---
> include/linux/mfd/stmpe.h | 2 --
> 3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/mach-ux500/board-mop500-stuib.c b/arch/arm/mach-ux500/board-mop500-stuib.c
> index 564f57d..0efcf97 100644
> --- a/arch/arm/mach-ux500/board-mop500-stuib.c
> +++ b/arch/arm/mach-ux500/board-mop500-stuib.c
> @@ -57,7 +57,6 @@ static struct stmpe_keypad_platform_data stmpe1601_keypad_data = {
> static struct stmpe_platform_data stmpe1601_data = {
> .id = 1,
> .blocks = STMPE_BLOCK_KEYPAD,
> - .irq_trigger = IRQF_TRIGGER_FALLING,
> .irq_base = MOP500_STMPE1601_IRQ(0),
> .keypad = &stmpe1601_keypad_data,
> .autosleep = true,
> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
> index 34408b4..10819e6 100644
> --- a/drivers/mfd/stmpe.c
> +++ b/drivers/mfd/stmpe.c
> @@ -914,7 +914,6 @@ static int __devinit stmpe_irq_init(struct stmpe *stmpe,
>
> static int __devinit stmpe_chip_init(struct stmpe *stmpe)
> {
> - unsigned int irq_trigger = stmpe->pdata->irq_trigger;
> int autosleep_timeout = stmpe->pdata->autosleep_timeout;
> struct stmpe_variant_info *variant = stmpe->variant;
> u8 icr = 0;
> @@ -941,6 +940,9 @@ static int __devinit stmpe_chip_init(struct stmpe *stmpe)
> return ret;
>
> if (stmpe->irq >= 0) {
> + unsigned int irq_trigger =
> + irqd_get_trigger_type(irq_get_irq_data(stmpe->irq));
> +
> if (id == STMPE801_ID)
> icr = STMPE801_REG_SYS_CTRL_INT_EN;
> else
> @@ -1118,8 +1120,7 @@ int __devinit stmpe_probe(struct stmpe_client_info *ci, int partnum)
> return ret;
>
> ret = devm_request_threaded_irq(ci->dev, stmpe->irq, NULL,
> - stmpe_irq, pdata->irq_trigger | IRQF_ONESHOT,
> - "stmpe", stmpe);
> + stmpe_irq, IRQF_ONESHOT, "stmpe", stmpe);

Forgive my ignorance, but you're no longer passing irq_trigger.

Is this intentional? If so, why was it needed before and not now?

--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2012-11-29 10:01:51

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH V4 Resend 2/3] mfd: stmpe: Remove irq_trigger from platform data

On 29 November 2012 15:04, Lee Jones <[email protected]> wrote:
> On Thu, 29 Nov 2012, Viresh Kumar wrote:
>
>> STMPE can confige
>
> configure?
>
>> the way the device emits interrupts and till now this
>
> until?

Ahh... Will fix them. This happens when you send patches at midnight. :)

>> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c

>> ret = devm_request_threaded_irq(ci->dev, stmpe->irq, NULL,
>> - stmpe_irq, pdata->irq_trigger | IRQF_ONESHOT,
>> - "stmpe", stmpe);
>> + stmpe_irq, IRQF_ONESHOT, "stmpe", stmpe);
>
> Forgive my ignorance, but you're no longer passing irq_trigger.
>
> Is this intentional? If so, why was it needed before and not now?

Yes, it was intentional. I thought it wasn't required at all. But my mind
is changing a bit now. I feel it is not required for DT, as trigger prop
is already passed in the interrupts cell. But for non-DT user, this is
still required. As there is not other way by which IRQ controller will
come to know what irq trigger type to enable for this irq line.

--
viresh

2012-11-30 10:55:51

by Samuel Ortiz

[permalink] [raw]
Subject: Re: [PATCH V4 Resend 1/3] mfd: stmpe: Get rid of irq_invert_polarity

Hi Viresh,

On Thu, Nov 29, 2012 at 12:17:15AM +0530, Viresh Kumar wrote:
> Since the very first patch, stmpe core driver is using irq_invert_polarity as
> part of platform data. But, nobody is actually using it in kernel till now.
>
> Also, this is not something part of hardware specs, but is included to cater
> some board mistakes or quirks.
>
> So, better get rid of it. This is earlier discussed here:
>
> https://lkml.org/lkml/2012/11/27/636
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> This is actually V1 of this patch.
>
> drivers/mfd/stmpe.c | 7 -------
> include/linux/mfd/stmpe.h | 2 --
> 2 files changed, 9 deletions(-)
Patch applied, thanks.

Cheers,
Samuel.

--
Intel Open Source Technology Centre
http://oss.intel.com/