linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/7] ARM: hs: add board support with device tree
Date: Thu, 28 Feb 2013 11:04:13 +0000	[thread overview]
Message-ID: <201302281104.13885.arnd@arndb.de> (raw)
In-Reply-To: <1362035966-17628-3-git-send-email-haojian.zhuang@linaro.org>

On Thursday 28 February 2013, Haojian Zhuang wrote:
> Add board support with device tree for Hisilicon Hi36xx/Hi37xx platform.
> 
> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>

Ah, nice and small ;-)

>  arch/arm/Kconfig          |    2 ++
>  arch/arm/Makefile         |    1 +
>  arch/arm/mach-hs/Kconfig  |   23 +++++++++++++
>  arch/arm/mach-hs/Makefile |    5 +++
>  arch/arm/mach-hs/hs-dt.c  |   83 +++++++++++++++++++++++++++++++++++++++++++++

Regarding the naming, I wonder if "hs" is unique enough for a platform name.
AFAIK, HiSilicon has a couple of independently developed SoC families,
so we might want to be a bit more specific here, e.g. mach-hi3xxx.

Regarding the file name, I think the "-dt" postfix is redundant, when we
only support booting using DT. I would call this one the same thing as the
platform name, whichever we go with.

> +config MACH_HS_DT
> +	bool "Hisilicon Development Board"
> +	default y
> +	help
> +	  Say 'Y' here if you want to support the Hisilicon Development
> +	  Board. 
> +
> +endif

I don't think we need this option. Let's just always build the file
when the platform is enabled, and build all .dtb files as well.

> +static struct clk_lookup sp804_lookup = {
> +	.dev_id	= "sp804",
> +	.clk	= NULL,
> +};

(adding Mike to Cc)

Shouldn't the clk_lookup be automatic with a fully DT enabled platform now?

> +extern void __init hs_init_clocks(void);
> +static void __init hs_timer_init(void)
> +{
> +	struct device_node *node = NULL;
> +	void __iomem *base;
> +	int irq;
> +
> +	hs_init_clocks();
> +
> +	node = of_find_matching_node(NULL, hs_timer_match);
> +	WARN_ON(!node);
> +	if (!node) {
> +		pr_err("Failed to find sp804 timer\n");
> +		return;
> +	}
> +	base = of_iomap(node, 0);
> +	WARN_ON(!base);
> +
> +	/* timer0 is used as clock event, and timer1 is clock source. */
> +	irq = irq_of_parse_and_map(node, 0);
> +	WARN_ON(!irq);
> +
> +	sp804_lookup.clk = of_clk_get(node, 0);
> +	clkdev_add(&sp804_lookup);
> +
> +	sp804_clocksource_and_sched_clock_init(base + TIMER_2_BASE, "timer1");
> +	sp804_clockevents_init(base, irq, "timer0");
> +}

I think for the clocksource/clockevents driver, we should move
arch/arm/common/timer-sp.c to drivers/clocksource now and integrate
it into the automatic probing through clocksource_of_init().

> +static void __init hs_init(void)
> +{
> +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +}
> +
> +static const char *hs_compat[] __initdata = {
> +	"hisilicon,hi3620-hi4511",
> +	NULL,
> +};
> +
> +DT_MACHINE_START(HS_DT, "Hisilicon Hi36xx/Hi37xx (Flattened Device Tree)")
> +	/* Maintainer: Haojian Zhuang <haojian.zhuang@linaro.org> */
> +	.map_io		= debug_ll_io_init,
> +	.init_irq	= irqchip_init,
> +	.init_time	= hs_timer_init,
> +	.init_machine	= hs_init,
> +	.dt_compat	= hs_compat,
> +MACHINE_END

This looks right at the moment, but I also have a patch to make it possible to drop the
irqchip_init, clocksource_of_init and hs_init() calls here, as they are all the
defaults. At that point, the platform would be essentially empty except for the
debug_ll_io_init part that is inherently platform specific. 

	Arnd

  reply	other threads:[~2013-02-28 11:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-28  7:19 [PATCH 1/7] ARM: debug: support debug ll on hisilicon soc Haojian Zhuang
2013-02-28  7:19 ` [PATCH 2/7] clk: hs: add clock support Haojian Zhuang
2013-02-28 12:48   ` Haojian Zhuang
2013-02-28  7:19 ` [PATCH 3/7] ARM: hs: add board support with device tree Haojian Zhuang
2013-02-28 11:04   ` Arnd Bergmann [this message]
2013-02-28 12:55     ` Haojian Zhuang
2013-02-28 14:17       ` Arnd Bergmann
2013-02-28 21:46   ` Michal Simek
2013-03-01 11:45     ` Arnd Bergmann
2013-02-28  7:19 ` [PATCH 4/7] ARM: hs: enable hi4511 " Haojian Zhuang
2013-02-28  7:19 ` [PATCH 5/7] document: append hisilicon clock binding Haojian Zhuang
2013-02-28  7:19 ` [PATCH 6/7] Document: dts: create hisilicon document Haojian Zhuang
2013-02-28  7:19 ` [PATCH 7/7] ARM: config: append arch hs into multi defconfig Haojian Zhuang

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=201302281104.13885.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 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).