gpiolib: demote the hogging log messages to debug

PATCH In review Bartosz Golaszewski 5 participants 9 messages Jun 5, 2023 – Jun 14, 2023

Subsystems (2)

GPIO Device tree

lore

Share & tools

lore

Bartosz Golaszewski

author

2023-06-05 12:53 UTC #0 lore raw

PATCH 1/1 gpiolib: demote the hogging log messages to debug

From: Bartosz Golaszewski <[email protected]>

Drivers should be silent when they work correctly. There's no reason to emit info messages when GPIO lines are hogged. Demote the message to debug.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpiolib.c |  2 +-
 drivers/of/unittest.c  | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

drivers/gpio/gpiolib.c +1 -1

index a7220e04a93e..e4515bda8915 100644
@@ -4243,7 +4243,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
 	/* Mark GPIO as hogged so it can be identified and removed later */
 	set_bit(FLAG_IS_HOGGED, &desc->flags);
 
-	gpiod_info(desc, "hogged as %s%s\n",
+	gpiod_dbg(desc, "hogged as %s%s\n",
 		(dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
 		(dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
 		  (dflags & GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low" : "");

drivers/of/unittest.c +8 -8

index 2191c0136531..0060334a98a7 100644
@@ -1849,19 +1849,19 @@ static void __init of_unittest_overlay_gpio(void)
 	 * driver is registered
 	 */
 
-	EXPECT_BEGIN(KERN_INFO,
+	EXPECT_BEGIN(KERN_DEBUG,
 		     "gpio-<<int>> (line-B-input): hogged as input\n");
 
-	EXPECT_BEGIN(KERN_INFO,
+	EXPECT_BEGIN(KERN_DEBUG,
 		     "gpio-<<int>> (line-A-input): hogged as input\n");
 
 	ret = platform_driver_register(&unittest_gpio_driver);
 	if (unittest(ret == 0, "could not register unittest gpio driver\n"))
 		return;
 
-	EXPECT_END(KERN_INFO,
+	EXPECT_END(KERN_DEBUG,
 		   "gpio-<<int>> (line-A-input): hogged as input\n");
-	EXPECT_END(KERN_INFO,
+	EXPECT_END(KERN_DEBUG,
 		   "gpio-<<int>> (line-B-input): hogged as input\n");
 
 	unittest(probe_pass_count + 2 == unittest_gpio_probe_pass_count,

@@ -1888,7 +1888,7 @@ static void __init of_unittest_overlay_gpio(void)
 	probe_pass_count = unittest_gpio_probe_pass_count;
 	chip_request_count = unittest_gpio_chip_request_count;
 
-	EXPECT_BEGIN(KERN_INFO,
+	EXPECT_BEGIN(KERN_DEBUG,
 		     "gpio-<<int>> (line-D-input): hogged as input\n");
 
 	/* overlay_gpio_03 contains gpio node and child gpio hog node */

@@ -1896,7 +1896,7 @@ static void __init of_unittest_overlay_gpio(void)
 	unittest(overlay_data_apply("overlay_gpio_03", NULL),
 		 "Adding overlay 'overlay_gpio_03' failed\n");
 
-	EXPECT_END(KERN_INFO,
+	EXPECT_END(KERN_DEBUG,
 		   "gpio-<<int>> (line-D-input): hogged as input\n");
 
 	unittest(probe_pass_count + 1 == unittest_gpio_probe_pass_count,

@@ -1935,7 +1935,7 @@ static void __init of_unittest_overlay_gpio(void)
 	 *   - processing gpio for overlay_gpio_04b
 	 */
 
-	EXPECT_BEGIN(KERN_INFO,
+	EXPECT_BEGIN(KERN_DEBUG,
 		     "gpio-<<int>> (line-C-input): hogged as input\n");
 
 	/* overlay_gpio_04b contains child gpio hog node */

@@ -1943,7 +1943,7 @@ static void __init of_unittest_overlay_gpio(void)
 	unittest(overlay_data_apply("overlay_gpio_04b", NULL),
 		 "Adding overlay 'overlay_gpio_04b' failed\n");
 
-	EXPECT_END(KERN_INFO,
+	EXPECT_END(KERN_DEBUG,
 		   "gpio-<<int>> (line-C-input): hogged as input\n");
 
 	unittest(chip_request_count + 1 == unittest_gpio_chip_request_count,

Signature · 1 line

2.39.2

Andy Shevchenko

Reviewed-by on v1 1/1

2023-06-06 15:00 UTC #1 lore raw

Quoted 5 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>
Drivers should be silent when they work correctly. There's no reason to emit info messages when GPIO lines are hogged. Demote the message to debug.

Reviewed-by Andy Shevchenko <[email protected]>

Quoted 87 lines

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpiolib.c |  2 +-
 drivers/of/unittest.c  | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

drivers/gpio/gpiolib.c +1 -1

index a7220e04a93e..e4515bda8915 100644
@@ -4243,7 +4243,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
 	/* Mark GPIO as hogged so it can be identified and removed later */
 	set_bit(FLAG_IS_HOGGED, &desc->flags);
 
-	gpiod_info(desc, "hogged as %s%s\n",
+	gpiod_dbg(desc, "hogged as %s%s\n",
 		(dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
 		(dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
 		  (dflags & GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low" : "");

drivers/of/unittest.c +8 -8

index 2191c0136531..0060334a98a7 100644
@@ -1849,19 +1849,19 @@ static void __init of_unittest_overlay_gpio(void)
 	 * driver is registered
 	 */
 
-	EXPECT_BEGIN(KERN_INFO,
+	EXPECT_BEGIN(KERN_DEBUG,
 		     "gpio-<<int>> (line-B-input): hogged as input\n");
 
-	EXPECT_BEGIN(KERN_INFO,
+	EXPECT_BEGIN(KERN_DEBUG,
 		     "gpio-<<int>> (line-A-input): hogged as input\n");
 
 	ret = platform_driver_register(&unittest_gpio_driver);
 	if (unittest(ret == 0, "could not register unittest gpio driver\n"))
 		return;
 
-	EXPECT_END(KERN_INFO,
+	EXPECT_END(KERN_DEBUG,
 		   "gpio-<<int>> (line-A-input): hogged as input\n");
-	EXPECT_END(KERN_INFO,
+	EXPECT_END(KERN_DEBUG,
 		   "gpio-<<int>> (line-B-input): hogged as input\n");
 
 	unittest(probe_pass_count + 2 == unittest_gpio_probe_pass_count,

@@ -1888,7 +1888,7 @@ static void __init of_unittest_overlay_gpio(void)
 	probe_pass_count = unittest_gpio_probe_pass_count;
 	chip_request_count = unittest_gpio_chip_request_count;
 
-	EXPECT_BEGIN(KERN_INFO,
+	EXPECT_BEGIN(KERN_DEBUG,
 		     "gpio-<<int>> (line-D-input): hogged as input\n");
 
 	/* overlay_gpio_03 contains gpio node and child gpio hog node */

@@ -1896,7 +1896,7 @@ static void __init of_unittest_overlay_gpio(void)
 	unittest(overlay_data_apply("overlay_gpio_03", NULL),
 		 "Adding overlay 'overlay_gpio_03' failed\n");
 
-	EXPECT_END(KERN_INFO,
+	EXPECT_END(KERN_DEBUG,
 		   "gpio-<<int>> (line-D-input): hogged as input\n");
 
 	unittest(probe_pass_count + 1 == unittest_gpio_probe_pass_count,

@@ -1935,7 +1935,7 @@ static void __init of_unittest_overlay_gpio(void)
 	 *   - processing gpio for overlay_gpio_04b
 	 */
 
-	EXPECT_BEGIN(KERN_INFO,
+	EXPECT_BEGIN(KERN_DEBUG,
 		     "gpio-<<int>> (line-C-input): hogged as input\n");
 
 	/* overlay_gpio_04b contains child gpio hog node */

@@ -1943,7 +1943,7 @@ static void __init of_unittest_overlay_gpio(void)
 	unittest(overlay_data_apply("overlay_gpio_04b", NULL),
 		 "Adding overlay 'overlay_gpio_04b' failed\n");
 
-	EXPECT_END(KERN_INFO,
+	EXPECT_END(KERN_DEBUG,
 		   "gpio-<<int>> (line-C-input): hogged as input\n");
 
 	unittest(chip_request_count + 1 == unittest_gpio_chip_request_count,

Signature · 1 line

2.39.2

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Linus Walleij

Reviewed-by on v1 1/1

2023-06-09 07:35 UTC #2 lore raw

In reply to Bartosz Golaszewski

Quoted 8 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>
Drivers should be silent when they work correctly. There's no reason to emit info messages when GPIO lines are hogged. Demote the message to debug.
Signed-off-by Bartosz Golaszewski <[email protected]>
Suggested-by Kent Gibson <[email protected]>

Reviewed-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

Rob Herring

2023-06-09 13:47 UTC #3 lore raw

In reply to Bartosz Golaszewski

Quoted 32 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Drivers should be silent when they work correctly. There's no reason to emit info messages when GPIO lines are hogged. Demote the message to debug.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpiolib.c |  2 +-
 drivers/of/unittest.c  | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

drivers/gpio/gpiolib.c +1 -1

index a7220e04a93e..e4515bda8915 100644
@@ -4243,7 +4243,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
        /* Mark GPIO as hogged so it can be identified and removed later */
        set_bit(FLAG_IS_HOGGED, &desc->flags);
 
-       gpiod_info(desc, "hogged as %s%s\n",
+       gpiod_dbg(desc, "hogged as %s%s\n",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
                  (dflags & GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low" : "");

drivers/of/unittest.c +0 -0

index 2191c0136531..0060334a98a7 100644

drivers/of/unittest.c static void __init of_unittest_overlay_gpio(void) · line 1853

Quoted 6 lines

drivers/of/unittest.c +1 -1

@@ -1849,19 +1849,19 @@ static void __init of_unittest_overlay_gpio(void)
         * driver is registered
         */
 
-       EXPECT_BEGIN(KERN_INFO,
+       EXPECT_BEGIN(KERN_DEBUG,
                     "gpio-<<int>> (line-B-input): hogged as input\n");

As debug messages are normally off, I think you can just remove these.

Rob

Bartosz Golaszewski

author

2023-06-09 14:20 UTC #4 lore raw

Quoted 37 lines · Rob Herring

Quoted 35 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Drivers should be silent when they work correctly. There's no reason to emit info messages when GPIO lines are hogged. Demote the message to debug.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpiolib.c |  2 +-
 drivers/of/unittest.c  | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

drivers/gpio/gpiolib.c +1 -1

index a7220e04a93e..e4515bda8915 100644
@@ -4243,7 +4243,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
        /* Mark GPIO as hogged so it can be identified and removed later */
        set_bit(FLAG_IS_HOGGED, &desc->flags);
 
-       gpiod_info(desc, "hogged as %s%s\n",
+       gpiod_dbg(desc, "hogged as %s%s\n",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
                  (dflags & GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low" : "");

drivers/of/unittest.c +0 -0

index 2191c0136531..0060334a98a7 100644
@@ -1849,19 +1849,19 @@ static void __init of_unittest_overlay_gpio(void)
         * driver is registered
         */

Quoted 7 lines

drivers/of/unittest.c static void __init of_unittest_overlay_gpio(void) · line 1853
Quoted 3 lines
drivers/of/unittest.c +1 -1
-       EXPECT_BEGIN(KERN_INFO,
+       EXPECT_BEGIN(KERN_DEBUG,
                     "gpio-<<int>> (line-B-input): hogged as input\n");
As debug messages are normally off, I think you can just remove these.
Rob

I will send a follow-up.

Bart

Frank Rowand

2023-06-11 12:40 UTC #5 lore raw

In reply to Rob Herring

Quoted 36 lines · Rob Herring

Quoted 35 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Drivers should be silent when they work correctly. There's no reason to emit info messages when GPIO lines are hogged. Demote the message to debug.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
 drivers/gpio/gpiolib.c |  2 +-
 drivers/of/unittest.c  | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

drivers/gpio/gpiolib.c +1 -1

index a7220e04a93e..e4515bda8915 100644
@@ -4243,7 +4243,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
        /* Mark GPIO as hogged so it can be identified and removed later */
        set_bit(FLAG_IS_HOGGED, &desc->flags);
 
-       gpiod_info(desc, "hogged as %s%s\n",
+       gpiod_dbg(desc, "hogged as %s%s\n",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
                  (dflags & GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low" : "");

drivers/of/unittest.c +0 -0

index 2191c0136531..0060334a98a7 100644
@@ -1849,19 +1849,19 @@ static void __init of_unittest_overlay_gpio(void)
         * driver is registered
         */

Quoted 5 lines

drivers/of/unittest.c static void __init of_unittest_overlay_gpio(void) · line 1853
Quoted 3 lines
drivers/of/unittest.c +1 -1
-       EXPECT_BEGIN(KERN_INFO,
+       EXPECT_BEGIN(KERN_DEBUG,
                     "gpio-<<int>> (line-B-input): hogged as input\n");
As debug messages are normally off, I think you can just remove these.

This patch is an example of exactly why the message level is the first parameter passed to EXPECT_*(). The test results are then _always_ valid, not just _normally_.

-Frank

Quoted 2 lines

Rob

Frank Rowand

2023-06-11 12:48 UTC #6 lore raw

Quoted 43 lines · Frank Rowand

Quoted 36 lines · Rob Herring
Quoted 35 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Drivers should be silent when they work correctly. There's no reason to emit info messages when GPIO lines are hogged. Demote the message to debug.
Signed-off-by Bartosz Golaszewski <[email protected]>
Suggested-by Kent Gibson <[email protected]>
---
 drivers/gpio/gpiolib.c |  2 +-
 drivers/of/unittest.c  | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)
drivers/gpio/gpiolib.c +1 -1
index a7220e04a93e..e4515bda8915 100644
@@ -4243,7 +4243,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
        /* Mark GPIO as hogged so it can be identified and removed later */
        set_bit(FLAG_IS_HOGGED, &desc->flags);
 
-       gpiod_info(desc, "hogged as %s%s\n",
+       gpiod_dbg(desc, "hogged as %s%s\n",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
                  (dflags & GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low" : "");
drivers/of/unittest.c +0 -0
index 2191c0136531..0060334a98a7 100644
@@ -1849,19 +1849,19 @@ static void __init of_unittest_overlay_gpio(void)
         * driver is registered
         */
 
Quoted 5 lines
drivers/of/unittest.c static void __init of_unittest_overlay_gpio(void) · line 1853
Quoted 3 lines
drivers/of/unittest.c +1 -1
-       EXPECT_BEGIN(KERN_INFO,
+       EXPECT_BEGIN(KERN_DEBUG,
                     "gpio-<<int>> (line-B-input): hogged as input\n");
As debug messages are normally off, I think you can just remove these.

Quoted 3 lines

This patch is an example of exactly why the message level is the first parameter passed to EXPECT_*(). The test results are then _always_ valid, not just _normally_.

One should never say never. One should never say always. :-)

Yes, there is still the exception where debug can be enabled independently for drivers/gpio/gpiolib.c vs for drivers/of/unittest.c. And dynamic debug can make things even more wonky.

-Frank

Quoted 6 lines

-Frank
Quoted 2 lines
Rob

Rob Herring

2023-06-12 15:43 UTC #7 lore raw

Quoted 49 lines · Frank Rowand

Quoted 43 lines · Frank Rowand
Quoted 36 lines · Rob Herring
Quoted 35 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Drivers should be silent when they work correctly. There's no reason to emit info messages when GPIO lines are hogged. Demote the message to debug.
Signed-off-by Bartosz Golaszewski <[email protected]>
Suggested-by Kent Gibson <[email protected]>
---
 drivers/gpio/gpiolib.c |  2 +-
 drivers/of/unittest.c  | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)
drivers/gpio/gpiolib.c +1 -1
index a7220e04a93e..e4515bda8915 100644
@@ -4243,7 +4243,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
        /* Mark GPIO as hogged so it can be identified and removed later */
        set_bit(FLAG_IS_HOGGED, &desc->flags);
 
-       gpiod_info(desc, "hogged as %s%s\n",
+       gpiod_dbg(desc, "hogged as %s%s\n",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
                  (dflags & GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low" : "");
drivers/of/unittest.c +0 -0
index 2191c0136531..0060334a98a7 100644
@@ -1849,19 +1849,19 @@ static void __init of_unittest_overlay_gpio(void)
         * driver is registered
         */
 
Quoted 5 lines
drivers/of/unittest.c static void __init of_unittest_overlay_gpio(void) · line 1853
Quoted 3 lines
drivers/of/unittest.c +1 -1
-       EXPECT_BEGIN(KERN_INFO,
+       EXPECT_BEGIN(KERN_DEBUG,
                     "gpio-<<int>> (line-B-input): hogged as input\n");
As debug messages are normally off, I think you can just remove these.
Quoted 3 lines
This patch is an example of exactly why the message level is the first parameter passed to EXPECT_*(). The test results are then _always_ valid, not just _normally_.

Quoted 5 lines

One should never say never. One should never say always. :-)
Yes, there is still the exception where debug can be enabled independently for drivers/gpio/gpiolib.c vs for drivers/of/unittest.c. And dynamic debug can make things even more wonky.

If we turned on debug messages for drivers/of/, the unittest would be hopelessly broken.

Debug messages are special compared to all the other levels as they are normally off whereas the rest are always on. For the unittest we should assume they are off.

Rob

Frank Rowand

2023-06-14 00:05 UTC #8 lore raw

Quoted 56 lines · Rob Herring

Quoted 49 lines · Frank Rowand
Quoted 43 lines · Frank Rowand
Quoted 36 lines · Rob Herring
Quoted 35 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Drivers should be silent when they work correctly. There's no reason to emit info messages when GPIO lines are hogged. Demote the message to debug.
Signed-off-by Bartosz Golaszewski <[email protected]>
Suggested-by Kent Gibson <[email protected]>
---
 drivers/gpio/gpiolib.c |  2 +-
 drivers/of/unittest.c  | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)
drivers/gpio/gpiolib.c +1 -1
index a7220e04a93e..e4515bda8915 100644
@@ -4243,7 +4243,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
        /* Mark GPIO as hogged so it can be identified and removed later */
        set_bit(FLAG_IS_HOGGED, &desc->flags);
 
-       gpiod_info(desc, "hogged as %s%s\n",
+       gpiod_dbg(desc, "hogged as %s%s\n",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
                (dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
                  (dflags & GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low" : "");
drivers/of/unittest.c +0 -0
index 2191c0136531..0060334a98a7 100644
@@ -1849,19 +1849,19 @@ static void __init of_unittest_overlay_gpio(void)
         * driver is registered
         */
 
Quoted 5 lines
drivers/of/unittest.c static void __init of_unittest_overlay_gpio(void) · line 1853
Quoted 3 lines
drivers/of/unittest.c +1 -1
-       EXPECT_BEGIN(KERN_INFO,
+       EXPECT_BEGIN(KERN_DEBUG,
                     "gpio-<<int>> (line-B-input): hogged as input\n");
As debug messages are normally off, I think you can just remove these.
Quoted 3 lines
This patch is an example of exactly why the message level is the first parameter passed to EXPECT_*(). The test results are then _always_ valid, not just _normally_.
Quoted 5 lines
One should never say never. One should never say always. :-)
Yes, there is still the exception where debug can be enabled independently for drivers/gpio/gpiolib.c vs for drivers/of/unittest.c. And dynamic debug can make things even more wonky.

Quoted 6 lines

If we turned on debug messages for drivers/of/, the unittest would be hopelessly broken.
Debug messages are special compared to all the other levels as they are normally off whereas the rest are always on. For the unittest we should assume they are off.

Sorry, I wasn't clear enough in my last email. My intent in that email is to essentially agree with what you say here - if debug is enabled then we are in a special world where the person who enabled debug has to deal with all of the side effects and consequences of enabling debug.

-Frank

Quoted 2 lines

Rob