ACPI: add missing KERN_* constants to printks

PATCH Superseded Frank Seidel 4 participants 16 messages Feb 4, 2009 – Feb 7, 2009

Subsystems (2)

ACPI PCI

lore

Share & tools

lore

Frank Seidel

author

2009-02-04 14:35 UTC #0 lore raw

PATCH 1/1 ACPI: add missing KERN_* constants to printks

From: Frank Seidel <[email protected]>

According to kerneljanitors todo list all printk calls should have an according KERN_* constant.
Those are the missing peaces here for the acpi subsystem.

Signed-off-by Frank Seidel <[email protected]>

---
 drivers/acpi/container.c      |    5 +++--
 drivers/acpi/dock.c           |    8 ++++----
 drivers/acpi/osl.c            |    4 ++--
 drivers/acpi/pci_irq.c        |    4 ++--
 drivers/acpi/pci_link.c       |   14 +++++++-------
 drivers/acpi/processor_core.c |    5 +++--
 drivers/acpi/processor_idle.c |    4 ++--
 drivers/acpi/sleep.c          |   10 +++++-----
 drivers/acpi/video.c          |    2 +-
 9 files changed, 29 insertions(+), 27 deletions(-)

drivers/acpi/video.c +1 -1

@@ -1260,7 +1260,7 @@ static int acpi_video_bus_POST_info_seq_
 			printk(KERN_WARNING PREFIX
 			       "This indicates a BIOS bug. Please contact the manufacturer.\n");
 		}
-		printk("%llx\n", options);
+		printk(KERN_WARNING "%llx\n", options);
 		seq_printf(seq, "can POST: <integrated video>");
 		if (options & 2)
 			seq_printf(seq, " <PCI video>");

drivers/acpi/container.c +3 -2

@@ -163,7 +163,7 @@ static void container_notify_cb(acpi_han
 	case ACPI_NOTIFY_BUS_CHECK:
 		/* Fall through */
 	case ACPI_NOTIFY_DEVICE_CHECK:
-		printk("Container driver received %s event\n",
+		printk(KERN_WARNING "Container driver received %s event\n",
 		       (type == ACPI_NOTIFY_BUS_CHECK) ?
 		       "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK");
 		status = acpi_bus_get_device(handle, &device);

@@ -174,7 +174,8 @@ static void container_notify_cb(acpi_han
 					kobject_uevent(&device->dev.kobj,
 						       KOBJ_ONLINE);
 				else
-					printk("Failed to add container\n");
+					printk(KERN_WARNING
+					       "Failed to add container\n");
 			}
 		} else {
 			if (ACPI_SUCCESS(status)) {

drivers/acpi/pci_irq.c +2 -2

@@ -400,12 +400,12 @@ int acpi_pci_irq_enable(struct pci_dev *
 		dev_warn(&dev->dev, "PCI INT %c: no GSI", pin_name(pin));
 		/* Interrupt Line values above 0xF are forbidden */
 		if (dev->irq > 0 && (dev->irq <= 0xF)) {
-			printk(" - using IRQ %d\n", dev->irq);
+			printk(KERN_WARNING " - using IRQ %d\n", dev->irq);
 			acpi_register_gsi(dev->irq, ACPI_LEVEL_SENSITIVE,
 					  ACPI_ACTIVE_LOW);
 			return 0;
 		} else {
-			printk("\n");
+			printk(KERN_WARNING "\n");
 			return 0;
 		}
 	}

drivers/acpi/pci_link.c +7 -7

@@ -593,7 +593,7 @@ static int acpi_pci_link_allocate(struct
 		return -ENODEV;
 	} else {
 		acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
-		printk(PREFIX "%s [%s] enabled at IRQ %d\n",
+		printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
 		       acpi_device_name(link->device),
 		       acpi_device_bid(link->device), link->irq.active);
 	}

@@ -751,21 +751,21 @@ static int acpi_pci_link_add(struct acpi
 	       acpi_device_bid(device));
 	for (i = 0; i < link->irq.possible_count; i++) {
 		if (link->irq.active == link->irq.possible[i]) {
-			printk(" *%d", link->irq.possible[i]);
+			printk(KERN_INFO " *%d", link->irq.possible[i]);
 			found = 1;
 		} else
-			printk(" %d", link->irq.possible[i]);
+			printk(KERN_INFO " %d", link->irq.possible[i]);
 	}
 
-	printk(")");
+	printk(KERN_INFO ")");
 
 	if (!found)
-		printk(" *%d", link->irq.active);
+		printk(KERN_INFO " *%d", link->irq.active);
 
 	if (!link->device->status.enabled)
-		printk(", disabled.");
+		printk(KERN_INFO ", disabled.");
 
-	printk("\n");
+	printk(KERN_INFO "\n");
 
 	/* TBD: Acquire/release lock */
 	list_add_tail(&link->node, &acpi_link.entries);

drivers/acpi/dock.c +4 -4

@@ -984,7 +984,7 @@ static int dock_add(acpi_handle handle)
 
 	ret = device_create_file(&dock_device->dev, &dev_attr_docked);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		platform_device_unregister(dock_device);
 		kfree(dock_station);
 		dock_station = NULL;

@@ -992,7 +992,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_undock);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		platform_device_unregister(dock_device);
 		kfree(dock_station);

@@ -1001,7 +1001,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_uid);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		device_remove_file(&dock_device->dev, &dev_attr_undock);
 		platform_device_unregister(dock_device);

@@ -1011,7 +1011,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_flags);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		device_remove_file(&dock_device->dev, &dev_attr_undock);
 		device_remove_file(&dock_device->dev, &dev_attr_uid);

