All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] Renesas R9A06G032 PINCTRL Driver
@ 2018-09-19 14:23 Phil Edworthy
  2018-09-19 14:23 ` [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-09-19 14:23 UTC (permalink / raw)
  To: Geert Uytterhoeven, Laurent Pinchart, Rob Herring, Mark Rutland
  Cc: Jacopo Mondi, Linus Walleij, Simon Horman, linux-gpio,
	linux-renesas-soc, linux-kernel, Phil Edworthy

This implements the pinctrl driver for the RZ/N1 family of devices, including
the R9A06G032 (RZ/N1D) device.

This series was originally written by Michel Pollet whilst at Renesas, and I
have taken over this work.

Main changes:
v4:
 - Address Jacopo's comments
 - Add alternative way to use the pinmux prop.
 - Remove mention of gpios.
 - Implement pin_config_group_get()
 - Fix function to get pin configs, i.e. return -EINVAL when disabled.

v3:
 - Use standard DT props instead of proprietary ones.
 - Replace virtual pins used for MDIO muxing with extra funcs.
 - Use pinctrl_utils funcs to handle the maps.
 - Remove the dbg functions to keep things simple.
 - Change the way the functions are defined so it is easy to check
   against the hardware numbering.

v2:
 - Change to generic rzn1 family driver, instead of device specific.
 - Review comments fixed.
 - Fix error handling during probe

Phil Edworthy (3):
  dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation
  pinctrl: renesas: Renesas RZ/N1 pinctrl driver
  ARM: dts: r9a06g032: Add pinctrl node

 .../bindings/pinctrl/renesas,rzn1-pinctrl.txt | 153 +++
 arch/arm/boot/dts/r9a06g032.dtsi              |   8 +
 drivers/pinctrl/Kconfig                       |  10 +
 drivers/pinctrl/Makefile                      |   1 +
 drivers/pinctrl/pinctrl-rzn1.c                | 951 ++++++++++++++++++
 include/dt-bindings/pinctrl/rzn1-pinctrl.h    | 141 +++
 6 files changed, 1264 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
 create mode 100644 drivers/pinctrl/pinctrl-rzn1.c
 create mode 100644 include/dt-bindings/pinctrl/rzn1-pinctrl.h

-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation
  2018-09-19 14:23 [PATCH v4 0/3] Renesas R9A06G032 PINCTRL Driver Phil Edworthy
@ 2018-09-19 14:23 ` Phil Edworthy
  2018-09-21 12:36   ` jacopo mondi
  2018-09-26 14:20   ` Rob Herring
  2018-09-19 14:23 ` [PATCH v4 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver Phil Edworthy
  2018-09-19 14:23 ` [PATCH v4 3/3] ARM: dts: r9a06g032: Add pinctrl node Phil Edworthy
  2 siblings, 2 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-09-19 14:23 UTC (permalink / raw)
  To: Geert Uytterhoeven, Laurent Pinchart, Rob Herring, Mark Rutland
  Cc: Jacopo Mondi, Linus Walleij, Simon Horman, linux-gpio,
	linux-renesas-soc, linux-kernel, Phil Edworthy

The Renesas RZ/N1 device family PINCTRL node description.

Based on a patch originally written by Michel Pollet at Renesas.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v4:
 - Add alternative way to use the pinmux prop.
 - Remove mention of gpios.

v3:
 - Use standard bindings
 - Change the way the functions are defined so it is easy to check
   against the hardware numbering.
 - Add functions for the MDIO source peripheral instead of using
   virtual pins.

v2:
 - Change filename to generic rzn1, instead of device specific.
 - Add "renesas,rzn1-pinctrl" compatible fallback string.
 - Example register size corrected.
 - Typos fixed.
 - Changes suggested by Jacopo Mondi.
 - rzn1-pinctrl.h squashed into this as requested by Rob Herring.
---
 .../bindings/pinctrl/renesas,rzn1-pinctrl.txt | 153 ++++++++++++++++++
 include/dt-bindings/pinctrl/rzn1-pinctrl.h    | 141 ++++++++++++++++
 2 files changed, 294 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
 create mode 100644 include/dt-bindings/pinctrl/rzn1-pinctrl.h

diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
new file mode 100644
index 000000000000..323ef5c6c1c9
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
@@ -0,0 +1,153 @@
+Renesas RZ/N1 SoC Pinctrl node description.
+
+Pin controller node
+-------------------
+Required properties:
+- compatible: SoC-specific compatible string "renesas,<soc-specific>-pinctrl"
+  followed by "renesas,rzn1-pinctrl" as fallback. The SoC-specific compatible
+  strings must be one of:
+	"renesas,r9a06g032-pinctrl" for RZ/N1D
+	"renesas,r9a06g033-pinctrl" for RZ/N1S
+- reg: Address base and length of the memory area where the pin controller
+  hardware is mapped to.
+- clocks: phandle for the clock, see the description of clock-names below.
+- clock-names: Contains the name of the clock:
+    "bus", the bus clock, sometimes described as pclk, for register accesses.
+
+Example:
+	pinctrl: pin-controller@40067000 {
+	    compatible = "renesas,r9a06g032-pinctrl", "renesas,rzn1-pinctrl";
+	    reg = <0x40067000 0x1000>, <0x51000000 0x480>;
+	    clocks = <&sysctrl R9A06G032_HCLK_PINCONFIG>;
+	    clock-names = "bus";
+	};
+
+Sub-nodes
+---------
+
+The child nodes of the pin controller node describe a pin multiplexing
+function.
+
+- Pin multiplexing sub-nodes:
+  A pin multiplexing sub-node describes how to configure a set of
+  (or a single) pin in some desired alternate function mode.
+  A single sub-node may define several pin configurations.
+  Please refer to pinctrl-bindings.txt to get to know more on generic
+  pin properties usage.
+
+  The allowed generic formats for a pin multiplexing sub-node are the
+  following ones:
+
+  node-1 {
+      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
+      GENERIC_PINCONFIG;
+  };
+
+  node-2 {
+      sub-node-1 {
+          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
+          GENERIC_PINCONFIG;
+      };
+
+      sub-node-2 {
+          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
+          GENERIC_PINCONFIG;
+      };
+
+      ...
+
+      sub-node-n {
+          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
+          GENERIC_PINCONFIG;
+      };
+  };
+
+  node-3 {
+      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
+      GENERIC_PINCONFIG;
+
+      sub-node-1 {
+          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
+          GENERIC_PINCONFIG;
+      };
+
+      ...
+
+      sub-node-n {
+          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
+          GENERIC_PINCONFIG;
+      };
+  };
+
+  Use the latter two formats when pins part of the same logical group need to
+  have different generic pin configuration flags applied. Note that the generic
+  pinconfig in node-3 does not apply to the sub-nodes.
+
+  Client sub-nodes shall refer to pin multiplexing sub-nodes using the phandle
+  of the most external one.
+
+  Eg.
+
+  client-1 {
+      ...
+      pinctrl-0 = <&node-1>;
+      ...
+  };
+
+  client-2 {
+      ...
+      pinctrl-0 = <&node-2>;
+      ...
+  };
+
+  Required properties:
+    - pinmux:
+      integer array representing pin number and pin multiplexing configuration.
+      When a pin has to be configured in alternate function mode, use this
+      property to identify the pin by its global index, and provide its
+      alternate function configuration number along with it.
+      When multiple pins are required to be configured as part of the same
+      alternate function they shall be specified as members of the same
+      argument list of a single "pinmux" property.
+      Integers values in the "pinmux" argument list are assembled as:
+      (PIN | MUX_FUNC << 8)
+      where PIN directly corresponds to the pl_gpio pin number and MUX_FUNC is
+      one of the alternate function identifiers defined in:
+      <include/dt-bindings/pinctrl/rzn1-pinctrl.h>
+      These identifiers collapse the IO Multiplex Configuration Level 1 and
+      Level 2 numbers that are detailed in the hardware reference manual into a
+      single number. The identifiers for Level 2 are simply offset by 10.
+      Additional identifiers are provided to specify the MDIO source peripheral.
+
+  Optional generic properties:
+    - bias-disable		- disable any pin bias
+    - bias-pull-up		- pull up the pin with 50 KOhm
+    - bias-pull-down		- pull down the pin with 50 KOhm
+    - bias-high-impedance	- high impedance mode
+    - drive-strength		- sink or source at most 4, 6, 8 or 12 mA
+
+  Example:
+  A serial communication interface with a TX output pin and an RX input pin.
+
+  &pinctrl {
+	pins_uart0: pins_uart0 {
+		pinmux = <
+			RZN1_MUX(103, UART0_I)	/* UART0_TXD */
+			RZN1_MUX(104, UART0_I)	/* UART0_RXD */
+		>;
+	};
+  };
+
+  Example 2:
+  Here we set the pull up on the RXD pin of the UART.
+
+  &pinctrl {
+	pins_uart0: pins_uart0 {
+		pinmux = <RZN1_MUX(103, UART0_I)>;	/* UART0_TXD */
+
+		pins_uart6_rx {
+			pinmux = <RZN1_MUX(104, UART0_I)>;	/* UART0_RXD */
+			bias-pull-up;
+		};
+	};
+  };
diff --git a/include/dt-bindings/pinctrl/rzn1-pinctrl.h b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
new file mode 100644
index 000000000000..40369ee36e6a
--- /dev/null
+++ b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
@@ -0,0 +1,141 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Defines macros and constants for Renesas RZ/N1 pin controller pin
+ * muxing functions.
+ */
+#ifndef __DT_BINDINGS_RZN1_PINCTRL_H
+#define __DT_BINDINGS_RZN1_PINCTRL_H
+
+#define RZN1_MUX(_gpio, _func) \
+	(((RZN1_FUNC_##_func) << 8) | (_gpio))
+
+/*
+ * Given the different levels of muxing on the SoC, it was decided to
+ * 'linearize' them into one numerical space. So mux level 1, 2 and the MDIO
+ * muxes are all represented by one single value.
+ *
+ * You can derive the hardware value pretty easily too, as
+ * 0...9   are Level 1
+ * 10...71 are Level 2. The Level 2 mux will be set to this
+ *         value - RZN1_FUNC_L2_OFFSET, and the Level 1 mux will be
+ *         set accordingly.
+ * 72...103 are for the 2 MDIO muxes.
+ */
+#define RZN1_FUNC_HIGHZ				0
+#define RZN1_FUNC_0L				1
+#define RZN1_FUNC_CLK_ETH_MII_RGMII_RMII	2
+#define RZN1_FUNC_CLK_ETH_NAND			3
+#define RZN1_FUNC_QSPI				4
+#define RZN1_FUNC_SDIO				5
+#define RZN1_FUNC_LCD				6
+#define RZN1_FUNC_LCD_E				7
+#define RZN1_FUNC_MSEBIM			8
+#define RZN1_FUNC_MSEBIS			9
+#define RZN1_FUNC_L2_OFFSET			10	/* I'm Special */
+
+#define RZN1_FUNC_HIGHZ1			(RZN1_FUNC_L2_OFFSET + 0)
+#define RZN1_FUNC_ETHERCAT			(RZN1_FUNC_L2_OFFSET + 1)
+#define RZN1_FUNC_SERCOS3			(RZN1_FUNC_L2_OFFSET + 2)
+#define RZN1_FUNC_SDIO_E			(RZN1_FUNC_L2_OFFSET + 3)
+#define RZN1_FUNC_ETH_MDIO			(RZN1_FUNC_L2_OFFSET + 4)
+#define RZN1_FUNC_ETH_MDIO_E1			(RZN1_FUNC_L2_OFFSET + 5)
+#define RZN1_FUNC_USB				(RZN1_FUNC_L2_OFFSET + 6)
+#define RZN1_FUNC_MSEBIM_E			(RZN1_FUNC_L2_OFFSET + 7)
+#define RZN1_FUNC_MSEBIS_E			(RZN1_FUNC_L2_OFFSET + 8)
+#define RZN1_FUNC_RSV				(RZN1_FUNC_L2_OFFSET + 9)
+#define RZN1_FUNC_RSV_E				(RZN1_FUNC_L2_OFFSET + 10)
+#define RZN1_FUNC_RSV_E1			(RZN1_FUNC_L2_OFFSET + 11)
+#define RZN1_FUNC_UART0_I			(RZN1_FUNC_L2_OFFSET + 12)
+#define RZN1_FUNC_UART0_I_E			(RZN1_FUNC_L2_OFFSET + 13)
+#define RZN1_FUNC_UART1_I			(RZN1_FUNC_L2_OFFSET + 14)
+#define RZN1_FUNC_UART1_I_E			(RZN1_FUNC_L2_OFFSET + 15)
+#define RZN1_FUNC_UART2_I			(RZN1_FUNC_L2_OFFSET + 16)
+#define RZN1_FUNC_UART2_I_E			(RZN1_FUNC_L2_OFFSET + 17)
+#define RZN1_FUNC_UART0				(RZN1_FUNC_L2_OFFSET + 18)
+#define RZN1_FUNC_UART0_E			(RZN1_FUNC_L2_OFFSET + 19)
+#define RZN1_FUNC_UART1				(RZN1_FUNC_L2_OFFSET + 20)
+#define RZN1_FUNC_UART1_E			(RZN1_FUNC_L2_OFFSET + 21)
+#define RZN1_FUNC_UART2				(RZN1_FUNC_L2_OFFSET + 22)
+#define RZN1_FUNC_UART2_E			(RZN1_FUNC_L2_OFFSET + 23)
+#define RZN1_FUNC_UART3				(RZN1_FUNC_L2_OFFSET + 24)
+#define RZN1_FUNC_UART3_E			(RZN1_FUNC_L2_OFFSET + 25)
+#define RZN1_FUNC_UART4				(RZN1_FUNC_L2_OFFSET + 26)
+#define RZN1_FUNC_UART4_E			(RZN1_FUNC_L2_OFFSET + 27)
+#define RZN1_FUNC_UART5				(RZN1_FUNC_L2_OFFSET + 28)
+#define RZN1_FUNC_UART5_E			(RZN1_FUNC_L2_OFFSET + 29)
+#define RZN1_FUNC_UART6				(RZN1_FUNC_L2_OFFSET + 30)
+#define RZN1_FUNC_UART6_E			(RZN1_FUNC_L2_OFFSET + 31)
+#define RZN1_FUNC_UART7				(RZN1_FUNC_L2_OFFSET + 32)
+#define RZN1_FUNC_UART7_E			(RZN1_FUNC_L2_OFFSET + 33)
+#define RZN1_FUNC_SPI0_M			(RZN1_FUNC_L2_OFFSET + 34)
+#define RZN1_FUNC_SPI0_M_E			(RZN1_FUNC_L2_OFFSET + 35)
+#define RZN1_FUNC_SPI1_M			(RZN1_FUNC_L2_OFFSET + 36)
+#define RZN1_FUNC_SPI1_M_E			(RZN1_FUNC_L2_OFFSET + 37)
+#define RZN1_FUNC_SPI2_M			(RZN1_FUNC_L2_OFFSET + 38)
+#define RZN1_FUNC_SPI2_M_E			(RZN1_FUNC_L2_OFFSET + 39)
+#define RZN1_FUNC_SPI3_M			(RZN1_FUNC_L2_OFFSET + 40)
+#define RZN1_FUNC_SPI3_M_E			(RZN1_FUNC_L2_OFFSET + 41)
+#define RZN1_FUNC_SPI4_S			(RZN1_FUNC_L2_OFFSET + 42)
+#define RZN1_FUNC_SPI4_S_E			(RZN1_FUNC_L2_OFFSET + 43)
+#define RZN1_FUNC_SPI5_S			(RZN1_FUNC_L2_OFFSET + 44)
+#define RZN1_FUNC_SPI5_S_E			(RZN1_FUNC_L2_OFFSET + 45)
+#define RZN1_FUNC_SGPIO0_M			(RZN1_FUNC_L2_OFFSET + 46)
+#define RZN1_FUNC_SGPIO1_M			(RZN1_FUNC_L2_OFFSET + 47)
+#define RZN1_FUNC_GPIO				(RZN1_FUNC_L2_OFFSET + 48)
+#define RZN1_FUNC_CAN				(RZN1_FUNC_L2_OFFSET + 49)
+#define RZN1_FUNC_I2C				(RZN1_FUNC_L2_OFFSET + 50)
+#define RZN1_FUNC_SAFE				(RZN1_FUNC_L2_OFFSET + 51)
+#define RZN1_FUNC_PTO_PWM			(RZN1_FUNC_L2_OFFSET + 52)
+#define RZN1_FUNC_PTO_PWM1			(RZN1_FUNC_L2_OFFSET + 53)
+#define RZN1_FUNC_PTO_PWM2			(RZN1_FUNC_L2_OFFSET + 54)
+#define RZN1_FUNC_PTO_PWM3			(RZN1_FUNC_L2_OFFSET + 55)
+#define RZN1_FUNC_PTO_PWM4			(RZN1_FUNC_L2_OFFSET + 56)
+#define RZN1_FUNC_DELTA_SIGMA			(RZN1_FUNC_L2_OFFSET + 57)
+#define RZN1_FUNC_SGPIO2_M			(RZN1_FUNC_L2_OFFSET + 58)
+#define RZN1_FUNC_SGPIO3_M			(RZN1_FUNC_L2_OFFSET + 59)
+#define RZN1_FUNC_SGPIO4_S			(RZN1_FUNC_L2_OFFSET + 60)
+#define RZN1_FUNC_MAC_MTIP_SWITCH		(RZN1_FUNC_L2_OFFSET + 61)
+
+#define RZN1_FUNC_MDIO_OFFSET			(RZN1_FUNC_L2_OFFSET + 62)
+
+/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO function */
+#define RZN1_FUNC_MDIO0_HIGHZ			(RZN1_FUNC_MDIO_OFFSET + 0)
+#define RZN1_FUNC_MDIO0_GMAC0			(RZN1_FUNC_MDIO_OFFSET + 1)
+#define RZN1_FUNC_MDIO0_GMAC1			(RZN1_FUNC_MDIO_OFFSET + 2)
+#define RZN1_FUNC_MDIO0_ECAT			(RZN1_FUNC_MDIO_OFFSET + 3)
+#define RZN1_FUNC_MDIO0_S3_MDIO0		(RZN1_FUNC_MDIO_OFFSET + 4)
+#define RZN1_FUNC_MDIO0_S3_MDIO1		(RZN1_FUNC_MDIO_OFFSET + 5)
+#define RZN1_FUNC_MDIO0_HWRTOS			(RZN1_FUNC_MDIO_OFFSET + 6)
+#define RZN1_FUNC_MDIO0_SWITCH			(RZN1_FUNC_MDIO_OFFSET + 7)
+/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
+#define RZN1_FUNC_MDIO0_E1_HIGHZ		(RZN1_FUNC_MDIO_OFFSET + 8)
+#define RZN1_FUNC_MDIO0_E1_GMAC0		(RZN1_FUNC_MDIO_OFFSET + 9)
+#define RZN1_FUNC_MDIO0_E1_GMAC1		(RZN1_FUNC_MDIO_OFFSET + 10)
+#define RZN1_FUNC_MDIO0_E1_ECAT			(RZN1_FUNC_MDIO_OFFSET + 11)
+#define RZN1_FUNC_MDIO0_E1_S3_MDIO0		(RZN1_FUNC_MDIO_OFFSET + 12)
+#define RZN1_FUNC_MDIO0_E1_S3_MDIO1		(RZN1_FUNC_MDIO_OFFSET + 13)
+#define RZN1_FUNC_MDIO0_E1_HWRTOS		(RZN1_FUNC_MDIO_OFFSET + 14)
+#define RZN1_FUNC_MDIO0_E1_SWITCH		(RZN1_FUNC_MDIO_OFFSET + 15)
+
+/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO function */
+#define RZN1_FUNC_MDIO1_HIGHZ			(RZN1_FUNC_MDIO_OFFSET + 16)
+#define RZN1_FUNC_MDIO1_GMAC0			(RZN1_FUNC_MDIO_OFFSET + 17)
+#define RZN1_FUNC_MDIO1_GMAC1			(RZN1_FUNC_MDIO_OFFSET + 18)
+#define RZN1_FUNC_MDIO1_ECAT			(RZN1_FUNC_MDIO_OFFSET + 19)
+#define RZN1_FUNC_MDIO1_S3_MDIO0		(RZN1_FUNC_MDIO_OFFSET + 20)
+#define RZN1_FUNC_MDIO1_S3_MDIO1		(RZN1_FUNC_MDIO_OFFSET + 21)
+#define RZN1_FUNC_MDIO1_HWRTOS			(RZN1_FUNC_MDIO_OFFSET + 22)
+#define RZN1_FUNC_MDIO1_SWITCH			(RZN1_FUNC_MDIO_OFFSET + 23)
+/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
+#define RZN1_FUNC_MDIO1_E1_HIGHZ		(RZN1_FUNC_MDIO_OFFSET + 24)
+#define RZN1_FUNC_MDIO1_E1_GMAC0		(RZN1_FUNC_MDIO_OFFSET + 25)
+#define RZN1_FUNC_MDIO1_E1_GMAC1		(RZN1_FUNC_MDIO_OFFSET + 26)
+#define RZN1_FUNC_MDIO1_E1_ECAT			(RZN1_FUNC_MDIO_OFFSET + 27)
+#define RZN1_FUNC_MDIO1_E1_S3_MDIO0		(RZN1_FUNC_MDIO_OFFSET + 28)
+#define RZN1_FUNC_MDIO1_E1_S3_MDIO1		(RZN1_FUNC_MDIO_OFFSET + 29)
+#define RZN1_FUNC_MDIO1_E1_HWRTOS		(RZN1_FUNC_MDIO_OFFSET + 30)
+#define RZN1_FUNC_MDIO1_E1_SWITCH		(RZN1_FUNC_MDIO_OFFSET + 31)
+
+#define RZN1_FUNC_MAX				(RZN1_FUNC_MDIO_OFFSET + 32)
+
+#endif /* __DT_BINDINGS_RZN1_PINCTRL_H */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver
  2018-09-19 14:23 [PATCH v4 0/3] Renesas R9A06G032 PINCTRL Driver Phil Edworthy
  2018-09-19 14:23 ` [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy
@ 2018-09-19 14:23 ` Phil Edworthy
  2018-09-21 13:33   ` jacopo mondi
  2018-09-24 11:58   ` Geert Uytterhoeven
  2018-09-19 14:23 ` [PATCH v4 3/3] ARM: dts: r9a06g032: Add pinctrl node Phil Edworthy
  2 siblings, 2 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-09-19 14:23 UTC (permalink / raw)
  To: Geert Uytterhoeven, Laurent Pinchart
  Cc: Jacopo Mondi, Linus Walleij, Simon Horman, linux-gpio,
	linux-renesas-soc, linux-kernel, Phil Edworthy

This provides a pinctrl driver for the Renesas RZ/N1 device family.

Based on a patch originally written by Michel Pollet at Renesas.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v4:
 - Address Jacopo's comments
 - Implement pin_config_group_get()
 - Fix function to get pin configs, i.e. return -EINVAL when disabled.

v3:
 - Use standard DT props instead of proprietary ones.
 - Replace virtual pins used for MDIO muxing with extra funcs.
 - Use pinctrl_utils funcs to handle the maps.
 - Remove the dbg functions to keep things simple.

v2:
 - Change filename to generic rzn1, instead of device specific.
 - Changed Kconfig symbol and file name to generic rzn1 family.
 - Added "renesas,rzn1-pinctrl" compatible fallback string
 - Changes suggested by Jacopo Mondi. Mainly formatting, plus:
   - Removed global ptr
   - Removed unused code accessing parent of node.
   - Removed check for null OF np that can't happen.
   - Replaced overlapping enums with #defines
 - Renamed some variables and symbols to clarify their use
 - Fix error handling during probe
 - Move probe from postcore_initcall to subsys_initcall to ensure
   drivers that require clocks get them instead of having to defer
   probing.
---
 drivers/pinctrl/Kconfig        |  10 +
 drivers/pinctrl/Makefile       |   1 +
 drivers/pinctrl/pinctrl-rzn1.c | 951 +++++++++++++++++++++++++++++++++
 3 files changed, 962 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-rzn1.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 978b2ed4d014..4d8c00eac742 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -195,6 +195,16 @@ config PINCTRL_RZA1
 	help
 	  This selects pinctrl driver for Renesas RZ/A1 platforms.
 
+config PINCTRL_RZN1
+	bool "Renesas RZ/N1 pinctrl driver"
+	depends on OF
+	depends on ARCH_RZN1 || COMPILE_TEST
+	select GENERIC_PINCTRL_GROUPS
+	select GENERIC_PINMUX_FUNCTIONS
+	select GENERIC_PINCONF
+	help
+	  This selects pinctrl driver for Renesas RZ/N1 devices.
+
 config PINCTRL_SINGLE
 	tristate "One-register-per-pin type device tree based pinctrl driver"
 	depends on OF
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 8e127bd8610f..18a13c1e2c21 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_PINCTRL_PIC32)	+= pinctrl-pic32.o
 obj-$(CONFIG_PINCTRL_PISTACHIO)	+= pinctrl-pistachio.o
 obj-$(CONFIG_PINCTRL_ROCKCHIP)	+= pinctrl-rockchip.o
 obj-$(CONFIG_PINCTRL_RZA1)	+= pinctrl-rza1.o
+obj-$(CONFIG_PINCTRL_RZN1)	+= pinctrl-rzn1.o
 obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
 obj-$(CONFIG_PINCTRL_SIRF)	+= sirf/
 obj-$(CONFIG_PINCTRL_SX150X)	+= pinctrl-sx150x.o
diff --git a/drivers/pinctrl/pinctrl-rzn1.c b/drivers/pinctrl/pinctrl-rzn1.c
new file mode 100644
index 000000000000..114fd2143c1d
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-rzn1.c
@@ -0,0 +1,951 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2014-2018 Renesas Electronics Europe Limited
+ *
+ * Phil Edworthy <phil.edworthy@renesas.com>
+ * Based on a driver originally written by Michel Pollet at Renesas.
+ */
+
+#include <dt-bindings/pinctrl/rzn1-pinctrl.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include "core.h"
+#include "pinconf.h"
+#include "pinctrl-utils.h"
+
+/* Field positions and masks in the pinmux registers */
+#define RZN1_L1_PIN_DRIVE_STRENGTH	10
+#define RZN1_L1_PIN_DRIVE_STRENGTH_4MA	0
+#define RZN1_L1_PIN_DRIVE_STRENGTH_6MA	1
+#define RZN1_L1_PIN_DRIVE_STRENGTH_8MA	2
+#define RZN1_L1_PIN_DRIVE_STRENGTH_12MA	3
+#define RZN1_L1_PIN_PULL		8
+#define RZN1_L1_PIN_PULL_NONE		0
+#define RZN1_L1_PIN_PULL_UP		1
+#define RZN1_L1_PIN_PULL_DOWN		3
+#define RZN1_L1_FUNCTION		0
+#define RZN1_L1_FUNC_MASK		0xf
+#define RZN1_L1_FUNCTION_L2		0xf
+
+/*
+ * The hardware manual describes two levels of multiplexing, but it's more
+ * logical to think of the hardware as three levels, with level 3 consisting of
+ * the multiplexing for Ethernet MDIO signals.
+ *
+ * Level 1 functions go from 0 to 9, with level 1 function '15' (0xf) specifying
+ * that level 2 functions are used instead. Level 2 has a lot more options,
+ * going from 0 to 61. Level 3 allows selection of MDIO functions which can be
+ * floating, or one of seven internal peripherals. Unfortunately, there are two
+ * level 2 functions that can select MDIO, and two MDIO channels so we have four
+ * sets of level 3 functions.
+ *
+ * For this driver, we've compounded the numbers together, so:
+ *    0 to   9 is level 1
+ *   10 to  71 is 10 + level 2 number
+ *   72 to  79 is 72 + MDIO0 source for level 2 MDIO function.
+ *   80 to  87 is 80 + MDIO0 source for level 2 MDIO_E1 function.
+ *   88 to  95 is 88 + MDIO1 source for level 2 MDIO function.
+ *   96 to 103 is 96 + MDIO1 source for level 2 MDIO_E1 function.
+ * Examples:
+ *  Function 28 corresponds UART0
+ *  Function 73 corresponds to MDIO0 to GMAC0
+ *
+ * There are 170 configurable pins (called PL_GPIO in the datasheet).
+ */
+
+/*
+ * Structure detailing the HW registers on the RZ/N1 devices.
+ * Both the Level 1 mux registers and Level 2 mux registers have the same
+ * structure. The only difference is that Level 2 has additional MDIO registers
+ * at the end.
+ */
+struct rzn1_pinctrl_regs {
+	union {
+		u32	conf[170];
+		u8	pad0[0x400];
+	};
+	u32	status_protect;	/* 0x400 */
+	/* MDIO mux registers, level2 only */
+	u32	l2_mdio[2];
+};
+
+/**
+ * struct rzn1_pmx_func - describes rzn1 pinmux functions
+ * @name: the name of this specific function
+ * @groups: corresponding pin groups
+ * @num_groups: the number of groups
+ */
+struct rzn1_pmx_func {
+	const char *name;
+	const char **groups;
+	unsigned int num_groups;
+};
+
+/**
+ * struct rzn1_pin_group - describes an rzn1 pin group
+ * @name: the name of this specific pin group
+ * @func: the name of the function selected by this group
+ * @npins: the number of pins in this group array, i.e. the number of
+ *	elements in .pins so we can iterate over that array
+ * @pin_ids: array of pin_ids, i.e. the value used to select the mux
+ * @pins: array of pins. Needed due to pinctrl_ops.get_group_pins()
+ */
+struct rzn1_pin_group {
+	const char *name;
+	const char *func;
+	unsigned int npins;
+	u8 *pin_ids;
+	u32 *pins;
+};
+
+struct rzn1_pinctrl {
+	struct device *dev;
+	struct clk *clk;
+	struct pinctrl_dev *pctl;
+	struct rzn1_pinctrl_regs __iomem *lev1;
+	struct rzn1_pinctrl_regs __iomem *lev2;
+	u32 lev1_protect_phys;
+	u32 lev2_protect_phys;
+	int mdio_func[2];
+
+	struct rzn1_pin_group *groups;
+	unsigned int ngroups;
+
+	struct rzn1_pmx_func *functions;
+	unsigned int nfunctions;
+};
+
+#define RZN1_PINS_PROP "pinmux"
+
+#define RZN1_PIN(pin) PINCTRL_PIN(pin, "pl_gpio"#pin)
+
+static const struct pinctrl_pin_desc rzn1_pins[] = {
+	RZN1_PIN(0), RZN1_PIN(1), RZN1_PIN(2), RZN1_PIN(3), RZN1_PIN(4),
+	RZN1_PIN(5), RZN1_PIN(6), RZN1_PIN(7), RZN1_PIN(8), RZN1_PIN(9),
+	RZN1_PIN(10), RZN1_PIN(11), RZN1_PIN(12), RZN1_PIN(13), RZN1_PIN(14),
+	RZN1_PIN(15), RZN1_PIN(16), RZN1_PIN(17), RZN1_PIN(18), RZN1_PIN(19),
+	RZN1_PIN(20), RZN1_PIN(21), RZN1_PIN(22), RZN1_PIN(23), RZN1_PIN(24),
+	RZN1_PIN(25), RZN1_PIN(26), RZN1_PIN(27), RZN1_PIN(28), RZN1_PIN(29),
+	RZN1_PIN(30), RZN1_PIN(31), RZN1_PIN(32), RZN1_PIN(33), RZN1_PIN(34),
+	RZN1_PIN(35), RZN1_PIN(36), RZN1_PIN(37), RZN1_PIN(38), RZN1_PIN(39),
+	RZN1_PIN(40), RZN1_PIN(41), RZN1_PIN(42), RZN1_PIN(43), RZN1_PIN(44),
+	RZN1_PIN(45), RZN1_PIN(46), RZN1_PIN(47), RZN1_PIN(48), RZN1_PIN(49),
+	RZN1_PIN(50), RZN1_PIN(51), RZN1_PIN(52), RZN1_PIN(53), RZN1_PIN(54),
+	RZN1_PIN(55), RZN1_PIN(56), RZN1_PIN(57), RZN1_PIN(58), RZN1_PIN(59),
+	RZN1_PIN(60), RZN1_PIN(61), RZN1_PIN(62), RZN1_PIN(63), RZN1_PIN(64),
+	RZN1_PIN(65), RZN1_PIN(66), RZN1_PIN(67), RZN1_PIN(68), RZN1_PIN(69),
+	RZN1_PIN(70), RZN1_PIN(71), RZN1_PIN(72), RZN1_PIN(73), RZN1_PIN(74),
+	RZN1_PIN(75), RZN1_PIN(76), RZN1_PIN(77), RZN1_PIN(78), RZN1_PIN(79),
+	RZN1_PIN(80), RZN1_PIN(81), RZN1_PIN(82), RZN1_PIN(83), RZN1_PIN(84),
+	RZN1_PIN(85), RZN1_PIN(86), RZN1_PIN(87), RZN1_PIN(88), RZN1_PIN(89),
+	RZN1_PIN(90), RZN1_PIN(91), RZN1_PIN(92), RZN1_PIN(93), RZN1_PIN(94),
+	RZN1_PIN(95), RZN1_PIN(96), RZN1_PIN(97), RZN1_PIN(98), RZN1_PIN(99),
+	RZN1_PIN(100), RZN1_PIN(101), RZN1_PIN(102), RZN1_PIN(103),
+	RZN1_PIN(104), RZN1_PIN(105), RZN1_PIN(106), RZN1_PIN(107),
+	RZN1_PIN(108), RZN1_PIN(109), RZN1_PIN(110), RZN1_PIN(111),
+	RZN1_PIN(112), RZN1_PIN(113), RZN1_PIN(114), RZN1_PIN(115),
+	RZN1_PIN(116), RZN1_PIN(117), RZN1_PIN(118), RZN1_PIN(119),
+	RZN1_PIN(120), RZN1_PIN(121), RZN1_PIN(122), RZN1_PIN(123),
+	RZN1_PIN(124), RZN1_PIN(125), RZN1_PIN(126), RZN1_PIN(127),
+	RZN1_PIN(128), RZN1_PIN(129), RZN1_PIN(130), RZN1_PIN(131),
+	RZN1_PIN(132), RZN1_PIN(133), RZN1_PIN(134), RZN1_PIN(135),
+	RZN1_PIN(136), RZN1_PIN(137), RZN1_PIN(138), RZN1_PIN(139),
+	RZN1_PIN(140), RZN1_PIN(141), RZN1_PIN(142), RZN1_PIN(143),
+	RZN1_PIN(144), RZN1_PIN(145), RZN1_PIN(146), RZN1_PIN(147),
+	RZN1_PIN(148), RZN1_PIN(149), RZN1_PIN(150), RZN1_PIN(151),
+	RZN1_PIN(152), RZN1_PIN(153), RZN1_PIN(154), RZN1_PIN(155),
+	RZN1_PIN(156), RZN1_PIN(157), RZN1_PIN(158), RZN1_PIN(159),
+	RZN1_PIN(160), RZN1_PIN(161), RZN1_PIN(162), RZN1_PIN(163),
+	RZN1_PIN(164), RZN1_PIN(165), RZN1_PIN(166), RZN1_PIN(167),
+	RZN1_PIN(168), RZN1_PIN(169),
+};
+
+enum {
+	LOCK_LEVEL1 = 0x1,
+	LOCK_LEVEL2 = 0x2,
+	LOCK_ALL = LOCK_LEVEL1 | LOCK_LEVEL2,
+};
+
+static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value)
+{
+	/*
+	 * The pinmux configuration is locked by writing the physical address of
+	 * the status_protect register to itself. It is unlocked by writing the
+	 * address | 1.
+	 */
+	if (lock & LOCK_LEVEL1) {
+		u32 val = ipctl->lev1_protect_phys | !(value & LOCK_LEVEL1);
+
+		writel(val, &ipctl->lev1->status_protect);
+	}
+
+	if (lock & LOCK_LEVEL2) {
+		u32 val = ipctl->lev2_protect_phys | !(value & LOCK_LEVEL2);
+
+		writel(val, &ipctl->lev2->status_protect);
+	}
+}
+
+static void rzn1_pinctrl_mdio_select(struct rzn1_pinctrl *ipctl, u8 mdio,
+				     u32 func)
+{
+	if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != func)
+		dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n", mdio);
+	ipctl->mdio_func[mdio] = func;
+
+	dev_dbg(ipctl->dev, "setting mdio%d to %d\n", mdio, func);
+
+	writel(func, &ipctl->lev2->l2_mdio[mdio]);
+}
+
+/*
+ * Using a composite pin description, set the hardware pinmux registers
+ * with the corresponding values.
+ * Make sure to unlock write protection and reset it afterward.
+ *
+ * NOTE: There is no protection for potential concurrency, it is assumed these
+ * calls are serialized already.
+ */
+static int rzn1_set_hw_pin_func(struct rzn1_pinctrl *ipctl, u32 pin,
+				u32 pin_config, u8 use_locks)
+{
+	u32 l1_cache;
+	u32 l2_cache;
+	u32 l1;
+	u32 l2;
+
+	/* Level 3 MDIO multiplexing */
+	if (pin_config >= RZN1_FUNC_MDIO0_HIGHZ &&
+	    pin_config <= RZN1_FUNC_MDIO1_E1_SWITCH) {
+		u8 mdio_channel;
+		u32 mdio_func;
+
+		if (pin_config <= RZN1_FUNC_MDIO1_HIGHZ)
+			mdio_channel = 0;
+		else
+			mdio_channel = 1;
+
+		/* Get MDIO func, and convert the func to the level 2 number */
+		if (pin_config <= RZN1_FUNC_MDIO0_SWITCH) {
+			mdio_func = pin_config - RZN1_FUNC_MDIO0_HIGHZ;
+			pin_config = RZN1_FUNC_ETH_MDIO;
+		} else if (pin_config <= RZN1_FUNC_MDIO0_E1_SWITCH) {
+			mdio_func = pin_config - RZN1_FUNC_MDIO0_E1_HIGHZ;
+			pin_config = RZN1_FUNC_ETH_MDIO_E1;
+		} else if (pin_config <= RZN1_FUNC_MDIO1_SWITCH) {
+			mdio_func = pin_config - RZN1_FUNC_MDIO1_HIGHZ;
+			pin_config = RZN1_FUNC_ETH_MDIO;
+		} else {
+			mdio_func = pin_config - RZN1_FUNC_MDIO1_E1_HIGHZ;
+			pin_config = RZN1_FUNC_ETH_MDIO_E1;
+		}
+		rzn1_pinctrl_mdio_select(ipctl, mdio_channel, mdio_func);
+	}
+
+	/* Note here, we do not allow anything past the MDIO Mux values */
+	if (pin >= ARRAY_SIZE(ipctl->lev1->conf) ||
+	    pin_config >= RZN1_FUNC_MDIO0_HIGHZ)
+		return -EINVAL;
+
+	l1 = readl(&ipctl->lev1->conf[pin]);
+	l1_cache = l1;
+	l2 = readl(&ipctl->lev2->conf[pin]);
+	l2_cache = l2;
+
+	dev_dbg(ipctl->dev, "setting func for pin %d to %d\n", pin, pin_config);
+
+	if (pin_config < RZN1_FUNC_L2_OFFSET) {
+		l1 &= ~(RZN1_L1_FUNC_MASK << RZN1_L1_FUNCTION);
+		l1 |= (pin_config << RZN1_L1_FUNCTION);
+	} else {
+		l1 &= ~(RZN1_L1_FUNC_MASK << RZN1_L1_FUNCTION);
+		l1 |= (RZN1_L1_FUNCTION_L2 << RZN1_L1_FUNCTION);
+
+		l2 = pin_config - RZN1_FUNC_L2_OFFSET;
+	}
+
+	/* If either configuration changes, we update both anyway */
+	if (l1 != l1_cache || l2 != l2_cache) {
+		writel(l1, &ipctl->lev1->conf[pin]);
+		writel(l2, &ipctl->lev2->conf[pin]);
+	}
+
+	return 0;
+}
+
+static const struct rzn1_pin_group *rzn1_pinctrl_find_group_by_name(
+	const struct rzn1_pinctrl *ipctl, const char *name)
+{
+	const struct rzn1_pin_group *grp = NULL;
+	int i;
+
+	for (i = 0; i < ipctl->ngroups; i++) {
+		if (!strcmp(ipctl->groups[i].name, name)) {
+			grp = &ipctl->groups[i];
+			break;
+		}
+	}
+
+	return grp;
+}
+
+static int rzn1_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+
+	return ipctl->ngroups;
+}
+
+static const char *rzn1_get_group_name(struct pinctrl_dev *pctldev,
+				       unsigned int selector)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+
+	return ipctl->groups[selector].name;
+}
+
+static int rzn1_get_group_pins(struct pinctrl_dev *pctldev,
+			       unsigned int selector, const unsigned int **pins,
+			       unsigned int *npins)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+
+	if (selector >= ipctl->ngroups)
+		return -EINVAL;
+
+	*pins = ipctl->groups[selector].pins;
+	*npins = ipctl->groups[selector].npins;
+
+	return 0;
+}
+
+/*
+ * This function is called for each pinctl 'Function' node.
+ * Sub-nodes can be used to describe multiple 'Groups' for the 'Function'
+ * If there aren't any sub-nodes, the 'Group' is essentially the 'Function'.
+ * Each 'Group' uses pinmux = <...> to detail the pins and data used to select
+ * the functionality. Each 'Group' has optional pin configurations that apply
+ * to all pins in the 'Group'.
+ */
+static int rzn1_dt_node_to_map_one(struct pinctrl_dev *pctldev,
+				   struct device_node *np,
+				   struct pinctrl_map **map,
+				   unsigned int *num_maps)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct rzn1_pin_group *grp;
+	unsigned long *configs = NULL;
+	unsigned int num_configs = 0;
+	unsigned int reserved_maps = *num_maps;
+	unsigned int reserve = 1;
+	int ret;
+
+	dev_dbg(ipctl->dev, "processing node %pOF\n", np);
+
+	grp = rzn1_pinctrl_find_group_by_name(ipctl, np->name);
+	if (!grp) {
+		dev_err(ipctl->dev, "unable to find group for node %pOF\n", np);
+
+		return -EINVAL;
+	}
+
+	/* Get the group's pin configuration */
+	ret = pinconf_generic_parse_dt_config(np, pctldev, &configs,
+					      &num_configs);
+	if (ret < 0) {
+		dev_err(ipctl->dev, "%pOF: could not parse property\n", np);
+
+		return ret;
+	}
+
+	if (num_configs)
+		reserve++;
+
+	/* Increase the number of maps to cover this group */
+	ret = pinctrl_utils_reserve_map(pctldev, map, &reserved_maps, num_maps,
+					reserve);
+	if (ret < 0)
+		goto out;
+
+	/* Associate the group with the function */
+	ret = pinctrl_utils_add_map_mux(pctldev, map, &reserved_maps, num_maps,
+					grp->name, grp->func);
+	if (ret < 0)
+		goto out;
+
+	if (num_configs) {
+		/* Associate the group's pin configuration with the group */
+		ret = pinctrl_utils_add_map_configs(pctldev, map,
+				&reserved_maps, num_maps, grp->name,
+				configs, num_configs,
+				PIN_MAP_TYPE_CONFIGS_GROUP);
+		if (ret < 0)
+			goto out;
+	}
+
+	dev_dbg(pctldev->dev, "maps: function %s group %s (%d pins)\n",
+		grp->func, grp->name, grp->npins);
+
+out:
+	kfree(configs);
+
+	return ret;
+}
+
+static int rzn1_dt_node_to_map(struct pinctrl_dev *pctldev,
+			       struct device_node *np,
+			       struct pinctrl_map **map,
+			       unsigned int *num_maps)
+{
+	struct device_node *child;
+	int ret;
+
+	*map = NULL;
+	*num_maps = 0;
+
+	ret = rzn1_dt_node_to_map_one(pctldev, np, map, num_maps);
+	if (ret < 0)
+		return ret;
+
+	for_each_child_of_node(np, child) {
+		ret = rzn1_dt_node_to_map_one(pctldev, child, map, num_maps);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct pinctrl_ops rzn1_pctrl_ops = {
+	.get_groups_count = rzn1_get_groups_count,
+	.get_group_name = rzn1_get_group_name,
+	.get_group_pins = rzn1_get_group_pins,
+	.dt_node_to_map = rzn1_dt_node_to_map,
+	.dt_free_map = pinctrl_utils_free_map,
+};
+
+static int rzn1_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+
+	return ipctl->nfunctions;
+}
+
+static const char *rzn1_pmx_get_func_name(struct pinctrl_dev *pctldev,
+					  unsigned int selector)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+
+	return ipctl->functions[selector].name;
+}
+
+static int rzn1_pmx_get_groups(struct pinctrl_dev *pctldev,
+			       unsigned int selector,
+			       const char * const **groups,
+			       unsigned int * const num_groups)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+
+	*groups = ipctl->functions[selector].groups;
+	*num_groups = ipctl->functions[selector].num_groups;
+
+	return 0;
+}
+
+static int rzn1_set_mux(struct pinctrl_dev *pctldev, unsigned int selector,
+			unsigned int group)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	struct rzn1_pin_group *grp = &ipctl->groups[group];
+	unsigned int i, grp_pins = grp->npins;
+
+	dev_dbg(ipctl->dev, "set mux %s(%d) group %s(%d)\n",
+		ipctl->functions[selector].name, selector, grp->name, group);
+
+	rzn1_hw_set_lock(ipctl, LOCK_ALL, LOCK_ALL);
+	for (i = 0; i < grp_pins; i++)
+		rzn1_set_hw_pin_func(ipctl, grp->pins[i], grp->pin_ids[i], 0);
+	rzn1_hw_set_lock(ipctl, LOCK_ALL, 0);
+
+	return 0;
+}
+
+static const struct pinmux_ops rzn1_pmx_ops = {
+	.get_functions_count = rzn1_pmx_get_funcs_count,
+	.get_function_name = rzn1_pmx_get_func_name,
+	.get_function_groups = rzn1_pmx_get_groups,
+	.set_mux = rzn1_set_mux,
+};
+
+static int rzn1_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
+			    unsigned long *config)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	const u32 reg_drive[4] = { 4, 6, 8, 12 };
+	u32 l1, l2;
+	u32 pull, drive, l1mux;
+	u32 arg = 0;
+
+	if (pin >= ARRAY_SIZE(ipctl->lev1->conf))
+		return -EINVAL;
+
+	l1 = readl(&ipctl->lev1->conf[pin]);
+
+	l1mux = l1 & RZN1_L1_FUNC_MASK;
+	pull = (l1 >> RZN1_L1_PIN_PULL) & 0x3;
+	drive = (l1 >> RZN1_L1_PIN_DRIVE_STRENGTH) & 0x3;
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_PULL_UP:
+		if (pull != RZN1_L1_PIN_PULL_UP)
+			return -EINVAL;
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		if (pull != RZN1_L1_PIN_PULL_DOWN)
+			return -EINVAL;
+		break;
+	case PIN_CONFIG_BIAS_DISABLE:
+		if (pull != RZN1_L1_PIN_PULL_NONE)
+			return -EINVAL;
+		break;
+	case PIN_CONFIG_DRIVE_STRENGTH:
+		arg = reg_drive[drive];
+		break;
+	case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
+		l2 = readl(&ipctl->lev2->conf[pin]);
+		if (l1mux == RZN1_L1_FUNCTION_L2) {
+			if (l2 != 0)
+				return -EINVAL;
+		} else if (l1mux != RZN1_FUNC_HIGHZ) {
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	*config = pinconf_to_config_packed(param, arg);
+
+	return 0;
+}
+
+static int rzn1_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
+			    unsigned long *configs, unsigned int num_configs)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param;
+	int i;
+	u32 arg;
+	u32 l1, l1_cache;
+	u32 drv;
+
+	if (pin >= ARRAY_SIZE(ipctl->lev1->conf))
+		return -EINVAL;
+
+	l1 = readl(&ipctl->lev1->conf[pin]);
+	l1_cache = l1;
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+		arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_PULL_UP:
+			dev_dbg(ipctl->dev, "set pin %d pull up\n", pin);
+			l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
+			l1 |= (RZN1_L1_PIN_PULL_UP << RZN1_L1_PIN_PULL);
+			break;
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+			dev_dbg(ipctl->dev, "set pin %d pull down\n", pin);
+			l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
+			l1 |= (RZN1_L1_PIN_PULL_DOWN << RZN1_L1_PIN_PULL);
+			break;
+		case PIN_CONFIG_BIAS_DISABLE:
+			dev_dbg(ipctl->dev, "set pin %d bias off\n", pin);
+			l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
+			l1 |= (RZN1_L1_PIN_PULL_NONE << RZN1_L1_PIN_PULL);
+			break;
+		case PIN_CONFIG_DRIVE_STRENGTH:
+			dev_dbg(ipctl->dev, "set pin %d drv %umA\n", pin, arg);
+			drv = 0;
+			switch (arg) {
+			case 4:
+				drv = RZN1_L1_PIN_DRIVE_STRENGTH_4MA;
+				break;
+			case 6:
+				drv = RZN1_L1_PIN_DRIVE_STRENGTH_6MA;
+				break;
+			case 8:
+				drv = RZN1_L1_PIN_DRIVE_STRENGTH_8MA;
+				break;
+			case 12:
+				drv = RZN1_L1_PIN_DRIVE_STRENGTH_12MA;
+				break;
+			default:
+				dev_err(ipctl->dev,
+					"Drive strength %umA not supported\n",
+					arg);
+
+				return -EINVAL;
+			}
+
+			l1 &= ~(0x3 << RZN1_L1_PIN_DRIVE_STRENGTH);
+			l1 |= (drv << RZN1_L1_PIN_DRIVE_STRENGTH);
+			break;
+
+		case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
+			dev_dbg(ipctl->dev, "set pin %d High-Z\n", pin);
+			l1 &= ~RZN1_L1_FUNC_MASK;
+			l1 |= RZN1_FUNC_HIGHZ;
+			break;
+		default:
+			return -ENOTSUPP;
+		}
+	}
+
+	if (l1 != l1_cache) {
+		rzn1_hw_set_lock(ipctl, LOCK_LEVEL1, LOCK_LEVEL1);
+		writel(l1, &ipctl->lev1->conf[pin]);
+		rzn1_hw_set_lock(ipctl, LOCK_LEVEL1, 0);
+	}
+
+	return 0;
+}
+
+static int rzn1_pinconf_group_get(struct pinctrl_dev *pctldev,
+				  unsigned int selector,
+				  unsigned long *config)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	struct rzn1_pin_group *grp = &ipctl->groups[selector];
+	unsigned long old = 0;
+	int i;
+
+	dev_dbg(ipctl->dev, "group get %s selector:%d\n", grp->name, selector);
+
+	for (i = 0; i < grp->npins; i++) {
+		if (rzn1_pinconf_get(pctldev, grp->pins[i], config))
+			return -ENOTSUPP;
+
+		/* configs do not match between two pins */
+		if (i && (old != *config))
+			return -ENOTSUPP;
+
+		old = *config;
+	}
+
+	return 0;
+}
+
+static int rzn1_pinconf_group_set(struct pinctrl_dev *pctldev,
+				  unsigned int selector,
+				  unsigned long *configs,
+				  unsigned int num_configs)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	struct rzn1_pin_group *grp = &ipctl->groups[selector];
+	int ret, i;
+
+	dev_dbg(ipctl->dev, "group set %s selector:%d configs:%p/%d\n",
+		grp->name, selector, configs, num_configs);
+
+	for (i = 0; i < grp->npins; i++) {
+		unsigned int pin = grp->pins[i];
+
+		ret = rzn1_pinconf_set(pctldev, pin, configs, num_configs);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct pinconf_ops rzn1_pinconf_ops = {
+	.is_generic = true,
+	.pin_config_get = rzn1_pinconf_get,
+	.pin_config_set = rzn1_pinconf_set,
+	.pin_config_group_get = rzn1_pinconf_group_get,
+	.pin_config_group_set = rzn1_pinconf_group_set,
+	.pin_config_config_dbg_show = pinconf_generic_dump_config,
+};
+
+static struct pinctrl_desc rzn1_pinctrl_desc = {
+	.pctlops = &rzn1_pctrl_ops,
+	.pmxops = &rzn1_pmx_ops,
+	.confops = &rzn1_pinconf_ops,
+	.owner = THIS_MODULE,
+};
+
+static int rzn1_pinctrl_parse_groups(struct device_node *np,
+				     struct rzn1_pin_group *grp,
+				     struct rzn1_pinctrl *ipctl)
+{
+	int size;
+	const __be32 *list;
+	unsigned int i;
+
+	dev_dbg(ipctl->dev, "%s: %s\n", __func__, np->name);
+
+	/* Initialise group */
+	grp->name = np->name;
+
+	/*
+	 * The binding format is
+	 *	pinmux = <PIN_FUNC_ID CONFIG ...>,
+	 * do sanity check and calculate pins number
+	 */
+	list = of_get_property(np, RZN1_PINS_PROP, &size);
+	if (!list) {
+		dev_err(ipctl->dev,
+			"no " RZN1_PINS_PROP " property in node %pOF\n", np);
+
+		return -EINVAL;
+	}
+
+	if (!size) {
+		dev_err(ipctl->dev, "Invalid " RZN1_PINS_PROP " in node %pOF\n",
+			np);
+
+		return -EINVAL;
+	}
+
+	grp->npins = size / sizeof(list[0]);
+	grp->pin_ids = devm_kmalloc_array(ipctl->dev,
+					  grp->npins, sizeof(grp->pin_ids[0]),
+					  GFP_KERNEL);
+	grp->pins = devm_kmalloc_array(ipctl->dev,
+				       grp->npins, sizeof(grp->pins[0]),
+				       GFP_KERNEL);
+	if (!grp->pin_ids || !grp->pins)
+		return -ENOMEM;
+
+	for (i = 0; i < grp->npins; i++) {
+		u32 pin_id = be32_to_cpu(*list++);
+
+		grp->pins[i] = pin_id & 0xff;
+		grp->pin_ids[i] = (pin_id >> 8) & 0x7f;
+	}
+
+	return grp->npins;
+}
+
+static int rzn1_pinctrl_count_function_groups(struct device_node *np)
+{
+	struct device_node *child;
+	int count = 0;
+
+	if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0)
+		count++;
+
+	for_each_child_of_node(np, child) {
+		if (of_property_count_u32_elems(child, RZN1_PINS_PROP) > 0)
+			count++;
+	}
+
+	return count;
+}
+
+static int rzn1_pinctrl_parse_functions(struct device_node *np,
+					struct rzn1_pinctrl *ipctl, u32 index)
+{
+	struct device_node *child;
+	struct rzn1_pmx_func *func;
+	struct rzn1_pin_group *grp;
+	u32 i = 0;
+	int ret;
+
+	func = &ipctl->functions[index];
+
+	/* Initialise function */
+	func->name = np->name;
+	func->num_groups = rzn1_pinctrl_count_function_groups(np);
+	if (func->num_groups == 0) {
+		dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
+		return -EINVAL;
+	}
+	dev_dbg(ipctl->dev, "function %s has %d groups\n",
+		np->name, func->num_groups);
+
+	func->groups = devm_kmalloc_array(ipctl->dev,
+					  func->num_groups, sizeof(char *),
+					  GFP_KERNEL);
+	if (!func->groups)
+		return -ENOMEM;
+
+	if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0) {
+		func->groups[i] = np->name;
+		grp = &ipctl->groups[ipctl->ngroups];
+		grp->func = func->name;
+		ret = rzn1_pinctrl_parse_groups(np, grp, ipctl);
+		if (ret < 0)
+			return ret;
+		i++;
+		ipctl->ngroups++;
+	}
+
+	for_each_child_of_node(np, child) {
+		func->groups[i] = child->name;
+		grp = &ipctl->groups[ipctl->ngroups];
+		grp->func = func->name;
+		ret = rzn1_pinctrl_parse_groups(child, grp, ipctl);
+		if (ret < 0)
+			return ret;
+		i++;
+		ipctl->ngroups++;
+	}
+
+	dev_dbg(ipctl->dev, "function %s parsed %d/%d groups\n",
+		np->name, i, func->num_groups);
+
+	return 0;
+}
+
+static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
+				 struct rzn1_pinctrl *ipctl)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	unsigned int maxgroups = 0;
+	u32 nfuncs = 0;
+	u32 i = 0;
+	int ret;
+
+	nfuncs = of_get_child_count(np);
+	if (nfuncs <= 0) {
+		dev_warn(&pdev->dev, "no functions defined\n");
+
+		return 0;
+	}
+
+	ipctl->nfunctions = nfuncs;
+	ipctl->functions = devm_kmalloc_array(&pdev->dev, nfuncs,
+					      sizeof(*ipctl->functions),
+					      GFP_KERNEL);
+	if (!ipctl->functions)
+		return -ENOMEM;
+
+	ipctl->ngroups = 0;
+	for_each_child_of_node(np, child)
+		maxgroups += rzn1_pinctrl_count_function_groups(child);
+
+	ipctl->groups = devm_kmalloc_array(&pdev->dev,
+					   maxgroups,
+					   sizeof(*ipctl->groups),
+					   GFP_KERNEL);
+	if (!ipctl->groups)
+		return -ENOMEM;
+
+	for_each_child_of_node(np, child) {
+		ret = rzn1_pinctrl_parse_functions(child, ipctl, i++);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int rzn1_pinctrl_probe(struct platform_device *pdev)
+{
+	struct rzn1_pinctrl *ipctl;
+	struct resource *res;
+	int ret;
+
+	/* Create state holders etc for this driver */
+	ipctl = devm_kzalloc(&pdev->dev, sizeof(*ipctl), GFP_KERNEL);
+	if (!ipctl)
+		return -ENOMEM;
+
+	ipctl->mdio_func[0] = -1;
+	ipctl->mdio_func[1] = -1;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ipctl->lev1_protect_phys = (u32)res->start + 0x400;
+	ipctl->lev1 = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ipctl->lev1))
+		return PTR_ERR(ipctl->lev1);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	ipctl->lev2_protect_phys = (u32)res->start + 0x400;
+	ipctl->lev2 = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ipctl->lev2))
+		return PTR_ERR(ipctl->lev2);
+
+	ipctl->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(ipctl->clk))
+		return PTR_ERR(ipctl->clk);
+	ret = clk_prepare_enable(ipctl->clk);
+	if (ret)
+		return ret;
+
+	ipctl->dev = &pdev->dev;
+	rzn1_pinctrl_desc.name = dev_name(&pdev->dev);
+	rzn1_pinctrl_desc.pins = rzn1_pins;
+	rzn1_pinctrl_desc.npins = ARRAY_SIZE(rzn1_pins);
+
+	ret = rzn1_pinctrl_probe_dt(pdev, ipctl);
+	if (ret) {
+		dev_err(&pdev->dev, "fail to probe dt properties\n");
+		goto err_clk;
+	}
+
+	platform_set_drvdata(pdev, ipctl);
+	ipctl->pctl = pinctrl_register(&rzn1_pinctrl_desc, &pdev->dev, ipctl);
+	if (!ipctl->pctl) {
+		dev_err(&pdev->dev, "could not register rzn1 pinctrl driver\n");
+		ret = -EINVAL;
+		goto err_clk;
+	}
+
+	dev_info(&pdev->dev, "probed\n");
+
+	return 0;
+
+err_clk:
+	clk_disable_unprepare(ipctl->clk);
+
+	return ret;
+}
+
+static int rzn1_pinctrl_remove(struct platform_device *pdev)
+{
+	struct rzn1_pinctrl *ipctl = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(ipctl->clk);
+
+	return 0;
+}
+
+static const struct of_device_id rzn1_pinctrl_match[] = {
+	{ .compatible = "renesas,rzn1-pinctrl", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rzn1_pinctrl_match);
+
+static struct platform_driver rzn1_pinctrl_driver = {
+	.probe	= rzn1_pinctrl_probe,
+	.remove = rzn1_pinctrl_remove,
+	.driver	= {
+		.name		= "rzn1-pinctrl",
+		.owner		= THIS_MODULE,
+		.of_match_table	= rzn1_pinctrl_match,
+	},
+};
+
+static int __init _pinctrl_drv_register(void)
+{
+	return platform_driver_register(&rzn1_pinctrl_driver);
+}
+subsys_initcall(_pinctrl_drv_register);
+
+MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
+MODULE_DESCRIPTION("Renesas RZ/N1 pinctrl driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/3] ARM: dts: r9a06g032: Add pinctrl node
  2018-09-19 14:23 [PATCH v4 0/3] Renesas R9A06G032 PINCTRL Driver Phil Edworthy
  2018-09-19 14:23 ` [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy
  2018-09-19 14:23 ` [PATCH v4 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver Phil Edworthy
@ 2018-09-19 14:23 ` Phil Edworthy
  2 siblings, 0 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-09-19 14:23 UTC (permalink / raw)
  To: Geert Uytterhoeven, Laurent Pinchart
  Cc: Jacopo Mondi, Linus Walleij, Simon Horman, linux-gpio,
	linux-renesas-soc, linux-kernel, Phil Edworthy

This provides a pinctrl driver for the Renesas R9A06G032 SoC

Based on a patch originally written by Michel Pollet at Renesas.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
v4:
 - No changes.

v3:
 - No changes.

v2:
 - Add "renesas,rzn1-pinctrl" compatible fallback string
 - Register size corrected.
---
 arch/arm/boot/dts/r9a06g032.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi
index eaf94976ed6d..2322268bc862 100644
--- a/arch/arm/boot/dts/r9a06g032.dtsi
+++ b/arch/arm/boot/dts/r9a06g032.dtsi
@@ -165,6 +165,14 @@
 			status = "disabled";
 		};
 
+		pinctrl: pin-controller@40067000 {
+			compatible = "renesas,r9a06g032-pinctrl", "renesas,rzn1-pinctrl";
+			reg = <0x40067000 0x1000>, <0x51000000 0x480>;
+			clocks = <&sysctrl R9A06G032_HCLK_PINCONFIG>;
+			clock-names = "bus";
+			status = "okay";
+		};
+
 		gic: gic@44101000 {
 			compatible = "arm,cortex-a7-gic", "arm,gic-400";
 			interrupt-controller;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation
  2018-09-19 14:23 ` [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy
@ 2018-09-21 12:36   ` jacopo mondi
  2018-09-26 14:20   ` Rob Herring
  1 sibling, 0 replies; 11+ messages in thread
From: jacopo mondi @ 2018-09-21 12:36 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Geert Uytterhoeven, Laurent Pinchart, Rob Herring, Mark Rutland,
	Linus Walleij, Simon Horman, linux-gpio, linux-renesas-soc,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 14510 bytes --]

Hi Phil,

On Wed, Sep 19, 2018 at 03:23:44PM +0100, Phil Edworthy wrote:
> The Renesas RZ/N1 device family PINCTRL node description.
>
> Based on a patch originally written by Michel Pollet at Renesas.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> v4:
>  - Add alternative way to use the pinmux prop.
>  - Remove mention of gpios.
>
> v3:
>  - Use standard bindings
>  - Change the way the functions are defined so it is easy to check
>    against the hardware numbering.
>  - Add functions for the MDIO source peripheral instead of using
>    virtual pins.
>
> v2:
>  - Change filename to generic rzn1, instead of device specific.
>  - Add "renesas,rzn1-pinctrl" compatible fallback string.
>  - Example register size corrected.
>  - Typos fixed.
>  - Changes suggested by Jacopo Mondi.
>  - rzn1-pinctrl.h squashed into this as requested by Rob Herring.
> ---
>  .../bindings/pinctrl/renesas,rzn1-pinctrl.txt | 153 ++++++++++++++++++
>  include/dt-bindings/pinctrl/rzn1-pinctrl.h    | 141 ++++++++++++++++
>  2 files changed, 294 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
>  create mode 100644 include/dt-bindings/pinctrl/rzn1-pinctrl.h
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> new file mode 100644
> index 000000000000..323ef5c6c1c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> @@ -0,0 +1,153 @@
> +Renesas RZ/N1 SoC Pinctrl node description.
> +
> +Pin controller node
> +-------------------
> +Required properties:
> +- compatible: SoC-specific compatible string "renesas,<soc-specific>-pinctrl"
> +  followed by "renesas,rzn1-pinctrl" as fallback. The SoC-specific compatible
> +  strings must be one of:
> +	"renesas,r9a06g032-pinctrl" for RZ/N1D
> +	"renesas,r9a06g033-pinctrl" for RZ/N1S
> +- reg: Address base and length of the memory area where the pin controller
> +  hardware is mapped to.
> +- clocks: phandle for the clock, see the description of clock-names below.
> +- clock-names: Contains the name of the clock:
> +    "bus", the bus clock, sometimes described as pclk, for register accesses.
> +
> +Example:
> +	pinctrl: pin-controller@40067000 {
> +	    compatible = "renesas,r9a06g032-pinctrl", "renesas,rzn1-pinctrl";
> +	    reg = <0x40067000 0x1000>, <0x51000000 0x480>;
> +	    clocks = <&sysctrl R9A06G032_HCLK_PINCONFIG>;
> +	    clock-names = "bus";
> +	};
> +
> +Sub-nodes
> +---------
> +
> +The child nodes of the pin controller node describe a pin multiplexing
> +function.
> +
> +- Pin multiplexing sub-nodes:
> +  A pin multiplexing sub-node describes how to configure a set of
> +  (or a single) pin in some desired alternate function mode.
> +  A single sub-node may define several pin configurations.
> +  Please refer to pinctrl-bindings.txt to get to know more on generic
> +  pin properties usage.
> +
> +  The allowed generic formats for a pin multiplexing sub-node are the
> +  following ones:
> +
> +  node-1 {
> +      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +      GENERIC_PINCONFIG;
> +  };
> +
> +  node-2 {
> +      sub-node-1 {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +
> +      sub-node-2 {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +
> +      ...
> +
> +      sub-node-n {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +  };
> +
> +  node-3 {
> +      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +      GENERIC_PINCONFIG;
> +
> +      sub-node-1 {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +
> +      ...
> +
> +      sub-node-n {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +  };

I can't think a use case for this format, but it doesn't hurt I guess

> +
> +  Use the latter two formats when pins part of the same logical group need to
> +  have different generic pin configuration flags applied. Note that the generic
> +  pinconfig in node-3 does not apply to the sub-nodes.
> +
> +  Client sub-nodes shall refer to pin multiplexing sub-nodes using the phandle
> +  of the most external one.
> +
> +  Eg.
> +
> +  client-1 {
> +      ...
> +      pinctrl-0 = <&node-1>;
> +      ...
> +  };
> +
> +  client-2 {
> +      ...
> +      pinctrl-0 = <&node-2>;
> +      ...
> +  };
> +
> +  Required properties:
> +    - pinmux:
> +      integer array representing pin number and pin multiplexing configuration.
> +      When a pin has to be configured in alternate function mode, use this
> +      property to identify the pin by its global index, and provide its
> +      alternate function configuration number along with it.
> +      When multiple pins are required to be configured as part of the same
> +      alternate function they shall be specified as members of the same
> +      argument list of a single "pinmux" property.
> +      Integers values in the "pinmux" argument list are assembled as:
> +      (PIN | MUX_FUNC << 8)
> +      where PIN directly corresponds to the pl_gpio pin number and MUX_FUNC is
> +      one of the alternate function identifiers defined in:
> +      <include/dt-bindings/pinctrl/rzn1-pinctrl.h>
> +      These identifiers collapse the IO Multiplex Configuration Level 1 and
> +      Level 2 numbers that are detailed in the hardware reference manual into a
> +      single number. The identifiers for Level 2 are simply offset by 10.
> +      Additional identifiers are provided to specify the MDIO source peripheral.
> +
> +  Optional generic properties:
Optional generic pinconf properties

Minors apart, please add my:
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks
   j

> +    - bias-disable		- disable any pin bias
> +    - bias-pull-up		- pull up the pin with 50 KOhm
> +    - bias-pull-down		- pull down the pin with 50 KOhm
> +    - bias-high-impedance	- high impedance mode
> +    - drive-strength		- sink or source at most 4, 6, 8 or 12 mA
> +
> +  Example:
> +  A serial communication interface with a TX output pin and an RX input pin.
> +
> +  &pinctrl {
> +	pins_uart0: pins_uart0 {
> +		pinmux = <
> +			RZN1_MUX(103, UART0_I)	/* UART0_TXD */
> +			RZN1_MUX(104, UART0_I)	/* UART0_RXD */
> +		>;
> +	};
> +  };
> +
> +  Example 2:
> +  Here we set the pull up on the RXD pin of the UART.
> +
> +  &pinctrl {
> +	pins_uart0: pins_uart0 {
> +		pinmux = <RZN1_MUX(103, UART0_I)>;	/* UART0_TXD */
> +
> +		pins_uart6_rx {
> +			pinmux = <RZN1_MUX(104, UART0_I)>;	/* UART0_RXD */
> +			bias-pull-up;
> +		};
> +	};
> +  };
> diff --git a/include/dt-bindings/pinctrl/rzn1-pinctrl.h b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
> new file mode 100644
> index 000000000000..40369ee36e6a
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
> @@ -0,0 +1,141 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Defines macros and constants for Renesas RZ/N1 pin controller pin
> + * muxing functions.
> + */
> +#ifndef __DT_BINDINGS_RZN1_PINCTRL_H
> +#define __DT_BINDINGS_RZN1_PINCTRL_H
> +
> +#define RZN1_MUX(_gpio, _func) \
> +	(((RZN1_FUNC_##_func) << 8) | (_gpio))
> +
> +/*
> + * Given the different levels of muxing on the SoC, it was decided to
> + * 'linearize' them into one numerical space. So mux level 1, 2 and the MDIO
> + * muxes are all represented by one single value.
> + *
> + * You can derive the hardware value pretty easily too, as
> + * 0...9   are Level 1
> + * 10...71 are Level 2. The Level 2 mux will be set to this
> + *         value - RZN1_FUNC_L2_OFFSET, and the Level 1 mux will be
> + *         set accordingly.
> + * 72...103 are for the 2 MDIO muxes.
> + */
> +#define RZN1_FUNC_HIGHZ				0
> +#define RZN1_FUNC_0L				1
> +#define RZN1_FUNC_CLK_ETH_MII_RGMII_RMII	2
> +#define RZN1_FUNC_CLK_ETH_NAND			3
> +#define RZN1_FUNC_QSPI				4
> +#define RZN1_FUNC_SDIO				5
> +#define RZN1_FUNC_LCD				6
> +#define RZN1_FUNC_LCD_E				7
> +#define RZN1_FUNC_MSEBIM			8
> +#define RZN1_FUNC_MSEBIS			9
> +#define RZN1_FUNC_L2_OFFSET			10	/* I'm Special */
> +
> +#define RZN1_FUNC_HIGHZ1			(RZN1_FUNC_L2_OFFSET + 0)
> +#define RZN1_FUNC_ETHERCAT			(RZN1_FUNC_L2_OFFSET + 1)
> +#define RZN1_FUNC_SERCOS3			(RZN1_FUNC_L2_OFFSET + 2)
> +#define RZN1_FUNC_SDIO_E			(RZN1_FUNC_L2_OFFSET + 3)
> +#define RZN1_FUNC_ETH_MDIO			(RZN1_FUNC_L2_OFFSET + 4)
> +#define RZN1_FUNC_ETH_MDIO_E1			(RZN1_FUNC_L2_OFFSET + 5)
> +#define RZN1_FUNC_USB				(RZN1_FUNC_L2_OFFSET + 6)
> +#define RZN1_FUNC_MSEBIM_E			(RZN1_FUNC_L2_OFFSET + 7)
> +#define RZN1_FUNC_MSEBIS_E			(RZN1_FUNC_L2_OFFSET + 8)
> +#define RZN1_FUNC_RSV				(RZN1_FUNC_L2_OFFSET + 9)
> +#define RZN1_FUNC_RSV_E				(RZN1_FUNC_L2_OFFSET + 10)
> +#define RZN1_FUNC_RSV_E1			(RZN1_FUNC_L2_OFFSET + 11)
> +#define RZN1_FUNC_UART0_I			(RZN1_FUNC_L2_OFFSET + 12)
> +#define RZN1_FUNC_UART0_I_E			(RZN1_FUNC_L2_OFFSET + 13)
> +#define RZN1_FUNC_UART1_I			(RZN1_FUNC_L2_OFFSET + 14)
> +#define RZN1_FUNC_UART1_I_E			(RZN1_FUNC_L2_OFFSET + 15)
> +#define RZN1_FUNC_UART2_I			(RZN1_FUNC_L2_OFFSET + 16)
> +#define RZN1_FUNC_UART2_I_E			(RZN1_FUNC_L2_OFFSET + 17)
> +#define RZN1_FUNC_UART0				(RZN1_FUNC_L2_OFFSET + 18)
> +#define RZN1_FUNC_UART0_E			(RZN1_FUNC_L2_OFFSET + 19)
> +#define RZN1_FUNC_UART1				(RZN1_FUNC_L2_OFFSET + 20)
> +#define RZN1_FUNC_UART1_E			(RZN1_FUNC_L2_OFFSET + 21)
> +#define RZN1_FUNC_UART2				(RZN1_FUNC_L2_OFFSET + 22)
> +#define RZN1_FUNC_UART2_E			(RZN1_FUNC_L2_OFFSET + 23)
> +#define RZN1_FUNC_UART3				(RZN1_FUNC_L2_OFFSET + 24)
> +#define RZN1_FUNC_UART3_E			(RZN1_FUNC_L2_OFFSET + 25)
> +#define RZN1_FUNC_UART4				(RZN1_FUNC_L2_OFFSET + 26)
> +#define RZN1_FUNC_UART4_E			(RZN1_FUNC_L2_OFFSET + 27)
> +#define RZN1_FUNC_UART5				(RZN1_FUNC_L2_OFFSET + 28)
> +#define RZN1_FUNC_UART5_E			(RZN1_FUNC_L2_OFFSET + 29)
> +#define RZN1_FUNC_UART6				(RZN1_FUNC_L2_OFFSET + 30)
> +#define RZN1_FUNC_UART6_E			(RZN1_FUNC_L2_OFFSET + 31)
> +#define RZN1_FUNC_UART7				(RZN1_FUNC_L2_OFFSET + 32)
> +#define RZN1_FUNC_UART7_E			(RZN1_FUNC_L2_OFFSET + 33)
> +#define RZN1_FUNC_SPI0_M			(RZN1_FUNC_L2_OFFSET + 34)
> +#define RZN1_FUNC_SPI0_M_E			(RZN1_FUNC_L2_OFFSET + 35)
> +#define RZN1_FUNC_SPI1_M			(RZN1_FUNC_L2_OFFSET + 36)
> +#define RZN1_FUNC_SPI1_M_E			(RZN1_FUNC_L2_OFFSET + 37)
> +#define RZN1_FUNC_SPI2_M			(RZN1_FUNC_L2_OFFSET + 38)
> +#define RZN1_FUNC_SPI2_M_E			(RZN1_FUNC_L2_OFFSET + 39)
> +#define RZN1_FUNC_SPI3_M			(RZN1_FUNC_L2_OFFSET + 40)
> +#define RZN1_FUNC_SPI3_M_E			(RZN1_FUNC_L2_OFFSET + 41)
> +#define RZN1_FUNC_SPI4_S			(RZN1_FUNC_L2_OFFSET + 42)
> +#define RZN1_FUNC_SPI4_S_E			(RZN1_FUNC_L2_OFFSET + 43)
> +#define RZN1_FUNC_SPI5_S			(RZN1_FUNC_L2_OFFSET + 44)
> +#define RZN1_FUNC_SPI5_S_E			(RZN1_FUNC_L2_OFFSET + 45)
> +#define RZN1_FUNC_SGPIO0_M			(RZN1_FUNC_L2_OFFSET + 46)
> +#define RZN1_FUNC_SGPIO1_M			(RZN1_FUNC_L2_OFFSET + 47)
> +#define RZN1_FUNC_GPIO				(RZN1_FUNC_L2_OFFSET + 48)
> +#define RZN1_FUNC_CAN				(RZN1_FUNC_L2_OFFSET + 49)
> +#define RZN1_FUNC_I2C				(RZN1_FUNC_L2_OFFSET + 50)
> +#define RZN1_FUNC_SAFE				(RZN1_FUNC_L2_OFFSET + 51)
> +#define RZN1_FUNC_PTO_PWM			(RZN1_FUNC_L2_OFFSET + 52)
> +#define RZN1_FUNC_PTO_PWM1			(RZN1_FUNC_L2_OFFSET + 53)
> +#define RZN1_FUNC_PTO_PWM2			(RZN1_FUNC_L2_OFFSET + 54)
> +#define RZN1_FUNC_PTO_PWM3			(RZN1_FUNC_L2_OFFSET + 55)
> +#define RZN1_FUNC_PTO_PWM4			(RZN1_FUNC_L2_OFFSET + 56)
> +#define RZN1_FUNC_DELTA_SIGMA			(RZN1_FUNC_L2_OFFSET + 57)
> +#define RZN1_FUNC_SGPIO2_M			(RZN1_FUNC_L2_OFFSET + 58)
> +#define RZN1_FUNC_SGPIO3_M			(RZN1_FUNC_L2_OFFSET + 59)
> +#define RZN1_FUNC_SGPIO4_S			(RZN1_FUNC_L2_OFFSET + 60)
> +#define RZN1_FUNC_MAC_MTIP_SWITCH		(RZN1_FUNC_L2_OFFSET + 61)
> +
> +#define RZN1_FUNC_MDIO_OFFSET			(RZN1_FUNC_L2_OFFSET + 62)
> +
> +/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO function */
> +#define RZN1_FUNC_MDIO0_HIGHZ			(RZN1_FUNC_MDIO_OFFSET + 0)
> +#define RZN1_FUNC_MDIO0_GMAC0			(RZN1_FUNC_MDIO_OFFSET + 1)
> +#define RZN1_FUNC_MDIO0_GMAC1			(RZN1_FUNC_MDIO_OFFSET + 2)
> +#define RZN1_FUNC_MDIO0_ECAT			(RZN1_FUNC_MDIO_OFFSET + 3)
> +#define RZN1_FUNC_MDIO0_S3_MDIO0		(RZN1_FUNC_MDIO_OFFSET + 4)
> +#define RZN1_FUNC_MDIO0_S3_MDIO1		(RZN1_FUNC_MDIO_OFFSET + 5)
> +#define RZN1_FUNC_MDIO0_HWRTOS			(RZN1_FUNC_MDIO_OFFSET + 6)
> +#define RZN1_FUNC_MDIO0_SWITCH			(RZN1_FUNC_MDIO_OFFSET + 7)
> +/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
> +#define RZN1_FUNC_MDIO0_E1_HIGHZ		(RZN1_FUNC_MDIO_OFFSET + 8)
> +#define RZN1_FUNC_MDIO0_E1_GMAC0		(RZN1_FUNC_MDIO_OFFSET + 9)
> +#define RZN1_FUNC_MDIO0_E1_GMAC1		(RZN1_FUNC_MDIO_OFFSET + 10)
> +#define RZN1_FUNC_MDIO0_E1_ECAT			(RZN1_FUNC_MDIO_OFFSET + 11)
> +#define RZN1_FUNC_MDIO0_E1_S3_MDIO0		(RZN1_FUNC_MDIO_OFFSET + 12)
> +#define RZN1_FUNC_MDIO0_E1_S3_MDIO1		(RZN1_FUNC_MDIO_OFFSET + 13)
> +#define RZN1_FUNC_MDIO0_E1_HWRTOS		(RZN1_FUNC_MDIO_OFFSET + 14)
> +#define RZN1_FUNC_MDIO0_E1_SWITCH		(RZN1_FUNC_MDIO_OFFSET + 15)
> +
> +/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO function */
> +#define RZN1_FUNC_MDIO1_HIGHZ			(RZN1_FUNC_MDIO_OFFSET + 16)
> +#define RZN1_FUNC_MDIO1_GMAC0			(RZN1_FUNC_MDIO_OFFSET + 17)
> +#define RZN1_FUNC_MDIO1_GMAC1			(RZN1_FUNC_MDIO_OFFSET + 18)
> +#define RZN1_FUNC_MDIO1_ECAT			(RZN1_FUNC_MDIO_OFFSET + 19)
> +#define RZN1_FUNC_MDIO1_S3_MDIO0		(RZN1_FUNC_MDIO_OFFSET + 20)
> +#define RZN1_FUNC_MDIO1_S3_MDIO1		(RZN1_FUNC_MDIO_OFFSET + 21)
> +#define RZN1_FUNC_MDIO1_HWRTOS			(RZN1_FUNC_MDIO_OFFSET + 22)
> +#define RZN1_FUNC_MDIO1_SWITCH			(RZN1_FUNC_MDIO_OFFSET + 23)
> +/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
> +#define RZN1_FUNC_MDIO1_E1_HIGHZ		(RZN1_FUNC_MDIO_OFFSET + 24)
> +#define RZN1_FUNC_MDIO1_E1_GMAC0		(RZN1_FUNC_MDIO_OFFSET + 25)
> +#define RZN1_FUNC_MDIO1_E1_GMAC1		(RZN1_FUNC_MDIO_OFFSET + 26)
> +#define RZN1_FUNC_MDIO1_E1_ECAT			(RZN1_FUNC_MDIO_OFFSET + 27)
> +#define RZN1_FUNC_MDIO1_E1_S3_MDIO0		(RZN1_FUNC_MDIO_OFFSET + 28)
> +#define RZN1_FUNC_MDIO1_E1_S3_MDIO1		(RZN1_FUNC_MDIO_OFFSET + 29)
> +#define RZN1_FUNC_MDIO1_E1_HWRTOS		(RZN1_FUNC_MDIO_OFFSET + 30)
> +#define RZN1_FUNC_MDIO1_E1_SWITCH		(RZN1_FUNC_MDIO_OFFSET + 31)
> +
> +#define RZN1_FUNC_MAX				(RZN1_FUNC_MDIO_OFFSET + 32)
> +
> +#endif /* __DT_BINDINGS_RZN1_PINCTRL_H */
> --
> 2.17.1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver
  2018-09-19 14:23 ` [PATCH v4 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver Phil Edworthy
@ 2018-09-21 13:33   ` jacopo mondi
  2018-09-24 11:58   ` Geert Uytterhoeven
  1 sibling, 0 replies; 11+ messages in thread
From: jacopo mondi @ 2018-09-21 13:33 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Geert Uytterhoeven, Laurent Pinchart, Linus Walleij,
	Simon Horman, linux-gpio, linux-renesas-soc, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 33393 bytes --]

Hi Phil,
   thanks for the patches.

Mostly minor comments left at this review stage, please see below.

Thanks for the offline clarifications and please bear with me a little
more.

On Wed, Sep 19, 2018 at 03:23:45PM +0100, Phil Edworthy wrote:
> This provides a pinctrl driver for the Renesas RZ/N1 device family.
>
> Based on a patch originally written by Michel Pollet at Renesas.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> v4:
>  - Address Jacopo's comments
>  - Implement pin_config_group_get()
>  - Fix function to get pin configs, i.e. return -EINVAL when disabled.
>
> v3:
>  - Use standard DT props instead of proprietary ones.
>  - Replace virtual pins used for MDIO muxing with extra funcs.
>  - Use pinctrl_utils funcs to handle the maps.
>  - Remove the dbg functions to keep things simple.
>
> v2:
>  - Change filename to generic rzn1, instead of device specific.
>  - Changed Kconfig symbol and file name to generic rzn1 family.
>  - Added "renesas,rzn1-pinctrl" compatible fallback string
>  - Changes suggested by Jacopo Mondi. Mainly formatting, plus:
>    - Removed global ptr
>    - Removed unused code accessing parent of node.
>    - Removed check for null OF np that can't happen.
>    - Replaced overlapping enums with #defines
>  - Renamed some variables and symbols to clarify their use
>  - Fix error handling during probe
>  - Move probe from postcore_initcall to subsys_initcall to ensure
>    drivers that require clocks get them instead of having to defer
>    probing.
> ---
>  drivers/pinctrl/Kconfig        |  10 +
>  drivers/pinctrl/Makefile       |   1 +
>  drivers/pinctrl/pinctrl-rzn1.c | 951 +++++++++++++++++++++++++++++++++
>  3 files changed, 962 insertions(+)
>  create mode 100644 drivers/pinctrl/pinctrl-rzn1.c
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 978b2ed4d014..4d8c00eac742 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -195,6 +195,16 @@ config PINCTRL_RZA1
>  	help
>  	  This selects pinctrl driver for Renesas RZ/A1 platforms.
>
> +config PINCTRL_RZN1
> +	bool "Renesas RZ/N1 pinctrl driver"
> +	depends on OF
> +	depends on ARCH_RZN1 || COMPILE_TEST
> +	select GENERIC_PINCTRL_GROUPS
> +	select GENERIC_PINMUX_FUNCTIONS
> +	select GENERIC_PINCONF
> +	help
> +	  This selects pinctrl driver for Renesas RZ/N1 devices.
> +
>  config PINCTRL_SINGLE
>  	tristate "One-register-per-pin type device tree based pinctrl driver"
>  	depends on OF
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index 8e127bd8610f..18a13c1e2c21 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_PINCTRL_PIC32)	+= pinctrl-pic32.o
>  obj-$(CONFIG_PINCTRL_PISTACHIO)	+= pinctrl-pistachio.o
>  obj-$(CONFIG_PINCTRL_ROCKCHIP)	+= pinctrl-rockchip.o
>  obj-$(CONFIG_PINCTRL_RZA1)	+= pinctrl-rza1.o
> +obj-$(CONFIG_PINCTRL_RZN1)	+= pinctrl-rzn1.o
>  obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
>  obj-$(CONFIG_PINCTRL_SIRF)	+= sirf/
>  obj-$(CONFIG_PINCTRL_SX150X)	+= pinctrl-sx150x.o
> diff --git a/drivers/pinctrl/pinctrl-rzn1.c b/drivers/pinctrl/pinctrl-rzn1.c
> new file mode 100644
> index 000000000000..114fd2143c1d
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-rzn1.c
> @@ -0,0 +1,951 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2014-2018 Renesas Electronics Europe Limited
> + *
> + * Phil Edworthy <phil.edworthy@renesas.com>
> + * Based on a driver originally written by Michel Pollet at Renesas.
> + */
> +
> +#include <dt-bindings/pinctrl/rzn1-pinctrl.h>
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include "core.h"
> +#include "pinconf.h"
> +#include "pinctrl-utils.h"
> +
> +/* Field positions and masks in the pinmux registers */
> +#define RZN1_L1_PIN_DRIVE_STRENGTH	10
> +#define RZN1_L1_PIN_DRIVE_STRENGTH_4MA	0
> +#define RZN1_L1_PIN_DRIVE_STRENGTH_6MA	1
> +#define RZN1_L1_PIN_DRIVE_STRENGTH_8MA	2
> +#define RZN1_L1_PIN_DRIVE_STRENGTH_12MA	3
> +#define RZN1_L1_PIN_PULL		8
> +#define RZN1_L1_PIN_PULL_NONE		0
> +#define RZN1_L1_PIN_PULL_UP		1
> +#define RZN1_L1_PIN_PULL_DOWN		3
> +#define RZN1_L1_FUNCTION		0
> +#define RZN1_L1_FUNC_MASK		0xf
> +#define RZN1_L1_FUNCTION_L2		0xf
> +
> +/*
> + * The hardware manual describes two levels of multiplexing, but it's more
> + * logical to think of the hardware as three levels, with level 3 consisting of
> + * the multiplexing for Ethernet MDIO signals.
> + *
> + * Level 1 functions go from 0 to 9, with level 1 function '15' (0xf) specifying
> + * that level 2 functions are used instead. Level 2 has a lot more options,
> + * going from 0 to 61. Level 3 allows selection of MDIO functions which can be
> + * floating, or one of seven internal peripherals. Unfortunately, there are two
> + * level 2 functions that can select MDIO, and two MDIO channels so we have four
> + * sets of level 3 functions.
> + *
> + * For this driver, we've compounded the numbers together, so:
> + *    0 to   9 is level 1
> + *   10 to  71 is 10 + level 2 number
> + *   72 to  79 is 72 + MDIO0 source for level 2 MDIO function.
> + *   80 to  87 is 80 + MDIO0 source for level 2 MDIO_E1 function.
> + *   88 to  95 is 88 + MDIO1 source for level 2 MDIO function.
> + *   96 to 103 is 96 + MDIO1 source for level 2 MDIO_E1 function.
> + * Examples:
> + *  Function 28 corresponds UART0
> + *  Function 73 corresponds to MDIO0 to GMAC0
> + *
> + * There are 170 configurable pins (called PL_GPIO in the datasheet).
> + */
> +
> +/*
> + * Structure detailing the HW registers on the RZ/N1 devices.
> + * Both the Level 1 mux registers and Level 2 mux registers have the same
> + * structure. The only difference is that Level 2 has additional MDIO registers
> + * at the end.
> + */
> +struct rzn1_pinctrl_regs {
> +	union {
> +		u32	conf[170];
> +		u8	pad0[0x400];
> +	};
> +	u32	status_protect;	/* 0x400 */
> +	/* MDIO mux registers, level2 only */
> +	u32	l2_mdio[2];
> +};
> +
> +/**
> + * struct rzn1_pmx_func - describes rzn1 pinmux functions
> + * @name: the name of this specific function
> + * @groups: corresponding pin groups
> + * @num_groups: the number of groups
> + */
> +struct rzn1_pmx_func {
> +	const char *name;
> +	const char **groups;
> +	unsigned int num_groups;
> +};
> +
> +/**
> + * struct rzn1_pin_group - describes an rzn1 pin group
> + * @name: the name of this specific pin group
> + * @func: the name of the function selected by this group
> + * @npins: the number of pins in this group array, i.e. the number of
> + *	elements in .pins so we can iterate over that array
> + * @pin_ids: array of pin_ids, i.e. the value used to select the mux
> + * @pins: array of pins. Needed due to pinctrl_ops.get_group_pins()
> + */
> +struct rzn1_pin_group {
> +	const char *name;
> +	const char *func;
> +	unsigned int npins;
> +	u8 *pin_ids;
> +	u32 *pins;
> +};
> +
> +struct rzn1_pinctrl {
> +	struct device *dev;
> +	struct clk *clk;
> +	struct pinctrl_dev *pctl;
> +	struct rzn1_pinctrl_regs __iomem *lev1;
> +	struct rzn1_pinctrl_regs __iomem *lev2;
> +	u32 lev1_protect_phys;
> +	u32 lev2_protect_phys;
> +	int mdio_func[2];
> +
> +	struct rzn1_pin_group *groups;
> +	unsigned int ngroups;
> +
> +	struct rzn1_pmx_func *functions;
> +	unsigned int nfunctions;
> +};
> +
> +#define RZN1_PINS_PROP "pinmux"
> +
> +#define RZN1_PIN(pin) PINCTRL_PIN(pin, "pl_gpio"#pin)
> +
> +static const struct pinctrl_pin_desc rzn1_pins[] = {
> +	RZN1_PIN(0), RZN1_PIN(1), RZN1_PIN(2), RZN1_PIN(3), RZN1_PIN(4),
> +	RZN1_PIN(5), RZN1_PIN(6), RZN1_PIN(7), RZN1_PIN(8), RZN1_PIN(9),
> +	RZN1_PIN(10), RZN1_PIN(11), RZN1_PIN(12), RZN1_PIN(13), RZN1_PIN(14),
> +	RZN1_PIN(15), RZN1_PIN(16), RZN1_PIN(17), RZN1_PIN(18), RZN1_PIN(19),
> +	RZN1_PIN(20), RZN1_PIN(21), RZN1_PIN(22), RZN1_PIN(23), RZN1_PIN(24),
> +	RZN1_PIN(25), RZN1_PIN(26), RZN1_PIN(27), RZN1_PIN(28), RZN1_PIN(29),
> +	RZN1_PIN(30), RZN1_PIN(31), RZN1_PIN(32), RZN1_PIN(33), RZN1_PIN(34),
> +	RZN1_PIN(35), RZN1_PIN(36), RZN1_PIN(37), RZN1_PIN(38), RZN1_PIN(39),
> +	RZN1_PIN(40), RZN1_PIN(41), RZN1_PIN(42), RZN1_PIN(43), RZN1_PIN(44),
> +	RZN1_PIN(45), RZN1_PIN(46), RZN1_PIN(47), RZN1_PIN(48), RZN1_PIN(49),
> +	RZN1_PIN(50), RZN1_PIN(51), RZN1_PIN(52), RZN1_PIN(53), RZN1_PIN(54),
> +	RZN1_PIN(55), RZN1_PIN(56), RZN1_PIN(57), RZN1_PIN(58), RZN1_PIN(59),
> +	RZN1_PIN(60), RZN1_PIN(61), RZN1_PIN(62), RZN1_PIN(63), RZN1_PIN(64),
> +	RZN1_PIN(65), RZN1_PIN(66), RZN1_PIN(67), RZN1_PIN(68), RZN1_PIN(69),
> +	RZN1_PIN(70), RZN1_PIN(71), RZN1_PIN(72), RZN1_PIN(73), RZN1_PIN(74),
> +	RZN1_PIN(75), RZN1_PIN(76), RZN1_PIN(77), RZN1_PIN(78), RZN1_PIN(79),
> +	RZN1_PIN(80), RZN1_PIN(81), RZN1_PIN(82), RZN1_PIN(83), RZN1_PIN(84),
> +	RZN1_PIN(85), RZN1_PIN(86), RZN1_PIN(87), RZN1_PIN(88), RZN1_PIN(89),
> +	RZN1_PIN(90), RZN1_PIN(91), RZN1_PIN(92), RZN1_PIN(93), RZN1_PIN(94),
> +	RZN1_PIN(95), RZN1_PIN(96), RZN1_PIN(97), RZN1_PIN(98), RZN1_PIN(99),
> +	RZN1_PIN(100), RZN1_PIN(101), RZN1_PIN(102), RZN1_PIN(103),
> +	RZN1_PIN(104), RZN1_PIN(105), RZN1_PIN(106), RZN1_PIN(107),
> +	RZN1_PIN(108), RZN1_PIN(109), RZN1_PIN(110), RZN1_PIN(111),
> +	RZN1_PIN(112), RZN1_PIN(113), RZN1_PIN(114), RZN1_PIN(115),
> +	RZN1_PIN(116), RZN1_PIN(117), RZN1_PIN(118), RZN1_PIN(119),
> +	RZN1_PIN(120), RZN1_PIN(121), RZN1_PIN(122), RZN1_PIN(123),
> +	RZN1_PIN(124), RZN1_PIN(125), RZN1_PIN(126), RZN1_PIN(127),
> +	RZN1_PIN(128), RZN1_PIN(129), RZN1_PIN(130), RZN1_PIN(131),
> +	RZN1_PIN(132), RZN1_PIN(133), RZN1_PIN(134), RZN1_PIN(135),
> +	RZN1_PIN(136), RZN1_PIN(137), RZN1_PIN(138), RZN1_PIN(139),
> +	RZN1_PIN(140), RZN1_PIN(141), RZN1_PIN(142), RZN1_PIN(143),
> +	RZN1_PIN(144), RZN1_PIN(145), RZN1_PIN(146), RZN1_PIN(147),
> +	RZN1_PIN(148), RZN1_PIN(149), RZN1_PIN(150), RZN1_PIN(151),
> +	RZN1_PIN(152), RZN1_PIN(153), RZN1_PIN(154), RZN1_PIN(155),
> +	RZN1_PIN(156), RZN1_PIN(157), RZN1_PIN(158), RZN1_PIN(159),
> +	RZN1_PIN(160), RZN1_PIN(161), RZN1_PIN(162), RZN1_PIN(163),
> +	RZN1_PIN(164), RZN1_PIN(165), RZN1_PIN(166), RZN1_PIN(167),
> +	RZN1_PIN(168), RZN1_PIN(169),
> +};
> +
> +enum {
> +	LOCK_LEVEL1 = 0x1,
> +	LOCK_LEVEL2 = 0x2,
> +	LOCK_ALL = LOCK_LEVEL1 | LOCK_LEVEL2,
> +};
> +
> +static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8 value)
> +{
> +	/*
> +	 * The pinmux configuration is locked by writing the physical address of
> +	 * the status_protect register to itself. It is unlocked by writing the
> +	 * address | 1.
> +	 */
> +	if (lock & LOCK_LEVEL1) {
> +		u32 val = ipctl->lev1_protect_phys | !(value & LOCK_LEVEL1);
> +
> +		writel(val, &ipctl->lev1->status_protect);
> +	}
> +
> +	if (lock & LOCK_LEVEL2) {
> +		u32 val = ipctl->lev2_protect_phys | !(value & LOCK_LEVEL2);
> +
> +		writel(val, &ipctl->lev2->status_protect);
> +	}
> +}
> +
> +static void rzn1_pinctrl_mdio_select(struct rzn1_pinctrl *ipctl, u8 mdio,
> +				     u32 func)
> +{
> +	if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != func)
> +		dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n", mdio);
> +	ipctl->mdio_func[mdio] = func;
> +
> +	dev_dbg(ipctl->dev, "setting mdio%d to %d\n", mdio, func);
> +
> +	writel(func, &ipctl->lev2->l2_mdio[mdio]);
> +}
> +
> +/*
> + * Using a composite pin description, set the hardware pinmux registers
> + * with the corresponding values.
> + * Make sure to unlock write protection and reset it afterward.
> + *
> + * NOTE: There is no protection for potential concurrency, it is assumed these
> + * calls are serialized already.
> + */
> +static int rzn1_set_hw_pin_func(struct rzn1_pinctrl *ipctl, u32 pin,
> +				u32 pin_config, u8 use_locks)
> +{
> +	u32 l1_cache;
> +	u32 l2_cache;
> +	u32 l1;
> +	u32 l2;
> +
> +	/* Level 3 MDIO multiplexing */
> +	if (pin_config >= RZN1_FUNC_MDIO0_HIGHZ &&
> +	    pin_config <= RZN1_FUNC_MDIO1_E1_SWITCH) {
> +		u8 mdio_channel;
> +		u32 mdio_func;
> +
> +		if (pin_config <= RZN1_FUNC_MDIO1_HIGHZ)
> +			mdio_channel = 0;
> +		else
> +			mdio_channel = 1;
> +
> +		/* Get MDIO func, and convert the func to the level 2 number */
> +		if (pin_config <= RZN1_FUNC_MDIO0_SWITCH) {
> +			mdio_func = pin_config - RZN1_FUNC_MDIO0_HIGHZ;
> +			pin_config = RZN1_FUNC_ETH_MDIO;
> +		} else if (pin_config <= RZN1_FUNC_MDIO0_E1_SWITCH) {
> +			mdio_func = pin_config - RZN1_FUNC_MDIO0_E1_HIGHZ;
> +			pin_config = RZN1_FUNC_ETH_MDIO_E1;
> +		} else if (pin_config <= RZN1_FUNC_MDIO1_SWITCH) {
> +			mdio_func = pin_config - RZN1_FUNC_MDIO1_HIGHZ;
> +			pin_config = RZN1_FUNC_ETH_MDIO;
> +		} else {
> +			mdio_func = pin_config - RZN1_FUNC_MDIO1_E1_HIGHZ;
> +			pin_config = RZN1_FUNC_ETH_MDIO_E1;
> +		}
> +		rzn1_pinctrl_mdio_select(ipctl, mdio_channel, mdio_func);
> +	}
> +
> +	/* Note here, we do not allow anything past the MDIO Mux values */
> +	if (pin >= ARRAY_SIZE(ipctl->lev1->conf) ||
> +	    pin_config >= RZN1_FUNC_MDIO0_HIGHZ)
> +		return -EINVAL;
> +
> +	l1 = readl(&ipctl->lev1->conf[pin]);
> +	l1_cache = l1;
> +	l2 = readl(&ipctl->lev2->conf[pin]);
> +	l2_cache = l2;
> +
> +	dev_dbg(ipctl->dev, "setting func for pin %d to %d\n", pin, pin_config);
> +
> +	if (pin_config < RZN1_FUNC_L2_OFFSET) {
> +		l1 &= ~(RZN1_L1_FUNC_MASK << RZN1_L1_FUNCTION);

Nit:
that's the same in both branches, if I see correctly. You can move it
one line up.

> +		l1 |= (pin_config << RZN1_L1_FUNCTION);
> +	} else {
> +		l1 &= ~(RZN1_L1_FUNC_MASK << RZN1_L1_FUNCTION);
> +		l1 |= (RZN1_L1_FUNCTION_L2 << RZN1_L1_FUNCTION);
> +
> +		l2 = pin_config - RZN1_FUNC_L2_OFFSET;
> +	}
> +
> +	/* If either configuration changes, we update both anyway */
> +	if (l1 != l1_cache || l2 != l2_cache) {
> +		writel(l1, &ipctl->lev1->conf[pin]);
> +		writel(l2, &ipctl->lev2->conf[pin]);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct rzn1_pin_group *rzn1_pinctrl_find_group_by_name(
> +	const struct rzn1_pinctrl *ipctl, const char *name)
> +{
> +	const struct rzn1_pin_group *grp = NULL;

Nit:
no need for local variable if you:

> +	int i;
> +
> +	for (i = 0; i < ipctl->ngroups; i++) {
> +		if (!strcmp(ipctl->groups[i].name, name)) {

return &ipctl->groups[i]; here

> +			grp = &ipctl->groups[i];
> +			break;
> +		}
> +	}
> +
> +	return grp;

return NULL; here

> +}
> +
> +static int rzn1_get_groups_count(struct pinctrl_dev *pctldev)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	return ipctl->ngroups;
> +}
> +
> +static const char *rzn1_get_group_name(struct pinctrl_dev *pctldev,
> +				       unsigned int selector)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	return ipctl->groups[selector].name;
> +}
> +
> +static int rzn1_get_group_pins(struct pinctrl_dev *pctldev,
> +			       unsigned int selector, const unsigned int **pins,
> +			       unsigned int *npins)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	if (selector >= ipctl->ngroups)
> +		return -EINVAL;
> +
> +	*pins = ipctl->groups[selector].pins;
> +	*npins = ipctl->groups[selector].npins;
> +
> +	return 0;
> +}
> +
> +/*
> + * This function is called for each pinctl 'Function' node.
> + * Sub-nodes can be used to describe multiple 'Groups' for the 'Function'
> + * If there aren't any sub-nodes, the 'Group' is essentially the 'Function'.
> + * Each 'Group' uses pinmux = <...> to detail the pins and data used to select
> + * the functionality. Each 'Group' has optional pin configurations that apply
> + * to all pins in the 'Group'.
> + */
> +static int rzn1_dt_node_to_map_one(struct pinctrl_dev *pctldev,
> +				   struct device_node *np,
> +				   struct pinctrl_map **map,
> +				   unsigned int *num_maps)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct rzn1_pin_group *grp;
> +	unsigned long *configs = NULL;
> +	unsigned int num_configs = 0;
> +	unsigned int reserved_maps = *num_maps;
> +	unsigned int reserve = 1;
> +	int ret;
> +
> +	dev_dbg(ipctl->dev, "processing node %pOF\n", np);
> +
> +	grp = rzn1_pinctrl_find_group_by_name(ipctl, np->name);
> +	if (!grp) {
> +		dev_err(ipctl->dev, "unable to find group for node %pOF\n", np);
> +
> +		return -EINVAL;
> +	}
> +
> +	/* Get the group's pin configuration */
> +	ret = pinconf_generic_parse_dt_config(np, pctldev, &configs,
> +					      &num_configs);
> +	if (ret < 0) {
> +		dev_err(ipctl->dev, "%pOF: could not parse property\n", np);
> +
> +		return ret;
> +	}
> +
> +	if (num_configs)
> +		reserve++;
> +
> +	/* Increase the number of maps to cover this group */
> +	ret = pinctrl_utils_reserve_map(pctldev, map, &reserved_maps, num_maps,
> +					reserve);
> +	if (ret < 0)
> +		goto out;
> +
> +	/* Associate the group with the function */
> +	ret = pinctrl_utils_add_map_mux(pctldev, map, &reserved_maps, num_maps,
> +					grp->name, grp->func);
> +	if (ret < 0)
> +		goto out;
> +
> +	if (num_configs) {
> +		/* Associate the group's pin configuration with the group */
> +		ret = pinctrl_utils_add_map_configs(pctldev, map,
> +				&reserved_maps, num_maps, grp->name,
> +				configs, num_configs,
> +				PIN_MAP_TYPE_CONFIGS_GROUP);
> +		if (ret < 0)
> +			goto out;
> +	}
> +
> +	dev_dbg(pctldev->dev, "maps: function %s group %s (%d pins)\n",
> +		grp->func, grp->name, grp->npins);
> +
> +out:
> +	kfree(configs);
> +
> +	return ret;
> +}
> +
> +static int rzn1_dt_node_to_map(struct pinctrl_dev *pctldev,
> +			       struct device_node *np,
> +			       struct pinctrl_map **map,
> +			       unsigned int *num_maps)
> +{
> +	struct device_node *child;
> +	int ret;
> +
> +	*map = NULL;
> +	*num_maps = 0;
> +
> +	ret = rzn1_dt_node_to_map_one(pctldev, np, map, num_maps);
> +	if (ret < 0)
> +		return ret;
> +
> +	for_each_child_of_node(np, child) {
> +		ret = rzn1_dt_node_to_map_one(pctldev, child, map, num_maps);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pinctrl_ops rzn1_pctrl_ops = {
> +	.get_groups_count = rzn1_get_groups_count,
> +	.get_group_name = rzn1_get_group_name,
> +	.get_group_pins = rzn1_get_group_pins,
> +	.dt_node_to_map = rzn1_dt_node_to_map,
> +	.dt_free_map = pinctrl_utils_free_map,
> +};
> +
> +static int rzn1_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	return ipctl->nfunctions;
> +}
> +
> +static const char *rzn1_pmx_get_func_name(struct pinctrl_dev *pctldev,
> +					  unsigned int selector)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	return ipctl->functions[selector].name;
> +}
> +
> +static int rzn1_pmx_get_groups(struct pinctrl_dev *pctldev,
> +			       unsigned int selector,
> +			       const char * const **groups,
> +			       unsigned int * const num_groups)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +
> +	*groups = ipctl->functions[selector].groups;
> +	*num_groups = ipctl->functions[selector].num_groups;
> +
> +	return 0;
> +}
> +
> +static int rzn1_set_mux(struct pinctrl_dev *pctldev, unsigned int selector,
> +			unsigned int group)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +	struct rzn1_pin_group *grp = &ipctl->groups[group];
> +	unsigned int i, grp_pins = grp->npins;
> +
> +	dev_dbg(ipctl->dev, "set mux %s(%d) group %s(%d)\n",
> +		ipctl->functions[selector].name, selector, grp->name, group);
> +
> +	rzn1_hw_set_lock(ipctl, LOCK_ALL, LOCK_ALL);
> +	for (i = 0; i < grp_pins; i++)
> +		rzn1_set_hw_pin_func(ipctl, grp->pins[i], grp->pin_ids[i], 0);
> +	rzn1_hw_set_lock(ipctl, LOCK_ALL, 0);
> +
> +	return 0;
> +}
> +
> +static const struct pinmux_ops rzn1_pmx_ops = {
> +	.get_functions_count = rzn1_pmx_get_funcs_count,
> +	.get_function_name = rzn1_pmx_get_func_name,
> +	.get_function_groups = rzn1_pmx_get_groups,
> +	.set_mux = rzn1_set_mux,
> +};
> +
> +static int rzn1_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
> +			    unsigned long *config)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +	enum pin_config_param param = pinconf_to_config_param(*config);
> +	const u32 reg_drive[4] = { 4, 6, 8, 12 };
> +	u32 l1, l2;
> +	u32 pull, drive, l1mux;

