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] [day] [month] [year] [list]
Message-ID: <ZvTWHZC2n+QkKWhq@hu-bjorande-lv.qualcomm.com>
Date: Wed, 25 Sep 2024 20:33:49 -0700
From: Bjorn Andersson <quic_bjorande@...cinc.com>
To: Aleksandrs Vinarskis <alex.vinarskis@...il.com>
CC: Bjorn Andersson <andersson@...nel.org>,
        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, Sep 25, 2024 at 12:05:22PM +0200, Aleksandrs Vinarskis wrote:
> 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].
> 

Thank you.

> >
> > > * 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?
> 

I'm referring to https://lore.kernel.org/all/ZsbomSG6DXTfYxXZ@hovoldconsulting.com/
although I'm not sure where it was first reported.

Chris Lew has been debugging this and the problem relates to intent
allocation in the underlying GLINK driver. We're discussing how to fix
this.

Hoping to conclude the discussion within the coming days.

> Were you running with [2] patch reverted or not?

I applied your patches on next-20240924, no other changes.

> 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.
> 

I think we want [1], but with a better argumentation. It's however
unrelated to the problem you're seeing.

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

I do have a few messages about "unknown notification", but it works
fine. I expect this is just luck...

> > >
> > > 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.

I have the OLED screen.

> 
> 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.
> 

The 0 is indeed weird, I don't know what's up with that.

I'd suggest that we start with 0x10 and then change things from there if
necessary.

Regards,
Bjorn

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