All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] pinctrl: display pin name instead of raw pin id
@ 2012-04-16 14:07 ` Dong Aisheng
  0 siblings, 0 replies; 10+ messages in thread
From: Dong Aisheng @ 2012-04-16 14:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-arm-kernel, linus.walleij, swarren

From: Dong Aisheng <dong.aisheng@linaro.org>

Pin name is more useful to users.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>

---
After changes, the debug information becomes when call pinctrl_get_*:
pinctrl-imx 20e0000.iomuxc: request pin 305 (MX6Q_PAD_SD4_CMD) for 219c000.usdhc
pinctrl-imx 20e0000.iomuxc: request pin 306 (MX6Q_PAD_SD4_CLK) for 219c000.usdhc
...

And for pingroups in sysfs:

root@freescale /sys/kernel/debug/pinctrl/20e0000.iomuxc$ cat pingroups
registered pin groups:
group: uart4grp-1
pin 219 (MX6Q_PAD_KEY_ROW0)
pin 218 (MX6Q_PAD_KEY_COL0)

group: usdhc4grp-1
pin 305 (MX6Q_PAD_SD4_CMD)
pin 306 (MX6Q_PAD_SD4_CLK)
pin 315 (MX6Q_PAD_SD4_DAT0)
pin 316 (MX6Q_PAD_SD4_DAT1)
pin 317 (MX6Q_PAD_SD4_DAT2)
pin 318 (MX6Q_PAD_SD4_DAT3)
pin 319 (MX6Q_PAD_SD4_DAT4)
pin 320 (MX6Q_PAD_SD4_DAT5)
pin 321 (MX6Q_PAD_SD4_DAT6)
pin 322 (MX6Q_PAD_SD4_DAT7)
---
 drivers/pinctrl/core.c   |   32 ++++++++++++++++++++++++++++----
 drivers/pinctrl/core.h   |    1 +
 drivers/pinctrl/pinmux.c |    5 +++--
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 59027ab..74a7843 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -125,6 +125,25 @@ int pin_get_from_name(struct pinctrl_dev *pctldev, const char *name)
 }
 
 /**
+ * pin_get_name_from_id() - look up a pin name from a pin id
+ * @pctldev: the pin control device to lookup the pin on
+ * @name: the name of the pin to look up
+ */
+const char *pin_get_name(struct pinctrl_dev *pctldev, const unsigned pin)
+{
+	const struct pin_desc *desc;
+
+	desc = pin_desc_get(pctldev, pin);
+	if (desc == NULL) {
+		dev_err(pctldev->dev, "failed to get pin(%d) name\n",
+			pin);
+		return NULL;
+	}
+
+	return desc->name;
+}
+
+/**
  * pin_is_valid() - check if pin exists on controller
  * @pctldev: the pin control device to check the pin on
  * @pin: pin to check, use the local pin controller index number
@@ -955,6 +974,7 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
 		const unsigned *pins;
 		unsigned num_pins;
 		const char *gname = ops->get_group_name(pctldev, selector);
+		const char *pname;
 		int ret;
 		int i;
 
@@ -964,10 +984,14 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
 			seq_printf(s, "%s [ERROR GETTING PINS]\n",
 				   gname);
 		else {
-			seq_printf(s, "group: %s, pins = [ ", gname);
-			for (i = 0; i < num_pins; i++)
-				seq_printf(s, "%d ", pins[i]);
-			seq_puts(s, "]\n");
+			seq_printf(s, "group: %s\n", gname);
+			for (i = 0; i < num_pins; i++) {
+				pname = pin_get_name(pctldev, pins[i]);
+				if (!pname)
+					return -EINVAL;
+				seq_printf(s, "pin %d (%s)\n", pins[i], pname);
+			}
+			seq_puts(s, "\n");
 		}
 		selector++;
 	}
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 98ae808..1f40ff6 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -148,6 +148,7 @@ struct pin_desc {
 
 struct pinctrl_dev *get_pinctrl_dev_from_devname(const char *dev_name);
 int pin_get_from_name(struct pinctrl_dev *pctldev, const char *name);
+const char *pin_get_name(struct pinctrl_dev *pctldev, const unsigned pin);
 int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
 			       const char *pin_group);
 
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index c494c37..fa0357b 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -88,8 +88,6 @@ static int pin_request(struct pinctrl_dev *pctldev,
 	const struct pinmux_ops *ops = pctldev->desc->pmxops;
 	int status = -EINVAL;
 
-	dev_dbg(pctldev->dev, "request pin %d for %s\n", pin, owner);
-
 	desc = pin_desc_get(pctldev, pin);
 	if (desc == NULL) {
 		dev_err(pctldev->dev,
@@ -97,6 +95,9 @@ static int pin_request(struct pinctrl_dev *pctldev,
 		goto out;
 	}
 
+	dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n",
+		pin, desc->name, owner);
+
 	if (gpio_range) {
 		/* There's no need to support multiple GPIO requests */
 		if (desc->gpio_owner) {
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 1/3] pinctrl: display pin name instead of raw pin id
@ 2012-04-16 14:07 ` Dong Aisheng
  0 siblings, 0 replies; 10+ messages in thread
From: Dong Aisheng @ 2012-04-16 14:07 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dong Aisheng <dong.aisheng@linaro.org>

Pin name is more useful to users.

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>

---
After changes, the debug information becomes when call pinctrl_get_*:
pinctrl-imx 20e0000.iomuxc: request pin 305 (MX6Q_PAD_SD4_CMD) for 219c000.usdhc
pinctrl-imx 20e0000.iomuxc: request pin 306 (MX6Q_PAD_SD4_CLK) for 219c000.usdhc
...

And for pingroups in sysfs:

root at freescale /sys/kernel/debug/pinctrl/20e0000.iomuxc$ cat pingroups
registered pin groups:
group: uart4grp-1
pin 219 (MX6Q_PAD_KEY_ROW0)
pin 218 (MX6Q_PAD_KEY_COL0)

group: usdhc4grp-1
pin 305 (MX6Q_PAD_SD4_CMD)
pin 306 (MX6Q_PAD_SD4_CLK)
pin 315 (MX6Q_PAD_SD4_DAT0)
pin 316 (MX6Q_PAD_SD4_DAT1)
pin 317 (MX6Q_PAD_SD4_DAT2)
pin 318 (MX6Q_PAD_SD4_DAT3)
pin 319 (MX6Q_PAD_SD4_DAT4)
pin 320 (MX6Q_PAD_SD4_DAT5)
pin 321 (MX6Q_PAD_SD4_DAT6)
pin 322 (MX6Q_PAD_SD4_DAT7)
---
 drivers/pinctrl/core.c   |   32 ++++++++++++++++++++++++++++----
 drivers/pinctrl/core.h   |    1 +
 drivers/pinctrl/pinmux.c |    5 +++--
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 59027ab..74a7843 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -125,6 +125,25 @@ int pin_get_from_name(struct pinctrl_dev *pctldev, const char *name)
 }
 
 /**
+ * pin_get_name_from_id() - look up a pin name from a pin id
+ * @pctldev: the pin control device to lookup the pin on
+ * @name: the name of the pin to look up
+ */
+const char *pin_get_name(struct pinctrl_dev *pctldev, const unsigned pin)
+{
+	const struct pin_desc *desc;
+
+	desc = pin_desc_get(pctldev, pin);
+	if (desc == NULL) {
+		dev_err(pctldev->dev, "failed to get pin(%d) name\n",
+			pin);
+		return NULL;
+	}
+
+	return desc->name;
+}
+
+/**
  * pin_is_valid() - check if pin exists on controller
  * @pctldev: the pin control device to check the pin on
  * @pin: pin to check, use the local pin controller index number
@@ -955,6 +974,7 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
 		const unsigned *pins;
 		unsigned num_pins;
 		const char *gname = ops->get_group_name(pctldev, selector);
+		const char *pname;
 		int ret;
 		int i;
 
@@ -964,10 +984,14 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
 			seq_printf(s, "%s [ERROR GETTING PINS]\n",
 				   gname);
 		else {
-			seq_printf(s, "group: %s, pins = [ ", gname);
-			for (i = 0; i < num_pins; i++)
-				seq_printf(s, "%d ", pins[i]);
-			seq_puts(s, "]\n");
+			seq_printf(s, "group: %s\n", gname);
+			for (i = 0; i < num_pins; i++) {
+				pname = pin_get_name(pctldev, pins[i]);
+				if (!pname)
+					return -EINVAL;
+				seq_printf(s, "pin %d (%s)\n", pins[i], pname);
+			}
+			seq_puts(s, "\n");
 		}
 		selector++;
 	}
diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h
index 98ae808..1f40ff6 100644
--- a/drivers/pinctrl/core.h
+++ b/drivers/pinctrl/core.h
@@ -148,6 +148,7 @@ struct pin_desc {
 
 struct pinctrl_dev *get_pinctrl_dev_from_devname(const char *dev_name);
 int pin_get_from_name(struct pinctrl_dev *pctldev, const char *name);
+const char *pin_get_name(struct pinctrl_dev *pctldev, const unsigned pin);
 int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
 			       const char *pin_group);
 
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index c494c37..fa0357b 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -88,8 +88,6 @@ static int pin_request(struct pinctrl_dev *pctldev,
 	const struct pinmux_ops *ops = pctldev->desc->pmxops;
 	int status = -EINVAL;
 
-	dev_dbg(pctldev->dev, "request pin %d for %s\n", pin, owner);
-
 	desc = pin_desc_get(pctldev, pin);
 	if (desc == NULL) {
 		dev_err(pctldev->dev,
@@ -97,6 +95,9 @@ static int pin_request(struct pinctrl_dev *pctldev,
 		goto out;
 	}
 
+	dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n",
+		pin, desc->name, owner);
+
 	if (gpio_range) {
 		/* There's no need to support multiple GPIO requests */
 		if (desc->gpio_owner) {
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] pinctrl: a minor fix of pin config debug information
  2012-04-16 14:07 ` Dong Aisheng
@ 2012-04-16 14:07   ` Dong Aisheng
  -1 siblings, 0 replies; 10+ messages in thread
From: Dong Aisheng @ 2012-04-16 14:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-arm-kernel, linus.walleij, swarren

From: Dong Aisheng <dong.aisheng@linaro.org>

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 drivers/pinctrl/pinconf.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
index 384dcc1..5d231b3 100644
--- a/drivers/pinctrl/pinconf.c
+++ b/drivers/pinctrl/pinconf.c
@@ -455,7 +455,7 @@ static int pinconf_pins_show(struct seq_file *s, void *what)
 		return 0;
 
 	seq_puts(s, "Pin config settings per pin\n");
-	seq_puts(s, "Format: pin (name): pinmux setting array\n");
+	seq_puts(s, "Format: pin (name): configs\n");
 
 	mutex_lock(&pinctrl_mutex);
 
@@ -506,7 +506,7 @@ static int pinconf_groups_show(struct seq_file *s, void *what)
 		return 0;
 
 	seq_puts(s, "Pin config settings per pin group\n");
-	seq_puts(s, "Format: group (name): pinmux setting array\n");
+	seq_puts(s, "Format: group (name): configs\n");
 
 	mutex_lock(&pinctrl_mutex);
 
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] pinctrl: a minor fix of pin config debug information
@ 2012-04-16 14:07   ` Dong Aisheng
  0 siblings, 0 replies; 10+ messages in thread
From: Dong Aisheng @ 2012-04-16 14:07 UTC (permalink / raw)
  To: linux-arm-kernel

From: Dong Aisheng <dong.aisheng@linaro.org>

Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>
---
 drivers/pinctrl/pinconf.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
index 384dcc1..5d231b3 100644
--- a/drivers/pinctrl/pinconf.c
+++ b/drivers/pinctrl/pinconf.c
@@ -455,7 +455,7 @@ static int pinconf_pins_show(struct seq_file *s, void *what)
 		return 0;
 
 	seq_puts(s, "Pin config settings per pin\n");
-	seq_puts(s, "Format: pin (name): pinmux setting array\n");
+	seq_puts(s, "Format: pin (name): configs\n");
 
 	mutex_lock(&pinctrl_mutex);
 
@@ -506,7 +506,7 @@ static int pinconf_groups_show(struct seq_file *s, void *what)
 		return 0;
 
 	seq_puts(s, "Pin config settings per pin group\n");
-	seq_puts(s, "Format: group (name): pinmux setting array\n");
+	seq_puts(s, "Format: group (name): configs\n");
 
 	mutex_lock(&pinctrl_mutex);
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] pinctrl: display pin name instead of raw pin id
  2012-04-16 14:07 ` Dong Aisheng
@ 2012-04-16 18:11   ` Stephen Warren
  -1 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2012-04-16 18:11 UTC (permalink / raw)
  To: Dong Aisheng; +Cc: linux-kernel, linux-arm-kernel, linus.walleij

On 04/16/2012 08:07 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Pin name is more useful to users.

> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c

> @@ -964,10 +984,14 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
>  			seq_printf(s, "%s [ERROR GETTING PINS]\n",
>  				   gname);
>  		else {
> +			seq_printf(s, "group: %s\n", gname);
> +			for (i = 0; i < num_pins; i++) {
> +				pname = pin_get_name(pctldev, pins[i]);
> +				if (!pname)
> +					return -EINVAL;

I'd rather see this say pname = "(unknown)" instead of return, to get as
much of the information into debugfs possible even in the face of errors.

But perhaps that condition should even be a BUG(); it would imply that
pinctrl_register_on_pin() had failed pretty badly.

> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> index c494c37..fa0357b 100644
> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c
> @@ -88,8 +88,6 @@ static int pin_request(struct pinctrl_dev *pctldev,
>  	const struct pinmux_ops *ops = pctldev->desc->pmxops;
>  	int status = -EINVAL;
>  
> -	dev_dbg(pctldev->dev, "request pin %d for %s\n", pin, owner);
> -
>  	desc = pin_desc_get(pctldev, pin);
>  	if (desc == NULL) {
>  		dev_err(pctldev->dev,
> @@ -97,6 +95,9 @@ static int pin_request(struct pinctrl_dev *pctldev,
>  		goto out;
>  	}
>  
> +	dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n",
> +		pin, desc->name, owner);
> +
>  	if (gpio_range) {
>  		/* There's no need to support multiple GPIO requests */
>  		if (desc->gpio_owner) {

That seems like it should be a separate patch?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] pinctrl: display pin name instead of raw pin id
@ 2012-04-16 18:11   ` Stephen Warren
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2012-04-16 18:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/16/2012 08:07 AM, Dong Aisheng wrote:
> From: Dong Aisheng <dong.aisheng@linaro.org>
> 
> Pin name is more useful to users.

> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c

