From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCHv2 arm: initial TI-Nspire support]
Date: Thu, 11 Apr 2013 14:30:05 +0200 [thread overview]
Message-ID: <201304111430.05528.arnd@arndb.de> (raw)
In-Reply-To: <8E071E2D-4FF3-487E-A53B-B6A7C427E421@gmail.com>
On Thursday 11 April 2013, Daniel Tang wrote:
> This is another updated patch for Linux on TI-Nspire support.
>
> Apologies for previously posting updated patches as replies to the first thread. I'll send updated patches in new threads from now to avoid confusion.
>
> Changes between http://archive.arm.linux.org.uk/lurker/message/20130408.113343.585af217.en.html and v2:
> * Added new drivers to support the irqchip and timers on older models.
> * Added new device trees to support the other models.
>
> Signed-off-by: Daniel Tang <dt.tangr@gmail.com>
Nice!
> arch/arm/Kconfig | 2 +
> arch/arm/Kconfig.debug | 16 ++
> arch/arm/Makefile | 1 +
> arch/arm/boot/dts/Makefile | 3 +
> arch/arm/boot/dts/nspire-classic.dtsi | 68 ++++++
> arch/arm/boot/dts/nspire-clp.dts | 45 ++++
> arch/arm/boot/dts/nspire-cx.dts | 115 ++++++++++
> arch/arm/boot/dts/nspire-tp.dts | 44 ++++
> arch/arm/boot/dts/nspire.dtsi | 159 ++++++++++++++
> arch/arm/include/debug/nspire.S | 28 +++
> arch/arm/mach-nspire/Kconfig | 15 ++
> arch/arm/mach-nspire/Makefile | 2 +
> arch/arm/mach-nspire/Makefile.boot | 0
> arch/arm/mach-nspire/clcd.c | 118 +++++++++++
> arch/arm/mach-nspire/clcd.h | 14 ++
> arch/arm/mach-nspire/mmio.h | 15 ++
> arch/arm/mach-nspire/nspire.c | 142 +++++++++++++
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/nspire-classic-timer.c | 216 +++++++++++++++++++
> drivers/input/keyboard/Kconfig | 10 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/nspire-keypad.c | 316 ++++++++++++++++++++++++++++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-nspire-classic.c | 177 ++++++++++++++++
> include/clocksource/nspire_classic_timer.h | 16 ++
> include/linux/platform_data/nspire-keypad.h | 28 +++
> 26 files changed, 1553 insertions(+)
Please split this up into a series of patches, one for each subsystem.
I would also keep the dts files in a separate patch from the platform
code.
> +bool timer_init;
> +
> +void __init nspire_classic_timer_init(void)
> +{
> + struct device_node *node;
> +
> + if (timer_init)
> + return;
> +
> + for_each_compatible_node(node, NULL, DT_COMPAT) {
> + nspire_timer_add(node);
> + }
> +
> + timer_init = 1;
> +}
> +
> +CLOCKSOURCE_OF_DECLARE(nspire_classic_timer,
> + DT_COMPAT, nspire_classic_timer_init)
Why do you need the logic to prevent it from being initilized
twice? Can't you just remove the direct call to nspire_classic_timer_init
from platform code and rely on of_clk_init() to call it?
Note that the interface has changed in linux-next, you now
get called separately for each matching device, with the device_node
as the argument, so you no longer have to search the device tree,
and can essentially do
CLOCKSOURCE_OF_DECLARE(nspire_classic_timer, DT_COMPAT, nspire_timer_add);
Feel free to rebase your patch on top of the clksrc/cleanup branch
in arm-soc to get the new behavior.
> +#ifdef CONFIG_OF
> +static const struct of_device_id nspire_keypad_dt_match[] = {
> + { .compatible = "nspire-keypad" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, nspire_keypad_dt_match);
> +#endif
You should not need the #ifdef.
> +static struct platform_driver nspire_keypad_driver = {
> + .driver = {
> + .name = "nspire-keypad",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(nspire_keypad_dt_match),
> + },
> + .remove = nspire_keypad_remove,
> + .probe = nspire_keypad_probe
> +};
Please put a comma at the end of the last line in definitions like this.
> diff --git a/include/clocksource/nspire_classic_timer.h b/include/clocksource/nspire_classic_timer.h
> new file mode 100644
> index 0000000..39236f1
> --- /dev/null
> +++ b/include/clocksource/nspire_classic_timer.h
You should not need this file if you do the above.
> diff --git a/include/linux/platform_data/nspire-keypad.h b/include/linux/platform_data/nspire-keypad.h
> new file mode 100644
> index 0000000..03deb64
> --- /dev/null
> +++ b/include/linux/platform_data/nspire-keypad.h
And this file now also isn't needed any more, you can just merge the
fields of struct nspire_keypad_data into struct nspire_keypad.
Arnd
WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: Daniel Tang <dt.tangr@gmail.com>
Cc: "linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
linux@arm.linux.org.uk, Linus Walleij <linus.walleij@linaro.org>,
"fabian@ritter-vogt.de Vogt" <fabian@ritter-vogt.de>,
Lionel Debroux <lionel_debroux@yahoo.fr>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCHv2 arm: initial TI-Nspire support]
Date: Thu, 11 Apr 2013 14:30:05 +0200 [thread overview]
Message-ID: <201304111430.05528.arnd@arndb.de> (raw)
In-Reply-To: <8E071E2D-4FF3-487E-A53B-B6A7C427E421@gmail.com>
On Thursday 11 April 2013, Daniel Tang wrote:
> This is another updated patch for Linux on TI-Nspire support.
>
> Apologies for previously posting updated patches as replies to the first thread. I'll send updated patches in new threads from now to avoid confusion.
>
> Changes between http://archive.arm.linux.org.uk/lurker/message/20130408.113343.585af217.en.html and v2:
> * Added new drivers to support the irqchip and timers on older models.
> * Added new device trees to support the other models.
>
> Signed-off-by: Daniel Tang <dt.tangr@gmail.com>
Nice!
> arch/arm/Kconfig | 2 +
> arch/arm/Kconfig.debug | 16 ++
> arch/arm/Makefile | 1 +
> arch/arm/boot/dts/Makefile | 3 +
> arch/arm/boot/dts/nspire-classic.dtsi | 68 ++++++
> arch/arm/boot/dts/nspire-clp.dts | 45 ++++
> arch/arm/boot/dts/nspire-cx.dts | 115 ++++++++++
> arch/arm/boot/dts/nspire-tp.dts | 44 ++++
> arch/arm/boot/dts/nspire.dtsi | 159 ++++++++++++++
> arch/arm/include/debug/nspire.S | 28 +++
> arch/arm/mach-nspire/Kconfig | 15 ++
> arch/arm/mach-nspire/Makefile | 2 +
> arch/arm/mach-nspire/Makefile.boot | 0
> arch/arm/mach-nspire/clcd.c | 118 +++++++++++
> arch/arm/mach-nspire/clcd.h | 14 ++
> arch/arm/mach-nspire/mmio.h | 15 ++
> arch/arm/mach-nspire/nspire.c | 142 +++++++++++++
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/nspire-classic-timer.c | 216 +++++++++++++++++++
> drivers/input/keyboard/Kconfig | 10 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/nspire-keypad.c | 316 ++++++++++++++++++++++++++++
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-nspire-classic.c | 177 ++++++++++++++++
> include/clocksource/nspire_classic_timer.h | 16 ++
> include/linux/platform_data/nspire-keypad.h | 28 +++
> 26 files changed, 1553 insertions(+)
Please split this up into a series of patches, one for each subsystem.
I would also keep the dts files in a separate patch from the platform
code.
> +bool timer_init;
> +
> +void __init nspire_classic_timer_init(void)
> +{
> + struct device_node *node;
> +
> + if (timer_init)
> + return;
> +
> + for_each_compatible_node(node, NULL, DT_COMPAT) {
> + nspire_timer_add(node);
> + }
> +
> + timer_init = 1;
> +}
> +
> +CLOCKSOURCE_OF_DECLARE(nspire_classic_timer,
> + DT_COMPAT, nspire_classic_timer_init)
Why do you need the logic to prevent it from being initilized
twice? Can't you just remove the direct call to nspire_classic_timer_init
from platform code and rely on of_clk_init() to call it?
Note that the interface has changed in linux-next, you now
get called separately for each matching device, with the device_node
as the argument, so you no longer have to search the device tree,
and can essentially do
CLOCKSOURCE_OF_DECLARE(nspire_classic_timer, DT_COMPAT, nspire_timer_add);
Feel free to rebase your patch on top of the clksrc/cleanup branch
in arm-soc to get the new behavior.
> +#ifdef CONFIG_OF
> +static const struct of_device_id nspire_keypad_dt_match[] = {
> + { .compatible = "nspire-keypad" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, nspire_keypad_dt_match);
> +#endif
You should not need the #ifdef.
> +static struct platform_driver nspire_keypad_driver = {
> + .driver = {
> + .name = "nspire-keypad",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(nspire_keypad_dt_match),
> + },
> + .remove = nspire_keypad_remove,
> + .probe = nspire_keypad_probe
> +};
Please put a comma at the end of the last line in definitions like this.
> diff --git a/include/clocksource/nspire_classic_timer.h b/include/clocksource/nspire_classic_timer.h
> new file mode 100644
> index 0000000..39236f1
> --- /dev/null
> +++ b/include/clocksource/nspire_classic_timer.h
You should not need this file if you do the above.
> diff --git a/include/linux/platform_data/nspire-keypad.h b/include/linux/platform_data/nspire-keypad.h
> new file mode 100644
> index 0000000..03deb64
> --- /dev/null
> +++ b/include/linux/platform_data/nspire-keypad.h
And this file now also isn't needed any more, you can just merge the
fields of struct nspire_keypad_data into struct nspire_keypad.
Arnd
next prev parent reply other threads:[~2013-04-11 12:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-11 11:51 [RFC PATCHv2 arm: initial TI-Nspire support] Daniel Tang
2013-04-11 11:51 ` Daniel Tang
2013-04-11 12:30 ` Arnd Bergmann [this message]
2013-04-11 12:30 ` Arnd Bergmann
2013-04-11 12:42 ` Daniel Tang
2013-04-11 12:42 ` Daniel Tang
2013-04-11 13:06 ` Arnd Bergmann
2013-04-11 13:06 ` Arnd Bergmann
2013-04-19 12:16 ` Russell King - ARM Linux
2013-04-19 12:16 ` Russell King - ARM Linux
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=201304111430.05528.arnd@arndb.de \
--to=arnd@arndb.de \
--cc=linux-arm-kernel@lists.infradead.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.