2012-06-18 02:25:20

by Joe Perches

[permalink] [raw]
Subject: [PATCH 0/5] printk: Add eliminated_printk

no_printk keeps side-effects.
eliminated_printk removes all side effects yet still keeps
gcc format and argument verificatino.
By definition, pr_devel should have no side-effects if not enabled.
Convert pr_devel from no_printk to eliminated_printk.

Joe Perches (5):
printk: Add eliminated_printk macro
printk: Convert pr_devel to use eliminated_printk
usb: Convert dbg to pr_eliminated and pr_debug
fujitsu-laptop: Convert dbg_printk and vdbg_printk to fuj_dbg
thinkpad_acpi: Convert dbg_printk to tp_dbg and tp_vdbg

drivers/platform/x86/fujitsu-laptop.c | 103 +++++-----
drivers/platform/x86/thinkpad_acpi.c | 358 +++++++++++++++------------------
include/linux/printk.h | 12 +-
include/linux/usb.h | 12 +-
4 files changed, 228 insertions(+), 257 deletions(-)

--
1.7.8.111.gad25c.dirty


2012-06-18 02:26:08

by Joe Perches

[permalink] [raw]
Subject: [PATCH 1/5] printk: Add eliminated_printk macro

no_printk keeps gcc side-effects.
Add a mechanism to eliminate the side-effects.

Signed-off-by: Joe Perches <[email protected]>
---
include/linux/printk.h | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 1bec2f7..84aa910 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -88,6 +88,16 @@ int no_printk(const char *fmt, ...)
return 0;
}

+/*
+ * Dummy printk for disabled printks maintaining
+ * gcc's format checking and eliminating side-effects
+ */
+#define eliminated_printk(fmt, ...) \
+do { \
+ if (0) \
+ printk(fmt, ##__VA_ARGS__); \
+} while (0)
+
extern asmlinkage __printf(1, 2)
void early_printk(const char *fmt, ...);

--
1.7.8.111.gad25c.dirty

2012-06-18 02:26:24

by Joe Perches

[permalink] [raw]
Subject: [PATCH 5/5] thinkpad_acpi: Convert dbg_printk to tp_dbg and tp_vdbg

Use a more current logging style.

Add #define DEBUG and use pr_debug to enable dynamic debugging.
Coalesce formats and align arguments.

Signed-off-by: Joe Perches <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 358 ++++++++++++++++------------------
1 files changed, 165 insertions(+), 193 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 8b5610d..7744bbe 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -392,34 +392,35 @@ static bool tpacpi_uwb_emulstate;
* Debugging helpers
*/

-#define dbg_printk(a_dbg_level, format, arg...) \
+#define DEBUG
+
+#define tp_dbg(a_dbg_level, format, ...) \
do { \
if (dbg_level & (a_dbg_level)) \
- printk(KERN_DEBUG pr_fmt("%s: " format), \
- __func__, ##arg); \
+ pr_debug("%s: " format, __func__, ##__VA_ARGS__); \
} while (0)

#ifdef CONFIG_THINKPAD_ACPI_DEBUG
-#define vdbg_printk dbg_printk
+#define tp_vdbg tp_dbg
static const char *str_supported(int is_supported);
#else
static inline const char *str_supported(int is_supported) { return ""; }
-#define vdbg_printk(a_dbg_level, format, arg...) \
- no_printk(format, ##arg)
+#define tp_vdbg(a_dbg_level, format, ...) \
+ eliminated_printk(format, ##__VA_ARGS__)
#endif

static void tpacpi_log_usertask(const char * const what)
{
- printk(KERN_DEBUG pr_fmt("%s: access by process with PID %d\n"),
- what, task_tgid_vnr(current));
+ pr_debug("%s: access by process with PID %d\n",
+ what, task_tgid_vnr(current));
}

-#define tpacpi_disclose_usertask(what, format, arg...) \
+#define tpacpi_disclose_usertask(what, format, ...) \
do { \
if (unlikely((dbg_level & TPACPI_DBG_DISCLOSETASK) && \
(tpacpi_lifecycle == TPACPI_LIFE_RUNNING))) { \
- printk(KERN_DEBUG pr_fmt("%s: PID %d: " format), \
- what, task_tgid_vnr(current), ## arg); \
+ pr_debug("%s: PID %d: " format, \
+ what, task_tgid_vnr(current), ##__VA_ARGS__); \
} \
} while (0)

@@ -680,21 +681,20 @@ static void __init drv_acpi_handle_init(const char *name,
int i;
acpi_status status;

- vdbg_printk(TPACPI_DBG_INIT, "trying to locate ACPI handle for %s\n",
+ tp_vdbg(TPACPI_DBG_INIT, "trying to locate ACPI handle for %s\n",
name);

for (i = 0; i < num_paths; i++) {
status = acpi_get_handle(parent, paths[i], handle);
if (ACPI_SUCCESS(status)) {
- dbg_printk(TPACPI_DBG_INIT,
- "Found ACPI handle %s for %s\n",
- paths[i], name);
+ tp_dbg(TPACPI_DBG_INIT,
+ "Found ACPI handle %s for %s\n",
+ paths[i], name);
return;
}
}

- vdbg_printk(TPACPI_DBG_INIT, "ACPI handle for %s not found\n",
- name);
+ tp_vdbg(TPACPI_DBG_INIT, "ACPI handle for %s not found\n", name);
*handle = NULL;
}

@@ -714,9 +714,9 @@ static void __init tpacpi_acpi_handle_locate(const char *name,
acpi_handle device_found;

BUG_ON(!name || !hid || !handle);
- vdbg_printk(TPACPI_DBG_INIT,
- "trying to locate ACPI handle for %s, using HID %s\n",
- name, hid);
+ tp_vdbg(TPACPI_DBG_INIT,
+ "trying to locate ACPI handle for %s, using HID %s\n",
+ name, hid);

memset(&device_found, 0, sizeof(device_found));
status = acpi_get_devices(hid, tpacpi_acpi_handle_locate_callback,
@@ -726,12 +726,11 @@ static void __init tpacpi_acpi_handle_locate(const char *name,

if (ACPI_SUCCESS(status)) {
*handle = device_found;
- dbg_printk(TPACPI_DBG_INIT,
- "Found ACPI handle for %s\n", name);
+ tp_dbg(TPACPI_DBG_INIT, "Found ACPI handle for %s\n", name);
} else {
- vdbg_printk(TPACPI_DBG_INIT,
- "Could not locate an ACPI handle for %s: %s\n",
- name, acpi_format_exception(status));
+ tp_vdbg(TPACPI_DBG_INIT,
+ "Could not locate an ACPI handle for %s: %s\n",
+ name, acpi_format_exception(status));
}
}

@@ -758,8 +757,7 @@ static int __init setup_acpi_notify(struct ibm_struct *ibm)
if (!*ibm->acpi->handle)
return 0;

- vdbg_printk(TPACPI_DBG_INIT,
- "setting up ACPI notify for %s\n", ibm->name);
+ tp_vdbg(TPACPI_DBG_INIT, "setting up ACPI notify for %s\n", ibm->name);

rc = acpi_bus_get_device(*ibm->acpi->handle, &ibm->acpi->device);
if (rc < 0) {
@@ -797,8 +795,8 @@ static int __init register_tpacpi_subdriver(struct ibm_struct *ibm)
{
int rc;

- dbg_printk(TPACPI_DBG_INIT,
- "registering %s as an ACPI driver\n", ibm->name);
+ tp_dbg(TPACPI_DBG_INIT, "registering %s as an ACPI driver\n",
+ ibm->name);

BUG_ON(!ibm->acpi);

@@ -1215,9 +1213,8 @@ static int tpacpi_rfk_hook_set_block(void *data, bool blocked)
struct tpacpi_rfk *tp_rfk = data;
int res;

- dbg_printk(TPACPI_DBG_RFKILL,
- "request to change radio state to %s\n",
- blocked ? "blocked" : "unblocked");
+ tp_dbg(TPACPI_DBG_RFKILL, "request to change radio state to %s\n",
+ blocked ? "blocked" : "unblocked");

/* try to set radio state */
res = (tp_rfk->ops->set_status)(blocked ?
@@ -3016,8 +3013,8 @@ static void hotkey_exit(void)

kfree(hotkey_keycode_map);

- dbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_HKEY,
- "restoring original HKEY status and mask\n");
+ tp_dbg(TPACPI_DBG_EXIT | TPACPI_DBG_HKEY,
+ "restoring original HKEY status and mask\n");
/* yes, there is a bitwise or below, we want the
* functions to be called even if one of them fail */
if (((tp_features.hotkey_mask &&
@@ -3223,8 +3220,8 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
unsigned long quirks;
unsigned long keymap_id;

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
- "initializing hotkey subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
+ "initializing hotkey subdriver\n");

BUG_ON(!tpacpi_inputdev);
BUG_ON(tpacpi_inputdev->open != NULL ||
@@ -3241,9 +3238,8 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
/* hotkey not supported on 570 */
tp_features.hotkey = hkey_handle != NULL;

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
- "hotkeys are %s\n",
- str_supported(tp_features.hotkey));
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
+ "hotkeys are %s\n", str_supported(tp_features.hotkey));

if (!tp_features.hotkey)
return 1;
@@ -3279,7 +3275,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
* MHKV 0x100 in A31, R40, R40e,
* T4x, X31, and later
*/
- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
"firmware HKEY interface version: 0x%x\n",
hkeyv);

@@ -3297,7 +3293,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
}
}

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
"hotkey masks are %s\n",
str_supported(tp_features.hotkey_mask));

@@ -3368,8 +3364,8 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
keymap_id = tpacpi_check_quirks(tpacpi_keymap_qtable,
ARRAY_SIZE(tpacpi_keymap_qtable));
BUG_ON(keymap_id >= ARRAY_SIZE(tpacpi_keymaps));
- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
- "using keymap number %lu\n", keymap_id);
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
+ "using keymap number %lu\n", keymap_id);

memcpy(hotkey_keycode_map, &tpacpi_keymaps[keymap_id],
TPACPI_HOTKEY_MAP_SIZE);
@@ -3424,13 +3420,13 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
& ~hotkey_all_mask
& ~hotkey_reserved_mask;

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
- "hotkey source mask 0x%08x, polling freq %u\n",
- hotkey_source_mask, hotkey_poll_freq);
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
+ "hotkey source mask 0x%08x, polling freq %u\n",
+ hotkey_source_mask, hotkey_poll_freq);
#endif

- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
- "enabling firmware HKEY event interface...\n");
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
+ "enabling firmware HKEY event interface...\n");
res = hotkey_status_set(true);
if (res) {
hotkey_exit();
@@ -3445,14 +3441,13 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
}
hotkey_user_mask = (hotkey_acpi_mask | hotkey_source_mask)
& ~hotkey_reserved_mask;
- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
"initial masks: user=0x%08x, fw=0x%08x, poll=0x%08x\n",
hotkey_user_mask, hotkey_acpi_mask, hotkey_source_mask);

- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
- "legacy ibm/hotkey event reporting over procfs %s\n",
- (hotkey_report_mode < 2) ?
- "enabled" : "disabled");
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
+ "legacy ibm/hotkey event reporting over procfs %s\n",
+ (hotkey_report_mode < 2) ? "enabled" : "disabled");

