gpiolib: friendly debug information for consumer

Message ID 1516685317-12867-1-git-send-email-dongsheng.wang@hxt-semitech.com
State New
Headers show
Series
  • gpiolib: friendly debug information for consumer
Related show

Commit Message

Wang, Dongsheng Jan. 23, 2018, 5:28 a.m.
mdiobus always try to get "reset" consumer, but not all platforms
have this capability. "failed" makes observer confuse when a
consumer can not find.

Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
---
 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Linus Walleij Feb. 7, 2018, 1:04 p.m. | #1
Hi Wang,

thank you for your patch!

On Tue, Jan 23, 2018 at 6:28 AM, Wang Dongsheng
<dongsheng.wang@hxt-semitech.com> wrote:

> mdiobus always try to get "reset" consumer, but not all platforms
> have this capability. "failed" makes observer confuse when a
> consumer can not find.
>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>

But the message is correct?

> -               dev_dbg(dev, "lookup for GPIO %s failed\n", con_id);

This is happening.

I would be more inclined to agree if it was dev_err() but this is
not even info, it is debug info.

> +               dev_dbg(dev, "Can not detect GPIO consumer %s\n", con_id);

This is not about any "detection", it is about looking something
up.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wang, Dongsheng Feb. 8, 2018, 5:51 a.m. | #2
Hi Linus,

Thanks for your feedback.

On 2018/2/7 21:04:07, "Linus Walleij" <linus.walleij@linaro.org> wrote:

>Hi Wang,

>

>thank you for your patch!

>

>On Tue, Jan 23, 2018 at 6:28 AM, Wang Dongsheng

><dongsheng.wang@hxt-semitech.com> wrote:

>

>>mdiobus always try to get "reset" consumer, but not all platforms

>>have this capability. "failed" makes observer confuse when a

>>consumer can not find.

>>

>>Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>

>

>But the message is correct?

>

>>-               dev_dbg(dev, "lookup for GPIO %s failed\n", con_id);

>

>This is happening.

>

>I would be more inclined to agree if it was dev_err() but this is

>not even info, it is debug info.

>

dev_dbg is often used when we develop code. I've been asked recently
about this GPIO "failed", and others may be less familiar with GPIO.
I'm also not familiar with it, just do some quick research here. :)
So them confuse why kernel got this. In fact, there is no such GPIO
feature(external PHY reset GPIO) on our platform. Therefore, I prefer
not to use the word "failed".

May be "No GPIO consumer %s found" is better?


Cheers,
-Dongsheng

>

>>+               dev_dbg(dev, "Can not detect GPIO consumer %s\n", 

>>con_id);

>

>This is not about any "detection", it is about looking something

>up.

>

>

>

>Yours,

>Linus Walleij
Linus Walleij Feb. 22, 2018, 1:01 p.m. | #3
On Thu, Feb 8, 2018 at 6:51 AM, Wang, Dongsheng
<dongsheng.wang@hxt-semitech.com> wrote:

>>But the message is correct?
>>
>>>-               dev_dbg(dev, "lookup for GPIO %s failed\n", con_id);
>>
>>This is happening.
>>
>>I would be more inclined to agree if it was dev_err() but this is
>>not even info, it is debug info.
>>
> dev_dbg is often used when we develop code. I've been asked recently
> about this GPIO "failed", and others may be less familiar with GPIO.

Aha I understand.

> I'm also not familiar with it, just do some quick research here. :)
> So them confuse why kernel got this. In fact, there is no such GPIO
> feature(external PHY reset GPIO) on our platform. Therefore, I prefer
> not to use the word "failed".
>
> May be "No GPIO consumer %s found" is better?

That is fine!

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 14532d9..1f98ea6 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3630,7 +3630,7 @@  struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
 	}
 
 	if (IS_ERR(desc)) {
-		dev_dbg(dev, "lookup for GPIO %s failed\n", con_id);
+		dev_dbg(dev, "Can not detect GPIO consumer %s\n", con_id);
 		return desc;
 	}