You -almost- sorted variable declaration in the reversed-xmas-tree
order, which is not a strict coding rule, but it's nice (and adopted
by most of Renesas pinctrl code). If you find yourself in a mood
for this masturbatory activity, could you sort them accordingly here
and in the other functions?

> +	u32 arg = 0;
> +
> +	if (pin >= ARRAY_SIZE(ipctl->lev1->conf))
> +		return -EINVAL;
> +
> +	l1 = readl(&ipctl->lev1->conf[pin]);
> +
> +	l1mux = l1 & RZN1_L1_FUNC_MASK;
> +	pull = (l1 >> RZN1_L1_PIN_PULL) & 0x3;
> +	drive = (l1 >> RZN1_L1_PIN_DRIVE_STRENGTH) & 0x3;
> +
> +	switch (param) {
> +	case PIN_CONFIG_BIAS_PULL_UP:
> +		if (pull != RZN1_L1_PIN_PULL_UP)
> +			return -EINVAL;
> +		break;
> +	case PIN_CONFIG_BIAS_PULL_DOWN:
> +		if (pull != RZN1_L1_PIN_PULL_DOWN)
> +			return -EINVAL;
> +		break;
> +	case PIN_CONFIG_BIAS_DISABLE:
> +		if (pull != RZN1_L1_PIN_PULL_NONE)
> +			return -EINVAL;
> +		break;
> +	case PIN_CONFIG_DRIVE_STRENGTH:
> +		arg = reg_drive[drive];
> +		break;
> +	case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
> +		l2 = readl(&ipctl->lev2->conf[pin]);
> +		if (l1mux == RZN1_L1_FUNCTION_L2) {
> +			if (l2 != 0)
> +				return -EINVAL;
> +		} else if (l1mux != RZN1_FUNC_HIGHZ) {
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -ENOTSUPP;
> +	}
> +
> +	*config = pinconf_to_config_packed(param, arg);
> +
> +	return 0;
> +}
> +
> +static int rzn1_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> +			    unsigned long *configs, unsigned int num_configs)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +	enum pin_config_param param;
> +	int i;
> +	u32 arg;
> +	u32 l1, l1_cache;
> +	u32 drv;
> +
> +	if (pin >= ARRAY_SIZE(ipctl->lev1->conf))
> +		return -EINVAL;
> +
> +	l1 = readl(&ipctl->lev1->conf[pin]);
> +	l1_cache = l1;
> +
> +	for (i = 0; i < num_configs; i++) {
> +		param = pinconf_to_config_param(configs[i]);
> +		arg = pinconf_to_config_argument(configs[i]);
> +
> +		switch (param) {
> +		case PIN_CONFIG_BIAS_PULL_UP:
> +			dev_dbg(ipctl->dev, "set pin %d pull up\n", pin);
> +			l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
> +			l1 |= (RZN1_L1_PIN_PULL_UP << RZN1_L1_PIN_PULL);
> +			break;
> +		case PIN_CONFIG_BIAS_PULL_DOWN:
> +			dev_dbg(ipctl->dev, "set pin %d pull down\n", pin);
> +			l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
> +			l1 |= (RZN1_L1_PIN_PULL_DOWN << RZN1_L1_PIN_PULL);
> +			break;
> +		case PIN_CONFIG_BIAS_DISABLE:
> +			dev_dbg(ipctl->dev, "set pin %d bias off\n", pin);
> +			l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
> +			l1 |= (RZN1_L1_PIN_PULL_NONE << RZN1_L1_PIN_PULL);
> +			break;
> +		case PIN_CONFIG_DRIVE_STRENGTH:
> +			dev_dbg(ipctl->dev, "set pin %d drv %umA\n", pin, arg);
> +			drv = 0;

No need to initialize drv here

> +			switch (arg) {
> +			case 4:
> +				drv = RZN1_L1_PIN_DRIVE_STRENGTH_4MA;
> +				break;
> +			case 6:
> +				drv = RZN1_L1_PIN_DRIVE_STRENGTH_6MA;
> +				break;
> +			case 8:
> +				drv = RZN1_L1_PIN_DRIVE_STRENGTH_8MA;
> +				break;
> +			case 12:
> +				drv = RZN1_L1_PIN_DRIVE_STRENGTH_12MA;
> +				break;
> +			default:
> +				dev_err(ipctl->dev,
> +					"Drive strength %umA not supported\n",
> +					arg);
> +
> +				return -EINVAL;
> +			}
> +
> +			l1 &= ~(0x3 << RZN1_L1_PIN_DRIVE_STRENGTH);
> +			l1 |= (drv << RZN1_L1_PIN_DRIVE_STRENGTH);
> +			break;
> +
> +		case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
> +			dev_dbg(ipctl->dev, "set pin %d High-Z\n", pin);
> +			l1 &= ~RZN1_L1_FUNC_MASK;
> +			l1 |= RZN1_FUNC_HIGHZ;
> +			break;
> +		default:
> +			return -ENOTSUPP;
> +		}
> +	}
> +
> +	if (l1 != l1_cache) {
> +		rzn1_hw_set_lock(ipctl, LOCK_LEVEL1, LOCK_LEVEL1);
> +		writel(l1, &ipctl->lev1->conf[pin]);
> +		rzn1_hw_set_lock(ipctl, LOCK_LEVEL1, 0);
> +	}
> +
> +	return 0;
> +}
> +
> +static int rzn1_pinconf_group_get(struct pinctrl_dev *pctldev,
> +				  unsigned int selector,
> +				  unsigned long *config)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +	struct rzn1_pin_group *grp = &ipctl->groups[selector];
> +	unsigned long old = 0;
> +	int i;
> +
> +	dev_dbg(ipctl->dev, "group get %s selector:%d\n", grp->name, selector);
> +
> +	for (i = 0; i < grp->npins; i++) {
> +		if (rzn1_pinconf_get(pctldev, grp->pins[i], config))
> +			return -ENOTSUPP;
> +
> +		/* configs do not match between two pins */
> +		if (i && (old != *config))
> +			return -ENOTSUPP;
> +
> +		old = *config;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rzn1_pinconf_group_set(struct pinctrl_dev *pctldev,
> +				  unsigned int selector,
> +				  unsigned long *configs,
> +				  unsigned int num_configs)
> +{
> +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +	struct rzn1_pin_group *grp = &ipctl->groups[selector];
> +	int ret, i;
> +
> +	dev_dbg(ipctl->dev, "group set %s selector:%d configs:%p/%d\n",
> +		grp->name, selector, configs, num_configs);
> +
> +	for (i = 0; i < grp->npins; i++) {
> +		unsigned int pin = grp->pins[i];
> +
> +		ret = rzn1_pinconf_set(pctldev, pin, configs, num_configs);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pinconf_ops rzn1_pinconf_ops = {
> +	.is_generic = true,
> +	.pin_config_get = rzn1_pinconf_get,
> +	.pin_config_set = rzn1_pinconf_set,
> +	.pin_config_group_get = rzn1_pinconf_group_get,
> +	.pin_config_group_set = rzn1_pinconf_group_set,
> +	.pin_config_config_dbg_show = pinconf_generic_dump_config,
> +};
> +
> +static struct pinctrl_desc rzn1_pinctrl_desc = {
> +	.pctlops = &rzn1_pctrl_ops,
> +	.pmxops = &rzn1_pmx_ops,
> +	.confops = &rzn1_pinconf_ops,
> +	.owner = THIS_MODULE,
> +};
> +
> +static int rzn1_pinctrl_parse_groups(struct device_node *np,
> +				     struct rzn1_pin_group *grp,
> +				     struct rzn1_pinctrl *ipctl)
> +{
> +	int size;
> +	const __be32 *list;
> +	unsigned int i;
> +
> +	dev_dbg(ipctl->dev, "%s: %s\n", __func__, np->name);
> +
> +	/* Initialise group */
> +	grp->name = np->name;
> +
> +	/*
> +	 * The binding format is
> +	 *	pinmux = <PIN_FUNC_ID CONFIG ...>,
> +	 * do sanity check and calculate pins number
> +	 */
> +	list = of_get_property(np, RZN1_PINS_PROP, &size);
> +	if (!list) {
> +		dev_err(ipctl->dev,
> +			"no " RZN1_PINS_PROP " property in node %pOF\n", np);
> +
> +		return -EINVAL;
> +	}
> +
> +	if (!size) {
> +		dev_err(ipctl->dev, "Invalid " RZN1_PINS_PROP " in node %pOF\n",
> +			np);
> +
> +		return -EINVAL;
> +	}
> +
> +	grp->npins = size / sizeof(list[0]);
> +	grp->pin_ids = devm_kmalloc_array(ipctl->dev,
> +					  grp->npins, sizeof(grp->pin_ids[0]),
> +					  GFP_KERNEL);
> +	grp->pins = devm_kmalloc_array(ipctl->dev,
> +				       grp->npins, sizeof(grp->pins[0]),
> +				       GFP_KERNEL);
> +	if (!grp->pin_ids || !grp->pins)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < grp->npins; i++) {
> +		u32 pin_id = be32_to_cpu(*list++);
> +
> +		grp->pins[i] = pin_id & 0xff;
> +		grp->pin_ids[i] = (pin_id >> 8) & 0x7f;
> +	}
> +
> +	return grp->npins;
> +}
> +
> +static int rzn1_pinctrl_count_function_groups(struct device_node *np)
> +{
> +	struct device_node *child;
> +	int count = 0;
> +
> +	if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0)
> +		count++;
> +
> +	for_each_child_of_node(np, child) {
> +		if (of_property_count_u32_elems(child, RZN1_PINS_PROP) > 0)
> +			count++;
> +	}
> +
> +	return count;
> +}
> +
> +static int rzn1_pinctrl_parse_functions(struct device_node *np,
> +					struct rzn1_pinctrl *ipctl, u32 index)
> +{
> +	struct device_node *child;
> +	struct rzn1_pmx_func *func;
> +	struct rzn1_pin_group *grp;
> +	u32 i = 0;
> +	int ret;
> +
> +	func = &ipctl->functions[index];
> +
> +	/* Initialise function */
> +	func->name = np->name;
> +	func->num_groups = rzn1_pinctrl_count_function_groups(np);
> +	if (func->num_groups == 0) {
> +		dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
> +		return -EINVAL;
> +	}
> +	dev_dbg(ipctl->dev, "function %s has %d groups\n",
> +		np->name, func->num_groups);
> +
> +	func->groups = devm_kmalloc_array(ipctl->dev,
> +					  func->num_groups, sizeof(char *),
> +					  GFP_KERNEL);
> +	if (!func->groups)
> +		return -ENOMEM;
> +
> +	if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0) {
> +		func->groups[i] = np->name;
> +		grp = &ipctl->groups[ipctl->ngroups];
> +		grp->func = func->name;
> +		ret = rzn1_pinctrl_parse_groups(np, grp, ipctl);
> +		if (ret < 0)
> +			return ret;
> +		i++;
> +		ipctl->ngroups++;
> +	}
> +
> +	for_each_child_of_node(np, child) {
> +		func->groups[i] = child->name;
> +		grp = &ipctl->groups[ipctl->ngroups];
> +		grp->func = func->name;
> +		ret = rzn1_pinctrl_parse_groups(child, grp, ipctl);
> +		if (ret < 0)
> +			return ret;
> +		i++;
> +		ipctl->ngroups++;
> +	}
> +
> +	dev_dbg(ipctl->dev, "function %s parsed %d/%d groups\n",
> +		np->name, i, func->num_groups);
> +
> +	return 0;
> +}
> +
> +static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
> +				 struct rzn1_pinctrl *ipctl)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *child;
> +	unsigned int maxgroups = 0;
> +	u32 nfuncs = 0;
> +	u32 i = 0;
> +	int ret;
> +
> +	nfuncs = of_get_child_count(np);
> +	if (nfuncs <= 0) {
> +		dev_warn(&pdev->dev, "no functions defined\n");

I don't think that's something to warn about. It's not mandatory to
have functions defined for a pinctrl to exist.

> +
> +		return 0;
> +	}
> +
> +	ipctl->nfunctions = nfuncs;
> +	ipctl->functions = devm_kmalloc_array(&pdev->dev, nfuncs,
> +					      sizeof(*ipctl->functions),
> +					      GFP_KERNEL);
> +	if (!ipctl->functions)
> +		return -ENOMEM;
> +
> +	ipctl->ngroups = 0;
> +	for_each_child_of_node(np, child)
> +		maxgroups += rzn1_pinctrl_count_function_groups(child);
> +
> +	ipctl->groups = devm_kmalloc_array(&pdev->dev,
> +					   maxgroups,
> +					   sizeof(*ipctl->groups),
> +					   GFP_KERNEL);
> +	if (!ipctl->groups)
> +		return -ENOMEM;
> +
> +	for_each_child_of_node(np, child) {
> +		ret = rzn1_pinctrl_parse_functions(child, ipctl, i++);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rzn1_pinctrl_probe(struct platform_device *pdev)
> +{
> +	struct rzn1_pinctrl *ipctl;
> +	struct resource *res;
> +	int ret;
> +
> +	/* Create state holders etc for this driver */
> +	ipctl = devm_kzalloc(&pdev->dev, sizeof(*ipctl), GFP_KERNEL);
> +	if (!ipctl)
> +		return -ENOMEM;
> +
> +	ipctl->mdio_func[0] = -1;
> +	ipctl->mdio_func[1] = -1;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	ipctl->lev1_protect_phys = (u32)res->start + 0x400;
> +	ipctl->lev1 = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ipctl->lev1))
> +		return PTR_ERR(ipctl->lev1);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	ipctl->lev2_protect_phys = (u32)res->start + 0x400;
> +	ipctl->lev2 = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ipctl->lev2))
> +		return PTR_ERR(ipctl->lev2);
> +
> +	ipctl->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(ipctl->clk))
> +		return PTR_ERR(ipctl->clk);
> +	ret = clk_prepare_enable(ipctl->clk);
> +	if (ret)
> +		return ret;
> +
> +	ipctl->dev = &pdev->dev;
> +	rzn1_pinctrl_desc.name = dev_name(&pdev->dev);
> +	rzn1_pinctrl_desc.pins = rzn1_pins;
> +	rzn1_pinctrl_desc.npins = ARRAY_SIZE(rzn1_pins);
> +
> +	ret = rzn1_pinctrl_probe_dt(pdev, ipctl);
> +	if (ret) {
> +		dev_err(&pdev->dev, "fail to probe dt properties\n");
> +		goto err_clk;
> +	}
> +
> +	platform_set_drvdata(pdev, ipctl);
> +	ipctl->pctl = pinctrl_register(&rzn1_pinctrl_desc, &pdev->dev, ipctl);

