[PATCH v2 0/2] Update fingerprint node on herobrine/trogdor

[PATCH v2 0/2] Update fingerprint node on herobrine/trogdor

[Stephen Boyd]

This patch series updates the binding for the Trogdor and Herobrine
boards' fingerprint node. Now that the binding has been accepted[1]
we're ready to merge this in the qcom tree.

Changes from v1 (https://lore.kernel.org/r/20220317010640.2498502-1-swboyd@chromium.org):
 * New patch for trogdor

Stephen Boyd (2):
  arm64: dts: qcom: Fully describe fingerprint node on Herobrine
  arm64: dts: qcom: Fully describe fingerprint node on Trogdor

 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi  | 20 +++++++++++++++++--
 .../arm64/boot/dts/qcom/sc7280-herobrine.dtsi |  5 ++++-
 2 files changed, 22 insertions(+), 3 deletions(-)

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Matthias Kaehlcke <mka@chromium.org>
Cc: Alexandru M Stan <amstan@chromium.org>

[1] https://lore.kernel.org/r/166718941539.790.4332712127794687523.git-patchwork-notify@kernel.org

base-commit: 95fade4016cbd57ee050ab226c8f0483af1753c4
-- 
https://chromeos.dev

[PATCH v2 1/2] arm64: dts: qcom: Fully describe fingerprint node on Herobrine

[Stephen Boyd]

Update the fingerprint node on Herobrine to match the fingerprint DT
binding. This will allow us to drive the reset and boot gpios from the
driver when it is re-attached after flashing. We'll also be able to boot
the fingerprint processor if the BIOS isn't doing it for us.

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Matthias Kaehlcke <mka@chromium.org>
Cc: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index ca02ef26f161..448da9794722 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -503,13 +503,16 @@ ap_spi_fp: &spi9 {
 	cs-gpios = <&tlmm 39 GPIO_ACTIVE_LOW>;
 
 	cros_ec_fp: ec@0 {
-		compatible = "google,cros-ec-spi";
+		compatible = "google,cros-ec-fp", "google,cros-ec-spi";
 		reg = <0>;
 		interrupt-parent = <&tlmm>;
 		interrupts = <61 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&fp_to_ap_irq_l>, <&fp_rst_l>, <&fpmcu_boot0>;
+		boot0-gpios = <&tlmm 68 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&tlmm 78 GPIO_ACTIVE_LOW>;
 		spi-max-frequency = <3000000>;
+		vdd-supply = <&pp3300_fp_mcu>;
 	};
 };
 
-- 
https://chromeos.dev

[PATCH v2 2/2] arm64: dts: qcom: Fully describe fingerprint node on Trogdor

[Stephen Boyd]

Update the fingerprint node on Trogdor to match the fingerprint DT
binding. This will allow us to drive the reset and boot gpios from the
driver when it is re-attached after flashing. We'll also be able to boot
the fingerprint processor if the BIOS isn't doing it for us.

Cc: Douglas Anderson <dianders@chromium.org>
Cc: Matthias Kaehlcke <mka@chromium.org>
Cc: Alexandru M Stan <amstan@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 4a5ea17a15ba..65601bea0797 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -894,13 +894,16 @@ ap_spi_fp: &spi10 {
 	cs-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
 
 	cros_ec_fp: ec@0 {
-		compatible = "google,cros-ec-spi";
+		compatible = "google,cros-ec-fp", "google,cros-ec-spi";
 		reg = <0>;
 		interrupt-parent = <&tlmm>;
 		interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
 		pinctrl-names = "default";
-		pinctrl-0 = <&fp_to_ap_irq_l>;
+		pinctrl-0 = <&fp_to_ap_irq_l>, <&fp_rst_l>, <&fpmcu_boot0>;
+		boot0-gpios = <&tlmm 10 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&tlmm 22 GPIO_ACTIVE_LOW>;
 		spi-max-frequency = <3000000>;
+		vdd-supply = <&pp3300_fp_tp>;
 	};
 };
 
@@ -1226,6 +1229,13 @@ en_pp3300_hub: en-pp3300-hub-state {
 		bias-disable;
 	};
 
+	fp_rst_l: fp-rst-l-state {
+		pins = "gpio22";
+		function = "gpio";
+		bias-disable;
+		drive-strength = <2>;
+	};
+
 	fp_to_ap_irq_l: fp-to-ap-irq-l-state {
 		pins = "gpio4";
 		function = "gpio";
@@ -1235,6 +1245,12 @@ fp_to_ap_irq_l: fp-to-ap-irq-l-state {
 		bias-disable;
 	};
 
+	fpmcu_boot0: fpmcu-boot0-state {
+		pins = "gpio10";
+		function = "gpio";
+		bias-disable;
+	};
+
 	h1_ap_int_odl: h1-ap-int-odl-state {
 		pins = "gpio42";
 		function = "gpio";
-- 
https://chromeos.dev

Re: [PATCH v2 1/2] arm64: dts: qcom: Fully describe fingerprint node on Herobrine

[Doug Anderson]

Hi,

On Mon, Nov 7, 2022 at 11:15 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Update the fingerprint node on Herobrine to match the fingerprint DT
> binding. This will allow us to drive the reset and boot gpios from the
> driver when it is re-attached after flashing. We'll also be able to boot
> the fingerprint processor if the BIOS isn't doing it for us.
>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

From Bjorn's recent comment [1], I think your subject needs modification to:

   arm64: dts: qcom: sc7280: Fully describe fingerprint node on Herobrine

Other than that this seems fine and is a better description of hardware, so:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

I guess we're still leaving gpio77 alone (AKA not describing it in the
fingerprint regulator)?

[1] https://lore.kernel.org/r/20221017224853.stuy5qeqxjxntob4@builder.lan/

Re: [PATCH v2 2/2] arm64: dts: qcom: Fully describe fingerprint node on Trogdor

[Doug Anderson]

Hi,

On Mon, Nov 7, 2022 at 11:15 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Update the fingerprint node on Trogdor to match the fingerprint DT
> binding. This will allow us to drive the reset and boot gpios from the
> driver when it is re-attached after flashing. We'll also be able to boot
> the fingerprint processor if the BIOS isn't doing it for us.
>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> index 4a5ea17a15ba..65601bea0797 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> @@ -894,13 +894,16 @@ ap_spi_fp: &spi10 {
>         cs-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
>
>         cros_ec_fp: ec@0 {
> -               compatible = "google,cros-ec-spi";
> +               compatible = "google,cros-ec-fp", "google,cros-ec-spi";
>                 reg = <0>;
>                 interrupt-parent = <&tlmm>;
>                 interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
>                 pinctrl-names = "default";
> -               pinctrl-0 = <&fp_to_ap_irq_l>;
> +               pinctrl-0 = <&fp_to_ap_irq_l>, <&fp_rst_l>, <&fpmcu_boot0>;
> +               boot0-gpios = <&tlmm 10 GPIO_ACTIVE_HIGH>;
> +               reset-gpios = <&tlmm 22 GPIO_ACTIVE_LOW>;

This isn't totally a no-op change since the pinctrl entries for
fp_rst_l and fpmcu_boot0 will now be applied. I assume you've
confirmed that it works as expected?

...other than the subject (same as patch #1), this LGTM.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Re: (subset) [PATCH v2 0/2] Update fingerprint node on herobrine/trogdor

[Bjorn Andersson]

On Mon, 7 Nov 2022 11:15:33 -0800, Stephen Boyd wrote:
> This patch series updates the binding for the Trogdor and Herobrine
> boards' fingerprint node. Now that the binding has been accepted[1]
> we're ready to merge this in the qcom tree.
> 
> Changes from v1 (https://lore.kernel.org/r/20220317010640.2498502-1-swboyd@chromium.org):
>  * New patch for trogdor
> 
> [...]

Applied, thanks!

[1/2] arm64: dts: qcom: Fully describe fingerprint node on Herobrine
      commit: aefd5370ab5e55a18c94573b9602083132e24601
[2/2] arm64: dts: qcom: Fully describe fingerprint node on Trogdor
      commit: 9ec68fea9e53d25177618d2ce1bc4a1b1b724938

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: [PATCH v2 2/2] arm64: dts: qcom: Fully describe fingerprint node on Trogdor

[Stephen Boyd]

Quoting Doug Anderson (2022-11-07 17:22:51)
> On Mon, Nov 7, 2022 at 11:15 AM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > index 4a5ea17a15ba..65601bea0797 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > @@ -894,13 +894,16 @@ ap_spi_fp: &spi10 {
> >         cs-gpios = <&tlmm 89 GPIO_ACTIVE_LOW>;
> >
> >         cros_ec_fp: ec@0 {
> > -               compatible = "google,cros-ec-spi";
> > +               compatible = "google,cros-ec-fp", "google,cros-ec-spi";
> >                 reg = <0>;
> >                 interrupt-parent = <&tlmm>;
> >                 interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
> >                 pinctrl-names = "default";
> > -               pinctrl-0 = <&fp_to_ap_irq_l>;
> > +               pinctrl-0 = <&fp_to_ap_irq_l>, <&fp_rst_l>, <&fpmcu_boot0>;
> > +               boot0-gpios = <&tlmm 10 GPIO_ACTIVE_HIGH>;
> > +               reset-gpios = <&tlmm 22 GPIO_ACTIVE_LOW>;
>
> This isn't totally a no-op change since the pinctrl entries for
> fp_rst_l and fpmcu_boot0 will now be applied. I assume you've
> confirmed that it works as expected?

Yes my fingerprint still works to unlock. I also confirmed that the bios
sets these pinctrl settings so this is to be more explicit and not rely
on bootloader state.

>
> ...other than the subject (same as patch #1), this LGTM.
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Cool thanks.