| Message ID | 20250318-xps13-fingerprint-v1-2-fbb02d5a34a7@oss.qualcomm.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: x1e80100-dell-xps13-9345: Enable fingerprint sensor | expand |
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
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 > >
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 > > > >
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>
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
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"; +};