applespi: Convert #ifdef'd debug logs to normal debug logs.

[roadrunner2]

I'm submitting this mainly for discussion rather than for actual merging. The biggest problem I see with this as it stands is that it's hard to enable debugging of just some packets, and enabling all results in a huge amount of logging in the kernel log, swamping pretty much everything else (mostly due to the touchpad events). E.g. while it's possible to enable logging of just the touchpad init messages, that's only because they are currently only ones using the applespi_sync_* functions, i.e. if something else starts using them in the future then that new stuff would get logged to when enabling logging for these functions. And enabling logging of received events does so for both keyboard and touchpad events. Also, as I've discovered recently, it doesn't seem possible to enable logging of just some functions via the kernel command line (the docs indicate it should be, but after debugging the dynamic-debuging code in the kernel I've come to the conclusion that just doesn't seem to be the case).

I think ideally we'd want a way to be able to (dynamically) enable/disable logging of messages based on type: init, keyboard events, touchpad events, backlight commands, etc. But AFAICT this can't be done cleanly with the dynamic-debug framework alone. So, maybe we need our own debug module parameter to control logging of messages.

Comments, ideas, thoughts?

[cb22]

@roadrunner2 hmm. Perhaps it might be useful before things are very well tested to allow full packet tracing to be turned on at runtime? My thinking is that if weird things start happening to the trackpad / keyboard, being able to SSH in and enable debugging on the fly (without having to reload the module) might prove to be useful.

That being said, I'm not sure how common (if it all) said issues are. Judging from your comments in #6 it seems like you're getting to the bottom of strange timing issues. In that case, I think just having a debug parameter for when the driver flat out refuses to work is a fair way to go.

[roadrunner2]

@cb22 Yes, absolutely: it must be controllable at runtime. My thinking is to create a "debug" module parameter that is a bitmap to control various bits of debug logging (init, caps-lock control, backlight control, keyboard event, touchpad event, unknown event); and by givng this param mode 644 it could be changed dynamically (by writing to /sys/modules/applespi/parameters/debug), just like the current iso_layout and fnmode params can. Compared to dyndbg this gives a more stable "interface" for enabling bits (not affected by function renaming or refactoring) and allows arbitrary debugging to be enabled at boot time (for some reason the dyndbg func and line match specs don't work when provided on the kernel command line), while still allowing full runtime control.

I'm also thinking that the touchpad dimensions logging should be changed to use this debug param, for consistency.

Thoughts?

[roadrunner2]

Here is a new version of this pull requests. As discussed above, instead of using the dynamic-debug facilities debugging is now controlled via a simple module parameter 'debug'. I added some notes about it to the README too, but you can now enable various debug logging simply with something like

echo 0x7 | sudo tee /sys/module/applespi/parameters/debug

(this would start logging all "commands", including touchpad init, caps-lock led, and backlight).

Logging of touchpad dimensions is now also enabled via this parameter.

I have already found this to be very useful in debugging the delay issues.

[cb22]

@roadrunner2 great! This looks good to me, so I'm going to go ahead and merge it in.

[roadrunner2]

Thanks.