Skip to contentSkip to conversation

Add support for muxing individual pins

PATCH In review Charles Keepax 4 participants 15 messages Sep 29, 2017 – Dec 9, 2017

Subsystems (21)
Arm i.MX and Layerscape GPIO Arm MediaTek Arm Samsung Exynos +17 more subsystems
lore
Share & tools
lore

Charles Keepax

author
#0 lore raw

cover Add support for muxing individual pins

This series add support for muxing individual pins within pin mux, rather than just whole groups. Mainly, I had two motivations here, one to avoid the need to add loads of groups containing individual pins and hardware that actually has some internal concept of groups of pins, and disambiguating that from individual pin muxing. I have marked it as RFC to just get peoples opinions at this stage, although it should be pretty well tested. Sorry about the amount of files touched in patch 2 it would be possible to drop it from the chain although it leaves the field rather inaccurately named.

Also I have left all the existing code paths parsing all mux options as groups from DT, and added a new helper to unlock the pin based functionality this should ease the transition across.

Thanks,
Charles

Charles Keepax (4):
pinctrl: Factor out individual pin handling from pinmux_pins_show
pinctrl: Rename mux group to group_or_pin to prepare for pin support
pinctrl: Add support for muxing individual pins
pinctrl: Add support for parsing individual pinmux from DT

 drivers/pinctrl/bcm/pinctrl-bcm2835.c         |   2 +-
 drivers/pinctrl/core.c                        |  26 ++-
 drivers/pinctrl/core.h                        |   3 +-
 drivers/pinctrl/freescale/pinctrl-imx.c       |   5 +-
 drivers/pinctrl/freescale/pinctrl-imx1-core.c |   5 +-
 drivers/pinctrl/freescale/pinctrl-mxs.c       |   4 +-
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c |   2 +-
 drivers/pinctrl/mvebu/pinctrl-mvebu.c         |   2 +-
 drivers/pinctrl/nomadik/pinctrl-abx500.c      |   2 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik.c     |   2 +-
 drivers/pinctrl/pinconf-generic.c             |  30 ++-
 drivers/pinctrl/pinctrl-at91.c                |   5 +-
 drivers/pinctrl/pinctrl-lantiq.c              |   2 +-
 drivers/pinctrl/pinctrl-rockchip.c            |   5 +-
 drivers/pinctrl/pinctrl-rza1.c                |   2 +-
 drivers/pinctrl/pinctrl-single.c              |   4 +-
 drivers/pinctrl/pinctrl-st.c                  |   2 +-
 drivers/pinctrl/pinctrl-tz1090-pdc.c          |   2 +-
 drivers/pinctrl/pinctrl-tz1090.c              |   2 +-
 drivers/pinctrl/pinctrl-utils.c               |  10 +-
 drivers/pinctrl/pinctrl-utils.h               |  14 +-
 drivers/pinctrl/pinmux.c                      | 311 ++++++++++++++++++--------
 drivers/pinctrl/samsung/pinctrl-exynos5440.c  |   4 +-
 drivers/pinctrl/samsung/pinctrl-samsung.c     |   2 +-
 drivers/pinctrl/sh-pfc/pinctrl.c              |   2 +-
 drivers/pinctrl/sirf/pinctrl-atlas7.c         |   3 +-
 drivers/pinctrl/sirf/pinctrl-sirf.c           |   2 +-
 drivers/pinctrl/spear/pinctrl-spear.c         |   2 +-
 drivers/pinctrl/stm32/pinctrl-stm32.c         |   2 +-
 drivers/pinctrl/sunxi/pinctrl-sunxi.c         |   2 +-
 drivers/pinctrl/vt8500/pinctrl-wmt.c          |   2 +-
 include/linux/pinctrl/machine.h               |   3 +-
 include/linux/pinctrl/pinconf-generic.h       |  24 +-
 include/linux/pinctrl/pinmux.h                |   7 +
 34 files changed, 338 insertions(+), 159 deletions(-)
Signature · 1 line
2.11.0
author
#1 lore raw

RFC PATCH 1/4 pinctrl: Factor out individual pin handling from pinmux_pins_show

Add a new helper function to be called for each pin rather than keeping everything in the same function. Primarily this just reduces the code indentation a bit.

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/pinctrl/pinmux.c | 100 +++++++++++++++++++++++++----------------------
 1 file changed, 53 insertions(+), 47 deletions(-)
drivers/pinctrl/pinmux.c +53 -47
index 55502fc4479c..e76e6f6a79d6 100644
@@ -555,12 +555,61 @@ static int pinmux_functions_show(struct seq_file *s, void *what)
 	return 0;
 }
 
-static int pinmux_pins_show(struct seq_file *s, void *what)
+static int pinmux_pin_show(struct seq_file *s, unsigned int pin)
 {
 	struct pinctrl_dev *pctldev = s->private;
 	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
 	const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
-	unsigned i, pin;
+	struct pin_desc *desc = pin_desc_get(pctldev, pin);
+	bool is_hog = false;
+
+	/* Skip if we cannot search the pin */
+	if (desc == NULL)
+		return 0;
+
+	if (desc->mux_owner &&
+	    !strcmp(desc->mux_owner, pinctrl_dev_get_name(pctldev)))
+		is_hog = true;
+
+	if (pmxops->strict) {
+		if (desc->mux_owner)
+			seq_printf(s, "pin %d (%s): device %s%s",
+				   pin, desc->name, desc->mux_owner,
+				   is_hog ? " (HOG)" : "");
+		else if (desc->gpio_owner)
+			seq_printf(s, "pin %d (%s): GPIO %s",
+				   pin, desc->name, desc->gpio_owner);
+		else
+			seq_printf(s, "pin %d (%s): UNCLAIMED",
+				   pin, desc->name);
+	} else {
+		/* For non-strict controllers */
+		seq_printf(s, "pin %d (%s): %s %s%s", pin, desc->name,
+			   desc->mux_owner ? desc->mux_owner
+			   : "(MUX UNCLAIMED)",
+			   desc->gpio_owner ? desc->gpio_owner
+			   : "(GPIO UNCLAIMED)",
+			   is_hog ? " (HOG)" : "");
+	}
+
+	/* If mux: print function+group claiming the pin */
+	if (desc->mux_setting)
+		seq_printf(s, " function %s group %s\n",
+			   pmxops->get_function_name(pctldev,
+				desc->mux_setting->func),
+			   pctlops->get_group_name(pctldev,
+				desc->mux_setting->group));
+	else
+		seq_puts(s, "\n");
+
+	return 0;
+}
+
+static int pinmux_pins_show(struct seq_file *s, void *what)
+{
+	struct pinctrl_dev *pctldev = s->private;
+	const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
+	unsigned int i;
 
 	if (!pmxops)
 		return 0;

@@ -576,51 +625,8 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
 	mutex_lock(&pctldev->mutex);
 
 	/* The pin number can be retrived from the pin controller descriptor */
-	for (i = 0; i < pctldev->desc->npins; i++) {
-		struct pin_desc *desc;
-		bool is_hog = false;
-
-		pin = pctldev->desc->pins[i].number;
-		desc = pin_desc_get(pctldev, pin);
-		/* Skip if we cannot search the pin */
-		if (desc == NULL)
-			continue;
-
-		if (desc->mux_owner &&
-		    !strcmp(desc->mux_owner, pinctrl_dev_get_name(pctldev)))
-			is_hog = true;
-
-		if (pmxops->strict) {
-			if (desc->mux_owner)
-				seq_printf(s, "pin %d (%s): device %s%s",
-					   pin, desc->name, desc->mux_owner,
-					   is_hog ? " (HOG)" : "");
-			else if (desc->gpio_owner)
-				seq_printf(s, "pin %d (%s): GPIO %s",
-					   pin, desc->name, desc->gpio_owner);
-			else
-				seq_printf(s, "pin %d (%s): UNCLAIMED",
-					   pin, desc->name);
-		} else {
-			/* For non-strict controllers */
-			seq_printf(s, "pin %d (%s): %s %s%s", pin, desc->name,
-				   desc->mux_owner ? desc->mux_owner
-				   : "(MUX UNCLAIMED)",
-				   desc->gpio_owner ? desc->gpio_owner
-				   : "(GPIO UNCLAIMED)",
-				   is_hog ? " (HOG)" : "");
-		}
-
-		/* If mux: print function+group claiming the pin */
-		if (desc->mux_setting)
-			seq_printf(s, " function %s group %s\n",
-				   pmxops->get_function_name(pctldev,
-					desc->mux_setting->func),
-				   pctlops->get_group_name(pctldev,
-					desc->mux_setting->group));
-		else
-			seq_printf(s, "\n");
-	}
+	for (i = 0; i < pctldev->desc->npins; i++)
+		pinmux_pin_show(s, pctldev->desc->pins[i].number);
 
 	mutex_unlock(&pctldev->mutex);
Signature · 1 line
2.11.0
author
#2 lore raw

RFC PATCH 2/4 pinctrl: Rename mux group to group_or_pin to prepare for pin support

In reply to Charles Keepax

From: Charles Keepax <[email protected]>

To prepare for adding support for muxing individual pins rename the group member of the pinctrl_map_mux and pinctrl_setting_mux structs to group_or_pin.

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/pinctrl/bcm/pinctrl-bcm2835.c         |  2 +-
 drivers/pinctrl/core.h                        |  2 +-
 drivers/pinctrl/freescale/pinctrl-imx.c       |  5 ++--
 drivers/pinctrl/freescale/pinctrl-imx1-core.c |  5 ++--
 drivers/pinctrl/freescale/pinctrl-mxs.c       |  4 +--
 drivers/pinctrl/mediatek/pinctrl-mtk-common.c |  2 +-
 drivers/pinctrl/mvebu/pinctrl-mvebu.c         |  2 +-
 drivers/pinctrl/nomadik/pinctrl-abx500.c      |  2 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik.c     |  2 +-
 drivers/pinctrl/pinctrl-at91.c                |  5 ++--
 drivers/pinctrl/pinctrl-lantiq.c              |  2 +-
 drivers/pinctrl/pinctrl-rockchip.c            |  5 ++--
 drivers/pinctrl/pinctrl-rza1.c                |  2 +-
 drivers/pinctrl/pinctrl-single.c              |  4 +--
 drivers/pinctrl/pinctrl-st.c                  |  2 +-
 drivers/pinctrl/pinctrl-tz1090-pdc.c          |  2 +-
 drivers/pinctrl/pinctrl-tz1090.c              |  2 +-
 drivers/pinctrl/pinctrl-utils.c               |  2 +-
 drivers/pinctrl/pinmux.c                      | 37 +++++++++++++++------------
 drivers/pinctrl/samsung/pinctrl-exynos5440.c  |  4 +--
 drivers/pinctrl/samsung/pinctrl-samsung.c     |  2 +-
 drivers/pinctrl/sh-pfc/pinctrl.c              |  2 +-
 drivers/pinctrl/sirf/pinctrl-sirf.c           |  2 +-
 drivers/pinctrl/spear/pinctrl-spear.c         |  2 +-
 drivers/pinctrl/stm32/pinctrl-stm32.c         |  2 +-
 drivers/pinctrl/sunxi/pinctrl-sunxi.c         |  2 +-
 drivers/pinctrl/vt8500/pinctrl-wmt.c          |  2 +-
 include/linux/pinctrl/machine.h               |  2 +-
 28 files changed, 59 insertions(+), 50 deletions(-)
drivers/pinctrl/bcm/pinctrl-bcm2835.c +1 -1
index ff782445dfb7..3ce874c979d4 100644
@@ -692,7 +692,7 @@ static int bcm2835_pctl_dt_node_to_map_func(struct bcm2835_pinctrl *pc,
 	}
 
 	map->type = PIN_MAP_TYPE_MUX_GROUP;
-	map->data.mux.group = bcm2835_gpio_groups[pin];
+	map->data.mux.group_or_pin = bcm2835_gpio_groups[pin];
 	map->data.mux.function = bcm2835_functions[fnum];
 	(*maps)++;
drivers/pinctrl/core.h +1 -1
index 7880c3adc450..96a003a719e3 100644
@@ -102,7 +102,7 @@ struct pinctrl_state {
  * @func: the function selector to program
  */
 struct pinctrl_setting_mux {
-	unsigned group;
+	unsigned group_or_pin;
 	unsigned func;
 };
drivers/pinctrl/freescale/pinctrl-imx.c +3 -2
index 6e472691d8ee..4d8d526214c4 100644
@@ -102,7 +102,7 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 	new_map[0].type = PIN_MAP_TYPE_MUX_GROUP;
 	new_map[0].data.mux.function = parent->name;
-	new_map[0].data.mux.group = np->name;
+	new_map[0].data.mux.group_or_pin = np->name;
 	of_node_put(parent);
 
 	/* create config map */

@@ -121,7 +121,8 @@ static int imx_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 
 	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
-		(*map)->data.mux.function, (*map)->data.mux.group, map_num);
+		(*map)->data.mux.function, (*map)->data.mux.group_or_pin,
+		map_num);
 
 	return 0;
 }
drivers/pinctrl/freescale/pinctrl-imx1-core.c +3 -2
index a4e9f430d452..e7a76171e922 100644
@@ -261,7 +261,7 @@ static int imx1_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 	new_map[0].type = PIN_MAP_TYPE_MUX_GROUP;
 	new_map[0].data.mux.function = parent->name;
-	new_map[0].data.mux.group = np->name;
+	new_map[0].data.mux.group_or_pin = np->name;
 	of_node_put(parent);
 
 	/* create config map */

@@ -276,7 +276,8 @@ static int imx1_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 
 	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
-		(*map)->data.mux.function, (*map)->data.mux.group, map_num);
+		(*map)->data.mux.function, (*map)->data.mux.group_or_pin,
+		map_num);
 
 	return 0;
 }
drivers/pinctrl/freescale/pinctrl-mxs.c +2 -2
index 6852010a6d70..81ae9d6258b4 100644
@@ -111,7 +111,7 @@ static int mxs_dt_node_to_map(struct pinctrl_dev *pctldev,
 			goto free;
 		}
 		snprintf(group, length, "%s.%d", np->name, reg);
-		new_map[i].data.mux.group = group;
+		new_map[i].data.mux.group_or_pin = group;
 		i++;
 	}
 

@@ -149,7 +149,7 @@ static void mxs_dt_free_map(struct pinctrl_dev *pctldev,
 
 	for (i = 0; i < num_maps; i++) {
 		if (map[i].type == PIN_MAP_TYPE_MUX_GROUP)
-			kfree(map[i].data.mux.group);
+			kfree(map[i].data.mux.group_or_pin);
 		if (map[i].type == PIN_MAP_TYPE_CONFIGS_GROUP)
 			kfree(map[i].data.configs.configs);
 	}
drivers/pinctrl/mediatek/pinctrl-mtk-common.c +1 -1
index 3cf384f8b122..340dd0e84886 100644
@@ -475,7 +475,7 @@ static int mtk_pctrl_dt_node_to_map_func(struct mtk_pinctrl *pctl,
 		return -ENOSPC;
 
 	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)[*num_maps].data.mux.group = grp->name;
+	(*map)[*num_maps].data.mux.group_or_pin = grp->name;
 
 	ret = mtk_pctrl_is_function_valid(pctl, pin, fnum);
 	if (!ret) {
drivers/pinctrl/mvebu/pinctrl-mvebu.c +1 -1
index 163d4614b0f8..174ea2d5d6df 100644
@@ -445,7 +445,7 @@ static int mvebu_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
 		}
 
 		(*map)[n].type = PIN_MAP_TYPE_MUX_GROUP;
-		(*map)[n].data.mux.group = group;
+		(*map)[n].data.mux.group_or_pin = group;
 		(*map)[n].data.mux.function = function;
 		n++;
 	}
drivers/pinctrl/nomadik/pinctrl-abx500.c +1 -1
index b32c0d602024..06ae64c72a6d 100644
@@ -815,7 +815,7 @@ static int abx500_dt_add_map_mux(struct pinctrl_map **map,
 		return -ENOSPC;
 
 	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)[*num_maps].data.mux.group = group;
+	(*map)[*num_maps].data.mux.group_or_pin = group;
 	(*map)[*num_maps].data.mux.function = function;
 	(*num_maps)++;
drivers/pinctrl/nomadik/pinctrl-nomadik.c +1 -1
index a53f1a9b1ed2..b808a54ee0a5 100644
@@ -1270,7 +1270,7 @@ static int nmk_dt_add_map_mux(struct pinctrl_map **map, unsigned *reserved_maps,
 		return -ENOSPC;
 
 	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)[*num_maps].data.mux.group = group;