The pinctrl_register documentation suggests to use
pinctrl_register_and_init() to avoid races. Be aware you need to
explicitly call pinctrl_enable() after.

This are mostly minors and all my comments on previous versions are now
clarified, so once you have addressed them please add my:
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks
   j

> +	if (!ipctl->pctl) {
> +		dev_err(&pdev->dev, "could not register rzn1 pinctrl driver\n");
> +		ret = -EINVAL;
> +		goto err_clk;
> +	}
> +
> +	dev_info(&pdev->dev, "probed\n");
> +
> +	return 0;
> +
> +err_clk:
> +	clk_disable_unprepare(ipctl->clk);
> +
> +	return ret;
> +}
> +
> +static int rzn1_pinctrl_remove(struct platform_device *pdev)
> +{
> +	struct rzn1_pinctrl *ipctl = platform_get_drvdata(pdev);
> +
> +	clk_disable_unprepare(ipctl->clk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rzn1_pinctrl_match[] = {
> +	{ .compatible = "renesas,rzn1-pinctrl", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, rzn1_pinctrl_match);
> +
> +static struct platform_driver rzn1_pinctrl_driver = {
> +	.probe	= rzn1_pinctrl_probe,
> +	.remove = rzn1_pinctrl_remove,
> +	.driver	= {
> +		.name		= "rzn1-pinctrl",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= rzn1_pinctrl_match,
> +	},
> +};
> +
> +static int __init _pinctrl_drv_register(void)
> +{
> +	return platform_driver_register(&rzn1_pinctrl_driver);
> +}
> +subsys_initcall(_pinctrl_drv_register);
> +
> +MODULE_AUTHOR("Phil Edworthy <phil.edworthy@renesas.com>");
> +MODULE_DESCRIPTION("Renesas RZ/N1 pinctrl driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver
  2018-09-19 14:23 ` [PATCH v4 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver Phil Edworthy
  2018-09-21 13:33   ` jacopo mondi
@ 2018-09-24 11:58   ` Geert Uytterhoeven
  2018-09-24 12:32     ` Phil Edworthy
  1 sibling, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-09-24 11:58 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Laurent Pinchart, Jacopo Mondi, Linus Walleij, Simon Horman,
	open list:GPIO SUBSYSTEM, Linux-Renesas,
	Linux Kernel Mailing List

Hi Phil,

On Wed, Sep 19, 2018 at 4:24 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> This provides a pinctrl driver for the Renesas RZ/N1 device family.
>
> Based on a patch originally written by Michel Pollet at Renesas.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-rzn1.c

> +/*
> + * Structure detailing the HW registers on the RZ/N1 devices.
> + * Both the Level 1 mux registers and Level 2 mux registers have the same
> + * structure. The only difference is that Level 2 has additional MDIO registers
> + * at the end.
> + */
> +struct rzn1_pinctrl_regs {
> +       union {
> +               u32     conf[170];
> +               u8      pad0[0x400];

This looks a bit confusing, and isn't really padding, as you use a union.
What about getting rid of the union, and making it real padding?

        u32 conf[170];
        u32 pad0[86];

> +       };
> +       u32     status_protect; /* 0x400 */
> +       /* MDIO mux registers, level2 only */
> +       u32     l2_mdio[2];
> +};

BTW, while using a struct instead of register offset definitions has its
merits, it also has its drawbacks, like the need for the "0x400" comment.
You don't have to change it, though.

> +static const struct rzn1_pin_group *rzn1_pinctrl_find_group_by_name(
> +       const struct rzn1_pinctrl *ipctl, const char *name)
> +{
> +       const struct rzn1_pin_group *grp = NULL;
> +       int i;

unsigned int i;
(rzn1_pinctrl.ngroups is unsigned int)

> +
> +       for (i = 0; i < ipctl->ngroups; i++) {
> +               if (!strcmp(ipctl->groups[i].name, name)) {
> +                       grp = &ipctl->groups[i];
> +                       break;
> +               }
> +       }
> +
> +       return grp;
> +}

> +static int rzn1_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> +                           unsigned long *configs, unsigned int num_configs)
> +{
> +       struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +       enum pin_config_param param;
> +       int i;

unsigned int i;

> +       u32 arg;
> +       u32 l1, l1_cache;
> +       u32 drv;
> +
> +       if (pin >= ARRAY_SIZE(ipctl->lev1->conf))
> +               return -EINVAL;
> +
> +       l1 = readl(&ipctl->lev1->conf[pin]);
> +       l1_cache = l1;
> +
> +       for (i = 0; i < num_configs; i++) {

> +static int rzn1_pinconf_group_get(struct pinctrl_dev *pctldev,
> +                                 unsigned int selector,
> +                                 unsigned long *config)
> +{
> +       struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +       struct rzn1_pin_group *grp = &ipctl->groups[selector];
> +       unsigned long old = 0;
> +       int i;

unsigned int i;

> +
> +       dev_dbg(ipctl->dev, "group get %s selector:%d\n", grp->name, selector);

%u to format unsigned int.

> +
> +       for (i = 0; i < grp->npins; i++) {

> +static int rzn1_pinconf_group_set(struct pinctrl_dev *pctldev,
> +                                 unsigned int selector,
> +                                 unsigned long *configs,
> +                                 unsigned int num_configs)
> +{
> +       struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> +       struct rzn1_pin_group *grp = &ipctl->groups[selector];
> +       int ret, i;

unsigned int i;

> +
> +       dev_dbg(ipctl->dev, "group set %s selector:%d configs:%p/%d\n",

%u

> +               grp->name, selector, configs, num_configs);
> +
> +       for (i = 0; i < grp->npins; i++) {
> +               unsigned int pin = grp->pins[i];
> +
> +               ret = rzn1_pinconf_set(pctldev, pin, configs, num_configs);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}


> +static int rzn1_pinctrl_parse_functions(struct device_node *np,
> +                                       struct rzn1_pinctrl *ipctl, u32 index)

Why u32 instead of plain unsigned int?

> +{
> +       struct device_node *child;
> +       struct rzn1_pmx_func *func;
> +       struct rzn1_pin_group *grp;
> +       u32 i = 0;

Why not plain unsigned int?

> +       int ret;
> +
> +       func = &ipctl->functions[index];
> +
> +       /* Initialise function */
> +       func->name = np->name;
> +       func->num_groups = rzn1_pinctrl_count_function_groups(np);
> +       if (func->num_groups == 0) {
> +               dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
> +               return -EINVAL;
> +       }
> +       dev_dbg(ipctl->dev, "function %s has %d groups\n",
> +               np->name, func->num_groups);
> +
> +       func->groups = devm_kmalloc_array(ipctl->dev,
> +                                         func->num_groups, sizeof(char *),
> +                                         GFP_KERNEL);
> +       if (!func->groups)
> +               return -ENOMEM;
> +
> +       if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0) {
> +               func->groups[i] = np->name;
> +               grp = &ipctl->groups[ipctl->ngroups];
> +               grp->func = func->name;
> +               ret = rzn1_pinctrl_parse_groups(np, grp, ipctl);
> +               if (ret < 0)
> +                       return ret;
> +               i++;
> +               ipctl->ngroups++;
> +       }
> +
> +       for_each_child_of_node(np, child) {
> +               func->groups[i] = child->name;
> +               grp = &ipctl->groups[ipctl->ngroups];
> +               grp->func = func->name;
> +               ret = rzn1_pinctrl_parse_groups(child, grp, ipctl);
> +               if (ret < 0)
> +                       return ret;
> +               i++;
> +               ipctl->ngroups++;
> +       }
> +
> +       dev_dbg(ipctl->dev, "function %s parsed %d/%d groups\n",

%u/%u

> +               np->name, i, func->num_groups);
> +
> +       return 0;
> +}
> +
> +static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
> +                                struct rzn1_pinctrl *ipctl)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct device_node *child;
> +       unsigned int maxgroups = 0;
> +       u32 nfuncs = 0;
> +       u32 i = 0;

Why not plain unsigned int, for both?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v4 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver
  2018-09-24 11:58   ` Geert Uytterhoeven
@ 2018-09-24 12:32     ` Phil Edworthy
  0 siblings, 0 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-09-24 12:32 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, Jacopo Mondi, Linus Walleij, Simon Horman,
	open list:GPIO SUBSYSTEM, Linux-Renesas,
	Linux Kernel Mailing List

Hi Geert,

On 24 September 2018 12:59 Geert Uytterhoeven wrote:
> On Wed, Sep 19, 2018 at 4:24 PM Phil Edworthy wrote:
> > This provides a pinctrl driver for the Renesas RZ/N1 device family.
> >
> > Based on a patch originally written by Michel Pollet at Renesas.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/drivers/pinctrl/pinctrl-rzn1.c
> 
> > +/*
> > + * Structure detailing the HW registers on the RZ/N1 devices.
> > + * Both the Level 1 mux registers and Level 2 mux registers have the
> > +same
> > + * structure. The only difference is that Level 2 has additional MDIO
> > +registers
> > + * at the end.
> > + */
> > +struct rzn1_pinctrl_regs {
> > +       union {
> > +               u32     conf[170];
> > +               u8      pad0[0x400];
> 
> This looks a bit confusing, and isn't really padding, as you use a union.
> What about getting rid of the union, and making it real padding?
> 
>         u32 conf[170];
>         u32 pad0[86];
> 
> > +       };
> > +       u32     status_protect; /* 0x400 */
> > +       /* MDIO mux registers, level2 only */
> > +       u32     l2_mdio[2];
> > +};
> 
> BTW, while using a struct instead of register offset definitions has its merits,
> it also has its drawbacks, like the need for the "0x400" comment.
> You don't have to change it, though.
> 
> > +static const struct rzn1_pin_group *rzn1_pinctrl_find_group_by_name(
> > +       const struct rzn1_pinctrl *ipctl, const char *name) {
> > +       const struct rzn1_pin_group *grp = NULL;
> > +       int i;
> 
> unsigned int i;
> (rzn1_pinctrl.ngroups is unsigned int)
> 
> > +
> > +       for (i = 0; i < ipctl->ngroups; i++) {
> > +               if (!strcmp(ipctl->groups[i].name, name)) {
> > +                       grp = &ipctl->groups[i];
> > +                       break;
> > +               }
> > +       }
> > +
> > +       return grp;
> > +}
> 
> > +static int rzn1_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> > +                           unsigned long *configs, unsigned int
> > +num_configs) {
> > +       struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +       enum pin_config_param param;
> > +       int i;
> 
> unsigned int i;
> 
> > +       u32 arg;
> > +       u32 l1, l1_cache;
> > +       u32 drv;
> > +
> > +       if (pin >= ARRAY_SIZE(ipctl->lev1->conf))
> > +               return -EINVAL;
> > +
> > +       l1 = readl(&ipctl->lev1->conf[pin]);
> > +       l1_cache = l1;
> > +
> > +       for (i = 0; i < num_configs; i++) {
> 
> > +static int rzn1_pinconf_group_get(struct pinctrl_dev *pctldev,
> > +                                 unsigned int selector,
> > +                                 unsigned long *config) {
> > +       struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +       struct rzn1_pin_group *grp = &ipctl->groups[selector];
> > +       unsigned long old = 0;
> > +       int i;
> 
> unsigned int i;
> 
> > +
> > +       dev_dbg(ipctl->dev, "group get %s selector:%d\n", grp->name,
> > + selector);
> 
> %u to format unsigned int.
> 
> > +
> > +       for (i = 0; i < grp->npins; i++) {
> 
> > +static int rzn1_pinconf_group_set(struct pinctrl_dev *pctldev,
> > +                                 unsigned int selector,
> > +                                 unsigned long *configs,
> > +                                 unsigned int num_configs) {
> > +       struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +       struct rzn1_pin_group *grp = &ipctl->groups[selector];
> > +       int ret, i;
> 
> unsigned int i;
> 
> > +
> > +       dev_dbg(ipctl->dev, "group set %s selector:%d
> > + configs:%p/%d\n",
> 
> %u
> 
> > +               grp->name, selector, configs, num_configs);
> > +
> > +       for (i = 0; i < grp->npins; i++) {
> > +               unsigned int pin = grp->pins[i];
> > +
> > +               ret = rzn1_pinconf_set(pctldev, pin, configs, num_configs);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> > +       return 0;
> > +}
> 
> 
> > +static int rzn1_pinctrl_parse_functions(struct device_node *np,
> > +                                       struct rzn1_pinctrl *ipctl,
> > +u32 index)
> 
> Why u32 instead of plain unsigned int?
> 
> > +{
> > +       struct device_node *child;
> > +       struct rzn1_pmx_func *func;
> > +       struct rzn1_pin_group *grp;
> > +       u32 i = 0;
> 
> Why not plain unsigned int?
> 
> > +       int ret;
> > +
> > +       func = &ipctl->functions[index];
> > +
> > +       /* Initialise function */
> > +       func->name = np->name;
> > +       func->num_groups = rzn1_pinctrl_count_function_groups(np);
> > +       if (func->num_groups == 0) {
> > +               dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
> > +               return -EINVAL;
> > +       }
> > +       dev_dbg(ipctl->dev, "function %s has %d groups\n",
> > +               np->name, func->num_groups);
> > +
> > +       func->groups = devm_kmalloc_array(ipctl->dev,
> > +                                         func->num_groups, sizeof(char *),
> > +                                         GFP_KERNEL);
> > +       if (!func->groups)
> > +               return -ENOMEM;
> > +
> > +       if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0) {
> > +               func->groups[i] = np->name;
> > +               grp = &ipctl->groups[ipctl->ngroups];
> > +               grp->func = func->name;
> > +               ret = rzn1_pinctrl_parse_groups(np, grp, ipctl);
> > +               if (ret < 0)
> > +                       return ret;
> > +               i++;
> > +               ipctl->ngroups++;
> > +       }
> > +
> > +       for_each_child_of_node(np, child) {
> > +               func->groups[i] = child->name;
> > +               grp = &ipctl->groups[ipctl->ngroups];
> > +               grp->func = func->name;
> > +               ret = rzn1_pinctrl_parse_groups(child, grp, ipctl);
> > +               if (ret < 0)
> > +                       return ret;
> > +               i++;
> > +               ipctl->ngroups++;
> > +       }
> > +
> > +       dev_dbg(ipctl->dev, "function %s parsed %d/%d groups\n",
> 
> %u/%u
> 
> > +               np->name, i, func->num_groups);
> > +
> > +       return 0;
> > +}
> > +
> > +static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
> > +                                struct rzn1_pinctrl *ipctl) {
> > +       struct device_node *np = pdev->dev.of_node;
> > +       struct device_node *child;
> > +       unsigned int maxgroups = 0;
> > +       u32 nfuncs = 0;
> > +       u32 i = 0;
> 
> Why not plain unsigned int, for both?

Thanks for your review and comments, you are right in all of them.
I'll fix them and re-post.

Thanks
Phil

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation
  2018-09-19 14:23 ` [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy
  2018-09-21 12:36   ` jacopo mondi
@ 2018-09-26 14:20   ` Rob Herring
  2018-09-26 14:40       ` Geert Uytterhoeven
  1 sibling, 1 reply; 11+ messages in thread
From: Rob Herring @ 2018-09-26 14:20 UTC (permalink / raw)
  To: phil.edworthy
  Cc: Geert Uytterhoeven, Laurent Pinchart, Rob Herring, Mark Rutland,
	jacopo, Linus Walleij, Simon Horman, linux-gpio,
	linux-renesas-soc, Linux Kernel Mailing List

DT bindings should be sent to DT list.

On Wed, Sep 19, 2018 at 9:24 AM Phil Edworthy <phil.edworthy@renesas.com> wrote:
>
> The Renesas RZ/N1 device family PINCTRL node description.
>
> Based on a patch originally written by Michel Pollet at Renesas.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> v4:
>  - Add alternative way to use the pinmux prop.
>  - Remove mention of gpios.
>
> v3:
>  - Use standard bindings
>  - Change the way the functions are defined so it is easy to check
>    against the hardware numbering.
>  - Add functions for the MDIO source peripheral instead of using
>    virtual pins.
>
> v2:
>  - Change filename to generic rzn1, instead of device specific.
>  - Add "renesas,rzn1-pinctrl" compatible fallback string.
>  - Example register size corrected.
>  - Typos fixed.
>  - Changes suggested by Jacopo Mondi.
>  - rzn1-pinctrl.h squashed into this as requested by Rob Herring.
> ---
>  .../bindings/pinctrl/renesas,rzn1-pinctrl.txt | 153 ++++++++++++++++++
>  include/dt-bindings/pinctrl/rzn1-pinctrl.h    | 141 ++++++++++++++++
>  2 files changed, 294 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
>  create mode 100644 include/dt-bindings/pinctrl/rzn1-pinctrl.h
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> new file mode 100644
> index 000000000000..323ef5c6c1c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> @@ -0,0 +1,153 @@
> +Renesas RZ/N1 SoC Pinctrl node description.
> +
> +Pin controller node
> +-------------------
> +Required properties:
> +- compatible: SoC-specific compatible string "renesas,<soc-specific>-pinctrl"
> +  followed by "renesas,rzn1-pinctrl" as fallback. The SoC-specific compatible
> +  strings must be one of:
> +       "renesas,r9a06g032-pinctrl" for RZ/N1D
> +       "renesas,r9a06g033-pinctrl" for RZ/N1S
> +- reg: Address base and length of the memory area where the pin controller
> +  hardware is mapped to.
> +- clocks: phandle for the clock, see the description of clock-names below.
> +- clock-names: Contains the name of the clock:
> +    "bus", the bus clock, sometimes described as pclk, for register accesses.
> +
> +Example:
> +       pinctrl: pin-controller@40067000 {
> +           compatible = "renesas,r9a06g032-pinctrl", "renesas,rzn1-pinctrl";
> +           reg = <0x40067000 0x1000>, <0x51000000 0x480>;
> +           clocks = <&sysctrl R9A06G032_HCLK_PINCONFIG>;
> +           clock-names = "bus";
> +       };
> +
> +Sub-nodes
> +---------
> +
> +The child nodes of the pin controller node describe a pin multiplexing
> +function.
> +
> +- Pin multiplexing sub-nodes:
> +  A pin multiplexing sub-node describes how to configure a set of
> +  (or a single) pin in some desired alternate function mode.
> +  A single sub-node may define several pin configurations.
> +  Please refer to pinctrl-bindings.txt to get to know more on generic
> +  pin properties usage.
> +
> +  The allowed generic formats for a pin multiplexing sub-node are the
> +  following ones:
> +
> +  node-1 {
> +      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +      GENERIC_PINCONFIG;
> +  };
> +
> +  node-2 {
> +      sub-node-1 {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +
> +      sub-node-2 {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +
> +      ...
> +
> +      sub-node-n {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +  };
> +
> +  node-3 {
> +      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +      GENERIC_PINCONFIG;
> +
> +      sub-node-1 {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +
> +      ...
> +
> +      sub-node-n {
> +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> +          GENERIC_PINCONFIG;
> +      };
> +  };
> +
> +  Use the latter two formats when pins part of the same logical group need to
> +  have different generic pin configuration flags applied. Note that the generic
> +  pinconfig in node-3 does not apply to the sub-nodes.
> +
> +  Client sub-nodes shall refer to pin multiplexing sub-nodes using the phandle
> +  of the most external one.
> +
> +  Eg.
> +
> +  client-1 {
> +      ...
> +      pinctrl-0 = <&node-1>;
> +      ...
> +  };
> +
> +  client-2 {
> +      ...
> +      pinctrl-0 = <&node-2>;
> +      ...
> +  };
> +
> +  Required properties:
> +    - pinmux:
> +      integer array representing pin number and pin multiplexing configuration.
> +      When a pin has to be configured in alternate function mode, use this
> +      property to identify the pin by its global index, and provide its
> +      alternate function configuration number along with it.
> +      When multiple pins are required to be configured as part of the same
> +      alternate function they shall be specified as members of the same
> +      argument list of a single "pinmux" property.
> +      Integers values in the "pinmux" argument list are assembled as:
> +      (PIN | MUX_FUNC << 8)
> +      where PIN directly corresponds to the pl_gpio pin number and MUX_FUNC is
> +      one of the alternate function identifiers defined in:
> +      <include/dt-bindings/pinctrl/rzn1-pinctrl.h>
> +      These identifiers collapse the IO Multiplex Configuration Level 1 and
> +      Level 2 numbers that are detailed in the hardware reference manual into a
> +      single number. The identifiers for Level 2 are simply offset by 10.
> +      Additional identifiers are provided to specify the MDIO source peripheral.
> +
> +  Optional generic properties:
> +    - bias-disable             - disable any pin bias
> +    - bias-pull-up             - pull up the pin with 50 KOhm
> +    - bias-pull-down           - pull down the pin with 50 KOhm
> +    - bias-high-impedance      - high impedance mode
> +    - drive-strength           - sink or source at most 4, 6, 8 or 12 mA
> +
> +  Example:
> +  A serial communication interface with a TX output pin and an RX input pin.
> +
> +  &pinctrl {
> +       pins_uart0: pins_uart0 {
> +               pinmux = <
> +                       RZN1_MUX(103, UART0_I)  /* UART0_TXD */
> +                       RZN1_MUX(104, UART0_I)  /* UART0_RXD */
> +               >;
> +       };
> +  };
> +
> +  Example 2:
> +  Here we set the pull up on the RXD pin of the UART.
> +
> +  &pinctrl {
> +       pins_uart0: pins_uart0 {
> +               pinmux = <RZN1_MUX(103, UART0_I)>;      /* UART0_TXD */
> +
> +               pins_uart6_rx {
> +                       pinmux = <RZN1_MUX(104, UART0_I)>;      /* UART0_RXD */
> +                       bias-pull-up;
> +               };
> +       };
> +  };
> diff --git a/include/dt-bindings/pinctrl/rzn1-pinctrl.h b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
> new file mode 100644
> index 000000000000..40369ee36e6a
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
> @@ -0,0 +1,141 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Defines macros and constants for Renesas RZ/N1 pin controller pin
> + * muxing functions.
> + */
> +#ifndef __DT_BINDINGS_RZN1_PINCTRL_H
> +#define __DT_BINDINGS_RZN1_PINCTRL_H
> +
> +#define RZN1_MUX(_gpio, _func) \
> +       (((RZN1_FUNC_##_func) << 8) | (_gpio))

I'm not a fan of token pasting and it also goes against kernel style.
If every other Renesas platform is doing this, then fine. Otherwise,
you can express it in pretty much the same (source) space:

pinmux = <RZN1_MUX_UART0_I 104>;

Yes, this is 2 cells instead of 1, but if you care about space, you
can use 8 or 16 bit size.

> +/*
> + * Given the different levels of muxing on the SoC, it was decided to
> + * 'linearize' them into one numerical space. So mux level 1, 2 and the MDIO
> + * muxes are all represented by one single value.
> + *
> + * You can derive the hardware value pretty easily too, as
> + * 0...9   are Level 1
> + * 10...71 are Level 2. The Level 2 mux will be set to this
> + *         value - RZN1_FUNC_L2_OFFSET, and the Level 1 mux will be
> + *         set accordingly.
> + * 72...103 are for the 2 MDIO muxes.

Whatever works best for the driver, but wouldn't aligning the levels
to power of 2 make decoding a bit easier? 0-15 for level 1, 16-127 for
2, 128+ for MDIO.

Rob

> + */
> +#define RZN1_FUNC_HIGHZ                                0
> +#define RZN1_FUNC_0L                           1
> +#define RZN1_FUNC_CLK_ETH_MII_RGMII_RMII       2
> +#define RZN1_FUNC_CLK_ETH_NAND                 3
> +#define RZN1_FUNC_QSPI                         4
> +#define RZN1_FUNC_SDIO                         5
> +#define RZN1_FUNC_LCD                          6
> +#define RZN1_FUNC_LCD_E                                7
> +#define RZN1_FUNC_MSEBIM                       8
> +#define RZN1_FUNC_MSEBIS                       9
> +#define RZN1_FUNC_L2_OFFSET                    10      /* I'm Special */
> +
> +#define RZN1_FUNC_HIGHZ1                       (RZN1_FUNC_L2_OFFSET + 0)
> +#define RZN1_FUNC_ETHERCAT                     (RZN1_FUNC_L2_OFFSET + 1)
> +#define RZN1_FUNC_SERCOS3                      (RZN1_FUNC_L2_OFFSET + 2)
> +#define RZN1_FUNC_SDIO_E                       (RZN1_FUNC_L2_OFFSET + 3)
> +#define RZN1_FUNC_ETH_MDIO                     (RZN1_FUNC_L2_OFFSET + 4)
> +#define RZN1_FUNC_ETH_MDIO_E1                  (RZN1_FUNC_L2_OFFSET + 5)
> +#define RZN1_FUNC_USB                          (RZN1_FUNC_L2_OFFSET + 6)
> +#define RZN1_FUNC_MSEBIM_E                     (RZN1_FUNC_L2_OFFSET + 7)
> +#define RZN1_FUNC_MSEBIS_E                     (RZN1_FUNC_L2_OFFSET + 8)
> +#define RZN1_FUNC_RSV                          (RZN1_FUNC_L2_OFFSET + 9)
> +#define RZN1_FUNC_RSV_E                                (RZN1_FUNC_L2_OFFSET + 10)
> +#define RZN1_FUNC_RSV_E1                       (RZN1_FUNC_L2_OFFSET + 11)
> +#define RZN1_FUNC_UART0_I                      (RZN1_FUNC_L2_OFFSET + 12)
> +#define RZN1_FUNC_UART0_I_E                    (RZN1_FUNC_L2_OFFSET + 13)
> +#define RZN1_FUNC_UART1_I                      (RZN1_FUNC_L2_OFFSET + 14)
> +#define RZN1_FUNC_UART1_I_E                    (RZN1_FUNC_L2_OFFSET + 15)
> +#define RZN1_FUNC_UART2_I                      (RZN1_FUNC_L2_OFFSET + 16)
> +#define RZN1_FUNC_UART2_I_E                    (RZN1_FUNC_L2_OFFSET + 17)
> +#define RZN1_FUNC_UART0                                (RZN1_FUNC_L2_OFFSET + 18)
> +#define RZN1_FUNC_UART0_E                      (RZN1_FUNC_L2_OFFSET + 19)
> +#define RZN1_FUNC_UART1                                (RZN1_FUNC_L2_OFFSET + 20)
> +#define RZN1_FUNC_UART1_E                      (RZN1_FUNC_L2_OFFSET + 21)
> +#define RZN1_FUNC_UART2                                (RZN1_FUNC_L2_OFFSET + 22)
> +#define RZN1_FUNC_UART2_E                      (RZN1_FUNC_L2_OFFSET + 23)
> +#define RZN1_FUNC_UART3                                (RZN1_FUNC_L2_OFFSET + 24)
> +#define RZN1_FUNC_UART3_E                      (RZN1_FUNC_L2_OFFSET + 25)
> +#define RZN1_FUNC_UART4                                (RZN1_FUNC_L2_OFFSET + 26)
> +#define RZN1_FUNC_UART4_E                      (RZN1_FUNC_L2_OFFSET + 27)
> +#define RZN1_FUNC_UART5                                (RZN1_FUNC_L2_OFFSET + 28)
> +#define RZN1_FUNC_UART5_E                      (RZN1_FUNC_L2_OFFSET + 29)
> +#define RZN1_FUNC_UART6                                (RZN1_FUNC_L2_OFFSET + 30)
> +#define RZN1_FUNC_UART6_E                      (RZN1_FUNC_L2_OFFSET + 31)
> +#define RZN1_FUNC_UART7                                (RZN1_FUNC_L2_OFFSET + 32)
> +#define RZN1_FUNC_UART7_E                      (RZN1_FUNC_L2_OFFSET + 33)
> +#define RZN1_FUNC_SPI0_M                       (RZN1_FUNC_L2_OFFSET + 34)
> +#define RZN1_FUNC_SPI0_M_E                     (RZN1_FUNC_L2_OFFSET + 35)
> +#define RZN1_FUNC_SPI1_M                       (RZN1_FUNC_L2_OFFSET + 36)
> +#define RZN1_FUNC_SPI1_M_E                     (RZN1_FUNC_L2_OFFSET + 37)
> +#define RZN1_FUNC_SPI2_M                       (RZN1_FUNC_L2_OFFSET + 38)
> +#define RZN1_FUNC_SPI2_M_E                     (RZN1_FUNC_L2_OFFSET + 39)
> +#define RZN1_FUNC_SPI3_M                       (RZN1_FUNC_L2_OFFSET + 40)
> +#define RZN1_FUNC_SPI3_M_E                     (RZN1_FUNC_L2_OFFSET + 41)
> +#define RZN1_FUNC_SPI4_S                       (RZN1_FUNC_L2_OFFSET + 42)
> +#define RZN1_FUNC_SPI4_S_E                     (RZN1_FUNC_L2_OFFSET + 43)
> +#define RZN1_FUNC_SPI5_S                       (RZN1_FUNC_L2_OFFSET + 44)
> +#define RZN1_FUNC_SPI5_S_E                     (RZN1_FUNC_L2_OFFSET + 45)
> +#define RZN1_FUNC_SGPIO0_M                     (RZN1_FUNC_L2_OFFSET + 46)
> +#define RZN1_FUNC_SGPIO1_M                     (RZN1_FUNC_L2_OFFSET + 47)
> +#define RZN1_FUNC_GPIO                         (RZN1_FUNC_L2_OFFSET + 48)
> +#define RZN1_FUNC_CAN                          (RZN1_FUNC_L2_OFFSET + 49)
> +#define RZN1_FUNC_I2C                          (RZN1_FUNC_L2_OFFSET + 50)
> +#define RZN1_FUNC_SAFE                         (RZN1_FUNC_L2_OFFSET + 51)
> +#define RZN1_FUNC_PTO_PWM                      (RZN1_FUNC_L2_OFFSET + 52)
> +#define RZN1_FUNC_PTO_PWM1                     (RZN1_FUNC_L2_OFFSET + 53)
> +#define RZN1_FUNC_PTO_PWM2                     (RZN1_FUNC_L2_OFFSET + 54)
> +#define RZN1_FUNC_PTO_PWM3                     (RZN1_FUNC_L2_OFFSET + 55)
> +#define RZN1_FUNC_PTO_PWM4                     (RZN1_FUNC_L2_OFFSET + 56)
> +#define RZN1_FUNC_DELTA_SIGMA                  (RZN1_FUNC_L2_OFFSET + 57)
> +#define RZN1_FUNC_SGPIO2_M                     (RZN1_FUNC_L2_OFFSET + 58)
> +#define RZN1_FUNC_SGPIO3_M                     (RZN1_FUNC_L2_OFFSET + 59)
> +#define RZN1_FUNC_SGPIO4_S                     (RZN1_FUNC_L2_OFFSET + 60)
> +#define RZN1_FUNC_MAC_MTIP_SWITCH              (RZN1_FUNC_L2_OFFSET + 61)
> +
> +#define RZN1_FUNC_MDIO_OFFSET                  (RZN1_FUNC_L2_OFFSET + 62)
> +
> +/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO function */
> +#define RZN1_FUNC_MDIO0_HIGHZ                  (RZN1_FUNC_MDIO_OFFSET + 0)
> +#define RZN1_FUNC_MDIO0_GMAC0                  (RZN1_FUNC_MDIO_OFFSET + 1)
> +#define RZN1_FUNC_MDIO0_GMAC1                  (RZN1_FUNC_MDIO_OFFSET + 2)
> +#define RZN1_FUNC_MDIO0_ECAT                   (RZN1_FUNC_MDIO_OFFSET + 3)
> +#define RZN1_FUNC_MDIO0_S3_MDIO0               (RZN1_FUNC_MDIO_OFFSET + 4)
> +#define RZN1_FUNC_MDIO0_S3_MDIO1               (RZN1_FUNC_MDIO_OFFSET + 5)
> +#define RZN1_FUNC_MDIO0_HWRTOS                 (RZN1_FUNC_MDIO_OFFSET + 6)
> +#define RZN1_FUNC_MDIO0_SWITCH                 (RZN1_FUNC_MDIO_OFFSET + 7)
> +/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
> +#define RZN1_FUNC_MDIO0_E1_HIGHZ               (RZN1_FUNC_MDIO_OFFSET + 8)
> +#define RZN1_FUNC_MDIO0_E1_GMAC0               (RZN1_FUNC_MDIO_OFFSET + 9)
> +#define RZN1_FUNC_MDIO0_E1_GMAC1               (RZN1_FUNC_MDIO_OFFSET + 10)
> +#define RZN1_FUNC_MDIO0_E1_ECAT                        (RZN1_FUNC_MDIO_OFFSET + 11)
> +#define RZN1_FUNC_MDIO0_E1_S3_MDIO0            (RZN1_FUNC_MDIO_OFFSET + 12)
> +#define RZN1_FUNC_MDIO0_E1_S3_MDIO1            (RZN1_FUNC_MDIO_OFFSET + 13)
> +#define RZN1_FUNC_MDIO0_E1_HWRTOS              (RZN1_FUNC_MDIO_OFFSET + 14)
> +#define RZN1_FUNC_MDIO0_E1_SWITCH              (RZN1_FUNC_MDIO_OFFSET + 15)
> +
> +/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO function */
> +#define RZN1_FUNC_MDIO1_HIGHZ                  (RZN1_FUNC_MDIO_OFFSET + 16)
> +#define RZN1_FUNC_MDIO1_GMAC0                  (RZN1_FUNC_MDIO_OFFSET + 17)
> +#define RZN1_FUNC_MDIO1_GMAC1                  (RZN1_FUNC_MDIO_OFFSET + 18)
> +#define RZN1_FUNC_MDIO1_ECAT                   (RZN1_FUNC_MDIO_OFFSET + 19)
> +#define RZN1_FUNC_MDIO1_S3_MDIO0               (RZN1_FUNC_MDIO_OFFSET + 20)
> +#define RZN1_FUNC_MDIO1_S3_MDIO1               (RZN1_FUNC_MDIO_OFFSET + 21)
> +#define RZN1_FUNC_MDIO1_HWRTOS                 (RZN1_FUNC_MDIO_OFFSET + 22)
> +#define RZN1_FUNC_MDIO1_SWITCH                 (RZN1_FUNC_MDIO_OFFSET + 23)
> +/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
> +#define RZN1_FUNC_MDIO1_E1_HIGHZ               (RZN1_FUNC_MDIO_OFFSET + 24)
> +#define RZN1_FUNC_MDIO1_E1_GMAC0               (RZN1_FUNC_MDIO_OFFSET + 25)
> +#define RZN1_FUNC_MDIO1_E1_GMAC1               (RZN1_FUNC_MDIO_OFFSET + 26)
> +#define RZN1_FUNC_MDIO1_E1_ECAT                        (RZN1_FUNC_MDIO_OFFSET + 27)
> +#define RZN1_FUNC_MDIO1_E1_S3_MDIO0            (RZN1_FUNC_MDIO_OFFSET + 28)
> +#define RZN1_FUNC_MDIO1_E1_S3_MDIO1            (RZN1_FUNC_MDIO_OFFSET + 29)
> +#define RZN1_FUNC_MDIO1_E1_HWRTOS              (RZN1_FUNC_MDIO_OFFSET + 30)
> +#define RZN1_FUNC_MDIO1_E1_SWITCH              (RZN1_FUNC_MDIO_OFFSET + 31)
> +
> +#define RZN1_FUNC_MAX                          (RZN1_FUNC_MDIO_OFFSET + 32)
> +
> +#endif /* __DT_BINDINGS_RZN1_PINCTRL_H */
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation
  2018-09-26 14:20   ` Rob Herring
@ 2018-09-26 14:40       ` Geert Uytterhoeven
  0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-09-26 14:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Phil Edworthy, Laurent Pinchart, Rob Herring, Mark Rutland,
	Jacopo Mondi, Linus Walleij, Simon Horman,
	open list:GPIO SUBSYSTEM, Linux-Renesas,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Rob,

CC devicetree

On Wed, Sep 26, 2018 at 4:20 PM Rob Herring <robh@kernel.org> wrote:
>
> DT bindings should be sent to DT list.
>
> On Wed, Sep 19, 2018 at 9:24 AM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> >
> > The Renesas RZ/N1 device family PINCTRL node description.
> >
> > Based on a patch originally written by Michel Pollet at Renesas.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> > v4:
> >  - Add alternative way to use the pinmux prop.
> >  - Remove mention of gpios.
> >
> > v3:
> >  - Use standard bindings
> >  - Change the way the functions are defined so it is easy to check
> >    against the hardware numbering.
> >  - Add functions for the MDIO source peripheral instead of using
> >    virtual pins.
> >
> > v2:
> >  - Change filename to generic rzn1, instead of device specific.
> >  - Add "renesas,rzn1-pinctrl" compatible fallback string.
> >  - Example register size corrected.
> >  - Typos fixed.
> >  - Changes suggested by Jacopo Mondi.
> >  - rzn1-pinctrl.h squashed into this as requested by Rob Herring.
> > ---
> >  .../bindings/pinctrl/renesas,rzn1-pinctrl.txt | 153 ++++++++++++++++++
> >  include/dt-bindings/pinctrl/rzn1-pinctrl.h    | 141 ++++++++++++++++
> >  2 files changed, 294 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> >  create mode 100644 include/dt-bindings/pinctrl/rzn1-pinctrl.h
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> > new file mode 100644
> > index 000000000000..323ef5c6c1c9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> > @@ -0,0 +1,153 @@
> > +Renesas RZ/N1 SoC Pinctrl node description.
> > +
> > +Pin controller node
> > +-------------------
> > +Required properties:
> > +- compatible: SoC-specific compatible string "renesas,<soc-specific>-pinctrl"
> > +  followed by "renesas,rzn1-pinctrl" as fallback. The SoC-specific compatible
> > +  strings must be one of:
> > +       "renesas,r9a06g032-pinctrl" for RZ/N1D
> > +       "renesas,r9a06g033-pinctrl" for RZ/N1S
> > +- reg: Address base and length of the memory area where the pin controller
> > +  hardware is mapped to.
> > +- clocks: phandle for the clock, see the description of clock-names below.
> > +- clock-names: Contains the name of the clock:
> > +    "bus", the bus clock, sometimes described as pclk, for register accesses.
> > +
> > +Example:
> > +       pinctrl: pin-controller@40067000 {
> > +           compatible = "renesas,r9a06g032-pinctrl", "renesas,rzn1-pinctrl";
> > +           reg = <0x40067000 0x1000>, <0x51000000 0x480>;
> > +           clocks = <&sysctrl R9A06G032_HCLK_PINCONFIG>;
> > +           clock-names = "bus";
> > +       };
> > +
> > +Sub-nodes
> > +---------
> > +
> > +The child nodes of the pin controller node describe a pin multiplexing
> > +function.
> > +
> > +- Pin multiplexing sub-nodes:
> > +  A pin multiplexing sub-node describes how to configure a set of
> > +  (or a single) pin in some desired alternate function mode.
> > +  A single sub-node may define several pin configurations.
> > +  Please refer to pinctrl-bindings.txt to get to know more on generic
> > +  pin properties usage.
> > +
> > +  The allowed generic formats for a pin multiplexing sub-node are the
> > +  following ones:
> > +
> > +  node-1 {
> > +      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +      GENERIC_PINCONFIG;
> > +  };
> > +
> > +  node-2 {
> > +      sub-node-1 {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +
> > +      sub-node-2 {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +
> > +      ...
> > +
> > +      sub-node-n {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +  };
> > +
> > +  node-3 {
> > +      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +      GENERIC_PINCONFIG;
> > +
> > +      sub-node-1 {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +
> > +      ...
> > +
> > +      sub-node-n {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +  };
> > +
> > +  Use the latter two formats when pins part of the same logical group need to
> > +  have different generic pin configuration flags applied. Note that the generic
> > +  pinconfig in node-3 does not apply to the sub-nodes.
> > +
> > +  Client sub-nodes shall refer to pin multiplexing sub-nodes using the phandle
> > +  of the most external one.
> > +
> > +  Eg.
> > +
> > +  client-1 {
> > +      ...
> > +      pinctrl-0 = <&node-1>;
> > +      ...
> > +  };
> > +
> > +  client-2 {
> > +      ...
> > +      pinctrl-0 = <&node-2>;
> > +      ...
> > +  };
> > +
> > +  Required properties:
> > +    - pinmux:
> > +      integer array representing pin number and pin multiplexing configuration.
> > +      When a pin has to be configured in alternate function mode, use this
> > +      property to identify the pin by its global index, and provide its
> > +      alternate function configuration number along with it.
> > +      When multiple pins are required to be configured as part of the same
> > +      alternate function they shall be specified as members of the same
> > +      argument list of a single "pinmux" property.
> > +      Integers values in the "pinmux" argument list are assembled as:
> > +      (PIN | MUX_FUNC << 8)
> > +      where PIN directly corresponds to the pl_gpio pin number and MUX_FUNC is
> > +      one of the alternate function identifiers defined in:
> > +      <include/dt-bindings/pinctrl/rzn1-pinctrl.h>
> > +      These identifiers collapse the IO Multiplex Configuration Level 1 and
> > +      Level 2 numbers that are detailed in the hardware reference manual into a
> > +      single number. The identifiers for Level 2 are simply offset by 10.
> > +      Additional identifiers are provided to specify the MDIO source peripheral.
> > +
> > +  Optional generic properties:
> > +    - bias-disable             - disable any pin bias
> > +    - bias-pull-up             - pull up the pin with 50 KOhm
> > +    - bias-pull-down           - pull down the pin with 50 KOhm
> > +    - bias-high-impedance      - high impedance mode
> > +    - drive-strength           - sink or source at most 4, 6, 8 or 12 mA
> > +
> > +  Example:
> > +  A serial communication interface with a TX output pin and an RX input pin.
> > +
> > +  &pinctrl {
> > +       pins_uart0: pins_uart0 {
> > +               pinmux = <
> > +                       RZN1_MUX(103, UART0_I)  /* UART0_TXD */
> > +                       RZN1_MUX(104, UART0_I)  /* UART0_RXD */
> > +               >;
> > +       };
> > +  };
> > +
> > +  Example 2:
> > +  Here we set the pull up on the RXD pin of the UART.
> > +
> > +  &pinctrl {
> > +       pins_uart0: pins_uart0 {
> > +               pinmux = <RZN1_MUX(103, UART0_I)>;      /* UART0_TXD */
> > +
> > +               pins_uart6_rx {
> > +                       pinmux = <RZN1_MUX(104, UART0_I)>;      /* UART0_RXD */
> > +                       bias-pull-up;
> > +               };
> > +       };
> > +  };
> > diff --git a/include/dt-bindings/pinctrl/rzn1-pinctrl.h b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
> > new file mode 100644
> > index 000000000000..40369ee36e6a
> > --- /dev/null
> > +++ b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
> > @@ -0,0 +1,141 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Defines macros and constants for Renesas RZ/N1 pin controller pin
> > + * muxing functions.
> > + */
> > +#ifndef __DT_BINDINGS_RZN1_PINCTRL_H
> > +#define __DT_BINDINGS_RZN1_PINCTRL_H
> > +
> > +#define RZN1_MUX(_gpio, _func) \
> > +       (((RZN1_FUNC_##_func) << 8) | (_gpio))
>
> I'm not a fan of token pasting and it also goes against kernel style.
> If every other Renesas platform is doing this, then fine. Otherwise,
> you can express it in pretty much the same (source) space:
>
> pinmux = <RZN1_MUX_UART0_I 104>;
>
> Yes, this is 2 cells instead of 1, but if you care about space, you
> can use 8 or 16 bit size.

Not every Renesas platform uses token pasting. Only RZ/A1 already does,
cfr. include/dt-bindings/pinctrl/r7s72100-pinctrl.h.
Most other SoCs used named pin groups, i.e. strings, without relying
on a header file.

>
> > +/*
> > + * Given the different levels of muxing on the SoC, it was decided to
> > + * 'linearize' them into one numerical space. So mux level 1, 2 and the MDIO
> > + * muxes are all represented by one single value.
> > + *
> > + * You can derive the hardware value pretty easily too, as
> > + * 0...9   are Level 1
> > + * 10...71 are Level 2. The Level 2 mux will be set to this
> > + *         value - RZN1_FUNC_L2_OFFSET, and the Level 1 mux will be
> > + *         set accordingly.
> > + * 72...103 are for the 2 MDIO muxes.
>
> Whatever works best for the driver, but wouldn't aligning the levels
> to power of 2 make decoding a bit easier? 0-15 for level 1, 16-127 for
> 2, 128+ for MDIO.
>
> Rob
>
> > + */
> > +#define RZN1_FUNC_HIGHZ                                0
> > +#define RZN1_FUNC_0L                           1
> > +#define RZN1_FUNC_CLK_ETH_MII_RGMII_RMII       2
> > +#define RZN1_FUNC_CLK_ETH_NAND                 3
> > +#define RZN1_FUNC_QSPI                         4
> > +#define RZN1_FUNC_SDIO                         5
> > +#define RZN1_FUNC_LCD                          6
> > +#define RZN1_FUNC_LCD_E                                7
> > +#define RZN1_FUNC_MSEBIM                       8
> > +#define RZN1_FUNC_MSEBIS                       9
> > +#define RZN1_FUNC_L2_OFFSET                    10      /* I'm Special */
> > +
> > +#define RZN1_FUNC_HIGHZ1                       (RZN1_FUNC_L2_OFFSET + 0)
> > +#define RZN1_FUNC_ETHERCAT                     (RZN1_FUNC_L2_OFFSET + 1)
> > +#define RZN1_FUNC_SERCOS3                      (RZN1_FUNC_L2_OFFSET + 2)
> > +#define RZN1_FUNC_SDIO_E                       (RZN1_FUNC_L2_OFFSET + 3)
> > +#define RZN1_FUNC_ETH_MDIO                     (RZN1_FUNC_L2_OFFSET + 4)
> > +#define RZN1_FUNC_ETH_MDIO_E1                  (RZN1_FUNC_L2_OFFSET + 5)
> > +#define RZN1_FUNC_USB                          (RZN1_FUNC_L2_OFFSET + 6)
> > +#define RZN1_FUNC_MSEBIM_E                     (RZN1_FUNC_L2_OFFSET + 7)
> > +#define RZN1_FUNC_MSEBIS_E                     (RZN1_FUNC_L2_OFFSET + 8)
> > +#define RZN1_FUNC_RSV                          (RZN1_FUNC_L2_OFFSET + 9)
> > +#define RZN1_FUNC_RSV_E                                (RZN1_FUNC_L2_OFFSET + 10)
> > +#define RZN1_FUNC_RSV_E1                       (RZN1_FUNC_L2_OFFSET + 11)
> > +#define RZN1_FUNC_UART0_I                      (RZN1_FUNC_L2_OFFSET + 12)
> > +#define RZN1_FUNC_UART0_I_E                    (RZN1_FUNC_L2_OFFSET + 13)
> > +#define RZN1_FUNC_UART1_I                      (RZN1_FUNC_L2_OFFSET + 14)
> > +#define RZN1_FUNC_UART1_I_E                    (RZN1_FUNC_L2_OFFSET + 15)
> > +#define RZN1_FUNC_UART2_I                      (RZN1_FUNC_L2_OFFSET + 16)
> > +#define RZN1_FUNC_UART2_I_E                    (RZN1_FUNC_L2_OFFSET + 17)
> > +#define RZN1_FUNC_UART0                                (RZN1_FUNC_L2_OFFSET + 18)
> > +#define RZN1_FUNC_UART0_E                      (RZN1_FUNC_L2_OFFSET + 19)
> > +#define RZN1_FUNC_UART1                                (RZN1_FUNC_L2_OFFSET + 20)
> > +#define RZN1_FUNC_UART1_E                      (RZN1_FUNC_L2_OFFSET + 21)
> > +#define RZN1_FUNC_UART2                                (RZN1_FUNC_L2_OFFSET + 22)
> > +#define RZN1_FUNC_UART2_E                      (RZN1_FUNC_L2_OFFSET + 23)
> > +#define RZN1_FUNC_UART3                                (RZN1_FUNC_L2_OFFSET + 24)
> > +#define RZN1_FUNC_UART3_E                      (RZN1_FUNC_L2_OFFSET + 25)
> > +#define RZN1_FUNC_UART4                                (RZN1_FUNC_L2_OFFSET + 26)
> > +#define RZN1_FUNC_UART4_E                      (RZN1_FUNC_L2_OFFSET + 27)
> > +#define RZN1_FUNC_UART5                                (RZN1_FUNC_L2_OFFSET + 28)
> > +#define RZN1_FUNC_UART5_E                      (RZN1_FUNC_L2_OFFSET + 29)
> > +#define RZN1_FUNC_UART6                                (RZN1_FUNC_L2_OFFSET + 30)
> > +#define RZN1_FUNC_UART6_E                      (RZN1_FUNC_L2_OFFSET + 31)
> > +#define RZN1_FUNC_UART7                                (RZN1_FUNC_L2_OFFSET + 32)
> > +#define RZN1_FUNC_UART7_E                      (RZN1_FUNC_L2_OFFSET + 33)
> > +#define RZN1_FUNC_SPI0_M                       (RZN1_FUNC_L2_OFFSET + 34)
> > +#define RZN1_FUNC_SPI0_M_E                     (RZN1_FUNC_L2_OFFSET + 35)
> > +#define RZN1_FUNC_SPI1_M                       (RZN1_FUNC_L2_OFFSET + 36)
> > +#define RZN1_FUNC_SPI1_M_E                     (RZN1_FUNC_L2_OFFSET + 37)
> > +#define RZN1_FUNC_SPI2_M                       (RZN1_FUNC_L2_OFFSET + 38)
> > +#define RZN1_FUNC_SPI2_M_E                     (RZN1_FUNC_L2_OFFSET + 39)
> > +#define RZN1_FUNC_SPI3_M                       (RZN1_FUNC_L2_OFFSET + 40)
> > +#define RZN1_FUNC_SPI3_M_E                     (RZN1_FUNC_L2_OFFSET + 41)
> > +#define RZN1_FUNC_SPI4_S                       (RZN1_FUNC_L2_OFFSET + 42)
> > +#define RZN1_FUNC_SPI4_S_E                     (RZN1_FUNC_L2_OFFSET + 43)
> > +#define RZN1_FUNC_SPI5_S                       (RZN1_FUNC_L2_OFFSET + 44)
> > +#define RZN1_FUNC_SPI5_S_E                     (RZN1_FUNC_L2_OFFSET + 45)
> > +#define RZN1_FUNC_SGPIO0_M                     (RZN1_FUNC_L2_OFFSET + 46)
> > +#define RZN1_FUNC_SGPIO1_M                     (RZN1_FUNC_L2_OFFSET + 47)
> > +#define RZN1_FUNC_GPIO                         (RZN1_FUNC_L2_OFFSET + 48)
> > +#define RZN1_FUNC_CAN                          (RZN1_FUNC_L2_OFFSET + 49)
> > +#define RZN1_FUNC_I2C                          (RZN1_FUNC_L2_OFFSET + 50)
> > +#define RZN1_FUNC_SAFE                         (RZN1_FUNC_L2_OFFSET + 51)
> > +#define RZN1_FUNC_PTO_PWM                      (RZN1_FUNC_L2_OFFSET + 52)
> > +#define RZN1_FUNC_PTO_PWM1                     (RZN1_FUNC_L2_OFFSET + 53)
> > +#define RZN1_FUNC_PTO_PWM2                     (RZN1_FUNC_L2_OFFSET + 54)
> > +#define RZN1_FUNC_PTO_PWM3                     (RZN1_FUNC_L2_OFFSET + 55)
> > +#define RZN1_FUNC_PTO_PWM4                     (RZN1_FUNC_L2_OFFSET + 56)
> > +#define RZN1_FUNC_DELTA_SIGMA                  (RZN1_FUNC_L2_OFFSET + 57)
> > +#define RZN1_FUNC_SGPIO2_M                     (RZN1_FUNC_L2_OFFSET + 58)
> > +#define RZN1_FUNC_SGPIO3_M                     (RZN1_FUNC_L2_OFFSET + 59)
> > +#define RZN1_FUNC_SGPIO4_S                     (RZN1_FUNC_L2_OFFSET + 60)
> > +#define RZN1_FUNC_MAC_MTIP_SWITCH              (RZN1_FUNC_L2_OFFSET + 61)
> > +
> > +#define RZN1_FUNC_MDIO_OFFSET                  (RZN1_FUNC_L2_OFFSET + 62)
> > +
> > +/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO function */
> > +#define RZN1_FUNC_MDIO0_HIGHZ                  (RZN1_FUNC_MDIO_OFFSET + 0)
> > +#define RZN1_FUNC_MDIO0_GMAC0                  (RZN1_FUNC_MDIO_OFFSET + 1)
> > +#define RZN1_FUNC_MDIO0_GMAC1                  (RZN1_FUNC_MDIO_OFFSET + 2)
> > +#define RZN1_FUNC_MDIO0_ECAT                   (RZN1_FUNC_MDIO_OFFSET + 3)
> > +#define RZN1_FUNC_MDIO0_S3_MDIO0               (RZN1_FUNC_MDIO_OFFSET + 4)
> > +#define RZN1_FUNC_MDIO0_S3_MDIO1               (RZN1_FUNC_MDIO_OFFSET + 5)
> > +#define RZN1_FUNC_MDIO0_HWRTOS                 (RZN1_FUNC_MDIO_OFFSET + 6)
> > +#define RZN1_FUNC_MDIO0_SWITCH                 (RZN1_FUNC_MDIO_OFFSET + 7)
> > +/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
> > +#define RZN1_FUNC_MDIO0_E1_HIGHZ               (RZN1_FUNC_MDIO_OFFSET + 8)
> > +#define RZN1_FUNC_MDIO0_E1_GMAC0               (RZN1_FUNC_MDIO_OFFSET + 9)
> > +#define RZN1_FUNC_MDIO0_E1_GMAC1               (RZN1_FUNC_MDIO_OFFSET + 10)
> > +#define RZN1_FUNC_MDIO0_E1_ECAT                        (RZN1_FUNC_MDIO_OFFSET + 11)
> > +#define RZN1_FUNC_MDIO0_E1_S3_MDIO0            (RZN1_FUNC_MDIO_OFFSET + 12)
> > +#define RZN1_FUNC_MDIO0_E1_S3_MDIO1            (RZN1_FUNC_MDIO_OFFSET + 13)
> > +#define RZN1_FUNC_MDIO0_E1_HWRTOS              (RZN1_FUNC_MDIO_OFFSET + 14)
> > +#define RZN1_FUNC_MDIO0_E1_SWITCH              (RZN1_FUNC_MDIO_OFFSET + 15)
> > +
> > +/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO function */
> > +#define RZN1_FUNC_MDIO1_HIGHZ                  (RZN1_FUNC_MDIO_OFFSET + 16)
> > +#define RZN1_FUNC_MDIO1_GMAC0                  (RZN1_FUNC_MDIO_OFFSET + 17)
> > +#define RZN1_FUNC_MDIO1_GMAC1                  (RZN1_FUNC_MDIO_OFFSET + 18)
> > +#define RZN1_FUNC_MDIO1_ECAT                   (RZN1_FUNC_MDIO_OFFSET + 19)
> > +#define RZN1_FUNC_MDIO1_S3_MDIO0               (RZN1_FUNC_MDIO_OFFSET + 20)
> > +#define RZN1_FUNC_MDIO1_S3_MDIO1               (RZN1_FUNC_MDIO_OFFSET + 21)
> > +#define RZN1_FUNC_MDIO1_HWRTOS                 (RZN1_FUNC_MDIO_OFFSET + 22)
> > +#define RZN1_FUNC_MDIO1_SWITCH                 (RZN1_FUNC_MDIO_OFFSET + 23)
> > +/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
> > +#define RZN1_FUNC_MDIO1_E1_HIGHZ               (RZN1_FUNC_MDIO_OFFSET + 24)
> > +#define RZN1_FUNC_MDIO1_E1_GMAC0               (RZN1_FUNC_MDIO_OFFSET + 25)
> > +#define RZN1_FUNC_MDIO1_E1_GMAC1               (RZN1_FUNC_MDIO_OFFSET + 26)
> > +#define RZN1_FUNC_MDIO1_E1_ECAT                        (RZN1_FUNC_MDIO_OFFSET + 27)
> > +#define RZN1_FUNC_MDIO1_E1_S3_MDIO0            (RZN1_FUNC_MDIO_OFFSET + 28)
> > +#define RZN1_FUNC_MDIO1_E1_S3_MDIO1            (RZN1_FUNC_MDIO_OFFSET + 29)
> > +#define RZN1_FUNC_MDIO1_E1_HWRTOS              (RZN1_FUNC_MDIO_OFFSET + 30)
> > +#define RZN1_FUNC_MDIO1_E1_SWITCH              (RZN1_FUNC_MDIO_OFFSET + 31)
> > +
> > +#define RZN1_FUNC_MAX                          (RZN1_FUNC_MDIO_OFFSET + 32)
> > +
> > +#endif /* __DT_BINDINGS_RZN1_PINCTRL_H */

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation
@ 2018-09-26 14:40       ` Geert Uytterhoeven
  0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-09-26 14:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Phil Edworthy, Laurent Pinchart, Rob Herring, Mark Rutland,
	Jacopo Mondi, Linus Walleij, Simon Horman,
	open list:GPIO SUBSYSTEM, Linux-Renesas,
	Linux Kernel Mailing List,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Rob,

CC devicetree

On Wed, Sep 26, 2018 at 4:20 PM Rob Herring <robh@kernel.org> wrote:
>
> DT bindings should be sent to DT list.
>
> On Wed, Sep 19, 2018 at 9:24 AM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> >
> > The Renesas RZ/N1 device family PINCTRL node description.
> >
> > Based on a patch originally written by Michel Pollet at Renesas.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > ---
> > v4:
> >  - Add alternative way to use the pinmux prop.
> >  - Remove mention of gpios.
> >
> > v3:
> >  - Use standard bindings
> >  - Change the way the functions are defined so it is easy to check
> >    against the hardware numbering.
> >  - Add functions for the MDIO source peripheral instead of using
> >    virtual pins.
> >
> > v2:
> >  - Change filename to generic rzn1, instead of device specific.
> >  - Add "renesas,rzn1-pinctrl" compatible fallback string.
> >  - Example register size corrected.
> >  - Typos fixed.
> >  - Changes suggested by Jacopo Mondi.
> >  - rzn1-pinctrl.h squashed into this as requested by Rob Herring.
> > ---
> >  .../bindings/pinctrl/renesas,rzn1-pinctrl.txt | 153 ++++++++++++++++++
> >  include/dt-bindings/pinctrl/rzn1-pinctrl.h    | 141 ++++++++++++++++
> >  2 files changed, 294 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> >  create mode 100644 include/dt-bindings/pinctrl/rzn1-pinctrl.h
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> > new file mode 100644
> > index 000000000000..323ef5c6c1c9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
> > @@ -0,0 +1,153 @@
> > +Renesas RZ/N1 SoC Pinctrl node description.
> > +
> > +Pin controller node
> > +-------------------
> > +Required properties:
> > +- compatible: SoC-specific compatible string "renesas,<soc-specific>-pinctrl"
> > +  followed by "renesas,rzn1-pinctrl" as fallback. The SoC-specific compatible
> > +  strings must be one of:
> > +       "renesas,r9a06g032-pinctrl" for RZ/N1D
> > +       "renesas,r9a06g033-pinctrl" for RZ/N1S
> > +- reg: Address base and length of the memory area where the pin controller
> > +  hardware is mapped to.
> > +- clocks: phandle for the clock, see the description of clock-names below.
> > +- clock-names: Contains the name of the clock:
> > +    "bus", the bus clock, sometimes described as pclk, for register accesses.
> > +
> > +Example:
> > +       pinctrl: pin-controller@40067000 {
> > +           compatible = "renesas,r9a06g032-pinctrl", "renesas,rzn1-pinctrl";
> > +           reg = <0x40067000 0x1000>, <0x51000000 0x480>;
> > +           clocks = <&sysctrl R9A06G032_HCLK_PINCONFIG>;
> > +           clock-names = "bus";
> > +       };
> > +
> > +Sub-nodes
> > +---------
> > +
> > +The child nodes of the pin controller node describe a pin multiplexing
> > +function.
> > +
> > +- Pin multiplexing sub-nodes:
> > +  A pin multiplexing sub-node describes how to configure a set of
> > +  (or a single) pin in some desired alternate function mode.
> > +  A single sub-node may define several pin configurations.
> > +  Please refer to pinctrl-bindings.txt to get to know more on generic
> > +  pin properties usage.
> > +
> > +  The allowed generic formats for a pin multiplexing sub-node are the
> > +  following ones:
> > +
> > +  node-1 {
> > +      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +      GENERIC_PINCONFIG;
> > +  };
> > +
> > +  node-2 {
> > +      sub-node-1 {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +
> > +      sub-node-2 {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +
> > +      ...
> > +
> > +      sub-node-n {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +  };
> > +
> > +  node-3 {
> > +      pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +      GENERIC_PINCONFIG;
> > +
> > +      sub-node-1 {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +
> > +      ...
> > +
> > +      sub-node-n {
> > +          pinmux = <PIN_ID_AND_MUX>, <PIN_ID_AND_MUX>, ... ;
> > +          GENERIC_PINCONFIG;
> > +      };
> > +  };
> > +
> > +  Use the latter two formats when pins part of the same logical group need to
> > +  have different generic pin configuration flags applied. Note that the generic
> > +  pinconfig in node-3 does not apply to the sub-nodes.
> > +
> > +  Client sub-nodes shall refer to pin multiplexing sub-nodes using the phandle
> > +  of the most external one.
> > +
> > +  Eg.
> > +
> > +  client-1 {
> > +      ...
> > +      pinctrl-0 = <&node-1>;
> > +      ...
> > +  };
> > +
> > +  client-2 {
> > +      ...
> > +      pinctrl-0 = <&node-2>;
> > +      ...
> > +  };
> > +
> > +  Required properties:
> > +    - pinmux:
> > +      integer array representing pin number and pin multiplexing configuration.
> > +      When a pin has to be configured in alternate function mode, use this
> > +      property to identify the pin by its global index, and provide its
> > +      alternate function configuration number along with it.
> > +      When multiple pins are required to be configured as part of the same
> > +      alternate function they shall be specified as members of the same
> > +      argument list of a single "pinmux" property.
> > +      Integers values in the "pinmux" argument list are assembled as:
> > +      (PIN | MUX_FUNC << 8)
> > +      where PIN directly corresponds to the pl_gpio pin number and MUX_FUNC is
> > +      one of the alternate function identifiers defined in:
> > +      <include/dt-bindings/pinctrl/rzn1-pinctrl.h>
> > +      These identifiers collapse the IO Multiplex Configuration Level 1 and
> > +      Level 2 numbers that are detailed in the hardware reference manual into a
> > +      single number. The identifiers for Level 2 are simply offset by 10.
> > +      Additional identifiers are provided to specify the MDIO source peripheral.
> > +
> > +  Optional generic properties:
> > +    - bias-disable             - disable any pin bias
> > +    - bias-pull-up             - pull up the pin with 50 KOhm
> > +    - bias-pull-down           - pull down the pin with 50 KOhm
> > +    - bias-high-impedance      - high impedance mode
> > +    - drive-strength           - sink or source at most 4, 6, 8 or 12 mA
> > +
> > +  Example:
> > +  A serial communication interface with a TX output pin and an RX input pin.
> > +
> > +  &pinctrl {
> > +       pins_uart0: pins_uart0 {
> > +               pinmux = <
> > +                       RZN1_MUX(103, UART0_I)  /* UART0_TXD */
> > +                       RZN1_MUX(104, UART0_I)  /* UART0_RXD */
> > +               >;
> > +       };
> > +  };
> > +
> > +  Example 2:
> > +  Here we set the pull up on the RXD pin of the UART.
> > +
> > +  &pinctrl {
> > +       pins_uart0: pins_uart0 {
> > +               pinmux = <RZN1_MUX(103, UART0_I)>;      /* UART0_TXD */
> > +
> > +               pins_uart6_rx {
> > +                       pinmux = <RZN1_MUX(104, UART0_I)>;      /* UART0_RXD */
> > +                       bias-pull-up;
> > +               };
> > +       };
> > +  };
> > diff --git a/include/dt-bindings/pinctrl/rzn1-pinctrl.h b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
> > new file mode 100644
> > index 000000000000..40369ee36e6a
> > --- /dev/null
> > +++ b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
> > @@ -0,0 +1,141 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Defines macros and constants for Renesas RZ/N1 pin controller pin
> > + * muxing functions.
> > + */
> > +#ifndef __DT_BINDINGS_RZN1_PINCTRL_H
> > +#define __DT_BINDINGS_RZN1_PINCTRL_H
> > +
> > +#define RZN1_MUX(_gpio, _func) \
> > +       (((RZN1_FUNC_##_func) << 8) | (_gpio))
>
> I'm not a fan of token pasting and it also goes against kernel style.
> If every other Renesas platform is doing this, then fine. Otherwise,
> you can express it in pretty much the same (source) space:
>
> pinmux = <RZN1_MUX_UART0_I 104>;
>
> Yes, this is 2 cells instead of 1, but if you care about space, you
> can use 8 or 16 bit size.

Not every Renesas platform uses token pasting. Only RZ/A1 already does,
cfr. include/dt-bindings/pinctrl/r7s72100-pinctrl.h.
Most other SoCs used named pin groups, i.e. strings, without relying
on a header file.

>
> > +/*
> > + * Given the different levels of muxing on the SoC, it was decided to
> > + * 'linearize' them into one numerical space. So mux level 1, 2 and the MDIO
> > + * muxes are all represented by one single value.
> > + *
> > + * You can derive the hardware value pretty easily too, as
> > + * 0...9   are Level 1
> > + * 10...71 are Level 2. The Level 2 mux will be set to this
> > + *         value - RZN1_FUNC_L2_OFFSET, and the Level 1 mux will be
> > + *         set accordingly.
> > + * 72...103 are for the 2 MDIO muxes.
>
> Whatever works best for the driver, but wouldn't aligning the levels
> to power of 2 make decoding a bit easier? 0-15 for level 1, 16-127 for
> 2, 128+ for MDIO.
>
> Rob
>
> > + */
> > +#define RZN1_FUNC_HIGHZ                                0
> > +#define RZN1_FUNC_0L                           1
> > +#define RZN1_FUNC_CLK_ETH_MII_RGMII_RMII       2
> > +#define RZN1_FUNC_CLK_ETH_NAND                 3
> > +#define RZN1_FUNC_QSPI                         4
> > +#define RZN1_FUNC_SDIO                         5
> > +#define RZN1_FUNC_LCD                          6
> > +#define RZN1_FUNC_LCD_E                                7
> > +#define RZN1_FUNC_MSEBIM                       8
> > +#define RZN1_FUNC_MSEBIS                       9
> > +#define RZN1_FUNC_L2_OFFSET                    10      /* I'm Special */
> > +
> > +#define RZN1_FUNC_HIGHZ1                       (RZN1_FUNC_L2_OFFSET + 0)
> > +#define RZN1_FUNC_ETHERCAT                     (RZN1_FUNC_L2_OFFSET + 1)
> > +#define RZN1_FUNC_SERCOS3                      (RZN1_FUNC_L2_OFFSET + 2)
> > +#define RZN1_FUNC_SDIO_E                       (RZN1_FUNC_L2_OFFSET + 3)
> > +#define RZN1_FUNC_ETH_MDIO                     (RZN1_FUNC_L2_OFFSET + 4)
> > +#define RZN1_FUNC_ETH_MDIO_E1                  (RZN1_FUNC_L2_OFFSET + 5)
> > +#define RZN1_FUNC_USB                          (RZN1_FUNC_L2_OFFSET + 6)
> > +#define RZN1_FUNC_MSEBIM_E                     (RZN1_FUNC_L2_OFFSET + 7)
> > +#define RZN1_FUNC_MSEBIS_E                     (RZN1_FUNC_L2_OFFSET + 8)
> > +#define RZN1_FUNC_RSV                          (RZN1_FUNC_L2_OFFSET + 9)
> > +#define RZN1_FUNC_RSV_E                                (RZN1_FUNC_L2_OFFSET + 10)
> > +#define RZN1_FUNC_RSV_E1                       (RZN1_FUNC_L2_OFFSET + 11)
> > +#define RZN1_FUNC_UART0_I                      (RZN1_FUNC_L2_OFFSET + 12)
> > +#define RZN1_FUNC_UART0_I_E                    (RZN1_FUNC_L2_OFFSET + 13)
> > +#define RZN1_FUNC_UART1_I                      (RZN1_FUNC_L2_OFFSET + 14)
> > +#define RZN1_FUNC_UART1_I_E                    (RZN1_FUNC_L2_OFFSET + 15)
> > +#define RZN1_FUNC_UART2_I                      (RZN1_FUNC_L2_OFFSET + 16)
> > +#define RZN1_FUNC_UART2_I_E                    (RZN1_FUNC_L2_OFFSET + 17)
> > +#define RZN1_FUNC_UART0                                (RZN1_FUNC_L2_OFFSET + 18)
> > +#define RZN1_FUNC_UART0_E                      (RZN1_FUNC_L2_OFFSET + 19)
> > +#define RZN1_FUNC_UART1                                (RZN1_FUNC_L2_OFFSET + 20)
> > +#define RZN1_FUNC_UART1_E                      (RZN1_FUNC_L2_OFFSET + 21)
> > +#define RZN1_FUNC_UART2                                (RZN1_FUNC_L2_OFFSET + 22)
> > +#define RZN1_FUNC_UART2_E                      (RZN1_FUNC_L2_OFFSET + 23)
> > +#define RZN1_FUNC_UART3                                (RZN1_FUNC_L2_OFFSET + 24)
> > +#define RZN1_FUNC_UART3_E                      (RZN1_FUNC_L2_OFFSET + 25)
> > +#define RZN1_FUNC_UART4                                (RZN1_FUNC_L2_OFFSET + 26)
> > +#define RZN1_FUNC_UART4_E                      (RZN1_FUNC_L2_OFFSET + 27)
> > +#define RZN1_FUNC_UART5                                (RZN1_FUNC_L2_OFFSET + 28)
> > +#define RZN1_FUNC_UART5_E                      (RZN1_FUNC_L2_OFFSET + 29)
> > +#define RZN1_FUNC_UART6                                (RZN1_FUNC_L2_OFFSET + 30)
> > +#define RZN1_FUNC_UART6_E                      (RZN1_FUNC_L2_OFFSET + 31)
> > +#define RZN1_FUNC_UART7                                (RZN1_FUNC_L2_OFFSET + 32)
> > +#define RZN1_FUNC_UART7_E                      (RZN1_FUNC_L2_OFFSET + 33)
> > +#define RZN1_FUNC_SPI0_M                       (RZN1_FUNC_L2_OFFSET + 34)
> > +#define RZN1_FUNC_SPI0_M_E                     (RZN1_FUNC_L2_OFFSET + 35)
> > +#define RZN1_FUNC_SPI1_M                       (RZN1_FUNC_L2_OFFSET + 36)
> > +#define RZN1_FUNC_SPI1_M_E                     (RZN1_FUNC_L2_OFFSET + 37)
> > +#define RZN1_FUNC_SPI2_M                       (RZN1_FUNC_L2_OFFSET + 38)
> > +#define RZN1_FUNC_SPI2_M_E                     (RZN1_FUNC_L2_OFFSET + 39)
> > +#define RZN1_FUNC_SPI3_M                       (RZN1_FUNC_L2_OFFSET + 40)
> > +#define RZN1_FUNC_SPI3_M_E                     (RZN1_FUNC_L2_OFFSET + 41)
> > +#define RZN1_FUNC_SPI4_S                       (RZN1_FUNC_L2_OFFSET + 42)
> > +#define RZN1_FUNC_SPI4_S_E                     (RZN1_FUNC_L2_OFFSET + 43)
> > +#define RZN1_FUNC_SPI5_S                       (RZN1_FUNC_L2_OFFSET + 44)
> > +#define RZN1_FUNC_SPI5_S_E                     (RZN1_FUNC_L2_OFFSET + 45)
> > +#define RZN1_FUNC_SGPIO0_M                     (RZN1_FUNC_L2_OFFSET + 46)
> > +#define RZN1_FUNC_SGPIO1_M                     (RZN1_FUNC_L2_OFFSET + 47)
> > +#define RZN1_FUNC_GPIO                         (RZN1_FUNC_L2_OFFSET + 48)
> > +#define RZN1_FUNC_CAN                          (RZN1_FUNC_L2_OFFSET + 49)
> > +#define RZN1_FUNC_I2C                          (RZN1_FUNC_L2_OFFSET + 50)
> > +#define RZN1_FUNC_SAFE                         (RZN1_FUNC_L2_OFFSET + 51)
> > +#define RZN1_FUNC_PTO_PWM                      (RZN1_FUNC_L2_OFFSET + 52)
> > +#define RZN1_FUNC_PTO_PWM1                     (RZN1_FUNC_L2_OFFSET + 53)
> > +#define RZN1_FUNC_PTO_PWM2                     (RZN1_FUNC_L2_OFFSET + 54)
> > +#define RZN1_FUNC_PTO_PWM3                     (RZN1_FUNC_L2_OFFSET + 55)
> > +#define RZN1_FUNC_PTO_PWM4                     (RZN1_FUNC_L2_OFFSET + 56)
> > +#define RZN1_FUNC_DELTA_SIGMA                  (RZN1_FUNC_L2_OFFSET + 57)
> > +#define RZN1_FUNC_SGPIO2_M                     (RZN1_FUNC_L2_OFFSET + 58)
> > +#define RZN1_FUNC_SGPIO3_M                     (RZN1_FUNC_L2_OFFSET + 59)
> > +#define RZN1_FUNC_SGPIO4_S                     (RZN1_FUNC_L2_OFFSET + 60)
> > +#define RZN1_FUNC_MAC_MTIP_SWITCH              (RZN1_FUNC_L2_OFFSET + 61)
> > +
> > +#define RZN1_FUNC_MDIO_OFFSET                  (RZN1_FUNC_L2_OFFSET + 62)
> > +
> > +/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO function */
> > +#define RZN1_FUNC_MDIO0_HIGHZ                  (RZN1_FUNC_MDIO_OFFSET + 0)
> > +#define RZN1_FUNC_MDIO0_GMAC0                  (RZN1_FUNC_MDIO_OFFSET + 1)
> > +#define RZN1_FUNC_MDIO0_GMAC1                  (RZN1_FUNC_MDIO_OFFSET + 2)
> > +#define RZN1_FUNC_MDIO0_ECAT                   (RZN1_FUNC_MDIO_OFFSET + 3)
> > +#define RZN1_FUNC_MDIO0_S3_MDIO0               (RZN1_FUNC_MDIO_OFFSET + 4)
> > +#define RZN1_FUNC_MDIO0_S3_MDIO1               (RZN1_FUNC_MDIO_OFFSET + 5)
> > +#define RZN1_FUNC_MDIO0_HWRTOS                 (RZN1_FUNC_MDIO_OFFSET + 6)
> > +#define RZN1_FUNC_MDIO0_SWITCH                 (RZN1_FUNC_MDIO_OFFSET + 7)
> > +/* These are MDIO0 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
> > +#define RZN1_FUNC_MDIO0_E1_HIGHZ               (RZN1_FUNC_MDIO_OFFSET + 8)
> > +#define RZN1_FUNC_MDIO0_E1_GMAC0               (RZN1_FUNC_MDIO_OFFSET + 9)
> > +#define RZN1_FUNC_MDIO0_E1_GMAC1               (RZN1_FUNC_MDIO_OFFSET + 10)
> > +#define RZN1_FUNC_MDIO0_E1_ECAT                        (RZN1_FUNC_MDIO_OFFSET + 11)
> > +#define RZN1_FUNC_MDIO0_E1_S3_MDIO0            (RZN1_FUNC_MDIO_OFFSET + 12)
> > +#define RZN1_FUNC_MDIO0_E1_S3_MDIO1            (RZN1_FUNC_MDIO_OFFSET + 13)
> > +#define RZN1_FUNC_MDIO0_E1_HWRTOS              (RZN1_FUNC_MDIO_OFFSET + 14)
> > +#define RZN1_FUNC_MDIO0_E1_SWITCH              (RZN1_FUNC_MDIO_OFFSET + 15)
> > +
> > +/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO function */
> > +#define RZN1_FUNC_MDIO1_HIGHZ                  (RZN1_FUNC_MDIO_OFFSET + 16)
> > +#define RZN1_FUNC_MDIO1_GMAC0                  (RZN1_FUNC_MDIO_OFFSET + 17)
> > +#define RZN1_FUNC_MDIO1_GMAC1                  (RZN1_FUNC_MDIO_OFFSET + 18)
> > +#define RZN1_FUNC_MDIO1_ECAT                   (RZN1_FUNC_MDIO_OFFSET + 19)
> > +#define RZN1_FUNC_MDIO1_S3_MDIO0               (RZN1_FUNC_MDIO_OFFSET + 20)
> > +#define RZN1_FUNC_MDIO1_S3_MDIO1               (RZN1_FUNC_MDIO_OFFSET + 21)
> > +#define RZN1_FUNC_MDIO1_HWRTOS                 (RZN1_FUNC_MDIO_OFFSET + 22)
> > +#define RZN1_FUNC_MDIO1_SWITCH                 (RZN1_FUNC_MDIO_OFFSET + 23)
> > +/* These are MDIO1 peripherals for the RZN1_FUNC_ETH_MDIO_E1 function */
> > +#define RZN1_FUNC_MDIO1_E1_HIGHZ               (RZN1_FUNC_MDIO_OFFSET + 24)
> > +#define RZN1_FUNC_MDIO1_E1_GMAC0               (RZN1_FUNC_MDIO_OFFSET + 25)
> > +#define RZN1_FUNC_MDIO1_E1_GMAC1               (RZN1_FUNC_MDIO_OFFSET + 26)
> > +#define RZN1_FUNC_MDIO1_E1_ECAT                        (RZN1_FUNC_MDIO_OFFSET + 27)
> > +#define RZN1_FUNC_MDIO1_E1_S3_MDIO0            (RZN1_FUNC_MDIO_OFFSET + 28)
> > +#define RZN1_FUNC_MDIO1_E1_S3_MDIO1            (RZN1_FUNC_MDIO_OFFSET + 29)
> > +#define RZN1_FUNC_MDIO1_E1_HWRTOS              (RZN1_FUNC_MDIO_OFFSET + 30)
> > +#define RZN1_FUNC_MDIO1_E1_SWITCH              (RZN1_FUNC_MDIO_OFFSET + 31)
> > +
> > +#define RZN1_FUNC_MAX                          (RZN1_FUNC_MDIO_OFFSET + 32)
> > +
> > +#endif /* __DT_BINDINGS_RZN1_PINCTRL_H */

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-09-26 14:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-19 14:23 [PATCH v4 0/3] Renesas R9A06G032 PINCTRL Driver Phil Edworthy
2018-09-19 14:23 ` [PATCH v4 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy
2018-09-21 12:36   ` jacopo mondi
2018-09-26 14:20   ` Rob Herring
2018-09-26 14:40     ` Geert Uytterhoeven
2018-09-26 14:40       ` Geert Uytterhoeven
2018-09-19 14:23 ` [PATCH v4 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver Phil Edworthy
2018-09-21 13:33   ` jacopo mondi
2018-09-24 11:58   ` Geert Uytterhoeven
2018-09-24 12:32     ` Phil Edworthy
2018-09-19 14:23 ` [PATCH v4 3/3] ARM: dts: r9a06g032: Add pinctrl node Phil Edworthy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.