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

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

 



Hi Jacopo,

On 18 September 2018 11:44 jacopo mondi wrote:
> Hi Phil,
>    thanks for the patch
Thanks for the review!


> On Mon, Sep 17, 2018 at 05:36:08PM +0100, Phil Edworthy wrote:
> > This provides a pinctrl driver for the Renesas RZ/N1 device family.
> >
> > Based on a patch originally written by Michel Pollet at Renesas.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> > ---
> > v3:
> >  - Use standard DT props instead of proprietary ones.
> >  - Replace virtual pins used for MDIO muxing with extra funcs.
> >  - Use pinctrl_utils funcs to handle the maps.
> >  - Remove the dbg functions to keep things simple.
> >
> > v2:
> >  - Change filename to generic rzn1, instead of device specific.
> >  - Changed Kconfig symbol and file name to generic rzn1 family.
> >  - Added "renesas,rzn1-pinctrl" compatible fallback string
> >  - Changes suggested by Jacopo Mondi. Mainly formatting, plus:
> >    - Removed global ptr
> >    - Removed unused code accessing parent of node.
> >    - Removed check for null OF np that can't happen.
> >    - Replaced overlapping enums with #defines
> >  - Renamed some variables and symbols to clarify their use
> >  - Fix error handling during probe
> >  - Move probe from postcore_initcall to subsys_initcall to ensure
> >    drivers that require clocks get them instead of having to defer
> >    probing.
> > ---
> >  drivers/pinctrl/Kconfig        |  10 +
> >  drivers/pinctrl/Makefile       |   1 +
> >  drivers/pinctrl/pinctrl-rzn1.c | 926
> > +++++++++++++++++++++++++++++++++
> >  3 files changed, 937 insertions(+)
> >  create mode 100644 drivers/pinctrl/pinctrl-rzn1.c
> >
> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig index
> > e86752be1f19..e524eb101384 100644
> > --- a/drivers/pinctrl/Kconfig
> > +++ b/drivers/pinctrl/Kconfig
> > @@ -195,6 +195,16 @@ config PINCTRL_RZA1
> >  	help
> >  	  This selects pinctrl driver for Renesas RZ/A1 platforms.
> >
> > +config PINCTRL_RZN1
> > +	bool "Renesas RZ/N1 pinctrl driver"
> > +	depends on OF
> > +	depends on ARCH_RZN1 || COMPILE_TEST
> > +	select GENERIC_PINCTRL_GROUPS
> > +	select GENERIC_PINMUX_FUNCTIONS
> > +	select GENERIC_PINCONF
> > +	help
> > +	  This selects pinctrl driver for Renesas RZ/N1 devices.
> > +
> >  config PINCTRL_SINGLE
> >  	tristate "One-register-per-pin type device tree based pinctrl driver"
> >  	depends on OF
> > diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile index
> > 46ef9bd52096..d07f9a20f6ae 100644
> > --- a/drivers/pinctrl/Makefile
> > +++ b/drivers/pinctrl/Makefile
> > @@ -27,6 +27,7 @@ obj-$(CONFIG_PINCTRL_PIC32)	+= pinctrl-pic32.o
> >  obj-$(CONFIG_PINCTRL_PISTACHIO)	+= pinctrl-pistachio.o
> >  obj-$(CONFIG_PINCTRL_ROCKCHIP)	+= pinctrl-rockchip.o
> >  obj-$(CONFIG_PINCTRL_RZA1)	+= pinctrl-rza1.o
> > +obj-$(CONFIG_PINCTRL_RZN1)	+= pinctrl-rzn1.o
> >  obj-$(CONFIG_PINCTRL_SINGLE)	+= pinctrl-single.o
> >  obj-$(CONFIG_PINCTRL_SIRF)	+= sirf/
> >  obj-$(CONFIG_PINCTRL_SX150X)	+= pinctrl-sx150x.o
> > diff --git a/drivers/pinctrl/pinctrl-rzn1.c
> > b/drivers/pinctrl/pinctrl-rzn1.c new file mode 100644 index
> > 000000000000..8f0caa266dbb
> > --- /dev/null
> > +++ b/drivers/pinctrl/pinctrl-rzn1.c
> > @@ -0,0 +1,926 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2014-2018 Renesas Electronics Europe Limited
> > + *
> > + * Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> > + * Based on a driver originally written by Michel Pollet at Renesas.
> > + */
> > +
> > +#include <dt-bindings/pinctrl/rzn1-pinctrl.h>
> > +#include <linux/clk.h>
> > +#include <linux/device.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/pinctrl/pinconf-generic.h> #include
> > +<linux/pinctrl/pinctrl.h> #include <linux/pinctrl/pinmux.h> #include
> > +<linux/platform_device.h> #include <linux/slab.h> #include "core.h"
> > +#include "pinconf.h"
> > +#include "pinctrl-utils.h"
> > +
> > +/* Field positions and masks in the pinmux registers */
> > +#define RZN1_L1_PIN_DRIVE_STRENGTH	10
> > +#define RZN1_L1_PIN_DRIVE_STRENGTH_4MA	0
> > +#define RZN1_L1_PIN_DRIVE_STRENGTH_6MA	1
> > +#define RZN1_L1_PIN_DRIVE_STRENGTH_8MA	2
> > +#define RZN1_L1_PIN_DRIVE_STRENGTH_12MA	3
> > +#define RZN1_L1_PIN_PULL		8
> > +#define RZN1_L1_PIN_PULL_NONE		0
> > +#define RZN1_L1_PIN_PULL_UP		1
> > +#define RZN1_L1_PIN_PULL_DOWN		3
> > +#define RZN1_L1_FUNCTION		0
> > +#define RZN1_L1_FUNC_MASK		0xf
> > +#define RZN1_L1_FUNCTION_L2		0xf
> > +
> > +/*
> > + * The hardware manual describes two levels of multiplexing, but it's
> > +more
> > + * logical to think of the hardware as three levels, with level 3
> > +consisting of
> > + * the multiplexing for Ethernet MDIO signals.
> > + *
> > + * Level 1 functions go from 0 to 9, with level 1 function '15' (0xf)
> > +specifying
> > + * that level 2 functions are used instead. Level 2 has a lot more
> > +options,
> > + * going from 0 to 61. Level 3 allows selection of MDIO functions
> > +which can be
> > + * floating, or one of seven internal peripherals. Unfortunately,
> > +there are two
> > + * level 2 functions that can select MDIO, and two MDIO channels so
> > +we have four
> > + * sets of level 3 functions.
> > + *
> > + * For this driver, we've compounded the numbers together, so:
> > + *    0 to   9 is level 1
> > + *   10 to  71 is 10 + level 2 number
> > + *   72 to  79 is 72 + MDIO0 source for level 2 MDIO function.
> > + *   80 to  87 is 80 + MDIO0 source for level 2 MDIO_E1 function.
> > + *   88 to  95 is 88 + MDIO1 source for level 2 MDIO function.
> > + *   96 to 103 is 96 + MDIO1 source for level 2 MDIO_E1 function.
> > + * Examples:
> > + *  Function 28 corresponds UART0
> > + *  Function 73 corresponds to MDIO0 to GMAC0
> > + *
> > + * There are 170 configurable pins (called PL_GPIO in the datasheet).
> > + */
> > +
> > +/*
> > + * Structure detailing the HW registers on the RZ/N1 devices.
> > + * Both the Level 1 mux registers and Level 2 mux registers have the
> > +same
> > + * structure. The only difference is that Level 2 has additional MDIO
> > +registers
> > + * at the end.
> > + */
> > +struct rzn1_pinctrl_regs {
> > +	union {
> > +		u32	conf[170];
> > +		u8	pad0[0x400];
> 
> Is pad0 actually used?
No, it's just to implement the padding. Would you prefer not using a union
here?

> > +	};
> > +	u32	status_protect;	/* 0x400 */
> > +	/* MDIO mux registers, level2 only */
> > +	u32	l2_mdio[2];
> > +};
> > +
> > +/**
> > + * struct rzn1_pmx_func - describes rzn1 pinmux functions
> > + * @name: the name of this specific function
> > + * @groups: corresponding pin groups
> > + * @num_groups: the number of groups
> > + */
> > +struct rzn1_pmx_func {
> > +	const char *name;
> > +	const char **groups;
> > +	unsigned int num_groups;
> > +};
> > +
> > +/**
> > + * struct rzn1_pin_group - describes an rzn1 pin group
> > + * @name: the name of this specific pin group
> > + * @func: the name of the function selected by this group
> > + * @npins: the number of pins in this group array, i.e. the number of
> > + *	elements in .pins so we can iterate over that array
> > + * @pin_ids: array of pin_ids, i.e. the value used to select the mux
> > + * @pins: array of pins. Needed due to pinctrl_ops.get_group_pins()
> > +*/ struct rzn1_pin_group {
> > +	const char *name;
> > +	const char *func;
> > +	unsigned int npins;
> > +	u8 *pin_ids;
> > +	u32 *pins;
> > +};
> > +
> > +struct rzn1_pinctrl {
> > +	struct device *dev;
> > +	struct clk *clk;
> > +	struct pinctrl_dev *pctl;
> > +	struct rzn1_pinctrl_regs __iomem *lev1;
> > +	struct rzn1_pinctrl_regs __iomem *lev2;
> > +	u32 lev1_protect_phys;
> > +	u32 lev2_protect_phys;
> > +	int mdio_func[2];
> > +
> > +	struct rzn1_pin_group *groups;
> > +	unsigned int ngroups;
> > +
> > +	struct rzn1_pmx_func *functions;
> > +	unsigned int nfunctions;
> > +};
> > +
> > +#define RZN1_PINS_PROP "pinmux"
> > +
> > +#define RZN1_PIN(pin) PINCTRL_PIN(pin, "pl_gpio"#pin)
> > +
> > +static const struct pinctrl_pin_desc rzn1_pins[] = {
> > +	RZN1_PIN(0), RZN1_PIN(1), RZN1_PIN(2), RZN1_PIN(3),
> RZN1_PIN(4),
> > +	RZN1_PIN(5), RZN1_PIN(6), RZN1_PIN(7), RZN1_PIN(8),
> RZN1_PIN(9),
> > +	RZN1_PIN(10), RZN1_PIN(11), RZN1_PIN(12), RZN1_PIN(13),
> RZN1_PIN(14),
> > +	RZN1_PIN(15), RZN1_PIN(16), RZN1_PIN(17), RZN1_PIN(18),
> RZN1_PIN(19),
> > +	RZN1_PIN(20), RZN1_PIN(21), RZN1_PIN(22), RZN1_PIN(23),
> RZN1_PIN(24),
> > +	RZN1_PIN(25), RZN1_PIN(26), RZN1_PIN(27), RZN1_PIN(28),
> RZN1_PIN(29),
> > +	RZN1_PIN(30), RZN1_PIN(31), RZN1_PIN(32), RZN1_PIN(33),
> RZN1_PIN(34),
> > +	RZN1_PIN(35), RZN1_PIN(36), RZN1_PIN(37), RZN1_PIN(38),
> RZN1_PIN(39),
> > +	RZN1_PIN(40), RZN1_PIN(41), RZN1_PIN(42), RZN1_PIN(43),
> RZN1_PIN(44),
> > +	RZN1_PIN(45), RZN1_PIN(46), RZN1_PIN(47), RZN1_PIN(48),
> RZN1_PIN(49),
> > +	RZN1_PIN(50), RZN1_PIN(51), RZN1_PIN(52), RZN1_PIN(53),
> RZN1_PIN(54),
> > +	RZN1_PIN(55), RZN1_PIN(56), RZN1_PIN(57), RZN1_PIN(58),
> RZN1_PIN(59),
> > +	RZN1_PIN(60), RZN1_PIN(61), RZN1_PIN(62), RZN1_PIN(63),
> RZN1_PIN(64),
> > +	RZN1_PIN(65), RZN1_PIN(66), RZN1_PIN(67), RZN1_PIN(68),
> RZN1_PIN(69),
> > +	RZN1_PIN(70), RZN1_PIN(71), RZN1_PIN(72), RZN1_PIN(73),
> RZN1_PIN(74),
> > +	RZN1_PIN(75), RZN1_PIN(76), RZN1_PIN(77), RZN1_PIN(78),
> RZN1_PIN(79),
> > +	RZN1_PIN(80), RZN1_PIN(81), RZN1_PIN(82), RZN1_PIN(83),
> RZN1_PIN(84),
> > +	RZN1_PIN(85), RZN1_PIN(86), RZN1_PIN(87), RZN1_PIN(88),
> RZN1_PIN(89),
> > +	RZN1_PIN(90), RZN1_PIN(91), RZN1_PIN(92), RZN1_PIN(93),
> RZN1_PIN(94),
> > +	RZN1_PIN(95), RZN1_PIN(96), RZN1_PIN(97), RZN1_PIN(98),
> RZN1_PIN(99),
> > +	RZN1_PIN(100), RZN1_PIN(101), RZN1_PIN(102), RZN1_PIN(103),
> > +	RZN1_PIN(104), RZN1_PIN(105), RZN1_PIN(106), RZN1_PIN(107),
> > +	RZN1_PIN(108), RZN1_PIN(109), RZN1_PIN(110), RZN1_PIN(111),
> > +	RZN1_PIN(112), RZN1_PIN(113), RZN1_PIN(114), RZN1_PIN(115),
> > +	RZN1_PIN(116), RZN1_PIN(117), RZN1_PIN(118), RZN1_PIN(119),
> > +	RZN1_PIN(120), RZN1_PIN(121), RZN1_PIN(122), RZN1_PIN(123),
> > +	RZN1_PIN(124), RZN1_PIN(125), RZN1_PIN(126), RZN1_PIN(127),
> > +	RZN1_PIN(128), RZN1_PIN(129), RZN1_PIN(130), RZN1_PIN(131),
> > +	RZN1_PIN(132), RZN1_PIN(133), RZN1_PIN(134), RZN1_PIN(135),
> > +	RZN1_PIN(136), RZN1_PIN(137), RZN1_PIN(138), RZN1_PIN(139),
> > +	RZN1_PIN(140), RZN1_PIN(141), RZN1_PIN(142), RZN1_PIN(143),
> > +	RZN1_PIN(144), RZN1_PIN(145), RZN1_PIN(146), RZN1_PIN(147),
> > +	RZN1_PIN(148), RZN1_PIN(149), RZN1_PIN(150), RZN1_PIN(151),
> > +	RZN1_PIN(152), RZN1_PIN(153), RZN1_PIN(154), RZN1_PIN(155),
> > +	RZN1_PIN(156), RZN1_PIN(157), RZN1_PIN(158), RZN1_PIN(159),
> > +	RZN1_PIN(160), RZN1_PIN(161), RZN1_PIN(162), RZN1_PIN(163),
> > +	RZN1_PIN(164), RZN1_PIN(165), RZN1_PIN(166), RZN1_PIN(167),
> > +	RZN1_PIN(168), RZN1_PIN(169),
> > +};
> > +
> > +enum {
> > +	LOCK_LEVEL1 = 0x1,
> > +	LOCK_LEVEL2 = 0x2,
> > +	LOCK_ALL = LOCK_LEVEL1 | LOCK_LEVEL2,
> 
> 0x03 is already (0x01 | 0x02) :)
> Not a big deal though.
Sure