tpacpi_inputdev->open = &hotkey_inputdev_open;
tpacpi_inputdev->close = &hotkey_inputdev_close;
@@ -3936,9 +3931,8 @@ static int bluetooth_set_status(enum tpacpi_rfkill_state state)
{
int status;

- vdbg_printk(TPACPI_DBG_RFKILL,
- "will attempt to %s bluetooth\n",
- (state == TPACPI_RFK_RADIO_ON) ? "enable" : "disable");
+ tp_vdbg(TPACPI_DBG_RFKILL, "will attempt to %s bluetooth\n",
+ state == TPACPI_RFK_RADIO_ON ? "enable" : "disable");

#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
if (dbg_bluetoothemul) {
@@ -4003,8 +3997,7 @@ static void bluetooth_shutdown(void)
TP_ACPI_BLTH_SAVE_STATE))
pr_notice("failed to save bluetooth state to NVRAM\n");
else
- vdbg_printk(TPACPI_DBG_RFKILL,
- "bluetooth state saved to NVRAM\n");
+ tp_vdbg(TPACPI_DBG_RFKILL, "bluetooth state saved to NVRAM\n");
}

static void bluetooth_exit(void)
@@ -4022,8 +4015,8 @@ static int __init bluetooth_init(struct ibm_init_struct *iibm)
int res;
int status = 0;

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
- "initializing bluetooth subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
+ "initializing bluetooth subdriver\n");

TPACPI_ACPIHANDLE_INIT(hkey);

@@ -4032,10 +4025,9 @@ static int __init bluetooth_init(struct ibm_init_struct *iibm)
tp_features.bluetooth = hkey_handle &&
acpi_evalf(hkey_handle, &status, "GBDC", "qd");

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
"bluetooth is %s, status 0x%02x\n",
- str_supported(tp_features.bluetooth),
- status);
+ str_supported(tp_features.bluetooth), status);

#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
if (dbg_bluetoothemul) {
@@ -4047,8 +4039,8 @@ static int __init bluetooth_init(struct ibm_init_struct *iibm)
!(status & TP_ACPI_BLUETOOTH_HWPRESENT)) {
/* no bluetooth hardware present in system */
tp_features.bluetooth = 0;
- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
- "bluetooth hardware not installed\n");
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
+ "bluetooth hardware not installed\n");
}

if (!tp_features.bluetooth)
@@ -4126,9 +4118,8 @@ static int wan_set_status(enum tpacpi_rfkill_state state)
{
int status;

- vdbg_printk(TPACPI_DBG_RFKILL,
- "will attempt to %s wwan\n",
- (state == TPACPI_RFK_RADIO_ON) ? "enable" : "disable");
+ tp_vdbg(TPACPI_DBG_RFKILL, "will attempt to %s wwan\n",
+ state == TPACPI_RFK_RADIO_ON ? "enable" : "disable");

#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
if (dbg_wwanemul) {
@@ -4193,8 +4184,7 @@ static void wan_shutdown(void)
TP_ACPI_WGSV_SAVE_STATE))
pr_notice("failed to save WWAN state to NVRAM\n");
else
- vdbg_printk(TPACPI_DBG_RFKILL,
- "WWAN state saved to NVRAM\n");
+ tp_vdbg(TPACPI_DBG_RFKILL, "WWAN state saved to NVRAM\n");
}

static void wan_exit(void)
@@ -4212,18 +4202,17 @@ static int __init wan_init(struct ibm_init_struct *iibm)
int res;
int status = 0;

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
- "initializing wan subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
+ "initializing wan subdriver\n");

TPACPI_ACPIHANDLE_INIT(hkey);

tp_features.wan = hkey_handle &&
acpi_evalf(hkey_handle, &status, "GWAN", "qd");

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
"wan is %s, status 0x%02x\n",
- str_supported(tp_features.wan),
- status);
+ str_supported(tp_features.wan), status);

#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
if (dbg_wwanemul) {
@@ -4235,8 +4224,8 @@ static int __init wan_init(struct ibm_init_struct *iibm)
!(status & TP_ACPI_WANCARD_HWPRESENT)) {
/* no wan hardware present in system */
tp_features.wan = 0;
- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
- "wan hardware not installed\n");
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
+ "wan hardware not installed\n");
}

if (!tp_features.wan)
@@ -4313,9 +4302,8 @@ static int uwb_set_status(enum tpacpi_rfkill_state state)
{
int status;

- vdbg_printk(TPACPI_DBG_RFKILL,
- "will attempt to %s UWB\n",
- (state == TPACPI_RFK_RADIO_ON) ? "enable" : "disable");
+ tp_vdbg(TPACPI_DBG_RFKILL, "will attempt to %s UWB\n",
+ state == TPACPI_RFK_RADIO_ON ? "enable" : "disable");

#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
if (dbg_uwbemul) {
@@ -4352,18 +4340,17 @@ static int __init uwb_init(struct ibm_init_struct *iibm)
int res;
int status = 0;

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
- "initializing uwb subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
+ "initializing uwb subdriver\n");

TPACPI_ACPIHANDLE_INIT(hkey);

tp_features.uwb = hkey_handle &&
acpi_evalf(hkey_handle, &status, "GUWB", "qd");

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
"uwb is %s, status 0x%02x\n",
- str_supported(tp_features.uwb),
- status);
+ str_supported(tp_features.uwb), status);

#ifdef CONFIG_THINKPAD_ACPI_DEBUGFACILITIES
if (dbg_uwbemul) {
@@ -4375,8 +4362,7 @@ static int __init uwb_init(struct ibm_init_struct *iibm)
!(status & TP_ACPI_UWB_HWPRESENT)) {
/* no uwb hardware present in system */
tp_features.uwb = 0;
- dbg_printk(TPACPI_DBG_INIT,
- "uwb hardware not installed\n");
+ tp_dbg(TPACPI_DBG_INIT, "uwb hardware not installed\n");
}

