* [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver
@ 2018-08-30 13:12 Phil Edworthy
2018-08-30 13:12 ` [PATCH v2 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Phil Edworthy @ 2018-08-30 13:12 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.
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
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v2 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation 2018-08-30 13:12 [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver Phil Edworthy @ 2018-08-30 13:12 ` Phil Edworthy 2018-08-30 13:12 ` [PATCH v2 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver Phil Edworthy ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Phil Edworthy @ 2018-08-30 13:12 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> --- 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver 2018-08-30 13:12 [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver Phil Edworthy 2018-08-30 13:12 ` [PATCH v2 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy @ 2018-08-30 13:12 ` Phil Edworthy 2018-08-30 13:12 ` [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node Phil Edworthy 2018-09-03 10:33 ` [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver jacopo mondi 3 siblings, 0 replies; 18+ messages in thread From: Phil Edworthy @ 2018-08-30 13:12 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> --- 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node 2018-08-30 13:12 [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver Phil Edworthy 2018-08-30 13:12 ` [PATCH v2 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy 2018-08-30 13:12 ` [PATCH v2 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver Phil Edworthy @ 2018-08-30 13:12 ` Phil Edworthy 2018-08-31 0:50 ` kbuild test robot 2018-09-03 10:33 ` [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver jacopo mondi 3 siblings, 1 reply; 18+ messages in thread From: Phil Edworthy @ 2018-08-30 13:12 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> --- 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 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node 2018-08-30 13:12 ` [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node Phil Edworthy @ 2018-08-31 0:50 ` kbuild test robot 2018-08-31 8:13 ` Phil Edworthy 0 siblings, 1 reply; 18+ messages in thread From: kbuild test robot @ 2018-08-31 0:50 UTC (permalink / raw) To: Phil Edworthy Cc: kbuild-all, Geert Uytterhoeven, Laurent Pinchart, Jacopo Mondi, Linus Walleij, Simon Horman, linux-gpio, linux-renesas-soc, linux-kernel, Phil Edworthy [-- 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 --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node 2018-08-31 0:50 ` kbuild test robot @ 2018-08-31 8:13 ` Phil Edworthy 2018-09-06 9:36 ` Simon Horman 0 siblings, 1 reply; 18+ messages in thread From: Phil Edworthy @ 2018-08-31 8:13 UTC (permalink / raw) To: Geert Uytterhoeven, Laurent Pinchart Cc: Jacopo Mondi, Linus Walleij, Simon Horman, linux-gpio, linux-renesas-soc, linux-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node 2018-08-31 8:13 ` Phil Edworthy @ 2018-09-06 9:36 ` Simon Horman 0 siblings, 0 replies; 18+ messages in thread From: Simon Horman @ 2018-09-06 9:36 UTC (permalink / raw) To: Phil Edworthy Cc: Geert Uytterhoeven, Laurent Pinchart, Jacopo Mondi, Linus Walleij, linux-gpio, linux-renesas-soc, linux-kernel 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-08-30 13:12 [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver Phil Edworthy ` (2 preceding siblings ...) 2018-08-30 13:12 ` [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node Phil Edworthy @ 2018-09-03 10:33 ` jacopo mondi 2018-09-03 11:03 ` Phil Edworthy 3 siblings, 1 reply; 18+ messages in thread From: jacopo mondi @ 2018-09-03 10:33 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: 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 --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-03 10:33 ` [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver jacopo mondi @ 2018-09-03 11:03 ` Phil Edworthy 2018-09-03 11:25 ` jacopo mondi 2018-09-05 9:36 ` Geert Uytterhoeven 0 siblings, 2 replies; 18+ messages in thread From: Phil Edworthy @ 2018-09-03 11:03 UTC (permalink / raw) To: jacopo mondi Cc: Geert Uytterhoeven, Laurent Pinchart, Rob Herring, Mark Rutland, Linus Walleij, Simon Horman, linux-gpio, linux-renesas-soc, linux-kernel 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 > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-03 11:03 ` Phil Edworthy @ 2018-09-03 11:25 ` jacopo mondi 2018-09-10 7:45 ` Linus Walleij 2018-09-05 9:36 ` Geert Uytterhoeven 1 sibling, 1 reply; 18+ messages in thread From: jacopo mondi @ 2018-09-03 11:25 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: 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 --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-03 11:25 ` jacopo mondi @ 2018-09-10 7:45 ` Linus Walleij 0 siblings, 0 replies; 18+ messages in thread From: Linus Walleij @ 2018-09-10 7:45 UTC (permalink / raw) To: jacopo, Geert Uytterhoeven Cc: Phil Edworthy, Laurent Pinchart, Rob Herring, Mark Rutland, Simon Horman, open list:GPIO SUBSYSTEM, Linux-Renesas, linux-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-03 11:03 ` Phil Edworthy 2018-09-03 11:25 ` jacopo mondi @ 2018-09-05 9:36 ` Geert Uytterhoeven 2018-09-05 9:58 ` Phil Edworthy 1 sibling, 1 reply; 18+ messages in thread From: Geert Uytterhoeven @ 2018-09-05 9:36 UTC (permalink / raw) To: Phil Edworthy Cc: Jacopo Mondi, Laurent Pinchart, Rob Herring, Mark Rutland, Linus Walleij, Simon Horman, open list:GPIO SUBSYSTEM, Linux-Renesas, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-05 9:36 ` Geert Uytterhoeven @ 2018-09-05 9:58 ` Phil Edworthy 2018-09-10 7:48 ` Linus Walleij 0 siblings, 1 reply; 18+ messages in thread From: Phil Edworthy @ 2018-09-05 9:58 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Jacopo Mondi, Laurent Pinchart, Rob Herring, Mark Rutland, Linus Walleij, Simon Horman, open list:GPIO SUBSYSTEM, Linux-Renesas, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-05 9:58 ` Phil Edworthy @ 2018-09-10 7:48 ` Linus Walleij 2018-09-10 9:42 ` Geert Uytterhoeven 2018-09-10 9:46 ` Laurent Pinchart 0 siblings, 2 replies; 18+ messages in thread From: Linus Walleij @ 2018-09-10 7:48 UTC (permalink / raw) To: Phil Edworthy Cc: Geert Uytterhoeven, jacopo, Laurent Pinchart, Rob Herring, Mark Rutland, Simon Horman, open list:GPIO SUBSYSTEM, Linux-Renesas, linux-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-10 7:48 ` Linus Walleij @ 2018-09-10 9:42 ` Geert Uytterhoeven 2018-09-10 9:54 ` Phil Edworthy 2018-09-10 9:46 ` Laurent Pinchart 1 sibling, 1 reply; 18+ messages in thread From: Geert Uytterhoeven @ 2018-09-10 9:42 UTC (permalink / raw) To: Linus Walleij Cc: Phil Edworthy, Jacopo Mondi, Laurent Pinchart, Rob Herring, Mark Rutland, Simon Horman, open list:GPIO SUBSYSTEM, Linux-Renesas, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-10 9:42 ` Geert Uytterhoeven @ 2018-09-10 9:54 ` Phil Edworthy 2018-09-10 9:58 ` Geert Uytterhoeven 0 siblings, 1 reply; 18+ messages in thread From: Phil Edworthy @ 2018-09-10 9:54 UTC (permalink / raw) To: Geert Uytterhoeven, Laurent Pinchart Cc: Jacopo Mondi, Rob Herring, Mark Rutland, Simon Horman, open list:GPIO SUBSYSTEM, Linux-Renesas, Linus Walleij, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-10 9:54 ` Phil Edworthy @ 2018-09-10 9:58 ` Geert Uytterhoeven 0 siblings, 0 replies; 18+ messages in thread From: Geert Uytterhoeven @ 2018-09-10 9:58 UTC (permalink / raw) To: Phil Edworthy Cc: Laurent Pinchart, Jacopo Mondi, Rob Herring, Mark Rutland, Simon Horman, open list:GPIO SUBSYSTEM, Linux-Renesas, Linus Walleij, Linux Kernel Mailing List 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver 2018-09-10 7:48 ` Linus Walleij 2018-09-10 9:42 ` Geert Uytterhoeven @ 2018-09-10 9:46 ` Laurent Pinchart 1 sibling, 0 replies; 18+ messages in thread From: Laurent Pinchart @ 2018-09-10 9:46 UTC (permalink / raw) To: Linus Walleij Cc: Phil Edworthy, Geert Uytterhoeven, jacopo, Rob Herring, Mark Rutland, Simon Horman, open list:GPIO SUBSYSTEM, Linux-Renesas, linux-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-09-10 9:58 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-30 13:12 [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver Phil Edworthy 2018-08-30 13:12 ` [PATCH v2 1/3] dt-bindings: pinctrl: renesas,rzn1-pinctrl: documentation Phil Edworthy 2018-08-30 13:12 ` [PATCH v2 2/3] pinctrl: renesas: Renesas RZ/N1 pinctrl driver Phil Edworthy 2018-08-30 13:12 ` [PATCH v2 3/3] ARM: dts: r9a06g032: Add pinctrl node Phil Edworthy 2018-08-31 0:50 ` kbuild test robot 2018-08-31 8:13 ` Phil Edworthy 2018-09-06 9:36 ` Simon Horman 2018-09-03 10:33 ` [PATCH v2 0/3] Renesas R9A06G032 PINCTRL Driver jacopo mondi 2018-09-03 11:03 ` Phil Edworthy 2018-09-03 11:25 ` jacopo mondi 2018-09-10 7:45 ` Linus Walleij 2018-09-05 9:36 ` Geert Uytterhoeven 2018-09-05 9:58 ` Phil Edworthy 2018-09-10 7:48 ` Linus Walleij 2018-09-10 9:42 ` Geert Uytterhoeven 2018-09-10 9:54 ` Phil Edworthy 2018-09-10 9:58 ` Geert Uytterhoeven 2018-09-10 9:46 ` Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).