Hi. W dniu 25.05.2021 o 10:22, Daniel Golle pisze: > Hi! > > thank you for submitting support for this interesting hardware. > There are some minor style issues which need to be addressed before > the patch can be merged into our tree. > It would also be better to split this into at least two patches: > [1/2] Add driver for Epson RX8130 RTC
This RTC seems to be supported for a long time by upstream rtc-ds1307 driver [1]. What does not work with upstream one that works with this one? > [2/2] Add support for Puzzla M90x > > Other than that, please see my comments inline below: > > On Mon, May 24, 2021 at 01:58:00AM +0800, eveans2...@gmail.com wrote: >> From: ianchang <ianch...@ieiworld.com> >> >> Hardware specification >> ---------------------- >> * CN9130 SoC, Quad-core ARMv8 Cortex-72 @ 2200 MHz >> * 4 GB DDR >> * 4 GB eMMC >> * 4 MB (SPI Flash) >> * 6 x 2.5 Gigabit ports (Puzzle-M901) >> - External PHY with 6 ports (AQR112R) >> * 6 x 2.5 Gigabit ports (Puzzle-M902) >> - External PHY with 6 ports (AQR112R) >> 3 x 10 Gigabit ports (Puzzle-M902) >> - External PHY with 3 ports (AQR113R) >> >> * 4 x Front panel LED >> * 1 x USB 3.0 >> * Reset button on Rear panel >> * UART (115200 8N1,header on PCB) >> >> Signed-off-by: ianchang <ianch...@ieiworld.com> > > Please use your full real name seperated by spaces (here in SoB and in > the Form: line above) > >> --- >> .../base-files/etc/board.d/02_network | 9 +- >> target/linux/mvebu/cortexa72/config-5.4 | 14 + >> .../boot/dts/marvell/armada-ap807-quad.dtsi | 93 + >> .../arm64/boot/dts/marvell/armada-ap807.dtsi | 30 + >> .../arm64/boot/dts/marvell/armada-ap80x.dtsi | 464 ++++ >> .../arm64/boot/dts/marvell/armada-cp115.dtsi | 12 + >> .../arm64/boot/dts/marvell/armada-cp11x.dtsi | 573 +++++ > > The above armada-*.dtsi are all already present in vanilla Linux. > Do not overwrite them here. Please amend them with patches, if needed. > > >> .../dts/marvell/puzzle-armada-common.dtsi | 11 + >> .../boot/dts/marvell/puzzle-armada-cp110.dtsi | 12 + >> .../arm64/boot/dts/marvell/puzzle-cn9130.dtsi | 37 + >> .../dts/marvell/puzzle-m901-cn9130-db.dts | 429 ++++ >> .../dts/marvell/puzzle-m901-cn9131-db.dts | 243 +++ >> .../dts/marvell/puzzle-m902-cn9130-db.dts | 430 ++++ >> .../dts/marvell/puzzle-m902-cn9131-db.dts | 247 +++ >> .../dts/marvell/puzzle-m902-cn9132-db.dts | 261 +++ >> target/linux/mvebu/files/drivers/rtc/Kconfig | 1943 +++++++++++++++++ >> target/linux/mvebu/files/drivers/rtc/Makefile | 189 ++ >> .../mvebu/files/drivers/rtc/rtc-rx8130.c | 807 +++++++ >> target/linux/mvebu/image/cortexa72.mk | 20 + >> 19 files changed, 5823 insertions(+), 1 deletion(-) >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/armada-ap807-quad.dtsi >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/armada-ap807.dtsi >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/armada-ap80x.dtsi >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/armada-cp115.dtsi >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-armada-common.dtsi >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-armada-cp110.dtsi >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-cn9130.dtsi >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-m901-cn9130-db.dts >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-m901-cn9131-db.dts >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-m902-cn9130-db.dts >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-m902-cn9131-db.dts >> create mode 100644 >> target/linux/mvebu/files/arch/arm64/boot/dts/marvell/puzzle-m902-cn9132-db.dts >> create mode 100644 target/linux/mvebu/files/drivers/rtc/Kconfig > > You should not overwrite existing files in the kernel tree > (drivers/rtc/Kconfig does exist already). Use a patch and add that to > target/linux/mvevu/patches-*/ instead to modify files which are present > in the Linux kernel tree. > >> create mode 100644 target/linux/mvebu/files/drivers/rtc/Makefile > > Same here. > > >> create mode 100644 target/linux/mvebu/files/drivers/rtc/rtc-rx8130.c >> >> diff --git a/target/linux/mvebu/cortexa72/base-files/etc/board.d/02_network >> b/target/linux/mvebu/cortexa72/base-files/etc/board.d/02_network >> index 9ab3c8174d..9ad043b343 100755 >> --- a/target/linux/mvebu/cortexa72/base-files/etc/board.d/02_network >> +++ b/target/linux/mvebu/cortexa72/base-files/etc/board.d/02_network >> @@ -21,8 +21,15 @@ marvell,armada8040-db) >> marvell,armada7040-db) >> ucidef_set_interfaces_lan_wan "eth0 eth2" "eth1" >> ;; >> +marvell,cn9131) >> + ucidef_set_interfaces_lan_wan "eth1 eth2 eth3 eth4 eth5" "eth0" >> + ;; >> +marvell,cn9132) >> + ucidef_set_interfaces_lan_wan "eth1 eth2 eth3 eth4 eth5 eth10 eth11 >> eth12" "eth0" >> + ;; >> *) >> - ucidef_set_interface_lan "eth0" >> +# ucidef_set_interface_lan "eth0" >> + ucidef_set_interfaces_lan_wan "eth0 eth3" "eth6" > > Please keep files with one style of indentation (ie. use tabs for > indentation in files already using tabs, use 4 or 8 spaces in files > already applying this style). Here you are indenting with spaces in a > file which is using tabs. Aside from tabs vs spaces, there's a lot of comented out code with C++ style in dts, it should be removed and all remaining comments converted to C style. > >> ;; >> esac >> >> diff --git a/target/linux/mvebu/cortexa72/config-5.4 >> b/target/linux/mvebu/cortexa72/config-5.4 >> index 5727ae5918..919f579157 100644 >> --- a/target/linux/mvebu/cortexa72/config-5.4 >> +++ b/target/linux/mvebu/cortexa72/config-5.4 >> @@ -175,3 +175,17 @@ CONFIG_THREAD_INFO_IN_TASK=y >> CONFIG_UNMAP_KERNEL_AT_EL0=y >> CONFIG_VMAP_STACK=y >> CONFIG_ZONE_DMA32=y >> +# CONFIG_PUZZLE-M90x >> +CONFIG_RTC_DRV_RX8130=y >> +# CONFIG_RTC_DRV_MV is not set >> +# CONFIG_RTC_DRV_ARMADA38X is not set >> +# Character devices >> +CONFIG_ARCH_HAS_DEVMEM_IS_ALLOWED=y >> +CONFIG_DEVMEM=y Some time ago there was enablement of DEVMEM for other target [2]. it lacked justification for providing it by default, pleas provide one here. Aside from this, changes to config for kernel 5.10 are lacking. [snip] 1. https://www.kernel.org/doc/Documentation/devicetree/bindings/rtc/rtc-ds1307.txt 2. http://lists.infradead.org/pipermail/openwrt-devel/2021-February/033702.html Regards -- TMN _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
