diff mbox

[v1,02/12] input: matrix-keymap: func call coding style nit

Message ID 1371838198-7327-3-git-send-email-gsi@denx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Gerhard Sittig June 21, 2013, 6:09 p.m. UTC
make the matrix_keypad_map_key() routine return an error code
instead of boolean, as its name suggests an action and not a query

Signed-off-by: Gerhard Sittig <gsi@denx.de>
---
 drivers/input/matrix-keymap.c |   19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Marek Vasut June 22, 2013, 2:18 a.m. UTC | #1
Dear Gerhard Sittig,

> make the matrix_keypad_map_key() routine return an error code
> instead of boolean, as its name suggests an action and not a query
> 
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
> ---
>  drivers/input/matrix-keymap.c |   19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
> index 08b61f5..b7091f2 100644
> --- a/drivers/input/matrix-keymap.c
> +++ b/drivers/input/matrix-keymap.c
> @@ -27,9 +27,10 @@
>  #include <linux/module.h>
>  #include <linux/input/matrix_keypad.h>
> 
> -static bool matrix_keypad_map_key(struct input_dev *input_dev,
> -				  unsigned int rows, unsigned int cols,
> -				  unsigned int row_shift, unsigned int key)
> +/* translates packed row/col/code specs to the corresponding keycode[]
> item */ +static int matrix_keypad_map_key(struct input_dev *input_dev,
> +				 unsigned int rows, unsigned int cols,
> +				 unsigned int row_shift, unsigned int key)
>  {
>  	unsigned short *keymap = input_dev->keycode;
>  	unsigned int row = KEY_ROW(key);
> @@ -40,13 +41,13 @@ static bool matrix_keypad_map_key(struct input_dev
> *input_dev, dev_err(input_dev->dev.parent,
>  			"%s: invalid keymap entry 0x%x (row: %d, col: %d, rows: 
%d, cols:
> %d)\n", __func__, key, row, col, rows, cols);
> -		return false;
> +		return -ERANGE;
>  	}
> 
>  	keymap[MATRIX_SCAN_CODE(row, col, row_shift)] = code;
>  	__set_bit(code, input_dev->keybit);
> 
> -	return true;
> +	return 0;
>  }
> 
>  #ifdef CONFIG_OF
> @@ -109,8 +110,8 @@ static int matrix_keypad_parse_of_keymap(const char
> *propname, for (i = 0; i < size; i++) {
>  		unsigned int key = be32_to_cpup(prop + i);
> 
> -		if (!matrix_keypad_map_key(input_dev, rows, cols,
> -					   row_shift, key))

ret = matrix_keypad_map_key(input_dev, rows, cols, row_shift, key);
if (ret)
	return ret;

Now that you return correct error codes from above, you should propagate them 
through.

Best regards,
Marek Vasut
Gerhard Sittig June 22, 2013, 8:22 a.m. UTC | #2
On Sat, Jun 22, 2013 at 04:18 +0200, Marek Vasut wrote:
> 
> ret = matrix_keypad_map_key(input_dev, rows, cols, row_shift, key);
> if (ret)
> 	return ret;
> 
> Now that you return correct error codes from above, you should propagate them 
> through.

Will do, queued for v2.  Thank you for reviewing this.


virtually yours
Gerhard Sittig
Marek Vasut June 22, 2013, 1:23 p.m. UTC | #3
Dear Gerhard Sittig,

> On Sat, Jun 22, 2013 at 04:18 +0200, Marek Vasut wrote:
> > ret = matrix_keypad_map_key(input_dev, rows, cols, row_shift, key);
> > if (ret)
> > 
> > 	return ret;
> > 
> > Now that you return correct error codes from above, you should propagate
> > them through.
> 
> Will do, queued for v2.  Thank you for reviewing this.

Wait a little bit for some more feedback, esp. from the marvell guys ;-)

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
index 08b61f5..b7091f2 100644
--- a/drivers/input/matrix-keymap.c
+++ b/drivers/input/matrix-keymap.c
@@ -27,9 +27,10 @@ 
 #include <linux/module.h>
 #include <linux/input/matrix_keypad.h>
 
-static bool matrix_keypad_map_key(struct input_dev *input_dev,
-				  unsigned int rows, unsigned int cols,
-				  unsigned int row_shift, unsigned int key)
+/* translates packed row/col/code specs to the corresponding keycode[] item */
+static int matrix_keypad_map_key(struct input_dev *input_dev,
+				 unsigned int rows, unsigned int cols,
+				 unsigned int row_shift, unsigned int key)
 {
 	unsigned short *keymap = input_dev->keycode;
 	unsigned int row = KEY_ROW(key);
@@ -40,13 +41,13 @@  static bool matrix_keypad_map_key(struct input_dev *input_dev,
 		dev_err(input_dev->dev.parent,
 			"%s: invalid keymap entry 0x%x (row: %d, col: %d, rows: %d, cols: %d)\n",
 			__func__, key, row, col, rows, cols);
-		return false;
+		return -ERANGE;
 	}
 
 	keymap[MATRIX_SCAN_CODE(row, col, row_shift)] = code;
 	__set_bit(code, input_dev->keybit);
 
-	return true;
+	return 0;
 }
 
 #ifdef CONFIG_OF
@@ -109,8 +110,8 @@  static int matrix_keypad_parse_of_keymap(const char *propname,
 	for (i = 0; i < size; i++) {
 		unsigned int key = be32_to_cpup(prop + i);
 
-		if (!matrix_keypad_map_key(input_dev, rows, cols,
-					   row_shift, key))
+		if (matrix_keypad_map_key(input_dev, rows, cols,
+					  row_shift, key) != 0)
 			return -EINVAL;
 	}
 
@@ -187,8 +188,8 @@  int matrix_keypad_build_keymap(const struct matrix_keymap_data *keymap_data,
 		for (i = 0; i < keymap_data->keymap_size; i++) {
 			unsigned int key = keymap_data->keymap[i];
 
-			if (!matrix_keypad_map_key(input_dev, rows, cols,
-						   row_shift, key))
+			if (matrix_keypad_map_key(input_dev, rows, cols,
+						  row_shift, key) != 0)
 				return -EINVAL;
 		}
 	} else {