diff mbox

[V2,2/3] pinctrl: allow concurrent gpio and mux function ownership of pins

Message ID 1330993336-9538-2-git-send-email-swarren@nvidia.com
State Not Applicable, archived
Headers show

Commit Message

Stephen Warren March 6, 2012, 12:22 a.m. UTC
Per recent updates to Documentation/gpio.txt, gpiolib drivers should
inform pinctrl when a GPIO is requested. pinctrl then marks that pin as
in-use for that GPIO function.

When an SoC muxes pins in a group, it's quite possible for the group to
contain e.g. 6 pins, but only 4 of them actually be needed by the HW
module that's mux'd to them. In this case, the other 2 pins could be
used as GPIOs. However, pinctrl marks all the pins within the group as
in-use by the selected mux function. To allow the expected gpiolib
interaction, separate the concepts of pin ownership into two parts: One
for the mux function and one for GPIO usage. Finally, allow those two
ownerships to exist in parallel.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2: New patch.
---
 drivers/pinctrl/core.h   |   13 ++++----
 drivers/pinctrl/pinmux.c |   72 +++++++++++++++++++++++++++++++--------------
 2 files changed, 56 insertions(+), 29 deletions(-)

Comments

Linus Walleij March 6, 2012, 10:03 a.m. UTC | #1
On Tue, Mar 6, 2012 at 1:22 AM, Stephen Warren <swarren@nvidia.com> wrote:

> Per recent updates to Documentation/gpio.txt, gpiolib drivers should
> inform pinctrl when a GPIO is requested. pinctrl then marks that pin as
> in-use for that GPIO function.
>
> When an SoC muxes pins in a group, it's quite possible for the group to
> contain e.g. 6 pins, but only 4 of them actually be needed by the HW
> module that's mux'd to them. In this case, the other 2 pins could be
> used as GPIOs. However, pinctrl marks all the pins within the group as
> in-use by the selected mux function. To allow the expected gpiolib
> interaction, separate the concepts of pin ownership into two parts: One
> for the mux function and one for GPIO usage. Finally, allow those two
> ownerships to exist in parallel.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Good, and 100% in accordance with earlier discussions.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Thanks,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Warren March 12, 2012, 6:24 p.m. UTC | #2
On 03/06/2012 03:03 AM, Linus Walleij wrote:
> On Tue, Mar 6, 2012 at 1:22 AM, Stephen Warren <swarren@nvidia.com> wrote:
> 
>> Per recent updates to Documentation/gpio.txt, gpiolib drivers should
>> inform pinctrl when a GPIO is requested. pinctrl then marks that pin as
>> in-use for that GPIO function.
>>
>> When an SoC muxes pins in a group, it's quite possible for the group to
>> contain e.g. 6 pins, but only 4 of them actually be needed by the HW
>> module that's mux'd to them. In this case, the other 2 pins could be
>> used as GPIOs. However, pinctrl marks all the pins within the group as
>> in-use by the selected mux function. To allow the expected gpiolib
>> interaction, separate the concepts of pin ownership into two parts: One
>> for the mux function and one for GPIO usage. Finally, allow those two
>> ownerships to exist in parallel.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> Good, and 100% in accordance with earlier discussions.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Linus, now that Grant has ack'd and applied the documentation change
behind this, could you take this one patch into the pinctrl tree. I
assume it's 3.5 material. After the 3.4 merge window closes, I'll
probably come back and ask for a stable pinctrl branch that I can use as
the basis for a Tegra branch that'll contain patch 3 in this series.

(Olof, I assume that's the right way to approach this; having Linus
apply both patches 2/3 in this series to pinctrl is going to make life
difficult in the Tegra tree, since my outstanding patches to convert
Tegra to use pinctrl will touch many of the same files as patch 3 in
this series, in conflicting or dependent ways.)

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij March 12, 2012, 9:42 p.m. UTC | #3
On Mon, Mar 12, 2012 at 7:24 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:

> Linus, now that Grant has ack'd and applied the documentation change
> behind this, could you take this one patch into the pinctrl tree.

Of course, applied.

> I assume it's 3.5 material.