drivers/acpi/processor_idle.c +2 -2

@@ -1861,9 +1861,9 @@ int __cpuinit acpi_processor_power_init(
 		printk(KERN_INFO PREFIX "CPU%d (power states:", pr->id);
 		for (i = 1; i <= pr->power.count; i++)
 			if (pr->power.states[i].valid)
-				printk(" C%d[C%d]", i,
+				printk(KERN_INFO " C%d[C%d]", i,
 				       pr->power.states[i].type);
-		printk(")\n");
+		printk(KERN_INFO ")\n");
 
 #ifndef CONFIG_CPU_IDLE
 		if (pr->id == 0) {

drivers/acpi/osl.c +2 -2

@@ -228,10 +228,10 @@ void acpi_os_vprintf(const char *fmt, va
 	if (acpi_in_debugger) {
 		kdb_printf("%s", buffer);
 	} else {
-		printk("%s", buffer);
+		printk(KERN_WARNING "%s", buffer);
 	}
 #else
-	printk("%s", buffer);
+	printk(KERN_WARNING "%s", buffer);
 #endif
 }

drivers/acpi/processor_core.c +3 -2

@@ -742,8 +742,9 @@ static int __cpuinit acpi_processor_star
 	if (pr->flags.throttling) {
 		printk(KERN_INFO PREFIX "%s [%s] (supports",
 		       acpi_device_name(device), acpi_device_bid(device));
-		printk(" %d throttling states", pr->throttling.state_count);
-		printk(")\n");
+		printk(KERN_INFO
+		       " %d throttling states", pr->throttling.state_count);
+		printk(KERN_INFO ")\n");
 	}
 
       end:

drivers/acpi/sleep.c +5 -5

@@ -679,7 +679,7 @@ static void acpi_power_off_prepare(void)
 static void acpi_power_off(void)
 {
 	/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */
-	printk("%s called\n", __func__);
+	printk(KERN_DEBUG "%s called\n", __func__);
 	local_irq_disable();
 	acpi_enable_wakeup_device(ACPI_STATE_S5);
 	acpi_enter_sleep_state(ACPI_STATE_S5);

@@ -706,7 +706,7 @@ int __init acpi_sleep_init(void)
 		status = acpi_get_sleep_type_data(i, &type_a, &type_b);
 		if (ACPI_SUCCESS(status)) {
 			sleep_states[i] = 1;
-			printk(" S%d", i);
+			printk(KERN_INFO " S%d", i);
 		}
 	}

@@ -720,7 +720,7 @@ int __init acpi_sleep_init(void)
 		hibernation_set_ops(old_suspend_ordering ?
 			&acpi_hibernation_ops_old : &acpi_hibernation_ops);
 		sleep_states[ACPI_STATE_S4] = 1;
-		printk(" S4");
+		printk(KERN_INFO " S4");
 		if (!nosigcheck) {
 			acpi_get_table(ACPI_SIG_FACS, 1,
 				(struct acpi_table_header **)&facs);

@@ -733,11 +733,11 @@ int __init acpi_sleep_init(void)
 	status = acpi_get_sleep_type_data(ACPI_STATE_S5, &type_a, &type_b);
 	if (ACPI_SUCCESS(status)) {
 		sleep_states[ACPI_STATE_S5] = 1;
-		printk(" S5");
+		printk(KERN_INFO " S5");
 		pm_power_off_prepare = acpi_power_off_prepare;
 		pm_power_off = acpi_power_off;
 	}
-	printk(")\n");
+	printk(KERN_INFO ")\n");
 	/*
 	 * Register the tts_notifier to reboot notifier list so that the _TTS
 	 * object can also be evaluated when the system enters S5.

Frans Pop

2009-02-04 15:28 UTC #1 lore raw

Quoted 19 lines · Frank Seidel

+++ b/drivers/acpi/pci_link.c

drivers/acpi/pci_link.c +2 -3

@@ -593,7 +593,7 @@ static int acpi_pci_link_allocate(struct
                return -ENODEV;
        } else {
                acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
-               printk(PREFIX "%s [%s] enabled at IRQ %d\n",
+               printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
                acpi_device_name(link->device),
                acpi_device_bid(link->device), link->irq.active);
        }

@@ -751,21 +751,21 @@ static int acpi_pci_link_add(struct acpi
        acpi_device_bid(device));
        for (i = 0; i < link->irq.possible_count; i++) {
                if (link->irq.active == link->irq.possible[i]) {
-                       printk(" *%d", link->irq.possible[i]);
+                       printk(KERN_INFO " *%d", link->irq.possible[i]);
                        found = 1;
                } else
-                       printk(" %d", link->irq.possible[i]);

drivers/acpi/pci_link.c static int acpi_pci_link_add(struct acpi · line 764

Quoted 10 lines

drivers/acpi/pci_link.c +3 -2

+                       printk(KERN_INFO " %d", link->irq.possible[i]);
        }
 
-       printk(")");
+       printk(KERN_INFO ")");
 
        if (!found)
-               printk(" *%d", link->irq.active);
+               printk(KERN_INFO " *%d", link->irq.active);

This patch looks broken to me, at least for some of your changes. For example, in the bit quoted above all printks together make up *one single* message, which means that only the _first_ of the printks should have the KERN_* prefix. printks that are continuations should not have the prefix.

Cheers,
FJP

Frans Pop

2009-02-04 15:28 UTC #12 lore raw

In reply to Frank Seidel

Quoted 19 lines · Frank Seidel

+++ b/drivers/acpi/pci_link.c

drivers/acpi/pci_link.c +2 -3

@@ -593,7 +593,7 @@ static int acpi_pci_link_allocate(struct
                return -ENODEV;
        } else {
                acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
-               printk(PREFIX "%s [%s] enabled at IRQ %d\n",
+               printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
                acpi_device_name(link->device),
                acpi_device_bid(link->device), link->irq.active);
        }

@@ -751,21 +751,21 @@ static int acpi_pci_link_add(struct acpi
        acpi_device_bid(device));
        for (i = 0; i < link->irq.possible_count; i++) {
                if (link->irq.active == link->irq.possible[i]) {
-                       printk(" *%d", link->irq.possible[i]);
+                       printk(KERN_INFO " *%d", link->irq.possible[i]);
                        found = 1;
                } else
-                       printk(" %d", link->irq.possible[i]);

drivers/acpi/pci_link.c static int acpi_pci_link_add(struct acpi · line 764

Quoted 10 lines

drivers/acpi/pci_link.c +3 -2

+                       printk(KERN_INFO " %d", link->irq.possible[i]);
        }
 
-       printk(")");
+       printk(KERN_INFO ")");
 
        if (!found)
-               printk(" *%d", link->irq.active);
+               printk(KERN_INFO " *%d", link->irq.active);

Cheers,
FJP

Frank Seidel

author

2009-02-04 15:31 UTC #2 lore raw

In reply to Frans Pop

Quoted 5 lines · Frans Pop

This patch looks broken to me, at least for some of your changes. For example, in the bit quoted above all printks together make up *one single* message, which means that only the _first_ of the printks should have the KERN_* prefix. printks that are continuations should not have the prefix.

Are you absolutely sure about that? The janitors todo lets me guess they should aswell me prefixed.
Otherwise yes, then some of the changes wouldn't be needed.

Thanks,
Frank

Frank Seidel

author

2009-02-04 15:31 UTC #3 lore raw

In reply to Frans Pop

Quoted 5 lines · Frans Pop

This patch looks broken to me, at least for some of your changes. For example, in the bit quoted above all printks together make up *one single* message, which means that only the _first_ of the printks should have the KERN_* prefix. printks that are continuations should not have the prefix.

Are you absolutely sure about that? The janitors todo lets me guess they should aswell me prefixed.
Otherwise yes, then some of the changes wouldn't be needed.

Thanks,
Frank

Geert Uytterhoeven

2009-02-04 15:44 UTC #4 lore raw

In reply to Frans Pop

Quoted 26 lines · Frans Pop

Quoted 25 lines · Frank Seidel

+++ b/drivers/acpi/pci_link.c

drivers/acpi/pci_link.c +4 -4

@@ -593,7 +593,7 @@ static int acpi_pci_link_allocate(struct
                return -ENODEV;
        } else {
                acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
-               printk(PREFIX "%s [%s] enabled at IRQ %d\n",
+               printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
                acpi_device_name(link->device),
                acpi_device_bid(link->device), link->irq.active);
        }

@@ -751,21 +751,21 @@ static int acpi_pci_link_add(struct acpi
        acpi_device_bid(device));
        for (i = 0; i < link->irq.possible_count; i++) {
                if (link->irq.active == link->irq.possible[i]) {
-                       printk(" *%d", link->irq.possible[i]);
+                       printk(KERN_INFO " *%d", link->irq.possible[i]);
                        found = 1;
                } else
-                       printk(" %d", link->irq.possible[i]);
+                       printk(KERN_INFO " %d", link->irq.possible[i]);
        }
 
-       printk(")");
+       printk(KERN_INFO ")");

Quoted 10 lines

drivers/acpi/pci_link.c static int acpi_pci_link_add(struct acpi · line 764
Quoted 4 lines
drivers/acpi/pci_link.c +1 -1
        if (!found)
-               printk(" *%d", link->irq.active);
+               printk(KERN_INFO " *%d", link->irq.active);
 
This patch looks broken to me, at least for some of your changes. For example, in the bit quoted above all printks together make up *one single* message, which means that only the _first_ of the printks should have the KERN_* prefix. printks that are continuations should not have the prefix.

Actually they should, but the right prefix :-)

Quoting include/linux/kernel.h:

| /*
|  * Annotation for a "continued" line of log printout (only done after a
|  * line that had no enclosing \n). Only to be used by core/arch code
|  * during early bootup (a continued line is not SMP-safe otherwise).
|  */
| #define KERN_CONT       ""

Please also consider the note about SMP-safeness.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   [email protected]
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V. VAT BE 0413.825.160 · RPR Brussels Fortis · BIC GEBABEBB · IBAN BE41293037680010

Geert Uytterhoeven

2009-02-04 15:44 UTC #5 lore raw

In reply to Frans Pop

Quoted 26 lines · Frans Pop

Quoted 25 lines · Frank Seidel

+++ b/drivers/acpi/pci_link.c

drivers/acpi/pci_link.c +4 -4

@@ -593,7 +593,7 @@ static int acpi_pci_link_allocate(struct
                return -ENODEV;
        } else {
                acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
-               printk(PREFIX "%s [%s] enabled at IRQ %d\n",
+               printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
                acpi_device_name(link->device),
                acpi_device_bid(link->device), link->irq.active);
        }

@@ -751,21 +751,21 @@ static int acpi_pci_link_add(struct acpi
        acpi_device_bid(device));
        for (i = 0; i < link->irq.possible_count; i++) {
                if (link->irq.active == link->irq.possible[i]) {
-                       printk(" *%d", link->irq.possible[i]);
+                       printk(KERN_INFO " *%d", link->irq.possible[i]);
                        found = 1;
                } else
-                       printk(" %d", link->irq.possible[i]);
+                       printk(KERN_INFO " %d", link->irq.possible[i]);
        }
 
-       printk(")");
+       printk(KERN_INFO ")");

Quoted 10 lines

drivers/acpi/pci_link.c static int acpi_pci_link_add(struct acpi · line 764
Quoted 4 lines
drivers/acpi/pci_link.c +1 -1
        if (!found)
-               printk(" *%d", link->irq.active);
+               printk(KERN_INFO " *%d", link->irq.active);
 
This patch looks broken to me, at least for some of your changes. For example, in the bit quoted above all printks together make up *one single* message, which means that only the _first_ of the printks should have the KERN_* prefix. printks that are continuations should not have the prefix.

Actually they should, but the right prefix :-)

Quoting include/linux/kernel.h:

| /*
|  * Annotation for a "continued" line of log printout (only done after a
|  * line that had no enclosing \n). Only to be used by core/arch code
|  * during early bootup (a continued line is not SMP-safe otherwise).
|  */
| #define KERN_CONT       ""

Please also consider the note about SMP-safeness.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   [email protected]
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Frank Seidel

author

2009-02-04 15:48 UTC #6 lore raw

Quoted 4 lines · Geert Uytterhoeven

Actually they should, but the right prefix :-)
Quoting include/linux/kernel.h:
...

Ah, no i see. And i missed the line (from janitors todo) that this should only be done for line beginnings (and now KERN_CONT for core/arch..).
Thank you very much that pointer. Will keep it in mind for my further cleanups of this topic.
Of course i will then also redo the above patch.

Thanks,
Frank

Frank Seidel

author

2009-02-04 15:48 UTC #7 lore raw

In reply to Geert Uytterhoeven

Quoted 4 lines · Geert Uytterhoeven

Actually they should, but the right prefix :-)
Quoting include/linux/kernel.h:
...

Thanks,
Frank

Frank Seidel

author

2009-02-04 16:03 UTC #13 lore raw

PATCH v2 1/1 ACPI: add missing KERN_* constants to printks

In reply to Frank Seidel

From: Frank Seidel <[email protected]>

According to kerneljanitors todo list all printk calls (beginning a new line) should have an according KERN_* constant. Those are the missing peaces here for the acpi subsystem.

Signed-off-by Frank Seidel <[email protected]>

---
 drivers/acpi/container.c |    5 +++--
 drivers/acpi/dock.c      |    8 ++++----
 drivers/acpi/osl.c       |    4 ++--
 drivers/acpi/pci_link.c  |    2 +-
 drivers/acpi/sleep.c     |    2 +-
 drivers/acpi/video.c     |    2 +-
 6 files changed, 12 insertions(+), 11 deletions(-)

drivers/acpi/video.c +1 -1

@@ -1260,7 +1260,7 @@ static int acpi_video_bus_POST_info_seq_
 			printk(KERN_WARNING PREFIX
 			       "This indicates a BIOS bug. Please contact the manufacturer.\n");
 		}
-		printk("%llx\n", options);
+		printk(KERN_WARNING "%llx\n", options);
 		seq_printf(seq, "can POST: <integrated video>");
 		if (options & 2)
 			seq_printf(seq, " <PCI video>");

drivers/acpi/container.c +3 -2

@@ -163,7 +163,7 @@ static void container_notify_cb(acpi_han
 	case ACPI_NOTIFY_BUS_CHECK:
 		/* Fall through */
 	case ACPI_NOTIFY_DEVICE_CHECK:
-		printk("Container driver received %s event\n",
+		printk(KERN_WARNING "Container driver received %s event\n",
 		       (type == ACPI_NOTIFY_BUS_CHECK) ?
 		       "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK");
 		status = acpi_bus_get_device(handle, &device);

@@ -174,7 +174,8 @@ static void container_notify_cb(acpi_han
 					kobject_uevent(&device->dev.kobj,
 						       KOBJ_ONLINE);
 				else
-					printk("Failed to add container\n");
+					printk(KERN_WARNING
+					       "Failed to add container\n");
 			}
 		} else {
 			if (ACPI_SUCCESS(status)) {

drivers/acpi/pci_link.c +1 -1

@@ -593,7 +593,7 @@ static int acpi_pci_link_allocate(struct
 		return -ENODEV;
 	} else {
 		acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
-		printk(PREFIX "%s [%s] enabled at IRQ %d\n",
+		printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
 		       acpi_device_name(link->device),
 		       acpi_device_bid(link->device), link->irq.active);
 	}

drivers/acpi/dock.c +4 -4

@@ -984,7 +984,7 @@ static int dock_add(acpi_handle handle)
 
 	ret = device_create_file(&dock_device->dev, &dev_attr_docked);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		platform_device_unregister(dock_device);
 		kfree(dock_station);
 		dock_station = NULL;

@@ -992,7 +992,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_undock);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		platform_device_unregister(dock_device);
 		kfree(dock_station);

@@ -1001,7 +1001,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_uid);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		device_remove_file(&dock_device->dev, &dev_attr_undock);
 		platform_device_unregister(dock_device);

@@ -1011,7 +1011,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_flags);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		device_remove_file(&dock_device->dev, &dev_attr_undock);
 		device_remove_file(&dock_device->dev, &dev_attr_uid);

drivers/acpi/osl.c +2 -2

@@ -228,10 +228,10 @@ void acpi_os_vprintf(const char *fmt, va
 	if (acpi_in_debugger) {
 		kdb_printf("%s", buffer);
 	} else {
-		printk("%s", buffer);
+		printk(KERN_WARNING "%s", buffer);
 	}
 #else
-	printk("%s", buffer);
+	printk(KERN_WARNING "%s", buffer);
 #endif
 }

drivers/acpi/sleep.c +1 -1

@@ -679,7 +679,7 @@ static void acpi_power_off_prepare(void)
 static void acpi_power_off(void)
 {
 	/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */
-	printk("%s called\n", __func__);
+	printk(KERN_DEBUG "%s called\n", __func__);
 	local_irq_disable();
 	acpi_enable_wakeup_device(ACPI_STATE_S5);
 	acpi_enter_sleep_state(ACPI_STATE_S5);

Frans Pop

2009-02-04 16:12 UTC #8 lore raw

In reply to Geert Uytterhoeven

Quoted 7 lines · Geert Uytterhoeven

Quoted 5 lines
This patch looks broken to me, at least for some of your changes. For example, in the bit quoted above all printks together make up *one single* message, which means that only the _first_ of the printks should have the KERN_* prefix. printks that are continuations should not have the prefix.
Actually they should, but the right prefix :-)

Hmm. I was going by memory and from what I've seen in existing code, but also found this (somewhat old) post:
https://lists.linux-foundation.org/pipermail/kernel-janitors/2005-October/014375.html

Quoted 11 lines

Quoting include/linux/kernel.h:
| /*
|  * Annotation for a "continued" line of log printout (only done after a
|  * line that had no enclosing \n). Only to be used by core/arch code
|  * during early bootup (a continued line is not SMP-safe otherwise).
|  */
| #define KERN_CONT       ""
Please also consider the note about SMP-safeness.
From that it looks like KERN_CONT should only be used in a very limited

context, but I guess this example qualifies.

Frans Pop

2009-02-04 16:13 UTC #9 lore raw

In reply to Geert Uytterhoeven

Quoted 7 lines · Geert Uytterhoeven

Quoted 5 lines
This patch looks broken to me, at least for some of your changes. For example, in the bit quoted above all printks together make up *one single* message, which means that only the _first_ of the printks should have the KERN_* prefix. printks that are continuations should not have the prefix.
Actually they should, but the right prefix :-)

Hmm. I was going by memory and from what I've seen in existing code, but also found this (somewhat old) post:
https://lists.linux-foundation.org/pipermail/kernel-janitors/2005-October/014375.html

Quoted 11 lines

Quoting include/linux/kernel.h:
| /*
|  * Annotation for a "continued" line of log printout (only done after a
|  * line that had no enclosing \n). Only to be used by core/arch code
|  * during early bootup (a continued line is not SMP-safe otherwise).
|  */
| #define KERN_CONT       ""
Please also consider the note about SMP-safeness.
From that it looks like KERN_CONT should only be used in a very limited

context, but I guess this example qualifies.

Geert Uytterhoeven

2009-02-04 16:48 UTC #10 lore raw

Quoted 7 lines · Frans Pop

Quoted 6 lines · Geert Uytterhoeven
Quoted 5 lines
This patch looks broken to me, at least for some of your changes. For example, in the bit quoted above all printks together make up *one single* message, which means that only the _first_ of the printks should have the KERN_* prefix. printks that are continuations should not have the prefix.

Quoted 5 lines

Quoted 1 line
Actually they should, but the right prefix :-)
Hmm. I was going by memory and from what I've seen in existing code, but also found this (somewhat old) post:
https://lists.linux-foundation.org/pipermail/kernel-janitors/2005-October/014375.html