if (!tp_features.uwb)
@@ -4444,7 +4430,7 @@ static int __init video_init(struct ibm_init_struct *iibm)
{
int ivga;

- vdbg_printk(TPACPI_DBG_INIT, "initializing video subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT, "initializing video subdriver\n");

TPACPI_ACPIHANDLE_INIT(vid);
if (tpacpi_is_ibm())
@@ -4469,7 +4455,7 @@ static int __init video_init(struct ibm_init_struct *iibm)
/* all others */
video_supported = TPACPI_VIDEO_NEW;

- vdbg_printk(TPACPI_DBG_INIT, "video is %s, mode %d\n",
+ tp_vdbg(TPACPI_DBG_INIT, "video is %s, mode %d\n",
str_supported(video_supported != TPACPI_VIDEO_NONE),
video_supported);

@@ -4478,8 +4464,7 @@ static int __init video_init(struct ibm_init_struct *iibm)

static void video_exit(void)
{
- dbg_printk(TPACPI_DBG_EXIT,
- "restoring original video autoswitch mode\n");
+ tp_dbg(TPACPI_DBG_EXIT, "restoring original video autoswitch mode\n");
if (video_autosw_set(video_orig_autosw))
pr_err("error while trying to restore original "
"video autoswitch mode\n");
@@ -4835,7 +4820,7 @@ static int __init light_init(struct ibm_init_struct *iibm)
{
int rc;

- vdbg_printk(TPACPI_DBG_INIT, "initializing light subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT, "initializing light subdriver\n");

if (tpacpi_is_ibm()) {
TPACPI_ACPIHANDLE_INIT(ledb);
@@ -4853,7 +4838,7 @@ static int __init light_init(struct ibm_init_struct *iibm)
tp_features.light_status =
acpi_evalf(ec_handle, NULL, "KBLT", "qv");

- vdbg_printk(TPACPI_DBG_INIT, "light is %s, light status is %s\n",
+ tp_vdbg(TPACPI_DBG_INIT, "light is %s, light status is %s\n",
str_supported(tp_features.light),
str_supported(tp_features.light_status));

@@ -4955,12 +4940,11 @@ static int __init cmos_init(struct ibm_init_struct *iibm)
{
int res;

- vdbg_printk(TPACPI_DBG_INIT,
- "initializing cmos commands subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT, "initializing cmos commands subdriver\n");

TPACPI_ACPIHANDLE_INIT(cmos);

- vdbg_printk(TPACPI_DBG_INIT, "cmos commands are %s\n",
+ tp_vdbg(TPACPI_DBG_INIT, "cmos commands are %s\n",
str_supported(cmos_handle != NULL));

res = device_create_file(&tpacpi_pdev->dev, &dev_attr_cmos_command);
@@ -5327,11 +5311,11 @@ static int __init led_init(struct ibm_init_struct *iibm)
int rc;
unsigned long useful_leds;

- vdbg_printk(TPACPI_DBG_INIT, "initializing LED subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT, "initializing LED subdriver\n");

led_supported = led_init_detect_mode();

- vdbg_printk(TPACPI_DBG_INIT, "LED commands are %s, mode %d\n",
+ tp_vdbg(TPACPI_DBG_INIT, "LED commands are %s, mode %d\n",
str_supported(led_supported), led_supported);

if (led_supported == TPACPI_LED_NONE)
@@ -5450,11 +5434,11 @@ static int __init beep_init(struct ibm_init_struct *iibm)
{
unsigned long quirks;

- vdbg_printk(TPACPI_DBG_INIT, "initializing beep subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT, "initializing beep subdriver\n");

TPACPI_ACPIHANDLE_INIT(beep);

- vdbg_printk(TPACPI_DBG_INIT, "beep is %s\n",
+ tp_vdbg(TPACPI_DBG_INIT, "beep is %s\n",
str_supported(beep_handle != NULL));

quirks = tpacpi_check_quirks(beep_quirk_table,
@@ -5729,7 +5713,7 @@ static int __init thermal_init(struct ibm_init_struct *iibm)
int acpi_tmp7;
int res;

- vdbg_printk(TPACPI_DBG_INIT, "initializing thermal subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT, "initializing thermal subdriver\n");

acpi_tmp7 = acpi_evalf(ec_handle, NULL, "TMP7", "qv");

@@ -5787,7 +5771,7 @@ static int __init thermal_init(struct ibm_init_struct *iibm)
thermal_read_mode = TPACPI_THERMAL_NONE;
}

- vdbg_printk(TPACPI_DBG_INIT, "thermal is %s, mode %d\n",
+ tp_vdbg(TPACPI_DBG_INIT, "thermal is %s, mode %d\n",
str_supported(thermal_read_mode != TPACPI_THERMAL_NONE),
thermal_read_mode);

@@ -5932,7 +5916,7 @@ static void tpacpi_brightness_checkpoint_nvram(void)
if (brightness_mode != TPACPI_BRGHT_MODE_ECNVRAM)
return;

- vdbg_printk(TPACPI_DBG_BRGHT,
+ tp_vdbg(TPACPI_DBG_BRGHT,
"trying to checkpoint backlight level to NVRAM...\n");

if (mutex_lock_killable(&brightness_mutex) < 0)
@@ -5950,13 +5934,13 @@ static void tpacpi_brightness_checkpoint_nvram(void)
TP_NVRAM_POS_LEVEL_BRIGHTNESS);
b_nvram |= lec;
nvram_write_byte(b_nvram, TP_NVRAM_ADDR_BRIGHTNESS);
- dbg_printk(TPACPI_DBG_BRGHT,
- "updated NVRAM backlight level to %u (0x%02x)\n",
- (unsigned int) lec, (unsigned int) b_nvram);
+ tp_dbg(TPACPI_DBG_BRGHT,
+ "updated NVRAM backlight level to %u (0x%02x)\n",
+ (unsigned int)lec, (unsigned int)b_nvram);
} else
- vdbg_printk(TPACPI_DBG_BRGHT,
- "NVRAM backlight level already is %u (0x%02x)\n",
- (unsigned int) lec, (unsigned int) b_nvram);
+ tp_vdbg(TPACPI_DBG_BRGHT,
+ "NVRAM backlight level already is %u (0x%02x)\n",
+ (unsigned int)lec, (unsigned int)b_nvram);

unlock:
mutex_unlock(&brightness_mutex);
@@ -6031,8 +6015,7 @@ static int brightness_set(unsigned int value)
if (value > bright_maxlvl || value < 0)
return -EINVAL;

- vdbg_printk(TPACPI_DBG_BRGHT,
- "set backlight level to %d\n", value);
+ tp_vdbg(TPACPI_DBG_BRGHT, "set backlight level to %d\n", value);

res = mutex_lock_killable(&brightness_mutex);
if (res < 0)
@@ -6063,9 +6046,8 @@ static int brightness_update_status(struct backlight_device *bd)
bd->props.power == FB_BLANK_UNBLANK) ?
bd->props.brightness : 0;

- dbg_printk(TPACPI_DBG_BRGHT,
- "backlight: attempt to set level to %d\n",
- level);
+ tp_dbg(TPACPI_DBG_BRGHT, "backlight: attempt to set level to %d\n",
+ level);

/* it is the backlight class's job (caller) to handle
* EINTR and other errors properly */
@@ -6187,8 +6169,8 @@ static void __init tpacpi_detect_brightness_capabilities(void)
{
unsigned int b;

- vdbg_printk(TPACPI_DBG_INIT,
- "detecting firmware brightness interface capabilities\n");
+ tp_vdbg(TPACPI_DBG_INIT,
+ "detecting firmware brightness interface capabilities\n");

/* we could run a quirks check here (same table used by
* brightness_init) if needed */
@@ -6223,7 +6205,7 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
int b;
unsigned long quirks;

- vdbg_printk(TPACPI_DBG_INIT, "initializing brightness subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT, "initializing brightness subdriver\n");

mutex_init(&brightness_mutex);

@@ -6237,9 +6219,8 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
return 1;

if (!brightness_enable) {
- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
- "brightness support disabled by "
- "module parameter\n");
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
+ "brightness support disabled by module parameter\n");
return 1;
}

@@ -6276,9 +6257,9 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
else
brightness_mode = TPACPI_BRGHT_MODE_UCMS_STEP;

- dbg_printk(TPACPI_DBG_BRGHT,
- "driver auto-selected brightness_mode=%d\n",
- brightness_mode);
+ tp_dbg(TPACPI_DBG_BRGHT,
+ "driver auto-selected brightness_mode=%d\n",
+ brightness_mode);
}

/* Safety */
@@ -6304,8 +6285,8 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
pr_err("Could not register backlight device\n");
return rc;
}
- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
- "brightness is supported\n");
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
+ "brightness is supported\n");

if (quirks & TPACPI_BRGHT_Q_ASK) {
pr_notice("brightness: will use unverified default: "
@@ -6320,9 +6301,8 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
* it in place just in case */
backlight_update_status(ibm_backlight_device);

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
- "brightness: registering brightness hotkeys "
- "as change notification\n");
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
+ "brightness: registering brightness hotkeys as change notification\n");
tpacpi_hotkey_driver_mask_set(hotkey_driver_mask
| TP_ACPI_HKEY_BRGHTUP_MASK
| TP_ACPI_HKEY_BRGHTDWN_MASK);
@@ -6342,8 +6322,8 @@ static void brightness_shutdown(void)
static void brightness_exit(void)
{
if (ibm_backlight_device) {
- vdbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_BRGHT,
- "calling backlight_device_unregister()\n");
+ tp_vdbg(TPACPI_DBG_EXIT | TPACPI_DBG_BRGHT,
+ "calling backlight_device_unregister()\n");
backlight_device_unregister(ibm_backlight_device);
}

@@ -6506,7 +6486,7 @@ static void tpacpi_volume_checkpoint_nvram(void)
if (!volume_control_allowed)
return;

- vdbg_printk(TPACPI_DBG_MIXER,
+ tp_vdbg(TPACPI_DBG_MIXER,
"trying to checkpoint mixer state to NVRAM...\n");

if (tp_features.mixer_no_level_control)
@@ -6527,13 +6507,13 @@ static void tpacpi_volume_checkpoint_nvram(void)
b_nvram &= ~ec_mask;
b_nvram |= lec;
nvram_write_byte(b_nvram, TP_NVRAM_ADDR_MIXER);
- dbg_printk(TPACPI_DBG_MIXER,
- "updated NVRAM mixer status to 0x%02x (0x%02x)\n",
- (unsigned int) lec, (unsigned int) b_nvram);
+ tp_dbg(TPACPI_DBG_MIXER,
+ "updated NVRAM mixer status to 0x%02x (0x%02x)\n",
+ (unsigned int)lec, (unsigned int)b_nvram);
} else {
- vdbg_printk(TPACPI_DBG_MIXER,
- "NVRAM mixer status already is 0x%02x (0x%02x)\n",
- (unsigned int) lec, (unsigned int) b_nvram);
+ tp_vdbg(TPACPI_DBG_MIXER,
+ "NVRAM mixer status already is 0x%02x (0x%02x)\n",
+ (unsigned int)lec, (unsigned int)b_nvram);
}

unlock:
@@ -6549,7 +6529,7 @@ static int volume_get_status_ec(u8 *status)

*status = s;

- dbg_printk(TPACPI_DBG_MIXER, "status 0x%02x\n", s);
+ tp_dbg(TPACPI_DBG_MIXER, "status 0x%02x\n", s);

return 0;
}
@@ -6564,7 +6544,7 @@ static int volume_set_status_ec(const u8 status)
if (!acpi_ec_write(TP_EC_AUDIO, status))
return -EIO;

- dbg_printk(TPACPI_DBG_MIXER, "set EC mixer to 0x%02x\n", status);
+ tp_dbg(TPACPI_DBG_MIXER, "set EC mixer to 0x%02x\n", status);

return 0;
}
@@ -6603,8 +6583,7 @@ unlock:

static int volume_alsa_set_mute(const bool mute)
{
- dbg_printk(TPACPI_DBG_MIXER, "ALSA: trying to %smute\n",
- (mute) ? "" : "un");
+ tp_dbg(TPACPI_DBG_MIXER, "ALSA: trying to %smute\n", mute ? "" : "un");
return __volume_set_mute_ec(mute);
}

@@ -6612,8 +6591,7 @@ static int volume_set_mute(const bool mute)
{
int rc;

- dbg_printk(TPACPI_DBG_MIXER, "trying to %smute\n",
- (mute) ? "" : "un");
+ tp_dbg(TPACPI_DBG_MIXER, "trying to %smute\n", mute ? "" : "un");

rc = __volume_set_mute_ec(mute);
return (rc < 0) ? rc : 0;
@@ -6650,8 +6628,8 @@ unlock:

static int volume_alsa_set_volume(const u8 vol)
{
- dbg_printk(TPACPI_DBG_MIXER,
- "ALSA: trying to set volume level to %hu\n", vol);
+ tp_dbg(TPACPI_DBG_MIXER,
+ "ALSA: trying to set volume level to %hu\n", vol);
return __volume_set_volume_ec(vol);
}

@@ -6879,7 +6857,7 @@ static int __init volume_init(struct ibm_init_struct *iibm)
unsigned long quirks;
int rc;

- vdbg_printk(TPACPI_DBG_INIT, "initializing volume subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT, "initializing volume subdriver\n");

mutex_init(&volume_mutex);

@@ -6905,9 +6883,8 @@ static int __init volume_init(struct ibm_init_struct *iibm)
* When disabled, don't install the subdriver at all
*/
if (!alsa_enable) {
- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
- "ALSA mixer disabled by parameter, "
- "not loading volume subdriver...\n");
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
+ "ALSA mixer disabled by parameter, not loading volume subdriver...\n");
return 1;
}

@@ -6934,26 +6911,26 @@ static int __init volume_init(struct ibm_init_struct *iibm)
}

if (volume_capabilities != TPACPI_VOL_CAP_AUTO)
- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
- "using user-supplied volume_capabilities=%d\n",
- volume_capabilities);
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
+ "using user-supplied volume_capabilities=%d\n",
+ volume_capabilities);

if (volume_mode == TPACPI_VOL_MODE_AUTO ||
volume_mode == TPACPI_VOL_MODE_MAX) {
volume_mode = TPACPI_VOL_MODE_ECNVRAM;

- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
- "driver auto-selected volume_mode=%d\n",
- volume_mode);
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
+ "driver auto-selected volume_mode=%d\n",
+ volume_mode);
} else {
- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
- "using user-supplied volume_mode=%d\n",
- volume_mode);
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
+ "using user-supplied volume_mode=%d\n",
+ volume_mode);
}

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
- "mute is supported, volume control is %s\n",
- str_supported(!tp_features.mixer_no_level_control));
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
+ "mute is supported, volume control is %s\n",
+ str_supported(!tp_features.mixer_no_level_control));

rc = volume_create_alsa_mixer();
if (rc) {
@@ -6966,7 +6943,7 @@ static int __init volume_init(struct ibm_init_struct *iibm)
"override (read/write)" :
"monitor (read only)");

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_MIXER,
"registering volume hotkeys as change notification\n");
tpacpi_hotkey_driver_mask_set(hotkey_driver_mask
| TP_ACPI_HKEY_VOLUP_MASK
@@ -7523,7 +7500,7 @@ static int fan_set_level(int level)
return -ENXIO;
}

