Re: [PATCH V3 8/8] ARM: dts: sunxi: add support for Anbernic RG-Nano

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

 



On Fri, Aug 25, 2023 at 11:37:58PM +0100, Andre Przywara wrote:
> On Wed, 23 Aug 2023 16:25:54 -0500
> Chris Morgan <macroalpha82@xxxxxxxxx> wrote:
> 
> Hi,
> 
> > From: Chris Morgan <macromorgan@xxxxxxxxxxx>
> > 
> > The Anbernic RG-Nano is a small portable game device based on the
> > Allwinner V3s SoC. It has GPIO buttons on the face and side for
> > input, a single mono speaker, a 240x240 SPI controlled display, a USB-C
> > OTG port, an SD card slot for booting, and 64MB of RAM included in the
> > SoC.
> > 
> > Working/Tested:
> > - SDMMC
> > - UART (for debugging)
> > - Buttons
> > - Charging/battery/PMIC
> > - Speaker
> > - USB Host and Gadget
> > - Display (at 60hz)
> 
> So this works now purely in software, by the panel driver constantly
> scanning out a framebuffer region via SPI?
> I wonder if enabling DMA support for SPI then helps the CPU load?
> Though this might be a performance optimisation in a later patch set
> (as it requires adding support for the V3s DMA controller).

I think it's already enabled, at least I see it in
/sys/kernel/debug/dmaengine/summary:
 dma0chan0    | 1c68000.spi:tx
 dma0chan1    | 1c68000.spi:rx

The idle CPU usage and CPU usage when running kmscube don't seem too
terrible, but I'm always open to improvements.

