[PATCH v3 0/8] Renesas RZ/A1 pin and gpio controller

[PATCH v3 0/8] Renesas RZ/A1 pin and gpio controller

[Jacopo Mondi]

Hello!

Third round for Renesas RZ/A1 PFC gpio and pin controller.

V3 fixes review comments on grammar and spelling by Geert.
Some other fixes from Geert's review as update of function argument names
to reflect the function purpose (s/offset/[pin|bit]), use irqsave/irqrestore
version of spinlocks locking/unlocking methods and other style fixes.

Still an open question on the use of "pins" or "renesas,pins" property in
device tree bindings:
https://www.spinics.net/lists/linux-renesas-soc/msg12765.html

Tested on Genmai and RSK boards with SCIF, I2c and GPIO (genmai) and SCIF, I2c,
SDHI and ethernet on RSK (thanks Chris).

Thanks
   j

v1 -> v2:
- change pin configuration flags as suggested by Chris
- gpio set direction function fixed as suggested by Chris
- add some more example on pin configuration flag usage to dt-binding doc
- fix gpio-controller names to remove unit address as suggested by Geert
- some comments chopped here and there to make the driver less verbose

v2 -> v3:
- fix grammar and syntax in comment and documentation
- fix code style (reverse xmas tree ordering in variable declaration)
- use irqsave/irqrestore in spinlock lock/unlock
- use devm_ version of kasprintf (memory returned was not properly free)
- use bitops.h operation ffs and fls to make sure a single bit is set in pmx
  mask
- Add Geert's reviewed-by to DTS patches

Jacopo Mondi (8):
  pinctrl: Renesas RZ/A1 pin and gpio controller
  dt-bindings: pinctrl: Add RZ/A1 bindings doc
  arm: dts: dt-bindings: Add Renesas RZ pinctrl header
  arm: dts: r7s72100: Add pin controller node
  arm: dts: genmai: Add SCIF2 pin group
  arm: dts: genmai: Add RIIC2 pin group
  arm: dts: genmai: Add user led device nodes
  arm: dts: genmai: Add ethernet pin group

 .../bindings/pinctrl/renesas,rza1-pinctrl.txt      | 143 +++
 arch/arm/boot/dts/r7s72100-genmai.dts              |  68 ++
 arch/arm/boot/dts/r7s72100.dtsi                    |  80 ++
 drivers/pinctrl/Kconfig                            |  10 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/pinctrl-rza1.c                     | 961 +++++++++++++++++++++
 include/dt-bindings/pinctrl/r7s72100-pinctrl.h     |  36 +
 7 files changed, 1299 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
 create mode 100644 drivers/pinctrl/pinctrl-rza1.c
 create mode 100644 include/dt-bindings/pinctrl/r7s72100-pinctrl.h

-- 
2.7.4

[PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

[Jacopo Mondi]

Add combined gpio and pin controller driver for Renesas RZ/A1
r7s72100 SoC.

Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
---
 drivers/pinctrl/Kconfig        |  10 +
 drivers/pinctrl/Makefile       |   1 +
 drivers/pinctrl/pinctrl-rza1.c | 961 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 972 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-rza1.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 8f8c2af..c9b55b9 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -163,6 +163,16 @@ config PINCTRL_ROCKCHIP
 	select GENERIC_IRQ_CHIP
 	select MFD_SYSCON
 
+config PINCTRL_RZA1
+	bool "Renesas RZ/A1 gpio and pinctrl driver"
+	depends on OF
+	depends on ARCH_R7S72100 || COMPILE_TEST
+	select GENERIC_PINCTRL_GROUPS
+	select GENERIC_PINMUX_FUNCTIONS
+	select GENERIC_PINCONF
+	help
+	  This selects pinctrl driver for Renesas RZ/A1 platforms.
+
 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 a251f43..0c2328d2 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_PINCTRL_PALMAS)	+= pinctrl-palmas.o
 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_SINGLE)	+= pinctrl-single.o
 obj-$(CONFIG_PINCTRL_SIRF)	+= sirf/
 obj-$(CONFIG_PINCTRL_SX150X)	+= pinctrl-sx150x.o
