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

Bjorn Andersson via B4 Relay posted 2 patches 9 months ago
There is a newer version of this series
[PATCH 2/2] arm64: dts: qcom: x1e80100-dell-xps13-9345: Enable fingerprint sensor
Posted by Bjorn Andersson via B4 Relay 9 months ago
From: Bjorn Andersson <bjorn.andersson@oss.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@oss.qualcomm.com>
---
 .../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>;
+
+	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
Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-dell-xps13-9345: Enable fingerprint sensor
Posted by Stefan Schmidt 8 months, 3 weeks ago
Hello Bjorn,

On Wed, 19 Mar 2025 at 04:23, Bjorn Andersson via B4 Relay
<devnull+bjorn.andersson.oss.qualcomm.com@kernel.org> wrote:
>
> From: Bjorn Andersson <bjorn.andersson@oss.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@oss.qualcomm.com>

Goodix Fingerprint USB Device fingerprint device shows up on my XPS 9345 now.

Tested-by: Stefan Schmidt <stefan.schmidt@linaro.org>

regards
Stefan Schmidt
Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-dell-xps13-9345: Enable fingerprint sensor
Posted by Bryan O'Donoghue 9 months ago
On 19/03/2025 03:22, Bjorn Andersson via B4 Relay wrote:
> From: Bjorn Andersson <bjorn.andersson@oss.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@oss.qualcomm.com>
> ---
>   .../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 {

How about

eusb6_frp_repeater: redriver@4f

leading to> +		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>;

phys = <&eusb6_frp_repeater>;

That's how I did it on Insprion14 and it helps me make sense of the dts 
back to the schematic.

> +
> +	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
> 
> 
> 
Completely optional obviously.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-dell-xps13-9345: Enable fingerprint sensor
Posted by Aleksandrs Vinarskis 9 months ago
On Wed, 19 Mar 2025 at 04:22, Bjorn Andersson via B4 Relay
<devnull+bjorn.andersson.oss.qualcomm.com@kernel.org> wrote:
>
> From: Bjorn Andersson <bjorn.andersson@oss.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@oss.qualcomm.com>
> ---

Thanks for getting to the bottom of this, it was certainly a long
awaited feature :)

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

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

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


> +
> +       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
>
>
Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-dell-xps13-9345: Enable fingerprint sensor
Posted by Bjorn Andersson 9 months ago
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@kernel.org> wrote:
> >
> > From: Bjorn Andersson <bjorn.andersson@oss.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@oss.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@gmail.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
> >
> >
Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-dell-xps13-9345: Enable fingerprint sensor
Posted by Konrad Dybcio 9 months ago
On 3/19/25 4:22 AM, Bjorn Andersson via B4 Relay wrote:
> From: Bjorn Andersson <bjorn.andersson@oss.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@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad