From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752200AbcLFJ5J (ORCPT ); Tue, 6 Dec 2016 04:57:09 -0500 Received: from mx07-00178001.pphosted.com ([62.209.51.94]:59297 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751569AbcLFJ5G (ORCPT ); Tue, 6 Dec 2016 04:57:06 -0500 Subject: Re: [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT To: Lee Jones References: <1480673842-20804-1-git-send-email-benjamin.gaignard@st.com> <1480673842-20804-8-git-send-email-benjamin.gaignard@st.com> <20161202132251.GL2683@dell> <20161206094835.GB25385@dell.home> CC: Benjamin Gaignard , , , , , , , , , , , , , , , , , , Benjamin Gaignard From: Alexandre Torgue Message-ID: <8f881cf7-ba63-315f-b852-71869ad480f3@st.com> Date: Tue, 6 Dec 2016 10:56:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161206094835.GB25385@dell.home> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.48.0.2] X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-12-06_04:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >>>> --- >>>> 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"; >>>> + }; >>>> +}; >>> >