[RFC] pinctrl: imx: use radix trees for groups and functions

[RFC] pinctrl: imx: use radix trees for groups and functions

[Gary Bisson]

This change is inspired from the pinctrl-single architecture.

The problem with current implementation is that it isn't possible
to add/remove functions and/or groups dynamically. The radix tree
offers an easy way to do so. The intent is to offer a follow-up
patch later that will enable the use of pinctrl nodes in dt-overlays.

Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
---
Hi all,

This patch is marked as RFC since I'm not sure if it is the right approach.
The story behind it is that I've been experiencing with dt-overlays. While
pretty much everything went smoothly using the configfs patches from Pantelis,
the imx pinctrl doesn't allow to add/remove groups/functions.

To be honest I don't see much value in dt-overlays if a new pinmux group can't
be added. So I've looked at other implementation, pinctrl-single was the
primary source of inspiration.

Also, the idea behind this patch is to make it as small as possible, not
breaking the current implementation and keeping the same legacy with both
flat_funcs and old device trees supported.

Anyway, let me know your thoughts. If that's an acceptable approach, I'll
offer it as a patch plus a follow up that adds an of_notifier most likely.

Note that Pantelis suggested to do lazy allocation instead of the current
implementation but it requires a much larger change in the driver.

Regards,
Gary
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 171 +++++++++++++++++++++++++++-----
 drivers/pinctrl/freescale/pinctrl-imx.h |   5 +-
 2 files changed, 148 insertions(+), 28 deletions(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 5ef7e87..460d268 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -53,10 +53,9 @@ static inline const struct imx_pin_group *imx_pinctrl_find_group_by_name(
 	int i;
 
 	for (i = 0; i < info->ngroups; i++) {
-		if (!strcmp(info->groups[i].name, name)) {
-			grp = &info->groups[i];
+		grp = radix_tree_lookup(info->pgtree, i);
+		if (grp && !strcmp(grp->name, name))
 			break;
-		}
 	}
 
 	return grp;
@@ -75,8 +74,13 @@ static const char *imx_get_group_name(struct pinctrl_dev *pctldev,
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	const struct imx_pin_group *grp = NULL;
 
-	return info->groups[selector].name;
+	grp = radix_tree_lookup(info->pgtree, selector);
+	if (!grp)
+		return NULL;
+
+	return grp->name;
 }
 
 static int imx_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
@@ -85,12 +89,17 @@ static int imx_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	const struct imx_pin_group *grp = NULL;
 
 	if (selector >= info->ngroups)
 		return -EINVAL;
 
-	*pins = info->groups[selector].pin_ids;
-	*npins = info->groups[selector].npins;
+	grp = radix_tree_lookup(info->pgtree, selector);
+	if (!grp)
+		return -EINVAL;
+
+	*pins = grp->pin_ids;
+	*npins = grp->npins;
 
 	return 0;
 }
@@ -190,17 +199,25 @@ static int imx_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
 	const struct imx_pin_reg *pin_reg;
 	unsigned int npins, pin_id;
 	int i;
-	struct imx_pin_group *grp;
+	struct imx_pin_group *grp = NULL;
+	struct imx_pmx_func *func = NULL;
 
 	/*
 	 * Configure the mux mode for each pin in the group for a specific
 	 * function.
 	 */
-	grp = &info->groups[group];
+	grp = radix_tree_lookup(info->pgtree, group);
+	if (!grp)
+		return -EINVAL;
+
+	func = radix_tree_lookup(info->ftree, selector);
+	if (!func)
+		return -EINVAL;
+
 	npins = grp->npins;
 
 	dev_dbg(ipctl->dev, "enable function %s group %s\n",
-		info->functions[selector].name, grp->name);
+		func->name, grp->name);
 
 	for (i = 0; i < npins; i++) {
 		struct imx_pin *pin = &grp->pins[i];
@@ -285,8 +302,13 @@ static const char *imx_pmx_get_func_name(struct pinctrl_dev *pctldev,
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pmx_func *func = NULL;
+
+	func = radix_tree_lookup(info->ftree, selector);
+	if (!func)
+		return NULL;
 
-	return info->functions[selector].name;
+	return func->name;
 }
 
 static int imx_pmx_get_groups(struct pinctrl_dev *pctldev, unsigned selector,
@@ -295,9 +317,14 @@ static int imx_pmx_get_groups(struct pinctrl_dev *pctldev, unsigned selector,
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pmx_func *func = NULL;
 
-	*groups = info->functions[selector].groups;
-	*num_groups = info->functions[selector].num_groups;
+	func = radix_tree_lookup(info->ftree, selector);
+	if (!func)
+		return -EINVAL;
+
+	*groups = func->groups;
+	*num_groups = func->num_groups;
 
 	return 0;
 }
@@ -323,7 +350,9 @@ static int imx_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 	/* Find the pinctrl config with GPIO mux mode for the requested pin */
 	for (group = 0; group < info->ngroups; group++) {
-		grp = &info->groups[group];
+		grp = radix_tree_lookup(info->pgtree, group);
+		if (!grp)
+			continue;
 		for (pin = 0; pin < grp->npins; pin++) {
 			imx_pin = &grp->pins[pin];
 			if (imx_pin->pin == offset && !imx_pin->mux_mode)
@@ -494,7 +523,10 @@ static void imx_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
 		return;
 
 	seq_printf(s, "\n");
-	grp = &info->groups[group];
+	grp = radix_tree_lookup(info->pgtree, group);
+	if (!grp)
+		return;
+
 	for (i = 0; i < grp->npins; i++) {
 		struct imx_pin *pin = &grp->pins[i];
 		name = pin_get_name(pctldev, pin->pin);
@@ -614,7 +646,9 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
 
 	dev_dbg(info->dev, "parse function(%d): %s\n", index, np->name);
 
-	func = &info->functions[index];
+	func = radix_tree_lookup(info->ftree, index);
+	if (!func)
+		return -EINVAL;
 
 	/* Initialise function */
 	func->name = np->name;
@@ -628,7 +662,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
 
 	for_each_child_of_node(np, child) {
 		func->groups[i] = child->name;
-		grp = &info->groups[info->group_index++];
+
+		grp = devm_kzalloc(info->dev, sizeof(struct imx_pin_group),
+				   GFP_KERNEL);
+		if (!grp)
+			return -ENOMEM;
+
+		mutex_lock(&info->mutex);
+		radix_tree_insert(info->pgtree, info->group_index++, grp);
+		mutex_unlock(&info->mutex);
+
 		imx_pinctrl_parse_groups(child, grp, info, i++);
 	}
 
@@ -681,11 +724,19 @@ static int imx_pinctrl_probe_dt(struct platform_device *pdev,
 		}
 	}
 
-	info->nfunctions = nfuncs;
-	info->functions = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pmx_func),
+	for (i = 0; i < nfuncs; i++) {
+		struct imx_pmx_func *function;
+
+		function = devm_kzalloc(&pdev->dev, sizeof(*function),
 					GFP_KERNEL);
-	if (!info->functions)
-		return -ENOMEM;
+		if (!function)
+			return -ENOMEM;
+
+		mutex_lock(&info->mutex);
+		radix_tree_insert(info->ftree, i, function);
+		mutex_unlock(&info->mutex);
+	}
+	info->nfunctions = nfuncs;
 
 	info->group_index = 0;
 	if (flat_funcs) {
@@ -695,14 +746,11 @@ static int imx_pinctrl_probe_dt(struct platform_device *pdev,
 		for_each_child_of_node(np, child)
 			info->ngroups += of_get_child_count(child);
 	}
-	info->groups = devm_kzalloc(&pdev->dev, info->ngroups * sizeof(struct imx_pin_group),
-					GFP_KERNEL);
-	if (!info->groups)
-		return -ENOMEM;
 
 	if (flat_funcs) {
 		imx_pinctrl_parse_functions(np, info, 0);
 	} else {
+		i = 0;
 		for_each_child_of_node(np, child)
 			imx_pinctrl_parse_functions(child, info, i++);
 	}
@@ -710,6 +758,59 @@ static int imx_pinctrl_probe_dt(struct platform_device *pdev,
 	return 0;
 }
 
+/*
+ * imx_free_funcs() - free memory used by functions
+ * @info: info driver instance
+ */
+static void imx_free_funcs(const struct imx_pinctrl_soc_info *info)
+{
+	int i;
+
+	mutex_lock((struct mutex*)&info->mutex);
+	for (i = 0; i < info->nfunctions; i++) {
+		struct imx_pmx_func *func;
+
+		func = radix_tree_lookup(info->ftree, i);
+		if (!func)
+			continue;
+		radix_tree_delete(info->ftree, i);
+	}
+	mutex_unlock((struct mutex*)&info->mutex);
+}
+
+/*
+ * imx_free_pingroups() - free memory used by pingroups
+ * @info: info driver instance
+ */
+static void imx_free_pingroups(const struct imx_pinctrl_soc_info *info)
+{
+	int i;
+
+	mutex_lock((struct mutex*)&info->mutex);
+	for (i = 0; i < info->ngroups; i++) {
+		struct imx_pin_group *pingroup;
+
+		pingroup = radix_tree_lookup(info->pgtree, i);
+		if (!pingroup)
+			continue;
+		radix_tree_delete(info->pgtree, i);
+	}
+	mutex_unlock((struct mutex*)&info->mutex);
+}
+
+/*
+ * imx_free_resources() - free memory used by this driver
+ * @info: info driver instance
+ */
+static void imx_free_resources(const struct imx_pinctrl *ipctl)
+{
+	if (ipctl->pctl)
+		pinctrl_unregister(ipctl->pctl);
+
+	imx_free_funcs(ipctl->info);
+	imx_free_pingroups(ipctl->info);
+}
+
 int imx_pinctrl_probe(struct platform_device *pdev,
 		      struct imx_pinctrl_soc_info *info)
 {
@@ -783,10 +884,22 @@ int imx_pinctrl_probe(struct platform_device *pdev,
 	imx_pinctrl_desc->confops = &imx_pinconf_ops;
 	imx_pinctrl_desc->owner = THIS_MODULE;
 
+	mutex_init(&info->mutex);
+
+	info->ftree = devm_kzalloc(&pdev->dev, sizeof(struct radix_tree_root),
+				   GFP_KERNEL);
+	info->pgtree = devm_kzalloc(&pdev->dev, sizeof(struct radix_tree_root),
+				    GFP_KERNEL);
+	if (!info->ftree || !info->pgtree)
+		return -ENOMEM;
+
+	INIT_RADIX_TREE(info->pgtree, GFP_KERNEL);
+	INIT_RADIX_TREE(info->ftree, GFP_KERNEL);
+
 	ret = imx_pinctrl_probe_dt(pdev, info);
 	if (ret) {
 		dev_err(&pdev->dev, "fail to probe dt properties\n");
-		return ret;
+		goto free;
 	}
 
 	ipctl->info = info;
@@ -796,10 +909,16 @@ int imx_pinctrl_probe(struct platform_device *pdev,
 					    imx_pinctrl_desc, ipctl);
 	if (IS_ERR(ipctl->pctl)) {
 		dev_err(&pdev->dev, "could not register IMX pinctrl driver\n");
-		return PTR_ERR(ipctl->pctl);
+		ret = PTR_ERR(ipctl->pctl);
+		goto free;
 	}
 
 	dev_info(&pdev->dev, "initialized IMX pinctrl driver\n");
 
 	return 0;
+
+free:
+	imx_free_resources(ipctl);
+
+	return ret;
 }
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
index 8af8aa2..34ad2d7 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.h
+++ b/drivers/pinctrl/freescale/pinctrl-imx.h
@@ -76,13 +76,14 @@ struct imx_pinctrl_soc_info {
 	const struct pinctrl_pin_desc *pins;
 	unsigned int npins;
 	struct imx_pin_reg *pin_regs;
-	struct imx_pin_group *groups;
 	unsigned int ngroups;
 	unsigned int group_index;
-	struct imx_pmx_func *functions;
 	unsigned int nfunctions;
 	unsigned int flags;
 	const char *gpr_compatible;
+	struct radix_tree_root *ftree;
+	struct radix_tree_root *pgtree;
+	struct mutex mutex;
 };
 
 #define SHARE_MUX_CONF_REG	0x1
-- 
2.9.3

Re: [RFC] pinctrl: imx: use radix trees for groups and functions

[Linus Walleij]

On Fri, Nov 25, 2016 at 12:38 AM, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:

> This change is inspired from the pinctrl-single architecture.
>
> The problem with current implementation is that it isn't possible
> to add/remove functions and/or groups dynamically. The radix tree
> offers an easy way to do so. The intent is to offer a follow-up
> patch later that will enable the use of pinctrl nodes in dt-overlays.
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>

I would like to see some feedback from one of the driver maintainers?
Else just resend it without RFC I guess.

Yours,
Linus Walleij

Re: [RFC] pinctrl: imx: use radix trees for groups and functions

[Fabio Estevam]

Hi Gary,

On Thu, Nov 24, 2016 at 9:38 PM, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:
> This change is inspired from the pinctrl-single architecture.
>
> The problem with current implementation is that it isn't possible
> to add/remove functions and/or groups dynamically. The radix tree
> offers an easy way to do so. The intent is to offer a follow-up
> patch later that will enable the use of pinctrl nodes in dt-overlays.

Maybe you could re-post it including the follow-up patch that allows
the usage  of pinctrl nodes in dt-overlays?

This way we can have a better picture of the final result.

Thanks

Re: [RFC] pinctrl: imx: use radix trees for groups and functions

[Gary Bisson]

Hi Fabio, All,

On Wed, Nov 30, 2016 at 11:19:53AM -0200, Fabio Estevam wrote:
> Hi Gary,
> 
> On Thu, Nov 24, 2016 at 9:38 PM, Gary Bisson
> <gary.bisson@boundarydevices.com> wrote:
> > This change is inspired from the pinctrl-single architecture.
> >
> > The problem with current implementation is that it isn't possible
> > to add/remove functions and/or groups dynamically. The radix tree
> > offers an easy way to do so. The intent is to offer a follow-up
> > patch later that will enable the use of pinctrl nodes in dt-overlays.
> 
> Maybe you could re-post it including the follow-up patch that allows
> the usage  of pinctrl nodes in dt-overlays?
> 
> This way we can have a better picture of the final result.

Sure. But before doing so I'd like to share my findings after adding the
of_notifier:

1- Needs to remove the const qualifier of info, otherwise each member is
read-only and can't be updated. Hope everybody is ok with that, but I
don't see any other option to be able to increase the number of
functions/groups.

2- The way to get groups from a function (get_function_groups) makes it
hard to modify the number of groups of a specific function. Indeed it
would require re-alloc'ing the char**. Instead, it'd be best to make a
function for each group like it is done in pinctrl-single.
In this case, the function name would be the same as its single group
name. Then it is easy to add functions.

This approach looks good, IMO, when a device tree is using the "new
iomux scheme" since there's no real function. For the "legacy" scheme it
can be confusing since there are functions in the device tree (each
iomux sub-node). Is the legacy iomux scheme here to stay or is there a
plan to modify every device tree to comply to the new layout?

The other solution I came up with was that each overlay must have its
own function and cannot expand an existing function. This is an
unnecessary limitation in my opinion but I wanted to discuss it here
before submitting any of the two solutions.

My testing was done adding a dt-overlay pinctrl node, currently I'm not
sure how to cleanly remove a pinctrl function/group. It is not supported
in any of the pinctrl for now, but maybe it'd be a good time to think of
it.

Let me know if you have any question.

Regards,
Gary

[RFC v2 0/3] pinctrl: imx: add support for dt-overlays

[Gary Bisson]

Hi all,

This RFC series is a follow-up to my previous RFC patch [1].

This time the series includes one possible of_notifier implementation
for people to understand the need of radix_tree (or similar mechanism)
as suggested by Fabio.

However, if you wish to try this on, you'll need a tree with patches
from the renesas-drivers tree to have configfs support [2].

To ease that process, my testing tree was pushed to github [3] which
is based on today's linux-next tree.

Finally, the device tree overlay was also pushed to github [4]. This
last repo also contains what is needed to build the overlay out of
tree plus some helper script to load the overlay.

With this tree and overlay, two spidev devices can be added from
user-space with the proper pinctrl being applied.

The patches of this series include some comment on their purpose /
implementation.

Regards,
Gary

[1] http://www.spinics.net/lists/kernel/msg2391326.html
[2] http://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/overlays
[3] https://github.com/gibsson/linux-next/tree/for-next-20161202
[4] https://github.com/gibsson/dt-overlays/tree/dt-overlays-next/overlays

Gary Bisson (3):
  pinctrl: imx: remove const qualifier of imx_pinctrl_soc_info
  pinctrl: imx: use radix trees for groups and functions
  pinctrl: imx: add of_notifier for dynamic pinctrl changes

 drivers/pinctrl/freescale/pinctrl-imx.c | 301 +++++++++++++++++++++++++++-----
 drivers/pinctrl/freescale/pinctrl-imx.h |   5 +-
 2 files changed, 261 insertions(+), 45 deletions(-)

-- 
2.9.3

[RFC v2 1/3] pinctrl: imx: remove const qualifier of imx_pinctrl_soc_info

[Gary Bisson]

Otherwise can't dynamically update fields such as ngroups which can
change over time (with a dt-overlay for instance).

Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
---
Hi all,

If this patch isn't applied the compiler gives errors like:
error: increment of member ‘nfunctions’ in read-only object

Regards,
Gary
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 34 ++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 5ef7e87..8697c1b 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -42,11 +42,11 @@ struct imx_pinctrl {
 	struct pinctrl_dev *pctl;
 	void __iomem *base;
 	void __iomem *input_sel_base;
-	const struct imx_pinctrl_soc_info *info;
+	struct imx_pinctrl_soc_info *info;
 };
 
 static inline const struct imx_pin_group *imx_pinctrl_find_group_by_name(
-				const struct imx_pinctrl_soc_info *info,
+				struct imx_pinctrl_soc_info *info,
 				const char *name)
 {
 	const struct imx_pin_group *grp = NULL;
@@ -65,7 +65,7 @@ static inline const struct imx_pin_group *imx_pinctrl_find_group_by_name(
 static int imx_get_groups_count(struct pinctrl_dev *pctldev)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 
 	return info->ngroups;
 }
@@ -74,7 +74,7 @@ static const char *imx_get_group_name(struct pinctrl_dev *pctldev,
 				       unsigned selector)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 
 	return info->groups[selector].name;
 }
@@ -84,7 +84,7 @@ static int imx_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
 			       unsigned *npins)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 
 	if (selector >= info->ngroups)
 		return -EINVAL;
@@ -106,7 +106,7 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
 			struct pinctrl_map **map, unsigned *num_maps)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 	const struct imx_pin_group *grp;
 	struct pinctrl_map *new_map;
 	struct device_node *parent;
@@ -186,7 +186,7 @@ static int imx_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
 		       unsigned group)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 	const struct imx_pin_reg *pin_reg;
 	unsigned int npins, pin_id;
 	int i;
@@ -275,7 +275,7 @@ static int imx_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
 static int imx_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 
 	return info->nfunctions;
 }
@@ -284,7 +284,7 @@ static const char *imx_pmx_get_func_name(struct pinctrl_dev *pctldev,
 					  unsigned selector)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 
 	return info->functions[selector].name;
 }