diff --git a/drivers/pinctrl/pinctrl-rza1.c b/drivers/pinctrl/pinctrl-rza1.c
new file mode 100644
index 0000000..0a4fc41
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-rza1.c
@@ -0,0 +1,961 @@
+/*
+ * Combined GPIO and pin controller support for Renesas RZ/A1 (r7s72100) SoC
+ *
+ * Copyright (C) 2017 Jacopo Mondi
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+/*
+ * This pincontroller/gpio combined driver support Renesas devices of RZ/A1
+ * family.
+ * This includes SoCs which are sub- or super- sets of this particular line,
+ * as RZ/A1H (r7s721000), RZ/A1M (r7s721010) and RZ/A1L (r7s721020) are.
+ */
+
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/slab.h>
+
+#include "core.h"
+#include "devicetree.h"
+#include "pinmux.h"
+
+#define DRIVER_NAME			"pinctrl-rza1"
+
+#define RZA1_PINMUX_OF_ARGS		2
+
+#define P_REG				0x0000
+#define PPR_REG				0x0200
+#define PM_REG				0x0300
+#define PMC_REG				0x0400
+#define PFC_REG				0x0500
+#define PFCE_REG			0x0600
+#define PFCEA_REG			0x0a00
+#define PIBC_REG			0x4000
+#define PBDC_REG			0x4100
+#define PIPC_REG			0x4200
+#define RZA1_ADDR(mem, reg, port)	((mem) + (reg) + ((port) * 4))
+
+#define RZA1_NPORTS			12
+#define RZA1_PINS_PER_PORT		16
+#define RZA1_NPINS			(RZA1_PINS_PER_PORT * RZA1_NPORTS)
+#define RZA1_PIN_TO_PORT(pin)		((pin) / RZA1_PINS_PER_PORT)
+#define RZA1_PIN_TO_OFFSET(pin)		((pin) % RZA1_PINS_PER_PORT)
+
+/*
+ * Be careful here: the pin configuration subnodes in device tree enumerate
+ * alternate functions from 1 to 8; subtract 1 before using macros so to match
+ * register configuration which expects numbers from 0 to 7 instead.
+ */
+#define MUX_FUNC_OFFS			3
+#define MUX_FUNC_MASK			(BIT(MUX_FUNC_OFFS) - 1)
+#define MUX_FUNC_PFC_MASK		BIT(0)
+#define MUX_FUNC_PFCE_MASK		BIT(1)
+#define MUX_FUNC_PFCEA_MASK		BIT(2)
+#define MUX_CONF_BIDIR			BIT(0)
+#define MUX_CONF_SWIO_INPUT		BIT(1)
+#define MUX_CONF_SWIO_OUTPUT		BIT(2)
+
+/**
+ * rza1_pin_conf - describes a pin position, id, mux config and output value
+ *
+ * @id: the pin identifier from 0 to RZA1_NPINS
+ * @port: the port where pin sits on
+ * @offset: pin offset in the port
+ * @mux: alternate function configuration settings
+ * @value: output value to set the pin to
+ */
+struct rza1_pin_conf {
+	u16 id;
+	u8 port;
+	u8 offset;
+	u8 mux_conf;
+	u8 value;
+};
+
+/**
+ * rza1_port - describes a pin port
+ *
+ * This is mostly useful to lock register writes per-bank and not globally.
+ *
+ * @lock: protect access to HW registers
+ * @id: port number
+ * @base: logical address base
+ * @pins: pins sitting on this port
+ */
+struct rza1_port {
+	spinlock_t lock;
+	unsigned int id;
+	void __iomem *base;
+	struct pinctrl_pin_desc *pins;
+};
+
+/**
+ * rza1_pinctrl - RZ pincontroller device
+ *
+ * @dev: parent device structure
+ * @mutex: protect [pinctrl|pinmux]_generic functions
+ * @base: logical address base
+ * @nports: number of pin controller ports
+ * @ports: pin controller banks
+ * @ngpiochips: number of gpio chips
+ * @gpio_ranges: gpio ranges for pinctrl core
+ * @pins: pin array for pinctrl core
+ * @desc: pincontroller desc for pinctrl core
+ * @pctl: pinctrl device
+ */
+struct rza1_pinctrl {
+	struct device *dev;
+
+	struct mutex mutex;
+
+	void __iomem *base;
+
+	unsigned int nport;
+	struct rza1_port *ports;
+
+	unsigned int ngpiochips;
+
+	struct pinctrl_gpio_range *gpio_ranges;
+	struct pinctrl_pin_desc *pins;
+	struct pinctrl_desc desc;
+	struct pinctrl_dev *pctl;
+};
+
+/* ----------------------------------------------------------------------------
+ * RZ/A1 SoC operations
+ */
+
+/**
+ * rza1_set_bit() - un-locked set/clear a single bit in pin configuration
+ *		    registers
+ */
+static inline void rza1_set_bit(struct rza1_port *port, unsigned int reg,
+				unsigned int bit, bool set)
+{
+	void __iomem *mem = RZA1_ADDR(port->base, reg, port->id);
+	u16 val = ioread16(mem);
+
+	if (set)
+		val |= BIT(bit);
+	else
+		val &= ~BIT(bit);
+
+	iowrite16(val, mem);
+}
+
+static inline int rza1_get_bit(struct rza1_port *port, unsigned int reg,
+			       unsigned int bit)
+{
+	void __iomem *mem = RZA1_ADDR(port->base, reg, port->id);
+
+	return ioread16(mem) & BIT(bit);
+}
+
+/**
+ * rza1_pin_reset() - reset a pin to default initial state
+ *
+ * Reset pin state disabling input buffer and bi-directional control,
+ * and configure it as input port.
+ * Note that pin is now configured with direction as input but with input
+ * buffer disabled. This implies the pin value cannot be read in this state.
+ *
+ * @port: port where pin sits on
+ * @pin: pin offset
+ */
+static void rza1_pin_reset(struct rza1_port *port, unsigned int pin)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+	rza1_set_bit(port, PIBC_REG, pin, 0);
+	rza1_set_bit(port, PBDC_REG, pin, 0);
+
+	rza1_set_bit(port, PM_REG, pin, 1);
+	rza1_set_bit(port, PMC_REG, pin, 0);
+	rza1_set_bit(port, PIPC_REG, pin, 0);
+	spin_unlock_irqrestore(&port->lock, irqflags);
+}
+
+static inline int rza1_pin_get_direction(struct rza1_port *port,
+					 unsigned int pin)
+{
+	unsigned long irqflags;
+	int input;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+	input = rza1_get_bit(port, PM_REG, pin);
+	spin_unlock_irqrestore(&port->lock, irqflags);
+
+	return input;
+}
+
+/**
+ * rza1_pin_set_direction() - set I/O direction on a pin in port mode
+ *
+ * When running in output port mode keep PBDC enabled to allow reading the
+ * pin value from PPR.
+ *
+ * @port: port where pin sits on
+ * @pin: pin offset
+ * @input: input enable/disable flag
+ */
+static inline void rza1_pin_set_direction(struct rza1_port *port,
+					  unsigned int pin, bool input)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+
+	rza1_set_bit(port, PIBC_REG, pin, 1);
+	if (input) {
+		rza1_set_bit(port, PM_REG, pin, 1);
+		rza1_set_bit(port, PBDC_REG, pin, 0);
+	} else {
+		rza1_set_bit(port, PM_REG, pin, 0);
+		rza1_set_bit(port, PBDC_REG, pin, 1);
+	}
+
+	spin_unlock_irqrestore(&port->lock, irqflags);
+}
+
+static inline void rza1_pin_set(struct rza1_port *port, unsigned int pin,
+				unsigned int value)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+	rza1_set_bit(port, P_REG, pin, !!value);
+	spin_unlock_irqrestore(&port->lock, irqflags);
+}
+
+static inline int rza1_pin_get(struct rza1_port *port, unsigned int pin)
+{
+	unsigned long irqflags;
+	int val;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+	val = rza1_get_bit(port, PPR_REG, pin);
+	spin_unlock_irqrestore(&port->lock, irqflags);
+
+	return val;
+}
+
+/**
+ * rza1_alternate_function_conf() - configure pin in alternate function mode
+ *
+ * @pinctrl: RZ/A1 pin controller device
+ * @pin_conf: single pin configuration descriptor
+ */
+static int rza1_alternate_function_conf(struct rza1_pinctrl *rza1_pctl,
+					struct rza1_pin_conf *pin_conf)
+{
+	struct rza1_port *port = &rza1_pctl->ports[pin_conf->port];
+	u8 mux_mode = (pin_conf->mux_conf - 1) & MUX_FUNC_MASK;
+	u8 mux_conf = pin_conf->mux_conf >> MUX_FUNC_OFFS;
+	bool swio_out = !!(mux_conf & MUX_CONF_SWIO_OUTPUT);
+	bool swio_in = !!(mux_conf & MUX_CONF_SWIO_INPUT);
+	bool bidir_en = !!(mux_conf & MUX_CONF_BIDIR);
+	unsigned int offset = pin_conf->offset;
+
+	/* Make sure a single bit is set in mux_conf mask. */
+	if (ffs(mux_conf) != fls(mux_conf)) {
+		dev_err(rza1_pctl->dev,
+			"Invalid pin configuration for pin %u:%u",
+			port->id, offset);
+		return -EINVAL;
+	}
+
+	rza1_pin_reset(port, offset);
+
+	if (bidir_en)
+		rza1_set_bit(port, PBDC_REG, offset, 1);
+
+	/*
+	 * Enable alternate function mode and select it.
+	 *
+	 * ----------------------------------------------------
+	 * Alternate mode selection table:
+	 *
+	 * PMC	PFC	PFCE	PFCAE	mux_mode
+	 * 1	0	0	0	0
+	 * 1	1	0	0	1
+	 * 1	0	1	0	2
+	 * 1	1	1	0	3
+	 * 1	0	0	1	4
+	 * 1	1	0	1	5
+	 * 1	0	1	1	6
+	 * 1	1	1	1	7
+	 * ----------------------------------------------------
+	 */
+	rza1_set_bit(port, PFC_REG, offset, mux_mode & MUX_FUNC_PFC_MASK);
+	rza1_set_bit(port, PFCE_REG, offset, mux_mode & MUX_FUNC_PFCE_MASK);
+	rza1_set_bit(port, PFCEA_REG, offset, mux_mode & MUX_FUNC_PFCEA_MASK);
+
+	/*
+	 * All alternate functions except a few (4) need PIPCn = 1.
+	 * If PIPCn has to stay disabled (SW IO mode), configure PMn according
+	 * to I/O direction specified by pin configuration -after- PMC has been
+	 * set to one.
+	 */
+	if (!(swio_in || swio_out))
+		rza1_set_bit(port, PIPC_REG, offset, 1);
+
+	rza1_set_bit(port, PMC_REG, offset, 1);
+	rza1_set_bit(port, PM_REG, offset, swio_in);
+
+	return 0;
+}
+
+/* ----------------------------------------------------------------------------
+ * pinctrl operations
+ */
+
+/**
+ * rza1_dt_node_to_map() - map a node to a function/group map
+ *
+ * Functions and groups are collected and registered to pinctrl_generic
+ * during DT parsing routine.
+ *
+ * @pctldev: pin controller device
+ * @np: device tree node to parse
+ * @map: pointer to pin map (output)
+ * @num_maps: number of collected maps (output)
+ */
+static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev,
+			       struct device_node *np,
+			       struct pinctrl_map **map,
+			       unsigned int *num_maps)
+{
+	struct rza1_pinctrl *rza1_pctl = pinctrl_dev_get_drvdata(pctldev);
+	struct group_desc *grp;
+	unsigned int grp_sel;
+
+	/*
+	 * Find the group of this node and check if we need create
+	 * config maps for pins.
+	 */
+	grp_sel = pinctrl_get_group_selector(pctldev, np->name);
+	if (grp_sel < 0) {
+		dev_err(rza1_pctl->dev, "unable to find group for node %s\n",
+			np->name);
+		return -EINVAL;
+	}
+
+	grp = pinctrl_generic_get_group(pctldev, grp_sel);
+	if (!grp) {
+		dev_err(rza1_pctl->dev, "unable to find group for node %s\n",
+			np->name);
+		return -EINVAL;
+	}
+
+	*num_maps = 0;
+	*map = kzalloc(sizeof(**map), GFP_KERNEL);
+	if (!*map)
+		return -ENOMEM;
+
+	(*map)->type	= PIN_MAP_TYPE_MUX_GROUP;
+	(*map)->data.mux.group	= np->name;
+	(*map)->data.mux.function = np->name;
+	*num_maps = 1;
+
+	return 0;
+}
+
+static void rza1_dt_free_map(struct pinctrl_dev *pctldev,
+			     struct pinctrl_map *map, unsigned int num_maps)
+{
+	kfree(map);
+}
+
+static const struct pinctrl_ops rza1_pinctrl_ops = {
+	.get_groups_count	= pinctrl_generic_get_group_count,
+	.get_group_name		= pinctrl_generic_get_group_name,
+	.get_group_pins		= pinctrl_generic_get_group_pins,
+	.dt_node_to_map		= rza1_dt_node_to_map,
+	.dt_free_map		= rza1_dt_free_map,
+};
+
+/* ----------------------------------------------------------------------------
+ * gpio operations
+ */
+
+/**
+ * rza1_gpio_request() - configure pin in port mode
+ *
+ * Configure a pin as gpio (port mode).
+ * After reset, the pin is in input mode with input buffer disabled.
+ * To use the pin as input or output, set_direction shall be called first
+ *
+ * @chip: gpio chip where the gpio sits on
+ * @gpio: gpio offset
+ */
+static int rza1_gpio_request(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	rza1_pin_reset(port, gpio);
+
+	return 0;
+}
+
+/**
+ * rza1_gpio_disable_free() - reset a pin
+ *
+ * Surprisingly, disable_free a gpio, is equivalent to request it.
+ * Reset pin to port mode, with input buffer disabled. This overwrites all
+ * port direction settings applied with set_direction
+ *
+ * @chip: gpio chip where the gpio sits on
+ * @gpio: gpio offset
+ */
+static void rza1_gpio_free(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	rza1_pin_reset(port, gpio);
+}
+
+static int rza1_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	return rza1_pin_get_direction(port, offset);
+}
+
+static int rza1_gpio_direction_input(struct gpio_chip *chip,
+				     unsigned int offset)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	rza1_pin_set_direction(port, offset, true);
+
+	return 0;
+}
+
+static int rza1_gpio_direction_output(struct gpio_chip *chip,
+				      unsigned int offset,
+				      int value)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	/* Set value before driving pin direction */
+	rza1_pin_set(port, offset, value);
+	rza1_pin_set_direction(port, offset, false);
+
+	return 0;
+}
+
+/**
+ * rza1_gpio_get() - read a gpio pin value
+ *
+ * Read gpio pin value through PPR register.
+ * Requires bi-directional mode to work when reading the value of a pin
+ * in output mode
+ *
+ * @chip: gpio chip where the gpio sits on
+ * @gpio: gpio offset
+ */
+static int rza1_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	return rza1_pin_get(port, offset);
+}
+
+static void rza1_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			  int value)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	rza1_pin_set(port, offset, value);
+}
+
+struct gpio_chip rza1_gpiochip_template = {
+	.request		= rza1_gpio_request,
+	.free			= rza1_gpio_free,
+	.get_direction		= rza1_gpio_get_direction,
+	.direction_input	= rza1_gpio_direction_input,
+	.direction_output	= rza1_gpio_direction_output,
+	.get			= rza1_gpio_get,
+	.set			= rza1_gpio_set,
+};
+
+/* ----------------------------------------------------------------------------
+ * pinmux operations
+ */
+
+/**
+ * rza1_pinmux_set() - retrieve pins from a group and apply their mux settings
+ *
+ * @pctldev: pin controller device
+ * @selector: function selector
+ * @group: group selector
+ */
+static int rza1_pinmux_set(struct pinctrl_dev *pctldev, unsigned int selector,
+			   unsigned int group)
+{
+	struct rza1_pinctrl *rza1_pctl = pinctrl_dev_get_drvdata(pctldev);
+	struct rza1_pin_conf *pin_confs;
+	struct function_desc *func;
+	struct group_desc *grp;
+	int i;
+
+	grp = pinctrl_generic_get_group(pctldev, group);
+	if (!grp)
+		return -EINVAL;
+
+	func = pinmux_generic_get_function(pctldev, selector);
+	if (!func)
+		return -EINVAL;
+
+	pin_confs = (struct rza1_pin_conf *)func->data;
+	for (i = 0; i < grp->num_pins; ++i) {
+		int ret;
+
+		ret = rza1_alternate_function_conf(rza1_pctl, &pin_confs[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+struct pinmux_ops rza1_pinmux_ops = {
+	.get_functions_count	= pinmux_generic_get_function_count,
+	.get_function_name	= pinmux_generic_get_function_name,
+	.get_function_groups	= pinmux_generic_get_function_groups,
+	.set_mux		= rza1_pinmux_set,
+	.strict			= true,
+};
+
+/* ----------------------------------------------------------------------------
+ * RZ/A1 pin controller driver operations
+ */
+
+static unsigned int rza1_count_gpio_chips(struct device_node *np)
+{
+	struct device_node *child;
+	unsigned int count = 0;
+
+	for_each_child_of_node(np, child) {
+		if (!of_property_read_bool(child, "gpio-controller"))
+			continue;
+
+		count++;
+	}
+
+	return count;
+}
+
+/**
+ * rza1_parse_pmx_function() - parse and register a pin mux function
+ *
+ * Pins for RZ SoC pin controller described by "renesas-pins" property.
+ *
+ * First argument in the list identifies the pin, while the second one
+ * describes the requested alternate function number and additional
+ * configuration parameter to be applied to the selected function.
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ * @np: of pmx sub-node
+ */
+static int rza1_parse_pmx_function(struct rza1_pinctrl *rza1_pctl,
+				   struct device_node *np)
+{
+	struct pinctrl_dev *pctldev = rza1_pctl->pctl;
+	char const *prop_name = "renesas,pins";
+	struct rza1_pin_conf *pin_confs;
+	unsigned int *grpins;
+	const char *grpname;
+	const char **fngrps;
+	unsigned int i;
+	int of_npins;
+	int ret;
+
+	of_npins = pinctrl_count_index_with_args(np, prop_name);
+	if (of_npins <= 0) {
+		dev_err(rza1_pctl->dev, "Missing %s property\n", prop_name);
+		return -ENOENT;
+	}
+
+	/*
+	 * Functions are made of 1 group only;
+	 * in fact, functions and groups are identical for this pin controller
+	 * except that functions carry an array of per-pin configuration
+	 * settings.
+	 */
+	pin_confs = devm_kcalloc(rza1_pctl->dev, of_npins, sizeof(*pin_confs),
+				 GFP_KERNEL);
+	grpins = devm_kcalloc(rza1_pctl->dev, of_npins, sizeof(*grpins),
+			      GFP_KERNEL);
+	fngrps = devm_kzalloc(rza1_pctl->dev, sizeof(*fngrps), GFP_KERNEL);
+
+	if (!pin_confs || !grpins || !fngrps)
+		return -ENOMEM;
+
+	/* Collect pin positions and mux settings to store them in function */
+	for (i = 0; i < of_npins; ++i) {
+		struct rza1_pin_conf *pin_conf = &pin_confs[i];
+		struct of_phandle_args of_npins_args;
+
+		ret = pinctrl_parse_index_with_args(np, prop_name, i,
+						    &of_npins_args);
+		if (ret)
+			return ret;
+
+		if (of_npins_args.args_count < RZA1_PINMUX_OF_ARGS) {
+			dev_err(rza1_pctl->dev,
+				"Wrong number of arguments for %s property\n",
+				prop_name);
+			return -EINVAL;
+		}
+
+		/*
+		 * This new pins configuration will be associated with a new
+		 * function, and later used to set-up pin muxing
+		 */
+		pin_conf->id = of_npins_args.args[0];
+		pin_conf->port = RZA1_PIN_TO_PORT(pin_conf->id);
+		pin_conf->offset = RZA1_PIN_TO_OFFSET(pin_conf->id);
+		pin_conf->mux_conf = of_npins_args.args[1];
+
+		if (pin_conf->port >= RZA1_NPORTS ||
+		    pin_conf->offset >= RZA1_PINS_PER_PORT) {
+			dev_err(rza1_pctl->dev,
+				"Wrong port %u pin %u for %s property\n",
+				pin_conf->port, pin_conf->offset, prop_name);
+			return -EINVAL;
+		}
+
+		grpins[i] = pin_conf->id;
+	}
+
+	grpname	= np->name;
+	fngrps[0] = grpname;
+
+	mutex_lock(&rza1_pctl->mutex);
+	ret = pinctrl_generic_add_group(pctldev, grpname, grpins, of_npins,
+					NULL);
+	if (ret) {
+		mutex_unlock(&rza1_pctl->mutex);
+		return ret;
+	}
+
+	ret = pinmux_generic_add_function(pctldev, grpname, fngrps, 1,
+					  pin_confs);
+	if (ret)
+		goto remove_group;
+	mutex_unlock(&rza1_pctl->mutex);
+
+	dev_info(rza1_pctl->dev, "Parsed function and group %s with %d pins\n",
+				 grpname, of_npins);
+
+	return 0;
+
+remove_group:
+	dev_info(rza1_pctl->dev, "Unable to parse function and group %s\n",
+				 grpname);
+	pinctrl_generic_remove_last_group(pctldev);
+	mutex_unlock(&rza1_pctl->mutex);
+
+	return ret;
+}
+
+/**
+ * rza1_remove_pmx_functions() - un-register pmx functions and groups
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ */
+static void rza1_remove_pmx_functions(struct rza1_pinctrl *rza1_pctl)
+{
+	struct pinctrl_dev *pctldev = rza1_pctl->pctl;
+
+	mutex_lock(&rza1_pctl->mutex);
+	pinmux_generic_free_functions(pctldev);
+	mutex_unlock(&rza1_pctl->mutex);
+}
+
+/**
+ * rza1_parse_gpiochip() - parse and register a gpio chip and pin range
+ *
+ * The gpio controller subnode shall provide a "gpio-ranges" list property as
+ * defined by gpio device tree binding documentation.
+ * Gpio chips and pin ranges are here collected, but ranges are registered
+ * later, after the pin controller has been registered too. Only gpiochips are
+ * registered here.
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ * @np: of gpio-controller node
+ * @chip: gpio chip to register to gpiolib
+ * @range: pin range to register to pinctrl core
+ */
+static int rza1_parse_gpiochip(struct rza1_pinctrl *rza1_pctl,
+			       struct device_node *np,
+			       struct gpio_chip *chip,
+			       struct pinctrl_gpio_range *range)
+{
+	const char *list_name = "gpio-ranges";
+	struct of_phandle_args of_args;
+	unsigned int gpioport;
+	u32 pinctrl_base;
+	int ret;
+
+	ret = of_parse_phandle_with_fixed_args(np, list_name, 3, 0, &of_args);
+	if (ret) {
+		dev_err(rza1_pctl->dev, "Unable to parse %s list property\n",
+			list_name);
+		return ret;
+	}
+
+	/*
+	 * Find out on which port this gpio-chip maps to by inspecting the
+	 * second argument of the "gpio-ranges" property.
+	 */
+	pinctrl_base = of_args.args[1];
+	gpioport = RZA1_PIN_TO_PORT(pinctrl_base);
+	if (gpioport > RZA1_NPORTS) {
+		dev_err(rza1_pctl->dev,
+			"Invalid values in property %s\n", list_name);
+		return -EINVAL;
+	}
+
+	*chip		= rza1_gpiochip_template;
+	chip->base	= -1;
+	chip->label	= devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%s-%u",
+					 np->name, gpioport);
+	chip->ngpio	= of_args.args[2];
+	chip->of_node	= np;
+	chip->parent	= rza1_pctl->dev;
+
+	range->id	= gpioport;
+	range->name	= chip->label;
+	range->pin_base	= range->base = pinctrl_base;
+	range->npins	= of_args.args[2];
+	range->gc	= chip;
+
+	ret = devm_gpiochip_add_data(rza1_pctl->dev, chip,
+				     &rza1_pctl->ports[gpioport]);
+	if (ret)
+		return ret;
+
+	dev_info(rza1_pctl->dev, "Parsed gpiochip %s with %d pins\n",
+		 chip->label, chip->ngpio);
+
+	return 0;
+}
+
+/**
+ * rza1_parse_dt() - parse DT to collect gpiochips and pmx functions
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ */
+static int rza1_parse_dt(struct rza1_pinctrl *rza1_pctl)
+{
+	struct device_node *np = rza1_pctl->dev->of_node;
+	struct pinctrl_gpio_range *gpio_ranges;
+	struct gpio_chip *gpio_chips;
+	struct device_node *child;
+	unsigned int ngpiochips;
+	unsigned int npmxfuncs;
+	unsigned int i;
+	int ret;
+
+	ngpiochips = rza1_count_gpio_chips(np);
+	if (ngpiochips) {
+		dev_info(rza1_pctl->dev, "Registering %u gpio chips\n",
+					 ngpiochips);
+
+		gpio_chips = devm_kcalloc(rza1_pctl->dev, ngpiochips,
+					  sizeof(*gpio_chips), GFP_KERNEL);
+		gpio_ranges = devm_kcalloc(rza1_pctl->dev, ngpiochips,
+					   sizeof(*gpio_ranges), GFP_KERNEL);
+		if (!gpio_chips || !gpio_ranges)
+			return -ENOMEM;
+	} else {
+		dev_dbg(rza1_pctl->dev, "No gpiochip registered\n");
+		gpio_ranges = NULL;
+	}
+
+	rza1_pctl->gpio_ranges	= gpio_ranges;
+
+	i = 0;
+	npmxfuncs = 0;
+	for_each_child_of_node(np, child) {
+		if (of_property_read_bool(child, "gpio-controller")) {
+			/* Never get here if ngpiochips == 0 */
+			ret = rza1_parse_gpiochip(rza1_pctl, child,
+						  &gpio_chips[i],
+						  &gpio_ranges[i]);
+			if (ret)
+				goto gpio_pmx_unregister;
+
+			++i;
+		} else {
+			ret = rza1_parse_pmx_function(rza1_pctl, child);
+			if (ret)
+				goto gpio_pmx_unregister;
+
+			++npmxfuncs;
+		}
+	}
+
+	rza1_pctl->ngpiochips = i;
+
+	dev_info(rza1_pctl->dev,
+		 "Registered %u gpio controllers and %u pin mux functions\n",
+		 rza1_pctl->ngpiochips, npmxfuncs);
+
+	return 0;
+
+gpio_pmx_unregister:
+	for (; i > 0; i--)
+		devm_gpiochip_remove(rza1_pctl->dev, &gpio_chips[i - 1]);
+
+	rza1_remove_pmx_functions(rza1_pctl);
+
+	return ret;
+}
+
+/**
+ * rza1_pinctrl_register() - Enumerate pins, ports, gpiochips, pmx functions
+ *			     and register to pinctrl and gpio cores
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ */
+static int rza1_pinctrl_register(struct rza1_pinctrl *rza1_pctl)
+{
+	struct pinctrl_pin_desc *pins;
+	struct rza1_port *ports;
+	unsigned int i;
+	int ret;
+
+	pins = devm_kcalloc(rza1_pctl->dev, RZA1_NPINS, sizeof(*pins),
+			    GFP_KERNEL);
+	ports = devm_kcalloc(rza1_pctl->dev, RZA1_NPORTS, sizeof(*ports),
+			     GFP_KERNEL);
+	if (!pins || !ports)
+		return -ENOMEM;
+
+	rza1_pctl->pins		= pins;
+	rza1_pctl->desc.pins	= pins;
+	rza1_pctl->desc.npins	= RZA1_NPINS;
+	rza1_pctl->ports	= ports;
+
+	for (i = 0; i < RZA1_NPINS; ++i) {
+		unsigned int offset = RZA1_PIN_TO_OFFSET(i);
+		unsigned int port = RZA1_PIN_TO_PORT(i);
+
+		pins[i].number = i;
+		pins[i].name = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL,
+					      "P%u-%u", port, offset);
+
+		if (i % RZA1_PINS_PER_PORT == 0) {
+			/*
+			 * Setup ports;
+			 * they provide per-port lock and logical base address.
+			 */
+			unsigned int port_id = RZA1_PIN_TO_PORT(i);
+
+			ports[port_id].id = port_id;
+			ports[port_id].base = rza1_pctl->base;
+			ports[port_id].pins = &pins[i];
+			spin_lock_init(&ports[port_id].lock);
+		}
+	}
+
+	ret = devm_pinctrl_register_and_init(rza1_pctl->dev, &rza1_pctl->desc,
+					     rza1_pctl, &rza1_pctl->pctl);
+	if (ret) {
+		dev_err(rza1_pctl->dev,
+			"RZ/A1 pin controller registration failed\n");
+		return ret;
+	}
+
+	ret = rza1_parse_dt(rza1_pctl);
+	if (ret) {
+		dev_err(rza1_pctl->dev, "RZ/A1 DT parsing failed\n");
+		return ret;
+	}
+
+	for (i = 0; i < rza1_pctl->ngpiochips; i++)
+		pinctrl_add_gpio_range(rza1_pctl->pctl,
+				       &rza1_pctl->gpio_ranges[i]);
+
+	return 0;
+}
+
+static int rza1_pinctrl_probe(struct platform_device *pdev)
+{
+	struct rza1_pinctrl *rza1_pctl;
+	struct resource *res;
+	int ret;
+
+	rza1_pctl = devm_kzalloc(&pdev->dev, sizeof(*rza1_pctl), GFP_KERNEL);
+	if (!rza1_pctl)
+		return -ENOMEM;
+
+	rza1_pctl->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (ret)
+		return -ENODEV;
+
+	rza1_pctl->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(rza1_pctl->base))
+		return PTR_ERR(rza1_pctl->base);
+
+	mutex_init(&rza1_pctl->mutex);
+
+	platform_set_drvdata(pdev, rza1_pctl);
+
+	rza1_pctl->desc.name	= DRIVER_NAME;
+	rza1_pctl->desc.pctlops	= &rza1_pinctrl_ops;
+	rza1_pctl->desc.pmxops	= &rza1_pinmux_ops;
+	rza1_pctl->desc.owner	= THIS_MODULE;
+
+	ret = rza1_pinctrl_register(rza1_pctl);
+	if (ret)
+		return ret;
+
+	dev_info(&pdev->dev,
+		 "RZ/A1 pin controller and gpio successfully registered\n");
+
+	return 0;
+}
+
+static const struct of_device_id rza1_pinctrl_of_match[] = {
+	{ .compatible = "renesas,r7s72100-ports", },
+	{ }
+};
+
+static struct platform_driver rza1_pinctrl_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = rza1_pinctrl_of_match,
+	},
+	.probe = rza1_pinctrl_probe,
+};
+
+static int __init rza1_pinctrl_init(void)
+{
+	return platform_driver_register(&rza1_pinctrl_driver);
+}
+core_initcall(rza1_pinctrl_init);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org");
+MODULE_DESCRIPTION("Pin and gpio controller driver for Reneas RZ/A1 SoC");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

