From patchwork Tue Mar 31 08:59:39 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Javier Martinez Canillas X-Patchwork-Id: 6128181 Return-Path: X-Original-To: patchwork-linux-samsung-soc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B1DDABF4A6 for ; Tue, 31 Mar 2015 09:00:01 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9EE7020166 for ; Tue, 31 Mar 2015 09:00:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 892162010B for ; Tue, 31 Mar 2015 08:59:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753581AbbCaI7s (ORCPT ); Tue, 31 Mar 2015 04:59:48 -0400 Received: from bhuna.collabora.co.uk ([93.93.135.160]:44161 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752512AbbCaI7p (ORCPT ); Tue, 31 Mar 2015 04:59:45 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: javier) with ESMTPSA id 0D5B860074A Message-ID: <551A61FB.1040401@collabora.co.uk> Date: Tue, 31 Mar 2015 10:59:39 +0200 From: Javier Martinez Canillas User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.2.0 MIME-Version: 1.0 To: Michael Turquette , Tomasz Figa CC: Stephen Boyd , Sylwester Nawrocki , Kukjin Kim , Olof Johansson , Doug Anderson , Krzysztof Kozlowski , Kevin Hilman , Tyler Baker , Abhilash Kesavan , Chanwoo Choi , linux-arm-kernel , "linux-samsung-soc@vger.kernel.org" , linux-kernel , Tomeu Vizoso Subject: Re: [RFC PATCH v3 1/2] clk: samsung: Add a clock lookup function References: <1427730803-28635-1-git-send-email-javier.martinez@collabora.co.uk> <1427730803-28635-2-git-send-email-javier.martinez@collabora.co.uk> <55197508.5030501@collabora.co.uk> <20150331014013.25195.26862@quantum> In-Reply-To: <20150331014013.25195.26862@quantum> Sender: linux-samsung-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-samsung-soc@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP +Tomeu who I forgot to add to the cc list. Hello Mike, Thanks a lot for your feedback. On 03/31/2015 03:40 AM, Michael Turquette wrote: >> >> I don't performance is a big issue here. I just thought that since the >> lookup table is already filled by the driver and the lookup function >> is one line, we could use that instead to get the performance benefit. >> >> But I don't mind to drop this patch and use the generic lookup function >> from the CCF API if that is preferred. > > Hello, > > I am not a fan of __clk_lookup and I don't like to see it used more and > more outside of drivers/clk/clk.c. You mentioned that performance wasn't > really the problem here. The real method for a driver to get a clock is > with clk_get(). Any reason to not use that? > I can certainly use clk_get() but I thought that the clk consumer API was not supposed to be used from within clock drivers. That's why I mentioned __clk_lookup() as a possibility since that is part of the provider API. Below is a RFC patch that uses clk_get() [0]. That needs another patch which was part of a previous RFC and adds an alias for the mdma0 clock: https://lkml.org/lkml/2015/3/27/769 If you think that is the correct approach then I can post it as a patch. It would be great if you can also provide some feedback about the other patch in the first RFC that instead of enabling and disabling the mdma0 clock in driver, does it in the exynos5420 platform PM callbacks: https://lkml.org/lkml/2015/3/27/770 I was asked to do it in the exynos5420 clk driver instead but maybe you have a different opinion on that. Best regards, Javier [0]: From c118df83da8cac65cc218ae9443622592222f5d0 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Mon, 30 Mar 2015 17:11:40 +0200 Subject: [RFC] clk: exynos5420: Make sure MDMA0 clock is enabled during suspend Commit ae43b3289186 ("ARM: 8202/1: dmaengine: pl330: Add runtime Power Management support v12") added pm support for the pl330 dma driver but it makes the clock for the Exynos5420 MDMA0 DMA controller to be gated during suspend and this clock needs to remain enabled in order to make the system resume from a system suspend state. To make sure that the clock is enabled during suspend, enable it prior to entering a suspend state and disable it once the system has resumed. Thanks to Abhilash Kesavan for figuring out that this was the issue. Fixes: ae43b32 ("ARM: 8202/1: dmaengine: pl330: Add runtime Power Management support v12") Signed-off-by: Javier Martinez Canillas --- drivers/clk/samsung/clk-exynos5420.c | 44 ++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/drivers/clk/samsung/clk-exynos5420.c b/drivers/clk/samsung/clk-exynos5420.c index 8b49e8b3b548..02029cf9fcb8 100644 --- a/drivers/clk/samsung/clk-exynos5420.c +++ b/drivers/clk/samsung/clk-exynos5420.c @@ -155,6 +155,7 @@ static enum exynos5x_soc exynos5x_soc; #ifdef CONFIG_PM_SLEEP static struct samsung_clk_reg_dump *exynos5x_save; static struct samsung_clk_reg_dump *exynos5800_save; +static struct clk **exynos5x_clks; /* * list of controller registers to be saved and restored during a @@ -275,8 +276,17 @@ static const struct samsung_clk_reg_dump exynos5420_set_clksrc[] = { { .offset = GATE_IP_PERIC, .value = 0xffffffff, }, }; +/* + * list of clocks that have to be kept enabled during suspend/resume cycle. + */ +static const char *exynos5x_clk_pm[] __initdata = { + "mdma0", +}; + static int exynos5420_clk_suspend(void) { + int i; + samsung_clk_save(reg_base, exynos5x_save, ARRAY_SIZE(exynos5x_clk_regs)); @@ -287,11 +297,19 @@ static int exynos5420_clk_suspend(void) samsung_clk_restore(reg_base, exynos5420_set_clksrc, ARRAY_SIZE(exynos5420_set_clksrc)); + for (i = 0; i < ARRAY_SIZE(exynos5x_clk_pm); i++) + clk_prepare_enable(exynos5x_clks[i]); + return 0; } static void exynos5420_clk_resume(void) { + int i; + + for (i = 0; i < ARRAY_SIZE(exynos5x_clk_pm); i++) + clk_disable_unprepare(exynos5x_clks[i]); + samsung_clk_restore(reg_base, exynos5x_save, ARRAY_SIZE(exynos5x_clk_regs)); @@ -307,6 +325,9 @@ static struct syscore_ops exynos5420_clk_syscore_ops = { static void exynos5420_clk_sleep_init(void) { + int i; + int clk_len = ARRAY_SIZE(exynos5x_clk_pm); + exynos5x_save = samsung_clk_alloc_reg_dump(exynos5x_clk_regs, ARRAY_SIZE(exynos5x_clk_regs)); if (!exynos5x_save) { @@ -323,8 +344,31 @@ static void exynos5420_clk_sleep_init(void) goto err_soc; } + exynos5x_clks = kzalloc(sizeof(struct clk *) * clk_len, GFP_KERNEL); + if (!exynos5x_clks) + goto err_clks; + + for (i = 0; i < clk_len; i++) { + exynos5x_clks[i] = clk_get(NULL, exynos5x_clk_pm[i]); + if (IS_ERR(exynos5x_clks[i])) { + pr_warn("Failed to get %s clk (%ld)\n", + exynos5x_clk_pm[i], PTR_ERR(exynos5x_clks[i])); + + while (i--) + clk_put(exynos5x_clks[i]); + + goto err_clkget; + } + } + register_syscore_ops(&exynos5420_clk_syscore_ops); return; +err_clkget: + kfree(exynos5x_clks); +err_clks: + kfree(exynos5800_save); + pr_warn("%s: failed to allocate suspend clocks, no sleep support!\n", + __func__); err_soc: kfree(exynos5x_save); pr_warn("%s: failed to allocate sleep save data, no sleep support!\n",