[PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Phil Edworthy]

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

One area that is likely to be contentious is the use of 'virtual pins' for the 
MDIO pinmuxing. The driver uses two pins (170 and 171) that don't exist on the 
device to configure the MDIO source within the RZ/N1 devices. On these devices, 
there are two Ethernet MACs, a 5-Port Switch, numerous industrial Ethernet 
peripherals, any of which can be the MDIO source. Configuring the MDIO source 
could be done without the virtual pins, e.g. by extending the functions to 
cover all MDIO variants (a total of 32 additional functions), but this would 
allow users to misconfigure individual MDIO pins, rather than assign all MDIO 
pins to a MDIO source. The choice of how to implement this will affect the
DT bindings.

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

One point from Michel's v1 series:
"Note, I used renesas,rzn1-pinmux node to specify the pinmux constants,
and I also don't use some of the properties documented in
pinctrl-bindings.txt on purpose, as they are too limited for my use
(I need to be able to set, clear, ignore or reset level, pull up/down
and function as the pinmux might be set by another OS/core running
concurently)."

Patch 0003 should really be applied after patch:
 "ARM: dts: r9a06g032: Correct UART and add all other UARTs", see
 https://www.spinics.net/lists/arm-kernel/msg673525.html

Main changes:
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      |  97 +++
 arch/arm/boot/dts/r9a06g032.dtsi                   |   8 +
 drivers/pinctrl/Kconfig                            |  10 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/pinctrl-rzn1.c                     | 844 +++++++++++++++++++++
 include/dt-bindings/pinctrl/rzn1-pinctrl.h         | 191 +++++
 6 files changed, 1151 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.7.4

