arm64: dts: x1e80100-crd: describe HID supplies

PATCH Superseded Johan Hovold 5 participants 8 messages Oct 15, 2024 – Oct 25, 2024

Subsystems (3)

Device tree bindings Arm Qualcomm mailing list Arm Qualcomm support

lore

Share & tools

lore

Johan Hovold

author

2024-10-15 12:25 UTC #0 lore raw

PATCH 1/1 arm64: dts: x1e80100-crd: describe HID supplies

Add the missing HID supplies to avoid relying on other consumers to keep them on.

This also avoids the following warnings on boot:

i2c_hid_of 0-0010: supply vdd not found, using dummy regulator i2c_hid_of 0-0010: supply vddl not found, using dummy regulator i2c_hid_of 1-0015: supply vdd not found, using dummy regulator i2c_hid_of 1-0015: supply vddl not found, using dummy regulator i2c_hid_of 1-003a: supply vdd not found, using dummy regulator i2c_hid_of 1-003a: supply vddl not found, using dummy regulator

Note that VREG_MISC_3P3 is also used for things like the fingerprint reader which are not yet fully described so mark the regulator as always on for now.

Fixes d7e03cce0400 ("arm64: dts: qcom: x1e80100-crd: Enable more support")

Cc Abel Vesa <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 34 +++++++++++++++++++++++
 1 file changed, 34 insertions(+)

arch/arm64/boot/dts/qcom/x1e80100-crd.dts +34 -0

index 10b28d870f08..4ab7078f76e0 100644
@@ -288,6 +288,23 @@ vreg_edp_3p3: regulator-edp-3p3 {
 		regulator-boot-on;
 	};
 
