Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753799AbbDPUha (ORCPT ); Thu, 16 Apr 2015 16:37:30 -0400 Received: from mail-wg0-f44.google.com ([74.125.82.44]:36615 "EHLO mail-wg0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751486AbbDPUhY (ORCPT ); Thu, 16 Apr 2015 16:37:24 -0400 Message-ID: <55301D7F.30708@gmail.com> Date: Thu, 16 Apr 2015 22:37:19 +0200 From: Sebastian Hesselbarth User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Michael Welling , Tero Kristo CC: Mike Turquette , Stephen Boyd , Linux OMAP Mailing List , "linux-kernel@vger.kernel.org" , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , devicetree , "linux-arm-kernel@lists.infradead.org" , Tony Lindgren , Greg Kroah-Hartman , Daniel Mack Subject: Re: AM335x OMAP2 common clock external fixed-clock registration References: <20150414211734.GA28974@deathray> <552E0688.1020903@ti.com> <20150415140945.GA30787@deathray> <552EB152.7050306@ti.com> <20150415194707.GA11007@deathray> <20150415205136.GA3399@deathray> <552F3B60.8050703@ti.com> <20150416161756.GA27590@deathray> In-Reply-To: <20150416161756.GA27590@deathray> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2162 Lines: 52 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 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/