[Jacopo Mondi]

Add combined gpio and pin controller driver for Renesas RZ/A1
r7s72100 SoC.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/pinctrl/Kconfig        |  10 +
 drivers/pinctrl/Makefile       |   1 +
 drivers/pinctrl/pinctrl-rza1.c | 961 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 972 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-rza1.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 8f8c2af..c9b55b9 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -163,6 +163,16 @@ config PINCTRL_ROCKCHIP
 	select GENERIC_IRQ_CHIP
 	select MFD_SYSCON
 
+config PINCTRL_RZA1
+	bool "Renesas RZ/A1 gpio and pinctrl driver"
+	depends on OF
+	depends on ARCH_R7S72100 || COMPILE_TEST
+	select GENERIC_PINCTRL_GROUPS
+	select GENERIC_PINMUX_FUNCTIONS
+	select GENERIC_PINCONF
+	help
+	  This selects pinctrl driver for Renesas RZ/A1 platforms.
+
 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 a251f43..0c2328d2 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_PINCTRL_PALMAS)	+= pinctrl-palmas.o
 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_SINGLE)	+= pinctrl-single.o
 obj-$(CONFIG_PINCTRL_SIRF)	+= sirf/
 obj-$(CONFIG_PINCTRL_SX150X)	+= pinctrl-sx150x.o
