| Message ID | 1392100183-30930-2-git-send-email-kgene.kim@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/12/14 20:40, Marc Zyngier wrote: > On 12/02/14 11:29, Mark Rutland wrote: >>>>> + gic: interrupt-controller@1C000000 { >>>>> + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; >>>> >>>> This looks incorrect -- you should at the very least have a more >>>> specific one than a15-gic? Marc? >>> >>> "arm,cortex-a9-gic" is definitely wrong (the A9 GIC doesn't have the >>> virt extensions). This binding matches what the A15 GIC has, so >>> "arm,cortex-a15-gic" is probably fine. Main issue here is that the GICv2 >>> driver has no compatible string for anything else. >>> >>> Should we define something more generic (like "arm,gic-v2")? Or carry on >>> adding more compatible strings? >> >> It's been proposed repeatedly, and it probably makes sense to add the >> generic versions to the driver, and allow for more specific ones in the >> binding which DTs can use. That way we don't get an explosion of strings >> in the driver, but if we need to handle any particular GIC specially in >> future we can do so. >> >> I guess for Linux we'd want to add "arm,gic-v1" and "arm,gic-v2" to the >> driver. We could just add "arm,gic-v1" and expect it later in the >> compatible list if v2 is a strict superset of v1; I think it is but I'm >> not a GIC expert. > > Sounds good to me. > OK, so did you guys agree to use version for gic? I'm fine to use gic-v2. And so who will take changing for others in mainline? ;-) - Kukjin
On 02/12/14 03:15, Mark Rutland wrote: > On Tue, Feb 11, 2014 at 06:29:41AM +0000, Kukjin Kim wrote: >> Signed-off-by: Kukjin Kim<kgene.kim@samsung.com> >> Reviewed-by: Thomas Abraham<thomas.ab@samsung.com> >> Cc: Catalin Marinas<catalin.marinas@arm.com> >> --- >> arch/arm64/boot/dts/samsung-gh7.dtsi | 108 ++++++++++++++++++++++++++++++ >> arch/arm64/boot/dts/samsung-ssdk-gh7.dts | 26 +++++++ >> 2 files changed, 134 insertions(+) >> create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi >> create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts >> >> diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi >> new file mode 100644 >> index 0000000..5b8785c >> --- /dev/null >> +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi >> @@ -0,0 +1,108 @@ >> +/* >> + * SAMSUNG GH7 SoC device tree source >> + * >> + * Copyright (c) 2014 Samsung Electronics Co., Ltd. >> + * http://www.samsung.com >> + * >> + * 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. >> +*/ >> + >> +/dts-v1/; >> + >> +/memreserve/ 0x80000000 0x0C400000; > > That looks _very_ large. What is this for? > Yes, I know but we need to reserve that for EL3 monitor, UEFI services, secure, hypervisor and scan chanin... > [...] > >> + timer { >> + compatible = "arm,armv8-timer"; >> + interrupts =<1 13 0xff01>, /* Secure Phys IRQ */ >> + <1 14 0xff01>, /* Non-secure Phys IRQ */ >> + <1 11 0xff01>, /* Virt IRQ */ >> + <1 10 0xff01>; /* Hyp IRQ */ >> + clock-frequency =<100000000>; > > Please, get your bootloader to set CNTFREQ. The clock frequency property > for the timer node is a horrible hack for buggy firmware. > You're right. OK. > [...] > >> + amba { >> + compatible = "arm,amba-bus"; >> + #address-cells =<1>; >> + #size-cells =<1>; >> + ranges; >> + >> + serial@12c00000 { >> + compatible = "arm,pl011", "arm,primecell"; >> + reg =<0x12c00000 0x10000>; >> + interrupts =<418>; >> + }; >> + >> + serial@12c20000 { >> + compatible = "arm,pl011", "arm,primecell"; >> + reg =<0x12c20000 0x10000>; >> + interrupts =<420>; >> + }; > > Don't these need clocks? > We don't need and the clocks will be handled by bootloader... > [...] > >> + memory@80000000 { >> + device_type = "memory"; >> + reg =<0x00000000 0x80000000 0 0x80000000>; > > Minor nit, but it would be nice for the 0 values to be consistently padded. > OK. Thanks, Kukjin
On 02/12/14 08:36, Olof Johansson wrote: > Hi, > Hi Olof, > Besides what Mark Rutland already commented on: > OK, thanks :-) > On Mon, Feb 10, 2014 at 10:29 PM, Kukjin Kim<kgene.kim@samsung.com> wrote: >> +/ { >> + model = "SAMSUNG GH7"; >> + compatible = "samsung,gh7"; > > Model and compatible in the dtsi should probably always be overridden > by a dts that includes it, so there's little use in having it here. > OK, makes sense. >> + interrupt-parent =<&gic>; >> + #address-cells =<2>; >> + #size-cells =<2>; >> + >> + cpus { >> + #address-cells =<2>; >> + #size-cells =<0>; >> + >> + cpu@000 { >> + device_type = "cpu"; >> + compatible = "arm,armv8"; >> + reg =<0x0 0x000>; > > No need to zero-pad cpu numbers in unit address or reg. > Yes it's correct without any consideration, but I'm going to add more cpus next time and they should be separated from this. So I used. >> + enable-method = "spin-table"; >> + cpu-release-addr =<0x0 0x8000fff8>; >> + }; >> + cpu@001 { >> + device_type = "cpu"; >> + compatible = "arm,armv8"; >> + reg =<0x0 0x001>; >> + enable-method = "spin-table"; >> + cpu-release-addr =<0x0 0x8000fff8>; >> + }; >> + cpu@002 { >> + device_type = "cpu"; >> + compatible = "arm,armv8"; >> + reg =<0x0 0x002>; >> + enable-method = "spin-table"; >> + cpu-release-addr =<0x0 0x8000fff8>; >> + }; >> + cpu@003 { >> + device_type = "cpu"; >> + compatible = "arm,armv8"; >> + reg =<0x0 0x003>; >> + enable-method = "spin-table"; >> + cpu-release-addr =<0x0 0x8000fff8>; >> + }; >> + }; >> + >> + gic: interrupt-controller@1C000000 { >> + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; > > This looks incorrect -- you should at the very least have a more > specific one than a15-gic? Marc? > OK, you're right. And I replied on other e-mail. Thanks, Kukjin
On Tue, Feb 11, 2014 at 06:29:41AM +0000, Kukjin Kim wrote: > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com> > Reviewed-by: Thomas Abraham <thomas.ab@samsung.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > --- > arch/arm64/boot/dts/samsung-gh7.dtsi | 108 ++++++++++++++++++++++++++++++ > arch/arm64/boot/dts/samsung-ssdk-gh7.dts | 26 +++++++ > 2 files changed, 134 insertions(+) > create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi > create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts > > diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi > new file mode 100644 > index 0000000..5b8785c > --- /dev/null > +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi > @@ -0,0 +1,108 @@ > +/* > + * SAMSUNG GH7 SoC device tree source > + * > + * Copyright (c) 2014 Samsung Electronics Co., Ltd. > + * http://www.samsung.com > + * > + * 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. > +*/ > + > +/dts-v1/; > + > +/memreserve/ 0x80000000 0x0C400000; That looks _very_ large. What is this for? [...] > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <1 13 0xff01>, /* Secure Phys IRQ */ > + <1 14 0xff01>, /* Non-secure Phys IRQ */ > + <1 11 0xff01>, /* Virt IRQ */ > + <1 10 0xff01>; /* Hyp IRQ */ > + clock-frequency = <100000000>; Please, get your bootloader to set CNTFREQ. The clock frequency property for the timer node is a horrible hack for buggy firmware. [...] > + amba { > + compatible = "arm,amba-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + serial@12c00000 { > + compatible = "arm,pl011", "arm,primecell"; > + reg = <0x12c00000 0x10000>; > + interrupts = <418>; > + }; > + > + serial@12c20000 { > + compatible = "arm,pl011", "arm,primecell"; > + reg = <0x12c20000 0x10000>; > + interrupts = <420>; > + }; Don't these need clocks? [...] > + memory@80000000 { > + device_type = "memory"; > + reg = <0x00000000 0x80000000 0 0x80000000>; Minor nit, but it would be nice for the 0 values to be consistently padded. Thanks, Mark.
Hi, Besides what Mark Rutland already commented on: On Mon, Feb 10, 2014 at 10:29 PM, Kukjin Kim <kgene.kim@samsung.com> wrote: > +/ { > + model = "SAMSUNG GH7"; > + compatible = "samsung,gh7"; Model and compatible in the dtsi should probably always be overridden by a dts that includes it, so there's little use in having it here. > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + cpus { > + #address-cells = <2>; > + #size-cells = <0>; > + > + cpu@000 { > + device_type = "cpu"; > + compatible = "arm,armv8"; > + reg = <0x0 0x000>; No need to zero-pad cpu numbers in unit address or reg. > + enable-method = "spin-table"; > + cpu-release-addr = <0x0 0x8000fff8>; > + }; > + cpu@001 { > + device_type = "cpu"; > + compatible = "arm,armv8"; > + reg = <0x0 0x001>; > + enable-method = "spin-table"; > + cpu-release-addr = <0x0 0x8000fff8>; > + }; > + cpu@002 { > + device_type = "cpu"; > + compatible = "arm,armv8"; > + reg = <0x0 0x002>; > + enable-method = "spin-table"; > + cpu-release-addr = <0x0 0x8000fff8>; > + }; > + cpu@003 { > + device_type = "cpu"; > + compatible = "arm,armv8"; > + reg = <0x0 0x003>; > + enable-method = "spin-table"; > + cpu-release-addr = <0x0 0x8000fff8>; > + }; > + }; > + > + gic: interrupt-controller@1C000000 { > + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; This looks incorrect -- you should at the very least have a more specific one than a15-gic? Marc? > + #interrupt-cells = <3>; > + #address-cells = <0>; > + interrupt-controller; > + reg = <0x0 0x1C001000 0 0x1000>, /* GIC Dist */ > + <0x0 0x1C002000 0 0x1000>, /* GIC CPU */ > + <0x0 0x1C004000 0 0x2000>, /* GIC VCPU Control */ > + <0x0 0x1C006000 0 0x2000>; /* GIC VCPU */ > + interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <1 13 0xff01>, /* Secure Phys IRQ */ > + <1 14 0xff01>, /* Non-secure Phys IRQ */ > + <1 11 0xff01>, /* Virt IRQ */ > + <1 10 0xff01>; /* Hyp IRQ */ > + clock-frequency = <100000000>; > + }; > + > + pmu { > + compatible = "arm,armv8-pmuv3"; > + interrupts = <0 294 8>, > + <0 295 8>, > + <0 296 8>, > + <0 297 8>, > + <0 298 8>, > + <0 299 8>, > + <0 300 8>, > + <0 301 8>; > + }; > + > + amba { > + compatible = "arm,amba-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + > + serial@12c00000 { > + compatible = "arm,pl011", "arm,primecell"; > + reg = <0x12c00000 0x10000>; > + interrupts = <418>; > + }; > + > + serial@12c20000 { > + compatible = "arm,pl011", "arm,primecell"; > + reg = <0x12c20000 0x10000>; > + interrupts = <420>; > + }; > + }; > +}; > diff --git a/arch/arm64/boot/dts/samsung-ssdk-gh7.dts b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts > new file mode 100644 > index 0000000..47afbc4 > --- /dev/null > +++ b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts > @@ -0,0 +1,26 @@ > +/* > + * SAMSUNG SSDK-GH7 board device tree source > + * > + * Copyright (c) 2014 Samsung Electronics Co., Ltd. > + * http://www.samsung.com > + * > + * 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. > +*/ > + > +/dts-v1/; > +#include "samsung-gh7.dtsi" > + > +/ { > + model = "SAMSUNG SSDK-GH7 board based on GH7 SoC"; > + compatible = "samsung,ssdk-gh7", "samsung,gh7"; > + > + chosen { > + }; > + > + memory@80000000 { > + device_type = "memory"; > + reg = <0x00000000 0x80000000 0 0x80000000>; > + }; > +}; > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/02/14 23:36, Olof Johansson wrote: > Hi, > > Besides what Mark Rutland already commented on: > > On Mon, Feb 10, 2014 at 10:29 PM, Kukjin Kim <kgene.kim@samsung.com> wrote: >> +/ { >> + model = "SAMSUNG GH7"; >> + compatible = "samsung,gh7"; > > Model and compatible in the dtsi should probably always be overridden > by a dts that includes it, so there's little use in having it here. > >> + interrupt-parent = <&gic>; >> + #address-cells = <2>; >> + #size-cells = <2>; >> + >> + cpus { >> + #address-cells = <2>; >> + #size-cells = <0>; >> + >> + cpu@000 { >> + device_type = "cpu"; >> + compatible = "arm,armv8"; >> + reg = <0x0 0x000>; > > No need to zero-pad cpu numbers in unit address or reg. > >> + enable-method = "spin-table"; >> + cpu-release-addr = <0x0 0x8000fff8>; >> + }; >> + cpu@001 { >> + device_type = "cpu"; >> + compatible = "arm,armv8"; >> + reg = <0x0 0x001>; >> + enable-method = "spin-table"; >> + cpu-release-addr = <0x0 0x8000fff8>; >> + }; >> + cpu@002 { >> + device_type = "cpu"; >> + compatible = "arm,armv8"; >> + reg = <0x0 0x002>; >> + enable-method = "spin-table"; >> + cpu-release-addr = <0x0 0x8000fff8>; >> + }; >> + cpu@003 { >> + device_type = "cpu"; >> + compatible = "arm,armv8"; >> + reg = <0x0 0x003>; >> + enable-method = "spin-table"; >> + cpu-release-addr = <0x0 0x8000fff8>; >> + }; >> + }; >> + >> + gic: interrupt-controller@1C000000 { >> + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; > > This looks incorrect -- you should at the very least have a more > specific one than a15-gic? Marc? "arm,cortex-a9-gic" is definitely wrong (the A9 GIC doesn't have the virt extensions). This binding matches what the A15 GIC has, so "arm,cortex-a15-gic" is probably fine. Main issue here is that the GICv2 driver has no compatible string for anything else. Should we define something more generic (like "arm,gic-v2")? Or carry on adding more compatible strings? M. >> + #interrupt-cells = <3>; >> + #address-cells = <0>; >> + interrupt-controller; >> + reg = <0x0 0x1C001000 0 0x1000>, /* GIC Dist */ >> + <0x0 0x1C002000 0 0x1000>, /* GIC CPU */ >> + <0x0 0x1C004000 0 0x2000>, /* GIC VCPU Control */ >> + <0x0 0x1C006000 0 0x2000>; /* GIC VCPU */ >> + interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ >> + };
> >> + gic: interrupt-controller@1C000000 { > >> + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; > > > > This looks incorrect -- you should at the very least have a more > > specific one than a15-gic? Marc? > > "arm,cortex-a9-gic" is definitely wrong (the A9 GIC doesn't have the > virt extensions). This binding matches what the A15 GIC has, so > "arm,cortex-a15-gic" is probably fine. Main issue here is that the GICv2 > driver has no compatible string for anything else. > > Should we define something more generic (like "arm,gic-v2")? Or carry on > adding more compatible strings? It's been proposed repeatedly, and it probably makes sense to add the generic versions to the driver, and allow for more specific ones in the binding which DTs can use. That way we don't get an explosion of strings in the driver, but if we need to handle any particular GIC specially in future we can do so. I guess for Linux we'd want to add "arm,gic-v1" and "arm,gic-v2" to the driver. We could just add "arm,gic-v1" and expect it later in the compatible list if v2 is a strict superset of v1; I think it is but I'm not a GIC expert. Thanks, Mark.
On 12/02/14 11:29, Mark Rutland wrote: >>>> + gic: interrupt-controller@1C000000 { >>>> + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; >>> >>> This looks incorrect -- you should at the very least have a more >>> specific one than a15-gic? Marc? >> >> "arm,cortex-a9-gic" is definitely wrong (the A9 GIC doesn't have the >> virt extensions). This binding matches what the A15 GIC has, so >> "arm,cortex-a15-gic" is probably fine. Main issue here is that the GICv2 >> driver has no compatible string for anything else. >> >> Should we define something more generic (like "arm,gic-v2")? Or carry on >> adding more compatible strings? > > It's been proposed repeatedly, and it probably makes sense to add the > generic versions to the driver, and allow for more specific ones in the > binding which DTs can use. That way we don't get an explosion of strings > in the driver, but if we need to handle any particular GIC specially in > future we can do so. > > I guess for Linux we'd want to add "arm,gic-v1" and "arm,gic-v2" to the > driver. We could just add "arm,gic-v1" and expect it later in the > compatible list if v2 is a strict superset of v1; I think it is but I'm > not a GIC expert. Sounds good to me. M.
On Tue, Feb 11, 2014 at 03:16:26AM +0000, Kukjin Kim wrote: > On 02/12/14 03:15, Mark Rutland wrote: > > On Tue, Feb 11, 2014 at 06:29:41AM +0000, Kukjin Kim wrote: > >> Signed-off-by: Kukjin Kim<kgene.kim@samsung.com> > >> Reviewed-by: Thomas Abraham<thomas.ab@samsung.com> > >> Cc: Catalin Marinas<catalin.marinas@arm.com> > >> --- > >> arch/arm64/boot/dts/samsung-gh7.dtsi | 108 ++++++++++++++++++++++++++++++ > >> arch/arm64/boot/dts/samsung-ssdk-gh7.dts | 26 +++++++ > >> 2 files changed, 134 insertions(+) > >> create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi > >> create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts > >> > >> diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi > >> new file mode 100644 > >> index 0000000..5b8785c > >> --- /dev/null > >> +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi > >> @@ -0,0 +1,108 @@ > >> +/* > >> + * SAMSUNG GH7 SoC device tree source > >> + * > >> + * Copyright (c) 2014 Samsung Electronics Co., Ltd. > >> + * http://www.samsung.com > >> + * > >> + * 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. > >> +*/ > >> + > >> +/dts-v1/; > >> + > >> +/memreserve/ 0x80000000 0x0C400000; > > > > That looks _very_ large. What is this for? > > > Yes, I know but we need to reserve that for EL3 monitor, UEFI services, > secure, hypervisor and scan chanin... OK. How much of that memory does the kernel need to know about then? Surely the kernel shouldn't be able to map the EL3 monitor or hypervisor at all? What address is the kernel getting loaded at if everything up to 0x8C400000 isn't usable? [...] > > > [...] > > > >> + amba { > >> + compatible = "arm,amba-bus"; > >> + #address-cells =<1>; > >> + #size-cells =<1>; > >> + ranges; > >> + > >> + serial@12c00000 { > >> + compatible = "arm,pl011", "arm,primecell"; > >> + reg =<0x12c00000 0x10000>; > >> + interrupts =<418>; > >> + }; > >> + > >> + serial@12c20000 { > >> + compatible = "arm,pl011", "arm,primecell"; > >> + reg =<0x12c20000 0x10000>; > >> + interrupts =<420>; > >> + }; > > > > Don't these need clocks? > > > We don't need and the clocks will be handled by bootloader... While that might be sufficient for the device to function, Linux doesn't know that from this DT, and as far as I can see the device can't possibly probe, as no clocks are provided through platform data: of_platform_bus_create will call of_amba_device_create for anyting compatible with "arm,primecell". This in turn will call amba_device_add, which will call amba_get_enable_pclk. Then clk_get(&pcdev->dev, "apb_pclk") should fail, amba_device_add should fail, and of_platform_bus_create will stop trying to probe the node. of_platform_populate will carry on probing other nodes. Surely the pl011 nodes at least need "apb_pclk"? Cheers, Mark.
On 02/18/14 19:30, Mark Rutland wrote: > On Tue, Feb 11, 2014 at 03:16:26AM +0000, Kukjin Kim wrote: >> On 02/12/14 03:15, Mark Rutland wrote: >>> On Tue, Feb 11, 2014 at 06:29:41AM +0000, Kukjin Kim wrote: >>>> Signed-off-by: Kukjin Kim<kgene.kim@samsung.com> >>>> Reviewed-by: Thomas Abraham<thomas.ab@samsung.com> >>>> Cc: Catalin Marinas<catalin.marinas@arm.com> >>>> --- >>>> arch/arm64/boot/dts/samsung-gh7.dtsi | 108 ++++++++++++++++++++++++++++++ >>>> arch/arm64/boot/dts/samsung-ssdk-gh7.dts | 26 +++++++ >>>> 2 files changed, 134 insertions(+) >>>> create mode 100644 arch/arm64/boot/dts/samsung-gh7.dtsi >>>> create mode 100644 arch/arm64/boot/dts/samsung-ssdk-gh7.dts >>>> >>>> diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi >>>> new file mode 100644 >>>> index 0000000..5b8785c >>>> --- /dev/null >>>> +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi >>>> @@ -0,0 +1,108 @@ >>>> +/* >>>> + * SAMSUNG GH7 SoC device tree source >>>> + * >>>> + * Copyright (c) 2014 Samsung Electronics Co., Ltd. >>>> + * http://www.samsung.com >>>> + * >>>> + * 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. >>>> +*/ >>>> + >>>> +/dts-v1/; >>>> + >>>> +/memreserve/ 0x80000000 0x0C400000; >>> >>> That looks _very_ large. What is this for? >>> >> Yes, I know but we need to reserve that for EL3 monitor, UEFI services, >> secure, hypervisor and scan chanin... > > OK. How much of that memory does the kernel need to know about then? > > Surely the kernel shouldn't be able to map the EL3 monitor or hypervisor > at all? > > What address is the kernel getting loaded at if everything up to > 0x8C400000 isn't usable? > > [...] > >> >>> [...] >>> >>>> + amba { >>>> + compatible = "arm,amba-bus"; >>>> + #address-cells =<1>; >>>> + #size-cells =<1>; >>>> + ranges; >>>> + >>>> + serial@12c00000 { >>>> + compatible = "arm,pl011", "arm,primecell"; >>>> + reg =<0x12c00000 0x10000>; >>>> + interrupts =<418>; >>>> + }; >>>> + >>>> + serial@12c20000 { >>>> + compatible = "arm,pl011", "arm,primecell"; >>>> + reg =<0x12c20000 0x10000>; >>>> + interrupts =<420>; >>>> + }; >>> >>> Don't these need clocks? >>> >> We don't need and the clocks will be handled by bootloader... > > While that might be sufficient for the device to function, Linux doesn't > know that from this DT, and as far as I can see the device can't > possibly probe, as no clocks are provided through platform data: > > of_platform_bus_create will call of_amba_device_create for anyting > compatible with "arm,primecell". This in turn will call amba_device_add, > which will call amba_get_enable_pclk. Then clk_get(&pcdev->dev, > "apb_pclk") should fail, amba_device_add should fail, and > of_platform_bus_create will stop trying to probe the node. > of_platform_populate will carry on probing other nodes. > > Surely the pl011 nodes at least need "apb_pclk"? > You're right. We should make a dummy clock for pl011, proper clocks will be added in next version. Thanks, Kukjin
diff --git a/arch/arm64/boot/dts/samsung-gh7.dtsi b/arch/arm64/boot/dts/samsung-gh7.dtsi new file mode 100644 index 0000000..5b8785c --- /dev/null +++ b/arch/arm64/boot/dts/samsung-gh7.dtsi @@ -0,0 +1,108 @@ +/* + * SAMSUNG GH7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * 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. +*/ + +/dts-v1/; + +/memreserve/ 0x80000000 0x0C400000; + +/ { + model = "SAMSUNG GH7"; + compatible = "samsung,gh7"; + interrupt-parent = <&gic>; + #address-cells = <2>; + #size-cells = <2>; + + cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu@000 { + device_type = "cpu"; + compatible = "arm,armv8"; + reg = <0x0 0x000>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x8000fff8>; + }; + cpu@001 { + device_type = "cpu"; + compatible = "arm,armv8"; + reg = <0x0 0x001>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x8000fff8>; + }; + cpu@002 { + device_type = "cpu"; + compatible = "arm,armv8"; + reg = <0x0 0x002>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x8000fff8>; + }; + cpu@003 { + device_type = "cpu"; + compatible = "arm,armv8"; + reg = <0x0 0x003>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x8000fff8>; + }; + }; + + gic: interrupt-controller@1C000000 { + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic"; + #interrupt-cells = <3>; + #address-cells = <0>; + interrupt-controller; + reg = <0x0 0x1C001000 0 0x1000>, /* GIC Dist */ + <0x0 0x1C002000 0 0x1000>, /* GIC CPU */ + <0x0 0x1C004000 0 0x2000>, /* GIC VCPU Control */ + <0x0 0x1C006000 0 0x2000>; /* GIC VCPU */ + interrupts = <1 9 0xf04>; /* GIC Maintenence IRQ */ + }; + + timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, /* Secure Phys IRQ */ + <1 14 0xff01>, /* Non-secure Phys IRQ */ + <1 11 0xff01>, /* Virt IRQ */ + <1 10 0xff01>; /* Hyp IRQ */ + clock-frequency = <100000000>; + }; + + pmu { + compatible = "arm,armv8-pmuv3"; + interrupts = <0 294 8>, + <0 295 8>, + <0 296 8>, + <0 297 8>, + <0 298 8>, + <0 299 8>, + <0 300 8>, + <0 301 8>; + }; + + amba { + compatible = "arm,amba-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + serial@12c00000 { + compatible = "arm,pl011", "arm,primecell"; + reg = <0x12c00000 0x10000>; + interrupts = <418>; + }; + + serial@12c20000 { + compatible = "arm,pl011", "arm,primecell"; + reg = <0x12c20000 0x10000>; + interrupts = <420>; + }; + }; +}; diff --git a/arch/arm64/boot/dts/samsung-ssdk-gh7.dts b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts new file mode 100644 index 0000000..47afbc4 --- /dev/null +++ b/arch/arm64/boot/dts/samsung-ssdk-gh7.dts @@ -0,0 +1,26 @@ +/* + * SAMSUNG SSDK-GH7 board device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * 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. +*/ + +/dts-v1/; +#include "samsung-gh7.dtsi" + +/ { + model = "SAMSUNG SSDK-GH7 board based on GH7 SoC"; + compatible = "samsung,ssdk-gh7", "samsung,gh7"; + + chosen { + }; + + memory@80000000 { + device_type = "memory"; + reg = <0x00000000 0x80000000 0 0x80000000>; + }; +};