On 5/31/2024 4:50 PM, Nemanov, Michael wrote:
On 5/22/2024 12:46 PM, Krzysztof Kozlowski wrote:>
+
+static int cc33xx_probe(struct platform_device *pdev)
+{
+ struct cc33xx *cc;
+ struct ieee80211_hw *hw;
+ struct cc33xx_platdev_data *pdev_data = dev_get_platdata(&pdev->dev);
+ const char *nvs_name;
+ int ret;
+
+ cc33xx_debug(DEBUG_CC33xx, "Wireless Driver Version %s", DRV_VERSION);
Drop
+
+ if (!pdev_data) {
+ cc33xx_error("can't access platform data");
Do not use your own print code. Use standard dev_() calls. This applies
*everywhere*.
[...]
+ cc33xx_debug(DEBUG_CC33xx, "WLAN CC33xx platform device probe done");
Drop, tracing/sysfs gices you this. Do not print simple
success/entry/exit messages.
[...]
+};
+MODULE_DEVICE_TABLE(platform, cc33xx_id_table);
+
+static struct platform_driver cc33xx_driver = {
+ .probe = cc33xx_probe,
+ .remove = cc33xx_remove,
+ .id_table = cc33xx_id_table,
+ .driver = {
+ .name = "cc33xx_driver",
+ }
+};
+
+u32 cc33xx_debug_level = DEBUG_NO_DATAPATH;
Why this is global? Why u32? Why global variable is defined at the end
of the file?!?!
cc33xx_debug_level together with cc33xx_debug/info/error() macros is how
all traces were done in drivers/net/wireless/ti/wlcore/ (originally was
wl1271_debug/info etc.)
It enables / disables traces without rebuilding or even reloading which
is very helpful for remote support. These macros map to dynamic_pr_debug
/ pr_debug. I saw similar wrappers in other wireless drivers (ath12k).
This is also why there are plenty of cc33xx_debug() all over the code,
most are silent by default.
Any more thoughts on debug traces? I'll remove all trivial function
entry / exit traces as Krzysztof requested. Is it OK to keep other
cc33xx_debug() calls which will be off by default?
Michael.