diff --git a/drivers/pinctrl/pinctrl-rza1.c b/drivers/pinctrl/pinctrl-rza1.c
new file mode 100644
index 0000000..0a4fc41
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-rza1.c
@@ -0,0 +1,961 @@
+/*
+ * Combined GPIO and pin controller support for Renesas RZ/A1 (r7s72100) SoC
+ *
+ * Copyright (C) 2017 Jacopo Mondi
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+/*
+ * This pincontroller/gpio combined driver support Renesas devices of RZ/A1
+ * family.
+ * This includes SoCs which are sub- or super- sets of this particular line,
+ * as RZ/A1H (r7s721000), RZ/A1M (r7s721010) and RZ/A1L (r7s721020) are.
+ */
+
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/slab.h>
+
+#include "core.h"
+#include "devicetree.h"
+#include "pinmux.h"
+
+#define DRIVER_NAME			"pinctrl-rza1"
+
+#define RZA1_PINMUX_OF_ARGS		2
+
+#define P_REG				0x0000
+#define PPR_REG				0x0200
+#define PM_REG				0x0300
+#define PMC_REG				0x0400
+#define PFC_REG				0x0500
+#define PFCE_REG			0x0600
+#define PFCEA_REG			0x0a00
+#define PIBC_REG			0x4000
+#define PBDC_REG			0x4100
+#define PIPC_REG			0x4200
+#define RZA1_ADDR(mem, reg, port)	((mem) + (reg) + ((port) * 4))
+
+#define RZA1_NPORTS			12
+#define RZA1_PINS_PER_PORT		16
+#define RZA1_NPINS			(RZA1_PINS_PER_PORT * RZA1_NPORTS)
+#define RZA1_PIN_TO_PORT(pin)		((pin) / RZA1_PINS_PER_PORT)
+#define RZA1_PIN_TO_OFFSET(pin)		((pin) % RZA1_PINS_PER_PORT)
+
+/*
+ * Be careful here: the pin configuration subnodes in device tree enumerate
+ * alternate functions from 1 to 8; subtract 1 before using macros so to match
+ * register configuration which expects numbers from 0 to 7 instead.
+ */
+#define MUX_FUNC_OFFS			3
+#define MUX_FUNC_MASK			(BIT(MUX_FUNC_OFFS) - 1)
+#define MUX_FUNC_PFC_MASK		BIT(0)
+#define MUX_FUNC_PFCE_MASK		BIT(1)
+#define MUX_FUNC_PFCEA_MASK		BIT(2)
+#define MUX_CONF_BIDIR			BIT(0)
+#define MUX_CONF_SWIO_INPUT		BIT(1)
+#define MUX_CONF_SWIO_OUTPUT		BIT(2)
+
+/**
+ * rza1_pin_conf - describes a pin position, id, mux config and output value
+ *
+ * @id: the pin identifier from 0 to RZA1_NPINS
+ * @port: the port where pin sits on
+ * @offset: pin offset in the port
+ * @mux: alternate function configuration settings
+ * @value: output value to set the pin to
+ */
+struct rza1_pin_conf {
+	u16 id;
+	u8 port;
+	u8 offset;
+	u8 mux_conf;
+	u8 value;
+};
+
+/**
+ * rza1_port - describes a pin port
+ *
+ * This is mostly useful to lock register writes per-bank and not globally.
+ *
+ * @lock: protect access to HW registers
+ * @id: port number
+ * @base: logical address base
+ * @pins: pins sitting on this port
+ */
+struct rza1_port {
+	spinlock_t lock;
+	unsigned int id;
+	void __iomem *base;
+	struct pinctrl_pin_desc *pins;
+};
+
+/**
+ * rza1_pinctrl - RZ pincontroller device
+ *
+ * @dev: parent device structure
+ * @mutex: protect [pinctrl|pinmux]_generic functions
+ * @base: logical address base
+ * @nports: number of pin controller ports
+ * @ports: pin controller banks
+ * @ngpiochips: number of gpio chips
+ * @gpio_ranges: gpio ranges for pinctrl core
+ * @pins: pin array for pinctrl core
+ * @desc: pincontroller desc for pinctrl core
+ * @pctl: pinctrl device
+ */
+struct rza1_pinctrl {
+	struct device *dev;
+
+	struct mutex mutex;
+
+	void __iomem *base;
+
+	unsigned int nport;
+	struct rza1_port *ports;
+
+	unsigned int ngpiochips;
+
+	struct pinctrl_gpio_range *gpio_ranges;
+	struct pinctrl_pin_desc *pins;
+	struct pinctrl_desc desc;
+	struct pinctrl_dev *pctl;
+};
+
+/* ----------------------------------------------------------------------------
+ * RZ/A1 SoC operations
+ */
+
+/**
+ * rza1_set_bit() - un-locked set/clear a single bit in pin configuration
+ *		    registers
+ */
+static inline void rza1_set_bit(struct rza1_port *port, unsigned int reg,
+				unsigned int bit, bool set)
+{
+	void __iomem *mem = RZA1_ADDR(port->base, reg, port->id);
+	u16 val = ioread16(mem);
+
+	if (set)
+		val |= BIT(bit);
+	else
+		val &= ~BIT(bit);
+
+	iowrite16(val, mem);
+}
+
+static inline int rza1_get_bit(struct rza1_port *port, unsigned int reg,
+			       unsigned int bit)
+{
+	void __iomem *mem = RZA1_ADDR(port->base, reg, port->id);
+
+	return ioread16(mem) & BIT(bit);
+}
+
+/**
+ * rza1_pin_reset() - reset a pin to default initial state
+ *
+ * Reset pin state disabling input buffer and bi-directional control,
+ * and configure it as input port.
+ * Note that pin is now configured with direction as input but with input
+ * buffer disabled. This implies the pin value cannot be read in this state.
+ *
+ * @port: port where pin sits on
+ * @pin: pin offset
+ */
+static void rza1_pin_reset(struct rza1_port *port, unsigned int pin)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+	rza1_set_bit(port, PIBC_REG, pin, 0);
+	rza1_set_bit(port, PBDC_REG, pin, 0);
+
+	rza1_set_bit(port, PM_REG, pin, 1);
+	rza1_set_bit(port, PMC_REG, pin, 0);
+	rza1_set_bit(port, PIPC_REG, pin, 0);
+	spin_unlock_irqrestore(&port->lock, irqflags);
+}
+
+static inline int rza1_pin_get_direction(struct rza1_port *port,
+					 unsigned int pin)
+{
+	unsigned long irqflags;
+	int input;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+	input = rza1_get_bit(port, PM_REG, pin);
+	spin_unlock_irqrestore(&port->lock, irqflags);
+
+	return input;
+}
+
+/**
+ * rza1_pin_set_direction() - set I/O direction on a pin in port mode
+ *
+ * When running in output port mode keep PBDC enabled to allow reading the
+ * pin value from PPR.
+ *
+ * @port: port where pin sits on
+ * @pin: pin offset
+ * @input: input enable/disable flag
+ */
+static inline void rza1_pin_set_direction(struct rza1_port *port,
+					  unsigned int pin, bool input)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+
+	rza1_set_bit(port, PIBC_REG, pin, 1);
+	if (input) {
+		rza1_set_bit(port, PM_REG, pin, 1);
+		rza1_set_bit(port, PBDC_REG, pin, 0);
+	} else {
+		rza1_set_bit(port, PM_REG, pin, 0);
+		rza1_set_bit(port, PBDC_REG, pin, 1);
+	}
+
+	spin_unlock_irqrestore(&port->lock, irqflags);
+}
+
+static inline void rza1_pin_set(struct rza1_port *port, unsigned int pin,
+				unsigned int value)
+{
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+	rza1_set_bit(port, P_REG, pin, !!value);
+	spin_unlock_irqrestore(&port->lock, irqflags);
+}
+
+static inline int rza1_pin_get(struct rza1_port *port, unsigned int pin)
+{
+	unsigned long irqflags;
+	int val;
+
+	spin_lock_irqsave(&port->lock, irqflags);
+	val = rza1_get_bit(port, PPR_REG, pin);
+	spin_unlock_irqrestore(&port->lock, irqflags);
+
+	return val;
+}
+
+/**
+ * rza1_alternate_function_conf() - configure pin in alternate function mode
+ *
+ * @pinctrl: RZ/A1 pin controller device
+ * @pin_conf: single pin configuration descriptor
+ */
+static int rza1_alternate_function_conf(struct rza1_pinctrl *rza1_pctl,
+					struct rza1_pin_conf *pin_conf)
+{
+	struct rza1_port *port = &rza1_pctl->ports[pin_conf->port];
+	u8 mux_mode = (pin_conf->mux_conf - 1) & MUX_FUNC_MASK;
+	u8 mux_conf = pin_conf->mux_conf >> MUX_FUNC_OFFS;
+	bool swio_out = !!(mux_conf & MUX_CONF_SWIO_OUTPUT);
+	bool swio_in = !!(mux_conf & MUX_CONF_SWIO_INPUT);
+	bool bidir_en = !!(mux_conf & MUX_CONF_BIDIR);
+	unsigned int offset = pin_conf->offset;
+
+	/* Make sure a single bit is set in mux_conf mask. */
+	if (ffs(mux_conf) != fls(mux_conf)) {
+		dev_err(rza1_pctl->dev,
+			"Invalid pin configuration for pin %u:%u",
+			port->id, offset);
+		return -EINVAL;
+	}
+
+	rza1_pin_reset(port, offset);
+
+	if (bidir_en)
+		rza1_set_bit(port, PBDC_REG, offset, 1);
+
+	/*
+	 * Enable alternate function mode and select it.
+	 *
+	 * ----------------------------------------------------
+	 * Alternate mode selection table:
+	 *
+	 * PMC	PFC	PFCE	PFCAE	mux_mode
+	 * 1	0	0	0	0
+	 * 1	1	0	0	1
+	 * 1	0	1	0	2
+	 * 1	1	1	0	3
+	 * 1	0	0	1	4
+	 * 1	1	0	1	5
+	 * 1	0	1	1	6
+	 * 1	1	1	1	7
+	 * ----------------------------------------------------
+	 */
+	rza1_set_bit(port, PFC_REG, offset, mux_mode & MUX_FUNC_PFC_MASK);
+	rza1_set_bit(port, PFCE_REG, offset, mux_mode & MUX_FUNC_PFCE_MASK);
+	rza1_set_bit(port, PFCEA_REG, offset, mux_mode & MUX_FUNC_PFCEA_MASK);
+
+	/*
+	 * All alternate functions except a few (4) need PIPCn = 1.
+	 * If PIPCn has to stay disabled (SW IO mode), configure PMn according
+	 * to I/O direction specified by pin configuration -after- PMC has been
+	 * set to one.
+	 */
+	if (!(swio_in || swio_out))
+		rza1_set_bit(port, PIPC_REG, offset, 1);
+
+	rza1_set_bit(port, PMC_REG, offset, 1);
+	rza1_set_bit(port, PM_REG, offset, swio_in);
+
+	return 0;
+}
+
+/* ----------------------------------------------------------------------------
+ * pinctrl operations
+ */
+
+/**
+ * rza1_dt_node_to_map() - map a node to a function/group map
+ *
+ * Functions and groups are collected and registered to pinctrl_generic
+ * during DT parsing routine.
+ *
+ * @pctldev: pin controller device
+ * @np: device tree node to parse
+ * @map: pointer to pin map (output)
+ * @num_maps: number of collected maps (output)
+ */
+static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev,
+			       struct device_node *np,
+			       struct pinctrl_map **map,
+			       unsigned int *num_maps)
+{
+	struct rza1_pinctrl *rza1_pctl = pinctrl_dev_get_drvdata(pctldev);
+	struct group_desc *grp;
+	unsigned int grp_sel;
+
+	/*
+	 * Find the group of this node and check if we need create
+	 * config maps for pins.
+	 */
+	grp_sel = pinctrl_get_group_selector(pctldev, np->name);
+	if (grp_sel < 0) {
+		dev_err(rza1_pctl->dev, "unable to find group for node %s\n",
+			np->name);
+		return -EINVAL;
+	}
+
+	grp = pinctrl_generic_get_group(pctldev, grp_sel);
+	if (!grp) {
+		dev_err(rza1_pctl->dev, "unable to find group for node %s\n",
+			np->name);
+		return -EINVAL;
+	}
+
+	*num_maps = 0;
+	*map = kzalloc(sizeof(**map), GFP_KERNEL);
+	if (!*map)
+		return -ENOMEM;
+
+	(*map)->type	= PIN_MAP_TYPE_MUX_GROUP;
+	(*map)->data.mux.group	= np->name;
+	(*map)->data.mux.function = np->name;
+	*num_maps = 1;
+
+	return 0;
+}
+
+static void rza1_dt_free_map(struct pinctrl_dev *pctldev,
+			     struct pinctrl_map *map, unsigned int num_maps)
+{
+	kfree(map);
+}
+
+static const struct pinctrl_ops rza1_pinctrl_ops = {
+	.get_groups_count	= pinctrl_generic_get_group_count,
+	.get_group_name		= pinctrl_generic_get_group_name,
+	.get_group_pins		= pinctrl_generic_get_group_pins,
+	.dt_node_to_map		= rza1_dt_node_to_map,
+	.dt_free_map		= rza1_dt_free_map,
+};
+
+/* ----------------------------------------------------------------------------
+ * gpio operations
+ */
+
+/**
+ * rza1_gpio_request() - configure pin in port mode
+ *
+ * Configure a pin as gpio (port mode).
+ * After reset, the pin is in input mode with input buffer disabled.
+ * To use the pin as input or output, set_direction shall be called first
+ *
+ * @chip: gpio chip where the gpio sits on
+ * @gpio: gpio offset
+ */
+static int rza1_gpio_request(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	rza1_pin_reset(port, gpio);
+
+	return 0;
+}
+
+/**
+ * rza1_gpio_disable_free() - reset a pin
+ *
+ * Surprisingly, disable_free a gpio, is equivalent to request it.
+ * Reset pin to port mode, with input buffer disabled. This overwrites all
+ * port direction settings applied with set_direction
+ *
+ * @chip: gpio chip where the gpio sits on
+ * @gpio: gpio offset
+ */
+static void rza1_gpio_free(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	rza1_pin_reset(port, gpio);
+}
+
+static int rza1_gpio_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	return rza1_pin_get_direction(port, offset);
+}
+
+static int rza1_gpio_direction_input(struct gpio_chip *chip,
+				     unsigned int offset)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	rza1_pin_set_direction(port, offset, true);
+
+	return 0;
+}
+
+static int rza1_gpio_direction_output(struct gpio_chip *chip,
+				      unsigned int offset,
+				      int value)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	/* Set value before driving pin direction */
+	rza1_pin_set(port, offset, value);
+	rza1_pin_set_direction(port, offset, false);
+
+	return 0;
+}
+
+/**
+ * rza1_gpio_get() - read a gpio pin value
+ *
+ * Read gpio pin value through PPR register.
+ * Requires bi-directional mode to work when reading the value of a pin
+ * in output mode
+ *
+ * @chip: gpio chip where the gpio sits on
+ * @gpio: gpio offset
+ */
+static int rza1_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	return rza1_pin_get(port, offset);
+}
+
+static void rza1_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			  int value)
+{
+	struct rza1_port *port = gpiochip_get_data(chip);
+
+	rza1_pin_set(port, offset, value);
+}
+
+struct gpio_chip rza1_gpiochip_template = {
+	.request		= rza1_gpio_request,
+	.free			= rza1_gpio_free,
+	.get_direction		= rza1_gpio_get_direction,
+	.direction_input	= rza1_gpio_direction_input,
+	.direction_output	= rza1_gpio_direction_output,
+	.get			= rza1_gpio_get,
+	.set			= rza1_gpio_set,
+};
+
+/* ----------------------------------------------------------------------------
+ * pinmux operations
+ */
+
+/**
+ * rza1_pinmux_set() - retrieve pins from a group and apply their mux settings
+ *
+ * @pctldev: pin controller device
+ * @selector: function selector
+ * @group: group selector
+ */
+static int rza1_pinmux_set(struct pinctrl_dev *pctldev, unsigned int selector,
+			   unsigned int group)
+{
+	struct rza1_pinctrl *rza1_pctl = pinctrl_dev_get_drvdata(pctldev);
+	struct rza1_pin_conf *pin_confs;
+	struct function_desc *func;
+	struct group_desc *grp;
+	int i;
+
+	grp = pinctrl_generic_get_group(pctldev, group);
+	if (!grp)
+		return -EINVAL;
+
+	func = pinmux_generic_get_function(pctldev, selector);
+	if (!func)
+		return -EINVAL;
+
+	pin_confs = (struct rza1_pin_conf *)func->data;
+	for (i = 0; i < grp->num_pins; ++i) {
+		int ret;
+
+		ret = rza1_alternate_function_conf(rza1_pctl, &pin_confs[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+struct pinmux_ops rza1_pinmux_ops = {
+	.get_functions_count	= pinmux_generic_get_function_count,
+	.get_function_name	= pinmux_generic_get_function_name,
+	.get_function_groups	= pinmux_generic_get_function_groups,
+	.set_mux		= rza1_pinmux_set,
+	.strict			= true,
+};
+
+/* ----------------------------------------------------------------------------
+ * RZ/A1 pin controller driver operations
+ */
+
+static unsigned int rza1_count_gpio_chips(struct device_node *np)
+{
+	struct device_node *child;
+	unsigned int count = 0;
+
+	for_each_child_of_node(np, child) {
+		if (!of_property_read_bool(child, "gpio-controller"))
+			continue;
+
+		count++;
+	}
+
+	return count;
+}
+
+/**
+ * rza1_parse_pmx_function() - parse and register a pin mux function
+ *
+ * Pins for RZ SoC pin controller described by "renesas-pins" property.
+ *
+ * First argument in the list identifies the pin, while the second one
+ * describes the requested alternate function number and additional
+ * configuration parameter to be applied to the selected function.
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ * @np: of pmx sub-node
+ */
+static int rza1_parse_pmx_function(struct rza1_pinctrl *rza1_pctl,
+				   struct device_node *np)
+{
+	struct pinctrl_dev *pctldev = rza1_pctl->pctl;
+	char const *prop_name = "renesas,pins";
+	struct rza1_pin_conf *pin_confs;
+	unsigned int *grpins;
+	const char *grpname;
+	const char **fngrps;
+	unsigned int i;
+	int of_npins;
+	int ret;
+
+	of_npins = pinctrl_count_index_with_args(np, prop_name);
+	if (of_npins <= 0) {
+		dev_err(rza1_pctl->dev, "Missing %s property\n", prop_name);
+		return -ENOENT;
+	}
+
+	/*
+	 * Functions are made of 1 group only;
+	 * in fact, functions and groups are identical for this pin controller
+	 * except that functions carry an array of per-pin configuration
+	 * settings.
+	 */
+	pin_confs = devm_kcalloc(rza1_pctl->dev, of_npins, sizeof(*pin_confs),
+				 GFP_KERNEL);
+	grpins = devm_kcalloc(rza1_pctl->dev, of_npins, sizeof(*grpins),
+			      GFP_KERNEL);
+	fngrps = devm_kzalloc(rza1_pctl->dev, sizeof(*fngrps), GFP_KERNEL);
+
+	if (!pin_confs || !grpins || !fngrps)
+		return -ENOMEM;
+
+	/* Collect pin positions and mux settings to store them in function */
+	for (i = 0; i < of_npins; ++i) {
+		struct rza1_pin_conf *pin_conf = &pin_confs[i];
+		struct of_phandle_args of_npins_args;
+
+		ret = pinctrl_parse_index_with_args(np, prop_name, i,
+						    &of_npins_args);
+		if (ret)
+			return ret;
+
+		if (of_npins_args.args_count < RZA1_PINMUX_OF_ARGS) {
+			dev_err(rza1_pctl->dev,
+				"Wrong number of arguments for %s property\n",
+				prop_name);
+			return -EINVAL;
+		}
+
+		/*
+		 * This new pins configuration will be associated with a new
+		 * function, and later used to set-up pin muxing
+		 */
+		pin_conf->id = of_npins_args.args[0];
+		pin_conf->port = RZA1_PIN_TO_PORT(pin_conf->id);
+		pin_conf->offset = RZA1_PIN_TO_OFFSET(pin_conf->id);
+		pin_conf->mux_conf = of_npins_args.args[1];
+
+		if (pin_conf->port >= RZA1_NPORTS ||
+		    pin_conf->offset >= RZA1_PINS_PER_PORT) {
+			dev_err(rza1_pctl->dev,
+				"Wrong port %u pin %u for %s property\n",
+				pin_conf->port, pin_conf->offset, prop_name);
+			return -EINVAL;
+		}
+
+		grpins[i] = pin_conf->id;
+	}
+
+	grpname	= np->name;
+	fngrps[0] = grpname;
+
+	mutex_lock(&rza1_pctl->mutex);
+	ret = pinctrl_generic_add_group(pctldev, grpname, grpins, of_npins,
+					NULL);
+	if (ret) {
+		mutex_unlock(&rza1_pctl->mutex);
+		return ret;
+	}
+
+	ret = pinmux_generic_add_function(pctldev, grpname, fngrps, 1,
+					  pin_confs);
+	if (ret)
+		goto remove_group;
+	mutex_unlock(&rza1_pctl->mutex);
+
+	dev_info(rza1_pctl->dev, "Parsed function and group %s with %d pins\n",
+				 grpname, of_npins);
+
+	return 0;
+
+remove_group:
+	dev_info(rza1_pctl->dev, "Unable to parse function and group %s\n",
+				 grpname);
+	pinctrl_generic_remove_last_group(pctldev);
+	mutex_unlock(&rza1_pctl->mutex);
+
+	return ret;
+}
+
+/**
+ * rza1_remove_pmx_functions() - un-register pmx functions and groups
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ */
+static void rza1_remove_pmx_functions(struct rza1_pinctrl *rza1_pctl)
+{
+	struct pinctrl_dev *pctldev = rza1_pctl->pctl;
+
+	mutex_lock(&rza1_pctl->mutex);
+	pinmux_generic_free_functions(pctldev);
+	mutex_unlock(&rza1_pctl->mutex);
+}
+
+/**
+ * rza1_parse_gpiochip() - parse and register a gpio chip and pin range
+ *
+ * The gpio controller subnode shall provide a "gpio-ranges" list property as
+ * defined by gpio device tree binding documentation.
+ * Gpio chips and pin ranges are here collected, but ranges are registered
+ * later, after the pin controller has been registered too. Only gpiochips are
+ * registered here.
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ * @np: of gpio-controller node
+ * @chip: gpio chip to register to gpiolib
+ * @range: pin range to register to pinctrl core
+ */
+static int rza1_parse_gpiochip(struct rza1_pinctrl *rza1_pctl,
+			       struct device_node *np,
+			       struct gpio_chip *chip,
+			       struct pinctrl_gpio_range *range)
+{
+	const char *list_name = "gpio-ranges";
+	struct of_phandle_args of_args;
+	unsigned int gpioport;
+	u32 pinctrl_base;
+	int ret;
+
+	ret = of_parse_phandle_with_fixed_args(np, list_name, 3, 0, &of_args);
+	if (ret) {
+		dev_err(rza1_pctl->dev, "Unable to parse %s list property\n",
+			list_name);
+		return ret;
+	}
+
+	/*
+	 * Find out on which port this gpio-chip maps to by inspecting the
+	 * second argument of the "gpio-ranges" property.
+	 */
+	pinctrl_base = of_args.args[1];
+	gpioport = RZA1_PIN_TO_PORT(pinctrl_base);
+	if (gpioport > RZA1_NPORTS) {
+		dev_err(rza1_pctl->dev,
+			"Invalid values in property %s\n", list_name);
+		return -EINVAL;
+	}
+
+	*chip		= rza1_gpiochip_template;
+	chip->base	= -1;
+	chip->label	= devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%s-%u",
+					 np->name, gpioport);
+	chip->ngpio	= of_args.args[2];
+	chip->of_node	= np;
+	chip->parent	= rza1_pctl->dev;
+
+	range->id	= gpioport;
+	range->name	= chip->label;
+	range->pin_base	= range->base = pinctrl_base;
+	range->npins	= of_args.args[2];
+	range->gc	= chip;
+
+	ret = devm_gpiochip_add_data(rza1_pctl->dev, chip,
+				     &rza1_pctl->ports[gpioport]);
+	if (ret)
+		return ret;
+
+	dev_info(rza1_pctl->dev, "Parsed gpiochip %s with %d pins\n",
+		 chip->label, chip->ngpio);
+
+	return 0;
+}
+
+/**
+ * rza1_parse_dt() - parse DT to collect gpiochips and pmx functions
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ */
+static int rza1_parse_dt(struct rza1_pinctrl *rza1_pctl)
+{
+	struct device_node *np = rza1_pctl->dev->of_node;
+	struct pinctrl_gpio_range *gpio_ranges;
+	struct gpio_chip *gpio_chips;
+	struct device_node *child;
+	unsigned int ngpiochips;
+	unsigned int npmxfuncs;
+	unsigned int i;
+	int ret;
+
+	ngpiochips = rza1_count_gpio_chips(np);
+	if (ngpiochips) {
+		dev_info(rza1_pctl->dev, "Registering %u gpio chips\n",
+					 ngpiochips);
+
+		gpio_chips = devm_kcalloc(rza1_pctl->dev, ngpiochips,
+					  sizeof(*gpio_chips), GFP_KERNEL);
+		gpio_ranges = devm_kcalloc(rza1_pctl->dev, ngpiochips,
+					   sizeof(*gpio_ranges), GFP_KERNEL);
+		if (!gpio_chips || !gpio_ranges)
+			return -ENOMEM;
+	} else {
+		dev_dbg(rza1_pctl->dev, "No gpiochip registered\n");
+		gpio_ranges = NULL;
+	}
+
+	rza1_pctl->gpio_ranges	= gpio_ranges;
+
+	i = 0;
+	npmxfuncs = 0;
+	for_each_child_of_node(np, child) {
+		if (of_property_read_bool(child, "gpio-controller")) {
+			/* Never get here if ngpiochips == 0 */
+			ret = rza1_parse_gpiochip(rza1_pctl, child,
+						  &gpio_chips[i],
+						  &gpio_ranges[i]);
+			if (ret)
+				goto gpio_pmx_unregister;
+
+			++i;
+		} else {
+			ret = rza1_parse_pmx_function(rza1_pctl, child);
+			if (ret)
+				goto gpio_pmx_unregister;
+
+			++npmxfuncs;
+		}
+	}
+
+	rza1_pctl->ngpiochips = i;
+
+	dev_info(rza1_pctl->dev,
+		 "Registered %u gpio controllers and %u pin mux functions\n",
+		 rza1_pctl->ngpiochips, npmxfuncs);
+
+	return 0;
+
+gpio_pmx_unregister:
+	for (; i > 0; i--)
+		devm_gpiochip_remove(rza1_pctl->dev, &gpio_chips[i - 1]);
+
+	rza1_remove_pmx_functions(rza1_pctl);
+
+	return ret;
+}
+
+/**
+ * rza1_pinctrl_register() - Enumerate pins, ports, gpiochips, pmx functions
+ *			     and register to pinctrl and gpio cores
+ *
+ * @rza1_pctl: RZ/A1 pin controller device
+ */
+static int rza1_pinctrl_register(struct rza1_pinctrl *rza1_pctl)
+{
+	struct pinctrl_pin_desc *pins;
+	struct rza1_port *ports;
+	unsigned int i;
+	int ret;
+
+	pins = devm_kcalloc(rza1_pctl->dev, RZA1_NPINS, sizeof(*pins),
+			    GFP_KERNEL);
+	ports = devm_kcalloc(rza1_pctl->dev, RZA1_NPORTS, sizeof(*ports),
+			     GFP_KERNEL);
+	if (!pins || !ports)
+		return -ENOMEM;
+
+	rza1_pctl->pins		= pins;
+	rza1_pctl->desc.pins	= pins;
+	rza1_pctl->desc.npins	= RZA1_NPINS;
+	rza1_pctl->ports	= ports;
+
+	for (i = 0; i < RZA1_NPINS; ++i) {
+		unsigned int offset = RZA1_PIN_TO_OFFSET(i);
+		unsigned int port = RZA1_PIN_TO_PORT(i);
+
+		pins[i].number = i;
+		pins[i].name = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL,
+					      "P%u-%u", port, offset);
+
+		if (i % RZA1_PINS_PER_PORT == 0) {
+			/*
+			 * Setup ports;
+			 * they provide per-port lock and logical base address.
+			 */
+			unsigned int port_id = RZA1_PIN_TO_PORT(i);
+
+			ports[port_id].id = port_id;
+			ports[port_id].base = rza1_pctl->base;
+			ports[port_id].pins = &pins[i];
+			spin_lock_init(&ports[port_id].lock);
+		}
+	}
+
+	ret = devm_pinctrl_register_and_init(rza1_pctl->dev, &rza1_pctl->desc,
+					     rza1_pctl, &rza1_pctl->pctl);
+	if (ret) {
+		dev_err(rza1_pctl->dev,
+			"RZ/A1 pin controller registration failed\n");
+		return ret;
+	}
+
+	ret = rza1_parse_dt(rza1_pctl);
+	if (ret) {
+		dev_err(rza1_pctl->dev, "RZ/A1 DT parsing failed\n");
+		return ret;
+	}
+
+	for (i = 0; i < rza1_pctl->ngpiochips; i++)
+		pinctrl_add_gpio_range(rza1_pctl->pctl,
+				       &rza1_pctl->gpio_ranges[i]);
+
+	return 0;
+}
+
+static int rza1_pinctrl_probe(struct platform_device *pdev)
+{
+	struct rza1_pinctrl *rza1_pctl;
+	struct resource *res;
+	int ret;
+
+	rza1_pctl = devm_kzalloc(&pdev->dev, sizeof(*rza1_pctl), GFP_KERNEL);
+	if (!rza1_pctl)
+		return -ENOMEM;
+
+	rza1_pctl->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (ret)
+		return -ENODEV;
+
+	rza1_pctl->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(rza1_pctl->base))
+		return PTR_ERR(rza1_pctl->base);
+
+	mutex_init(&rza1_pctl->mutex);
+
+	platform_set_drvdata(pdev, rza1_pctl);
+
+	rza1_pctl->desc.name	= DRIVER_NAME;
+	rza1_pctl->desc.pctlops	= &rza1_pinctrl_ops;
+	rza1_pctl->desc.pmxops	= &rza1_pinmux_ops;
+	rza1_pctl->desc.owner	= THIS_MODULE;
+
+	ret = rza1_pinctrl_register(rza1_pctl);
+	if (ret)
+		return ret;
+
+	dev_info(&pdev->dev,
+		 "RZ/A1 pin controller and gpio successfully registered\n");
+
+	return 0;
+}
+
+static const struct of_device_id rza1_pinctrl_of_match[] = {
+	{ .compatible = "renesas,r7s72100-ports", },
+	{ }
+};
+
+static struct platform_driver rza1_pinctrl_driver = {
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = rza1_pinctrl_of_match,
+	},
+	.probe = rza1_pinctrl_probe,
+};
+
+static int __init rza1_pinctrl_init(void)
+{
+	return platform_driver_register(&rza1_pinctrl_driver);
+}
+core_initcall(rza1_pinctrl_init);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas@jmondi.org");
+MODULE_DESCRIPTION("Pin and gpio controller driver for Reneas RZ/A1 SoC");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

