On Fri, 2016-07-22 at 11:05 +0530, Bhaskar Upadhaya wrote:
> diff --git a/arch/arm64/boot/dts/freescale/Makefile
> b/arch/arm64/boot/dts/freescale/Makefile
> index 1b7783d..4aa3bee 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -3,6 +3,9 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1043a-rdb.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2080a-qds.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2080a-rdb.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2080a-simu.dtb
> +dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-qds.dtb
> +dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-rdb.dtb
> +dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1012a-frdm.dtb
Please keep such lists sorted.
> +&i2c0 {
> + status = "okay";
> +
> + codec: sgtl5000@a {
> + #sound-dai-cells = <0>;
> + compatible = "fsl,sgtl5000";
> + reg = <0xa>;
> + VDDA-supply = <®_1p8v>;
> + VDDIO-supply = <®_1p8v>;
> + clocks = <&sys_mclk 1>;
sys_mclk is a fixed-clock, with #clock-cells = <0>. What is the "1" for?
> + ethernet@1 {
> + compatible = "fsl,pfe-gemac-port";
Binding?
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = < 0x1 >; /* GEM_ID */
> + fsl,gemac-bus-id = < 0x1 >; /* BUS_ID */
> + fsl,gemac-phy-id = < 0x1 >; /* PHY_ID */
> + fsl,mdio-mux-val = <0x0>;
No spaces inside <>
> + local-mac-address = [ 00 AA BB CC DD EE ];
No.
> + regulators {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <0>;
simple-bus does not make sense with #size-cells = <0>. It's for memory-mapped
devices.
> +};
> +&ftm0 {
Leave a blank line between nodes.
> +/ {
> + compatible = "fsl,ls1012a";
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + /*
> + * We expect the enable-method for cpu's to be "psci", but
> this
> + * is dependent on the SoC FW, which will fill this in.
> + *
> + * Currently supported enable-method is psci v0.2
> + */
Why do you expect any enable-method on a chip with only one CPU?
> + sysclk: sysclk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <100000000>;
> + clock-output-names = "sysclk";
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <1 13 0x1>, /* Physical Secure PPI */
> + <1 14 0x1>, /* Physical Non-Secure PPI */
> + <1 11 0x1>, /* Virtual PPI */
> + <1 10 0x1>; /* Hypervisor PPI */
> + arm,reread-timer;
> + };
arm,reread-timer does not belong here.
Please don't blindly copy and paste things, either from one chip to another or
from old deprecated internal stuff.
> +
> + clockgen: clocking@1ee1000 {
> + compatible = "fsl,ls1012a-clockgen","fsl,ls1043a-
> clockgen";
> + reg = <0x0 0x1ee1000 0x0 0x1000>;
> + #clock-cells = <2>;
> + clocks = <&sysclk>;
> + };
clockgen nodes should not claim compatibility with another SoC. The clocking
options on ls1012a are not the same as on ls1043a.
> +
> + scfg: scfg@1570000 {
> + compatible = "fsl,ls1012a-scfg", "fsl,ls1043a-
> scfg", "syscon";
> + reg = <0x0 0x1570000 0x0 0x10000>;
> + big-endian;
> + };
The SCFG on ls1021a is not compatible with ls1043a.
> + reset: reset@1EE00B0 {
> + compatible = "fsl,ls-reset";
> + reg = <0x0 0x1EE00B0 0x0 0x4>;
> + big-endian;
> + };
This was an old internal hack that doesn't belong here.
> +
> + rcpm: rcpm@1ee2000 {
> + compatible = "fsl,ls1012a-rcpm", "fsl,ls1043a-
> rcpm", "fsl,qoriq-rcpm-2.1";
> + reg = <0x0 0x1ee2000 0x0 0x10000>;
The RCPM on ls1043a has several registers that ls1012a does not have. They
are not compatible.
"rcpm-2.1" is probably not appropriate either.
> + };
> +
> + ftm0: ftm0@29d0000 {
> + compatible = "fsl,ftm-alarm";
> + reg = <0x0 0x29d0000 0x0 0x10000>;
> + interrupts = <0 86 0x4>;
> + big-endian;
> + rcpm-wakeup = <&rcpm 0x00020000 0x0>;
> + status = "disabled";
> + };
> +
> + esdhc0: esdhc@1560000 {
Whitespace
> + tmu: tmu@1f00000 {
> + compatible = "fsl,qoriq-tmu", "fsl,ls1012a-tmu";
More specific compatibles come first.
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + pfe_reserved: packetbuffer@83400000 {
> + reg = <0 0x83400000 0 0xc00000>;
> + };
> + };
Could you explain this reservation?
> +
> + pfe: pfe@04000000 {
> + compatible = "fsl,pfe";
> + ranges = <0x0 0x00 0x04000000 0xc00000
> + 0x1 0x00 0x83400000 0xc00000>;
> + reg = <0x0 0x90500000 0x0 0x10000>, /* APB 64K */
> + <0x0 0x04000000 0x0 0xc00000>, /* AXI 16M */
> + <0x0 0x83400000 0x0 0xc00000>, /* PFE DDR 12M */
> + <0x0 0x10000000 0x0 0x2000>; /* OCRAM 8K */
> + fsl,pfe-num-interfaces = < 0x2 >;
> + interrupts = <0 172 0x4>;
> + #interrupt-names = "hifirq";
> + memory-region = <&pfe_reserved>;
> + fsl,pfe-scfg = <&scfg 0>;
Binding?
> + };
> +
> +};
Don't put a blank line between these.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
