On Thu, Jan 19, 2023 at 01:31:12PM +0000, Phil Elwell wrote: > Hi all, > > > On Wed, 18 Jan 2023 at 17:55, Stefan Wahren <stefan.wahren@xxxxxxxx> wrote: > > > > Hi Umang, > > > > [add Phil] > > > > Am 18.01.23 um 12:58 schrieb Umang Jain: > > > Drop custom logging from the vchiq interface. > > > Mostly of them are replaced with dev_dbg and friends > > > and/or pr_info and friends. > > > > > > The debugfs log levels (in 4/4) are mapped to kernel > > > logs levels (coming from include/linux/kern_levels.h) > > > Would like some thoughts on it as I am not sure (hence > > > marking this is RFC) > > > > > > From drivers/staging/vc04_services/interface/TODO: > > > > > > """ > > > * Cleanup logging mechanism > > > > > > The driver should probably be using the standard kernel logging mechanisms > > > such as dev_info, dev_dbg, and friends. > > > > i don't have any experience with vchiq logging/debug. So i'm not sure if > > it's acceptable to lose the second log level dimension (like > > vchiq_arm_log_level) completely. Complex drivers like brcmfmac have a > > debug mask to avoid log spamming [1]. Maybe this is a compromise. > > > > Btw some loglevel locations has already been messed up during > > refactoring :-( > > > > [1] - drivers/net/wireless/broadcom/brcm80211/brcmfmac/debug.h > > > > > """ > > > > > > Umang Jain (4): > > > staging: vc04_services: vchiq_core: Drop custom logging > > > staging: vc04_services: vchiq_arm: Drop custom logging > > > staging: vc04_services: Drop custom logging > > > staging: vc04_services: Drop remnants of custom logging > > > > > > .../interface/vchiq_arm/vchiq_arm.c | 151 +++--- > > > .../interface/vchiq_arm/vchiq_connected.c | 5 +- > > > .../interface/vchiq_arm/vchiq_core.c | 479 ++++++++---------- > > > .../interface/vchiq_arm/vchiq_core.h | 39 -- > > > .../interface/vchiq_arm/vchiq_debugfs.c | 26 +- > > > .../interface/vchiq_arm/vchiq_dev.c | 78 ++- > > > 6 files changed, 329 insertions(+), 449 deletions(-) > > > > > Thanks for the nudge - this patch set hasn't yet made its way through > the sluggish rpi-kernel moderation. > > I understand the desire to remove the custom logging. I don't welcome > the loss of flexibility that comes with such a strategy What "loss of flexibility"? You now have access to the full dynamic debugging facilities that all of the rest of the kernel has. What is lacking? > , but I'm not > going to argue about it. What's harder to understand is the state that > this patchset leaves VCHIQ logging in. From what I can see, the > per-service logging control has gone, but the code still contains > macros that hint at something useful. Similarly, the debugfs support > is completely vestigial, giving the appearance of control while > actually achieving nothing. The debugfs files should also be removed if they don't do anything anymore. thanks, greg k-h
