On Wed, Oct 02, 2019 at 02:59:03PM -0600, Stephen Warren wrote: > On 10/2/19 5:04 AM, Thierry Reding wrote: > > On Tue, Oct 01, 2019 at 03:13:43PM -0600, Stephen Warren wrote: > > > From: Stephen Warren > > > > > > For a little over a year, U-Boot has configured the flow controller to > > > perform automatic RAM re-repair on off->on power transitions of the CPU > > > rail1]. This is mandatory for correct operation of Tegra124. However, RAM > > > re-repair relies on certain clocks, which the kernel must enable and > > > leave running. The fuse clock is one of those clocks. Enable this clock > > > so that LP1 power mode (system suspend) operates correctly. > > > > > > [1] 3cc7942a4ae5 ARM: tegra: implement RAM repair > > > > > > Reported-by: Jonathan Hunter > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Stephen Warren > > > --- > > > drivers/clk/tegra/clk-tegra124.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c > > > index 0224fdc4766f..f53f6315c646 100644 > > > --- a/drivers/clk/tegra/clk-tegra124.c > > > +++ b/drivers/clk/tegra/clk-tegra124.c > > > @@ -1291,6 +1291,7 @@ static struct tegra_clk_init_table common_init_table[] __initdata = { > > > }; > > > static struct tegra_clk_init_table tegra124_init_table[] __initdata = { > > > + { TEGRA124_CLK_FUSE, -1, 0, 1 }, > > > > I think the correct way to do this these days is to mark the clock as > > CRITICAL. Not sure if there's an easy way to do that given that the > > clock init table doesn't allow storing flags. > > > > Do you have any good ideas on how to achieve this with the critical flag > > instead of forcing the refcount to 1? > > > > Perhaps something like the below would work? > > ... > > The following works for me; does this seem like a reasonable approach? It > does set the critical flag for all SoCs, including any that don't require > RAM re-repair. I'm not sure which do; I know it's more than just Tegra124, > but I'm not sure how far back/forward the requirement goes. > > > diff --git a/drivers/clk/tegra/clk-tegra-periph.c b/drivers/clk/tegra/clk-tegra-periph.c > > index 1ed85f120a1b..76dd91eebd13 100644 > > --- a/drivers/clk/tegra/clk-tegra-periph.c > > +++ b/drivers/clk/tegra/clk-tegra-periph.c > > @@ -785,7 +785,7 @@ static struct tegra_periph_init_data gate_clks[] = { > > GATE("ahbdma", "hclk", 33, 0, tegra_clk_ahbdma, 0), > > GATE("apbdma", "pclk", 34, 0, tegra_clk_apbdma, 0), > > GATE("kbc", "clk_32k", 36, TEGRA_PERIPH_ON_APB | TEGRA_PERIPH_NO_RESET, tegra_clk_kbc, 0), > > - GATE("fuse", "clk_m", 39, TEGRA_PERIPH_ON_APB, tegra_clk_fuse, 0), > > + GATE("fuse", "clk_m", 39, TEGRA_PERIPH_ON_APB, tegra_clk_fuse, CLK_IS_CRITICAL), > > GATE("fuse_burn", "clk_m", 39, TEGRA_PERIPH_ON_APB, tegra_clk_fuse_burn, 0), > > GATE("kfuse", "clk_m", 40, TEGRA_PERIPH_ON_APB, tegra_clk_kfuse, 0), > > GATE("apbif", "clk_m", 107, TEGRA_PERIPH_ON_APB, tegra_clk_apbif, 0), It's probably fine to do this. The patch I proposed would've restricted the change to just Tegra124. But if we need this on other generations, I don't think the extra complexity is justified, especially since I can't imagine that the FUSE clock remaining always on would consume a lot of extra power. Thierry