That one predates KERN_CONT.

Quoted 8 lines

Quoting include/linux/kernel.h:

| /*
|  * Annotation for a "continued" line of log printout (only done after a
|  * line that had no enclosing \n). Only to be used by core/arch code
|  * during early bootup (a continued line is not SMP-safe otherwise).
|  */
| #define KERN_CONT       ""

Quoted 4 lines

Quoted 1 line
Please also consider the note about SMP-safeness.
From that it looks like KERN_CONT should only be used in a very limited context, but I guess this example qualifies.

Yep, the general advise is to avoid using continuations.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   [email protected]
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V. VAT BE 0413.825.160 · RPR Brussels Fortis · BIC GEBABEBB · IBAN BE41293037680010

Geert Uytterhoeven

2009-02-04 16:48 UTC #11 lore raw

In reply to Frans Pop

Quoted 7 lines · Frans Pop

Quoted 6 lines · Geert Uytterhoeven
Quoted 5 lines
This patch looks broken to me, at least for some of your changes. For example, in the bit quoted above all printks together make up *one single* message, which means that only the _first_ of the printks should have the KERN_* prefix. printks that are continuations should not have the prefix.

Quoted 5 lines

Quoted 1 line
Actually they should, but the right prefix :-)
Hmm. I was going by memory and from what I've seen in existing code, but also found this (somewhat old) post:
https://lists.linux-foundation.org/pipermail/kernel-janitors/2005-October/014375.html

