diff mbox series

arm64: dts: qcom: sc7280-herobrine: Fix PCIe regulator glitch at bootup

Message ID 20220310130429.1.Id41fda1d7f5d9230bc45c1b85b06b0fb0ddd29af@changeid (mailing list archive)
State Accepted
Commit 0d40497d054194768b3ddbf3a676d481b38b96eb
Headers show
Series arm64: dts: qcom: sc7280-herobrine: Fix PCIe regulator glitch at bootup | expand

Commit Message

Doug Anderson March 10, 2022, 9:04 p.m. UTC
While scoping signals, we found that the PCIe signals weren't
compliant at bootup. Specifically, the bootloader was setting up PCIe
and leaving it configured, then jumping to the kernel. The kernel was
turning off the regulator while leaving the PCIe clock running, which
was a violation.

In the regulator bindings (and the Linux kernel driver that uses
them), there's currently no way to specify that a GPIO-controlled
regulator should keep its state at bootup. You've got to pick either
"on" or "off". Let's switch it so that the PCIe regulator defaults to
"on" instead of "off". This should be a much safer way to go and
avoids the timing violation. The regulator will still be turned off
later if there are no users.

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

 arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Matthias Kaehlcke March 10, 2022, 10:34 p.m. UTC | #1
On Thu, Mar 10, 2022 at 01:04:34PM -0800, Douglas Anderson wrote:
> While scoping signals, we found that the PCIe signals weren't
> compliant at bootup. Specifically, the bootloader was setting up PCIe
> and leaving it configured, then jumping to the kernel. The kernel was
> turning off the regulator while leaving the PCIe clock running, which
> was a violation.
> 
> In the regulator bindings (and the Linux kernel driver that uses
> them), there's currently no way to specify that a GPIO-controlled
> regulator should keep its state at bootup. You've got to pick either
> "on" or "off". Let's switch it so that the PCIe regulator defaults to
> "on" instead of "off". This should be a much safer way to go and
> avoids the timing violation. The regulator will still be turned off
> later if there are no users.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Stephen Boyd March 10, 2022, 10:49 p.m. UTC | #2
Quoting Douglas Anderson (2022-03-10 13:04:34)
> While scoping signals, we found that the PCIe signals weren't
> compliant at bootup. Specifically, the bootloader was setting up PCIe
> and leaving it configured, then jumping to the kernel. The kernel was
> turning off the regulator while leaving the PCIe clock running, which
> was a violation.
>
> In the regulator bindings (and the Linux kernel driver that uses
> them), there's currently no way to specify that a GPIO-controlled
> regulator should keep its state at bootup. You've got to pick either
> "on" or "off". Let's switch it so that the PCIe regulator defaults to
> "on" instead of "off". This should be a much safer way to go and
> avoids the timing violation. The regulator will still be turned off
> later if there are no users.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
patchwork-bot+linux-arm-msm@kernel.org April 11, 2022, 9 p.m. UTC | #3
Hello:

This patch was applied to qcom/linux.git (for-next)
by Bjorn Andersson <bjorn.andersson@linaro.org>:

On Thu, 10 Mar 2022 13:04:34 -0800 you wrote:
> While scoping signals, we found that the PCIe signals weren't
> compliant at bootup. Specifically, the bootloader was setting up PCIe
> and leaving it configured, then jumping to the kernel. The kernel was
> turning off the regulator while leaving the PCIe clock running, which
> was a violation.
> 
> In the regulator bindings (and the Linux kernel driver that uses
> them), there's currently no way to specify that a GPIO-controlled
> regulator should keep its state at bootup. You've got to pick either
> "on" or "off". Let's switch it so that the PCIe regulator defaults to
> "on" instead of "off". This should be a much safer way to go and
> avoids the timing violation. The regulator will still be turned off
> later if there are no users.
> 
> [...]

Here is the summary with links:
  - arm64: dts: qcom: sc7280-herobrine: Fix PCIe regulator glitch at bootup
    https://git.kernel.org/qcom/c/0d40497d0541

You are awesome, thank you!
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index dc17f2079695..042a4a59e3dc 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -178,6 +178,13 @@  pp3300_ssd: pp3300-ssd-regulator {
 		pinctrl-names = "default";
 		pinctrl-0 = <&ssd_en>;
 
+		/*
+		 * The bootloaer may have left PCIe configured. Powering this
+		 * off while the PCIe clocks are still running isn't great,
+		 * so it's better to default to this regulator being on.
+		 */
+		regulator-boot-on;
+
 		vin-supply = <&pp3300_z1>;
 	};