| Message ID | 54cf5d8c.sl7aclyeuuh66jXt%tsahee@annapurnalabs.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 02, 2015 at 11:20:44AM +0000, Tsahee Zidenberg wrote: > This patch introduces devicetree for the Alpine platform, and > for a development board based on the same platform. > > Signed-off-by: Barak Wasserstrom <barak@annapurnalabs.com> > Signed-off-by: Tsahee Zidenberg <tsahee@annapurnalabs.com> > --- > arch/arm/boot/dts/Makefile | 4 ++ > arch/arm/boot/dts/alpine-db.dts | 35 ++++++++++ > arch/arm/boot/dts/alpine.dtsi | 141 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 180 insertions(+) > create mode 100644 arch/arm/boot/dts/alpine-db.dts > create mode 100644 arch/arm/boot/dts/alpine.dtsi [...] > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + compatible = "simple-bus"; > + interrupt-parent = <&gic>; > + ranges; > + > + arch-timer { > + compatible = "arm,cortex-a15-timer", > + "arm,armv7-timer"; > + interrupts = > + <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > + clock-frequency = <0>; /* Filled by loader */ Your loader doesn't configure CNTFRQ? > + }; > + > + /* Interrupt Controller */ > + gic: gic@fb001000 { > + compatible = "arm,cortex-a15-gic"; > + #interrupt-cells = <3>; > + #size-cells = <0>; > + #address-cells = <0>; > + interrupt-controller; > + reg = <0x0 0xfb001000 0x0 0x1000>, > + <0x0 0xfb002000 0x0 0x2000>, > + <0x0 0xfb004000 0x0 0x1000>, > + <0x0 0xfb006000 0x0 0x2000>; > + interrupts = > + <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>; > + }; > + > + /* CPU Resume registers */ > + cpu-resume@fbff5ec0 { > + compatible = "al,alpine-cpu-resume"; > + reg = <0x0 0xfbff5ec0 0x0 0x30>; > + }; > + > + /* North Bridge Service Registers */ > + sysfabric-service@fb070000 { > + compatible = "al,alpine-sysfabric-service", "syscon", "simple-bus"; > + reg = <0x0 0xfb070000 0x0 0x10000>; > + }; That compatible list makes no sense whatsoever. Why is "simple-bus" on the end? Mark.
Thank you for your review! On 2 February 2015 at 15:40, Mark Rutland <mark.rutland@arm.com> wrote: > On Mon, Feb 02, 2015 at 11:20:44AM +0000, Tsahee Zidenberg wrote: >> + arch-timer { >> + compatible = "arm,cortex-a15-timer", >> + "arm,armv7-timer"; >> + interrupts = >> + <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, >> + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; >> + clock-frequency = <0>; /* Filled by loader */ > > Your loader doesn't configure CNTFRQ? > Not currently. setting CNTFRQ must be done in the firmware, to be valid across all CPUs and through power-cycles. This isn't supported on current firmware, which is used in currently available devices. I will add this as a required feature from next-gen firmware. clock-frequency property is read by linux-kernel before attempting to read CNTFRQ. >> + /* North Bridge Service Registers */ >> + sysfabric-service@fb070000 { >> + compatible = "al,alpine-sysfabric-service", "syscon", "simple-bus"; >> + reg = <0x0 0xfb070000 0x0 0x10000>; >> + }; > > That compatible list makes no sense whatsoever. > > Why is "simple-bus" on the end? > Nodes that are used with "syscon_regmap_lookup_by_compatible" appear both with and without compatibility to "simple-bus" in the device-trees. examples with: "fsl,imx6q-anatop", "xlnx,zynq-slcr" examples without: "fsl,imx6q-iomuxc-gpr", "rockchip,rk3066-pmu" Both ways work, I'm not if there is reasoning behind this difference in current device-trees or which is the better example to follow. I have no problem working either way. Is there a consensus on this? > Mark.
On Mon, Feb 02, 2015 at 03:27:48PM +0000, Tsahee Zidenberg wrote: > Thank you for your review! > > On 2 February 2015 at 15:40, Mark Rutland <mark.rutland@arm.com> wrote: > > On Mon, Feb 02, 2015 at 11:20:44AM +0000, Tsahee Zidenberg wrote: > >> + arch-timer { > >> + compatible = "arm,cortex-a15-timer", > >> + "arm,armv7-timer"; > >> + interrupts = > >> + <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > >> + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > >> + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, > >> + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; > >> + clock-frequency = <0>; /* Filled by loader */ > > > > Your loader doesn't configure CNTFRQ? > > > > Not currently. setting CNTFRQ must be done in the firmware, to be > valid across all CPUs and through power-cycles. This isn't supported > on current firmware, which is used in currently available devices. I > will add this as a required feature from next-gen firmware. > clock-frequency property is read by linux-kernel before attempting to > read CNTFRQ. Ok. The "clock-frequency" property is unfortunately a poor workaround for CNTFRQ not being set, due to CNTFRQ being exposed to guests in the presence of virtualisation (and potentially userspace were we to have a VDSO). > > > >> + /* North Bridge Service Registers */ > >> + sysfabric-service@fb070000 { > >> + compatible = "al,alpine-sysfabric-service", "syscon", "simple-bus"; > >> + reg = <0x0 0xfb070000 0x0 0x10000>; > >> + }; > > > > That compatible list makes no sense whatsoever. > > > > Why is "simple-bus" on the end? > > > > Nodes that are used with "syscon_regmap_lookup_by_compatible" appear > both with and without compatibility to "simple-bus" in the > device-trees. > examples with: "fsl,imx6q-anatop", "xlnx,zynq-slcr" > examples without: "fsl,imx6q-iomuxc-gpr", "rockchip,rk3066-pmu" > Both ways work, I'm not if there is reasoning behind this difference > in current device-trees or which is the better example to follow. I > have no problem working either way. Is there a consensus on this? The "simple-bus" entry is wrong, and should be removed. In other cases it's also likely to be wrong, but I'd have to inspect them to be sure. Mark.
On 2 February 2015 at 17:41, Mark Rutland <mark.rutland@arm.com> wrote: >> > Why is "simple-bus" on the end? >> > >> >> Nodes that are used with "syscon_regmap_lookup_by_compatible" appear >> both with and without compatibility to "simple-bus" in the >> device-trees. >> examples with: "fsl,imx6q-anatop", "xlnx,zynq-slcr" >> examples without: "fsl,imx6q-iomuxc-gpr", "rockchip,rk3066-pmu" >> Both ways work, I'm not if there is reasoning behind this difference >> in current device-trees or which is the better example to follow. I >> have no problem working either way. Is there a consensus on this? > > The "simple-bus" entry is wrong, and should be removed. > > In other cases it's also likely to be wrong, but I'd have to inspect > them to be sure. > Will be fixed, then. Thank you! Tsahee.
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index a1c776b..024d107 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -2,6 +2,10 @@ ifeq ($(CONFIG_OF),y) dtb-$(CONFIG_MACH_ASM9260) += \ alphascale-asm9260-devkit.dtb + +dtb-$(CONFIG_ARCH_ALPINE) += \ + alpine_db.dtb + # Keep at91 dtb files sorted alphabetically for each SoC dtb-$(CONFIG_SOC_SAM_V4_V5) += \ at91rm9200ek.dtb \ diff --git a/arch/arm/boot/dts/alpine-db.dts b/arch/arm/boot/dts/alpine-db.dts new file mode 100644 index 0000000..dfb5a08 --- /dev/null +++ b/arch/arm/boot/dts/alpine-db.dts @@ -0,0 +1,35 @@ +/* + * Copyright 2015 Annapurna Labs Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * Alternatively, redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + */ + +/dts-v1/; + +#include "alpine.dtsi" + +/ { + model = "Annapurna Labs Alpine Dev Board"; + /* no need for anything outside SOC */ +}; + diff --git a/arch/arm/boot/dts/alpine.dtsi b/arch/arm/boot/dts/alpine.dtsi new file mode 100644 index 0000000..3cce55a --- /dev/null +++ b/arch/arm/boot/dts/alpine.dtsi @@ -0,0 +1,141 @@ +/* + * Copyright 2015 Annapurna Labs Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * Alternatively, redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + */ + +#include <dt-bindings/interrupt-controller/arm-gic.h> +#include "skeleton64.dtsi" + +/ { + /* SOC compatibility */ + compatible = "al,alpine"; + + /* CPU Configuration */ + cpus { + #address-cells = <1>; + #size-cells = <0>; + enable-method = "al,alpine-smp"; + + cpu@0 { + compatible = "arm,cortex-a15"; + device_type = "cpu"; + reg = <0>; + clock-frequency = <0>; /* Filled by loader */ + }; + + cpu@1 { + compatible = "arm,cortex-a15"; + device_type = "cpu"; + reg = <1>; + clock-frequency = <0>; /* Filled by loader */ + }; + + cpu@2 { + compatible = "arm,cortex-a15"; + device_type = "cpu"; + reg = <2>; + clock-frequency = <0>; /* Filled by loader */ + }; + + cpu@3 { + compatible = "arm,cortex-a15"; + device_type = "cpu"; + reg = <3>; + clock-frequency = <0>; /* Filled by loader */ + }; + }; + + soc { + #address-cells = <2>; + #size-cells = <2>; + compatible = "simple-bus"; + interrupt-parent = <&gic>; + ranges; + + arch-timer { + compatible = "arm,cortex-a15-timer", + "arm,armv7-timer"; + interrupts = + <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>, + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>; + clock-frequency = <0>; /* Filled by loader */ + }; + + /* Interrupt Controller */ + gic: gic@fb001000 { + compatible = "arm,cortex-a15-gic"; + #interrupt-cells = <3>; + #size-cells = <0>; + #address-cells = <0>; + interrupt-controller; + reg = <0x0 0xfb001000 0x0 0x1000>, + <0x0 0xfb002000 0x0 0x2000>, + <0x0 0xfb004000 0x0 0x1000>, + <0x0 0xfb006000 0x0 0x2000>; + interrupts = + <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>; + }; + + /* CPU Resume registers */ + cpu-resume@fbff5ec0 { + compatible = "al,alpine-cpu-resume"; + reg = <0x0 0xfbff5ec0 0x0 0x30>; + }; + + /* North Bridge Service Registers */ + sysfabric-service@fb070000 { + compatible = "al,alpine-sysfabric-service", "syscon", "simple-bus"; + reg = <0x0 0xfb070000 0x0 0x10000>; + }; + + /* Performance Monitor Unit */ + pmu { + compatible = "arm,cortex-a15-pmu"; + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>; + }; + + uart0:uart@fd883000 { + compatible = "ns16550a"; + reg = <0x0 0xfd883000 0x0 0x1000>; + clock-frequency = <0>; /* Filled by loader */ + interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>; + reg-shift = <2>; + reg-io-width = <4>; + }; + + uart1:uart@0xfd884000 { + compatible = "ns16550a"; + reg = <0x0 0xfd884000 0x0 0x1000>; + clock-frequency = <0>; /* Filled by loader */ + interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>; + reg-shift = <2>; + reg-io-width = <4>; + }; + }; +};