linux-kernel - Re: [PATCH 2/2] dt-bindings: timer: imx: gpt: Add pin group bindings for input capture

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2daa37a6-83a7-ec08-b89c-a07268b3ea4a@gmail.com>
Date:   Mon, 28 Oct 2019 16:59:36 -0700
From:   Steve Longerbeam <slongerbeam@...il.com>
To:     Rob Herring <robh@...nel.org>
Cc:     Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Mark Rutland <mark.rutland@....com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        "open list:CLOCKSOURCE, CLOCKEVENT DRIVERS" 
        <linux-kernel@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 2/2] dt-bindings: timer: imx: gpt: Add pin group bindings
 for input capture

Hi Rob,

Thanks for reviewing.

On 10/27/19 2:21 PM, Rob Herring wrote:
> On Tue, Oct 15, 2019 at 06:05:44PM -0700, Steve Longerbeam wrote:
>> Add pin group bindings to support input capture function of the i.MX
>> GPT.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@...il.com>
>> ---
>>   .../devicetree/bindings/timer/fsl,imxgpt.txt  | 28 +++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt b/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
>> index 5d8fd5b52598..32797b7b0d02 100644
>> --- a/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
>> +++ b/Documentation/devicetree/bindings/timer/fsl,imxgpt.txt
>> @@ -33,6 +33,13 @@ Required properties:
>>              an entry for each entry in clock-names.
>>   - clock-names : must include "ipg" entry first, then "per" entry.
>>   
>> +Optional properties:
>> +
>> +- pinctrl-0: For the i.MX GPT to support the Input Capture function,
>> +  	     the input capture channel pin groups must be listed here.
>> +- pinctrl-names: must be "default".
>> +
>> +
>>   Example:
>>   
>>   gpt1: timer@...03000 {
>> @@ -43,3 +50,24 @@ gpt1: timer@...03000 {
>>   		 <&clks IMX27_CLK_PER1_GATE>;
>>   	clock-names = "ipg", "per";
>>   };
>> +
>> +
>> +Example with input capture channel 0 support:
>> +
>> +pinctrl_gpt_input_capture0: gptinputcapture0grp {
>> +	fsl,pins = <
>> +		MX6QDL_PAD_SD1_DAT0__GPT_CAPTURE1 0x1b0b0
>> +	>;
>> +};
>> +
>> +gpt: gpt@...8000 {
> timer@...

Ok.

>
> I don't really think this merits another example though.

Ok.

But for version 2 of this patch-set I'd like to run some ideas by you.

Because in this version I did not make any attempt to create a generic 
timer capture framework. I just exported a couple imx-specific functions 
to request and free a timer input capture channel in the imx-gpt driver.

So for version 2 I am thinking about a simple framework that other SoC 
timers with timer input capture support can make use of.

To begin with I don't see that timer input capture warrants the 
definition of a new device. At least for imx, timer input capture is 
just one function of the imx GPT, where the other is Output Compare 
which is used for the system timer. I think that is likely the case for 
most all SoC timers, that is, input capture and output compare are 
tightly interwoven functions of general purpose timers.

So I'm thinking there needs to be an additional #input-capture-cells 
property that defines how many input capture channels the timer 
contains, where a channel refers to a single input signal edge that can 
capture the timer counter. The imx GPT has two input capture channels (2 
separate input signals).

For example, on imx:

gpt: timer@...8000 {
	compatible = "fsl,imx6q-gpt", "fsl,imx31-gpt";
	/* ... */
	#input-capture-cells = <1>;
	pinctrl-names = "default", "icap1";
	pinctrl-0 = <&pinctrl_gpt_input_capture0>;
	pinctrl-1 = <&pinctrl_gpt_input_capture1>;
};

A device that is a listener/consumer of an timer capture event would then refer to a timer capture channel:

some-device {
	/* ... */
	timer-input-capture = <&gpt 0>;
};

Is this a sound approach? Let me know what you think.

Thanks,
Steve