Re: [PATCH v3] ARM: dts: stm32: change pinctrl bindings definition

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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<SOC>-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 <alexandre.torgue@xxxxxx>
Reviewed-by: Vikas MANOCHA <vikas.manocha@xxxxxx>
Reviewed-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxx>

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/<soc>-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 = <STM32_PINMUX(PIN_NO('A', 9), AF7)>;
+
+  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 = <STM32F429_PA9_FUNC_USART1_TX>;
+			pinmux = <STM32_PINMUX(PIN_NO('A', 9), AF7)>; /* USART1_TX */
  			bias-disable;
  			drive-push-pull;
  			slew-rate = <0>;
  		};
  		pins2 {
-			pinmux = <STM32F429_PA10_FUNC_USART1_RX>;
+			pinmux = <STM32_PINMUX(PIN_NO('A', 10), AF7)>; /* USART1_RX */
  			bias-disable;
  		};
  	};
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux