From: Tony Lindgren <tony@atomide.com>
To: Sebastian Reichel <sre@debian.org>
Cc: "Sebastian Reichel" <sre@ring0.de>,
linux-input@vger.kernel.org,
"'Benoît Cousson'" <bcousson@baylibre.com>,
"Rob Herring" <rob.herring@calxeda.com>,
"Pawel Moll" <pawel.moll@arm.com>,
"Mark Rutland" <mark.rutland@arm.com>,
"Stephen Warren" <swarren@wwwdotorg.org>,
"Ian Campbell" <ijc+devicetree@hellion.org.uk>,
"Rob Landley" <rob@landley.net>,
"Russell King" <linux@arm.linux.org.uk>,
"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
"Grant Likely" <grant.likely@linaro.org>,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org
Subject: Re: [PATCH 1/2] Input: twl4030_keypad - add device tree support
Date: Fri, 11 Oct 2013 16:38:33 -0700 [thread overview]
Message-ID: <20131011233833.GE29913@atomide.com> (raw)
In-Reply-To: <1381353447-32708-1-git-send-email-sre@debian.org>
* Sebastian Reichel <sre@debian.org> [131009 14:25]:
> Add device tree support for twl4030 keypad driver and update the
> Documentation with twl4030 keypad device tree binding information.
>
> This patch also adds a twl4030 keypad node to the twl4030.dtsi file,
> so that board files can just add the keymap.
>
> Tested on Nokia N900.
Nice :) Just few cosmetic comments below.
>
> +#ifdef CONFIG_OF
> +static int twl4030_keypad_parse_dt(struct device *dev,
> + struct twl4030_keypad *keypad_data)
> +{
I guess the way to go nowadays is to use #IS_ENABLED(CONFIG_OF) here
and later on in this patch.
> @@ -331,20 +358,12 @@ static int twl4030_kp_program(struct twl4030_keypad *kp)
> static int twl4030_kp_probe(struct platform_device *pdev)
> {
> struct twl4030_keypad_data *pdata = pdev->dev.platform_data;
> - const struct matrix_keymap_data *keymap_data;
> + const struct matrix_keymap_data *keymap_data = NULL;
> struct twl4030_keypad *kp;
> struct input_dev *input;
> u8 reg;
> int error;
>
> - if (!pdata || !pdata->rows || !pdata->cols || !pdata->keymap_data ||
> - pdata->rows > TWL4030_MAX_ROWS || pdata->cols > TWL4030_MAX_COLS) {
> - dev_err(&pdev->dev, "Invalid platform_data\n");
> - return -EINVAL;
> - }
> -
> - keymap_data = pdata->keymap_data;
> -
> kp = kzalloc(sizeof(*kp), GFP_KERNEL);
> input = input_allocate_device();
> if (!kp || !input) {
I assume you have tested the above so it does not break things
for legacy booting?
Other than that:
Acked-by: Tony Lindgren <tony@atomide.com>
WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] Input: twl4030_keypad - add device tree support
Date: Fri, 11 Oct 2013 16:38:33 -0700 [thread overview]
Message-ID: <20131011233833.GE29913@atomide.com> (raw)
In-Reply-To: <1381353447-32708-1-git-send-email-sre@debian.org>
* Sebastian Reichel <sre@debian.org> [131009 14:25]:
> Add device tree support for twl4030 keypad driver and update the
> Documentation with twl4030 keypad device tree binding information.
>
> This patch also adds a twl4030 keypad node to the twl4030.dtsi file,
> so that board files can just add the keymap.
>
> Tested on Nokia N900.
Nice :) Just few cosmetic comments below.
>
> +#ifdef CONFIG_OF
> +static int twl4030_keypad_parse_dt(struct device *dev,
> + struct twl4030_keypad *keypad_data)
> +{
I guess the way to go nowadays is to use #IS_ENABLED(CONFIG_OF) here
and later on in this patch.
> @@ -331,20 +358,12 @@ static int twl4030_kp_program(struct twl4030_keypad *kp)
> static int twl4030_kp_probe(struct platform_device *pdev)
> {
> struct twl4030_keypad_data *pdata = pdev->dev.platform_data;
> - const struct matrix_keymap_data *keymap_data;
> + const struct matrix_keymap_data *keymap_data = NULL;
> struct twl4030_keypad *kp;
> struct input_dev *input;
> u8 reg;
> int error;
>
> - if (!pdata || !pdata->rows || !pdata->cols || !pdata->keymap_data ||
> - pdata->rows > TWL4030_MAX_ROWS || pdata->cols > TWL4030_MAX_COLS) {
> - dev_err(&pdev->dev, "Invalid platform_data\n");
> - return -EINVAL;
> - }
> -
> - keymap_data = pdata->keymap_data;
> -
> kp = kzalloc(sizeof(*kp), GFP_KERNEL);
> input = input_allocate_device();
> if (!kp || !input) {
I assume you have tested the above so it does not break things
for legacy booting?
Other than that:
Acked-by: Tony Lindgren <tony@atomide.com>
next prev parent reply other threads:[~2013-10-11 23:38 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-09 21:17 [PATCH 1/2] Input: twl4030_keypad - add device tree support Sebastian Reichel
2013-10-09 21:17 ` Sebastian Reichel
2013-10-09 21:17 ` [PATCH 2/2] ARM: dts: N900: TWL4030 Keypad Matrix definition Sebastian Reichel
2013-10-09 21:17 ` Sebastian Reichel
2013-10-11 23:39 ` Tony Lindgren
2013-10-11 23:39 ` Tony Lindgren
2013-10-11 23:38 ` Tony Lindgren [this message]
2013-10-11 23:38 ` [PATCH 1/2] Input: twl4030_keypad - add device tree support Tony Lindgren
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=20131011233833.GE29913@atomide.com \
--to=tony@atomide.com \
--cc=bcousson@baylibre.com \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=grant.likely@linaro.org \
--cc=ijc+devicetree@hellion.org.uk \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=mark.rutland@arm.com \
--cc=pawel.moll@arm.com \
--cc=rob.herring@calxeda.com \
--cc=rob@landley.net \
--cc=sre@debian.org \
--cc=sre@ring0.de \
--cc=swarren@wwwdotorg.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.