lkml.org 
[lkml]   [2012]   [Oct]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/3] PWM: vt8500: Update vt8500 PWM driver support
On Tue, Oct 23, 2012 at 10:31:28AM +0100, Russell King - ARM Linux wrote:
> 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-<input-name> = <&provider-node clk-output-index>;
>
> where the property name is generated by:
>
> snprintf(prop, sizeof(prop), "clk-%s", name ? name : "default");

But we already have this, only with slightly different syntax:

clocks = <&provider foo-index>, <&provider bar-index>;
clock-names = "foo", "bar";

> 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.

I think the reason that Tony brought this up is that with this API, the
clock-names property becomes mandatory if you have more than one input
clock.

Thierry
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2012-10-23 13:01    [W:0.085 / U:0.404 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site