[PATCH] cn913x: add support for iEi Puzzle-M901/Puzzle-M902

Tomasz Maciej Nowak tmn505 at gmail.com
Tue May 25 07:05:02 PDT 2021


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, eveans2002 at gmail.com wrote:
>> From: ianchang <ianchang at 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 <ianchang at 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



More information about the openwrt-devel mailing list