[PATCH v2 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation

[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>
---
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      |  97 +++++++++++
 include/dt-bindings/pinctrl/rzn1-pinctrl.h         | 191 +++++++++++++++++++++
 2 files changed, 288 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 0000000..d254388
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzn1-pinctrl.txt
@@ -0,0 +1,97 @@
+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: phandles 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
+  group that can be used by device nodes.
+
+  A pin multiplexing sub-node describes how to configure a set of pins, or a
+  single pin, in some desired alternate function mode.
+  A single sub-node may define several pin configurations.
+
+  The allowed generic formats for a pin multiplexing sub-node are the
+  following ones:
+
+  Client sub-nodes shall refer to pin multiplexing sub-nodes using the phandle
+  of the most external one. See the generic pinctrl-bindings.txt for details.
+
+  Required properties:
+    - renesas,rzn1-pinctrl:
+      Array of integers representing each 'pin' and it's configuration.
+
+      A 'pin number' does not correspond 1:1 to the hardware manual notion of
+      PL_GPIO directly. Numbers 0...169 are PL_GPIOs, however there is also two
+      extra 170 and 171 that corresponds to the MDIO0 and MDIO1 bus config.
+
+      A 'function' also does not correspond 1:1 to the hardware manual. There
+      are 2 levels of pin muxing, Level 1, level 2 -- to this are added the
+      MDIO configurations.
+
+      Helper macros to ease assembling the pin index and function identifier
+      are provided by the pin controller header file at:
+      <include/dt-bindings/pinctrl/rzn1-pinctrl.h>
+
+Example #1:
+  A simple case configuring only the function for a given 'pin' works as follow:
+	#include <include/dt-bindings/pinctrl/rzn1-pinctrl.h>
+	&pinctrl {
+		pinsuart0: pinsuart0 {
+			renesas,rzn1-pinmux-ids = <
+				RZN1_MUX(103, UART0_I)	/* UART0_TXD */
+				RZN1_MUX(104, UART0_I)	/* UART0_RXD */
+			>;
+		};
+	};
+
+  Note that in this case the other attributes of the pins, pull up/down/none and
+  drive strength, are not changed.
+
+Example #2:
+  Here we also set the pullups on the RXD pin:
+	&pinctrl {
+		pinsuart0: pinsuart0 {
+			renesas,rzn1-pinmux-ids = <
+				RZN1_MUX(103, UART0_I)	/* UART0_TXD */
+				RZN1_MUX_PUP(104, UART0_I)	/* UART0_RXD */
+			>;
+		};
+	};
+  There are many alternative macros to set the pull up/down/none and the drive
+  strength in the rzn1-pinctrl.h header file.
+
+Example #3:
+  The SoC has two configurable MDIO muxes, these can also be configured
+  with this interface using the 'special' MDIO constants:
+
+	&pinctrl {
+		mdio_mux: mdiomux {
+			renesas,rzn1-pinmux-ids = <
+				RZN1_MUX(RZN1_MDIO_BUS0, RZN1_FUNC_MDIO_MUX_MAC0)
+				RZN1_MUX(RZN1_MDIO_BUS1, RZN1_FUNC_MDIO_MUX_SWITCH)
+			>;
+		};
+	};
+  Clearly the pull up/down/none and drive constants will be ignored, even if
+  specified.
diff --git a/include/dt-bindings/pinctrl/rzn1-pinctrl.h b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
new file mode 100644
index 0000000..dc90d35
--- /dev/null
+++ b/include/dt-bindings/pinctrl/rzn1-pinctrl.h
@@ -0,0 +1,191 @@
+/* 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_FUNC_BIT			8
+#define RZN1_MUX_HAS_FUNC_BIT			15
+#define RZN1_MUX_HAS_DRIVE_BIT			16
+#define RZN1_MUX_DRIVE_BIT			17
+#define RZN1_MUX_HAS_PULL_BIT			19
+#define RZN1_MUX_PULL_BIT			20
+
+#define RZN1_MUX_PULL_UP			1
+#define RZN1_MUX_PULL_DOWN			3
+#define RZN1_MUX_PULL_NONE			0
+
+#define RZN1_MUX_DRIVE_4MA			0
+#define RZN1_MUX_DRIVE_6MA			1
+#define RZN1_MUX_DRIVE_8MA			2
+#define RZN1_MUX_DRIVE_12MA			3
+
+#define RZN1_MUX(_gpio, _func) \
+	(((RZN1_FUNC_##_func) << RZN1_MUX_FUNC_BIT) | \
+		(1 << RZN1_MUX_HAS_FUNC_BIT) | \
+		(_gpio))
+
+#define RZN1_MUX_PULL(_pull) \
+		((1 << RZN1_MUX_HAS_PULL_BIT) | \
+		((_pull) << RZN1_MUX_PULL_BIT))
+
+#define RZN1_MUX_DRIVE(_drive) \
+		((1 << RZN1_MUX_HAS_DRIVE_BIT) | \
+		((_drive) << RZN1_MUX_DRIVE_BIT))
+
+#define RZN1_MUX_PUP(_gpio, _func) \
+	(RZN1_MUX(_gpio, _func) | RZN1_MUX_PULL(RZN1_MUX_PULL_UP))
+#define RZN1_MUX_PDOWN(_gpio, _func) \
+	(RZN1_MUX(_gpio, _func) | RZN1_MUX_PULL(RZN1_MUX_PULL_DOWN))
+#define RZN1_MUX_PNONE(_gpio, _func) \
+	(RZN1_MUX(_gpio, _func) | RZN1_MUX_PULL(RZN1_MUX_PULL_NONE))
+
+#define RZN1_MUX_4MA(_gpio, _func) \
+	(RZN1_MUX(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_4MA))
+#define RZN1_MUX_6MA(_gpio, _func) \
+	(RZN1_MUX(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_6MA))
+#define RZN1_MUX_8MA(_gpio, _func) \
+	(RZN1_MUX(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_8MA))
+#define RZN1_MUX_12MA(_gpio, _func) \
+	(RZN1_MUX(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_12MA))
+
+#define RZN1_MUX_PUP_4MA(_gpio, _func) \
+	(RZN1_MUX_PUP(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_4MA))
+#define RZN1_MUX_PUP_6MA(_gpio, _func) \
+	(RZN1_MUX_PUP(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_6MA))
+#define RZN1_MUX_PUP_8MA(_gpio, _func) \
+	(RZN1_MUX_PUP(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_8MA))
+#define RZN1_MUX_PUP_12MA(_gpio, _func) \
+	(RZN1_MUX_PUP(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_12MA))
+
+#define RZN1_MUX_PDOWN_4MA(_gpio, _func) \
+	(RZN1_MUX_PDOWN(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_4MA))
+#define RZN1_MUX_PDOWN_6MA(_gpio, _func) \
+	(RZN1_MUX_PDOWN(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_6MA))
+#define RZN1_MUX_PDOWN_8MA(_gpio, _func) \
+	(RZN1_MUX_PDOWN(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_8MA))
+#define RZN1_MUX_PDOWN_12MA(_gpio, _func) \
+	(RZN1_MUX_PDOWN(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_12MA))
+
+#define RZN1_MUX_PNONE_4MA(_gpio, _func) \
+	(RZN1_MUX_PNONE(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_4MA))
+#define RZN1_MUX_PNONE_6MA(_gpio, _func) \
+	(RZN1_MUX_PNONE(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_6MA))
+#define RZN1_MUX_PNONE_8MA(_gpio, _func) \
+	(RZN1_MUX_PNONE(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_8MA))
+#define RZN1_MUX_PNONE_12MA(_gpio, _func) \
+	(RZN1_MUX_PNONE(_gpio, _func) | RZN1_MUX_DRIVE(RZN1_MUX_DRIVE_12MA))
+
+/*
+ * Use these "gpio" numbers with the RZN1_FUNC_MDIO_MUX* functions
+ * to set the destination of the two MDIO busses.
+ */
+#define RZN1_MDIO_BUS0				170
+#define RZN1_MDIO_BUS1				171
+
+/*
+ * This can be used to set pullups/down/driver for pins without changing
+ * any function that might have already been set
+ */
+#define RZN1_FUNC_NONE				0xff
+
+/*
+ * 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_LEVEL2_OFFSET, and the Level 1 mux will be
+ *         set accordingly.
+ * 72...79 are for the 2 MUDIO muxes for "GPIO" 170/171. These muxes will
+ *         be set to this value - 72.
+ */
+#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_LEVEL2_OFFSET			10	/* I'm Special */
+#define RZN1_FUNC_HIGHZ1			10
+#define RZN1_FUNC_ETHERCAT			11
+#define RZN1_FUNC_SERCOS3			12
+#define RZN1_FUNC_SDIO_E			13
+#define RZN1_FUNC_ETH_MDIO			14
+#define RZN1_FUNC_ETH_MDIO_E1			15
+#define RZN1_FUNC_USB				16
+#define RZN1_FUNC_MSEBIM_E			17
+#define RZN1_FUNC_MSEBIS_E			18
+#define RZN1_FUNC_RSV				19
+#define RZN1_FUNC_RSV_E				20
+#define RZN1_FUNC_RSV_E1			21
+#define RZN1_FUNC_UART0_I			22
+#define RZN1_FUNC_UART0_I_E			23
+#define RZN1_FUNC_UART1_I			24
+#define RZN1_FUNC_UART1_I_E			25
+#define RZN1_FUNC_UART2_I			26
+#define RZN1_FUNC_UART2_I_E			27
+#define RZN1_FUNC_UART0				28
+#define RZN1_FUNC_UART0_E			29
+#define RZN1_FUNC_UART1				30
+#define RZN1_FUNC_UART1_E			31
+#define RZN1_FUNC_UART2				32
+#define RZN1_FUNC_UART2_E			33
+#define RZN1_FUNC_UART3				34
+#define RZN1_FUNC_UART3_E			35
+#define RZN1_FUNC_UART4				36
+#define RZN1_FUNC_UART4_E			37
+#define RZN1_FUNC_UART5				38
+#define RZN1_FUNC_UART5_E			39
+#define RZN1_FUNC_UART6				40
+#define RZN1_FUNC_UART6_E			41
+#define RZN1_FUNC_UART7				42
+#define RZN1_FUNC_UART7_E			43
+#define RZN1_FUNC_SPI0_M			44
+#define RZN1_FUNC_SPI0_M_E			45
+#define RZN1_FUNC_SPI1_M			46
+#define RZN1_FUNC_SPI1_M_E			47
+#define RZN1_FUNC_SPI2_M			48
+#define RZN1_FUNC_SPI2_M_E			49
+#define RZN1_FUNC_SPI3_M			50
+#define RZN1_FUNC_SPI3_M_E			51
+#define RZN1_FUNC_SPI4_S			52
+#define RZN1_FUNC_SPI4_S_E			53
+#define RZN1_FUNC_SPI5_S			54
+#define RZN1_FUNC_SPI5_S_E			55
+#define RZN1_FUNC_SGPIO0_M			56
+#define RZN1_FUNC_SGPIO1_M			57
+#define RZN1_FUNC_GPIO				58
+#define RZN1_FUNC_CAN				59
+#define RZN1_FUNC_I2C				60
+#define RZN1_FUNC_SAFE				61
+#define RZN1_FUNC_PTO_PWM			62
+#define RZN1_FUNC_PTO_PWM1			63
+#define RZN1_FUNC_PTO_PWM2			64
+#define RZN1_FUNC_PTO_PWM3			65
+#define RZN1_FUNC_PTO_PWM4			66
+#define RZN1_FUNC_DELTA_SIGMA			67
+#define RZN1_FUNC_SGPIO2_M			68
+#define RZN1_FUNC_SGPIO3_M			69
+#define RZN1_FUNC_SGPIO4_S			70
+#define RZN1_FUNC_MAC_MTIP_SWITCH		71
+/* These correspond to the functions used for the two MDIO muxes. */
+#define RZN1_FUNC_MDIO_MUX_HIGHZ		72
+#define RZN1_FUNC_MDIO_MUX_MAC0			73
+#define RZN1_FUNC_MDIO_MUX_MAC1			74
+#define RZN1_FUNC_MDIO_MUX_ECAT			75
+#define RZN1_FUNC_MDIO_MUX_S3_MDIO0		76
+#define RZN1_FUNC_MDIO_MUX_S3_MDIO1		77
+#define RZN1_FUNC_MDIO_MUX_HWRTOS		78
+#define RZN1_FUNC_MDIO_MUX_SWITCH		79
+#define RZN1_FUNC_MAX				80
+
+#endif /* __DT_BINDINGS_RZN1_PINCTRL_H */
-- 
2.7.4

[PATCH v2 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver

[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>
---
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 | 844 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 855 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-rzn1.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index e86752b..e524eb1 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 46ef9bd..d07f9a2 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 0000000..411d82e
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-rzn1.c
@@ -0,0 +1,844 @@
+// 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/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include "core.h"
+
+/*
+ * The pinmux hardware has two levels. The first level functions goes from
+ * 0 to 9, and the level 1 mode '15' (0xf) specifies that the second level
+ * of pinmux should be used instead, that level has a lot more options,
+ * and goes from 0 to ~60.
+ *
+ * For Linux, we've compounded both numbers together, so 0 to 9 is level 1,
+ * and anything higher is in fact 10 + level 2 number, so we end up with one
+ * value from 0 to 70 or so.
+ *
+ * There are 170 configurable pins (called PL_GPIO in the datasheet).
+ *
+ * Furthermore, the two MDIO outputs also have a mux each, that can be set
+ * to 8 different values (including highz as well).
+ *
+ * So, for Linux, we also made up to extra "GPIOs" 170 and 171, and also added
+ * extra functions to match their mux. This allow the device tree to be
+ * completely transparent to these subtleties.
+ */
+
+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. pinctrl forces us to maintain such an array
+ * @pins: array of pins
+ */
+struct rzn1_pin_group {
+	const char *name;
+	const char *func;
+	unsigned int npins;
+	u32 *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;
+
+	struct rzn1_pin_group *groups;
+	unsigned int ngroups, maxgroups;
+
+	struct rzn1_pmx_func *functions;
+	unsigned int nfunctions;
+};
+
+#define RZN1_PINS_PROP "renesas,rzn1-pinmux-ids"
+
+#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),
+	PINCTRL_PIN(170, "mdio0"), PINCTRL_PIN(171, "mdio1")
+};
+
+/* Field positions and masks in the pinmux registers */
+#define RZN1_L1_PIN_DRIVE_STRENGTH	10
+#define RZN1_L1_PIN_PULL		8
+#define RZN1_FUNCTION			0
+#define RZN1_L1_FUNC_MASK		0xf
+#define RZN1_L1_FUNCTION_L2		0xf
+
+enum {
+	MDIO_MUX_HIGHZ = 0,
+	MDIO_MUX_MAC0,
+	MDIO_MUX_MAC1,
+	MDIO_MUX_ECAT,
+	MDIO_MUX_S3_MDIO0,
+	MDIO_MUX_S3_MDIO1,
+	MDIO_MUX_HWRTOS,
+	MDIO_MUX_SWITCH,
+};
+
+struct rzn1_pin_desc {
+	u32	pin : 8, func : 7, has_func : 1, has_drive : 1,
+		drive : 2, has_pull : 1, pull : 2;
+};
+
+/*
+ * Breaks down the configuration word (as present in the DT) into
+ * a manageable structural description
+ */
+static void rzn1_get_pin_desc_from_config(struct rzn1_pinctrl *ipctl,
+					  u32 pin_config,
+					  struct rzn1_pin_desc *o)
+{
+	struct rzn1_pin_desc p = {
+		.pin = pin_config,
+		.func = pin_config >> RZN1_MUX_FUNC_BIT,
+		.has_func = pin_config >> RZN1_MUX_HAS_FUNC_BIT,
+		.has_drive = pin_config >> RZN1_MUX_HAS_DRIVE_BIT,
+		.drive = pin_config >> RZN1_MUX_DRIVE_BIT,
+		.has_pull = pin_config >> RZN1_MUX_HAS_PULL_BIT,
+		.pull = pin_config >> RZN1_MUX_PULL_BIT,
+	};
+
+	if (o)
+		*o = p;
+}
+
+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)
+{
+	dev_info(ipctl->dev, "setting mdio %d to 0x%x\n", mdio, func);
+
+	rzn1_hw_set_lock(ipctl, LOCK_LEVEL2, LOCK_LEVEL2);
+	writel(func, &ipctl->lev2->l2_mdio[mdio]);
+	rzn1_hw_set_lock(ipctl, LOCK_LEVEL2, 0);
+}
+
+/*
+ * 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_parameters(struct rzn1_pinctrl *ipctl,
+				      u32 pin_config, u8 use_locks)
+{
+	struct rzn1_pin_desc p;
+	u32 l1_cache;
+	u32 l2_cache;
+	u32 l1;
+	u32 l2;
+
+	rzn1_get_pin_desc_from_config(ipctl, pin_config, &p);
+
+	if (p.pin >= RZN1_MDIO_BUS0 && p.pin <= RZN1_MDIO_BUS1) {
+		if (p.has_func && p.func >= RZN1_FUNC_MDIO_MUX_HIGHZ &&
+		    p.func <= RZN1_FUNC_MDIO_MUX_SWITCH) {
+			p.pin -= RZN1_MDIO_BUS0;
+			p.func -= RZN1_FUNC_MDIO_MUX_HIGHZ;
+			dev_dbg(ipctl->dev, "MDIO MUX[%d] set to %d\n",
+				p.pin, p.func);
+			rzn1_pinctrl_mdio_select(ipctl, p.pin, p.func);
+		} else {
+			dev_warn(ipctl->dev, "MDIO[%d] Invalid configuration: %d\n",
+				 p.pin - RZN1_MDIO_BUS0, p.func);
+			return -EINVAL;
+		}
+
+		return 0;
+	}
+
+	/* Note here, we do not allow anything past the MDIO Mux values */
+	if (p.pin >= ARRAY_SIZE(ipctl->lev1->conf) ||
+	    p.func >= RZN1_FUNC_MDIO_MUX_HIGHZ)
+		return -EINVAL;
+
+	l1 = readl(&ipctl->lev1->conf[p.pin]);
+	l1_cache = l1;
+	l2 = readl(&ipctl->lev2->conf[p.pin]);
+	l2_cache = l2;
+
+	if (p.has_drive) {
+		l1 &= ~(0x3 << RZN1_L1_PIN_DRIVE_STRENGTH);
+		l1 |= (p.drive << RZN1_L1_PIN_DRIVE_STRENGTH);
+	}
+
+	if (p.has_pull) {
+		l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
+		l1 |= (p.pull << RZN1_L1_PIN_PULL);
+	}
+
+	if (p.has_func) {
+		if (p.func < RZN1_FUNC_LEVEL2_OFFSET) {
+			l1 &= ~(RZN1_L1_FUNC_MASK << RZN1_FUNCTION);
+			l1 |= (p.func << RZN1_FUNCTION);
+		} else {
+			l1 &= ~(RZN1_L1_FUNC_MASK << RZN1_FUNCTION);
+			l1 |= (RZN1_L1_FUNCTION_L2 << RZN1_FUNCTION);
+
+			l2 = p.func - RZN1_FUNC_LEVEL2_OFFSET;
+		}
+	}
+
+	/* If either configuration changes, we update both anyway */
+	if (l1 != l1_cache || l2 != l2_cache) {
+		rzn1_hw_set_lock(ipctl, use_locks, LOCK_ALL);
+		writel(l1, &ipctl->lev1->conf[p.pin]);
+		writel(l2, &ipctl->lev2->conf[p.pin]);
+		rzn1_hw_set_lock(ipctl, use_locks, 0);
+	}
+
+	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;
+}
+
+static void rzn1_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+			      unsigned int offset)
+{
+	seq_printf(s, "%s", dev_name(pctldev->dev));
+}
+
+static int rzn1_dt_node_to_map(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;
+	struct pinctrl_map *new_map;
+	int map_num = 2;
+
+	/*
+	 * First find the group of this node and check if we need create
+	 * config maps for pins
+	 */
+	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;
+	}
+
+	new_map = devm_kmalloc_array(ipctl->dev, map_num, sizeof(*new_map),
+				     GFP_KERNEL);
+	if (!new_map)
+		return -ENOMEM;
+
+	*map = new_map;
+	*num_maps = map_num;
+
+	new_map[0].type = PIN_MAP_TYPE_MUX_GROUP;
+	new_map[0].data.mux.function = grp->func;
+	new_map[0].data.mux.group = grp->name;
+
+	new_map[1].type = PIN_MAP_TYPE_CONFIGS_GROUP;
+	new_map[1].data.configs.group_or_pin = grp->name;
+	new_map[1].data.configs.configs = (unsigned long *)grp->pin_ids;
+	new_map[1].data.configs.num_configs = grp->npins;
+
+	dev_dbg(pctldev->dev, "maps: function %s group %s (%d pins)\n",
+		(*map)->data.mux.function, (*map)->data.mux.group,
+		grp->npins);
+
+	return 0;
+}
+
+static void rzn1_dt_free_map(struct pinctrl_dev *pctldev,
+			     struct pinctrl_map *map, unsigned int num_maps)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+
+	devm_kfree(ipctl->dev, map);
+}
+
+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,
+	.pin_dbg_show = rzn1_pin_dbg_show,
+	.dt_node_to_map = rzn1_dt_node_to_map,
+	.dt_free_map = rzn1_dt_free_map,
+};
+
+static int rzn1_pmx_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;
+
+	/*
+	 * Configure the mux mode for each pin in the group for a specific
+	 * function.
+	 */
+	grp = &ipctl->groups[group];
+
+	dev_dbg(ipctl->dev, "enable function %s(%d) group %s(%d)\n",
+		ipctl->functions[selector].name, selector, grp->name, group);
+	/*
+	 * There's not much to do here as the individual pin callback is going
+	 * to be called anyway
+	 */
+
+	return 0;
+}
+
+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 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_pmx_set_mux,
+};
+
+static int rzn1_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin_id,
+			    unsigned long *config)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+
+	pin_id &= 0xff;
+	if (pin_id >= ARRAY_SIZE(ipctl->lev1->conf))
+		return -EINVAL;
+
+	*config = readl(&ipctl->lev1->conf[pin_id]) & 0xf;
+	if (*config == 0xf)
+		*config = (readl(&ipctl->lev2->conf[pin_id]) & 0x3f) +
+				RZN1_FUNC_LEVEL2_OFFSET;
+	*config = (*config << RZN1_MUX_FUNC_BIT) | pin_id;
+
+	return 0;
+}
+
+static int rzn1_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin_id,
+			    unsigned long *configs, unsigned int num_configs)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	int i;
+
+	dev_dbg(ipctl->dev, "pinconf set pin %s (%d configs)\n",
+		rzn1_pins[pin_id].name, num_configs);
+
+	for (i = 0; i < num_configs; i++)
+		rzn1_set_hw_pin_parameters(ipctl, configs[i], LOCK_ALL);
+
+	return 0;
+}
+
+static int rzn1_pin_config_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 i;
+
+	/*
+	 * Configure the mux mode for each pin in the group for a specific
+	 * function.
+	 */
+	dev_dbg(ipctl->dev, "group set %s selector:%d configs:%p/%d\n",
+		grp->name, selector, configs, num_configs);
+
+	rzn1_hw_set_lock(ipctl, LOCK_ALL, LOCK_ALL);
+	for (i = 0; i < num_configs; i++)
+		rzn1_set_hw_pin_parameters(ipctl, configs[i], 0);
+	rzn1_hw_set_lock(ipctl, LOCK_ALL, 0);
+
+	return 0;
+}
+
+static void rzn1_pinconf_dbg_show(struct pinctrl_dev *pctldev,
+				  struct seq_file *s, unsigned int pin_id)
+{
+	unsigned long config = pin_id;
+
+	seq_printf(s, "0x%lx", config);
+}
+
+static void rzn1_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
+					struct seq_file *s,
+					unsigned int group)
+{
+	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
+	struct rzn1_pin_group *grp;
+	unsigned long config;
+	const char *name;
+	int i, ret;
+
+	if (group > ipctl->ngroups)
+		return;
+
+	seq_puts(s, "\n");
+	grp = &ipctl->groups[group];
+	for (i = 0; i < grp->npins; i++) {
+		name = pin_get_name(pctldev, grp->pin_ids[i] & 0xff);
+		ret = rzn1_pinconf_get(pctldev, grp->pin_ids[i], &config);
+		if (ret)
+			return;
+
+		seq_printf(s, "%s: 0x%lx", name, config);
+	}
+}
+
+static const struct pinconf_ops rzn1_pinconf_ops = {
+	.pin_config_get = rzn1_pinconf_get,
+	.pin_config_set = rzn1_pinconf_set,
+	.pin_config_group_set = rzn1_pin_config_group_set,
+	.pin_config_dbg_show = rzn1_pinconf_dbg_show,
+	.pin_config_group_dbg_show = rzn1_pinconf_group_dbg_show,
+};
+
+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;
+	int i;
+
+	dev_dbg(ipctl->dev, "%s: %s\n", __func__, np->name);
+
+	/* Initialise group */
+	grp->name = np->name;
+
+	/*
+	 * The binding format is
+	 *	renesas,rzn1-pinmux-ids = <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 %s\n",
+			np->full_name);
+		return -EINVAL;
+	}
+
+	/* We do not check return since it's safe node passed down */
+	if (!size) {
+		dev_err(ipctl->dev, "Invalid " RZN1_PINS_PROP " in node %s\n",
+			np->full_name);
+		return -EINVAL;
+	}
+
+	grp->npins = size / sizeof(list[0]);
+	if (!grp->npins)
+		return 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;
+	}
+
+	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;
+
+	dev_dbg(ipctl->dev, "parse function(%d): %s\n", index, np->name);
+
+	func = &ipctl->functions[index];
+
+	/* Initialise function */
+	func->name = np->name;
+	func->num_groups = rzn1_pinctrl_count_function_groups(np);
+	dev_dbg(ipctl->dev, "function %s has %d groups\n",
+		np->name, func->num_groups);
+	if (func->num_groups == 0) {
+		dev_err(ipctl->dev, "no groups defined in %s\n", np->full_name);
+		return -EINVAL;
+	}
+
+	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;
+		if (ret > 0) {
+			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;
+		if (ret > 0) {
+			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;
+	u32 nfuncs = 0;
+	u32 i = 0;
+	int ret;
+
+	nfuncs = of_get_child_count(np);
+	if (nfuncs <= 0) {
+		dev_err(&pdev->dev, "no functions defined\n");
+		return -EINVAL;
+	}
+
+	ipctl->nfunctions = nfuncs;
+	ipctl->functions = devm_kmalloc_array(&pdev->dev, nfuncs,
+					      sizeof(*ipctl->functions),
+					      GFP_KERNEL);
+	if (!ipctl->functions)
+		return -ENOMEM;
+
+	ipctl->ngroups = 0;
+	ipctl->maxgroups = 0;
+	for_each_child_of_node(np, child)
+		ipctl->maxgroups += rzn1_pinctrl_count_function_groups(child);
+
+	ipctl->groups = devm_kmalloc_array(&pdev->dev,
+					   ipctl->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;
+
+	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, "bus");
+	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.7.4

[PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node

[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>
---
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 1bc1f36..282ec11 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.7.4

Re: [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node

[kbuild test robot]

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

Hi Phil,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pinctrl/devel]
[also build test ERROR on v4.19-rc1 next-20180830]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Phil-Edworthy/Renesas-R9A06G032-PINCTRL-Driver/20180831-050708
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
config: arm-mvebu_v5_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> Error: arch/arm/boot/dts/r9a06g032.dtsi:93.23-24 syntax error
   FATAL ERROR: Unable to parse input tree

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23513 bytes --]

RE: [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node

[Phil Edworthy]

On 31 August 2018 01:51 kbuild test robot wrote:
> Hi Phil,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on pinctrl/devel] [also build test ERROR on v4.19-rc1
> next-20180830] [if your patch is applied to the wrong git tree, please drop us
> a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Phil-Edworthy/Renesas-
> R9A06G032-PINCTRL-Driver/20180831-050708
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-
> pinctrl.git devel
> config: arm-mvebu_v5_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-
> tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.2.0 make.cross ARCH=arm
> 
> All errors (new ones prefixed by >>):
> 
> >> Error: arch/arm/boot/dts/r9a06g032.dtsi:93.23-24 syntax error
>    FATAL ERROR: Unable to parse input tree

This error is because the patch depends on a patch from Geert:
"ARM: dts: r9a06g032: Use r9a06g032-sysctrl binding definitions"
https://patchwork.kernel.org/patch/10578707/

BR
Phil

Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[jacopo mondi]

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

Hi Phil,

On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote:
> This implements the pinctrl driver for the RZ/N1 family of devices, including
> the R9A06G032 (RZ/N1D) device.
>
> One area that is likely to be contentious is the use of 'virtual pins' for the
> MDIO pinmuxing. The driver uses two pins (170 and 171) that don't exist on the
> device to configure the MDIO source within the RZ/N1 devices. On these devices,
> there are two Ethernet MACs, a 5-Port Switch, numerous industrial Ethernet
> peripherals, any of which can be the MDIO source. Configuring the MDIO source
> could be done without the virtual pins, e.g. by extending the functions to
> cover all MDIO variants (a total of 32 additional functions), but this would
> allow users to misconfigure individual MDIO pins, rather than assign all MDIO
> pins to a MDIO source. The choice of how to implement this will affect the
> DT bindings.
>
> This series was originally written by Michel Pollet whilst at Renesas, and I
> have taken over this work.
>
> One point from Michel's v1 series:
> "Note, I used renesas,rzn1-pinmux node to specify the pinmux constants,
> and I also don't use some of the properties documented in
> pinctrl-bindings.txt on purpose, as they are too limited for my use
> (I need to be able to set, clear, ignore or reset level, pull up/down
> and function as the pinmux might be set by another OS/core running
> concurently)."
>

I start by saying that I don't know this HW pin controller well, so
I might be missing something, but as commented on the original series from
Micheal, I still don't see why you need a custom property here...

My understanding, looking at this comment and the header provided by
patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that
basically need to control pull-up/down and the output driver strength.

Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt reports
a set of generic pin configuration properties to be applied to a pin
configuration (and multiplexing) pin controller child node that fully
express all (most?) of your needs.

Eg. a pin configuration with pull up applied, using examples from your
cover letter should be expressed as

Your example:
         &pinctrl {
                 pinsuart0: pinsuart0 {
                         renesas,rzn1-pinmux-ids = <
                                 RZN1_MUX(103, UART0_I)  /* UART0_TXD */
                                 RZN1_MUX_PUP(104, UART0_I)      /* UART0_RXD */
                         >;
                 };
         };

Using standard pinctroller bindings pin configuration properties:

         &pinctrl {
                 pinsuart0: uart0  {
                        pinsuart_tx0 {
                                pinmux = <103, UART0_I>;  /* UART0_TXD */
                        };

                        pinsuart_rx0 {
                                 pinmux = <104, UART0_I>; /* UART0_RXD */
                                 bias-pull-up;
                        };
                 };
         };

Is there anything I am missing? Maybe from the interaction with
"another OS/core running concurrently" you mentioned? In this case if
you only have to perform pin configuration (because muxing is handled
already) things are even simpler, just use the pin configuration
bindings, without involving muxing at all:

        &pinctrl {
                pinsuart_conf: uart0 {
                        pins = <103, 104>;
                        bias-pull-up;
                 };
         };

Thanks
  j

> Patch 0003 should really be applied after patch:
>  "ARM: dts: r9a06g032: Correct UART and add all other UARTs", see
>  https://www.spinics.net/lists/arm-kernel/msg673525.html
>
> Main changes:
> 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      |  97 +++
>  arch/arm/boot/dts/r9a06g032.dtsi                   |   8 +
>  drivers/pinctrl/Kconfig                            |  10 +
>  drivers/pinctrl/Makefile                           |   1 +
>  drivers/pinctrl/pinctrl-rzn1.c                     | 844 +++++++++++++++++++++
>  include/dt-bindings/pinctrl/rzn1-pinctrl.h         | 191 +++++
>  6 files changed, 1151 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.7.4
>

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

RE: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Phil Edworthy]

Hi Jacopo,

On 03 September 2018 11:34, jacopo mondi wrote:
> On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote:
> > This implements the pinctrl driver for the RZ/N1 family of devices, including
> > the R9A06G032 (RZ/N1D) device.
> >
> > One area that is likely to be contentious is the use of 'virtual pins' for the
> > MDIO pinmuxing. The driver uses two pins (170 and 171) that don't exist on
> the
> > device to configure the MDIO source within the RZ/N1 devices. On these
> devices,
> > there are two Ethernet MACs, a 5-Port Switch, numerous industrial
> Ethernet
> > peripherals, any of which can be the MDIO source. Configuring the MDIO
> source
> > could be done without the virtual pins, e.g. by extending the functions to
> > cover all MDIO variants (a total of 32 additional functions), but this would
> > allow users to misconfigure individual MDIO pins, rather than assign all
> MDIO
> > pins to a MDIO source. The choice of how to implement this will affect the
> > DT bindings.
> >
> > This series was originally written by Michel Pollet whilst at Renesas, and I
> > have taken over this work.
> >
> > One point from Michel's v1 series:
> > "Note, I used renesas,rzn1-pinmux node to specify the pinmux constants,
> > and I also don't use some of the properties documented in
> > pinctrl-bindings.txt on purpose, as they are too limited for my use
> > (I need to be able to set, clear, ignore or reset level, pull up/down
> > and function as the pinmux might be set by another OS/core running
> > concurently)."
> >
> 
> I start by saying that I don't know this HW pin controller well, so
> I might be missing something, but as commented on the original series from
> Micheal, I still don't see why you need a custom property here...
> 
> My understanding, looking at this comment and the header provided by
> patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that
> basically need to control pull-up/down and the output driver strength.
> 
> Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt reports
> a set of generic pin configuration properties to be applied to a pin
> configuration (and multiplexing) pin controller child node that fully
> express all (most?) of your needs.
> 
> Eg. a pin configuration with pull up applied, using examples from your
> cover letter should be expressed as
> 
> Your example:
>          &pinctrl {
>                  pinsuart0: pinsuart0 {
>                          renesas,rzn1-pinmux-ids = <
>                                  RZN1_MUX(103, UART0_I)  /* UART0_TXD */
>                                  RZN1_MUX_PUP(104, UART0_I)      /* UART0_RXD */
>                          >;
>                  };
>          };
> 
> Using standard pinctroller bindings pin configuration properties:
> 
>          &pinctrl {
>                  pinsuart0: uart0  {
>                         pinsuart_tx0 {
>                                 pinmux = <103, UART0_I>;  /* UART0_TXD */
>                         };
> 
>                         pinsuart_rx0 {
>                                  pinmux = <104, UART0_I>; /* UART0_RXD */
>                                  bias-pull-up;
>                         };
>                  };
>          };
> 
> Is there anything I am missing? Maybe from the interaction with
> "another OS/core running concurrently" you mentioned? In this case if
> you only have to perform pin configuration (because muxing is handled
> already) things are even simpler, just use the pin configuration
> bindings, without involving muxing at all:
> 
>         &pinctrl {
>                 pinsuart_conf: uart0 {
>                         pins = <103, 104>;
>                         bias-pull-up;
>                  };
>          };

Sorry I didn’t address your point.
The only reason we want to use new properties is so the driver can process
dts files that have been generated from an existing PinMux App. That output
is used by VxWorks as well as our out-of-tree Linux port. If that is not a
good enough reason to add new properties, then I can't see any technical
reason not to use the existing bindings.
The use with another OS running on a different core should not be a barrier
as it must not use the same pins as Linux.

Thanks
Phil

> Thanks
>   j
> 
> > Patch 0003 should really be applied after patch:
> >  "ARM: dts: r9a06g032: Correct UART and add all other UARTs", see
> >  https://www.spinics.net/lists/arm-kernel/msg673525.html
> >
> > Main changes:
> > 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      |  97 +++
> >  arch/arm/boot/dts/r9a06g032.dtsi                   |   8 +
> >  drivers/pinctrl/Kconfig                            |  10 +
> >  drivers/pinctrl/Makefile                           |   1 +
> >  drivers/pinctrl/pinctrl-rzn1.c                     | 844 +++++++++++++++++++++
> >  include/dt-bindings/pinctrl/rzn1-pinctrl.h         | 191 +++++
> >  6 files changed, 1151 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.7.4
> >

Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[jacopo mondi]

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

Hi Phil,

On Mon, Sep 03, 2018 at 11:03:18AM +0000, Phil Edworthy wrote:
> Hi Jacopo,
>
> On 03 September 2018 11:34, jacopo mondi wrote:
> > On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote:
> > > This implements the pinctrl driver for the RZ/N1 family of devices, including
> > > the R9A06G032 (RZ/N1D) device.
> > >
> > > One area that is likely to be contentious is the use of 'virtual pins' for the
> > > MDIO pinmuxing. The driver uses two pins (170 and 171) that don't exist on
> > the
> > > device to configure the MDIO source within the RZ/N1 devices. On these
> > devices,
> > > there are two Ethernet MACs, a 5-Port Switch, numerous industrial
> > Ethernet
> > > peripherals, any of which can be the MDIO source. Configuring the MDIO
> > source
> > > could be done without the virtual pins, e.g. by extending the functions to
> > > cover all MDIO variants (a total of 32 additional functions), but this would
> > > allow users to misconfigure individual MDIO pins, rather than assign all
> > MDIO
> > > pins to a MDIO source. The choice of how to implement this will affect the
> > > DT bindings.
> > >
> > > This series was originally written by Michel Pollet whilst at Renesas, and I
> > > have taken over this work.
> > >
> > > One point from Michel's v1 series:
> > > "Note, I used renesas,rzn1-pinmux node to specify the pinmux constants,
> > > and I also don't use some of the properties documented in
> > > pinctrl-bindings.txt on purpose, as they are too limited for my use
> > > (I need to be able to set, clear, ignore or reset level, pull up/down
> > > and function as the pinmux might be set by another OS/core running
> > > concurently)."
> > >
> >
> > I start by saying that I don't know this HW pin controller well, so
> > I might be missing something, but as commented on the original series from
> > Micheal, I still don't see why you need a custom property here...
> >
> > My understanding, looking at this comment and the header provided by
> > patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that
> > basically need to control pull-up/down and the output driver strength.
> >
> > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt reports
> > a set of generic pin configuration properties to be applied to a pin
> > configuration (and multiplexing) pin controller child node that fully
> > express all (most?) of your needs.
> >
> > Eg. a pin configuration with pull up applied, using examples from your
> > cover letter should be expressed as
> >
> > Your example:
> >          &pinctrl {
> >                  pinsuart0: pinsuart0 {
> >                          renesas,rzn1-pinmux-ids = <
> >                                  RZN1_MUX(103, UART0_I)  /* UART0_TXD */
> >                                  RZN1_MUX_PUP(104, UART0_I)      /* UART0_RXD */
> >                          >;
> >                  };
> >          };
> >
> > Using standard pinctroller bindings pin configuration properties:
> >
> >          &pinctrl {
> >                  pinsuart0: uart0  {
> >                         pinsuart_tx0 {
> >                                 pinmux = <103, UART0_I>;  /* UART0_TXD */
> >                         };
> >
> >                         pinsuart_rx0 {
> >                                  pinmux = <104, UART0_I>; /* UART0_RXD */
> >                                  bias-pull-up;
> >                         };
> >                  };
> >          };
> >
> > Is there anything I am missing? Maybe from the interaction with
> > "another OS/core running concurrently" you mentioned? In this case if
> > you only have to perform pin configuration (because muxing is handled
> > already) things are even simpler, just use the pin configuration
> > bindings, without involving muxing at all:
> >
> >         &pinctrl {
> >                 pinsuart_conf: uart0 {
> >                         pins = <103, 104>;
> >                         bias-pull-up;
> >                  };
> >          };
>
> Sorry I didn’t address your point.
> The only reason we want to use new properties is so the driver can process
> dts files that have been generated from an existing PinMux App. That output
> is used by VxWorks as well as our out-of-tree Linux port. If that is not a
> good enough reason to add new properties, then I can't see any technical
> reason not to use the existing bindings.

I see. I step back then and let this to be handled by the pinctrl
subsystem people and maintainer :)

Thanks
   j

> The use with another OS running on a different core should not be a barrier
> as it must not use the same pins as Linux.
>
> Thanks
> Phil
>
> > Thanks
> >   j
> >
> > > Patch 0003 should really be applied after patch:
> > >  "ARM: dts: r9a06g032: Correct UART and add all other UARTs", see
> > >  https://www.spinics.net/lists/arm-kernel/msg673525.html
> > >
> > > Main changes:
> > > 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      |  97 +++
> > >  arch/arm/boot/dts/r9a06g032.dtsi                   |   8 +
> > >  drivers/pinctrl/Kconfig                            |  10 +
> > >  drivers/pinctrl/Makefile                           |   1 +
> > >  drivers/pinctrl/pinctrl-rzn1.c                     | 844 +++++++++++++++++++++
> > >  include/dt-bindings/pinctrl/rzn1-pinctrl.h         | 191 +++++
> > >  6 files changed, 1151 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.7.4
> > >

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

Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Geert Uytterhoeven]

Hi Phil,

On Mon, Sep 3, 2018 at 1:03 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> On 03 September 2018 11:34, jacopo mondi wrote:
> > On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote:
> > > This implements the pinctrl driver for the RZ/N1 family of devices, including
> > > the R9A06G032 (RZ/N1D) device.
> > >
> > > One area that is likely to be contentious is the use of 'virtual pins' for the
> > > MDIO pinmuxing. The driver uses two pins (170 and 171) that don't exist on
> > the
> > > device to configure the MDIO source within the RZ/N1 devices. On these
> > devices,
> > > there are two Ethernet MACs, a 5-Port Switch, numerous industrial
> > Ethernet
> > > peripherals, any of which can be the MDIO source. Configuring the MDIO
> > source
> > > could be done without the virtual pins, e.g. by extending the functions to
> > > cover all MDIO variants (a total of 32 additional functions), but this would
> > > allow users to misconfigure individual MDIO pins, rather than assign all
> > MDIO
> > > pins to a MDIO source. The choice of how to implement this will affect the
> > > DT bindings.
> > >
> > > This series was originally written by Michel Pollet whilst at Renesas, and I
> > > have taken over this work.
> > >
> > > One point from Michel's v1 series:
> > > "Note, I used renesas,rzn1-pinmux node to specify the pinmux constants,
> > > and I also don't use some of the properties documented in
> > > pinctrl-bindings.txt on purpose, as they are too limited for my use
> > > (I need to be able to set, clear, ignore or reset level, pull up/down
> > > and function as the pinmux might be set by another OS/core running
> > > concurently)."
> > >
> >
> > I start by saying that I don't know this HW pin controller well, so
> > I might be missing something, but as commented on the original series from
> > Micheal, I still don't see why you need a custom property here...
> >
> > My understanding, looking at this comment and the header provided by
> > patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that
> > basically need to control pull-up/down and the output driver strength.
> >
> > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt reports
> > a set of generic pin configuration properties to be applied to a pin
> > configuration (and multiplexing) pin controller child node that fully
> > express all (most?) of your needs.
> >
> > Eg. a pin configuration with pull up applied, using examples from your
> > cover letter should be expressed as
> >
> > Your example:
> >          &pinctrl {
> >                  pinsuart0: pinsuart0 {
> >                          renesas,rzn1-pinmux-ids = <
> >                                  RZN1_MUX(103, UART0_I)  /* UART0_TXD */
> >                                  RZN1_MUX_PUP(104, UART0_I)      /* UART0_RXD */
> >                          >;
> >                  };
> >          };
> >
> > Using standard pinctroller bindings pin configuration properties:
> >
> >          &pinctrl {
> >                  pinsuart0: uart0  {
> >                         pinsuart_tx0 {
> >                                 pinmux = <103, UART0_I>;  /* UART0_TXD */
> >                         };
> >
> >                         pinsuart_rx0 {
> >                                  pinmux = <104, UART0_I>; /* UART0_RXD */
> >                                  bias-pull-up;
> >                         };
> >                  };
> >          };
> >
> > Is there anything I am missing? Maybe from the interaction with
> > "another OS/core running concurrently" you mentioned? In this case if
> > you only have to perform pin configuration (because muxing is handled
> > already) things are even simpler, just use the pin configuration
> > bindings, without involving muxing at all:
> >
> >         &pinctrl {
> >                 pinsuart_conf: uart0 {
> >                         pins = <103, 104>;
> >                         bias-pull-up;
> >                  };
> >          };
>
> Sorry I didn’t address your point.
> The only reason we want to use new properties is so the driver can process
> dts files that have been generated from an existing PinMux App. That output
> is used by VxWorks as well as our out-of-tree Linux port. If that is not a
> good enough reason to add new properties, then I can't see any technical
> reason not to use the existing bindings.
> The use with another OS running on a different core should not be a barrier
> as it must not use the same pins as Linux.

Have the VxWorks DT bindings been submitted for review to the devicetree
mailing list?

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

RE: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Phil Edworthy]

