2016-12-27 17:20:23

by Tony Lindgren

[permalink] [raw]
Subject: [PATCHv2 0/5] Add generic pinctrl helpers for managing groups and function

Hi all,

Here are some changes to add generic helpers for managing pinctrl groups and
functions.

Regards,

Tony

Changes since v1:

- Updates to the first patch in the series to use delayed work for pinctrl
hogs based on what was discussed on the mailing list, mostly to pass
pctldev to the parsers and add pinctrl_dt_has_hogs() check

Tony Lindgren (5):
pinctrl: core: Use delayed work for hogs
pinctrl: core: Add generic pinctrl functions for managing groups
pinctrl: core: Add generic pinctrl functions for managing groups
pinctrl: single: Use generic pinctrl helpers for managing groups
pinctrl: single: Use generic pinmux helpers for managing functions

drivers/pinctrl/Kconfig | 11 +-
drivers/pinctrl/core.c | 270 +++++++++++++++++++++++++++++++-----
drivers/pinctrl/core.h | 67 +++++++++
drivers/pinctrl/devicetree.c | 28 +++-
drivers/pinctrl/devicetree.h | 12 +-
drivers/pinctrl/pinctrl-single.c | 290 ++++-----------------------------------
drivers/pinctrl/pinmux.c | 173 +++++++++++++++++++++++
drivers/pinctrl/pinmux.h | 42 ++++++
8 files changed, 591 insertions(+), 302 deletions(-)

--
2.11.0


2016-12-27 17:20:31

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 3/5] pinctrl: core: Add generic pinctrl functions for managing groups

We can add generic helpers for function handling for cases where the pin
controller driver does not need to use static arrays.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/pinctrl/Kconfig | 4 ++
drivers/pinctrl/core.c | 2 +
drivers/pinctrl/core.h | 18 +++++
drivers/pinctrl/pinmux.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++
drivers/pinctrl/pinmux.h | 42 ++++++++++++
5 files changed, 239 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -14,6 +14,10 @@ config GENERIC_PINCTRL
config PINMUX
bool "Support pin multiplexing controllers" if COMPILE_TEST

+config GENERIC_PINMUX
+ bool
+ select PINMUX
+
config PINCONF
bool "Support pin configuration controllers" if COMPILE_TEST

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1988,6 +1988,7 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
pctldev->driver_data = driver_data;
INIT_RADIX_TREE(&pctldev->pin_desc_tree, GFP_KERNEL);
INIT_RADIX_TREE(&pctldev->pin_group_tree, GFP_KERNEL);
+ INIT_RADIX_TREE(&pctldev->pin_function_tree, GFP_KERNEL);
INIT_LIST_HEAD(&pctldev->gpio_ranges);
INIT_DELAYED_WORK(&pctldev->late_init, pinctrl_late_init);
pctldev->dev = dev;
@@ -2062,6 +2063,7 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
mutex_lock(&pctldev->mutex);
/* TODO: check that no pinmuxes are still active? */
list_del(&pctldev->node);
+ pinmux_generic_free_functions(pctldev);
pinctrl_generic_free_groups(pctldev);
/* Destroy descriptor tree */
pinctrl_free_pindescs(pctldev, pctldev->desc->pins,
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -25,7 +25,9 @@ struct pinctrl_gpio_range;
* @pin_desc_tree: each pin descriptor for this pin controller is stored in
* this radix tree
* @pin_group_tree: optionally each pin group can be stored in this radix tree
+ * @pin_function_tree: optionally each function can be stored in this radix tree
* @num_groups: optionally number of groups can be kept here
+ * @num_functions: optionally number of functions can be kept here
* @gpio_ranges: a list of GPIO ranges that is handled by this pin controller,
* ranges are added to this list at runtime
* @dev: the device entry for this pin controller
@@ -44,7 +46,9 @@ struct pinctrl_dev {
struct pinctrl_desc *desc;
struct radix_tree_root pin_desc_tree;
struct radix_tree_root pin_group_tree;
+ struct radix_tree_root pin_function_tree;
unsigned int num_groups;
+ unsigned int num_functions;
struct list_head gpio_ranges;
struct device *dev;
struct module *owner;
@@ -180,6 +184,20 @@ struct group_desc {
};

/**
+ * struct function_desc - generic function descriptor
+ * @name: name of the function
+ * @group_names: array of pin group names
+ * @num_group_names: number of pin group names
+ * @data: pin controller driver specific data
+ */
+struct function_desc {
+ const char *name;
+ const char **group_names;
+ int num_group_names;
+ void *data;
+};
+
+/**
* struct pinctrl_maps - a list item containing part of the mapping table
* @node: mapping table list node
* @maps: array of mapping table entries
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -695,3 +695,176 @@ void pinmux_init_device_debugfs(struct dentry *devroot,
}

#endif /* CONFIG_DEBUG_FS */
+
+#ifdef CONFIG_GENERIC_PINMUX
+
+/**
+ * pinmux_generic_get_function_count() - returns number of functions
+ * @pctldev: pin controller device
+ */
+int pinmux_generic_get_function_count(struct pinctrl_dev *pctldev)
+{
+ return pctldev->num_functions;
+}
+EXPORT_SYMBOL_GPL(pinmux_generic_get_function_count);
+
+/**
+ * pinmux_generic_get_function_name() - returns the function name
+ * @pctldev: pin controller device
+ * @selector: function number
+ */
+const char *
+pinmux_generic_get_function_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ struct function_desc *function;
+
+ function = radix_tree_lookup(&pctldev->pin_function_tree,
+ selector);
+ if (!function)
+ return NULL;
+
+ return function->name;
+}
+EXPORT_SYMBOL_GPL(pinmux_generic_get_function_name);
+
+/**
+ * pinmux_generic_get_function_groups() - gets the function groups
+ * @pctldev: pin controller device
+ * @selector: function number
+ * @groups: array of pin groups
+ * @num_groups: number of pin groups
+ */
+int pinmux_generic_get_function_groups(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const char * const **groups,
+ unsigned * const num_groups)
+{
+ struct function_desc *function;
+
+ function = radix_tree_lookup(&pctldev->pin_function_tree,
+ selector);
+ if (!function) {
+ dev_err(pctldev->dev, "%s could not find function%i\n",
+ __func__, selector);
+ return -EINVAL;
+ }
+ *groups = function->group_names;
+ *num_groups = function->num_group_names;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinmux_generic_get_function_groups);
+
+/**
+ * pinmux_generic_get_function() - returns a function based on the number
+ * @pctldev: pin controller device
+ * @group_selector: function number
+ */
+struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ struct function_desc *function;
+
+ function = radix_tree_lookup(&pctldev->pin_function_tree,
+ selector);
+ if (!function)
+ return NULL;
+
+ return function;
+}
+EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
+
+/**
+ * pinmux_generic_get_function_groups() - gets the function groups
+ * @pctldev: pin controller device
+ * @name: name of the function
+ * @groups: array of pin groups
+ * @num_groups: number of pin groups
+ * @data: pin controller driver specific data
+ */
+int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
+ const char *name,
+ const char **groups,
+ const unsigned int num_groups,
+ void *data)
+{
+ struct function_desc *function;
+
+ function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL);
+ if (!function)
+ return -ENOMEM;
+
+ function->name = name;
+ function->group_names = groups;
+ function->num_group_names = num_groups;
+ function->data = data;
+
+ radix_tree_insert(&pctldev->pin_function_tree, pctldev->num_functions,
+ function);
+
+ pctldev->num_functions++;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinmux_generic_add_function);
+
+/**
+ * pinmux_generic_remove_function() - removes a numbered function
+ * @pctldev: pin controller device
+ * @selector: function number
+ *
+ * Note that the caller must take care of locking.
+ */
+int pinmux_generic_remove_function(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ struct function_desc *function;
+
+ function = radix_tree_lookup(&pctldev->pin_function_tree,
+ selector);
+ if (!function)
+ return -ENOENT;
+
+ radix_tree_delete(&pctldev->pin_function_tree, selector);
+ devm_kfree(pctldev->dev, function);
+
+ pctldev->num_functions--;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinmux_generic_remove_function);
+
+/**
+ * pinmux_generic_free_functions() - removes all functions
+ * @pctldev: pin controller device
+ *
+ * Note that the caller must take care of locking.
+ */
+void pinmux_generic_free_functions(struct pinctrl_dev *pctldev)
+{
+ struct radix_tree_iter iter;
+ struct function_desc *function;
+ unsigned long *indices;
+ void **slot;
+ int i = 0;
+
+ indices = devm_kzalloc(pctldev->dev, sizeof(*indices) *
+ pctldev->num_functions, GFP_KERNEL);
+ if (!indices)
+ return;
+
+ radix_tree_for_each_slot(slot, &pctldev->pin_function_tree, &iter, 0)
+ indices[i++] = iter.index;
+
+ for (i = 0; i < pctldev->num_functions; i++) {
+ function = radix_tree_lookup(&pctldev->pin_function_tree,
+ indices[i]);
+ radix_tree_delete(&pctldev->pin_function_tree, indices[i]);
+ devm_kfree(pctldev->dev, function);
+ }
+
+ pctldev->num_functions = 0;
+}
+
+#endif /* CONFIG_GENERIC_PINMUX */
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -111,3 +111,45 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
}

