From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751704AbdH1P0x (ORCPT ); Mon, 28 Aug 2017 11:26:53 -0400 Received: from mx08-00178001.pphosted.com ([91.207.212.93]:60691 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751242AbdH1P0v (ORCPT ); Mon, 28 Aug 2017 11:26:51 -0400 Subject: Re: [PATCH v3] ARM: dts: stm32: change pinctrl bindings definition To: Rob Herring CC: Maxime Coquelin , Linus Walleij , Mark Rutland , Arnd Bergmann , Russell King , , , , , Benjamin Gaignard References: <1501163393-28884-1-git-send-email-alexandre.torgue@st.com> <1501163393-28884-2-git-send-email-alexandre.torgue@st.com> <20170803202134.ne3wkzremxf5mbuw@rob-hp-laptop> From: Alexandre Torgue Message-ID: Date: Mon, 28 Aug 2017 17:26:01 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170803202134.ne3wkzremxf5mbuw@rob-hp-laptop> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.75.127.45] X-ClientProxiedBy: SFHDAG7NODE1.st.com (10.75.127.19) To SFHDAG3NODE2.st.com (10.75.127.8) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-08-28_09:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Rob, On 08/03/2017 10:21 PM, Rob Herring wrote: > On Thu, Jul 27, 2017 at 03:49:53PM +0200, Alexandre Torgue wrote: >> Initially each pin was declared in "include/dt-bindings/stm32-pinfunc.h" >> and each definition contained SOC names (ex: STM32F429_PA9_FUNC_USART1_TX). >> Since this approach was approved, the number of supported MCU has >> increased (STM32F429/STM32F469/STM32f746/STM32H743). To avoid to add a new >> file in "include/dt-bindings" each time a new STM32 SOC arrives I propose >> a new approach which consist to use a macro to define pin muxing in device >> tree. All STM32 will use the common macro to define pinmux. Furthermore, it >> will make STM32 maintenance and integration of new SOC easier . >> >> Signed-off-by: Alexandre TORGUE >> Reviewed-by: Vikas MANOCHA >> Reviewed-by: Benjamin Gaignard >> >> diff --git a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt >> index d907a74..567aa72 100644 >> --- a/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt >> +++ b/Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.txt >> @@ -126,22 +126,27 @@ configuration, pullups, drive, output high/low and output speed. >> }; >> >> Required properties: >> -- pinmux: integer array, represents gpio pin number and mux setting. >> - Supported pin number and mux varies for different SoCs, and are defined in >> - dt-bindings/pinctrl/-pinfunc.h directly. >> - These defines are calculated as: >> - ((port * 16 + line) << 8) | function >> +- pinmux: integer array, represents gpio pin number and mux setting. Use >> + following macro: STM32_PINMUX(PIN_NO(port_name, line), mode) to declare it. > > I would keep the above formula. It can't change because that is the ABI. > The macro is just convenience. > > Is there any reason to have 2 macros? I'd just do STM32_PINMUX(port, > line, mode) and make port names defines (PA, PB, PC, etc.). No. It has been done in this way to simplify macro definition, but it could be done in another way. I can try to create a macro like you propose: STM32_PINMUX(port, line, mode) if you think it is more readable. > >> + >> With: >> - - port: The gpio port index (PA = 0, PB = 1, ..., PK = 11) >> - - line: The line offset within the port (PA0 = 0, PA1 = 1, ..., PA15 = 15) >> - - function: The function number, can be: >> - * 0 : GPIO >> - * 1 : Alternate Function 0 >> - * 2 : Alternate Function 1 >> - * 3 : Alternate Function 2 >> + - port_name: The gpio port name ('A', 'B', ..., 'K') >> + - line: The line offset within the port (0, 1, ..., 15) >> + - mode: The mode can be: >> + * GPIO >> + * AF0 : Alternate Function 0 >> + * AF1 : Alternate Function 1 >> + * AF2 : Alternate Function 2 >> * ... >> - * 16 : Alternate Function 15 >> - * 17 : Analog >> + * AF15 : Alternate Function 15 >> + * ANALOG > > Here too, keeping the numbers is important. > > Perhaps the macro description should either be its own additional > section or document it inline with the macro definition. I agree. I can keep previous definition like it was done, and add a section only for the new macro. Thanks for your review. Alex > > Overall, it does seem like a nice shrinking of the header files. > >> + >> + Example: >> + To declare pin PA7 in mode "alternate function 7" you have to >> + declare: >> + pinmux = ; >> + >> + This macro is defined in dt-bindings/pinctrl/stm32-pinfunc.h >> >> Optional properties: >> - GENERIC_PINCONFIG: is the generic pinconfig options to use. >> @@ -165,13 +170,13 @@ pin-controller { >> ... >> usart1_pins_a: usart1@0 { >> pins1 { >> - pinmux = ; >> + pinmux = ; /* USART1_TX */ >> bias-disable; >> drive-push-pull; >> slew-rate = <0>; >> }; >> pins2 { >> - pinmux = ; >> + pinmux = ; /* USART1_RX */ >> bias-disable; >> }; >> };