[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