@@ -294,7 +294,7 @@ static int imx_pmx_get_groups(struct pinctrl_dev *pctldev, unsigned selector,
 			       unsigned * const num_groups)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 
 	*groups = info->functions[selector].groups;
 	*num_groups = info->functions[selector].num_groups;
@@ -306,7 +306,7 @@ static int imx_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range, unsigned offset)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 	const struct imx_pin_reg *pin_reg;
 	struct imx_pin_group *grp;
 	struct imx_pin *imx_pin;
@@ -346,7 +346,7 @@ static void imx_pmx_gpio_disable_free(struct pinctrl_dev *pctldev,
 			struct pinctrl_gpio_range *range, unsigned offset)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 	const struct imx_pin_reg *pin_reg;
 	u32 reg;
 
@@ -371,7 +371,7 @@ static int imx_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
 	   struct pinctrl_gpio_range *range, unsigned offset, bool input)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 	const struct imx_pin_reg *pin_reg;
 	u32 reg;
 
@@ -411,7 +411,7 @@ static int imx_pinconf_get(struct pinctrl_dev *pctldev,
 			     unsigned pin_id, unsigned long *config)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 	const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
 
 	if (pin_reg->conf_reg == -1) {
@@ -433,7 +433,7 @@ static int imx_pinconf_set(struct pinctrl_dev *pctldev,
 			     unsigned num_configs)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 	const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
 	int i;
 
@@ -467,7 +467,7 @@ static void imx_pinconf_dbg_show(struct pinctrl_dev *pctldev,
 				   struct seq_file *s, unsigned pin_id)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 	const struct imx_pin_reg *pin_reg = &info->pin_regs[pin_id];
 	unsigned long config;
 
@@ -484,7 +484,7 @@ static void imx_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
 					 struct seq_file *s, unsigned group)
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
-	const struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pinctrl_soc_info *info = ipctl->info;
 	struct imx_pin_group *grp;
 	unsigned long config;
 	const char *name;