Hi Geert,

On 05 September 2018 10:37, Geert Uytterhoeven wrote:
> On Mon, Sep 3, 2018 at 1:03 PM Phil Edworthy wrote:
> > On 03 September 2018 11:34, jacopo mondi wrote:
> > > On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote:
> > > > This implements the pinctrl driver for the RZ/N1 family of
> > > > devices, including the R9A06G032 (RZ/N1D) device.
> > > >
> > > > One area that is likely to be contentious is the use of 'virtual
> > > > pins' for the MDIO pinmuxing. The driver uses two pins (170 and
> > > > 171) that don't exist on
> > > the
> > > > device to configure the MDIO source within the RZ/N1 devices. On
> > > > these
> > > devices,
> > > > there are two Ethernet MACs, a 5-Port Switch, numerous industrial
> > > Ethernet
> > > > peripherals, any of which can be the MDIO source. Configuring the
> > > > MDIO
> > > source
> > > > could be done without the virtual pins, e.g. by extending the
> > > > functions to cover all MDIO variants (a total of 32 additional
> > > > functions), but this would allow users to misconfigure individual
> > > > MDIO pins, rather than assign all
> > > MDIO
> > > > pins to a MDIO source. The choice of how to implement this will
> > > > affect the DT bindings.
> > > >
> > > > This series was originally written by Michel Pollet whilst at
> > > > Renesas, and I have taken over this work.
> > > >
> > > > One point from Michel's v1 series:
> > > > "Note, I used renesas,rzn1-pinmux node to specify the pinmux
> > > > constants, and I also don't use some of the properties documented
> > > > in pinctrl-bindings.txt on purpose, as they are too limited for my
> > > > use (I need to be able to set, clear, ignore or reset level, pull
> > > > up/down and function as the pinmux might be set by another OS/core
> > > > running concurently)."
> > > >
> > >
> > > I start by saying that I don't know this HW pin controller well, so
> > > I might be missing something, but as commented on the original
> > > series from Micheal, I still don't see why you need a custom property
> here...
> > >
> > > My understanding, looking at this comment and the header provided by
> > > patch [1/3] (include/dt-bindings/pinctrl/rzn1-pinctrl.h) is that
> > > basically need to control pull-up/down and the output driver strength.
> > >
> > > Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > > reports a set of generic pin configuration properties to be applied
> > > to a pin configuration (and multiplexing) pin controller child node
> > > that fully express all (most?) of your needs.
> > >
> > > Eg. a pin configuration with pull up applied, using examples from
> > > your cover letter should be expressed as
> > >
> > > Your example:
> > >          &pinctrl {
> > >                  pinsuart0: pinsuart0 {
> > >                          renesas,rzn1-pinmux-ids = <
> > >                                  RZN1_MUX(103, UART0_I)  /* UART0_TXD */
> > >                                  RZN1_MUX_PUP(104, UART0_I)      /* UART0_RXD */
> > >                          >;
> > >                  };
> > >          };
> > >
> > > Using standard pinctroller bindings pin configuration properties:
> > >
> > >          &pinctrl {
> > >                  pinsuart0: uart0  {
> > >                         pinsuart_tx0 {
> > >                                 pinmux = <103, UART0_I>;  /* UART0_TXD */
> > >                         };
> > >
> > >                         pinsuart_rx0 {
> > >                                  pinmux = <104, UART0_I>; /* UART0_RXD */
> > >                                  bias-pull-up;
> > >                         };
> > >                  };
> > >          };
> > >
> > > Is there anything I am missing? Maybe from the interaction with
> > > "another OS/core running concurrently" you mentioned? In this case
> > > if you only have to perform pin configuration (because muxing is
> > > handled
> > > already) things are even simpler, just use the pin configuration
> > > bindings, without involving muxing at all:
> > >
> > >         &pinctrl {
> > >                 pinsuart_conf: uart0 {
> > >                         pins = <103, 104>;
> > >                         bias-pull-up;
> > >                  };
> > >          };
> >
> > Sorry I didn’t address your point.
> > The only reason we want to use new properties is so the driver can
> > process dts files that have been generated from an existing PinMux
> > App. That output is used by VxWorks as well as our out-of-tree Linux
> > port. If that is not a good enough reason to add new properties, then
> > I can't see any technical reason not to use the existing bindings.
> > The use with another OS running on a different core should not be a
> > barrier as it must not use the same pins as Linux.
> 
> Have the VxWorks DT bindings been submitted for review to the devicetree
> mailing list?
I'm not involved with the VxWorks port, but I am pretty sure that they have
not been submitted for review.

Thanks
Phil

Re: [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node

[Simon Horman]

On Fri, Aug 31, 2018 at 08:13:07AM +0000, Phil Edworthy wrote:
> 
> On 31 August 2018 01:51 kbuild test robot wrote:
> > Hi Phil,
> > 
> > Thank you for the patch! Yet something to improve:
> > 
> > [auto build test ERROR on pinctrl/devel] [also build test ERROR on v4.19-rc1
> > next-20180830] [if your patch is applied to the wrong git tree, please drop us
> > a note to help improve the system]
> > 
> > url:    https://github.com/0day-ci/linux/commits/Phil-Edworthy/Renesas-
> > R9A06G032-PINCTRL-Driver/20180831-050708
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-
> > pinctrl.git devel
> > config: arm-mvebu_v5_defconfig (attached as .config)
> > compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-
> > tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         GCC_VERSION=7.2.0 make.cross ARCH=arm
> > 
> > All errors (new ones prefixed by >>):
> > 
> > >> Error: arch/arm/boot/dts/r9a06g032.dtsi:93.23-24 syntax error
> >    FATAL ERROR: Unable to parse input tree
> 
> This error is because the patch depends on a patch from Geert:
> "ARM: dts: r9a06g032: Use r9a06g032-sysctrl binding definitions"
> https://patchwork.kernel.org/patch/10578707/

Thanks Phil,

got it. I would like to wait for the binding to be acked before
applying this patch.

Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Linus Walleij]

On Mon, Sep 3, 2018 at 1:25 PM jacopo mondi <jacopo@jmondi.org> wrote:
> On Mon, Sep 03, 2018 at 11:03:18AM +0000, Phil Edworthy wrote:
> > On 03 September 2018 11:34, jacopo mondi wrote:
> > > On Thu, Aug 30, 2018 at 02:12:52PM +0100, Phil Edworthy wrote:
> > > > This implements the pinctrl driver for the RZ/N1 family of devices, including
> > > > the R9A06G032 (RZ/N1D) device.
(...)
> > > Using standard pinctroller bindings pin configuration properties:
> > >
> > >          &pinctrl {
> > >                  pinsuart0: uart0  {
> > >                         pinsuart_tx0 {
> > >                                 pinmux = <103, UART0_I>;  /* UART0_TXD */
> > >                         };
> > >
> > >                         pinsuart_rx0 {
> > >                                  pinmux = <104, UART0_I>; /* UART0_RXD */
> > >                                  bias-pull-up;
> > >                         };
> > >                  };
> > >          };
> > >
> > > Is there anything I am missing? Maybe from the interaction with
> > > "another OS/core running concurrently" you mentioned? In this case if
> > > you only have to perform pin configuration (because muxing is handled
> > > already) things are even simpler, just use the pin configuration
> > > bindings, without involving muxing at all:
> > >
> > >         &pinctrl {
> > >                 pinsuart_conf: uart0 {
> > >                         pins = <103, 104>;
> > >                         bias-pull-up;
> > >                  };
> > >          };
> >
> > Sorry I didn’t address your point.
> > The only reason we want to use new properties is so the driver can process
> > dts files that have been generated from an existing PinMux App. That output
> > is used by VxWorks as well as our out-of-tree Linux port. If that is not a
> > good enough reason to add new properties, then I can't see any technical
> > reason not to use the existing bindings.
>
> I see. I step back then and let this to be handled by the pinctrl
> subsystem people and maintainer :)

