All of lore.kernel.org
 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 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.