Groups > linux.kernel > #1382432 > unrolled thread

[PATCH 0/8] STMPE fixes/rework and add STMPE1600 support

Started by	<patrice.chotard@st.com>
First post	2016-04-19 14:30 +0200
Last post	2016-04-20 16:30 +0200
Articles	20 — 8 participants

Back to article view | Back to linux.kernel

  [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support <patrice.chotard@st.com> - 2016-04-19 14:30 +0200
    [PATCH 7/8] gpio: stmpe: Add STMPE1600 support <patrice.chotard@st.com> - 2016-04-19 14:30 +0200
      Re: [PATCH 7/8] gpio: stmpe: Add STMPE1600 support Linus Walleij <linus.walleij@linaro.org> - 2016-04-20 17:00 +0200
        Re: [PATCH 7/8] gpio: stmpe: Add STMPE1600 support Patrice Chotard <patrice.chotard@st.com> - 2016-04-22 09:20 +0200
    [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection <patrice.chotard@st.com> - 2016-04-19 14:30 +0200
      Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge  detection kbuild test robot <lkp@intel.com> - 2016-04-19 14:40 +0200
      Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection Linus Walleij <linus.walleij@linaro.org> - 2016-04-20 16:40 +0200
        Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge  detection Patrice Chotard <patrice.chotard@st.com> - 2016-04-21 15:50 +0200
    [PATCH 8/8] gpio: stmpe: configure GPIO as output by default <patrice.chotard@st.com> - 2016-04-19 14:30 +0200
      Re: [PATCH 8/8] gpio: stmpe: configure GPIO as output by default Linus Walleij <linus.walleij@linaro.org> - 2016-04-20 17:00 +0200
    [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant <patrice.chotard@st.com> - 2016-04-19 14:30 +0200
      Re: [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant Linus Walleij <linus.walleij@linaro.org> - 2016-04-20 16:40 +0200
      Re: [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant Lee Jones <lee.jones@linaro.org> - 2016-04-26 10:20 +0200
    Re: [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support Thierry Reding <thierry.reding@gmail.com> - 2016-04-19 14:50 +0200
      Re: [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support Marcel Ziswiler <marcel.ziswiler@toradex.com> - 2016-04-20 20:00 +0200
    Re: [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support Stephen Warren <swarren@wwwdotorg.org> - 2016-04-19 18:00 +0200
      Re: [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support Patrice Chotard <patrice.chotard@st.com> - 2016-04-20 09:50 +0200
        Re: [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support Stephen Warren <swarren@wwwdotorg.org> - 2016-04-20 18:10 +0200
          Re: [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support Marcel Ziswiler <marcel.ziswiler@toradex.com> - 2016-04-21 05:20 +0200
      Re: [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support Linus Walleij <linus.walleij@linaro.org> - 2016-04-20 16:30 +0200

#1382432 — [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support

From	<patrice.chotard@st.com>
Date	2016-04-19 14:30 +0200
Subject	[PATCH 0/8] STMPE fixes/rework and add STMPE1600 support
Message-ID	<rpCNk-6J2-3@gated-at.bofh.it>

From: Patrice Chotard <patrice.chotard@st.com>

This series cleans and fixes some bugs in MFD/GPIO STMPE drivers and prepare
 the ground to add new STMPE1600 support.

STMPE1600 datasheet is available here : 
http://www2.st.com/content/st_com/en/products/interfaces-and-transceivers/
i-o-expanders-and-level-translators/i-o-expanders/stmpe1600.html

Only STMPE1600 has been tested on STM32 platform. As i have no board with
others STMPE variant(STMPE610/STMPE801/STMPE811/STMPE1601/STMPE1801/STMPE2401
and STMPE2403), i put in CC boards's maintainers which are using others STMPE variant.

If they can kindly check that no regression has been introduce by this series
:

For ARM/FREESCALE IMX / MXC ARM ARCHITECTURE:
	_ Shawn Guo <shawnguo@kernel.org>
	_ Sascha Hauer <kernel@pengutronix.de>

For ARM/SOCFPGA ARCHITECTURE
	_ Dinh Nguyen <dinguyen@opensource.altera.com>

For SPEAR PLATFORM SUPPORT
	_ Viresh Kumar <vireshk@kernel.org>
	_ Shiraz Hashim <shiraz.linux.kernel@gmail.com>

For TEGRA ARCHITECTURE SUPPORT
	_ Stephen Warren <swarren@wwwdotorg.org>
	_ Thierry Reding <thierry.reding@gmail.com>
	_ Alexandre Courbot <gnurou@gmail.com> 

For ARM/Ux500 ARM ARCHITECTURE
	_ Linus Walleij <linus.walleij@linaro.org>

Patrice Chotard (8):
  mfd: stmpe: Add STMPE_IDX_SYS_CTRL/2 enum
  mfd: stmpe: Add reset support for all STMPE variant
  gpio: stmpe: fix edge and rising/falling edge detection
  gpio: stmpe: write int status register only when needed
  Documentation: dt: add stmpe1600 compatible string to stmpe mfd
  mfd: Add STMPE1600 support
  gpio: stmpe: Add STMPE1600 support
  gpio: stmpe: configure GPIO as output by default

 Documentation/devicetree/bindings/mfd/stmpe.txt |   2 +-
 drivers/gpio/gpio-stmpe.c                       | 142 ++++++++++++++++++------
 drivers/mfd/stmpe-i2c.c                         |   2 +
 drivers/mfd/stmpe.c                             | 105 ++++++++++++++----
 drivers/mfd/stmpe.h                             |  30 ++++-
 include/linux/mfd/stmpe.h                       |   3 +
 6 files changed, 223 insertions(+), 61 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1382435 — [PATCH 7/8] gpio: stmpe: Add STMPE1600 support

From	<patrice.chotard@st.com>
Date	2016-04-19 14:30 +0200
Subject	[PATCH 7/8] gpio: stmpe: Add STMPE1600 support
Message-ID	<rpCX0-6Oi-9@gated-at.bofh.it>
In reply to	#1382432

From: Patrice Chotard <patrice.chotard@st.com>

The particularities of this variant are:
- GPIO_XXX_LSB and GPIO_XXX_MSB memory locations are inverted compared
  to other variants.
- There is no Edge detection, Rising Edge and Falling Edge registers.
- IRQ flags are cleared when read, no need to write in Status register.

Signed-off-by: Amelie DELAUNAY <amelie.delaunay@st.com>
Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/gpio/gpio-stmpe.c | 74 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 60 insertions(+), 14 deletions(-)

diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c
index f8c9d22..45e5b92 100644
--- a/drivers/gpio/gpio-stmpe.c
+++ b/drivers/gpio/gpio-stmpe.c
@@ -40,10 +40,15 @@ static int stmpe_gpio_get(struct gpio_chip *chip, unsigned offset)
 {
 	struct stmpe_gpio *stmpe_gpio = gpiochip_get_data(chip);
 	struct stmpe *stmpe = stmpe_gpio->stmpe;
-	u8 reg = stmpe->regs[STMPE_IDX_GPMR_LSB] - (offset / 8);
+	u8 reg;
 	u8 mask = 1 << (offset % 8);
 	int ret;
 
+	if (stmpe->partnum == STMPE1600)
+		reg = stmpe->regs[STMPE_IDX_GPMR_LSB] + (offset / 8);
+	else
+		reg = stmpe->regs[STMPE_IDX_GPMR_LSB] - (offset / 8);
+
 	ret = stmpe_reg_read(stmpe, reg);
 	if (ret < 0)
 		return ret;
@@ -56,9 +61,14 @@ static void stmpe_gpio_set(struct gpio_chip *chip, unsigned offset, int val)
 	struct stmpe_gpio *stmpe_gpio = gpiochip_get_data(chip);
 	struct stmpe *stmpe = stmpe_gpio->stmpe;
 	int which = val ? STMPE_IDX_GPSR_LSB : STMPE_IDX_GPCR_LSB;
-	u8 reg = stmpe->regs[which] - (offset / 8);
+	u8 reg;
 	u8 mask = 1 << (offset % 8);
 
+	if (stmpe->partnum == STMPE1600)
+		reg = stmpe->regs[which] + (offset / 8);
+	else
+		reg = stmpe->regs[which] - (offset / 8);
+
 	/*
 	 * Some variants have single register for gpio set/clear functionality.
 	 * For them we need to write 0 to clear and 1 to set.
@@ -74,9 +84,14 @@ static int stmpe_gpio_direction_output(struct gpio_chip *chip,
 {
 	struct stmpe_gpio *stmpe_gpio = gpiochip_get_data(chip);
 	struct stmpe *stmpe = stmpe_gpio->stmpe;
-	u8 reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);
+	u8 reg;
 	u8 mask = 1 << (offset % 8);
 
+	if (stmpe->partnum == STMPE1600)
+		reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8);
+	else
+		reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);
+
 	stmpe_gpio_set(chip, offset, val);
 
 	return stmpe_set_bits(stmpe, reg, mask, mask);
@@ -87,9 +102,14 @@ static int stmpe_gpio_direction_input(struct gpio_chip *chip,
 {
 	struct stmpe_gpio *stmpe_gpio = gpiochip_get_data(chip);
 	struct stmpe *stmpe = stmpe_gpio->stmpe;
-	u8 reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);
+	u8 reg;
 	u8 mask = 1 << (offset % 8);
 
+	if (stmpe->partnum == STMPE1600)
+		reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8);
+	else
+		reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);
+
 	return stmpe_set_bits(stmpe, reg, mask, 0);
 }
 
@@ -126,8 +146,9 @@ static int stmpe_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 	if (type & IRQ_TYPE_LEVEL_LOW || type & IRQ_TYPE_LEVEL_HIGH)
 		return -EINVAL;
 
-	/* STMPE801 doesn't have RE and FE registers */
-	if (stmpe_gpio->stmpe->partnum == STMPE801)
+	/* STMPE801 and STMPE 1600 don't have RE and FE registers */
+	if (stmpe_gpio->stmpe->partnum == STMPE801 ||
+	    stmpe_gpio->stmpe->partnum == STMPE1600)
 		return 0;
 
 	if (type & IRQ_TYPE_EDGE_RISING)
@@ -165,9 +186,10 @@ static void stmpe_gpio_irq_sync_unlock(struct irq_data *d)
 	int i, j;
 
 	for (i = 0; i < CACHE_NR_REGS; i++) {
-		/* STMPE801 doesn't have RE and FE registers */
-		if ((stmpe->partnum == STMPE801) &&
-				(i != REG_IE))
+		/* STMPE801 and STMPE1600 don't have RE and FE registers */
+		if ((stmpe->partnum == STMPE801 ||
+		     stmpe->partnum == STMPE1600) &&
+		     (i != REG_IE))
 			continue;
 
 		for (j = 0; j < num_banks; j++) {
@@ -178,7 +200,14 @@ static void stmpe_gpio_irq_sync_unlock(struct irq_data *d)
 				continue;
 
 			stmpe_gpio->oldregs[i][j] = new;
-			stmpe_reg_write(stmpe, stmpe->regs[regmap[i]] - j, new);
+			if (stmpe->partnum == STMPE1600)
+				stmpe_reg_write(stmpe,
+						stmpe->regs[regmap[i]] + j,
+						new);
+			else
+				stmpe_reg_write(stmpe,
+						stmpe->regs[regmap[i]] - j,
+						new);
 		}
 	}
 
@@ -216,11 +245,16 @@ static void stmpe_dbg_show_one(struct seq_file *s,
 	const char *label = gpiochip_is_requested(gc, offset);
 	int num_banks = DIV_ROUND_UP(stmpe->num_gpios, 8);
 	bool val = !!stmpe_gpio_get(gc, offset);
-	u8 dir_reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);
+	u8 dir_reg;
 	u8 mask = 1 << (offset % 8);
 	int ret;
 	u8 dir;
 
+	if (stmpe->partnum == STMPE1600)
+		dir_reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8);
+	else
+		dir_reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);
+
 	ret = stmpe_reg_read(stmpe, dir_reg);
 	if (ret < 0)
 		return;
@@ -272,6 +306,11 @@ static void stmpe_dbg_show_one(struct seq_file *s,
 				    (offset / 8);
 			break;
 
+		case STMPE1600:
+			irqen_reg = stmpe->regs[STMPE_IDX_IEGPIOR_LSB] +
+				    (offset / 8);
+			break;
+
 		default:
 			return;
 		}
@@ -321,12 +360,18 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev)
 	int ret;
 	int i;
 
+	if (stmpe->partnum == STMPE1600)
+		statmsbreg = stmpe->regs[STMPE_IDX_ISGPIOR_LSB];
+	else
+		statmsbreg = stmpe->regs[STMPE_IDX_ISGPIOR_MSB];
+
 	ret = stmpe_block_read(stmpe, statmsbreg, num_banks, status);
 	if (ret < 0)
 		return IRQ_NONE;
 
 	for (i = 0; i < num_banks; i++) {
-		int bank = num_banks - i - 1;
+		int bank = (stmpe_gpio->stmpe->partnum == STMPE1600) ? i :
+			   num_banks - i - 1;
 		unsigned int enabled = stmpe_gpio->regs[REG_IE][bank];
 		unsigned int stat = status[i];
 
@@ -346,10 +391,11 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev)
 
 		/*
 		 * interrupt status register write has no effect on
-		 * 801 and 1801, bits are cleared when read.
+		 * 801/1801/1600, bits are cleared when read.
 		 * Edge detect register is not present on 801 and 1801
 		 */
-		if (stmpe->partnum != STMPE801 || stmpe->partnum != STMPE1801) {
+		if (stmpe->partnum != STMPE801 || stmpe->partnum != STMPE1600 ||
+		    stmpe->partnum != STMPE1801) {
 			stmpe_reg_write(stmpe, statmsbreg + i, status[i]);
 			stmpe_reg_write(stmpe, stmpe->regs[STMPE_IDX_GPEDR_MSB]
 					+ i, status[i]);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1383465 — Re: [PATCH 7/8] gpio: stmpe: Add STMPE1600 support

From	Linus Walleij <linus.walleij@linaro.org>
Date	2016-04-20 17:00 +0200
Subject	Re: [PATCH 7/8] gpio: stmpe: Add STMPE1600 support
Message-ID	<rq1LI-13P-3@gated-at.bofh.it>
In reply to	#1382435

On Tue, Apr 19, 2016 at 2:18 PM,  <patrice.chotard@st.com> wrote:

> From: Patrice Chotard <patrice.chotard@st.com>
>
> The particularities of this variant are:
> - GPIO_XXX_LSB and GPIO_XXX_MSB memory locations are inverted compared
>   to other variants.
> - There is no Edge detection, Rising Edge and Falling Edge registers.
> - IRQ flags are cleared when read, no need to write in Status register.
>
> Signed-off-by: Amelie DELAUNAY <amelie.delaunay@st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

> -       u8 reg = stmpe->regs[STMPE_IDX_GPMR_LSB] - (offset / 8);
> +       u8 reg;
>         u8 mask = 1 << (offset % 8);
>         int ret;
>
> +       if (stmpe->partnum == STMPE1600)
> +               reg = stmpe->regs[STMPE_IDX_GPMR_LSB] + (offset / 8);
> +       else
> +               reg = stmpe->regs[STMPE_IDX_GPMR_LSB] - (offset / 8);

This construct is a bit hard to grasp.

Can we think of something more intuitive? Maybe using more
code lines but easier to understand.

Subtracting the offset is just totally unintuitive in the first place,
the STMPE1600 arrangement is much more intuitive.

I would prefer if we address the LSB+MSB register explicitly
instead of adding or subtracting 1 to the LSB register to get
to the MSB register.

> +       if (stmpe->partnum == STMPE1600)
> +               reg = stmpe->regs[which] + (offset / 8);
> +       else
> +               reg = stmpe->regs[which] - (offset / 8);

Same.

> +       if (stmpe->partnum == STMPE1600)
> +               reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8);
> +       else
> +               reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);

Same.

> +       if (stmpe->partnum == STMPE1600)
> +               reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8);
> +       else
> +               reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);

Same.

> +                               stmpe_reg_write(stmpe,
> +                                               stmpe->regs[regmap[i]] + j,
> +                                               new);
> +                       else
> +                               stmpe_reg_write(stmpe,
> +                                               stmpe->regs[regmap[i]] - j,
> +                                               new);

This is also unintuitively backwards.

> +       if (stmpe->partnum == STMPE1600)
> +               dir_reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8);
> +       else
> +               dir_reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);

Same.

> +       if (stmpe->partnum == STMPE1600)
> +               statmsbreg = stmpe->regs[STMPE_IDX_ISGPIOR_LSB];
> +       else
> +               statmsbreg = stmpe->regs[STMPE_IDX_ISGPIOR_MSB];

And this kind of points at the problem.

Can we write this in some way that make it super-clear which register
we're using and why?

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1384737 — Re: [PATCH 7/8] gpio: stmpe: Add STMPE1600 support

From	Patrice Chotard <patrice.chotard@st.com>
Date	2016-04-22 09:20 +0200
Subject	Re: [PATCH 7/8] gpio: stmpe: Add STMPE1600 support
Message-ID	<rqDxE-6zr-9@gated-at.bofh.it>
In reply to	#1383465


On 04/20/2016 04:53 PM, Linus Walleij wrote:
> On Tue, Apr 19, 2016 at 2:18 PM,  <patrice.chotard@st.com> wrote:
>
>> From: Patrice Chotard <patrice.chotard@st.com>
>>
>> The particularities of this variant are:
>> - GPIO_XXX_LSB and GPIO_XXX_MSB memory locations are inverted compared
>>    to other variants.
>> - There is no Edge detection, Rising Edge and Falling Edge registers.
>> - IRQ flags are cleared when read, no need to write in Status register.
>>
>> Signed-off-by: Amelie DELAUNAY <amelie.delaunay@st.com>
>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>> -       u8 reg = stmpe->regs[STMPE_IDX_GPMR_LSB] - (offset / 8);
>> +       u8 reg;
>>          u8 mask = 1 << (offset % 8);
>>          int ret;
>>
>> +       if (stmpe->partnum == STMPE1600)
>> +               reg = stmpe->regs[STMPE_IDX_GPMR_LSB] + (offset / 8);
>> +       else
>> +               reg = stmpe->regs[STMPE_IDX_GPMR_LSB] - (offset / 8);
> This construct is a bit hard to grasp.
>
> Can we think of something more intuitive? Maybe using more
> code lines but easier to understand.
>
> Subtracting the offset is just totally unintuitive in the first place,
> the STMPE1600 arrangement is much more intuitive.
>
> I would prefer if we address the LSB+MSB register explicitly
> instead of adding or subtracting 1 to the LSB register to get
> to the MSB register.
>
>> +       if (stmpe->partnum == STMPE1600)
>> +               reg = stmpe->regs[which] + (offset / 8);
>> +       else
>> +               reg = stmpe->regs[which] - (offset / 8);
> Same.
>
>> +       if (stmpe->partnum == STMPE1600)
>> +               reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8);
>> +       else
>> +               reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);
> Same.
>
>> +       if (stmpe->partnum == STMPE1600)
>> +               reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8);
>> +       else
>> +               reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);
> Same.
>
>> +                               stmpe_reg_write(stmpe,
>> +                                               stmpe->regs[regmap[i]] + j,
>> +                                               new);
>> +                       else
>> +                               stmpe_reg_write(stmpe,
>> +                                               stmpe->regs[regmap[i]] - j,
>> +                                               new);
> This is also unintuitively backwards.
>
>> +       if (stmpe->partnum == STMPE1600)
>> +               dir_reg = stmpe->regs[STMPE_IDX_GPDR_LSB] + (offset / 8);
>> +       else
>> +               dir_reg = stmpe->regs[STMPE_IDX_GPDR_LSB] - (offset / 8);
> Same.
>
>> +       if (stmpe->partnum == STMPE1600)
>> +               statmsbreg = stmpe->regs[STMPE_IDX_ISGPIOR_LSB];
>> +       else
>> +               statmsbreg = stmpe->regs[STMPE_IDX_ISGPIOR_MSB];
> And this kind of points at the problem.
>
> Can we write this in some way that make it super-clear which register
> we're using and why?

Ok i will rework all these points

Thanks

Patrice

>
> Yours,
> Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1382437 — [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection

From	<patrice.chotard@st.com>
Date	2016-04-19 14:30 +0200
Subject	[PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection
Message-ID	<rpCX0-6Oi-7@gated-at.bofh.it>
In reply to	#1382432

From: Patrice Chotard <patrice.chotard@st.com>

By cross-checking STMPE 610/801/811/1601/2401/2403 datasheets,
it appears that edge detection and rising/falling edge detection
is not supported by all STMPE variant:

           GPIO              GPIO
      Edge detection     rising/falling
                         edge detection
 610 |      X        |         X       |
 801 |               |                 |
 811 |      X        |         X       |
1600 |               |                 |
1601 |      X        |         X       |
1801 |               |         X       |
2401 |      X        |         X       |
2403 |      X        |         X       |

Rework stmpe_dbg_show_one() and stmpe_gpio_irq to correctly
take these cases into account.

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/gpio/gpio-stmpe.c | 58 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c
index 5197edf..225e075 100644
--- a/drivers/gpio/gpio-stmpe.c
+++ b/drivers/gpio/gpio-stmpe.c
@@ -231,27 +231,51 @@ static void stmpe_dbg_show_one(struct seq_file *s,
 			   gpio, label ?: "(none)",
 			   val ? "hi" : "lo");
 	} else {
-		u8 edge_det_reg = stmpe->regs[STMPE_IDX_GPEDR_MSB] + num_banks - 1 - (offset / 8);
-		u8 rise_reg = stmpe->regs[STMPE_IDX_GPRER_LSB] - (offset / 8);
-		u8 fall_reg = stmpe->regs[STMPE_IDX_GPFER_LSB] - (offset / 8);
-		u8 irqen_reg = stmpe->regs[STMPE_IDX_IEGPIOR_LSB] - (offset / 8);
+		u8 edge_det_reg;
+		u8 rise_reg;
+		u8 fall_reg;
+		u8 irqen_reg;
 		bool edge_det;
 		bool rise;
 		bool fall;
 		bool irqen;
 
-		ret = stmpe_reg_read(stmpe, edge_det_reg);
-		if (ret < 0)
-			return;
-		edge_det = !!(ret & mask);
-		ret = stmpe_reg_read(stmpe, rise_reg);
-		if (ret < 0)
+		switch (stmpe->partnum) {
+		case STMPE610:
+		case STMPE811:
+		case STMPE1601:
+		case STMPE2401:
+		case STMPE2403:
+			edge_det_reg = stmpe->regs[STMPE_IDX_GPEDR_MSB] +
+				       num_banks - 1 - (offset / 8);
+			ret = stmpe_reg_read(stmpe, edge_det_reg);
+			if (ret < 0)
+				return;
+			edge_det = !!(ret & mask);
+
+		case STMPE1801:
+			rise_reg = stmpe->regs[STMPE_IDX_GPRER_LSB] -
+				   (offset / 8);
+			fall_reg = stmpe->regs[STMPE_IDX_GPFER_LSB] -
+				   (offset / 8);
+			ret = stmpe_reg_read(stmpe, rise_reg);
+			if (ret < 0)
+				return;
+			rise = !!(ret & mask);
+			ret = stmpe_reg_read(stmpe, fall_reg);
+			if (ret < 0)
+				return;
+			fall = !!(ret & mask);
+
+		case STMPE801:
+			irqen_reg = stmpe->regs[STMPE_IDX_IEGPIOR_LSB] -
+				    (offset / 8);
+			break;
+
+		default:
 			return;
-		rise = !!(ret & mask);
-		ret = stmpe_reg_read(stmpe, fall_reg);
-		if (ret < 0)
-			return;
-		fall = !!(ret & mask);
+		}
+
 		ret = stmpe_reg_read(stmpe, irqen_reg);
 		if (ret < 0)
 			return;
@@ -322,8 +346,8 @@ static irqreturn_t stmpe_gpio_irq(int irq, void *dev)
 
 		stmpe_reg_write(stmpe, statmsbreg + i, status[i]);
 
-		/* Edge detect register is not present on 801 */
-		if (stmpe->partnum != STMPE801)
+		/* Edge detect register is not present on 801 and 1801 */
+		if (stmpe->partnum != STMPE801 || stmpe->partnum != STMPE1801)
 			stmpe_reg_write(stmpe, stmpe->regs[STMPE_IDX_GPEDR_MSB]
 					+ i, status[i]);
 	}
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1382449 — Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection

From	kbuild test robot <lkp@intel.com>
Date	2016-04-19 14:40 +0200
Subject	Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection
Message-ID	<rpD6F-6Tc-1@gated-at.bofh.it>
In reply to	#1382437

[Multipart message — attachments visible in raw view] — view raw

Hi,

[auto build test WARNING on ljones-mfd/for-mfd-next]
[also build test WARNING on v4.6-rc4 next-20160419]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/patrice-chotard-st-com/STMPE-fixes-rework-and-add-STMPE1600-support/20160419-202526
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: x86_64-randconfig-x010-201616 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/gpio/gpio-stmpe.c: In function 'stmpe_dbg_show':
>> drivers/gpio/gpio-stmpe.c:284:3: warning: 'fall' may be used uninitialized in this function [-Wmaybe-uninitialized]
      seq_printf(s, " gpio-%-3d (%-20.20s) in  %s %s %s%s%s",
      ^
   drivers/gpio/gpio-stmpe.c:240:8: note: 'fall' was declared here
      bool fall;
           ^
>> drivers/gpio/gpio-stmpe.c:284:3: warning: 'rise' may be used uninitialized in this function [-Wmaybe-uninitialized]
      seq_printf(s, " gpio-%-3d (%-20.20s) in  %s %s %s%s%s",
      ^
   drivers/gpio/gpio-stmpe.c:239:8: note: 'rise' was declared here
      bool rise;
           ^
>> drivers/gpio/gpio-stmpe.c:284:3: warning: 'edge_det' may be used uninitialized in this function [-Wmaybe-uninitialized]
      seq_printf(s, " gpio-%-3d (%-20.20s) in  %s %s %s%s%s",
      ^
   drivers/gpio/gpio-stmpe.c:238:8: note: 'edge_det' was declared here
      bool edge_det;
           ^

vim +/fall +284 drivers/gpio/gpio-stmpe.c

98190e59 Patrice Chotard 2016-04-19  234  		u8 edge_det_reg;
98190e59 Patrice Chotard 2016-04-19  235  		u8 rise_reg;
98190e59 Patrice Chotard 2016-04-19  236  		u8 fall_reg;
98190e59 Patrice Chotard 2016-04-19  237  		u8 irqen_reg;
27ec8a9c Linus Walleij   2014-10-02  238  		bool edge_det;
27ec8a9c Linus Walleij   2014-10-02  239  		bool rise;
27ec8a9c Linus Walleij   2014-10-02 @240  		bool fall;
27ec8a9c Linus Walleij   2014-10-02  241  		bool irqen;
27ec8a9c Linus Walleij   2014-10-02  242  
98190e59 Patrice Chotard 2016-04-19  243  		switch (stmpe->partnum) {
98190e59 Patrice Chotard 2016-04-19  244  		case STMPE610:
98190e59 Patrice Chotard 2016-04-19  245  		case STMPE811:
98190e59 Patrice Chotard 2016-04-19  246  		case STMPE1601:
98190e59 Patrice Chotard 2016-04-19  247  		case STMPE2401:
98190e59 Patrice Chotard 2016-04-19  248  		case STMPE2403:
98190e59 Patrice Chotard 2016-04-19  249  			edge_det_reg = stmpe->regs[STMPE_IDX_GPEDR_MSB] +
98190e59 Patrice Chotard 2016-04-19  250  				       num_banks - 1 - (offset / 8);
27ec8a9c Linus Walleij   2014-10-02  251  			ret = stmpe_reg_read(stmpe, edge_det_reg);
27ec8a9c Linus Walleij   2014-10-02  252  			if (ret < 0)
27ec8a9c Linus Walleij   2014-10-02  253  				return;
27ec8a9c Linus Walleij   2014-10-02  254  			edge_det = !!(ret & mask);
98190e59 Patrice Chotard 2016-04-19  255  
98190e59 Patrice Chotard 2016-04-19  256  		case STMPE1801:
98190e59 Patrice Chotard 2016-04-19  257  			rise_reg = stmpe->regs[STMPE_IDX_GPRER_LSB] -
98190e59 Patrice Chotard 2016-04-19  258  				   (offset / 8);
98190e59 Patrice Chotard 2016-04-19  259  			fall_reg = stmpe->regs[STMPE_IDX_GPFER_LSB] -
98190e59 Patrice Chotard 2016-04-19  260  				   (offset / 8);
27ec8a9c Linus Walleij   2014-10-02  261  			ret = stmpe_reg_read(stmpe, rise_reg);
27ec8a9c Linus Walleij   2014-10-02  262  			if (ret < 0)
27ec8a9c Linus Walleij   2014-10-02  263  				return;
27ec8a9c Linus Walleij   2014-10-02  264  			rise = !!(ret & mask);
27ec8a9c Linus Walleij   2014-10-02  265  			ret = stmpe_reg_read(stmpe, fall_reg);
27ec8a9c Linus Walleij   2014-10-02  266  			if (ret < 0)
27ec8a9c Linus Walleij   2014-10-02  267  				return;
27ec8a9c Linus Walleij   2014-10-02  268  			fall = !!(ret & mask);
98190e59 Patrice Chotard 2016-04-19  269  
98190e59 Patrice Chotard 2016-04-19  270  		case STMPE801:
98190e59 Patrice Chotard 2016-04-19  271  			irqen_reg = stmpe->regs[STMPE_IDX_IEGPIOR_LSB] -
98190e59 Patrice Chotard 2016-04-19  272  				    (offset / 8);
98190e59 Patrice Chotard 2016-04-19  273  			break;
98190e59 Patrice Chotard 2016-04-19  274  
98190e59 Patrice Chotard 2016-04-19  275  		default:
98190e59 Patrice Chotard 2016-04-19  276  			return;
98190e59 Patrice Chotard 2016-04-19  277  		}
98190e59 Patrice Chotard 2016-04-19  278  
27ec8a9c Linus Walleij   2014-10-02  279  		ret = stmpe_reg_read(stmpe, irqen_reg);
27ec8a9c Linus Walleij   2014-10-02  280  		if (ret < 0)
27ec8a9c Linus Walleij   2014-10-02  281  			return;
27ec8a9c Linus Walleij   2014-10-02  282  		irqen = !!(ret & mask);
27ec8a9c Linus Walleij   2014-10-02  283  
27ec8a9c Linus Walleij   2014-10-02 @284  		seq_printf(s, " gpio-%-3d (%-20.20s) in  %s %s %s%s%s",
27ec8a9c Linus Walleij   2014-10-02  285  			   gpio, label ?: "(none)",
27ec8a9c Linus Walleij   2014-10-02  286  			   val ? "hi" : "lo",
27ec8a9c Linus Walleij   2014-10-02  287  			   edge_det ? "edge-asserted" : "edge-inactive",

:::::: The code at line 284 was first introduced by commit
:::::: 27ec8a9cb504e9995c123dc74e0cca0cba81d07f gpio: stmpe: add verbose debug code

:::::: TO: Linus Walleij <linus.walleij@linaro.org>
:::::: CC: Linus Walleij <linus.walleij@linaro.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1383449 — Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection

From	Linus Walleij <linus.walleij@linaro.org>
Date	2016-04-20 16:40 +0200
Subject	Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection
Message-ID	<rq1sn-VS-23@gated-at.bofh.it>
In reply to	#1382437

On Tue, Apr 19, 2016 at 2:18 PM,  <patrice.chotard@st.com> wrote:

> From: Patrice Chotard <patrice.chotard@st.com>
>
> By cross-checking STMPE 610/801/811/1601/2401/2403 datasheets,
> it appears that edge detection and rising/falling edge detection
> is not supported by all STMPE variant:
>
>            GPIO              GPIO
>       Edge detection     rising/falling
>                          edge detection
>  610 |      X        |         X       |
>  801 |               |                 |
>  811 |      X        |         X       |
> 1600 |               |                 |
> 1601 |      X        |         X       |
> 1801 |               |         X       |
> 2401 |      X        |         X       |
> 2403 |      X        |         X       |
>
> Rework stmpe_dbg_show_one() and stmpe_gpio_irq to correctly
> take these cases into account.
>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

Very nice.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

I expect this to go into the MFD tree with the rest, I guess
Lee will cook me an immutable branch for the whole thing
once he's happy with it.

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1384240 — Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection

From	Patrice Chotard <patrice.chotard@st.com>
Date	2016-04-21 15:50 +0200
Subject	Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection
Message-ID	<rqn9w-1tU-5@gated-at.bofh.it>
In reply to	#1383449


On 04/20/2016 04:37 PM, Linus Walleij wrote:
> On Tue, Apr 19, 2016 at 2:18 PM,  <patrice.chotard@st.com> wrote:
>
>> From: Patrice Chotard <patrice.chotard@st.com>
>>
>> By cross-checking STMPE 610/801/811/1601/2401/2403 datasheets,
>> it appears that edge detection and rising/falling edge detection
>> is not supported by all STMPE variant:
>>
>>             GPIO              GPIO
>>        Edge detection     rising/falling
>>                           edge detection
>>   610 |      X        |         X       |
>>   801 |               |                 |
>>   811 |      X        |         X       |
>> 1600 |               |                 |
>> 1601 |      X        |         X       |
>> 1801 |               |         X       |
>> 2401 |      X        |         X       |
>> 2403 |      X        |         X       |
>>
>> Rework stmpe_dbg_show_one() and stmpe_gpio_irq to correctly
>> take these cases into account.
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> Very nice.
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> I expect this to go into the MFD tree with the rest, I guess
> Lee will cook me an immutable branch for the whole thing
> once he's happy with it.
>
> Yours,
> Linus Walleij

Hi Linus

I will send a v2 as warnings as been detected by kbuild test robot

Thanks

Patrice

[toc] | [prev] | [next] | [standalone]

#1382438 — [PATCH 8/8] gpio: stmpe: configure GPIO as output by default

From	<patrice.chotard@st.com>
Date	2016-04-19 14:30 +0200
Subject	[PATCH 8/8] gpio: stmpe: configure GPIO as output by default
Message-ID	<rpCX0-6Oi-17@gated-at.bofh.it>
In reply to	#1382432

From: Patrice Chotard <patrice.chotard@st.com>

Configures all GPIOs as output, in order to minimize power
consumption when GPIOs are unused.

Signed-off-by: Amelie DELAUNAY <amelie.delaunay@st.com>
Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/gpio/gpio-stmpe.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c
index 45e5b92..80c6ae6 100644
--- a/drivers/gpio/gpio-stmpe.c
+++ b/drivers/gpio/gpio-stmpe.c
@@ -410,7 +410,7 @@ static int stmpe_gpio_probe(struct platform_device *pdev)
 	struct stmpe *stmpe = dev_get_drvdata(pdev->dev.parent);
 	struct device_node *np = pdev->dev.of_node;
 	struct stmpe_gpio *stmpe_gpio;
-	int ret;
+	int ret, i;
 	int irq = 0;
 
 	irq = platform_get_irq(pdev, 0);
@@ -475,6 +475,10 @@ static int stmpe_gpio_probe(struct platform_device *pdev)
 					     NULL);
 	}
 
+	/* To minimize power consumption, configure unused GPIOs as outputs */
+	for (i = 0; i < stmpe_gpio->chip.ngpio; i++)
+		stmpe_gpio_direction_output(&stmpe_gpio->chip, i, 0);
+
 	platform_set_drvdata(pdev, stmpe_gpio);
 
 	return 0;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1383467 — Re: [PATCH 8/8] gpio: stmpe: configure GPIO as output by default

From	Linus Walleij <linus.walleij@linaro.org>
Date	2016-04-20 17:00 +0200
Subject	Re: [PATCH 8/8] gpio: stmpe: configure GPIO as output by default
Message-ID	<rq1LI-13P-9@gated-at.bofh.it>
In reply to	#1382438

On Tue, Apr 19, 2016 at 2:18 PM,  <patrice.chotard@st.com> wrote:

> From: Patrice Chotard <patrice.chotard@st.com>
>
> Configures all GPIOs as output, in order to minimize power
> consumption when GPIOs are unused.
>
> Signed-off-by: Amelie DELAUNAY <amelie.delaunay@st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

Not only do you set them all to outout but also:

> +       /* To minimize power consumption, configure unused GPIOs as outputs */
> +       for (i = 0; i < stmpe_gpio->chip.ngpio; i++)
> +               stmpe_gpio_direction_output(&stmpe_gpio->chip, i, 0);

You are driving them all low.

Now GPIO is general purpose: what if they are connected to a
line with a pull-up resistor?

That is not saving power, instead consuming more than if you
were setting them all to 1.

I am afraid this is wrong.

What you need is to be able to define in the DT (or similar)
a set of initial values for the GPIO lines, and set them to
0 for this design.

Such bindings have been discussed but no conclusion
or merged patch has emerged. Please help out in driving
a standard for this!

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1382440 — [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant

From	<patrice.chotard@st.com>
Date	2016-04-19 14:30 +0200
Subject	[PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant
Message-ID	<rpCX1-6Oi-29@gated-at.bofh.it>
In reply to	#1382432

From: Patrice Chotard <patrice.chotard@st.com>

Reset was only implemented for STMPE1801 variant despite
all variant have a SOFT_RESET bit.

For STMPE2401/2403/801/1601/1801 SOFT_RESET bit is bit 7
of SYS_CTRL register.
For STMPE610/811 (which have the same variant id) SOFT_RESET
bit is bit 1 of SYS_CTRL register.

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/mfd/stmpe.c | 23 +++++++++++++++--------
 drivers/mfd/stmpe.h |  7 +++++--
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
index c553b73..af682d0 100644
--- a/drivers/mfd/stmpe.c
+++ b/drivers/mfd/stmpe.c
@@ -735,13 +735,22 @@ static int stmpe1801_enable(struct stmpe *stmpe, unsigned int blocks,
 				enable ? mask : 0);
 }
 
-static int stmpe1801_reset(struct stmpe *stmpe)
+static int stmpe_reset(struct stmpe *stmpe)
 {
+	u16 id_val = stmpe->variant->id_val;
 	unsigned long timeout;
 	int ret = 0;
+	u8 reset_bit;
+
+	if (id_val == STMPE811_ID)
+		/* STMPE801 and STMPE610 use bit 1 of SYS_CTRL register */
+		reset_bit = STMPE811_SYS_CTRL_RESET;
+	else
+		/* all other STMPE variant use bit 7 of SYS_CTRL register */
+		reset_bit = STMPE_SYS_CTRL_RESET;
 
 	ret = __stmpe_set_bits(stmpe, stmpe->regs[STMPE_IDX_SYS_CTRL],
-		STMPE1801_MSK_SYS_CTRL_RESET, STMPE1801_MSK_SYS_CTRL_RESET);
+			       reset_bit, reset_bit);
 	if (ret < 0)
 		return ret;
 
@@ -750,7 +759,7 @@ static int stmpe1801_reset(struct stmpe *stmpe)
 		ret = __stmpe_reg_read(stmpe, stmpe->regs[STMPE_IDX_SYS_CTRL]);
 		if (ret < 0)
 			return ret;
-		if (!(ret & STMPE1801_MSK_SYS_CTRL_RESET))
+		if (!(ret & reset_bit))
 			return 0;
 		usleep_range(100, 200);
 	}
@@ -1074,11 +1083,9 @@ static int stmpe_chip_init(struct stmpe *stmpe)
 	if (ret)
 		return ret;
 
-	if (id == STMPE1801_ID)	{
-		ret =  stmpe1801_reset(stmpe);
-		if (ret < 0)
-			return ret;
-	}
+	ret =  stmpe_reset(stmpe);
+	if (ret < 0)
+		return ret;
 
 	if (stmpe->irq >= 0) {
 		if (id == STMPE801_ID)
diff --git a/drivers/mfd/stmpe.h b/drivers/mfd/stmpe.h
index 406f9f2..4ae343d 100644
--- a/drivers/mfd/stmpe.h
+++ b/drivers/mfd/stmpe.h
@@ -104,6 +104,8 @@ int stmpe_remove(struct stmpe *stmpe);
 #define STMPE_ICR_LSB_EDGE	(1 << 1)
 #define STMPE_ICR_LSB_GIM	(1 << 0)
 
+#define STMPE_SYS_CTRL_RESET	(1 << 7)
+
 /*
  * STMPE801
  */
@@ -126,6 +128,7 @@ int stmpe_remove(struct stmpe *stmpe);
 /*
  * STMPE811
  */
+#define STMPE811_ID			0x0811
 
 #define STMPE811_IRQ_TOUCH_DET		0
 #define STMPE811_IRQ_FIFO_TH		1
@@ -155,6 +158,8 @@ int stmpe_remove(struct stmpe *stmpe);
 #define STMPE811_REG_GPIO_FE		0x16
 #define STMPE811_REG_GPIO_AF		0x17
 
+#define STMPE811_SYS_CTRL_RESET		(1 << 1)
+
 #define STMPE811_SYS_CTRL2_ADC_OFF	(1 << 0)
 #define STMPE811_SYS_CTRL2_TSC_OFF	(1 << 1)
 #define STMPE811_SYS_CTRL2_GPIO_OFF	(1 << 2)
@@ -244,8 +249,6 @@ int stmpe_remove(struct stmpe *stmpe);
 #define STMPE1801_REG_GPIO_PULL_UP_MID		0x23
 #define STMPE1801_REG_GPIO_PULL_UP_HIGH		0x24
 
-#define STMPE1801_MSK_SYS_CTRL_RESET		(1 << 7)
-
 #define STMPE1801_MSK_INT_EN_KPC		(1 << 1)
 #define STMPE1801_MSK_INT_EN_GPIO		(1 << 3)
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1383445 — Re: [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant

From	Linus Walleij <linus.walleij@linaro.org>
Date	2016-04-20 16:40 +0200
Subject	Re: [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant
Message-ID	<rq1sm-VS-11@gated-at.bofh.it>
In reply to	#1382440

On Tue, Apr 19, 2016 at 2:18 PM,  <patrice.chotard@st.com> wrote:

> From: Patrice Chotard <patrice.chotard@st.com>
>
> Reset was only implemented for STMPE1801 variant despite
> all variant have a SOFT_RESET bit.
>
> For STMPE2401/2403/801/1601/1801 SOFT_RESET bit is bit 7
> of SYS_CTRL register.
> For STMPE610/811 (which have the same variant id) SOFT_RESET
> bit is bit 1 of SYS_CTRL register.
>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1387220 — Re: [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant

From	Lee Jones <lee.jones@linaro.org>
Date	2016-04-26 10:20 +0200
Subject	Re: [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant
Message-ID	<rs6nV-45z-25@gated-at.bofh.it>
In reply to	#1382440

On Tue, 19 Apr 2016, patrice.chotard@st.com wrote:

> From: Patrice Chotard <patrice.chotard@st.com>
> 
> Reset was only implemented for STMPE1801 variant despite
> all variant have a SOFT_RESET bit.
> 
> For STMPE2401/2403/801/1601/1801 SOFT_RESET bit is bit 7
> of SYS_CTRL register.
> For STMPE610/811 (which have the same variant id) SOFT_RESET
> bit is bit 1 of SYS_CTRL register.
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>  drivers/mfd/stmpe.c | 23 +++++++++++++++--------
>  drivers/mfd/stmpe.h |  7 +++++--
>  2 files changed, 20 insertions(+), 10 deletions(-)

When you fix and resubmit, please add my:

  Acked-by: Lee Jones <lee.jones@linaro.org>

... for my own personal reference.

> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c
> index c553b73..af682d0 100644
> --- a/drivers/mfd/stmpe.c
> +++ b/drivers/mfd/stmpe.c
> @@ -735,13 +735,22 @@ static int stmpe1801_enable(struct stmpe *stmpe, unsigned int blocks,
>  				enable ? mask : 0);
>  }
>  
> -static int stmpe1801_reset(struct stmpe *stmpe)
> +static int stmpe_reset(struct stmpe *stmpe)
>  {
> +	u16 id_val = stmpe->variant->id_val;
>  	unsigned long timeout;
>  	int ret = 0;
> +	u8 reset_bit;
> +
> +	if (id_val == STMPE811_ID)
> +		/* STMPE801 and STMPE610 use bit 1 of SYS_CTRL register */
> +		reset_bit = STMPE811_SYS_CTRL_RESET;
> +	else
> +		/* all other STMPE variant use bit 7 of SYS_CTRL register */
> +		reset_bit = STMPE_SYS_CTRL_RESET;
>  
>  	ret = __stmpe_set_bits(stmpe, stmpe->regs[STMPE_IDX_SYS_CTRL],
> -		STMPE1801_MSK_SYS_CTRL_RESET, STMPE1801_MSK_SYS_CTRL_RESET);
> +			       reset_bit, reset_bit);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -750,7 +759,7 @@ static int stmpe1801_reset(struct stmpe *stmpe)
>  		ret = __stmpe_reg_read(stmpe, stmpe->regs[STMPE_IDX_SYS_CTRL]);
>  		if (ret < 0)
>  			return ret;
> -		if (!(ret & STMPE1801_MSK_SYS_CTRL_RESET))
> +		if (!(ret & reset_bit))
>  			return 0;
>  		usleep_range(100, 200);
>  	}
> @@ -1074,11 +1083,9 @@ static int stmpe_chip_init(struct stmpe *stmpe)
>  	if (ret)
>  		return ret;
>  
> -	if (id == STMPE1801_ID)	{
> -		ret =  stmpe1801_reset(stmpe);
> -		if (ret < 0)
> -			return ret;
> -	}
> +	ret =  stmpe_reset(stmpe);
> +	if (ret < 0)
> +		return ret;
>  
>  	if (stmpe->irq >= 0) {
>  		if (id == STMPE801_ID)
> diff --git a/drivers/mfd/stmpe.h b/drivers/mfd/stmpe.h
> index 406f9f2..4ae343d 100644
> --- a/drivers/mfd/stmpe.h
> +++ b/drivers/mfd/stmpe.h
> @@ -104,6 +104,8 @@ int stmpe_remove(struct stmpe *stmpe);
>  #define STMPE_ICR_LSB_EDGE	(1 << 1)
>  #define STMPE_ICR_LSB_GIM	(1 << 0)
>  
> +#define STMPE_SYS_CTRL_RESET	(1 << 7)
> +
>  /*
>   * STMPE801
>   */
> @@ -126,6 +128,7 @@ int stmpe_remove(struct stmpe *stmpe);
>  /*
>   * STMPE811
>   */
> +#define STMPE811_ID			0x0811
>  
>  #define STMPE811_IRQ_TOUCH_DET		0
>  #define STMPE811_IRQ_FIFO_TH		1
> @@ -155,6 +158,8 @@ int stmpe_remove(struct stmpe *stmpe);
>  #define STMPE811_REG_GPIO_FE		0x16
>  #define STMPE811_REG_GPIO_AF		0x17
>  
> +#define STMPE811_SYS_CTRL_RESET		(1 << 1)
> +
>  #define STMPE811_SYS_CTRL2_ADC_OFF	(1 << 0)
>  #define STMPE811_SYS_CTRL2_TSC_OFF	(1 << 1)
>  #define STMPE811_SYS_CTRL2_GPIO_OFF	(1 << 2)
> @@ -244,8 +249,6 @@ int stmpe_remove(struct stmpe *stmpe);
>  #define STMPE1801_REG_GPIO_PULL_UP_MID		0x23
>  #define STMPE1801_REG_GPIO_PULL_UP_HIGH		0x24
>  
> -#define STMPE1801_MSK_SYS_CTRL_RESET		(1 << 7)
> -
>  #define STMPE1801_MSK_INT_EN_KPC		(1 << 1)
>  #define STMPE1801_MSK_INT_EN_GPIO		(1 << 3)
>  

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

[toc] | [prev] | [next] | [standalone]

#1382477

From	Thierry Reding <thierry.reding@gmail.com>
Date	2016-04-19 14:50 +0200
Message-ID	<rpDgn-6X3-25@gated-at.bofh.it>
In reply to	#1382432

[Multipart message — attachments visible in raw view] — view raw

On Tue, Apr 19, 2016 at 02:18:29PM +0200, patrice.chotard@st.com wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
> 
> This series cleans and fixes some bugs in MFD/GPIO STMPE drivers and prepare
>  the ground to add new STMPE1600 support.
> 
> STMPE1600 datasheet is available here : 
> http://www2.st.com/content/st_com/en/products/interfaces-and-transceivers/
> i-o-expanders-and-level-translators/i-o-expanders/stmpe1600.html
> 
> Only STMPE1600 has been tested on STM32 platform. As i have no board with
> others STMPE variant(STMPE610/STMPE801/STMPE811/STMPE1601/STMPE1801/STMPE2401
> and STMPE2403), i put in CC boards's maintainers which are using others STMPE variant.
> 
> If they can kindly check that no regression has been introduce by this series
> :
> 
> For ARM/FREESCALE IMX / MXC ARM ARCHITECTURE:
> 	_ Shawn Guo <shawnguo@kernel.org>
> 	_ Sascha Hauer <kernel@pengutronix.de>
> 
> For ARM/SOCFPGA ARCHITECTURE
> 	_ Dinh Nguyen <dinguyen@opensource.altera.com>
> 
> For SPEAR PLATFORM SUPPORT
> 	_ Viresh Kumar <vireshk@kernel.org>
> 	_ Shiraz Hashim <shiraz.linux.kernel@gmail.com>
> 
> For TEGRA ARCHITECTURE SUPPORT
> 	_ Stephen Warren <swarren@wwwdotorg.org>
> 	_ Thierry Reding <thierry.reding@gmail.com>
> 	_ Alexandre Courbot <gnurou@gmail.com> 

Adding Marcel Ziswiler, who's better suited at judging whether or not
this has any impact on Apalis/Colibri.

Marcel, in case you don't have these in your inbox you can find them on
linux-gpio's patchwork:

	https://patchwork.ozlabs.org/project/linux-gpio/list/

Thierry

[toc] | [prev] | [next] | [standalone]

#1383594

From	Marcel Ziswiler <marcel.ziswiler@toradex.com>
Date	2016-04-20 20:00 +0200
Message-ID	<rq4zU-3eo-7@gated-at.bofh.it>
In reply to	#1382477

On Apr 19, 2016 14:41, Thierry Reding <thierry.reding@gmail.com> wrote:
>...
> Adding Marcel Ziswiler, who's better suited a

On Tue, Apr 19, 2016 at 02:18:29PM +0200, patrice.chotard@st.com wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
>
> This series cleans and fixes some bugs in MFD/GPIO STMPE drivers and prepare
>  the ground to add new STMPE1600 support.
>
> STMPE1600 datasheet is available here :
> http://www2.st.com/content/st_com/en/products/interfaces-and-transceivers/
> i-o-expanders-and-level-translators/i-o-expanders/stmpe1600.html
>
> Only STMPE1600 has been tested on STM32 platform. As i have no board with
> others STMPE variant(STMPE610/STMPE801/STMPE811/STMPE1601/STMPE1801/STMPE2401
> and STMPE2403), i put in CC boards's maintainers which are using others STMPE variant.
>
> If they can kindly check that no regression has been introduce by this series
> :
>
> For ARM/FREESCALE IMX / MXC ARM ARCHITECTURE:
>       _ Shawn Guo <shawnguo@kernel.org>
>       _ Sascha Hauer <kernel@pengutronix.de>
>
> For ARM/SOCFPGA ARCHITECTURE
>       _ Dinh Nguyen <dinguyen@opensource.altera.com>
>
> For SPEAR PLATFORM SUPPORT
>       _ Viresh Kumar <vireshk@kernel.org>
>       _ Shiraz Hashim <shiraz.linux.kernel@gmail.com>
>
> For TEGRA ARCHITECTURE SUPPORT
>       _ Stephen Warren <swarren@wwwdotorg.org>
>       _ Thierry Reding <thierry.reding@gmail.com>
>       _ Alexandre Courbot <gnurou@gmail.com>

Adding Marcel Ziswiler, who's better suited at judging whether or not
this has any impact on Apalis/Colibri.

Marcel, in case you don't have these in your inbox you can find them on
linux-gpio's patchwork:

        https://patchwork.ozlabs.org/project/linux-gpio/list/

Thierry

[toc] | [prev] | [next] | [standalone]

#1382634

From	Stephen Warren <swarren@wwwdotorg.org>
Date	2016-04-19 18:00 +0200
Message-ID	<rpGee-KT-9@gated-at.bofh.it>
In reply to	#1382432

On 04/19/2016 06:18 AM, patrice.chotard@st.com wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
>
> This series cleans and fixes some bugs in MFD/GPIO STMPE drivers and prepare
>   the ground to add new STMPE1600 support.
>
> STMPE1600 datasheet is available here :
> http://www2.st.com/content/st_com/en/products/interfaces-and-transceivers/
> i-o-expanders-and-level-translators/i-o-expanders/stmpe1600.html
>
> Only STMPE1600 has been tested on STM32 platform. As i have no board with
> others STMPE variant(STMPE610/STMPE801/STMPE811/STMPE1601/STMPE1801/STMPE2401
> and STMPE2403), i put in CC boards's maintainers which are using others STMPE variant.
>
> If they can kindly check that no regression has been introduce by this series

> For TEGRA ARCHITECTURE SUPPORT
> 	_ Stephen Warren <swarren@wwwdotorg.org>
> 	_ Thierry Reding <thierry.reding@gmail.com>
> 	_ Alexandre Courbot <gnurou@gmail.com>

I don't know what STMPE is, and I don't believe it is used on Tegra; 
what makes you think it is?

[toc] | [prev] | [next] | [standalone]

#1383117

From	Patrice Chotard <patrice.chotard@st.com>
Date	2016-04-20 09:50 +0200
Message-ID	<rpV3A-47Y-13@gated-at.bofh.it>
In reply to	#1382634


On 04/19/2016 05:53 PM, Stephen Warren wrote:
> On 04/19/2016 06:18 AM, patrice.chotard@st.com wrote:
>> From: Patrice Chotard <patrice.chotard@st.com>
>>
>> This series cleans and fixes some bugs in MFD/GPIO STMPE drivers and 
>> prepare
>>   the ground to add new STMPE1600 support.
>>
>> STMPE1600 datasheet is available here :
>> http://www2.st.com/content/st_com/en/products/interfaces-and-transceivers/ 
>>
>> i-o-expanders-and-level-translators/i-o-expanders/stmpe1600.html
>>
>> Only STMPE1600 has been tested on STM32 platform. As i have no board 
>> with
>> others STMPE 
>> variant(STMPE610/STMPE801/STMPE811/STMPE1601/STMPE1801/STMPE2401
>> and STMPE2403), i put in CC boards's maintainers which are using 
>> others STMPE variant.
>>
>> If they can kindly check that no regression has been introduce by 
>> this series
>
>> For TEGRA ARCHITECTURE SUPPORT
>>     _ Stephen Warren <swarren@wwwdotorg.org>
>>     _ Thierry Reding <thierry.reding@gmail.com>
>>     _ Alexandre Courbot <gnurou@gmail.com>
>
> I don't know what STMPE is, and I don't believe it is used on Tegra; 
> what makes you think it is?

Hi Stephen

STMPE family is GPIO expander, and for some of them, it includes others 
fonctionnality:
     _STMPE811 and STMPE610: touchscreen controller
     _ STMPE1601, STMPE2401 and STMPE2403: keypad and PWM controller
     _ STMPE1801: keypad controller

For more informations, some datasheets are available here: 
http://www2.st.com/content/st_com/en/products/interfaces-and-transceivers/i-o-expanders-and-level-translators/i-o-expanders.html?querycriteria=productId=SC1027

drivers/mfd/stmpe.c and drivers/gpio/gpio-stmpe.c are drivers which 
support all STMPE variant.
I put you in copy as STMPE811 is used on tegra30-apalis and 
tegra30-colibri platforms.

Thanks

Patrice

[toc] | [prev] | [next] | [standalone]

#1383533

From	Stephen Warren <swarren@wwwdotorg.org>
Date	2016-04-20 18:10 +0200
Message-ID	<rq2Rs-29W-3@gated-at.bofh.it>
In reply to	#1383117

On 04/20/2016 01:40 AM, Patrice Chotard wrote:
> On 04/19/2016 05:53 PM, Stephen Warren wrote:
>> On 04/19/2016 06:18 AM, patrice.chotard@st.com wrote:
>>> From: Patrice Chotard <patrice.chotard@st.com>
>>>
>>> This series cleans and fixes some bugs in MFD/GPIO STMPE drivers and
>>> prepare
>>>   the ground to add new STMPE1600 support.
>>>
>>> STMPE1600 datasheet is available here :
>>> http://www2.st.com/content/st_com/en/products/interfaces-and-transceivers/
>>>
>>> i-o-expanders-and-level-translators/i-o-expanders/stmpe1600.html
>>>
>>> Only STMPE1600 has been tested on STM32 platform. As i have no board
>>> with
>>> others STMPE
>>> variant(STMPE610/STMPE801/STMPE811/STMPE1601/STMPE1801/STMPE2401
>>> and STMPE2403), i put in CC boards's maintainers which are using
>>> others STMPE variant.
>>>
>>> If they can kindly check that no regression has been introduce by
>>> this series
>>
>>> For TEGRA ARCHITECTURE SUPPORT
>>>     _ Stephen Warren <swarren@wwwdotorg.org>
>>>     _ Thierry Reding <thierry.reding@gmail.com>
>>>     _ Alexandre Courbot <gnurou@gmail.com>
>>
>> I don't know what STMPE is, and I don't believe it is used on Tegra;
>> what makes you think it is?
...
> I put you in copy as STMPE811 is used on tegra30-apalis and
> tegra30-colibri platforms.

Ah. You'd best contact the individual board owners, since those are 
3rd-party Tegra boards and I don't believe anyone at NVIDIA has them to 
test with etc. I added likely candidates to Cc and dropped all the 
individuals unrelated to Tegra to keep the CC list low.

[toc] | [prev] | [next] | [standalone]

#1383849

From	Marcel Ziswiler <marcel.ziswiler@toradex.com>
Date	2016-04-21 05:20 +0200
Message-ID	<rqdjP-27E-1@gated-at.bofh.it>
In reply to	#1383533

On Wed, 2016-04-20 at 10:02 -0600, Stephen Warren wrote:
> On 04/20/2016 01:40 AM, Patrice Chotard wrote:
> > 
> > On 04/19/2016 05:53 PM, Stephen Warren wrote:
> > > 
> > > On 04/19/2016 06:18 AM, patrice.chotard@st.com wrote:
> > > > 
> > > > From: Patrice Chotard <patrice.chotard@st.com>
> > > > 
> > > > This series cleans and fixes some bugs in MFD/GPIO STMPE
> > > > drivers and
> > > > prepare
> > > >   the ground to add new STMPE1600 support.
> > > > 
> > > > STMPE1600 datasheet is available here :
> > > > http://www2.st.com/content/st_com/en/products/interfaces-and-tr
> > > > ansceivers/
> > > > 
> > > > i-o-expanders-and-level-translators/i-o-
> > > > expanders/stmpe1600.html
> > > > 
> > > > Only STMPE1600 has been tested on STM32 platform. As i have no
> > > > board
> > > > with
> > > > others STMPE
> > > > variant(STMPE610/STMPE801/STMPE811/STMPE1601/STMPE1801/STMPE240
> > > > 1
> > > > and STMPE2403), i put in CC boards's maintainers which are
> > > > using
> > > > others STMPE variant.
> > > > 
> > > > If they can kindly check that no regression has been introduce
> > > > by
> > > > this series
> > > > 
> > > > For TEGRA ARCHITECTURE SUPPORT
> > > >     _ Stephen Warren <swarren@wwwdotorg.org>
> > > >     _ Thierry Reding <thierry.reding@gmail.com>
> > > >     _ Alexandre Courbot <gnurou@gmail.com>
> > > I don't know what STMPE is, and I don't believe it is used on
> > > Tegra;
> > > what makes you think it is?
> ...
> > 
> > I put you in copy as STMPE811 is used on tegra30-apalis and
> > tegra30-colibri platforms.
> Ah. You'd best contact the individual board owners, since those are 
> 3rd-party Tegra boards and I don't believe anyone at NVIDIA has them
> to 
> test with etc. I added likely candidates to Cc and dropped all the 
> individuals unrelated to Tegra to keep the CC list low.

I gave the whole series a spin on Apalis T30 2GB V1.1A featuring a
STMPE811 and the EDT VGA touch panel connected to it still works
perfectly running LXDE on top of the modesetting X driver.
Unfortunately the 4.6.0-rc4-next-20160420 I used for testing is broken
beyond easy quick repair on i.MX 6 so I was unable to validate it on
there, sorry.


Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

[toc] | [prev] | [next] | [standalone]

#1383438

From	Linus Walleij <linus.walleij@linaro.org>
Date	2016-04-20 16:30 +0200
Message-ID	<rq1iG-Pz-17@gated-at.bofh.it>
In reply to	#1382634

On Tue, Apr 19, 2016 at 5:53 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/19/2016 06:18 AM, patrice.chotard@st.com wrote:

>> For TEGRA ARCHITECTURE SUPPORT
>>         _ Stephen Warren <swarren@wwwdotorg.org>
>>         _ Thierry Reding <thierry.reding@gmail.com>
>>         _ Alexandre Courbot <gnurou@gmail.com>
>
> I don't know what STMPE is,

ST Microelectronics Multi-Purpose Expander.
Some GPIO and keypad and touchscreen, PWM and what
not.

> and I don't believe it is used on Tegra; what
> makes you think it is?

Probably this:

$ git grep stmpe arch/arm/boot/dts/

arch/arm/boot/dts/tegra30-apalis.dtsi:          stmpe811@41 {
arch/arm/boot/dts/tegra30-apalis.dtsi:                  compatible =
"st,stmpe811";
arch/arm/boot/dts/tegra30-apalis.dtsi:                  stmpe_touchscreen {
arch/arm/boot/dts/tegra30-apalis.dtsi:
compatible = "st,stmpe-ts";
arch/arm/boot/dts/tegra30-colibri.dtsi:         stmpe811@41 {
arch/arm/boot/dts/tegra30-colibri.dtsi:                 compatible =
"st,stmpe811";
arch/arm/boot/dts/tegra30-colibri.dtsi:                 stmpe_touchscreen {
arch/arm/boot/dts/tegra30-colibri.dtsi:
compatible = "st,stmpe-ts";

Yours,
Linus Walleij

[toc] | [prev] | [standalone]

csiph-web

[PATCH 0/8] STMPE fixes/rework and add STMPE1600 support

Contents

#1382432 — [PATCH 0/8] STMPE fixes/rework and add STMPE1600 support

#1382435 — [PATCH 7/8] gpio: stmpe: Add STMPE1600 support

#1383465 — Re: [PATCH 7/8] gpio: stmpe: Add STMPE1600 support

#1384737 — Re: [PATCH 7/8] gpio: stmpe: Add STMPE1600 support

#1382437 — [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection

#1382449 — Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection

#1383449 — Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection

#1384240 — Re: [PATCH 3/8] gpio: stmpe: fix edge and rising/falling edge detection

#1382438 — [PATCH 8/8] gpio: stmpe: configure GPIO as output by default

#1383467 — Re: [PATCH 8/8] gpio: stmpe: configure GPIO as output by default

#1382440 — [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant

#1383445 — Re: [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant

#1387220 — Re: [PATCH 2/8] mfd: stmpe: Add reset support for all STMPE variant

#1382477

#1383594

#1382634

#1383117

#1383533

#1383849

#1383438