> > +};
> > +
> > +static void rzn1_hw_set_lock(struct rzn1_pinctrl *ipctl, u8 lock, u8
> > +value) {
> > +	/*
> > +	 * The pinmux configuration is locked by writing the physical address
> of
> > +	 * the status_protect register to itself. It is unlocked by writing the
> > +	 * address | 1.
> > +	 */
> > +	if (lock & LOCK_LEVEL1) {
> > +		u32 val = ipctl->lev1_protect_phys | !(value & LOCK_LEVEL1);
> > +
> > +		writel(val, &ipctl->lev1->status_protect);
> > +	}
> > +
> > +	if (lock & LOCK_LEVEL2) {
> > +		u32 val = ipctl->lev2_protect_phys | !(value & LOCK_LEVEL2);
> > +
> > +		writel(val, &ipctl->lev2->status_protect);
> > +	}
> > +}
> > +
> > +static void rzn1_pinctrl_mdio_select(struct rzn1_pinctrl *ipctl, u8 mdio,
> > +				     u32 func)
> > +{
> > +	if (ipctl->mdio_func[mdio] >= 0 && ipctl->mdio_func[mdio] != func)
> > +		dev_warn(ipctl->dev, "conflicting setting for mdio%d!\n",
> mdio);
> > +	ipctl->mdio_func[mdio] = func;
> > +
> > +	dev_dbg(ipctl->dev, "setting mdio%d to %d\n", mdio, func);
> > +
> > +	writel(func, &ipctl->lev2->l2_mdio[mdio]); }
> > +
> > +/*
> > + * Using a composite pin description, set the hardware pinmux
> > +registers
> > + * with the corresponding values.
> > + * Make sure to unlock write protection and reset it afterward.
> > + *
> > + * NOTE: There is no protection for potential concurrency, it is
> > +assumed these
> > + * calls are serialized already.
> > + */
> > +static int rzn1_set_hw_pin_func(struct rzn1_pinctrl *ipctl, u32 pin,
> > +				u32 pin_config, u8 use_locks)
> > +{
> > +	u32 l1_cache;
> > +	u32 l2_cache;
> > +	u32 l1;
> > +	u32 l2;
> > +
> > +	/* Level 3 MDIO multiplexing */
> > +	if (pin_config >= RZN1_FUNC_MDIO0_HIGHZ &&
> > +	    pin_config <= RZN1_FUNC_MDIO1_E1_SWITCH) {
> > +		u8 mdio_channel;
> > +		u32 mdio_func;
> > +
> > +		if (pin_config <= RZN1_FUNC_MDIO1_HIGHZ)
> > +			mdio_channel = 0;
> > +		else
> > +			mdio_channel = 1;
> > +
> > +		/* Get MDIO func, and convert the func to the level 2
> number */
> > +		if (pin_config <= RZN1_FUNC_MDIO0_SWITCH) {
> > +			mdio_func = pin_config -
> RZN1_FUNC_MDIO0_HIGHZ;
> > +			pin_config = RZN1_FUNC_ETH_MDIO;
> > +		} else if (pin_config <= RZN1_FUNC_MDIO0_E1_SWITCH) {
> > +			mdio_func = pin_config -
> RZN1_FUNC_MDIO0_E1_HIGHZ;
> > +			pin_config = RZN1_FUNC_ETH_MDIO_E1;
> > +		} else if (pin_config <= RZN1_FUNC_MDIO1_SWITCH) {
> > +			mdio_func = pin_config -
> RZN1_FUNC_MDIO1_HIGHZ;
> > +			pin_config = RZN1_FUNC_ETH_MDIO;
> > +		} else {
> > +			mdio_func = pin_config -
> RZN1_FUNC_MDIO1_E1_HIGHZ;
> > +			pin_config = RZN1_FUNC_ETH_MDIO_E1;
> > +		}
> > +		rzn1_pinctrl_mdio_select(ipctl, mdio_channel, mdio_func);
> > +	}
> > +
> > +	/* Note here, we do not allow anything past the MDIO Mux values */
> > +	if (pin >= ARRAY_SIZE(ipctl->lev1->conf) ||
> > +	    pin_config >= RZN1_FUNC_MDIO0_HIGHZ)
> > +		return -EINVAL;
> > +
> > +	l1 = readl(&ipctl->lev1->conf[pin]);
> > +	l1_cache = l1;
> > +	l2 = readl(&ipctl->lev2->conf[pin]);
> > +	l2_cache = l2;
> > +
> > +	dev_dbg(ipctl->dev, "setting func for pin %d to %d\n", pin,
> > +pin_config);
> > +
> > +	if (pin_config < RZN1_FUNC_L2_OFFSET) {
> > +		l1 &= ~(RZN1_L1_FUNC_MASK << RZN1_L1_FUNCTION);
> > +		l1 |= (pin_config << RZN1_L1_FUNCTION);
> > +	} else {
> > +		l1 &= ~(RZN1_L1_FUNC_MASK << RZN1_L1_FUNCTION);
> > +		l1 |= (RZN1_L1_FUNCTION_L2 << RZN1_L1_FUNCTION);
> > +
> > +		l2 = pin_config - RZN1_FUNC_L2_OFFSET;
> > +	}
> > +
> > +	/* If either configuration changes, we update both anyway */
> > +	if (l1 != l1_cache || l2 != l2_cache) {
> > +		writel(l1, &ipctl->lev1->conf[pin]);
> > +		writel(l2, &ipctl->lev2->conf[pin]);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct rzn1_pin_group *rzn1_pinctrl_find_group_by_name(
> > +	const struct rzn1_pinctrl *ipctl, const char *name) {
> > +	const struct rzn1_pin_group *grp = NULL;
> > +	int i;
> > +
> > +	for (i = 0; i < ipctl->ngroups; i++) {
> > +		if (!strcmp(ipctl->groups[i].name, name)) {
> > +			grp = &ipctl->groups[i];
> > +			break;
> > +		}
> > +	}
> > +
> > +	return grp;
> > +}
> > +
> > +static int rzn1_get_groups_count(struct pinctrl_dev *pctldev) {
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +
> > +	return ipctl->ngroups;
> > +}
> > +
> > +static const char *rzn1_get_group_name(struct pinctrl_dev *pctldev,
> > +				       unsigned int selector)
> > +{
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +
> > +	return ipctl->groups[selector].name; }
> > +
> > +static int rzn1_get_group_pins(struct pinctrl_dev *pctldev,
> > +			       unsigned int selector, const unsigned int **pins,
> > +			       unsigned int *npins)
> > +{
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +
> > +	if (selector >= ipctl->ngroups)
> > +		return -EINVAL;
> > +
> > +	*pins = ipctl->groups[selector].pins;
> > +	*npins = ipctl->groups[selector].npins;
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * This function is called for each pinctl 'Function' node.
> > + * Sub-nodes can be used to describe multiple 'Groups' for the 'Function'
> > + * If there aren't any sub-nodes, the 'Group' is essentially the 'Function'.
> > + * Each 'Group' uses pinmux = <...> to detail the pins and data used
> > +to select
> > + * the functionality. Each 'Group' has optional pin configurations
> > +that apply
> > + * to all pins in the 'Group'.
> > + */
> > +static int rzn1_dt_node_to_map_one(struct pinctrl_dev *pctldev,
> > +				   struct device_node *np,
> > +				   struct pinctrl_map **map,
> > +				   unsigned int *num_maps)
> > +{
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +	const struct rzn1_pin_group *grp;
> > +	unsigned long *configs = NULL;
> > +	unsigned int num_configs = 0;
> > +	unsigned int reserved_maps = *num_maps;
> > +	unsigned int reserve = 1;
> > +	int ret;
> > +
> > +	dev_dbg(ipctl->dev, "processing node %pOF\n", np);
> > +
> > +	grp = rzn1_pinctrl_find_group_by_name(ipctl, np->name);
> > +	if (!grp) {
> > +		dev_err(ipctl->dev, "unable to find group for node %pOF\n",
> np);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Get the group's pin configuration */
> > +	ret = pinconf_generic_parse_dt_config(np, pctldev, &configs,
> > +					      &num_configs);
> > +	if (ret < 0) {
> > +		dev_err(ipctl->dev, "%s: could not parse node property\n",
> > +			of_node_full_name(np));
> 
> Why %pOF above and of_node_full_name? I would use the first, if nothing
> changed recently with OF naming.
You are right, %pOF should be used


> > +		return ret;
> > +	}
> > +
> > +	if (num_configs)
> > +		reserve++;
> > +
> > +	/* Increase the number of maps to cover this group */
> > +	ret = pinctrl_utils_reserve_map(pctldev, map, &reserved_maps,
> num_maps,
> > +					reserve);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	/* Associate the group with the function */
> > +	ret = pinctrl_utils_add_map_mux(pctldev, map, &reserved_maps,
> num_maps,
> > +					grp->name, grp->func);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	if (num_configs) {
> > +		/* Associate the group's pin configuration with the group */
> > +		ret = pinctrl_utils_add_map_configs(pctldev, map,
> > +				&reserved_maps, num_maps, grp->name,
> > +				configs, num_configs,
> > +				PIN_MAP_TYPE_CONFIGS_GROUP);
> > +		if (ret < 0)
> > +			goto out;
> > +	}
> > +
> > +	dev_dbg(pctldev->dev, "maps: function %s group %s (%d pins)\n",
> > +		grp->func, grp->name, grp->npins);
> > +
> > +out:
> > +	kfree(configs);
> > +	return 0;
> 
> You never return error here (nit: newline before returning, here and in other
> places)
Ok, will fix.


> > +}
> > +
> > +static int rzn1_dt_node_to_map(struct pinctrl_dev *pctldev,
> > +			       struct device_node *np,
> > +			       struct pinctrl_map **map,
> > +			       unsigned int *num_maps)
> > +{
> > +	struct device_node *child;
> > +	int ret;
> > +
> > +	*map = NULL;
> > +	*num_maps = 0;
> > +
> > +	ret = rzn1_dt_node_to_map_one(pctldev, np, map, num_maps);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	for_each_child_of_node(np, child) {
> > +		ret = rzn1_dt_node_to_map_one(pctldev, child, map,
> num_maps);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct pinctrl_ops rzn1_pctrl_ops = {
> > +	.get_groups_count = rzn1_get_groups_count,
> > +	.get_group_name = rzn1_get_group_name,
> > +	.get_group_pins = rzn1_get_group_pins,
> > +	.dt_node_to_map = rzn1_dt_node_to_map,
> > +	.dt_free_map = pinctrl_utils_free_map, };
> > +
> > +static int rzn1_pmx_get_funcs_count(struct pinctrl_dev *pctldev) {
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +
> > +	return ipctl->nfunctions;
> > +}
> > +
> > +static const char *rzn1_pmx_get_func_name(struct pinctrl_dev *pctldev,
> > +					  unsigned int selector)
> > +{
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +
> > +	return ipctl->functions[selector].name; }
> > +
> > +static int rzn1_pmx_get_groups(struct pinctrl_dev *pctldev,
> > +			       unsigned int selector,
> > +			       const char * const **groups,
> > +			       unsigned int * const num_groups) {
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +
> > +	*groups = ipctl->functions[selector].groups;
> > +	*num_groups = ipctl->functions[selector].num_groups;
> > +
> > +	return 0;
> > +}
> > +
> > +static int rzn1_set_mux(struct pinctrl_dev *pctldev, unsigned int selector,
> > +			unsigned int group)
> > +{
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +	struct rzn1_pin_group *grp = &ipctl->groups[group];
> > +	unsigned int i, grp_pins = grp->npins;
> > +
> > +	dev_dbg(ipctl->dev, "set mux %s(%d) group %s(%d)\n",
> > +		ipctl->functions[selector].name, selector, grp->name,
> group);
> > +
> > +	rzn1_hw_set_lock(ipctl, LOCK_ALL, LOCK_ALL);
> > +	for (i = 0; i < grp_pins; i++)
> > +		rzn1_set_hw_pin_func(ipctl, grp->pins[i], grp->pin_ids[i], 0);
> > +	rzn1_hw_set_lock(ipctl, LOCK_ALL, 0);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct pinmux_ops rzn1_pmx_ops = {
> > +	.get_functions_count = rzn1_pmx_get_funcs_count,
> > +	.get_function_name = rzn1_pmx_get_func_name,
> > +	.get_function_groups = rzn1_pmx_get_groups,
> > +	.set_mux = rzn1_set_mux,
> > +};
> > +
> > +static int rzn1_pinconf_get(struct pinctrl_dev *pctldev, unsigned int pin,
> > +			    unsigned long *config)
> > +{
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +	enum pin_config_param param =
> pinconf_to_config_param(*config);
> > +	const u32 reg_drive[4] = { 4, 6, 8, 12 };
> > +	u32 l1, l2;
> > +	u32 pull, drive, l1mux;
> > +	u32 arg = 0;
> > +
> > +	if (pin >= ARRAY_SIZE(ipctl->lev1->conf))
> > +		return -EINVAL;
> > +
> > +	l1 = readl(&ipctl->lev1->conf[pin]);
> > +
> > +	l1mux = l1 & RZN1_L1_FUNC_MASK;
> > +	pull = (l1 >> RZN1_L1_PIN_PULL) & 0x3;
> > +	drive = (l1 >> RZN1_L1_PIN_DRIVE_STRENGTH) & 0x3;
> > +
> > +	switch (param) {
> > +	case PIN_CONFIG_BIAS_PULL_UP:
> > +		if (pull == RZN1_L1_PIN_PULL_UP)
> > +			arg = 1;
> > +		break;
> > +	case PIN_CONFIG_BIAS_PULL_DOWN:
> > +		if (pull == RZN1_L1_PIN_PULL_DOWN)
> > +			arg = 1;
> > +		break;
> > +	case PIN_CONFIG_BIAS_DISABLE:
> > +		if (pull == RZN1_L1_PIN_PULL_NONE)
> > +			arg = 1;
> > +		break;
> > +	case PIN_CONFIG_DRIVE_STRENGTH:
> > +		arg = reg_drive[drive];
> > +		break;
> > +	case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
> > +		l2 = readl(&ipctl->lev1->conf[pin]);
> > +		if (l1mux == RZN1_FUNC_HIGHZ)
> > +			arg = 1;
> > +		if (l1mux == RZN1_L1_FUNCTION_L2 && l2 == 0)
> > +			arg = 1;
> > +		break;
> > +	default:
> > +		return -ENOTSUPP;
> > +	}
> > +
> > +	*config = pinconf_to_config_packed(param, arg);
> > +
> > +	return 0;
> > +}
> > +
> > +static int rzn1_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
> > +			    unsigned long *configs, unsigned int num_configs)
> {
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +	enum pin_config_param param;
> > +	int i;
> > +	u32 arg;
> > +	u32 l1, l1_cache;
> > +	u32 drv;
> > +
> > +	if (pin >= ARRAY_SIZE(ipctl->lev1->conf))
> > +		return -EINVAL;
> > +
> > +	l1 = readl(&ipctl->lev1->conf[pin]);
> > +	l1_cache = l1;
> > +
> > +	for (i = 0; i < num_configs; i++) {
> > +		param = pinconf_to_config_param(configs[i]);
> > +		arg = pinconf_to_config_argument(configs[i]);
> > +
> > +		switch (param) {
> > +		case PIN_CONFIG_BIAS_PULL_UP:
> > +			dev_dbg(ipctl->dev, "set pin %d pull up\n", pin);
> > +			l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
> > +			l1 |= (RZN1_L1_PIN_PULL_UP <<
> RZN1_L1_PIN_PULL);
> > +			break;
> > +		case PIN_CONFIG_BIAS_PULL_DOWN:
> > +			dev_dbg(ipctl->dev, "set pin %d pull down\n", pin);
> > +			l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
> > +			l1 |= (RZN1_L1_PIN_PULL_DOWN <<
> RZN1_L1_PIN_PULL);
> > +			break;
> > +		case PIN_CONFIG_BIAS_DISABLE:
> > +			dev_dbg(ipctl->dev, "set pin %d bias off\n", pin);
> > +			l1 &= ~(0x3 << RZN1_L1_PIN_PULL);
> > +			l1 |= (RZN1_L1_PIN_PULL_NONE <<
> RZN1_L1_PIN_PULL);
> > +			break;
> > +		case PIN_CONFIG_DRIVE_STRENGTH:
> > +			dev_dbg(ipctl->dev, "set pin %d drv %umA\n", pin,
> arg);
> > +			drv = 0;
> > +			switch (arg) {
> > +			case 4:
> > +				drv = RZN1_L1_PIN_DRIVE_STRENGTH_4MA;
> > +				break;
> > +			case 6:
> > +				drv = RZN1_L1_PIN_DRIVE_STRENGTH_6MA;
> > +				break;
> > +			case 8:
> > +				drv = RZN1_L1_PIN_DRIVE_STRENGTH_8MA;
> > +				break;
> > +			case 12:
> > +				drv =
> RZN1_L1_PIN_DRIVE_STRENGTH_12MA;
> > +				break;
> > +			default:
> > +				dev_err(ipctl->dev,
> > +					"Drive strength %umA not
> supported\n",
> > +					arg);
> > +				return -EINVAL;
> > +			}
> > +
> > +			l1 &= ~(0x3 << RZN1_L1_PIN_DRIVE_STRENGTH);
> > +			l1 |= (drv << RZN1_L1_PIN_DRIVE_STRENGTH);
> > +			break;
> > +
> > +		case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
> > +			dev_dbg(ipctl->dev, "set pin %d High-Z\n", pin);
> > +			l1 &= ~RZN1_L1_FUNC_MASK;
> > +			l1 |= RZN1_FUNC_HIGHZ;
> > +			break;
> > +		default:
> 
> I'll re-comment on bindings too, but the pinconf properties you support
> should be listed there
Ok, I'll fix that.


> > +			return -ENOTSUPP;
> > +		}
> > +	}
> > +
> > +	if (l1 != l1_cache) {
> > +		rzn1_hw_set_lock(ipctl, LOCK_LEVEL1, LOCK_LEVEL1);
> > +		writel(l1, &ipctl->lev1->conf[pin]);
> > +		rzn1_hw_set_lock(ipctl, LOCK_LEVEL1, 0);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int rzn1_pin_config_group_set(struct pinctrl_dev *pctldev,
> > +				     unsigned int selector,
> > +				     unsigned long *configs,
> > +				     unsigned int num_configs)
> > +{
> > +	struct rzn1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
> > +	struct rzn1_pin_group *grp = &ipctl->groups[selector];
> > +	int ret, i;
> > +
> > +	dev_dbg(ipctl->dev, "group set %s selector:%d configs:%p/%d\n",
> > +		grp->name, selector, configs, num_configs);
> > +
> > +	for (i = 0; i < grp->npins; i++) {
> > +		unsigned int pin = grp->pins[i];
> > +
> > +		ret = rzn1_pinconf_set(pctldev, pin, configs, num_configs);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct pinconf_ops rzn1_pinconf_ops = {
> > +	.pin_config_get = rzn1_pinconf_get,
> > +	.pin_config_set = rzn1_pinconf_set,
> > +	.pin_config_group_set = rzn1_pin_config_group_set,
> 
> Nit: you register a pinconf map with PIN_MAP_TYPE_CONFIGS_GROUP so
> the 'pinconf_[set|get]' should never be called. Have you tested if they do ?
> (just for my better understanding of the pinctrl framework :)
Ah, right!  pin_config_get is not called at all. pin_config_set is only called
from rzn1_pin_config_group_set().

So, I guess that means I should implement pin_config_group_get as well.


> > +};
> > +
> > +static struct pinctrl_desc rzn1_pinctrl_desc = {
> > +	.pctlops = &rzn1_pctrl_ops,
> > +	.pmxops = &rzn1_pmx_ops,
> > +	.confops = &rzn1_pinconf_ops,
> > +	.owner = THIS_MODULE,
> > +};
> > +
> > +static int rzn1_pinctrl_parse_groups(struct device_node *np,
> > +				     struct rzn1_pin_group *grp,
> > +				     struct rzn1_pinctrl *ipctl)
> > +{
> > +	int size;
> > +	const __be32 *list;
> > +	int i;
> 
> unsigned int i;
> 
> It is only used in the bottom for loop afaict
Yes indeed, I'll fix.


> > +
> > +	dev_dbg(ipctl->dev, "%s: %s\n", __func__, np->name);
> > +
> > +	/* Initialise group */
> > +	grp->name = np->name;
> > +
> > +	/*
> > +	 * The binding format is
> > +	 *	pinmux = <PIN_FUNC_ID CONFIG ...>,
> > +	 * do sanity check and calculate pins number
> > +	 */
> > +	list = of_get_property(np, RZN1_PINS_PROP, &size);
> > +	if (!list) {
> > +		dev_err(ipctl->dev,
> > +			"no " RZN1_PINS_PROP " property in node %s\n",
> > +			np->full_name);
> 
> Isn't it better to print out the OF node name with %pOF? Here and below...
Sure


> > +		return -EINVAL;
> > +	}
> > +
> > +	/* We do not check return since it's safe node passed down */
> > +	if (!size) {
> > +		dev_err(ipctl->dev, "Invalid " RZN1_PINS_PROP " in node
> %s\n",
> > +			np->full_name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	grp->npins = size / sizeof(list[0]);
> > +	if (!grp->npins)
> > +		return 0;
> 
> If you get here the property is there and has > 0 length
True


> > +
> > +	grp->pin_ids = devm_kmalloc_array(ipctl->dev,
> > +					  grp->npins, sizeof(grp->pin_ids[0]),
> > +					  GFP_KERNEL);
> > +	grp->pins = devm_kmalloc_array(ipctl->dev,
> > +				       grp->npins, sizeof(grp->pins[0]),
> > +				       GFP_KERNEL);
> > +	if (!grp->pin_ids || !grp->pins)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < grp->npins; i++) {
> > +		u32 pin_id = be32_to_cpu(*list++);
> > +
> > +		grp->pins[i] = pin_id & 0xff;
> > +		grp->pin_ids[i] = (pin_id >> 8) & 0x7f;
> > +	}
> > +
> > +	return grp->npins;
> > +}
> > +
> > +static int rzn1_pinctrl_count_function_groups(struct device_node *np)
> > +{
> > +	struct device_node *child;
> > +	int count = 0;
> > +
> > +	if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0)
> > +		count++;
> > +
> > +	for_each_child_of_node(np, child) {
> > +		if (of_property_count_u32_elems(child, RZN1_PINS_PROP)
> > 0)
> > +			count++;
> > +	}
> > +
> > +	return count;
> > +}
> > +
> > +static int rzn1_pinctrl_parse_functions(struct device_node *np,
> > +					struct rzn1_pinctrl *ipctl, u32 index) {
> > +	struct device_node *child;
> > +	struct rzn1_pmx_func *func;
> > +	struct rzn1_pin_group *grp;
> > +	u32 i = 0;
> > +	int ret;
> > +
> > +	dev_dbg(ipctl->dev, "parse function(%d): %s\n", index, np->name);
> > +
> > +	func = &ipctl->functions[index];
> > +
> > +	/* Initialise function */
> > +	func->name = np->name;
> > +	func->num_groups = rzn1_pinctrl_count_function_groups(np);
> > +	dev_dbg(ipctl->dev, "function %s has %d groups\n",
> > +		np->name, func->num_groups);
> 
> Maybe move this debug printout below the error check, it helps to know
> parsing went fine.
Makes sense


> > +	if (func->num_groups == 0) {
> > +		dev_err(ipctl->dev, "no groups defined in %s\n", np-
> >full_name);
> > +		return -EINVAL;
> > +	}
> > +
> > +	func->groups = devm_kmalloc_array(ipctl->dev,
> > +					  func->num_groups, sizeof(char *),
> > +					  GFP_KERNEL);
> > +	if (!func->groups)
> > +		return -ENOMEM;
> > +
> > +	if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0) {
> > +		func->groups[i] = np->name;
> > +		grp = &ipctl->groups[ipctl->ngroups];
> > +		grp->func = func->name;
> > +		ret = rzn1_pinctrl_parse_groups(np, grp, ipctl);
> > +		if (ret < 0)
> > +			return ret;
> > +		if (ret > 0) {
> 
> You never return grp->npins == 0; it's either error or > 0. I think you can drop
> this check.
> 
> > +			i++;
> > +			ipctl->ngroups++;
> > +		}
> 
> This design allows a sub-node with the following layout
> 
>         n-1 {
>                 pinmux = <..>;
> 
>                 sub-n-1 {
>                         pinmux = <..>;
>                 };
> 
>                 sub-n-2 {
>                         pinmux = <..>;
>                 };
>         };
> 
> The bindings documentation only allows either this
> 
>         n-1 {
>                 pinmux = <..>;
>         };
> 
> Or this
>         n-1 {
>                 sub-n-1 {
>                         pinmux = <..>;
>                 };
> 
>                 sub-n-2 {
>                         pinmux = <..>;
>                 };
>         };
> 
> So I guess it's one or the other here. What do you think?
Indeed, the bindings need updating to cover the extra case. I think
this extra case is quite neat.


> > +	}
> > +
> > +	for_each_child_of_node(np, child) {
> > +		func->groups[i] = child->name;
> > +		grp = &ipctl->groups[ipctl->ngroups];
> > +		grp->func = func->name;
> > +		ret = rzn1_pinctrl_parse_groups(child, grp, ipctl);
> > +		if (ret < 0)
> > +			return ret;
> > +		if (ret > 0) {
> > +			i++;
> > +			ipctl->ngroups++;
> > +		}
> > +	}
> > +
> > +	dev_dbg(ipctl->dev, "function %s parsed %d/%d groups\n",
> > +		np->name, i, func->num_groups);
> > +
> > +	return 0;
> > +}
> > +
> > +static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
> > +				 struct rzn1_pinctrl *ipctl)
> > +{
> > +	struct device_node *np = pdev->dev.of_node;
> > +	struct device_node *child;
> > +	unsigned int maxgroups = 0;
> > +	u32 nfuncs = 0;
> > +	u32 i = 0;
> > +	int ret;
> > +
> > +	nfuncs = of_get_child_count(np);
> > +	if (nfuncs <= 0) {
> > +		dev_err(&pdev->dev, "no functions defined\n");
> > +		return -EINVAL;
> > +	}
> 
> Do you really want to fail if no pinmuxing sub-node is defined? I can think of
> boards which includes the SoC .dtsi but do not define (yet) any pinmux/conf
> sub-nodes. This shouldn't be an error.
Makes sense.


> > +
> > +	ipctl->nfunctions = nfuncs;
> > +	ipctl->functions = devm_kmalloc_array(&pdev->dev, nfuncs,
> > +					      sizeof(*ipctl->functions),
> > +					      GFP_KERNEL);
> > +	if (!ipctl->functions)
> > +		return -ENOMEM;
> > +
> > +	ipctl->ngroups = 0;
> > +	for_each_child_of_node(np, child)
> > +		maxgroups += rzn1_pinctrl_count_function_groups(child);
> > +
> > +	ipctl->groups = devm_kmalloc_array(&pdev->dev,
> > +					   maxgroups,
> > +					   sizeof(*ipctl->groups),
> > +					   GFP_KERNEL);
> > +	if (!ipctl->groups)
> > +		return -ENOMEM;
> > +
> > +	for_each_child_of_node(np, child) {
> > +		ret = rzn1_pinctrl_parse_functions(child, ipctl, i++);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int rzn1_pinctrl_probe(struct platform_device *pdev) {
> > +	struct rzn1_pinctrl *ipctl;
> > +	struct resource *res;
> > +	int ret;
> > +
> > +	/* Create state holders etc for this driver */
> > +	ipctl = devm_kzalloc(&pdev->dev, sizeof(*ipctl), GFP_KERNEL);
> > +	if (!ipctl)
> > +		return -ENOMEM;
> > +
> > +	ipctl->mdio_func[0] = -1;
> > +	ipctl->mdio_func[1] = -1;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	ipctl->lev1_protect_phys = (u32)res->start + 0x400;
> > +	ipctl->lev1 = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(ipctl->lev1))
> > +		return PTR_ERR(ipctl->lev1);
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > +	ipctl->lev2_protect_phys = (u32)res->start + 0x400;
> > +	ipctl->lev2 = devm_ioremap_resource(&pdev->dev, res);
> > +	if (IS_ERR(ipctl->lev2))
> > +		return PTR_ERR(ipctl->lev2);
> > +
> > +	ipctl->clk = devm_clk_get(&pdev->dev, "bus");
> > +	if (IS_ERR(ipctl->clk))
> > +		return PTR_ERR(ipctl->clk);
> > +	ret = clk_prepare_enable(ipctl->clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ipctl->dev = &pdev->dev;
> > +	rzn1_pinctrl_desc.name = dev_name(&pdev->dev);
> > +	rzn1_pinctrl_desc.pins = rzn1_pins;
> > +	rzn1_pinctrl_desc.npins = ARRAY_SIZE(rzn1_pins);
> > +
> > +	ret = rzn1_pinctrl_probe_dt(pdev, ipctl);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "fail to probe dt properties\n");
> > +		goto err_clk;
> > +	}
> > +
> > +	platform_set_drvdata(pdev, ipctl);
> > +	ipctl->pctl = pinctrl_register(&rzn1_pinctrl_desc, &pdev->dev, ipctl);
> > +	if (!ipctl->pctl) {
> > +		dev_err(&pdev->dev, "could not register rzn1 pinctrl
> driver\n");
> > +		ret = -EINVAL;
> > +		goto err_clk;
> > +	}
> 
> Bindings claim this is a pinctrl/gpio controller, but I don't see any gpio support
> here. I'll comment on the bindings for reference too.
Indeed, I'll remove from the binding.

Thanks again for the review!
Phil


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




[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux