Johannes Weiner wrote:
Your patch has broken lines where there shouldn't be any.
OK.
+ struct trec_buffer_struct * pNext;
+ struct trec_struct * pCur;
+ struct trec_struct * pEnd;
Please don't use camel-case - in general.
Would p_next, p_cur and p_end be OK?
+static int snprint_address(char *b, int bsize, unsigned long address)
+{
+#ifdef CONFIG_KALLSYMS
+ unsigned long offset = 0, symsize;
+#else
+ return snprintf(b, bsize, "0x%016lx", address);
+#endif
+}
Would it make sense to #ifdef the whole function?
Make it static int (*)(...) if CONFIG_KALLSYMS and otherwise just a
static inline int (*)(...) { return 0; }
Maybe, but I think just letting the compiler decide is better.
[...]
+static int trec_device_init(void)
+{
+ int result;
+ DPK("trec_device_init: cdev_add failed\n");
+ goto done;
If you jump to `done' here, unregister_chrdev_region is never called.
You should also declare trec_device_init as __init and trex_device_exit
as __exit.
I'll fix this.
--- /dev/null
+++ b/include/linux/trec.h
@@ -0,0 +1,34 @@
+/*
+#define TREC0() trec_write(TREC_PC_ADDR, TREC_PID, 0, 0)
+
+#define ZREC0() do { } while (0)
Why not seperate them by an #ifndef? So you don't have to replace TREC?
with ZREC? but rather change one #define knob.
=Hannes
The reason is to allow the user easily change individual TREC's from
active to inactive by just
changing a single character. Eventually I envision adding runtime
support for changing if a
particular set of TREC's are active or not, but this was simple.
Thanks for the feed back.
Wink
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
