* [PATCHv2 0/4] Generic #pinctrl-cells and and pinctrl_parse_index_with_args
@ 2016-11-03 16:35 Tony Lindgren
2016-11-03 16:35 ` [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells " Tony Lindgren
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Tony Lindgren @ 2016-11-03 16:35 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, linux-omap
Hi all,
Here a repost of some pinctrl changes to introduce #pinctrl-cells and
a generic parser pinctrl_parse_index_with_args that the drivers can
optionally use.
Regards,
Tony
Changes from v1:
- Update based on comments from Linus Walleij to not pass
#pinctrl-cells as an argument and improve the binding
documentation
Tony Lindgren (4):
pinctrl: Introduce generic #pinctrl-cells and
pinctrl_parse_index_with_args
pinctrl: single: Use generic parser and #pinctrl-cells for
pinctrl-single,pins
pinctrl: single: Use generic parser and #pinctrl-cells for
pinctrl-single,bits
ARM: dts: Add #pinctrl-cells for pinctrl-single instances
.../bindings/pinctrl/pinctrl-bindings.txt | 44 +++++-
.../devicetree/bindings/pinctrl/pinctrl-single.txt | 3 +
arch/arm/boot/dts/am33xx.dtsi | 2 +
arch/arm/boot/dts/am3517.dtsi | 1 +
arch/arm/boot/dts/am4372.dtsi | 1 +
arch/arm/boot/dts/da850.dtsi | 1 +
arch/arm/boot/dts/dm814x.dtsi | 1 +
arch/arm/boot/dts/dm816x.dtsi | 2 +
arch/arm/boot/dts/dra7.dtsi | 1 +
arch/arm/boot/dts/hi3620.dtsi | 2 +
arch/arm/boot/dts/keystone-k2g.dtsi | 1 +
arch/arm/boot/dts/keystone-k2l.dtsi | 1 +
arch/arm/boot/dts/omap2420.dtsi | 2 +
arch/arm/boot/dts/omap2430.dtsi | 2 +
arch/arm/boot/dts/omap3.dtsi | 2 +
arch/arm/boot/dts/omap34xx.dtsi | 1 +
arch/arm/boot/dts/omap36xx.dtsi | 1 +
arch/arm/boot/dts/omap4.dtsi | 2 +
arch/arm/boot/dts/omap5.dtsi | 2 +
arch/arm/boot/dts/pxa3xx.dtsi | 1 +
arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 3 +
drivers/pinctrl/devicetree.c | 144 +++++++++++++++++++
drivers/pinctrl/devicetree.h | 21 +++
drivers/pinctrl/pinctrl-single.c | 159 +++++++++++++++------
24 files changed, 357 insertions(+), 43 deletions(-)
--
2.10.2
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-11-03 16:35 [PATCHv2 0/4] Generic #pinctrl-cells and and pinctrl_parse_index_with_args Tony Lindgren
@ 2016-11-03 16:35 ` Tony Lindgren
2016-11-03 20:28 ` kbuild test robot
` (2 more replies)
2016-11-03 16:35 ` [PATCH 2/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,pins Tony Lindgren
` (2 subsequent siblings)
3 siblings, 3 replies; 22+ messages in thread
From: Tony Lindgren @ 2016-11-03 16:35 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, linux-omap
Introduce #pinctrl-cells helper binding and generic helper functions
pinctrl_count_index_with_args() and pinctrl_parse_index_with_args().
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
.../bindings/pinctrl/pinctrl-bindings.txt | 44 ++++++-
drivers/pinctrl/devicetree.c | 144 +++++++++++++++++++++
drivers/pinctrl/devicetree.h | 21 +++
3 files changed, 208 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -97,6 +97,11 @@ For example:
};
== Pin controller devices ==
+Required properties: See the pin controller driver specific documentation
+
+Optional properties:
+#pinctrl-cells: Number of pin control cells in addition to the index within the
+ pin controller device instance
Pin controller devices should contain the pin configuration nodes that client
devices reference.
@@ -119,7 +124,8 @@ For example:
The contents of each of those pin configuration child nodes is defined
entirely by the binding for the individual pin controller device. There
-exists no common standard for this content.
+exists no common standard for this content. The pinctrl framework only
+provides generic helper bindings that the pin controller driver can use.
The pin configuration nodes need not be direct children of the pin controller
device; they may be grandchildren, for example. Whether this is legal, and
@@ -156,6 +162,42 @@ state_2_node_a {
pins = "mfio29", "mfio30";
};
+Optionally an altenative binding can be used if more suitable depending on the
+pin controller hardware. For hardaware where there is a large number of identical
+pin controller instances, naming each pin and function can easily become
+unmaintainable. This is especially the case if the same controller is used for
+different pins and functions depending on the SoC revision and packaging.
+
+For cases like this, the pin controller driver may use pinctrl-pin-array helper
+binding with a hardware based index and a number of pin configuration values:
+
+pincontroller {
+ ... /* Standard DT properties for the device itself elided */
+ #pinctrl-cells = <2>;
+
+ state_0_node_a {
+ pinctrl-pin-array = <
+ 0 A_DELAY_PS(0) G_DELAY_PS(120)
+ 4 A_DELAY_PS(0) G_DELAY_PS(360)
+ ...
+ >;
+ };
+ ...
+};
+
+Above #pinctrl-cells specifies the number of value cells in addition to the
+index of the registers. This is similar to the interrupts-extended binding with
+one exception. There is no need to specify the phandle for each entry as that
+is already known as the defined pins are always children of the pin controller
+node. Further having the phandle pointing to another pin controller would not
+currently work as the pinctrl framework uses named modes to group pins for each
+pin control device.
+
+The index for pinctrl-pin-array must relate to the hardware for the pinctrl
+registers, and must not be a virtual index of pin instances. The reason for
+this is to avoid mapping of the index in the dts files and the pin controller
+driver as it can change.
+
== Generic pin configuration node content ==
Many data items that are represented in a pin configuration node are common
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -253,3 +253,147 @@ int pinctrl_dt_to_map(struct pinctrl *p)
pinctrl_dt_free_maps(p);
return ret;
}
+
+/*
+ * For pinctrl binding, typically #pinctrl-cells is for the pin controller
+ * device, so either parent or grandparent. See pinctrl-bindings.txt.
+ */
+static int pinctrl_find_cells_size(const struct device_node *np)
+{
+ const char *cells_name = "#pinctrl-cells";
+ int cells_size, error;
+
+ error = of_property_read_u32(np->parent, cells_name, &cells_size);
+ if (error) {
+ error = of_property_read_u32(np->parent->parent,
+ cells_name, &cells_size);
+ if (error)
+ return -ENOENT;
+ }
+
+ return cells_size;
+}
+
+/**
+ * pinctrl_get_list_and_count - Gets the list and it's cell size and number
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ * @list: pointer for the list found
+ * @cells_size: pointer for the cell size found
+ * @nr_elements: pointer for the number of elements found
+ *
+ * Typically np is a single pinctrl entry containing the list.
+ */
+static int pinctrl_get_list_and_count(const struct device_node *np,
+ const char *list_name,
+ const __be32 **list,
+ int *cells_size,
+ int *nr_elements)
+{
+ int size;
+
+ *cells_size = 0;
+ *nr_elements = 0;
+
+ *list = of_get_property(np, list_name, &size);
+ if (!*list)
+ return -ENOENT;
+
+ *cells_size = pinctrl_find_cells_size(np);
+ if (*cells_size < 0)
+ return -ENOENT;
+
+ /* First element is always the index within the pinctrl device */
+ *nr_elements = (size / sizeof(**list)) / (*cells_size + 1);
+
+ return 0;
+}
+
+/**
+ * pinctrl_count_index_with_args - Count number of elements in a pinctrl entry
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ *
+ * Counts the number of elements in a pinctrl array consisting of an index
+ * within the controller and a number of u32 entries specified for each
+ * entry. Note that device_node is always for the parent pin controller device.
+ */
+int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name)
+{
+ const __be32 *list;
+ int size, nr_cells, error;
+
+ error = pinctrl_get_list_and_count(np, list_name, &list,
+ &nr_cells, &size);
+ if (error)
+ return error;
+
+ return size;
+}
+EXPORT_SYMBOL_GPL(pinctrl_count_index_with_args);
+
+/**
+ * pinctrl_copy_args - Populates of_phandle_args based on index
+ * @np: pointer to device node with the property
+ * @list: pointer to a list with the elements
+ * @index: entry within the list of elements
+ * @nr_cells: number of cells in the list
+ * @nr_elem: number of elements for each entry in the list
+ * @out_args: returned values
+ *
+ * Populates the of_phandle_args based on the index in the list.
+ */
+static int pinctrl_copy_args(const struct device_node *np,
+ const __be32 *list,
+ int index, int nr_cells, int nr_elem,
+ struct of_phandle_args *out_args)
+{
+ int i;
+
+ memset(out_args, 0, sizeof(*out_args));
+ out_args->np = (struct device_node *)np;
+ out_args->args_count = nr_cells + 1;
+
+ if (index >= nr_elem)
+ return -EINVAL;
+
+ list += index * (nr_cells + 1);
+
+ for (i = 0; i < nr_cells + 1; i++)
+ out_args->args[i] = be32_to_cpup(list++);
+
+ return 0;
+}
+
+/**
+ * pinctrl_parse_index_with_args - Find a node pointed by index in a list
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ * @index: index within the list
+ * @out_arts: entries in the list pointed by index
+ *
+ * Finds the selected element in a pinctrl array consisting of an index
+ * within the controller and a number of u32 entries specified for each
+ * entry. Note that device_node is always for the parent pin controller device.
+ */
+int pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, int index,
+ struct of_phandle_args *out_args)
+{
+ const __be32 *list;
+ int nr_elem, nr_cells, error;
+
+ error = pinctrl_get_list_and_count(np, list_name, &list,
+ &nr_cells, &nr_elem);
+ if (error || !nr_cells)
+ return error;
+
+ error = pinctrl_copy_args(np, list, index, nr_cells, nr_elem,
+ out_args);
+ if (error)
+ return error;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_parse_index_with_args);
diff --git a/drivers/pinctrl/devicetree.h b/drivers/pinctrl/devicetree.h
--- a/drivers/pinctrl/devicetree.h
+++ b/drivers/pinctrl/devicetree.h
@@ -21,6 +21,13 @@
void pinctrl_dt_free_maps(struct pinctrl *p);
int pinctrl_dt_to_map(struct pinctrl *p);
+int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name);
+
+int pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, int index,
+ struct of_phandle_args *out_args);
+
#else
static inline int pinctrl_dt_to_map(struct pinctrl *p)
@@ -32,4 +39,18 @@ static inline void pinctrl_dt_free_maps(struct pinctrl *p)
{
}
+static inline int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name)
+{
+ return -ENODEV;
+}
+
+static inline int
+pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, int index,
+ struct of_phandle_args *out_args)
+{
+ return -ENODEV;
+}
+
#endif
--
2.10.2
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,pins
2016-11-03 16:35 [PATCHv2 0/4] Generic #pinctrl-cells and and pinctrl_parse_index_with_args Tony Lindgren
2016-11-03 16:35 ` [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells " Tony Lindgren
@ 2016-11-03 16:35 ` Tony Lindgren
2016-11-04 21:52 ` Linus Walleij
2016-11-03 16:35 ` [PATCH 3/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,bits Tony Lindgren
2016-11-03 16:35 ` [PATCH 4/4] ARM: dts: Add #pinctrl-cells for pinctrl-single instances Tony Lindgren
3 siblings, 1 reply; 22+ messages in thread
From: Tony Lindgren @ 2016-11-03 16:35 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, linux-omap
We can now use generic parser. To support the legacy binding without
#pinctrl-cells, add pcs_quirk_missing_pinctrl_cells() and warn about
missing #pinctrl-cells.
Let's also update the documentation for struct pcs_soc_data while at it
as that seems to be out of date.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/pinctrl/pinctrl-single.c | 111 ++++++++++++++++++++++++++++++++-------
1 file changed, 93 insertions(+), 18 deletions(-)
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
@@ -31,10 +31,10 @@
#include <linux/platform_data/pinctrl-single.h>
#include "core.h"
+#include "devicetree.h"
#include "pinconf.h"
#define DRIVER_NAME "pinctrl-single"
-#define PCS_MUX_PINS_NAME "pinctrl-single,pins"
#define PCS_MUX_BITS_NAME "pinctrl-single,bits"
#define PCS_OFF_DISABLED ~0U
@@ -162,8 +162,11 @@ struct pcs_soc_data {
* @base: virtual address of the controller
* @size: size of the ioremapped area
* @dev: device entry
+ * @np: device tree node
* @pctl: pin controller device
* @flags: mask of PCS_FEAT_xxx values
+ * @missing_nr_pinctrl_cells: for legacy binding, may go away
+ * @socdata: soc specific data
* @lock: spinlock for register access
* @mutex: mutex protecting the lists
* @width: bits per mux register
@@ -171,7 +174,8 @@ struct pcs_soc_data {
* @fshift: function register shift
* @foff: value to turn mux off
* @fmax: max number of functions in fmask
- * @bits_per_pin:number of bits per pin
+ * @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
@@ -192,11 +196,13 @@ struct pcs_device {
void __iomem *base;
unsigned size;
struct device *dev;
+ struct device_node *np;
struct pinctrl_dev *pctl;
unsigned flags;
#define PCS_QUIRK_SHARED_IRQ (1 << 2)
#define PCS_FEAT_IRQ (1 << 1)
#define PCS_FEAT_PINCONF (1 << 0)
+ struct property *missing_nr_pinctrl_cells;
struct pcs_soc_data socdata;
raw_spinlock_t lock;
struct mutex mutex;
@@ -1125,20 +1131,14 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
unsigned *num_maps,
const char **pgnames)
{
+ const char *name = "pinctrl-single,pins";
struct pcs_func_vals *vals;
- const __be32 *mux;
- int size, rows, *pins, index = 0, found = 0, res = -ENOMEM;
+ int rows, *pins, found = 0, res = -ENOMEM, i;
struct pcs_function *function;
- mux = of_get_property(np, PCS_MUX_PINS_NAME, &size);
- if ((!mux) || (size < sizeof(*mux) * 2)) {
- dev_err(pcs->dev, "bad data for mux %s\n",
- np->name);
- return -EINVAL;
- }
-
- size /= sizeof(*mux); /* Number of elements in array */
- rows = size / 2;
+ rows = pinctrl_count_index_with_args(np, name);
+ if (rows == -EINVAL)
+ return rows;
vals = devm_kzalloc(pcs->dev, sizeof(*vals) * rows, GFP_KERNEL);
if (!vals)
@@ -1148,14 +1148,28 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
if (!pins)
goto free_vals;
- while (index < size) {
- unsigned offset, val;
+ for (i = 0; i < rows; i++) {
+ struct of_phandle_args pinctrl_spec;
+ unsigned int offset;
int pin;
- offset = be32_to_cpup(mux + index++);
- val = be32_to_cpup(mux + index++);
+ res = pinctrl_parse_index_with_args(np, name, i, &pinctrl_spec);
+ if (res)
+ return res;
+
+ if (pinctrl_spec.args_count < 2) {
+ dev_err(pcs->dev, "invalid args_count for spec: %i\n",
+ pinctrl_spec.args_count);
+ break;
+ }
+
+ /* Index plus one value cell */
+ offset = pinctrl_spec.args[0];
vals[found].reg = pcs->base + offset;
- vals[found].val = val;
+ vals[found].val = pinctrl_spec.args[1];
+
+ dev_dbg(pcs->dev, "%s index: 0x%x value: 0x%x\n",
+ pinctrl_spec.np->name, offset, pinctrl_spec.args[1]);
pin = pcs_get_pin_by_offset(pcs, offset);
if (pin < 0) {
@@ -1473,6 +1487,10 @@ static void pcs_free_resources(struct pcs_device *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);
+#endif
}
static const struct of_device_id pcs_of_match[];
@@ -1790,6 +1808,55 @@ static int pinctrl_single_resume(struct platform_device *pdev)
}
#endif
+/**
+ * pcs_quirk_missing_pinctrl_cells - handle legacy binding
+ * @pcs: pinctrl driver instance
+ * @np: device tree node
+ * @cells: number of cells
+ *
+ * Handle legacy binding with no #pinctrl-cells. This should be
+ * always two pinctrl-single,bit-per-mux and one for others.
+ * At some point we may want to consider removing this.
+ */
+static int pcs_quirk_missing_pinctrl_cells(struct pcs_device *pcs,
+ struct device_node *np,
+ int cells)
+{
+ struct property *p;
+ const char *name = "#pinctrl-cells";
+ int error;
+ u32 val;
+
+ error = of_property_read_u32(np, name, &val);
+ if (!error)
+ return 0;
+
+ dev_warn(pcs->dev, "please update dts to use %s = <%i>\n",
+ name, cells);
+
+ p = devm_kzalloc(pcs->dev, sizeof(*p), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ p->length = sizeof(__be32);
+ p->value = devm_kzalloc(pcs->dev, sizeof(__be32), GFP_KERNEL);
+ if (!p->value)
+ return -ENOMEM;
+ *(__be32 *)p->value = cpu_to_be32(cells);
+
+ p->name = devm_kstrdup(pcs->dev, name, GFP_KERNEL);
+ if (!p->name)
+ return -ENOMEM;
+
+ pcs->missing_nr_pinctrl_cells = p;
+
+#if IS_BUILTIN(CONFIG_PINCTRL_SINGLE)
+ error = of_add_property(np, pcs->missing_nr_pinctrl_cells);
+#endif
+
+ return error;
+}
+
static int pcs_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -1810,6 +1877,7 @@ static int pcs_probe(struct platform_device *pdev)
return -ENOMEM;
}
pcs->dev = &pdev->dev;
+ pcs->np = np;
raw_spin_lock_init(&pcs->lock);
mutex_init(&pcs->mutex);
INIT_LIST_HEAD(&pcs->pingroups);
@@ -1846,6 +1914,13 @@ static int pcs_probe(struct platform_device *pdev)
pcs->bits_per_mux = of_property_read_bool(np,
"pinctrl-single,bit-per-mux");
+ ret = pcs_quirk_missing_pinctrl_cells(pcs, np,
+ pcs->bits_per_mux ? 2 : 1);
+ if (ret) {
+ dev_err(&pdev->dev, "unable to patch #pinctrl-cells\n");
+
+ return ret;
+ }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
--
2.10.2
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,bits
2016-11-03 16:35 [PATCHv2 0/4] Generic #pinctrl-cells and and pinctrl_parse_index_with_args Tony Lindgren
2016-11-03 16:35 ` [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells " Tony Lindgren
2016-11-03 16:35 ` [PATCH 2/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,pins Tony Lindgren
@ 2016-11-03 16:35 ` Tony Lindgren
2016-11-04 21:54 ` Linus Walleij
2016-11-03 16:35 ` [PATCH 4/4] ARM: dts: Add #pinctrl-cells for pinctrl-single instances Tony Lindgren
3 siblings, 1 reply; 22+ messages in thread
From: Tony Lindgren @ 2016-11-03 16:35 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, linux-omap
We can now use generic parser and keep things compatible with the
old binding.
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
drivers/pinctrl/pinctrl-single.c | 48 ++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 24 deletions(-)
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
@@ -35,7 +35,6 @@
#include "pinconf.h"
#define DRIVER_NAME "pinctrl-single"
-#define PCS_MUX_BITS_NAME "pinctrl-single,bits"
#define PCS_OFF_DISABLED ~0U
/**
@@ -1219,36 +1218,22 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
return res;
}
-#define PARAMS_FOR_BITS_PER_MUX 3
-
static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
struct device_node *np,
struct pinctrl_map **map,
unsigned *num_maps,
const char **pgnames)
{
+ const char *name = "pinctrl-single,pins";
struct pcs_func_vals *vals;
- const __be32 *mux;
- int size, rows, *pins, index = 0, found = 0, res = -ENOMEM;
+ int rows, *pins, found = 0, res = -ENOMEM, i;
int npins_in_row;
struct pcs_function *function;
- mux = of_get_property(np, PCS_MUX_BITS_NAME, &size);
-
- if (!mux) {
- dev_err(pcs->dev, "no valid property for %s\n", np->name);
- return -EINVAL;
- }
-
- if (size < (sizeof(*mux) * PARAMS_FOR_BITS_PER_MUX)) {
- dev_err(pcs->dev, "bad data for %s\n", np->name);
- return -EINVAL;
- }
-
- /* Number of elements in array */
- size /= sizeof(*mux);
+ rows = pinctrl_count_index_with_args(np, name);
+ if (rows == -EINVAL)
+ return rows;
- rows = size / PARAMS_FOR_BITS_PER_MUX;
npins_in_row = pcs->width / pcs->bits_per_pin;
vals = devm_kzalloc(pcs->dev, sizeof(*vals) * rows * npins_in_row,
@@ -1261,15 +1246,30 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
if (!pins)
goto free_vals;
- while (index < size) {
+ for (i = 0; i < rows; i++) {
+ struct of_phandle_args pinctrl_spec;
unsigned offset, val;
unsigned mask, bit_pos, val_pos, mask_pos, submask;
unsigned pin_num_from_lsb;
int pin;
- offset = be32_to_cpup(mux + index++);
- val = be32_to_cpup(mux + index++);
- mask = be32_to_cpup(mux + index++);
+ res = pinctrl_parse_index_with_args(np, name, i, &pinctrl_spec);
+ if (res)
+ return res;
+
+ if (pinctrl_spec.args_count < 3) {
+ dev_err(pcs->dev, "invalid args_count for spec: %i\n",
+ pinctrl_spec.args_count);
+ break;
+ }
+
+ /* Index plus two value cells */
+ offset = pinctrl_spec.args[0];
+ val = pinctrl_spec.args[1];
+ mask = pinctrl_spec.args[2];
+
+ dev_dbg(pcs->dev, "%s index: 0x%x value: 0x%x mask: 0x%x\n",
+ pinctrl_spec.np->name, offset, val, mask);
/* Parse pins in each row from LSB */
while (mask) {
--
2.10.2
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/4] ARM: dts: Add #pinctrl-cells for pinctrl-single instances
2016-11-03 16:35 [PATCHv2 0/4] Generic #pinctrl-cells and and pinctrl_parse_index_with_args Tony Lindgren
` (2 preceding siblings ...)
2016-11-03 16:35 ` [PATCH 3/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,bits Tony Lindgren
@ 2016-11-03 16:35 ` Tony Lindgren
2016-11-04 21:55 ` Linus Walleij
3 siblings, 1 reply; 22+ messages in thread
From: Tony Lindgren @ 2016-11-03 16:35 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, linux-omap
Drivers using pinctrl-single,pins have #pinctrl-cells = <1>, while
pinctrl-single,bits need #pinctrl-cells = <2>.
Note that this patch can be optionally applied separately from the
driver changes as the driver supports also the legacy binding without
#pinctrl-cells.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt | 3 +++
arch/arm/boot/dts/am33xx.dtsi | 2 ++
arch/arm/boot/dts/am3517.dtsi | 1 +
arch/arm/boot/dts/am4372.dtsi | 1 +
arch/arm/boot/dts/da850.dtsi | 1 +
arch/arm/boot/dts/dm814x.dtsi | 1 +
arch/arm/boot/dts/dm816x.dtsi | 2 ++
arch/arm/boot/dts/dra7.dtsi | 1 +
arch/arm/boot/dts/hi3620.dtsi | 2 ++
arch/arm/boot/dts/keystone-k2g.dtsi | 1 +
arch/arm/boot/dts/keystone-k2l.dtsi | 1 +
arch/arm/boot/dts/omap2420.dtsi | 2 ++
arch/arm/boot/dts/omap2430.dtsi | 2 ++
arch/arm/boot/dts/omap3.dtsi | 2 ++
arch/arm/boot/dts/omap34xx.dtsi | 1 +
arch/arm/boot/dts/omap36xx.dtsi | 1 +
arch/arm/boot/dts/omap4.dtsi | 2 ++
arch/arm/boot/dts/omap5.dtsi | 2 ++
arch/arm/boot/dts/pxa3xx.dtsi | 1 +
arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 3 +++
20 files changed, 32 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-single.txt
@@ -7,6 +7,9 @@ Required properties:
- reg : offset and length of the register set for the mux registers
+- #pinctrl-cells : number of cells in addition to the index, set to 1
+ for pinctrl-single,pins and 2 for pinctrl-single,bits
+
- pinctrl-single,register-width : pinmux register access width in bits
- pinctrl-single,function-mask : mask of allowed pinmux function bits
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -130,6 +130,7 @@
reg = <0x210000 0x2000>;
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
ranges = <0 0x210000 0x2000>;
am33xx_pinmux: pinmux@800 {
@@ -137,6 +138,7 @@
reg = <0x800 0x238>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
pinctrl-single,register-width = <32>;
pinctrl-single,function-mask = <0x7f>;
};
diff --git a/arch/arm/boot/dts/am3517.dtsi b/arch/arm/boot/dts/am3517.dtsi
--- a/arch/arm/boot/dts/am3517.dtsi
+++ b/arch/arm/boot/dts/am3517.dtsi
@@ -66,6 +66,7 @@
reg = <0x480025d8 0x24>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <16>;
diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -189,6 +189,7 @@
reg = <0x800 0x31c>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <32>;
diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -36,6 +36,7 @@
reg = <0x14120 0x50>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <2>;
pinctrl-single,bit-per-mux;
pinctrl-single,register-width = <32>;
pinctrl-single,function-mask = <0xf>;
diff --git a/arch/arm/boot/dts/dm814x.dtsi b/arch/arm/boot/dts/dm814x.dtsi
--- a/arch/arm/boot/dts/dm814x.dtsi
+++ b/arch/arm/boot/dts/dm814x.dtsi
@@ -374,6 +374,7 @@
reg = <0x800 0x438>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
pinctrl-single,register-width = <32>;
pinctrl-single,function-mask = <0x307ff>;
};
diff --git a/arch/arm/boot/dts/dm816x.dtsi b/arch/arm/boot/dts/dm816x.dtsi
--- a/arch/arm/boot/dts/dm816x.dtsi
+++ b/arch/arm/boot/dts/dm816x.dtsi
@@ -83,6 +83,7 @@
reg = <0x48140000 0x21000>;
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
ranges = <0 0x48140000 0x21000>;
dm816x_pinmux: pinmux@800 {
@@ -90,6 +91,7 @@
reg = <0x800 0x50a>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
pinctrl-single,register-width = <16>;
pinctrl-single,function-mask = <0xf>;
};
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -171,6 +171,7 @@
reg = <0x1400 0x0468>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <32>;
diff --git a/arch/arm/boot/dts/hi3620.dtsi b/arch/arm/boot/dts/hi3620.dtsi
--- a/arch/arm/boot/dts/hi3620.dtsi
+++ b/arch/arm/boot/dts/hi3620.dtsi
@@ -537,6 +537,7 @@
reg = <0x803000 0x188>;
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
#gpio-range-cells = <3>;
ranges;
@@ -558,6 +559,7 @@
reg = <0x803800 0x2dc>;
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
ranges;
pinctrl-single,register-width = <32>;
diff --git a/arch/arm/boot/dts/keystone-k2g.dtsi b/arch/arm/boot/dts/keystone-k2g.dtsi
--- a/arch/arm/boot/dts/keystone-k2g.dtsi
+++ b/arch/arm/boot/dts/keystone-k2g.dtsi
@@ -72,6 +72,7 @@
soc {
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
compatible = "ti,keystone","simple-bus";
ranges = <0x0 0x0 0x0 0xc0000000>;
dma-ranges = <0x80000000 0x8 0x00000000 0x80000000>;
diff --git a/arch/arm/boot/dts/keystone-k2l.dtsi b/arch/arm/boot/dts/keystone-k2l.dtsi
--- a/arch/arm/boot/dts/keystone-k2l.dtsi
+++ b/arch/arm/boot/dts/keystone-k2l.dtsi
@@ -59,6 +59,7 @@
reg = <0x02620690 0xc>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <2>;
pinctrl-single,bit-per-mux;
pinctrl-single,register-width = <32>;
pinctrl-single,function-mask = <0x1>;
diff --git a/arch/arm/boot/dts/omap2420.dtsi b/arch/arm/boot/dts/omap2420.dtsi
--- a/arch/arm/boot/dts/omap2420.dtsi
+++ b/arch/arm/boot/dts/omap2420.dtsi
@@ -38,6 +38,7 @@
reg = <0x0 0x1000>;
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
ranges = <0 0x0 0x1000>;
omap2420_pmx: pinmux@30 {
@@ -46,6 +47,7 @@
reg = <0x30 0x0113>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
pinctrl-single,register-width = <8>;
pinctrl-single,function-mask = <0x3f>;
};
diff --git a/arch/arm/boot/dts/omap2430.dtsi b/arch/arm/boot/dts/omap2430.dtsi
--- a/arch/arm/boot/dts/omap2430.dtsi
+++ b/arch/arm/boot/dts/omap2430.dtsi
@@ -38,6 +38,7 @@
reg = <0x2000 0x1000>;
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
ranges = <0 0x2000 0x1000>;
omap2430_pmx: pinmux@30 {
@@ -46,6 +47,7 @@
reg = <0x30 0x0154>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
pinctrl-single,register-width = <8>;
pinctrl-single,function-mask = <0x3f>;
};
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -106,6 +106,7 @@
reg = <0x30 0x238>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <16>;
@@ -145,6 +146,7 @@
reg = <0xa00 0x5c>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <16>;
diff --git a/arch/arm/boot/dts/omap34xx.dtsi b/arch/arm/boot/dts/omap34xx.dtsi
--- a/arch/arm/boot/dts/omap34xx.dtsi
+++ b/arch/arm/boot/dts/omap34xx.dtsi
@@ -34,6 +34,7 @@
reg = <0x480025d8 0x24>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <16>;
diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
--- a/arch/arm/boot/dts/omap36xx.dtsi
+++ b/arch/arm/boot/dts/omap36xx.dtsi
@@ -66,6 +66,7 @@
reg = <0x480025a0 0x5c>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <16>;
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -184,6 +184,7 @@
reg = <0x40 0x0196>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <16>;
@@ -256,6 +257,7 @@
reg = <0x1e040 0x0038>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <16>;
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -171,6 +171,7 @@
reg = <0x40 0x01b6>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <16>;
@@ -270,6 +271,7 @@
reg = <0xc840 0x003c>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
#interrupt-cells = <1>;
interrupt-controller;
pinctrl-single,register-width = <16>;
diff --git a/arch/arm/boot/dts/pxa3xx.dtsi b/arch/arm/boot/dts/pxa3xx.dtsi
--- a/arch/arm/boot/dts/pxa3xx.dtsi
+++ b/arch/arm/boot/dts/pxa3xx.dtsi
@@ -138,6 +138,7 @@
reg = <0x40e10000 0xffff>;
#address-cells = <1>;
#size-cells = <0>;
+ #pinctrl-cells = <1>;
pinctrl-single,register-width = <32>;
pinctrl-single,function-mask = <0x7>;
};
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
--- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
@@ -364,6 +364,7 @@
reg = <0x0 0xf7010000 0x0 0x27c>;
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
#gpio-range-cells = <3>;
pinctrl-single,register-width = <32>;
pinctrl-single,function-mask = <7>;
@@ -402,6 +403,7 @@
reg = <0x0 0xf7010800 0x0 0x28c>;
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
pinctrl-single,register-width = <32>;
};
@@ -410,6 +412,7 @@
reg = <0x0 0xf8001800 0x0 0x78>;
#address-cells = <1>;
#size-cells = <1>;
+ #pinctrl-cells = <1>;
pinctrl-single,register-width = <32>;
};
--
2.10.2
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-11-03 16:35 ` [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells " Tony Lindgren
@ 2016-11-03 20:28 ` kbuild test robot
2016-11-03 20:48 ` Tony Lindgren
2016-11-04 6:03 ` kbuild test robot
2016-11-04 21:50 ` Linus Walleij
2 siblings, 1 reply; 22+ messages in thread
From: kbuild test robot @ 2016-11-03 20:28 UTC (permalink / raw)
To: Tony Lindgren
Cc: kbuild-all, Linus Walleij, Jon Hunter, Mark Rutland, Rob Herring,
Grygorii Strashko, Nishanth Menon, linux-gpio, devicetree,
linux-kernel, linux-omap
[-- Attachment #1: Type: text/plain, Size: 2379 bytes --]
Hi Tony,
[auto build test WARNING on pinctrl/for-next]
[also build test WARNING on v4.9-rc3 next-20161028]
[cannot apply to robh/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Tony-Lindgren/Generic-pinctrl-cells-and-and-pinctrl_parse_index_with_args/20161104-004449
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git for-next
config: mips-generic_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips
All warnings (new ones prefixed by >>):
In file included from drivers/pinctrl/core.c:36:0:
>> drivers/pinctrl/devicetree.h:29:14: warning: 'struct of_phandle_args' declared inside parameter list will not be visible outside of this definition or declaration
struct of_phandle_args *out_args);
^~~~~~~~~~~~~~~
vim +29 drivers/pinctrl/devicetree.h
13 * more details.
14 *
15 * You should have received a copy of the GNU General Public License
16 * along with this program. If not, see <http://www.gnu.org/licenses/>.
17 */
18
19 #ifdef CONFIG_OF
20
21 void pinctrl_dt_free_maps(struct pinctrl *p);
22 int pinctrl_dt_to_map(struct pinctrl *p);
23
24 int pinctrl_count_index_with_args(const struct device_node *np,
25 const char *list_name);
26
27 int pinctrl_parse_index_with_args(const struct device_node *np,
28 const char *list_name, int index,
> 29 struct of_phandle_args *out_args);
30
31 #else
32
33 static inline int pinctrl_dt_to_map(struct pinctrl *p)
34 {
35 return 0;
36 }
37
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 10604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-11-03 20:28 ` kbuild test robot
@ 2016-11-03 20:48 ` Tony Lindgren
0 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2016-11-03 20:48 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Linus Walleij, Jon Hunter, Mark Rutland, Rob Herring,
Grygorii Strashko, Nishanth Menon, linux-gpio, devicetree,
linux-kernel, linux-omap
* kbuild test robot <lkp@intel.com> [161103 13:29]:
> In file included from drivers/pinctrl/core.c:36:0:
> >> drivers/pinctrl/devicetree.h:29:14: warning: 'struct of_phandle_args' declared inside parameter list will not be visible outside of this definition or declaratio
Hmm maybe we should just include of.h in core.c?
Regards,
Tony
8< ----------------------
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -23,6 +23,7 @@
#include <linux/list.h>
#include <linux/sysfs.h>
#include <linux/debugfs.h>
+#include <linux/of.h>
#include <linux/seq_file.h>
#include <linux/pinctrl/consumer.h>
#include <linux/pinctrl/pinctrl.h>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-11-03 16:35 ` [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells " Tony Lindgren
2016-11-03 20:28 ` kbuild test robot
@ 2016-11-04 6:03 ` kbuild test robot
2016-11-04 21:50 ` Linus Walleij
2 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2016-11-04 6:03 UTC (permalink / raw)
To: Tony Lindgren
Cc: kbuild-all, Linus Walleij, Jon Hunter, Mark Rutland, Rob Herring,
Grygorii Strashko, Nishanth Menon, linux-gpio, devicetree,
linux-kernel, linux-omap
[-- Attachment #1: Type: text/plain, Size: 2027 bytes --]
Hi Tony,
[auto build test WARNING on pinctrl/for-next]
[also build test WARNING on v4.9-rc3 next-20161028]
[cannot apply to robh/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tony-Lindgren/Generic-pinctrl-cells-and-and-pinctrl_parse_index_with_args/20161104-004449
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git for-next
config: x86_64-randconfig-i0-201644 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from drivers/pinctrl/core.c:36:0:
>> drivers/pinctrl/devicetree.h:29:14: warning: 'struct of_phandle_args' declared inside parameter list
struct of_phandle_args *out_args);
^
>> drivers/pinctrl/devicetree.h:29:14: warning: its scope is only this definition or declaration, which is probably not what you want
vim +29 drivers/pinctrl/devicetree.h
13 * more details.
14 *
15 * You should have received a copy of the GNU General Public License
16 * along with this program. If not, see <http://www.gnu.org/licenses/>.
17 */
18
19 #ifdef CONFIG_OF
20
21 void pinctrl_dt_free_maps(struct pinctrl *p);
22 int pinctrl_dt_to_map(struct pinctrl *p);
23
24 int pinctrl_count_index_with_args(const struct device_node *np,
25 const char *list_name);
26
27 int pinctrl_parse_index_with_args(const struct device_node *np,
28 const char *list_name, int index,
> 29 struct of_phandle_args *out_args);
30
31 #else
32
33 static inline int pinctrl_dt_to_map(struct pinctrl *p)
34 {
35 return 0;
36 }
37
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24875 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-11-03 16:35 ` [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells " Tony Lindgren
2016-11-03 20:28 ` kbuild test robot
2016-11-04 6:03 ` kbuild test robot
@ 2016-11-04 21:50 ` Linus Walleij
2016-11-07 15:26 ` Tony Lindgren
2 siblings, 1 reply; 22+ messages in thread
From: Linus Walleij @ 2016-11-04 21:50 UTC (permalink / raw)
To: Tony Lindgren
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, Linux-OMAP
On Thu, Nov 3, 2016 at 5:35 PM, Tony Lindgren <tony@atomide.com> wrote:
> Introduce #pinctrl-cells helper binding and generic helper functions
> pinctrl_count_index_with_args() and pinctrl_parse_index_with_args().
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
Ooops applied this v2 version instead of the v1.
* kbuild test robot <lkp@intel.com> [161103 13:29]:
> In file included from drivers/pinctrl/core.c:36:0:
> >> drivers/pinctrl/devicetree.h:29:14: warning: 'struct of_phandle_args' declared inside parameter list will not be visible outside of this
definition or declaratio
> Hmm maybe we should just include of.h in core.c?
Nah. I just did this:
diff --git a/drivers/pinctrl/devicetree.h b/drivers/pinctrl/devicetree.h
index 7f0a5c4e15ad..c2d1a5505850 100644
--- a/drivers/pinctrl/devicetree.h
+++ b/drivers/pinctrl/devicetree.h
@@ -16,6 +16,8 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+struct of_phandle_args;
+
#ifdef CONFIG_OF
Let's see if it works!
Yours,
Linus Walleij
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,pins
2016-11-03 16:35 ` [PATCH 2/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,pins Tony Lindgren
@ 2016-11-04 21:52 ` Linus Walleij
0 siblings, 0 replies; 22+ messages in thread
From: Linus Walleij @ 2016-11-04 21:52 UTC (permalink / raw)
To: Tony Lindgren
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, Linux-OMAP
On Thu, Nov 3, 2016 at 5:35 PM, Tony Lindgren <tony@atomide.com> wrote:
> We can now use generic parser. To support the legacy binding without
> #pinctrl-cells, add pcs_quirk_missing_pinctrl_cells() and warn about
> missing #pinctrl-cells.
>
> Let's also update the documentation for struct pcs_soc_data while at it
> as that seems to be out of date.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
This v2 patch applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,bits
2016-11-03 16:35 ` [PATCH 3/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,bits Tony Lindgren
@ 2016-11-04 21:54 ` Linus Walleij
0 siblings, 0 replies; 22+ messages in thread
From: Linus Walleij @ 2016-11-04 21:54 UTC (permalink / raw)
To: Tony Lindgren
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, Linux-OMAP
On Thu, Nov 3, 2016 at 5:35 PM, Tony Lindgren <tony@atomide.com> wrote:
> We can now use generic parser and keep things compatible with the
> old binding.
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
V2 Patch applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/4] ARM: dts: Add #pinctrl-cells for pinctrl-single instances
2016-11-03 16:35 ` [PATCH 4/4] ARM: dts: Add #pinctrl-cells for pinctrl-single instances Tony Lindgren
@ 2016-11-04 21:55 ` Linus Walleij
2016-11-07 17:39 ` Tony Lindgren
0 siblings, 1 reply; 22+ messages in thread
From: Linus Walleij @ 2016-11-04 21:55 UTC (permalink / raw)
To: Tony Lindgren
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, Linux-OMAP
On Thu, Nov 3, 2016 at 5:35 PM, Tony Lindgren <tony@atomide.com> wrote:
> Drivers using pinctrl-single,pins have #pinctrl-cells = <1>, while
> pinctrl-single,bits need #pinctrl-cells = <2>.
>
> Note that this patch can be optionally applied separately from the
> driver changes as the driver supports also the legacy binding without
> #pinctrl-cells.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Please take this through the OMAP tree to avoid hazzle.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-11-04 21:50 ` Linus Walleij
@ 2016-11-07 15:26 ` Tony Lindgren
2016-11-08 10:32 ` Linus Walleij
0 siblings, 1 reply; 22+ messages in thread
From: Tony Lindgren @ 2016-11-07 15:26 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, Linux-OMAP
* Linus Walleij <linus.walleij@linaro.org> [161104 14:50]:
> On Thu, Nov 3, 2016 at 5:35 PM, Tony Lindgren <tony@atomide.com> wrote:
>
> > Introduce #pinctrl-cells helper binding and generic helper functions
> > pinctrl_count_index_with_args() and pinctrl_parse_index_with_args().
> >
> > Acked-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> Ooops applied this v2 version instead of the v1.
No problem, only the pinctrl-single patches needed
reposting.
> * kbuild test robot <lkp@intel.com> [161103 13:29]:
> > In file included from drivers/pinctrl/core.c:36:0:
> > >> drivers/pinctrl/devicetree.h:29:14: warning: 'struct of_phandle_args' declared inside parameter list will not be visible outside of this
> definition or declaratio
>
> > Hmm maybe we should just include of.h in core.c?
>
> Nah. I just did this:
>
> diff --git a/drivers/pinctrl/devicetree.h b/drivers/pinctrl/devicetree.h
> index 7f0a5c4e15ad..c2d1a5505850 100644
> --- a/drivers/pinctrl/devicetree.h
> +++ b/drivers/pinctrl/devicetree.h
> @@ -16,6 +16,8 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +struct of_phandle_args;
> +
> #ifdef CONFIG_OF
>
> Let's see if it works!
OK so do we know now? It seems there was one more email
about it but it may have been without it.
Thanks,
Tony
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/4] ARM: dts: Add #pinctrl-cells for pinctrl-single instances
2016-11-04 21:55 ` Linus Walleij
@ 2016-11-07 17:39 ` Tony Lindgren
0 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2016-11-07 17:39 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, Linux-OMAP
* Linus Walleij <linus.walleij@linaro.org> [161104 14:55]:
> On Thu, Nov 3, 2016 at 5:35 PM, Tony Lindgren <tony@atomide.com> wrote:
>
> > Drivers using pinctrl-single,pins have #pinctrl-cells = <1>, while
> > pinctrl-single,bits need #pinctrl-cells = <2>.
> >
> > Note that this patch can be optionally applied separately from the
> > driver changes as the driver supports also the legacy binding without
> > #pinctrl-cells.
> >
> > Acked-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Please take this through the OMAP tree to avoid hazzle.
OK applied into omap-for-v4.10/pinctrl-cells.
Thanks,
Tony
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-11-07 15:26 ` Tony Lindgren
@ 2016-11-08 10:32 ` Linus Walleij
2016-11-08 14:44 ` Tony Lindgren
0 siblings, 1 reply; 22+ messages in thread
From: Linus Walleij @ 2016-11-08 10:32 UTC (permalink / raw)
To: Tony Lindgren
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, Linux-OMAP
On Mon, Nov 7, 2016 at 4:26 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Linus Walleij <linus.walleij@linaro.org> [161104 14:50]:
>> +struct of_phandle_args;
>> +
>> #ifdef CONFIG_OF
>>
>> Let's see if it works!
>
> OK so do we know now? It seems there was one more email
> about it but it may have been without it.
I haven't seen anything, I think it JustWorks(TM).
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-11-08 10:32 ` Linus Walleij
@ 2016-11-08 14:44 ` Tony Lindgren
0 siblings, 0 replies; 22+ messages in thread
From: Tony Lindgren @ 2016-11-08 14:44 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, Linux-OMAP
* Linus Walleij <linus.walleij@linaro.org> [161108 03:32]:
> On Mon, Nov 7, 2016 at 4:26 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Linus Walleij <linus.walleij@linaro.org> [161104 14:50]:
>
> >> +struct of_phandle_args;
> >> +
> >> #ifdef CONFIG_OF
> >>
> >> Let's see if it works!
> >
> > OK so do we know now? It seems there was one more email
> > about it but it may have been without it.
>
> I haven't seen anything, I think it JustWorks(TM).
OK good to to hear :)
Thanks,
Tony
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-10-28 16:53 ` Tony Lindgren
2016-10-31 6:13 ` Rob Herring
@ 2016-11-04 21:36 ` Linus Walleij
1 sibling, 0 replies; 22+ messages in thread
From: Linus Walleij @ 2016-11-04 21:36 UTC (permalink / raw)
To: Tony Lindgren
Cc: Rob Herring, linux-kernel, Jon Hunter, Mark Rutland,
Grygorii Strashko, Nishanth Menon, linux-gpio, devicetree,
Linux-OMAP
On Fri, Oct 28, 2016 at 6:53 PM, Tony Lindgren <tony@atomide.com> wrote:
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren <tony@atomide.com>
> Date: Tue, 25 Oct 2016 08:33:34 -0700
> Subject: [PATCH] pinctrl: Introduce generic #pinctrl-cells and
> pinctrl_parse_index_with_args
>
> Introduce #pinctrl-cells helper binding and generic helper functions
> pinctrl_count_index_with_args() and pinctrl_parse_index_with_args().
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
This updated version applied with Rob's ACK.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-10-28 16:53 ` Tony Lindgren
@ 2016-10-31 6:13 ` Rob Herring
2016-11-04 21:36 ` Linus Walleij
1 sibling, 0 replies; 22+ messages in thread
From: Rob Herring @ 2016-10-31 6:13 UTC (permalink / raw)
To: Tony Lindgren
Cc: Linus Walleij, linux-kernel, Jon Hunter, Mark Rutland,
Grygorii Strashko, Nishanth Menon, linux-gpio, devicetree,
Linux-OMAP
On Fri, Oct 28, 2016 at 09:53:38AM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [161027 07:59]:
> > * Linus Walleij <linus.walleij@linaro.org> [161027 00:57]:
> > > On Tue, Oct 25, 2016 at 6:45 PM, Tony Lindgren <tony@atomide.com> wrote:
> > > > +/*
> > > > + * For pinctrl binding, typically #pinctrl-cells is for the pin controller
> > > > + * device, so either parent or grandparent. See pinctrl-bindings.txt.
> > > > + */
> > > > +static int pinctrl_find_cells_size(const struct device_node *np,
> > > > + const char *cells_name)
> > > > +{
> > > > + int cells_size, error;
> > > > +
> > > > + error = of_property_read_u32(np->parent, cells_name, &cells_size);
> > > > + if (error) {
> > > > + error = of_property_read_u32(np->parent->parent,
> > > > + cells_name, &cells_size);
> > > > + if (error)
> > > > + return -ENOENT;
> > > > + }
> > > > +
> > > > + return cells_size;
> > > > +}
> > >
> > > Can't we just hardcode this to "#pinctrl-cells" and skip the cells_name
> > > parameter? We can parametrize it the day we need it instead.
> >
> > Sure we can do that.
> >
> > > The rest of the helpers look nice and clean.
> >
> > OK cool thanks,
>
> Below is an updated version of this patch with documentation updated
> and cells_name removed. I'll repost the whole series once the DT
> binding has been reviewed.
>
> Regards,
>
> Tony
> 8< -------------------------
> From tony Mon Sep 17 00:00:00 2001
> From: Tony Lindgren <tony@atomide.com>
> Date: Tue, 25 Oct 2016 08:33:34 -0700
> Subject: [PATCH] pinctrl: Introduce generic #pinctrl-cells and
> pinctrl_parse_index_with_args
>
> Introduce #pinctrl-cells helper binding and generic helper functions
> pinctrl_count_index_with_args() and pinctrl_parse_index_with_args().
>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> .../bindings/pinctrl/pinctrl-bindings.txt | 44 ++++++-
Acked-by: Rob Herring <robh@kernel.org>
> drivers/pinctrl/devicetree.c | 144 +++++++++++++++++++++
> drivers/pinctrl/devicetree.h | 21 +++
> 3 files changed, 208 insertions(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-10-27 14:11 ` Tony Lindgren
@ 2016-10-28 16:53 ` Tony Lindgren
2016-10-31 6:13 ` Rob Herring
2016-11-04 21:36 ` Linus Walleij
0 siblings, 2 replies; 22+ messages in thread
From: Tony Lindgren @ 2016-10-28 16:53 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, linux-kernel, Jon Hunter, Mark Rutland,
Grygorii Strashko, Nishanth Menon, linux-gpio, devicetree,
Linux-OMAP
* Tony Lindgren <tony@atomide.com> [161027 07:59]:
> * Linus Walleij <linus.walleij@linaro.org> [161027 00:57]:
> > On Tue, Oct 25, 2016 at 6:45 PM, Tony Lindgren <tony@atomide.com> wrote:
> > > +/*
> > > + * For pinctrl binding, typically #pinctrl-cells is for the pin controller
> > > + * device, so either parent or grandparent. See pinctrl-bindings.txt.
> > > + */
> > > +static int pinctrl_find_cells_size(const struct device_node *np,
> > > + const char *cells_name)
> > > +{
> > > + int cells_size, error;
> > > +
> > > + error = of_property_read_u32(np->parent, cells_name, &cells_size);
> > > + if (error) {
> > > + error = of_property_read_u32(np->parent->parent,
> > > + cells_name, &cells_size);
> > > + if (error)
> > > + return -ENOENT;
> > > + }
> > > +
> > > + return cells_size;
> > > +}
> >
> > Can't we just hardcode this to "#pinctrl-cells" and skip the cells_name
> > parameter? We can parametrize it the day we need it instead.
>
> Sure we can do that.
>
> > The rest of the helpers look nice and clean.
>
> OK cool thanks,
Below is an updated version of this patch with documentation updated
and cells_name removed. I'll repost the whole series once the DT
binding has been reviewed.
Regards,
Tony
8< -------------------------
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Tue, 25 Oct 2016 08:33:34 -0700
Subject: [PATCH] pinctrl: Introduce generic #pinctrl-cells and
pinctrl_parse_index_with_args
Introduce #pinctrl-cells helper binding and generic helper functions
pinctrl_count_index_with_args() and pinctrl_parse_index_with_args().
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
.../bindings/pinctrl/pinctrl-bindings.txt | 44 ++++++-
drivers/pinctrl/devicetree.c | 144 +++++++++++++++++++++
drivers/pinctrl/devicetree.h | 21 +++
3 files changed, 208 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -97,6 +97,11 @@ For example:
};
== Pin controller devices ==
+Required properties: See the pin controller driver specific documentation
+
+Optional properties:
+#pinctrl-cells: Number of pin control cells in addition to the index within the
+ pin controller device instance
Pin controller devices should contain the pin configuration nodes that client
devices reference.
@@ -119,7 +124,8 @@ For example:
The contents of each of those pin configuration child nodes is defined
entirely by the binding for the individual pin controller device. There
-exists no common standard for this content.
+exists no common standard for this content. The pinctrl framework only
+provides generic helper bindings that the pin controller driver can use.
The pin configuration nodes need not be direct children of the pin controller
device; they may be grandchildren, for example. Whether this is legal, and
@@ -156,6 +162,42 @@ state_2_node_a {
pins = "mfio29", "mfio30";
};
+Optionally an altenative binding can be used if more suitable depending on the
+pin controller hardware. For hardaware where there is a large number of identical
+pin controller instances, naming each pin and function can easily become
+unmaintainable. This is especially the case if the same controller is used for
+different pins and functions depending on the SoC revision and packaging.
+
+For cases like this, the pin controller driver may use pinctrl-pin-array helper
+binding with a hardware based index and a number of pin configuration values:
+
+pincontroller {
+ ... /* Standard DT properties for the device itself elided */
+ #pinctrl-cells = <2>;
+
+ state_0_node_a {
+ pinctrl-pin-array = <
+ 0 A_DELAY_PS(0) G_DELAY_PS(120)
+ 4 A_DELAY_PS(0) G_DELAY_PS(360)
+ ...
+ >;
+ };
+ ...
+};
+
+Above #pinctrl-cells specifies the number of value cells in addition to the
+index of the registers. This is similar to the interrupts-extended binding with
+one exception. There is no need to specify the phandle for each entry as that
+is already known as the defined pins are always children of the pin controller
+node. Further having the phandle pointing to another pin controller would not
+currently work as the pinctrl framework uses named modes to group pins for each
+pin control device.
+
+The index for pinctrl-pin-array must relate to the hardware for the pinctrl
+registers, and must not be a virtual index of pin instances. The reason for
+this is to avoid mapping of the index in the dts files and the pin controller
+driver as it can change.
+
== Generic pin configuration node content ==
Many data items that are represented in a pin configuration node are common
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -253,3 +253,147 @@ int pinctrl_dt_to_map(struct pinctrl *p)
pinctrl_dt_free_maps(p);
return ret;
}
+
+/*
+ * For pinctrl binding, typically #pinctrl-cells is for the pin controller
+ * device, so either parent or grandparent. See pinctrl-bindings.txt.
+ */
+static int pinctrl_find_cells_size(const struct device_node *np)
+{
+ const char *cells_name = "#pinctrl-cells";
+ int cells_size, error;
+
+ error = of_property_read_u32(np->parent, cells_name, &cells_size);
+ if (error) {
+ error = of_property_read_u32(np->parent->parent,
+ cells_name, &cells_size);
+ if (error)
+ return -ENOENT;
+ }
+
+ return cells_size;
+}
+
+/**
+ * pinctrl_get_list_and_count - Gets the list and it's cell size and number
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ * @list: pointer for the list found
+ * @cells_size: pointer for the cell size found
+ * @nr_elements: pointer for the number of elements found
+ *
+ * Typically np is a single pinctrl entry containing the list.
+ */
+static int pinctrl_get_list_and_count(const struct device_node *np,
+ const char *list_name,
+ const __be32 **list,
+ int *cells_size,
+ int *nr_elements)
+{
+ int size;
+
+ *cells_size = 0;
+ *nr_elements = 0;
+
+ *list = of_get_property(np, list_name, &size);
+ if (!*list)
+ return -ENOENT;
+
+ *cells_size = pinctrl_find_cells_size(np);
+ if (*cells_size < 0)
+ return -ENOENT;
+
+ /* First element is always the index within the pinctrl device */
+ *nr_elements = (size / sizeof(**list)) / (*cells_size + 1);
+
+ return 0;
+}
+
+/**
+ * pinctrl_count_index_with_args - Count number of elements in a pinctrl entry
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ *
+ * Counts the number of elements in a pinctrl array consisting of an index
+ * within the controller and a number of u32 entries specified for each
+ * entry. Note that device_node is always for the parent pin controller device.
+ */
+int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name)
+{
+ const __be32 *list;
+ int size, nr_cells, error;
+
+ error = pinctrl_get_list_and_count(np, list_name, &list,
+ &nr_cells, &size);
+ if (error)
+ return error;
+
+ return size;
+}
+EXPORT_SYMBOL_GPL(pinctrl_count_index_with_args);
+
+/**
+ * pinctrl_copy_args - Populates of_phandle_args based on index
+ * @np: pointer to device node with the property
+ * @list: pointer to a list with the elements
+ * @index: entry within the list of elements
+ * @nr_cells: number of cells in the list
+ * @nr_elem: number of elements for each entry in the list
+ * @out_args: returned values
+ *
+ * Populates the of_phandle_args based on the index in the list.
+ */
+static int pinctrl_copy_args(const struct device_node *np,
+ const __be32 *list,
+ int index, int nr_cells, int nr_elem,
+ struct of_phandle_args *out_args)
+{
+ int i;
+
+ memset(out_args, 0, sizeof(*out_args));
+ out_args->np = (struct device_node *)np;
+ out_args->args_count = nr_cells + 1;
+
+ if (index >= nr_elem)
+ return -EINVAL;
+
+ list += index * (nr_cells + 1);
+
+ for (i = 0; i < nr_cells + 1; i++)
+ out_args->args[i] = be32_to_cpup(list++);
+
+ return 0;
+}
+
+/**
+ * pinctrl_parse_index_with_args - Find a node pointed by index in a list
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ * @index: index within the list
+ * @out_arts: entries in the list pointed by index
+ *
+ * Finds the selected element in a pinctrl array consisting of an index
+ * within the controller and a number of u32 entries specified for each
+ * entry. Note that device_node is always for the parent pin controller device.
+ */
+int pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, int index,
+ struct of_phandle_args *out_args)
+{
+ const __be32 *list;
+ int nr_elem, nr_cells, error;
+
+ error = pinctrl_get_list_and_count(np, list_name, &list,
+ &nr_cells, &nr_elem);
+ if (error || !nr_cells)
+ return error;
+
+ error = pinctrl_copy_args(np, list, index, nr_cells, nr_elem,
+ out_args);
+ if (error)
+ return error;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_parse_index_with_args);
diff --git a/drivers/pinctrl/devicetree.h b/drivers/pinctrl/devicetree.h
--- a/drivers/pinctrl/devicetree.h
+++ b/drivers/pinctrl/devicetree.h
@@ -21,6 +21,13 @@
void pinctrl_dt_free_maps(struct pinctrl *p);
int pinctrl_dt_to_map(struct pinctrl *p);
+int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name);
+
+int pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, int index,
+ struct of_phandle_args *out_args);
+
#else
static inline int pinctrl_dt_to_map(struct pinctrl *p)
@@ -32,4 +39,18 @@ static inline void pinctrl_dt_free_maps(struct pinctrl *p)
{
}
+static inline int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name)
+{
+ return -ENODEV;
+}
+
+static inline int
+pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, int index,
+ struct of_phandle_args *out_args)
+{
+ return -ENODEV;
+}
+
#endif
--
2.9.3
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-10-27 7:56 ` Linus Walleij
@ 2016-10-27 14:11 ` Tony Lindgren
2016-10-28 16:53 ` Tony Lindgren
0 siblings, 1 reply; 22+ messages in thread
From: Tony Lindgren @ 2016-10-27 14:11 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, linux-kernel, Jon Hunter, Mark Rutland,
Grygorii Strashko, Nishanth Menon, linux-gpio, devicetree,
Linux-OMAP
* Linus Walleij <linus.walleij@linaro.org> [161027 00:57]:
> On Tue, Oct 25, 2016 at 6:45 PM, Tony Lindgren <tony@atomide.com> wrote:
>
> I need some DT person to take a look at this binding and ACK it.
>
> > +For pin controller hardware with a large number of identical registers naming
> > +each bit both can be unmaintainable. Further there can be a large number of similar
> > +pinctrl hardware using the same registers for different purposes depending on the
> > +packaging. For cases like this, the pinctrl driver may use pinctrl-pin-array helper
> > +binding using a hardware based index and a number of configuration values:
>
> Maybe we can reword it a bit so that it is clear that this is an
> either-or approach
> for the pin controller, either they use the pins/groups/functions scheme
> or they use this scheme.
Sure, this is just an optional helper.
> > +pincontroller {
> > + ... /* Standard DT properties for the device itself elided */
> > + #pinctrl-cells = <2>;
> > +
> > + state_0_node_a {
> > + pinctrl-pin-array = <
> > + 0 A_DELAY_PS(0) G_DELAY_PS(120)
> > + 4 A_DELAY_PS(0) G_DELAY_PS(360)
> > + ...
> > + >;
> > + };
> > + ...
> > +};
>
> Looks all right to me. Sad to add to the binding mess, but on the other
> hand, in the overall picture this nicely consolidates and structure
> pinctrl-single.
>
> > +The index for pinctrl-pin-array must relate to the hardware for the pinctrl
> > +registers, and must not be a virtual index of pin instances. The reason for
> > +this is to avoid mapping of the index in the dts files and the pin controller
> > +driver as it can change.
>
> OK
>
> > And we want to avoid another case of interrupt
> > +numbering with pinctrl numbering.
>
> Maybe this file is not a good place for making technical arguments,
> more describing what we agreed on, so cut that sentence IMO.
Sure :)
> > +/*
> > + * For pinctrl binding, typically #pinctrl-cells is for the pin controller
> > + * device, so either parent or grandparent. See pinctrl-bindings.txt.
> > + */
> > +static int pinctrl_find_cells_size(const struct device_node *np,
> > + const char *cells_name)
> > +{
> > + int cells_size, error;
> > +
> > + error = of_property_read_u32(np->parent, cells_name, &cells_size);
> > + if (error) {
> > + error = of_property_read_u32(np->parent->parent,
> > + cells_name, &cells_size);
> > + if (error)
> > + return -ENOENT;
> > + }
> > +
> > + return cells_size;
> > +}
>
> Can't we just hardcode this to "#pinctrl-cells" and skip the cells_name
> parameter? We can parametrize it the day we need it instead.
Sure we can do that.
> The rest of the helpers look nice and clean.
OK cool thanks,
Tony
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-10-25 16:45 ` [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells " Tony Lindgren
@ 2016-10-27 7:56 ` Linus Walleij
2016-10-27 14:11 ` Tony Lindgren
0 siblings, 1 reply; 22+ messages in thread
From: Linus Walleij @ 2016-10-27 7:56 UTC (permalink / raw)
To: Tony Lindgren, Rob Herring, linux-kernel
Cc: Jon Hunter, Mark Rutland, Grygorii Strashko, Nishanth Menon,
linux-gpio, devicetree, Linux-OMAP
On Tue, Oct 25, 2016 at 6:45 PM, Tony Lindgren <tony@atomide.com> wrote:
I need some DT person to take a look at this binding and ACK it.
> +For pin controller hardware with a large number of identical registers naming
> +each bit both can be unmaintainable. Further there can be a large number of similar
> +pinctrl hardware using the same registers for different purposes depending on the
> +packaging. For cases like this, the pinctrl driver may use pinctrl-pin-array helper
> +binding using a hardware based index and a number of configuration values:
Maybe we can reword it a bit so that it is clear that this is an
either-or approach
for the pin controller, either they use the pins/groups/functions scheme
or they use this scheme.
> +pincontroller {
> + ... /* Standard DT properties for the device itself elided */
> + #pinctrl-cells = <2>;
> +
> + state_0_node_a {
> + pinctrl-pin-array = <
> + 0 A_DELAY_PS(0) G_DELAY_PS(120)
> + 4 A_DELAY_PS(0) G_DELAY_PS(360)
> + ...
> + >;
> + };
> + ...
> +};
Looks all right to me. Sad to add to the binding mess, but on the other
hand, in the overall picture this nicely consolidates and structure
pinctrl-single.
> +The index for pinctrl-pin-array must relate to the hardware for the pinctrl
> +registers, and must not be a virtual index of pin instances. The reason for
> +this is to avoid mapping of the index in the dts files and the pin controller
> +driver as it can change.
OK
> And we want to avoid another case of interrupt
> +numbering with pinctrl numbering.
Maybe this file is not a good place for making technical arguments,
more describing what we agreed on, so cut that sentence IMO.
> +/*
> + * For pinctrl binding, typically #pinctrl-cells is for the pin controller
> + * device, so either parent or grandparent. See pinctrl-bindings.txt.
> + */
> +static int pinctrl_find_cells_size(const struct device_node *np,
> + const char *cells_name)
> +{
> + int cells_size, error;
> +
> + error = of_property_read_u32(np->parent, cells_name, &cells_size);
> + if (error) {
> + error = of_property_read_u32(np->parent->parent,
> + cells_name, &cells_size);
> + if (error)
> + return -ENOENT;
> + }
> +
> + return cells_size;
> +}
Can't we just hardcode this to "#pinctrl-cells" and skip the cells_name
parameter? We can parametrize it the day we need it instead.
The rest of the helpers look nice and clean.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
2016-10-25 16:45 [PATCH 0/4] Generic #pinctrl-cells and and pinctrl_parse_index_with_args Tony Lindgren
@ 2016-10-25 16:45 ` Tony Lindgren
2016-10-27 7:56 ` Linus Walleij
0 siblings, 1 reply; 22+ messages in thread
From: Tony Lindgren @ 2016-10-25 16:45 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, linux-omap
Introduce #pinctrl-cells helper binding and generic helper functions
pinctrl_count_index_with_args() and pinctrl_parse_index_with_args().
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
.../bindings/pinctrl/pinctrl-bindings.txt | 42 +++++-
drivers/pinctrl/devicetree.c | 151 +++++++++++++++++++++
drivers/pinctrl/devicetree.h | 24 ++++
3 files changed, 216 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -97,6 +97,11 @@ For example:
};
== Pin controller devices ==
+Required properties: See the pin controller driver specific documentation
+
+Optional properties:
+#pinctrl-cells: Number of pin control cells in addition to the index within the
+ pin controller device instance
Pin controller devices should contain the pin configuration nodes that client
devices reference.
@@ -119,7 +124,8 @@ For example:
The contents of each of those pin configuration child nodes is defined
entirely by the binding for the individual pin controller device. There
-exists no common standard for this content.
+exists no common standard for this content. The pinctrl framework only
+provides generic helper bindings that the pin controller driver can use.
The pin configuration nodes need not be direct children of the pin controller
device; they may be grandchildren, for example. Whether this is legal, and
@@ -156,6 +162,40 @@ state_2_node_a {
pins = "mfio29", "mfio30";
};
+For pin controller hardware with a large number of identical registers naming
+each bit both can be unmaintainable. Further there can be a large number of similar
+pinctrl hardware using the same registers for different purposes depending on the
+packaging. For cases like this, the pinctrl driver may use pinctrl-pin-array helper
+binding using a hardware based index and a number of configuration values:
+
+pincontroller {
+ ... /* Standard DT properties for the device itself elided */
+ #pinctrl-cells = <2>;
+
+ state_0_node_a {
+ pinctrl-pin-array = <
+ 0 A_DELAY_PS(0) G_DELAY_PS(120)
+ 4 A_DELAY_PS(0) G_DELAY_PS(360)
+ ...
+ >;
+ };
+ ...
+};
+
+The #pinctrl-cells specifies the number of value cells in addition to the
+index of the registers. This is similar to the interrupts-extended binding with
+one exception. There is no need to specify the phandle for each entry as that
+is already known as the defined pins are always children of the pin controller
+node. Further having the phandle pointing to another pin controller would not
+currently work as the pinctrl framework uses named modes to group pins for each
+pin control device.
+
+The index for pinctrl-pin-array must relate to the hardware for the pinctrl
+registers, and must not be a virtual index of pin instances. The reason for
+this is to avoid mapping of the index in the dts files and the pin controller
+driver as it can change. And we want to avoid another case of interrupt
+numbering with pinctrl numbering.
+
== Generic pin configuration node content ==
Many data items that are represented in a pin configuration node are common
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -253,3 +253,154 @@ int pinctrl_dt_to_map(struct pinctrl *p)
pinctrl_dt_free_maps(p);
return ret;
}
+
+/*
+ * For pinctrl binding, typically #pinctrl-cells is for the pin controller
+ * device, so either parent or grandparent. See pinctrl-bindings.txt.
+ */
+static int pinctrl_find_cells_size(const struct device_node *np,
+ const char *cells_name)
+{
+ int cells_size, error;
+
+ error = of_property_read_u32(np->parent, cells_name, &cells_size);
+ if (error) {
+ error = of_property_read_u32(np->parent->parent,
+ cells_name, &cells_size);
+ if (error)
+ return -ENOENT;
+ }
+
+ return cells_size;
+}
+
+/**
+ * pinctrl_get_list_and_count - Gets the list and it's cell size and number
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ * @cells_name: property name that specifies phandle's argument count
+ * @list: pointer for the list found
+ * @cells_size: pointer for the cell size found
+ * @nr_elements: pointer for the number of elements found
+ *
+ * Typically np is a single pinctrl entry containing the list and
+ * cells_name is "#pinctrl-cells" for the parent pin controller.
+ */
+static int pinctrl_get_list_and_count(const struct device_node *np,
+ const char *list_name,
+ const char *cells_name,
+ int nr_elem,
+ const __be32 **list,
+ int *cells_size,
+ int *nr_elements)
+{
+ int size;
+
+ *cells_size = 0;
+ *nr_elements = 0;
+
+ *list = of_get_property(np, list_name, &size);
+ if (!*list)
+ return -ENOENT;
+
+ *cells_size = pinctrl_find_cells_size(np, cells_name);
+ if (*cells_size < 0)
+ return -ENOENT;
+
+ /* First element is always the index within the pinctrl device */
+ *nr_elements = (size / sizeof(**list)) / (*cells_size + 1);
+
+ return 0;
+}
+
+/**
+ * pinctrl_count_index_with_args - Count number of elements in a pinctrl entry
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ * @cells_name: property name that specifies the argument count
+ *
+ * Counts the number of elements in a pinctrl array consisting of an index
+ * within the controller and a number of u32 entries specified for each
+ * entry. Note that device_node is always for the parent pin controller device.
+ */
+int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name,
+ const char *cells_name)
+{
+ const __be32 *list;
+ int size, nr_cells, error;
+
+ error = pinctrl_get_list_and_count(np, list_name, cells_name, -1,
+ &list, &nr_cells, &size);
+ if (error)
+ return error;
+
+ return size;
+}
+EXPORT_SYMBOL_GPL(pinctrl_count_index_with_args);
+
+/**
+ * pinctrl_copy_args - Populates of_phandle_args based on index
+ * @np: pointer to device node with the property
+ * @list: pointer to a list with the elements
+ * @index: entry within the list of elements
+ * @nr_cells: number of cells in the list
+ * @nr_elem: number of elements for each entry in the list
+ * @out_args: returned values
+ *
+ * Populates the of_phandle_args based on the index in the list.
+ */
+static int pinctrl_copy_args(const struct device_node *np,
+ const __be32 *list,
+ int index, int nr_cells, int nr_elem,
+ struct of_phandle_args *out_args)
+{
+ int i;
+
+ memset(out_args, 0, sizeof(*out_args));
+ out_args->np = (struct device_node *)np;
+ out_args->args_count = nr_cells + 1;
+
+ if (index >= nr_elem)
+ return -EINVAL;
+
+ list += index * (nr_cells + 1);
+
+ for (i = 0; i < nr_cells + 1; i++)
+ out_args->args[i] = be32_to_cpup(list++);
+
+ return 0;
+}
+
+/**
+ * pinctrl_parse_index_with_args - Find a node pointed by index in a list
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ * @cells_name: property name that specifies phandle's argument count
+ * @index: index within the list
+ * @out_arts: entries in the list pointed by index
+ *
+ * Finds the selected element in a pinctrl array consisting of an index
+ * within the controller and a number of u32 entries specified for each
+ * entry. Note that device_node is always for the parent pin controller device.
+ */
+int pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, const char *cells_name,
+ int index, struct of_phandle_args *out_args)
+{
+ const __be32 *list;
+ int nr_elem, nr_cells, error;
+
+ error = pinctrl_get_list_and_count(np, list_name, cells_name, -1,
+ &list, &nr_cells, &nr_elem);
+ if (error || !nr_cells)
+ return error;
+
+ error = pinctrl_copy_args(np, list, index, nr_cells, nr_elem,
+ out_args);
+ if (error)
+ return error;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_parse_index_with_args);
diff --git a/drivers/pinctrl/devicetree.h b/drivers/pinctrl/devicetree.h
--- a/drivers/pinctrl/devicetree.h
+++ b/drivers/pinctrl/devicetree.h
@@ -21,6 +21,14 @@
void pinctrl_dt_free_maps(struct pinctrl *p);
int pinctrl_dt_to_map(struct pinctrl *p);
+int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name,
+ const char *cells_name);
+
+int pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, const char *cells_name,
+ int index, struct of_phandle_args *out_args);
+
#else
static inline int pinctrl_dt_to_map(struct pinctrl *p)
@@ -32,4 +40,20 @@ static inline void pinctrl_dt_free_maps(struct pinctrl *p)
{
}
+static inline int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name,
+ const char *cells_name)
+{
+ return -ENODEV;
+}
+
+static inline int
+pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name,
+ const char *cells_name, int index,
+ struct of_phandle_args *out_args)
+{
+ return -ENODEV;
+}
+
#endif
--
2.9.3
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-11-08 14:44 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-03 16:35 [PATCHv2 0/4] Generic #pinctrl-cells and and pinctrl_parse_index_with_args Tony Lindgren
2016-11-03 16:35 ` [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells " Tony Lindgren
2016-11-03 20:28 ` kbuild test robot
2016-11-03 20:48 ` Tony Lindgren
2016-11-04 6:03 ` kbuild test robot
2016-11-04 21:50 ` Linus Walleij
2016-11-07 15:26 ` Tony Lindgren
2016-11-08 10:32 ` Linus Walleij
2016-11-08 14:44 ` Tony Lindgren
2016-11-03 16:35 ` [PATCH 2/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,pins Tony Lindgren
2016-11-04 21:52 ` Linus Walleij
2016-11-03 16:35 ` [PATCH 3/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,bits Tony Lindgren
2016-11-04 21:54 ` Linus Walleij
2016-11-03 16:35 ` [PATCH 4/4] ARM: dts: Add #pinctrl-cells for pinctrl-single instances Tony Lindgren
2016-11-04 21:55 ` Linus Walleij
2016-11-07 17:39 ` Tony Lindgren
-- strict thread matches above, loose matches on Subject: below --
2016-10-25 16:45 [PATCH 0/4] Generic #pinctrl-cells and and pinctrl_parse_index_with_args Tony Lindgren
2016-10-25 16:45 ` [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells " Tony Lindgren
2016-10-27 7:56 ` Linus Walleij
2016-10-27 14:11 ` Tony Lindgren
2016-10-28 16:53 ` Tony Lindgren
2016-10-31 6:13 ` Rob Herring
2016-11-04 21:36 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).