I rely on Geert Uytterhoeven and Laurent Pinchart to tell me
what is best for Renesas.

A good hint is that Geert will merge this driver when finished and
send it to me, so if he's not happy it is unlikely to get merged.
So make sure Geert is happy and I will be happy too.

Geert, as I, likely rely on the DT maintainers to tell me what is
best for DT. But with pinctrl they often pass it back to us.

Yours,
Linus Walleij

Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Linus Walleij]

On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> On 05 September 2018 10:37, Geert Uytterhoeven wrote:

> > Have the VxWorks DT bindings been submitted for review to the devicetree
> > mailing list?

> I'm not involved with the VxWorks port, but I am pretty sure that they have
> not been submitted for review.

We have had other cases where deployments on other OSes have been
establishing bindings (even not very pretty ones).

Examples include typically the Power MacIntosh and the original SPARC
bindings.

Yours,
Linus Walleij

Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Geert Uytterhoeven]

On Mon, Sep 10, 2018 at 9:49 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> > On 05 September 2018 10:37, Geert Uytterhoeven wrote:
>
> > > Have the VxWorks DT bindings been submitted for review to the devicetree
> > > mailing list?
>
> > I'm not involved with the VxWorks port, but I am pretty sure that they have
> > not been submitted for review.
>
> We have had other cases where deployments on other OSes have been
> establishing bindings (even not very pretty ones).
>
> Examples include typically the Power MacIntosh and the original SPARC
> bindings.