That one predates KERN_CONT.

Quoted 8 lines

Quoting include/linux/kernel.h:

| /*
|  * Annotation for a "continued" line of log printout (only done after a
|  * line that had no enclosing \n). Only to be used by core/arch code
|  * during early bootup (a continued line is not SMP-safe otherwise).
|  */
| #define KERN_CONT       ""

Quoted 4 lines

Quoted 1 line
Please also consider the note about SMP-safeness.
From that it looks like KERN_CONT should only be used in a very limited context, but I guess this example qualifies.

Yep, the general advise is to avoid using continuations.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   [email protected]
Internet: http://www.sony-europe.com/

Len Brown

reviewer

2009-02-07 05:38 UTC #14 lore raw

In reply to Frank Seidel

applied -- though I changed the two in acpi_os_vprintf() to be KERN_CONT, or we'd get a bunch of <4>'s in constructed messages. Yes, we take our chances with SMP on those.

thanks,
Len Brown, Intel Open Source Technology Center

Quoted 129 lines · Frank Seidel

From: Frank Seidel <[email protected]>

According to kerneljanitors todo list all printk calls (beginning a new line) should have an according KERN_* constant. Those are the missing peaces here for the acpi subsystem.

Signed-off-by Frank Seidel <[email protected]>

---
 drivers/acpi/container.c |    5 +++--
 drivers/acpi/dock.c      |    8 ++++----
 drivers/acpi/osl.c       |    4 ++--
 drivers/acpi/pci_link.c  |    2 +-
 drivers/acpi/sleep.c     |    2 +-
 drivers/acpi/video.c     |    2 +-
 6 files changed, 12 insertions(+), 11 deletions(-)

drivers/acpi/video.c +1 -1

@@ -1260,7 +1260,7 @@ static int acpi_video_bus_POST_info_seq_
 			printk(KERN_WARNING PREFIX
 			       "This indicates a BIOS bug. Please contact the manufacturer.\n");
 		}
-		printk("%llx\n", options);
+		printk(KERN_WARNING "%llx\n", options);
 		seq_printf(seq, "can POST: <integrated video>");
 		if (options & 2)
 			seq_printf(seq, " <PCI video>");

drivers/acpi/container.c +3 -2

@@ -163,7 +163,7 @@ static void container_notify_cb(acpi_han
 	case ACPI_NOTIFY_BUS_CHECK:
 		/* Fall through */
 	case ACPI_NOTIFY_DEVICE_CHECK:
-		printk("Container driver received %s event\n",
+		printk(KERN_WARNING "Container driver received %s event\n",
 		       (type == ACPI_NOTIFY_BUS_CHECK) ?
 		       "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK");
 		status = acpi_bus_get_device(handle, &device);

@@ -174,7 +174,8 @@ static void container_notify_cb(acpi_han
 					kobject_uevent(&device->dev.kobj,
 						       KOBJ_ONLINE);
 				else
-					printk("Failed to add container\n");
+					printk(KERN_WARNING
+					       "Failed to add container\n");
 			}
 		} else {
 			if (ACPI_SUCCESS(status)) {

drivers/acpi/pci_link.c +1 -1

@@ -593,7 +593,7 @@ static int acpi_pci_link_allocate(struct
 		return -ENODEV;
 	} else {
 		acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
-		printk(PREFIX "%s [%s] enabled at IRQ %d\n",
+		printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
 		       acpi_device_name(link->device),
 		       acpi_device_bid(link->device), link->irq.active);
 	}

drivers/acpi/dock.c +4 -4

@@ -984,7 +984,7 @@ static int dock_add(acpi_handle handle)
 
 	ret = device_create_file(&dock_device->dev, &dev_attr_docked);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		platform_device_unregister(dock_device);
 		kfree(dock_station);
 		dock_station = NULL;

@@ -992,7 +992,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_undock);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		platform_device_unregister(dock_device);
 		kfree(dock_station);

@@ -1001,7 +1001,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_uid);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		device_remove_file(&dock_device->dev, &dev_attr_undock);
 		platform_device_unregister(dock_device);

@@ -1011,7 +1011,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_flags);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		device_remove_file(&dock_device->dev, &dev_attr_undock);
 		device_remove_file(&dock_device->dev, &dev_attr_uid);

drivers/acpi/osl.c +2 -2

@@ -228,10 +228,10 @@ void acpi_os_vprintf(const char *fmt, va
 	if (acpi_in_debugger) {
 		kdb_printf("%s", buffer);
 	} else {
-		printk("%s", buffer);
+		printk(KERN_WARNING "%s", buffer);
 	}
 #else
-	printk("%s", buffer);
+	printk(KERN_WARNING "%s", buffer);
 #endif
 }

drivers/acpi/sleep.c +1 -2

@@ -679,7 +679,7 @@ static void acpi_power_off_prepare(void)
 static void acpi_power_off(void)
 {
 	/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */
-	printk("%s called\n", __func__);
+	printk(KERN_DEBUG "%s called\n", __func__);
 	local_irq_disable();
 	acpi_enable_wakeup_device(ACPI_STATE_S5);
 	acpi_enter_sleep_state(ACPI_STATE_S5);
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Len Brown

reviewer

2009-02-07 05:38 UTC #15 lore raw

In reply to Frank Seidel

applied -- though I changed the two in acpi_os_vprintf() to be KERN_CONT, or we'd get a bunch of <4>'s in constructed messages. Yes, we take our chances with SMP on those.

thanks,
Len Brown, Intel Open Source Technology Center

Quoted 129 lines · Frank Seidel

From: Frank Seidel <[email protected]>

According to kerneljanitors todo list all printk calls (beginning a new line) should have an according KERN_* constant. Those are the missing peaces here for the acpi subsystem.

Signed-off-by Frank Seidel <[email protected]>

---
 drivers/acpi/container.c |    5 +++--
 drivers/acpi/dock.c      |    8 ++++----
 drivers/acpi/osl.c       |    4 ++--
 drivers/acpi/pci_link.c  |    2 +-
 drivers/acpi/sleep.c     |    2 +-
 drivers/acpi/video.c     |    2 +-
 6 files changed, 12 insertions(+), 11 deletions(-)

drivers/acpi/video.c +1 -1

@@ -1260,7 +1260,7 @@ static int acpi_video_bus_POST_info_seq_
 			printk(KERN_WARNING PREFIX
 			       "This indicates a BIOS bug. Please contact the manufacturer.\n");
 		}
