From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.netcube.li (mail.netcube.li [173.249.15.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7E7231DE8A9; Mon, 6 Jan 2025 17:26:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=173.249.15.149 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736184385; cv=none; b=mF8o/886U43yiyKAjOPcp4uF+apQx0vmv6la9Oq8swFuE7/zODrCoY2nleQahSdB+VxUQ2lvcO3ntXeeFIOqVoP4HMFvNukupd6kVwIXNzDN6siJn3qnPYlClVH9B0Dm7WIYjS3gvE9IzO0M8uCnsgPUL2Od1aahKotQvbPU0vE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736184385; c=relaxed/simple; bh=PHQrqVZzB/oQNtxKdvu2zpgY9kyyb63nwO2xRl8xoBY=; h=MIME-Version:Date:From:To:Cc:Subject:In-Reply-To:References: Message-ID:Content-Type; b=Xlqq9Wm/IpIjpEjHA1+bRthMyfGOufOH6CyU0CyBqRm2L071n9Ak1Ct1PgeKFFED6Imlaf7v3PuTOJNypvUUIFa94PC015yhvD6RE6Bb0iQiVLL4tH+MUe7wBP41Xqo5dya23vSr2zwt1xQ6k0NLE6ZP47R9e4U6m7k75n+fl6c= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=netcube.li; spf=pass smtp.mailfrom=netcube.li; dkim=pass (1024-bit key) header.d=netcube.li header.i=@netcube.li header.b=m4fMITPh; arc=none smtp.client-ip=173.249.15.149 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=netcube.li Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=netcube.li Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=netcube.li header.i=@netcube.li header.b="m4fMITPh" dkim-signature: v=1; a=rsa-sha256; d=netcube.li; s=s1; c=relaxed/relaxed; q=dns/txt; h=From:Subject:Date:Message-ID:To:CC:MIME-Version:Content-Type:Content-Transfer-Encoding:In-Reply-To:References; bh=MMf6JRJY8m7QgssvlVp48fqo3zwDrilE3i09LwS90PA=; b=m4fMITPhwuUdX5vb6kmzTzAib6uATLj75lbYqexMciLgMRqsvafyqGcsM5FtOLRmvHRifi+9ObS9ntUMS+RFV5HFRqLHhSxDMvxQrWL5TgiLPG9bI3DPoTf0audIkPyj0qWSvzFTXOsfXiEIDCEEbpfhTo45SMwqwPJ/aqbH4J8= Received: from webmail.netcube.li (WIN-IJ7TS3MJ5LT [127.0.0.1]) by mail.netcube.li with ESMTPA ; Mon, 6 Jan 2025 18:25:24 +0100 Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Date: Mon, 06 Jan 2025 18:25:23 +0100 From: Lukas Schmid To: Andre Przywara Cc: Icenowy Zheng , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Linus Walleij , Maxime Ripard , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org Subject: Re: [PATCH v3 4/4] ARM: dts: sunxi: add support for NetCube Systems Kumquat In-Reply-To: <20250106004705.10a8c616@minigeek.lan> References: <20250103204523.3779-1-lukas.schmid@netcube.li> <20250103204523.3779-5-lukas.schmid@netcube.li> <20250103235723.6a893773@minigeek.lan> <083add26c987bfbfedcda3ddbe5b9880@netcube.li> <9191d32ae352f311ad2c2751bca23a7d@netcube.li> <20250106004705.10a8c616@minigeek.lan> User-Agent: Roundcube Webmail/1.4.11 Message-ID: <431fdf8c83178155cd731800be03b8ef@netcube.li> X-Sender: lukas.schmid@netcube.li Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Am 2025-01-06 01:47, schrieb Andre Przywara: > On Sun, 05 Jan 2025 12:25:26 +0100 > Lukas Schmid wrote: > >> 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 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 >> >> > > > --- >> >> > > >  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 >> >> > > > + */ >> >> > > > + >> >> > > > +/dts-v1/; >> >> > > > +#include "sun8i-v3s.dtsi" >> >> > > > + >> >> > > > +#include >> >> > > > +#include >> >> > > > +#include >> >> > > > + >> >> > > > +/{ >> >> > > > +       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 = ; >> >> > > > +                       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 = ; >> >> > > > +                       function = LED_FUNCTION_HEARTBEAT; >> >> > > > +               }; >> >> > > > + >> >> > > > +               led-mmc0-act { >> >> > > > +                       gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* >> >> > > > PF6 >> >> > > > */ >> >> > > > +                       linux,default-trigger = "mmc0"; >> >> > > > +                       color = ; >> >> > > > +                       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 = <®_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 = <®_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 = <®_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 = <®_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 > > That sounds actually like a bug somewhere, which should be investigated > and fixed. Disabling the RTC is a quick and dirty workaround, but > nothing for upstream. > There is a 50ms timeout, which sounds plenty, although you could > experiment whether it's too short here: > drivers/rtc/rtc-sun6i.c:sun6i_rtc_settime() > My hunch is that even longer timeouts won't change that, so we should > have a look what's going wrong here, why the RTC does not respond. > Don't you supply VCC_RTC to the SoC at all, maybe? Maybe we need > the 3.3V for the RTC to work, regardless of whether that is battery > backed or not? > >> > 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. > > That's a DT issue, you don't *need* the LOSC crystal for operation: > The clock controller and the GPIO device reference the RTC LOSC clock, > so the respective drivers go ahead and request them, which triggers > probing the RTC. If that now fails, because the RTC is disabled, that > clock cannot be used, and this has consequences: > - The (combinded) GPIO/pinctrl driver only needs the LOSC clock for the > debouncing part. If that fails, the debouncing init routine will bail > out (I guess it will not work), but the driver ignores that error and > proceeds probing, so you are saved, but more by accident. I wonder if > we should at least print a message here. > - The CCU clock driver only uses the LOSC as a potential parent for > some muxes (CPU, AHB, APB2), and the kernel will probably never > change > those muxes, and even if, not to the LOSC. So again saved here, by > chance. > > So this means your system continues working, but it's very sketchy, and > technically wrong. So disabling the RTC is not really an option for > mainline, I'd say. Okay, I'll submit v4 with all changes now, and keep the rtc enabled. I will try to find out what causes the issue and hopefull create another patch. I just checked, and the rtc does not throw the error in 6.8.7, so that's something to go by... > >> >> 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 = <®_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>; >> >> > > >> >> > > 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? > > Our sunxi pinctrl driver does not use this information, we have the > number of pins per port and their availability hardcoded in that large > table in pinctrl-sun8i-v3s.c. > > If there is more code in the kernel that uses this DT property to > filter the (userspace?) view of the GPIOs, that's a side effect. If we > want this filtering, this code should query the pinctrl driver for this > map, but that's a separate issue. The binding pretty clearly speaks of > some per-port GPIO drivers to use this map, ours it not one of them. > >> > Although technically they are not "used" but "unavailable". >> > >> >> Sure, I think I'll remove it from my Patchset for now > > Yes, please, do that, I think it's wrong here. > >> , but maybe will create another set in the future. > > If that solves a problem for you, it should be investigated why, to me > it looks like it's a side effect, and that should be done properly > instead - for instance by using all those empty strings below as an > indicator for availability. > But I don't think insisting on that property is the right thing. > > Cheers, > Andre > >> >> > > > +       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 = <®_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 = <®_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 = <ð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>; >> >> > > >> >> > > 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 = <®_vcc3v3>; >> >> > > > +               xceiver-supply = <®_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"; >> >> > > >> >> > > >> >> >> >> >> >>