Hi Yu,
On Mon, Aug 01, 2022 at 01:50:36AM -0600, Yu Zhao via openwrt-devel wrote:
> This patch adds the DTS [8] from the stock firmware [2], and makes
> ER-8 use it. Interestingly, the only change in the device tree after
> this patch is the eMMC clock speed [9].
This usually means it'd be a better idea to share the DTS source
(probably as a DTSI) and only override a single property.
I haven't built a full octeon image yet nor grokked this target's
Makefile device definitions; what DTS are you replacing, anyway? Seems
very likely that you're repeating too much. At a minimum, I bet a large
chunk of this file can be replaced with just:
#include "cn71xx.dtsi"
> # dtc -I fs -O dts /proc/device-tree/ -o ubnt_e200.dts
That's cute, but that doesn't quite produce a proper (in terms of
readability and usability) source file. Source files have things like
labels and reference/pandle syntax, instead of bare 'phandle' properties
with indexes.
A few notes to that end below, as well as other tips on how these kinds
of files are typically written. You might also consider reading through
a few other files (e.g., in the arch/mips/boot/dts/cavium-octeon
directory) to see what they look like.
> --- /dev/null
> +++ b/target/linux/octeon/files/arch/mips/boot/dts/cavium-octeon/ubnt_e200.dts
> @@ -0,0 +1,388 @@
> +/dts-v1/;
> +
> +/ {
> + compatible = "ubnt,e200";
> + interrupt-parent = <0x01>;
This (and any other interrupt-parent) should be a <&phandle> reference,
not a bare integer. e.g.:
interrupt-parent = <&ciu>;
> + model = "ubnt,e200";
> + #address-cells = <0x02>;
> + #size-cells = <0x02>;
#{address,size}-cells are almost always spelled out in decimal, not hex.
#address-cells = <2>;
#size-cells = <2>;
(Same for pretty much anything else that's not an address or length.)
> + memory {
> + reg = <0x00 0x00 0x00 0x10000000 0x00 0x20000000 0x00
> 0x70000000>;
> + device_type = "memory";
> + };
> +
> + soc@0 {
> + compatible = "simple-bus";
> + ranges;
> + #address-cells = <0x02>;
> + #size-cells = <0x02>;
> +
> + dma-engine@1180000000108 {
> + interrupts = <0x00 0x3f>;
> + compatible = "cavium,octeon-5750-bootbus-dma";
> + reg = <0x11800 0x108 0x00 0x08>;
> + };
> +
> + interrupt-controller@1070000000000 {
If you give this a proper label like all other cavium DTs do, then you
could refer to it by name. e.g.:
/ {
...
interrupt-parent = <&ciu>;
...
ciu: interrupt-controller@1070000000000 {
> + interrupt-controller;
> + compatible = "cavium,octeon-3860-ciu";
> + reg = <0x10700 0x00 0x00 0x7000>;
> + #interrupt-cells = <0x02>;
> + phandle = <0x01>;
> + linux,phandle = <0x01>;
Drop the 'phandle' and 'linux,phandle' properties. Any time you see
these, that means the source should *really* have a label instead, and
references to that label should be made using &your_label_name.
> + };
...
> + interface@1 {
> + compatible = "cavium,octeon-3860-pip-interface";
> + reg = <0x01>;
> + #address-cells = <0x01>;
> + #size-cells = <0x00>;
> +
> + ethernet@0 {
> + compatible =
> "cavium,octeon-3860-pip-port";
> + phy-handle = <0x09>;
> + reg = <0x00>;
When the 'reg' is just an index (like a "port" number in this case),
it probably makes more sense in decimal.
> + local-mac-address = [00 00 00 00 00 00];
> + };
...
> + mmc@1180000002000 {
> + interrupts = <0x01 0x13 0x00 0x3f>;
> + compatible = "cavium,octeon-6130-mmc";
> + reg = <0x11800 0x2000 0x00 0x100 0x11800 0x168 0x00
> 0x20>;
> + #address-cells = <0x01>;
> + #size-cells = <0x00>;
> +
> + mmc-slot@0 {
> + cavium,bus-max-width = <0x08>;
> + compatible =
> "cavium,octeon-6130-mmc-slot\0mmc-spi-slot";
Instead of including 'nil' characters in the midst of a string, try the
common syntax:
compatible = "cavium,octeon-6130-mmc-slot",
"mmc-spi-slot";
> + voltage-ranges = <0xce4 0xce4>;
These are much better spelled out in decimal:
voltage-ranges = <3300 3300>;
> + reg = <0x00>;
> + spi-max-frequency = <0x18cba80>;
Decimal.
> + };
> + };
> +
> + bootbus@1180000000000 {
> + compatible = "cavium,octeon-3860-bootbus";
> + reg = <0x11800 0x00 0x00 0x200>;
> + ranges = <0x00 0x00 0x00 0x1f400000 0xc00000 0x01 0x00
> 0x10000 0x30000000 0x00 0x02 0x00 0x00 0x1d040000 0x10000 0x03 0x00 0x00
> 0x1d050000 0x10000 0x04 0x00 0x00 0x1d020000 0x10000 0x05 0x00 0x10000
> 0x40000000 0x00 0x06 0x00 0x10000 0x50000000 0x00 0x07 0x00 0x10000
> 0x90000000 0x00>;
I think this works better if you wrap it :) And probably give it some
reasonable formatting/alignment, so each range actually lines up.
...
> + aliases {
> + smi1 = "/soc@0/mdio@1180000001900";
That's not how aliases are normally written. You probably want
something like:
smi = &smi1;
and earlier:
smi1: mdio@1180000001900 {
...
Same for all the other aliases.
Brian
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