+	vreg_misc_3p3: regulator-misc-3p3 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_MISC_3P3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pm8550ve_8_gpios 6 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&misc_3p3_reg_en>;
+
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
 	vreg_nvme: regulator-nvme {
 		compatible = "regulator-fixed";

@@ -689,6 +706,9 @@ touchpad@15 {
 		hid-descr-addr = <0x1>;
 		interrupts-extended = <&tlmm 3 IRQ_TYPE_LEVEL_LOW>;
 
+		vdd-supply = <&vreg_misc_3p3>;
+		vddl-supply = <&vreg_l12b_1p2>;
+
 		pinctrl-0 = <&tpad_default>;
 		pinctrl-names = "default";

@@ -702,6 +722,9 @@ keyboard@3a {
 		hid-descr-addr = <0x1>;
 		interrupts-extended = <&tlmm 67 IRQ_TYPE_LEVEL_LOW>;
 
+		vdd-supply = <&vreg_misc_3p3>;
+		vddl-supply = <&vreg_l12b_1p2>;
+
 		pinctrl-0 = <&kybd_default>;
 		pinctrl-names = "default";

@@ -721,6 +744,9 @@ touchscreen@10 {
 		hid-descr-addr = <0x1>;
 		interrupts-extended = <&tlmm 51 IRQ_TYPE_LEVEL_LOW>;
 
+		vdd-supply = <&vreg_misc_3p3>;
+		vddl-supply = <&vreg_l15b_1p8>;
+
 		pinctrl-0 = <&ts0_default>;
 		pinctrl-names = "default";
 	};

@@ -854,6 +880,14 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pm8550ve_8_gpios {
+	misc_3p3_reg_en: misc-3p3-reg-en-state {
+		pins = "gpio6";
+		function = "normal";
+		bias-disable;
+	};
+};
+
 &pmc8380_3_gpios {
 	edp_bl_en: edp-bl-en-state {
 		pins = "gpio4";

Signature · 1 line

2.45.2

Dmitry Baryshkov

Reviewed-by on v1 1/1

2024-10-15 13:24 UTC #1 lore raw

Quoted 16 lines · Johan Hovold

Add the missing HID supplies to avoid relying on other consumers to keep them on.
This also avoids the following warnings on boot:
i2c_hid_of 0-0010: supply vdd not found, using dummy regulator i2c_hid_of 0-0010: supply vddl not found, using dummy regulator i2c_hid_of 1-0015: supply vdd not found, using dummy regulator i2c_hid_of 1-0015: supply vddl not found, using dummy regulator i2c_hid_of 1-003a: supply vdd not found, using dummy regulator i2c_hid_of 1-003a: supply vddl not found, using dummy regulator
Note that VREG_MISC_3P3 is also used for things like the fingerprint reader which are not yet fully described so mark the regulator as always on for now.

Quoted 7 lines

Fixes d7e03cce0400 ("arm64: dts: qcom: x1e80100-crd: Enable more support")
Cc Abel Vesa <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 34 +++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Reviewed-by Dmitry Baryshkov <[email protected]>

Signature · 2 lines

With best wishes
Dmitry

Abel Vesa

Reviewed-by on v1 1/1

2024-10-15 13:41 UTC #2 lore raw

In reply to Johan Hovold

Quoted 31 lines · Johan Hovold

Add the missing HID supplies to avoid relying on other consumers to keep them on.
This also avoids the following warnings on boot:
i2c_hid_of 0-0010: supply vdd not found, using dummy regulator i2c_hid_of 0-0010: supply vddl not found, using dummy regulator i2c_hid_of 1-0015: supply vdd not found, using dummy regulator i2c_hid_of 1-0015: supply vddl not found, using dummy regulator i2c_hid_of 1-003a: supply vdd not found, using dummy regulator i2c_hid_of 1-003a: supply vddl not found, using dummy regulator
Note that VREG_MISC_3P3 is also used for things like the fingerprint reader which are not yet fully described so mark the regulator as always on for now.
Fixes d7e03cce0400 ("arm64: dts: qcom: x1e80100-crd: Enable more support")
Cc Abel Vesa <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 34 +++++++++++++++++++++++
 1 file changed, 34 insertions(+)
arch/arm64/boot/dts/qcom/x1e80100-crd.dts +0 -0
index 10b28d870f08..4ab7078f76e0 100644
@@ -288,6 +288,23 @@ vreg_edp_3p3: regulator-edp-3p3 {
 		regulator-boot-on;
 	};
 

[...]

arch/arm64/boot/dts/qcom/x1e80100-crd.dts &pcie6a_phy { · line 887

Quoted 6 lines

+&pm8550ve_8_gpios {
arch/arm64/boot/dts/qcom/x1e80100-crd.dts +4 -0
+	misc_3p3_reg_en: misc-3p3-reg-en-state {
+		pins = "gpio6";
+		function = "normal";
+		bias-disable;

Maybe output-enable and input-disable are needed. Can you please check?

Otherwise, LGTM:

Reviewed-by Abel Vesa <[email protected]>

Quoted 9 lines

arch/arm64/boot/dts/qcom/x1e80100-crd.dts +3 -0
+	};
+};
+
 &pmc8380_3_gpios {
 	edp_bl_en: edp-bl-en-state {
 		pins = "gpio4";
Signature · 1 line
2.45.2

Stephan Gerhold

2024-10-16 16:43 UTC #3 lore raw

Quoted 36 lines · Abel Vesa

Quoted 31 lines · Johan Hovold
Add the missing HID supplies to avoid relying on other consumers to keep them on.
This also avoids the following warnings on boot:
i2c_hid_of 0-0010: supply vdd not found, using dummy regulator i2c_hid_of 0-0010: supply vddl not found, using dummy regulator i2c_hid_of 1-0015: supply vdd not found, using dummy regulator i2c_hid_of 1-0015: supply vddl not found, using dummy regulator i2c_hid_of 1-003a: supply vdd not found, using dummy regulator i2c_hid_of 1-003a: supply vddl not found, using dummy regulator
Note that VREG_MISC_3P3 is also used for things like the fingerprint reader which are not yet fully described so mark the regulator as always on for now.
Fixes d7e03cce0400 ("arm64: dts: qcom: x1e80100-crd: Enable more support")
Cc Abel Vesa <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 34 +++++++++++++++++++++++
 1 file changed, 34 insertions(+)
arch/arm64/boot/dts/qcom/x1e80100-crd.dts +0 -0
index 10b28d870f08..4ab7078f76e0 100644
@@ -288,6 +288,23 @@ vreg_edp_3p3: regulator-edp-3p3 {
 		regulator-boot-on;
 	};
 
[...]
Quoted 1 line

Quoted 8 lines

arch/arm64/boot/dts/qcom/x1e80100-crd.dts &pcie6a_phy { · line 887
Quoted 5 lines
+&pm8550ve_8_gpios {
arch/arm64/boot/dts/qcom/x1e80100-crd.dts +4 -0
+	misc_3p3_reg_en: misc-3p3-reg-en-state {
+		pins = "gpio6";
+		function = "normal";
+		bias-disable;
Maybe output-enable and input-disable are needed. Can you please check?

FWIW, there is a reason behind explicitly describing the intended direction of the pin for PMIC GPIOs with properties like "output-enable" or "input-disable": On QC platforms, PMIC GPIOs can be either in "input" mode, "output" mode, or "input+output" mode. If you don't specify exactly what you want, then the pinctrl-spmi-gpio driver will only add to the existing configuration.

For the configuration above this means:

1. If GPIO6 is disabled or in "output" mode during boot, the resulting mode will be "output".

2. If GPIO6 is in "input" mode during boot, the resulting mode will be

    "input+output".

I don't know if "input+output" mode has any negative impact compared to pure "output" mode. We usually want to have the pins in a consistent state though (i.e. independent of the boot up state).

Thanks,
Stephan

Johan Hovold

2024-10-25 15:17 UTC #4 lore raw

Quoted 23 lines · Stephan Gerhold

Quoted 9 lines · Abel Vesa
arch/arm64/boot/dts/qcom/x1e80100-crd.dts &pcie6a_phy { · line 887
Quoted 5 lines · Johan Hovold
+&pm8550ve_8_gpios {
arch/arm64/boot/dts/qcom/x1e80100-crd.dts +4 -0
+	misc_3p3_reg_en: misc-3p3-reg-en-state {
+		pins = "gpio6";
+		function = "normal";
+		bias-disable;
Maybe output-enable and input-disable are needed. Can you please check?
FWIW, there is a reason behind explicitly describing the intended direction of the pin for PMIC GPIOs with properties like "output-enable" or "input-disable": On QC platforms, PMIC GPIOs can be either in "input" mode, "output" mode, or "input+output" mode. If you don't specify exactly what you want, then the pinctrl-spmi-gpio driver will only add to the existing configuration.
For the configuration above this means:
1. If GPIO6 is disabled or in "output" mode during boot, the resulting mode will be "output".

Quoted 6 lines

2. If GPIO6 is in "input" mode during boot, the resulting mode will be
    "input+output".
I don't know if "input+output" mode has any negative impact compared to pure "output" mode. We usually want to have the pins in a consistent state though (i.e. independent of the boot up state).

Fair enough. I was worried that configuring the pin as an output without setting the output value could cause trouble (e.g. always default to low), but it seems at least the Linux driver handles that.

Johan

Stephan Gerhold

2024-10-15 14:57 UTC #5 lore raw

In reply to Johan Hovold

Quoted 82 lines · Johan Hovold

Add the missing HID supplies to avoid relying on other consumers to keep them on.
This also avoids the following warnings on boot:
i2c_hid_of 0-0010: supply vdd not found, using dummy regulator i2c_hid_of 0-0010: supply vddl not found, using dummy regulator i2c_hid_of 1-0015: supply vdd not found, using dummy regulator i2c_hid_of 1-0015: supply vddl not found, using dummy regulator i2c_hid_of 1-003a: supply vdd not found, using dummy regulator i2c_hid_of 1-003a: supply vddl not found, using dummy regulator
Note that VREG_MISC_3P3 is also used for things like the fingerprint reader which are not yet fully described so mark the regulator as always on for now.
Fixes d7e03cce0400 ("arm64: dts: qcom: x1e80100-crd: Enable more support")
Cc Abel Vesa <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 34 +++++++++++++++++++++++
 1 file changed, 34 insertions(+)
arch/arm64/boot/dts/qcom/x1e80100-crd.dts +26 -0
index 10b28d870f08..4ab7078f76e0 100644
@@ -288,6 +288,23 @@ vreg_edp_3p3: regulator-edp-3p3 {
 		regulator-boot-on;
 	};
 
+	vreg_misc_3p3: regulator-misc-3p3 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_MISC_3P3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pm8550ve_8_gpios 6 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&misc_3p3_reg_en>;
+
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
 	vreg_nvme: regulator-nvme {
 		compatible = "regulator-fixed";
 
@@ -689,6 +706,9 @@ touchpad@15 {
 		hid-descr-addr = <0x1>;
 		interrupts-extended = <&tlmm 3 IRQ_TYPE_LEVEL_LOW>;
 
+		vdd-supply = <&vreg_misc_3p3>;
+		vddl-supply = <&vreg_l12b_1p2>;
+
 		pinctrl-0 = <&tpad_default>;
 		pinctrl-names = "default";
 
@@ -702,6 +722,9 @@ keyboard@3a {
 		hid-descr-addr = <0x1>;
 		interrupts-extended = <&tlmm 67 IRQ_TYPE_LEVEL_LOW>;
 
+		vdd-supply = <&vreg_misc_3p3>;
+		vddl-supply = <&vreg_l12b_1p2>;
+
 		pinctrl-0 = <&kybd_default>;
 		pinctrl-names = "default";
 
@@ -721,6 +744,9 @@ touchscreen@10 {
 		hid-descr-addr = <0x1>;
 		interrupts-extended = <&tlmm 51 IRQ_TYPE_LEVEL_LOW>;
 
+		vdd-supply = <&vreg_misc_3p3>;
+		vddl-supply = <&vreg_l15b_1p8>;
+
 		pinctrl-0 = <&ts0_default>;
 		pinctrl-names = "default";
 	};

arch/arm64/boot/dts/qcom/x1e80100-crd.dts &pcie6a_phy { · line 887

Quoted 8 lines

arch/arm64/boot/dts/qcom/x1e80100-crd.dts +5 -0

@@ -854,6 +880,14 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pm8550ve_8_gpios {
+	misc_3p3_reg_en: misc-3p3-reg-en-state {
+		pins = "gpio6";
+		function = "normal";
+		bias-disable;

Can we add a "power-source" here? PMIC GPIOs can be either ~3.7V (VPH_PWR) or 1.8V, depending on which power-source is selected. Without that, we rely on the firmware to set the voltage level for the GPIO during boot.

I'm not sure which one would be suitable here. I guess we can just replicate what the firmware configures during boot.

Thanks,
Stephan

Abel Vesa

2024-10-18 17:19 UTC #6 lore raw

Quoted 86 lines · Stephan Gerhold

Quoted 85 lines · Johan Hovold
Add the missing HID supplies to avoid relying on other consumers to keep them on.
This also avoids the following warnings on boot:
i2c_hid_of 0-0010: supply vdd not found, using dummy regulator i2c_hid_of 0-0010: supply vddl not found, using dummy regulator i2c_hid_of 1-0015: supply vdd not found, using dummy regulator i2c_hid_of 1-0015: supply vddl not found, using dummy regulator i2c_hid_of 1-003a: supply vdd not found, using dummy regulator i2c_hid_of 1-003a: supply vddl not found, using dummy regulator
Note that VREG_MISC_3P3 is also used for things like the fingerprint reader which are not yet fully described so mark the regulator as always on for now.
Fixes d7e03cce0400 ("arm64: dts: qcom: x1e80100-crd: Enable more support")
Cc Abel Vesa <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/arm64/boot/dts/qcom/x1e80100-crd.dts | 34 +++++++++++++++++++++++
 1 file changed, 34 insertions(+)
arch/arm64/boot/dts/qcom/x1e80100-crd.dts +26 -0
index 10b28d870f08..4ab7078f76e0 100644
@@ -288,6 +288,23 @@ vreg_edp_3p3: regulator-edp-3p3 {
 		regulator-boot-on;
 	};
 
+	vreg_misc_3p3: regulator-misc-3p3 {
+		compatible = "regulator-fixed";
+
+		regulator-name = "VREG_MISC_3P3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+
+		gpio = <&pm8550ve_8_gpios 6 GPIO_ACTIVE_HIGH>;
+		enable-active-high;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&misc_3p3_reg_en>;
+
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
 	vreg_nvme: regulator-nvme {
 		compatible = "regulator-fixed";
 
@@ -689,6 +706,9 @@ touchpad@15 {
 		hid-descr-addr = <0x1>;
 		interrupts-extended = <&tlmm 3 IRQ_TYPE_LEVEL_LOW>;
 
+		vdd-supply = <&vreg_misc_3p3>;
+		vddl-supply = <&vreg_l12b_1p2>;
+
 		pinctrl-0 = <&tpad_default>;
 		pinctrl-names = "default";
 
@@ -702,6 +722,9 @@ keyboard@3a {
 		hid-descr-addr = <0x1>;
 		interrupts-extended = <&tlmm 67 IRQ_TYPE_LEVEL_LOW>;
 
+		vdd-supply = <&vreg_misc_3p3>;
+		vddl-supply = <&vreg_l12b_1p2>;
+
 		pinctrl-0 = <&kybd_default>;
 		pinctrl-names = "default";
 
@@ -721,6 +744,9 @@ touchscreen@10 {
 		hid-descr-addr = <0x1>;
 		interrupts-extended = <&tlmm 51 IRQ_TYPE_LEVEL_LOW>;
 
+		vdd-supply = <&vreg_misc_3p3>;
+		vddl-supply = <&vreg_l15b_1p8>;
+
 		pinctrl-0 = <&ts0_default>;
 		pinctrl-names = "default";
 	};
@@ -854,6 +880,14 @@ &pcie6a_phy {
 	status = "okay";
 };
 

Quoted 10 lines

arch/arm64/boot/dts/qcom/x1e80100-crd.dts &pcie6a_phy { · line 887
Quoted 5 lines
arch/arm64/boot/dts/qcom/x1e80100-crd.dts +5 -0
+&pm8550ve_8_gpios {
+	misc_3p3_reg_en: misc-3p3-reg-en-state {
+		pins = "gpio6";
+		function = "normal";
+		bias-disable;
Can we add a "power-source" here? PMIC GPIOs can be either ~3.7V (VPH_PWR) or 1.8V, depending on which power-source is selected. Without that, we rely on the firmware to set the voltage level for the GPIO during boot.

AFAIU, the power-source here should be 0, which selects VPH_PWR which is 3.3V. In that case I think we can avoid explicitly setting it.

Quoted 6 lines

I'm not sure which one would be suitable here. I guess we can just replicate what the firmware configures during boot.
Thanks,
Stephan

Johan Hovold

2024-10-25 15:21 UTC #7 lore raw

Quoted 14 lines · Abel Vesa

Quoted 12 lines · Stephan Gerhold
arch/arm64/boot/dts/qcom/x1e80100-crd.dts &pcie6a_phy { · line 887
Quoted 5 lines · Johan Hovold
+&pm8550ve_8_gpios {
arch/arm64/boot/dts/qcom/x1e80100-crd.dts +4 -0
+	misc_3p3_reg_en: misc-3p3-reg-en-state {
+		pins = "gpio6";
+		function = "normal";
+		bias-disable;
Can we add a "power-source" here? PMIC GPIOs can be either ~3.7V (VPH_PWR) or 1.8V, depending on which power-source is selected. Without that, we rely on the firmware to set the voltage level for the GPIO during boot.

Quoted 2 lines

AFAIU, the power-source here should be 0, which selects VPH_PWR which is 3.3V. In that case I think we can avoid explicitly setting it.

The firmware uses 1.8 V here in fact, but it seems like 3.3 V would work as well.

Quoted 2 lines

Quoted 2 lines
I'm not sure which one would be suitable here. I guess we can just replicate what the firmware configures during boot.

Let's start with that at least.

Johan