On 05/08/2013 04:53 AM, Daniel Lezcano wrote: > On 05/06/2013 10:39 PM, Stephen Warren wrote: >> From: Stephen Warren <[email protected]> >> >> Tegra20 HW appears to have a bug such that PCIe device interrupts, whether >> they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around >> this, simply disable LP2 if the PCI driver and DT node are both enabled. >> >> Signed-off-by: Stephen Warren <[email protected]> >> --- >> Thierry, >> >> This patch is physically based on next-20130506 so that it doesn't >> conflict with any of the recent cpuidle cleanup work. I'm sending it with >> the expectation that you'll apply it to your PCIe development branch >> though. If you do that, you'll see some conflicts unless you rebase your >> dev branch onto something more recent than next-20130422; I assume you'll >> do that rebase soon enough anyway, but if you weren't planning to, I can >> resend the patch based on your current dev branch. >> >> arch/arm/mach-tegra/cpuidle-tegra20.c | 30 ++++++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c >> b/arch/arm/mach-tegra/cpuidle-tegra20.c >> index 0cdba8d..d2c9349 100644 >> --- a/arch/arm/mach-tegra/cpuidle-tegra20.c >> +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c >> @@ -25,6 +25,7 @@ >> #include <linux/cpu_pm.h> >> #include <linux/clockchips.h> >> #include <linux/clk/tegra.h> >> +#include <linux/of.h> >> >> #include <asm/cpuidle.h> >> #include <asm/proc-fns.h> >> @@ -212,10 +213,39 @@ static int tegra20_idle_lp2_coupled(struct >> cpuidle_device *dev, >> } >> #endif >> >> +static const struct of_device_id pcie_matches[] __initconst = { >> + { .compatible = "nvidia,tegra20-pcie" }, >> + { } >> +}; >> + >> +/* >> + * Tegra20 HW appears to have a bug such that PCIe device interrupts, >> whether >> + * they are legacy IRQs or MSI, are lost when LP2 is enabled. To work around >> + * this, simply disable LP2 if the PCI driver and DT node are both enabled. >> + */ >> +static void __init tegra20_cpuidle_disable_lp2_with_pcie(void) >> +{ >> + struct device_node *np; >> + >> + if (!IS_ENABLED(CONFIG_PCI_TEGRA)) >> + return; >> + >> + np = of_find_matching_node(NULL, pcie_matches); >> + if (!np) >> + return; >> + >> + if (!of_device_is_available(np)) >> + return; >> + >> + pr_info("Disabling LP2 cpuidle state, since PCIe is enabled\n"); >> + tegra_idle_driver.state_count = 1; > > Won't be more clear to have the state disabled in the init function like > this ? > > int __init tegra20_cpuidle_init(void) > { > ... > if (tegra20_has_lp2_pcie_bug(void)) > tegra_idle_driver.states[1].disabled = true; > ... > }
I'm not sure that's any clearer on its own. One big advantage of that technique is that if we end up with 3 cpuidle states and need to disable the one in the middle, this alternative technique does work, whereas the one in my patch doesn't. So, I'll convert to this technique. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