[Linus Walleij]

On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:

I assume Geert will queue this driver even if it is outside of sh-pfc?

> Add combined gpio and pin controller driver for Renesas RZ/A1
> r7s72100 SoC.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/pinctrl/Kconfig        |  10 +
>  drivers/pinctrl/Makefile       |   1 +
>  drivers/pinctrl/pinctrl-rza1.c | 961 +++++++++++++++++++++++++++++++++++++++++

So this is very different from the SH-PFC family and should not
be in drivers/pinctrl/sh-pfc?

> +config PINCTRL_RZA1
> +       bool "Renesas RZ/A1 gpio and pinctrl driver"
> +       depends on OF
> +       depends on ARCH_R7S72100 || COMPILE_TEST
> +       select GENERIC_PINCTRL_GROUPS
> +       select GENERIC_PINMUX_FUNCTIONS
> +       select GENERIC_PINCONF

If it is also a GPIO driver I guess it should
select GPIOLIB as well.

This was not possible in the past, but it is possible nowadays.

> +struct gpio_chip rza1_gpiochip_template = {
> +       .request                = rza1_gpio_request,
> +       .free                   = rza1_gpio_free,
> +       .get_direction          = rza1_gpio_get_direction,
> +       .direction_input        = rza1_gpio_direction_input,
> +       .direction_output       = rza1_gpio_direction_output,
> +       .get                    = rza1_gpio_get,
> +       .set                    = rza1_gpio_set,
> +};

We now also have .set_multiple() and more interestingly
.set_config() which can be backed by pinctrl if you want
to e.g. support debouncing and/or open drain/open source.

Maybe this is stuff your pin controller can do, but not needed
in the initial submission for sure.

> +static int rza1_pinmux_set(struct pinctrl_dev *pctldev, unsigned int selector,
> +                          unsigned int group)

Please name it rza1_set_mux() to correspond with the ops field.

Yours,
Linus Walleij

Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

[jacopo]

Hi Linus,

On Fri, Mar 24, 2017 at 04:42:47PM +0100, Linus Walleij wrote:
> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> 
> I assume Geert will queue this driver even if it is outside of sh-pfc?
> 
> > Add combined gpio and pin controller driver for Renesas RZ/A1
> > r7s72100 SoC.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/pinctrl/Kconfig        |  10 +
> >  drivers/pinctrl/Makefile       |   1 +
> >  drivers/pinctrl/pinctrl-rza1.c | 961 +++++++++++++++++++++++++++++++++++++++++
> 
> So this is very different from the SH-PFC family and should not
> be in drivers/pinctrl/sh-pfc?
> 

Correct. The pin controller hardware in RZ/A(1) devices configures pin
functions per-pin and not per-group as the SH/R-Car family does.

There has been an attempt to support this platforms with the existing
sh-pfc/ infrastructure in the past (https://lwn.net/Articles/573222/)
which I re-proposed for v4.9, but that driver simply does not fit
this hardware which is different from the one found in R-Car
devices.

More on the background of this series in this email thread:
https://marc.info/?l=linux-gpio&m=148536779917834&w=2

I initially created a whole new sub-directory where all the Renesas
devices with this kind of pin controller would have gone
(drivers/pinctrl/rz-pfc) but since as of now the only available
hardware of that type is RZ/A1 and some of its variants, we decided to go
with a single driver supporting that platform only.
If more will come, we'll think about supporting them through this
driver if possible, or create a dedicated directory.

Are you ok with this?

Thanks
  j

> > +config PINCTRL_RZA1
> > +       bool "Renesas RZ/A1 gpio and pinctrl driver"
> > +       depends on OF
> > +       depends on ARCH_R7S72100 || COMPILE_TEST
> > +       select GENERIC_PINCTRL_GROUPS
> > +       select GENERIC_PINMUX_FUNCTIONS
> > +       select GENERIC_PINCONF
> 
> If it is also a GPIO driver I guess it should
> select GPIOLIB as well.
> 
> This was not possible in the past, but it is possible nowadays.
> 
> > +struct gpio_chip rza1_gpiochip_template = {
> > +       .request                = rza1_gpio_request,
> > +       .free                   = rza1_gpio_free,
> > +       .get_direction          = rza1_gpio_get_direction,
> > +       .direction_input        = rza1_gpio_direction_input,
> > +       .direction_output       = rza1_gpio_direction_output,
> > +       .get                    = rza1_gpio_get,
> > +       .set                    = rza1_gpio_set,
> > +};
> 
> We now also have .set_multiple() and more interestingly
> .set_config() which can be backed by pinctrl if you want
> to e.g. support debouncing and/or open drain/open source.
> 
> Maybe this is stuff your pin controller can do, but not needed
> in the initial submission for sure.
> 
> > +static int rza1_pinmux_set(struct pinctrl_dev *pctldev, unsigned int selector,
> > +                          unsigned int group)
> 
> Please name it rza1_set_mux() to correspond with the ops field.
> 
> Yours,
> Linus Walleij

Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

[Linus Walleij]

On Fri, Mar 24, 2017 at 5:45 PM, jacopo <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org> wrote:

> I initially created a whole new sub-directory where all the Renesas
> devices with this kind of pin controller would have gone
> (drivers/pinctrl/rz-pfc) but since as of now the only available
> hardware of that type is RZ/A1 and some of its variants, we decided to go
> with a single driver supporting that platform only.
> If more will come, we'll think about supporting them through this
> driver if possible, or create a dedicated directory.
>
> Are you ok with this?

Yep it's OK. It's no big deal to move it into a new subdir if many
new drivers start popping in anyway.

Right now I see the use of renesas,pins as the only big blocker,
I would much like it to use just pins = <>;

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

[Linus Walleij]

On Fri, Mar 24, 2017 at 5:45 PM, jacopo <jacopo@jmondi.org> wrote:

> I initially created a whole new sub-directory where all the Renesas
> devices with this kind of pin controller would have gone
> (drivers/pinctrl/rz-pfc) but since as of now the only available
> hardware of that type is RZ/A1 and some of its variants, we decided to go
> with a single driver supporting that platform only.
> If more will come, we'll think about supporting them through this
> driver if possible, or create a dedicated directory.
>
> Are you ok with this?

Yep it's OK. It's no big deal to move it into a new subdir if many
new drivers start popping in anyway.

Right now I see the use of renesas,pins as the only big blocker,
I would much like it to use just pins = <>;

Yours,
Linus Walleij

Re: [PATCH v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

[Geert Uytterhoeven]

Hi Linus,

On Fri, Mar 24, 2017 at 4:42 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
>
> I assume Geert will queue this driver even if it is outside of sh-pfc?

OK for me, thanks. I was actually wondering about that ;-)

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 v3 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

[Linus Walleij]

On Wed, Mar 29, 2017 at 9:30 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Fri, Mar 24, 2017 at 4:42 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
>>
>> I assume Geert will queue this driver even if it is outside of sh-pfc?
>
> OK for me, thanks. I was actually wondering about that ;-)

Thanks Geert, much appreciated.

Yours,
Linus Walleij

[PATCH v3 4/7] arm: dts: r7s72100: Add pin controller node

[Jacopo Mondi]

Add pin controller node with 12 gpio controller sub-nodes to
r7s72100 dtsi.

Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
 arch/arm/boot/dts/r7s72100.dtsi | 80 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index b8aa256..83e8f27 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -180,6 +180,86 @@
 		};
 	};
 
+	pinctrl: pinctrl@fcfe3000 {
+		compatible = "renesas,r7s72100-ports";
+
+		#pinctrl-cells = <1>;
+
+		reg = <0xfcfe3000 0x42C0>;
+
+		port0: gpio-0 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 0 6>;
+		};
+
+		port1: gpio-1 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 16 16>;
+		};
+
+		port2: gpio-2 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 32 16>;
+		};
+
+		port3: gpio-3 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 48 16>;
+		};
+
+		port4: gpio-4 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 64 16>;
+		};
+
+		port5: gpio-5 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 80 11>;
+		};
+
+		port6: gpio-6 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 96 16>;
+		};
+
+		port7: gpio-7 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 112 16>;
+		};
+
+		port8: gpio-8 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 128 16>;
+		};
+
+		port9: gpio-9 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 144 8>;
+		};
+
+		port10: gpio-10 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 160 16>;
+		};
+
+		port11: gpio-11 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 176 16>;
+		};
+	};
+
 	scif0: serial@e8007000 {
 		compatible = "renesas,scif-r7s72100", "renesas,scif";
 		reg = <0xe8007000 64>;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 4/7] arm: dts: r7s72100: Add pin controller node

[Jacopo Mondi]

Add pin controller node with 12 gpio controller sub-nodes to
r7s72100 dtsi.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r7s72100.dtsi | 80 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index b8aa256..83e8f27 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -180,6 +180,86 @@
 		};
 	};
 
