From: a0131647 <sourav.poddar@ti.com>
To: Simon Glass <sjg@chromium.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Samuel Ortiz <sameo@linux.intel.com>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
Bill Pemberton <wfp5p@virginia.edu>,
Mark Brown <broonie@opensource.wolfsonmicro.com>,
Javier Martinez Canillas <javier@dowhile0.org>,
Felipe Balbi <balbi@ti.com>,
Alban Bedel <alban.bedel@avionic-design.de>,
<linux-input@vger.kernel.org>
Subject: Re: [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding
Date: Wed, 13 Feb 2013 11:02:49 +0530 [thread overview]
Message-ID: <511B2581.2050706@ti.com> (raw)
In-Reply-To: <1360723347-28701-6-git-send-email-sjg@chromium.org>
Hi,
On Wednesday 13 February 2013 08:12 AM, Simon Glass wrote:
> We now have a binding which adds two parameters to the matrix keypad DT
> node. This is separate from the GPIO-driven matrix keypad binding, and
> unfortunately incompatible, since that uses row-gpios/col-gpios for the
> row and column counts.
>
> So the easiest option here is to provide a function for non-GPIO drivers
> to use to decode the binding.
>
> Note: We could in fact create an entirely separate structure to hold
> these two fields, but it does not seem worth it, yet. If we have more
> parameters then we can add this, and then refactor each driver to hold
> such a structure.
>
> Signed-off-by: Simon Glass<sjg@chromium.org>
> ---
> Changes in v2:
> - Add new patch to decode matrix-keypad DT binding
>
> drivers/input/keyboard/lpc32xx-keys.c | 11 ++++++-----
> drivers/input/keyboard/omap4-keypad.c | 16 +++++-----------
> drivers/input/keyboard/tca8418_keypad.c | 11 +++++++----
> drivers/input/matrix-keymap.c | 20 ++++++++++++++++++++
> include/linux/input/matrix_keypad.h | 11 +++++++++++
> 5 files changed, 49 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/input/keyboard/lpc32xx-keys.c b/drivers/input/keyboard/lpc32xx-keys.c
> index 1b8add6..4218143 100644
> --- a/drivers/input/keyboard/lpc32xx-keys.c
> +++ b/drivers/input/keyboard/lpc32xx-keys.c
> @@ -144,12 +144,13 @@ static int lpc32xx_parse_dt(struct device *dev,
> {
> struct device_node *np = dev->of_node;
> u32 rows = 0, columns = 0;
> + int err;
>
> - of_property_read_u32(np, "keypad,num-rows",&rows);
> - of_property_read_u32(np, "keypad,num-columns",&columns);
> - if (!rows || rows != columns) {
> - dev_err(dev,
> - "rows and columns must be specified and be equal!\n");
> + err = matrix_keypad_parse_of_params(dev,&rows,&columns);
> + if (err)
> + return err;
> + if (rows != columns) {
> + dev_err(dev, "rows and columns must be equal!\n");
> return -EINVAL;
> }
>
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> index e25b022..1b28909 100644
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -215,18 +215,12 @@ static int omap4_keypad_parse_dt(struct device *dev,
> struct omap4_keypad *keypad_data)
> {
> struct device_node *np = dev->of_node;
> + int err;
>
> - if (!np) {
> - dev_err(dev, "missing DT data");
> - return -EINVAL;
> - }
> -
> - of_property_read_u32(np, "keypad,num-rows",&keypad_data->rows);
> - of_property_read_u32(np, "keypad,num-columns",&keypad_data->cols);
> - if (!keypad_data->rows || !keypad_data->cols) {
> - dev_err(dev, "number of keypad rows/columns not specified\n");
> - return -EINVAL;
> - }
> + err = matrix_keypad_parse_of_params(dev,&keypad_data->rows,
> + &keypad_data->cols);
> + if (err)
> + return err;
>
> if (of_get_property(np, "linux,input-no-autorepeat", NULL))
> keypad_data->no_autorepeat = true;
> diff --git a/drivers/input/keyboard/tca8418_keypad.c b/drivers/input/keyboard/tca8418_keypad.c
> index a34cc67..4d5a6d5 100644
> --- a/drivers/input/keyboard/tca8418_keypad.c
> +++ b/drivers/input/keyboard/tca8418_keypad.c
> @@ -288,17 +288,20 @@ static int tca8418_keypad_probe(struct i2c_client *client,
> irq_is_gpio = pdata->irq_is_gpio;
> } else {
> struct device_node *np = dev->of_node;
> - of_property_read_u32(np, "keypad,num-rows",&rows);
> - of_property_read_u32(np, "keypad,num-columns",&cols);
> + int err;
> +
> + err = matrix_keypad_parse_of_params(dev,&rows,&cols);
> + if (err)
> + return err;
> rep = of_property_read_bool(np, "keypad,autorepeat");
> }
>
> - if (!rows || rows> TCA8418_MAX_ROWS) {
> + if (rows> TCA8418_MAX_ROWS) {
> dev_err(dev, "invalid rows\n");
> return -EINVAL;
> }
>
> - if (!cols || cols> TCA8418_MAX_COLS) {
> + if (cols> TCA8418_MAX_COLS) {
> dev_err(dev, "invalid columns\n");
> return -EINVAL;
> }
> diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
> index 3ae496e..64bad81 100644
> --- a/drivers/input/matrix-keymap.c
> +++ b/drivers/input/matrix-keymap.c
> @@ -50,6 +50,25 @@ static bool matrix_keypad_map_key(struct input_dev *input_dev,
> }
>
> #ifdef CONFIG_OF
> +int matrix_keypad_parse_of_params(struct device *dev,
> + unsigned int *rows, unsigned int *cols)
> +{
> + struct device_node *np = dev->of_node;
> +
> + if (!np) {
> + dev_err(dev, "missing DT data");
> + return -EINVAL;
> + }
> + of_property_read_u32(np, "keypad,num-rows", rows);
> + of_property_read_u32(np, "keypad,num-columns", cols);
> + if (!*rows || !*cols) {
> + dev_err(dev, "number of keypad rows/columns not specified\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int matrix_keypad_parse_of_keymap(const char *propname,
> unsigned int rows, unsigned int cols,
> struct input_dev *input_dev)
> @@ -112,6 +131,7 @@ static int matrix_keypad_parse_of_keymap(const char *propname,
> * tree support is enabled).
> * @rows: number of rows in target keymap array
> * @cols: number of cols in target keymap array
> + * (if either/both is 0 then they will be read from the FDT if available)
> * @keymap: expanded version of keymap that is suitable for use by
> * matrix keyboard driver
> * @input_dev: input devices for which we are setting up the keymap
> diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
> index 5f3aa6b..01a30cf 100644
> --- a/include/linux/input/matrix_keypad.h
> +++ b/include/linux/input/matrix_keypad.h
> @@ -81,4 +81,15 @@ int matrix_keypad_build_keymap(const struct matrix_keymap_data *keymap_data,
> unsigned short *keymap,
> struct input_dev *input_dev);
>
> +/**
> + * matrix_keypad_parse_of_params() - Read parameters from matrix-keypad node
> + *
> + * @dev: Device containing of_node
> + * @rows: Returns number of matrix rows
> + * @cols: Returns number of matrix columns
> + * @return 0 if OK,<0 on error
> + */
> +int matrix_keypad_parse_of_params(struct device *dev,
> + unsigned int *rows, unsigned int *cols);
> +
> #endif /* _MATRIX_KEYPAD_H */
Looks good to me.You can add
Reviewed-by: Sourav Poddar <sourav.poddar@ti.com>
Also tested this patch on omap4430 sdp, keypad works fine. You can also
add a
Tested-by: Sourav Poddar <sourav.poddar@ti.com>
~Sourav
next prev parent reply other threads:[~2013-02-13 5:33 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-13 2:42 [PATCH v2 0/6] Add ChromeOS Embedded Controller support Simon Glass
2013-02-13 2:42 ` [PATCH v2 1/6] mfd: Add ChromeOS EC messages header Simon Glass
2013-02-13 2:42 ` [PATCH v2 2/6] mfd: Add ChromeOS EC implementation Simon Glass
2013-02-13 3:35 ` Joe Perches
2013-02-16 3:58 ` Simon Glass
2013-02-13 2:42 ` [PATCH v2 3/6] mfd: Add ChromeOS EC I2C driver Simon Glass
2013-02-13 2:42 ` [PATCH v2 4/6] mfd: Add ChromeOS EC SPI driver Simon Glass
2013-02-13 2:42 ` [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding Simon Glass
2013-02-13 5:32 ` a0131647 [this message]
2013-02-13 19:43 ` Dmitry Torokhov
2013-02-14 5:40 ` Simon Glass
2013-02-13 2:42 ` [PATCH v2 6/6] Input: Add ChromeOS EC keyboard driver Simon Glass
2013-02-13 20:02 ` Dmitry Torokhov
2013-02-14 6:45 ` Simon Glass
2013-02-14 17:31 ` Dmitry Torokhov
2013-02-16 3:56 ` Simon Glass
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=511B2581.2050706@ti.com \
--to=sourav.poddar@ti.com \
--cc=alban.bedel@avionic-design.de \
--cc=balbi@ti.com \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=dmitry.torokhov@gmail.com \
--cc=javier@dowhile0.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sameo@linux.intel.com \
--cc=sjg@chromium.org \
--cc=wfp5p@virginia.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).