Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932461AbcDLILq (ORCPT ); Tue, 12 Apr 2016 04:11:46 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:60540 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932191AbcDLILk (ORCPT ); Tue, 12 Apr 2016 04:11:40 -0400 Subject: Re: [PATCH v5 1/6] pwms: pwm-ti*: Get the clock from the PWMSS (parent) To: Rob Herring References: <1457400224-24797-1-git-send-email-fcooper@ti.com> <1457400224-24797-2-git-send-email-fcooper@ti.com> <5703564C.7090700@ti.com> <5703A0C4.6010406@ti.com> <570B8F51.6040108@ti.com> CC: Paul Walmsley , "Franklin S Cooper Jr." , "Kristo, Tero" , Thierry Reding , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Benoit Cousson , Tony Lindgren , Russell King - ARM Linux , Linux PWM List , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , linux-omap , "linux-arm-kernel@lists.infradead.org" , Vignesh R From: Sekhar Nori Message-ID: <570CAD84.2090701@ti.com> Date: Tue, 12 Apr 2016 13:40:44 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3420 Lines: 73 On Monday 11 April 2016 06:15 PM, Rob Herring wrote: > On Mon, Apr 11, 2016 at 6:49 AM, Sekhar Nori wrote: >> On Monday 11 April 2016 02:21 AM, Paul Walmsley wrote: >>> On Tue, 5 Apr 2016, Franklin S Cooper Jr. wrote: >>>> On 04/05/2016 01:08 AM, Sekhar Nori wrote: >>>>> On Tuesday 08 March 2016 06:53 AM, Franklin S Cooper Jr wrote: >>>>>> The eCAP and ePWM doesn't have their own separate clocks. They simply >>>>>> utilize the clock provided directly by the PWMSS. Therefore, they simply >>>>>> need to grab a reference to their parent's clock. >>>>>> >>>>>> Signed-off-by: Franklin S Cooper Jr >>>>> >>>>> So this assumes that eCAP and eHRPWM are always under the PWMSS >>>>> umbrella. But on TI AM18x, thats not true. These IPs exist independently >>>>> and receive functional clock from PLL sysclk outputs. >>>>> >>>>>> --- >>>>>> drivers/pwm/pwm-tiecap.c | 2 +- >>>>>> drivers/pwm/pwm-tiehrpwm.c | 2 +- >>>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/pwm/pwm-tiecap.c b/drivers/pwm/pwm-tiecap.c >>>>>> index 616af76..9418159 100644 >>>>>> --- a/drivers/pwm/pwm-tiecap.c >>>>>> +++ b/drivers/pwm/pwm-tiecap.c >>>>>> @@ -212,7 +212,7 @@ static int ecap_pwm_probe(struct platform_device *pdev) >>>>>> if (!pc) >>>>>> return -ENOMEM; >>>>>> >>>>>> - clk = devm_clk_get(&pdev->dev, "fck"); >>>>>> + clk = devm_clk_get(pdev->dev.parent, "fck"); >>>>> >>>>> Even keeping the AM18x usecase aside, this seems to be pushing too much >>>>> platform information into the driver. The "fck" is a valid connection id >>>>> for the eCAP IP. Whether its valid for the parent device too is not >>>>> something this driver should need to know. >>>>> >>>>> So it looks like what you need is for the clock hierarchy for the >>>>> platform to have clocks for eHRPWM and eCAP derived out of PWMSS clock? >>>> >>>> So I believe this is a question on if we want to hide the minor >>>> delta between AM18 vs AM335x, AM437x and AM57x/DRA7 in the driver >>>> or within the DT. >>>> >>>> Note that handling this by defining new clocks in DT will then >>>> result in older DTBs not working. I don't think its worth breaking >>>> backwards compatibility for AM335x and AM437x DTBs for fixing support >>>> for AM18 based SOCs. Especially since those SOCs haven't worked with >>>> this driver for several years. By handling things within the driver rather >>>> than DT we can atleast insure that we can get everything working while >>>> avoiding breaking backwards compatibility. >>> >>> I agree with Sekhar that we shouldn't embed this parent clock quirk >>> into the driver. >>> >>> Can you just define a new compatibility string such that the driver can be >>> written with no embedded integration quirks? Then add a workaround in the >>> driver that will use pdev->dev.parent for the old (deprecated) >>> compatibility string and log a warning to the kernel console that the DT >>> needs to be updated. >> >> Thanks Paul! Although not sure if adding a new compatible for the IP is >> the best way (since that would denote a different version of the IP). >> How about checking for parent clock iff clk_get() on own device fails >> and of_machine_is_compatible() matches the platforms where backward >> compatibility needs to be maintained? > > New compatible strings are acceptable. Alright, thanks for clarifying that. Regards, Sekhar