+	pinctrl: pinctrl@fcfe3000 {
+		compatible = "renesas,r7s72100-ports";
+
+		#pinctrl-cells = <1>;
+
+		reg = <0xfcfe3000 0x42C0>;
+
+		port0: gpio-0 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 0 6>;
+		};
+
+		port1: gpio-1 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 16 16>;
+		};
+
+		port2: gpio-2 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 32 16>;
+		};
+
+		port3: gpio-3 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 48 16>;
+		};
+
+		port4: gpio-4 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 64 16>;
+		};
+
+		port5: gpio-5 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 80 11>;
+		};
+
+		port6: gpio-6 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 96 16>;
+		};
+
+		port7: gpio-7 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 112 16>;
+		};
+
+		port8: gpio-8 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 128 16>;
+		};
+
+		port9: gpio-9 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 144 8>;
+		};
+
+		port10: gpio-10 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 160 16>;
+		};
+
+		port11: gpio-11 {
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-ranges = <&pinctrl 0 176 16>;
+		};
+	};
+
 	scif0: serial@e8007000 {
 		compatible = "renesas,scif-r7s72100", "renesas,scif";
 		reg = <0xe8007000 64>;
-- 
2.7.4

Re: [PATCH v3 4/7] arm: dts: r7s72100: Add pin controller node

[Sergei Shtylyov]

Hello!

On 03/24/2017 06:22 PM, Jacopo Mondi wrote:

> Add pin controller node with 12 gpio controller sub-nodes to
> r7s72100 dtsi.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm/boot/dts/r7s72100.dtsi | 80 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>
> diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
> index b8aa256..83e8f27 100644
> --- a/arch/arm/boot/dts/r7s72100.dtsi
> +++ b/arch/arm/boot/dts/r7s72100.dtsi
> @@ -180,6 +180,86 @@
>  		};
>  	};
>
> +	pinctrl: pinctrl@fcfe3000 {

    I suggest to call it "pin-controller@...", no need to reduce.

[...]

MBR, Sergei

RE: [PATCH v3 4/7] arm: dts: r7s72100: Add pin controller node

[Chris Brandt]

Hi Jacopo,


On Friday, March 24, 2017, Jacopo Mondi
> +	pinctrl: pinctrl@fcfe3000 {
> +		compatible = "renesas,r7s72100-ports";
> +
> +		#pinctrl-cells = <1>;
> +
> +		reg = <0xfcfe3000 0x42C0>;

Out of curiosity, why did you change this from 0x4248 to 0x42C0?

In your update for renesas,rza1-pinctrl.txt, for the 'Example', you changed it from 0x4248 to 0x4230 (which Geert pointed out makes more sense), but then in the actual DT file you changed it to 0x42C0. Typo?


Chris

RE: [PATCH v3 4/7] arm: dts: r7s72100: Add pin controller node

[Chris Brandt]

Hi Jacopo,


On Friday, March 24, 2017, Jacopo Mondi
> +	pinctrl: pinctrl@fcfe3000 {
> +		compatible = "renesas,r7s72100-ports";
> +
> +		#pinctrl-cells = <1>;
> +
> +		reg = <0xfcfe3000 0x42C0>;

Out of curiosity, why did you change this from 0x4248 to 0x42C0?

In your update for renesas,rza1-pinctrl.txt, for the 'Example', you changed it from 0x4248 to 0x4230 (which Geert pointed out makes more sense), but then in the actual DT file you changed it to 0x42C0. Typo?


Chris

Re: [PATCH v3 4/7] arm: dts: r7s72100: Add pin controller node

[jacopo]

Hi Chris,

On Mon, Mar 27, 2017 at 05:12:04PM +0000, Chris Brandt wrote:
> Hi Jacopo,
> 
> 
> On Friday, March 24, 2017, Jacopo Mondi
> > +	pinctrl: pinctrl@fcfe3000 {
> > +		compatible = "renesas,r7s72100-ports";
> > +
> > +		#pinctrl-cells = <1>;
> > +
> > +		reg = <0xfcfe3000 0x42C0>;
> 
> Out of curiosity, why did you change this from 0x4248 to 0x42C0?
> 
> In your update for renesas,rza1-pinctrl.txt, for the 'Example', you changed it from 0x4248 to 0x4230 (which Geert pointed out makes more sense), but then in the actual DT file you changed it to 0x42C0. Typo?
>

Yes, type.
Or inability to perform basic mathematical operations.
Not sure yet.

Thanks for spotting this :)
   j


> 
> Chris
> 

Re: [PATCH v3 4/7] arm: dts: r7s72100: Add pin controller node

[jacopo]

Hi Chris,

On Mon, Mar 27, 2017 at 05:12:04PM +0000, Chris Brandt wrote:
> Hi Jacopo,
> 
> 
> On Friday, March 24, 2017, Jacopo Mondi
> > +	pinctrl: pinctrl@fcfe3000 {
> > +		compatible = "renesas,r7s72100-ports";
> > +
> > +		#pinctrl-cells = <1>;
> > +
> > +		reg = <0xfcfe3000 0x42C0>;
> 
> Out of curiosity, why did you change this from 0x4248 to 0x42C0?
> 
> In your update for renesas,rza1-pinctrl.txt, for the 'Example', you changed it from 0x4248 to 0x4230 (which Geert pointed out makes more sense), but then in the actual DT file you changed it to 0x42C0. Typo?
>

Yes, type.
Or inability to perform basic mathematical operations.
Not sure yet.

Thanks for spotting this :)
   j


> 
> Chris
> 

[PATCH v3 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc

[Jacopo Mondi]

Add device tree bindings documentation for Renesas RZ/A1 gpio and pin
controller.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 .../bindings/pinctrl/renesas,rza1-pinctrl.txt      | 143 +++++++++++++++++++++
 1 file changed, 143 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt

diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
new file mode 100644
index 0000000..69aafd1
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
@@ -0,0 +1,143 @@
+Renesas RZ/A1 combined Pin and GPIO controller
+
+The Renesas SoCs of RZ/A1 family feature a combined Pin and GPIO controller
+hardware controller, named "Ports" in the hardware reference manual.
+Pin multiplexing and GPIO configuration is performed on a per-pin basis
+writing configuration values to per-port register sets.
+Each "port" features up to 16 pins, each of them configurable for GPIO
+function (port mode) or in alternate function mode.
+Up to 8 different alternate function modes exist for each single pin.
+
+Pin controller node
+-------------------
+
+Required properties:
+  - compatible
+    this shall be "renesas,r7s72100-ports".
+
+  - #pinctrl-cells
+    as defined by pinctrl-bindings.txt, this is the number
+    of cells (in addition to pin index) required to configure a single pin.
+    Shall be set to 1.
+
+  - reg
+    address base and length of the memory area where pin controller
+    hardware is mapped to.
+
+Example:
+Pin controller node for RZ/A1H SoC (r7s72100)
+
+pinctrl: pinctrl@fcfe3000 {
+	compatible = "renesas,r7s72100-ports";
+
+	#pinctrl-cells = <1>;
+
+	reg = <0xfcfe3000 0x4230>;
+};
+
+Sub-nodes
+---------
+
+The child nodes of the pin controller node describe a pin multiplexing
+function or a gpio controller alternatively.
+
+- Pin multiplexing sub-nodes:
+  A pin multiplexing sub-node describes how to configure a set of
+  (or a single) pin in some desired alternate function mode.
+  A single sub-node may define several pin configurations groups.
+
+  Required properties:
+    - renesas,pins
+      describes an array of pin multiplexing configurations.
+      When a pin has to be configured in alternate function mode, use this
+      property to identify the pin by its global index, and provide its
+      alternate function configuration description along with it.
+      When multiple pins are required to be configured as part of the same
+      alternate function (odds are single-pin alternate functions exist) they
+      shall be specified as members of the same argument list of a single
+      "renesas-pins" property.
+      Helper macros to ease calculating the pin index from its position
+      (port where it sits on and pin offset), and alternate function
+      configuration options are provided in the pin controller header file at:
+      include/dt-bindings/pinctrl/r7s72100-pinctrl.h
+
+  Example:
+  A serial communication interface with a TX output pin and an RX input pin.
+
+  &pinctrl {
+	scif2_pins: serial2 {
+		renesas,pins = <PIN(3, 0) 6>, <PIN(3, 2) 4>;
+	};
+  }
+
+  Pin #0 on port #3 is configured as alternate function #6.
+  Pin #2 on port #3 is configured as alternate function #4.
+
+  Example 2:
+  I2c master: both SDA and SCL pins need bi-directional operations
+
+  &pinctrl {
+	i2c2_pins: i2c2 {
+		renesas,pins = <PIN(1, 4) (1 | BI_DIR)>,
+			       <PIN(1, 5) (1 | BI_DIR)>;
+	};
+  }
+
+  Pin #4 on port #1 is configured as alternate function #1.
+  Pin #5 on port #1 is configured as alternate function #1.
+  Both need to work in bi-directional mode.
+
+  Example 3:
+  Multi-function timer input and output compare pins.
+  The hardware manual prescribes this pins to have input/output direction
+  specified by software. Configure TIOC0A as input and TIOC0B as output.
+
+  &pinctrl {
+	tioc0_pins: tioc0 {
+		renesas,pins = <PIN(4, 0) (2 | SWIO_IN)>,
+			       <PIN(4, 1) (2 | SWIO_OUT)>;
+	};
+  }
+
+  Pin #0 on port #4 is configured as alternate function #2 with IO direction
+  specified by software as input.
+  Pin #1 on port #4 is configured as alternate function #1 with IO direction
+  specified by software as output.
+
+- GPIO controller sub-nodes:
+  Each port of the r7s72100 pin controller hardware is itself a gpio controller.
+  Different SoCs have different number of available pins per port, but
+  generally speaking, each of them can be configured in GPIO ("port") mode
+  on this hardware.
+  Describe gpio-controllers using sub-nodes with the following properties.
+
+  Required properties:
+    - gpio-controller
+      empty property as defined by the gpio bindings documentation.
+    - #gpio-cells
+      number of cells required to identify and configure a GPIO.
+      Shall be 2.
+    - gpio-ranges
+      Describes a gpio controller specifying its specific pin base, the pin
+      base in the global pin numbering space, and the number of controlled
+      pins, as defined by the gpio bindings documentation. Refer to this file
+      for a more detailed description.
+
+  Example:
+  A gpio controller node, controlling 16 pins indexed from 0.
+  The gpio controller base in the global pin indexing space is pin 48, thus
+  pins [0 - 15] on this controller map to pins [48 - 63] in the global pin
+  indexing space.
+
+  port3: gpio-3 {
+	gpio-controller;
+	#gpio-cells = <2>;
+	gpio-ranges = <&pinctrl 0 48 16>;
+  };
+
+  A device node willing to use pins controlled by this gpio controller, shall
+  refer to it as follows:
+
+  led1 {
+	gpios = <&port3 10 GPIO_ACTIVE_LOW>;
+  };
-- 
2.7.4

Re: [PATCH v3 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc

[Rob Herring]

On Fri, Mar 24, 2017 at 04:22:09PM +0100, Jacopo Mondi wrote:
> Add device tree bindings documentation for Renesas RZ/A1 gpio and pin
> controller.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> ---
>  .../bindings/pinctrl/renesas,rza1-pinctrl.txt      | 143 +++++++++++++++++++++
>  1 file changed, 143 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc

[Rob Herring]

On Fri, Mar 24, 2017 at 04:22:09PM +0100, Jacopo Mondi wrote:
> Add device tree bindings documentation for Renesas RZ/A1 gpio and pin
> controller.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  .../bindings/pinctrl/renesas,rza1-pinctrl.txt      | 143 +++++++++++++++++++++
>  1 file changed, 143 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc

[jacopo]

Hi Rob,
   thanks for the ack

On Thu, Mar 30, 2017 at 05:39:03PM -0500, Rob Herring wrote:
> On Fri, Mar 24, 2017 at 04:22:09PM +0100, Jacopo Mondi wrote:
> > Add device tree bindings documentation for Renesas RZ/A1 gpio and pin
> > controller.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
> > ---
> >  .../bindings/pinctrl/renesas,rza1-pinctrl.txt      | 143 +++++++++++++++++++++
> >  1 file changed, 143 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> 
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Just an heads-up: in v4 dt bindings will change considerably to meet a
format more close to standard pin controller bindings.

I cannot take you ack here and apply to next iteration as it will
require you or other dt guardians to have a look at it again. Sorry
about this.

Thanks
   j

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc

[jacopo]

Hi Rob,
   thanks for the ack

On Thu, Mar 30, 2017 at 05:39:03PM -0500, Rob Herring wrote:
> On Fri, Mar 24, 2017 at 04:22:09PM +0100, Jacopo Mondi wrote:
> > Add device tree bindings documentation for Renesas RZ/A1 gpio and pin
> > controller.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  .../bindings/pinctrl/renesas,rza1-pinctrl.txt      | 143 +++++++++++++++++++++
> >  1 file changed, 143 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pinctrl/renesas,rza1-pinctrl.txt
> 
> Acked-by: Rob Herring <robh@kernel.org>

Just an heads-up: in v4 dt bindings will change considerably to meet a
format more close to standard pin controller bindings.

I cannot take you ack here and apply to next iteration as it will
require you or other dt guardians to have a look at it again. Sorry
about this.

Thanks
   j

[PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Jacopo Mondi]

Add dt-bindings for Renesas r7s72100 pin controller header file.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 include/dt-bindings/pinctrl/r7s72100-pinctrl.h | 36 ++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 include/dt-bindings/pinctrl/r7s72100-pinctrl.h

diff --git a/include/dt-bindings/pinctrl/r7s72100-pinctrl.h b/include/dt-bindings/pinctrl/r7s72100-pinctrl.h
new file mode 100644
index 0000000..170338b
--- /dev/null
+++ b/include/dt-bindings/pinctrl/r7s72100-pinctrl.h
@@ -0,0 +1,36 @@
+/*
+ * Defines macros and constants for Renesas RZ/A1 pin controller pin
+ * muxing functions.
+ */
+#ifndef __DT_BINDINGS_PINCTRL_RENESAS_RZA1_H
+#define __DT_BINDINGS_PINCTRL_RENESAS_RZA1_H
+
+#define RZA1_PINS_PER_PORT	16
+
+/* Create the pin index from its bank and position numbers */
+#define PIN(b, p)		((b) * RZA1_PINS_PER_PORT + (p))
+
+/*
+ * Flags to apply to alternate function configuration
+ * All of the following are mutually exclusive.
+ */
+
+/*
+ * Pin is bi-directional.
+ * An alternate function that needs both input/output functionalities shall
+ * be configured as bidirectional.
+ * Eg. SDA/SCL pins of an I2c interface.
+ */
+#define BI_DIR			(1 << 3)
+
+/*
+ * Flags used to ask software to drive the pin I/O direction overriding the
+ * alternate function configuration.
+ * Some alternate functions require software to force I/O direction of a pin,
+ * overriding the designated one.
+ * Refer to the HW manual to know when this flag shall be used.
+ */
+#define SWIO_IN			(1 << 4)
+#define SWIO_OUT		(1 << 5)
+
+#endif
-- 
2.7.4

Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Linus Walleij]

On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org> wrote:

> Add dt-bindings for Renesas r7s72100 pin controller header file.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

> +/*
> + * Pin is bi-directional.
> + * An alternate function that needs both input/output functionalities shall
> + * be configured as bidirectional.
> + * Eg. SDA/SCL pins of an I2c interface.
> + */
> +#define BI_DIR                 (1 << 3)

Any specific reason why this should not simply be added to
include/linux/pinctrl/pinconf-generic.h
as PIN_CONFIG_BIDIRECTIONAL and parsed in
drivers/pinctrl/pinconf-generic.c
from the (new) DT property "bidirectional" simply?

> +/*
> + * Flags used to ask software to drive the pin I/O direction overriding the
> + * alternate function configuration.
> + * Some alternate functions require software to force I/O direction of a pin,
> + * overriding the designated one.
> + * Refer to the HW manual to know when this flag shall be used.
> + */
> +#define SWIO_IN                        (1 << 4)
> +#define SWIO_OUT               (1 << 5)

What is wrong in doing this with generic pin config using
PIN_CONFIG_INPUT_ENABLE and PIN_CONFIG_OUTPUT
(ignoring the argument)?

In the device tree use input-enable and add a new output-enable
(with unspecified value) with proper description and DT bindings?

And if you think these have no general applicability, by the end
of the day they are *still* pin config, not magic flags we can choose to
toss in with the muxing, so you can do what the Qualcomm driver
does and add custom pin configurations extending the generic
pin config, see drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
qcom,pull-up-strength etc.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Linus Walleij]

On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:

> Add dt-bindings for Renesas r7s72100 pin controller header file.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

> +/*
> + * Pin is bi-directional.
> + * An alternate function that needs both input/output functionalities shall
> + * be configured as bidirectional.
> + * Eg. SDA/SCL pins of an I2c interface.
> + */
> +#define BI_DIR                 (1 << 3)

Any specific reason why this should not simply be added to
include/linux/pinctrl/pinconf-generic.h
as PIN_CONFIG_BIDIRECTIONAL and parsed in
drivers/pinctrl/pinconf-generic.c
from the (new) DT property "bidirectional" simply?

> +/*
> + * Flags used to ask software to drive the pin I/O direction overriding the
> + * alternate function configuration.
> + * Some alternate functions require software to force I/O direction of a pin,
> + * overriding the designated one.
> + * Refer to the HW manual to know when this flag shall be used.
> + */
> +#define SWIO_IN                        (1 << 4)
> +#define SWIO_OUT               (1 << 5)

What is wrong in doing this with generic pin config using
PIN_CONFIG_INPUT_ENABLE and PIN_CONFIG_OUTPUT
(ignoring the argument)?

In the device tree use input-enable and add a new output-enable
(with unspecified value) with proper description and DT bindings?

And if you think these have no general applicability, by the end
of the day they are *still* pin config, not magic flags we can choose to
toss in with the muxing, so you can do what the Qualcomm driver
does and add custom pin configurations extending the generic
pin config, see drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
qcom,pull-up-strength etc.

Yours,
Linus Walleij

RE: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Chris Brandt]

On Wednesday, March 29, 2017, Linus Walleij wrote:
> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org>
> wrote:
> 
> > Add dt-bindings for Renesas r7s72100 pin controller header file.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> > +/*
> > + * Pin is bi-directional.
> > + * An alternate function that needs both input/output functionalities
> > +shall
> > + * be configured as bidirectional.
> > + * Eg. SDA/SCL pins of an I2c interface.
> > + */
> > +#define BI_DIR                 (1 << 3)
> 
> Any specific reason why this should not simply be added to
> include/linux/pinctrl/pinconf-generic.h
> as PIN_CONFIG_BIDIRECTIONAL and parsed in drivers/pinctrl/pinconf-
> generic.c from the (new) DT property "bidirectional" simply?

I see your point. It would cut down from every driver out there
inventing some new property or config instead of everyone just sharing
a fixed set.
Maybe someone else out there will end up having a need for a
"bidirectional" option.


> > +/*
> > + * Flags used to ask software to drive the pin I/O direction
> > +overriding the
> > + * alternate function configuration.
> > + * Some alternate functions require software to force I/O direction
> > +of a pin,
> > + * overriding the designated one.
> > + * Refer to the HW manual to know when this flag shall be used.
> > + */
> > +#define SWIO_IN                        (1 << 4)
> > +#define SWIO_OUT               (1 << 5)
> 
> What is wrong in doing this with generic pin config using
> PIN_CONFIG_INPUT_ENABLE and PIN_CONFIG_OUTPUT (ignoring the argument)?
> 
> In the device tree use input-enable and add a new output-enable (with
> unspecified value) with proper description and DT bindings?

Again, that's probably fine. It seems we are still doing the same thing
which is using the DT to pass extra config information to the driver.
And, we can do whatever we want with that info.


> And if you think these have no general applicability, by the end of the
> day they are *still* pin config, not magic flags we can choose to toss in
> with the muxing, so you can do what the Qualcomm driver does and add
> custom pin configurations extending the generic pin config, see
> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> qcom,pull-up-strength etc.

But, it seems that when you set a config option, it applies to everything
in "pins"?

I2C Example: (seem OK)
	/* P1_6 = RIIC3SCL (bi dir) */
	/* P1_7 = RIIC3SDA (bi dir) */
	i2c3_pins: i2c3 {
		pins = <PIN(1, 6) | FUNC_1>,
		       <PIN(1, 7) | FUNC_1>;
		bidirectional;
	};


But, what do we do for Ethernet? All the pins are "normal" except just
the MDIO pin needs to be bidirectional.
That's the part I'm confused by.
How do we flag that just the ET_MDIO needs "bidirectional"?

	/* Ethernet */
	ether_pins: ether {
		/* Ethernet on Ports 1,2,3,5 */
		pins = <PIN(1, 14) | FUNC_4)>, /* P1_14 = ET_COL */
			<PIN(5, 9) | FUNC_2)>,	/* P5_9 = ET_MDC */
			<PIN(3, 3) | FUNC_2)>,	/* P3_3 = ET_MDIO !!!! (bi dir) !!!!!! */
			<PIN(3, 4) | FUNC_2)>,	/* P3_4 = ET_RXCLK */
			<PIN(3, 5) | FUNC_2)>,	/* P3_5 = ET_RXER */
			<PIN(3, 6) | FUNC_2)>,	/* P3_6 = ET_RXDV */
			<PIN(2, 0) | FUNC_2)>,	/* P2_0 = ET_TXCLK */
			<PIN(2, 1) | FUNC_2)>,	/* P2_1 = ET_TXER */
			<PIN(2, 2) | FUNC_2)>,	/* P2_2 = ET_TXEN */
			<PIN(2, 3) | FUNC_2)>,	/* P2_3 = ET_CRS */
			<PIN(2, 4) | FUNC_2)>,	/* P2_4 = ET_TXD0 */
			<PIN(2, 5) | FUNC_2)>,	/* P2_5 = ET_TXD1 */
			<PIN(2, 6) | FUNC_2)>,	/* P2_6 = ET_TXD2 */
			<PIN(2, 7) | FUNC_2)>,	/* P2_7 = ET_TXD3 */
			<PIN(2, 8) | FUNC_2)>,	/* P2_8 = ET_RXD0 */
			<PIN(2, 9) | FUNC_2)>,	/* P2_9 = ET_RXD1 */
			<PIN(2, 10) | FUNC_2)>, /* P2_10 = ET_RXD2 */
			<PIN(2, 11) | FUNC_2)>; /* P2_11 = ET_RXD3 */
	};


Chris

Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Geert Uytterhoeven]

Hi Chris,

