arm64: dts: Add initial device tree support for Tegra132
Subsystems (3)
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
Add an initial device tree file for the Tegra132 SoC. The DT file is based on arch/arm/boot/dts/tegra124.dtsi and arch/arm/boot/dts/tegra114.dtsi, with the following significant changes:
- Tegra132 uses a Denver CPU cluster rather than an ARMv7 CPU cluster
- Devices are arranged by bus, rather than in a flat topology
- No polling delays have been defined for the thermal zones. I don't believe that this is a property of the SoC hardware, but rather of a given use-case.
DT nodes representing IP blocks have generally been labeled according to the names used in Section 2.1 "System Address Map" of the _Tegra K1 64-Bit Mobile Processor Technical Reference Manual_ (DP-07148-001_ALPHA), with a few exceptions for disambiguation or abbreviated naming. Some of the known IP block aliases used by PCB designers (e.g., "GEN2_I2C" for "I2C2") have been noted in DT node comments.
Known future work:
- Add support for the Denver CLUSTER_clocks IP block
- Add support for the CPU thermal zone; now handled by a CCPLEX IP block
- The CPU spin_table enable-method may change to PSCI at some point
- Add support for several missing IP blocks
- Some drivers use unusual address spaces for devices which don't match the TRM and/or hardware decode. At some point these should be reconciled.
This patch was originally based on a patch by Allen Martin <[email protected]> and the Tegra124 and Tegra114 DTS files.
--- arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/tegra/Makefile | 3 + arch/arm64/boot/dts/tegra/tegra132.dtsi | 997 ++++++++++++++++++++++++++++++++ 3 files changed, 1001 insertions(+) create mode 100644 arch/arm64/boot/dts/tegra/Makefile create mode 100644 arch/arm64/boot/dts/tegra/tegra132.dtsi
arch/arm64/boot/dts/Makefile +1 -0
@@ -3,6 +3,7 @@ dts-dirs += apm dts-dirs += arm dts-dirs += cavium dts-dirs += exynos +dts-dirs += tegra always := $(dtb-y) subdir-y := $(dts-dirs)
arch/arm64/boot/dts/tegra/Makefile new file +3 -0
@@ -0,0 +1,3 @@ +always := $(dtb-y) +subdir-y := $(dts-dirs) +clean-files := *.dtb
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +997 -0
997 lines not on this page · raw message
Signature · 1 line
2.0.0.GIT
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
Hi Paul,
As mentioned in my reply to the DT list patch [1], there are a couple of bits I'd like to see cleaned up first, but in the meantime I have some comments from my first pass of the dtsi below. Some of these may equally apply to existing dts(i) files.
I see a few undocumented compatible strings (at least when comparing against mainline). If there are other series or trees I should be looking at, any pointers would be appreciated. If not, documentation updates would be nice (checkpatch should complain otherwise).
Quoted 20 lines · Paul Walmsley
Add an initial device tree file for the Tegra132 SoC. The DT file is based on
arch/arm/boot/dts/tegra124.dtsiandarch/arm/boot/dts/tegra114.dtsi, with the following significant changes:- Tegra132 uses a Denver CPU cluster rather than an ARMv7 CPU cluster
- Devices are arranged by bus, rather than in a flat topology
- No polling delays have been defined for the thermal zones. I don't believe that this is a property of the SoC hardware, but rather of a given use-case.DT nodes representing IP blocks have generally been labeled according to the names used in Section 2.1 "System Address Map" of the
_TegraK1 64-Bit Mobile Processor Technical ReferenceManual_(DP-07148-001_ALPHA), with a few exceptions for disambiguation or abbreviated naming. Some of the known IP block aliases used by PCB designers (e.g., "GEN2_I2C" for "I2C2") have been noted in DT node comments.
Quoted 5 lines
Known future work:
- Add support for the Denver
CLUSTER_clocksIP block
- Add support for the CPU thermal zone; now handled by a CCPLEX IP block
- The CPUspin_tableenable-method may change to PSCI at some point
That sounds nice. Any idea if/when that would be likely to happen?
Quoted 38 lines
- Add support for several missing IP blocks
- Some drivers use unusual address spaces for devices which don't match the TRM and/or hardware decode. At some point these should be reconciled.This patch was originally based on a patch by Allen Martin <[email protected]> and the Tegra124 and Tegra114 DTS files.
Signed-off-by Paul Walmsley <[email protected]>Cc Paul Walmsley <[email protected]>Cc Allen Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Pawel Moll <[email protected]>Cc Mark Rutland <[email protected]>Cc Ian Campbell <[email protected]>Cc Kumar Gala <[email protected]>Cc Russell King <[email protected]>Cc Stephen Warren <[email protected]>Cc Thierry Reding <[email protected]>Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Alexandre Courbot <[email protected]>Cc devicetree at vger.kernel.orgCc linux-arm-kernel at lists.infradead.orgCc linux-tegra at vger.kernel.org--- arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/tegra/Makefile | 3 + arch/arm64/boot/dts/tegra/tegra132.dtsi | 997 ++++++++++++++++++++++++++++++++ 3 files changed, 1001 insertions(+) create mode 100644 arch/arm64/boot/dts/tegra/Makefile create mode 100644 arch/arm64/boot/dts/tegra/tegra132.dtsiarch/arm64/boot/dts/Makefile +0
-0
arch/arm64/boot/dts/Makefile dts-dirs += apm · line 6
Quoted 4 lines
arch/arm64/boot/dts/Makefile +1
-0@@ -3,6 +3,7 @@ dts-dirs += apm dts-dirs += arm dts-dirs += cavium dts-dirs += exynos +dts-dirs += tegra
This should probably be 'nvidia' (and 'exynos' probably should have been 'samsung', given all the other directories are vendor names rather than SoC family names.
Quoted 18 lines
arch/arm64/boot/dts/Makefile +0
-0always := $(dtb-y) subdir-y := $(dts-dirs)arch/arm64/boot/dts/tegra/Makefile new file +3
-0@@ -0,0 +1,3 @@ +always := $(dtb-y) +subdir-y := $(dts-dirs) +clean-files := *.dtbarch/arm64/boot/dts/tegra/tegra132.dtsi new file +0
-0
arch/arm64/boot/dts/tegra/tegra132.dtsi line 9
Quoted 9 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +9
-0@@ -0,0 +1,997 @@ +#include <dt-bindings/clock/tegra124-car.h> +#include <dt-bindings/gpio/tegra-gpio.h> +#include <dt-bindings/memory/tegra124-mc.h> +#include <dt-bindings/pinctrl/pinctrl-tegra.h> +#include <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> +#include <dt-bindings/interrupt-controller/arm-gic.h> +#include <dt-bindings/thermal/tegra124-soctherm.h> + +#include "skeleton.dtsi"
I'd recommend against including skeleton.dtsi, it attempts to be helpful but IMO it's more problematic than it's worth. Especially given it expects #size-cells = <1> (which you override below), and therefore creates a memory node for which the reg entry is too small.
arch/arm64/boot/dts/tegra/tegra132.dtsi line 18
Quoted 9 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +9
-0+ +/ { + compatible = "nvidia,tegra132"; + interrupt-parent = <&gic>; + #address-cells = <2>; + #size-cells = <2>; + + pcie-controller at 0,01003000 { + compatible = "nvidia,tegra132-pcie", "nvidia,tegra124-pcie";
I couldn't spot "nvidia,tegra132-pcie" in mainline anywhere. Does it differ significantly from "nvidia,tegra124-pcie"?
arch/arm64/boot/dts/tegra/tegra132.dtsi line 22
Quoted 4 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ device_type = "pci"; + reg = <0x0 0x01003000 0x0 0x00000800 /* PADS registers */ + 0x0 0x01003800 0x0 0x00000800 /* AFI registers */ + 0x0 0x02000000 0x0 0x10000000>; /* configuration space */
Nit: for properties which are lists, please bracket each entry, e.g.
reg = <0x0 0x01003000 0x0 0x00000800>, /* PADS registers */
<0x0 0x01003800 0x0 0x00000800>,
<0x0 0x02000000 0x0 0x10000000>;I appreciate the newlines serve as an obvious delimiter here, but in single-line cases it really helps, and it would be nice to be consistent. It looks like most instances in this file already do this anyway.
Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +8
-0+ reg-names = "pads", "afi", "cs"; + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */ + interrupt-names = "intr", "msi"; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &gic GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
arch/arm64/boot/dts/tegra/tegra132.dtsi line 40
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ + bus-range = <0x00 0xff>; + #address-cells = <3>; + #size-cells = <2>; + + ranges = <0x82000000 0 0x01000000 0x0 0x01000000 0 0x00001000 /* port 0 configuration space */ + 0x82000000 0 0x01001000 0x0 0x01001000 0 0x00001000 /* port 1 configuration space */ + 0x81000000 0 0x0 0x0 0x12000000 0 0x00010000 /* downstream I/O (64 KiB) */ + 0x82000000 0 0x13000000 0x0 0x13000000 0 0x0d000000 /* non-prefetchable memory (208 MiB) */ + 0xc2000000 0 0x20000000 0x0 0x20000000 0 0x20000000>; /* prefetchable memory (512 MiB) */
Same here w.r.t. list bracketing.
Quoted 14 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +14
-0+ + clocks = <&tegra_car TEGRA124_CLK_PCIE>, + <&tegra_car TEGRA124_CLK_AFI>, + <&tegra_car TEGRA124_CLK_PLL_E>, + <&tegra_car TEGRA124_CLK_CML0>; + clock-names = "pex", "afi", "pll_e", "cml"; + resets = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 74>; + reset-names = "pex", "afi", "pcie_x"; + status = "disabled"; + + phys = <&xusb_padctl TEGRA_XUSB_PADCTL_PCIE>; + phy-names = "pcie";
arch/arm64/boot/dts/tegra/tegra132.dtsi line 64
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ + pci at 1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x01000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges;
I'm not sure why these three properties are here. Surely this is a separate address space (so isn't ranges invalid?), and do we define any sub-nodes anywhere?
Quoted 4 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ + nvidia,num-lanes = <2>; + }; +
arch/arm64/boot/dts/tegra/tegra132.dtsi line 78
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ pci at 2,0 { + device_type = "pci"; + assigned-addresses = <0x82001000 0 0x01001000 0 0x1000>; + reg = <0x001000 0 0 0 0>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; +
Likewise.
Quoted 3 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +3
-0+ nvidia,num-lanes = <1>; + }; + };
[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 104
Quoted 2 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +2
-0+ host1x at 0,50000000 { + compatible = "nvidia,tegra132-host1x", "nvidia,tegra124-host1x", "simple-bus";
Regarding simple-bus, are the sub-nodes usable if this didn't probe as "nvidia,tegra124-host1x" or "nvidia,tegra132-host1x"? Do the subnodes require anything from this node?
It looks like we expect/require the host1x node to be probed and initialised before we probe the children. Which would imply the simple-bus annotation is wrong.
Quoted 81 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +81
-0+ reg = <0x0 0x50000000 0x0 0x00034000>; + interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>, /* syncpt */ + <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>; /* general */ + clocks = <&tegra_car TEGRA124_CLK_HOST1X>; + resets = <&tegra_car 28>; + reset-names = "host1x"; + + #address-cells = <2>; + #size-cells = <2>; + + ranges = <0 0x54000000 0 0x54000000 0 0x01000000>; + + dc at 0,54200000 { + compatible = "nvidia,tegra132-dc", "nvidia,tegra124-dc"; + reg = <0x0 0x54200000 0x0 0x00040000>; + interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_DISP1>, + <&tegra_car TEGRA124_CLK_PLL_P>; + clock-names = "dc", "parent"; + resets = <&tegra_car 27>; + reset-names = "dc"; + + iommus = <&mc TEGRA_SWGROUP_DC>; + + nvidia,head = <0>; + }; + + dc at 0,54240000 { + compatible = "nvidia,tegra132-dc", "nvidia,tegra124-dc"; + reg = <0x0 0x54240000 0x0 0x00040000>; + interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_DISP2>, + <&tegra_car TEGRA124_CLK_PLL_P>; + clock-names = "dc", "parent"; + resets = <&tegra_car 26>; + reset-names = "dc"; + + iommus = <&mc TEGRA_SWGROUP_DCB>; + + nvidia,head = <1>; + }; + + hdmi at 0,54280000 { + compatible = "nvidia,tegra132-hdmi", "nvidia,tegra124-hdmi"; + reg = <0x0 0x54280000 0x0 0x00040000>; + interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_HDMI>, + <&tegra_car TEGRA124_CLK_PLL_D2_OUT0>; + clock-names = "hdmi", "parent"; + resets = <&tegra_car 51>; + reset-names = "hdmi"; + status = "disabled"; + }; + + sor at 0,54540000 { + compatible = "nvidia,tegra132-sor", "nvidia,tegra124-sor"; + reg = <0x0 0x54540000 0x0 0x00040000>; + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_SOR0>, + <&tegra_car TEGRA124_CLK_PLL_D_OUT0>, + <&tegra_car TEGRA124_CLK_PLL_DP>, + <&tegra_car TEGRA124_CLK_CLK_M>; + clock-names = "sor", "parent", "dp", "safe"; + resets = <&tegra_car 182>; + reset-names = "sor"; + status = "disabled"; + }; + + dpaux: dpaux at 0,545c0000 { + compatible = "nvidia,tegra132-dpaux", "nvidia,tegra124-dpaux"; + reg = <0x0 0x545c0000 0x0 0x00040000>; + interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_DPAUX>, + <&tegra_car TEGRA124_CLK_PLL_DP>; + clock-names = "dpaux", "parent"; + resets = <&tegra_car 181>; + reset-names = "dpaux"; + status = "disabled"; + }; + }; +
[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 196
Quoted 11 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +11
-0+ gic: interrupt-controller at 0,50041000 { + compatible = "arm,cortex-a15-gic"; + #interrupt-cells = <3>; + interrupt-controller; + reg = <0x0 0x50041000 0x0 0x1000>, + <0x0 0x50042000 0x0 0x1000>, + <0x0 0x50044000 0x0 0x2000>, + <0x0 0x50046000 0x0 0x2000>; + interrupts = <GIC_PPI 9 + (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>; + };
/cpus (below) only has two CPUs listed, so that mask doensn't look right.
[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 218
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0+ ppsb: ppsb at 0,60000000 { + compatible = "simple-bus"; + reg = <0x0 0x60000000 0x0 0x01000000>; + #address-cells = <2>; + #size-cells = <2>; + ranges;
This reg overlaps the devices described below, and this node is only described as a simple-bus (for whch reg is not a valid property). That doesn't look right to me.
If you want to describe that the bus covers a particular range, you can encode that in the ranges property.
[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 253
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ ahb_gizmo: ahb-gizmo at 0,6000c004 { + compatible = "nvidia,tegra132-ahb", "nvidia,tegra124-ahb", "nvidia,tegra30-ahb"; + /* + * This IP block actually starts at 0x6000c000, + * but all of the register offsets in the driver + * have 0x4 subtracted from them. So handle + * it this way until the driver is fixed. + */ + reg = <0x0 0x6000c004 0x0 0x14d>; + };
That doesn't sound great. Can we not fix the driver and limit that mistake to existing DTBs?
[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 318
Quoted 3 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +3
-0+ apb: apb at 0,70000000 { + compatible = "simple-bus"; + reg = <0x0 0x70000000 0x0 0x01000000>;
As with previously, a reg property doesn't make sense for a block that's only a "simple-bus"
Quoted 17 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +17
-0+ #address-cells = <2>; + #size-cells = <2>; + ranges; + + apbmisc: apbmisc at 0,70000800 { + compatible = "nvidia,tegra132-apbmisc", "nvidia,tegra124-apbmisc", "nvidia,tegra20-apbmisc"; + reg = <0x0 0x70000800 0x0 0x64>, /* Chip revision */ + <0x0 0x7000E864 0x0 0x04>; /* Strapping options */ + }; + + pinmux: pinmux at 0,70000868 { + compatible = "nvidia,tegra132-pinmux", "nvidia,tegra124-pinmux"; + reg = <0x0 0x70000868 0x0 0x164>, /* Pad control registers */ + <0x0 0x70003000 0x0 0x434>; /* Mux registers */ + }; + + /*
arch/arm64/boot/dts/tegra/tegra132.dtsi line 345
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ * There are two serial drivers: an 8250 based simple + * serial driver and an APB DMA based serial driver + * for higher baudrate and performance. To enable the + * 8250 based driver, the compatible string is + * "nvidia,tegra132-uart", "nvidia,tegra124-uart", + * "nvidia,tegra20-uart" and to enable the APB DMA + * based serial driver, the compatible string is + * "nvidia,tegra132-hsuart", "nvidia,tegra124-hsuart", + * "nvidia,tegra30-hsuart". + */
Is there any reason to continue with this split?
Surely if the only difference is DMA, the presence of dmas and dma-names should be sufficient to get the driver to do the right thing, and if you need to disable DMA for debugging that could be a command-line option.
Does the binding and/or driver support aliases so you can get consistent numbering? It would be nice to have.
Do these UARTs work with earlycon?
It would be nice to have a /chosen/stdout-path (ideally with rate) so as to get output consistently without command line options being required.
Quoted 51 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +51
-0+ uarta: serial at 0,70006000 { + compatible = "nvidia,tegra132-uart", "nvidia,tegra124-uart", "nvidia,tegra20-uart"; + reg = <0x0 0x70006000 0x0 0x40>; + reg-shift = <2>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_UARTA>; + resets = <&tegra_car 6>; + reset-names = "serial"; + dmas = <&apbdma 8>, <&apbdma 8>; + dma-names = "rx", "tx"; + status = "disabled"; + }; + + uartb: serial at 0,70006040 { + compatible = "nvidia,tegra132-uart", "nvidia,tegra124-uart", "nvidia,tegra20-uart"; + reg = <0x0 0x70006040 0x0 0x40>; + reg-shift = <2>; + interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_UARTB>; + resets = <&tegra_car 7>; + reset-names = "serial"; + dmas = <&apbdma 9>, <&apbdma 9>; + dma-names = "rx", "tx"; + status = "disabled"; + }; + + uartc: serial at 0,70006200 { + compatible = "nvidia,tegra132-uart", "nvidia,tegra124-uart", "nvidia,tegra20-uart"; + reg = <0x0 0x70006200 0x0 0x40>; + reg-shift = <2>; + interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_UARTC>; + resets = <&tegra_car 55>; + reset-names = "serial"; + dmas = <&apbdma 10>, <&apbdma 10>; + dma-names = "rx", "tx"; + status = "disabled"; + }; + + uartd: serial at 0,70006300 { + compatible = "nvidia,tegra132-uart", "nvidia,tegra124-uart", "nvidia,tegra20-uart"; + reg = <0x0 0x70006300 0x0 0x40>; + reg-shift = <2>; + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_UARTD>; + resets = <&tegra_car 65>; + reset-names = "serial"; + dmas = <&apbdma 19>, <&apbdma 19>; + dma-names = "rx", "tx"; + status = "disabled"; + };
[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 833
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0+ ahb_a2: ahb at 0,7c000000 { + compatible = "simple-bus"; + reg = <0x0 0x7c000000 0x0 0x02000000>; + #address-cells = <2>; + #size-cells = <2>; + ranges;
Again, this doesn't look right for a plain simple-bus.
[...]
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x0>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>;
arch/arm64/boot/dts/tegra/tegra132.dtsi line 965
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ }; + + cpu at 1 { + device_type = "cpu"; + compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x1>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>; + }; + };
It would be nice if this were near the top, as in other dts files.
It's a shame these share the same release address, though I guess that doesn't matter too much if PSCI is forthcoming.
I didn't spot a memory node or /memreserve/ entries. Is the memory used here for the spin-table guaranteed to be protected?
If the enable-method might change, it's probably best to leave this to the bootloader to fill in, or to have it in the board files if the boot loader isn't that clever (perhaps via a dtsi).
arch/arm64/boot/dts/tegra/tegra132.dtsi line 969
Quoted 4 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ + thermal-zones { + /* XXX T132 CPU thermal zone - still TBD */ +
What is still TBD here?
Quoted 16 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +16
-0+ mem { + thermal-sensors = + <&soc_therm TEGRA124_SOCTHERM_SENSOR_MEM>; + }; + + gpu { + thermal-sensors = + <&soc_therm TEGRA124_SOCTHERM_SENSOR_GPU>; + }; + + pllx { + thermal-sensors = + <&soc_therm TEGRA124_SOCTHERM_SENSOR_PLLX>; + }; + }; +
arch/arm64/boot/dts/tegra/tegra132.dtsi line 995
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ timer { + compatible = "arm,armv8-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)>;
Those masks look wrong given there were two CPUs described above.
Thanks,
Mark.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/317010.html
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
+ Arto, Terje for comments on the host1x section
+ Stephen Warren for comments on the serial DT data
Hi Mark,
thanks for the review.
Quoted 9 lines · Mark Rutland
As mentioned in my reply to the DT list patch [1], there are a couple of bits I'd like to see cleaned up first, but in the meantime I have some comments from my first pass of the dtsi below. Some of these may equally apply to existing dts(i) files.
I see a few undocumented compatible strings (at least when comparing against mainline). If there are other series or trees I should be looking at, any pointers would be appreciated. If not, documentation updates would be nice (checkpatch should complain otherwise).
(discussed in the tegra132-pcie comments below)
Quoted 22 lines · Paul Walmsley
Quoted 20 lines
Add an initial device tree file for the Tegra132 SoC. The DT file is based on
arch/arm/boot/dts/tegra124.dtsiandarch/arm/boot/dts/tegra114.dtsi, with the following significant changes:- Tegra132 uses a Denver CPU cluster rather than an ARMv7 CPU cluster
- Devices are arranged by bus, rather than in a flat topology
- No polling delays have been defined for the thermal zones. I don't believe that this is a property of the SoC hardware, but rather of a given use-case.DT nodes representing IP blocks have generally been labeled according to the names used in Section 2.1 "System Address Map" of the
_TegraK1 64-Bit Mobile Processor Technical ReferenceManual_(DP-07148-001_ALPHA), with a few exceptions for disambiguation or abbreviated naming. Some of the known IP block aliases used by PCB designers (e.g., "GEN2_I2C" for "I2C2") have been noted in DT node comments.
Quoted 7 lines
Quoted 5 lines
Known future work:
- Add support for the Denver
CLUSTER_clocksIP block
- Add support for the CPU thermal zone; now handled by a CCPLEX IP block
- The CPUspin_tableenable-method may change to PSCI at some pointThat sounds nice. Any idea if/when that would be likely to happen?
The PSCI aspect? Unfortunately not at the moment due to the secure firmware dependency.
Quoted 34 lines
Quoted 34 lines
- Add support for several missing IP blocks
- Some drivers use unusual address spaces for devices which don't match the TRM and/or hardware decode. At some point these should be reconciled.This patch was originally based on a patch by Allen Martin <[email protected]> and the Tegra124 and Tegra114 DTS files.
Signed-off-by Paul Walmsley <[email protected]>Cc Paul Walmsley <[email protected]>Cc Allen Martin <[email protected]>Cc Rob Herring <[email protected]>Cc Pawel Moll <[email protected]>Cc Mark Rutland <[email protected]>Cc Ian Campbell <[email protected]>Cc Kumar Gala <[email protected]>Cc Russell King <[email protected]>Cc Stephen Warren <[email protected]>Cc Thierry Reding <[email protected]>Cc Catalin Marinas <[email protected]>Cc Will Deacon <[email protected]>Cc Alexandre Courbot <[email protected]>Cc devicetree at vger.kernel.orgCc linux-arm-kernel at lists.infradead.orgCc linux-tegra at vger.kernel.org--- arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/tegra/Makefile | 3 + arch/arm64/boot/dts/tegra/tegra132.dtsi | 997 ++++++++++++++++++++++++++++++++ 3 files changed, 1001 insertions(+) create mode 100644 arch/arm64/boot/dts/tegra/Makefile create mode 100644 arch/arm64/boot/dts/tegra/tegra132.dtsi
Quoted 13 lines
arch/arm64/boot/dts/Makefile dts-dirs += apm · line 6
Quoted 9 lines
arch/arm64/boot/dts/Makefile +1
-0@@ -3,6 +3,7 @@ dts-dirs += apm dts-dirs += arm dts-dirs += cavium dts-dirs += exynos +dts-dirs += tegraThis should probably be 'nvidia' (and 'exynos' probably should have been 'samsung', given all the other directories are vendor names rather than SoC family names.
OK, will change.
Quoted 24 lines
Quoted 24 lines
arch/arm64/boot/dts/Makefile +0
-0always := $(dtb-y) subdir-y := $(dts-dirs)arch/arm64/boot/dts/tegra/Makefile new file +3
-0@@ -0,0 +1,3 @@ +always := $(dtb-y) +subdir-y := $(dts-dirs) +clean-files := *.dtbarch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0@@ -0,0 +1,997 @@ +#include <dt-bindings/clock/tegra124-car.h> +#include <dt-bindings/gpio/tegra-gpio.h> +#include <dt-bindings/memory/tegra124-mc.h> +#include <dt-bindings/pinctrl/pinctrl-tegra.h> +#include <dt-bindings/pinctrl/pinctrl-tegra-xusb.h> +#include <dt-bindings/interrupt-controller/arm-gic.h>
Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 9
Quoted 3 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +3
-0+#include <dt-bindings/thermal/tegra124-soctherm.h> + +#include "skeleton.dtsi"I'd recommend against including
skeleton.dtsi, it attempts to be helpful but IMO it's more problematic than it's worth. Especially given it expects #size-cells = <1> (which you override below), and therefore creates a memory node for which the reg entry is too small.
OK, will drop.
Quoted 13 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 18
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +9
-0+ +/ { + compatible = "nvidia,tegra132"; + interrupt-parent = <&gic>; + #address-cells = <2>; + #size-cells = <2>; + + pcie-controller at 0,01003000 { + compatible = "nvidia,tegra132-pcie", "nvidia,tegra124-pcie";I couldn't spot "nvidia,tegra132-pcie" in mainline anywhere. Does it differ significantly from "nvidia,tegra124-pcie"?
I'm not sure if the T132 PCIe driver will need any T132-specific workarounds. I just followed what seems to be at least one DT practice discussed in the past of adding SoC-specific variants for the IP blocks in the event that a SoC-specific driver change is needed in the future. I'm probably out-of-date on the current doctrine here, though.
Anyway, I have no problem with dropping the tegra132- variants, except in cases where changes are specifically known to be needed, although this seems likely to strengthen the binding between DT data and kernel versions. Let me know.
Quoted 7 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 22
Quoted 4 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ device_type = "pci"; + reg = <0x0 0x01003000 0x0 0x00000800 /* PADS registers */ + 0x0 0x01003800 0x0 0x00000800 /* AFI registers */ + 0x0 0x02000000 0x0 0x10000000>; /* configuration space */Nit: for properties which are lists, please bracket each entry, e.g.
Quoted 8 lines
reg = <
0x00x010030000x00x00000800>, /* PADS registers */<0x0 0x01003800 0x0 0x00000800>, <0x0 0x02000000 0x0 0x10000000>;I appreciate the newlines serve as an obvious delimiter here, but in single-line cases it really helps, and it would be nice to be consistent. It looks like most instances in this file already do this anyway.
OK will do.
Quoted 13 lines
Quoted 13 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +12
-0+ reg-names = "pads", "afi", "cs"; + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */ + interrupt-names = "intr", "msi"; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &gic GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; + + bus-range = <0x00 0xff>; + #address-cells = <3>; + #size-cells = <2>;
Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 40
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0+ + ranges = <0x82000000 0 0x01000000 0x0 0x01000000 0 0x00001000 /* port 0 configuration space */ + 0x82000000 0 0x01001000 0x0 0x01001000 0 0x00001000 /* port 1 configuration space */ + 0x81000000 0 0x0 0x0 0x12000000 0 0x00010000 /* downstream I/O (64 KiB) */ + 0x82000000 0 0x13000000 0x0 0x13000000 0 0x0d000000 /* non-prefetchable memory (208 MiB) */ + 0xc2000000 0 0x20000000 0x0 0x20000000 0 0x20000000>; /* prefetchable memory (512 MiB) */Same here w.r.t. list bracketing.
OK
Quoted 21 lines
Quoted 21 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +20
-0+ + clocks = <&tegra_car TEGRA124_CLK_PCIE>, + <&tegra_car TEGRA124_CLK_AFI>, + <&tegra_car TEGRA124_CLK_PLL_E>, + <&tegra_car TEGRA124_CLK_CML0>; + clock-names = "pex", "afi", "pll_e", "cml"; + resets = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 74>; + reset-names = "pex", "afi", "pcie_x"; + status = "disabled"; + + phys = <&xusb_padctl TEGRA_XUSB_PADCTL_PCIE>; + phy-names = "pcie"; + + pci at 1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x01000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + status = "disabled";
Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 64
Quoted 4 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ + #address-cells = <3>; + #size-cells = <2>; + ranges;I'm not sure why these three properties are here. Surely this is a separate address space (so isn't ranges invalid?), and do we define any sub-nodes anywhere?
Hmm not sure. This was originally copied from arch/arm/boot/dts/tegra124.dtsi. Will plan to drop them for now, and then if the properties (or ones like them) turn out to be needed in the future, someone else can deal with it :-)
Quoted 17 lines
Quoted 5 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ + nvidia,num-lanes = <2>; + }; +arch/arm64/boot/dts/tegra/tegra132.dtsi line 78
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ pci at 2,0 { + device_type = "pci"; + assigned-addresses = <0x82001000 0 0x01001000 0 0x1000>; + reg = <0x001000 0 0 0 0>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; +Likewise.
OK
Quoted 10 lines
Quoted 4 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +3
-0+ nvidia,num-lanes = <1>; + }; + };[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 104
Quoted 2 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +2
-0+ host1x at 0,50000000 { + compatible = "nvidia,tegra132-host1x", "nvidia,tegra124-host1x", "simple-bus";
Quoted 7 lines
Regarding simple-bus, are the sub-nodes usable if this didn't probe as "nvidia,tegra124-host1x" or "nvidia,tegra132-host1x"?
Do the subnodes require anything from this node?It looks like we expect/require the host1x node to be probed and initialised before we probe the children. Which would imply the simple-bus annotation is wrong.
Haven't tried booting with just simple-bus here. I believe these devices are accessible without the involvement of host1x. In other words, host1x is not a bus; I believe it might be most accurately described as a type of DMA controller. So in theory it should be possible for the CPU complex to read and write to these devices directly without the involvement of host1x, although I believe NVIDIA discourages this.
Under the theory that DT data should be hardware-specific, not software-specific, in an ideal world I think we would list these devices outside the embrace of the host1x node. However the existing Tegra124 DT file listed them together, and I am unsure whether that is required for the host1x code to function correctly.
Arto may be able to comment further.
Quoted 97 lines
Quoted 82 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +81
-0+ reg = <0x0 0x50000000 0x0 0x00034000>; + interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>, /* syncpt */ + <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>; /* general */ + clocks = <&tegra_car TEGRA124_CLK_HOST1X>; + resets = <&tegra_car 28>; + reset-names = "host1x"; + + #address-cells = <2>; + #size-cells = <2>; + + ranges = <0 0x54000000 0 0x54000000 0 0x01000000>; + + dc at 0,54200000 { + compatible = "nvidia,tegra132-dc", "nvidia,tegra124-dc"; + reg = <0x0 0x54200000 0x0 0x00040000>; + interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_DISP1>, + <&tegra_car TEGRA124_CLK_PLL_P>; + clock-names = "dc", "parent"; + resets = <&tegra_car 27>; + reset-names = "dc"; + + iommus = <&mc TEGRA_SWGROUP_DC>; + + nvidia,head = <0>; + }; + + dc at 0,54240000 { + compatible = "nvidia,tegra132-dc", "nvidia,tegra124-dc"; + reg = <0x0 0x54240000 0x0 0x00040000>; + interrupts = <GIC_SPI 74 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_DISP2>, + <&tegra_car TEGRA124_CLK_PLL_P>; + clock-names = "dc", "parent"; + resets = <&tegra_car 26>; + reset-names = "dc"; + + iommus = <&mc TEGRA_SWGROUP_DCB>; + + nvidia,head = <1>; + }; + + hdmi at 0,54280000 { + compatible = "nvidia,tegra132-hdmi", "nvidia,tegra124-hdmi"; + reg = <0x0 0x54280000 0x0 0x00040000>; + interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_HDMI>, + <&tegra_car TEGRA124_CLK_PLL_D2_OUT0>; + clock-names = "hdmi", "parent"; + resets = <&tegra_car 51>; + reset-names = "hdmi"; + status = "disabled"; + }; + + sor at 0,54540000 { + compatible = "nvidia,tegra132-sor", "nvidia,tegra124-sor"; + reg = <0x0 0x54540000 0x0 0x00040000>; + interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_SOR0>, + <&tegra_car TEGRA124_CLK_PLL_D_OUT0>, + <&tegra_car TEGRA124_CLK_PLL_DP>, + <&tegra_car TEGRA124_CLK_CLK_M>; + clock-names = "sor", "parent", "dp", "safe"; + resets = <&tegra_car 182>; + reset-names = "sor"; + status = "disabled"; + }; + + dpaux: dpaux at 0,545c0000 { + compatible = "nvidia,tegra132-dpaux", "nvidia,tegra124-dpaux"; + reg = <0x0 0x545c0000 0x0 0x00040000>; + interrupts = <GIC_SPI 159 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_DPAUX>, + <&tegra_car TEGRA124_CLK_PLL_DP>; + clock-names = "dpaux", "parent"; + resets = <&tegra_car 181>; + reset-names = "dpaux"; + status = "disabled"; + }; + }; +[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 196
Quoted 11 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +11
-0+ gic: interrupt-controller at 0,50041000 { + compatible = "arm,cortex-a15-gic"; + #interrupt-cells = <3>; + interrupt-controller; + reg = <0x0 0x50041000 0x0 0x1000>, + <0x0 0x50042000 0x0 0x1000>, + <0x0 0x50044000 0x0 0x2000>, + <0x0 0x50046000 0x0 0x2000>; + interrupts = <GIC_PPI 9 + (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>; + };
Quoted 2 lines
/cpus (below) only has two CPUs listed, so that mask doensn't look right.
OK will fix
Quoted 10 lines
[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 218
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0+ ppsb: ppsb at 0,60000000 { + compatible = "simple-bus"; + reg = <0x0 0x60000000 0x0 0x01000000>; + #address-cells = <2>; + #size-cells = <2>; + ranges;
Quoted 6 lines
This reg overlaps the devices described below, and this node is only described as a simple-bus (for whch reg is not a valid property). That doesn't look right to me.
If you want to describe that the bus covers a particular range, you can encode that in the ranges property.
OK will fix.
Quoted 14 lines
[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 253
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ ahb_gizmo: ahb-gizmo at 0,6000c004 { + compatible = "nvidia,tegra132-ahb", "nvidia,tegra124-ahb", "nvidia,tegra30-ahb"; + /* + * This IP block actually starts at 0x6000c000, + * but all of the register offsets in the driver + * have 0x4 subtracted from them. So handle + * it this way until the driver is fixed. + */ + reg = <0x0 0x6000c004 0x0 0x14d>; + };
Quoted 2 lines
That doesn't sound great. Can we not fix the driver and limit that mistake to existing DTBs?
We can do it that way if you think it's important to fix now. Have been trying to avoid gating our initial ARM64 support on fixing all of our hardware misalignment problems accumulated over the years, but if you think it's a blocker, it can be done.
Quoted 9 lines
[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 318
Quoted 3 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +3
-0+ apb: apb at 0,70000000 { + compatible = "simple-bus"; + reg = <0x0 0x70000000 0x0 0x01000000>;As with previously, a reg property doesn't make sense for a block that's only a "simple-bus"
OK will fix.
Quoted 35 lines
Quoted 18 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +17
-0+ #address-cells = <2>; + #size-cells = <2>; + ranges; + + apbmisc: apbmisc at 0,70000800 { + compatible = "nvidia,tegra132-apbmisc", "nvidia,tegra124-apbmisc", "nvidia,tegra20-apbmisc"; + reg = <0x0 0x70000800 0x0 0x64>, /* Chip revision */ + <0x0 0x7000E864 0x0 0x04>; /* Strapping options */ + }; + + pinmux: pinmux at 0,70000868 { + compatible = "nvidia,tegra132-pinmux", "nvidia,tegra124-pinmux"; + reg = <0x0 0x70000868 0x0 0x164>, /* Pad control registers */ + <0x0 0x70003000 0x0 0x434>; /* Mux registers */ + }; + + /*arch/arm64/boot/dts/tegra/tegra132.dtsi line 345
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ * There are two serial drivers: an 8250 based simple + * serial driver and an APB DMA based serial driver + * for higher baudrate and performance. To enable the + * 8250 based driver, the compatible string is + * "nvidia,tegra132-uart", "nvidia,tegra124-uart", + * "nvidia,tegra20-uart" and to enable the APB DMA + * based serial driver, the compatible string is + * "nvidia,tegra132-hsuart", "nvidia,tegra124-hsuart", + * "nvidia,tegra30-hsuart". + */Is there any reason to continue with this split?
Surely if the only difference is DMA, the presence of dmas and dma-names should be sufficient to get the driver to do the right thing, and if you need to disable DMA for debugging that could be a command-line option.
Quoted 7 lines
Does the binding and/or driver support aliases so you can get consistent numbering? It would be nice to have.
Do these UARTs work with earlycon?
It would be nice to have a /chosen/stdout-path (ideally with rate) so as to get output consistently without command line options being required.
Stephen Warren might be the best person to directly respond to these issues. This is all legacy DT data from previous Tegra SoCs.
Quoted 55 lines
Quoted 52 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +51
-0+ uarta: serial at 0,70006000 { + compatible = "nvidia,tegra132-uart", "nvidia,tegra124-uart", "nvidia,tegra20-uart"; + reg = <0x0 0x70006000 0x0 0x40>; + reg-shift = <2>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_UARTA>; + resets = <&tegra_car 6>; + reset-names = "serial"; + dmas = <&apbdma 8>, <&apbdma 8>; + dma-names = "rx", "tx"; + status = "disabled"; + }; + + uartb: serial at 0,70006040 { + compatible = "nvidia,tegra132-uart", "nvidia,tegra124-uart", "nvidia,tegra20-uart"; + reg = <0x0 0x70006040 0x0 0x40>; + reg-shift = <2>; + interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_UARTB>; + resets = <&tegra_car 7>; + reset-names = "serial"; + dmas = <&apbdma 9>, <&apbdma 9>; + dma-names = "rx", "tx"; + status = "disabled"; + }; + + uartc: serial at 0,70006200 { + compatible = "nvidia,tegra132-uart", "nvidia,tegra124-uart", "nvidia,tegra20-uart"; + reg = <0x0 0x70006200 0x0 0x40>; + reg-shift = <2>; + interrupts = <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_UARTC>; + resets = <&tegra_car 55>; + reset-names = "serial"; + dmas = <&apbdma 10>, <&apbdma 10>; + dma-names = "rx", "tx"; + status = "disabled"; + }; + + uartd: serial at 0,70006300 { + compatible = "nvidia,tegra132-uart", "nvidia,tegra124-uart", "nvidia,tegra20-uart"; + reg = <0x0 0x70006300 0x0 0x40>; + reg-shift = <2>; + interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_UARTD>; + resets = <&tegra_car 65>; + reset-names = "serial"; + dmas = <&apbdma 19>, <&apbdma 19>; + dma-names = "rx", "tx"; + status = "disabled"; + };[...]
Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 833
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0+ ahb_a2: ahb at 0,7c000000 { + compatible = "simple-bus"; + reg = <0x0 0x7c000000 0x0 0x02000000>; + #address-cells = <2>; + #size-cells = <2>; + ranges;Again, this doesn't look right for a plain simple-bus.
OK will fix.
Quoted 17 lines
[...]
Quoted 14 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +14
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x0>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>; + }; + + cpu at 1 { + device_type = "cpu";
Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 965
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0+ compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x1>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>; + }; + };It would be nice if this were near the top, as in other dts files.
OK will move.
Quoted 10 lines
It's a shame these share the same release address, though I guess that doesn't matter too much if PSCI is forthcoming.
I didn't spot a memory node or /memreserve/ entries. Is the memory used here for the spin-table guaranteed to be protected?
If the enable-method might change, it's probably best to leave this to the bootloader to fill in, or to have it in the board files if the boot loader isn't that clever (perhaps via a dtsi).
OK will drop the enable-method for the time being.
Quoted 7 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 969
Quoted 5 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ + thermal-zones { + /* XXX T132 CPU thermal zone - still TBD */ +What is still TBD here?
The T132 CPU thermal zone thermal sensor data. Specifically (and I hope this comes as no surprise), there's some power/thermal management data missing from this file that is intended to be added later. The placeholder is simply to indicate that.
Quoted 21 lines
Quoted 21 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +20
-0+ mem { + thermal-sensors = + <&soc_therm TEGRA124_SOCTHERM_SENSOR_MEM>; + }; + + gpu { + thermal-sensors = + <&soc_therm TEGRA124_SOCTHERM_SENSOR_GPU>; + }; + + pllx { + thermal-sensors = + <&soc_therm TEGRA124_SOCTHERM_SENSOR_PLLX>; + }; + }; + + timer { + compatible = "arm,armv8-timer"; + interrupts = <GIC_PPI 13 + (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 995
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0+ <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)>;Those masks look wrong given there were two CPUs described above.
OK will fix.
Quoted 6 lines
Thanks,
Mark.[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/317010.html
- Paul
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
Quoted 45 lines · Paul Walmsley
+ Arto, Terje for comments on the host1x section
+ Stephen Warren for comments on the serial DT dataHi Mark,
thanks for the review.
Quoted 9 lines · Mark Rutland
As mentioned in my reply to the DT list patch [1], there are a couple of bits I'd like to see cleaned up first, but in the meantime I have some comments from my first pass of the dtsi below. Some of these may equally apply to existing dts(i) files.
I see a few undocumented compatible strings (at least when comparing against mainline). If there are other series or trees I should be looking at, any pointers would be appreciated. If not, documentation updates would be nice (checkpatch should complain otherwise).
(discussed in the tegra132-pcie comments below)
Quoted 24 lines · Paul Walmsley
Quoted 22 lines
Add an initial device tree file for the Tegra132 SoC. The DT file is based on
arch/arm/boot/dts/tegra124.dtsiandarch/arm/boot/dts/tegra114.dtsi, with the following significant changes:- Tegra132 uses a Denver CPU cluster rather than an ARMv7 CPU cluster
- Devices are arranged by bus, rather than in a flat topology
- No polling delays have been defined for the thermal zones. I don't believe that this is a property of the SoC hardware, but rather of a given use-case.DT nodes representing IP blocks have generally been labeled according to the names used in Section 2.1 "System Address Map" of the
_TegraK1 64-Bit Mobile Processor Technical ReferenceManual_(DP-07148-001_ALPHA), with a few exceptions for disambiguation or abbreviated naming. Some of the known IP block aliases used by PCB designers (e.g., "GEN2_I2C" for "I2C2") have been noted in DT node comments.Known future work:
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
- Add support for the Denver
CLUSTER_clocksIP block
- Add support for the CPU thermal zone; now handled by a CCPLEX IP block
- The CPUspin_tableenable-method may change to PSCI at some pointThat sounds nice. Any idea if/when that would be likely to happen?
The PSCI aspect? Unfortunately not at the moment due to the secure firmware dependency.
Ah, ok.
[...]
Quoted 18 lines
Quoted 11 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 18
Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +8
-0+/ { + compatible = "nvidia,tegra132"; + interrupt-parent = <&gic>; + #address-cells = <2>; + #size-cells = <2>; + + pcie-controller at 0,01003000 { + compatible = "nvidia,tegra132-pcie", "nvidia,tegra124-pcie";I couldn't spot "nvidia,tegra132-pcie" in mainline anywhere. Does it differ significantly from "nvidia,tegra124-pcie"?
I'm not sure if the T132 PCIe driver will need any T132-specific workarounds. I just followed what seems to be at least one DT practice discussed in the past of adding SoC-specific variants for the IP blocks in the event that a SoC-specific driver change is needed in the future. I'm probably out-of-date on the current doctrine here, though.
Quoted 4 lines
Anyway, I have no problem with dropping the tegra132- variants, except in cases where changes are specifically known to be needed, although this seems likely to strengthen the binding between DT data and kernel versions. Let me know.
I agree we should have the tegra132- variants, even if we don't know how different the blocks happen to be from their predecessors. So please don't drop them.
It would just be nice to dump the strings in binding docs so as to not have checkpatch complaints.
Quoted 73 lines
Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 22
Quoted 5 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ device_type = "pci"; + reg = <0x0 0x01003000 0x0 0x00000800 /* PADS registers */ + 0x0 0x01003800 0x0 0x00000800 /* AFI registers */ + 0x0 0x02000000 0x0 0x10000000>; /* configuration space */Nit: for properties which are lists, please bracket each entry, e.g.
Quoted 8 lines
reg = <
0x00x010030000x00x00000800>, /* PADS registers */<0x0 0x01003800 0x0 0x00000800>, <0x0 0x02000000 0x0 0x10000000>;I appreciate the newlines serve as an obvious delimiter here, but in single-line cases it really helps, and it would be nice to be consistent. It looks like most instances in this file already do this anyway.
OK will do.
Quoted 13 lines
Quoted 13 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +12
-0+ reg-names = "pads", "afi", "cs"; + interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>, /* controller interrupt */ + <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>; /* MSI interrupt */ + interrupt-names = "intr", "msi"; + + #interrupt-cells = <1>; + interrupt-map-mask = <0 0 0 0>; + interrupt-map = <0 0 0 0 &gic GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>; + + bus-range = <0x00 0xff>; + #address-cells = <3>; + #size-cells = <2>;Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 40
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0+ + ranges = <0x82000000 0 0x01000000 0x0 0x01000000 0 0x00001000 /* port 0 configuration space */ + 0x82000000 0 0x01001000 0x0 0x01001000 0 0x00001000 /* port 1 configuration space */ + 0x81000000 0 0x0 0x0 0x12000000 0 0x00010000 /* downstream I/O (64 KiB) */ + 0x82000000 0 0x13000000 0x0 0x13000000 0 0x0d000000 /* non-prefetchable memory (208 MiB) */ + 0xc2000000 0 0x20000000 0x0 0x20000000 0 0x20000000>; /* prefetchable memory (512 MiB) */Same here w.r.t. list bracketing.
OK
Quoted 21 lines
Quoted 21 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +20
-0+ + clocks = <&tegra_car TEGRA124_CLK_PCIE>, + <&tegra_car TEGRA124_CLK_AFI>, + <&tegra_car TEGRA124_CLK_PLL_E>, + <&tegra_car TEGRA124_CLK_CML0>; + clock-names = "pex", "afi", "pll_e", "cml"; + resets = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 74>; + reset-names = "pex", "afi", "pcie_x"; + status = "disabled"; + + phys = <&xusb_padctl TEGRA_XUSB_PADCTL_PCIE>; + phy-names = "pcie"; + + pci at 1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x01000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + status = "disabled";Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 64
Quoted 4 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ + #address-cells = <3>; + #size-cells = <2>; + ranges;I'm not sure why these three properties are here. Surely this is a separate address space (so isn't ranges invalid?), and do we define any sub-nodes anywhere?
Quoted 4 lines
Hmm not sure. This was originally copied from
arch/arm/boot/dts/tegra124.dtsi. Will plan to drop them for now, and then if the properties (or ones like them) turn out to be needed in the future, someone else can deal with it :-)
That sounds sane to me.
Quoted 46 lines
Quoted 18 lines
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ + nvidia,num-lanes = <2>; + }; +arch/arm64/boot/dts/tegra/tegra132.dtsi line 78
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ pci at 2,0 { + device_type = "pci"; + assigned-addresses = <0x82001000 0 0x01001000 0 0x1000>; + reg = <0x001000 0 0 0 0>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges; +Likewise.
OK
Quoted 10 lines
Quoted 4 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +3
-0+ nvidia,num-lanes = <1>; + }; + };[...]
arch/arm64/boot/dts/tegra/tegra132.dtsi line 104
Quoted 2 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +2
-0+ host1x at 0,50000000 { + compatible = "nvidia,tegra132-host1x", "nvidia,tegra124-host1x", "simple-bus";Quoted 7 lines
Regarding simple-bus, are the sub-nodes usable if this didn't probe as "nvidia,tegra124-host1x" or "nvidia,tegra132-host1x"?
Do the subnodes require anything from this node?It looks like we expect/require the host1x node to be probed and initialised before we probe the children. Which would imply the simple-bus annotation is wrong.
Haven't tried booting with just simple-bus here. I believe these devices are accessible without the involvement of host1x. In other words, host1x is not a bus; I believe it might be most accurately described as a type of DMA controller. So in theory it should be possible for the CPU complex to read and write to these devices directly without the involvement of host1x, although I believe NVIDIA discourages this.
Quoted 7 lines
Under the theory that DT data should be hardware-specific, not software-specific, in an ideal world I think we would list these devices outside the embrace of the host1x node. However the existing Tegra124 DT file listed them together, and I am unsure whether that is required for the host1x code to function correctly.
Arto may be able to comment further.
Hmm, It would be good to hear from someone familar with that then. I'll wait for Arto.
[...]
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ ahb_gizmo: ahb-gizmo at 0,6000c004 { + compatible = "nvidia,tegra132-ahb", "nvidia,tegra124-ahb", "nvidia,tegra30-ahb"; + /* + * This IP block actually starts at 0x6000c000, + * but all of the register offsets in the driver + * have 0x4 subtracted from them. So handle + * it this way until the driver is fixed. + */ + reg = <0x0 0x6000c004 0x0 0x14d>; + };
Quoted 7 lines
Quoted 2 lines
That doesn't sound great. Can we not fix the driver and limit that mistake to existing DTBs?
We can do it that way if you think it's important to fix now. Have been trying to avoid gating our initial ARM64 support on fixing all of our hardware misalignment problems accumulated over the years, but if you think it's a blocker, it can be done.
I appreciate it's a bit of a pain, but once we allow it we're stuck with it forever, so I'd prefer that we fix these known binding issues first.
[...]
Quoted 33 lines
Quoted 33 lines
Quoted 13 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +13
-0+ apbmisc: apbmisc at 0,70000800 { + compatible = "nvidia,tegra132-apbmisc", "nvidia,tegra124-apbmisc", "nvidia,tegra20-apbmisc"; + reg = <0x0 0x70000800 0x0 0x64>, /* Chip revision */ + <0x0 0x7000E864 0x0 0x04>; /* Strapping options */ + }; + + pinmux: pinmux at 0,70000868 { + compatible = "nvidia,tegra132-pinmux", "nvidia,tegra124-pinmux"; + reg = <0x0 0x70000868 0x0 0x164>, /* Pad control registers */ + <0x0 0x70003000 0x0 0x434>; /* Mux registers */ + }; + + /*arch/arm64/boot/dts/tegra/tegra132.dtsi line 345
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ * There are two serial drivers: an 8250 based simple + * serial driver and an APB DMA based serial driver + * for higher baudrate and performance. To enable the + * 8250 based driver, the compatible string is + * "nvidia,tegra132-uart", "nvidia,tegra124-uart", + * "nvidia,tegra20-uart" and to enable the APB DMA + * based serial driver, the compatible string is + * "nvidia,tegra132-hsuart", "nvidia,tegra124-hsuart", + * "nvidia,tegra30-hsuart". + */Is there any reason to continue with this split?
Surely if the only difference is DMA, the presence of dmas and dma-names should be sufficient to get the driver to do the right thing, and if you need to disable DMA for debugging that could be a command-line option.
Does the binding and/or driver support aliases so you can get consistent numbering? It would be nice to have.
Quoted 7 lines
Quoted 4 lines
Do these UARTs work with earlycon?
It would be nice to have a /chosen/stdout-path (ideally with rate) so as to get output consistently without command line options being required.
Stephen Warren might be the best person to directly respond to these issues. This is all legacy DT data from previous Tegra SoCs.
Ok. I'll wait for something from Stephen.
[...]
Quoted 29 lines
Quoted 14 lines
Quoted 14 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +14
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x0>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>; + }; + + cpu at 1 { + device_type = "cpu";Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 965
Quoted 6 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +6
-0+ compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x1>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>; + }; + };It would be nice if this were near the top, as in other dts files.
OK will move.
Quoted 4 lines
It's a shame these share the same release address, though I guess that doesn't matter too much if PSCI is forthcoming.
Quoted 8 lines
Quoted 6 lines
I didn't spot a memory node or /memreserve/ entries. Is the memory used here for the spin-table guaranteed to be protected?
If the enable-method might change, it's probably best to leave this to the bootloader to fill in, or to have it in the board files if the boot loader isn't that clever (perhaps via a dtsi).
OK will drop the enable-method for the time being.
Ok.
Quoted 10 lines
Quoted 5 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 969
Quoted 3 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +3
-0+ thermal-zones { + /* XXX T132 CPU thermal zone - still TBD */ +What is still TBD here?
The T132 CPU thermal zone thermal sensor data. Specifically (and I hope this comes as no surprise), there's some power/thermal management data missing from this file that is intended to be added later. The placeholder is simply to indicate that.
So the other data is correct, but just incomplete?
I'm trying to get a feel for how this is likely to change and what that means for long-term compatibility. If what's here is sufficient but sub-optimal, that's fine.
Thanks,
Mark.
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
Quoted 3 lines · Mark Rutland
Quoted 2 lines · Paul Walmsley
Quoted 1 line · Mark Rutland
On Fri, Jan 16, 2015 at 11:45:29AM +0000, Paul Walmsley wrote:
[...]
Quoted 29 lines
Quoted 29 lines
Quoted 28 lines
Quoted 14 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +14
-0+ + clocks = <&tegra_car TEGRA124_CLK_PCIE>, + <&tegra_car TEGRA124_CLK_AFI>, + <&tegra_car TEGRA124_CLK_PLL_E>, + <&tegra_car TEGRA124_CLK_CML0>; + clock-names = "pex", "afi", "pll_e", "cml"; + resets = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 74>; + reset-names = "pex", "afi", "pcie_x"; + status = "disabled"; + + phys = <&xusb_padctl TEGRA_XUSB_PADCTL_PCIE>; + phy-names = "pcie";arch/arm64/boot/dts/tegra/tegra132.dtsi line 64
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ + pci at 1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x01000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + status = "disabled"; + + #address-cells = <3>; + #size-cells = <2>; + ranges;I'm not sure why these three properties are here. Surely this is a separate address space (so isn't ranges invalid?), and do we define any sub-nodes anywhere?
Quoted 6 lines
Quoted 4 lines
Hmm not sure. This was originally copied from
arch/arm/boot/dts/tegra124.dtsi. Will plan to drop them for now, and then if the properties (or ones like them) turn out to be needed in the future, someone else can deal with it :-)That sounds sane to me.
This follows the bindings defined almost two years ago. There was a lot of discussion back then and this is what we agreed upon. #address-cells and #size-cells are needed as per the PCI device tree bindings and the ranges property needed because the PCIe root ports translate addresses between the host bridge and the PCI endpoint devices.
Quoted 24 lines
Quoted 24 lines
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 104
Quoted 2 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +2
-0+ host1x at 0,50000000 { + compatible = "nvidia,tegra132-host1x", "nvidia,tegra124-host1x", "simple-bus";Regarding simple-bus, are the sub-nodes usable if this didn't probe as "nvidia,tegra124-host1x" or "nvidia,tegra132-host1x"?
Do the subnodes require anything from this node?It looks like we expect/require the host1x node to be probed and initialised before we probe the children. Which would imply the simple-bus annotation is wrong.
Haven't tried booting with just simple-bus here. I believe these devices are accessible without the involvement of host1x. In other words, host1x is not a bus; I believe it might be most accurately described as a type of DMA controller. So in theory it should be possible for the CPU complex to read and write to these devices directly without the involvement of host1x, although I believe NVIDIA discourages this.
Under the theory that DT data should be hardware-specific, not software-specific, in an ideal world I think we would list these devices outside the embrace of the host1x node. However the existing Tegra124 DT file listed them together, and I am unsure whether that is required for the host1x code to function correctly.
Quoted 4 lines
Quoted 1 line
Arto may be able to comment further.
Hmm, It would be good to hear from someone familar with that then. I'll wait for Arto.
We actually rely on the parent-child relationship in drivers, so we can't just go and reorganize things at will. This is documented in the existing binding for host1x, which again, was agreed upon a long time ago.
As for the simple-bus compatible, I think that was used way back to make sure of_platform_populate() gets called. I suppose we could drop it and call of_platform_populate() from the driver if its not there. The reason why we never considered cases where it would probe as simple-bus is that it was deemed unreasonable for a driver to bind to simple-bus.
Quoted 23 lines
Quoted 23 lines
Quoted 22 lines
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x0>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>;arch/arm64/boot/dts/tegra/tegra132.dtsi line 965
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ }; + + cpu at 1 { + device_type = "cpu"; + compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x1>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>; + }; + };It would be nice if this were near the top, as in other dts files.
Quoted 1 line
Quoted 1 line
OK will move.
Actually this follows the rules that we've been following with every DTS so far. Nodes with a unit address go first, sorted by unit address. They are followed by nodes without a unit address, sorted alphabetically.
I'd prefer keeping it this way for consistency across Tegra DTS files.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150123/d718b19e/attachment.sig>
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
Quoted 39 lines · Thierry Reding
Quoted 3 lines · Mark Rutland
Quoted 2 lines · Paul Walmsley
Quoted 1 line · Mark Rutland
On Fri, Jan 16, 2015 at 11:45:29AM +0000, Paul Walmsley wrote:
[...]
Quoted 34 lines
Quoted 33 lines
Quoted 20 lines
Quoted 20 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +20
-0+ + clocks = <&tegra_car TEGRA124_CLK_PCIE>, + <&tegra_car TEGRA124_CLK_AFI>, + <&tegra_car TEGRA124_CLK_PLL_E>, + <&tegra_car TEGRA124_CLK_CML0>; + clock-names = "pex", "afi", "pll_e", "cml"; + resets = <&tegra_car 70>, + <&tegra_car 72>, + <&tegra_car 74>; + reset-names = "pex", "afi", "pcie_x"; + status = "disabled"; + + phys = <&xusb_padctl TEGRA_XUSB_PADCTL_PCIE>; + phy-names = "pcie"; + + pci at 1,0 { + device_type = "pci"; + assigned-addresses = <0x82000800 0 0x01000000 0 0x1000>; + reg = <0x000800 0 0 0 0>; + status = "disabled";Quoted 8 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 64
Quoted 4 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +4
-0+ + #address-cells = <3>; + #size-cells = <2>; + ranges;I'm not sure why these three properties are here. Surely this is a separate address space (so isn't ranges invalid?), and do we define any sub-nodes anywhere?
Hmm not sure. This was originally copied from
arch/arm/boot/dts/tegra124.dtsi. Will plan to drop them for now, and then if the properties (or ones like them) turn out to be needed in the future, someone else can deal with it :-)
Quoted 7 lines
Quoted 1 line
That sounds sane to me.
This follows the bindings defined almost two years ago. There was a lot of discussion back then and this is what we agreed upon. #address-cells and #size-cells are needed as per the PCI device tree bindings and the ranges property needed because the PCIe root ports translate addresses between the host bridge and the PCI endpoint devices.
Ok. That sounds a little surprising to me, but I am admittedly not familiar with this binding and PCI more generally. I'll dig a bit deeper.
Quoted 34 lines
Quoted 24 lines
Quoted 24 lines
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi line 104
Quoted 2 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +2
-0+ host1x at 0,50000000 { + compatible = "nvidia,tegra132-host1x", "nvidia,tegra124-host1x", "simple-bus";Regarding simple-bus, are the sub-nodes usable if this didn't probe as "nvidia,tegra124-host1x" or "nvidia,tegra132-host1x"?
Do the subnodes require anything from this node?It looks like we expect/require the host1x node to be probed and initialised before we probe the children. Which would imply the simple-bus annotation is wrong.
Haven't tried booting with just simple-bus here. I believe these devices are accessible without the involvement of host1x. In other words, host1x is not a bus; I believe it might be most accurately described as a type of DMA controller. So in theory it should be possible for the CPU complex to read and write to these devices directly without the involvement of host1x, although I believe NVIDIA discourages this.
Under the theory that DT data should be hardware-specific, not software-specific, in an ideal world I think we would list these devices outside the embrace of the host1x node. However the existing Tegra124 DT file listed them together, and I am unsure whether that is required for the host1x code to function correctly.
Quoted 4 lines
Quoted 1 line
Arto may be able to comment further.
Hmm, It would be good to hear from someone familar with that then. I'll wait for Arto.
We actually rely on the parent-child relationship in drivers, so we can't just go and reorganize things at will. This is documented in the existing binding for host1x, which again, was agreed upon a long time ago.
Quoted 5 lines
As for the simple-bus compatible, I think that was used way back to make sure
of_platform_populate()gets called. I suppose we could drop it and callof_platform_populate()from the driver if its not there. The reason why we never considered cases where it would probe as simple-bus is that it was deemed unreasonable for a driver to bind to simple-bus.
Labelling something as simple-bus just to get an implicit of_platform_populate is an abuse of simple-bus. If you have a driver for handling the device as something more complex than a simple-bus, it absolutely must do that probing itself (because there could be some ordering requirement on re-initialisation of the bus).
If the sub-nodes don't make sense on their own, the "simple-bus" string is not appropriate regardless.
One thing I've been hoping/trying to do for a while (with little success so far) was to split simple-bus handling into a simple MMIO bus driver, which exposes and/or blows up in cases like this.
Quoted 23 lines
Quoted 23 lines
Quoted 23 lines
Quoted 22 lines
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x0>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>;arch/arm64/boot/dts/tegra/tegra132.dtsi line 965
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +10
-0+ }; + + cpu at 1 { + device_type = "cpu"; + compatible = "nvidia,denver", "arm,armv8"; + reg = <0x0 0x1>; + enable-method = "spin-table"; + cpu-release-addr = <0x0 0x80000008>; + }; + };It would be nice if this were near the top, as in other dts files.
Quoted 7 lines
Quoted 1 line
Quoted 1 line
OK will move.
Actually this follows the rules that we've been following with every DTS so far. Nodes with a unit address go first, sorted by unit address. They are followed by nodes without a unit address, sorted alphabetically.
I'd prefer keeping it this way for consistency across Tegra DTS files.
Ok.
I guess the ordering of this w.r.t. other nodes isn't too important. While I find it surprising, at least it shouldn't cause issues in the DTB itself.
However, it would be nice if we aligned all dts a bit better long-term.
Thanks,
Mark.
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
In reply to Paul WalmsleyHi all,
Quoted 41 lines · Paul Walmsley
+ Arto, Terje for comments on the host1x section
+ Stephen Warren for comments on the serial DT dataHi Mark,
thanks for the review.
Quoted 9 lines · Mark Rutland
As mentioned in my reply to the DT list patch [1], there are a couple of bits I'd like to see cleaned up first, but in the meantime I have some comments from my first pass of the dtsi below. Some of these may equally apply to existing dts(i) files.
I see a few undocumented compatible strings (at least when comparing against mainline). If there are other series or trees I should be looking at, any pointers would be appreciated. If not, documentation updates would be nice (checkpatch should complain otherwise).
(discussed in the tegra132-pcie comments below)
Quoted 12 lines · Paul Walmsley
arch/arm64/boot/dts/tegra/tegra132.dtsi line 104
Quoted 2 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +2
-0+ host1x at 0,50000000 { + compatible = "nvidia,tegra132-host1x", "nvidia,tegra124-host1x", "simple-bus";Regarding simple-bus, are the sub-nodes usable if this didn't probe as "nvidia,tegra124-host1x" or "nvidia,tegra132-host1x"?
Do the subnodes require anything from this node?It looks like we expect/require the host1x node to be probed and initialised before we probe the children. Which would imply the simple-bus annotation is wrong.
Haven't tried booting with just simple-bus here. I believe these devices are accessible without the involvement of host1x. In other words, host1x is not a bus; I believe it might be most accurately described as a type of DMA controller. So in theory it should be possible for the CPU complex to read and write to these devices directly without the involvement of host1x, although I believe NVIDIA discourages this.
Quoted 7 lines
Under the theory that DT data should be hardware-specific, not software-specific, in an ideal world I think we would list these devices outside the embrace of the host1x node. However the existing Tegra124 DT file listed them together, and I am unsure whether that is required for the host1x code to function correctly.
Arto may be able to comment further.
Placing the devices behind host1x is an accurate description of
hardware: Despite the direct register access path (writel/readl targeting a host1x client device) is transparent to the CPU, the host1x hardware is internally handling the requests and passing them forward to the host1x client in interest.
Above implies also a strict parent-child dependency on hardware level:
If the CPU tries to access a register in a host1x client before the host1x clock has been enabled, host1x will not be able to forward the requests and the access will fail. This also defines the importance of probe order (i.e. host1x must be initialized before client devices).
In addition, the host1x indirect register programming path ("channel path") is operational only for the host1x client devices and our current host1x driver relies on parent-child relationship in device tree.
- Arto
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
Quoted 58 lines · Arto Merilainen
Hi all,
Quoted 41 lines · Paul Walmsley
+ Arto, Terje for comments on the host1x section
+ Stephen Warren for comments on the serial DT dataHi Mark,
thanks for the review.
Quoted 9 lines · Mark Rutland
As mentioned in my reply to the DT list patch [1], there are a couple of bits I'd like to see cleaned up first, but in the meantime I have some comments from my first pass of the dtsi below. Some of these may equally apply to existing dts(i) files.
I see a few undocumented compatible strings (at least when comparing against mainline). If there are other series or trees I should be looking at, any pointers would be appreciated. If not, documentation updates would be nice (checkpatch should complain otherwise).
(discussed in the tegra132-pcie comments below)
Quoted 12 lines · Paul Walmsley
arch/arm64/boot/dts/tegra/tegra132.dtsi line 104
Quoted 2 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi new file +2
-0+ host1x at 0,50000000 { + compatible = "nvidia,tegra132-host1x", "nvidia,tegra124-host1x", "simple-bus";Regarding simple-bus, are the sub-nodes usable if this didn't probe as "nvidia,tegra124-host1x" or "nvidia,tegra132-host1x"?
Do the subnodes require anything from this node?It looks like we expect/require the host1x node to be probed and initialised before we probe the children. Which would imply the simple-bus annotation is wrong.
Haven't tried booting with just simple-bus here. I believe these devices are accessible without the involvement of host1x. In other words, host1x is not a bus; I believe it might be most accurately described as a type of DMA controller. So in theory it should be possible for the CPU complex to read and write to these devices directly without the involvement of host1x, although I believe NVIDIA discourages this.
Quoted 7 lines
Under the theory that DT data should be hardware-specific, not software-specific, in an ideal world I think we would list these devices outside the embrace of the host1x node. However the existing Tegra124 DT file listed them together, and I am unsure whether that is required for the host1x code to function correctly.
Arto may be able to comment further.
Placing the devices behind host1x is an accurate description of
hardware: Despite the direct register access path (writel/readl targeting a host1x client device) is transparent to the CPU, the host1x hardware is internally handling the requests and passing them forward to the host1x client in interest.
Quoted 5 lines
Above implies also a strict parent-child dependency on hardware level:
If the CPU tries to access a register in a host1x client before the host1x clock has been enabled, host1x will not be able to forward the requests and the access will fail. This also defines the importance of probe order (i.e. host1x must be initialized before client devices).
In that case, it sounds like the "simple-bus" string is bogus unless the host1x is at minimum pre-initialised by some firmware and/or bootloader into a state where the child devices could theoretically be used on their own.
So I think "simple-bus" should be dropped here.
Quoted 3 lines
In addition, the host1x indirect register programming path ("channel path") is operational only for the host1x client devices and our current host1x driver relies on parent-child relationship in device tree.
So I guess this is more Linux-specific, but still adds to the fuel for removing "simple-bus" here.
Thanks,
Mark.
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
In reply to Paul WalmsleyQuoted 46 lines · Paul Walmsley
+ Arto, Terje for comments on the host1x section
+ Stephen Warren for comments on the serial DT dataQuoted 41 lines · Mark Rutland
As mentioned in my reply to the DT list patch [1], there are a couple of bits I'd like to see cleaned up first, but in the meantime I have some comments from my first pass of the dtsi below. Some of these may equally apply to existing dts(i) files.
I see a few undocumented compatible strings (at least when comparing against mainline). If there are other series or trees I should be looking at, any pointers would be appreciated. If not, documentation updates would be nice (checkpatch should complain otherwise).
Quoted 14 lines · Paul Walmsley
Add an initial device tree file for the Tegra132 SoC. The DT file is based on
arch/arm/boot/dts/tegra124.dtsiandarch/arm/boot/dts/tegra114.dtsi, with the following significant changes:- Tegra132 uses a Denver CPU cluster rather than an ARMv7 CPU cluster
- Devices are arranged by bus, rather than in a flat topology
- No polling delays have been defined for the thermal zones. I don't believe that this is a property of the SoC hardware, but rather of a given use-case.arch/arm64/boot/dts/tegra/tegra132.dtsi +1
-0+ /*arch/arm64/boot/dts/tegra/tegra132.dtsi line 345
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi +10
-0+ * There are two serial drivers: an 8250 based simple + * serial driver and an APB DMA based serial driver + * for higher baudrate and performance. To enable the + * 8250 based driver, the compatible string is + * "nvidia,tegra132-uart", "nvidia,tegra124-uart", + * "nvidia,tegra20-uart" and to enable the APB DMA + * based serial driver, the compatible string is + * "nvidia,tegra132-hsuart", "nvidia,tegra124-hsuart", + * "nvidia,tegra30-hsuart". + */Is there any reason to continue with this split?
Surely if the only difference is DMA, the presence of dmas and dma-names should be sufficient to get the driver to do the right thing, and if you need to disable DMA for debugging that could be a command-line option.
I vaguely recall asking for the DMA support to be integrated into the regular 8250 driver instead when the separate DMA-capable driver was first added. I /think/ there was resistance to this because adding lots of different SoC-specific ways to do DMA into the existing 8250 driver would complicate it even more, and hence be unmaintainable.
I assume that reasoning is still valid.
Perhaps what we need is more fine-grained driver selection, not just based on compatible value. Something like:
if compatible == nvidia,tegra20-uart:
if node.has_prop("enable-dma"):
driver = Tegra-specfic DMA capable UART driver
else:
driver = Common 8250 driver
Is that something that of_serial.c could/should be enhanced to do? That said, the whole reasoning behind separate compatible properties before was that compatible is supposed to "drive" driver selection. At least, that's what I was told then.
Quoted 2 lines
Quoted 2 lines
Does the binding and/or driver support aliases so you can get consistent numbering? It would be nice to have.
I believe either the serial core or serial driver(s) support DT aliases now, yes.
Quoted 1 line
Quoted 1 line
Do these UARTs work with earlycon?
I do not know.
Quoted 19 lines
Quoted 2 lines
It would be nice to have a /chosen/stdout-path (ideally with rate) so as to get output consistently without command line options being required.
Stephen Warren might be the best person to directly respond to these issues. This is all legacy DT data from previous Tegra SoCs.
Quoted 13 lines
Quoted 13 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi +12
-0+ uarta: serial at 0,70006000 { + compatible = "nvidia,tegra132-uart", "nvidia,tegra124-uart", "nvidia,tegra20-uart"; + reg = <0x0 0x70006000 0x0 0x40>; + reg-shift = <2>; + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_UARTA>; + resets = <&tegra_car 6>; + reset-names = "serial"; + dmas = <&apbdma 8>, <&apbdma 8>; + dma-names = "rx", "tx"; + status = "disabled"; + };
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
Quoted 56 lines · Stephen Warren
Quoted 46 lines · Paul Walmsley
+ Arto, Terje for comments on the host1x section
+ Stephen Warren for comments on the serial DT dataQuoted 41 lines · Mark Rutland
As mentioned in my reply to the DT list patch [1], there are a couple of bits I'd like to see cleaned up first, but in the meantime I have some comments from my first pass of the dtsi below. Some of these may equally apply to existing dts(i) files.
I see a few undocumented compatible strings (at least when comparing against mainline). If there are other series or trees I should be looking at, any pointers would be appreciated. If not, documentation updates would be nice (checkpatch should complain otherwise).
Quoted 14 lines · Paul Walmsley
Add an initial device tree file for the Tegra132 SoC. The DT file is based on
arch/arm/boot/dts/tegra124.dtsiandarch/arm/boot/dts/tegra114.dtsi, with the following significant changes:- Tegra132 uses a Denver CPU cluster rather than an ARMv7 CPU cluster
- Devices are arranged by bus, rather than in a flat topology
- No polling delays have been defined for the thermal zones. I don't believe that this is a property of the SoC hardware, but rather of a given use-case.arch/arm64/boot/dts/tegra/tegra132.dtsi +1
-0+ /*arch/arm64/boot/dts/tegra/tegra132.dtsi line 345
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi +10
-0+ * There are two serial drivers: an 8250 based simple + * serial driver and an APB DMA based serial driver + * for higher baudrate and performance. To enable the + * 8250 based driver, the compatible string is + * "nvidia,tegra132-uart", "nvidia,tegra124-uart", + * "nvidia,tegra20-uart" and to enable the APB DMA + * based serial driver, the compatible string is + * "nvidia,tegra132-hsuart", "nvidia,tegra124-hsuart", + * "nvidia,tegra30-hsuart". + */Is there any reason to continue with this split?
Surely if the only difference is DMA, the presence of dmas and dma-names should be sufficient to get the driver to do the right thing, and if you need to disable DMA for debugging that could be a command-line option.
I vaguely recall asking for the DMA support to be integrated into the regular 8250 driver instead when the separate DMA-capable driver was first added. I /think/ there was resistance to this because adding lots of different SoC-specific ways to do DMA into the existing 8250 driver would complicate it even more, and hence be unmaintainable.
I assume that reasoning is still valid.
Quoted 8 lines
Perhaps what we need is more fine-grained driver selection, not just based on compatible value. Something like:
if compatible == nvidia,tegra20-uart:
ifnode.has_prop("enable-dma"):
driver = Tegra-specfic DMA capable UART driver
else:
driver = Common 8250 driver
Surely we should assume that if the DMAs are listed (and not disabled) they are usable? I'm not sure I see the point in an additional property to force Linux-internal driver selection in this way.
What happens if we have a driver for a particular string, but fail to probe? Can we fall back to a more generic driver?
Quoted 4 lines
Is that something that
of_serial.ccould/should be enhanced to do? That said, the whole reasoning behind separate compatible properties before was that compatible is supposed to "drive" driver selection. At least, that's what I was told then.
The compatible string is meant to describe the programming interface the device is compatible with. The kernel should then choose the most appropriate driver it knows can handle that HW programming interface.
Considering it as a way to choose the driver is backwards. I'd expect the device node to have both strings (with the DMA capable string first) unless it really doesn't have any DMAs.
Thanks,
Mark.
PATCH 1/1 arm64: dts: Add initial device tree support for Tegra132
Quoted 60 lines · Mark Rutland
Quoted 59 lines · Stephen Warren
Quoted 46 lines · Paul Walmsley
+ Arto, Terje for comments on the host1x section
+ Stephen Warren for comments on the serial DT dataQuoted 41 lines · Mark Rutland
As mentioned in my reply to the DT list patch [1], there are a couple of bits I'd like to see cleaned up first, but in the meantime I have some comments from my first pass of the dtsi below. Some of these may equally apply to existing dts(i) files.
I see a few undocumented compatible strings (at least when comparing against mainline). If there are other series or trees I should be looking at, any pointers would be appreciated. If not, documentation updates would be nice (checkpatch should complain otherwise).
Quoted 14 lines · Paul Walmsley
Add an initial device tree file for the Tegra132 SoC. The DT file is based on
arch/arm/boot/dts/tegra124.dtsiandarch/arm/boot/dts/tegra114.dtsi, with the following significant changes:- Tegra132 uses a Denver CPU cluster rather than an ARMv7 CPU cluster
- Devices are arranged by bus, rather than in a flat topology
- No polling delays have been defined for the thermal zones. I don't believe that this is a property of the SoC hardware, but rather of a given use-case.arch/arm64/boot/dts/tegra/tegra132.dtsi +1
-0+ /*arch/arm64/boot/dts/tegra/tegra132.dtsi line 345
Quoted 10 lines
arch/arm64/boot/dts/tegra/tegra132.dtsi +10
-0+ * There are two serial drivers: an 8250 based simple + * serial driver and an APB DMA based serial driver + * for higher baudrate and performance. To enable the + * 8250 based driver, the compatible string is + * "nvidia,tegra132-uart", "nvidia,tegra124-uart", + * "nvidia,tegra20-uart" and to enable the APB DMA + * based serial driver, the compatible string is + * "nvidia,tegra132-hsuart", "nvidia,tegra124-hsuart", + * "nvidia,tegra30-hsuart". + */Is there any reason to continue with this split?
Surely if the only difference is DMA, the presence of dmas and dma-names should be sufficient to get the driver to do the right thing, and if you need to disable DMA for debugging that could be a command-line option.
I vaguely recall asking for the DMA support to be integrated into the regular 8250 driver instead when the separate DMA-capable driver was first added. I /think/ there was resistance to this because adding lots of different SoC-specific ways to do DMA into the existing 8250 driver would complicate it even more, and hence be unmaintainable.
I assume that reasoning is still valid.
Perhaps what we need is more fine-grained driver selection, not just based on compatible value. Something like:
Quoted 8 lines
Quoted 5 lines
if compatible == nvidia,tegra20-uart:
ifnode.has_prop("enable-dma"):
driver = Tegra-specfic DMA capable UART driver
else:
driver = Common 8250 driverSurely we should assume that if the DMAs are listed (and not disabled) they are usable?
I would assume so, yes.
Quoted 2 lines
I'm not sure I see the point in an additional property to force Linux-internal driver selection in this way.
I think it comes down to whether you want to use DMA for that UART or not. I don't recall the exact reason why this might be important. Perhaps it's one/more of:
- The Tegra-specific DMA-capable driver can't be used for early debug but the core 8250 driver can. I don't know if this is true though.
- There are a limited number of DMA channels on Tegra. If all ports blindly used DMA simply because the dmas property was included, then we'd risk running out of DMA channels for cases where it absolutely had to be used; e.g. a low-volume debug console might prevent a high-volume MODEM or BT UART from using DMA if they probed in the "wrong" order.
Laxman might be able to shed more light on this.
Quoted 2 lines
What happens if we have a driver for a particular string, but fail to probe? Can we fall back to a more generic driver?
At least in this case, I'd assume so, yes.
Quoted 5 lines
Quoted 4 lines
Is that something that
of_serial.ccould/should be enhanced to do? That said, the whole reasoning behind separate compatible properties before was that compatible is supposed to "drive" driver selection. At least, that's what I was told then.
Quoted 7 lines
The compatible string is meant to describe the programming interface the device is compatible with. The kernel should then choose the most appropriate driver it knows can handle that HW programming interface.
Considering it as a way to choose the driver is backwards. I'd expect the device node to have both strings (with the DMA capable string first) unless it really doesn't have any DMAs.
That's a subtly different interpretation of DT compatible usage than was given (I think it was by Mitch Bradley???) when this driver was introduced. In the conversation then, it was asserted that using different compatible values to describe that SW should use a different interface to the HW (non-DMA vs. DMA) was exactly in line with the semantic concept of compatible.