From patchwork Tue Jun 7 08:58:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lee Jones X-Patchwork-Id: 69484 Delivered-To: patch@linaro.org Received: by 10.140.106.246 with SMTP id e109csp1869179qgf; Tue, 7 Jun 2016 01:57:52 -0700 (PDT) X-Received: by 10.98.79.73 with SMTP id d70mr30422529pfb.120.1465289872485; Tue, 07 Jun 2016 01:57:52 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id m68si33936275pfj.187.2016.06.07.01.57.50; Tue, 07 Jun 2016 01:57:52 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754570AbcFGI5r (ORCPT + 31 others); Tue, 7 Jun 2016 04:57:47 -0400 Received: from mail-wm0-f41.google.com ([74.125.82.41]:36867 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754522AbcFGI5n (ORCPT ); Tue, 7 Jun 2016 04:57:43 -0400 Received: by mail-wm0-f41.google.com with SMTP id k204so59073961wmk.0 for ; Tue, 07 Jun 2016 01:57:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=AqzURH/CnhS+DowMsU4Xe6xNkFJ8dwrD8lUvcWiEOAQ=; b=LMf1ICcn+cbpT2e8TtDoZmensEDJv0qwrSQG1Hzq1xKjED5EJ2P3MXp4DsXzNiDgy4 zXAf5i/QzCr1XcsF6seT6mrrftUoq6mS3g7QY7rgv0zAXo4aSoqsXg6i5FqiDaUG57Gu DcJbo0Er1z0je6xKcbZirk8NH6ovOdlJ58f+k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=AqzURH/CnhS+DowMsU4Xe6xNkFJ8dwrD8lUvcWiEOAQ=; b=e/K395K2UhDz+M2TwX2CjUSQajV7Adidw4xfY/Xs5mbYNa4WBpnN3qGB3cbCLWb5J+ jJ4bggv/N0b44EEsIpD9sIfK+ZLO//Hu7+dNvHivXazzSu3IpEKuWavfm4JfK7C5o7oQ ZQlnxRSEYKMwBX2Dt089DwMrABOpX+Id5O+BFDIgGt26k37E3htErEoKd4crdc/CBpj3 waadFxXA1+sAuuDJzb1jgnadjNFa1N2jraJ28JcM0gXPzXE3NeUuG4HLzEdbLo/kIRxP Hu7436gILjBe9Xnlrrunxgk14hl+5oMrOCCAJQtAerYzpm0Q4tRVuklNELT6JgOD9pcz hZmg== X-Gm-Message-State: ALyK8tKhWoLZZ5vwUkqNhQ5pMqeHkr59vtR8nNbJGzV5bCo/AUcUgk+BZBIq4jzJUm30T911 X-Received: by 10.194.77.140 with SMTP id s12mr19557627wjw.24.1465289861576; Tue, 07 Jun 2016 01:57:41 -0700 (PDT) Received: from dell (host81-129-171-215.range81-129.btcentralplus.com. [81.129.171.215]) by smtp.gmail.com with ESMTPSA id d7sm18355291wmd.11.2016.06.07.01.57.40 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 07 Jun 2016 01:57:40 -0700 (PDT) Date: Tue, 7 Jun 2016 09:58:14 +0100 From: Lee Jones To: Peter Griffin Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org, kernel@stlinux.com, thierry.reding@gmail.com Subject: Re: [STLinux Kernel] [[PATCH v2] 08/11] pwm: sti: Initialise PWM Capture device data Message-ID: <20160607085814.GA18047@dell> References: <1461320295-20414-1-git-send-email-lee.jones@linaro.org> <1461320295-20414-9-git-send-email-lee.jones@linaro.org> <20160607082807.GC22744@griffinp-ThinkPad-X1-Carbon-2nd> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160607082807.GC22744@griffinp-ThinkPad-X1-Carbon-2nd> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 07 Jun 2016, Peter Griffin wrote: > On Fri, 22 Apr 2016, Lee Jones wrote: > > > Each PWM Capture device is allocated a structure to hold its own > > state. During a capture the device may be partaking in one of 3 > > phases. Initial (rising) phase change, a subsequent (falling) > > phase change indicating end of the duty-cycle phase and finally > > a final (rising) phase change indicating the end of the period. > > The timer value snapshot each event is held in a variable of the > > same name, and the phase number (0, 1, 2) is contained in the > > index variable. Other device specific information, such as GPIO > > pin, the IRQ wait queue and locking is also contained in the > > structure. This patch initialises this structure for each of > > the available devices. > > > > Signed-off-by: Lee Jones > > --- > > drivers/pwm/pwm-sti.c | 45 ++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 38 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/pwm/pwm-sti.c b/drivers/pwm/pwm-sti.c > > index 78979d0..9d597bb 100644 > > --- a/drivers/pwm/pwm-sti.c > > +++ b/drivers/pwm/pwm-sti.c [...] > > @@ -307,10 +318,15 @@ static int sti_pwm_probe_dt(struct sti_pwm_chip *pc) > > struct device_node *np = dev->of_node; > > struct sti_pwm_compat_data *cdata = pc->cdata; > > u32 num_devs; > > + int ret; > > > > - of_property_read_u32(np, "st,pwm-num-devs", &num_devs); > > - if (num_devs) > > - cdata->num_devs = num_devs; > > + ret = of_property_read_u32(np, "st,pwm-num-devs", &num_devs); > > + if (!ret) > > + cdata->pwm_num_devs = num_devs; > > dt binding document documents this as st,pwm-num-chan currently. It does, but see PATCH 2. > Why do you need > this binding anyway? Why can't you determine how many channels you have based on > the number of pinctrl groups you are given? That sounds like a horrible idea; a) I'm not even sure if that's possible with the Pinctrl Consumer API and b) even if is it physically possible, it sounds messy. It's much better for code to be clear and simple. Code attempting to use clever inference techniques is usually pretty hard to understand and maintain. We use num- a lot in DT, and for good reason. Besides, not every PWM channel is capable of capture. > Also with this series I don't currently understand how the driver is configured to be > pwm-in or pwm-out. Can you explain how that decision is made? This patch-set does not concern itself with the platform specific side. I have a subsequent patch-set for that. Perhaps it might be nice to move the documentation update into this set though. > For example at the moment I can't see any code in this series for handling the different > pinctrl groups which presumably are required to have this working. To ease your curiosity, here is the patch which does that for the 407: Author: Lee Jones Date: Wed Feb 3 14:24:01 2016 +0000 ARM: dts: STiH407: Declare PWM Capture data lines via Pinctrl Signed-off-by: Lee Jones > If I look at pinctrl_pwm1_chan0_default and friends declared in > stih407-pinctrl.dtsi all are currently named pwm-out, and declared as outputs. For > pwm-in functionality I was expecting to see another set of pinctrl definitions, > declaring these pins as inputs, and something in the driver selecting the > correct pin config depending on how the device has been configured? See above. > Maybe an updated dt example / bindings would make this clearer. Happy to provide the DT documentation in this patch-set. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog diff --git a/arch/arm/boot/dts/stih407-pinctrl.dtsi b/arch/arm/boot/dts/stih407-pinctrl.dtsi index a538ae5..bc22122 100644 --- a/arch/arm/boot/dts/stih407-pinctrl.dtsi +++ b/arch/arm/boot/dts/stih407-pinctrl.dtsi @@ -289,10 +289,12 @@ pinctrl_pwm1_chan0_default: pwm1-0-default { st,pins { pwm-out = <&pio3 0 ALT1 OUT>; + pwm-capturein = <&pio3 2 ALT1 IN>; }; }; pinctrl_pwm1_chan1_default: pwm1-1-default { st,pins { + pwm-capturein = <&pio4 3 ALT1 IN>; pwm-out = <&pio4 4 ALT1 OUT>; }; }; @@ -1030,6 +1032,7 @@ pwm0 { pinctrl_pwm0_chan0_default: pwm0-0-default { st,pins { + pwm-capturein = <&pio31 0 ALT1 IN>; pwm-out = <&pio31 1 ALT1 OUT>; }; };