| Message ID | 20240210-topic-8280_pcie-v2-1-1cef4b606883@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Qualcomm PCIe RC shutdown & reinit | expand |
Would be nice to have a hint in the subject line about what this does.
Also capitalize to match the others ("PCI: qcom: <Capitalized verb>").
On Sat, Feb 10, 2024 at 06:10:05PM +0100, Konrad Dybcio wrote:
> At least on SC8280XP, if the PCIe reset is asserted, the corresponding
> AUX_CLK will be stuck at 'off'. This has not been an issue so far,
> since the reset is both left de-asserted by the previous boot stages
> and the driver only toggles it briefly in .init.
>
> As part of the upcoming suspend prodecure however, the reset will be
> held asserted.
s/prodecure/procedure/
> Assert the reset (which may end up being a NOP in some cases) and
> de-assert it back *before* turning on the clocks in preparation for
> introducing RC powerdown and reinitialization.
Bjorn
On 12.02.2024 22:14, Bjorn Helgaas wrote: > Would be nice to have a hint in the subject line about what this does. > Also capitalize to match the others ("PCI: qcom: <Capitalized verb>"). > > On Sat, Feb 10, 2024 at 06:10:05PM +0100, Konrad Dybcio wrote: >> At least on SC8280XP, if the PCIe reset is asserted, the corresponding >> AUX_CLK will be stuck at 'off'. This has not been an issue so far, >> since the reset is both left de-asserted by the previous boot stages >> and the driver only toggles it briefly in .init. >> >> As part of the upcoming suspend prodecure however, the reset will be >> held asserted. > > s/prodecure/procedure/ Before I get around to resending, does the commit look fine otherwise? Konrad
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c index 2ce2a3bd932b..cbde9effa352 100644 --- a/drivers/pci/controller/dwc/pcie-qcom.c +++ b/drivers/pci/controller/dwc/pcie-qcom.c @@ -900,27 +900,27 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) return ret; } - ret = clk_bulk_prepare_enable(res->num_clks, res->clks); - if (ret < 0) - goto err_disable_regulators; - + /* Assert the reset to hold the RC in a known state */ ret = reset_control_assert(res->rst); if (ret) { dev_err(dev, "reset assert failed (%d)\n", ret); - goto err_disable_clocks; + goto err_disable_regulators; } - usleep_range(1000, 1500); + /* GCC_PCIE_n_AUX_CLK won't come up if the reset is asserted */ ret = reset_control_deassert(res->rst); if (ret) { dev_err(dev, "reset deassert failed (%d)\n", ret); - goto err_disable_clocks; + goto err_disable_regulators; } - /* Wait for reset to complete, required on SM8450 */ usleep_range(1000, 1500); + ret = clk_bulk_prepare_enable(res->num_clks, res->clks); + if (ret < 0) + goto err_disable_regulators; + /* configure PCIe to RC mode */ writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE); @@ -951,8 +951,6 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie) writel(val, pcie->parf + PARF_AXI_MSTR_WR_ADDR_HALT_V2); return 0; -err_disable_clocks: - clk_bulk_disable_unprepare(res->num_clks, res->clks); err_disable_regulators: regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
At least on SC8280XP, if the PCIe reset is asserted, the corresponding AUX_CLK will be stuck at 'off'. This has not been an issue so far, since the reset is both left de-asserted by the previous boot stages and the driver only toggles it briefly in .init. As part of the upcoming suspend prodecure however, the reset will be held asserted. Assert the reset (which may end up being a NOP in some cases) and de-assert it back *before* turning on the clocks in preparation for introducing RC powerdown and reinitialization. Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/pci/controller/dwc/pcie-qcom.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)