Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932839Ab2JWJb5 (ORCPT ); Tue, 23 Oct 2012 05:31:57 -0400 Received: from caramon.arm.linux.org.uk ([78.32.30.218]:57197 "EHLO caramon.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932768Ab2JWJb4 (ORCPT ); Tue, 23 Oct 2012 05:31:56 -0400 Date: Tue, 23 Oct 2012 10:31:28 +0100 From: Russell King - ARM Linux To: Thierry Reding Cc: Tony Prisk , devicetree-discuss@lists.ozlabs.org, arm@kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Arnd Bergmann Subject: Re: [PATCH 2/3] PWM: vt8500: Update vt8500 PWM driver support Message-ID: <20121023093128.GR21164@n2100.arm.linux.org.uk> References: <1350643135-13197-1-git-send-email-linux@prisktech.co.nz> <1350643135-13197-2-git-send-email-linux@prisktech.co.nz> <20121022063423.GA17181@avionic-0098.mockup.avionic-design.de> <1350888712.3592.11.camel@gitbox> <1350889747.3592.17.camel@gitbox> <20121022072448.GB30026@avionic-0098.mockup.avionic-design.de> <1350891382.3592.22.camel@gitbox> <20121022080414.GB4931@avionic-0098.mockup.avionic-design.de> <1350981706.20938.7.camel@gitbox> <20121023092247.GA13220@avionic-0098.mockup.avionic-design.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20121023092247.GA13220@avionic-0098.mockup.avionic-design.de> User-Agent: Mutt/1.5.19 (2009-01-05) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2594 Lines: 57 On Tue, Oct 23, 2012 at 11:22:47AM +0200, Thierry Reding wrote: > On Tue, Oct 23, 2012 at 09:41:46PM +1300, Tony Prisk wrote: > > Further to the discussion, my preference is still for of_clk_get() > > (although I've changed the patch anyway as you saw because it makes no > > difference in this case) :) > > > > clk_get(x, NULL) and devm_clk_get(x, NULL) both seems like 'hacks' to > > allow platforms to convert to DT without having to update all their > > drivers first. It only allows the first (default) clock, as your pointed > > out. Getting a 2nd... clock relies on an optional property in DT (which > > again, seems like it is there to support 'old' drivers) which allows you > > to request clocks by name. > > > > of_clk_get() on the other hand seems like a properly native DT function. > > You don't need to know anything about the clock, as long as the correct > > clock is specified in the correct order as documented by the binding. > > Relying on 'pre-OF' code for a OF-only driver also seems > > counter-intuitive. > > I do agree with those arguments. What I was saying is that for drivers > which aren't DT only, of_clk_get() is not an option and that maybe > others would be encouraged by the example to not use the generic APIs > even if their driver could be used in non-DT setups. But maybe I'm > worrying needlessly. > > That said, maybe somebody with a broader view of things like Arnd > (Cc'ed) could share his thoughts. As I have already said, the way the DT bindings were done for the clk stuff was wrong. A little thought put into it would've come up with a much better solution which wouldn't have needed of_clk_get() at all. How? The arguments for clk_get() are: 1. the struct device, which you can get the OF-node from. 2. a _device_ _specific_ _clock_ _input_ _name_ (or NULL if there's only one.) So, we have something that defines a hardware clock input name, which can be used to generate a property name for OF. So, what _could_ have been done is this: clock- = <&provider-node clk-output-index>; where the property name is generated by: snprintf(prop, sizeof(prop), "clk-%s", name ? name : "default"); So I continue to assert that our current design is wrong - and it will cause driver authors to pointlessly have to make a choice at every stage between DT and non-DT based systems. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/