diff mbox

[3/8] gpio: stmpe: fix edge and rising/falling edge detection

Message ID 1461068317-28016-4-git-send-email-patrice.chotard@st.com (mailing list archive)
State New, archived
Headers show

Commit Message

Patrice CHOTARD April 19, 2016, 12:18 p.m. UTC 
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(-)

Comments

kernel test robot April 19, 2016, 12:38 p.m. UTC | #1 
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
Linus Walleij April 20, 2016, 2:37 p.m. UTC | #2 
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
Patrice CHOTARD April 21, 2016, 1:48 p.m. UTC | #3 
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
diff mbox

Patch

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]);
 	}