+	(*map)[*num_maps].data.mux.group_or_pin = group;
 	(*map)[*num_maps].data.mux.function = function;
 	(*num_maps)++;
drivers/pinctrl/pinctrl-at91.c +3 -2
index 569bc28cb909..2bbdb0e26a29 100644
@@ -284,7 +284,7 @@ static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 	new_map[0].type = PIN_MAP_TYPE_MUX_GROUP;
 	new_map[0].data.mux.function = parent->name;
-	new_map[0].data.mux.group = np->name;
+	new_map[0].data.mux.group_or_pin = np->name;
 	of_node_put(parent);
 
 	/* create config map */

@@ -298,7 +298,8 @@ static int at91_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 
 	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
-		(*map)->data.mux.function, (*map)->data.mux.group, map_num);
+		(*map)->data.mux.function, (*map)->data.mux.group_or_pin,
+		map_num);
 
 	return 0;
 }
drivers/pinctrl/pinctrl-lantiq.c +1 -1
index 41dc39c7a7b1..4d499df5d65f 100644
@@ -96,7 +96,7 @@ static void ltq_pinctrl_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		of_property_for_each_string(np, "lantiq,groups", prop, group) {
 			(*map)->type = PIN_MAP_TYPE_MUX_GROUP;
 			(*map)->name = function;
-			(*map)->data.mux.group = group;
+			(*map)->data.mux.group_or_pin = group;
 			(*map)->data.mux.function = function;
 			(*map)++;
 		}
drivers/pinctrl/pinctrl-rockchip.c +3 -2
index b5cb7858ffdc..e700a052715e 100644
@@ -522,7 +522,7 @@ static int rockchip_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 	new_map[0].type = PIN_MAP_TYPE_MUX_GROUP;
 	new_map[0].data.mux.function = parent->name;
-	new_map[0].data.mux.group = np->name;
+	new_map[0].data.mux.group_or_pin = np->name;
 	of_node_put(parent);
 
 	/* create config map */

@@ -536,7 +536,8 @@ static int rockchip_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 
 	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
-		(*map)->data.mux.function, (*map)->data.mux.group, map_num);
+		(*map)->data.mux.function, (*map)->data.mux.group_or_pin,
+		map_num);
 
 	return 0;
 }
drivers/pinctrl/pinctrl-rza1.c +1 -1
index 04d058706b80..8d1bd8591624 100644
@@ -952,7 +952,7 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev,
 	}
 
 	(*map)->type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)->data.mux.group = np->name;
+	(*map)->data.mux.group_or_pin = np->name;
 	(*map)->data.mux.function = np->name;
 	*num_maps = 1;
drivers/pinctrl/pinctrl-single.c +2 -2
index e6cd8de793e2..b48d9fb265c1 100644
@@ -1019,7 +1019,7 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs,
 		goto free_function;
 
 	(*map)->type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)->data.mux.group = np->name;
+	(*map)->data.mux.group_or_pin = np->name;
 	(*map)->data.mux.function = np->name;
 
 	if (PCS_HAS_PINCONF) {

@@ -1153,7 +1153,7 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs,
 		goto free_function;
 
 	(*map)->type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)->data.mux.group = np->name;
+	(*map)->data.mux.group_or_pin = np->name;
 	(*map)->data.mux.function = np->name;
 
 	if (PCS_HAS_PINCONF) {
drivers/pinctrl/pinctrl-st.c +1 -1
index a5205b94b2e6..87c1d0ba962d 100644
@@ -838,7 +838,7 @@ static int st_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
 	*num_maps = map_num;
 	new_map[0].type = PIN_MAP_TYPE_MUX_GROUP;
 	new_map[0].data.mux.function = parent->name;
-	new_map[0].data.mux.group = np->name;
+	new_map[0].data.mux.group_or_pin = np->name;
 	of_node_put(parent);
 
 	/* create config map per pin */
drivers/pinctrl/pinctrl-tz1090-pdc.c +1 -1
index 5cfa93cecf73..89a8ef08ed22 100644
@@ -318,7 +318,7 @@ static int add_map_mux(struct pinctrl_map **map, unsigned int *reserved_maps,
 		return -ENOSPC;
 
 	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)[*num_maps].data.mux.group = group;
+	(*map)[*num_maps].data.mux.group_or_pin = group;
 	(*map)[*num_maps].data.mux.function = function;
 	(*num_maps)++;
drivers/pinctrl/pinctrl-tz1090.c +1 -1
index 74d1ffcc2199..1f72a52102ab 100644
@@ -1062,7 +1062,7 @@ static int add_map_mux(struct pinctrl_map **map, unsigned int *reserved_maps,
 		return -ENOSPC;
 
 	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)[*num_maps].data.mux.group = group;
+	(*map)[*num_maps].data.mux.group_or_pin = group;
 	(*map)[*num_maps].data.mux.function = function;
 	(*num_maps)++;
drivers/pinctrl/pinctrl-utils.c +1 -1
index 9189fbafb102..d9c65a24eb30 100644
@@ -62,7 +62,7 @@ int pinctrl_utils_add_map_mux(struct pinctrl_dev *pctldev,
 		return -ENOSPC;
 
 	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)[*num_maps].data.mux.group = group;
+	(*map)[*num_maps].data.mux.group_or_pin = group;
 	(*map)[*num_maps].data.mux.function = function;
 	(*num_maps)++;
drivers/pinctrl/pinmux.c +21 -16
index e76e6f6a79d6..699b3e406d10 100644
@@ -348,8 +348,8 @@ int pinmux_map_to_setting(const struct pinctrl_map *map,
 			map->data.mux.function);
 		return -EINVAL;
 	}
-	if (map->data.mux.group) {
-		group = map->data.mux.group;
+	if (map->data.mux.group_or_pin) {
+		group = map->data.mux.group_or_pin;
 		ret = match_string(groups, num_groups, group);
 		if (ret < 0) {
 			dev_err(pctldev->dev,

@@ -364,10 +364,10 @@ int pinmux_map_to_setting(const struct pinctrl_map *map,
 	ret = pinctrl_get_group_selector(pctldev, group);
 	if (ret < 0) {
 		dev_err(pctldev->dev, "invalid group %s in map table\n",
-			map->data.mux.group);
+			map->data.mux.group_or_pin);
 		return ret;
 	}
-	setting->data.mux.group = ret;
+	setting->data.mux.group_or_pin = ret;
 
 	return 0;
 }

@@ -389,7 +389,8 @@ int pinmux_enable_setting(const struct pinctrl_setting *setting)
 	struct pin_desc *desc;
 
 	if (pctlops->get_group_pins)
-		ret = pctlops->get_group_pins(pctldev, setting->data.mux.group,
+		ret = pctlops->get_group_pins(pctldev,
+					      setting->data.mux.group_or_pin,
 					      &pins, &num_pins);
 
 	if (ret) {

@@ -397,7 +398,7 @@ int pinmux_enable_setting(const struct pinctrl_setting *setting)
 
 		/* errors only affect debug data, so just warn */
 		gname = pctlops->get_group_name(pctldev,
-						setting->data.mux.group);
+						setting->data.mux.group_or_pin);
 		dev_warn(pctldev->dev,
 			 "could not get pins for group %s\n",
 			 gname);

@@ -414,7 +415,7 @@ int pinmux_enable_setting(const struct pinctrl_setting *setting)
 			desc = pin_desc_get(pctldev, pins[i]);
 			pname = desc ? desc->name : "non-existing";
 			gname = pctlops->get_group_name(pctldev,
-						setting->data.mux.group);
+						setting->data.mux.group_or_pin);
 			dev_err(pctldev->dev,
 				"could not request pin %d (%s) from group %s "
 				" on device %s\n",

@@ -437,7 +438,7 @@ int pinmux_enable_setting(const struct pinctrl_setting *setting)
 	}
 
 	ret = ops->set_mux(pctldev, setting->data.mux.func,
-			   setting->data.mux.group);
+			   setting->data.mux.group_or_pin);
 
 	if (ret)
 		goto err_set_mux;

@@ -469,14 +470,15 @@ void pinmux_disable_setting(const struct pinctrl_setting *setting)
 	struct pin_desc *desc;
 
 	if (pctlops->get_group_pins)
-		ret = pctlops->get_group_pins(pctldev, setting->data.mux.group,
+		ret = pctlops->get_group_pins(pctldev,
+					      setting->data.mux.group_or_pin,
 					      &pins, &num_pins);
 	if (ret) {
 		const char *gname;
 
 		/* errors only affect debug data, so just warn */
 		gname = pctlops->get_group_name(pctldev,
-						setting->data.mux.group);
+						setting->data.mux.group_or_pin);
 		dev_warn(pctldev->dev,
 			 "could not get pins for group %s\n",
 			 gname);

@@ -500,7 +502,7 @@ void pinmux_disable_setting(const struct pinctrl_setting *setting)
 			const char *gname;
 
 			gname = pctlops->get_group_name(pctldev,
-						setting->data.mux.group);
+						setting->data.mux.group_or_pin);
 			dev_warn(pctldev->dev,
 				 "not freeing pin %d (%s) as part of "
 				 "deactivating group %s - it is already "

@@ -598,7 +600,7 @@ static int pinmux_pin_show(struct seq_file *s, unsigned int pin)
 			   pmxops->get_function_name(pctldev,
 				desc->mux_setting->func),
 			   pctlops->get_group_name(pctldev,
-				desc->mux_setting->group));
+				desc->mux_setting->group_or_pin));
 	else
 		seq_puts(s, "\n");
 