> 
> > 
> > Signed-off-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
> > ---
> >  arch/arm/boot/dts/allwinner/Makefile          |   1 +
> >  .../allwinner/sun8i-v3s-anbernic-rg-nano.dts  | 275 ++++++++++++++++++
> >  2 files changed, 276 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/allwinner/sun8i-v3s-anbernic-rg-nano.dts
> > 
> > diff --git a/arch/arm/boot/dts/allwinner/Makefile b/arch/arm/boot/dts/allwinner/Makefile
> > index 589a1ce1120a..2be83a1edcbb 100644
> > --- a/arch/arm/boot/dts/allwinner/Makefile
> > +++ b/arch/arm/boot/dts/allwinner/Makefile
> > @@ -237,6 +237,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> >  	sun8i-t113s-mangopi-mq-r-t113.dtb \
> >  	sun8i-t3-cqa3t-bv3.dtb \
> >  	sun8i-v3-sl631-imx179.dtb \
> > +	sun8i-v3s-anbernic-rg-nano.dtb \
> >  	sun8i-v3s-licheepi-zero.dtb \
> >  	sun8i-v3s-licheepi-zero-dock.dtb \
> >  	sun8i-v40-bananapi-m2-berry.dtb
> > diff --git a/arch/arm/boot/dts/allwinner/sun8i-v3s-anbernic-rg-nano.dts b/arch/arm/boot/dts/allwinner/sun8i-v3s-anbernic-rg-nano.dts
> > new file mode 100644
> > index 000000000000..c2866a884f0e
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/allwinner/sun8i-v3s-anbernic-rg-nano.dts
> > @@ -0,0 +1,275 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +
> > +/dts-v1/;
> > +#include <dt-bindings/input/linux-event-codes.h>
> > +#include "sun8i-v3s.dtsi"
> > +#include "sunxi-common-regulators.dtsi"
> > +
> > +/ {
> > +	model = "Anbernic RG Nano";
> > +	compatible = "anbernic,rg-nano", "allwinner,sun8i-v3s";
> > +
> > +	aliases {
> > +		serial0 = &uart0;
> > +	};
> > +
> > +	backlight: backlight {
> > +		compatible = "pwm-backlight";
> > +		brightness-levels = <0 1 2 3 8 14 21 32 46 60 80 100>;
> > +		default-brightness-level = <11>;
> > +		power-supply = <&reg_vcc5v0>;
> > +		pwms = <&pwm 0 40000 1>;
> > +	};
> > +
> > +	chosen {
> > +		stdout-path = "serial0:115200n8";
> > +	};
> > +
> > +	gpio_keys: gpio-keys {
> > +		compatible = "gpio-keys";
> > +
> > +		button-a {
> > +			gpios = <&gpio_expander 12 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "BTN-A";
> > +			linux,code = <BTN_EAST>;
> > +		};
> > +
> > +		button-b {
> > +			gpios = <&gpio_expander 14 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "BTN-B";
> > +			linux,code = <BTN_SOUTH>;
> > +		};
> > +
> > +		button-down {
> > +			gpios = <&gpio_expander 1 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "DPAD-DOWN";
> > +			linux,code = <BTN_DPAD_DOWN>;
> > +		};
> > +
> > +		button-left {
> > +			gpios = <&gpio_expander 4 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "DPAD-LEFT";
> > +			linux,code = <BTN_DPAD_LEFT>;
> > +		};
> > +
> > +		button-right {
> > +			gpios = <&gpio_expander 0 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "DPAD-RIGHT";
> > +			linux,code = <BTN_DPAD_RIGHT>;
> > +		};
> > +
> > +		button-se {
> > +			gpios = <&gpio_expander 7 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "BTN-SELECT";
> > +			linux,code = <BTN_SELECT>;
> > +		};
> > +
> > +		button-st {
> > +			gpios = <&gpio_expander 6 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "BTN-START";
> > +			linux,code = <BTN_START>;
> > +		};
> > +
> > +		button-tl {
> > +			gpios = <&gpio_expander 2 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "BTN-L";
> > +			linux,code = <BTN_TL>;
> > +		};
> > +
> > +		button-tr {
> > +			gpios = <&gpio_expander 15 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "BTN-R";
> > +			linux,code = <BTN_TR>;
> > +		};
> > +
> > +		button-up {
> > +			gpios = <&gpio_expander 3 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "DPAD-UP";
> > +			linux,code = <BTN_DPAD_UP>;
> > +		};
> > +
> > +		button-x {
> > +			gpios = <&gpio_expander 11 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "BTN-X";
> > +			linux,code = <BTN_NORTH>;
> > +		};
> > +
> > +		button-y {
> > +			gpios = <&gpio_expander 13 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +			label = "BTN-Y";
> > +			linux,code = <BTN_WEST>;
> > +		};
> > +	};
> > +};
> > +
> > +&codec {
> > +	allwinner,audio-routing = "Speaker", "HP",
> > +				  "MIC1", "Mic",
> > +				  "Mic", "HBIAS";
> > +	allwinner,pa-gpios = <&pio 5 6 (GPIO_ACTIVE_HIGH | GPIO_PULL_UP)>;
> 
> Can you please specify the pin number (PF6) as a comment here, as done
> in the other DTs? Because reading this "5 6" is not very intuitive.
> Same for the other &pio GPIO references.
> 

Will do. Thank you.

> > +	status = "okay";
> > +};
> > +
> > +&ehci {
> > +	status = "okay";
> > +};
> > +
> > +&i2c0 {
> > +	status = "okay";
> > +
> > +	gpio_expander: gpio@20 {
> > +		compatible = "nxp,pcal6416";
> > +		reg = <0x20>;
> > +		gpio-controller;
> > +		#gpio-cells = <2>;
> > +		#interrupt-cells = <2>;
> > +		interrupt-controller;
> > +		interrupt-parent = <&pio>;
> > +		interrupts = <1 3 IRQ_TYPE_EDGE_BOTH>;
> > +		vcc-supply = <&reg_vcc3v3>;
> > +	};
> > +
> > +	axp209: pmic@34 {
> > +		reg = <0x34>;
> > +		interrupt-parent = <&pio>;
> > +		interrupts = <1 5 IRQ_TYPE_EDGE_FALLING>;
> > +	};
> > +
> > +	pcf8563: rtc@51 {
> > +		compatible = "nxp,pcf8563";
> > +		reg = <0x51>;
> > +	};
> > +};
> > +
> > +#include "axp209.dtsi"
> > +
> > +/* Out of alphabetical order for dependencies sake. */
> 
> I think you don't need this comment here.
> 

Okay, I'll remove it. It just looked out of order so I thought
explaining it was warranted.

