From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH v3] clk: x86: Do not gate clocks enabled by the firmware To: Carlo Caione , alan@linux.intel.com Cc: Michael Turquette , "open list:COMMON CLK FRAMEWORK" , Darren Hart , Stephen Boyd , Linux Upstreaming Team , Enric Balletbo Serra , andriy.shevchenko@linux.intel.com, Carlo Caione References: <20170714082356.28117-1-carlo@caione.org> From: Pierre-Louis Bossart Message-ID: Date: Thu, 7 Sep 2017 06:59:45 -0500 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed List-ID: On 9/7/17 4:22 AM, Carlo Caione wrote: > On Fri, Jul 14, 2017 at 10:23 AM, Carlo Caione wrote: >> From: Carlo Caione >> >> Read the enable register to determine if the clock is already in use by >> the firmware. In this case avoid gating the clock. >> >> Tested-by: Enric Balletbo i Serra >> Acked-by: Andy Shevchenko >> Acked-by: Darren Hart (VMware) >> Signed-off-by: Carlo Caione >> --- >> drivers/clk/x86/clk-pmc-atom.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/drivers/clk/x86/clk-pmc-atom.c b/drivers/clk/x86/clk-pmc-atom.c >> index 2b60577703ef..be8d821ce625 100644 >> --- a/drivers/clk/x86/clk-pmc-atom.c >> +++ b/drivers/clk/x86/clk-pmc-atom.c >> @@ -185,6 +185,13 @@ static struct clk_plt *plt_clk_register(struct platform_device *pdev, int id, >> pclk->reg = base + PMC_CLK_CTL_OFFSET + id * PMC_CLK_CTL_SIZE; >> spin_lock_init(&pclk->lock); >> >> + /* >> + * If the clock was already enabled by the firmware mark it as critical >> + * to avoid it being gated by the clock framework if no driver owns it. >> + */ >> + if (plt_clk_is_enabled(&pclk->hw)) >> + init.flags |= CLK_IS_CRITICAL; >> + >> ret = devm_clk_hw_register(&pdev->dev, &pclk->hw); >> if (ret) { >> pclk = ERR_PTR(ret); > > Hi, > > I just discovered that this fix is not working anymore on my platform > (with a baytrail processor). > This fix was needed on my setup because the clock pmc_plt_clk_4 used > by the Ethernet and enabled by the firmware, was being gated by the > clock framework since the r8169 driver was failing to claim it. You > can read the whole discussion about this in [0]. This fix was also > needed for some other audio machine drivers. > > I bisected the problem down to commit 49d25364df ("staging/atomisp: > Add support for the Intel IPU v2"). The clock driver is basically > shutting down all the clocks (see [1]) at probe time, so the > clk-pmc-atom driver is seeing all the clocks already disabled when > going to check. > > Any idea how to proper fix this? Looks like the same code that was initially used as a reference for support of PMC clocks in the clock framework, so now we have 2 drivers programming the same PMC hardware, not so good. We'd probably have to move the atomisp driver to move to clk_get/prepare/enable instead of the old vlv2_clck_get? > > Cheers, > > [0] https://www.spinics.net/lists/platform-driver-x86/msg12092.html > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/media/atomisp/platform/clock/vlv2_plat_clock.c?h=v4.13#n200 >