From patchwork Fri Feb 7 13:06:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Emilio_L=C3=B3pez?= X-Patchwork-Id: 3604291 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E6D649F2E9 for ; Fri, 7 Feb 2014 13:07:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1361920108 for ; Fri, 7 Feb 2014 13:07:16 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EDB25200BE for ; Fri, 7 Feb 2014 13:07:14 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WBl9C-0004nL-0p; Fri, 07 Feb 2014 13:07:10 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WBl99-0000j5-Kb; Fri, 07 Feb 2014 13:07:07 +0000 Received: from yotta.elopez.com.ar ([2a00:1768:1004:d00d:c0de:4:f00d:cafe]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WBl95-0000hy-JQ for linux-arm-kernel@lists.infradead.org; Fri, 07 Feb 2014 13:07:04 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=elopez.com.ar; s=mail; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:References:Subject:CC:To:MIME-Version:From:Date:Message-ID; bh=5kDXrNZ9pjUOW4AOySGsLrkgBiPpQEI8ffQERcTWufc=; b=EBz4oU9TrCfJ9tFQ944sK1Qm4HI7VPMsr3w7CaTX1VROCGSjIza8Mq7G5rLasmYw4yimGQeoOKNj4bpm/EPIRYl1K0IJZTE3aaXIxfc8EupF8KU3NzdISWCNLYxfjXzI7jhhptiwfobGUFbIckgsBcySlKWt8rP7TmmIFlw74A93on1VB1F62GozbobTVaEeok9In39A0u4vJQwDc0KAeFvFUJsbVgdboXZ+Ur9oVfaNGbO3WghFUK6nQR2+m5hYr6MpOIDKfnC6uhttLXF6WTkCrNagV4kufJVu8qHjuebAM/KY1TUAjrD8T2GwHHNrG5blEMsUlPArUYm9gH/DjA==; Received: from yotta.elopez.com.ar ([31.220.24.173] helo=[0.0.0.0]) by yotta.elopez.com.ar with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.82) id 1WBl8J-0006U1-G2; Fri, 07 Feb 2014 10:06:17 -0300 Message-ID: <52F4DA40.4090804@elopez.com.ar> Date: Fri, 07 Feb 2014 10:06:08 -0300 From: =?ISO-8859-1?Q?Emilio_L=F3pez?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: Gregory CLEMENT , Mike Turquette , Ezequiel Garcia Subject: Re: [PATCH] clk: respect the clock dependencies in of_clk_init References: <1391554766-11285-1-git-send-email-gregory.clement@free-electrons.com> In-Reply-To: <1391554766-11285-1-git-send-email-gregory.clement@free-electrons.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140207_080703_914757_B04C8B27 X-CRM114-Status: GOOD ( 24.14 ) X-Spam-Score: -2.6 (--) Cc: Thomas Petazzoni , Andrew Lunn , Jason Cooper , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,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 Hi all, El 04/02/14 19:59, Gregory CLEMENT escribió: > Until now the clock providers were initialized in the order found in > the device tree. This led to have the dependencies between the clocks > not respected: children clocks could be initialized before their > parent clocks. > > Instead of forcing each platform to manage its own initialization order, > this patch adds this work inside the framework itself. > > Using the data of the device tree the of_clk_init function now delayed > the initialization of a clock provider if its parent provider was not > ready yet. > > Signed-off-by: Gregory CLEMENT After discussing this on IRC with Ezequiel, I must say I'm not really fond of this solution, as it adds unnecessary complexity to the framework and slows down clock registration to solve one particular problem that can be resolved in a simpler, less intrusive way. The triggering issue which has resulted on this patch is really a simple dependency on the name of the parent clock - something that should not be an issue if DT is used to provide them (see of_clk_get_parent_name), and even when this is not the case, it should not require poking the clock tree to find out. It wouldn't be hard to fix however, even if you cannot modify your device trees to provide proper naming; a tiny patch like the one below should suffice (Ezequiel has kindly tested it today on a a370-rd and found it working correctly). In other words, I feel this patch is a pretty good demonstration of over-engineering and bloating of an otherwise good framework to cover up for bugs/bad design on other parts of the kernel. Think about it; why does the framework require char *parent instead of struct clk *parent to link a child with their parent? Randomly-ordered registration is a pretty core principle of it; whether by design or not. So, until someone has a real reason to enforce dependencies on clock registration time, I would like this to stay uncommited. And, if the time comes and something like this is ever needed, *please* make it optional (ie, have a special macro and separate table to register clocks which are dependency-sensitive, and do so after registering all the non-dependency-sensitive clocks). Cheers, Emilio -----8<------ From ffdb49506e3ce92090c15e1f9b37f4d465097ac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20L=C3=B3pez?= Date: Thu, 6 Feb 2014 18:07:07 -0300 Subject: [PATCH] clk: mvebu: fix name dependency during registration time Currently, mvebu_clk_gating_setup has a silly dependency on clock registration order just to gather the parent clock name. This is completely unnecesary, as it supports using an already provided name via the clk_gating_soc_desc structs, and we can therefore solve this issue with a 69+/- line patch. But, given that the parent name is always "tclk" as default-hardcoded on mvebu_coreclk_setup(), we can just default-hardcode it here too and get away with solving this problem with a one-liner. Tested-by: Ezequiel Garcia --- drivers/clk/mvebu/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base = of_iomap(np, 0); diff --git a/drivers/clk/mvebu/common.c b/drivers/clk/mvebu/common.c index 25ceccf..6c63b43 100644 --- a/drivers/clk/mvebu/common.c +++ b/drivers/clk/mvebu/common.c @@ -121,7 +121,7 @@ void __init mvebu_clk_gating_setup(struct device_node *np, struct clk_gating_ctrl *ctrl; struct clk *clk; void __iomem *base; - const char *default_parent = NULL; + const char *default_parent = "tclk"; int n;