| Message ID | 1331287760-10546-1-git-send-email-mad_soft@inbox.ru |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 03/09/2012 03:09 AM, Dmitry Artamonow wrote: > Tegra PCIe driver waits for PLL to lock using busy loop. > If PLL fails to lock for some reason, this leads to silent lockup > while booting (PCIe code is not modular). > > Fix by adding timeout, so if PLL doesn't lock in a couple > of seconds, just PCIe driver fails and machine continues to boot. > > Signed-off-by: Dmitry Artamonow <mad_soft@inbox.ru> > diff --git a/arch/arm/mach-tegra/pcie.c b/arch/arm/mach-tegra/pcie.c > /* Wait for the PLL to lock */ > + timeout = 2000; > do { > val = pads_readl(PADS_PLL_CTL); > + usleep_range(1000, 1000); > + if (--timeout == 0) { > + pr_err("Tegra PCIe error: timeout waiting for PLL\n"); > + return -EBUSY; > + } > } while (!(val & PADS_PLL_CTL_LOCKDET)); Thierry pointed out that one of NVIDIA's downstream kernels uses a timeout of 300 here, rather than 2000 above. Do you see a specific need for this timeout for be 2000 rather than 300? It might be nice to be consistent. Olof, I notice you've already applied V1 of this, which has the return statement issue. Can you replace it with this, or should Dmitry send an incremental patch? -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12:09 Mon 12 Mar , Stephen Warren wrote: > Thierry pointed out that one of NVIDIA's downstream kernels uses a > timeout of 300 here, rather than 2000 above. Do you see a specific need > for this timeout for be 2000 rather than 300? It might be nice to be > consistent. No, there's no specific need for it to be 2000 - it may as well be 300. I just wanted to stay on the safe side, but I think 300 should be still more than enough time for PLL to lock. > > Olof, I notice you've already applied V1 of this, which has the return > statement issue. Can you replace it with this, or should Dmitry send an > incremental patch? Yes, V1 breaks more things than it fixes, so it would be nice if it can be replaced with fixed version (I hope it's not too late yet). BTW, regarding timeout discussion above - should I resend patch with adjusted timeout, or can you change it while applying? (of course, if we settle on incremental patch, I'll roll this change in)
On Mon, Mar 12, 2012 at 12:30 PM, Dmitry Artamonow <mad_soft@inbox.ru> wrote: > On 12:09 Mon 12 Mar , Stephen Warren wrote: >> Thierry pointed out that one of NVIDIA's downstream kernels uses a >> timeout of 300 here, rather than 2000 above. Do you see a specific need >> for this timeout for be 2000 rather than 300? It might be nice to be >> consistent. > No, there's no specific need for it to be 2000 - it may as well be 300. > I just wanted to stay on the safe side, but I think 300 should be still > more than enough time for PLL to lock. > >> >> Olof, I notice you've already applied V1 of this, which has the return >> statement issue. Can you replace it with this, or should Dmitry send an >> incremental patch? > > Yes, V1 breaks more things than it fixes, so it would be nice if > it can be replaced with fixed version (I hope it's not too late yet). > BTW, regarding timeout discussion above - should I resend patch with > adjusted timeout, or can you change it while applying? (of course, > if we settle on incremental patch, I'll roll this change in) Please send an incremental patch which also solves the above question. :) -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-tegra/pcie.c b/arch/arm/mach-tegra/pcie.c index 14b29ab..90f85eb 100644 --- a/arch/arm/mach-tegra/pcie.c +++ b/arch/arm/mach-tegra/pcie.c @@ -585,10 +585,10 @@ static void tegra_pcie_setup_translations(void) afi_writel(0, AFI_MSI_BAR_SZ); } -static void tegra_pcie_enable_controller(void) +static int tegra_pcie_enable_controller(void) { u32 val, reg; - int i; + int i, timeout; /* Enable slot clock and pulse the reset signals */ for (i = 0, reg = AFI_PEX0_CTRL; i < 2; i++, reg += 0x8) { @@ -639,8 +639,14 @@ static void tegra_pcie_enable_controller(void) pads_writel(0xfa5cfa5c, 0xc8); /* Wait for the PLL to lock */ + timeout = 2000; do { val = pads_readl(PADS_PLL_CTL); + usleep_range(1000, 1000); + if (--timeout == 0) { + pr_err("Tegra PCIe error: timeout waiting for PLL\n"); + return -EBUSY; + } } while (!(val & PADS_PLL_CTL_LOCKDET)); /* turn off IDDQ override */ @@ -671,7 +677,7 @@ static void tegra_pcie_enable_controller(void) /* Disable all execptions */ afi_writel(0, AFI_FPCI_ERROR_MASKS); - return; + return 0; } static void tegra_pcie_xclk_clamp(bool clamp) @@ -921,7 +927,9 @@ int __init tegra_pcie_init(bool init_port0, bool init_port1) if (err) return err; - tegra_pcie_enable_controller(); + err = tegra_pcie_enable_controller(); + if (err) + return err; /* setup the AFI address translations */ tegra_pcie_setup_translations();
Tegra PCIe driver waits for PLL to lock using busy loop. If PLL fails to lock for some reason, this leads to silent lockup while booting (PCIe code is not modular). Fix by adding timeout, so if PLL doesn't lock in a couple of seconds, just PCIe driver fails and machine continues to boot. Signed-off-by: Dmitry Artamonow <mad_soft@inbox.ru> --- Changes v1 -> v2: * msleep is changed to usleep_range * function now returns 0 on success instead of undefined value arch/arm/mach-tegra/pcie.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)