adb: properly mark continued kernel messages

PATCH Superseded Andreas Schwab 5 participants 7 messages Nov 26, 2016 – Jan 29, 2018 Linux for power macintosh

lore

Share & tools

lore

Andreas Schwab

author

2016-11-26 13:30 UTC #0 lore raw

PATCH 1/1 adb: properly mark continued kernel messages

Use pr_cont where appropriate, and switch to pr_foo throughout.

Signed-off-by Andreas Schwab <[email protected]>

---
 drivers/macintosh/adb.c    | 24 +++++++++-----------
 drivers/macintosh/adbhid.c | 56 +++++++++++++++++++++++-----------------------
 2 files changed, 39 insertions(+), 41 deletions(-)

drivers/macintosh/adb.c +11 ~~-13~~

index 226179b975..bb6c759b66 100644
@@ -206,18 +206,17 @@ static int adb_scan_bus(void)
 	}
 
 	/* Now fill in the handler_id field of the adb_handler entries. */
-	printk(KERN_DEBUG "adb devices:");
+	pr_debug("adb devices:\n");
 	for (i = 1; i < 16; i++) {
 		if (adb_handler[i].original_address == 0)
 			continue;
 		adb_request(&req, NULL, ADBREQ_SYNC | ADBREQ_REPLY, 1,
 			    (i << 4) | 0xf);
 		adb_handler[i].handler_id = req.reply[2];
-		printk(" [%d]: %d %x", i, adb_handler[i].original_address,
-		       adb_handler[i].handler_id);
+		pr_debug(" [%d]: %d %x\n", i, adb_handler[i].original_address,
+			 adb_handler[i].handler_id);
 		devmask |= 1 << i;
 	}
-	printk("\n");
 	return devmask;
 }

@@ -228,9 +227,9 @@ static int adb_scan_bus(void)
 static int
 adb_probe_task(void *x)
 {
-	printk(KERN_INFO "adb: starting probe task...\n");
+	pr_info("adb: starting probe task...\n");
 	do_adb_reset_bus();
-	printk(KERN_INFO "adb: finished probe task...\n");
+	pr_info("adb: finished probe task...\n");
 
 	up(&adb_probe_mutex);

@@ -340,7 +339,7 @@ static int __init adb_init(void)
 	    adb_controller->init())
 		adb_controller = NULL;
 	if (adb_controller == NULL) {
-		printk(KERN_WARNING "Warning: no ADB interface detected\n");
+		pr_warn("Warning: no ADB interface detected\n");
 	} else {
 #ifdef CONFIG_PPC
 		if (of_machine_is_compatible("AAPL,PowerBook1998") ||

@@ -483,8 +482,7 @@ adb_register(int default_id, int handler_id, struct adb_ids *ids,
 		    (!handler_id || (handler_id == adb_handler[i].handler_id) || 
 		    try_handler_change(i, handler_id))) {
 			if (adb_handler[i].handler != 0) {
-				printk(KERN_ERR
-				       "Two handlers for ADB device %d\n",
+				pr_err("Two handlers for ADB device %d\n",
 				       default_id);
 				continue;
 			}

@@ -538,10 +536,10 @@ adb_input(unsigned char *buf, int nb, int autopoll)
 		
 	id = buf[0] >> 4;
 	if (dump_adb_input) {
-		printk(KERN_INFO "adb packet: ");
+		pr_info("adb packet: ");
 		for (i = 0; i < nb; ++i)
-			printk(" %x", buf[i]);
-		printk(", id = %d\n", id);
+			pr_cont(" %x", buf[i]);
+		pr_cont(", id = %d\n", id);
 	}
 	write_lock_irqsave(&adb_handler_lock, flags);
 	handler = adb_handler[id].handler;

@@ -891,7 +889,7 @@ static void __init
 adbdev_init(void)
 {
 	if (register_chrdev(ADB_MAJOR, "adb", &adb_fops)) {
-		printk(KERN_ERR "adb: unable to get major %d\n", ADB_MAJOR);
+		pr_err("adb: unable to get major %d\n", ADB_MAJOR);
 		return;
 	}

drivers/macintosh/adbhid.c +28 ~~-28~~

184 lines not on this page · raw message

Signature · 7 lines

2.10.2


-- 
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Benjamin Herrenschmidt

2016-11-26 22:43 UTC #1 lore raw

Quoted 8 lines · Andreas Schwab

{
-       printk(KERN_INFO "adb: starting probe task...\n"); +       pr_info("adb: starting probe task...\n"); do_adb_reset_bus();
-       printk(KERN_INFO "adb: finished probe task...\n"); +       pr_info("adb: finished probe task...\n");
        up(&adb_probe_mutex

While at it, that should probably be pr_debug...

Cheers,
Ben.

Andreas Schwab

author

2016-11-28 20:29 UTC #2 lore raw

PATCH v2 1/1 adb: properly mark continued kernel messages

Use pr_cont where appropriate, and switch to pr_foo throughout. Additionally, lower messages in adb_probe_task to debug level.

Signed-off-by Andreas Schwab <[email protected]>

---
 drivers/macintosh/adb.c    | 24 +++++++++-----------
 drivers/macintosh/adbhid.c | 56 +++++++++++++++++++++++-----------------------
 2 files changed, 39 insertions(+), 41 deletions(-)

drivers/macintosh/adb.c +11 ~~-13~~

index 226179b975..5abc265383 100644
@@ -206,18 +206,17 @@ static int adb_scan_bus(void)
 	}
 
 	/* Now fill in the handler_id field of the adb_handler entries. */
-	printk(KERN_DEBUG "adb devices:");
+	pr_debug("adb devices:\n");
 	for (i = 1; i < 16; i++) {
 		if (adb_handler[i].original_address == 0)
 			continue;
 		adb_request(&req, NULL, ADBREQ_SYNC | ADBREQ_REPLY, 1,
 			    (i << 4) | 0xf);
 		adb_handler[i].handler_id = req.reply[2];
-		printk(" [%d]: %d %x", i, adb_handler[i].original_address,
-		       adb_handler[i].handler_id);
+		pr_debug(" [%d]: %d %x\n", i, adb_handler[i].original_address,
+			 adb_handler[i].handler_id);
 		devmask |= 1 << i;
 	}
-	printk("\n");
 	return devmask;
 }

@@ -228,9 +227,9 @@ static int adb_scan_bus(void)
 static int
 adb_probe_task(void *x)
 {
-	printk(KERN_INFO "adb: starting probe task...\n");
+	pr_debug("adb: starting probe task...\n");
 	do_adb_reset_bus();
-	printk(KERN_INFO "adb: finished probe task...\n");
+	pr_debug("adb: finished probe task...\n");
 
 	up(&adb_probe_mutex);

@@ -340,7 +339,7 @@ static int __init adb_init(void)
 	    adb_controller->init())
 		adb_controller = NULL;
 	if (adb_controller == NULL) {
-		printk(KERN_WARNING "Warning: no ADB interface detected\n");
+		pr_warn("Warning: no ADB interface detected\n");
 	} else {
 #ifdef CONFIG_PPC
 		if (of_machine_is_compatible("AAPL,PowerBook1998") ||

@@ -483,8 +482,7 @@ adb_register(int default_id, int handler_id, struct adb_ids *ids,
 		    (!handler_id || (handler_id == adb_handler[i].handler_id) || 
 		    try_handler_change(i, handler_id))) {
 			if (adb_handler[i].handler != 0) {
-				printk(KERN_ERR
-				       "Two handlers for ADB device %d\n",
+				pr_err("Two handlers for ADB device %d\n",
 				       default_id);
 				continue;
 			}

@@ -538,10 +536,10 @@ adb_input(unsigned char *buf, int nb, int autopoll)
 		
 	id = buf[0] >> 4;
 	if (dump_adb_input) {
-		printk(KERN_INFO "adb packet: ");
+		pr_info("adb packet: ");
 		for (i = 0; i < nb; ++i)
-			printk(" %x", buf[i]);
-		printk(", id = %d\n", id);
+			pr_cont(" %x", buf[i]);
+		pr_cont(", id = %d\n", id);
 	}
 	write_lock_irqsave(&adb_handler_lock, flags);
 	handler = adb_handler[id].handler;

@@ -891,7 +889,7 @@ static void __init
 adbdev_init(void)
 {
 	if (register_chrdev(ADB_MAJOR, "adb", &adb_fops)) {
-		printk(KERN_ERR "adb: unable to get major %d\n", ADB_MAJOR);
+		pr_err("adb: unable to get major %d\n", ADB_MAJOR);
 		return;
 	}

drivers/macintosh/adbhid.c +28 ~~-28~~

184 lines not on this page · raw message

Signature · 6 lines

2.10.2

-- 
Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

David Laight

2016-11-30 15:20 UTC #3 lore raw

From: Andreas Schwab

Quoted 3 lines

Sent: 28 November 2016 20:29
Use pr_cont where appropriate, and switch to pr_foo throughout. Additionally, lower messages in adb_probe_task to debug level.

Doesn't pr_cont() have its own major problem - garbled messages if there are concurrent writers?
So it is best avoided if at all possible.

David
=20

Andreas Schwab

author

2016-11-30 17:58 UTC #4 lore raw

Quoted 3 lines · David Laight

Doesn't pr_cont() have its own major problem - garbled messages if there are concurrent writers?
So it is best avoided if at all possible.

You are free to submit a followup patch the rework the messaging. My patch is a strict improvement.

Andreas.

Signature · 3 lines

Andreas Schwab, [email protected]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Michael Ellerman

2016-12-02 04:10 UTC #5 lore raw

In reply to Andreas Schwab

Quoted 14 lines · Andreas Schwab

Use pr_cont where appropriate, and switch to pr_foo throughout. Additionally, lower messages in adb_probe_task to debug level.
Signed-off-by Andreas Schwab <[email protected]>
---
 drivers/macintosh/adb.c    | 24 +++++++++-----------
 drivers/macintosh/adbhid.c | 56 +++++++++++++++++++++++-----------------------
 2 files changed, 39 insertions(+), 41 deletions(-)
drivers/macintosh/adb.c +0 -0
index 226179b975..5abc265383 100644

drivers/macintosh/adb.c static int adb_scan_bus(void) · line 209

Quoted 5 lines

drivers/macintosh/adb.c +1 -1

@@ -206,18 +206,17 @@ static int adb_scan_bus(void)
 	}
 
 	/* Now fill in the handler_id field of the adb_handler entries. */
-	printk(KERN_DEBUG "adb devices:");
+	pr_debug("adb devices:\n");

Unfortunately pr_debug() is not directly equivalent to printk(KERN_DEBUG). The latter always prints at DEBUG level, the former prints at DEBUG level if it is enabled via dynamic debug, or if DEBUG is #defined in the source.

So in general using pr_debug() means the messages won't be printed, unless a user takes some action to enable it.

Perhaps in this case that's actually what we want to do, but if so we should call it out in the change log.

cheers

Michael Ellerman

Applied

2018-01-29 04:13 UTC #6 lore raw

In reply to Andreas Schwab

Quoted 4 lines · Andreas Schwab

Use pr_cont where appropriate, and switch to pr_foo throughout. Additionally, lower messages in adb_probe_task to debug level.
Signed-off-by Andreas Schwab <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/f2be6295684b0fe879a824ac974797

cheers