Which predated FDT support in Linux by more than a decade, with the
latter predating even p1275?

While I see merit in compatibility with other existing OSes, I prefer not
to deviate from standard DT bindings, if they exist.

Over time, we've even migrated from Renesas-specific DT bindings to
standard DT bindings, in already upstreamed SoC support.

If needed, I guess a script can be written to convert DTS files from the
old to the new syntax?

Thanks!

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

Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Laurent Pinchart]

Hello,

On Monday, 10 September 2018 10:48:58 EEST Linus Walleij wrote:
> On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy wrote:
> > On 05 September 2018 10:37, Geert Uytterhoeven wrote:
> > > Have the VxWorks DT bindings been submitted for review to the devicetree
> > > mailing list?
> > 
> > I'm not involved with the VxWorks port, but I am pretty sure that they
> > have not been submitted for review.
> 
> We have had other cases where deployments on other OSes have been
> establishing bindings (even not very pretty ones).

I hardly see that as a good reason to accept the bindings blindly. If they 
haven't been submitted for review, it's their problem, not ours. Even worse in 
my opinion is the fact that those bindings are used by the "out-of-tree Linux 
port". We've hammered the "upstream first" message for years if not decades, 
if the industry still can't get it, I don't see why we should be the ones 
suffering.

No ack from me for these new bindings, sorry.