> > +&battery_power_supply {
> > +	status = "okay";
> > +};
> > +
> > +&mmc0 {
> > +	broken-cd;
> > +	bus-width = <4>;
> > +	disable-wp;
> > +	vmmc-supply = <&reg_vcc3v3>;
> > +	vqmmc-supply = <&reg_vcc3v3>;
> > +	status = "okay";
> > +};
> > +
> > +&ohci {
> > +	status = "okay";
> > +};
> > +
> > +&pio {
> > +	vcc-pb-supply = <&reg_vcc3v3>;
> > +	vcc-pc-supply = <&reg_vcc3v3>;
> > +	vcc-pf-supply = <&reg_vcc3v3>;
> > +	vcc-pg-supply = <&reg_vcc3v3>;
> > +
> > +	spi0_3wire_pins: spi0-3wire-pins {
> > +		pins = "PC1", "PC2", "PC3";
> 
> Huh, interesting, is that because the display chip is purely
> unidirectional? In any case, can you add a comment saying what's going
> on, I guess: "MISO is not connected"?
> And I wonder if "3 wire mode" is somewhat misleading here, since this
> seems to refer to bidirectional SPI over a combined MISO/MOSI line, and
> requires explicit controller support? Which the Allwinner controllers
> lack?
> I think for the node and alias name that's fine, but a comment would
> clear things up.

Yeah, I originally thought that's what it was doing but nope. The MOSI,
CLK, and CS pins are hooked up to the MOSI, CLK, and CS pins of the
panel. The MISO pin is hooked up to the RS/DC pin (which switches
between command and data mode). There's also a reset pin as well as a
tear pin that's hooked to PB1. I'll rename the node to be less
confusing and leave a comment as such. If it turns out we need the tear
pin I'll either find a way to add it to the generic driver I'm using
now or write a driver just for this panel.

> 
> The rest looks fine, though we would still need to fix the USB PHY
> issue.

Yeah, crazy thing that defining it correctly (where OHCI, EHCI and OTG
are all on <usbphy 0>) actually appears to cause it to not work. I got
a single message in my dmesg log that it was setting it to host mode,
and when I unplugged it the extcon kept saying it was host. However,
when I didn't have the phy defined on OHCI/EHCI or I set it incorrect
as <usbphy 1> it worked. Of course defining ldo1 correctly also broke
stuff...

> 
> Cheers,
> Andre
> 
> > +		function = "spi0";
> > +	};
> > +};
> > +
> > +&pwm {
> > +	pinctrl-0 = <&pwm0_pin>;
> > +	pinctrl-names = "default";
> > +	status = "okay";
> > +};
> > +
> > +/* DCDC2 wired into vdd-cpu, vdd-sys, and vdd-ephy. */
> > +&reg_dcdc2 {
> > +	regulator-always-on;
> > +	regulator-max-microvolt = <1250000>;
> > +	regulator-min-microvolt = <1250000>;
> > +	regulator-name = "vdd-cpu";
> > +};
> > +
> > +/* DCDC3 wired into every 3.3v input that isn't the RTC. */
> > +&reg_dcdc3 {
> > +	regulator-always-on;
> > +	regulator-max-microvolt = <3300000>;
> > +	regulator-min-microvolt = <3300000>;
> > +	regulator-name = "vcc-io";
> > +};
> > +
> > +/*
> > + * LDO1 wired into RTC, voltage is hard-wired at 3.3v and cannot be
> > + * software modified. Note that setting voltage here to 3.3v for accuracy
> > + * sake causes an issue with the driver that causes it to fail to probe
> > + * because of a voltage constraint in the driver.
> > + */
> > +&reg_ldo1 {
> > +	regulator-always-on;
> > +	regulator-name = "vcc-rtc";
> > +};
> > +
> > +/* LDO2 wired into VCC-PLL and audio codec. */
> > +&reg_ldo2 {
> > +	regulator-always-on;
> > +	regulator-max-microvolt = <3000000>;
> > +	regulator-min-microvolt = <3000000>;
> > +	regulator-name = "vcc-pll";
> > +};
> > +
> > +/* LDO3, LDO4, and LDO5 unused. */
> > +&reg_ldo3 {
> > +	status = "disabled";
> > +};
> > +
> > +&reg_ldo4 {
> > +	status = "disabled";
> > +};
> > +
> > +&spi0 {
> > +	pinctrl-0 = <&spi0_3wire_pins>;
> > +	pinctrl-names = "default";
> > +	status = "okay";
> > +
> > +	display@0 {
> > +		compatible = "saef,sftc154b", "panel-mipi-dbi-spi";
> > +		reg = <0>;
> > +		backlight = <&backlight>;
> > +		dc-gpios = <&pio 2 0 GPIO_ACTIVE_HIGH>;
> > +		reset-gpios = <&pio 1 2 GPIO_ACTIVE_HIGH>;
> > +		spi-max-frequency = <100000000>;
> > +
> > +		height-mm = <39>;
> > +		width-mm = <39>;
> > +
> > +		/* Set hb-porch to compensate for non-visible area */
> > +		panel-timing {
> > +			hactive = <240>;
> > +			vactive = <240>;
> > +			hback-porch = <80>;
> > +			vback-porch = <0>;
> > +			clock-frequency = <0>;
> > +			hfront-porch = <0>;
> > +			hsync-len = <0>;
> > +			vfront-porch = <0>;
> > +			vsync-len = <0>;
> > +		};
> > +	};
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-0 = <&uart0_pb_pins>;
> > +	pinctrl-names = "default";
> > +	status = "okay";
> > +};
> > +
> > +&usb_otg {
> > +	dr_mode = "otg";
> > +	status = "okay";
> > +};
> > +
> > +&usb_power_supply {
> > +	status = "okay";
> > +};
> > +
> > +&usbphy {
> > +	usb0_id_det-gpios = <&pio 6 5 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> > +	status = "okay";
> > +};
> 

Thank you,
Chris



[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