Message-ID: <d455a8d0-0a43-4bb5-8592-f22f1835a3c6@linaro.org>
Date: Wed, 21 May 2025 14:43:03 +0200
From: neil.armstrong@...aro.org
To: Pengyu Luo <mitltlatltl@...il.com>, konrad.dybcio@....qualcomm.com
Cc: andersson@...nel.org, conor+dt@...nel.org, devicetree@...r.kernel.org,
 konradybcio@...nel.org, krzk+dt@...nel.org, linux-arm-msm@...r.kernel.org,
 linux-kernel@...r.kernel.org, robh@...nel.org
Subject: Re: [PATCH 2/2] arm64: dts: qcom: sm8650: Add support for Oneplus Pad
 Pro (caihong)

On 21/05/2025 10:37, Pengyu Luo wrote:
> On Wed, May 21, 2025 at 5:54 AM Konrad Dybcio <konrad.dybcio@....qualcomm.com> wrote:
>> On 5/20/25 6:42 PM, Pengyu Luo wrote:
>>> The OnePlus Pad Pro is an Android tablet based on the Qualcomm SM8650
>>> platform. Its device codename is "caihong". This patch adds an initial
>>> devicetree for basic functionality.
>>>
>>> Currently working components include:
>>> - Backlight
>>> - Bluetooth
>>> - Battery charging (up to 5v 0.5a) & reporting via pmic-glink (There
>>> are many unknown notifications)
>>> - Display panel ([1])
>>> - Keyboard (via BT)
>>> - Power key & volume keys
>>> - Touchscreen & stylus ([2])
>>> - USB Type-c port
>>> - UFS storage
>>> - Wi-Fi
>>>
>>> The following components are currently non-functional:
>>> - Audio
>>> - Cameras
>>> - Charging pump (dual sc8547)
>>> - Keyboard (via pogo pin)
>>> - Stylus wireless charger (cps8601)
>>> - UCSI over GLINK (PPM init fails)
>>>
>>> [1]: The panel is a dual-DSI, dual-DSC display that requires setting
>>>       'slice_per_pkt = 2' in the DPU configuration. The panel driver
>>>       will be submitted separately later.
>>> [2]: Touchscreen/stylus driver available at:
>>>       https://github.com/OnePlusOSS/android_kernel_modules_and_devicetree_oneplus_sm8650/blob/oneplus/sm8650_v_15.0.0_pad_pro/vendor/oplus/kernel/touchpanel/oplus_touchscreen_v2/Novatek/NT36532_noflash/nvt_drivers_nt36532_noflash.c
>>>       The downstream driver has been ported and tested locally, but
>>>       requires cleanup, it may be submitted separately later.
>>
>> I have a Lenovo Tab P11 with a nt36523w (-23, not -32) for which I have once
>> poked at the driver for.. I see the driver you posted mentions -23 as well,
>> please keep me in the loop if you're going to upstream it
>>
> 
> I see. Actually, they share the most part of nt36xxx, but with
> different memory maps. See
> https://github.com/MiCode/Xiaomi_Kernel_OpenSource/blob/elish-r-oss/drivers/input/touchscreen/nt36xxx/nt36xxx_mem_map.h
> 
>> [...]
>>
>>> +             /*
>>> +              * This memory region is required to initialize the backlight
>>> +              * and display for bootloader. Normally, this region is not
>>> +              * needed. However, due to limitations in the current mainline
>>> +              * KTZ8866 driver, dual backlight ICs cannot be properly
>>> +              * initialized.
>>> +              *
>>> +              * A workaround involving secondary registration was proposed,
>>> +              * but rejected by reviewers. This reserved region is kept as
>>> +              * a temporary solution until a proper initialization method
>>> +              * that satisfies upstream requirements is found.
>>> +              */
>>> +             splash_region {
>>> +                     reg = <0 0xd5100000 0 0x2b00000>;
>>> +                     no-map;
>>> +             };
>>
>> I assume this means "if the bootloader sees /reserved-memory/splash_region,
>> it keeps the display online" - let's not do that, as underscores are not
>> allowed in node names (kernel coding style, not dt syntax)
>>
> 
> Right, without it, BL won't initialize backlight and display. We need
> it to initialize backlight here since mainline ktz8866 won't program
> partial registers properly. If there is no other workaround, I will
> remove it to keep kernel coding style.

Can't you add a simple-framebuffer for v1 and drop all the DSI stuff until
you figured out the backlight and upstreamed the panel driver ?

Neil

> 
>>> +     };
>>> +
>>> +     /* No Modem */
>>> +     smp2p-modem {
>>> +             status = "disabled";
>>> +     };
>>
>> There shouldn't be any harm in keeping this node enabled
>>
> 
> Ack
> 
>> [...]
>>
>>> +
>>> +     vph_pwr: vph-pwr-regulator {
>>> +             compatible = "regulator-fixed";
>>> +
>>> +             regulator-name = "vph_pwr";
>>> +             regulator-min-microvolt = <3700000>;
>>> +             regulator-max-microvolt = <3700000>;
>>> +
>>> +             regulator-always-on;
>>> +             regulator-boot-on;
>>> +     };
>>> +
>>> +     wcn7850-pmu {
>>> +             compatible = "qcom,wcn7850-pmu";
>>> +
>>> +             pinctrl-names = "default";
>>> +             pinctrl-0 = <&wlan_en>, <&bt_default>;
>>
>> property-n
>> property-names
>>
>> please
>>
> 
> Ack
> 
>> [...]
>>
>>> +&i2c_hub_0 {
>>> +     clock-frequency = <400000>;
>>> +
>>> +     status = "okay";
>>> +
>>> +     /* sc8547-charger-secondary@6F */
>>> +};
>>> +
>>> +&i2c_hub_2 {
>>> +     clock-frequency = <400000>;
>>> +
>>> +     status = "okay";
>>> +
>>> +     /* sc8547-charger-primary@6F */
>>> +};
>>> +
>>> +&i2c_hub_3 {
>>> +     status = "okay";
>>> +
>>> +     /* pencil-wireless-charger-cps8601@41 */
>>> +};
>>> +
>>> +&i2c_hub_4 {
>>> +     clock-frequency = <400000>;
>>> +
>>> +     status = "okay";
>>> +
>>> +     /* awinic,aw88261_smartpa @34,35,37 */
>>
>> We have drivers for these!
>>
>> sound/soc/codecs/aw88261.c
>>
> 
> I noticed that. But I have not looked into the sound yet. I may add
> the nodes after test. BTW the mainline one is quite simple compared
> to the downstream, doubting if it is really working
> https://github.com/OnePlusOSS/android_kernel_modules_and_devicetree_oneplus_sm8650/tree/oneplus/sm8650_v_15.0.0_pad_pro/vendor/oplus/kernel/audio/codecs/aw882xx
> 
>>> +};
>>> +
>>> +&i2c_hub_7 {
>>> +     clock-frequency = <400000>;
>>> +
>>> +     status = "okay";
>>> +
>>> +     /* awinic,aw88261_smartpa @34,35,37 */
>>> +};
>>> +
>>> +&i2c2 {
>>> +     status = "okay";
>>> +
>>> +     /* secondary kinetic,ktz8866@11 */
>>
>> You can describe it, the driver sets some nonzero default brightness
>>
> 
> But the backlight framework won't index them, which causes sysfs
> collision when the second instance is registering.
> 
> Best wishes,
> Pengyu
> 

Powered by blists - more mailing lists