No it goes in now, we can't have documentation that is misleading...
Besides, I like it.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olof Johansson March 14, 2012, 5:27 p.m. UTC | #4
On Mon, Mar 12, 2012 at 12:24:35PM -0600, Stephen Warren wrote:
> On 03/06/2012 03:03 AM, Linus Walleij wrote:
> > On Tue, Mar 6, 2012 at 1:22 AM, Stephen Warren <swarren@nvidia.com> wrote:
> > 
> >> Per recent updates to Documentation/gpio.txt, gpiolib drivers should
> >> inform pinctrl when a GPIO is requested. pinctrl then marks that pin as
> >> in-use for that GPIO function.
> >>
> >> When an SoC muxes pins in a group, it's quite possible for the group to
> >> contain e.g. 6 pins, but only 4 of them actually be needed by the HW
> >> module that's mux'd to them. In this case, the other 2 pins could be
> >> used as GPIOs. However, pinctrl marks all the pins within the group as
> >> in-use by the selected mux function. To allow the expected gpiolib
> >> interaction, separate the concepts of pin ownership into two parts: One
> >> for the mux function and one for GPIO usage. Finally, allow those two
> >> ownerships to exist in parallel.
> >>
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > 
> > Good, and 100% in accordance with earlier discussions.
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Linus, now that Grant has ack'd and applied the documentation change
> behind this, could you take this one patch into the pinctrl tree. I
> assume it's 3.5 material. After the 3.4 merge window closes, I'll
> probably come back and ask for a stable pinctrl branch that I can use as
> the basis for a Tegra branch that'll contain patch 3 in this series.
> 
> (Olof, I assume that's the right way to approach this; having Linus
> apply both patches 2/3 in this series to pinctrl is going to make life
> difficult in the Tegra tree, since my outstanding patches to convert
> Tegra to use pinctrl will touch many of the same files as patch 3 in
> this series, in conflicting or dependent ways.)

Yes, that seems like a good way to handle it.


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Olof Johansson March 14, 2012, 5:29 p.m. UTC | #5
On Mon, Mar 12, 2012 at 10:42:48PM +0100, Linus Walleij wrote:
> On Mon, Mar 12, 2012 at 7:24 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> 
> > Linus, now that Grant has ack'd and applied the documentation change
> > behind this, could you take this one patch into the pinctrl tree.
> 
> Of course, applied.
> 
> > I assume it's 3.5 material.
> 
> No it goes in now, we can't have documentation that is misleading...
> Besides, I like it.

Oops, this reply didn't show before I hit reply on the other one.

Stephen, that makes things easy for you if you still need time to sort things
out on the tegra side, since 3.4-rc1 can be the base for that work.


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 5b3ff13..2d365a3 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -118,15 +118,15 @@  struct pinctrl_setting {
  * @name: a name for the pin, e.g. the name of the pin/pad/finger on a
  *	datasheet or such
  * @dynamic_name: if the name of this pin was dynamically allocated
- * @usecount: If zero, the pin is not claimed, and @owner should be NULL.
+ * @mux_usecount: If zero, the pin is not claimed, and @owner should be NULL.
  *	If non-zero, this pin is claimed by @owner. This field is an integer
  *	rather than a boolean, since pinctrl_get() might process multiple
  *	mapping table entries that refer to, and hence claim, the same group
  *	or pin, and each of these will increment the @usecount.
- * @owner: The name of the entity owning the pin. Typically, this is the name
- *	of the device that called pinctrl_get(). Alternatively, it may be the
- *	name of the GPIO passed to pinctrl_request_gpio().
+ * @mux_owner: The name of device that called pinctrl_get().
  * @mux_setting: The most recent selected mux setting for this pin, if any.
+ * @gpio_owner: If pinctrl_request_gpio() was called for this pin, this is
+ *	the name of the GPIO that "owns" this pin.
  */
 struct pin_desc {
 	struct pinctrl_dev *pctldev;
@@ -134,9 +134,10 @@  struct pin_desc {
 	bool dynamic_name;
 	/* These fields only added when supporting pinmux drivers */
 #ifdef CONFIG_PINMUX
-	unsigned usecount;
-	const char *owner;
+	unsigned mux_usecount;
+	const char *mux_owner;
 	const struct pinctrl_setting_mux *mux_setting;
+	const char *gpio_owner;
 #endif
 };
 
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 86e4017..4e62783 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -94,17 +94,28 @@  static int pin_request(struct pinctrl_dev *pctldev,
 		goto out;
 	}
 
-	if (desc->usecount && strcmp(desc->owner, owner)) {
-		dev_err(pctldev->dev,
-			"pin already requested\n");
-		goto out;
-	}
+	if (gpio_range) {
+		/* There's no need to support multiple GPIO requests */
+		if (desc->gpio_owner) {
+			dev_err(pctldev->dev,
+				"pin already requested\n");
+			goto out;
+		}
 
-	desc->usecount++;
-	if (desc->usecount > 1)
-		return 0;
+		desc->gpio_owner = owner;
+	} else {
+		if (desc->mux_usecount && strcmp(desc->mux_owner, owner)) {
+			dev_err(pctldev->dev,
+				"pin already requested\n");
+			goto out;
+		}
 
-	desc->owner = owner;
+		desc->mux_usecount++;
+		if (desc->mux_usecount > 1)
+			return 0;
+
+		desc->mux_owner = owner;
+	}
 
 	/* Let each pin increase references to this module */
 	if (!try_module_get(pctldev->owner)) {
@@ -135,9 +146,13 @@  static int pin_request(struct pinctrl_dev *pctldev,
 
 out_free_pin:
 	if (status) {
-		desc->usecount--;
-		if (!desc->usecount)
-			desc->owner = NULL;
+		if (gpio_range) {
+			desc->gpio_owner = NULL;
+		} else {
+			desc->mux_usecount--;
+			if (!desc->mux_usecount)
+				desc->mux_owner = NULL;
+		}
 	}
 out:
 	if (status)
@@ -172,9 +187,11 @@  static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
 		return NULL;
 	}
 
-	desc->usecount--;
-	if (desc->usecount)
-		return NULL;
+	if (!gpio_range) {
+		desc->mux_usecount--;
+		if (desc->mux_usecount)
+			return NULL;
+	}
 
 	/*
 	 * If there is no kind of request function for the pin we just assume
@@ -185,9 +202,15 @@  static const char *pin_free(struct pinctrl_dev *pctldev, int pin,
 	else if (ops->free)
 		ops->free(pctldev, pin);
 
-	owner = desc->owner;
-	desc->owner = NULL;
-	desc->mux_setting = NULL;
+	if (gpio_range) {
+		owner = desc->gpio_owner;
+		desc->gpio_owner = NULL;
+	} else {
+		owner = desc->mux_owner;
+		desc->mux_owner = NULL;
+		desc->mux_setting = NULL;
+	}
+
 	module_put(pctldev->owner);
 
 	return owner;
@@ -493,7 +516,7 @@  static int pinmux_pins_show(struct seq_file *s, void *what)
 	unsigned i, pin;
 
 	seq_puts(s, "Pinmux settings per pin\n");
-	seq_puts(s, "Format: pin (name): owner\n");
+	seq_puts(s, "Format: pin (name): mux_owner gpio_owner hog?\n");
 
 	mutex_lock(&pinctrl_mutex);
 
@@ -508,13 +531,16 @@  static int pinmux_pins_show(struct seq_file *s, void *what)
 		if (desc == NULL)
 			continue;
 
-		if (desc->owner &&
-		    !strcmp(desc->owner, pinctrl_dev_get_name(pctldev)))
+		if (desc->mux_owner &&
+		    !strcmp(desc->mux_owner, pinctrl_dev_get_name(pctldev)))
 			is_hog = true;
 
-		seq_printf(s, "pin %d (%s): %s%s", pin,
+		seq_printf(s, "pin %d (%s): %s %s%s", pin,
 			   desc->name ? desc->name : "unnamed",
-			   desc->owner ? desc->owner : "UNCLAIMED",
+			   desc->mux_owner ? desc->mux_owner
+				: "(MUX UNCLAIMED)",
+			   desc->gpio_owner ? desc->gpio_owner
+				: "(GPIO UNCLAIMED)",
 			   is_hog ? " (HOG)" : "");
 
 		if (desc->mux_setting)