#endif
+
+#ifdef CONFIG_GENERIC_PINMUX
+
+int pinmux_generic_get_function_count(struct pinctrl_dev *pctldev);
+
+const char *
+pinmux_generic_get_function_name(struct pinctrl_dev *pctldev,
+ unsigned int selector);
+
+int pinmux_generic_get_function_groups(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const char * const **groups,
+ unsigned * const num_groups);
+
+struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
+ unsigned int selector);
+
+int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
+ const char *name,
+ const char **groups,
+ unsigned const num_groups,
+ void *data);
+
+int pinmux_generic_remove_function(struct pinctrl_dev *pctldev,
+ unsigned int selector);
+
+static inline int
+pinmux_generic_remove_last_function(struct pinctrl_dev *pctldev)
+{
+ return pinmux_generic_remove_function(pctldev,
+ pctldev->num_functions - 1);
+}
+
+void pinmux_generic_free_functions(struct pinctrl_dev *pctldev);
+
+#else
+
+static inline void pinmux_generic_free_functions(struct pinctrl_dev *pctldev)
+{
+}
+
+#endif
--
2.11.0

2016-12-27 17:20:44

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 4/5] pinctrl: single: Use generic pinctrl helpers for managing groups

We can now drop the driver specific code for managing groups.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/pinctrl/Kconfig | 2 +-
drivers/pinctrl/pinctrl-single.c | 156 +++------------------------------------
2 files changed, 12 insertions(+), 146 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -166,8 +166,8 @@ config PINCTRL_ROCKCHIP
config PINCTRL_SINGLE
tristate "One-register-per-pin type device tree based pinctrl driver"
depends on OF
+ select GENERIC_PINCTRL
select PINMUX
- select PINCONF
select GENERIC_PINCONF
help
This selects the device tree based generic pinctrl driver.
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -38,22 +38,6 @@
#define PCS_OFF_DISABLED ~0U

