Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758152AbcDEL0u (ORCPT ); Tue, 5 Apr 2016 07:26:50 -0400 Received: from arroyo.ext.ti.com ([192.94.94.40]:52125 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757849AbcDEL0r (ORCPT ); Tue, 5 Apr 2016 07:26:47 -0400 From: "Franklin S Cooper Jr." To: Sekhar Nori , "Kristo, Tero" CC: , , , , , , , , , , , , , , , Subject: Re: [PATCH v5 1/6] pwms: pwm-ti*: Get the clock from the PWMSS (parent) References: <1457400224-24797-1-git-send-email-fcooper@ti.com> <1457400224-24797-2-git-send-email-fcooper@ti.com> <5703564C.7090700@ti.com> Message-ID: <5703A0C4.6010406@ti.com> Date: Tue, 5 Apr 2016 06:25:56 -0500 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: <5703564C.7090700@ti.com> 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: 2169 Lines: 52 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. > > Thanks, > Sekhar >