linux-kernel - Re: [PATCH 2/3] PWM: vt8500: Update vt8500 PWM driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121023093128.GR21164@n2100.arm.linux.org.uk>
Date:	Tue, 23 Oct 2012 10:31:28 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Thierry Reding <thierry.reding@...onic-design.de>
Cc:	Tony Prisk <linux@...sktech.co.nz>,
	devicetree-discuss@...ts.ozlabs.org, arm@...nel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH 2/3] PWM: vt8500: Update vt8500 PWM driver support

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");

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/