Re: [PATCH v3 4/4] ARM: dts: sunxi: add support for NetCube Systems Kumquat

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 2025-01-05 05:17, schrieb Icenowy Zheng:
在 2025-01-04星期六的 14:03 +0100,Lukas Schmid写道:
Am 2025-01-04 06:12, schrieb Icenowy Zheng:
> 在 2025-01-03星期五的 23:57 +0000,Andre Przywara写道:
> > On Fri,  3 Jan 2025 20:45:20 +0000
> > Lukas Schmid <lukas.schmid@xxxxxxxxxx> wrote:
> >
> > (CC:ing Icenowy for a question about the RTC below ...)
> >
> > > 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   | 290
> > > ++++++++++++++++++
> > >  arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi    |   6 +
> > >  3 files changed, 298 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..e5d2a716eb69
> > > --- /dev/null
> > > +++ b/arch/arm/boot/dts/allwinner/sun8i-v3s-netcube-kumquat.dts
> > > @@ -0,0 +1,290 @@
> > > +// 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";
> > > +       };
> > > +
> > > +       /* 40 MHz Crystal Oscillator on PCB */
> > > +       clk_can0: clock-can0 {
> > > +               compatible = "fixed-clock";
> > > +               #clock-cells = <0>;
> > > +               clock-frequency  = <40000000>;
> > > +       };
> > > +
> > > +       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 */
> > > +               };
> > > +       };
> > > +
> > > +       leds {
> > > +               compatible = "gpio-leds";
> > > +
> > > +               led-heartbeat {
> > > +                       gpios = <&pio 4 4 GPIO_ACTIVE_HIGH>; /*
> > > PE4
> > > */
> > > +                       linux,default-trigger = "heartbeat";
> > > +                       color = <LED_COLOR_ID_GREEN>;
> > > +                       function = LED_FUNCTION_HEARTBEAT;
> > > +               };
> > > +
> > > +               led-mmc0-act {
> > > +                       gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /*
> > > PF6
> > > */
> > > +                       linux,default-trigger = "mmc0";
> > > +                       color = <LED_COLOR_ID_GREEN>;
> > > +                       function = LED_FUNCTION_DISK;
> > > +               };
> > > +       };
> > > +
> > > +       /* XC6206-3.0 Linear Regualtor */
> > > +       reg_vcc3v0: regulator-3v0 {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "vcc3v0";
> > > +               regulator-min-microvolt = <3000000>;
> > > +               regulator-max-microvolt = <3000000>;
> > > +               vin-supply = <&reg_vcc3v3>;
> > > +       };
> > > +
> > > +       /* EA3036C Switching 3 Channel Regulator - Channel 2 */
> > > +       reg_vcc3v3: regulator-3v3 {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "vcc3v3";
> > > +               regulator-min-microvolt = <3300000>;
> > > +               regulator-max-microvolt = <3300000>;
> > > +               vin-supply = <&reg_vcc5v0>;
> > > +       };
> > > +
> > > +       /* K7805-1000R3 Switching Regulator supplied from main
> > > 12/24V terminal block */
> > > +       reg_vcc5v0: regulator-5v0 {
> > > +               compatible = "regulator-fixed";
> > > +               regulator-name = "vcc5v0";
> > > +               regulator-min-microvolt = <5000000>;
> > > +               regulator-max-microvolt = <5000000>;
> > > +       };
> > > +};
> > > +
> > > +&mmc0 {
> > > +       pinctrl-names = "default";
> > > +       pinctrl-0 = <&mmc0_pins>;
> > > +       vmmc-supply = <&reg_vcc3v3>;
> > > +       bus-width = <4>;
> > > +       broken-cd;
> > > +       status = "okay";
> > > +};
> > > +
> > > +&mmc1 {
> >
> > what's connected here? Are both MMC ports on headers/connectors,
> > and
> > it's up to the user to connect some SDIO device or an SD/eMMC
> > card/slot? Or is this port connected to the ESP32?
> >
> > > +       pinctrl-names = "default";
> > > +       pinctrl-0 = <&mmc1_pins>;
> > > +       vmmc-supply = <&reg_vcc3v3>;
> > > +       bus-width = <4>;
> > > +       broken-cd;
> > > +       status = "okay";
> > > +};
> > > +
> > > +&usb_otg {
> >
> > I think traditionally referenced nodes in the board .dts files
> > are
> > ordered by label name, so usb_otg is but-last. Yes, this is in
> > contrast
> > to nodes in the SoC .dtsi file, which are ordered by MMIO
> > addresses.
> >
> > > +       extcon = <&tusb320 0>;
> > > +       dr_mode = "otg";
> > > +       status = "okay";
> > > +};
> > > +
> > > +&usbphy {
> > > +       usb0_id_det-gpios = <&pio 1 4 GPIO_ACTIVE_HIGH>; /* PB4
> > > */
> > > +       status = "okay";
> > > +};
> > > +
> > > +&ehci {
> > > +       status = "okay";
> > > +};
> > > +
> > > +&ohci {
> > > +       status = "okay";
> > > +};
> > > +
> > > +&rtc {
> > > +       status = "disabled";
> >
> > Please can you explain a bit more what's going on here? I saw you
> > mentioning in the cover letter that you brought the "disabled"
> > back,
> > but I still don't see how this is working when the CCU and the
> > pinctrl
> > nodes reference the RTC clocks? So what is broken, exactly? Is
> > this
> > some bug in the SoC? Or don't you supply the SoC's VCC_RTC, so
> > the
> > calendar is not working when the board is not powered - in
> > contrast
> > to
> > the external RTC?
>
> Maybe a lack of crystal? But I can understand nothing here, and a
> detailed explaination is needed.
>

I have tried to enable the RTC, but I get a "RTC still busy" from the
sunxi-rtc module. The RTC Power is connected, and the 32kHz Crystal
is

Weird... Never heard such kind of things...

How do modules in the main SoC that depend on LOSC (32768 Hz osc)
perform?


I didn't notice anything running the board. I had one running for
about a month on Kernel 6.8.7 without issues, with the rtc disabled

Do you have anything I should check in particular? From the
Datasheet it seems to me that all peripherals in the SoC should
be able to run from the hosc too.

also
on Board. If the RTC's driver wouldn't throw the error I have no
issue
with
leaving the rtc enabled. It would however loose it's memory after
power
cycle as the Battery is only connected to the DS3232+, hence the
alias
to
rtc0

> >
> > > +};
> > > +
> > > +&pio {
> > > +       vcc-pb-supply = <&reg_vcc3v3>;
> > > +       vcc-pc-supply = <&reg_vcc3v3>;
> > > +       vcc-pe-supply = <&reg_vcc3v3>;
> > > +       vcc-pf-supply = <&reg_vcc3v3>;
> > > +       vcc-pg-supply = <&reg_vcc3v3>;
> > > +
> > > +       gpio-reserved-ranges = <0 32>, <42 22>, <68 28>, <96
> > > 32>,
> > > <153 7>, <167 25>, <198 26>;
> >
> > As mentioned in the reply to the previous patch, this doesn't
> > look
> > right here. Why do you need this, exactly?
>
> Ah? I don't think there's any tradition on Allwinner platforms to
> reserve any GPIOs, except if you have another firmware running on
> another processor (e.g. AR100) that needs some GPIO.
>
> My previous sight of such property is on Qualcomm smartphones,
> where a
> few GPIOs are reserved for "Trusted" thingy.
>

My intention here was to have the GPIOs which are not accessible on
the
SoC's
package disabled so that stuff like libgpiod cannot try to access
them.
The
gpiodetect tool did show me them as 'used' when I added the
reserved-ranges,
so I thought the driver does understand this tag.

Interesting point... Maybe this kind of things should enter the per-
SoC(package) DTSI?

Although technically they are not "used" but "unavailable".


Sure, I think I'll remove it from my Patchset for now, but maybe
will create another set in the future.


> >
> > > +       gpio-line-names = "", "", "", "", // PA
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "CAN_nCS", "CAN_nINT", "USER_SW",
> > > "PB3",
> > > // PB
> > > +                         "USB_ID", "USBC_nINT", "I2C0_SCL",
> > > "I2C0_SDA",
> > > +                         "UART0_TX", "UART0_RX", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "SPI_MISO", "SPI_SCK", "FLASH_nCS",
> > > "SPI_MOSI", // PC
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "", // PD
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "Q12", "Q11", "Q10", "Q9", // PE
> > > +                         "LED_SYS0", "I1", "Q1", "Q2",
> > > +                         "I2", "I3", "Q3", "Q4",
> > > +                         "I4", "I5", "Q5", "Q6",
> > > +                         "I6", "I7", "Q7", "Q8",
> > > +                         "I8", "UART1_TXD", "UART1_RXD",
> > > "ESP_nRST",
> > > +                         "ESP_nBOOT", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "SD_D1", "SD_D0", "SD_CLK", "SD_CMD",
> > > //
> > > PF
> > > +                         "SD_D3", "SD_D2", "LED_SYS1", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "ESP_CLK", "ESP_CMD", "ESP_D0",
> > > "ESP_D1",
> > > // PG
> > > +                         "ESP_D2", "ESP_D3", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "",
> > > +                         "", "", "", "";
> > > +};
> > > +
> > > +&lradc {
> > > +       vref-supply = <&reg_vcc3v0>;
> > > +       status = "okay";
> > > +};
> > > +
> > > +&codec {
> > > +       allwinner,audio-routing =
> > > +               "Headphone", "HP",
> > > +               "Headphone", "HPCOM",
> > > +               "MIC1", "Mic",
> > > +               "Mic", "HBIAS";
> > > +       status = "okay";
> > > +};
> > > +
> > > +&uart0 {
> > > +       pinctrl-0 = <&uart0_pb_pins>;
> > > +       pinctrl-names = "default";
> > > +       status = "okay";
> > > +};
> > > +
> > > +&uart1 {
> > > +       pinctrl-0 = <&uart1_pe_pins>;
> > > +       pinctrl-names = "default";
> > > +       status = "okay";
> > > +};
> > > +
> > > +&i2c0 {
> > > +       pinctrl-0 = <&i2c0_pins>;
> > > +       pinctrl-names = "default";
> > > +       status = "okay";
> > > +
> > > +       ds3232: rtc@68 {
> > > +               compatible = "dallas,ds3232";
> > > +               reg = <0x68>;
> > > +       };
>
> If you're afraid of the non-running internal RTC superseding this
> external RTC, you can use an alias rtc0 = &ds3232 to force the ext.
> one
> to be the first.
>

Yes exactly, I already have an alias to this rtc as rtc0

> > > +
> > > +       eeprom0: eeprom@50 {
> > > +               compatible = "atmel,24c02";             /*
> > > actually
> > > it's a 24AA02E48 */
> > > +               pagesize = <16>;
> > > +               read-only;
> > > +               reg = <0x50>;
> > > +               vcc-supply = <&reg_vcc3v3>;
> > > +
> > > +               #address-cells = <1>;
> > > +               #size-cells = <1>;
> > > +
> > > +               eth0_macaddress: macaddress@fa {
> > > +                       reg = <0xfa 0x06>;
> > > +               };
> > > +       };
> > > +
> > > +       tusb320: typec@60 {
> > > +               compatible = "ti,tusb320";
> > > +               reg = <0x60>;
> > > +               interrupt-parent = <&pio>;
> > > +               interrupts = <1 5 IRQ_TYPE_EDGE_FALLING>;
> > > +       };
> > > +};
> > > +
> > > +&emac {
> > > +       allwinner,leds-active-low;
> > > +       nvmem-cells = <&eth0_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>;
> >
> > Those two lines look redundant, as they are already specified in
> > the
> > .dtsi file.
> >
> > > +       cs-gpios = <0>, <&pio 1 0 GPIO_ACTIVE_LOW>; /* PB0 */
> > > +       status = "okay";
> > > +
> > > +       flash@0 {
> > > +               #address-cells = <1>;
> > > +               #size-cells = <1>;
> > > +               compatible = "jedec,spi-nor";
> >
> > I think traditionally we have the compatible first, and #a-c and
> > #s-c
> > last in the node.
> > And do you have anything partitioned in there? If not, you
> > wouldn't
> > need the #a-c and #s-c properties, I think.
> >
> > > +               reg = <0>;
> > > +               label = "firmware";
> > > +               spi-max-frequency = <40000000>;
> > > +       };
> > > +
> > > +       can@1 {
> > > +               compatible = "microchip,mcp2518fd";
> > > +               reg = <1>;
> > > +               clocks = <&clk_can0>;
> > > +               interrupt-parent = <&pio>;
> > > +               interrupts = <1 1 IRQ_TYPE_LEVEL_LOW>;  /* PB1
> > > */
> > > +               spi-max-frequency = <20000000>;
> > > +               vdd-supply = <&reg_vcc3v3>;
> > > +               xceiver-supply = <&reg_vcc3v3>;
> > > +       };
> > > +};
> > > \ No newline at end of file
> >
> > Please add a newline at the end.
>
> Well maybe this file is written with some non-Unix-traditional
> editor,
> well Linux is something Unix-traditional, and for these editors
> manual
> insertion of an empty line will be needed (on Unix-traditional
> things
> e.g. Vim, no empty lines should be presented at all.)
>
> >
> > > diff --git a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> > > b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> >
> > I don't know for sure if people want SoC .dtsi patches
> > separately?
> >
> > Cheers,
> > Andre
> >
> > > index 9e13c2aa8911..f909b1d4dbca 100644
> > > --- a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> > > +++ b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> > > @@ -416,6 +416,12 @@ uart0_pb_pins: uart0-pb-pins {
> > >                                 function = "uart0";
> > >                         };
> > >  
> > > +                       /omit-if-no-ref/
> > > +                       uart1_pe_pins: uart1-pe-pins {
> > > +                               pins = "PE21", "PE22";
> > > +                               function = "uart1";
> > > +                       };
> > > +
> > >                         uart2_pins: uart2-pins {
> > >                                 pins = "PB0", "PB1";
> > >                                 function = "uart2";
> >
> >







[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux