[PATCH 1/4] pinctrl: get_group_pins() const fixes

[PATCH 1/4] pinctrl: get_group_pins() const fixes

[Stephen Warren]

get_group_pins() "returns" a pointer to an array of const objects, through
a pointer parameter. Fix the prototype so what's pointed at by the returned
pointer is const, rather than the function parameter being const.

This also allows the removal of a cast in each of the two current pinmux
drivers.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/pinctrl/core.c          |    2 +-
 drivers/pinctrl/pinmux-sirf.c   |    6 +++---
 drivers/pinctrl/pinmux-u300.c   |    6 +++---
 drivers/pinctrl/pinmux.c        |    4 ++--
 include/linux/pinctrl/pinctrl.h |    4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 623afbd..97a4eeb 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -332,7 +332,7 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
 
 	seq_puts(s, "registered pin groups:\n");
 	while (ops->list_groups(pctldev, selector) >= 0) {
-		unsigned *pins;
+		const unsigned *pins;
 		unsigned num_pins;
 		const char *gname = ops->get_group_name(pctldev, selector);
 		int ret;
diff --git a/drivers/pinctrl/pinmux-sirf.c b/drivers/pinctrl/pinmux-sirf.c
index de5c78a..cad4f5d 100644
--- a/drivers/pinctrl/pinmux-sirf.c
+++ b/drivers/pinctrl/pinmux-sirf.c
@@ -869,12 +869,12 @@ static const char *sirfsoc_get_group_name(struct pinctrl_dev *pctldev,
 }
 
 static int sirfsoc_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
-			       unsigned ** const pins,
-			       unsigned * const num_pins)
+			       const unsigned **pins,
+			       const unsigned *num_pins)
 {
 	if (selector >= ARRAY_SIZE(sirfsoc_pin_groups))
 		return -EINVAL;
-	*pins = (unsigned *) sirfsoc_pin_groups[selector].pins;
+	*pins = sirfsoc_pin_groups[selector].pins;
 	*num_pins = sirfsoc_pin_groups[selector].num_pins;
 	return 0;
 }
diff --git a/drivers/pinctrl/pinmux-u300.c b/drivers/pinctrl/pinmux-u300.c
index 23f082f..be6e04d 100644
--- a/drivers/pinctrl/pinmux-u300.c
+++ b/drivers/pinctrl/pinmux-u300.c
@@ -849,12 +849,12 @@ static const char *u300_get_group_name(struct pinctrl_dev *pctldev,
 }
 
 static int u300_get_group_pins(struct pinctrl_dev *pctldev, unsigned selector,
-			       unsigned ** const pins,
-			       unsigned * const num_pins)
+			       const unsigned **pins,
+			       unsigned *num_pins)
 {
 	if (selector >= ARRAY_SIZE(u300_pin_groups))
 		return -EINVAL;
-	*pins = (unsigned *) u300_pin_groups[selector].pins;
+	*pins = u300_pin_groups[selector].pins;
 	*num_pins = u300_pin_groups[selector].num_pins;
 	return 0;
 }
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 08e7b0f..4679ae6 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -326,7 +326,7 @@ static int acquire_pins(struct pinctrl_dev *pctldev,
 	const struct pinmux_ops *pmxops = pctldev->desc->pmxops;
 	const char *func = pmxops->get_function_name(pctldev,
 						     func_selector);
-	unsigned *pins;
+	const unsigned *pins;
 	unsigned num_pins;
 	int ret;
 	int i;
@@ -367,7 +367,7 @@ static void release_pins(struct pinctrl_dev *pctldev,
 			 unsigned group_selector)
 {
 	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
-	unsigned *pins;
+	const unsigned *pins;
 	unsigned num_pins;
 	int ret;
 	int i;
diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h
index 1e6f67e..be9f537 100644
--- a/include/linux/pinctrl/pinctrl.h
+++ b/include/linux/pinctrl/pinctrl.h
@@ -75,8 +75,8 @@ struct pinctrl_ops {
 				       unsigned selector);
 	int (*get_group_pins) (struct pinctrl_dev *pctldev,
 			       unsigned selector,
-			       unsigned ** const pins,
-			       unsigned * const num_pins);
+			       const unsigned **pins,
+			       unsigned *num_pins);
 	void (*pin_dbg_show) (struct pinctrl_dev *pctldev, struct seq_file *s,
 			  unsigned offset);
 };
-- 
1.7.0.4

[PATCH 2/4] pinctrl: Remove unsafe __refdata

[Stephen Warren]

A pin controller's pin definitions are used both during pinctrl_register()
and pinctrl_unregister(). The latter happens outside of __init/__devinit
time, and hence it is unsafe to mark the pin array as __refdata.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/pinctrl/pinmux-sirf.c |    2 +-
 drivers/pinctrl/pinmux-u300.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinmux-sirf.c b/drivers/pinctrl/pinmux-sirf.c
index cad4f5d..3bc083c 100644
--- a/drivers/pinctrl/pinmux-sirf.c
+++ b/drivers/pinctrl/pinmux-sirf.c
@@ -30,7 +30,7 @@
  * pad list for the pinmux subsystem
  * refer to CS-131858-DC-6A.xls
  */
-static const struct pinctrl_pin_desc __refdata sirfsoc_pads[] = {
+static const struct pinctrl_pin_desc sirfsoc_pads[] = {
 	PINCTRL_PIN(4, "pwm0"),
 	PINCTRL_PIN(5, "pwm1"),
 	PINCTRL_PIN(6, "pwm2"),
diff --git a/drivers/pinctrl/pinmux-u300.c b/drivers/pinctrl/pinmux-u300.c
index be6e04d..ed8bcd8 100644
--- a/drivers/pinctrl/pinmux-u300.c
+++ b/drivers/pinctrl/pinmux-u300.c
@@ -179,7 +179,7 @@
 #define U300_NUM_PADS 467
 
 /* Pad names for the pinmux subsystem */
-static const struct pinctrl_pin_desc __refdata u300_pads[] = {
+static const struct pinctrl_pin_desc u300_pads[] = {
 	/* Pads along the top edge of the chip */
 	PINCTRL_PIN(0, "P PAD VDD 28"),
 	PINCTRL_PIN(1, "P PAD GND 28"),
-- 
1.7.0.4

Re: [PATCH 2/4] pinctrl: Remove unsafe __refdata

[Linus Walleij]

On Thu, Oct 20, 2011 at 12:19 AM, Stephen Warren <swarren@nvidia.com> wrote:

> A pin controller's pin definitions are used both during pinctrl_register()
> and pinctrl_unregister(). The latter happens outside of __init/__devinit
> time, and hence it is unsafe to mark the pin array as __refdata.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Thanks, applied.
Linus Walleij

Re: [PATCH 2/4] pinctrl: Remove unsafe __refdata

[Barry Song]

2011/10/20 Stephen Warren <swarren@nvidia.com>:
> A pin controller's pin definitions are used both during pinctrl_register()
> and pinctrl_unregister(). The latter happens outside of __init/__devinit
> time, and hence it is unsafe to mark the pin array as __refdata.

Thanks.
Acked-by: Barry Song <Baohua.Song@csr.com>

missed this when porting prima2 pinmux driver. is this __refdata used
just due to a typo?
i don't find prima2 pinctrl_pin_desc tables uses any code/data in __init/__exit.

if we need the table to be in init section, we should have used
__initconst/__initdata instead.

>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  drivers/pinctrl/pinmux-sirf.c |    2 +-
>  drivers/pinctrl/pinmux-u300.c |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/pinmux-sirf.c b/drivers/pinctrl/pinmux-sirf.c
> index cad4f5d..3bc083c 100644
> --- a/drivers/pinctrl/pinmux-sirf.c
> +++ b/drivers/pinctrl/pinmux-sirf.c
> @@ -30,7 +30,7 @@
>  * pad list for the pinmux subsystem
>  * refer to CS-131858-DC-6A.xls
>  */
> -static const struct pinctrl_pin_desc __refdata sirfsoc_pads[] = {
> +static const struct pinctrl_pin_desc sirfsoc_pads[] = {
>        PINCTRL_PIN(4, "pwm0"),
>        PINCTRL_PIN(5, "pwm1"),
>        PINCTRL_PIN(6, "pwm2"),
> diff --git a/drivers/pinctrl/pinmux-u300.c b/drivers/pinctrl/pinmux-u300.c
> index be6e04d..ed8bcd8 100644
> --- a/drivers/pinctrl/pinmux-u300.c
> +++ b/drivers/pinctrl/pinmux-u300.c
> @@ -179,7 +179,7 @@
>  #define U300_NUM_PADS 467
>
>  /* Pad names for the pinmux subsystem */
> -static const struct pinctrl_pin_desc __refdata u300_pads[] = {
> +static const struct pinctrl_pin_desc u300_pads[] = {
>        /* Pads along the top edge of the chip */
>        PINCTRL_PIN(0, "P PAD VDD 28"),
>        PINCTRL_PIN(1, "P PAD GND 28"),
> --
> 1.7.0.4

-barry

Re: [PATCH 2/4] pinctrl: Remove unsafe __refdata

[Linus Walleij]

On Thu, Oct 20, 2011 at 11:46 AM, Barry Song <21cnbao@gmail.com> wrote:

> 2011/10/20 Stephen Warren <swarren@nvidia.com>:
>> A pin controller's pin definitions are used both during pinctrl_register()
>> and pinctrl_unregister(). The latter happens outside of __init/__devinit
>> time, and hence it is unsafe to mark the pin array as __refdata.
>
> Thanks.
> Acked-by: Barry Song <Baohua.Song@csr.com>
>
> missed this when porting prima2 pinmux driver. is this __refdata used
> just due to a typo?

No the problem was I misunderstood what __refdata means.

It means it is kept around but can reference initidata.

But I thought it meant "kept around in general".

I don't think it's causing bugs actually because it wasn't referencing
any initdata. But it's better like this.

Thanks,
Linus Walleij

[PATCH 3/4] pinctrl: Don't copy pin names when registering them

[Stephen Warren]

A pin controller's names array is no longer marked __refdata. Hence, we
can avoid copying a pin's name into the descriptor when registering it.
Instead, just point at the string supplied in the pin array.

This both simplifies and speeds up pin controller initialization, but
also removes the hard-coded maximum pin name length.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/pinctrl/core.c |    5 ++---
 drivers/pinctrl/core.h |    2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 97a4eeb..15d7e5a 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -168,9 +168,8 @@ static int pinctrl_register_one_pin(struct pinctrl_dev *pctldev,
 	/* Set owner */
 	pindesc->pctldev = pctldev;
 
-	/* Copy optional basic pin info */
-	if (name)
-		strlcpy(pindesc->name, name, sizeof(pindesc->name));
+	/* Copy basic pin info */
+	pindesc->name = name;
 
 	spin_lock(&pctldev->pin_desc_tree_lock);
 	radix_tree_insert(&pctldev->pin_desc_tree, number, pindesc);
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 698221e..a493ba6 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -55,7 +55,7 @@ struct pinctrl_dev {
  */
 struct pin_desc {
 	struct pinctrl_dev *pctldev;
-	char	name[16];
+	const char *name;
 	spinlock_t lock;
 	/* These fields only added when supporting pinmux drivers */
 #ifdef CONFIG_PINMUX
-- 
1.7.0.4

Re: [PATCH 3/4] pinctrl: Don't copy pin names when registering them

[Linus Walleij]

On Thu, Oct 20, 2011 at 12:19 AM, Stephen Warren <swarren@nvidia.com> wrote:

> A pin controller's names array is no longer marked __refdata. Hence, we
> can avoid copying a pin's name into the descriptor when registering it.
> Instead, just point at the string supplied in the pin array.
>
> This both simplifies and speeds up pin controller initialization, but
> also removes the hard-coded maximum pin name length.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Thanks, applied.
Linus Walleij

[PATCH 4/4] pinctrl: Don't copy function name when requesting a pin

[Stephen Warren]

Instead, store a pointer to the currently assigned function.

This allows us to delete the mux_requested variable from pin_desc; a pin
is requested if its currently assigned function is non-NULL.

When a pin is requested as a GPIO rather than a regular function, the
assigned function name is dynamically constructed. In this case, we have
to kstrdup() the dynamically constructed name, so that mux_function doesn't
pointed at stack data. This requires pin_free to be told whether to free
the mux_function pointer or not.

This removes the hard-coded maximum function name length.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/pinctrl/core.h   |    3 +--
 drivers/pinctrl/pinmux.c |   36 +++++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index a493ba6..783c075 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -59,8 +59,7 @@ struct pin_desc {
 	spinlock_t lock;
 	/* These fields only added when supporting pinmux drivers */
 #ifdef CONFIG_PINMUX
-	bool	mux_requested;
-	char	mux_function[16];
+	const char *mux_function;
 #endif
 };
 
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 4679ae6..f139609 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -130,14 +130,13 @@ static int pin_request(struct pinctrl_dev *pctldev,
 	}
 
 	spin_lock(&desc->lock);
-	if (desc->mux_requested) {
+	if (desc->mux_function) {
 		spin_unlock(&desc->lock);
 		dev_err(&pctldev->dev,
 			"pin already requested\n");
 		goto out;
 	}
-	desc->mux_requested = true;
-	strncpy(desc->mux_function, function, sizeof(desc->mux_function));
+	desc->mux_function = function;
 	spin_unlock(&desc->lock);
 
 	/* Let each pin increase references to this module */
@@ -168,8 +167,7 @@ static int pin_request(struct pinctrl_dev *pctldev,
 out_free_pin:
 	if (status) {
 		spin_lock(&desc->lock);
-		desc->mux_requested = false;
-		desc->mux_function[0] = '\0';
+		desc->mux_function = NULL;
 		spin_unlock(&desc->lock);
 	}
 out:
@@ -184,8 +182,9 @@ out:
  * pin_free() - release a single muxed in pin so something else can be muxed
  * @pctldev: pin controller device handling this pin
  * @pin: the pin to free
+ * @free_func: whether to free the pin's assigned function name string
  */
-static void pin_free(struct pinctrl_dev *pctldev, int pin)
+static void pin_free(struct pinctrl_dev *pctldev, int pin, int free_func)
 {
 	const struct pinmux_ops *ops = pctldev->desc->pmxops;
 	struct pin_desc *desc;
@@ -201,8 +200,9 @@ static void pin_free(struct pinctrl_dev *pctldev, int pin)
 		ops->free(pctldev, pin);
 
 	spin_lock(&desc->lock);
-	desc->mux_requested = false;
-	desc->mux_function[0] = '\0';
+	if (free_func)
+		kfree(desc->mux_function);
+	desc->mux_function = NULL;
 	spin_unlock(&desc->lock);
 	module_put(pctldev->owner);
 }
@@ -214,6 +214,7 @@ static void pin_free(struct pinctrl_dev *pctldev, int pin)
 int pinmux_request_gpio(unsigned gpio)
 {
 	char gpiostr[16];
+	const char *function;
 	struct pinctrl_dev *pctldev;
 	struct pinctrl_gpio_range *range;
 	int ret;
@@ -229,7 +230,15 @@ int pinmux_request_gpio(unsigned gpio)
 	/* Conjure some name stating what chip and pin this is taken by */
 	snprintf(gpiostr, 15, "%s:%d", range->name, gpio);
 
-	return pin_request(pctldev, pin, gpiostr, true, range);
+	function = kstrdup(gpiostr, GFP_KERNEL);
+	if (!function)
+		return -EINVAL;
+
+	ret = pin_request(pctldev, pin, function, true, range);
+	if (ret < 0)
+		kfree(function);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(pinmux_request_gpio);
 
@@ -251,7 +260,7 @@ void pinmux_free_gpio(unsigned gpio)
 	/* Convert to the pin controllers number space */
 	pin = gpio - range->base;
 
-	pin_free(pctldev, pin);
+	pin_free(pctldev, pin, true);
 }
 EXPORT_SYMBOL_GPL(pinmux_free_gpio);
 
@@ -351,7 +360,7 @@ static int acquire_pins(struct pinctrl_dev *pctldev,
 			/* On error release all taken pins */
 			i--; /* this pin just failed */
 			for (; i >= 0; i--)
-				pin_free(pctldev, pins[i]);
+				pin_free(pctldev, pins[i], false);
 			return -ENODEV;
 		}
 	}
@@ -381,7 +390,7 @@ static void release_pins(struct pinctrl_dev *pctldev,
 		return;
 	}
 	for (i = 0; i < num_pins; i++)
-		pin_free(pctldev, pins[i]);
+		pin_free(pctldev, pins[i], false);
 }
 
 /**
@@ -1013,7 +1022,8 @@ static int pinmux_pins_show(struct seq_file *s, void *what)
 
 		seq_printf(s, "pin %d (%s): %s\n", pin,
 			   desc->name ? desc->name : "unnamed",
-			   desc->mux_requested ? desc->mux_function : "UNCLAIMED");
+			   desc->mux_function ? desc->mux_function
+					      : "UNCLAIMED");
 	}
 
 	return 0;
-- 
1.7.0.4

Re: [PATCH 4/4] pinctrl: Don't copy function name when requesting a pin

[Linus Walleij]

On Thu, Oct 20, 2011 at 12:19 AM, Stephen Warren <swarren@nvidia.com> wrote:

> Instead, store a pointer to the currently assigned function.
>
> This allows us to delete the mux_requested variable from pin_desc; a pin
> is requested if its currently assigned function is non-NULL.
>
> When a pin is requested as a GPIO rather than a regular function, the
> assigned function name is dynamically constructed. In this case, we have
> to kstrdup() the dynamically constructed name, so that mux_function doesn't
> pointed at stack data. This requires pin_free to be told whether to free
> the mux_function pointer or not.
>
> This removes the hard-coded maximum function name length.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Thanks, applied.
Linus Walleij

Re: [PATCH 1/4] pinctrl: get_group_pins() const fixes

[Linus Walleij]

On Thu, Oct 20, 2011 at 12:19 AM, Stephen Warren <swarren@nvidia.com> wrote:

> get_group_pins() "returns" a pointer to an array of const objects, through
> a pointer parameter. Fix the prototype so what's pointed at by the returned
> pointer is const, rather than the function parameter being const.
>
> This also allows the removal of a cast in each of the two current pinmux
> drivers.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Thanks, applied.
Linus Walleij