Message-ID:
 <SEZPR04MB6873411EA1A8B69AD45B286CA397A@SEZPR04MB6873.apcprd04.prod.outlook.com>
Date: Thu, 22 Jan 2026 17:34:34 +0800
From: Kancy Joe <kancy2333@...look.com>
To: Neil Armstrong <neil.armstrong@...aro.org>,
 Konrad Dybcio <konrad.dybcio@....qualcomm.com>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>,
 Rob Clark <robin.clark@....qualcomm.com>, Dmitry Baryshkov
 <lumag@...nel.org>, Abhinav Kumar <abhinav.kumar@...ux.dev>,
 Jessica Zhang <jesszhan0024@...il.com>, Sean Paul <sean@...rly.run>,
 Marijn Suijten <marijn.suijten@...ainline.org>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 freedreno@...ts.freedesktop.org
Subject: Re: [PATCH 3/3] arm64: dts: qcom: add basic devicetree for Ayaneo
 Pocket S2 gaming console


On 1/22/2026 5:25 PM, Neil Armstrong wrote:
> On 1/22/26 10:15, Konrad Dybcio wrote:
>> On 1/21/26 5:40 PM, Neil Armstrong wrote:
>>> From: KancyJoe <kancy2333@...look.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@...look.com>
>>> Signed-off-by: Neil Armstrong <neil.armstrong@...aro.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 ?

No it is not a preexisting map. I add it(and the thermal part) myself to 
get dynamic fan speed control work. Perhaps you can also use userspace 
fan control daemon instead of hardcode it here. In android the vendor 
control the fan speed in userspace too.

Following block is what the stock fw defined. I changed the granularity 
to make fan speed (or noise actually) sounds more "smooth".

```

cooling-levels = <0 64 128 255>;

```

>
>>
>>> +    };
>>> +
>>> +    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

Powered by blists - more mailing lists