[RFC PATCH v2 1/3] ramips: add support for the RT6855A SoC
Daniel Golle
daniel at makrotopia.org
Mon Dec 28 08:28:35 EST 2020
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-bounces at lists.openwrt.org]
> > On Behalf Of Rafaël Carré
> > Sent: Montag, 28. Dezember 2020 01:25
> > To: openwrt-devel at lists.openwrt.org
> > Cc: Rafaël Carré <funman at 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é <funman at 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 at 0 {
> > + compatible = "mips,mips34Kc";
> > + };
> > + };
> > +
> > + cpuintc: cpuintc {
> > + #address-cells = <0>;
> > + #interrupt-cells = <1>;
> > + interrupt-controller;
> > + compatible = "mti,cpu-interrupt-controller";
> > + };
> > +
> > + palmbus at 1fb00000 {
> > + compatible = "palmbus";
> > + reg = <0x1fb00000 0xe0000>;
> > + ranges = <0x0 0x1fb00000 0x100000>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + sysc at 800 {
> > + compatible = "ralink,rt6855a-sysc";
> > + reg = <0x800 0x100>;
> > + };
> > +
> > + intc: intc at 40000 {
> > + compatible = "ralink,rt6855a-intc";
> > + reg = <0x40000 0x100>;
> > +
> > + interrupt-controller;
> > + #interrupt-cells = <1>;
> > +
> > + interrupt-parent = <&cpuintc>;
> > + };
> > +
> > + memc at 300 {
> > + compatible = "ralink,rt6855a-memc", "ralink,rt3050-
> > memc";
> > + reg = <0x300 0x100>;
> > + };
> > +
> > + watchdog at f0100 {
> > + compatible = "ralink,rt6855a-wdt";
> > + reg = <0xf0100 0x10>;
> > + };
> > +
> > + uart: uart at 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 at 30000 {
> > + compatible = "ralink,gdma-rt2880";
> > + reg = <0x30000 0x100>;
> > + };
> > +
> > + ethernet: ethernet at 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 at 60000 {
> > + compatible = "ralink,rt3050-esw";
> > + reg = <0x60000 0x8000>;
> > +
> > + interrupt-parent = <&intc>;
> > + interrupts = <15>;
> > + };
> > +
> > + spi0: spi at 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 at 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 at 0 {
> > + compatible = "jedec,spi-nor";
> > + reg = <0>;
> > + spi-max-frequency = <10000000>;
> > +
> > + partitions {
> > + compatible = "fixed-partitions";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + partition at 0 {
> > + label = "Bootloader";
> > + reg = <0x0 0x30000>;
> > + read-only;
> > + };
> > +
> > + partition at 30000 {
> > + label = "Config";
> > + reg = <0x30000 0x10000>;
> > + read-only;
> > + };
> > +
> > + factory: partition at 40000 {
> > + label = "Factory";
> > + reg = <0x40000 0x10000>;
> > + read-only;
> > + };
> > +
> > + partition at 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 at 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 at 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 at 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 at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list