On Wed, Mar 29, 2017 at 4:55 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Wednesday, March 29, 2017, Linus Walleij wrote:
>> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
>> > +/*
>> > + * Flags used to ask software to drive the pin I/O direction
>> > +overriding the
>> > + * alternate function configuration.
>> > + * Some alternate functions require software to force I/O direction
>> > +of a pin,
>> > + * overriding the designated one.
>> > + * Refer to the HW manual to know when this flag shall be used.
>> > + */
>> > +#define SWIO_IN                        (1 << 4)
>> > +#define SWIO_OUT               (1 << 5)
>>
>> What is wrong in doing this with generic pin config using
>> PIN_CONFIG_INPUT_ENABLE and PIN_CONFIG_OUTPUT (ignoring the argument)?
>>
>> In the device tree use input-enable and add a new output-enable (with
>> unspecified value) with proper description and DT bindings?
>
> Again, that's probably fine. It seems we are still doing the same thing
> which is using the DT to pass extra config information to the driver.
> And, we can do whatever we want with that info.
>
>
>> And if you think these have no general applicability, by the end of the
>> day they are *still* pin config, not magic flags we can choose to toss in
>> with the muxing, so you can do what the Qualcomm driver does and add
>> custom pin configurations extending the generic pin config, see
>> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> qcom,pull-up-strength etc.
>
> But, it seems that when you set a config option, it applies to everything
> in "pins"?
>
> I2C Example: (seem OK)
>         /* P1_6 = RIIC3SCL (bi dir) */
>         /* P1_7 = RIIC3SDA (bi dir) */
>         i2c3_pins: i2c3 {
>                 pins = <PIN(1, 6) | FUNC_1>,
>                        <PIN(1, 7) | FUNC_1>;
>                 bidirectional;
>         };

Correct.

> But, what do we do for Ethernet? All the pins are "normal" except just
> the MDIO pin needs to be bidirectional.
> That's the part I'm confused by.
> How do we flag that just the ET_MDIO needs "bidirectional"?

You add subnodes, cfr. arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts:

        avb_pins: avb {
                mux {
                        groups = "avb_link", "avb_phy_int", "avb_mdc",
                                 "avb_mii";
                        function = "avb";
                };

                pins_mdc {
                        groups = "avb_mdc";
                        drive-strength = <24>;
                };

                pins_mii_tx {
                        pins = "PIN_AVB_TX_CTL", "PIN_AVB_TXC", "PIN_AVB_TD0",
                               "PIN_AVB_TD1", "PIN_AVB_TD2", "PIN_AVB_TD3";
                        drive-strength = <12>;
                };
        };

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 v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Chris Brandt]

Hi Geert,

On Wednesday, March 29, 2017, Geert Uytterhoeven wrote:
> > But, what do we do for Ethernet? All the pins are "normal" except just
> > the MDIO pin needs to be bidirectional.
> > That's the part I'm confused by.
> > How do we flag that just the ET_MDIO needs "bidirectional"?
> 
> You add subnodes, cfr. arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts:
> 
>         avb_pins: avb {
>                 mux {
>                         groups = "avb_link", "avb_phy_int", "avb_mdc",
>                                  "avb_mii";
>                         function = "avb";
>                 };
> 
>                 pins_mdc {
>                         groups = "avb_mdc";
>                         drive-strength = <24>;
>                 };
> 
>                 pins_mii_tx {
>                         pins = "PIN_AVB_TX_CTL", "PIN_AVB_TXC",
> "PIN_AVB_TD0",
>                                "PIN_AVB_TD1", "PIN_AVB_TD2",
> "PIN_AVB_TD3";
>                         drive-strength = <12>;
>                 };
>         };

Oh, so there is a way!

Thank you.


So, for clarity:

	/* Ethernet */
	ether_pins: ether {
		/* Ethernet on Ports 1,2,3,5 */
		mux {
			pins = <PIN(1, 14) | FUNC_4)>, /* P1_14 = ET_COL */
				<PIN(5, 9) | FUNC_2)>,	/* P5_9 = ET_MDC */
				<PIN(3, 4) | FUNC_2)>,	/* P3_4 = ET_RXCLK */
				<PIN(3, 5) | FUNC_2)>,	/* P3_5 = ET_RXER */
				<PIN(3, 6) | FUNC_2)>,	/* P3_6 = ET_RXDV */
				<PIN(2, 0) | FUNC_2)>,	/* P2_0 = ET_TXCLK */
				<PIN(2, 1) | FUNC_2)>,	/* P2_1 = ET_TXER */
				<PIN(2, 2) | FUNC_2)>,	/* P2_2 = ET_TXEN */
				<PIN(2, 3) | FUNC_2)>,	/* P2_3 = ET_CRS */
				<PIN(2, 4) | FUNC_2)>,	/* P2_4 = ET_TXD0 */
				<PIN(2, 5) | FUNC_2)>,	/* P2_5 = ET_TXD1 */
				<PIN(2, 6) | FUNC_2)>,	/* P2_6 = ET_TXD2 */
				<PIN(2, 7) | FUNC_2)>,	/* P2_7 = ET_TXD3 */
				<PIN(2, 8) | FUNC_2)>,	/* P2_8 = ET_RXD0 */
				<PIN(2, 9) | FUNC_2)>,	/* P2_9 = ET_RXD1 */
				<PIN(2, 10) | FUNC_2)>, /* P2_10 = ET_RXD2 */
				<PIN(2, 11) | FUNC_2)>; /* P2_11 = ET_RXD3 */
		};
		pins_bidir {
			pins = <PIN(3, 3) | FUNC_2)>,	/* P3_3 = ET_MDIO */
			bidirectional;
		};
	};



Chris

RE: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Chris Brandt]

On Wednesday, March 29, 2017, Chris Brandt wrote:
> On Wednesday, March 29, 2017, Geert Uytterhoeven wrote:
> > > But, what do we do for Ethernet? All the pins are "normal" except
> > > just the MDIO pin needs to be bidirectional.
> > > That's the part I'm confused by.
> > > How do we flag that just the ET_MDIO needs "bidirectional"?
> >
> > You add subnodes, cfr. arch/arm64/boot/dts/renesas/r8a7795-salvator-
> x.dts:
> >
> >         avb_pins: avb {
> >                 mux {
> >                         groups = "avb_link", "avb_phy_int", "avb_mdc",
> >                                  "avb_mii";
> >                         function = "avb";
> >                 };
> >
> >                 pins_mdc {
> >                         groups = "avb_mdc";
> >                         drive-strength = <24>;
> >                 };
> >
> >                 pins_mii_tx {
> >                         pins = "PIN_AVB_TX_CTL", "PIN_AVB_TXC",
> > "PIN_AVB_TD0",
> >                                "PIN_AVB_TD1", "PIN_AVB_TD2",
> > "PIN_AVB_TD3";
> >                         drive-strength = <12>;
> >                 };
> >         };
> 
> Oh, so there is a way!
> 
> Thank you.
> 
> 
> So, for clarity:


Actually, Linus's request was to use "pinmux", not "pins".

So, it should be:

	/* P1_6 = RIIC3SCL (bi dir) */
	/* P1_7 = RIIC3SDA (bi dir) */
	i2c3_pins: i2c3 {
		pinmux = <PIN(1, 6) | FUNC_1>,
		         <PIN(1, 7) | FUNC_1>;
		bidirectional;
	};

 
	/* Ethernet */
	ether_pins: ether {
		/* Ethernet on Ports 1,2,3,5 */
		mux {
			pinmux = <PIN(1, 14) | FUNC_4)>, /* P1_14 = ET_COL */
				<PIN(5, 9) | FUNC_2)>,	/* P5_9 = ET_MDC */
				<PIN(3, 4) | FUNC_2)>,	/* P3_4 = ET_RXCLK */
				<PIN(3, 5) | FUNC_2)>,	/* P3_5 = ET_RXER */
				<PIN(3, 6) | FUNC_2)>,	/* P3_6 = ET_RXDV */
				<PIN(2, 0) | FUNC_2)>,	/* P2_0 = ET_TXCLK */
				<PIN(2, 1) | FUNC_2)>,	/* P2_1 = ET_TXER */
				<PIN(2, 2) | FUNC_2)>,	/* P2_2 = ET_TXEN */
				<PIN(2, 3) | FUNC_2)>,	/* P2_3 = ET_CRS */
				<PIN(2, 4) | FUNC_2)>,	/* P2_4 = ET_TXD0 */
				<PIN(2, 5) | FUNC_2)>,	/* P2_5 = ET_TXD1 */
				<PIN(2, 6) | FUNC_2)>,	/* P2_6 = ET_TXD2 */
				<PIN(2, 7) | FUNC_2)>,	/* P2_7 = ET_TXD3 */
				<PIN(2, 8) | FUNC_2)>,	/* P2_8 = ET_RXD0 */
				<PIN(2, 9) | FUNC_2)>,	/* P2_9 = ET_RXD1 */
				<PIN(2, 10) | FUNC_2)>, /* P2_10 = ET_RXD2 */
				<PIN(2, 11) | FUNC_2)>; /* P2_11 = ET_RXD3 */
		};
		pins_bidir {
			pinmux = <PIN(3, 3) | FUNC_2)>,	/* P3_3 = ET_MDIO */
			bidirectional;
		};
	};


  NOTE: "FUNC_2" can just be "2" as per Geert's request.



Chris

Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Linus Walleij]

On Wed, Mar 29, 2017 at 4:55 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Wednesday, March 29, 2017, Linus Walleij wrote:
>> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org>
>> wrote:
>>
>> > Add dt-bindings for Renesas r7s72100 pin controller header file.
>> >
>> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>>
>> > +/*
>> > + * Pin is bi-directional.
>> > + * An alternate function that needs both input/output functionalities
>> > +shall
>> > + * be configured as bidirectional.
>> > + * Eg. SDA/SCL pins of an I2c interface.
>> > + */
>> > +#define BI_DIR                 (1 << 3)
>>
>> Any specific reason why this should not simply be added to
>> include/linux/pinctrl/pinconf-generic.h
>> as PIN_CONFIG_BIDIRECTIONAL and parsed in drivers/pinctrl/pinconf-
>> generic.c from the (new) DT property "bidirectional" simply?
>
> I see your point. It would cut down from every driver out there
> inventing some new property or config instead of everyone just sharing
> a fixed set.
> Maybe someone else out there will end up having a need for a
> "bidirectional" option.

I was thinking about that one. It is a bit weird electrically, like what
kind of electronics is really bidirectional?

It seems like a fancy name for open drain/open source, what we
call "single ended" configuration. (See docs in
Documentation/gpio/driver.txt)

It would be great if you could check if that is what they mean,
actually.

> But, what do we do for Ethernet? All the pins are "normal" except just
> the MDIO pin needs to be bidirectional.

I see Geert clarified what we could do here.

Yours,
Linus Walleij

RE: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Chris Brandt]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4517 bytes --]

On Thursday, March 30, 2017, Linus Walleij wrote:
> >> > +/*
> >> > + * Pin is bi-directional.
> >> > + * An alternate function that needs both input/output
> >> > +functionalities shall
> >> > + * be configured as bidirectional.
> >> > + * Eg. SDA/SCL pins of an I2c interface.
> >> > + */
> >> > +#define BI_DIR                 (1 << 3)
> >>
> >> Any specific reason why this should not simply be added to
> >> include/linux/pinctrl/pinconf-generic.h
> >> as PIN_CONFIG_BIDIRECTIONAL and parsed in drivers/pinctrl/pinconf-
> >> generic.c from the (new) DT property "bidirectional" simply?
> >
> > I see your point. It would cut down from every driver out there
> > inventing some new property or config instead of everyone just sharing
> > a fixed set.
> > Maybe someone else out there will end up having a need for a
> > "bidirectional" option.
> 
> I was thinking about that one. It is a bit weird electrically, like what
> kind of electronics is really bidirectional?
> 
> It seems like a fancy name for open drain/open source, what we call
> "single ended" configuration. (See docs in
> Documentation/gpio/driver.txt)
> 
> It would be great if you could check if that is what they mean, actually.

Here is the definition of the register in the hardware manual:

---
54.3.13 Port Bidirection Control Register (PBDCn)

This register enables or disables the input buffer while the output buffer is enabled. When the input buffer is enabled
while the output buffer is enabled, the bidirectional mode is entered, allowing the level of the Pn_m pin to always be read
via the PPRn.PPRnm bit.
---


But...what they don't really tell you is that any peripheral that needs to TX and RX data over a pin needs this set.


In the hardware manual, there is a pretty picture (Figure 54.1 Logical Diagram of Port Control) that shows a detailed logic diagram of the interface between the peripheral bus and the pin pad.
As far as I can tell, the "function mux" portion of this controller only knows how to set a pin as direction=in or direction=out.
So, in the case of I2C where each the SCL and SDA pins needs to be driven and read...it can't do that.
Therefore, there is an additional register to manually enable the input buffer and let the mux enable the output buffer.

So while yes, I2C is open-drain, this is also the case for the data pins for the SDHI. And the MDIO pin from Ethernet. It really has to do with the fact that this pin controller wasn't designed to enable both the input and output buffers at the same time.

The situation is similar for our SWIO_IN and SWIO_OUT flags. For example, to use the SSI sound interface, you have to set the TX signals to "input" (SWIO_IN). Makes sense, right??

I could argue that all of these "FLAGS" settings should have been incorporated in the HW logic of the pin controller...but for whatever reason, the they had to make them separate registers and make SW do it.
So, I could argue that these registers settings are really part of pin muxing, not really user config....and hence belong in the "pinmux" property.


How about this compromise: Instead of BI_DIR, we use "TYPE_I2C", "TYPE_SDDATA", "TYPE_MDIO", "TYPE_LVDS", etc... to describe the 'special' ones. That way it can go back under "pinmux". Like Jacopo said, these register settings are really supposed to be set when you are selecting the pin-mux option.


Of course behind the scenes, it's really:
  #define TYPE_I2C    BI_DIR 
  #define TYPE_SDDATA BI_DIR
  #define TYPE_SDCMD  BI_DIR
  #define TYPE_LVDS   SWIO_OUT

Examples:

	i2c3_pins: i2c3 {
		pinmux = <PIN(1, 6) | FUNC_1 | TYPE_I2C>,
		         <PIN(1, 7) | FUNC_1 | TYPE_I2C>;
	};

	/* SHDI ch1 on CN1 */
	sdhi1_pins: sdhi1 {
		/* SHDI ch1 on Port 3 */
		pinmux = <PIN(3, 8) | FUNC_7>,			/* SDHI1 CD */
				<PIN(3, 9) FUNC_7)>,			/* SDHI1 WP */
				<PIN(3, 10) FUNC_7 | TYPE_SDDATA)>,	/* SDHI1 DAT1 */
				<PIN(3, 11) FUNC_7 | TYPE_SDDATA)>,	/* SDHI1 DAT0 */
				<PIN(3, 12) FUNC_7)>,			/* SDHI1 CLK */
				<PIN(3, 13) FUNC_7 | TYPE_SDCMD)>,	/* SDHI1 CMD */
				<PIN(3, 14) FUNC_7 | TYPE_SDDATA)>,	/* SDHI1 DAT3 */
				<PIN(3, 15) FUNC_7 | TYPE_SDDATA)>;	/* SDHI1 DAT2 */
	};



# Honestly, I have no idea where this pin controller came from. I have not seen it in any other Renesas part (Mitsubishi/Hitachi/NEC).


Chris


N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

RE: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Chris Brandt]

On Thursday, March 30, 2017, Linus Walleij wrote:
> >> > +/*
> >> > + * Pin is bi-directional.
> >> > + * An alternate function that needs both input/output
> >> > +functionalities shall
> >> > + * be configured as bidirectional.
> >> > + * Eg. SDA/SCL pins of an I2c interface.
> >> > + */
> >> > +#define BI_DIR                 (1 << 3)
> >>
> >> Any specific reason why this should not simply be added to
> >> include/linux/pinctrl/pinconf-generic.h
> >> as PIN_CONFIG_BIDIRECTIONAL and parsed in drivers/pinctrl/pinconf-
> >> generic.c from the (new) DT property "bidirectional" simply?
> >
> > I see your point. It would cut down from every driver out there
> > inventing some new property or config instead of everyone just sharing
> > a fixed set.
> > Maybe someone else out there will end up having a need for a
> > "bidirectional" option.
> 
> I was thinking about that one. It is a bit weird electrically, like what
> kind of electronics is really bidirectional?
> 
> It seems like a fancy name for open drain/open source, what we call
> "single ended" configuration. (See docs in
> Documentation/gpio/driver.txt)
> 
> It would be great if you could check if that is what they mean, actually.

Here is the definition of the register in the hardware manual:

---
54.3.13 Port Bidirection Control Register (PBDCn)

