Re: [PATCH 3/3] arm64: dts: qcom: add basic devicetree for Ayaneo Pocket S2 gaming console

[Neil Armstrong <neil.armstrong@linaro.org>]

On 1/22/26 10:15, Konrad Dybcio wrote:
> On 1/21/26 5:40 PM, Neil Armstrong wrote:
>> From: KancyJoe <kancy2333@outlook.com>
>>
>> Add initial Device Tree for the Ayaneo Pocket S2 gaming console based
>> on the Qualcomm Snapdragon 8 Gen 3 platform.
>>
>> The design is similar to a phone wihout the modem, the game control
>> is handled via a standalone controller connected to a PCIe USB
>> controller.
>>
>> Display support will be added in a second time.
>>
>> Signed-off-by: KancyJoe <kancy2333@outlook.com>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
> 
> [...]
> 
>> +	fan: pwm-fan {
> 
> I'd call it fan {} but gray/grey
> 
>> +		status = "okay";
> 
> You can drop this line (nothing disables it)

Oops will remove

> 
>> +		compatible = "pwm-fan";
>> +
>> +		interrupt-parent = <&tlmm>;
>> +		interrupts = <14 IRQ_TYPE_EDGE_FALLING>;
> 
> interrupts-extended looks neater

Ack

> 
>> +
>> +		pinctrl-0 = <&fan_pwr_active>,
>> +			    <&pwm_fan_ctrl_default>,
>> +			    <&fan_int_active>;
>> +		pinctrl-1 = <&fan_pwr_sleep>;
> 
> fan-pwr looks like an EN pin of a GPIO-controlled regulator

Probably, will model it as a regulator

> 
>> +		pinctrl-names = "default",
>> +				"sleep";
>> +
>> +		pwms = <&pm8550_pwm 3 50000>;
>> +
>> +		#cooling-cells = <2>;
>> +		cooling-levels = <0 16 32 45 60 80 105 130 155 180 205 230 255>;
> 
> Does this come from a preexisting map?

Kancy ?

> 
>> +	};
>> +
>> +	gpio-keys {
>> +		compatible = "gpio-keys";
>> +
>> +		pinctrl-0 = <&volume_up_n>;
>> +		pinctrl-names = "default";
>> +
>> +		key-volume-up {
>> +			label = "Volume Up";
>> +			linux,code = <KEY_VOLUMEUP>;
>> +			gpios = <&pm8550_gpios 6 GPIO_ACTIVE_LOW>;
>> +			debounce-interval = <15>;
>> +			linux,can-disable;
>> +			wakeup-source;
>> +		};
>> +	};
>> +
>> +	pmic-glink {
>> +		compatible = "qcom,sm8650-pmic-glink",
>> +			     "qcom,sm8550-pmic-glink",
>> +			     "qcom,pmic-glink";
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
>> +
>> +		orientation-gpios = <&tlmm 29 GPIO_ACTIVE_HIGH>;
>> +
>> +		connector@0 {
>> +			compatible = "usb-c-connector";
>> +			reg = <0>;
>> +
>> +			power-role = "dual";
>> +			data-role = "dual";
>> +			self-powered;
> 
> Is this property interpreted at all by our setup?

Kancy did add self-powered, but it does charging so it should be dropped.

> 
> [...]
> 
>> +	sound {
>> +		compatible = "qcom,sm8650-sndcard", "qcom,sm8450-sndcard";
>> +		model = "SM8650-APS2";
>> +		audio-routing = "SpkrLeft IN", "WSA_SPK1 OUT",
>> +				"SpkrRight IN", "WSA_SPK2 OUT",
>> +				"IN1_HPHL", "HPHL_OUT",
>> +				"IN2_HPHR", "HPHR_OUT",
>> +				"DMIC1", "MIC BIAS1",
>> +				"DMIC2", "MIC BIAS2",
>> +				"AMIC2", "MIC BIAS2",
>> +				"TX SWR_INPUT1", "ADC2_OUTPUT",
>> +				"TX SWR_INPUT7", "DMIC1_OUTPUT",
>> +				"TX SWR_INPUT8", "DMIC2_OUTPUT";
>> +
>> +		wcd-playback-dai-link {
>> +			link-name = "WCD Playback";
>> +
>> +			cpu {
>> +				sound-dai = <&q6apmbedai RX_CODEC_DMA_RX_0>;
>> +			};
>> +
>> +			codec {
>> +				sound-dai = <&wcd939x 0>,
>> +					    <&swr1 0>,
>> +					    <&lpass_rxmacro 0>;
>> +			};
> 
> 'co'dec < 'cp'u
> 
> [...]
> 
>> +	wcd939x: audio-codec {
> 
> 'a'udio-codec should be way higher

ack

> 
> [...]
> 
>> +	thermal-zones {
>> +		cpu2-top-thermal {
>> +			trips {
>> +				cpu2_active: cpu2-active {
>> +					temperature = <38000>;
>> +					hysteresis = <2000>;
>> +					type = "active";
> 
> This is shaky.. let's perhaps reference each thermal zone that you want
> to extend with a label.. Or maybe a pair of labels for trips/cooling-maps
> per zone?

Yep, will clean that by adding labels


> 
> [...]
> 
>> +&pcieport1 {
>> +	pinctrl-0 = <&upd720201_active>;
> 
> Is this a regulator?

There's s 3 gpios, the 3 are required to have the controller to show up,
it could be 3 regulators and a reset line, I don't know. The controller
needs 1.05v and 3.3v plus a reset signal, but I don't know which one
is which and if it's really regulators...

> 
>> +	pinctrl-names = "default";
>> +
>> +	/* Renesas μPD720201 PCIe USB3.0 HOST CONTROLLER */
> 
> DON'T SCREAM! :P
> 
>> +	usb-controller@0 {
>> +		compatible = "pci1912,0014";
>> +		reg = <0x10000 0x0 0x0 0x0 0x0>;
>> +
>> +		pinctrl-0 = <&gamepad_pwr_en>;
>> +		pinctrl-names = "default";
> 
> Is there a hub connected to it? Or does it go directly to the
> aforementioned (game) controller?

Directly connected

> 
> [...]
> 
>> +&pm8550_pwm {
>> +	status = "okay";
>> +
>> +	multi-led {
>> +		color = <LED_COLOR_ID_RGB>;
>> +		function = LED_FUNCTION_STATUS;
>> +
>> +		#address-cells = <1>;
>> +		#size-cells = <0>;
> 
> Would a label="xyz" be useful here?

Probably yes

> 
> [...]
> 
>> +&tlmm {
>> +	/* Reserved I/Os for NFC */
>> +	gpio-reserved-ranges = <32 4>,  <36 1>, <38 6>, <74 1>;
> 
> double space
> 
> Are they all for NFC, are they all required?

They are reserved, usually for NFC to be used by the secure enclave,
but we don't have nfc but they are still reserved...

> 
> [...]
> 
>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> index 07ae74851621..fcd5a1a45803 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> @@ -3917,7 +3917,7 @@ opp-32000000-4 {
>>   				};
>>   			};
>>   
>> -			pcie@0 {
>> +			pcieport1: pcie@0 {
> 
> pcie1_port0, please

Ack

> 
> Konrad

Thanks,
Neil