> @@ -964,10 +984,14 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
>  			seq_printf(s, "%s [ERROR GETTING PINS]\n",
>  				   gname);
>  		else {
> +			seq_printf(s, "group: %s\n", gname);
> +			for (i = 0; i < num_pins; i++) {
> +				pname = pin_get_name(pctldev, pins[i]);
> +				if (!pname)
> +					return -EINVAL;

I'd rather see this say pname = "(unknown)" instead of return, to get as
much of the information into debugfs possible even in the face of errors.

But perhaps that condition should even be a BUG(); it would imply that
pinctrl_register_on_pin() had failed pretty badly.

> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> index c494c37..fa0357b 100644
> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c
> @@ -88,8 +88,6 @@ static int pin_request(struct pinctrl_dev *pctldev,
>  	const struct pinmux_ops *ops = pctldev->desc->pmxops;
>  	int status = -EINVAL;
>  
> -	dev_dbg(pctldev->dev, "request pin %d for %s\n", pin, owner);
> -
>  	desc = pin_desc_get(pctldev, pin);
>  	if (desc == NULL) {
>  		dev_err(pctldev->dev,
> @@ -97,6 +95,9 @@ static int pin_request(struct pinctrl_dev *pctldev,
>  		goto out;
>  	}
>  
> +	dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n",
> +		pin, desc->name, owner);
> +
>  	if (gpio_range) {
>  		/* There's no need to support multiple GPIO requests */
>  		if (desc->gpio_owner) {

That seems like it should be a separate patch?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] pinctrl: display pin name instead of raw pin id
  2012-04-16 18:11   ` Stephen Warren
@ 2012-04-17  6:34     ` Dong Aisheng
  -1 siblings, 0 replies; 10+ messages in thread
From: Dong Aisheng @ 2012-04-17  6:34 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Dong Aisheng-B29396, linux-kernel, linux-arm-kernel, linus.walleij

On Tue, Apr 17, 2012 at 02:11:58AM +0800, Stephen Warren wrote:
> On 04/16/2012 08:07 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Pin name is more useful to users.
> 
> > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> 
> > @@ -964,10 +984,14 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
> >  			seq_printf(s, "%s [ERROR GETTING PINS]\n",
> >  				   gname);
> >  		else {
> > +			seq_printf(s, "group: %s\n", gname);
> > +			for (i = 0; i < num_pins; i++) {
> > +				pname = pin_get_name(pctldev, pins[i]);
> > +				if (!pname)
> > +					return -EINVAL;
> 
> I'd rather see this say pname = "(unknown)" instead of return, to get as
> much of the information into debugfs possible even in the face of errors.
> 
pinctrl_register_one_pin will guarantee the pname should never be NULL.
So this condition is a never happen case.
That's why i simply returned an error with no debug information.

> But perhaps that condition should even be a BUG(); it would imply that
> pinctrl_register_on_pin() had failed pretty badly.
> 
>From the comments of BUG() in bug.h, it looks using BUG() here is a little
too seriously.
=====
 * Don't use BUG() or BUG_ON() unless there's really no way out;

Maybe WARN_ON is more suitable here, right?
I will send a patch for your review.

> > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> > index c494c37..fa0357b 100644
> > --- a/drivers/pinctrl/pinmux.c
> > +++ b/drivers/pinctrl/pinmux.c
> > @@ -88,8 +88,6 @@ static int pin_request(struct pinctrl_dev *pctldev,
> >  	const struct pinmux_ops *ops = pctldev->desc->pmxops;
> >  	int status = -EINVAL;
> >  
> > -	dev_dbg(pctldev->dev, "request pin %d for %s\n", pin, owner);
> > -
> >  	desc = pin_desc_get(pctldev, pin);
> >  	if (desc == NULL) {
> >  		dev_err(pctldev->dev,
> > @@ -97,6 +95,9 @@ static int pin_request(struct pinctrl_dev *pctldev,
> >  		goto out;
> >  	}
> >  
> > +	dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n",
> > +		pin, desc->name, owner);
> > +
> >  	if (gpio_range) {
> >  		/* There's no need to support multiple GPIO requests */
> >  		if (desc->gpio_owner) {
> 
> That seems like it should be a separate patch?
> 
Ok to me, will make a separate one.

Regards
Dong Aisheng


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] pinctrl: display pin name instead of raw pin id
@ 2012-04-17  6:34     ` Dong Aisheng
  0 siblings, 0 replies; 10+ messages in thread
From: Dong Aisheng @ 2012-04-17  6:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 17, 2012 at 02:11:58AM +0800, Stephen Warren wrote:
> On 04/16/2012 08:07 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@linaro.org>
> > 
> > Pin name is more useful to users.
> 
> > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> 
> > @@ -964,10 +984,14 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
> >  			seq_printf(s, "%s [ERROR GETTING PINS]\n",
> >  				   gname);
> >  		else {
> > +			seq_printf(s, "group: %s\n", gname);
> > +			for (i = 0; i < num_pins; i++) {
> > +				pname = pin_get_name(pctldev, pins[i]);
> > +				if (!pname)
> > +					return -EINVAL;
> 
> I'd rather see this say pname = "(unknown)" instead of return, to get as
> much of the information into debugfs possible even in the face of errors.
> 
pinctrl_register_one_pin will guarantee the pname should never be NULL.
So this condition is a never happen case.
That's why i simply returned an error with no debug information.

> But perhaps that condition should even be a BUG(); it would imply that
> pinctrl_register_on_pin() had failed pretty badly.
> 
>From the comments of BUG() in bug.h, it looks using BUG() here is a little
too seriously.
=====
 * Don't use BUG() or BUG_ON() unless there's really no way out;

Maybe WARN_ON is more suitable here, right?
I will send a patch for your review.

> > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> > index c494c37..fa0357b 100644
> > --- a/drivers/pinctrl/pinmux.c
> > +++ b/drivers/pinctrl/pinmux.c
> > @@ -88,8 +88,6 @@ static int pin_request(struct pinctrl_dev *pctldev,
> >  	const struct pinmux_ops *ops = pctldev->desc->pmxops;
> >  	int status = -EINVAL;
> >  
> > -	dev_dbg(pctldev->dev, "request pin %d for %s\n", pin, owner);
> > -
> >  	desc = pin_desc_get(pctldev, pin);
> >  	if (desc == NULL) {
> >  		dev_err(pctldev->dev,
> > @@ -97,6 +95,9 @@ static int pin_request(struct pinctrl_dev *pctldev,
> >  		goto out;
> >  	}
> >  
> > +	dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n",
> > +		pin, desc->name, owner);
> > +
> >  	if (gpio_range) {
> >  		/* There's no need to support multiple GPIO requests */
> >  		if (desc->gpio_owner) {
> 
> That seems like it should be a separate patch?
> 
Ok to me, will make a separate one.

Regards
Dong Aisheng

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] pinctrl: a minor fix of pin config debug information
  2012-04-16 14:07   ` Dong Aisheng
@ 2012-04-18 11:15     ` Linus Walleij
  -1 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2012-04-18 11:15 UTC (permalink / raw)
  To: Dong Aisheng; +Cc: linux-kernel, linux-arm-kernel, linus.walleij, swarren

On Mon, Apr 16, 2012 at 4:07 PM, Dong Aisheng <b29396@freescale.com> wrote:

> From: Dong Aisheng <dong.aisheng@linaro.org>
>
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>

Obviously correct, so applied this.

Thanks,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] pinctrl: a minor fix of pin config debug information
@ 2012-04-18 11:15     ` Linus Walleij
  0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2012-04-18 11:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Apr 16, 2012 at 4:07 PM, Dong Aisheng <b29396@freescale.com> wrote:

> From: Dong Aisheng <dong.aisheng@linaro.org>
>
> Signed-off-by: Dong Aisheng <dong.aisheng@linaro.org>

Obviously correct, so applied this.

Thanks,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-04-18 11:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-16 14:07 [PATCH 1/3] pinctrl: display pin name instead of raw pin id Dong Aisheng
2012-04-16 14:07 ` Dong Aisheng
2012-04-16 14:07 ` [PATCH 2/3] pinctrl: a minor fix of pin config debug information Dong Aisheng
2012-04-16 14:07   ` Dong Aisheng
2012-04-18 11:15   ` Linus Walleij
2012-04-18 11:15     ` Linus Walleij
2012-04-16 18:11 ` [PATCH 1/3] pinctrl: display pin name instead of raw pin id Stephen Warren
2012-04-16 18:11   ` Stephen Warren
2012-04-17  6:34   ` Dong Aisheng
2012-04-17  6:34     ` Dong Aisheng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.