> Examples include typically the Power MacIntosh and the original SPARC
> bindings.

-- 
Regards,

Laurent Pinchart

RE: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Phil Edworthy]

Hi Geert, Laurent,

On 10 September 2018 10:42 Geert Uytterhoeven wrote:
> On Mon, Sep 10, 2018 at 9:49 AM Linus Walleij  wrote:
> > On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy wrote:
> > > On 05 September 2018 10:37, Geert Uytterhoeven wrote:
> >
> > > > Have the VxWorks DT bindings been submitted for review to the
> > > > devicetree mailing list?
> >
> > > I'm not involved with the VxWorks port, but I am pretty sure that
> > > they have not been submitted for review.
> >
> > We have had other cases where deployments on other OSes have been
> > establishing bindings (even not very pretty ones).
> >
> > Examples include typically the Power MacIntosh and the original SPARC
> > bindings.
> 
> Which predated FDT support in Linux by more than a decade, with the latter
> predating even p1275?
> 
> While I see merit in compatibility with other existing OSes, I prefer not to
> deviate from standard DT bindings, if they exist.
> 
> Over time, we've even migrated from Renesas-specific DT bindings to
> standard DT bindings, in already upstreamed SoC support.
> 
> If needed, I guess a script can be written to convert DTS files from the old to
> the new syntax?

Thanks for your comments, that is also my preference. Since I have to juggle
effort vs. features I thought it best to send the driver with existing out-of-tree
bindings first just to see.

I'll modify the driver for the standard bindings.

Thanks
Phil

Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver

[Geert Uytterhoeven]

Hi Phil,

On Mon, Sep 10, 2018 at 11:54 AM Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> On 10 September 2018 10:42 Geert Uytterhoeven wrote:
> > On Mon, Sep 10, 2018 at 9:49 AM Linus Walleij  wrote:
> > > On Wed, Sep 5, 2018 at 11:59 AM Phil Edworthy wrote:
> > > > On 05 September 2018 10:37, Geert Uytterhoeven wrote:
> > >
> > > > > Have the VxWorks DT bindings been submitted for review to the
> > > > > devicetree mailing list?
> > >
> > > > I'm not involved with the VxWorks port, but I am pretty sure that
> > > > they have not been submitted for review.
> > >
> > > We have had other cases where deployments on other OSes have been
> > > establishing bindings (even not very pretty ones).
> > >
> > > Examples include typically the Power MacIntosh and the original SPARC
> > > bindings.
> >
> > Which predated FDT support in Linux by more than a decade, with the latter
> > predating even p1275?
> >
> > While I see merit in compatibility with other existing OSes, I prefer not to
> > deviate from standard DT bindings, if they exist.
> >
> > Over time, we've even migrated from Renesas-specific DT bindings to
> > standard DT bindings, in already upstreamed SoC support.
> >
> > If needed, I guess a script can be written to convert DTS files from the old to
> > the new syntax?
>
> Thanks for your comments, that is also my preference. Since I have to juggle
> effort vs. features I thought it best to send the driver with existing out-of-tree
> bindings first just to see.
>
> I'll modify the driver for the standard bindings.

Thank you, much appreciated!

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