@@ -635,9 +637,11 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
 
 void pinmux_show_map(struct seq_file *s, const struct pinctrl_map *map)
 {
+	const struct pinctrl_map_mux *mux = &map->data.mux;
+
 	seq_printf(s, "group %s\nfunction %s\n",
-		map->data.mux.group ? map->data.mux.group : "(default)",
-		map->data.mux.function);
+		   mux->group_or_pin ? mux->group_or_pin : "(default)",
+		   mux->function);
 }
 
 void pinmux_show_setting(struct seq_file *s,

@@ -648,8 +652,9 @@ void pinmux_show_setting(struct seq_file *s,
 	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
 
 	seq_printf(s, "group: %s (%u) function: %s (%u)\n",
-		   pctlops->get_group_name(pctldev, setting->data.mux.group),
-		   setting->data.mux.group,
+		   pctlops->get_group_name(pctldev,
+					   setting->data.mux.group_or_pin),
+		   setting->data.mux.group_or_pin,
 		   pmxops->get_function_name(pctldev, setting->data.mux.func),
 		   setting->data.mux.func);
 }
drivers/pinctrl/samsung/pinctrl-exynos5440.c +2 -2
index 32a3a9fd65c4..b6dfba7ac249 100644
@@ -254,7 +254,7 @@ static int exynos5440_dt_node_to_map(struct pinctrl_dev *pctldev,
 		if (!fname)
 			goto free_cfg;
 
-		map[*nmaps].data.mux.group = gname;
+		map[*nmaps].data.mux.group_or_pin = gname;
 		map[*nmaps].data.mux.function = fname;
 		map[*nmaps].type = PIN_MAP_TYPE_MUX_GROUP;
 		*nmaps += 1;

@@ -282,7 +282,7 @@ static void exynos5440_dt_free_map(struct pinctrl_dev *pctldev,
 		if (map[idx].type == PIN_MAP_TYPE_MUX_GROUP) {
 			kfree(map[idx].data.mux.function);
 			if (!idx)
-				kfree(map[idx].data.mux.group);
+				kfree(map[idx].data.mux.group_or_pin);
 		} else if (map->type == PIN_MAP_TYPE_CONFIGS_GROUP) {
 			kfree(map[idx].data.configs.configs);
 			if (!idx)
drivers/pinctrl/samsung/pinctrl-samsung.c +1 -1
index e04f7fe0a65d..cd5b81446e49 100644
@@ -111,7 +111,7 @@ static int add_map_mux(struct pinctrl_map **map, unsigned *reserved_maps,
 		return -ENOSPC;
 
 	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)[*num_maps].data.mux.group = group;
+	(*map)[*num_maps].data.mux.group_or_pin = group;
 	(*map)[*num_maps].data.mux.function = function;
 	(*num_maps)++;
drivers/pinctrl/sh-pfc/pinctrl.c +1 -1
index 5c9d79981e6d..dc456b3e578f 100644
@@ -202,7 +202,7 @@ static int sh_pfc_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 	of_property_for_each_string(np, pmx->groups_prop_name, prop, group) {
 		if (function) {
 			maps[idx].type = PIN_MAP_TYPE_MUX_GROUP;
-			maps[idx].data.mux.group = group;
+			maps[idx].data.mux.group_or_pin = group;
 			maps[idx].data.mux.function = function;
 			idx++;
 		}
drivers/pinctrl/sirf/pinctrl-sirf.c +1 -1
index d3ef05973901..caf384eea5b1 100644
@@ -116,7 +116,7 @@ static int sirfsoc_dt_node_to_map(struct pinctrl_dev *pctldev,
 		of_property_read_string(np, "sirf,function", &function);
 		of_property_for_each_string(np, "sirf,pins", prop, group) {
 			(*map)[index].type = PIN_MAP_TYPE_MUX_GROUP;
-			(*map)[index].data.mux.group = group;
+			(*map)[index].data.mux.group_or_pin = group;
 			(*map)[index].data.mux.function = function;
 			index++;
 		}
drivers/pinctrl/spear/pinctrl-spear.c +1 -1
index 4db52ba38d8d..811bf0e718ff 100644
@@ -180,7 +180,7 @@ static int spear_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
 		of_property_read_string(np, "st,function", &function);
 		of_property_for_each_string(np, "st,pins", prop, group) {
 			(*map)[index].type = PIN_MAP_TYPE_MUX_GROUP;
-			(*map)[index].data.mux.group = group;
+			(*map)[index].data.mux.group_or_pin = group;
 			(*map)[index].data.mux.function = function;
 			index++;
 		}
drivers/pinctrl/stm32/pinctrl-stm32.c +1 -1
index 50299ad96659..ddf9f86ab550 100644
@@ -375,7 +375,7 @@ static int stm32_pctrl_dt_node_to_map_func(struct stm32_pinctrl *pctl,
 		return -ENOSPC;
 
 	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)[*num_maps].data.mux.group = grp->name;
+	(*map)[*num_maps].data.mux.group_or_pin = grp->name;
 
 	if (!stm32_pctrl_is_function_valid(pctl, pin, fnum)) {
 		dev_err(pctl->dev, "invalid function %d on pin %d .\n",
drivers/pinctrl/sunxi/pinctrl-sunxi.c +1 -1
index 52edf3b5988d..9e87ff53f0e7 100644
@@ -377,7 +377,7 @@ static int sunxi_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
 		}
 
 		(*map)[i].type = PIN_MAP_TYPE_MUX_GROUP;
-		(*map)[i].data.mux.group = group;
+		(*map)[i].data.mux.group_or_pin = group;
 		(*map)[i].data.mux.function = function;
 
 		i++;
drivers/pinctrl/vt8500/pinctrl-wmt.c +1 -1
index d73956bdc211..13d5e9ed0aa8 100644
@@ -232,7 +232,7 @@ static int wmt_pctl_dt_node_to_map_func(struct wmt_pinctrl_data *data,
 	}
 
 	map->type = PIN_MAP_TYPE_MUX_GROUP;
-	map->data.mux.group = data->groups[group];
+	map->data.mux.group_or_pin = data->groups[group];
 	map->data.mux.function = wmt_functions[fnum];
 	(*maps)++;
include/linux/pinctrl/machine.h +1 -1
index 7fa5d87190c2..ec58c3647eb8 100644
@@ -32,7 +32,7 @@ enum pinctrl_map_type {
  * @function: the mux function to select for the group
  */
 struct pinctrl_map_mux {
-	const char *group;
+	const char *group_or_pin;
 	const char *function;
 };
Signature · 1 line
2.11.0
author
#3 lore raw

Re: [PATCH RFC 2/4] pinctrl: Rename mux group to group_or_pin to prepare for pin support

Quoted 16 lines · Charles Keepax

From: Charles Keepax <[email protected]>

To prepare for adding support for muxing individual pins rename the group member of the pinctrl_map_mux and pinctrl_setting_mux structs to group_or_pin.

Signed-off-by Charles Keepax <[email protected]>
---
 include/linux/pinctrl/machine.h               |  2 +-
 28 files changed, 59 insertions(+), 50 deletions(-)
include/linux/pinctrl/machine.h +0 -0
index 7fa5d87190c2..ec58c3647eb8 100644

include/linux/pinctrl/machine.h enum pinctrl_map_type { · line 37

Quoted 7 lines
include/linux/pinctrl/machine.h +1 -1
@@ -32,7 +32,7 @@ enum pinctrl_map_type {
  * @function: the mux function to select for the group
  */
 struct pinctrl_map_mux {
-	const char *group;
+	const char *group_or_pin;
 	const char *function;
 };

One additional fix is required for PIN_MAP_MUX_GROUP in this file.

Thanks,
Charles

author
#4 lore raw

RFC PATCH 3/4 pinctrl: Add support for muxing individual pins

In reply to Charles Keepax

From: Charles Keepax <[email protected]>

Currently pinmuxing can only be applied to groups, this has lead to several drivers within the kernel adding many pin groups that just contain a single pin to allow muxing of individual pins.

Add support to the core for muxing individual pins. Groups are still used as a mechanism to determine if a function is suitable to be applied to a specific pin. For example a pin might be in the GPIO group allowing it to be muxed to a GPIO but it would now be possible to just mux one pin from that group.

This change makes no changes to the current DT parsing, everything will still be parsed into a group type map.

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/pinctrl/core.c          |  26 ++++-
 drivers/pinctrl/core.h          |   1 +
 drivers/pinctrl/pinmux.c        | 218 +++++++++++++++++++++++++++++-----------
 include/linux/pinctrl/machine.h |   1 +
 include/linux/pinctrl/pinmux.h  |   7 ++
 5 files changed, 192 insertions(+), 61 deletions(-)
drivers/pinctrl/core.c +22 -4
index 56fbe4c3e800..17994b690bc8 100644
@@ -950,6 +950,7 @@ static int add_setting(struct pinctrl *p, struct pinctrl_dev *pctldev,
 	setting->dev_name = map->dev_name;
 
 	switch (map->type) {
+	case PIN_MAP_TYPE_MUX_PIN:
 	case PIN_MAP_TYPE_MUX_GROUP:
 		ret = pinmux_map_to_setting(map, setting);
 		break;

@@ -1104,6 +1105,7 @@ static void pinctrl_free_setting(bool disable_setting,
 				 struct pinctrl_setting *setting)
 {
 	switch (setting->type) {
+	case PIN_MAP_TYPE_MUX_PIN:
 	case PIN_MAP_TYPE_MUX_GROUP:
 		if (disable_setting)
 			pinmux_disable_setting(setting);

@@ -1210,9 +1212,14 @@ int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
 		 * to pinmux_enable_setting() in the loop below.
 		 */
 		list_for_each_entry(setting, &p->state->settings, node) {
-			if (setting->type != PIN_MAP_TYPE_MUX_GROUP)
-				continue;
-			pinmux_disable_setting(setting);
+			switch (setting->type) {
+			case PIN_MAP_TYPE_MUX_GROUP:
+			case PIN_MAP_TYPE_MUX_PIN:
+				pinmux_disable_setting(setting);
+				break;
+			default:
+				break;
+			}
 		}
 	}
 

@@ -1221,6 +1228,7 @@ int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
 	/* Apply all the settings for the new state */
 	list_for_each_entry(setting, &state->settings, node) {
 		switch (setting->type) {
+		case PIN_MAP_TYPE_MUX_PIN:
 		case PIN_MAP_TYPE_MUX_GROUP:
 			ret = pinmux_enable_setting(setting);
 			break;

@@ -1255,8 +1263,14 @@ int pinctrl_select_state(struct pinctrl *p, struct pinctrl_state *state)
 		 * "unmux a pin"!), but it's not a big deal since the pins
 		 * are free to be muxed by another apply_setting.
 		 */
-		if (setting2->type == PIN_MAP_TYPE_MUX_GROUP)
+		switch (setting2->type) {
+		case PIN_MAP_TYPE_MUX_GROUP:
+		case PIN_MAP_TYPE_MUX_PIN:
 			pinmux_disable_setting(setting2);
+			break;
+		default:
+			break;
+		}
 	}
 
 	/* There's no infinite recursive loop here because p->state is NULL */

@@ -1353,6 +1367,7 @@ int pinctrl_register_map(const struct pinctrl_map *maps, unsigned num_maps,
 		switch (maps[i].type) {
 		case PIN_MAP_TYPE_DUMMY_STATE:
 			break;
+		case PIN_MAP_TYPE_MUX_PIN:
 		case PIN_MAP_TYPE_MUX_GROUP:
 			ret = pinmux_validate_map(&maps[i], i);
 			if (ret < 0)

@@ -1686,6 +1701,7 @@ static inline const char *map_type(enum pinctrl_map_type type)
 	static const char * const names[] = {
 		"INVALID",
 		"DUMMY_STATE",
+		"MUX_PIN",
 		"MUX_GROUP",
 		"CONFIGS_PIN",
 		"CONFIGS_GROUP",

@@ -1716,6 +1732,7 @@ static int pinctrl_maps_show(struct seq_file *s, void *what)
 				   map->ctrl_dev_name);
 
 		switch (map->type) {
+		case PIN_MAP_TYPE_MUX_PIN:
 		case PIN_MAP_TYPE_MUX_GROUP:
 			pinmux_show_map(s, map);
 			break;

@@ -1760,6 +1777,7 @@ static int pinctrl_show(struct seq_file *s, void *what)
 					   pinctrl_dev_get_name(pctldev));
 
 				switch (setting->type) {
+				case PIN_MAP_TYPE_MUX_PIN:
 				case PIN_MAP_TYPE_MUX_GROUP:
 					pinmux_show_setting(s, setting);
 					break;
drivers/pinctrl/core.h +1 -0
index 96a003a719e3..863132d7b217 100644
@@ -166,6 +166,7 @@ struct pin_desc {
 #ifdef CONFIG_PINMUX
 	unsigned mux_usecount;
 	const char *mux_owner;
+	enum pinctrl_map_type mux_type;
 	const struct pinctrl_setting_mux *mux_setting;
 	const char *gpio_owner;
 #endif
drivers/pinctrl/pinmux.c +161 -57

327 lines not on this page · raw message

include/linux/pinctrl/machine.h +1 -0
index ec58c3647eb8..c7e50fa8434e 100644
@@ -19,6 +19,7 @@
 enum pinctrl_map_type {
 	PIN_MAP_TYPE_INVALID,
 	PIN_MAP_TYPE_DUMMY_STATE,
+	PIN_MAP_TYPE_MUX_PIN,
 	PIN_MAP_TYPE_MUX_GROUP,
 	PIN_MAP_TYPE_CONFIGS_PIN,
 	PIN_MAP_TYPE_CONFIGS_GROUP,
include/linux/pinctrl/pinmux.h +7 -0
index ace60d775b20..1c315dcc1992 100644
@@ -45,6 +45,11 @@ struct pinctrl_dev;
  *	are handled by the pinmux subsystem. The @func_selector selects a
  *	certain function whereas @group_selector selects a certain set of pins
  *	to be used. On simple controllers the latter argument may be ignored
+ * @set_mux_pin: enable a certain muxing function with a certain pin. The driver
+ *	does not need to figure out whether enabling this function conflicts
+ *	some other use of the pin, such collisions are handled by the pinmux
+ *	subsystem. The @func_selector selects a certain function whereas
+ *	@pin_selector selects a certain pin to be used.
  * @gpio_request_enable: requests and enables GPIO on a certain pin.
  *	Implement this only if you can mux every pin individually as GPIO. The
  *	affected GPIO range is passed along with an offset(pin number) into that

@@ -72,6 +77,8 @@ struct pinmux_ops {
 				  unsigned *num_groups);
 	int (*set_mux) (struct pinctrl_dev *pctldev, unsigned func_selector,
 			unsigned group_selector);
+	int (*set_mux_pin) (struct pinctrl_dev *pctldev, unsigned func_selector,
+			    unsigned pin_selector);
 	int (*gpio_request_enable) (struct pinctrl_dev *pctldev,
 				    struct pinctrl_gpio_range *range,
 				    unsigned offset);
Signature · 1 line
2.11.0
author
#5 lore raw

RFC PATCH 4/4 pinctrl: Add support for parsing individual pinmux from DT

In reply to Charles Keepax

From: Charles Keepax <[email protected]>

So as to not break existing drivers that rely on pins properties being treated as a group, leave functionality of the existing generic parsing functions the same. Add a new parsing function pinctrl_generic_dt_node_to_map_all that will map pin properties from DT to the newly created pin type maps.

Signed-off-by Charles Keepax <[email protected]>
---
 drivers/pinctrl/pinconf-generic.c       | 30 +++++++++++++++++++-----------
 drivers/pinctrl/pinctrl-utils.c         |  8 ++++----
 drivers/pinctrl/pinctrl-utils.h         | 14 ++++++++++++--
 drivers/pinctrl/sirf/pinctrl-atlas7.c   |  3 ++-
 include/linux/pinctrl/pinconf-generic.h | 24 +++++++++++++++++++-----
 5 files changed, 56 insertions(+), 23 deletions(-)
drivers/pinctrl/pinconf-generic.c +19 -11
index 8eaa25c3384f..3f5cbe7e96dd 100644
@@ -287,7 +287,8 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
 int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np, struct pinctrl_map **map,
 		unsigned *reserved_maps, unsigned *num_maps,
-		enum pinctrl_map_type type)
+		enum pinctrl_map_type conf_type,
+		enum pinctrl_map_type mux_type)
 {
 	int ret;
 	const char *function;

@@ -305,12 +306,16 @@ int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		if (ret < 0)
 			/* skip this node; may contain config child nodes */
 			return 0;
-		if (type == PIN_MAP_TYPE_INVALID)
-			type = PIN_MAP_TYPE_CONFIGS_GROUP;
+		if (conf_type == PIN_MAP_TYPE_INVALID)
+			conf_type = PIN_MAP_TYPE_CONFIGS_GROUP;
+		if (mux_type == PIN_MAP_TYPE_INVALID)
+			mux_type = PIN_MAP_TYPE_MUX_GROUP;
 		subnode_target_type = "groups";
 	} else {
-		if (type == PIN_MAP_TYPE_INVALID)
-			type = PIN_MAP_TYPE_CONFIGS_PIN;
+		if (conf_type == PIN_MAP_TYPE_INVALID)
+			conf_type = PIN_MAP_TYPE_CONFIGS_PIN;
+		if (mux_type == PIN_MAP_TYPE_INVALID)
+			mux_type = PIN_MAP_TYPE_MUX_PIN;
 	}
 	strings_count = ret;
 

@@ -345,9 +350,9 @@ int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 
 	of_property_for_each_string(np, subnode_target_type, prop, group) {
 		if (function) {
-			ret = pinctrl_utils_add_map_mux(pctldev, map,
+			ret = pinctrl_utils_add_map_mux_type(pctldev, map,
 					reserved_maps, num_maps, group,
-					function);
+					function, mux_type);
 			if (ret < 0)
 				goto exit;
 		}

@@ -355,7 +360,7 @@ int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		if (num_configs) {
 			ret = pinctrl_utils_add_map_configs(pctldev, map,
 					reserved_maps, num_maps, group, configs,
-					num_configs, type);
+					num_configs, conf_type);
 			if (ret < 0)
 				goto exit;
 		}

@@ -370,7 +375,8 @@ EXPORT_SYMBOL_GPL(pinconf_generic_dt_subnode_to_map);
 
 int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np_config, struct pinctrl_map **map,
-		unsigned *num_maps, enum pinctrl_map_type type)
+		unsigned *num_maps, enum pinctrl_map_type conf_type,
+		enum pinctrl_map_type mux_type)
 {
 	unsigned reserved_maps;
 	struct device_node *np;

@@ -381,13 +387,15 @@ int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
 	*num_maps = 0;
 
 	ret = pinconf_generic_dt_subnode_to_map(pctldev, np_config, map,
-						&reserved_maps, num_maps, type);
+						&reserved_maps, num_maps,
+						conf_type, mux_type);
 	if (ret < 0)
 		goto exit;
 
 	for_each_available_child_of_node(np_config, np) {
 		ret = pinconf_generic_dt_subnode_to_map(pctldev, np, map,
-					&reserved_maps, num_maps, type);
+					&reserved_maps, num_maps,
+					conf_type, mux_type);
 		if (ret < 0)
 			goto exit;
 	}
drivers/pinctrl/pinctrl-utils.c +4 -4
index d9c65a24eb30..c7bb7d46bbbc 100644
@@ -53,22 +53,22 @@ int pinctrl_utils_reserve_map(struct pinctrl_dev *pctldev,
 }
 EXPORT_SYMBOL_GPL(pinctrl_utils_reserve_map);
 
-int pinctrl_utils_add_map_mux(struct pinctrl_dev *pctldev,
+int pinctrl_utils_add_map_mux_type(struct pinctrl_dev *pctldev,
 		struct pinctrl_map **map, unsigned *reserved_maps,
 		unsigned *num_maps, const char *group,
-		const char *function)
+		const char *function, enum pinctrl_map_type type)
 {
 	if (WARN_ON(*num_maps == *reserved_maps))
 		return -ENOSPC;
 
-	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
+	(*map)[*num_maps].type = type;
 	(*map)[*num_maps].data.mux.group_or_pin = group;
 	(*map)[*num_maps].data.mux.function = function;
 	(*num_maps)++;
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(pinctrl_utils_add_map_mux);
+EXPORT_SYMBOL_GPL(pinctrl_utils_add_map_mux_type);
 
 int pinctrl_utils_add_map_configs(struct pinctrl_dev *pctldev,
 		struct pinctrl_map **map, unsigned *reserved_maps,
drivers/pinctrl/pinctrl-utils.h +12 -2
index 8f9f2d28c5b8..fc3dfb3245ee 100644
@@ -25,10 +25,10 @@
 int pinctrl_utils_reserve_map(struct pinctrl_dev *pctldev,
 		struct pinctrl_map **map, unsigned *reserved_maps,
 		unsigned *num_maps, unsigned reserve);
-int pinctrl_utils_add_map_mux(struct pinctrl_dev *pctldev,
+int pinctrl_utils_add_map_mux_type(struct pinctrl_dev *pctldev,
 		struct pinctrl_map **map, unsigned *reserved_maps,
 		unsigned *num_maps, const char *group,
-		const char *function);
+		const char *function, enum pinctrl_map_type type);
 int pinctrl_utils_add_map_configs(struct pinctrl_dev *pctldev,
 		struct pinctrl_map **map, unsigned *reserved_maps,
 		unsigned *num_maps, const char *group,

@@ -40,4 +40,14 @@ int pinctrl_utils_add_config(struct pinctrl_dev *pctldev,
 void pinctrl_utils_free_map(struct pinctrl_dev *pctldev,
 		struct pinctrl_map *map, unsigned num_maps);
 
+static inline int pinctrl_utils_add_map_mux(struct pinctrl_dev *pctldev,
+		struct pinctrl_map **map, unsigned *reserved_maps,
+		unsigned *num_maps, const char *group,
+		const char *function)
+{
+	return pinctrl_utils_add_map_mux_type(pctldev, map, reserved_maps,
+					      num_maps, group, function,
+					      PIN_MAP_TYPE_MUX_GROUP);
+}
+
 #endif /* __PINCTRL_UTILS_H__ */
drivers/pinctrl/sirf/pinctrl-atlas7.c +2 -1
index 4db9323251e3..7407be0c176a 100644
@@ -5301,7 +5301,8 @@ static int atlas7_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
 					u32 *num_maps)
 {
 	return pinconf_generic_dt_node_to_map(pctldev, np_config, map,
-				num_maps, PIN_MAP_TYPE_INVALID);
+				num_maps, PIN_MAP_TYPE_INVALID,
+				PIN_MAP_TYPE_MUX_GROUP);
 }
 
 static void atlas7_pinctrl_dt_free_map(struct pinctrl_dev *pctldev,
include/linux/pinctrl/pinconf-generic.h +19 -5
index 5d8bc7f21c2a..6da4bad1b81c 100644
@@ -180,10 +180,12 @@ struct pinconf_generic_params {
 int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np, struct pinctrl_map **map,
 		unsigned *reserved_maps, unsigned *num_maps,
-		enum pinctrl_map_type type);
+		enum pinctrl_map_type conf_type,
+		enum pinctrl_map_type mux_type);
 int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np_config, struct pinctrl_map **map,
-		unsigned *num_maps, enum pinctrl_map_type type);
+		unsigned *num_maps, enum pinctrl_map_type conf_type,
+		enum pinctrl_map_type mux_type);
 void pinconf_generic_dt_free_map(struct pinctrl_dev *pctldev,
 		struct pinctrl_map *map, unsigned num_maps);
 

@@ -192,7 +194,7 @@ static inline int pinconf_generic_dt_node_to_map_group(
 		struct pinctrl_map **map, unsigned *num_maps)
 {
 	return pinconf_generic_dt_node_to_map(pctldev, np_config, map, num_maps,
-			PIN_MAP_TYPE_CONFIGS_GROUP);
+			PIN_MAP_TYPE_CONFIGS_GROUP, PIN_MAP_TYPE_MUX_GROUP);
 }
 
 static inline int pinconf_generic_dt_node_to_map_pin(

@@ -200,7 +202,7 @@ static inline int pinconf_generic_dt_node_to_map_pin(
 		struct pinctrl_map **map, unsigned *num_maps)
 {
 	return pinconf_generic_dt_node_to_map(pctldev, np_config, map, num_maps,
-			PIN_MAP_TYPE_CONFIGS_PIN);
+			PIN_MAP_TYPE_CONFIGS_PIN, PIN_MAP_TYPE_MUX_GROUP);
 }
 
 static inline int pinconf_generic_dt_node_to_map_all(

@@ -212,7 +214,19 @@ static inline int pinconf_generic_dt_node_to_map_all(
 	 * to infer the map type from the DT properties used.
 	 */
 	return pinconf_generic_dt_node_to_map(pctldev, np_config, map, num_maps,
-			PIN_MAP_TYPE_INVALID);
+			PIN_MAP_TYPE_INVALID, PIN_MAP_TYPE_MUX_GROUP);
+}
+
+static inline int pinctrl_generic_dt_node_to_map_all(
+		struct pinctrl_dev *pctldev, struct device_node *np_config,
+		struct pinctrl_map **map, unsigned *num_maps)
+{
+	/*
+	 * passing the type as PIN_MAP_TYPE_INVALID causes the underlying parser
+	 * to infer the map type from the DT properties used.
+	 */
+	return pinconf_generic_dt_node_to_map(pctldev, np_config, map, num_maps,
+			PIN_MAP_TYPE_INVALID, PIN_MAP_TYPE_INVALID);
 }
 #endif
Signature · 1 line
2.11.0
#6 lore raw
In reply to Charles Keepax
Quoted 11 lines · Charles Keepax

This series add support for muxing individual pins within pin mux, rather than just whole groups. Mainly, I had two motivations here, one to avoid the need to add loads of groups containing individual pins and hardware that actually has some internal concept of groups of pins, and disambiguating that from individual pin muxing. I have marked it as RFC to just get peoples opinions at this stage, although it should be pretty well tested. Sorry about the amount of files touched in patch 2 it would be possible to drop it from the chain although it leaves the field rather inaccurately named.

Quoted 3 lines

Also I have left all the existing code paths parsing all mux options as groups from DT, and added a new helper to unlock the pin based functionality this should ease the transition across.

There is currently a driver in the pin control subsystem that handles individual pins and that is pinctrl-single.c.

The driver is deployed for single pins muxed by a single register, and if this infrastructure is to be deployed it must be applied also in pinctrl-single. We cannot have several ways of doing the same thing, that way lies madness.

So you need Tony Lindgren's review and direction on this patch series.

I see the problem you are setting out to solve. I too have ran into the situation (on systems such as Qualcomm's) where single-pin groups are more rule than exception. It would be good to alleviate this and handle it in the core somehow.

Yours,
Linus Walleij

author
#7 lore raw
Quoted 18 lines · Linus Walleij
Quoted 11 lines · Charles Keepax

This series add support for muxing individual pins within pin mux, rather than just whole groups. Mainly, I had two motivations here, one to avoid the need to add loads of groups containing individual pins and hardware that actually has some internal concept of groups of pins, and disambiguating that from individual pin muxing. I have marked it as RFC to just get peoples opinions at this stage, although it should be pretty well tested. Sorry about the amount of files touched in patch 2 it would be possible to drop it from the chain although it leaves the field rather inaccurately named.

Quoted 3 lines

Also I have left all the existing code paths parsing all mux options as groups from DT, and added a new helper to unlock the pin based functionality this should ease the transition across.

Quoted 8 lines

There is currently a driver in the pin control subsystem that handles individual pins and that is pinctrl-single.c.

The driver is deployed for single pins muxed by a single register, and if this infrastructure is to be deployed it must be applied also in pinctrl-single. We cannot have several ways of doing the same thing, that way lies madness.

Apologies for the oversight I will have a look at that and try to update the series as appropriate. I am travelling for the next couple of weeks so there might be a small lag in getting an updated version out.

Quoted 3 lines

So you need Tony Lindgren's review and direction on this patch series.

Any thoughts would be greatly appreciated.

Quoted 4 lines

I see the problem you are setting out to solve. I too have ran into the situation (on systems such as Qualcomm's) where single-pin groups are more rule than exception. It would be good to alleviate this and handle it in the core somehow.

Thanks,
Charles

author
#8 lore raw
In reply to Linus Walleij
Quoted 19 lines · Linus Walleij
Quoted 11 lines · Charles Keepax

This series add support for muxing individual pins within pin mux, rather than just whole groups. Mainly, I had two motivations here, one to avoid the need to add loads of groups containing individual pins and hardware that actually has some internal concept of groups of pins, and disambiguating that from individual pin muxing. I have marked it as RFC to just get peoples opinions at this stage, although it should be pretty well tested. Sorry about the amount of files touched in patch 2 it would be possible to drop it from the chain although it leaves the field rather inaccurately named.

Quoted 3 lines

Also I have left all the existing code paths parsing all mux options as groups from DT, and added a new helper to unlock the pin based functionality this should ease the transition across.

There is currently a driver in the pin control subsystem that handles individual pins and that is pinctrl-single.c.

Quoted 7 lines

The driver is deployed for single pins muxed by a single register, and if this infrastructure is to be deployed it must be applied also in pinctrl-single. We cannot have several ways of doing the same thing, that way lies madness.

So you need Tony Lindgren's review and direction on this patch series.

Apologies for the delay on this one, I got some what snowed under with other tasks. Please let me know if you would rather I just resent the series to refresh everyones memory. But I have finally managed to get some time to look over the pinctrl-single stuff.

Naively one could convert the pinctrl-single stuff over to use the patches I proposed creating one large group for the driver and then mux each pin individually from within that. However I am not really sure it would make sense. From the implementation so far the pinctrl-single stuff appears to target systems where there isn't really a concept of groups. Each pin is just a completely separate entry and you can only configure things one pin at a time. In that case it almost makes more sense to model each pin as an individual group such that it is clearly distinct from the others. My thinking had been more along the lines of you perhaps have a group that represents an I2S port but you can also individually assign each of those pins as a GPIO when not in use as the I2S port.

Alternatively one could perhaps look at expanding the pinctrl-single stuff to have some notion of groups as well. So you could define sub groups of the pins that can be set as a block. Allowing users to configure either groups or individual pins.

I guess my main question is what is the intention of the pinctrl-single code, is this something that should be seeing expansion to handle groups as well or is it firmly for the unrelated pins cases?

Thanks,
Charles

#9 lore raw
Quoted 16 lines · Charles Keepax

(...) I have finally
managed to get some time to look over the pinctrl-single stuff.

Naively one could convert the pinctrl-single stuff over to use the patches I proposed creating one large group for the driver and then mux each pin individually from within that. However I am not really sure it would make sense. From the implementation so far the pinctrl-single stuff appears to target systems where there isn't really a concept of groups. Each pin is just a completely separate entry and you can only configure things one pin at a time. In that case it almost makes more sense to model each pin as an individual group such that it is clearly distinct from the others. My thinking had been more along the lines of you perhaps have a group that represents an I2S port but you can also individually assign each of those pins as a GPIO when not in use as the I2S port.

So then I toss the qcom driver into the game instead :)

If you look at drivers/pinctrl/qcom/* e.g. pinctrl-ipq4019.c or essentially any of the subdrivers, you find exactly this scenario.

I am concerned that if we add infrastructure for this, it needs to have more than one user. Qualcomm does fit your description above I think.

Yours,
Linus Walleij

author
#10 lore raw
Quoted 20 lines · Linus Walleij
Quoted 16 lines · Charles Keepax

(...) I have finally
managed to get some time to look over the pinctrl-single stuff.

Naively one could convert the pinctrl-single stuff over to use the patches I proposed creating one large group for the driver and then mux each pin individually from within that. However I am not really sure it would make sense. From the implementation so far the pinctrl-single stuff appears to target systems where there isn't really a concept of groups. Each pin is just a completely separate entry and you can only configure things one pin at a time. In that case it almost makes more sense to model each pin as an individual group such that it is clearly distinct from the others. My thinking had been more along the lines of you perhaps have a group that represents an I2S port but you can also individually assign each of those pins as a GPIO when not in use as the I2S port.

Quoted 9 lines

So then I toss the qcom driver into the game instead :)

If you look at drivers/pinctrl/qcom/* e.g. pinctrl-ipq4019.c or essentially any of the subdrivers, you find exactly this scenario.

I am concerned that if we add infrastructure for this, it needs to have more than one user. Qualcomm does fit your description above I think.

Yeah I could certainly have a hunt through for other users that would make good candidates to update. The QC driver certainly looks like it would be capable of muxing individual pins, although it looks like it might not let you mux an individual GPIO at the moment, need to dig into that more.

I guess maybe a bigger question is do we think this is a problem worth solving or should really just be adding a group for each pin I want to be able to mux?

Thanks,
Charles

#11 lore raw
Quoted 31 lines · Charles Keepax
Quoted 20 lines · Linus Walleij
Quoted 16 lines · Charles Keepax

(...) I have finally
managed to get some time to look over the pinctrl-single stuff.

Naively one could convert the pinctrl-single stuff over to use the patches I proposed creating one large group for the driver and then mux each pin individually from within that. However I am not really sure it would make sense. From the implementation so far the pinctrl-single stuff appears to target systems where there isn't really a concept of groups. Each pin is just a completely separate entry and you can only configure things one pin at a time. In that case it almost makes more sense to model each pin as an individual group such that it is clearly distinct from the others. My thinking had been more along the lines of you perhaps have a group that represents an I2S port but you can also individually assign each of those pins as a GPIO when not in use as the I2S port.

Quoted 9 lines

So then I toss the qcom driver into the game instead :)

If you look at drivers/pinctrl/qcom/* e.g. pinctrl-ipq4019.c or essentially any of the subdrivers, you find exactly this scenario.

I am concerned that if we add infrastructure for this, it needs to have more than one user. Qualcomm does fit your description above I think.

Quoted 6 lines

Yeah I could certainly have a hunt through for other users that would make good candidates to update. The QC driver certainly looks like it would be capable of muxing individual pins, although it looks like it might not let you mux an individual GPIO at the moment, need to dig into that more.

In the Qualcomm hardware we have X GPIO pins and some number of special purpose pins. GPIO pins can be muxed and controlled individually and the special purpose pins (e.g. SDCC) are controlled as a group.

I expected to describe both of these types as "pinctrl groups", but after struggling with this I realized that as the control is on pin-granularity (for the GPIOs) this is what should be represented as a group in pinctl/pinmux/pinconf.

So each GPIO pin can be controlled/muxed individually and as such is represented as a group, special groups are describes as one group (matching the single set of registers).

To configure a "logical group" (e.g. a UART), we list each pin (technically group), specify the same function and the necessary configuration options.

So if you hardware supports configuring individual pins within a group my suggestion is that you should rework the driver to make "group" match "configurable thing" and then tie together logical groups when configuring your system.

PS. A side effect of this is that e.g. UART is a function of 4 pins, but by specifying the pinmux/pinconf of only two of them I have a 2 pin UART, without having to describe this setup in the pinctrl driver.

Regards,
Bjorn

maintainer
#12 lore raw
In reply to Charles Keepax
Quoted 6 lines · Charles Keepax

From the implementation so far the pinctrl-single stuff appears to target systems where there isn't really a concept of groups. Each pin is just a completely separate entry and you can only configure things one pin at a time. In that case it almost makes more sense to model each pin as an individual group such that it is clearly distinct from the others.

Maybe check again or else I don't follow you :)

The pinctrl groups are created dynamically with pinctrl-single based on how the pins are grouped in the dts file:

mmc1_pins: pinmux_mmc1_pins {
	pinctrl-single,pins = <
		OMAP4_IOPAD(0x0e2, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e4, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e6, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e8, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0ea, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0ec, PIN_INPUT_PULLUP | MUX_MODE0)
	>;
};

Then on a booted system we have the following under /sys/kernel/debug/pinctrl/4a100040.pinmux/pingroups:

group: pinmux_mmc1_pins
pin 81 (PIN81)
pin 82 (PIN82)
pin 83 (PIN83)
pin 84 (PIN84)
pin 85 (PIN85)
pin 86 (PIN86)

Regards,

Tony

author
#13 lore raw
Quoted 27 lines · Tony Lindgren
Quoted 6 lines · Charles Keepax

From the implementation so far the pinctrl-single stuff appears to target systems where there isn't really a concept of groups. Each pin is just a completely separate entry and you can only configure things one pin at a time. In that case it almost makes more sense to model each pin as an individual group such that it is clearly distinct from the others.

Maybe check again or else I don't follow you :)

The pinctrl groups are created dynamically with pinctrl-single based on how the pins are grouped in the dts file:

mmc1_pins: pinmux_mmc1_pins {
	pinctrl-single,pins = <
		OMAP4_IOPAD(0x0e2, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e4, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e6, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e8, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0ea, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0ec, PIN_INPUT_PULLUP | MUX_MODE0)
	>;
};

Then on a booted system we have the following under /sys/kernel/debug/pinctrl/4a100040.pinmux/pingroups:

Quoted 8 lines

group: pinmux_mmc1_pins
pin 81 (PIN81)
pin 82 (PIN82)
pin 83 (PIN83)
pin 84 (PIN84)
pin 85 (PIN85)
pin 86 (PIN86)

Hmm... apologies, I suspect this is me that needs to review the code some more. So this is actually creating a group per user rather than per controller.

Although I guess my original point still stands that I am not clear how this would benefit from my patches to allow muxing of individual pins within a group. Since I guess each user will just get a group created for the pins they use regardless. Or am I missing something here too?

Thanks,
Charles

maintainer
#14 lore raw
Quoted 41 lines · Charles Keepax
Quoted 27 lines · Tony Lindgren
Quoted 6 lines · Charles Keepax

From the implementation so far the pinctrl-single stuff appears to target systems where there isn't really a concept of groups. Each pin is just a completely separate entry and you can only configure things one pin at a time. In that case it almost makes more sense to model each pin as an individual group such that it is clearly distinct from the others.

Maybe check again or else I don't follow you :)

The pinctrl groups are created dynamically with pinctrl-single based on how the pins are grouped in the dts file:

mmc1_pins: pinmux_mmc1_pins {
	pinctrl-single,pins = <
		OMAP4_IOPAD(0x0e2, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e4, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e6, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e8, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0ea, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0ec, PIN_INPUT_PULLUP | MUX_MODE0)
	>;
};

Then on a booted system we have the following under /sys/kernel/debug/pinctrl/4a100040.pinmux/pingroups:

Quoted 8 lines

group: pinmux_mmc1_pins
pin 81 (PIN81)
pin 82 (PIN82)
pin 83 (PIN83)
pin 84 (PIN84)
pin 85 (PIN85)
pin 86 (PIN86)

Hmm... apologies, I suspect this is me that needs to review the code some more. So this is actually creating a group per user rather than per controller.

Quoted 5 lines

Although I guess my original point still stands that I am not clear how this would benefit from my patches to allow muxing of individual pins within a group. Since I guess each user will just get a group created for the pins they use regardless. Or am I missing something here too?

Yeah it seems pinctrl-single does not need anything specific for muxing individual pins within a group. You can already do that quite easily by creating multiple groups in the dts for the device. Let's assume you need to dynamically remux MMC dat1 line between GPIO and MMC mode (similar to what we do on am335x for dat1 wake-up interrupts). In that case you would just modify the grouping example above with:

mmc1_dat1_pin: pinmux_mmc1_dat1_pin {
	pinctrl-single,pins = <
		OMAP4_IOPAD(0x0e8, PIN_INPUT_PULLUP | MUX_MODE0)
	>;
};
mmc1_pins: pinmux_mmc1_pins {
	pinctrl-single,pins = <
		OMAP4_IOPAD(0x0e2, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e4, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0e6, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0ea, PIN_INPUT_PULLUP | MUX_MODE0)
		OMAP4_IOPAD(0x0ec, PIN_INPUT_PULLUP | MUX_MODE0)
	>;
};

And that allows you to dynamically change dat1 pin only.

Not sure if that helps in your case though, just trying to clarify how it currently works :)

Regards,

Tony