On Fri, Jun 28, 2013 at 02:22:11PM +0300, Luciano Coelho wrote:

On Fri, 2013-06-28 at 13:31 +0300, Luciano Coelho wrote:

quoted

(fixed Mike's address)

On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote:

quoted

On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote:

quoted

On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote:

quoted

On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote:

quoted

+Optional properties:
+--------------------
+
+- refclock: the internal WLAN reference clock frequency (required for
+  WiLink6 and WiLink7; not used for WiLink8).  Must be one of the
+  following:
+	0 = 19.2 MHz
+	1 = 26.0 MHz
+	2 = 38.4 MHz
+	3 = 52.0 MHz
+	4 = 38.4 MHz, XTAL
+	5 = 26.0 MHz, XTAL
+
+- tcxoclock: the internal WLAN TCXO clock frequency (required for
+  WiLink7 not used for WiLink6 and WiLink8).  Must be one of the
+  following:
+	0 = 19.200 MHz
+	1 = 26.000 MHz
+	2 = 38.400 MHz
+	3 = 52.000 MHz
+	4 = 16.368 MHz
+	5 = 32.736 MHz
+	6 = 16.800 MHz
+	7 = 33.600 MHz

This looks suspiciously like what we have the common clock bindings for:

refclk {
	compatible = "fixed-clock";
	#clock-cells = <0>;
	clock-frequency = <19200000>;
}

wilink {
	compatible = "ti,wilink7";
	interrupt-parent = <&some_interrupt_controller>;
	interrupts = <0 1 1>;
	clocks = <&refclk>, <&refclk>;
	clock-names = "refclk", "txoclk";
};

Could you not use them?

Hmmm... this actually does look good.  But these are internal clocks in
the modules, they cannot be accessed from outside.  Does it make sense
to register them with the clock framework?

Given we already have a common way of describing clocks, I think it
makes sense to use it -- people already understand the common bindings,
and it's less code to add add to the kernel. I don't think the fact
these clocks are internal should prevent us from describing them as we
would an external clock.

Yes, I agree with you.  Thanks for the suggestion! I think it will look
much better.  And now that I dug a bit more into the code, I can see
that there are only structs being populated, so there shouldn't be any
other side-effects.

Hmmm, one thing that escaped me.  Besides the frequency, I also need a
boolean that tells if the clock is XTAL or not.  I can't figure out how
to pass this if I use the generic clock framework.  Any suggestions?

Could you use clock-output-names for that ?

XTAL clock:

refclk {
	compatible = "fixed-clock";
	#clock cells = <0>;
	clock-frequency = <19200000>;
	clock-output-names = "xtal";
};

non-XTAL clock:

refclk {
	compatible = "fixed-clock";
	#clock cells = <0>;
	clock-frequency = <19200000>;
	clock-output-names = "osc"; /* any better name ? */
};

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130628/eb516e5a/attachment.sig>