/**
- * struct pcs_pingroup - pingroups for a function
- * @np: pingroup device node pointer
- * @name: pingroup name
- * @gpins: array of the pins in the group
- * @ngpins: number of pins in the group
- * @node: list node
- */
-struct pcs_pingroup {
- struct device_node *np;
- const char *name;
- int *gpins;
- int ngpins;
- struct list_head node;
-};
-
-/**
* struct pcs_func_vals - mux function register offset and value pair
* @reg: register virtual address
* @val: register value
@@ -176,15 +160,12 @@ struct pcs_soc_data {
* @bits_per_mux: number of bits per mux
* @bits_per_pin: number of bits per pin
* @pins: physical pins on the SoC
- * @pgtree: pingroup index radix tree
* @ftree: function index radix tree
- * @pingroups: list of pingroups
* @functions: list of functions
* @gpiofuncs: list of gpio functions
* @irqs: list of interrupt registers
* @chip: chip container for this instance
* @domain: IRQ domain for this instance
- * @ngroups: number of pingroups
* @nfuncs: number of functions
* @desc: pin controller descriptor
* @read: register read function to use
@@ -213,15 +194,12 @@ struct pcs_device {
bool bits_per_mux;
unsigned bits_per_pin;
struct pcs_data pins;
- struct radix_tree_root pgtree;
struct radix_tree_root ftree;
- struct list_head pingroups;
struct list_head functions;
struct list_head gpiofuncs;
struct list_head irqs;
struct irq_chip chip;
struct irq_domain *domain;
- unsigned ngroups;
unsigned nfuncs;
struct pinctrl_desc desc;
unsigned (*read)(void __iomem *reg);
@@ -288,54 +266,6 @@ static void __maybe_unused pcs_writel(unsigned val, void __iomem *reg)
writel(val, reg);
}

-static int pcs_get_groups_count(struct pinctrl_dev *pctldev)
-{
- struct pcs_device *pcs;
-
- pcs = pinctrl_dev_get_drvdata(pctldev);
-
- return pcs->ngroups;
-}
-
-static const char *pcs_get_group_name(struct pinctrl_dev *pctldev,
- unsigned gselector)
-{
- struct pcs_device *pcs;
- struct pcs_pingroup *group;
-
- pcs = pinctrl_dev_get_drvdata(pctldev);
- group = radix_tree_lookup(&pcs->pgtree, gselector);
- if (!group) {
- dev_err(pcs->dev, "%s could not find pingroup%i\n",
- __func__, gselector);
- return NULL;
- }
-
- return group->name;
-}
-
-static int pcs_get_group_pins(struct pinctrl_dev *pctldev,
- unsigned gselector,
- const unsigned **pins,
- unsigned *npins)
-{
- struct pcs_device *pcs;
- struct pcs_pingroup *group;
-
- pcs = pinctrl_dev_get_drvdata(pctldev);
- group = radix_tree_lookup(&pcs->pgtree, gselector);
- if (!group) {
- dev_err(pcs->dev, "%s could not find pingroup%i\n",
- __func__, gselector);
- return -EINVAL;
- }
-
- *pins = group->gpins;
- *npins = group->ngpins;
-
- return 0;
-}
-
static void pcs_pin_dbg_show(struct pinctrl_dev *pctldev,
struct seq_file *s,
unsigned pin)
@@ -369,9 +299,9 @@ static int pcs_dt_node_to_map(struct pinctrl_dev *pctldev,
struct pinctrl_map **map, unsigned *num_maps);

static const struct pinctrl_ops pcs_pinctrl_ops = {
- .get_groups_count = pcs_get_groups_count,
- .get_group_name = pcs_get_group_name,
- .get_group_pins = pcs_get_group_pins,
+ .get_groups_count = pinctrl_generic_get_group_count,
+ .get_group_name = pinctrl_generic_get_group_name,
+ .get_group_pins = pinctrl_generic_get_group_pins,
.pin_dbg_show = pcs_pin_dbg_show,
.dt_node_to_map = pcs_dt_node_to_map,
.dt_free_map = pcs_dt_free_map,
@@ -685,7 +615,7 @@ static int pcs_pinconf_group_get(struct pinctrl_dev *pctldev,
unsigned npins, old = 0;
int i, ret;

- ret = pcs_get_group_pins(pctldev, group, &pins, &npins);
+ ret = pinctrl_generic_get_group_pins(pctldev, group, &pins, &npins);
if (ret)
return ret;
for (i = 0; i < npins; i++) {
@@ -707,7 +637,7 @@ static int pcs_pinconf_group_set(struct pinctrl_dev *pctldev,
unsigned npins;
int i, ret;

- ret = pcs_get_group_pins(pctldev, group, &pins, &npins);
+ ret = pinctrl_generic_get_group_pins(pctldev, group, &pins, &npins);
if (ret)
return ret;
for (i = 0; i < npins; i++) {
@@ -897,40 +827,6 @@ static void pcs_remove_function(struct pcs_device *pcs,
}

/**
- * pcs_add_pingroup() - add a pingroup to the pingroup list
- * @pcs: pcs driver instance
- * @np: device node of the mux entry
- * @name: name of the pingroup
- * @gpins: array of the pins that belong to the group
- * @ngpins: number of pins in the group
- */
-static int pcs_add_pingroup(struct pcs_device *pcs,
- struct device_node *np,
- const char *name,
- int *gpins,
- int ngpins)
-{
- struct pcs_pingroup *pingroup;
-
- pingroup = devm_kzalloc(pcs->dev, sizeof(*pingroup), GFP_KERNEL);
- if (!pingroup)
- return -ENOMEM;
-
- pingroup->name = name;
- pingroup->np = np;
- pingroup->gpins = gpins;
- pingroup->ngpins = ngpins;
-
- mutex_lock(&pcs->mutex);
- list_add_tail(&pingroup->node, &pcs->pingroups);
- radix_tree_insert(&pcs->pgtree, pcs->ngroups, pingroup);
- pcs->ngroups++;
- mutex_unlock(&pcs->mutex);
-
- return 0;
-}
-
-/**
* pcs_get_pin_by_offset() - get a pin index based on the register offset
* @pcs: pcs driver instance
* @offset: register offset from the base
@@ -1100,10 +996,9 @@ static int pcs_parse_pinconf(struct pcs_device *pcs, struct device_node *np,
return 0;
}

-static void pcs_free_pingroups(struct pcs_device *pcs);
-
/**
* smux_parse_one_pinctrl_entry() - parses a device tree mux entry
+ * @pctldev: pin controller device
* @pcs: pinctrl driver instance
* @np: device node of the mux entry
* @map: map entry
@@ -1186,7 +1081,7 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
goto free_pins;
}

- res = pcs_add_pingroup(pcs, np, np->name, pins, found);
+ res = pinctrl_generic_add_group(pcs->pctl, np->name, pins, found, pcs);
if (res < 0)
goto free_function;

@@ -1205,7 +1100,7 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
return 0;

free_pingroups:
- pcs_free_pingroups(pcs);
+ pinctrl_generic_remove_last_group(pcs->pctl);
*num_maps = 1;
free_function:
pcs_remove_function(pcs, function);
@@ -1320,7 +1215,7 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
goto free_pins;
}

- res = pcs_add_pingroup(pcs, np, np->name, pins, found);
+ res = pinctrl_generic_add_group(pcs->pctl, np->name, pins, found, pcs);
if (res < 0)
goto free_function;

@@ -1337,7 +1232,7 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
return 0;

free_pingroups:
- pcs_free_pingroups(pcs);
+ pinctrl_generic_remove_last_group(pcs->pctl);
*num_maps = 1;
free_function:
pcs_remove_function(pcs, function);
@@ -1436,33 +1331,6 @@ static void pcs_free_funcs(struct pcs_device *pcs)
}

/**
- * pcs_free_pingroups() - free memory used by pingroups
- * @pcs: pcs driver instance
- */
-static void pcs_free_pingroups(struct pcs_device *pcs)
-{
- struct list_head *pos, *tmp;
- int i;
-
- mutex_lock(&pcs->mutex);
- for (i = 0; i < pcs->ngroups; i++) {
- struct pcs_pingroup *pingroup;
-
- pingroup = radix_tree_lookup(&pcs->pgtree, i);
- if (!pingroup)
- continue;
- radix_tree_delete(&pcs->pgtree, i);
- }
- list_for_each_safe(pos, tmp, &pcs->pingroups) {
- struct pcs_pingroup *pingroup;
-
- pingroup = list_entry(pos, struct pcs_pingroup, node);
- list_del(&pingroup->node);
- }
- mutex_unlock(&pcs->mutex);
-}
-
-/**
* pcs_irq_free() - free interrupt
* @pcs: pcs driver instance
*/
@@ -1491,7 +1359,7 @@ static void pcs_free_resources(struct pcs_device *pcs)
pcs_irq_free(pcs);
pinctrl_unregister(pcs->pctl);
pcs_free_funcs(pcs);
- pcs_free_pingroups(pcs);
+
#if IS_BUILTIN(CONFIG_PINCTRL_SINGLE)
if (pcs->missing_nr_pinctrl_cells)
of_remove_property(pcs->np, pcs->missing_nr_pinctrl_cells);
@@ -1885,7 +1753,6 @@ static int pcs_probe(struct platform_device *pdev)
pcs->np = np;
raw_spin_lock_init(&pcs->lock);
mutex_init(&pcs->mutex);
- INIT_LIST_HEAD(&pcs->pingroups);
INIT_LIST_HEAD(&pcs->functions);
INIT_LIST_HEAD(&pcs->gpiofuncs);
soc = match->data;
@@ -1947,7 +1814,6 @@ static int pcs_probe(struct platform_device *pdev)
return -ENODEV;
}

- INIT_RADIX_TREE(&pcs->pgtree, GFP_KERNEL);
INIT_RADIX_TREE(&pcs->ftree, GFP_KERNEL);
platform_set_drvdata(pdev, pcs);

--
2.11.0

2016-12-27 17:20:43

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 5/5] pinctrl: single: Use generic pinmux helpers for managing functions

We can now drop the driver specific code for managing functions.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/pinctrl/Kconfig | 2 +-
drivers/pinctrl/pinctrl-single.c | 134 ++++++---------------------------------
2 files changed, 19 insertions(+), 117 deletions(-)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -167,7 +167,7 @@ config PINCTRL_SINGLE
tristate "One-register-per-pin type device tree based pinctrl driver"
depends on OF
select GENERIC_PINCTRL
- select PINMUX
+ select GENERIC_PINMUX
select GENERIC_PINCONF
help
This selects the device tree based generic pinctrl driver.
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -33,6 +33,7 @@
#include "core.h"
#include "devicetree.h"
#include "pinconf.h"
+#include "pinmux.h"