This register enables or disables the input buffer while the output buffer is enabled. When the input buffer is enabled
while the output buffer is enabled, the bidirectional mode is entered, allowing the level of the Pn_m pin to always be read
via the PPRn.PPRnm bit.
---


But...what they don't really tell you is that any peripheral that needs to TX and RX data over a pin needs this set.


In the hardware manual, there is a pretty picture (Figure 54.1 Logical Diagram of Port Control) that shows a detailed logic diagram of the interface between the peripheral bus and the pin pad.
As far as I can tell, the "function mux" portion of this controller only knows how to set a pin as direction=in or direction=out.
So, in the case of I2C where each the SCL and SDA pins needs to be driven and read...it can't do that.
Therefore, there is an additional register to manually enable the input buffer and let the mux enable the output buffer.

So while yes, I2C is open-drain, this is also the case for the data pins for the SDHI. And the MDIO pin from Ethernet. It really has to do with the fact that this pin controller wasn't designed to enable both the input and output buffers at the same time.

The situation is similar for our SWIO_IN and SWIO_OUT flags. For example, to use the SSI sound interface, you have to set the TX signals to "input" (SWIO_IN). Makes sense, right??

I could argue that all of these "FLAGS" settings should have been incorporated in the HW logic of the pin controller...but for whatever reason, the they had to make them separate registers and make SW do it.
So, I could argue that these registers settings are really part of pin muxing, not really user config....and hence belong in the "pinmux" property.


How about this compromise: Instead of BI_DIR, we use "TYPE_I2C", "TYPE_SDDATA", "TYPE_MDIO", "TYPE_LVDS", etc... to describe the 'special' ones. That way it can go back under "pinmux". Like Jacopo said, these register settings are really supposed to be set when you are selecting the pin-mux option.


Of course behind the scenes, it's really:
  #define TYPE_I2C    BI_DIR 
  #define TYPE_SDDATA BI_DIR
  #define TYPE_SDCMD  BI_DIR
  #define TYPE_LVDS   SWIO_OUT

Examples:

	i2c3_pins: i2c3 {
		pinmux = <PIN(1, 6) | FUNC_1 | TYPE_I2C>,
		         <PIN(1, 7) | FUNC_1 | TYPE_I2C>;
	};

	/* SHDI ch1 on CN1 */
	sdhi1_pins: sdhi1 {
		/* SHDI ch1 on Port 3 */
		pinmux = <PIN(3, 8) | FUNC_7>,			/* SDHI1 CD */
				<PIN(3, 9) FUNC_7)>,			/* SDHI1 WP */
				<PIN(3, 10) FUNC_7 | TYPE_SDDATA)>,	/* SDHI1 DAT1 */
				<PIN(3, 11) FUNC_7 | TYPE_SDDATA)>,	/* SDHI1 DAT0 */
				<PIN(3, 12) FUNC_7)>,			/* SDHI1 CLK */
				<PIN(3, 13) FUNC_7 | TYPE_SDCMD)>,	/* SDHI1 CMD */
				<PIN(3, 14) FUNC_7 | TYPE_SDDATA)>,	/* SDHI1 DAT3 */
				<PIN(3, 15) FUNC_7 | TYPE_SDDATA)>;	/* SDHI1 DAT2 */
	};



# Honestly, I have no idea where this pin controller came from. I have not seen it in any other Renesas part (Mitsubishi/Hitachi/NEC).


Chris

Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[jacopo]

Hi Linus,
    another reply to your email, please don't feel assaulted :)

On Wed, Mar 29, 2017 at 03:22:23PM +0200, Linus Walleij wrote:
> On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas@jmondi.org> wrote:
> 
> > Add dt-bindings for Renesas r7s72100 pin controller header file.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> > +/*
> > + * Pin is bi-directional.
> > + * An alternate function that needs both input/output functionalities shall
> > + * be configured as bidirectional.
> > + * Eg. SDA/SCL pins of an I2c interface.
> > + */
> > +#define BI_DIR                 (1 << 3)
> 
> Any specific reason why this should not simply be added to
> include/linux/pinctrl/pinconf-generic.h
> as PIN_CONFIG_BIDIRECTIONAL and parsed in
> drivers/pinctrl/pinconf-generic.c
> from the (new) DT property "bidirectional" simply?
> 

I can try to give you a few reasons why I don't see those flags fit in
the pin configuration flags definition.

*) those flags are used during pin multiplexing procedure only and that
procedure has a specific order to be respected:

You can have a look here:
https://www.spinics.net/lists/linux-renesas-soc/msg12793.html
In "rza1_alternate_function_conf()" function, we need to set bidir
before setting every other register.
The same applies some lines below:, the PIPC, PMC and PM register set order
has to be respected, and depends on those BIDIR and SWIO_* parameters.
This implies those configuration cannot be applied after pin muxing,
certainly not in pin_config[_group]_set() whose invocation time
is independent from pin_mux_set()'s one.

One way forward would be, every time we mux a pin, look for a pinconf group
that includes the pin we're muxing. That would happen for each pin,
for no added benefits imo.

*) as Geert already pointed out, we may need dedicated subnodes to
specify those pin configuration flags, not only because of what Chris
said, but because pinconf_generic_dt_subnode_to_map() wants "pins" or
"groups" to be there in the subnode, and in our pin multiplexing
sub-nodes we only have "pinmux" property (say: we cannot specify
pin_conf flags in the same sub-node where we describe pin
multiplexing, we always need a dedicated sub-node).
Chris and Geert gave some examples in their replies on how that would
like, and how it makes the bindings a little more complex.

*) those flags, according to Chris, won't be used in RZ/A2, and
reasonably not in any other RZ device. Do we want to add them to the
generic bindings, being them so specific to this hardware platform?

One thing I suggest considering is to get rid of those flags, at
least in bindings, and introduce 3 variants for each pin multiplexing
function identifier.

Say:
include/dt-bindings/pinctrl/r7s72100-pinctrl.h:
#define MUX_1               (1 << 16)
#define MUX_1_BIDIR         (1 << 16 | 1 << 24)
#define MUX_1_SWIO_IN       (1 << 16 | 2 << 24)
#define MUX_1_SWIO_OUT      (1 << 16 | 3 << 24)
...
#define MUX_8               (8 << 16)
#define MUX_8_BIDIR         (8 << 16 | 1 << 24)
....

this way we get rid of those extra flag values and squeeze pin id
and mux function id in a single integer, part of the "pinmux"
arguments list.

i2c_pins {
    pinmux = <(PIN(1, 6) | MUX_1_BIDIR)>,
             <(PIN(1, 7) | MUX_2_BIDIR)>;
};

The driver will parse the bits from [31:24] to find out if it needs
to enable some special multiplexing property, and performs
multiplexing as it is doing right now.

> > +/*
> > + * Flags used to ask software to drive the pin I/O direction overriding the
> > + * alternate function configuration.
> > + * Some alternate functions require software to force I/O direction of a pin,
> > + * overriding the designated one.
> > + * Refer to the HW manual to know when this flag shall be used.
> > + */
> > +#define SWIO_IN                        (1 << 4)
> > +#define SWIO_OUT               (1 << 5)
> 
> What is wrong in doing this with generic pin config using
> PIN_CONFIG_INPUT_ENABLE and PIN_CONFIG_OUTPUT
> (ignoring the argument)?
> 
> In the device tree use input-enable and add a new output-enable
> (with unspecified value) with proper description and DT bindings?
> 
> And if you think these have no general applicability, by the end
> of the day they are *still* pin config, not magic flags we can choose to
> toss in with the muxing, so you can do what the Qualcomm driver
> does and add custom pin configurations extending the generic
> pin config, see drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> qcom,pull-up-strength etc.
> 

I see, but that custom pin configuration flag can be applied
independently from pin muxing procedure and it can be applied to pins
while they're configured in GPIO mode.
Our "flags" are not of that nature, and only apply to some register
setting during pinmux, as I hopefully tried to clarify above.

Thanks for time and patience in this long email thread.
    j

> Yours,
> Linus Walleij

Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header

[Linus Walleij]

On Wed, Mar 29, 2017 at 5:56 PM, jacopo <jacopo@jmondi.org> wrote:

> I can try to give you a few reasons why I don't see those flags fit in
> the pin configuration flags definition.
>
> *) those flags are used during pin multiplexing procedure only and that
> procedure has a specific order to be respected:
>
> You can have a look here:
> https://www.spinics.net/lists/linux-renesas-soc/msg12793.html
> In "rza1_alternate_function_conf()" function, we need to set bidir
> before setting every other register.
> The same applies some lines below:, the PIPC, PMC and PM register set order
> has to be respected, and depends on those BIDIR and SWIO_* parameters.
> This implies those configuration cannot be applied after pin muxing,
> certainly not in pin_config[_group]_set() whose invocation time
> is independent from pin_mux_set()'s one.

But now you are mixing up syntax and semantics.

You are describing what steps are necessary on this hardware to
apply a certain setting. That is fine, if you didn't need any specific
semantics there, you could be using pinctrl-single which will just
hammer stuff into registers, one register per pin. You have a pin
controller driver exactly beacause your hardware has semantics.

How this is described in the device tree or a board file is a different
thing from how you write your driver.

I understand why i makes for easier code, but does it make for more
generic and understandable device trees for people maintaining
several systems? I don't think so.

> One way forward would be, every time we mux a pin, look for a pinconf group
> that includes the pin we're muxing. That would happen for each pin,
> for no added benefits imo.

The benefit is clearly abstraction, standardization and readability of the
device tree. I, as developer, understand what is going on electrically,
having seen this on other systems, and being able to access generic
documentation on generic pin config properties.

> *) as Geert already pointed out, we may need dedicated subnodes to
> specify those pin configuration flags, not only because of what Chris
> said, but because pinconf_generic_dt_subnode_to_map() wants "pins" or
> "groups" to be there in the subnode, and in our pin multiplexing
> sub-nodes we only have "pinmux" property (say: we cannot specify
> pin_conf flags in the same sub-node where we describe pin
> multiplexing, we always need a dedicated sub-node).
> Chris and Geert gave some examples in their replies on how that would
> like, and how it makes the bindings a little more complex.

Very little more complex, and actually it could be argued that this
is exactly why subnodes exist: to be able to have different pin config
on pins. I think it is very readable.

> *) those flags, according to Chris, won't be used in RZ/A2, and
> reasonably not in any other RZ device. Do we want to add them to the
> generic bindings, being them so specific to this hardware platform?

I have seen so much stuff that people say is "necessarily different"
for their platform. It turns our that silicon IO and solid state physics
isn't that much different between systems. The same things invariably
pop up in several chips.

Hell this was what people said about this whole subsystem from the
beginning: pin control is so necessarily different that there is no point
in trying to create an abstraction for it.

If I had listened to that kind of advice we wouldn't be where we are today.

And that said, I have already pointed out that two of them already
exist in the pin control subsystem (PIN_CONFIG*). Because other SoCs
are doing similar things.

> One thing I suggest considering is to get rid of those flags, at
> least in bindings, and introduce 3 variants for each pin multiplexing
> function identifier.
>
> Say:
> include/dt-bindings/pinctrl/r7s72100-pinctrl.h:
> #define MUX_1               (1 << 16)
> #define MUX_1_BIDIR         (1 << 16 | 1 << 24)
> #define MUX_1_SWIO_IN       (1 << 16 | 2 << 24)
> #define MUX_1_SWIO_OUT      (1 << 16 | 3 << 24)
> ...
> #define MUX_8               (8 << 16)
> #define MUX_8_BIDIR         (8 << 16 | 1 << 24)
> ....

I understand they can be made more beautiful, but in my view
that is putting make up on a pig, I want generic pin config for these
things.


>> What is wrong in doing this with generic pin config using
>> PIN_CONFIG_INPUT_ENABLE and PIN_CONFIG_OUTPUT
>> (ignoring the argument)?
>>
>> In the device tree use input-enable and add a new output-enable
>> (with unspecified value) with proper description and DT bindings?
>>
>> And if you think these have no general applicability, by the end
>> of the day they are *still* pin config, not magic flags we can choose to
>> toss in with the muxing, so you can do what the Qualcomm driver
>> does and add custom pin configurations extending the generic
>> pin config, see drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> qcom,pull-up-strength etc.
>>
>
> I see, but that custom pin configuration flag can be applied
> independently from pin muxing procedure and it can be applied to pins
> while they're configured in GPIO mode.

See "GPIO mode pitfalls" in Documentation/pinctrl.txt
I've been over this hardware lingo so many times already.

It is not about "GPIO" at all, it is about pin configuration. The
generic pin config was not invented for GPIO, it was just recently
that we started to provide pin config back-ends for GPIO. Only
Intel do that so far.

> Our "flags" are not of that nature, and only apply to some register
> setting during pinmux, as I hopefully tried to clarify above.

And that is a driver semantic. Or even a subsystem semantic. No
big deal, accumulate such writes in the driver and apply it all when you have
it all available no matter if pin multiplexing or pin config happens first?
Surely this is just a hardware pecularity, then it warrants some special
driver code.

If you definately feel you must get a call from the pin control core setting
up muxing and config at the same time we need to think of a way to
augment the pin control core if necessary?

The fact that Linux pin control subsystem semantics you don't
like does not affect the relevant device tree bindings.

Yours,
Linus Walleij

[PATCH v3 5/7] arm: dts: genmai: Add SCIF2 pin group

[Jacopo Mondi]

Add pin configuration subnode for SCIF2 serial debug interface.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r7s72100-genmai.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100-genmai.dts b/arch/arm/boot/dts/r7s72100-genmai.dts
index 118a8e2..569c86a 100644
--- a/arch/arm/boot/dts/r7s72100-genmai.dts
+++ b/arch/arm/boot/dts/r7s72100-genmai.dts
@@ -11,6 +11,7 @@
 
 /dts-v1/;
 #include "r7s72100.dtsi"
+#include <dt-bindings/pinctrl/r7s72100-pinctrl.h>
 
 / {
 	model = "Genmai";
@@ -36,6 +37,15 @@
 	};
 };
 
+&pinctrl {
+
+	scif2_pins: serial2 {
+		/* P3_0 as TxD2; P3_2 as RxD2 */
+		renesas,pins = <PIN(3, 0) 6>,
+			       <PIN(3, 2) 4>;
+	};
+};
+
 &extal_clk {
 	clock-frequency = <13330000>;
 };
@@ -60,6 +70,9 @@
 };
 
 &scif2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&scif2_pins>;
+
 	status = "okay";
 };
 
-- 
2.7.4

[PATCH v3 6/7] arm: dts: genmai: Add RIIC2 pin group

[Jacopo Mondi]

Add pin configuration subnode for RIIC2 interface.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r7s72100-genmai.dts | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100-genmai.dts b/arch/arm/boot/dts/r7s72100-genmai.dts
index 569c86a..30992b1 100644
--- a/arch/arm/boot/dts/r7s72100-genmai.dts
+++ b/arch/arm/boot/dts/r7s72100-genmai.dts
@@ -44,6 +44,12 @@
 		renesas,pins = <PIN(3, 0) 6>,
 			       <PIN(3, 2) 4>;
 	};
+
+	i2c2_pins: i2c2 {
+		/* RIIC2: P1_4 as SCL, P1_5 as SDA */
+		renesas,pins = <PIN(1, 4) (1 | BI_DIR)>,
+			       <PIN(1, 5) (1 | BI_DIR)>;
+	};
 };
 
 &extal_clk {
@@ -62,6 +68,9 @@
 	status = "okay";
 	clock-frequency = <400000>;
 
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pins>;
+
 	eeprom@50 {
 		compatible = "renesas,24c128";
 		reg = <0x50>;
-- 
2.7.4

[PATCH v3 7/7] arm: dts: genmai: Add user led device nodes

[Jacopo Mondi]

Add device nodes for user leds on Genmai board.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r7s72100-genmai.dts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100-genmai.dts b/arch/arm/boot/dts/r7s72100-genmai.dts
index 30992b1..94b9ab7 100644
--- a/arch/arm/boot/dts/r7s72100-genmai.dts
+++ b/arch/arm/boot/dts/r7s72100-genmai.dts
@@ -11,6 +11,7 @@
 
 /dts-v1/;
 #include "r7s72100.dtsi"
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/pinctrl/r7s72100-pinctrl.h>
 
 / {
@@ -35,6 +36,19 @@
 		#address-cells = <1>;
 		#size-cells = <1>;
 	};
+
+	leds {
+		status = "okay";
+		compatible = "gpio-leds";
+
+		led1 {
+			gpios = <&port4 10 GPIO_ACTIVE_LOW>;
+		};
+
+		led2 {
+			gpios = <&port4 11 GPIO_ACTIVE_LOW>;
+		};
+	};
 };
 
 &pinctrl {
-- 
2.7.4