linux-kernel - Re: [PATCH v2 3/3] arm64: dts: qcom: Add support for X1-based Dell XPS 13 9345

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMcHhXp=nw8XXNFdw+a7+qcMbJVvkqzBcgZo0fombokBj2tD2Q@mail.gmail.com>
Date: Wed, 25 Sep 2024 12:05:22 +0200
From: Aleksandrs Vinarskis <alex.vinarskis@...il.com>
To: Bjorn Andersson <andersson@...nel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>, 
	Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, linux-arm-msm@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Rob Clark <robdclark@...il.com>, Peter de Kraker <peterdekraker@...to.nl>
Subject: Re: [PATCH v2 3/3] arm64: dts: qcom: Add support for X1-based Dell
 XPS 13 9345

On Wed, 25 Sept 2024 at 00:15, Bjorn Andersson <andersson@...nel.org> wrote:
>
> On Sat, Sep 21, 2024 at 06:33:33PM GMT, Aleksandrs Vinarskis wrote:
> > Initial support for Dell XPS 9345 13" 2024 (Tributo) based on X1E80100.
>
> Very nice.
>
> >
> > Working:
> > * Touchpad
> > * Keyboard (only post suspend&resume, i2c-hid patch WIP)
>
> Hitting scroll lock/unlock on a USB keyboard once fixes this issue for
> me as well. Looking forward to your WIP patch.

Thanks for your review.
Just submitted the series [3].

>
> > * eDP, with brightness control
> > * NVME
> > * USB Type-C ports in USB2/USB3 (one orientation)
> > * WiFi
> > * GPU/aDSP/cDSP firmware loading (requires binaries from Windows)
> > * Lid switch
> > * Sleep/suspend, nothing visibly broken on resume
> >
> > Not working:
> > * Speakers (WIP, pin guessing, x4 WSA8845)
> > * Microphones (WIP, pin guessing)
> > * Fingerprint Reader (WIP, USB MP with ptn3222)
> > * USB as DP/USB3 (WIP, PS8830 based)
> > * Camera
> > * Battery Info
>
> Adding the ADSP firmware gave me both battery status and info, but
> perhaps you're hitting the previously reported issue in pmic_glink?
>

Could you please share a bug report for the mentioned issue?

Were you running with [2] patch reverted or not?
Without reverting it, I cannot boot Ubuntu at all - it is spamming
qcom_battmngr errors and holding services back.
With patch reverted I do not get any battery related info, which I
guess makes sense.  I tried applying [1], however it did help.

There are a few pmic_glink related errors in dmesg, so perhaps its related.

> >
> > Should be working, but cannot be tested due to lack of hw:
> > * Higher res OLED, higher res IPS panels
>
> I tried closing the lid and opening it again (which I believe is what
> was reported to not work on the other devices), and that seems to work
> fine.
>
> > * Touchscreen
>
> See below
>
> >
> [..]
> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100-dell-tributo-13.dts b/arch/arm64/boot/dts/qcom/x1e80100-dell-tributo-13.dts
> [..]
> > +&i2c8 {
> > +     clock-frequency = <400000>;
> > +
> > +     status = "okay";
> > +
> > +     touchscreen@0 {
> > +             compatible = "hid-over-i2c";
> > +             reg = <0x0>;
>
> Make this 0x10 (and update the unit address accordingly) and we have
> touchscreen.

Awesome, thanks for testing. Will add it.
Do you have an OLED variant, or high-res IPS? Will update description
when respinning to include it.

Thinking about it, perhaps depending on the OLED/IPS variant they have
different touchscreen models with different addresses? I find it weird
that the address was 0 as per ACPI.

> > +
> > +             hid-descr-addr = <0x1>;
> > +             interrupts-extended = <&tlmm 51 IRQ_TYPE_LEVEL_LOW>;
> > +
> > +             pinctrl-0 = <&ts0_default>;
> > +             pinctrl-names = "default";
> > +     };
> > +};
> [..]
> > +&mdss_dp3 {
> > +     compatible = "qcom,x1e80100-dp";
>
> This isn't needed, right?

Indeed. Will fix it.

>
> [..]
> > +&uart21 {
>
> This fails to probe, because we don't have an alias for it, which in
> turn prevents sync_state on interconnects...
>

Indeed. Will fix it.

Thanks,
Alex

[1] https://lore.kernel.org/all/20240918-x1e-fix-pdm-pdr-v1-1-cefc79bb33d1@linaro.org/
[2] https://lore.kernel.org/all/20240708-x1e80100-pd-mapper-v1-1-854386af4cf5@linaro.org/
[3] https://lore.kernel.org/all/20240925100303.9112-1-alex.vinarskis@gmail.com/


> > +     compatible = "qcom,geni-debug-uart";
> > +     status = "okay";
> > +};
> > +
>
> Regards,
> Bjorn