From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752158AbcKCEUt (ORCPT ); Thu, 3 Nov 2016 00:20:49 -0400 Received: from vern.gendns.com ([206.190.152.46]:37463 "EHLO vern.gendns.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750838AbcKCEUs (ORCPT ); Thu, 3 Nov 2016 00:20:48 -0400 Subject: Re: [PATCH] ARM: davinci: da850: Fix pwm name matching To: Sekhar Nori , Kevin Hilman References: <1477418044-24603-1-git-send-email-david@lechnology.com> <3fd93d1b-8ba2-5440-e37d-0c55928e7e51@lechnology.com> Cc: Russell King , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org From: David Lechner Message-ID: <4ae45a27-3484-9d2c-4857-83d669851e72@lechnology.com> Date: Wed, 2 Nov 2016 23:20:42 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/01/2016 05:31 AM, Sekhar Nori wrote: > On Monday 31 October 2016 08:18 PM, David Lechner wrote: >> On 10/31/2016 05:18 AM, Sekhar Nori wrote: >>> On Tuesday 25 October 2016 11:24 PM, David Lechner wrote: >>>> This fixes pwm name matching for DA850 familiy devices. When using >>>> device >>>> tree, the da850_auxdata_lookup[] table caused pwm devices to have the >>>> exact >>>> same name, which caused errors when trying to register the devices. >>>> >>>> The names for clock matching in da850_clks[] also have to be updated to >>>> to exactly match in order for the clock lookup to work correctly. >>>> >>>> Signed-off-by: David Lechner >>>> --- >>>> >>>> Tested working on LEGO MINDSTORMS EV3. >>>> >>>> >>>> arch/arm/mach-davinci/da850.c | 10 +++++++--- >>>> arch/arm/mach-davinci/da8xx-dt.c | 10 +++++----- >>>> 2 files changed, 12 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/arch/arm/mach-davinci/da850.c >>>> b/arch/arm/mach-davinci/da850.c >>>> index ed3d0e9..6b78a8f 100644 >>>> --- a/arch/arm/mach-davinci/da850.c >>>> +++ b/arch/arm/mach-davinci/da850.c >>>> @@ -510,9 +510,13 @@ static struct clk_lookup da850_clks[] = { >>>> CLK("vpif", NULL, &vpif_clk), >>>> CLK("ahci_da850", NULL, &sata_clk), >>>> CLK("davinci-rproc.0", NULL, &dsp_clk), >>>> - CLK("ehrpwm", "fck", &ehrpwm_clk), >>>> - CLK("ehrpwm", "tbclk", &ehrpwm_tbclk), >>>> - CLK("ecap", "fck", &ecap_clk), >>>> + CLK("ehrpwm.0", "fck", &ehrpwm_clk), >>>> + CLK("ehrpwm.0", "tbclk", &ehrpwm_tbclk), >>>> + CLK("ehrpwm.1", "fck", &ehrpwm_clk), >>>> + CLK("ehrpwm.1", "tbclk", &ehrpwm_tbclk), >>>> + CLK("ecap.0", "fck", &ecap_clk), >>>> + CLK("ecap.1", "fck", &ecap_clk), >>>> + CLK("ecap.2", "fck", &ecap_clk), >>> >>> This has exposed a limitation of DaVinci clock framework. The struct clk >>> are stored as a linked list themselves. So a node repeating in the table >>> above will create a loop in the linked list. This is easily seen on the >>> LCDK board. davinci_clk_disable_unused() never returns. PWMs are unused >>> on that board. >>> >>> There is no "simple" solution to this AFAICS. One solution is to >>> separate the iterator from the clock hardware structure and use struct >>> clk_hw available in struct clk_lookup. >>> >>> Or move DaVinci to common clock framework. This is of course preferred >>> but much more involved as all 6 supported SoCs have to be moved together. >>> >>> Thanks, >>> Sekhar >>> >>> >> >> The simple solution for now could be to make child clocks for each of >> these that simply enable the parent clock. e.g. ehrpwm0_clk and >> ehpwm1_clk are children of ehrpwm_clk, etc. > > That sounds good. I saw the v2 patch you sent and it looks good. I > noticed that aemif clock is repeated in da850.c too. That needs fixing. Same with emac clock. >> Looking at da830.c, it looks like the solution was to make multiple >> clocks that use the same LPSC, but this does not seem right to me. > > The PWM clock definitions there are definitely wrong. It should be > fixed. Looks like its been like that forever. > > Thanks, > Sekhar >