- vdbg_printk(TPACPI_DBG_FAN,
+ tp_vdbg(TPACPI_DBG_FAN,
"fan control: set fan control register to 0x%02x\n", level);
return 0;
}
@@ -7604,9 +7581,8 @@ static int fan_set_enable(void)
mutex_unlock(&fan_mutex);

if (!rc)
- vdbg_printk(TPACPI_DBG_FAN,
- "fan control: set fan control register to 0x%02x\n",
- s);
+ tp_vdbg(TPACPI_DBG_FAN,
+ "fan control: set fan control register to 0x%02x\n", s);
return rc;
}

@@ -7644,7 +7620,7 @@ static int fan_set_disable(void)
}

if (!rc)
- vdbg_printk(TPACPI_DBG_FAN,
+ tp_vdbg(TPACPI_DBG_FAN,
"fan control: set fan control register to 0\n");

mutex_unlock(&fan_mutex);
@@ -7980,8 +7956,8 @@ static int __init fan_init(struct ibm_init_struct *iibm)
int rc;
unsigned long quirks;

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
- "initializing fan subdriver\n");
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
+ "initializing fan subdriver\n");

mutex_init(&fan_mutex);
fan_status_access_mode = TPACPI_FAN_NONE;
@@ -8014,8 +7990,8 @@ static int __init fan_init(struct ibm_init_struct *iibm)
fan_quirk1_setup();
if (quirks & TPACPI_FAN_2FAN) {
tp_features.second_fan = 1;
- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
- "secondary fan support enabled\n");
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
+ "secondary fan support enabled\n");
}
} else {
pr_err("ThinkPad ACPI EC access misbehaving, "
@@ -8051,18 +8027,18 @@ static int __init fan_init(struct ibm_init_struct *iibm)
}
}

- vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
+ tp_vdbg(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
"fan is %s, modes %d, %d\n",
str_supported(fan_status_access_mode != TPACPI_FAN_NONE ||
- fan_control_access_mode != TPACPI_FAN_WR_NONE),
+ fan_control_access_mode != TPACPI_FAN_WR_NONE),
fan_status_access_mode, fan_control_access_mode);

/* fan control master switch */
if (!fan_control_allowed) {
fan_control_access_mode = TPACPI_FAN_WR_NONE;
fan_control_commands = 0;
- dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
- "fan control features disabled by parameter\n");
+ tp_dbg(TPACPI_DBG_INIT | TPACPI_DBG_FAN,
+ "fan control features disabled by parameter\n");
}

/* update fan_control_desired_level */
@@ -8095,8 +8071,8 @@ static int __init fan_init(struct ibm_init_struct *iibm)

static void fan_exit(void)
{
- vdbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_FAN,
- "cancelling any pending fan watchdog tasks\n");
+ tp_vdbg(TPACPI_DBG_EXIT | TPACPI_DBG_FAN,
+ "cancelling any pending fan watchdog tasks\n");

/* FIXME: can we really do this unconditionally? */
sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj, &fan_attr_group);
@@ -8450,13 +8426,13 @@ static const char * __init str_supported(int is_supported)

static void ibm_exit(struct ibm_struct *ibm)
{
- dbg_printk(TPACPI_DBG_EXIT, "removing %s\n", ibm->name);
+ tp_dbg(TPACPI_DBG_EXIT, "removing %s\n", ibm->name);

list_del_init(&ibm->all_drivers);

if (ibm->flags.acpi_notify_installed) {
- dbg_printk(TPACPI_DBG_EXIT,
- "%s: acpi_remove_notify_handler\n", ibm->name);
+ tp_dbg(TPACPI_DBG_EXIT,
+ "%s: acpi_remove_notify_handler\n", ibm->name);
BUG_ON(!ibm->acpi);
acpi_remove_notify_handler(*ibm->acpi->handle,
ibm->acpi->type,
@@ -8465,15 +8441,14 @@ static void ibm_exit(struct ibm_struct *ibm)
}

if (ibm->flags.proc_created) {
- dbg_printk(TPACPI_DBG_EXIT,
- "%s: remove_proc_entry\n", ibm->name);
+ tp_dbg(TPACPI_DBG_EXIT, "%s: remove_proc_entry\n", ibm->name);
remove_proc_entry(ibm->name, proc_dir);
ibm->flags.proc_created = 0;
}

if (ibm->flags.acpi_driver_registered) {
- dbg_printk(TPACPI_DBG_EXIT,
- "%s: acpi_bus_unregister_driver\n", ibm->name);
+ tp_dbg(TPACPI_DBG_EXIT,
+ "%s: acpi_bus_unregister_driver\n", ibm->name);
BUG_ON(!ibm->acpi);
acpi_bus_unregister_driver(ibm->acpi->driver);
kfree(ibm->acpi->driver);
@@ -8486,7 +8461,7 @@ static void ibm_exit(struct ibm_struct *ibm)
ibm->flags.init_called = 0;
}

- dbg_printk(TPACPI_DBG_INIT, "finished removing %s\n", ibm->name);
+ tp_dbg(TPACPI_DBG_INIT, "finished removing %s\n", ibm->name);
}

static int __init ibm_init(struct ibm_init_struct *iibm)
@@ -8502,8 +8477,7 @@ static int __init ibm_init(struct ibm_init_struct *iibm)
if (ibm->flags.experimental && !experimental)
return 0;

- dbg_printk(TPACPI_DBG_INIT,
- "probing for %s\n", ibm->name);
+ tp_dbg(TPACPI_DBG_INIT, "probing for %s\n", ibm->name);

if (iibm->init) {
ret = iibm->init(iibm);
@@ -8535,8 +8509,7 @@ static int __init ibm_init(struct ibm_init_struct *iibm)
}
}

- dbg_printk(TPACPI_DBG_INIT,
- "%s installed\n", ibm->name);
+ tp_dbg(TPACPI_DBG_INIT, "%s installed\n", ibm->name);

if (ibm->read) {
umode_t mode = iibm->base_procfs_mode;
@@ -8560,9 +8533,8 @@ static int __init ibm_init(struct ibm_init_struct *iibm)
return 0;

err_out:
- dbg_printk(TPACPI_DBG_INIT,
- "%s: at error exit path with result %d\n",
- ibm->name, ret);
+ tp_dbg(TPACPI_DBG_INIT, "%s: at error exit path with result %d\n",
+ ibm->name, ret);

ibm_exit(ibm);
return (ret < 0)? ret : 0;
@@ -8928,7 +8900,7 @@ static void thinkpad_acpi_module_exit(void)
ibm_exit(ibm);
}

- dbg_printk(TPACPI_DBG_INIT, "finished subdriver exit path...\n");
+ tp_dbg(TPACPI_DBG_INIT, "finished subdriver exit path...\n");

