Re: [PATCH][1/3] RapidIO support: core

[Stephen Hemminger]

Quick review comments:


1. Why hack up vmlinux.lds.h? wouldn't declaring rio_route_ops
to be a constant do what you want?


2. Unnecessary initializers

+struct bus_type rio_bus_type = {
+	.name = "rapidio",
+	.match = rio_match_bus,
+	.suspend = NULL,
+	.resume = NULL,

	NULL is unnecessary here.


3. Use existing macro's

+#define DEBUG
+
+#ifdef DEBUG
+#define DBG(x...) printk(x)
+#else
+#define DBG(x...)
+#endif
+

	Use pr_debug() and isn't this debugged yet?

4. Lists should be static if possible

+
+LIST_HEAD(rio_mports);
	static LIST_HEAD(rio_mports)

5. rio_nets defined but not used (if it is used later then
   make it visible to only that code.)

6. rio_net_lock does it need to be global.
   also maybe a name change to rio_global_list_lock


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/