On Mon, Dec 28, 2020 at 02:16:29PM +0100, Adrian Schmutzler wrote: > Hi, > > some additional (general) remarks below. > > > -----Original Message----- > > From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org] > > On Behalf Of Rafaël Carré > > Sent: Montag, 28. Dezember 2020 01:25 > > To: openwrt-devel@lists.openwrt.org > > Cc: Rafaël Carré <fun...@videolan.org> > > Subject: [RFC PATCH v2 1/3] ramips: add support for the RT6855A SoC > > > > Add Linksys WAP300N target > > > > Signed-off-by: Rafaël Carré <fun...@videolan.org> > > --- > > Changes since v1: > > - Use OpenWrt .dts (CONFIG_MIPS_RAW_APPENDED_DTB) > > - clean up rt6855a.mk > > - add case/esac in /etc/board.d/* > > - tidy up config-5.4 > > > > target/linux/ramips/Makefile | 2 +- > > target/linux/ramips/dts/rt6855a.dtsi | 190 ++++++++++ > > .../ramips/dts/rt6855a_linksys_wap300n.dts | 22 ++ > > target/linux/ramips/image/Makefile | 1 + > > target/linux/ramips/image/rt6855a.mk | 12 + > > .../rt6855a/base-files/etc/board.d/01_leds | 17 + > > .../rt6855a/base-files/etc/board.d/02_network | 17 + > > .../base-files/etc/board.d/03_gpio_switches | 16 + > > target/linux/ramips/rt6855a/config-5.4 | 344 ++++++++++++++++++ > > .../ramips/rt6855a/profiles/00-default.mk | 17 + > > target/linux/ramips/rt6855a/target.mk | 15 + > > 11 files changed, 652 insertions(+), 1 deletion(-) create mode 100644 > > target/linux/ramips/dts/rt6855a.dtsi > > create mode 100644 target/linux/ramips/dts/rt6855a_linksys_wap300n.dts > > create mode 100644 target/linux/ramips/image/rt6855a.mk > > create mode 100755 target/linux/ramips/rt6855a/base- > > files/etc/board.d/01_leds > > create mode 100755 target/linux/ramips/rt6855a/base- > > files/etc/board.d/02_network > > create mode 100755 target/linux/ramips/rt6855a/base- > > files/etc/board.d/03_gpio_switches > > create mode 100644 target/linux/ramips/rt6855a/config-5.4 > > create mode 100644 target/linux/ramips/rt6855a/profiles/00-default.mk > > create mode 100644 target/linux/ramips/rt6855a/target.mk > > > > diff --git a/target/linux/ramips/Makefile b/target/linux/ramips/Makefile > > index c3d118b2ab..f03118c1aa 100644 > > --- a/target/linux/ramips/Makefile > > +++ b/target/linux/ramips/Makefile > > @@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk ARCH:=mipsel > > BOARD:=ramips BOARDNAME:=MediaTek Ralink MIPS > > -SUBTARGETS:=mt7620 mt7621 mt76x8 rt288x rt305x rt3883 > > +SUBTARGETS:=mt7620 mt7621 mt76x8 rt288x rt305x rt3883 rt6855a > > FEATURES:=squashfs gpio > > > > KERNEL_PATCHVER:=5.4 > > diff --git a/target/linux/ramips/dts/rt6855a.dtsi > > b/target/linux/ramips/dts/rt6855a.dtsi > > new file mode 100644 > > index 0000000000..76cd3da568 > > --- /dev/null > > +++ b/target/linux/ramips/dts/rt6855a.dtsi > > @@ -0,0 +1,190 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > Please add /dts-v1/; here, with empty line before and after. > > > +/ { > > + #address-cells = <1>; > > + #size-cells = <1>; > > + compatible = "ralink,rt6855a-soc"; > > + > > + cpus { > > + cpu@0 { > > + compatible = "mips,mips34Kc"; > > + }; > > + }; > > + > > + cpuintc: cpuintc { > > + #address-cells = <0>; > > + #interrupt-cells = <1>; > > + interrupt-controller; > > + compatible = "mti,cpu-interrupt-controller"; > > + }; > > + > > + palmbus@1fb00000 { > > + compatible = "palmbus"; > > + reg = <0x1fb00000 0xe0000>; > > + ranges = <0x0 0x1fb00000 0x100000>; > > + > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + sysc@800 { > > + compatible = "ralink,rt6855a-sysc"; > > + reg = <0x800 0x100>; > > + }; > > + > > + intc: intc@40000 { > > + compatible = "ralink,rt6855a-intc"; > > + reg = <0x40000 0x100>; > > + > > + interrupt-controller; > > + #interrupt-cells = <1>; > > + > > + interrupt-parent = <&cpuintc>; > > + }; > > + > > + memc@300 { > > + compatible = "ralink,rt6855a-memc", "ralink,rt3050- > > memc"; > > + reg = <0x300 0x100>; > > + }; > > + > > + watchdog@f0100 { > > + compatible = "ralink,rt6855a-wdt"; > > + reg = <0xf0100 0x10>; > > + }; > > + > > + uart: uart@f0000 { > > + compatible = "ns8250"; > > + reg = <0xf0000 0x30>; > > + interrupts = <0>; > > + > > + clock-frequency = <921600>; > > + > > + reg-io-width = <4>; > > + reg-shift = <2>; > > + no-loopback-test; > > + > > + status = "okay"; > > + > > + interrupt-parent = <&intc>; > > + }; > > + > > + gdma: gdma@30000 { > > + compatible = "ralink,gdma-rt2880"; > > + reg = <0x30000 0x100>; > > + }; > > + > > + ethernet: ethernet@50000{ > > + compatible = "ralink,rt6855a-eth"; > > + reg = <0x50000 0x10000>; > > + > > + interrupt-parent = <&intc>; > > + interrupts = <21>; > > + > > + mediatek,switch = <&esw>; > > + mtd-mac-address = <&factory 0xe000>; > > + }; > > Please make sure indent is all tabs. > > > + > > + esw: esw@60000 { > > + compatible = "ralink,rt3050-esw"; > > + reg = <0x60000 0x8000>; > > + > > + interrupt-parent = <&intc>; > > + interrupts = <15>; > > + }; > > + > > + spi0: spi@c0b00 { > > + status = "disabled"; > > + > > + compatible = "ralink,mt7621-spi"; > > + reg = <0xc0b00 0x100>; > > + > > + //clocks = <&pll MT7621_CLK_BUS>; > > + > > + //resets = <&rstctrl 18>; > > + //reset-names = "spi"; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + //pinctrl-names = "default"; > > + //pinctrl-0 = <&spi_pins>; > > + }; > > + }; > > + > > + pcie: pcie@1fb80000 { > > + compatible = "ralink,rt3883-pci"; > > + reg = <0x1fb80000 0x20000>; > > + #address-cells = <3>; > > + #size-cells = <2>; > > + > > + #interrupt-cells = <1>; > > + > > + device_type = "pci"; > > + interrupt-map-mask = <0x0 0 0 0>; > > + interrupt-map = <0x0 0 0 0 &pciintc 20>; > > + > > + ranges = < > > + 0x02000000 0 0x20000000 0x20000000 0 0x10000000 /* pci memory > > */ > > + 0x01000000 0 0x1f600000 0x1f600000 0 0x00010000 /* io space */ > > + >; > > + > > + bus-range = <0 255>; > > + status = "disabled"; > > + > > + pciintc: interrupt-controller { > > + interrupt-controller; > > + #address-cells = <0>; > > + #interrupt-cells = <1>; > > + > > + interrupt-parent = <&intc>; > > + interrupts = <24>; > > + }; > > + > > + }; > > + > > Remove useless empty lines between closing brackets. > > > +}; > > + > > +&spi0 { > > + status = "okay"; > > + > > + flash@0 { > > + compatible = "jedec,spi-nor"; > > + reg = <0>; > > + spi-max-frequency = <10000000>; > > + > > + partitions { > > + compatible = "fixed-partitions"; > > + #address-cells = <1>; > > + #size-cells = <1>; > > + > > + partition@0 { > > + label = "Bootloader"; > > + reg = <0x0 0x30000>; > > + read-only; > > + }; > > + > > + partition@30000 { > > + label = "Config"; > > + reg = <0x30000 0x10000>; > > + read-only; > > + }; > > + > > + factory: partition@40000 { > > + label = "Factory"; > > + reg = <0x40000 0x10000>; > > + read-only; > > + }; > > + > > + partition@50000 { > > + compatible = "denx,uimage"; > > + label = "Kernel"; > > + reg = <0x50000 0x7b0000>; > > + }; > > + }; > > I don't think a partitioning should be located in the SoC DTSI.
In case of Ralink SoCs I would see that as legitimate as truely a very large number of devices usually uses that (reference) partitioning. > > > + }; > > +}; > > + > > +&pcie { > > + wifi@0,0 { > > + compatible = "pci1814,3091"; > > + ralink,mtd-eeprom = <&factory 0x8000>; > > + }; > > Same here, depends on partitioning. The whole &pcie { ... } section should indeed go into the device DTS. > > > +}; > > diff --git a/target/linux/ramips/dts/rt6855a_linksys_wap300n.dts > > b/target/linux/ramips/dts/rt6855a_linksys_wap300n.dts > > new file mode 100644 > > index 0000000000..4b502ec0a7 > > --- /dev/null > > +++ b/target/linux/ramips/dts/rt6855a_linksys_wap300n.dts > > @@ -0,0 +1,22 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > There is no GPL-2.0, either GPL-2.0-only or GPL-2.0-or-later > > > +/dts-v1/; > > Drop dts-v1 here, we'll have that in DTSI. > > > + > > +/include/ "rt6855a.dtsi" > > We typically use #include here. > > > + > > +/ { > > + compatible = "ralink,rt6855a-soc"; > > Device compatible is missing. > > > + model = "Linksys foobar WAP300n"; > > Use proper name. > > > + > > + memory@0 { > > + device_type = "memory"; > > + reg = <0x20000 0x3fe0000>; > > + }; > > Do we need this, on other ramips platforms memory is auto-detected? > > > + > > + chosen { > > + bootargs = "console=ttyS0,57600"; > > + }; > > + > > + pcie@1fb80000 { > > + status = "okay"; > > + }; > > Use a DT label instead. ... and add the EEPROM information here as well instead of having it in the SoC's DTSI. > > > +}; > > diff --git a/target/linux/ramips/image/Makefile > > b/target/linux/ramips/image/Makefile > > index 4274c24884..8c916c072b 100644 > > --- a/target/linux/ramips/image/Makefile > > +++ b/target/linux/ramips/image/Makefile > > @@ -17,6 +17,7 @@ DEVICE_VARS += SERCOMM_PAD JCG_MAXSIZE > > loadaddr-y := 0x80000000 > > loadaddr-$(CONFIG_TARGET_ramips_rt288x) := 0x88000000 > > loadaddr-$(CONFIG_TARGET_ramips_mt7621) := 0x80001000 > > +loadaddr-$(CONFIG_TARGET_ramips_rt6855a) := 0x80020000 > > > > ldrplatform-y := ralink > > ldrplatform-$(CONFIG_TARGET_ramips_mt7621) := mt7621 diff --git > > a/target/linux/ramips/image/rt6855a.mk > > b/target/linux/ramips/image/rt6855a.mk > > new file mode 100644 > > index 0000000000..678f12eb65 > > --- /dev/null > > +++ b/target/linux/ramips/image/rt6855a.mk > > @@ -0,0 +1,12 @@ > > +# > > +# RT6855A Profiles > > +# > > + > > +define Device/linksys_wap300n > > + SOC := rt6855a > > + IMAGE_SIZE := 7936k > > + DEVICE_VENDOR := Linksys > > + DEVICE_MODEL := WAP300N > > + DEVICE_PACKAGES:= kmod-rt2800-pci > > +endef > > +TARGET_DEVICES += linksys_wap300n > > diff --git a/target/linux/ramips/rt6855a/base-files/etc/board.d/01_leds > > b/target/linux/ramips/rt6855a/base-files/etc/board.d/01_leds > > new file mode 100755 > > index 0000000000..575a36cf40 > > --- /dev/null > > +++ b/target/linux/ramips/rt6855a/base-files/etc/board.d/01_leds > > @@ -0,0 +1,17 @@ > > +#!/bin/sh > > + > > +. /lib/functions/leds.sh > > +. /lib/functions/uci-defaults.sh > > + > > +board=$(board_name) > > + > > +board_config_update > > + > > +case $board in > > +linksys,wap300n) > > + ;; > > If we don't need the file, I don't see a reason to create a dummy now. > > > +esac > > + > > +board_config_flush > > + > > +exit 0 > > diff --git a/target/linux/ramips/rt6855a/base-files/etc/board.d/02_network > > b/target/linux/ramips/rt6855a/base-files/etc/board.d/02_network > > new file mode 100755 > > index 0000000000..84753ce846 > > --- /dev/null > > +++ b/target/linux/ramips/rt6855a/base-files/etc/board.d/02_network > > @@ -0,0 +1,17 @@ > > +#!/bin/sh > > + > > +. /lib/functions.sh > > +. /lib/functions/uci-defaults.sh > > +. /lib/functions/system.sh > > You don't need all of these, do you? > > > + > > +board_config_update > > + > > +case $board in > > This is broken copy-paste, $board is never assigned. > > > +linksys,wap300n) > > + ucidef_set_interface_lan "eth0" > > + ;; > > +esac > > + > > +board_config_flush > > + > > +exit 0 > > diff --git a/target/linux/ramips/rt6855a/base- > > files/etc/board.d/03_gpio_switches b/target/linux/ramips/rt6855a/base- > > files/etc/board.d/03_gpio_switches > > new file mode 100755 > > index 0000000000..51f7e2dee7 > > --- /dev/null > > +++ b/target/linux/ramips/rt6855a/base-files/etc/board.d/03_gpio_switche > > +++ s > > @@ -0,0 +1,16 @@ > > +#!/bin/sh > > + > > +. /lib/functions/uci-defaults.sh > > + > > +board=$(board_name) > > + > > +board_config_update > > + > > +case $board in > > +linksys,wap300n) > > + ;; > > Again, no need for useless file. > > Best > > Adrian > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel