| Message ID | 1362035966-17628-3-git-send-email-haojian.zhuang@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
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
On 28 February 2013 19:04, Arnd Bergmann <arnd@arndb.de> wrote: > 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. > OK >> +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. > OK >> +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(). > Sounds good. >> +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 Could you give me the link of your patch? I could rebase this patch. Regards Haojian
On Thursday 28 February 2013, Haojian Zhuang wrote: > > > > 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 > > Could you give me the link of your patch? I could rebase this patch. I only posted an RFC so far, at https://patchwork.kernel.org/patch/2074871/ It's probably easier for now if you ignore it. We will see how to stage things after the merge window, once we are putting both your patch and mine into arm-soc. Arnd
2013/2/27 Haojian Zhuang <haojian.zhuang@linaro.org>: > Add board support with device tree for Hisilicon Hi36xx/Hi37xx platform. > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> > --- > 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 +++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 114 insertions(+) > create mode 100644 arch/arm/mach-hs/Kconfig > create mode 100644 arch/arm/mach-hs/Makefile > create mode 100644 arch/arm/mach-hs/hs-dt.c > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index dedf02b..dfe70aa 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1055,6 +1055,8 @@ source "arch/arm/mach-h720x/Kconfig" > > source "arch/arm/mach-highbank/Kconfig" > > +source "arch/arm/mach-hs/Kconfig" > + > source "arch/arm/mach-integrator/Kconfig" > > source "arch/arm/mach-iop32x/Kconfig" > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > index ee4605f..08a913b 100644 > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -149,6 +149,7 @@ machine-$(CONFIG_ARCH_EP93XX) += ep93xx > machine-$(CONFIG_ARCH_GEMINI) += gemini > machine-$(CONFIG_ARCH_H720X) += h720x > machine-$(CONFIG_ARCH_HIGHBANK) += highbank > +machine-$(CONFIG_ARCH_HS) += hs > machine-$(CONFIG_ARCH_INTEGRATOR) += integrator > machine-$(CONFIG_ARCH_IOP13XX) += iop13xx > machine-$(CONFIG_ARCH_IOP32X) += iop32x > diff --git a/arch/arm/mach-hs/Kconfig b/arch/arm/mach-hs/Kconfig > new file mode 100644 > index 0000000..491c6da > --- /dev/null > +++ b/arch/arm/mach-hs/Kconfig > @@ -0,0 +1,23 @@ > +config ARCH_HS > + bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7 > + select ARM_AMBA > + select ARM_GIC > + select CACHE_L2X0 > + select CACHE_PL310 > + select PINCTRL > + select PINCTRL_SINGLE > + select SERIAL_AMBA_PL011 > + select SERIAL_AMBA_PL011_CONSOLE > + help > + Support for Hisilicon Hi36xx/Hi37xx processor family > + > +if ARCH_HS > + > +config MACH_HS_DT > + bool "Hisilicon Development Board" > + default y > + help > + Say 'Y' here if you want to support the Hisilicon Development > + Board. > + > +endif > diff --git a/arch/arm/mach-hs/Makefile b/arch/arm/mach-hs/Makefile > new file mode 100644 > index 0000000..d79ecb5 > --- /dev/null > +++ b/arch/arm/mach-hs/Makefile > @@ -0,0 +1,5 @@ > +# > +# Makefile for Hisilicon Hi36xx/Hi37xx processors line > +# > + > +obj-$(CONFIG_MACH_HS_DT) += hs-dt.o > diff --git a/arch/arm/mach-hs/hs-dt.c b/arch/arm/mach-hs/hs-dt.c > new file mode 100644 > index 0000000..813ebb0 > --- /dev/null > +++ b/arch/arm/mach-hs/hs-dt.c > @@ -0,0 +1,83 @@ > +/* > + * (Hisilicon's Hi36xx/Hi37xx SoC based) flattened device tree enabled machine > + * > + * Copyright (c) 2012-2013 Hisilicon Ltd. > + * Copyright (c) 2012-2013 Linaro Ltd. > + * > + * Author: Haojian Zhuang <haojian.zhuang@linaro.org> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > +*/ > + > +#include <linux/clk.h> > +#include <linux/clkdev.h> > +#include <linux/irqchip.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > +#include <linux/of_platform.h> > + > +#include <asm/hardware/arm_timer.h> > +#include <asm/hardware/timer-sp.h> > +#include <asm/mach/arch.h> > +#include <asm/mach/map.h> > +#include <asm/mach/time.h> > + > +static struct of_device_id hs_timer_match[] __initdata = { > + { .compatible = "arm,sp804", }, > + {} > +}; > + > +static struct clk_lookup sp804_lookup = { > + .dev_id = "sp804", > + .clk = NULL, > +}; > + > +extern void __init hs_init_clocks(void); this shouldn't be in this file. Run checkpatch.pl on it. Thanks, Michal
On Thursday 28 February 2013, Michal Simek wrote: > > + > > +extern void __init hs_init_clocks(void); > > this shouldn't be in this file. Run checkpatch.pl on it. Right, I believe that the new CLK_OF_DECLARE macro should take care of this. For now, you still need to call "of_clk_init(NULL);" from platform code when using this, but I suppose we will find a way to mvoe that into common arch code eventually. Arnd
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index dedf02b..dfe70aa 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1055,6 +1055,8 @@ source "arch/arm/mach-h720x/Kconfig" source "arch/arm/mach-highbank/Kconfig" +source "arch/arm/mach-hs/Kconfig" + source "arch/arm/mach-integrator/Kconfig" source "arch/arm/mach-iop32x/Kconfig" diff --git a/arch/arm/Makefile b/arch/arm/Makefile index ee4605f..08a913b 100644 --- a/arch/arm/Makefile +++ b/arch/arm/Makefile @@ -149,6 +149,7 @@ machine-$(CONFIG_ARCH_EP93XX) += ep93xx machine-$(CONFIG_ARCH_GEMINI) += gemini machine-$(CONFIG_ARCH_H720X) += h720x machine-$(CONFIG_ARCH_HIGHBANK) += highbank +machine-$(CONFIG_ARCH_HS) += hs machine-$(CONFIG_ARCH_INTEGRATOR) += integrator machine-$(CONFIG_ARCH_IOP13XX) += iop13xx machine-$(CONFIG_ARCH_IOP32X) += iop32x diff --git a/arch/arm/mach-hs/Kconfig b/arch/arm/mach-hs/Kconfig new file mode 100644 index 0000000..491c6da --- /dev/null +++ b/arch/arm/mach-hs/Kconfig @@ -0,0 +1,23 @@ +config ARCH_HS + bool "Hisilicon Hi36xx/Hi37xx family" if ARCH_MULTI_V7 + select ARM_AMBA + select ARM_GIC + select CACHE_L2X0 + select CACHE_PL310 + select PINCTRL + select PINCTRL_SINGLE + select SERIAL_AMBA_PL011 + select SERIAL_AMBA_PL011_CONSOLE + help + Support for Hisilicon Hi36xx/Hi37xx processor family + +if ARCH_HS + +config MACH_HS_DT + bool "Hisilicon Development Board" + default y + help + Say 'Y' here if you want to support the Hisilicon Development + Board. + +endif diff --git a/arch/arm/mach-hs/Makefile b/arch/arm/mach-hs/Makefile new file mode 100644 index 0000000..d79ecb5 --- /dev/null +++ b/arch/arm/mach-hs/Makefile @@ -0,0 +1,5 @@ +# +# Makefile for Hisilicon Hi36xx/Hi37xx processors line +# + +obj-$(CONFIG_MACH_HS_DT) += hs-dt.o diff --git a/arch/arm/mach-hs/hs-dt.c b/arch/arm/mach-hs/hs-dt.c new file mode 100644 index 0000000..813ebb0 --- /dev/null +++ b/arch/arm/mach-hs/hs-dt.c @@ -0,0 +1,83 @@ +/* + * (Hisilicon's Hi36xx/Hi37xx SoC based) flattened device tree enabled machine + * + * Copyright (c) 2012-2013 Hisilicon Ltd. + * Copyright (c) 2012-2013 Linaro Ltd. + * + * Author: Haojian Zhuang <haojian.zhuang@linaro.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ + +#include <linux/clk.h> +#include <linux/clkdev.h> +#include <linux/irqchip.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/of_platform.h> + +#include <asm/hardware/arm_timer.h> +#include <asm/hardware/timer-sp.h> +#include <asm/mach/arch.h> +#include <asm/mach/map.h> +#include <asm/mach/time.h> + +static struct of_device_id hs_timer_match[] __initdata = { + { .compatible = "arm,sp804", }, + {} +}; + +static struct clk_lookup sp804_lookup = { + .dev_id = "sp804", + .clk = NULL, +}; + +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"); +} + +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
Add board support with device tree for Hisilicon Hi36xx/Hi37xx platform. Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> --- 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 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 114 insertions(+) create mode 100644 arch/arm/mach-hs/Kconfig create mode 100644 arch/arm/mach-hs/Makefile create mode 100644 arch/arm/mach-hs/hs-dt.c