Re: [PATCH v1 01/16] net-next/yunsilicon: Add xsc driver basic framework
From: Andrew Lunn <andrew@lunn.ch>
Date: 2024-12-18 18:20:12
quoted hunk ↗ jump to hunk
+enum { + XSC_LOG_LEVEL_DBG = 0, + XSC_LOG_LEVEL_INFO = 1, + XSC_LOG_LEVEL_WARN = 2, + XSC_LOG_LEVEL_ERR = 3, +}; + +#define xsc_dev_log(condition, level, dev, fmt, ...) \ +do { \ + if (condition) \ + dev_printk(level, dev, dev_fmt(fmt), ##__VA_ARGS__); \ +} while (0) + +#define xsc_core_dbg(__dev, format, ...) \ + xsc_dev_log(xsc_log_level <= XSC_LOG_LEVEL_DBG, KERN_DEBUG, \ + &(__dev)->pdev->dev, "%s:%d:(pid %d): " format, \ + __func__, __LINE__, current->pid, ##__VA_ARGS__) + +#define xsc_core_dbg_once(__dev, format, ...) \ + dev_dbg_once(&(__dev)->pdev->dev, "%s:%d:(pid %d): " format, \ + __func__, __LINE__, current->pid, \ + ##__VA_ARGS__) + +#define xsc_core_dbg_mask(__dev, mask, format, ...) \ +do { \ + if ((mask) & xsc_debug_mask) \ + xsc_core_dbg(__dev, format, ##__VA_ARGS__); \ +} while (0)
You where asked to throw all these away and just use the existing methods. If you disagree with a comment, please reply and ask for more details, understand the reason behind the comment, or maybe try to justify your solution over what already exists. Maybe look at the ethtool .get_msglevel & .set_msglevel if you are not already using them.
quoted hunk ↗ jump to hunk
+unsigned int xsc_log_level = XSC_LOG_LEVEL_WARN; +module_param_named(log_level, xsc_log_level, uint, 0644); +MODULE_PARM_DESC(log_level, + "lowest log level to print: 0=debug, 1=info, 2=warning, 3=error. Default=1");
Module parameters are not liked. You will however find quite a few drivers with something like: MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)"); which is used to set the initial msglevel. That will probably be accepted.
+EXPORT_SYMBOL(xsc_log_level);
I've not looked at your overall structure yet, but why export this? Are there multiple modules involved? Andrew