linux-kernel - Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT

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]

Date:   Tue, 6 Dec 2016 10:56:13 +0100
From:   Alexandre Torgue <alexandre.torgue@...com>
To:     Lee Jones <lee.jones@...aro.org>
CC:     Benjamin Gaignard <benjamin.gaignard@...aro.org>,
        <robh+dt@...nel.org>, <mark.rutland@....com>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <thierry.reding@...il.com>, <linux-pwm@...r.kernel.org>,
        <jic23@...nel.org>, <knaack.h@....de>, <lars@...afoo.de>,
        <pmeerw@...erw.net>, <linux-iio@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <fabrice.gasnier@...com>,
        <gerald.baeza@...com>, <arnaud.pouliquen@...com>,
        <linus.walleij@...aro.org>, <linaro-kernel@...ts.linaro.org>,
        Benjamin Gaignard <benjamin.gaignard@...com>
Subject: Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer
 driver in DT

Hi Lee,

On 12/06/2016 10:48 AM, Lee Jones wrote:
> On Mon, 05 Dec 2016, Alexandre Torgue wrote:
>> On 12/02/2016 02:22 PM, Lee Jones wrote:
>>> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
>>>
>>>> Add general purpose timers and it sub-nodes into DT for stm32f4.
>>>> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>>>>
>>>> version 3:
>>>> - use "st,stm32-timer-trigger" in DT
>>>>
>>>> version 2:
>>>> - use parameters to describe hardware capabilities
>>>> - do not use references for pwm and iio timer subnodes
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@...com>
>>>> ---
>>>>  arch/arm/boot/dts/stm32f429.dtsi      | 333 +++++++++++++++++++++++++++++++++-
>>>>  arch/arm/boot/dts/stm32f469-disco.dts |  28 +++
>>>>  2 files changed, 360 insertions(+), 1 deletion(-)
>
> [...]
>
> If you're only commenting on a little piece of the patch, it's always
> a good idea to trim the rest.
>
>>>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> index 8a163d7..df4ca7e 100644
>>>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>>>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>>>> @@ -81,3 +81,31 @@
>>>>  &usart3 {
>>>>  	status = "okay";
>>>>  };
>>>> +
>>>> +&gptimer1 {
>>>> +	status = "okay";
>>>> +
>>>> +	pwm1@0 {
>>>> +		pinctrl-0	= <&pwm1_pins>;
>>>> +		pinctrl-names	= "default";
>>>> +		status = "okay";
>>>> +	};
>>>> +
>>>> +	timer1@0 {
>>>> +		status = "okay";
>>>> +	};
>>>> +};
>>>
>>> This is a much *better* format than before.
>>>
>>> I still don't like the '&' syntax though.
>>
>> Please keep "&" format to match with existing nodes.
>
> Right.  I wasn't suggesting that he differs from the current format in
> *this* set.  I am suggesting that we change the format in a subsequent
> set though.

Why change? Looking at Linux ARM kernel patchwork, new DT board file 
contains this format. Did you already discuss with Arnd or Olof about it 
?

regards
Alex

>
>>>> +&gptimer3 {
>>>> +	status = "okay";
>>>> +
>>>> +	pwm3@0 {
>>>> +		pinctrl-0	= <&pwm3_pins>;
>>>> +		pinctrl-names	= "default";
>>>> +		status = "okay";
>>>> +	};
>>>> +
>>>> +	timer3@0 {
>>>> +		status = "okay";
>>>> +	};
>>>> +};
>>>
>