[PATCH v2 46/46] dm: Print device name in dev_xxx like Linux

Sean Anderson
2020-09-15T14:45:22Z

This adorns messages generated by dev_xxx with the device and driver
names. It also redirects dev_xxx to log when it is available. The names
of these functions very roughly take inspiration from Linux, but there is
no deeper correlation.

Both struct udevice and struct device are supported when logging, though
logging with struct device is no better than using log_xxx. The latter is
supported because of the large amount of existing code which logs with
struct device.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v2:
- Support logging with struct device as well as struct udevice. A lot of
drivers, especially USB gadgets, log with devices and not udevices. There
is no major reason why they can't use udevice, but big changes like that
are outside the scope of this series.
- Add some comments to __dev_printk and dev_printk_emit
- Handle struct device as well as struct udevice
- Match format strings for the NULL path to the regular path. This reduces the
amount of duplicated strings.
- Print the device name before the driver name

include/dm/device_compat.h | 110 +++++++++++++++++++++++++++----------
1 file changed, 82 insertions(+), 28 deletions(-)

diff --git a/include/dm/device_compat.h b/include/dm/device_compat.h
index 7c14aa464d..8f26053b45 100644
--- a/include/dm/device_compat.h
+++ b/include/dm/device_compat.h
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0+ */
/*
+ * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
* Copyright (c) 2013 Google, Inc
*
* (C) Copyright 2012
@@ -11,6 +12,7 @@
#define _DM_DEVICE_COMPAT_H

#include <log.h>
+#include <linux/build_bug.h>
#include <linux/compat.h>

/*
@@ -34,18 +36,87 @@
#endif

/*
- * REVISIT:
- * print device name like Linux
+ * Define a new identifier which can be tested on by C code. A similar
+ * definition is made for DEBUG in <log.h>.
*/
-#define dev_printk(dev, fmt, ...) \
-({ \
- printk(fmt, ##__VA_ARGS__); \
+#ifdef VERBOSE_DEBUG
+#define _VERBOSE_DEBUG 1
+#else
+#define _VERBOSE_DEBUG 0
+#endif
+
+/**
+ * dev_printk_emit() - Emit a formatted log message
+ * @cat: Category of the message
+ * @level: Log level of the message
+ * @fmt: Format string
+ * @...: Arguments for @fmt
+ *
+ * This macro logs a message through the appropriate channel. It is a macro so
+ * the if statements can be optimized out (as @level should be a constant known
+ * at compile-time).
+ *
+ * If DEBUG or VERBOSE_DEBUG is defined, then some messages are always printed
+ * (through printf()). This is to match the historical behavior of the dev_xxx
+ * functions.
+ *
+ * If LOG is enabled, use log() to emit the message, otherwise print it based on
+ * the console loglevel.
+ */
+#define dev_printk_emit(cat, level, fmt, ...) \
+({ \
+ if ((_DEBUG && level == LOGL_DEBUG) || \
+ (_VERBOSE_DEBUG && level == LOGL_DEBUG_CONTENT)) \
+ printf(fmt, ##__VA_ARGS__); \
+ else if (CONFIG_IS_ENABLED(LOG)) \
+ log(cat, level, fmt, ##__VA_ARGS__); \
+ else if (level < CONFIG_VAL(LOGLEVEL)) \
+ printf(fmt, ##__VA_ARGS__); \
})

-#define __dev_printk(level, dev, fmt, ...) \
-({ \
- if (level < CONFIG_VAL(LOGLEVEL)) \
- dev_printk(dev, fmt, ##__VA_ARGS__); \
+/**
+ * __dev_printk() - Log a message for a device
+ * @level: Log level of the message
+ * @dev: A &struct udevice or &struct device
+ * @fmt: Format string
+ * @...: Arguments for @fmt
+ *
+ * This macro formats and prints dev_xxx log messages. It is done as a macro
+ * because working with variadic argument is much easier this way, we can
+ * interrogate the type of device we are passed (and whether it *is* a &struct
+ * udevice or &struct device), and dev_printk_emit() can optimize out unused if
+ * branches.
+ *
+ * Because this is a macro, we must enforce type checks ourselves. Ideally, we
+ * would only accept udevices, but there is a significant amount of code (mostly
+ * USB) which calls dev_xxx with &struct device. When assigning ``__dev``, we
+ * must first cast ``dev`` to ``void *`` so we don't get warned when ``dev`` is
+ * a &struct device. Even though the latter branch is not taken, it will still
+ * get compiled and type-checked.
+ *
+ * The format strings in case of a ``NULL`` ``dev`` MUST be kept the same.
+ * Otherwise, @fmt will be duplicated in the data section (with slightly
+ * different prefixes). This is why ``(NULL udevice *)`` is printed as two
+ * string arguments, and not by string pasting.
+ */
+#define __dev_printk(level, dev, fmt, ...) \
+({ \
+ if (__same_type(dev, struct device *)) { \
+ dev_printk_emit(LOG_CATEGORY, level, fmt, ##__VA_ARGS__); \
+ } else { \
+ BUILD_BUG_ON(!__same_type(dev, struct udevice *)); \
+ struct udevice *__dev = (void *)dev; \
+ if (__dev) \
+ dev_printk_emit(__dev->driver->id, level, \
+ "%s %s: " fmt, \
+ __dev->driver->name, __dev->name, \
+ ##__VA_ARGS__); \
+ else \
+ dev_printk_emit(LOG_CATEGORY, level, \
+ "%s %s: " fmt, \
+ "(NULL", "udevice *)", \
+ ##__VA_ARGS__); \
+ } \
})

#define dev_emerg(dev, fmt, ...) \
@@ -62,26 +133,9 @@
__dev_printk(LOGL_NOTICE, dev, fmt, ##__VA_ARGS__)
#define dev_info(dev, fmt, ...) \
__dev_printk(LOGL_INFO, dev, fmt, ##__VA_ARGS__)
-
-#ifdef DEBUG
#define dev_dbg(dev, fmt, ...) \
__dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__)
-#else
-#define dev_dbg(dev, fmt, ...) \
-({ \
- if (0) \
- __dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__); \
-})
-#endif
-
-#ifdef VERBOSE_DEBUG
-#define dev_vdbg dev_dbg
-#else
-#define dev_vdbg(dev, fmt, ...) \
-({ \
- if (0) \
- __dev_printk(LOGL_DEBUG, dev, fmt, ##__VA_ARGS__); \
-})
-#endif
+#define dev_vdbg(dev, fmt, ...) \
+ __dev_printk(LOGL_DEBUG_CONTENT, dev, fmt, ##__VA_ARGS__)

#endif
--
2.28.0