Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030912AbcDMLbg (ORCPT ); Wed, 13 Apr 2016 07:31:36 -0400 Received: from mail-qk0-f195.google.com ([209.85.220.195]:36850 "EHLO mail-qk0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750848AbcDMLbc (ORCPT ); Wed, 13 Apr 2016 07:31:32 -0400 MIME-Version: 1.0 In-Reply-To: <570E2123.1000107@samsung.com> References: <1460459243-27120-1-git-send-email-alim.akhtar@samsung.com> <570E2123.1000107@samsung.com> Date: Wed, 13 Apr 2016 14:31:31 +0300 Message-ID: Subject: Re: [PATCH] clk: samsung: exynos7: Enable clocks for CMU_CCORE and CMU_FSYS0 blocks From: Tomasz Figa To: Sylwester Nawrocki Cc: Alim Akhtar , linux-arm-kernel , Thomas Abraham , =?UTF-8?Q?Krzysztof_Koz=C5=82owski?= , linux-kernel , "linux-samsung-soc@vger.kernel.org" , linux-clk@vger.kernel.org, Stephen Boyd , Michael Turquette Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2262 Lines: 59 2016-04-13 13:36 GMT+03:00 Sylwester Nawrocki : > On 04/13/2016 08:26 AM, Tomasz Figa wrote: >>> > @@ -205,7 +206,11 @@ static struct samsung_cmu_info topc_cmu_info __initdata = { >>> > >>> > static void __init exynos7_clk_topc_init(struct device_node *np) >>> > { >>> > + struct clk *clk; >>> > + >>> > samsung_cmu_register_one(np, &topc_cmu_info); >>> > + clk = __clk_lookup("aclk_ccore_133"); >>> > + clk_prepare_enable(clk); >> >> Shouldn't this be rather done before calling >> samsung_cmu_register_one()? I don't remember exactly, but wouldn't >> clock registration trigger reading back current (mux, div) values from >> registers? >> >> Also, do we have any guarantees on order of initialization of >> particular CMUs? I believe this will happen in order of DT nodes and >> so would be not any kind of guarantee at all. > > If these clocks need to be kept enabled perhaps it's better to just set > CLK_IS_CRITICAL flag for them? Patches adding this flag are already in > clk-next branch in the clk git tree. This way the clocks would get > enabled within the clk_register() call. > Oh, I didn't know about this. Sounds like a good idea. :) > The CMU registration order is enforced by listing input clocks to each > CMU in DT. Ah, right, I recall now, I even reviewed relevant patch. Thanks for refreshing my memory. > However, I wouldn't be concerned much about it in context > of this patch. We are enabling here clocks which belong to same CMU. > samsung_cmu_register_one() needs to be called first for subsequent > __clk_lookup() calls to work. I thought a clock from another CMU has to be enabled for the CPU to be able to access the CMU being registered. > > Perhaps related bits need to be set manually in CMU registers before > registering a clock provider for the CMU, to fulfil the requirements. > > Anyway, summary of the $subject patch seems not precise enough: > > "This patch enables clocks for CMU_CCORE and CMU_FSYS0 blocks. This is > required before accessing registers of these blocks." > > We need to enable selected clocks (i.e. access the CMU's registers) > before accessing this CMU's registers? Yeah, maybe the explanation could be a bit more precise. Best regards, Tomasz