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

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Maya Matuszczyk <maccraft123mc@gmail.com>,
	Chris Morgan <macroalpha82@gmail.com>
Cc: devicetree <devicetree@vger.kernel.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Peter Geis <pgwipeout@gmail.com>, Chris Healy <cphealy@gmail.com>,
	Chris Morgan <macromorgan@hotmail.com>
Subject: Re: [PATCH v2 3/3] arm64: dts: rockchip: add Anbernic RG353P and RG503
Date: Tue, 23 Aug 2022 15:28:03 +0300	[thread overview]
Message-ID: <da8df5f3-a69f-6807-3104-37e6c92e5788@linaro.org> (raw)
In-Reply-To: <CAO_MupKkmbEjEqkrUQ2DtY=SA4ULv8CLVWPD92hxrMPuRYsS+w@mail.gmail.com>

On 20/08/2022 11:40, Maya Matuszczyk wrote:
> Hello,
> 
> sob., 20 sie 2022 o 00:26 Chris Morgan <macroalpha82@gmail.com> napisał(a):
>>
>> 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 {
> I don't think just having the buttons numbered sequentially
> is the best course of action, but this preserves the GPIO
> ordering while other options don't...
> I'm thinking about either having them named after
> their function, or named after what they're labeled
> on the PCB of the device.
> Can any of DT maintainers give their input on this?

Names should be generic and button-1 is a nice generic name. Adding
specific prefix/suffix makes something less generic, more specific, thus
I prefer without prefixes/suffixes. I don't mind them, though for the
cases it brings value. Here the role is clearly described by label, so
why adding suffix?

Best regards,
Krzysztof

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