-- 
2.9.3

[RFC v2 2/3] pinctrl: imx: use radix trees for groups and functions

[Gary Bisson]

This change is inspired from the pinctrl-single architecture.

The problem with current implementation is that it isn't possible
to add/remove functions and/or groups dynamically. The radix tree
offers an easy way to do so. The intent is to offer a follow-up
patch later that will enable the use of pinctrl nodes in dt-overlays.

Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 164 +++++++++++++++++++++++++++-----
 drivers/pinctrl/freescale/pinctrl-imx.h |   5 +-
 2 files changed, 141 insertions(+), 28 deletions(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 8697c1b..e832a2c 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -53,10 +53,9 @@ static inline const struct imx_pin_group *imx_pinctrl_find_group_by_name(
 	int i;
 
 	for (i = 0; i < info->ngroups; i++) {
-		if (!strcmp(info->groups[i].name, name)) {
-			grp = &info->groups[i];
+		grp = radix_tree_lookup(&info->pgtree, i);
+		if (grp && !strcmp(grp->name, name))
 			break;
-		}
 	}
 
 	return grp;
@@ -75,8 +74,13 @@ static const char *imx_get_group_name(struct pinctrl_dev *pctldev,
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pin_group *grp = NULL;
+
+	grp = radix_tree_lookup(&info->pgtree, selector);
+	if (!grp)
+		return NULL;
 
-	return info->groups[selector].name;
+	return grp->name;
 }
 
 static int imx_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
@@ -85,12 +89,17 @@ static int imx_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pin_group *grp = NULL;
 
 	if (selector >= info->ngroups)
 		return -EINVAL;
 
-	*pins = info->groups[selector].pin_ids;
-	*npins = info->groups[selector].npins;
+	grp = radix_tree_lookup(&info->pgtree, selector);
+	if (!grp)
+		return -EINVAL;
+
+	*pins = grp->pin_ids;
+	*npins = grp->npins;
 
 	return 0;
 }
@@ -190,17 +199,25 @@ static int imx_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
 	const struct imx_pin_reg *pin_reg;
 	unsigned int npins, pin_id;
 	int i;
-	struct imx_pin_group *grp;
+	struct imx_pin_group *grp = NULL;
+	struct imx_pmx_func *func = NULL;
 
 	/*
 	 * Configure the mux mode for each pin in the group for a specific
 	 * function.
 	 */
-	grp = &info->groups[group];
+	grp = radix_tree_lookup(&info->pgtree, group);
+	if (!grp)
+		return -EINVAL;
+
+	func = radix_tree_lookup(&info->ftree, selector);
+	if (!func)
+		return -EINVAL;
+
 	npins = grp->npins;
 
 	dev_dbg(ipctl->dev, "enable function %s group %s\n",
-		info->functions[selector].name, grp->name);
+		func->name, grp->name);
 
 	for (i = 0; i < npins; i++) {
 		struct imx_pin *pin = &grp->pins[i];
@@ -285,8 +302,13 @@ static const char *imx_pmx_get_func_name(struct pinctrl_dev *pctldev,
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pmx_func *func = NULL;
+
+	func = radix_tree_lookup(&info->ftree, selector);
+	if (!func)
+		return NULL;
 
-	return info->functions[selector].name;
+	return func->name;
 }
 
 static int imx_pmx_get_groups(struct pinctrl_dev *pctldev, unsigned selector,
@@ -295,9 +317,14 @@ static int imx_pmx_get_groups(struct pinctrl_dev *pctldev, unsigned selector,
 {
 	struct imx_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
 	struct imx_pinctrl_soc_info *info = ipctl->info;
+	struct imx_pmx_func *func = NULL;
+
+	func = radix_tree_lookup(&info->ftree, selector);
+	if (!func)
+		return -EINVAL;
 
-	*groups = info->functions[selector].groups;
-	*num_groups = info->functions[selector].num_groups;
+	*groups = func->groups;
+	*num_groups = func->num_groups;
 
 	return 0;
 }
@@ -323,7 +350,9 @@ static int imx_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
 
 	/* Find the pinctrl config with GPIO mux mode for the requested pin */
 	for (group = 0; group < info->ngroups; group++) {
-		grp = &info->groups[group];
+		grp = radix_tree_lookup(&info->pgtree, group);
+		if (!grp)
+			continue;
 		for (pin = 0; pin < grp->npins; pin++) {
 			imx_pin = &grp->pins[pin];
 			if (imx_pin->pin == offset && !imx_pin->mux_mode)
@@ -494,7 +523,10 @@ static void imx_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
 		return;
 
 	seq_printf(s, "\n");
-	grp = &info->groups[group];
+	grp = radix_tree_lookup(&info->pgtree, group);
+	if (!grp)
+		return;
+
 	for (i = 0; i < grp->npins; i++) {
 		struct imx_pin *pin = &grp->pins[i];
 		name = pin_get_name(pctldev, pin->pin);
@@ -614,7 +646,9 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
 
 	dev_dbg(info->dev, "parse function(%d): %s\n", index, np->name);
 
-	func = &info->functions[index];
+	func = radix_tree_lookup(&info->ftree, index);
+	if (!func)
+		return -EINVAL;
 
 	/* Initialise function */
 	func->name = np->name;
@@ -628,7 +662,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
 
 	for_each_child_of_node(np, child) {
 		func->groups[i] = child->name;
-		grp = &info->groups[info->group_index++];
+
+		grp = devm_kzalloc(info->dev, sizeof(struct imx_pin_group),
+				   GFP_KERNEL);
+		if (!grp)
+			return -ENOMEM;
+
+		mutex_lock(&info->mutex);
+		radix_tree_insert(&info->pgtree, info->group_index++, grp);
+		mutex_unlock(&info->mutex);
+
 		imx_pinctrl_parse_groups(child, grp, info, i++);
 	}
 
@@ -681,11 +724,19 @@ static int imx_pinctrl_probe_dt(struct platform_device *pdev,
 		}
 	}
 
-	info->nfunctions = nfuncs;
-	info->functions = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pmx_func),
+	for (i = 0; i < nfuncs; i++) {
+		struct imx_pmx_func *function;
+
+		function = devm_kzalloc(&pdev->dev, sizeof(*function),
 					GFP_KERNEL);
-	if (!info->functions)
-		return -ENOMEM;
+		if (!function)
+			return -ENOMEM;
+
+		mutex_lock(&info->mutex);
+		radix_tree_insert(&info->ftree, i, function);
+		mutex_unlock(&info->mutex);
+	}
+	info->nfunctions = nfuncs;
 
 	info->group_index = 0;
 	if (flat_funcs) {
@@ -695,14 +746,11 @@ static int imx_pinctrl_probe_dt(struct platform_device *pdev,
 		for_each_child_of_node(np, child)
 			info->ngroups += of_get_child_count(child);
 	}
-	info->groups = devm_kzalloc(&pdev->dev, info->ngroups * sizeof(struct imx_pin_group),
-					GFP_KERNEL);
-	if (!info->groups)
-		return -ENOMEM;
 
 	if (flat_funcs) {
 		imx_pinctrl_parse_functions(np, info, 0);
 	} else {
+		i = 0;
 		for_each_child_of_node(np, child)
 			imx_pinctrl_parse_functions(child, info, i++);
 	}
@@ -710,6 +758,59 @@ static int imx_pinctrl_probe_dt(struct platform_device *pdev,
 	return 0;
 }
 
+/*
+ * imx_free_funcs() - free memory used by functions
+ * @info: info driver instance
+ */
+static void imx_free_funcs(struct imx_pinctrl_soc_info *info)
+{
+	int i;
+
+	mutex_lock(&info->mutex);
+	for (i = 0; i < info->nfunctions; i++) {
+		struct imx_pmx_func *func;
+
+		func = radix_tree_lookup(&info->ftree, i);
+		if (!func)
+			continue;
+		radix_tree_delete(&info->ftree, i);
+	}
+	mutex_unlock(&info->mutex);
+}
+
+/*
+ * imx_free_pingroups() - free memory used by pingroups
+ * @info: info driver instance
+ */
+static void imx_free_pingroups(struct imx_pinctrl_soc_info *info)
+{
+	int i;
+
+	mutex_lock(&info->mutex);
+	for (i = 0; i < info->ngroups; i++) {
+		struct imx_pin_group *pingroup;
+
+		pingroup = radix_tree_lookup(&info->pgtree, i);
+		if (!pingroup)
+			continue;
+		radix_tree_delete(&info->pgtree, i);
+	}
+	mutex_unlock(&info->mutex);
+}
+
+/*
+ * imx_free_resources() - free memory used by this driver
+ * @info: info driver instance
+ */
+static void imx_free_resources(struct imx_pinctrl *ipctl)
+{
+	if (ipctl->pctl)
+		pinctrl_unregister(ipctl->pctl);
+
+	imx_free_funcs(ipctl->info);
+	imx_free_pingroups(ipctl->info);
+}
+
 int imx_pinctrl_probe(struct platform_device *pdev,
 		      struct imx_pinctrl_soc_info *info)
 {
@@ -783,10 +884,15 @@ int imx_pinctrl_probe(struct platform_device *pdev,
 	imx_pinctrl_desc->confops = &imx_pinconf_ops;
 	imx_pinctrl_desc->owner = THIS_MODULE;
 
+	mutex_init(&info->mutex);
+
+	INIT_RADIX_TREE(&info->pgtree, GFP_KERNEL);
+	INIT_RADIX_TREE(&info->ftree, GFP_KERNEL);
+
 	ret = imx_pinctrl_probe_dt(pdev, info);
 	if (ret) {
 		dev_err(&pdev->dev, "fail to probe dt properties\n");
-		return ret;
+		goto free;
 	}
 
 	ipctl->info = info;
@@ -796,10 +902,16 @@ int imx_pinctrl_probe(struct platform_device *pdev,
 					    imx_pinctrl_desc, ipctl);
 	if (IS_ERR(ipctl->pctl)) {
 		dev_err(&pdev->dev, "could not register IMX pinctrl driver\n");
-		return PTR_ERR(ipctl->pctl);
+		ret = PTR_ERR(ipctl->pctl);
+		goto free;
 	}
 
 	dev_info(&pdev->dev, "initialized IMX pinctrl driver\n");
 
 	return 0;
+
+free:
+	imx_free_resources(ipctl);
+
+	return ret;
 }
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.h b/drivers/pinctrl/freescale/pinctrl-imx.h
index 8af8aa2..62aa8f8 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.h
+++ b/drivers/pinctrl/freescale/pinctrl-imx.h
@@ -76,13 +76,14 @@ struct imx_pinctrl_soc_info {
 	const struct pinctrl_pin_desc *pins;
 	unsigned int npins;
 	struct imx_pin_reg *pin_regs;
-	struct imx_pin_group *groups;
 	unsigned int ngroups;
 	unsigned int group_index;
-	struct imx_pmx_func *functions;
 	unsigned int nfunctions;
 	unsigned int flags;
 	const char *gpr_compatible;
+	struct radix_tree_root ftree;
+	struct radix_tree_root pgtree;
+	struct mutex mutex;
 };
 
 #define SHARE_MUX_CONF_REG	0x1
-- 
2.9.3

[RFC v2 3/3] pinctrl: imx: add of_notifier for dynamic pinctrl changes

[Gary Bisson]

Currently the implementation is limited:
- can only add a new function
- if function already exists, it is discarded
- requires the dtso to know about this architecture

Another option would be to do like the pinctrl-single where each
group has its own function. Therefore it is easy to manage them.

Currently there's no easy way to add/remove groups to an existing
function.

Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
---
Hi all,

This implementation is far from ideal. It must be seen as a starting
point more than anything else.

The goal was to have the simplest patch possible. Here a group cannot
be added to an existing function. But a new function can be created.

There's currently no way to remove the group that has been added.

Regards,
Gary
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 103 ++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index e832a2c..5951a31 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -811,6 +811,106 @@ static void imx_free_resources(struct imx_pinctrl *ipctl)
 	imx_free_pingroups(ipctl->info);
 }
 
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
+static int imx_pinctrl_add_function(struct device_node *np,
+				    struct imx_pinctrl_soc_info *info)
+{
+	struct imx_pmx_func *func;
+	u32 i = 0;
+
+	for (i = 0; i < info->nfunctions; i++) {
+		struct imx_pmx_func *func;
+
+		func = radix_tree_lookup(&info->ftree, i);
+		if (!func)
+			continue;
+
+		if (!strcmp(func->name, np->name)) {
+			dev_err(info->dev, "function %s already exists!\n",
+				np->name);
+			return -EINVAL;
+		}
+	}
+
+	/* Create a new function */
+	func = devm_kzalloc(info->dev, sizeof(*func), GFP_KERNEL);
+	if (!func)
+		return -ENOMEM;
+
+	mutex_lock(&info->mutex);
+	radix_tree_insert(&info->ftree, info->nfunctions, func);
+	info->ngroups += of_get_child_count(np);
+	mutex_unlock(&info->mutex);
+
+	imx_pinctrl_parse_functions(np, info, info->nfunctions);
+
+	mutex_lock(&info->mutex);
+	info->nfunctions++;
+	mutex_unlock(&info->mutex);
+
+	return 0;
+}
+
+static inline struct platform_device *imx_get_device(struct device_node *np)
+{
+	struct platform_device *pdev = of_find_device_by_node(np->parent);
+
+	if (!pdev)
+		pdev = of_find_device_by_node(np->parent->parent);
+
+	return pdev;
+}
+
+static int of_pinctrl_notify(struct notifier_block *nb, unsigned long action,
+			     void *arg)
+{
+	struct of_reconfig_data *rd = arg;
+	struct platform_device *pdev;
+	struct imx_pinctrl *ipctl;
+	int ret;
+
+	/* Looking for a pinctrl group node */
+	if (!of_property_read_bool(rd->dn, "fsl,pins"))
+		return NOTIFY_OK; /* not for us */
+
+	switch (of_reconfig_get_state_change(action, rd)) {
+	case OF_RECONFIG_CHANGE_ADD:
+		pdev = imx_get_device(rd->dn);
+		if (!pdev) {
+			pr_err("%s: couldn't find device\n", __func__);
+			return NOTIFY_OK;
+		}
+
+		ipctl = dev_get_drvdata(&pdev->dev);
+		if (!pdev) {
+			dev_err(&pdev->dev, "driver data not found\n");
+			return NOTIFY_OK;
+		}
+
+		ret = imx_pinctrl_add_function(rd->dn->parent, ipctl->info);
+		if (ret < 0)
+			dev_err(&pdev->dev, "couldn't add function: %d\n", ret);
+
+		break;
+	case OF_RECONFIG_CHANGE_REMOVE:
+		pdev = imx_get_device(rd->dn);
+		if (!pdev) {
+			pr_err("%s: couldn't find device\n", __func__);
+			return NOTIFY_OK;
+		}
+		dev_err(&pdev->dev, "removing a function isn't supported\n");
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+static struct notifier_block pinctrl_of_notifier = {
+	.notifier_call = of_pinctrl_notify,
+};
+#else
+extern struct notifier_block pinctrl_of_notifier;
+#endif /* CONFIG_OF_DYNAMIC */
+
 int imx_pinctrl_probe(struct platform_device *pdev,
 		      struct imx_pinctrl_soc_info *info)
 {
@@ -906,6 +1006,9 @@ int imx_pinctrl_probe(struct platform_device *pdev,
 		goto free;
 	}
 
+	if (IS_ENABLED(CONFIG_OF_DYNAMIC))
+		WARN_ON(of_reconfig_notifier_register(&pinctrl_of_notifier));
+
 	dev_info(&pdev->dev, "initialized IMX pinctrl driver\n");
 
 	return 0;
-- 
2.9.3

Re: [RFC v2 2/3] pinctrl: imx: use radix trees for groups and functions

[Linus Walleij]

On Fri, Dec 2, 2016 at 5:35 PM, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:

> This change is inspired from the pinctrl-single architecture.
>
> The problem with current implementation is that it isn't possible
> to add/remove functions and/or groups dynamically. The radix tree
> offers an easy way to do so. The intent is to offer a follow-up
> patch later that will enable the use of pinctrl nodes in dt-overlays.
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>

I would like some ACK from the i.MX pinctrl maintainers.

Fabio, do you have the context you need to ack/nack this?

Yours,
Linus Walleij

Re: [RFC v2 2/3] pinctrl: imx: use radix trees for groups and functions

[Fabio Estevam]

On Tue, Dec 27, 2016 at 11:05 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Fri, Dec 2, 2016 at 5:35 PM, Gary Bisson
> <gary.bisson@boundarydevices.com> wrote:
>
>> This change is inspired from the pinctrl-single architecture.
>>
>> The problem with current implementation is that it isn't possible
>> to add/remove functions and/or groups dynamically. The radix tree
>> offers an easy way to do so. The intent is to offer a follow-up
>> patch later that will enable the use of pinctrl nodes in dt-overlays.
>>
>> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
>
> I would like some ACK from the i.MX pinctrl maintainers.
>
> Fabio, do you have the context you need to ack/nack this?

Looks good:

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [RFC v2 2/3] pinctrl: imx: use radix trees for groups and functions

[Linus Walleij]

On Fri, Dec 2, 2016 at 5:35 PM, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:

> This change is inspired from the pinctrl-single architecture.
>
> The problem with current implementation is that it isn't possible
> to add/remove functions and/or groups dynamically. The radix tree
> offers an easy way to do so. The intent is to offer a follow-up
> patch later that will enable the use of pinctrl nodes in dt-overlays.
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>

This patch doesn't apply on v4.10-rc1 so please rebase it
on the mainline and resend as v3, include Fabio's review tag.

Yours,
Linus Walleij

Re: [RFC v2 2/3] pinctrl: imx: use radix trees for groups and functions

[Gary Bisson]

Hi Linus, All,

On Tue, Dec 27, 2016 at 11:07 PM, Linus Walleij
<linus.walleij@linaro.org> wrote:
>
> On Fri, Dec 2, 2016 at 5:35 PM, Gary Bisson
> <gary.bisson@boundarydevices.com> wrote:
>
> > This change is inspired from the pinctrl-single architecture.
> >
> > The problem with current implementation is that it isn't possible
> > to add/remove functions and/or groups dynamically. The radix tree
> > offers an easy way to do so. The intent is to offer a follow-up
> > patch later that will enable the use of pinctrl nodes in dt-overlays.
> >
> > Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
>
> This patch doesn't apply on v4.10-rc1 so please rebase it
> on the mainline and resend as v3, include Fabio's review tag.

Actually it applies if the first patch of this v2 series is also applied first.
https://patchwork.kernel.org/patch/9458883/
https://patchwork.kernel.org/patch/9458889/

The 3rd patch of the series can be discarded though since it was just
an example to show the benefit of the first two.

I guess someone needs to also ack/review the first one. Because
switching to radix tree keeping the const qualifier of the driver
structure isn't really useful.

Regards,
Gary

Re: [RFC v2 1/3] pinctrl: imx: remove const qualifier of imx_pinctrl_soc_info

[Fabio Estevam]

On Fri, Dec 2, 2016 at 2:35 PM, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:
> Otherwise can't dynamically update fields such as ngroups which can
> change over time (with a dt-overlay for instance).
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

Re: [RFC v2 2/3] pinctrl: imx: use radix trees for groups and functions

[Linus Walleij]

n Thu, Dec 29, 2016 at 10:44 AM, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:
> On Tue, Dec 27, 2016 at 11:07 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
>>
>> On Fri, Dec 2, 2016 at 5:35 PM, Gary Bisson
>> <gary.bisson@boundarydevices.com> wrote:
>>
>> > This change is inspired from the pinctrl-single architecture.
>> >
>> > The problem with current implementation is that it isn't possible
>> > to add/remove functions and/or groups dynamically. The radix tree
>> > offers an easy way to do so. The intent is to offer a follow-up
>> > patch later that will enable the use of pinctrl nodes in dt-overlays.
>> >
>> > Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
>>
>> This patch doesn't apply on v4.10-rc1 so please rebase it
>> on the mainline and resend as v3, include Fabio's review tag.
>
> Actually it applies if the first patch of this v2 series is also applied first.
> https://patchwork.kernel.org/patch/9458883/
> https://patchwork.kernel.org/patch/9458889/

Heh if it is part of a patch series then send it as part of the series
and don't confuse me ;)

> The 3rd patch of the series can be discarded though since it was just
> an example to show the benefit of the first two.
>
> I guess someone needs to also ack/review the first one. Because
> switching to radix tree keeping the const qualifier of the driver
> structure isn't really useful.

Sure, Fabio?

Yours,
Linus Walleij

Re: [RFC v2 1/3] pinctrl: imx: remove const qualifier of imx_pinctrl_soc_info

[Linus Walleij]

On Fri, Dec 2, 2016 at 5:35 PM, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:

> Otherwise can't dynamically update fields such as ngroups which can
> change over time (with a dt-overlay for instance).
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>

Patch applied with Fabio's review tag.

Yours,
Linus Walleij

Re: [RFC v2 2/3] pinctrl: imx: use radix trees for groups and functions

[Linus Walleij]

On Fri, Dec 2, 2016 at 5:35 PM, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:

> This change is inspired from the pinctrl-single architecture.
>
> The problem with current implementation is that it isn't possible
> to add/remove functions and/or groups dynamically. The radix tree
> offers an easy way to do so. The intent is to offer a follow-up
> patch later that will enable the use of pinctrl nodes in dt-overlays.
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>

Patch applied with Fabio's ACK.

Yours,
Linus Walleij