linux-kernel - Re: AM335x OMAP2 common clock external fixed-clock registration

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: <55301D7F.30708@gmail.com>
Date:	Thu, 16 Apr 2015 22:37:19 +0200
From:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
To:	Michael Welling <mwelling@...e.org>, Tero Kristo <t-kristo@...com>
CC:	Mike Turquette <mturquette@...aro.org>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Russell King <linux@....linux.org.uk>,
	devicetree <devicetree@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Tony Lindgren <tony@...mide.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Daniel Mack <daniel@...que.org>
Subject: Re: AM335x OMAP2 common clock external fixed-clock registration

On 16.04.2015 18:17, Michael Welling wrote:
> On Thu, Apr 16, 2015 at 07:32:32AM +0300, Tero Kristo wrote:
>> On 04/15/2015 11:51 PM, Michael Welling wrote:
>>> On Wed, Apr 15, 2015 at 01:45:53PM -0700, Mike Turquette wrote:
>>>> On Wed, Apr 15, 2015 at 12:47 PM, Michael Welling <mwelling@...e.org> wrote:
[...]
>>>>> There is still an issue with the si5351.
>>>>>
>>>>> I had to comment out the clk_put here for the frequency to show up:
>>>>> http://lxr.free-electrons.com/source/drivers/clk/clk-si5351.c#L1133
>>>>>
>>>>> Ideas?
>>>>
>>>> What is the most recent upstream commit that you are based on?
>>>
>>> I am working from 4.0.0-rc7.
>>>
>>> 7b43b47373d40d557cd7e1a84a0bd8ebc4d745ab
>>
>> Hmm, I wonder why si5351 calls clk_put immediately after of_clk_get
>> in the first place, as far as I understand this destroys the clock
>> handle, which is still being used later in the code.
>
> Not sure how this ever worked. This has been in the code since the
> initial commit.

The reason it worked before may be related with recent rework of
clk_put() itself and clk cookies instead of pointers. I lost track on
the recent clk subsystem changes here, sorry.

However, droping the clk immediately surely isn't right.
The thing is, we can remove the clk_put() just because there is no
_remove() for that driver. I remember that back in the days the driver
was mainlined, clk removal wasn't too easy.

FWIW, as soon as _remove() support will be added by someone, we'll have
to rethink passing struct clk* by platform_data or at least
double-check if we ever used [of_]clk_get() to obtain it.

Mind to send a patch removing the clk_put() on !IS_ERR and add a proper
error path instead? While of_clk_get() is the only calls that need
cleanup on error in si5351_dt_parse() we should probably move that
calls to the end of this function. Otherwise we'd also have to cleanup
on every of_parse_foo() failure.

Sebastian

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