linux-kernel - Re: [RFC] clk: add flags to distinguish xtal clocks

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: <20130705132107.GL19262@arwen.pp.htv.fi>
Date:	Fri, 5 Jul 2013 16:21:07 +0300
From:	Felipe Balbi <balbi@...com>
To:	James Hogan <james.hogan@...tec.com>
CC:	Luciano Coelho <coelho@...com>,
	Mike Turquette <mturquette@...aro.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	LKML <linux-kernel@...r.kernel.org>, <balbi@...com>
Subject: Re: [RFC] clk: add flags to distinguish xtal clocks

On Fri, Jul 05, 2013 at 02:12:08PM +0100, James Hogan wrote:
> On 4 July 2013 22:05, Luciano Coelho <coelho@...com> wrote:
> > Add a flag that indicate whether the clock is a crystal or not.  Since
> > no clocks set this flag right now, include an additional flag that
> > indicates whether the type is set or not.  If the CLK_IS_TYPE_DEFINED
> > flag is not set, the value of the CLK_IS_TYPE_XTAL flag is undefined.
> > This ensures backwards compatibility.
> >
> > Additionally, parse a new device tree binding in clk-fixed-rate to set
> > this flag.
> >
> > Signed-off-by: Luciano Coelho <coelho@...com>
> > ---
> >
> > I'm not  familiar with the common clock framework and I'm not
> > entirely sure the flags can be used in such a way, but to me it looks
> > reasonable, since some clock consumers may need to know what type of
> > clock is being provided.
> >
> > Specifically, the wl12xx firmware needs to know if the clock is XTAL
> > or not to handle the stabilization and boosts properly.
> >
> > My main idea is that I need to pass this information in the device
> > tree definition of the clocks, so that the driver can pass this
> > information on to the firmware.
> >
> > Please let me know if this looks ok or not.  If not, please let me
> > know if you have any other ideas on how to solve my problem (of
> > knowing whether the clock attached to the WiLink chip is XTAL or not).
> 
> The TZ1090 SoC has something that sounds possibly similar, where some
> of the XTAL pads have a bypass bit, which according to the hardware
> engineer I asked should be enabled when you want to use the
> corresponding XTAL pads as a clock input pad rather than an
> oscillator. I was considering extending clk-fixed-rate (via a wrapper
> driver) to parse a custom DT property and a register address / bit
> number and set the bypass bit as appropriate itself.
> 
> So I was wondering, is there a particular reason you don't have a DT
> property on the node for the device that needs to know what type of
> clock it is, rather than the clock node itself? That way you're not
> depending directly on the generic common clock framework to be able to
> tell you such electrical details.

three things here:

1) you end up with several devices implementing the clock type
attribute.

2) the type is a property of the clock itself

3) At least WiLink, can work with both types of clocks.

considering those, I really think this should belong to the clock node.
Otherwise Imagine how your DT would look like:

clock {
	compatible = "fixed-rate";
	...
};

wilink {
	clocks = &clock;
	wilink,btw-this-time-we-are-using-xtal;
	...
};

where you could:

clock {
	compatible = "fixed-rate";
	clock-type = "xtal";
	...
};

wilink {
	clocks = &clock;
	...
};

second option looks a lot cleaner to me.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)