if (tpacpi_inputdev) {
if (tp_features.input_device_registered)
--
1.7.8.111.gad25c.dirty

2012-06-18 02:26:20

by Joe Perches

[permalink] [raw]
Subject: [PATCH 3/5] usb: Convert dbg to pr_eliminated and pr_debug

Change the dbg macro to use pr_eliminated when not
DEBUG and pr_debug with DEBUG so dynamic_debug can
be used.

Signed-off-by: Joe Perches <[email protected]>
---
include/linux/usb.h | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/linux/usb.h b/include/linux/usb.h
index f717fbd..56b88b8 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1714,14 +1714,12 @@ extern void usb_register_notify(struct notifier_block *nb);
extern void usb_unregister_notify(struct notifier_block *nb);

#ifdef DEBUG
-#define dbg(format, arg...) \
- printk(KERN_DEBUG "%s: " format "\n", __FILE__, ##arg)
+#define dbg(format, ...) \
+ pr_debug("%s: " format "\n", __FILE__, ##__VA_ARGS__)
#else
-#define dbg(format, arg...) \
-do { \
- if (0) \
- printk(KERN_DEBUG "%s: " format "\n", __FILE__, ##arg); \
-} while (0)
+#define dbg(format, ...) \
+ printk_eliminated(KERN_DEBUG "%s: " format "\n", \
+ __FILE__, ##__VA_ARGS)
#endif

/* debugfs stuff */
--
1.7.8.111.gad25c.dirty

2012-06-18 02:26:22

by Joe Perches

[permalink] [raw]
Subject: [PATCH 4/5] fujitsu-laptop: Convert dbg_printk and vdbg_printk to fuj_dbg

Use more current logging styles.

Add #define DEBUG to enable dynamic debug.
Remove unused #defines.
Use eliminated_printk to verify fuj_dbg arguments.
Argument alignment and format coalescing.

Signed-off-by: Joe Perches <[email protected]>
---
drivers/platform/x86/fujitsu-laptop.c | 103 +++++++++++++++------------------
1 files changed, 47 insertions(+), 56 deletions(-)

diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
index c4c1a54..e2aae57 100644
--- a/drivers/platform/x86/fujitsu-laptop.c
+++ b/drivers/platform/x86/fujitsu-laptop.c
@@ -118,27 +118,22 @@
#define RINGBUFFERSIZE 40

/* Debugging */
-#define FUJLAPTOP_LOG ACPI_FUJITSU_HID ": "
-#define FUJLAPTOP_ERR KERN_ERR FUJLAPTOP_LOG
-#define FUJLAPTOP_NOTICE KERN_NOTICE FUJLAPTOP_LOG
-#define FUJLAPTOP_INFO KERN_INFO FUJLAPTOP_LOG
-#define FUJLAPTOP_DEBUG KERN_DEBUG FUJLAPTOP_LOG
-
#define FUJLAPTOP_DBG_ALL 0xffff
#define FUJLAPTOP_DBG_ERROR 0x0001
#define FUJLAPTOP_DBG_WARN 0x0002
#define FUJLAPTOP_DBG_INFO 0x0004
#define FUJLAPTOP_DBG_TRACE 0x0008

-#define dbg_printk(a_dbg_level, format, arg...) \
- do { if (dbg_level & a_dbg_level) \
- printk(FUJLAPTOP_DEBUG "%s: " format, __func__ , ## arg); \
- } while (0)
#ifdef CONFIG_FUJITSU_LAPTOP_DEBUG
-#define vdbg_printk(a_dbg_level, format, arg...) \
- dbg_printk(a_dbg_level, format, ## arg)
+#define DEBUG
+#define fuj_dbg(a_dbg_level, format, ...) \
+do { \
+ if (dbg_level & a_dbg_level) \
+ pr_debug("%s: " format, __func__ , ##__VA_ARGS__); \
+} while (0)
#else
-#define vdbg_printk(a_dbg_level, format, arg...)
+#define fuj_dbg(a_dbg_level, format, ...) \
+ eliminated_printk(format, ##__VA_ARGS__)
#endif

/* Device controlling the backlight and associated keys */
@@ -225,8 +220,7 @@ static int call_fext_func(int cmd, int arg0, int arg1, int arg2)

status = acpi_get_handle(fujitsu_hotkey->acpi_handle, "FUNC", &handle);
if (ACPI_FAILURE(status)) {
- vdbg_printk(FUJLAPTOP_DBG_ERROR,
- "FUNC interface is not present\n");
+ fuj_dbg(FUJLAPTOP_DBG_ERROR, "FUNC interface is not present\n");
return -ENODEV;
}

@@ -240,23 +234,22 @@ static int call_fext_func(int cmd, int arg0, int arg1, int arg2)

status = acpi_evaluate_object(handle, NULL, &arg_list, &output);
if (ACPI_FAILURE(status)) {
- vdbg_printk(FUJLAPTOP_DBG_WARN,
+ fuj_dbg(FUJLAPTOP_DBG_WARN,
"FUNC 0x%x (args 0x%x, 0x%x, 0x%x) call failed\n",
- cmd, arg0, arg1, arg2);
+ cmd, arg0, arg1, arg2);
return -ENODEV;
}

if (out_obj.type != ACPI_TYPE_INTEGER) {
- vdbg_printk(FUJLAPTOP_DBG_WARN,
- "FUNC 0x%x (args 0x%x, 0x%x, 0x%x) did not "
- "return an integer\n",
+ fuj_dbg(FUJLAPTOP_DBG_WARN,
+ "FUNC 0x%x (args 0x%x, 0x%x, 0x%x) did not return an integer\n",
cmd, arg0, arg1, arg2);
return -ENODEV;
}

- vdbg_printk(FUJLAPTOP_DBG_TRACE,
+ fuj_dbg(FUJLAPTOP_DBG_TRACE,
"FUNC 0x%x (args 0x%x, 0x%x, 0x%x) returned 0x%x\n",
- cmd, arg0, arg1, arg2, (int)out_obj.integer.value);
+ cmd, arg0, arg1, arg2, (int)out_obj.integer.value);
return out_obj.integer.value;
}

@@ -321,15 +314,14 @@ static int set_lcd_level(int level)
struct acpi_object_list arg_list = { 1, &arg0 };
acpi_handle handle = NULL;

- vdbg_printk(FUJLAPTOP_DBG_TRACE, "set lcd level via SBLL [%d]\n",
- level);
+ fuj_dbg(FUJLAPTOP_DBG_TRACE, "set lcd level via SBLL [%d]\n", level);

if (level < 0 || level >= fujitsu->max_brightness)
return -EINVAL;

status = acpi_get_handle(fujitsu->acpi_handle, "SBLL", &handle);
if (ACPI_FAILURE(status)) {
- vdbg_printk(FUJLAPTOP_DBG_ERROR, "SBLL not present\n");
+ fuj_dbg(FUJLAPTOP_DBG_ERROR, "SBLL not present\n");
return -ENODEV;
}

@@ -349,15 +341,14 @@ static int set_lcd_level_alt(int level)
struct acpi_object_list arg_list = { 1, &arg0 };
acpi_handle handle = NULL;

- vdbg_printk(FUJLAPTOP_DBG_TRACE, "set lcd level via SBL2 [%d]\n",
- level);
+ fuj_dbg(FUJLAPTOP_DBG_TRACE, "set lcd level via SBL2 [%d]\n", level);

if (level < 0 || level >= fujitsu->max_brightness)
return -EINVAL;

status = acpi_get_handle(fujitsu->acpi_handle, "SBL2", &handle);
if (ACPI_FAILURE(status)) {
- vdbg_printk(FUJLAPTOP_DBG_ERROR, "SBL2 not present\n");
+ fuj_dbg(FUJLAPTOP_DBG_ERROR, "SBL2 not present\n");
return -ENODEV;
}

@@ -375,7 +366,7 @@ static int get_lcd_level(void)
unsigned long long state = 0;
acpi_status status = AE_OK;

- vdbg_printk(FUJLAPTOP_DBG_TRACE, "get lcd level via GBLL\n");
+ fuj_dbg(FUJLAPTOP_DBG_TRACE, "get lcd level via GBLL\n");

status =
acpi_evaluate_integer(fujitsu->acpi_handle, "GBLL", NULL, &state);
@@ -397,7 +388,7 @@ static int get_max_brightness(void)
unsigned long long state = 0;
acpi_status status = AE_OK;

- vdbg_printk(FUJLAPTOP_DBG_TRACE, "get max lcd level via RBLL\n");
+ fuj_dbg(FUJLAPTOP_DBG_TRACE, "get max lcd level via RBLL\n");

status =
acpi_evaluate_integer(fujitsu->acpi_handle, "RBLL", NULL, &state);
@@ -424,7 +415,7 @@ static int bl_update_status(struct backlight_device *b)
else
ret = call_fext_func(FUNC_BACKLIGHT, 0x1, 0x4, 0x0);
if (ret != 0)
- vdbg_printk(FUJLAPTOP_DBG_ERROR,
+ fuj_dbg(FUJLAPTOP_DBG_ERROR,
"Unable to adjust backlight power, error code %i\n",
ret);

@@ -433,7 +424,7 @@ static int bl_update_status(struct backlight_device *b)
else
ret = set_lcd_level(b->props.brightness);
if (ret != 0)
- vdbg_printk(FUJLAPTOP_DBG_ERROR,
+ fuj_dbg(FUJLAPTOP_DBG_ERROR,
"Unable to adjust LCD brightness, error code %i\n",
ret);
return ret;
@@ -594,8 +585,8 @@ static void dmi_check_cb_common(const struct dmi_system_id *id)
use_alt_lcd_levels = 1;
else
use_alt_lcd_levels = 0;
- vdbg_printk(FUJLAPTOP_DBG_TRACE, "auto-detected usealt as "
- "%i\n", use_alt_lcd_levels);
+ fuj_dbg(FUJLAPTOP_DBG_TRACE, "auto-detected usealt as %i\n",
+ use_alt_lcd_levels);
}
}

@@ -704,7 +695,7 @@ static int acpi_fujitsu_add(struct acpi_device *device)

if (ACPI_SUCCESS
(acpi_get_handle(device->handle, METHOD_NAME__INI, &handle))) {
- vdbg_printk(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
+ fuj_dbg(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
if (ACPI_FAILURE
(acpi_evaluate_object
(device->handle, METHOD_NAME__INI, NULL, NULL)))
@@ -714,9 +705,9 @@ static int acpi_fujitsu_add(struct acpi_device *device)
/* do config (detect defaults) */
use_alt_lcd_levels = use_alt_lcd_levels == 1 ? 1 : 0;
disable_brightness_adjust = disable_brightness_adjust == 1 ? 1 : 0;
- vdbg_printk(FUJLAPTOP_DBG_INFO,
- "config: [alt interface: %d], [adjust disable: %d]\n",
- use_alt_lcd_levels, disable_brightness_adjust);
+ fuj_dbg(FUJLAPTOP_DBG_INFO,
+ "config: [alt interface: %d], [adjust disable: %d]\n",
+ use_alt_lcd_levels, disable_brightness_adjust);

if (get_max_brightness() <= 0)
fujitsu->max_brightness = FUJITSU_LCD_N_LEVELS;
@@ -762,9 +753,9 @@ static void acpi_fujitsu_notify(struct acpi_device *device, u32 event)
get_lcd_level();
newb = fujitsu->brightness_level;

- vdbg_printk(FUJLAPTOP_DBG_TRACE,
- "brightness button event [%i -> %i (%i)]\n",
- oldb, newb, fujitsu->brightness_changed);
+ fuj_dbg(FUJLAPTOP_DBG_TRACE,
+ "brightness button event [%i -> %i (%i)]\n",
+ oldb, newb, fujitsu->brightness_changed);

if (oldb < newb) {
if (disable_brightness_adjust != 1) {
@@ -790,8 +781,8 @@ static void acpi_fujitsu_notify(struct acpi_device *device, u32 event)
break;
default:
keycode = KEY_UNKNOWN;
- vdbg_printk(FUJLAPTOP_DBG_WARN,
- "unsupported event [0x%x]\n", event);
+ fuj_dbg(FUJLAPTOP_DBG_WARN, "unsupported event [0x%x]\n",
+ event);
break;
}

@@ -872,7 +863,7 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)

if (ACPI_SUCCESS
(acpi_get_handle(device->handle, METHOD_NAME__INI, &handle))) {
- vdbg_printk(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
+ fuj_dbg(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
if (ACPI_FAILURE
(acpi_evaluate_object
(device->handle, METHOD_NAME__INI, NULL, NULL)))
@@ -883,7 +874,7 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
while (call_fext_func(FUNC_BUTTONS, 0x1, 0x0, 0x0) != 0
&& (i++) < MAX_HOTKEY_RINGBUFFER_SIZE)
; /* No action, result is discarded */
- vdbg_printk(FUJLAPTOP_DBG_INFO, "Discarded %i ringbuffer entries\n", i);
+ fuj_dbg(FUJLAPTOP_DBG_INFO, "Discarded %i ringbuffer entries\n", i);

fujitsu_hotkey->rfkill_supported =
call_fext_func(FUNC_RFKILL, 0x0, 0x0, 0x0);
@@ -996,13 +987,13 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
keycode = 0;
break;
default:
- vdbg_printk(FUJLAPTOP_DBG_WARN,
- "Unknown GIRB result [%x]\n", irb);
+ fuj_dbg(FUJLAPTOP_DBG_WARN,
+ "Unknown GIRB result [%x]\n", irb);
keycode = -1;
break;
}
if (keycode > 0) {
- vdbg_printk(FUJLAPTOP_DBG_TRACE,
+ fuj_dbg(FUJLAPTOP_DBG_TRACE,
"Push keycode into ringbuffer [%d]\n",
keycode);
status = kfifo_in_locked(&fujitsu_hotkey->fifo,
@@ -1010,9 +1001,9 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
sizeof(keycode),
&fujitsu_hotkey->fifo_lock);
if (status != sizeof(keycode)) {
- vdbg_printk(FUJLAPTOP_DBG_WARN,
- "Could not push keycode [0x%x]\n",
- keycode);
+ fuj_dbg(FUJLAPTOP_DBG_WARN,
+ "Could not push keycode [0x%x]\n",
+ keycode);
} else {
input_report_key(input, keycode, 1);
input_sync(input);
@@ -1027,9 +1018,9 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
== sizeof(keycode_r)) {
input_report_key(input, keycode_r, 0);
input_sync(input);
- vdbg_printk(FUJLAPTOP_DBG_TRACE,
- "Pop keycode from ringbuffer [%d]\n",
- keycode_r);
+ fuj_dbg(FUJLAPTOP_DBG_TRACE,
+ "Pop keycode from ringbuffer [%d]\n",
+ keycode_r);
}
}
}
@@ -1037,8 +1028,8 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
break;
default:
keycode = KEY_UNKNOWN;
- vdbg_printk(FUJLAPTOP_DBG_WARN,
- "Unsupported event [0x%x]\n", event);
+ fuj_dbg(FUJLAPTOP_DBG_WARN, "Unsupported event [0x%x]\n",
+ event);
input_report_key(input, keycode, 1);
input_sync(input);
input_report_key(input, keycode, 0);
--
1.7.8.111.gad25c.dirty

2012-06-18 02:26:17

by Joe Perches

[permalink] [raw]
Subject: [PATCH 2/5] printk: Convert pr_devel to use eliminated_printk

Make sure there are no side effects to pr_devel uses.

Signed-off-by: Joe Perches <[email protected]>
---
include/linux/printk.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 84aa910..628054f 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -207,7 +207,7 @@ extern void dump_stack(void) __cold;
printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#else
#define pr_devel(fmt, ...) \
- no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
+ eliminated_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
#endif

/* If you are writing a driver, please use dev_dbg instead */
--
1.7.8.111.gad25c.dirty

2012-06-18 06:08:31

by Paul Mundt

[permalink] [raw]
Subject: Re: [PATCH 3/5] usb: Convert dbg to pr_eliminated and pr_debug

On Sun, Jun 17, 2012 at 07:25:10PM -0700, Joe Perches wrote:
> Change the dbg macro to use pr_eliminated when not
> DEBUG and pr_debug with DEBUG so dynamic_debug can
> be used.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> include/linux/usb.h | 12 +++++-------
> 1 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index f717fbd..56b88b8 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -1714,14 +1714,12 @@ extern void usb_register_notify(struct notifier_block *nb);
> extern void usb_unregister_notify(struct notifier_block *nb);
>
> #ifdef DEBUG
> -#define dbg(format, arg...) \
> - printk(KERN_DEBUG "%s: " format "\n", __FILE__, ##arg)
> +#define dbg(format, ...) \
> + pr_debug("%s: " format "\n", __FILE__, ##__VA_ARGS__)
> #else
> -#define dbg(format, arg...) \
> -do { \
> - if (0) \
> - printk(KERN_DEBUG "%s: " format "\n", __FILE__, ##arg); \
> -} while (0)
> +#define dbg(format, ...) \
> + printk_eliminated(KERN_DEBUG "%s: " format "\n", \
> + __FILE__, ##__VA_ARGS)
> #endif
>
Either you didn't test this or your API needs to be punched in the face.

The interface you just added is eliminated_printk(), I sure hope the
above is a typo and you don't actually also have a printk_eliminated().

2012-06-18 07:08:22

by Jonathan Woithe

[permalink] [raw]
Subject: Re: [PATCH 4/5] fujitsu-laptop: Convert dbg_printk and vdbg_printk to fuj_dbg

On Sun, Jun 17, 2012 at 07:25:11PM -0700, Joe Perches wrote:
> Use more current logging styles.
>
> Add #define DEBUG to enable dynamic debug.
> Remove unused #defines.
> Use eliminated_printk to verify fuj_dbg arguments.
> Argument alignment and format coalescing.
>
> Signed-off-by: Joe Perches <[email protected]>

Looks reasonable to me.

Acked-by: Jonathan Woithe <[email protected]>

> ---
> drivers/platform/x86/fujitsu-laptop.c | 103 +++++++++++++++------------------
> 1 files changed, 47 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c
> index c4c1a54..e2aae57 100644
> --- a/drivers/platform/x86/fujitsu-laptop.c
> +++ b/drivers/platform/x86/fujitsu-laptop.c
> @@ -118,27 +118,22 @@
> #define RINGBUFFERSIZE 40
>
> /* Debugging */
> -#define FUJLAPTOP_LOG ACPI_FUJITSU_HID ": "
> -#define FUJLAPTOP_ERR KERN_ERR FUJLAPTOP_LOG
> -#define FUJLAPTOP_NOTICE KERN_NOTICE FUJLAPTOP_LOG
> -#define FUJLAPTOP_INFO KERN_INFO FUJLAPTOP_LOG
> -#define FUJLAPTOP_DEBUG KERN_DEBUG FUJLAPTOP_LOG
> -
> #define FUJLAPTOP_DBG_ALL 0xffff
> #define FUJLAPTOP_DBG_ERROR 0x0001
> #define FUJLAPTOP_DBG_WARN 0x0002
> #define FUJLAPTOP_DBG_INFO 0x0004
> #define FUJLAPTOP_DBG_TRACE 0x0008
>
> -#define dbg_printk(a_dbg_level, format, arg...) \
> - do { if (dbg_level & a_dbg_level) \
> - printk(FUJLAPTOP_DEBUG "%s: " format, __func__ , ## arg); \
> - } while (0)
> #ifdef CONFIG_FUJITSU_LAPTOP_DEBUG
> -#define vdbg_printk(a_dbg_level, format, arg...) \
> - dbg_printk(a_dbg_level, format, ## arg)
> +#define DEBUG
> +#define fuj_dbg(a_dbg_level, format, ...) \
> +do { \
> + if (dbg_level & a_dbg_level) \
> + pr_debug("%s: " format, __func__ , ##__VA_ARGS__); \
> +} while (0)
> #else
> -#define vdbg_printk(a_dbg_level, format, arg...)
> +#define fuj_dbg(a_dbg_level, format, ...) \
> + eliminated_printk(format, ##__VA_ARGS__)
> #endif
>
> /* Device controlling the backlight and associated keys */
> @@ -225,8 +220,7 @@ static int call_fext_func(int cmd, int arg0, int arg1, int arg2)
>
> status = acpi_get_handle(fujitsu_hotkey->acpi_handle, "FUNC", &handle);
> if (ACPI_FAILURE(status)) {
> - vdbg_printk(FUJLAPTOP_DBG_ERROR,
> - "FUNC interface is not present\n");
> + fuj_dbg(FUJLAPTOP_DBG_ERROR, "FUNC interface is not present\n");
> return -ENODEV;
> }
>
> @@ -240,23 +234,22 @@ static int call_fext_func(int cmd, int arg0, int arg1, int arg2)
>
> status = acpi_evaluate_object(handle, NULL, &arg_list, &output);
> if (ACPI_FAILURE(status)) {
> - vdbg_printk(FUJLAPTOP_DBG_WARN,
> + fuj_dbg(FUJLAPTOP_DBG_WARN,
> "FUNC 0x%x (args 0x%x, 0x%x, 0x%x) call failed\n",
> - cmd, arg0, arg1, arg2);
> + cmd, arg0, arg1, arg2);
> return -ENODEV;
> }
>
> if (out_obj.type != ACPI_TYPE_INTEGER) {
> - vdbg_printk(FUJLAPTOP_DBG_WARN,
> - "FUNC 0x%x (args 0x%x, 0x%x, 0x%x) did not "
> - "return an integer\n",
> + fuj_dbg(FUJLAPTOP_DBG_WARN,
> + "FUNC 0x%x (args 0x%x, 0x%x, 0x%x) did not return an integer\n",
> cmd, arg0, arg1, arg2);
> return -ENODEV;
> }
>
> - vdbg_printk(FUJLAPTOP_DBG_TRACE,
> + fuj_dbg(FUJLAPTOP_DBG_TRACE,
> "FUNC 0x%x (args 0x%x, 0x%x, 0x%x) returned 0x%x\n",
> - cmd, arg0, arg1, arg2, (int)out_obj.integer.value);
> + cmd, arg0, arg1, arg2, (int)out_obj.integer.value);
> return out_obj.integer.value;
> }
>
> @@ -321,15 +314,14 @@ static int set_lcd_level(int level)
> struct acpi_object_list arg_list = { 1, &arg0 };
> acpi_handle handle = NULL;
>
> - vdbg_printk(FUJLAPTOP_DBG_TRACE, "set lcd level via SBLL [%d]\n",
> - level);
> + fuj_dbg(FUJLAPTOP_DBG_TRACE, "set lcd level via SBLL [%d]\n", level);
>
> if (level < 0 || level >= fujitsu->max_brightness)
> return -EINVAL;
>
> status = acpi_get_handle(fujitsu->acpi_handle, "SBLL", &handle);
> if (ACPI_FAILURE(status)) {
> - vdbg_printk(FUJLAPTOP_DBG_ERROR, "SBLL not present\n");
> + fuj_dbg(FUJLAPTOP_DBG_ERROR, "SBLL not present\n");
> return -ENODEV;
> }
>
> @@ -349,15 +341,14 @@ static int set_lcd_level_alt(int level)
> struct acpi_object_list arg_list = { 1, &arg0 };
> acpi_handle handle = NULL;
>
> - vdbg_printk(FUJLAPTOP_DBG_TRACE, "set lcd level via SBL2 [%d]\n",
> - level);
> + fuj_dbg(FUJLAPTOP_DBG_TRACE, "set lcd level via SBL2 [%d]\n", level);
>
> if (level < 0 || level >= fujitsu->max_brightness)
> return -EINVAL;
>
> status = acpi_get_handle(fujitsu->acpi_handle, "SBL2", &handle);
> if (ACPI_FAILURE(status)) {
> - vdbg_printk(FUJLAPTOP_DBG_ERROR, "SBL2 not present\n");
> + fuj_dbg(FUJLAPTOP_DBG_ERROR, "SBL2 not present\n");
> return -ENODEV;
> }
>
> @@ -375,7 +366,7 @@ static int get_lcd_level(void)
> unsigned long long state = 0;
> acpi_status status = AE_OK;
>
> - vdbg_printk(FUJLAPTOP_DBG_TRACE, "get lcd level via GBLL\n");
> + fuj_dbg(FUJLAPTOP_DBG_TRACE, "get lcd level via GBLL\n");
>
> status =
> acpi_evaluate_integer(fujitsu->acpi_handle, "GBLL", NULL, &state);
> @@ -397,7 +388,7 @@ static int get_max_brightness(void)
> unsigned long long state = 0;
> acpi_status status = AE_OK;
>
> - vdbg_printk(FUJLAPTOP_DBG_TRACE, "get max lcd level via RBLL\n");
> + fuj_dbg(FUJLAPTOP_DBG_TRACE, "get max lcd level via RBLL\n");
>
> status =
> acpi_evaluate_integer(fujitsu->acpi_handle, "RBLL", NULL, &state);
> @@ -424,7 +415,7 @@ static int bl_update_status(struct backlight_device *b)
> else
> ret = call_fext_func(FUNC_BACKLIGHT, 0x1, 0x4, 0x0);
> if (ret != 0)
> - vdbg_printk(FUJLAPTOP_DBG_ERROR,
> + fuj_dbg(FUJLAPTOP_DBG_ERROR,
> "Unable to adjust backlight power, error code %i\n",
> ret);
>
> @@ -433,7 +424,7 @@ static int bl_update_status(struct backlight_device *b)
> else
> ret = set_lcd_level(b->props.brightness);
> if (ret != 0)
> - vdbg_printk(FUJLAPTOP_DBG_ERROR,
> + fuj_dbg(FUJLAPTOP_DBG_ERROR,
> "Unable to adjust LCD brightness, error code %i\n",
> ret);
> return ret;
> @@ -594,8 +585,8 @@ static void dmi_check_cb_common(const struct dmi_system_id *id)
> use_alt_lcd_levels = 1;
> else
> use_alt_lcd_levels = 0;
> - vdbg_printk(FUJLAPTOP_DBG_TRACE, "auto-detected usealt as "
> - "%i\n", use_alt_lcd_levels);
> + fuj_dbg(FUJLAPTOP_DBG_TRACE, "auto-detected usealt as %i\n",
> + use_alt_lcd_levels);
> }
> }
>
> @@ -704,7 +695,7 @@ static int acpi_fujitsu_add(struct acpi_device *device)
>
> if (ACPI_SUCCESS
> (acpi_get_handle(device->handle, METHOD_NAME__INI, &handle))) {
> - vdbg_printk(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
> + fuj_dbg(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
> if (ACPI_FAILURE
> (acpi_evaluate_object
> (device->handle, METHOD_NAME__INI, NULL, NULL)))
> @@ -714,9 +705,9 @@ static int acpi_fujitsu_add(struct acpi_device *device)
> /* do config (detect defaults) */
> use_alt_lcd_levels = use_alt_lcd_levels == 1 ? 1 : 0;
> disable_brightness_adjust = disable_brightness_adjust == 1 ? 1 : 0;
> - vdbg_printk(FUJLAPTOP_DBG_INFO,
> - "config: [alt interface: %d], [adjust disable: %d]\n",
> - use_alt_lcd_levels, disable_brightness_adjust);
> + fuj_dbg(FUJLAPTOP_DBG_INFO,
> + "config: [alt interface: %d], [adjust disable: %d]\n",
> + use_alt_lcd_levels, disable_brightness_adjust);
>
> if (get_max_brightness() <= 0)
> fujitsu->max_brightness = FUJITSU_LCD_N_LEVELS;
> @@ -762,9 +753,9 @@ static void acpi_fujitsu_notify(struct acpi_device *device, u32 event)
> get_lcd_level();
> newb = fujitsu->brightness_level;
>
> - vdbg_printk(FUJLAPTOP_DBG_TRACE,
> - "brightness button event [%i -> %i (%i)]\n",
> - oldb, newb, fujitsu->brightness_changed);
> + fuj_dbg(FUJLAPTOP_DBG_TRACE,
> + "brightness button event [%i -> %i (%i)]\n",
> + oldb, newb, fujitsu->brightness_changed);
>
> if (oldb < newb) {
> if (disable_brightness_adjust != 1) {
> @@ -790,8 +781,8 @@ static void acpi_fujitsu_notify(struct acpi_device *device, u32 event)
> break;
> default:
> keycode = KEY_UNKNOWN;
> - vdbg_printk(FUJLAPTOP_DBG_WARN,
> - "unsupported event [0x%x]\n", event);
> + fuj_dbg(FUJLAPTOP_DBG_WARN, "unsupported event [0x%x]\n",
> + event);
> break;
> }
>
> @@ -872,7 +863,7 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
>
> if (ACPI_SUCCESS
> (acpi_get_handle(device->handle, METHOD_NAME__INI, &handle))) {
> - vdbg_printk(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
> + fuj_dbg(FUJLAPTOP_DBG_INFO, "Invoking _INI\n");
> if (ACPI_FAILURE
> (acpi_evaluate_object
> (device->handle, METHOD_NAME__INI, NULL, NULL)))
> @@ -883,7 +874,7 @@ static int acpi_fujitsu_hotkey_add(struct acpi_device *device)
> while (call_fext_func(FUNC_BUTTONS, 0x1, 0x0, 0x0) != 0
> && (i++) < MAX_HOTKEY_RINGBUFFER_SIZE)
> ; /* No action, result is discarded */
> - vdbg_printk(FUJLAPTOP_DBG_INFO, "Discarded %i ringbuffer entries\n", i);
> + fuj_dbg(FUJLAPTOP_DBG_INFO, "Discarded %i ringbuffer entries\n", i);
>
> fujitsu_hotkey->rfkill_supported =
> call_fext_func(FUNC_RFKILL, 0x0, 0x0, 0x0);
> @@ -996,13 +987,13 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
> keycode = 0;
> break;
> default:
> - vdbg_printk(FUJLAPTOP_DBG_WARN,
> - "Unknown GIRB result [%x]\n", irb);
> + fuj_dbg(FUJLAPTOP_DBG_WARN,
> + "Unknown GIRB result [%x]\n", irb);
> keycode = -1;
> break;
> }
> if (keycode > 0) {
> - vdbg_printk(FUJLAPTOP_DBG_TRACE,
> + fuj_dbg(FUJLAPTOP_DBG_TRACE,
> "Push keycode into ringbuffer [%d]\n",
> keycode);
> status = kfifo_in_locked(&fujitsu_hotkey->fifo,
> @@ -1010,9 +1001,9 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
> sizeof(keycode),
> &fujitsu_hotkey->fifo_lock);
> if (status != sizeof(keycode)) {
> - vdbg_printk(FUJLAPTOP_DBG_WARN,
> - "Could not push keycode [0x%x]\n",
> - keycode);
> + fuj_dbg(FUJLAPTOP_DBG_WARN,
> + "Could not push keycode [0x%x]\n",
> + keycode);
> } else {
> input_report_key(input, keycode, 1);
> input_sync(input);
> @@ -1027,9 +1018,9 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
> == sizeof(keycode_r)) {
> input_report_key(input, keycode_r, 0);
> input_sync(input);
> - vdbg_printk(FUJLAPTOP_DBG_TRACE,
> - "Pop keycode from ringbuffer [%d]\n",
> - keycode_r);
> + fuj_dbg(FUJLAPTOP_DBG_TRACE,
> + "Pop keycode from ringbuffer [%d]\n",
> + keycode_r);
> }
> }
> }
> @@ -1037,8 +1028,8 @@ static void acpi_fujitsu_hotkey_notify(struct acpi_device *device, u32 event)
> break;
> default:
> keycode = KEY_UNKNOWN;
> - vdbg_printk(FUJLAPTOP_DBG_WARN,
> - "Unsupported event [0x%x]\n", event);
> + fuj_dbg(FUJLAPTOP_DBG_WARN, "Unsupported event [0x%x]\n",
> + event);
> input_report_key(input, keycode, 1);
> input_sync(input);
> input_report_key(input, keycode, 0);
> --
> 1.7.8.111.gad25c.dirty

2012-06-18 15:23:23

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 3/5] usb: Convert dbg to pr_eliminated and pr_debug

On Sun, Jun 17, 2012 at 07:25:10PM -0700, Joe Perches wrote:
> Change the dbg macro to use pr_eliminated when not
> DEBUG and pr_debug with DEBUG so dynamic_debug can
> be used.
>
> Signed-off-by: Joe Perches <[email protected]>
> ---
> include/linux/usb.h | 12 +++++-------
> 1 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index f717fbd..56b88b8 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -1714,14 +1714,12 @@ extern void usb_register_notify(struct notifier_block *nb);
> extern void usb_unregister_notify(struct notifier_block *nb);
>
> #ifdef DEBUG
> -#define dbg(format, arg...) \
> - printk(KERN_DEBUG "%s: " format "\n", __FILE__, ##arg)
> +#define dbg(format, ...) \
> + pr_debug("%s: " format "\n", __FILE__, ##__VA_ARGS__)

No, I really want to delete this macro entirely, and am slowly working
toward that, moving drivers to use the correct dev_dbg() macro instead.

> #else
> -#define dbg(format, arg...) \
> -do { \
> - if (0) \
> - printk(KERN_DEBUG "%s: " format "\n", __FILE__, ##arg); \
> -} while (0)
> +#define dbg(format, ...) \
> + printk_eliminated(KERN_DEBUG "%s: " format "\n", \
> + __FILE__, ##__VA_ARGS)

As Paul pointed out, this breaks the build, please build test your
patches before sending them out, it just wastes our time otherwise :(

greg k-h

2012-06-18 16:07:13

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 3/5] usb: Convert dbg to pr_eliminated and pr_debug

On Mon, 2012-06-18 at 08:23 -0700, Greg Kroah-Hartman wrote:
> On Sun, Jun 17, 2012 at 07:25:10PM -0700, Joe Perches wrote:
> > Change the dbg macro to use pr_eliminated when not
> > DEBUG and pr_debug with DEBUG so dynamic_debug can
> > be used.
> >
> > Signed-off-by: Joe Perches <[email protected]>
> > ---
> > include/linux/usb.h | 12 +++++-------
> > 1 files changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/linux/usb.h b/include/linux/usb.h
> > index f717fbd..56b88b8 100644
> > --- a/include/linux/usb.h
> > +++ b/include/linux/usb.h
> > @@ -1714,14 +1714,12 @@ extern void usb_register_notify(struct notifier_block *nb);
> > extern void usb_unregister_notify(struct notifier_block *nb);
> >
> > #ifdef DEBUG
> > -#define dbg(format, arg...) \
> > - printk(KERN_DEBUG "%s: " format "\n", __FILE__, ##arg)
> > +#define dbg(format, ...) \
> > + pr_debug("%s: " format "\n", __FILE__, ##__VA_ARGS__)
>
> No, I really want to delete this macro entirely, and am slowly working
> toward that, moving drivers to use the correct dev_dbg() macro instead.

It'd be hard to replace it with only dev_dbg
as no struct device is always available.

And anyway, why go slowly? Why not just do it?

There's 41 files / 1442 instances

$ git grep --name-only -E "^\s*#\s*include\s*[\<\"]linux/usb\.h[\>\"]" | \
xargs grep -El "\bdbg\s*\("|wc -l
41
$ git grep --name-only -E "^\s*#\s*include\s*[\<\"]linux/usb\.h[\>\"]" | \
xargs grep -E "\bdbg\s*\(" | wc -l
1442

There are also 5 redefinitions of dbg in there.

> > -#define dbg(format, arg...) \
> > -do { \
> > - if (0) \
> > - printk(KERN_DEBUG "%s: " format "\n", __FILE__, ##arg); \
> > -} while (0)
> > +#define dbg(format, ...) \
> > + printk_eliminated(KERN_DEBUG "%s: " format "\n", \
> > + __FILE__, ##__VA_ARGS)
>
> As Paul pointed out, this breaks the build,

Yeah, Typos happen.

Paul's imagery was certainly colorful though a bit
umm, dispassionate. ;)

2012-06-18 16:14:52

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 3/5] usb: Convert dbg to pr_eliminated and pr_debug

On Mon, Jun 18, 2012 at 09:07:10AM -0700, Joe Perches wrote:
> On Mon, 2012-06-18 at 08:23 -0700, Greg Kroah-Hartman wrote:
> > On Sun, Jun 17, 2012 at 07:25:10PM -0700, Joe Perches wrote:
> > > Change the dbg macro to use pr_eliminated when not
> > > DEBUG and pr_debug with DEBUG so dynamic_debug can
> > > be used.
> > >
> > > Signed-off-by: Joe Perches <[email protected]>
> > > ---
> > > include/linux/usb.h | 12 +++++-------
> > > 1 files changed, 5 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/include/linux/usb.h b/include/linux/usb.h
> > > index f717fbd..56b88b8 100644
> > > --- a/include/linux/usb.h
> > > +++ b/include/linux/usb.h
> > > @@ -1714,14 +1714,12 @@ extern void usb_register_notify(struct notifier_block *nb);
> > > extern void usb_unregister_notify(struct notifier_block *nb);
> > >
> > > #ifdef DEBUG
> > > -#define dbg(format, arg...) \
> > > - printk(KERN_DEBUG "%s: " format "\n", __FILE__, ##arg)
> > > +#define dbg(format, ...) \
> > > + pr_debug("%s: " format "\n", __FILE__, ##__VA_ARGS__)
> >
> > No, I really want to delete this macro entirely, and am slowly working
> > toward that, moving drivers to use the correct dev_dbg() macro instead.
>
> It'd be hard to replace it with only dev_dbg
> as no struct device is always available.

Agreed, but for almost all cases, a USB driver does have a struct
device. If it doesn't, odds are, the driver shouldn't be sending out
debug messages anyway :)

> And anyway, why go slowly? Why not just do it?

I take it you didn't see the 200+ patches in 3.5-rc1 that did a lot of
this work already? Is that slow?

> There's 41 files / 1442 instances
>
> $ git grep --name-only -E "^\s*#\s*include\s*[\<\"]linux/usb\.h[\>\"]" | \
> xargs grep -El "\bdbg\s*\("|wc -l
> 41
> $ git grep --name-only -E "^\s*#\s*include\s*[\<\"]linux/usb\.h[\>\"]" | \
> xargs grep -E "\bdbg\s*\(" | wc -l
> 1442
>
> There are also 5 redefinitions of dbg in there.

Yup, the usb-serial drivers have their own version as well.

It will happen, just give it a release or two to complete.

thanks,

greg k-h

2012-06-18 16:39:13

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 3/5] usb: Convert dbg to pr_eliminated and pr_debug

On Mon, 2012-06-18 at 09:14 -0700, Greg Kroah-Hartman wrote:
> On Mon, Jun 18, 2012 at 09:07:10AM -0700, Joe Perches wrote:
> > On Mon, 2012-06-18 at 08:23 -0700, Greg Kroah-Hartman wrote:
[]
> > > No, I really want to delete this macro entirely, and am slowly working
> > > toward that, moving drivers to use the correct dev_dbg() macro instead.
[]
> > And anyway, why go slowly? Why not just do it?
>
> I take it you didn't see the 200+ patches in 3.5-rc1 that did a lot of
> this work already? Is that slow?

You said slowly.

I don't care much one way or another but
scripts work pretty quickly, humans less so.

dbg is certainly an ugly little wart though.

Subject: Re: [PATCH 5/5] thinkpad_acpi: Convert dbg_printk to tp_dbg and tp_vdbg

On Sun, 17 Jun 2012, Joe Perches wrote:
> Use a more current logging style.
>
> Add #define DEBUG and use pr_debug to enable dynamic debugging.
> Coalesce formats and align arguments.
>
> Signed-off-by: Joe Perches <[email protected]>

Joe, are there any extra procedures required from the user to get the debug
messages with this patch applied?

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2012-06-24 03:06:48

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 5/5] thinkpad_acpi: Convert dbg_printk to tp_dbg and tp_vdbg

On Sat, 2012-06-23 at 21:40 -0300, Henrique de Moraes Holschuh wrote:
> On Sun, 17 Jun 2012, Joe Perches wrote:
> > Use a more current logging style.
> >
> > Add #define DEBUG and use pr_debug to enable dynamic debugging.
> > Coalesce formats and align arguments.
> >
> > Signed-off-by: Joe Perches <[email protected]>
>
> Joe, are there any extra procedures required from the user to get the debug
> messages with this patch applied?

No.

The added #define DEBUG makes dynamic_debug emit those
messages automatically. Without the #define DEBUG, then
these tp_[v]dbg calls might not be compiled in at all.

These to_[v]dbg calls can be turned off via the normal
dyn_debug mechanisms.

see: Documentation/dynamic-debug-howto.txt

cheers, Joe

Subject: Re: [PATCH 5/5] thinkpad_acpi: Convert dbg_printk to tp_dbg and tp_vdbg

On Sat, 23 Jun 2012, Joe Perches wrote:
> On Sat, 2012-06-23 at 21:40 -0300, Henrique de Moraes Holschuh wrote:
> > On Sun, 17 Jun 2012, Joe Perches wrote:
> > > Use a more current logging style.
> > >
> > > Add #define DEBUG and use pr_debug to enable dynamic debugging.
> > > Coalesce formats and align arguments.
> > >
> > > Signed-off-by: Joe Perches <[email protected]>
> >
> > Joe, are there any extra procedures required from the user to get the debug
> > messages with this patch applied?
>
> No.
>
> The added #define DEBUG makes dynamic_debug emit those
> messages automatically. Without the #define DEBUG, then

I suspected as much, but I wanted direct confirmation. That
#define DEBUG is a relatively recent enhancement to dynamic printk, isn't
it? Something I will have to keep in mind if I resume doing backports of
this driver for 2.6.32, 3.0 and 3.2 ?

> these tp_[v]dbg calls might not be compiled in at all.
>
> These to_[v]dbg calls can be turned off via the normal
> dyn_debug mechanisms.
>
> see: Documentation/dynamic-debug-howto.txt

Ok. That's good enough for me.

Acked-by: Henrique de Moraes Holschuh <[email protected]>

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh