linux-kernel - Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-dell-xps13-9345: Enable fingerprint sensor

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: <4kh3zg3ohqzarbhv4r64iapi5x7paz2r7z3wkesgbjmm5fvgsy@dkkbbgmgt6mo>
Date: Wed, 19 Mar 2025 14:45:29 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Aleksandrs Vinarskis <alex.vinarskis@...il.com>
Cc: bjorn.andersson@....qualcomm.com, Vinod Koul <vkoul@...nel.org>, 
	Kishon Vijay Abraham I <kishon@...nel.org>, Konrad Dybcio <konradybcio@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-dell-xps13-9345: Enable
 fingerprint sensor

On Wed, Mar 19, 2025 at 04:05:41PM +0100, Aleksandrs Vinarskis wrote:
> On Wed, 19 Mar 2025 at 04:22, Bjorn Andersson via B4 Relay
> <devnull+bjorn.andersson.oss.qualcomm.com@...nel.org> wrote:
> >
> > From: Bjorn Andersson <bjorn.andersson@....qualcomm.com>
> >
> > The fingerprint sensor, hidden in the power button, is connected to one
> > of the USB multiport ports; while the other port is unused.
> >
> > Describe the USB controller, the four phys and the repeater involved to
> > make the fingerprint sensor operational.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@....qualcomm.com>
> > ---
> 
> Thanks for getting to the bottom of this, it was certainly a long
> awaited feature :)
> 

Didn't think it was something I wanted, but now that it's working I
proved myself wrong ;)

> >  .../boot/dts/qcom/x1e80100-dell-xps13-9345.dts     | 59 +++++++++++++++++++++-
> >  1 file changed, 57 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
> > index 967f6dba0878b51a985fd7c9570b8c4e71afe57d..a35557c562d771e2ce209fca05b82c1943d70f63 100644
> > --- a/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
> > +++ b/arch/arm64/boot/dts/qcom/x1e80100-dell-xps13-9345.dts
> > @@ -744,8 +744,21 @@ touchscreen@10 {
> >
> >  &i2c9 {
> >         clock-frequency = <400000>;
> > -       status = "disabled";
> > -       /* USB3 retimer device @0x4f */
> > +       status = "okay";
> > +
> > +       eusb6_repeater: redriver@4f {
> > +               compatible = "nxp,ptn3222";
> > +               reg = <0x4f>;
> > +               #phy-cells = <0>;
> > +
> > +               vdd3v3-supply = <&vreg_l13b_3p0>;
> > +               vdd1v8-supply = <&vreg_l4b_1p8>;
> > +
> > +               reset-gpios = <&tlmm 184 GPIO_ACTIVE_LOW>;
> > +
> > +               pinctrl-0 = <&eusb6_reset_n>;
> > +               pinctrl-names = "default";
> > +       };
> >  };
> >
> >  &i2c17 {
> > @@ -967,6 +980,14 @@ edp_reg_en: edp-reg-en-state {
> >                 bias-disable;
> >         };
> >
> > +       eusb6_reset_n: eusb6-reset-n-state {
> > +               pins = "gpio184";
> > +               function = "gpio";
> > +               drive-strength = <2>;
> > +               bias-disable;
> > +               output-low;
> > +       };
> > +
> >         hall_int_n_default: hall-int-n-state {
> >                 pins = "gpio92";
> >                 function = "gpio";
> > @@ -1172,3 +1193,37 @@ &usb_1_ss1_dwc3_hs {
> >  &usb_1_ss1_qmpphy_out {
> >         remote-endpoint = <&retimer_ss1_ss_in>;
> >  };
> > +
> > +&usb_mp {
> > +       status = "okay";
> > +};
> > +
> > +&usb_mp_hsphy0 {
> > +       vdd-supply = <&vreg_l2e_0p8>;
> > +       vdda12-supply = <&vreg_l3e_1p2>;
> > +
> > +       phys = <&eusb6_repeater>;
> 
> I was under the impression that the fingerprint reader is on the 2nd
> port of the root hub, as:
> * In ACPI, the only USB device of MP is listed under PRT1, PRT0 is empty
> * On Windows the device is listed as PORT2...HUB1...
> * `lsusb -t` for the device gives `Port 002: Dev 002,...12M`
> 
> Do `usb_mp_hsphy0` and `usb_mp_hsphy1` translate to port 1 and 2
> respectively? Because if yes, repeater may belong to `usb_mp_hsphy1`
> instead?
> 

That would be more logical, I'll dig up some documentation for the
SoC and see if I can better understand the naming of these instances.

> Current series works. Moving `phys = <&eusb6_repeater>;` to
> `usb_mp_hsphy1` also works, I'm assuming because we are not actually
> disabling unused phys.
> 

While not being used for any communication, the PHY is there and the
multiport controller seems to need them both to be present. Further
regardless of something being connected to the PHY, it's still there, so
it seems correct to represent it in the dtsi. That said, I didn't dig
deeper into the exact details here.

> Tested-by: Aleksandrs Vinarskis <alex.vinarskis@...il.com>
> 

Thanks.

Regards,
Bjorn

> 
> > +
> > +       status = "okay";
> > +};
> > +
> > +&usb_mp_hsphy1 {
> > +       vdd-supply = <&vreg_l2e_0p8>;
> > +       vdda12-supply = <&vreg_l3e_1p2>;
> > +
> > +       status = "okay";
> > +};
> > +
> > +&usb_mp_qmpphy0 {
> > +       vdda-phy-supply = <&vreg_l3e_1p2>;
> > +       vdda-pll-supply = <&vreg_l3c_0p9>;
> > +
> > +       status = "okay";
> > +};
> > +
> > +&usb_mp_qmpphy1 {
> > +       vdda-phy-supply = <&vreg_l3e_1p2>;
> > +       vdda-pll-supply = <&vreg_l3c_0p9>;
> > +
> > +       status = "okay";
> > +};
> >
> > --
> > 2.48.1
> >
> >