-		printk("%llx\n", options);
+		printk(KERN_WARNING "%llx\n", options);
 		seq_printf(seq, "can POST: <integrated video>");
 		if (options & 2)
 			seq_printf(seq, " <PCI video>");

drivers/acpi/container.c +3 -2

@@ -163,7 +163,7 @@ static void container_notify_cb(acpi_han
 	case ACPI_NOTIFY_BUS_CHECK:
 		/* Fall through */
 	case ACPI_NOTIFY_DEVICE_CHECK:
-		printk("Container driver received %s event\n",
+		printk(KERN_WARNING "Container driver received %s event\n",
 		       (type == ACPI_NOTIFY_BUS_CHECK) ?
 		       "ACPI_NOTIFY_BUS_CHECK" : "ACPI_NOTIFY_DEVICE_CHECK");
 		status = acpi_bus_get_device(handle, &device);

@@ -174,7 +174,8 @@ static void container_notify_cb(acpi_han
 					kobject_uevent(&device->dev.kobj,
 						       KOBJ_ONLINE);
 				else
-					printk("Failed to add container\n");
+					printk(KERN_WARNING
+					       "Failed to add container\n");
 			}
 		} else {
 			if (ACPI_SUCCESS(status)) {

drivers/acpi/pci_link.c +1 -1

@@ -593,7 +593,7 @@ static int acpi_pci_link_allocate(struct
 		return -ENODEV;
 	} else {
 		acpi_irq_penalty[link->irq.active] += PIRQ_PENALTY_PCI_USING;
-		printk(PREFIX "%s [%s] enabled at IRQ %d\n",
+		printk(KERN_WARNING PREFIX "%s [%s] enabled at IRQ %d\n",
 		       acpi_device_name(link->device),
 		       acpi_device_bid(link->device), link->irq.active);
 	}

drivers/acpi/dock.c +4 -4

@@ -984,7 +984,7 @@ static int dock_add(acpi_handle handle)
 
 	ret = device_create_file(&dock_device->dev, &dev_attr_docked);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		platform_device_unregister(dock_device);
 		kfree(dock_station);
 		dock_station = NULL;

@@ -992,7 +992,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_undock);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		platform_device_unregister(dock_device);
 		kfree(dock_station);

@@ -1001,7 +1001,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_uid);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		device_remove_file(&dock_device->dev, &dev_attr_undock);
 		platform_device_unregister(dock_device);

@@ -1011,7 +1011,7 @@ static int dock_add(acpi_handle handle)
 	}
 	ret = device_create_file(&dock_device->dev, &dev_attr_flags);
 	if (ret) {
-		printk("Error %d adding sysfs file\n", ret);
+		printk(KERN_ERR "Error %d adding sysfs file\n", ret);
 		device_remove_file(&dock_device->dev, &dev_attr_docked);
 		device_remove_file(&dock_device->dev, &dev_attr_undock);
 		device_remove_file(&dock_device->dev, &dev_attr_uid);

drivers/acpi/osl.c +2 -2

@@ -228,10 +228,10 @@ void acpi_os_vprintf(const char *fmt, va
 	if (acpi_in_debugger) {
 		kdb_printf("%s", buffer);
 	} else {
-		printk("%s", buffer);
+		printk(KERN_WARNING "%s", buffer);
 	}
 #else
-	printk("%s", buffer);
+	printk(KERN_WARNING "%s", buffer);
 #endif
 }

drivers/acpi/sleep.c +1 -2

@@ -679,7 +679,7 @@ static void acpi_power_off_prepare(void)
 static void acpi_power_off(void)
 {
 	/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */
-	printk("%s called\n", __func__);
+	printk(KERN_DEBUG "%s called\n", __func__);
 	local_irq_disable();
 	acpi_enable_wakeup_device(ACPI_STATE_S5);
 	acpi_enter_sleep_state(ACPI_STATE_S5);
 
--

To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html