linux-kernel - Re: [PATCH v3 2/4] clk: arizona: Add clock driver for the Arizona devices

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: <20160509104728.GG1646@localhost.localdomain>
Date:	Mon, 9 May 2016 11:47:28 +0100
From:	Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>
To:	Stephen Boyd <sboyd@...eaurora.org>
CC:	<mturquette@...libre.com>, <cw00.choi@...sung.com>,
	<lee.jones@...aro.org>, <myungjoo.ham@...sung.com>,
	<devicetree@...r.kernel.org>, <linux-clk@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>,
	<patches@...nsource.wolfsonmicro.com>
Subject: Re: [PATCH v3 2/4] clk: arizona: Add clock driver for the Arizona
 devices

On Fri, May 06, 2016 at 05:55:01PM -0700, Stephen Boyd wrote:
> I've applied this to clk-next but still have a question, see
> below.
> 
> On 01/08, Charles Keepax wrote:
> > diff --git a/drivers/clk/clk-arizona.c b/drivers/clk/clk-arizona.c
> > new file mode 100644
> > index 0000000..eaf2877
> > --- /dev/null
> > +++ b/drivers/clk/clk-arizona.c
> > +
> > +static int arizona_clk_of_get_pdata(struct arizona *arizona)
> > +{
> > +	const char * const pins[] = { "mclk1", "mclk2" };
> > +	struct clk *mclk;
> > +	int i;
> > +
> > +	if (!of_property_read_bool(arizona->dev->of_node, "clocks"))
> > +		return 0;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(pins); ++i) {
> > +		mclk = of_clk_get_by_name(arizona->dev->of_node, pins[i]);
> > +		if (IS_ERR(mclk))
> > +			return PTR_ERR(mclk);
> > +
> > +		if (clk_get_rate(mclk) == CLK32K_RATE) {
> > +			arizona->pdata.clk32k_src = ARIZONA_32KZ_MCLK1 + i;
> > +			arizona->pdata.clk32k_parent = __clk_get_name(mclk);
> > +		}
> > +
> > +		clk_put(mclk);
> 
> Could this be done through assigned parents instead of this rate
> checking stuff? Presumably DT could tell us how the clk tree
> should be configured.
> 

Apologies, I have been working on a v4 that includes these
improvements. It does indeed look much nicer using assigned
parents etc. I think it might be best to drop these for now until
those are ready to send.

The only problem I really have left to sort out before I can send
it are some locking issues. It is quite tricky to get interaction
between the clocking and SPI frameworks to play nicely. The SPI
framework will sometimes punt the actually processing for the
transfer to a worker thread which will often perform operations
on clocks required for the SPI. Because this is a seperate
thread it isn't handled by the re-enterant locking in the clock
framework. I had been working around this using async transfers
for the SPI, but even then I have since found you can get lockdep
warnings because of the potential mutex inversion (SPI mutex and
the clock one).

Any suggestions on this front would be greatly appreciated?

Thanks, Charles