From: Bo Shen <voice.shen@atmel.com>
To: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: devicetree-discuss <devicetree-discuss@lists.ozlabs.org>,
dmitry.torokhov@gmail.com, shubhrajyoti@ti.com,
linux-input@vger.kernel.org, jm.lin@atmel.com,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3] input: qt1070: Add device tree support
Date: Tue, 18 Sep 2012 10:21:03 +0800 [thread overview]
Message-ID: <5057DA8F.70407@atmel.com> (raw)
In-Reply-To: <5056EB60.8080100@atmel.com>
Hi Nicolas,
On 9/17/2012 17:20, Nicolas Ferre wrote:
> On 09/11/2012 12:14 PM, Bo Shen :
>> Add qt1070 device tree support
>> Using device node to parse irq
>>
>> Test this on at91sam9x5ek board
>>
>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>> ---
>> change since v2
>> Using device node to parse irq,
>> Remove gpio relative stuff
>>
>> change since v1
>> Add gpio relative stuff, gpio_is_valid, gpio_request
>> ---
>> .../devicetree/bindings/input/atmel-qt1070.txt | 16 ++++++++++++++++
>> drivers/input/keyboard/qt1070.c | 9 +++++++++
>> 2 files changed, 25 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/atmel-qt1070.txt
>>
>> diff --git a/Documentation/devicetree/bindings/input/atmel-qt1070.txt b/Documentation/devicetree/bindings/input/atmel-qt1070.txt
>> new file mode 100644
>> index 0000000..e1fc09e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/atmel-qt1070.txt
>> @@ -0,0 +1,16 @@
>> +Atmel qt1070 keyboard interface
>
> Nitpicking: there is usually a "*" at the beginning to give it a
> title-ish look ;-)
>
> Please tell that
OK, I will add it. Thanks.
>
>> +
>> +Required Properties:
>> +- compatible: Should be "atmel,qt1070"
>> +- reg: I2C slave address
>> +- interrupt-parent: Depends on SoC
>> +- interrupts: The interrupt pin number to the cpu, and trigger mode.
>> +
>> +Example:
>> +
>> + qt1070@1b {
>
> I think that you need to replace by a generic name for the node name:
>
> qt1070: keyboard@1b {
> compatible = "atmel,qt1070";
> [..]
> };
>
> I found "keyboard" to be the most appropriate in the ePAPR.
OK. I will modify it like this. Thanks.
>
>> + compatible = "atmel,qt1070";
>> + reg = <0x1b>;
>> + interrupt-parent = <&pioA>;
>> + interrupts = <7 0x0>;
>> + };
>> diff --git a/drivers/input/keyboard/qt1070.c b/drivers/input/keyboard/qt1070.c
>> index ca68f29..60457dc 100644
>> --- a/drivers/input/keyboard/qt1070.c
>> +++ b/drivers/input/keyboard/qt1070.c
>> @@ -249,10 +249,19 @@ static const struct i2c_device_id qt1070_id[] = {
>> };
>> MODULE_DEVICE_TABLE(i2c, qt1070_id);
>>
>> +#ifdef CONFIG_OF
>> +static struct of_device_id atmel_qt1070_of_match[] = {
>> + { .compatible = "atmel,qt1070" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, atmel_qt1070_of_match);
>> +#endif
>> +
>> static struct i2c_driver qt1070_driver = {
>> .driver = {
>> .name = "qt1070",
>> .owner = THIS_MODULE,
>> + .of_match_table = of_match_ptr(atmel_qt1070_of_match),
>> },
>> .id_table = qt1070_id,
>> .probe = qt1070_probe,
>>
>
> Otherwise, looks good. So, if you agree on my comments and for next
> revision, you can add my:
>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>
> Bye,
>
WARNING: multiple messages have this Message-ID (diff)
From: voice.shen@atmel.com (Bo Shen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3] input: qt1070: Add device tree support
Date: Tue, 18 Sep 2012 10:21:03 +0800 [thread overview]
Message-ID: <5057DA8F.70407@atmel.com> (raw)
In-Reply-To: <5056EB60.8080100@atmel.com>
Hi Nicolas,
On 9/17/2012 17:20, Nicolas Ferre wrote:
> On 09/11/2012 12:14 PM, Bo Shen :
>> Add qt1070 device tree support
>> Using device node to parse irq
>>
>> Test this on at91sam9x5ek board
>>
>> Signed-off-by: Bo Shen <voice.shen@atmel.com>
>> ---
>> change since v2
>> Using device node to parse irq,
>> Remove gpio relative stuff
>>
>> change since v1
>> Add gpio relative stuff, gpio_is_valid, gpio_request
>> ---
>> .../devicetree/bindings/input/atmel-qt1070.txt | 16 ++++++++++++++++
>> drivers/input/keyboard/qt1070.c | 9 +++++++++
>> 2 files changed, 25 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/atmel-qt1070.txt
>>
>> diff --git a/Documentation/devicetree/bindings/input/atmel-qt1070.txt b/Documentation/devicetree/bindings/input/atmel-qt1070.txt
>> new file mode 100644
>> index 0000000..e1fc09e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/atmel-qt1070.txt
>> @@ -0,0 +1,16 @@
>> +Atmel qt1070 keyboard interface
>
> Nitpicking: there is usually a "*" at the beginning to give it a
> title-ish look ;-)
>
> Please tell that
OK, I will add it. Thanks.
>
>> +
>> +Required Properties:
>> +- compatible: Should be "atmel,qt1070"
>> +- reg: I2C slave address
>> +- interrupt-parent: Depends on SoC
>> +- interrupts: The interrupt pin number to the cpu, and trigger mode.
>> +
>> +Example:
>> +
>> + qt1070 at 1b {
>
> I think that you need to replace by a generic name for the node name:
>
> qt1070: keyboard at 1b {
> compatible = "atmel,qt1070";
> [..]
> };
>
> I found "keyboard" to be the most appropriate in the ePAPR.
OK. I will modify it like this. Thanks.
>
>> + compatible = "atmel,qt1070";
>> + reg = <0x1b>;
>> + interrupt-parent = <&pioA>;
>> + interrupts = <7 0x0>;
>> + };
>> diff --git a/drivers/input/keyboard/qt1070.c b/drivers/input/keyboard/qt1070.c
>> index ca68f29..60457dc 100644
>> --- a/drivers/input/keyboard/qt1070.c
>> +++ b/drivers/input/keyboard/qt1070.c
>> @@ -249,10 +249,19 @@ static const struct i2c_device_id qt1070_id[] = {
>> };
>> MODULE_DEVICE_TABLE(i2c, qt1070_id);
>>
>> +#ifdef CONFIG_OF
>> +static struct of_device_id atmel_qt1070_of_match[] = {
>> + { .compatible = "atmel,qt1070" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, atmel_qt1070_of_match);
>> +#endif
>> +
>> static struct i2c_driver qt1070_driver = {
>> .driver = {
>> .name = "qt1070",
>> .owner = THIS_MODULE,
>> + .of_match_table = of_match_ptr(atmel_qt1070_of_match),
>> },
>> .id_table = qt1070_id,
>> .probe = qt1070_probe,
>>
>
> Otherwise, looks good. So, if you agree on my comments and for next
> revision, you can add my:
>
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>
> Bye,
>
next prev parent reply other threads:[~2012-09-18 2:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-11 10:14 [PATCH v3] input: qt1070: Add device tree support Bo Shen
2012-09-11 10:14 ` Bo Shen
2012-09-17 3:34 ` Bo Shen
2012-09-17 3:34 ` Bo Shen
[not found] ` <1347358459-11598-1-git-send-email-voice.shen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
2012-09-17 9:20 ` Nicolas Ferre
2012-09-17 9:20 ` Nicolas Ferre
2012-09-18 2:21 ` Bo Shen [this message]
2012-09-18 2:21 ` Bo Shen
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=5057DA8F.70407@atmel.com \
--to=voice.shen@atmel.com \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=dmitry.torokhov@gmail.com \
--cc=jm.lin@atmel.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-input@vger.kernel.org \
--cc=nicolas.ferre@atmel.com \
--cc=shubhrajyoti@ti.com \
/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.