Re: [PATCH v2 3/3] arm64: dts: rockchip: add Anbernic RG353P and RG503

From: Chris Morgan <macromorgan@hotmail.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Chris Morgan <macroalpha82@gmail.com>,
	devicetree@vger.kernel.org, linux-rockchip@lists.infradead.org,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	heiko@sntech.de, pgwipeout@gmail.com, cphealy@gmail.com
Subject: Re: [PATCH v2 3/3] arm64: dts: rockchip: add Anbernic RG353P and RG503
Date: Tue, 23 Aug 2022 07:40:22 -0500	[thread overview]
Message-ID: <SN6PR06MB534215A808939107AFDE5CFEA5709@SN6PR06MB5342.namprd06.prod.outlook.com> (raw)
In-Reply-To: <273e27e6-4def-2424-b838-de217f10e4a3@linaro.org>

On Tue, Aug 23, 2022 at 03:26:07PM +0300, Krzysztof Kozlowski wrote:
> On 20/08/2022 01:25, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Anbernic RG353P and RG503 are both RK3566 based handheld gaming devices
> > from Anbernic.
> > 
> > Both devices have:
> >  - 2 SDMMC slots.
> >  - A Realtek rtl8821cs WiFi/Bluetooth adapter.
> >  - A mini HDMI port.
> >  - A USB C host port and a USB C otg port (currently only working as
> >    device).
> >  - Multiple GPIO buttons and a single ADC button.
> >  - Dual analog joysticks controlled via a GPIO mux.
> >  - A headphone jack with amplified stereo speakers via a SGM4865 amp.
> >  - A PWM based vibrator for force feedback.
> > 
> > The RG353P has:
> >  - 2GB LPDDR4 RAM.
> >  - A 32GB eMMC.
> >  - A 3.5 inch 640x480 4-lane DSI panel of unknown origin with an i2c
> >    controlled touchscreen (touchscreen is a Hynitron CST340).
> > 
> > The RG503 has:
> >  - 1GB LPDDR4 RAM.
> >  - A 5 inch 960x544 AMOLED 2-lane DSI/DBI panel manufactured by Samsung
> >    with part number ams495qa04. Data for this panel is provided via the
> >    DSI interface, however commands are sent via a 9-bit 3-wire SPI
> >    interface. The MISO pin of SPI3 of the SOC is wired to the input of
> >    the panel, so it must be bitbanged.
> > 
> > This devicetree enables the following hardware:
> >  - HDMI (plus audio).
> >  - Analog audio, including speakers.
> >  - All buttons.
> >  - All SDMMC/eMMC/SDIO controllers.
> >  - The ADC joysticks (note a pending patch is required to use them).
> >  - WiFi/Bluetooth (note out of tree drivers are required).
> >  - The PWM based vibrator motor.
> > 
> > The following hardware is not enabled:
> >  - The display panels (drivers are being written and there are issues
> >    with the upstream DSI and VOP2 subsystems).
> >  - Battery (driver pending).
> >  - Touchscreen on the RG353P (note the i2c2 bus is enabled for it).
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  arch/arm64/boot/dts/rockchip/Makefile         |   2 +
> >  .../dts/rockchip/rk3566-anbernic-rg353p.dts   | 103 +++
> >  .../dts/rockchip/rk3566-anbernic-rg503.dts    |  93 ++
> >  .../dts/rockchip/rk3566-anbernic-rgxx3.dtsi   | 821 ++++++++++++++++++
> >  4 files changed, 1019 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353p.dts
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg503.dts
> >  create mode 100644 arch/arm64/boot/dts/rockchip/rk3566-anbernic-rgxx3.dtsi
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> > index ef79a672804a..1402274a78a0 100644
> > --- a/arch/arm64/boot/dts/rockchip/Makefile
> > +++ b/arch/arm64/boot/dts/rockchip/Makefile
> > @@ -57,6 +57,8 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-rockpro64.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-sapphire.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399-sapphire-excavator.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3399pro-rock-pi-n10.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg353p.dtb
> > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-anbernic-rg503.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.1.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-pinenote-v1.2.dtb
> >  dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3566-quartz64-a.dtb
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353p.dts b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353p.dts
> > new file mode 100644
> > index 000000000000..f9333ed1ecc7
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/rockchip/rk3566-anbernic-rg353p.dts
> > @@ -0,0 +1,103 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/linux-event-codes.h>
> > +#include <dt-bindings/pinctrl/rockchip.h>
> > +#include "rk3566-anbernic-rgxx3.dtsi"
> > +
> > +/ {
> > +	model = "RG353P";
> > +	compatible = "anbernic,rg353p", "rockchip,rk3566";
> > +
> > +	aliases {
> > +		mmc0 = &sdhci;
> > +		mmc1 = &sdmmc0;
> > +		mmc2 = &sdmmc1;
> > +		mmc3 = &sdmmc2;
> > +	};
> > +
> > +	backlight: backlight {
> > +		compatible = "pwm-backlight";
> > +		power-supply = <&vcc_sys>;
> > +		pwms = <&pwm4 0 25000 0>;
> > +	};
> > +};
> > +
> > +&gpio_keys_control {
> > +	button-5 {
> > +		gpios = <&gpio3 RK_PA5 GPIO_ACTIVE_LOW>;
> > +		label = "DPAD-LEFT";
> > +		linux,code = <BTN_DPAD_RIGHT>;
> > +	};
> > +
> > +	button-6 {
> > +		gpios = <&gpio3 RK_PA6 GPIO_ACTIVE_LOW>;
> > +		label = "DPAD-RIGHT";
> > +		linux,code = <BTN_DPAD_LEFT>;
> > +	};
> > +
> > +	button-9 {
> > +		gpios = <&gpio3 RK_PB3 GPIO_ACTIVE_LOW>;
> > +		label = "TR";
> > +		linux,code = <BTN_TR2>;
> > +	};
> > +
> > +	button-10 {
> > +		gpios = <&gpio3 RK_PB4 GPIO_ACTIVE_LOW>;
> > +		label = "TR2";
> > +		linux,code = <BTN_TR>;
> > +	};
> > +
> > +	button-14 {
> > +		gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_LOW>;
> > +		label = "WEST";
> > +		linux,code = <BTN_WEST>;
> > +	};
> > +
> > +	button-15 {
> > +		gpios = <&gpio3 RK_PC2 GPIO_ACTIVE_LOW>;
> > +		label = "EAST";
> > +		linux,code = <BTN_EAST>;
> > +	};
> > +};
> > +
> > +&i2c0 {
> > +	/* This hardware is physically present but unused. */
> > +	cw2015@62 {
> 
> Node names should be generic.

Understood, I was just copying an existing devicetree. I'll make it
generic.

> 
> > +		compatible = "cellwise,cw2015";
> > +		reg = <0x62>;
> > +		status = "disabled";
> > +	};
> > +};
> > +
> > +&i2c2 {
> > +	pintctrl-names = "default";
> > +	pinctrl-0 = <&i2c2m1_xfer>;
> > +	status = "okay";
> > +};
> > +
> 
> (...)
> 
> > +
> > +&hdmi_sound {
> > +	status = "okay";
> > +};
> > +
> > +&i2c0 {
> > +	status = "okay";
> > +
> > +	rk817: pmic@20 {
> > +		compatible = "rockchip,rk817";
> > +		reg = <0x20>;
> > +		interrupt-parent = <&gpio0>;
> > +		interrupts = <RK_PA3 IRQ_TYPE_LEVEL_LOW>;
> > +		clock-output-names = "rk808-clkout1", "rk808-clkout2";
> > +		clock-names = "mclk";
> > +		clocks = <&cru I2S1_MCLKOUT_TX>;
> > +		assigned-clocks = <&cru I2S1_MCLKOUT_TX>;
> > +		assigned-clock-parents = <&cru CLK_I2S1_8CH_TX>;
> > +		#clock-cells = <1>;
> > +		#sound-dai-cells = <0>;
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&i2s1m0_mclk>, <&pmic_int_l>;
> > +		wakeup-source;
> > +
> > +		vcc1-supply = <&vcc_sys>;
> > +		vcc2-supply = <&vcc_sys>;
> > +		vcc3-supply = <&vcc_sys>;
> > +		vcc4-supply = <&vcc_sys>;
> > +		vcc5-supply = <&vcc_sys>;
> > +		vcc6-supply = <&vcc_sys>;
> > +		vcc7-supply = <&vcc_sys>;
> > +		vcc8-supply = <&vcc_sys>;
> > +		vcc9-supply = <&dcdc_boost>;
> > +
> > +		regulators {
> > +			vdd_logic: DCDC_REG1 {
> 
> I commented here and there was no feedback, so please implement the change.
> 

Sorry I didn't explicitly call it out, I only made a note in the
revision notes. The regulator names for the RK808 series are hard-coded, so
I can't change them without modifying the driver (and every existing DTS that
uses them).

> 
> Best regards,
> Krzysztof

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip