On 02/01/2025 16:05, Lukas Schmid wrote:
> NetCube Systems Kumquat is a board based on the Allwinner V3s SoC,
> including:
>
> - 64MB DDR2 included in SoC
> - 10/100 Mbps Ethernet
> - USB-C DRD
> - Audio Codec
> - Isolated CAN-FD
> - ESP32 over SDIO
> - 8MB SPI-NOR Flash for bootloader
> - I2C EEPROM for MAC addresses
> - SDIO Connector for eMMC or SD-Card
> - 8x 12/24V IOs, 4x normally open relays
> - DS3232 RTC
> - QWIIC connectors for external I2C devices
>
> Signed-off-by: Lukas Schmid <lukas.schmid@xxxxxxxxxx>
> ---
> arch/arm/boot/dts/allwinner/Makefile | 2 +
> .../allwinner/sun8i-v3s-netcube-kumquat.dts | 237 ++++++++++++++++++
> arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi | 6 +
> 3 files changed, 245 insertions(+)
> create mode 100644 arch/arm/boot/dts/allwinner/sun8i-v3s-netcube-kumquat.dts
>
> diff --git a/arch/arm/boot/dts/allwinner/Makefile b/arch/arm/boot/dts/allwinner/Makefile
> index 48666f73e638..d799ad153b37 100644
> --- a/arch/arm/boot/dts/allwinner/Makefile
> +++ b/arch/arm/boot/dts/allwinner/Makefile
> @@ -199,6 +199,7 @@ DTC_FLAGS_sun8i-h3-nanopi-r1 := -@
> DTC_FLAGS_sun8i-h3-orangepi-pc := -@
> DTC_FLAGS_sun8i-h3-bananapi-m2-plus-v1.2 := -@
> DTC_FLAGS_sun8i-h3-orangepi-pc-plus := -@
> +DTC_FLAGS_sun8i-v3s-netcube-kumquat := -@
> dtb-$(CONFIG_MACH_SUN8I) += \
> sun8i-a23-evb.dtb \
> sun8i-a23-gt90h-v4.dtb \
> @@ -261,6 +262,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> sun8i-v3s-anbernic-rg-nano.dtb \
> sun8i-v3s-licheepi-zero.dtb \
> sun8i-v3s-licheepi-zero-dock.dtb \
> + sun8i-v3s-netcube-kumquat.dtb \
> sun8i-v40-bananapi-m2-berry.dtb
> dtb-$(CONFIG_MACH_SUN9I) += \
> sun9i-a80-optimus.dtb \
> diff --git a/arch/arm/boot/dts/allwinner/sun8i-v3s-netcube-kumquat.dts b/arch/arm/boot/dts/allwinner/sun8i-v3s-netcube-kumquat.dts
> new file mode 100644
> index 000000000000..7fe83d91adee
> --- /dev/null
> +++ b/arch/arm/boot/dts/allwinner/sun8i-v3s-netcube-kumquat.dts
> @@ -0,0 +1,237 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (C) 2025 Lukas Schmid <lukas.schmid@xxxxxxxxxx>
> + */
> +
> +/dts-v1/;
> +#include "sun8i-v3s.dtsi"
> +
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/{
> + model = "NetCube Systems Kumquat";
> + compatible = "netcube,kumquat", "allwinner,sun8i-v3s";
> +
> + aliases {
> + serial0 = &uart0;
> + ethernet0 = &emac;
> + rtc0 = &ds3232;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> +
> + heartbeat_led {
Please follow DTS coding style.
> + gpios = <&pio 4 4 GPIO_ACTIVE_HIGH>; /* PE4 */
> + linux,default-trigger = "heartbeat";
> + function = LED_FUNCTION_HEARTBEAT;
> + color = <LED_COLOR_ID_GREEN>;
> + };
> +
> + mmc0_act_led {
Ditto
> + gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
> + linux,default-trigger = "mmc0";
> + function = LED_FUNCTION_DISK;
> + color = <LED_COLOR_ID_GREEN>;
> + };
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> + autorepeat;
> +
> + key-user {
> + label = "GPIO Key User";
> + linux,code = <KEY_PROG1>;
> + gpios = <&pio 1 2 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>; /* PB2 */
> + };
> + };
> +
> + /* K7805-1000R3 Switching Regulator supplied from main 12/24V terminal block */
> + reg_vcc5v0: vcc5v0 {
Please use name for all fixed regulators which matches current format
recommendation: 'regulator-[0-9]v[0-9]'
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml?h=v6.11-rc1#n46
(or at least regulator prefix)
> + compatible = "regulator-fixed";
> + regulator-name = "vcc5v0";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + };
> +
> + /* EA3036C Switching 3 Channel Regulator - Channel 2 */
> + reg_vcc3v3: vcc3v3 {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc3v3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + vin-supply = <®_vcc5v0>;
> + };
> +
> + /* XC6206-3.0 Linear Regualtor */
> + reg_vcc3v0: vcc3v0 {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc3v0";
> + regulator-min-microvolt = <3000000>;
> + regulator-max-microvolt = <3000000>;
> + vin-supply = <®_vcc3v3>;
> + };
> +
> + /* 40 MHz Crystal Oscillator on PCB */
> + can0_osc: can0_osc {
DTS coding style.
...
> +
> + eeprom0: eeprom@50 {
> + compatible = "atmel,24c02"; /* actually it's a 24AA02E48 */
> + pagesize = <16>;
> + read-only;
> + reg = <0x50>;
> + vcc-supply = <®_vcc3v3>;
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + eth0_macaddress: eth0_macaddress@FA {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.
> + reg = <0xFA 0x06>;
Here and in multiple other places - it is always lowercase hex.
Again: please follow DTS coding style.
> + };
> + };
> +
> + tusb320: tusb320@60 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "ti,tusb320";
> + reg = <0x60>;
> + interrupt-parent = <&pio>;
> + interrupts = <1 5 IRQ_TYPE_EDGE_FALLING>;
> + };
> +
> + ds3232: rtc@68 {
> + compatible = "dallas,ds3232";
> + reg = <0x68>;
> + };
> +};
> +
> +&emac {
> + allwinner,leds-active-low;
> + nvmem-cells = <ð0_macaddress>; /* custom nvmem reference */
> + nvmem-cell-names = "mac-address"; /* see ethernet-controller.yaml */
> + status = "okay";
> +};
> +
> +&spi0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi0_pins>;
> + cs-gpios = <0>, <&pio 1 0 GPIO_ACTIVE_LOW>; /* PB0 */
> + status = "okay";
> +
> + flash@0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0>;
> + compatible = "jedec,spi-nor";
Odd ordering of properties.
> + label = "firmware";
> + spi-max-frequency = <40000000>;
> + };
> +
> + can@1 {
> + compatible = "microchip,mcp2518fd";
> + reg = <1>;
And here totally different. DTS coding style defines one (this is correct).
> + clocks = <&can0_osc>;
> + spi-max-frequency = <20000000>;
> + interrupt-parent = <&pio>;
> + interrupts = <1 1 IRQ_TYPE_LEVEL_LOW>; /* PB1 */
> + vdd-supply = <®_vcc3v3>;
> + xceiver-supply = <®_vcc3v3>;
> + };
> +};
> +
> +&pio {
> + vcc-pb-supply = <®_vcc3v3>;
> + vcc-pc-supply = <®_vcc3v3>;
> + vcc-pe-supply = <®_vcc3v3>;
> + vcc-pf-supply = <®_vcc3v3>;
> + vcc-pg-supply = <®_vcc3v3>;
> +
> + gpio-reserved-ranges = <0 32>, <42 22>, <68 28>, <96 32>, <153 7>, <167 25>, <198 26>;
> + gpio-line-names = "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", //PA
> + "CAN_nCS", "CAN_nINT", "USER_SW", "PB3", "USB_ID", "USBC_nINT", "I2C0_SCL", "I2C0_SDA", "UART0_TX", "UART0_RX", "", "", "", "", "", "", "", "", "",
Totally messed wrapping and alignment/indentation.
...
>
> + /omit-if-no-ref/
> + uart1_pe_pins: uart1_pe_pins {
look here...
> + pins = "PE21", "PE22";
> + function = "uart1";
> + };
> +
> uart2_pins: uart2-pins {
and here and the difference in style should trigger your review before
submitting it. Align your code with upstream, not downstream coding style.
Best regards,
Krzysztof