#define DRIVER_NAME "pinctrl-single"
#define PCS_OFF_DISABLED ~0U
@@ -160,13 +161,10 @@ struct pcs_soc_data {
* @bits_per_mux: number of bits per mux
* @bits_per_pin: number of bits per pin
* @pins: physical pins on the SoC
- * @ftree: function index radix tree
- * @functions: list of functions
* @gpiofuncs: list of gpio functions
* @irqs: list of interrupt registers
* @chip: chip container for this instance
* @domain: IRQ domain for this instance
- * @nfuncs: number of functions
* @desc: pin controller descriptor
* @read: register read function to use
* @write: register write function to use
@@ -194,13 +192,10 @@ struct pcs_device {
bool bits_per_mux;
unsigned bits_per_pin;
struct pcs_data pins;
- struct radix_tree_root ftree;
- struct list_head functions;
struct list_head gpiofuncs;
struct list_head irqs;
struct irq_chip chip;
struct irq_domain *domain;
- unsigned nfuncs;
struct pinctrl_desc desc;
unsigned (*read)(void __iomem *reg);
void (*write)(unsigned val, void __iomem *reg);
@@ -307,59 +302,13 @@ static const struct pinctrl_ops pcs_pinctrl_ops = {
.dt_free_map = pcs_dt_free_map,
};

-static int pcs_get_functions_count(struct pinctrl_dev *pctldev)
-{
- struct pcs_device *pcs;
-
- pcs = pinctrl_dev_get_drvdata(pctldev);
-
- return pcs->nfuncs;
-}
-
-static const char *pcs_get_function_name(struct pinctrl_dev *pctldev,
- unsigned fselector)
-{
- struct pcs_device *pcs;
- struct pcs_function *func;
-
- pcs = pinctrl_dev_get_drvdata(pctldev);
- func = radix_tree_lookup(&pcs->ftree, fselector);
- if (!func) {
- dev_err(pcs->dev, "%s could not find function%i\n",
- __func__, fselector);
- return NULL;
- }
-
- return func->name;
-}
-
-static int pcs_get_function_groups(struct pinctrl_dev *pctldev,
- unsigned fselector,
- const char * const **groups,
- unsigned * const ngroups)
-{
- struct pcs_device *pcs;
- struct pcs_function *func;
-
- pcs = pinctrl_dev_get_drvdata(pctldev);
- func = radix_tree_lookup(&pcs->ftree, fselector);
- if (!func) {
- dev_err(pcs->dev, "%s could not find function%i\n",
- __func__, fselector);
- return -EINVAL;
- }
- *groups = func->pgnames;
- *ngroups = func->npgnames;
-
- return 0;
-}
-
static int pcs_get_function(struct pinctrl_dev *pctldev, unsigned pin,
struct pcs_function **func)
{
struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
struct pin_desc *pdesc = pin_desc_get(pctldev, pin);
const struct pinctrl_setting_mux *setting;
+ struct function_desc *function;
unsigned fselector;

/* If pin is not described in DTS & enabled, mux_setting is NULL. */
@@ -367,7 +316,8 @@ static int pcs_get_function(struct pinctrl_dev *pctldev, unsigned pin,
if (!setting)
return -ENOTSUPP;
fselector = setting->func;
- *func = radix_tree_lookup(&pcs->ftree, fselector);
+ function = pinmux_generic_get_function(pctldev, fselector);
+ *func = function->data;
if (!(*func)) {
dev_err(pcs->dev, "%s could not find function%i\n",
__func__, fselector);
@@ -380,6 +330,7 @@ static int pcs_set_mux(struct pinctrl_dev *pctldev, unsigned fselector,
unsigned group)
{
struct pcs_device *pcs;
+ struct function_desc *function;
struct pcs_function *func;
int i;

@@ -387,7 +338,8 @@ static int pcs_set_mux(struct pinctrl_dev *pctldev, unsigned fselector,
/* If function mask is null, needn't enable it. */
if (!pcs->fmask)
return 0;
- func = radix_tree_lookup(&pcs->ftree, fselector);
+ function = pinmux_generic_get_function(pctldev, fselector);
+ func = function->data;
if (!func)
return -EINVAL;

@@ -445,9 +397,9 @@ static int pcs_request_gpio(struct pinctrl_dev *pctldev,
}

static const struct pinmux_ops pcs_pinmux_ops = {
- .get_functions_count = pcs_get_functions_count,
- .get_function_name = pcs_get_function_name,
- .get_function_groups = pcs_get_function_groups,
+ .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 = pcs_set_mux,
.gpio_request_enable = pcs_request_gpio,
};
@@ -789,43 +741,24 @@ static struct pcs_function *pcs_add_function(struct pcs_device *pcs,
unsigned npgnames)
{
struct pcs_function *function;
+ int res;

function = devm_kzalloc(pcs->dev, sizeof(*function), GFP_KERNEL);
if (!function)
return NULL;

- function->name = name;
function->vals = vals;
function->nvals = nvals;
- function->pgnames = pgnames;
- function->npgnames = npgnames;

- mutex_lock(&pcs->mutex);
- list_add_tail(&function->node, &pcs->functions);
- radix_tree_insert(&pcs->ftree, pcs->nfuncs, function);
- pcs->nfuncs++;
- mutex_unlock(&pcs->mutex);
+ res = pinmux_generic_add_function(pcs->pctl, name,
+ pgnames, npgnames,
+ function);
+ if (res)
+ return NULL;

return function;
}

-static void pcs_remove_function(struct pcs_device *pcs,
- struct pcs_function *function)
-{
- int i;
-
- mutex_lock(&pcs->mutex);
- for (i = 0; i < pcs->nfuncs; i++) {
- struct pcs_function *found;
-
- found = radix_tree_lookup(&pcs->ftree, i);
- if (found == function)
- radix_tree_delete(&pcs->ftree, i);
- }
- list_del(&function->node);
- mutex_unlock(&pcs->mutex);
-}
-
/**
* pcs_get_pin_by_offset() - get a pin index based on the register offset
* @pcs: pcs driver instance
@@ -1103,7 +1036,7 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
pinctrl_generic_remove_last_group(pcs->pctl);
*num_maps = 1;
free_function:
- pcs_remove_function(pcs, function);
+ pinmux_generic_remove_last_function(pcs->pctl);

free_pins:
devm_kfree(pcs->dev, pins);
@@ -1235,8 +1168,7 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
pinctrl_generic_remove_last_group(pcs->pctl);
*num_maps = 1;
free_function:
- pcs_remove_function(pcs, function);
-
+ pinmux_generic_remove_last_function(pcs->pctl);
free_pins:
devm_kfree(pcs->dev, pins);

@@ -1304,33 +1236,6 @@ static int pcs_dt_node_to_map(struct pinctrl_dev *pctldev,
}

/**
- * pcs_free_funcs() - free memory used by functions
- * @pcs: pcs driver instance
- */
-static void pcs_free_funcs(struct pcs_device *pcs)
-{
- struct list_head *pos, *tmp;
- int i;
-
- mutex_lock(&pcs->mutex);
- for (i = 0; i < pcs->nfuncs; i++) {
- struct pcs_function *func;
-
- func = radix_tree_lookup(&pcs->ftree, i);
- if (!func)
- continue;
- radix_tree_delete(&pcs->ftree, i);
- }
- list_for_each_safe(pos, tmp, &pcs->functions) {
- struct pcs_function *function;
-
- function = list_entry(pos, struct pcs_function, node);
- list_del(&function->node);
- }
- mutex_unlock(&pcs->mutex);
-}
-
-/**
* pcs_irq_free() - free interrupt
* @pcs: pcs driver instance
*/
@@ -1358,7 +1263,6 @@ static void pcs_free_resources(struct pcs_device *pcs)
{
pcs_irq_free(pcs);
pinctrl_unregister(pcs->pctl);
- pcs_free_funcs(pcs);

#if IS_BUILTIN(CONFIG_PINCTRL_SINGLE)
if (pcs->missing_nr_pinctrl_cells)
@@ -1753,7 +1657,6 @@ static int pcs_probe(struct platform_device *pdev)
pcs->np = np;
raw_spin_lock_init(&pcs->lock);
mutex_init(&pcs->mutex);
- INIT_LIST_HEAD(&pcs->functions);
INIT_LIST_HEAD(&pcs->gpiofuncs);
soc = match->data;
pcs->flags = soc->flags;
@@ -1814,7 +1717,6 @@ static int pcs_probe(struct platform_device *pdev)
return -ENODEV;
}

- INIT_RADIX_TREE(&pcs->ftree, GFP_KERNEL);
platform_set_drvdata(pdev, pcs);

switch (pcs->width) {
--
2.11.0

2016-12-27 17:21:22

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 2/5] pinctrl: core: Add generic pinctrl functions for managing groups

We can add generic helpers for pin group handling for cases where the pin
controller driver does not need to use static arrays.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/pinctrl/Kconfig | 3 +
drivers/pinctrl/core.c | 178 ++++++++++++++++++++++++++++++++++++++++++++++++
drivers/pinctrl/core.h | 47 +++++++++++++
3 files changed, 228 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -8,6 +8,9 @@ config PINCTRL
menu "Pin controllers"
depends on PINCTRL

+config GENERIC_PINCTRL
+ bool
+
config PINMUX
bool "Support pin multiplexing controllers" if COMPILE_TEST

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -540,6 +540,182 @@ void pinctrl_remove_gpio_range(struct pinctrl_dev *pctldev,
}
EXPORT_SYMBOL_GPL(pinctrl_remove_gpio_range);

+#ifdef CONFIG_GENERIC_PINCTRL
+
+/**
+ * pinctrl_generic_get_group_count() - returns the number of pin groups
+ * @pctldev: pin controller device
+ */
+int pinctrl_generic_get_group_count(struct pinctrl_dev *pctldev)
+{
+ return pctldev->num_groups;
+}
+EXPORT_SYMBOL_GPL(pinctrl_generic_get_group_count);
+
+/**
+ * pinctrl_generic_get_group_name() - returns the name of a pin group
+ * @pctldev: pin controller device
+ * @selector: group number
+ */
+const char *pinctrl_generic_get_group_name(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ struct group_desc *group;
+
+ group = radix_tree_lookup(&pctldev->pin_group_tree,
+ selector);
+ if (!group)
+ return NULL;
+
+ return group->name;
+}
+EXPORT_SYMBOL_GPL(pinctrl_generic_get_group_name);
+
+/**
+ * pinctrl_generic_get_group_pins() - gets the pin group pins
+ * @pctldev: pin controller device
+ * @selector: group number
+ * @pins: pins in the group
+ * @num_pins: number of pins in the group
+ */
+int pinctrl_generic_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int selector,
+ const unsigned int **pins,
+ unsigned int *num_pins)
+{
+ struct group_desc *group;
+
+ group = radix_tree_lookup(&pctldev->pin_group_tree,
+ selector);
+ if (!group) {
+ dev_err(pctldev->dev, "%s could not find pingroup%i\n",
+ __func__, selector);
+ return -EINVAL;
+ }
+
+ *pins = group->pins;
+ *num_pins = group->num_pins;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_generic_get_group_pins);
+
+/**
+ * pinctrl_generic_get_group() - returns a pin group based on the number
+ * @pctldev: pin controller device
+ * @gselector: group number
+ */
+struct group_desc *pinctrl_generic_get_group(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ struct group_desc *group;
+
+ group = radix_tree_lookup(&pctldev->pin_group_tree,
+ selector);
+ if (!group)
+ return NULL;
+
+ return group;
+}
+EXPORT_SYMBOL_GPL(pinctrl_generic_get_group);
+
+/**
+ * pinctrl_generic_add_group() - adds a new pin group
+ * @pctldev: pin controller device
+ * @name: name of the pin group
+ * @pins: pins in the pin group
+ * @num_pins: number of pins in the pin group
+ * @data: pin controller driver specific data
+ *
+ * Note that the caller must take care of locking.
+ */
+int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name,
+ int *pins, int num_pins, void *data)
+{
+ struct group_desc *group;
+
+ group = devm_kzalloc(pctldev->dev, sizeof(*group), GFP_KERNEL);
+ if (!group)
+ return -ENOMEM;
+
+ group->name = name;
+ group->pins = pins;
+ group->num_pins = num_pins;
+ group->data = data;
+
+ radix_tree_insert(&pctldev->pin_group_tree, pctldev->num_groups,
+ group);
+
+ pctldev->num_groups++;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_generic_add_group);
+
+/**
+ * pinctrl_generic_remove_group() - removes a numbered pin group
+ * @pctldev: pin controller device
+ * @selector: group number
+ *
+ * Note that the caller must take care of locking.
+ */
+int pinctrl_generic_remove_group(struct pinctrl_dev *pctldev,
+ unsigned int selector)
+{
+ struct group_desc *group;
+
+ group = radix_tree_lookup(&pctldev->pin_group_tree,
+ selector);
+ if (!group)
+ return -ENOENT;
+
+ radix_tree_delete(&pctldev->pin_group_tree, selector);
+ devm_kfree(pctldev->dev, group);
+
+ pctldev->num_groups--;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_generic_remove_group);
+
+/**
+ * pinctrl_generic_free_groups() - removes all pin groups
+ * @pctldev: pin controller device
+ *
+ * Note that the caller must take care of locking.
+ */
+static void pinctrl_generic_free_groups(struct pinctrl_dev *pctldev)
+{
+ struct radix_tree_iter iter;
+ struct group_desc *group;
+ unsigned long *indices;
+ void **slot;
+ int i = 0;
+
+ indices = devm_kzalloc(pctldev->dev, sizeof(*indices) *
+ pctldev->num_groups, GFP_KERNEL);
+ if (!indices)
+ return;
+
+ radix_tree_for_each_slot(slot, &pctldev->pin_group_tree, &iter, 0)
+ indices[i++] = iter.index;
+
+ for (i = 0; i < pctldev->num_groups; i++) {
+ group = radix_tree_lookup(&pctldev->pin_group_tree,
+ indices[i]);
+ radix_tree_delete(&pctldev->pin_group_tree, indices[i]);
+ devm_kfree(pctldev->dev, group);
+ }
+
+ pctldev->num_groups = 0;
+}
+
+#else
+static inline void pinctrl_generic_free_groups(struct pinctrl_dev *pctldev)
+{
+}
+#endif /* CONFIG_GENERIC_PINCTRL */
+
/**
* pinctrl_get_group_selector() - returns the group selector for a group
* @pctldev: the pin controller handling the group
@@ -1811,6 +1987,7 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
pctldev->desc = pctldesc;
pctldev->driver_data = driver_data;
INIT_RADIX_TREE(&pctldev->pin_desc_tree, GFP_KERNEL);
+ INIT_RADIX_TREE(&pctldev->pin_group_tree, GFP_KERNEL);
INIT_LIST_HEAD(&pctldev->gpio_ranges);
INIT_DELAYED_WORK(&pctldev->late_init, pinctrl_late_init);
pctldev->dev = dev;
@@ -1885,6 +2062,7 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
mutex_lock(&pctldev->mutex);
/* TODO: check that no pinmuxes are still active? */
list_del(&pctldev->node);
+ pinctrl_generic_free_groups(pctldev);
/* Destroy descriptor tree */
pinctrl_free_pindescs(pctldev, pctldev->desc->pins,
pctldev->desc->npins);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -24,6 +24,8 @@ struct pinctrl_gpio_range;
* controller
* @pin_desc_tree: each pin descriptor for this pin controller is stored in
* this radix tree
+ * @pin_group_tree: optionally each pin group can be stored in this radix tree
+ * @num_groups: optionally number of groups can be kept here
* @gpio_ranges: a list of GPIO ranges that is handled by this pin controller,
* ranges are added to this list at runtime
* @dev: the device entry for this pin controller
@@ -41,6 +43,8 @@ struct pinctrl_dev {
struct list_head node;
struct pinctrl_desc *desc;
struct radix_tree_root pin_desc_tree;
+ struct radix_tree_root pin_group_tree;
+ unsigned int num_groups;
struct list_head gpio_ranges;
struct device *dev;
struct module *owner;
@@ -162,6 +166,20 @@ struct pin_desc {
};

/**
+ * struct group_desc - generic pin group descriptor
+ * @name: name of the pin group
+ * @pins: array of pins that belong to the group
+ * @num_pins: number of pins in the group
+ * @data: pin controller driver specific data
+ */
+struct group_desc {
+ const char *name;
+ int *pins;
+ int num_pins;
+ void *data;
+};
+
+/**
* struct pinctrl_maps - a list item containing part of the mapping table
* @node: mapping table list node
* @maps: array of mapping table entries
@@ -173,6 +191,35 @@ struct pinctrl_maps {
unsigned num_maps;
};

+#ifdef CONFIG_GENERIC_PINCTRL
+
+int pinctrl_generic_get_group_count(struct pinctrl_dev *pctldev);
+
+const char *pinctrl_generic_get_group_name(struct pinctrl_dev *pctldev,
+ unsigned int group_selector);
+
+int pinctrl_generic_get_group_pins(struct pinctrl_dev *pctldev,
+ unsigned int group_selector,
+ const unsigned int **pins,
+ unsigned int *npins);
+
+struct group_desc *pinctrl_generic_get_group(struct pinctrl_dev *pctldev,
+ unsigned int group_selector);
+
+int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name,
+ int *gpins, int ngpins, void *data);
+
+int pinctrl_generic_remove_group(struct pinctrl_dev *pctldev,
+ unsigned int group_selector);
+
+static inline int
+pinctrl_generic_remove_last_group(struct pinctrl_dev *pctldev)
+{
+ return pinctrl_generic_remove_group(pctldev, pctldev->num_groups - 1);
+}
+
+#endif /* CONFIG_GENERIC_PINCTRL */
+
struct pinctrl_dev *get_pinctrl_dev_from_devname(const char *dev_name);
struct pinctrl_dev *get_pinctrl_dev_from_of_node(struct device_node *np);
int pin_get_from_name(struct pinctrl_dev *pctldev, const char *name);
--
2.11.0

2016-12-27 17:21:32

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 1/5] pinctrl: core: Use delayed work for hogs

Having the pin control framework call pin controller functions
before it's probe has finished is not nice as the pin controller
device driver does not yet have struct pinctrl_dev handle.

Let's fix this issue by adding deferred work for late init. This is
needed to be able to add pinctrl generic helper functions that expect
to know struct pinctrl_dev handle. Note that we now need to call
create_pinctrl() directly as we don't want to add the pin controller
to the list of controllers until the hogs are claimed. We also need
to pass the pinctrl_dev to the device tree parser functions as they
otherwise won't find the right controller at this point.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/pinctrl/core.c | 90 ++++++++++++++++++++++++++++----------------
drivers/pinctrl/core.h | 2 +
drivers/pinctrl/devicetree.c | 28 +++++++++++---
drivers/pinctrl/devicetree.h | 12 +++++-
4 files changed, 93 insertions(+), 39 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -720,7 +720,8 @@ static struct pinctrl_state *create_state(struct pinctrl *p,
return state;
}

-static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)
+static int add_setting(struct pinctrl *p, struct pinctrl_dev *pctldev,
+ struct pinctrl_map const *map)
{
struct pinctrl_state *state;
struct pinctrl_setting *setting;
@@ -744,7 +745,11 @@ static int add_setting(struct pinctrl *p, struct pinctrl_map const *map)

setting->type = map->type;

- setting->pctldev = get_pinctrl_dev_from_devname(map->ctrl_dev_name);
+ if (pctldev)
+ setting->pctldev = pctldev;
+ else
+ setting->pctldev =
+ get_pinctrl_dev_from_devname(map->ctrl_dev_name);
if (setting->pctldev == NULL) {
kfree(setting);
/* Do not defer probing of hogs (circular loop) */
@@ -800,7 +805,8 @@ static struct pinctrl *find_pinctrl(struct device *dev)

static void pinctrl_free(struct pinctrl *p, bool inlist);

-static struct pinctrl *create_pinctrl(struct device *dev)
+static struct pinctrl *create_pinctrl(struct device *dev,
+ struct pinctrl_dev *pctldev)
{
struct pinctrl *p;
const char *devname;
@@ -823,7 +829,7 @@ static struct pinctrl *create_pinctrl(struct device *dev)
INIT_LIST_HEAD(&p->states);
INIT_LIST_HEAD(&p->dt_maps);

- ret = pinctrl_dt_to_map(p);
+ ret = pinctrl_dt_to_map(p, pctldev);
if (ret < 0) {
kfree(p);
return ERR_PTR(ret);
@@ -838,7 +844,7 @@ static struct pinctrl *create_pinctrl(struct device *dev)
if (strcmp(map->dev_name, devname))
continue;

- ret = add_setting(p, map);
+ ret = add_setting(p, pctldev, map);
/*
* At this point the adding of a setting may:
*
@@ -899,7 +905,7 @@ struct pinctrl *pinctrl_get(struct device *dev)
return p;
}

- return create_pinctrl(dev);
+ return create_pinctrl(dev, NULL);
}
EXPORT_SYMBOL_GPL(pinctrl_get);

@@ -1738,6 +1744,46 @@ static int pinctrl_check_ops(struct pinctrl_dev *pctldev)
}

/**
+ * pinctrl_late_init() - finish pin controller device registration
+ * @work: work struct
+ */
+static void pinctrl_late_init(struct work_struct *work)
+{
+ struct pinctrl_dev *pctldev;
+
+ pctldev = container_of(work, struct pinctrl_dev, late_init.work);
+
+ pctldev->p = create_pinctrl(pctldev->dev, pctldev);
+ if (!IS_ERR(pctldev->p)) {
+ kref_get(&pctldev->p->users);
+ pctldev->hog_default =
+ pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
+ if (IS_ERR(pctldev->hog_default)) {
+ dev_dbg(pctldev->dev,
+ "failed to lookup the default state\n");
+ } else {
+ if (pinctrl_select_state(pctldev->p,
+ pctldev->hog_default))
+ dev_err(pctldev->dev,
+ "failed to select default state\n");
+ }
+
+ pctldev->hog_sleep =
+ pinctrl_lookup_state(pctldev->p,
+ PINCTRL_STATE_SLEEP);
+ if (IS_ERR(pctldev->hog_sleep))
+ dev_dbg(pctldev->dev,
+ "failed to lookup the sleep state\n");
+ }
+
+ mutex_lock(&pinctrldev_list_mutex);
+ list_add_tail(&pctldev->node, &pinctrldev_list);
+ mutex_unlock(&pinctrldev_list_mutex);
+
+ pinctrl_init_device_debugfs(pctldev);
+}
+
+/**
* pinctrl_register() - register a pin controller device
* @pctldesc: descriptor for this pin controller
* @dev: parent device for this pin controller
@@ -1766,6 +1812,7 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
pctldev->driver_data = driver_data;
INIT_RADIX_TREE(&pctldev->pin_desc_tree, GFP_KERNEL);
INIT_LIST_HEAD(&pctldev->gpio_ranges);
+ INIT_DELAYED_WORK(&pctldev->late_init, pinctrl_late_init);
pctldev->dev = dev;
mutex_init(&pctldev->mutex);

@@ -1800,32 +1847,10 @@ struct pinctrl_dev *pinctrl_register(struct pinctrl_desc *pctldesc,
goto out_err;
}

- mutex_lock(&pinctrldev_list_mutex);
- list_add_tail(&pctldev->node, &pinctrldev_list);
- mutex_unlock(&pinctrldev_list_mutex);
-
- pctldev->p = pinctrl_get(pctldev->dev);
-
- if (!IS_ERR(pctldev->p)) {
- pctldev->hog_default =
- pinctrl_lookup_state(pctldev->p, PINCTRL_STATE_DEFAULT);
- if (IS_ERR(pctldev->hog_default)) {
- dev_dbg(dev, "failed to lookup the default state\n");
- } else {
- if (pinctrl_select_state(pctldev->p,
- pctldev->hog_default))
- dev_err(dev,
- "failed to select default state\n");
- }
-
- pctldev->hog_sleep =
- pinctrl_lookup_state(pctldev->p,
- PINCTRL_STATE_SLEEP);
- if (IS_ERR(pctldev->hog_sleep))
- dev_dbg(dev, "failed to lookup the sleep state\n");
- }
-
- pinctrl_init_device_debugfs(pctldev);
+ if (pinctrl_dt_has_hogs(pctldev))
+ schedule_delayed_work(&pctldev->late_init, 0);
+ else
+ pinctrl_late_init(&pctldev->late_init.work);

return pctldev;

@@ -1848,6 +1873,7 @@ void pinctrl_unregister(struct pinctrl_dev *pctldev)
if (pctldev == NULL)
return;

+ cancel_delayed_work_sync(&pctldev->late_init);
mutex_lock(&pctldev->mutex);
pinctrl_remove_device_debugfs(pctldev);
mutex_unlock(&pctldev->mutex);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -33,6 +33,7 @@ struct pinctrl_gpio_range;
* @p: result of pinctrl_get() for this device
* @hog_default: default state for pins hogged by this device
* @hog_sleep: sleep state for pins hogged by this device
+ * @late_init: delayed work for pin controller to finish registration
* @mutex: mutex taken on each pin controller specific action
* @device_root: debugfs root for this device
*/
@@ -47,6 +48,7 @@ struct pinctrl_dev {
struct pinctrl *p;
struct pinctrl_state *hog_default;
struct pinctrl_state *hog_sleep;
+ struct delayed_work late_init;
struct mutex mutex;
#ifdef CONFIG_DEBUG_FS
struct dentry *device_root;
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -100,11 +100,12 @@ struct pinctrl_dev *of_pinctrl_get(struct device_node *np)
return get_pinctrl_dev_from_of_node(np);
}

-static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
+static int dt_to_map_one_config(struct pinctrl *p,
+ struct pinctrl_dev *pctldev,
+ const char *statename,
struct device_node *np_config)
{
struct device_node *np_pctldev;
- struct pinctrl_dev *pctldev;
const struct pinctrl_ops *ops;
int ret;
struct pinctrl_map *map;
@@ -121,7 +122,8 @@ static int dt_to_map_one_config(struct pinctrl *p, const char *statename,
/* OK let's just assume this will appear later then */
return -EPROBE_DEFER;
}
- pctldev = get_pinctrl_dev_from_of_node(np_pctldev);
+ if (!pctldev)
+ pctldev = get_pinctrl_dev_from_of_node(np_pctldev);
if (pctldev)
break;
/* Do not defer probing of hogs (circular loop) */
@@ -166,7 +168,22 @@ static int dt_remember_dummy_state(struct pinctrl *p, const char *statename)
return dt_remember_or_free_map(p, statename, NULL, map, 1);
}

-int pinctrl_dt_to_map(struct pinctrl *p)
+bool pinctrl_dt_has_hogs(struct pinctrl_dev *pctldev)
+{
+ struct device_node *np;
+ struct property *prop;
+ int size;
+
+ np = pctldev->dev->of_node;
+ if (!np)
+ return false;
+
+ prop = of_find_property(np, "pinctrl-0", &size);
+
+ return prop ? true : false;
+}
+
+int pinctrl_dt_to_map(struct pinctrl *p, struct pinctrl_dev *pctldev)
{
struct device_node *np = p->dev->of_node;
int state, ret;
@@ -233,7 +250,8 @@ int pinctrl_dt_to_map(struct pinctrl *p)
}

/* Parse the node */
- ret = dt_to_map_one_config(p, statename, np_config);
+ ret = dt_to_map_one_config(p, pctldev, statename,
+ np_config);
of_node_put(np_config);
if (ret < 0)
goto err;
diff --git a/drivers/pinctrl/devicetree.h b/drivers/pinctrl/devicetree.h
--- a/drivers/pinctrl/devicetree.h
+++ b/drivers/pinctrl/devicetree.h
@@ -20,8 +20,10 @@ struct of_phandle_args;

#ifdef CONFIG_OF

+bool pinctrl_dt_has_hogs(struct pinctrl_dev *pctldev);
+
void pinctrl_dt_free_maps(struct pinctrl *p);
-int pinctrl_dt_to_map(struct pinctrl *p);
+int pinctrl_dt_to_map(struct pinctrl *p, struct pinctrl_dev *pctldev);

int pinctrl_count_index_with_args(const struct device_node *np,
const char *list_name);
@@ -32,7 +34,13 @@ int pinctrl_parse_index_with_args(const struct device_node *np,

#else

-static inline int pinctrl_dt_to_map(struct pinctrl *p)
+static inline bool pinctrl_dt_has_hogs(struct pinctrl_dev *pctldev)
+{
+ return false;
+}
+
+static inline int pinctrl_dt_to_map(struct pinctrl *p,
+ struct pinctrl_dev *pctldev)
{
return 0;
}
--
2.11.0

2016-12-30 13:46:34

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/5] pinctrl: core: Use delayed work for hogs

On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren <[email protected]> wrote:

> Having the pin control framework call pin controller functions
> before it's probe has finished is not nice as the pin controller
> device driver does not yet have struct pinctrl_dev handle.
> Let's fix this issue by adding deferred work for late init. This is
> needed to be able to add pinctrl generic helper functions that expect
> to know struct pinctrl_dev handle. Note that we now need to call
> create_pinctrl() directly as we don't want to add the pin controller
> to the list of controllers until the hogs are claimed. We also need
> to pass the pinctrl_dev to the device tree parser functions as they
> otherwise won't find the right controller at this point.
>
> Signed-off-by: Tony Lindgren <[email protected]>

Patch applied.

I felt the code path needed some extra comments so I sent those
as a separate patch.

Yours,
Linus Walleij

2016-12-30 14:09:46

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 3/5] pinctrl: core: Add generic pinctrl functions for managing groups

On Tue, Dec 27, 2016 at 6:20 PM, Tony Lindgren <[email protected]> wrote:

> We can add generic helpers for function handling for cases where the pin
> controller driver does not need to use static arrays.
>
> Signed-off-by: Tony Lindgren <[email protected]>

Patch applied!

> +config GENERIC_PINMUX
> + bool
> + select PINMUX

I applied the first that had GENERIC_PINCTRL instead,

Then I went in and renamed this GENERIC_PINCTRL_GROUPS
since that is what it's all about.

Sent out as separate patch!

Yours,
Linus Walleij

2016-12-30 14:12:08

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 2/5] pinctrl: core: Add generic pinctrl functions for managing groups

On Tue, Dec 27, 2016 at 6:20 PM, Tony Lindgren <[email protected]> wrote:

> We can add generic helpers for pin group handling for cases where the pin
> controller driver does not need to use static arrays.
>
> Signed-off-by: Tony Lindgren <[email protected]>

Patch applied.

> +config GENERIC_PINCTRL
> + bool

Then I renamed *this* to GENERIC_PINCTRL_GROUPS.
(Not the other patch, I got confused because gmail threads the
second and third patch together, sorry.)

Let's see if I manage to rebase the next patch on this.

Yours,
Linus Walleij

2016-12-30 14:28:46

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 3/5] pinctrl: core: Add generic pinctrl functions for managing groups

On Tue, Dec 27, 2016 at 6:20 PM, Tony Lindgren <[email protected]> wrote:

> We can add generic helpers for function handling for cases where the pin
> controller driver does not need to use static arrays.
>
> Signed-off-by: Tony Lindgren <[email protected]>

Patch applied.

> +config GENERIC_PINMUX
> + bool
> + select PINMUX

I renamed this GENERIC_PINMUX_FUNCTIONS

> + INIT_RADIX_TREE(&pctldev->pin_function_tree, GFP_KERNEL);

#ifdefed this

> + struct radix_tree_root pin_function_tree;
> unsigned int num_groups;
> + unsigned int num_functions;

#ifdefed these

> /**
> + * struct function_desc - generic function descriptor
> + * @name: name of the function
> + * @group_names: array of pin group names
> + * @num_group_names: number of pin group names
> + * @data: pin controller driver specific data
> + */
> +struct function_desc {
> + const char *name;
> + const char **group_names;
> + int num_group_names;
> + void *data;
> +};

And moved this into pinmux.h

Yours,
Linus Walleij

2016-12-30 14:32:27

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 4/5] pinctrl: single: Use generic pinctrl helpers for managing groups

On Tue, Dec 27, 2016 at 6:20 PM, Tony Lindgren <[email protected]> wrote:

> We can now drop the driver specific code for managing groups.
>
> Signed-off-by: Tony Lindgren <[email protected]>

Patch applied!

> + select GENERIC_PINCTRL

Replaced this with GENERIC_PINCTRL_GROUPS

Yours,
Linus Walleij

2016-12-30 14:35:25

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 5/5] pinctrl: single: Use generic pinmux helpers for managing functions

On Tue, Dec 27, 2016 at 6:20 PM, Tony Lindgren <[email protected]> wrote:

> We can now drop the driver specific code for managing functions.
>
> Signed-off-by: Tony Lindgren <[email protected]>

Patch applied.

> - select PINMUX
> + select GENERIC_PINMUX

Replaced this with GENERIC_PINMUX_FUNCTIONS

Yours,
Linus Walleij

2016-12-30 14:40:04

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCHv2 0/5] Add generic pinctrl helpers for managing groups and function

On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren <[email protected]> wrote:

> Here are some changes to add generic helpers for managing pinctrl groups and
> functions.

I applied it, screwed around with it and pushed to the build servers to see
if it survived.

I really like the look of this and I hope lots of driver start to use it.

Gary, I just applied your radix patches for i.MX, can you look if you can
use the GENERIC_PINCTRL_GROUPS and GENERIC_PINMUX_FUCNTIONS
that Tony invented and that I just merged to my devel branch in the
pinctrl tree?

It seems the i.MX could just reuse this right off and slim down quite a bit
but the devil is in the details.

Yours,
Linus Walleij

2016-12-30 15:43:27

by Gary Bisson

[permalink] [raw]
Subject: Re: [PATCHv2 0/5] Add generic pinctrl helpers for managing groups and function

Hi Linus,

On Fri, Dec 30, 2016 at 3:39 PM, Linus Walleij <[email protected]> wrote:
> On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren <[email protected]> wrote:
>
>> Here are some changes to add generic helpers for managing pinctrl groups and
>> functions.
>
> I applied it, screwed around with it and pushed to the build servers to see
> if it survived.
>
> I really like the look of this and I hope lots of driver start to use it.
>
> Gary, I just applied your radix patches for i.MX, can you look if you can
> use the GENERIC_PINCTRL_GROUPS and GENERIC_PINMUX_FUCNTIONS
> that Tony invented and that I just merged to my devel branch in the
> pinctrl tree?

Yes I will have a look. It does sound like a good idea. I'll share my
findings beginning of next week.

Regards,
Gary

2016-12-30 15:57:52

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/5] pinctrl: core: Add generic pinctrl functions for managing groups

* Linus Walleij <[email protected]> [161230 06:12]:
> On Tue, Dec 27, 2016 at 6:20 PM, Tony Lindgren <[email protected]> wrote:
>
> > We can add generic helpers for pin group handling for cases where the pin
> > controller driver does not need to use static arrays.
> >
> > Signed-off-by: Tony Lindgren <[email protected]>
>
> Patch applied.
>
> > +config GENERIC_PINCTRL
> > + bool
>
> Then I renamed *this* to GENERIC_PINCTRL_GROUPS.
> (Not the other patch, I got confused because gmail threads the
> second and third patch together, sorry.)

Yeah OK nice, the renames make sense to me.

> Let's see if I manage to rebase the next patch on this.

I'll do some testing with your devel branch today to make
sure things still work for me.

Regards,

Tony

2016-12-30 16:00:03

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCHv2 0/5] Add generic pinctrl helpers for managing groups and function

* Gary Bisson <[email protected]> [161230 07:43]:
> Hi Linus,
>
> On Fri, Dec 30, 2016 at 3:39 PM, Linus Walleij <[email protected]> wrote:
> > On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren <[email protected]> wrote:
> >
> >> Here are some changes to add generic helpers for managing pinctrl groups and
> >> functions.
> >
> > I applied it, screwed around with it and pushed to the build servers to see
> > if it survived.
> >
> > I really like the look of this and I hope lots of driver start to use it.
> >
> > Gary, I just applied your radix patches for i.MX, can you look if you can
> > use the GENERIC_PINCTRL_GROUPS and GENERIC_PINMUX_FUCNTIONS
> > that Tony invented and that I just merged to my devel branch in the
> > pinctrl tree?
>
> Yes I will have a look. It does sound like a good idea. I'll share my
> findings beginning of next week.

OK great. Note that we may be able to come up also with a generic
iterator function for the node_to_map functions so maybe see if
you come up with some ideas for that while experimenting :)

Tony