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

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

 



On Sat, Aug 19, 2023 at 11:03:52PM +0100, Andre Przywara wrote:
> On Fri, 18 Aug 2023 22:21:05 -0500
> Chris Morgan <macroalpha82@xxxxxxxxx> wrote:
> 
> Hi Chris,
> 
> thanks for the update!
> 
> > 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.
> > 
> > The SPI display is currently unsupported, as it will either require
> > a new tinydrm driver or changes to the staging fbtft driver to support.
> > I plan on working on a tinydrm driver to properly support it. The USB-C
> > port currently only works as a peripheral port, however in the BSP
> > kernel it also works in host mode allowing included USB-C headphones
> > with a built-in DAC to work.
> > 
> > Working:
> > - SDMMC
> > - UART (for debugging)
> > - Buttons
> > - Charging/battery/PMIC
> > - Speaker
> > - USB Gadget
> > 
> > Not working:
> > - Display
> > - USB Host
> > 
> > Signed-off-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
> > ---
> >  arch/arm/boot/dts/allwinner/Makefile          |   1 +
> >  .../allwinner/sun8i-v3s-anbernic-rg-nano.dts  | 219 ++++++++++++++++++
> >  2 files changed, 220 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..c49b5431d04e
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/allwinner/sun8i-v3s-anbernic-rg-nano.dts
> > @@ -0,0 +1,219 @@
> > +// 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";
> > +		pwms = <&pwm 0 40000 1>;
> > +		brightness-levels = <0 1 2 3 8 14 21 32 46 60 80 100>;
> > +		default-brightness-level = <11>;
> > +		power-supply = <&reg_vcc5v0>;
> > +	};
> > +
> > +	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)>;
> > +	status = "okay";
> > +};
> > +
> > +&cpu0 {
> > +	clock-frequency = <1296000>;
> 
> I understand that the kernel complains when this is missing, but I
> think this property is some ancient legacy, as there is no such thing
> as a fixed CPU frequency anymore. That becomes evident with the OPPs
> below. So please remove it.

Okay, I'll remove it. I always try to make sure dmesg log is as error
free as possible, and yes, that's the only reason I put this here.

> 
> > +	clock-latency = <244144>;
> > +	operating-points = <
> > +			/* kHz    uV */
> > +			1296000 1200000
> > +			1008000 1200000
> > +			864000  1200000
> > +			720000  1100000
> > +			480000  1000000>;
> 
> Don't you need a cpu-supply property to point to the regulator in
> charge of providing the core voltage? Otherwise I don't see how the
> kernel would be able to adjust the voltage, to program the OPPs.
> 

I'll add cpu-supply. Based on the schematic it looks like the
cpu_supply is the DCDC2. DCDC2 is also hooked up to some other inactive
pins according to the schematic but I can't see those pins on the
SoC datasheet (so maybe it's just a schematic quirk?).

> > +};
> > +
> > +&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"
> > +
> > +&battery_power_supply {
> > +	status = "okay";
> > +};
> > +
> > +&mmc0 {
> > +	broken-cd;
> > +	bus-width = <4>;
> > +	disable-wp;
> > +	vmmc-supply = <&reg_vcc3v3>;
> > +	vqmmc-supply = <&reg_vcc3v3>;
> > +	status = "okay";
> > +};
> > +
> > +&pio {
> > +	vcc-pb-supply = <&reg_vcc3v3>;
> > +	vcc-pc-supply = <&reg_vcc3v3>;
> > +	vcc-pf-supply = <&reg_vcc3v3>;
> > +	vcc-pg-supply = <&reg_vcc3v3>;
> > +};
> > +
> > +&pwm {
> > +	pinctrl-0 = <&pwm0_pins>;
> > +	pinctrl-names = "default";
> > +	status = "okay";
> > +};
> > +
> > +&reg_dcdc2 {
> 
> Any reason your dropped the regulator-name property? This would help to
> identify what this regulator is used for and would explain why it needs
> to be always on.
> In v1 this was "vdd-cpu-sys-ephy", are you sure about this name? I
> guess it supplies the CPU, but I wonder if there are other users of
> this rail, which would possibly throw a spanner into DVFS. So what's the
> "ephy" doing here? And can you adjust the voltage, if this also driving
> VDD-SYS? What does the BSP kernel do?
> 

This rail appears to supply the CPU (vdd-cpu) SYS (vdd-sys) and
vdd-ephy. The ratings for each of these according to the datasheet are
between -0.3v and 1.3v (CPU/SYS) or 1.4v (for the EPHY). The BSP kernel
has this as variable between 1v and 1.25v, but it doesn't appear to
have DVFS enabled. I can change the range back to 1/1.25v since it
supplies the CPU so we can enable DVFS; the datasheet isn't clear
what vdd-sys or vdd-ephy do, but in the case of EPHY we aren't using
that feature at least (and these voltages are within range).

Do you think I should keep the cpu-supply and frequency stuff in place,
or should I drop it all instead?

> > +	regulator-always-on;
> > +	regulator-max-microvolt = <1250000>;
> > +	regulator-min-microvolt = <1250000>;
> > +};
> > +
> > +&reg_dcdc3 {
> 
> Same here, please provide a speaking name, or a comment explaining
> why this must be always on. There is no need to enumerate every user,
> just "vdd-3v3" seems to be a common name for that regulator.
> 

I'll go with vcc-io like the BSP, unless you feel strongly otherwise.
The regulator appears to power all the different IO pins on the SoC
and basically everything else that uses 3.3v that isn't the RTC.

> > +	regulator-always-on;
> > +	regulator-max-microvolt = <3300000>;
> > +	regulator-min-microvolt = <3300000>;
> > +};
> > +
> > +&reg_ldo2 {
> 
> Same here, name or comment, please.
> 

You got it.

> > +	regulator-always-on;
> > +	regulator-max-microvolt = <3000000>;
> > +	regulator-min-microvolt = <3000000>;
> > +};
> > +
> > +&spi0 {
> 
> Can you add a comment here mentioning the not-yet-supported display?
> And you should specify the pins used here.
> 

The SPI pins are defined in the main v3s file. Additionally I have a
TE pin (GPIO PB1) and a RESET pin for the panel (GPIO PB2). The RESET
pin looks like it has an external pull-up to 3.3v. The RS pin on the
panel is wired to MISO, otherwise CS on the panel goes to CS on the
SoC, CLK to CLK, and MOSI to MOSI. Everything but the RESET panel
is pulled to ground (I think, still not perfect at schematics).

> Cheers,
> Andre
> 
> 
> > +	status = "okay";
> > +};
> > +
> > +&uart0 {
> > +	pinctrl-0 = <&uart0_pb_pins>;
> > +	pinctrl-names = "default";
> > +	status = "okay";
> > +};
> > +
> > +&usb_otg {
> > +	dr_mode = "otg";
> > +	status = "okay";
> > +};
> > +
> > +&usb_power_supply {
> > +	status = "okay";
> > +};
> > +
> > +&usbphy {
> > +	status = "okay";
> > +};
> 

Also, I did another comparison with the BSP kernel and realized I was
missing the OHCI/EHCI and now OTG is working (at least the extcon
shows host when it has a device attached and not host when it's hooked
to my computer).

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