On Sat, Feb 06, 2016 at 11:44:20AM -0700, Sagar Dharia wrote:

+static void schedule_slim_report(struct slim_controller *ctrl,
+				 struct slim_device *sb, bool report)
+{
+	struct sb_report_wd *sbw;
+
+	dev_dbg(&ctrl->dev, "report:%d for slave:%s\n", report, sb->name);
+
+	sbw = kmalloc(sizeof(*sbw), GFP_KERNEL);
+	if (!sbw)
+		return;
+
+	INIT_WORK(&sbw->wd, slim_report);
+	sbw->sb = sb;
+	sbw->report = report;
+	if (!queue_work(ctrl->wq, &sbw->wd)) {
+		dev_err(&ctrl->dev, "failed to queue report:%d slave:%s\n",
+				    report, sb->name);
+		kfree(sbw);
+	}
+}

I'm not 100% clear why we're scheduling this into a workqueue, it'd
probably help to at least explain what's going on in the code for future
reference.

+}
+/**

There's an awful lot of places in this which look like they're missing
blank lines.

+ret_assigned_laddr:
+	mutex_unlock(&ctrl->m_ctrl);
+	if (exists || ret)
+		return ret;
+
+	pr_info("setting slimbus l-addr:%x, ea:%x,%x,%x,%x\n",
+		*laddr, e_addr->manf_id, e_addr->prod_code,
+		e_addr->dev_index, e_addr->instance);

Not a dev_ print?

+	slim = slim_query_device(ctrl, e_addr);
+	if (!slim) {
+		ret = -ENOMEM;

-ENOMEM?

+static void __exit slimbus_exit(void)
+{
+	bus_unregister(&slimbus_type);
+}
+
+static int __init slimbus_init(void)
+{
+	return bus_register(&slimbus_type);
+}
+postcore_initcall(slimbus_init);
+module_exit(slimbus_exit);

Put the annotatations next to their functions.

+MODULE_DESCRIPTION("Slimbus module");
+MODULE_ALIAS("platform:slimbus");

This isn't a platform driver, it shouldn't have this alias.

Probably good to add a module_slimbus_device() macro like other buses
have.