[PATCH 0/3] Introduce framework for SLIMbus device drivers

[PATCH 0/3] Introduce framework for SLIMbus device drivers

[Sagar Dharia]

SLIMbus (Serial Low Power Interchip Media Bus) is a specification
developed by MIPI (Mobile Industry Processor Interface) alliance.
SLIMbus is a 2-wire implementation, which is used to communicate with
peripheral components like audio-codec.
SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
channels, and control channel. Control channel has messages to do
device-enumeration, messages to send/receive control-data to/from
slimbus devices, messages for port/channel management, and messages to
do bandwidth allocation.
Framework is introduced to support  multiple instances of the bus
(1 controller per bus), and multiple slave devices per controller.
SPI and I2C frameworks, and comments from last time when I submitted
the patches were referred-to while working on this framework.

These patchsets introduce device-management, OF helpers, and messaging
APIs for SLIMbus. Framework patches to do channel, port and bandwidth
management are work-in-progress and will be sent out soon.

These patchsets were tested on Qualcomm Snapdragon processor board
using a controller driver, and a slimbus-dev module. This driver and
module will be sent out after the framework patches mentioned above.

Sagar Dharia (3):
  SLIMbus: Device management on SLIMbus
  of/slimbus: OF helper for SLIMbus
  slimbus: Add messaging APIs to slimbus framework

 drivers/Kconfig                 |    2 +
 drivers/Makefile                |    1 +
 drivers/slimbus/Kconfig         |    9 +
 drivers/slimbus/Makefile        |    4 +
 drivers/slimbus/slimbus.c       | 1117 +++++++++++++++++++++++++++++++++++++++
 include/linux/mod_devicetable.h |   13 +
 include/linux/slimbus.h         |  536 +++++++++++++++++++
 7 files changed, 1682 insertions(+)
 create mode 100644 drivers/slimbus/Kconfig
 create mode 100644 drivers/slimbus/Makefile
 create mode 100644 drivers/slimbus/slimbus.c
 create mode 100644 include/linux/slimbus.h

-- 
1.8.2.1

[PATCH 1/3] SLIMbus: Device management on SLIMbus

[Sagar Dharia]

SLIMbus (Serial Low Power Interchip Media Bus) is a specification
developed by MIPI (Mobile Industry Processor Interface) alliance.
SLIMbus is a 2-wire implementation, which is used to communicate with
peripheral components like audio-codec.
SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
channels, and control channel. Control channel has messages to do
device-enumeration, messages to send/receive control-data to/from
slimbus devices, messages for port/channel management, and messages to
do bandwidth allocation.
The framework supports multiple instances of the bus (1 controller per
bus), and multiple slave devices per controller.

This patch does device enumeration, logical address assignment,
informing device when the device reports present/absent etc.
Reporting present may need the driver to do the needful (e.g. turning
on voltage regulators powering the device). So probe is called
if the device is added to board-info list for a controller.
Additionally device is probed when it reports present if that device
doesn't need any such steps mentioned above.

Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
---
 drivers/Kconfig                 |   2 +
 drivers/Makefile                |   1 +
 drivers/slimbus/Kconfig         |   9 +
 drivers/slimbus/Makefile        |   4 +
 drivers/slimbus/slimbus.c       | 767 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mod_devicetable.h |  13 +
 include/linux/slimbus.h         | 393 ++++++++++++++++++++
 7 files changed, 1189 insertions(+)
 create mode 100644 drivers/slimbus/Kconfig
 create mode 100644 drivers/slimbus/Makefile
 create mode 100644 drivers/slimbus/slimbus.c
 create mode 100644 include/linux/slimbus.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index c0cc96b..e39c969 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -182,4 +182,6 @@ source "drivers/thunderbolt/Kconfig"
 
 source "drivers/android/Kconfig"
 
+source "drivers/slimbus/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 46d2554..37c1c88 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -74,6 +74,7 @@ obj-$(CONFIG_TARGET_CORE)	+= target/
 obj-$(CONFIG_MTD)		+= mtd/
 obj-$(CONFIG_SPI)		+= spi/
 obj-$(CONFIG_SPMI)		+= spmi/
+obj-$(CONFIG_SLIMBUS)		+= slimbus/
 obj-y				+= hsi/
 obj-y				+= net/
 obj-$(CONFIG_ATM)		+= atm/
diff --git a/drivers/slimbus/Kconfig b/drivers/slimbus/Kconfig
new file mode 100644
index 0000000..fb30497
--- /dev/null
+++ b/drivers/slimbus/Kconfig
@@ -0,0 +1,9 @@
+#
+# SLIMBUS driver configuration
+#
+menuconfig SLIMBUS
+	tristate "Slimbus support"
+	help
+	  Slimbus is standard interface between baseband and audio codec,
+	  and other peripheral components in mobile terminals.
+
diff --git a/drivers/slimbus/Makefile b/drivers/slimbus/Makefile
new file mode 100644
index 0000000..05f53bc
--- /dev/null
+++ b/drivers/slimbus/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for kernel slimbus framework.
+#
+obj-$(CONFIG_SLIMBUS)			+= slimbus.o
diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
new file mode 100644
index 0000000..be4f2c7
--- /dev/null
+++ b/drivers/slimbus/slimbus.c
@@ -0,0 +1,767 @@
+/* Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/gcd.h>
+#include <linux/completion.h>
+#include <linux/idr.h>
+#include <linux/pm_runtime.h>
+#include <linux/slimbus.h>
+
+static DEFINE_MUTEX(slim_lock);
+static DEFINE_IDR(ctrl_idr);
+static struct device_type slim_dev_type;
+static struct device_type slim_ctrl_type;
+
+static int slim_compare_eaddr(struct slim_eaddr *a, struct slim_eaddr *b)
+{
+	if (a->manf_id == b->manf_id && a->prod_code == b->prod_code &&
+			a->dev_index == b->dev_index &&
+			a->instance == b->instance)
+		return 0;
+	return -EIO;
+}
+
+static const struct slim_device_id *slim_match(const struct slim_device_id *id,
+					const struct slim_device *slim_dev)
+{
+	while (id->manf_id != 0 || id->prod_code != 0) {
+		if (id->manf_id == slim_dev->e_addr.manf_id &&
+				id->prod_code == slim_dev->e_addr.prod_code &&
+				id->dev_index == slim_dev->e_addr.dev_index)
+			return id;
+		id++;
+	}
+	return NULL;
+}
+
+static int slim_device_match(struct device *dev, struct device_driver *driver)
+{
+	struct slim_device *slim_dev;
+	struct slim_driver *drv = to_slim_driver(driver);
+
+	if (dev->type == &slim_dev_type)
+		slim_dev = to_slim_device(dev);
+	else
+		return 0;
+	if (drv->id_table)
+		return slim_match(drv->id_table, slim_dev) != NULL;
+	return 0;
+}
+
+static int slim_device_probe(struct device *dev)
+{
+	struct slim_device	*slim_dev;
+	struct slim_driver	*driver;
+	struct slim_controller *ctrl;
+	int status = 0;
+
+	if (dev->type == &slim_dev_type)
+		slim_dev = to_slim_device(dev);
+	else
+		return -ENXIO;
+
+	driver = to_slim_driver(dev->driver);
+	if (!driver->id_table)
+		return -ENODEV;
+	slim_dev->driver = driver;
+
+	if (driver->probe)
+		status = driver->probe(slim_dev);
+	if (status) {
+		slim_dev->driver = NULL;
+	} else if (driver->device_up) {
+		ctrl = slim_dev->ctrl;
+		queue_work(ctrl->wq, &slim_dev->wd);
+	}
+	return status;
+}
+
+static int slim_device_remove(struct device *dev)
+{
+	struct slim_device *slim_dev;
+	struct slim_driver *driver;
+	int status = 0;
+
+	if (dev->type == &slim_dev_type)
+		slim_dev = to_slim_device(dev);
+	else
+		return -ENXIO;
+
+	if (!dev->driver)
+		return 0;
+	driver = to_slim_driver(dev->driver);
+	if (driver->remove)
+		status = driver->remove(slim_dev);
+
+	slim_dev->notified = false;
+	if (status == 0)
+		slim_dev->driver = NULL;
+	return status;
+}
+
+static void slim_device_shutdown(struct device *dev)
+{
+	struct slim_device *slim_dev;
+	struct slim_driver *driver;
+
+	if (dev->type == &slim_dev_type)
+		slim_dev = to_slim_device(dev);
+	else
+		return;
+
+	if (!dev->driver)
+		return;
+	driver = to_slim_driver(dev->driver);
+	if (driver->shutdown)
+		driver->shutdown(slim_dev);
+}
+
+#ifdef CONFIG_PM_SLEEP
+
+static int slim_pm_suspend(struct device *dev)
+{
+	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+
+	if (pm)
+		return pm_generic_suspend(dev);
+	else
+		return 0;
+}
+
+static int slim_pm_resume(struct device *dev)
+{
+	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+
+	if (pm)
+		return pm_generic_resume(dev);
+	else
+		return 0;
+}
+
+#else
+#define slim_pm_suspend		NULL
+#define slim_pm_resume		NULL
+#endif
+
+static const struct dev_pm_ops slimbus_pm = {
+	.suspend = slim_pm_suspend,
+	.resume = slim_pm_resume,
+	SET_RUNTIME_PM_OPS(
+		pm_generic_suspend,
+		pm_generic_resume,
+		NULL
+		)
+};
+struct bus_type slimbus_type = {
+	.name		= "slimbus",
+	.match		= slim_device_match,
+	.probe		= slim_device_probe,
+	.remove		= slim_device_remove,
+	.shutdown	= slim_device_shutdown,
+	.pm		= &slimbus_pm,
+};
+EXPORT_SYMBOL(slimbus_type);
+
+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);
+
+/*
+ * slim_driver_register: Client driver registration with slimbus
+ * @drv:Client driver to be associated with client-device.
+ * This API will register the client driver with the slimbus
+ * It is called from the driver's module-init function.
+ */
+int slim_driver_register(struct slim_driver *drv)
+{
+	drv->driver.bus = &slimbus_type;
+
+	return driver_register(&drv->driver);
+}
+EXPORT_SYMBOL(slim_driver_register);
+
+/*
+ * slim_driver_unregister: Undo effect of slim_driver_register
+ * @drv: Client driver to be unregistered
+ */
+void slim_driver_unregister(struct slim_driver *drv)
+{
+	if (drv)
+		driver_unregister(&drv->driver);
+}
+
+#define slim_ctrl_attr_gr NULL
+
+static void slim_ctrl_release(struct device *dev)
+{
+	struct slim_controller *ctrl = to_slim_controller(dev);
+
+	complete(&ctrl->dev_released);
+}
+
+static struct device_type slim_ctrl_type = {
+	.groups		= slim_ctrl_attr_gr,
+	.release	= slim_ctrl_release,
+};
+
+static struct slim_controller *slim_ctrl_get(struct slim_controller *ctrl)
+{
+	if (!ctrl || !get_device(&ctrl->dev))
+		return NULL;
+
+	return ctrl;
+}
+
+static void slim_ctrl_put(struct slim_controller *ctrl)
+{
+	if (ctrl)
+		put_device(&ctrl->dev);
+}
+
+#define slim_device_attr_gr NULL
+#define slim_device_uevent NULL
+static void slim_dev_release(struct device *dev)
+{
+	struct slim_device *sbdev = to_slim_device(dev);
+
+	slim_ctrl_put(sbdev->ctrl);
+}
+
+static struct device_type slim_dev_type = {
+	.groups		= slim_device_attr_gr,
+	.uevent		= slim_device_uevent,
+	.release	= slim_dev_release,
+};
+
+static void slim_report(struct work_struct *work)
+{
+	struct slim_driver *sbdrv;
+	struct slim_device *sbdev =
+			container_of(work, struct slim_device, wd);
+	if (!sbdev->dev.driver)
+		return;
+	/* check if device-up or down needs to be called */
+	if ((!sbdev->reported && !sbdev->notified) ||
+			(sbdev->reported && sbdev->notified))
+		return;
+
+	sbdrv = to_slim_driver(sbdev->dev.driver);
+	/*
+	 * address no longer valid, means device reported absent, whereas
+	 * address valid, means device reported present
+	 */
+	if (sbdev->notified && !sbdev->reported) {
+		sbdev->notified = false;
+		if (sbdrv->device_down)
+			sbdrv->device_down(sbdev);
+	} else if (!sbdev->notified && sbdev->reported) {
+		sbdev->notified = true;
+		if (sbdrv->device_up)
+			sbdrv->device_up(sbdev);
+	}
+}
+
+/*
+ * slim_add_device: Add a new device without register board info.
+ * @ctrl: Controller to which this device is to be added to.
+ * Called when device doesn't have an explicit client-driver to be probed, or
+ * the client-driver is a module installed dynamically.
+ */
+int slim_add_device(struct slim_controller *ctrl, struct slim_device *sbdev)
+{
+	sbdev->dev.bus = &slimbus_type;
+	sbdev->dev.parent = ctrl->dev.parent;
+	sbdev->dev.type = &slim_dev_type;
+	sbdev->dev.driver = NULL;
+	sbdev->ctrl = ctrl;
+	slim_ctrl_get(ctrl);
+	if (!sbdev->name) {
+		sbdev->name = kcalloc(SLIMBUS_NAME_SIZE, sizeof(char),
+					GFP_KERNEL);
+		if (!sbdev->name)
+			return -ENOMEM;
+		snprintf(sbdev->name, SLIMBUS_NAME_SIZE, "0x%x:0x%x:0x%x:0x%x",
+				sbdev->e_addr.manf_id, sbdev->e_addr.prod_code,
+				sbdev->e_addr.dev_index,
+				sbdev->e_addr.instance);
+	}
+	dev_dbg(&ctrl->dev, "adding device:%s", sbdev->name);
+	dev_set_name(&sbdev->dev, "%s", sbdev->name);
+	INIT_WORK(&sbdev->wd, slim_report);
+	mutex_lock(&ctrl->m_ctrl);
+	list_add_tail(&sbdev->dev_list, &ctrl->devs);
+	mutex_unlock(&ctrl->m_ctrl);
+	/* probe slave on this controller */
+	return device_register(&sbdev->dev);
+}
+EXPORT_SYMBOL(slim_add_device);
+
+struct sbi_boardinfo {
+	struct list_head	list;
+	struct slim_boardinfo	board_info;
+};
+
+static LIST_HEAD(board_list);
+static LIST_HEAD(slim_ctrl_list);
+static DEFINE_MUTEX(board_lock);
+
+/* If controller is not present, only add to boards list */
+static void slim_match_ctrl_to_boardinfo(struct slim_controller *ctrl,
+				struct slim_boardinfo *bi)
+{
+	int ret;
+
+	if (ctrl->nr != bi->bus_num)
+		return;
+
+	ret = slim_add_device(ctrl, bi->slim_slave);
+	if (ret != 0)
+		dev_err(ctrl->dev.parent, "can't create new device %s, ret:%d",
+			bi->slim_slave->name, ret);
+}
+
+/*
+ * slim_register_board_info: Board-initialization routine.
+ * @info: List of all devices on all controllers present on the board.
+ * @n: number of entries.
+ * API enumerates respective devices on corresponding controller.
+ * Called from board-init function.
+ */
+int slim_register_board_info(struct slim_boardinfo const *info, unsigned n)
+{
+	struct sbi_boardinfo *bi;
+	int i;
+
+	bi = kcalloc(n, sizeof(*bi), GFP_KERNEL);
+	if (!bi)
+		return -ENOMEM;
+
+	for (i = 0; i < n; i++, bi++, info++) {
+		struct slim_controller *ctrl;
+
+		memcpy(&bi->board_info, info, sizeof(*info));
+		mutex_lock(&board_lock);
+		list_add_tail(&bi->list, &board_list);
+		list_for_each_entry(ctrl, &slim_ctrl_list, list)
+			slim_match_ctrl_to_boardinfo(ctrl, &bi->board_info);
+		mutex_unlock(&board_lock);
+	}
+	return 0;
+}
+EXPORT_SYMBOL(slim_register_board_info);
+
+/*
+ * slim_ctrl_add_boarddevs: Add devices registered by board-info
+ * @ctrl: Controller to which these devices are to be added to.
+ * This API is called by controller when it is up and running.
+ * If devices on a controller were registered before controller,
+ * this will make sure that they get probed when controller is up.
+ */
+void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)
+{
+	struct sbi_boardinfo *bi;
+
+	mutex_lock(&board_lock);
+	list_add_tail(&ctrl->list, &slim_ctrl_list);
+	list_for_each_entry(bi, &board_list, list)
+		slim_match_ctrl_to_boardinfo(ctrl, &bi->board_info);
+	mutex_unlock(&board_lock);
+}
+EXPORT_SYMBOL(slim_ctrl_add_boarddevs);
+
+static int slim_register_controller(struct slim_controller *ctrl)
+{
+	int ret = 0;
+
+	/* Can't register until after driver model init */
+	if (WARN_ON(!slimbus_type.p)) {
+		ret = -EAGAIN;
+		goto out_list;
+	}
+
+	dev_set_name(&ctrl->dev, "sb-%d", ctrl->nr);
+	ctrl->dev.bus = &slimbus_type;
+	ctrl->dev.type = &slim_ctrl_type;
+	ctrl->num_dev = 0;
+	if (!ctrl->min_cg)
+		ctrl->min_cg = SLIM_MIN_CLK_GEAR;
+	if (!ctrl->max_cg)
+		ctrl->max_cg = SLIM_MAX_CLK_GEAR;
+	mutex_init(&ctrl->m_ctrl);
+	ret = device_register(&ctrl->dev);
+	if (ret)
+		goto out_list;
+
+	dev_dbg(&ctrl->dev, "Bus [%s] registered:dev:%p\n", ctrl->name,
+							&ctrl->dev);
+
+	INIT_LIST_HEAD(&ctrl->devs);
+	ctrl->wq = create_singlethread_workqueue(dev_name(&ctrl->dev));
+	if (!ctrl->wq)
+		goto err_workq_failed;
+
+	return 0;
+
+err_workq_failed:
+	device_unregister(&ctrl->dev);
+out_list:
+	mutex_lock(&slim_lock);
+	idr_remove(&ctrl_idr, ctrl->nr);
+	mutex_unlock(&slim_lock);
+	return ret;
+}
+
+/* slim_remove_device: Remove the effect of slim_add_device() */
+void slim_remove_device(struct slim_device *sbdev)
+{
+	struct slim_controller *ctrl = sbdev->ctrl;
+
+	mutex_lock(&ctrl->m_ctrl);
+	list_del_init(&sbdev->dev_list);
+	mutex_unlock(&ctrl->m_ctrl);
+	device_unregister(&sbdev->dev);
+}
+EXPORT_SYMBOL(slim_remove_device);
+
+static void slim_ctrl_remove_device(struct slim_controller *ctrl,
+				struct slim_boardinfo *bi)
+{
+	if (ctrl->nr == bi->bus_num)
+		slim_remove_device(bi->slim_slave);
+}
+
+/*
+ * slim_del_controller: Controller tear-down.
+ * Controller added with the above API is teared down using this API.
+ */
+int slim_del_controller(struct slim_controller *ctrl)
+{
+	struct slim_controller *found;
+	struct sbi_boardinfo *bi;
+
+	/* First make sure that this bus was added */
+	mutex_lock(&slim_lock);
+	found = idr_find(&ctrl_idr, ctrl->nr);
+	mutex_unlock(&slim_lock);
+	if (found != ctrl)
+		return -EINVAL;
+
+	/* Remove all clients */
+	mutex_lock(&board_lock);
+	list_for_each_entry(bi, &board_list, list)
+		slim_ctrl_remove_device(ctrl, &bi->board_info);
+	mutex_unlock(&board_lock);
+
+	init_completion(&ctrl->dev_released);
+	device_unregister(&ctrl->dev);
+
+	wait_for_completion(&ctrl->dev_released);
+	list_del(&ctrl->list);
+	destroy_workqueue(ctrl->wq);
+	/* free bus id */
+	mutex_lock(&slim_lock);
+	idr_remove(&ctrl_idr, ctrl->nr);
+	mutex_unlock(&slim_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(slim_del_controller);
+
+/*
+ * slim_add_numbered_controller: Controller bring-up.
+ * @ctrl: Controller to be registered.
+ * A controller is registered with the framework using this API. ctrl->nr is the
+ * desired number with which slimbus framework registers the controller.
+ * Function will return -EBUSY if the number is in use.
+ */
+int slim_add_numbered_controller(struct slim_controller *ctrl)
+{
+	int	id;
+
+	mutex_lock(&slim_lock);
+	id = idr_alloc(&ctrl_idr, ctrl, ctrl->nr, ctrl->nr + 1, GFP_KERNEL);
+	mutex_unlock(&slim_lock);
+
+	if (id < 0)
+		return id;
+
+	ctrl->nr = id;
+	return slim_register_controller(ctrl);
+}
+EXPORT_SYMBOL(slim_add_numbered_controller);
+
+/*
+ * slim_report_absent: Controller calls this function when a device
+ *	reports absent, OR when the device cannot be communicated with
+ * @sbdev: Device that cannot be reached, or sent report absent
+ */
+void slim_report_absent(struct slim_device *sbdev)
+{
+	struct slim_controller *ctrl;
+	int i;
+
+	if (!sbdev)
+		return;
+	ctrl = sbdev->ctrl;
+	if (!ctrl)
+		return;
+	/* invalidate logical addresses */
+	mutex_lock(&ctrl->m_ctrl);
+	for (i = 0; i < ctrl->num_dev; i++) {
+		if (sbdev->laddr == ctrl->addrt[i].laddr)
+			ctrl->addrt[i].valid = false;
+	}
+	mutex_unlock(&ctrl->m_ctrl);
+	sbdev->reported = false;
+	queue_work(ctrl->wq, &sbdev->wd);
+}
+EXPORT_SYMBOL(slim_report_absent);
+
+/*
+ * slim_framer_booted: This function is called by controller after the active
+ * framer has booted (using Bus Reset sequence, or after it has shutdown and has
+ * come back up). Components, devices on the bus may be in undefined state,
+ * and this function triggers their drivers to do the needful
+ * to bring them back in Reset state so that they can acquire sync, report
+ * present and be operational again.
+ */
+void slim_framer_booted(struct slim_controller *ctrl)
+{
+	struct slim_device *sbdev;
+	struct list_head *pos, *next;
+
+	if (!ctrl)
+		return;
+	mutex_lock(&ctrl->m_ctrl);
+	list_for_each_safe(pos, next, &ctrl->devs) {
+		struct slim_driver *sbdrv;
+
+		sbdev = list_entry(pos, struct slim_device, dev_list);
+		mutex_unlock(&ctrl->m_ctrl);
+		if (sbdev && sbdev->dev.driver) {
+			sbdrv = to_slim_driver(sbdev->dev.driver);
+			if (sbdrv->reset_device)
+				sbdrv->reset_device(sbdev);
+		}
+		mutex_lock(&ctrl->m_ctrl);
+	}
+	mutex_unlock(&ctrl->m_ctrl);
+}
+EXPORT_SYMBOL(slim_framer_booted);
+
+/*
+ * slim_query_device: Query and get handle to a device.
+ * @ctrl: Controller on which this device will be added/queried
+ * @e_addr: Enumeration address of the device to be queried
+ * Returns pointer to a device if it has already reported. Creates a new
+ * device and returns pointer to it if the device has not yet enumerated.
+ */
+struct slim_device *slim_query_device(struct slim_controller *ctrl,
+					struct slim_eaddr *e_addr)
+{
+	struct slim_device *slim = NULL;
+	struct sbi_boardinfo *bi;
+
+	/* Check if the device is already present */
+	mutex_lock(&board_lock);
+	list_for_each_entry(bi, &board_list, list) {
+		if (bi->board_info.bus_num != ctrl->nr)
+			continue;
+		if (slim_compare_eaddr(&bi->board_info.slim_slave->e_addr,
+				e_addr) == 0) {
+			slim = bi->board_info.slim_slave;
+			break;
+		}
+	}
+	mutex_unlock(&board_lock);
+	if (slim)
+		return slim;
+
+	mutex_lock(&ctrl->m_ctrl);
+	list_for_each_entry(slim, &ctrl->devs, dev_list) {
+		if (slim_compare_eaddr(&slim->e_addr, e_addr) == 0) {
+			mutex_unlock(&ctrl->m_ctrl);
+			return slim;
+		}
+	}
+	mutex_unlock(&ctrl->m_ctrl);
+
+	slim = kcalloc(1, sizeof(struct slim_device), GFP_KERNEL);
+	if (IS_ERR(slim))
+		return NULL;
+	slim->e_addr = *e_addr;
+	if (slim_add_device(ctrl, slim) != 0) {
+		kfree(slim);
+		return NULL;
+	}
+	return slim;
+}
+EXPORT_SYMBOL(slim_query_device);
+
+static int ctrl_getaddr_entry(struct slim_controller *ctrl,
+				struct slim_eaddr *eaddr, u8 *entry)
+{
+	int i;
+
+	for (i = 0; i < ctrl->num_dev; i++) {
+		if (ctrl->addrt[i].valid &&
+		slim_compare_eaddr(&ctrl->addrt[i].eaddr, eaddr) == 0) {
+			*entry = i;
+			return 0;
+		}
+	}
+	return -ENXIO;
+}
+
+/*
+ * slim_assign_laddr: Assign logical address to a device enumerated.
+ * @ctrl: Controller with which device is enumerated.
+ * @e_addr: Enumeration address of the device.
+  * @laddr: Return logical address (if valid flag is false)
+  * @valid: true if laddr holds a valid address that controller wants to
+  *	set for this enumeration address. Otherwise framework sets index into
+  *	address table as logical address.
+ * Called by controller in response to REPORT_PRESENT. Framework will assign
+ * a logical address to this enumeration address.
+ * Function returns -EXFULL to indicate that all logical addresses are already
+ * taken.
+ */
+int slim_assign_laddr(struct slim_controller *ctrl, struct slim_eaddr *e_addr,
+				u8 *laddr, bool valid)
+{
+	int ret;
+	u8 i = 0;
+	bool exists = false;
+	struct slim_device *slim;
+	struct slim_addrt *temp;
+
+	mutex_lock(&ctrl->m_ctrl);
+	/* already assigned */
+	if (ctrl_getaddr_entry(ctrl, e_addr, &i) == 0) {
+		*laddr = ctrl->addrt[i].laddr;
+		exists = true;
+	} else {
+		if (ctrl->num_dev >= (SLIM_LA_MANAGER - 1)) {
+			ret = -EXFULL;
+			goto ret_assigned_laddr;
+		}
+		for (i = 0; i < ctrl->num_dev; i++) {
+			if (ctrl->addrt[i].valid == false)
+				break;
+		}
+		if (i == ctrl->num_dev) {
+			temp = krealloc(ctrl->addrt,
+					(ctrl->num_dev + 1) *
+					sizeof(struct slim_addrt),
+					GFP_KERNEL);
+			if (!temp) {
+				ret = -ENOMEM;
+				goto ret_assigned_laddr;
+			}
+			ctrl->addrt = temp;
+			ctrl->num_dev++;
+		}
+		ctrl->addrt[i].eaddr = *e_addr;
+		ctrl->addrt[i].valid = true;
+		/* Preferred address is index into table */
+		if (!valid)
+			*laddr = i;
+	}
+
+	ret = ctrl->set_laddr(ctrl, &ctrl->addrt[i].eaddr, *laddr);
+	if (ret) {
+		ctrl->addrt[i].valid = false;
+		goto ret_assigned_laddr;
+	}
+	ctrl->addrt[i].laddr = *laddr;
+
+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",
+				*laddr, e_addr->manf_id, e_addr->prod_code,
+				e_addr->dev_index, e_addr->instance);
+	/*
+	 * Add this device to list of devices on this controller if it's
+	 * not already present
+	 */
+	slim = slim_query_device(ctrl, e_addr);
+	if (!slim) {
+		ret = -ENOMEM;
+	} else {
+		struct slim_driver *sbdrv;
+
+		slim->laddr = *laddr;
+		mutex_lock(&ctrl->m_ctrl);
+		if (slim->dev.driver) {
+			sbdrv = to_slim_driver(slim->dev.driver);
+			if (sbdrv->device_up)
+				queue_work(ctrl->wq, &slim->wd);
+		}
+		mutex_unlock(&ctrl->m_ctrl);
+	}
+	return ret;
+}
+EXPORT_SYMBOL(slim_assign_laddr);
+
+/*
+ * slim_get_logical_addr: Return the logical address of a slimbus device.
+ * @sb: client handle requesting the adddress.
+ * @e_addr: Enumeration address of the device.
+ * @laddr: output buffer to store the address
+ * context: can sleep
+ * -EINVAL is returned in case of invalid parameters, and -ENXIO is returned if
+ *  the device with this enumeration address is not found.
+ */
+int slim_get_logical_addr(struct slim_device *sb, struct slim_eaddr *e_addr,
+				u8 *laddr)
+{
+	int ret;
+	u8 entry;
+	struct slim_controller *ctrl = sb->ctrl;
+
+	if (!ctrl || !laddr || !e_addr)
+		return -EINVAL;
+
+	mutex_lock(&ctrl->m_ctrl);
+	ret = ctrl_getaddr_entry(ctrl, e_addr, &entry);
+	if (!ret)
+		*laddr = ctrl->addrt[entry].laddr;
+	mutex_unlock(&ctrl->m_ctrl);
+
+	if (ret == -ENXIO && ctrl->get_laddr) {
+		ret = ctrl->get_laddr(ctrl, e_addr, laddr);
+		if (!ret)
+			ret = slim_assign_laddr(ctrl, e_addr, laddr, true);
+	}
+	return ret;
+}
+EXPORT_SYMBOL(slim_get_logical_addr);
+
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("0.1");
+MODULE_DESCRIPTION("Slimbus module");
+MODULE_ALIAS("platform:slimbus");
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 3bfd567..94abc09 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -427,6 +427,19 @@ struct spi_device_id {
 	kernel_ulong_t driver_data;	/* Data private to the driver */
 };
 
+/* SLIMbus */
+
+#define SLIMBUS_NAME_SIZE	32
+#define SLIMBUS_MODULE_PREFIX	"slim:"
+
+struct slim_device_id {
+	__u16 manf_id, prod_code;
+	__u8 dev_index, instance;
+
+	/* Data private to the driver */
+	kernel_ulong_t driver_data;
+};
+
 #define SPMI_NAME_SIZE	32
 #define SPMI_MODULE_PREFIX "spmi:"
 
diff --git a/include/linux/slimbus.h b/include/linux/slimbus.h
new file mode 100644
index 0000000..05b7594
--- /dev/null
+++ b/include/linux/slimbus.h
@@ -0,0 +1,393 @@
+/* Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _LINUX_SLIMBUS_H
+#define _LINUX_SLIMBUS_H
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/mod_devicetable.h>
+
+/*
+ * Interfaces between SLIMbus manager drivers, SLIMbus client drivers, and
+ * SLIMbus infrastructure.
+ */
+
+extern struct bus_type slimbus_type;
+
+/* Standard values per SLIMbus spec needed by controllers and devices */
+#define SLIM_CL_PER_SUPERFRAME		6144
+#define SLIM_CL_PER_SUPERFRAME_DIV8	(SLIM_CL_PER_SUPERFRAME >> 3)
+#define SLIM_MAX_CLK_GEAR		10
+#define SLIM_MIN_CLK_GEAR		1
+#define SLIM_CL_PER_SL			4
+#define SLIM_SL_PER_SUPERFRAME		(SLIM_CL_PER_SUPERFRAME >> 2)
+#define SLIM_FRM_SLOTS_PER_SUPERFRAME	16
+#define SLIM_GDE_SLOTS_PER_SUPERFRAME	2
+
+struct slim_controller;
+struct slim_device;
+
+/*
+ * struct slim_eaddr: Enumeration address for a slimbus device
+ * @manf_id: Manufacturer Id for the device
+ * @prod_code: Product code
+ * @dev_index: Device index
+ * @instance: Instance value
+ */
+struct slim_eaddr {
+	u16 manf_id;
+	u16 prod_code;
+	u8 dev_index;
+	u8 instance;
+};
+
+/*
+ * struct slim_framer - Represents Slimbus framer.
+ * Every controller may have multiple framers. There is 1 active framer device
+ * responsible for clocking the bus.
+ * Manager is responsible for framer hand-over.
+ * @e_addr: Enumeration address of the framer.
+ * @rootfreq: Root Frequency at which the framer can run. This is maximum
+ *	frequency ('clock gear 10') at which the bus can operate.
+ * @superfreq: Superframes per root frequency. Every frame is 6144 bits.
+ */
+struct slim_framer {
+	struct slim_eaddr	e_addr;
+	int			rootfreq;
+	int			superfreq;
+};
+#define to_slim_framer(d) container_of(d, struct slim_framer, dev)
+
+/*
+ * struct slim_addrt: slimbus address used internally by the slimbus framework.
+ * @valid: If the device is present. Valid is set to false when device reports
+ *	absent.
+ * @eaddr: Enumeration address
+ * @laddr: It is possible that controller will set a predefined logical address
+ *	rather than the one assigned by framework. (i.e. logical address may
+ *	not be same as index into this table). This entry will store the
+ *	logical address value for this enumeration address.
+ */
+struct slim_addrt {
+	bool			valid;
+	struct slim_eaddr	eaddr;
+	u8			laddr;
+};
+
+/* SLIMbus message types. Related to interpretation of message code. */
+#define SLIM_MSG_MT_CORE			0x0
+#define SLIM_MSG_MT_DEST_REFERRED_CLASS		0x1
+#define SLIM_MSG_MT_DEST_REFERRED_USER		0x2
+#define SLIM_MSG_MT_SRC_REFERRED_CLASS		0x5
+#define SLIM_MSG_MT_SRC_REFERRED_USER		0x6
+
+/* SLIMbus core type Message Codes. */
+/* Device management messages used by this framework */
+#define SLIM_MSG_MC_REPORT_PRESENT               0x1
+#define SLIM_MSG_MC_ASSIGN_LOGICAL_ADDRESS       0x2
+#define SLIM_MSG_MC_REPORT_ABSENT                0xF
+
+/* Destination type Values */
+#define SLIM_MSG_DEST_LOGICALADDR	0
+#define SLIM_MSG_DEST_ENUMADDR		1
+#define	SLIM_MSG_DEST_BROADCAST		3
+
+/*
+ * struct slim_controller: Controls every instance of SLIMbus
+ *				(similar to 'master' on SPI)
+ *	'Manager device' is responsible for  device management, bandwidth
+ *	allocation, channel setup, and port associations per channel.
+ *	Device management means Logical address assignment/removal based on
+ *	enumeration (report-present, report-absent) if a device.
+ *	Bandwidth allocation is done dynamically by the manager based on active
+ *	channels on the bus, message-bandwidth requests made by slimbus devices.
+ *	Based on current bandwidth usage, manager chooses a frequency to run
+ *	the bus at (in steps of 'clock-gear', 1 through 10, each clock gear
+ *	representing twice the frequency than the previous gear).
+ *	Manager is also responsible for entering (and exiting) low-power-mode
+ *	(known as 'clock pause').
+ *	Manager can do handover of framer if there are multiple framers on the
+ *	bus and a certain usecase warrants using certain framer to avoid keeping
+ *	previous framer being powered-on.
+ *
+ *	Controller here performs duties of the manager device, and 'interface
+ *	device'. Interface device is responsible for monitoring the bus and
+ *	reporting information such as loss-of-synchronization, data
+ *	slot-collision.
+ * @dev: Device interface to this driver
+ * @nr: Board-specific number identifier for this controller/bus
+ * @list: Link with other slimbus controllers
+ * @name: Name for this controller
+ * @min_cg: Minimum clock gear supported by this controller (default value: 1)
+ * @max_cg: Maximum clock gear supported by this controller (default value: 10)
+ * @clkgear: Current clock gear in which this bus is running
+ * @a_framer: Active framer which is clocking the bus managed by this controller
+ * @m_ctrl: Mutex protecting controller data structures
+ * @addrt: Logical address table
+ * @num_dev: Number of active slimbus slaves on this bus
+ * @devs: List of devices on this controller
+ * @wq: Workqueue per controller used to notify devices when they report present
+ * @dev_released: completion used to signal when sysfs has released this
+ *	controller so that it can be deleted during shutdown
+ * @xfer_msg: Transfer a message on this controller (this can be a broadcast
+ *	control/status message like data channel setup, or a unicast message
+ *	like value element read/write.
+ * @set_laddr: Setup logical address at laddr for the slave with elemental
+ *	address e_addr. Drivers implementing controller will be expected to
+ *	send unicast message to this device with its logical address.
+ * @get_laddr: It is possible that controller needs to set fixed logical
+ *	address table and get_laddr can be used in that case so that controller
+ *	can do this assignment.
+ */
+struct slim_controller {
+	struct device		dev;
+	unsigned int		nr;
+	struct list_head	list;
+	char			name[SLIMBUS_NAME_SIZE];
+	int			min_cg;
+	int			max_cg;
+	int			clkgear;
+	struct slim_framer	*a_framer;
+	struct mutex		m_ctrl;
+	struct slim_addrt	*addrt;
+	u8			num_dev;
+	struct list_head	devs;
+	struct workqueue_struct *wq;
+	struct completion	dev_released;
+	int			(*set_laddr)(struct slim_controller *ctrl,
+				struct slim_eaddr *ea, u8 laddr);
+	int			(*get_laddr)(struct slim_controller *ctrl,
+				struct slim_eaddr *ea, u8 *laddr);
+};
+#define to_slim_controller(d) container_of(d, struct slim_controller, dev)
+
+/*
+ * struct slim_driver: Slimbus 'generic device' (slave) device driver
+ *				(similar to 'spi_device' on SPI)
+ * @probe: Binds this driver to a slimbus device.
+ * @remove: Unbinds this driver from the slimbus device.
+ * @shutdown: Standard shutdown callback used during powerdown/halt.
+ * @suspend: Standard suspend callback used during system suspend
+ * @resume: Standard resume callback used during system resume
+ * @device_up: This callback is called when the device reports present and
+ *		gets a logical address assigned to it
+ * @device_down: This callback is called when device reports absent, or the
+ *		bus goes down. Device will report present when bus is up and
+ *		device_up callback will be called again when that happens
+ * @reset_device: This callback is called after framer is booted.
+ *		Driver should do the needful to reset the device,
+ *		so that device acquires sync and be operational.
+ * @driver: Slimbus device drivers should initialize name and owner field of
+ *	this structure
+ * @id_table: List of slimbus devices supported by this driver
+ */
+struct slim_driver {
+	int				(*probe)(struct slim_device *sl);
+	int				(*remove)(struct slim_device *sl);
+	void				(*shutdown)(struct slim_device *sl);
+	int				(*suspend)(struct slim_device *sl,
+					pm_message_t pmesg);
+	int				(*resume)(struct slim_device *sl);
+	int				(*device_up)(struct slim_device *sl);
+	int				(*device_down)(struct slim_device *sl);
+	int				(*reset_device)(struct slim_device *sl);
+
+	struct device_driver		driver;
+	const struct slim_device_id	*id_table;
+};
+#define to_slim_driver(d) container_of(d, struct slim_driver, driver)
+
+/*
+ * Client/device handle (struct slim_device):
+ * ------------------------------------------
+ *  This is the client/device handle returned when a slimbus
+ *  device is registered with a controller. This structure can be provided
+ *  during register_board_info, or can be allocated using slim_add_device API.
+ *  Pointer to this structure is used by client-driver as a handle.
+ *  @dev: Driver model representation of the device.
+ *  @name: Name of driver to use with this device.
+ *  @e_addr: Enumeration address of this device.
+ *  @driver: Device's driver. Pointer to access routines.
+ *  @ctrl: Slimbus controller managing the bus hosting this device.
+ *  @laddr: 1-byte Logical address of this device.
+ *  @reported: Flag to indicate whether this device reported present. The flag
+ *	is set when device reports present, and is reset when it reports
+ *	absent. This flag alongwith notified flag below is used to call
+ *	device_up, or device_down callbacks for driver of this device.
+ *  @notified: Flag to indicate whether this device has been notified. The
+ *	device may report present multiple times, but should be notified only
+ *	first time it has reported present.
+ *  @dev_list: List of devices on a controller
+ *  @wd: Work structure associated with workqueue for presence notification
+ */
+struct slim_device {
+	struct device		dev;
+	char		*name;
+	struct slim_eaddr	e_addr;
+	struct slim_driver	*driver;
+	struct slim_controller	*ctrl;
+	u8			laddr;
+	bool			reported;
+	bool			notified;
+	struct list_head	dev_list;
+	struct work_struct	wd;
+};
+#define to_slim_device(d) container_of(d, struct slim_device, dev)
+
+/*
+ * struct slim_boardinfo: Declare board info for Slimbus device bringup.
+ * @bus_num: Controller number (bus) on which this device will sit.
+ * @slim_slave: Device to be registered with slimbus.
+ */
+struct slim_boardinfo {
+	int			bus_num;
+	struct slim_device	*slim_slave;
+};
+
+/* Manager's logical address is set to 0xFF per spec */
+#define SLIM_LA_MANAGER 0xFF
+/*
+ * slim_get_logical_addr: Return the logical address of a slimbus device.
+ * @sb: client handle requesting the adddress.
+ * @e_addr: Enumeration address of the device.
+ * @laddr: output buffer to store the address
+ * context: can sleep
+ * -EINVAL is returned in case of invalid parameters, and -ENXIO is returned if
+ *  the device with this elemental address is not found.
+ */
+
+extern int slim_get_logical_addr(struct slim_device *sb,
+				struct slim_eaddr *e_addr, u8 *laddr);
+
+/*
+ * slim_driver_register: Client driver registration with slimbus
+ * @drv:Client driver to be associated with client-device.
+ * This API will register the client driver with the slimbus
+ * It is called from the driver's module-init function.
+ */
+extern int slim_driver_register(struct slim_driver *drv);
+
+/*
+ * slim_driver_unregister: Undo effects of slim_driver_register
+ * @drv: Client driver to be unregistered
+ */
+extern void slim_driver_unregister(struct slim_driver *drv);
+
+/*
+ * slim_add_numbered_controller: Controller bring-up.
+ * @ctrl: Controller to be registered.
+ * A controller is registered with the framework using this API. ctrl->nr is the
+ * desired number with which slimbus framework registers the controller.
+ * Function will return -EBUSY if the number is in use.
+ */
+extern int slim_add_numbered_controller(struct slim_controller *ctrl);
+
+/*
+ * slim_del_controller: Controller tear-down.
+ * Controller added with the above API is teared down using this API.
+ */
+extern int slim_del_controller(struct slim_controller *ctrl);
+
+/*
+ * slim_add_device: Add a new device without register board info.
+ * @ctrl: Controller to which this device is to be added to.
+ * Called when device doesn't have an explicit client-driver to be probed, or
+ * the client-driver is a module installed dynamically.
+ */
+extern int slim_add_device(struct slim_controller *ctrl,
+			struct slim_device *sbdev);
+
+/* slim_remove_device: Remove the effect of slim_add_device() */
+extern void slim_remove_device(struct slim_device *sbdev);
+
+/*
+ * slim_assign_laddr: Assign logical address to a device enumerated.
+ * @ctrl: Controller with which device is enumerated.
+ * @e_addr: Enumeration address of the device.
+ * @laddr: Return logical address (if valid flag is false)
+ * @valid: true if laddr holds a valid address that controller wants to
+ *	set for this enumeration address. Otherwise framework sets index into
+ *	address table as logical address.
+ * Called by controller in response to REPORT_PRESENT. Framework will assign
+ * a logical address to this enumeration address.
+ * Function returns -EXFULL to indicate that all logical addresses are already
+ * taken.
+ */
+extern int slim_assign_laddr(struct slim_controller *ctrl,
+			struct slim_eaddr *e_addr, u8 *laddr, bool valid);
+
+/*
+ * slim_report_absent: Controller calls this function when a device
+ *	reports absent, OR when the device cannot be communicated with
+ * @sbdev: Device that cannot be reached, or that sent report absent
+ */
+void slim_report_absent(struct slim_device *sbdev);
+
+/*
+ * slim_framer_booted: This function is called by controller after the active
+ * framer has booted (using Bus Reset sequence, or after it has shutdown and has
+ * come back up). Components, devices on the bus may be in undefined state,
+ * and this function triggers their drivers to do the needful
+ * to bring them back in Reset state so that they can acquire sync, report
+ * present and be operational again.
+ */
+void slim_framer_booted(struct slim_controller *ctrl);
+
+/*
+ * slim_ctrl_add_boarddevs: Add devices registered by board-info
+ * @ctrl: Controller to which these devices are to be added to.
+ * This API is called by controller when it is up and running.
+ * If devices on a controller were registered before controller,
+ * this will make sure that they get probed when controller is up
+ */
+extern void slim_ctrl_add_boarddevs(struct slim_controller *ctrl);
+
+/*
+ * slim_register_board_info: Board-initialization routine.
+ * @info: List of all devices on all controllers present on the board.
+ * @n: number of entries.
+ * API enumerates respective devices on corresponding controller.
+ * Called from board-init function.
+ */
+#ifdef CONFIG_SLIMBUS
+extern int slim_register_board_info(struct slim_boardinfo const *info,
+					unsigned n);
+#else
+static inline int slim_register_board_info(struct slim_boardinfo const *info,
+					unsigned n)
+{
+	return 0;
+}
+#endif
+
+static inline void *slim_get_ctrldata(const struct slim_controller *dev)
+{
+	return dev_get_drvdata(&dev->dev);
+}
+
+static inline void slim_set_ctrldata(struct slim_controller *dev, void *data)
+{
+	dev_set_drvdata(&dev->dev, data);
+}
+
+static inline void *slim_get_devicedata(const struct slim_device *dev)
+{
+	return dev_get_drvdata(&dev->dev);
+}
+
+static inline void slim_set_clientdata(struct slim_device *dev, void *data)
+{
+	dev_set_drvdata(&dev->dev, data);
+}
+
+#endif /* _LINUX_SLIMBUS_H */
-- 
1.8.2.1

[PATCH 2/3] of/slimbus: OF helper for SLIMbus

[Sagar Dharia]

OF helper routine scans the SLIMbus DeviceTree, allocates resources,
and creates slim_devices according to the hierarchy.

Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
---
 drivers/slimbus/slimbus.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/slimbus.h   | 19 ++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
index be4f2c7..f51b503 100644
--- a/drivers/slimbus/slimbus.c
+++ b/drivers/slimbus/slimbus.c
@@ -19,6 +19,7 @@
 #include <linux/idr.h>
 #include <linux/pm_runtime.h>
 #include <linux/slimbus.h>
+#include <linux/of.h>
 
 static DEFINE_MUTEX(slim_lock);
 static DEFINE_IDR(ctrl_idr);
@@ -761,6 +762,82 @@ int slim_get_logical_addr(struct slim_device *sb, struct slim_eaddr *e_addr,
 }
 EXPORT_SYMBOL(slim_get_logical_addr);
 
+#if IS_ENABLED(CONFIG_OF)
+/* OF helpers for SLIMbus */
+int of_register_slim_devices(struct slim_controller *ctrl)
+{
+	struct device_node *node;
+	struct slim_boardinfo *temp, *binfo = NULL;
+	int n = 0;
+	int ret = 0;
+
+	if (!ctrl->dev.of_node)
+		return -EINVAL;
+
+	for_each_child_of_node(ctrl->dev.of_node, node) {
+		struct property *prop;
+		u8 *ea;
+		struct slim_device *slim;
+		char *name;
+
+		prop = of_find_property(node, "enumeration-addr", NULL);
+		if (!prop || prop->length != 6) {
+			dev_err(&ctrl->dev, "of_slim: invalid E-addr");
+			continue;
+		}
+		ea = (u8 *)prop->value;
+		name = kcalloc(SLIMBUS_NAME_SIZE, sizeof(char), GFP_KERNEL);
+		if (!name) {
+			ret = -ENOMEM;
+			goto of_slim_err;
+		}
+		ret = of_modalias_node(node, name, SLIMBUS_NAME_SIZE);
+		if (ret < 0) {
+			dev_err(&ctrl->dev, "of_slim: modalias fail:%d on %s\n",
+				ret, node->full_name);
+			kfree(name);
+			continue;
+		}
+		slim = kcalloc(1, sizeof(struct slim_device), GFP_KERNEL);
+		if (!slim) {
+			ret = -ENOMEM;
+			kfree(name);
+			goto of_slim_err;
+		}
+		slim->e_addr.manf_id = (u16)(ea[5] << 8) | ea[4];
+		slim->e_addr.prod_code = (u16)(ea[3] << 8) | ea[2];
+		slim->e_addr.dev_index = ea[1];
+		slim->e_addr.instance = ea[0];
+
+
+		temp = krealloc(binfo, (n + 1) * sizeof(struct slim_boardinfo),
+					GFP_KERNEL);
+		if (!temp) {
+			kfree(name);
+			kfree(slim);
+			goto of_slim_err;
+		}
+		binfo = temp;
+		slim->dev.of_node = of_node_get(node);
+		slim->name = name;
+		binfo[n].bus_num = ctrl->nr;
+		binfo[n].slim_slave = slim;
+		n++;
+	}
+	return slim_register_board_info(binfo, n);
+
+of_slim_err:
+	n--;
+	while (n >= 0) {
+		kfree(binfo[n].slim_slave->name);
+		kfree(binfo[n].slim_slave);
+	}
+	kfree(binfo);
+	return ret;
+}
+EXPORT_SYMBOL(of_register_slim_devices);
+#endif
+
 MODULE_LICENSE("GPL v2");
 MODULE_VERSION("0.1");
 MODULE_DESCRIPTION("Slimbus module");
diff --git a/include/linux/slimbus.h b/include/linux/slimbus.h
index 05b7594..61b7c74 100644
--- a/include/linux/slimbus.h
+++ b/include/linux/slimbus.h
@@ -370,6 +370,25 @@ static inline int slim_register_board_info(struct slim_boardinfo const *info,
 }
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+/*
+ * of_slim_register_devices() - Register devices in the SLIMbus Device Tree
+ * @ctrl: slim_controller which devices should be registered to.
+ *
+ * This routine scans the SLIMbus Device Tree, allocating resources and
+ * creating slim_devices according to the SLIMbus Device Tree
+ * hierarchy.
+ * This routine is normally called from the probe routine of the driver
+ * registering as a slim_controller.
+ */
+extern int of_register_slim_devices(struct slim_controller *ctrl);
+#else
+static int of_register_slim_devices(struct slim_controller *ctrl)
+{
+	return 0;
+}
+#endif
+
 static inline void *slim_get_ctrldata(const struct slim_controller *dev)
 {
 	return dev_get_drvdata(&dev->dev);
-- 
1.8.2.1

[PATCH 3/3] slimbus: Add messaging APIs to slimbus framework

[Sagar Dharia]

Slimbus devices use value-element, and information elements to
control device parameters (e.g. value element is used to represent
gain for codec, information element is used to represent interrupt
status for codec when codec interrupt fires).
Messaging APIs are used to set/get these value and information
elements. Slimbus specification uses 8-bit "transaction IDs" for
messages where a read-value is anticipated. Framework uses a table
of pointers to store those TIDs and responds back to the caller in
O(1).
Caller can opt to do synchronous, or asynchronous reads/writes. For
asynchronous operations, the callback can be called from atomic
context.

Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
Tested-by: Naveen Kaje <nkaje@codeaurora.org>
---
 drivers/slimbus/slimbus.c | 273 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/slimbus.h   | 124 +++++++++++++++++++++
 2 files changed, 397 insertions(+)

diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
index f51b503..44614e1 100644
--- a/drivers/slimbus/slimbus.c
+++ b/drivers/slimbus/slimbus.c
@@ -26,6 +26,14 @@ static DEFINE_IDR(ctrl_idr);
 static struct device_type slim_dev_type;
 static struct device_type slim_ctrl_type;
 
+#define DEFINE_SLIM_LDEST_TXN(name, mc, rl, la, msg) \
+	struct slim_msg_txn name = { rl, 0, mc, SLIM_MSG_DEST_LOGICALADDR, 0,\
+					0, la, msg, }
+
+#define DEFINE_SLIM_BCAST_TXN(name, mc, rl, la, msg) \
+	struct slim_msg_txn name = { rl, 0, mc, SLIM_MSG_DEST_BROADCAST, 0,\
+					0, la, msg, }
+
 static int slim_compare_eaddr(struct slim_eaddr *a, struct slim_eaddr *b)
 {
 	if (a->manf_id == b->manf_id && a->prod_code == b->prod_code &&
@@ -762,6 +770,271 @@ int slim_get_logical_addr(struct slim_device *sb, struct slim_eaddr *e_addr,
 }
 EXPORT_SYMBOL(slim_get_logical_addr);
 
+/*
+ * slim_msg_response: Deliver Message response received from a device to the
+ *	framework.
+ * @ctrl: Controller handle
+ * @reply: Reply received from the device
+ * @len: Length of the reply
+ * @tid: Transaction ID received with which framework can associate reply.
+ * Called by controller to inform framework about the response received.
+ * This helps in making the API asynchronous, and controller-driver doesn't need
+ * to manage 1 more table other than the one managed by framework mapping TID
+ * with buffers
+ */
+void slim_msg_response(struct slim_controller *ctrl, u8 *reply, u8 tid, u8 len)
+{
+	struct slim_val_inf *msg;
+
+	spin_lock(&ctrl->txn_lock);
+	msg = ctrl->txnt[tid];
+	if (msg == NULL || msg->rbuf == NULL) {
+		spin_unlock(&ctrl->txn_lock);
+		dev_err(&ctrl->dev, "Got response to invalid TID:%d, len:%d",
+				tid, len);
+		return;
+	}
+	memcpy(msg->rbuf, reply, len);
+	ctrl->txnt[tid] = NULL;
+	if (msg->comp_cb)
+		msg->comp_cb(msg->ctx);
+	spin_unlock(&ctrl->txn_lock);
+}
+EXPORT_SYMBOL(slim_msg_response);
+
+static int slim_processtxn(struct slim_controller *ctrl,
+				struct slim_msg_txn *txn)
+{
+	int i = 0, ret = 0;
+	unsigned long flags;
+
+	if (txn->mt == SLIM_MSG_MT_CORE &&
+		(txn->mc == SLIM_MSG_MC_REQUEST_INFORMATION ||
+		 txn->mc == SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION ||
+		 txn->mc == SLIM_MSG_MC_REQUEST_VALUE ||
+		 txn->mc == SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION)) {
+		spin_lock_irqsave(&ctrl->txn_lock, flags);
+		for (i = 0; i < ctrl->last_tid; i++) {
+			if (ctrl->txnt[i] == NULL)
+				break;
+		}
+		if (i >= ctrl->last_tid) {
+			if (ctrl->last_tid == 255) {
+				spin_unlock_irqrestore(&ctrl->txn_lock, flags);
+				return -ENOMEM;
+			}
+			ctrl->last_tid++;
+		}
+		ctrl->txnt[i] = txn->msg;
+		txn->tid = i;
+		spin_unlock_irqrestore(&ctrl->txn_lock, flags);
+	}
+
+	ret = ctrl->xfer_msg(ctrl, txn);
+	return ret;
+}
+
+static int slim_val_inf_sanity(struct slim_val_inf *msg, int oper)
+{
+	if (!msg || msg->num_bytes > 16 ||
+		(msg->start_offset + msg->num_bytes) > 0xC00)
+		return -EINVAL;
+	switch (oper) {
+	case SLIM_MSG_MC_REQUEST_VALUE:
+	case SLIM_MSG_MC_REQUEST_INFORMATION:
+		if (msg->rbuf == NULL)
+			return -EINVAL;
+		return 0;
+	case SLIM_MSG_MC_CHANGE_VALUE:
+	case SLIM_MSG_MC_CLEAR_INFORMATION:
+		if (msg->wbuf == NULL)
+			return -EINVAL;
+		return 0;
+	case SLIM_MSG_MC_REQUEST_CHANGE_VALUE:
+	case SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION:
+		if (msg->rbuf == NULL || msg->wbuf == NULL)
+			return -EINVAL;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static u16 slim_slicecodefromsize(u16 req)
+{
+	static const u8 codetosize[8] = {1, 2, 3, 4, 6, 8, 12, 16};
+
+	if (req >= ARRAY_SIZE(codetosize))
+		return 0;
+	else
+		return codetosize[req];
+}
+
+static u16 slim_slicesize(int code)
+{
+	static const u8 sizetocode[16] = {
+		0, 1, 2, 3, 3, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7
+	};
+
+	clamp(code, 1, (int)ARRAY_SIZE(sizetocode));
+	return sizetocode[code - 1];
+}
+
+static void slim_sync_default_cb(void *ctx)
+{
+	struct completion *comp = ctx;
+
+	complete(comp);
+}
+
+static int slim_xfer_msg(struct slim_controller *ctrl,
+			struct slim_device *sbdev, struct slim_val_inf *msg,
+			u8 mc)
+{
+	DEFINE_SLIM_LDEST_TXN(txn_stack, mc, 6, sbdev->laddr, msg);
+	struct slim_msg_txn *txn = &txn_stack;
+	int ret;
+	unsigned long flags;
+	u16 sl, cur;
+	bool tid_txn, async = false;
+	DECLARE_COMPLETION_ONSTACK(complete);
+
+	ret = slim_val_inf_sanity(msg, mc);
+	if (ret) {
+		pr_err("Sanity check failed for msg:offset:0x%x, mc:%d",
+				msg->start_offset, mc);
+		return ret;
+	}
+
+	tid_txn = slim_tid_txn(mc);
+
+	sl = slim_slicesize(msg->num_bytes);
+	dev_err(&ctrl->dev, "SB xfer msg:os:%x, len:%d, MC:%x, sl:%x\n",
+				msg->start_offset, msg->num_bytes, mc, sl);
+
+	cur = slim_slicecodefromsize(sl);
+	txn->ec = ((sl | (1 << 3)) | ((msg->start_offset & 0xFFF) << 4));
+
+	if (!msg->comp_cb && tid_txn) {
+		msg->comp_cb = slim_sync_default_cb;
+		msg->ctx = &complete;
+	} else
+		async = true;
+
+	if (mc == SLIM_MSG_MC_REQUEST_CHANGE_VALUE ||
+		mc == SLIM_MSG_MC_CHANGE_VALUE ||
+		mc == SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION ||
+		mc == SLIM_MSG_MC_CLEAR_INFORMATION)
+		txn->rl += msg->num_bytes;
+	if (tid_txn)
+		txn->rl++;
+
+	ret = slim_processtxn(ctrl, txn);
+
+	/* sync read */
+	if (!ret && tid_txn && !async) {
+		ret = wait_for_completion_timeout(&complete, HZ);
+		if (!ret)
+			ret = -ETIMEDOUT;
+		else
+			ret = 0;
+	}
+
+	if (ret && tid_txn) {
+		spin_lock_irqsave(&ctrl->txn_lock, flags);
+		/* Invalidate the transaction */
+		ctrl->txnt[txn->tid] = NULL;
+		spin_unlock_irqrestore(&ctrl->txn_lock, flags);
+	}
+	if (ret)
+		pr_err("slimbus transfer error:%d:offset:0x%x, MC:%d", ret,
+					msg->start_offset, mc);
+	if (!async && tid_txn) {
+		msg->comp_cb = NULL;
+		msg->ctx = NULL;
+	}
+	return ret;
+}
+EXPORT_SYMBOL(slim_xfer_msg);
+
+/* Message APIs Unicast message APIs used by slimbus slave drivers */
+
+/*
+ * Message API access routines.
+ * @sb: client handle requesting elemental message reads, writes.
+ * @msg: Input structure for start-offset, number of bytes to read.
+ * context: can sleep
+ * Returns:
+ * -EINVAL: Invalid parameters
+ * -ETIMEDOUT: If controller could not complete the request. This may happen if
+ *  the bus lines are not clocked, controller is not powered-on, slave with
+ *  given address is not enumerated/responding.
+ */
+int slim_request_val_element(struct slim_device *sb,
+				struct slim_val_inf *msg)
+{
+	struct slim_controller *ctrl = sb->ctrl;
+
+	if (!ctrl)
+		return -EINVAL;
+	return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_REQUEST_VALUE);
+}
+EXPORT_SYMBOL(slim_request_val_element);
+
+int slim_request_inf_element(struct slim_device *sb,
+				struct slim_val_inf *msg)
+{
+	struct slim_controller *ctrl = sb->ctrl;
+
+	if (!ctrl)
+		return -EINVAL;
+	return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_REQUEST_INFORMATION);
+}
+EXPORT_SYMBOL(slim_request_inf_element);
+
+int slim_change_val_element(struct slim_device *sb, struct slim_val_inf *msg)
+{
+	struct slim_controller *ctrl = sb->ctrl;
+
+	if (!ctrl)
+		return -EINVAL;
+	return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_CHANGE_VALUE);
+}
+EXPORT_SYMBOL(slim_change_val_element);
+
+int slim_clear_inf_element(struct slim_device *sb, struct slim_val_inf *msg)
+{
+	struct slim_controller *ctrl = sb->ctrl;
+
+	if (!ctrl)
+		return -EINVAL;
+	return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_CLEAR_INFORMATION);
+}
+EXPORT_SYMBOL(slim_clear_inf_element);
+
+int slim_request_change_val_element(struct slim_device *sb,
+					struct slim_val_inf *msg)
+{
+	struct slim_controller *ctrl = sb->ctrl;
+
+	if (!ctrl)
+		return -EINVAL;
+	return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_REQUEST_CHANGE_VALUE);
+}
+EXPORT_SYMBOL(slim_request_change_val_element);
+
+int slim_request_clear_inf_element(struct slim_device *sb,
+					struct slim_val_inf *msg)
+{
+	struct slim_controller *ctrl = sb->ctrl;
+
+	if (!ctrl)
+		return -EINVAL;
+	return slim_xfer_msg(ctrl, sb, msg,
+					SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION);
+}
+EXPORT_SYMBOL(slim_request_clear_inf_element);
+
 #if IS_ENABLED(CONFIG_OF)
 /* OF helpers for SLIMbus */
 int of_register_slim_devices(struct slim_controller *ctrl)
diff --git a/include/linux/slimbus.h b/include/linux/slimbus.h
index 61b7c74..1d98c58 100644
--- a/include/linux/slimbus.h
+++ b/include/linux/slimbus.h
@@ -34,6 +34,7 @@ extern struct bus_type slimbus_type;
 #define SLIM_FRM_SLOTS_PER_SUPERFRAME	16
 #define SLIM_GDE_SLOTS_PER_SUPERFRAME	2
 
+#define SLIM_MAX_TXNS			256
 struct slim_controller;
 struct slim_device;
 
@@ -97,12 +98,67 @@ struct slim_addrt {
 #define SLIM_MSG_MC_ASSIGN_LOGICAL_ADDRESS       0x2
 #define SLIM_MSG_MC_REPORT_ABSENT                0xF
 
+/* Information Element management messages */
+#define SLIM_MSG_MC_REQUEST_INFORMATION          0x20
+#define SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION    0x21
+#define SLIM_MSG_MC_REPLY_INFORMATION            0x24
+#define SLIM_MSG_MC_CLEAR_INFORMATION            0x28
+#define SLIM_MSG_MC_REPORT_INFORMATION           0x29
+
+/* Value Element management messages */
+#define SLIM_MSG_MC_REQUEST_VALUE                0x60
+#define SLIM_MSG_MC_REQUEST_CHANGE_VALUE         0x61
+#define SLIM_MSG_MC_REPLY_VALUE                  0x64
+#define SLIM_MSG_MC_CHANGE_VALUE                 0x68
+
 /* Destination type Values */
 #define SLIM_MSG_DEST_LOGICALADDR	0
 #define SLIM_MSG_DEST_ENUMADDR		1
 #define	SLIM_MSG_DEST_BROADCAST		3
 
 /*
+ * struct slim_val_inf: Slimbus value or information element
+ * @start_offset: Specifies starting offset in information/value element map
+ * @num_bytes: upto 16. This ensures that the message will fit the slicesize
+ *		per slimbus spec
+ * @comp_cb: Callback if this read/write is asynchronous
+ * @ctx: Argument for comp_cb
+ */
+struct slim_val_inf {
+	u16			start_offset;
+	u8			num_bytes;
+	u8			*rbuf;
+	const u8		*wbuf;
+	void			(*comp_cb)(void *ctx);
+	void			*ctx;
+};
+
+/*
+ * struct slim_msg_txn: Message to be sent by the controller.
+ * This structure has packet header, payload and buffer to be filled (if any)
+ * @rl: Header field. remaining length.
+ * @mt: Header field. Message type.
+ * @mc: Header field. LSB is message code for type mt.
+ * @dt: Header field. Destination type.
+ * @ec: Element code. Used for elemental access APIs.
+ * @len: Length of payload. (excludes ec)
+ * @tid: Transaction ID. Used for messages expecting response.
+ *	(relevant for message-codes involving read operation)
+ * @la: Logical address of the device this message is going to.
+ *	(Not used when destination type is broadcast.)
+ * @msg: Elemental access message to be read/written
+ */
+struct slim_msg_txn {
+	u8			rl;
+	u8			mt;
+	u8			mc;
+	u8			dt;
+	u16			ec;
+	u8			tid;
+	u8			la;
+	struct slim_val_inf	*msg;
+};
+/*
  * struct slim_controller: Controls every instance of SLIMbus
  *				(similar to 'master' on SPI)
  *	'Manager device' is responsible for  device management, bandwidth
@@ -137,6 +193,9 @@ struct slim_addrt {
  * @num_dev: Number of active slimbus slaves on this bus
  * @devs: List of devices on this controller
  * @wq: Workqueue per controller used to notify devices when they report present
+ * @txnt: Table of transactions having transaction ID
+ * @txn_lock: Lock to protect table of transactions
+ * @last_tid: size of the table txnt (can't grow beyond 256 since TID is 8-bits)
  * @dev_released: completion used to signal when sysfs has released this
  *	controller so that it can be deleted during shutdown
  * @xfer_msg: Transfer a message on this controller (this can be a broadcast
@@ -163,7 +222,12 @@ struct slim_controller {
 	u8			num_dev;
 	struct list_head	devs;
 	struct workqueue_struct *wq;
+	struct slim_val_inf	*txnt[SLIM_MAX_TXNS];
+	u8			last_tid;
+	spinlock_t		txn_lock;
 	struct completion	dev_released;
+	int			(*xfer_msg)(struct slim_controller *ctrl,
+				struct slim_msg_txn *txn);
 	int			(*set_laddr)(struct slim_controller *ctrl,
 				struct slim_eaddr *ea, u8 laddr);
 	int			(*get_laddr)(struct slim_controller *ctrl,
@@ -409,4 +473,64 @@ static inline void slim_set_clientdata(struct slim_device *dev, void *data)
 	dev_set_drvdata(&dev->dev, data);
 }
 
+/* Message APIs Unicast message APIs used by slimbus slave drivers */
+
+/*
+ * Message API access routines for value elements.
+ * @sb: client handle requesting elemental message reads, writes.
+ * @msg: Input structure for start-offset, number of bytes to read.
+ * context: can sleep
+ * Returns:
+ * -EINVAL: Invalid parameters
+ * -ETIMEDOUT: If controller could not complete the request. This may happen if
+ *  the bus lines are not clocked, controller is not powered-on, slave with
+ *  given address is not enumerated/responding.
+ */
+extern int slim_request_val_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+extern int slim_change_val_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+extern int slim_request_change_val_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+
+
+/*
+ * Message API access routines for information elements.
+ * @sb: client handle requesting elemental message reads, writes.
+ * @msg: Input structure for start-offset, number of bytes to read
+ *	wbuf will contain information element(s) bit masks to be cleared.
+ *	rbuf will return what the information element value was
+ */
+
+extern int slim_request_inf_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+extern int slim_clear_inf_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+extern int slim_request_clear_inf_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+
+/*
+ * slim_msg_response: Deliver Message response received from a device to the
+ *	framework.
+ * @ctrl: Controller handle
+ * @reply: Reply received from the device
+ * @len: Length of the reply
+ * @tid: Transaction ID received with which framework can associate reply.
+ * Called by controller to inform framework about the response received.
+ * This helps in making the API asynchronous, and controller-driver doesn't need
+ * to manage 1 more table other than the one managed by framework mapping TID
+ * with buffers
+ */
+extern void slim_msg_response(struct slim_controller *ctrl, u8 *reply, u8 tid,
+				u8 len);
+
+/* end of message apis */
+
+static inline bool slim_tid_txn(u8 mc)
+{
+	return (mc == SLIM_MSG_MC_REQUEST_INFORMATION ||
+		mc == SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION ||
+		mc == SLIM_MSG_MC_REQUEST_VALUE ||
+		mc == SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION);
+}
 #endif /* _LINUX_SLIMBUS_H */
-- 
1.8.2.1

Re: [PATCH 0/3] Introduce framework for SLIMbus device drivers

[Greg KH]

On Sat, Jun 13, 2015 at 11:49:15PM -0600, Sagar Dharia wrote:
> SLIMbus (Serial Low Power Interchip Media Bus) is a specification
> developed by MIPI (Mobile Industry Processor Interface) alliance.
> SLIMbus is a 2-wire implementation, which is used to communicate with
> peripheral components like audio-codec.
> SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
> channels, and control channel. Control channel has messages to do
> device-enumeration, messages to send/receive control-data to/from
> slimbus devices, messages for port/channel management, and messages to
> do bandwidth allocation.
> Framework is introduced to support  multiple instances of the bus
> (1 controller per bus), and multiple slave devices per controller.
> SPI and I2C frameworks, and comments from last time when I submitted
> the patches were referred-to while working on this framework.
> 
> These patchsets introduce device-management, OF helpers, and messaging
> APIs for SLIMbus. Framework patches to do channel, port and bandwidth
> management are work-in-progress and will be sent out soon.
> 
> These patchsets were tested on Qualcomm Snapdragon processor board
> using a controller driver, and a slimbus-dev module. This driver and
> module will be sent out after the framework patches mentioned above.

I'd like to see the patches that use this infrastructure before
reviewing these as it's kind of pointless for us to accept these at this
point in time.

thanks,

greg k-h

Re: [PATCH 1/3] SLIMbus: Device management on SLIMbus

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

On Sat, Jun 13, 2015 at 11:49:16PM -0600, Sagar Dharia wrote:

> +static int slim_compare_eaddr(struct slim_eaddr *a, struct slim_eaddr *b)
> +{
> +	if (a->manf_id == b->manf_id && a->prod_code == b->prod_code &&
> +			a->dev_index == b->dev_index &&
> +			a->instance == b->instance)
> +		return 0;
> +	return -EIO;
> +}

-EIO?  That seems a bit random.

> +static int slim_pm_resume(struct device *dev)
> +{
> +	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +
> +	if (pm)
> +		return pm_generic_resume(dev);
> +	else
> +		return 0;
> +}

I'm really surprised this needs to be open coded, none of this
references anything Slimbus specific.  Why not fix the need for open
coding?

There's also a bunch of other ops like poweroff and freeze/thaw that
you're not doing anything with.

> +void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)
> +{

Why are these operations split?

> + * @reset_device: This callback is called after framer is booted.
> + *		Driver should do the needful to reset the device,
> + *		so that device acquires sync and be operational.

boot_device or init_device perhaps?  Reset sounds like it takes a
running device and resets it.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 2/3] of/slimbus: OF helper for SLIMbus

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 406 bytes --]

On Sat, Jun 13, 2015 at 11:49:17PM -0600, Sagar Dharia wrote:
> OF helper routine scans the SLIMbus DeviceTree, allocates resources,
> and creates slim_devices according to the hierarchy.

You've not provided any sort of binding documentation, that is a
requirement for any new bindings and is really helpful for reviewing as
without documentation we can't be 100% certain what the code is intended
to do!

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 3/3] slimbus: Add messaging APIs to slimbus framework

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2344 bytes --]

On Sat, Jun 13, 2015 at 11:49:18PM -0600, Sagar Dharia wrote:

> +	if (txn->mt == SLIM_MSG_MT_CORE &&
> +		(txn->mc == SLIM_MSG_MC_REQUEST_INFORMATION ||
> +		 txn->mc == SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION ||
> +		 txn->mc == SLIM_MSG_MC_REQUEST_VALUE ||
> +		 txn->mc == SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION)) {

The mc comparison here looks like you meant to write a switch
statement.

> +	ret = ctrl->xfer_msg(ctrl, txn);
> +	return ret;
> +}

No need for ret here.

> +static int slim_xfer_msg(struct slim_controller *ctrl,
> +			struct slim_device *sbdev, struct slim_val_inf *msg,
> +			u8 mc)
> +{
> +	DEFINE_SLIM_LDEST_TXN(txn_stack, mc, 6, sbdev->laddr, msg);
> +	struct slim_msg_txn *txn = &txn_stack;
> +	int ret;
> +	unsigned long flags;
> +	u16 sl, cur;
> +	bool tid_txn, async = false;
> +	DECLARE_COMPLETION_ONSTACK(complete);
> +
> +	ret = slim_val_inf_sanity(msg, mc);
> +	if (ret) {
> +		pr_err("Sanity check failed for msg:offset:0x%x, mc:%d",
> +				msg->start_offset, mc);

dev_err() seems better, and if you're going to print an error on this
why not move the error prints into the sanity check so someone seeing
the error message can tell what went wrong?

> +	sl = slim_slicesize(msg->num_bytes);
> +	dev_err(&ctrl->dev, "SB xfer msg:os:%x, len:%d, MC:%x, sl:%x\n",
> +				msg->start_offset, msg->num_bytes, mc, sl);

Looks like you left osme debug statements in here.

> +	if (!msg->comp_cb && tid_txn) {
> +		msg->comp_cb = slim_sync_default_cb;
> +		msg->ctx = &complete;
> +	} else
> +		async = true;

Coding style: if you have braces on one branch of an if they should be
on both.

> +	/* sync read */
> +	if (!ret && tid_txn && !async) {
> +		ret = wait_for_completion_timeout(&complete, HZ);
> +		if (!ret)
> +			ret = -ETIMEDOUT;
> +		else
> +			ret = 0;
> +	}

Are we sure that HZ is a good timeout here - might it be too short or
too long for some users?

> diff --git a/include/linux/slimbus.h b/include/linux/slimbus.h
> index 61b7c74..1d98c58 100644
> --- a/include/linux/slimbus.h
> +++ b/include/linux/slimbus.h
> @@ -34,6 +34,7 @@ extern struct bus_type slimbus_type;
>  #define SLIM_FRM_SLOTS_PER_SUPERFRAME	16
>  #define SLIM_GDE_SLOTS_PER_SUPERFRAME	2
>  
> +#define SLIM_MAX_TXNS			256

Where did this number come from?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 0/3] Introduce framework for SLIMbus device drivers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Sun, Jun 14, 2015 at 08:32:44AM -0700, Greg KH wrote:

> I'd like to see the patches that use this infrastructure before
> reviewing these as it's kind of pointless for us to accept these at this
> point in time.

It would be nice to see a controller driver at least.  I'm a bit more
relaxed about seeing client drivers, while obviously it would be good to
see them for most applications there's quite a bit of framework work to
be done in order to implement them usefully (and not just at the bus
level) - we need to cope with streaming data and some fun system designs
as well.  Getting the core bus in will help reduce the out of tree diff
which should help people collaborate on those extra bits of work, as an
out of tree patch stack Slimbus support gets too large.

Having this work all happening out of tree (and mostly in vendor kernels
as a result) has been causing issues for everyone trying to work on
this, let's try to pull people into working on mainline as much as we
can.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 1/3] SLIMbus: Device management on SLIMbus

[Sagar Dharia]

On 6/15/2015 4:54 AM, Mark Brown wrote:
> On Sat, Jun 13, 2015 at 11:49:16PM -0600, Sagar Dharia wrote:
Thanks for the comments. I will take care of most of the comments from 
you, and Joe and upload V2 shortly.
I will also include a controller driver this time.
>
>> +void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)
>> +{
> Why are these operations split?
Some slaves may choose to do expensive operations in their probes (or 
wait for logical address assignment). That will delay/block controller 
registration and further HW initialization of the controller.
I did not see any downside/side-effect in splitting them. I am open 
however to have them combined if that's the preferred way.

Thanks
Sagar

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 1/3] SLIMbus: Device management on SLIMbus

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

On Tue, Jun 16, 2015 at 09:22:31AM -0600, Sagar Dharia wrote:
> On 6/15/2015 4:54 AM, Mark Brown wrote:
> >On Sat, Jun 13, 2015 at 11:49:16PM -0600, Sagar Dharia wrote:

> >>+void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)
> >>+{

> >Why are these operations split?

> Some slaves may choose to do expensive operations in their probes (or wait
> for logical address assignment). That will delay/block controller
> registration and further HW initialization of the controller.
> I did not see any downside/side-effect in splitting them. I am open however
> to have them combined if that's the preferred way.

How does this resolve the issue?  As far as I can see it just shuffles
it around so any delay happens later (and possibly not at a convenient
time), possibly not helping if there's multiple controllers.  If devices
are doing excessively expensive things in their probe that seems like
something that we should fix in the drivers.  For LA assignment
shouldn't we be addressing that by having a callback when the LA is
assigned?  That will avoid single threading effects and is more like the
pattern for other buses.  Slimbus isn't particularly unique in this
regard.

Greg, the LA assignment thing here is an example of the issue is an
example of the pattern I raised a while ago (but never got round to
coding up handling of) where we have devices on an enumerable bus with
static registrations.  LA assignment is the end of the hotplug process
for a Slimbus device but most devices will need something doing to power
them up so they enumerate.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 1/3] SLIMbus: Device management on SLIMbus

[Sagar Dharia]

On 6/17/2015 5:45 AM, Mark Brown wrote:
> On Tue, Jun 16, 2015 at 09:22:31AM -0600, Sagar Dharia wrote:
>> On 6/15/2015 4:54 AM, Mark Brown wrote:
>>> On Sat, Jun 13, 2015 at 11:49:16PM -0600, Sagar Dharia wrote:
>>>> +void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)
>>>> +{
>>> Why are these operations split?
>> Some slaves may choose to do expensive operations in their probes (or wait
>> for logical address assignment). That will delay/block controller
>> registration and further HW initialization of the controller.
>> I did not see any downside/side-effect in splitting them. I am open however
>> to have them combined if that's the preferred way.
> How does this resolve the issue?  As far as I can see it just shuffles
> it around so any delay happens later (and possibly not at a convenient
> time), possibly not helping if there's multiple controllers.  If devices
> are doing excessively expensive things in their probe that seems like
> something that we should fix in the drivers.  For LA assignment
> shouldn't we be addressing that by having a callback when the LA is
> assigned?  That will avoid single threading effects and is more like the
> pattern for other buses.  Slimbus isn't particularly unique in this
> regard.
I agree that using the 'device_up' callback (it's called when the device 
has LA assignment) will make sure slaves can keep their probes non-blocking.
I was just trying to accommodate that blocking call by slaves and did 
not realize the downside you now mentioned about multiple-controllers
I will combine these in next patchset.
Thanks
Sagar
>
> Greg, the LA assignment thing here is an example of the issue is an
> example of the pattern I raised a while ago (but never got round to
> coding up handling of) where we have devices on an enumerable bus with
> static registrations.  LA assignment is the end of the hotplug process
> for a Slimbus device but most devices will need something doing to power
> them up so they enumerate.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 1/3] SLIMbus: Device management on SLIMbus

[Srinivas Kandagatla]

Hi Sagar,

On 14/06/15 06:49, Sagar Dharia wrote:
> SLIMbus (Serial Low Power Interchip Media Bus) is a specification
> developed by MIPI (Mobile Industry Processor Interface) alliance.
> SLIMbus is a 2-wire implementation, which is used to communicate with
> peripheral components like audio-codec.
> SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
> channels, and control channel. Control channel has messages to do
> device-enumeration, messages to send/receive control-data to/from
> slimbus devices, messages for port/channel management, and messages to
> do bandwidth allocation.
> The framework supports multiple instances of the bus (1 controller per
> bus), and multiple slave devices per controller.
>
> This patch does device enumeration, logical address assignment,
> informing device when the device reports present/absent etc.
> Reporting present may need the driver to do the needful (e.g. turning
> on voltage regulators powering the device). So probe is called
> if the device is added to board-info list for a controller.
> Additionally device is probed when it reports present if that device
> doesn't need any such steps mentioned above.
>
> Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
> ---
>   drivers/Kconfig                 |   2 +
>   drivers/Makefile                |   1 +
>   drivers/slimbus/Kconfig         |   9 +
>   drivers/slimbus/Makefile        |   4 +
>   drivers/slimbus/slimbus.c       | 767 ++++++++++++++++++++++++++++++++++++++++
>   include/linux/mod_devicetable.h |  13 +
>   include/linux/slimbus.h         | 393 ++++++++++++++++++++
>   7 files changed, 1189 insertions(+)
>   create mode 100644 drivers/slimbus/Kconfig
>   create mode 100644 drivers/slimbus/Makefile
>   create mode 100644 drivers/slimbus/slimbus.c
>   create mode 100644 include/linux/slimbus.h
>

Good to see the slimbus patches :-)

Can you also add patch to add MAINTAINERS to this?

I like to try these patches on APQ8064 or any upstream Qcom platform, Do 
you have other patches to test this on APQ8064 or any A family SOCs/ B 
family SOCs which have upstream support?

Also I keep getting lost as I start looking at code, Am missing 
understanding of how these exported functions are going to be used by 
consumers/clients/controllers?
It would help if
1> document which explains how these apis are supposed to be used.
2> split up this patch into small patches, so that you can get good 
review comments.

Pl ignore if i have repeated the same comments someone else commented on.

> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index c0cc96b..e39c969 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -182,4 +182,6 @@ source "drivers/thunderbolt/Kconfig"
>
>   source "drivers/android/Kconfig"
>
> +source "drivers/slimbus/Kconfig"
> +
>   endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 46d2554..37c1c88 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -74,6 +74,7 @@ obj-$(CONFIG_TARGET_CORE)	+= target/
>   obj-$(CONFIG_MTD)		+= mtd/
>   obj-$(CONFIG_SPI)		+= spi/
>   obj-$(CONFIG_SPMI)		+= spmi/
> +obj-$(CONFIG_SLIMBUS)		+= slimbus/
>   obj-y				+= hsi/
>   obj-y				+= net/
>   obj-$(CONFIG_ATM)		+= atm/
> diff --git a/drivers/slimbus/Kconfig b/drivers/slimbus/Kconfig
> new file mode 100644
> index 0000000..fb30497
> --- /dev/null
> +++ b/drivers/slimbus/Kconfig
> @@ -0,0 +1,9 @@
> +#
> +# SLIMBUS driver configuration
> +#
> +menuconfig SLIMBUS
> +	tristate "Slimbus support"
> +	help
> +	  Slimbus is standard interface between baseband and audio codec,
> +	  and other peripheral components in mobile terminals.
> +
For consistency reasons could you fix on single style of SLIMbus vs 
Slimbus string in comments or description.

> diff --git a/drivers/slimbus/Makefile b/drivers/slimbus/Makefile
> new file mode 100644
> index 0000000..05f53bc
> --- /dev/null
> +++ b/drivers/slimbus/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# Makefile for kernel slimbus framework.
> +#
> +obj-$(CONFIG_SLIMBUS)			+= slimbus.o
> diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
> new file mode 100644
> index 0000000..be4f2c7
> --- /dev/null
> +++ b/drivers/slimbus/slimbus.c
> @@ -0,0 +1,767 @@
> +/* Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/slab.h>
> +#include <linux/init.h>
> +#include <linux/gcd.h>
Do you need this?
> +#include <linux/completion.h>
> +#include <linux/idr.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slimbus.h>
> +
> +static DEFINE_MUTEX(slim_lock);
> +static DEFINE_IDR(ctrl_idr);
> +static struct device_type slim_dev_type;
> +static struct device_type slim_ctrl_type;
> +
> +static int slim_compare_eaddr(struct slim_eaddr *a, struct slim_eaddr *b)
> +{
> +	if (a->manf_id == b->manf_id && a->prod_code == b->prod_code &&
> +			a->dev_index == b->dev_index &&
> +			a->instance == b->instance)
> +		return 0;
> +	return -EIO;
A new line before return would make the code more readable, pl fix it in 
other instances too.

> +}
> +
> +static const struct slim_device_id *slim_match(const struct slim_device_id *id,
> +					const struct slim_device *slim_dev)
> +{
> +	while (id->manf_id != 0 || id->prod_code != 0) {
> +		if (id->manf_id == slim_dev->e_addr.manf_id &&
> +				id->prod_code == slim_dev->e_addr.prod_code &&
> +				id->dev_index == slim_dev->e_addr.dev_index)
> +			return id;
> +		id++;
> +	}
> +	return NULL;
> +}
> +
> +static int slim_device_match(struct device *dev, struct device_driver *driver)
> +{
> +	struct slim_device *slim_dev;
> +	struct slim_driver *drv = to_slim_driver(driver);
> +
> +	if (dev->type == &slim_dev_type)
> +		slim_dev = to_slim_device(dev);
> +	else
> +		return 0;
> +	if (drv->id_table)
Please use new lines where it makes things clear, not having new lines 
in instances like this makes the code look odd.

> +		return slim_match(drv->id_table, slim_dev) != NULL;
> +	return 0;
> +}
> +
...
> +
> +static void slim_device_shutdown(struct device *dev)
> +{
> +	struct slim_device *slim_dev;
> +	struct slim_driver *driver;
> +
> +	if (dev->type == &slim_dev_type)
> +		slim_dev = to_slim_device(dev);
> +	else
> +		return;
> +
> +	if (!dev->driver)
> +		return;
> +	driver = to_slim_driver(dev->driver);
> +	if (driver->shutdown)
> +		driver->shutdown(slim_dev);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +
> +static int slim_pm_suspend(struct device *dev)
> +{
> +	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +
Not sure why you need to do this, this is already in 
pm_generic_suspend/resume
> +	if (pm)
> +		return pm_generic_suspend(dev);
> +	else
> +		return 0;
You could just use return pm_generic_suspend(dev);
Or use directly.

	SET_SYSTEM_SLEEP_PM_OPS(pm_generic_suspend, pm_generic_resume)

> +}
> +
> +static int slim_pm_resume(struct device *dev)
> +{
> +	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +
> +	if (pm)
> +		return pm_generic_resume(dev);
> +	else
> +		return 0;
> +}
> +
> +#else
> +#define slim_pm_suspend		NULL
> +#define slim_pm_resume		NULL
> +#endif
> +
> +static const struct dev_pm_ops slimbus_pm = {
> +	.suspend = slim_pm_suspend,
> +	.resume = slim_pm_resume,
> +	SET_RUNTIME_PM_OPS(
> +		pm_generic_suspend,
> +		pm_generic_resume,
> +		NULL
> +		)
> +};
> +struct bus_type slimbus_type = {
> +	.name		= "slimbus",
> +	.match		= slim_device_match,
> +	.probe		= slim_device_probe,
> +	.remove		= slim_device_remove,
> +	.shutdown	= slim_device_shutdown,
> +	.pm		= &slimbus_pm,
> +};
> +EXPORT_SYMBOL(slimbus_type);
> +
This module is GPL v2. why the symbols are not marked as GPL?
> +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);
> +

Looks like this file is appended with multiple files..
Ideally init calls stay at the end of the file..

> +/*
> + * slim_driver_register: Client driver registration with slimbus
> + * @drv:Client driver to be associated with client-device.
> + * This API will register the client driver with the slimbus
> + * It is called from the driver's module-init function.
> + */
> +int slim_driver_register(struct slim_driver *drv)
> +{
> +	drv->driver.bus = &slimbus_type;
> +
> +	return driver_register(&drv->driver);
> +}
> +EXPORT_SYMBOL(slim_driver_register);
> +
> +/*
> + * slim_driver_unregister: Undo effect of slim_driver_register
> + * @drv: Client driver to be unregistered
> + */
> +void slim_driver_unregister(struct slim_driver *drv)
> +{
> +	if (drv)
> +		driver_unregister(&drv->driver);
> +}
> +
> +#define slim_ctrl_attr_gr NULL
Would prefer to see all the defines on top of the file.
> +
> +static void slim_ctrl_release(struct device *dev)
> +{
> +	struct slim_controller *ctrl = to_slim_controller(dev);
> +
> +	complete(&ctrl->dev_released);
> +}
> +
> +static struct device_type slim_ctrl_type = {
> +	.groups		= slim_ctrl_attr_gr,
> +	.release	= slim_ctrl_release,
> +};
> +
> +static struct slim_controller *slim_ctrl_get(struct slim_controller *ctrl)
> +{
> +	if (!ctrl || !get_device(&ctrl->dev))
> +		return NULL;
> +
> +	return ctrl;
> +}
> +
> +static void slim_ctrl_put(struct slim_controller *ctrl)
> +{
> +	if (ctrl)
> +		put_device(&ctrl->dev);
> +}
> +
> +#define slim_device_attr_gr NULL
> +#define slim_device_uevent NULL
Defines go on top.

I get a feeling that this file should be split into more files, so its 
easy to navigate.

> +static void slim_dev_release(struct device *dev)
> +{
> +	struct slim_device *sbdev = to_slim_device(dev);
> +
> +	slim_ctrl_put(sbdev->ctrl);
> +}
> +
> +static struct device_type slim_dev_type = {
> +	.groups		= slim_device_attr_gr,
> +	.uevent		= slim_device_uevent,
> +	.release	= slim_dev_release,
> +};
> +
> +static void slim_report(struct work_struct *work)
> +{
> +	struct slim_driver *sbdrv;
> +	struct slim_device *sbdev =
> +			container_of(work, struct slim_device, wd);
> +	if (!sbdev->dev.driver)
> +		return;
> +	/* check if device-up or down needs to be called */
> +	if ((!sbdev->reported && !sbdev->notified) ||
> +			(sbdev->reported && sbdev->notified))
> +		return;
> +
> +	sbdrv = to_slim_driver(sbdev->dev.driver);
> +	/*
> +	 * address no longer valid, means device reported absent, whereas
> +	 * address valid, means device reported present
> +	 */
> +	if (sbdev->notified && !sbdev->reported) {
> +		sbdev->notified = false;
> +		if (sbdrv->device_down)
> +			sbdrv->device_down(sbdev);
> +	} else if (!sbdev->notified && sbdev->reported) {
> +		sbdev->notified = true;
> +		if (sbdrv->device_up)
> +			sbdrv->device_up(sbdev);
> +	}
> +}
> +
> +/*
> + * slim_add_device: Add a new device without register board info.
> + * @ctrl: Controller to which this device is to be added to.
> + * Called when device doesn't have an explicit client-driver to be probed, or
> + * the client-driver is a module installed dynamically.
> + */
> +int slim_add_device(struct slim_controller *ctrl, struct slim_device *sbdev)
> +{
> +	sbdev->dev.bus = &slimbus_type;
> +	sbdev->dev.parent = ctrl->dev.parent;
> +	sbdev->dev.type = &slim_dev_type;
> +	sbdev->dev.driver = NULL;
> +	sbdev->ctrl = ctrl;
> +	slim_ctrl_get(ctrl);
> +	if (!sbdev->name) {
> +		sbdev->name = kcalloc(SLIMBUS_NAME_SIZE, sizeof(char),
> +					GFP_KERNEL);
> +		if (!sbdev->name)
> +			return -ENOMEM;
> +		snprintf(sbdev->name, SLIMBUS_NAME_SIZE, "0x%x:0x%x:0x%x:0x%x",
> +				sbdev->e_addr.manf_id, sbdev->e_addr.prod_code,
> +				sbdev->e_addr.dev_index,
> +				sbdev->e_addr.instance);
> +	}

kasprintf ?

> +	dev_dbg(&ctrl->dev, "adding device:%s", sbdev->name);
> +	dev_set_name(&sbdev->dev, "%s", sbdev->name);
> +	INIT_WORK(&sbdev->wd, slim_report);
> +	mutex_lock(&ctrl->m_ctrl);
> +	list_add_tail(&sbdev->dev_list, &ctrl->devs);
> +	mutex_unlock(&ctrl->m_ctrl);
> +	/* probe slave on this controller */
> +	return device_register(&sbdev->dev);
> +}
> +EXPORT_SYMBOL(slim_add_device);
> +
> +struct sbi_boardinfo {
> +	struct list_head	list;
> +	struct slim_boardinfo	board_info;
> +};
> +
> +static LIST_HEAD(board_list);
> +static LIST_HEAD(slim_ctrl_list);
> +static DEFINE_MUTEX(board_lock);
> +
> +/* If controller is not present, only add to boards list */
> +static void slim_match_ctrl_to_boardinfo(struct slim_controller *ctrl,
> +				struct slim_boardinfo *bi)
> +{
> +	int ret;
> +
> +	if (ctrl->nr != bi->bus_num)
> +		return;
> +
> +	ret = slim_add_device(ctrl, bi->slim_slave);
> +	if (ret != 0)
> +		dev_err(ctrl->dev.parent, "can't create new device %s, ret:%d",
> +			bi->slim_slave->name, ret);
> +}
> +
> +
> +/* slim_remove_device: Remove the effect of slim_add_device() */
> +void slim_remove_device(struct slim_device *sbdev)
> +{
> +	struct slim_controller *ctrl = sbdev->ctrl;
> +
> +	mutex_lock(&ctrl->m_ctrl);
> +	list_del_init(&sbdev->dev_list);
> +	mutex_unlock(&ctrl->m_ctrl);
> +	device_unregister(&sbdev->dev);
> +}
> +EXPORT_SYMBOL(slim_remove_device);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_VERSION("0.1");
> +MODULE_DESCRIPTION("Slimbus module");
> +MODULE_ALIAS("platform:slimbus");

> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 3bfd567..94abc09 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -427,6 +427,19 @@ struct spi_device_id {
>   	kernel_ulong_t driver_data;	/* Data private to the driver */
>   };
>
> +/* SLIMbus */
> +
> +#define SLIMBUS_NAME_SIZE	32
> +#define SLIMBUS_MODULE_PREFIX	"slim:"
> +
> +struct slim_device_id {
> +	__u16 manf_id, prod_code;
> +	__u8 dev_index, instance;
> +
> +	/* Data private to the driver */
> +	kernel_ulong_t driver_data;
> +};
> +
>   #define SPMI_NAME_SIZE	32
>   #define SPMI_MODULE_PREFIX "spmi:"
>
> diff --git a/include/linux/slimbus.h b/include/linux/slimbus.h
> new file mode 100644
> index 0000000..05b7594
> --- /dev/null
> +++ b/include/linux/slimbus.h
> @@ -0,0 +1,393 @@
> +/* Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef _LINUX_SLIMBUS_H
> +#define _LINUX_SLIMBUS_H
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/mod_devicetable.h>
> +
> +/*
> + * Interfaces between SLIMbus manager drivers, SLIMbus client drivers, and
> + * SLIMbus infrastructure.
> + */
> +
> +extern struct bus_type slimbus_type;
> +
> +/* Standard values per SLIMbus spec needed by controllers and devices */
> +#define SLIM_CL_PER_SUPERFRAME		6144
> +#define SLIM_CL_PER_SUPERFRAME_DIV8	(SLIM_CL_PER_SUPERFRAME >> 3)
> +#define SLIM_MAX_CLK_GEAR		10
> +#define SLIM_MIN_CLK_GEAR		1
> +#define SLIM_CL_PER_SL			4
> +#define SLIM_SL_PER_SUPERFRAME		(SLIM_CL_PER_SUPERFRAME >> 2)
> +#define SLIM_FRM_SLOTS_PER_SUPERFRAME	16
> +#define SLIM_GDE_SLOTS_PER_SUPERFRAME	2
> +
> +struct slim_controller;
> +struct slim_device;
> +
> +/*
> + * struct slim_eaddr: Enumeration address for a slimbus device
> + * @manf_id: Manufacturer Id for the device
> + * @prod_code: Product code
> + * @dev_index: Device index
> + * @instance: Instance value
> + */
> +struct slim_eaddr {
> +	u16 manf_id;
> +	u16 prod_code;
> +	u8 dev_index;
> +	u8 instance;
> +};
> +
> +/*
> + * struct slim_framer - Represents Slimbus framer.
> + * Every controller may have multiple framers. There is 1 active framer device
> + * responsible for clocking the bus.
> + * Manager is responsible for framer hand-over.
> + * @e_addr: Enumeration address of the framer.
> + * @rootfreq: Root Frequency at which the framer can run. This is maximum
> + *	frequency ('clock gear 10') at which the bus can operate.
> + * @superfreq: Superframes per root frequency. Every frame is 6144 bits.
> + */
> +struct slim_framer {
> +	struct slim_eaddr	e_addr;
> +	int			rootfreq;
> +	int			superfreq;
> +};
> +#define to_slim_framer(d) container_of(d, struct slim_framer, dev)
?? there is no dev in slim_framer ?
> +
> +/*
> + * struct slim_addrt: slimbus address used internally by the slimbus framework.
> + * @valid: If the device is present. Valid is set to false when device reports
> + *	absent.
> + * @eaddr: Enumeration address
> + * @laddr: It is possible that controller will set a predefined logical address
> + *	rather than the one assigned by framework. (i.e. logical address may
> + *	not be same as index into this table). This entry will store the
> + *	logical address value for this enumeration address.
> + */
> +struct slim_addrt {
> +	bool			valid;
> +	struct slim_eaddr	eaddr;
> +	u8			laddr;
> +};
> +
> +/* SLIMbus message types. Related to interpretation of message code. */
> +#define SLIM_MSG_MT_CORE			0x0
> +#define SLIM_MSG_MT_DEST_REFERRED_CLASS		0x1
> +#define SLIM_MSG_MT_DEST_REFERRED_USER		0x2
> +#define SLIM_MSG_MT_SRC_REFERRED_CLASS		0x5
> +#define SLIM_MSG_MT_SRC_REFERRED_USER		0x6
> +
> +/* SLIMbus core type Message Codes. */
> +/* Device management messages used by this framework */
> +#define SLIM_MSG_MC_REPORT_PRESENT               0x1
> +#define SLIM_MSG_MC_ASSIGN_LOGICAL_ADDRESS       0x2
> +#define SLIM_MSG_MC_REPORT_ABSENT                0xF
> +
> +/* Destination type Values */
> +#define SLIM_MSG_DEST_LOGICALADDR	0
> +#define SLIM_MSG_DEST_ENUMADDR		1
> +#define	SLIM_MSG_DEST_BROADCAST		3
> +
> +/*
> + * struct slim_controller: Controls every instance of SLIMbus
> + *				(similar to 'master' on SPI)
> + *	'Manager device' is responsible for  device management, bandwidth
> + *	allocation, channel setup, and port associations per channel.
> + *	Device management means Logical address assignment/removal based on
> + *	enumeration (report-present, report-absent) if a device.
> + *	Bandwidth allocation is done dynamically by the manager based on active
> + *	channels on the bus, message-bandwidth requests made by slimbus devices.
> + *	Based on current bandwidth usage, manager chooses a frequency to run
> + *	the bus at (in steps of 'clock-gear', 1 through 10, each clock gear
> + *	representing twice the frequency than the previous gear).
> + *	Manager is also responsible for entering (and exiting) low-power-mode
> + *	(known as 'clock pause').
> + *	Manager can do handover of framer if there are multiple framers on the
> + *	bus and a certain usecase warrants using certain framer to avoid keeping
> + *	previous framer being powered-on.
> + *
> + *	Controller here performs duties of the manager device, and 'interface
> + *	device'. Interface device is responsible for monitoring the bus and
> + *	reporting information such as loss-of-synchronization, data
> + *	slot-collision.
> + * @dev: Device interface to this driver
> + * @nr: Board-specific number identifier for this controller/bus
> + * @list: Link with other slimbus controllers
> + * @name: Name for this controller
> + * @min_cg: Minimum clock gear supported by this controller (default value: 1)
> + * @max_cg: Maximum clock gear supported by this controller (default value: 10)
> + * @clkgear: Current clock gear in which this bus is running
> + * @a_framer: Active framer which is clocking the bus managed by this controller
> + * @m_ctrl: Mutex protecting controller data structures
> + * @addrt: Logical address table
> + * @num_dev: Number of active slimbus slaves on this bus
> + * @devs: List of devices on this controller
> + * @wq: Workqueue per controller used to notify devices when they report present
> + * @dev_released: completion used to signal when sysfs has released this
> + *	controller so that it can be deleted during shutdown
> + * @xfer_msg: Transfer a message on this controller (this can be a broadcast
> + *	control/status message like data channel setup, or a unicast message
> + *	like value element read/write.
> + * @set_laddr: Setup logical address at laddr for the slave with elemental
> + *	address e_addr. Drivers implementing controller will be expected to
> + *	send unicast message to this device with its logical address.
> + * @get_laddr: It is possible that controller needs to set fixed logical
> + *	address table and get_laddr can be used in that case so that controller
> + *	can do this assignment.
> + */
> +struct slim_controller {
> +	struct device		dev;
> +	unsigned int		nr;
> +	struct list_head	list;
> +	char			name[SLIMBUS_NAME_SIZE];
> +	int			min_cg;
> +	int			max_cg;
> +	int			clkgear;
> +	struct slim_framer	*a_framer;
> +	struct mutex		m_ctrl;
> +	struct slim_addrt	*addrt;
> +	u8			num_dev;
> +	struct list_head	devs;
> +	struct workqueue_struct *wq;
> +	struct completion	dev_released;
> +	int			(*set_laddr)(struct slim_controller *ctrl,
> +				struct slim_eaddr *ea, u8 laddr);
> +	int			(*get_laddr)(struct slim_controller *ctrl,
> +				struct slim_eaddr *ea, u8 *laddr);
> +};
> +#define to_slim_controller(d) container_of(d, struct slim_controller, dev)
> +
> +/*
> + * struct slim_driver: Slimbus 'generic device' (slave) device driver
> + *				(similar to 'spi_device' on SPI)
> + * @probe: Binds this driver to a slimbus device.
> + * @remove: Unbinds this driver from the slimbus device.
> + * @shutdown: Standard shutdown callback used during powerdown/halt.
> + * @suspend: Standard suspend callback used during system suspend
> + * @resume: Standard resume callback used during system resume
> + * @device_up: This callback is called when the device reports present and
> + *		gets a logical address assigned to it
> + * @device_down: This callback is called when device reports absent, or the
> + *		bus goes down. Device will report present when bus is up and
> + *		device_up callback will be called again when that happens
> + * @reset_device: This callback is called after framer is booted.
> + *		Driver should do the needful to reset the device,
> + *		so that device acquires sync and be operational.
> + * @driver: Slimbus device drivers should initialize name and owner field of
> + *	this structure
> + * @id_table: List of slimbus devices supported by this driver
> + */
> +struct slim_driver {
> +	int				(*probe)(struct slim_device *sl);
> +	int				(*remove)(struct slim_device *sl);
> +	void				(*shutdown)(struct slim_device *sl);
> +	int				(*suspend)(struct slim_device *sl,
> +					pm_message_t pmesg);
> +	int				(*resume)(struct slim_device *sl);
> +	int				(*device_up)(struct slim_device *sl);
> +	int				(*device_down)(struct slim_device *sl);
> +	int				(*reset_device)(struct slim_device *sl);
> +
> +	struct device_driver		driver;
> +	const struct slim_device_id	*id_table;
> +};
> +#define to_slim_driver(d) container_of(d, struct slim_driver, driver)
> +
> +/*
> + * Client/device handle (struct slim_device):
> + * ------------------------------------------
> + *  This is the client/device handle returned when a slimbus
> + *  device is registered with a controller. This structure can be provided
> + *  during register_board_info, or can be allocated using slim_add_device API.
> + *  Pointer to this structure is used by client-driver as a handle.
> + *  @dev: Driver model representation of the device.
> + *  @name: Name of driver to use with this device.
> + *  @e_addr: Enumeration address of this device.
> + *  @driver: Device's driver. Pointer to access routines.
> + *  @ctrl: Slimbus controller managing the bus hosting this device.
> + *  @laddr: 1-byte Logical address of this device.
> + *  @reported: Flag to indicate whether this device reported present. The flag
> + *	is set when device reports present, and is reset when it reports
> + *	absent. This flag alongwith notified flag below is used to call
> + *	device_up, or device_down callbacks for driver of this device.
> + *  @notified: Flag to indicate whether this device has been notified. The
> + *	device may report present multiple times, but should be notified only
> + *	first time it has reported present.
> + *  @dev_list: List of devices on a controller
> + *  @wd: Work structure associated with workqueue for presence notification
> + */
> +struct slim_device {
> +	struct device		dev;
> +	char		*name;
> +	struct slim_eaddr	e_addr;
> +	struct slim_driver	*driver;
> +	struct slim_controller	*ctrl;
> +	u8			laddr;
> +	bool			reported;
> +	bool			notified;
> +	struct list_head	dev_list;
> +	struct work_struct	wd;
> +};
> +#define to_slim_device(d) container_of(d, struct slim_device, dev)
> +
> +/*
> + * struct slim_boardinfo: Declare board info for Slimbus device bringup.
> + * @bus_num: Controller number (bus) on which this device will sit.
> + * @slim_slave: Device to be registered with slimbus.
> + */
> +struct slim_boardinfo {
> +	int			bus_num;
> +	struct slim_device	*slim_slave;
> +};
> +
> +/* Manager's logical address is set to 0xFF per spec */
> +#define SLIM_LA_MANAGER 0xFF
> +/*
> + * slim_get_logical_addr: Return the logical address of a slimbus device.
> + * @sb: client handle requesting the adddress.
> + * @e_addr: Enumeration address of the device.
> + * @laddr: output buffer to store the address
> + * context: can sleep
> + * -EINVAL is returned in case of invalid parameters, and -ENXIO is returned if
> + *  the device with this elemental address is not found.
> + */

documentation should go to c-file..
> +
> +extern int slim_get_logical_addr(struct slim_device *sb,
> +				struct slim_eaddr *e_addr, u8 *laddr);
> +
> +/*
> + * slim_driver_register: Client driver registration with slimbus
> + * @drv:Client driver to be associated with client-device.
> + * This API will register the client driver with the slimbus
> + * It is called from the driver's module-init function.
> + */
> +extern int slim_driver_register(struct slim_driver *drv);
> +
> +/*
> + * slim_driver_unregister: Undo effects of slim_driver_register
> + * @drv: Client driver to be unregistered
> + */
> +extern void slim_driver_unregister(struct slim_driver *drv);
> +
> +/*
> + * slim_add_numbered_controller: Controller bring-up.
> + * @ctrl: Controller to be registered.
> + * A controller is registered with the framework using this API. ctrl->nr is the
> + * desired number with which slimbus framework registers the controller.
> + * Function will return -EBUSY if the number is in use.
> + */
> +extern int slim_add_numbered_controller(struct slim_controller *ctrl);
> +
> +/*
> + * slim_del_controller: Controller tear-down.
> + * Controller added with the above API is teared down using this API.
> + */
> +extern int slim_del_controller(struct slim_controller *ctrl);
> +
> +/*
> + * slim_add_device: Add a new device without register board info.
> + * @ctrl: Controller to which this device is to be added to.
> + * Called when device doesn't have an explicit client-driver to be probed, or
> + * the client-driver is a module installed dynamically.
> + */
> +extern int slim_add_device(struct slim_controller *ctrl,
> +			struct slim_device *sbdev);
> +
> +/* slim_remove_device: Remove the effect of slim_add_device() */
> +extern void slim_remove_device(struct slim_device *sbdev);
> +
> +/*
> + * slim_assign_laddr: Assign logical address to a device enumerated.
> + * @ctrl: Controller with which device is enumerated.
> + * @e_addr: Enumeration address of the device.
> + * @laddr: Return logical address (if valid flag is false)
> + * @valid: true if laddr holds a valid address that controller wants to
> + *	set for this enumeration address. Otherwise framework sets index into
> + *	address table as logical address.
> + * Called by controller in response to REPORT_PRESENT. Framework will assign
> + * a logical address to this enumeration address.
> + * Function returns -EXFULL to indicate that all logical addresses are already
> + * taken.
> + */
> +extern int slim_assign_laddr(struct slim_controller *ctrl,
> +			struct slim_eaddr *e_addr, u8 *laddr, bool valid);
> +
> +/*
> + * slim_report_absent: Controller calls this function when a device
> + *	reports absent, OR when the device cannot be communicated with
> + * @sbdev: Device that cannot be reached, or that sent report absent
> + */
> +void slim_report_absent(struct slim_device *sbdev);
> +
> +/*
> + * slim_framer_booted: This function is called by controller after the active
> + * framer has booted (using Bus Reset sequence, or after it has shutdown and has
> + * come back up). Components, devices on the bus may be in undefined state,
> + * and this function triggers their drivers to do the needful
> + * to bring them back in Reset state so that they can acquire sync, report
> + * present and be operational again.
> + */
> +void slim_framer_booted(struct slim_controller *ctrl);
> +
> +/*
> + * slim_ctrl_add_boarddevs: Add devices registered by board-info
> + * @ctrl: Controller to which these devices are to be added to.
> + * This API is called by controller when it is up and running.
> + * If devices on a controller were registered before controller,
> + * this will make sure that they get probed when controller is up
> + */
> +extern void slim_ctrl_add_boarddevs(struct slim_controller *ctrl);
> +
> +/*
> + * slim_register_board_info: Board-initialization routine.
> + * @info: List of all devices on all controllers present on the board.
> + * @n: number of entries.
> + * API enumerates respective devices on corresponding controller.
> + * Called from board-init function.
> + */
> +#ifdef CONFIG_SLIMBUS
> +extern int slim_register_board_info(struct slim_boardinfo const *info,
> +					unsigned n);
> +#else
> +static inline int slim_register_board_info(struct slim_boardinfo const *info,
> +					unsigned n)
> +{
> +	return 0;
Should'nt it return error code?

Also don't you want to add dummy functions for other functions?
--srini
> +}
> +#endif
> +
> +static inline void *slim_get_ctrldata(const struct slim_controller *dev)
> +{
> +	return dev_get_drvdata(&dev->dev);
> +}
> +
> +static inline void slim_set_ctrldata(struct slim_controller *dev, void *data)
> +{
> +	dev_set_drvdata(&dev->dev, data);
> +}
> +
> +static inline void *slim_get_devicedata(const struct slim_device *dev)
> +{
> +	return dev_get_drvdata(&dev->dev);
> +}
> +
> +static inline void slim_set_clientdata(struct slim_device *dev, void *data)
> +{
> +	dev_set_drvdata(&dev->dev, data);
> +}
> +
> +#endif /* _LINUX_SLIMBUS_H */
>

Re: [PATCH 1/3] SLIMbus: Device management on SLIMbus

[Sagar Dharia]

Hi Srini
Thank you for the review.
On 6/18/2015 3:23 PM, Srinivas Kandagatla wrote:
> Hi Sagar,
>
> On 14/06/15 06:49, Sagar Dharia wrote:
>> SLIMbus (Serial Low Power Interchip Media Bus) is a specification
>> developed by MIPI (Mobile Industry Processor Interface) alliance.
>> SLIMbus is a 2-wire implementation, which is used to communicate with
>> peripheral components like audio-codec.
>> SLIMbus uses Time-Division-Multiplexing to accommodate multiple data
>> channels, and control channel. Control channel has messages to do
>> device-enumeration, messages to send/receive control-data to/from
>> slimbus devices, messages for port/channel management, and messages to
>> do bandwidth allocation.
>> The framework supports multiple instances of the bus (1 controller per
>> bus), and multiple slave devices per controller.
>>
>> This patch does device enumeration, logical address assignment,
>> informing device when the device reports present/absent etc.
>> Reporting present may need the driver to do the needful (e.g. turning
>> on voltage regulators powering the device). So probe is called
>> if the device is added to board-info list for a controller.
>> Additionally device is probed when it reports present if that device
>> doesn't need any such steps mentioned above.
>>
>> Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
>> ---
>>   drivers/Kconfig                 |   2 +
>>   drivers/Makefile                |   1 +
>>   drivers/slimbus/Kconfig         |   9 +
>>   drivers/slimbus/Makefile        |   4 +
>>   drivers/slimbus/slimbus.c       | 767 
>> ++++++++++++++++++++++++++++++++++++++++
>>   include/linux/mod_devicetable.h |  13 +
>>   include/linux/slimbus.h         | 393 ++++++++++++++++++++
>>   7 files changed, 1189 insertions(+)
>>   create mode 100644 drivers/slimbus/Kconfig
>>   create mode 100644 drivers/slimbus/Makefile
>>   create mode 100644 drivers/slimbus/slimbus.c
>>   create mode 100644 include/linux/slimbus.h
>>
>
> Good to see the slimbus patches :-)
>
> Can you also add patch to add MAINTAINERS to this?
>
> I like to try these patches on APQ8064 or any upstream Qcom platform, 
> Do you have other patches to test this on APQ8064 or any A family 
> SOCs/ B family SOCs which have upstream support?

I am testing this IFC6410 (APQ8064 based platform). I've the test-ioctl 
module that's work-in-progress. But to do some device-management like 
logical address assignment, you don't really need it (only this 
framework, and controller should be okay) since slimbus HW components 
inside the SOC reports present, enumerates when it boots. The IOCTL 
module binds to one of those devices, and makes sure probe, device-up, 
remove happen as expected.
>
> Also I keep getting lost as I start looking at code, Am missing 
> understanding of how these exported functions are going to be used by 
> consumers/clients/controllers?
> It would help if
> 1> document which explains how these apis are supposed to be used.
> 2> split up this patch into small patches, so that you can get good 
> review comments.

I have already split the files and have uploaded only device-management 
in this patch. This can't really be further broken while providing some 
useful functionality. There are in-fact multiple other patches coming up 
as described in the cover-letter. I will try to provide documentation 
(call-flows etc) next week when I upload next set of patches, and take 
care of some of comments you have given below.
Thanks
Sagar

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

re: [PATCH 1/3] SLIMbus: Device management on SLIMbus

[Joe Perches]

> SLIMbus (Serial Low Power Interchip Media Bus) is a specification
> developed by MIPI (Mobile Industry Processor Interface) alliance.
> SLIMbus is a 2-wire implementation, which is used to communicate with
> peripheral components like audio-codec.

Perhaps these trivial style things could be considered

Use kernel-doc style /**
Argument alignment
Add newlines to logging formats
Convert if/else to if
Rename int slim_compare_eaddr to bool slim_eaddr_equal
---
 drivers/slimbus/slimbus.c | 152 ++++++++++++++++++++++------------------------
 include/linux/slimbus.h   |  72 ++++++++++++----------
 2 files changed, 111 insertions(+), 113 deletions(-)

diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
index be4f2c7..278e5a1 100644
--- a/drivers/slimbus/slimbus.c
+++ b/drivers/slimbus/slimbus.c
@@ -25,22 +25,21 @@ static DEFINE_IDR(ctrl_idr);
 static struct device_type slim_dev_type;
 static struct device_type slim_ctrl_type;
 
-static int slim_compare_eaddr(struct slim_eaddr *a, struct slim_eaddr *b)
+static bool slim_eaddr_equal(struct slim_eaddr *a, struct slim_eaddr *b)
 {
-	if (a->manf_id == b->manf_id && a->prod_code == b->prod_code &&
-			a->dev_index == b->dev_index &&
-			a->instance == b->instance)
-		return 0;
-	return -EIO;
+	return (a->manf_id == b->manf_id &&
+		a->prod_code == b->prod_code &&
+		a->dev_index == b->dev_index &&
+		a->instance == b->instance);
 }
 
-static const struct slim_device_id *slim_match(const struct slim_device_id *id,
-					const struct slim_device *slim_dev)
+static const struct slim_device_id *
+slim_match(const struct slim_device_id *id, const struct slim_device *slim_dev)
 {
 	while (id->manf_id != 0 || id->prod_code != 0) {
 		if (id->manf_id == slim_dev->e_addr.manf_id &&
-				id->prod_code == slim_dev->e_addr.prod_code &&
-				id->dev_index == slim_dev->e_addr.dev_index)
+		    id->prod_code == slim_dev->e_addr.prod_code &&
+		    id->dev_index == slim_dev->e_addr.dev_index)
 			return id;
 		id++;
 	}
@@ -52,10 +51,10 @@ static int slim_device_match(struct device *dev, struct device_driver *driver)
 	struct slim_device *slim_dev;
 	struct slim_driver *drv = to_slim_driver(driver);
 
-	if (dev->type == &slim_dev_type)
-		slim_dev = to_slim_device(dev);
-	else
+	if (dev->type != &slim_dev_type)
 		return 0;
+
+	slim_dev = to_slim_device(dev);
 	if (drv->id_table)
 		return slim_match(drv->id_table, slim_dev) != NULL;
 	return 0;
@@ -65,14 +64,13 @@ static int slim_device_probe(struct device *dev)
 {
 	struct slim_device	*slim_dev;
 	struct slim_driver	*driver;
-	struct slim_controller *ctrl;
+	struct slim_controller	*ctrl;
 	int status = 0;
 
-	if (dev->type == &slim_dev_type)
-		slim_dev = to_slim_device(dev);
-	else
+	if (dev->type != &slim_dev_type)
 		return -ENXIO;
 
+	slim_dev = to_slim_device(dev);
 	driver = to_slim_driver(dev->driver);
 	if (!driver->id_table)
 		return -ENODEV;
@@ -95,13 +93,13 @@ static int slim_device_remove(struct device *dev)
 	struct slim_driver *driver;
 	int status = 0;
 
-	if (dev->type == &slim_dev_type)
-		slim_dev = to_slim_device(dev);
-	else
+	if (dev->type != &slim_dev_type)
 		return -ENXIO;
 
 	if (!dev->driver)
 		return 0;
+
+	slim_dev = to_slim_device(dev);
 	driver = to_slim_driver(dev->driver);
 	if (driver->remove)
 		status = driver->remove(slim_dev);
@@ -117,13 +115,11 @@ static void slim_device_shutdown(struct device *dev)
 	struct slim_device *slim_dev;
 	struct slim_driver *driver;
 
-	if (dev->type == &slim_dev_type)
-		slim_dev = to_slim_device(dev);
-	else
+	if (dev->type != &slim_dev_type)
 		return;
-
 	if (!dev->driver)
 		return;
+	slim_dev = to_slim_device(dev);
 	driver = to_slim_driver(dev->driver);
 	if (driver->shutdown)
 		driver->shutdown(slim_dev);
@@ -135,20 +131,18 @@ static int slim_pm_suspend(struct device *dev)
 {
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
-	if (pm)
-		return pm_generic_suspend(dev);
-	else
+	if (!pm)
 		return 0;
+	return pm_generic_suspend(dev);
 }
 
 static int slim_pm_resume(struct device *dev)
 {
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
-	if (pm)
-		return pm_generic_resume(dev);
-	else
+	if (!pm)
 		return 0;
+	return pm_generic_resume(dev);
 }
 
 #else
@@ -159,12 +153,9 @@ static int slim_pm_resume(struct device *dev)
 static const struct dev_pm_ops slimbus_pm = {
 	.suspend = slim_pm_suspend,
 	.resume = slim_pm_resume,
-	SET_RUNTIME_PM_OPS(
-		pm_generic_suspend,
-		pm_generic_resume,
-		NULL
-		)
+	SET_RUNTIME_PM_OPS(pm_generic_suspend, pm_generic_resume, NULL)
 };
+
 struct bus_type slimbus_type = {
 	.name		= "slimbus",
 	.match		= slim_device_match,
@@ -257,13 +248,13 @@ static struct device_type slim_dev_type = {
 static void slim_report(struct work_struct *work)
 {
 	struct slim_driver *sbdrv;
-	struct slim_device *sbdev =
-			container_of(work, struct slim_device, wd);
+	struct slim_device *sbdev = container_of(work, struct slim_device, wd);
+
 	if (!sbdev->dev.driver)
 		return;
 	/* check if device-up or down needs to be called */
 	if ((!sbdev->reported && !sbdev->notified) ||
-			(sbdev->reported && sbdev->notified))
+	    (sbdev->reported && sbdev->notified))
 		return;
 
 	sbdrv = to_slim_driver(sbdev->dev.driver);
@@ -282,7 +273,7 @@ static void slim_report(struct work_struct *work)
 	}
 }
 
-/*
+/**
  * slim_add_device: Add a new device without register board info.
  * @ctrl: Controller to which this device is to be added to.
  * Called when device doesn't have an explicit client-driver to be probed, or
@@ -298,15 +289,15 @@ int slim_add_device(struct slim_controller *ctrl, struct slim_device *sbdev)
 	slim_ctrl_get(ctrl);
 	if (!sbdev->name) {
 		sbdev->name = kcalloc(SLIMBUS_NAME_SIZE, sizeof(char),
-					GFP_KERNEL);
+				      GFP_KERNEL);
 		if (!sbdev->name)
 			return -ENOMEM;
 		snprintf(sbdev->name, SLIMBUS_NAME_SIZE, "0x%x:0x%x:0x%x:0x%x",
-				sbdev->e_addr.manf_id, sbdev->e_addr.prod_code,
-				sbdev->e_addr.dev_index,
-				sbdev->e_addr.instance);
+			 sbdev->e_addr.manf_id, sbdev->e_addr.prod_code,
+			 sbdev->e_addr.dev_index,
+			 sbdev->e_addr.instance);
 	}
-	dev_dbg(&ctrl->dev, "adding device:%s", sbdev->name);
+	dev_dbg(&ctrl->dev, "adding device: %s\n", sbdev->name);
 	dev_set_name(&sbdev->dev, "%s", sbdev->name);
 	INIT_WORK(&sbdev->wd, slim_report);
 	mutex_lock(&ctrl->m_ctrl);
@@ -328,7 +319,7 @@ static DEFINE_MUTEX(board_lock);
 
 /* If controller is not present, only add to boards list */
 static void slim_match_ctrl_to_boardinfo(struct slim_controller *ctrl,
-				struct slim_boardinfo *bi)
+					 struct slim_boardinfo *bi)
 {
 	int ret;
 
@@ -337,7 +328,7 @@ static void slim_match_ctrl_to_boardinfo(struct slim_controller *ctrl,
 
 	ret = slim_add_device(ctrl, bi->slim_slave);
 	if (ret != 0)
-		dev_err(ctrl->dev.parent, "can't create new device %s, ret:%d",
+		dev_err(ctrl->dev.parent, "can't create new device %s, ret:%d\n",
 			bi->slim_slave->name, ret);
 }
 
@@ -371,7 +362,7 @@ int slim_register_board_info(struct slim_boardinfo const *info, unsigned n)
 }
 EXPORT_SYMBOL(slim_register_board_info);
 
-/*
+/**
  * slim_ctrl_add_boarddevs: Add devices registered by board-info
  * @ctrl: Controller to which these devices are to be added to.
  * This API is called by controller when it is up and running.
@@ -413,8 +404,8 @@ static int slim_register_controller(struct slim_controller *ctrl)
 	if (ret)
 		goto out_list;
 
-	dev_dbg(&ctrl->dev, "Bus [%s] registered:dev:%p\n", ctrl->name,
-							&ctrl->dev);
+	dev_dbg(&ctrl->dev, "Bus [%s] registered:dev:%p\n",
+		ctrl->name, &ctrl->dev);
 
 	INIT_LIST_HEAD(&ctrl->devs);
 	ctrl->wq = create_singlethread_workqueue(dev_name(&ctrl->dev));
@@ -445,15 +436,15 @@ void slim_remove_device(struct slim_device *sbdev)
 EXPORT_SYMBOL(slim_remove_device);
 
 static void slim_ctrl_remove_device(struct slim_controller *ctrl,
-				struct slim_boardinfo *bi)
+				    struct slim_boardinfo *bi)
 {
 	if (ctrl->nr == bi->bus_num)
 		slim_remove_device(bi->slim_slave);
 }
 
-/*
+/**
  * slim_del_controller: Controller tear-down.
- * Controller added with the above API is teared down using this API.
+ * @ctrl: Controller to tear-down.
  */
 int slim_del_controller(struct slim_controller *ctrl)
 {
@@ -488,7 +479,7 @@ int slim_del_controller(struct slim_controller *ctrl)
 }
 EXPORT_SYMBOL(slim_del_controller);
 
-/*
+/**
  * slim_add_numbered_controller: Controller bring-up.
  * @ctrl: Controller to be registered.
  * A controller is registered with the framework using this API. ctrl->nr is the
@@ -511,7 +502,7 @@ int slim_add_numbered_controller(struct slim_controller *ctrl)
 }
 EXPORT_SYMBOL(slim_add_numbered_controller);
 
-/*
+/**
  * slim_report_absent: Controller calls this function when a device
  *	reports absent, OR when the device cannot be communicated with
  * @sbdev: Device that cannot be reached, or sent report absent
@@ -538,13 +529,14 @@ void slim_report_absent(struct slim_device *sbdev)
 }
 EXPORT_SYMBOL(slim_report_absent);
 
-/*
+/**
  * slim_framer_booted: This function is called by controller after the active
  * framer has booted (using Bus Reset sequence, or after it has shutdown and has
- * come back up). Components, devices on the bus may be in undefined state,
- * and this function triggers their drivers to do the needful
- * to bring them back in Reset state so that they can acquire sync, report
- * present and be operational again.
+ * come back up).
+ * @ctrl: Controller
+ * Components, devices on the bus may be in undefined state, and this function
+ * triggers their drivers to do the needful to bring them back in Reset state
+ * so that they can acquire sync, report present and be operational again.
  */
 void slim_framer_booted(struct slim_controller *ctrl)
 {
@@ -570,7 +562,7 @@ void slim_framer_booted(struct slim_controller *ctrl)
 }
 EXPORT_SYMBOL(slim_framer_booted);
 
-/*
+/**
  * slim_query_device: Query and get handle to a device.
  * @ctrl: Controller on which this device will be added/queried
  * @e_addr: Enumeration address of the device to be queried
@@ -578,7 +570,7 @@ EXPORT_SYMBOL(slim_framer_booted);
  * device and returns pointer to it if the device has not yet enumerated.
  */
 struct slim_device *slim_query_device(struct slim_controller *ctrl,
-					struct slim_eaddr *e_addr)
+				      struct slim_eaddr *e_addr)
 {
 	struct slim_device *slim = NULL;
 	struct sbi_boardinfo *bi;
@@ -588,8 +580,8 @@ struct slim_device *slim_query_device(struct slim_controller *ctrl,
 	list_for_each_entry(bi, &board_list, list) {
 		if (bi->board_info.bus_num != ctrl->nr)
 			continue;
-		if (slim_compare_eaddr(&bi->board_info.slim_slave->e_addr,
-				e_addr) == 0) {
+		if (slim_eaddr_equal(&bi->board_info.slim_slave->e_addr,
+				     e_addr)) {
 			slim = bi->board_info.slim_slave;
 			break;
 		}
@@ -600,14 +592,14 @@ struct slim_device *slim_query_device(struct slim_controller *ctrl,
 
 	mutex_lock(&ctrl->m_ctrl);
 	list_for_each_entry(slim, &ctrl->devs, dev_list) {
-		if (slim_compare_eaddr(&slim->e_addr, e_addr) == 0) {
+		if (slim_eaddr_equal(&slim->e_addr, e_addr)) {
 			mutex_unlock(&ctrl->m_ctrl);
 			return slim;
 		}
 	}
 	mutex_unlock(&ctrl->m_ctrl);
 
-	slim = kcalloc(1, sizeof(struct slim_device), GFP_KERNEL);
+	slim = kzalloc(sizeof(struct slim_device), GFP_KERNEL);
 	if (IS_ERR(slim))
 		return NULL;
 	slim->e_addr = *e_addr;
@@ -620,13 +612,13 @@ struct slim_device *slim_query_device(struct slim_controller *ctrl,
 EXPORT_SYMBOL(slim_query_device);
 
 static int ctrl_getaddr_entry(struct slim_controller *ctrl,
-				struct slim_eaddr *eaddr, u8 *entry)
+			      struct slim_eaddr *eaddr, u8 *entry)
 {
 	int i;
 
 	for (i = 0; i < ctrl->num_dev; i++) {
 		if (ctrl->addrt[i].valid &&
-		slim_compare_eaddr(&ctrl->addrt[i].eaddr, eaddr) == 0) {
+		    slim_eaddr_equal(&ctrl->addrt[i].eaddr, eaddr)) {
 			*entry = i;
 			return 0;
 		}
@@ -634,21 +626,21 @@ static int ctrl_getaddr_entry(struct slim_controller *ctrl,
 	return -ENXIO;
 }
 
-/*
+/**
  * slim_assign_laddr: Assign logical address to a device enumerated.
  * @ctrl: Controller with which device is enumerated.
  * @e_addr: Enumeration address of the device.
-  * @laddr: Return logical address (if valid flag is false)
-  * @valid: true if laddr holds a valid address that controller wants to
-  *	set for this enumeration address. Otherwise framework sets index into
-  *	address table as logical address.
+ * @laddr: Return logical address (if valid flag is false)
+ * @valid: true if laddr holds a valid address that controller wants to
+ *	set for this enumeration address. Otherwise framework sets index into
+ *	address table as logical address.
  * Called by controller in response to REPORT_PRESENT. Framework will assign
  * a logical address to this enumeration address.
  * Function returns -EXFULL to indicate that all logical addresses are already
  * taken.
  */
 int slim_assign_laddr(struct slim_controller *ctrl, struct slim_eaddr *e_addr,
-				u8 *laddr, bool valid)
+		      u8 *laddr, bool valid)
 {
 	int ret;
 	u8 i = 0;
@@ -701,9 +693,9 @@ ret_assigned_laddr:
 	if (exists || ret)
 		return ret;
 
-	pr_info("setting slimbus l-addr:%x, ea:%x,%x,%x,%x",
-				*laddr, e_addr->manf_id, e_addr->prod_code,
-				e_addr->dev_index, e_addr->instance);
+	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);
 	/*
 	 * Add this device to list of devices on this controller if it's
 	 * not already present
@@ -727,17 +719,17 @@ ret_assigned_laddr:
 }
 EXPORT_SYMBOL(slim_assign_laddr);
 
-/*
+/**
  * slim_get_logical_addr: Return the logical address of a slimbus device.
  * @sb: client handle requesting the adddress.
  * @e_addr: Enumeration address of the device.
  * @laddr: output buffer to store the address
  * context: can sleep
- * -EINVAL is returned in case of invalid parameters, and -ENXIO is returned if
- *  the device with this enumeration address is not found.
+ * -EINVAL is returned in case of invalid parameters
+ * -ENXIO is returned if the device with this enumeration address is not found
  */
 int slim_get_logical_addr(struct slim_device *sb, struct slim_eaddr *e_addr,
-				u8 *laddr)
+			  u8 *laddr)
 {
 	int ret;
 	u8 entry;
diff --git a/include/linux/slimbus.h b/include/linux/slimbus.h
index 05b7594..ed93da0 100644
--- a/include/linux/slimbus.h
+++ b/include/linux/slimbus.h
@@ -37,7 +37,7 @@ extern struct bus_type slimbus_type;
 struct slim_controller;
 struct slim_device;
 
-/*
+/**
  * struct slim_eaddr: Enumeration address for a slimbus device
  * @manf_id: Manufacturer Id for the device
  * @prod_code: Product code
@@ -51,7 +51,7 @@ struct slim_eaddr {
 	u8 instance;
 };
 
-/*
+/**
  * struct slim_framer - Represents Slimbus framer.
  * Every controller may have multiple framers. There is 1 active framer device
  * responsible for clocking the bus.
@@ -66,9 +66,10 @@ struct slim_framer {
 	int			rootfreq;
 	int			superfreq;
 };
+
 #define to_slim_framer(d) container_of(d, struct slim_framer, dev)
 
-/*
+/**
  * struct slim_addrt: slimbus address used internally by the slimbus framework.
  * @valid: If the device is present. Valid is set to false when device reports
  *	absent.
@@ -102,7 +103,7 @@ struct slim_addrt {
 #define SLIM_MSG_DEST_ENUMADDR		1
 #define	SLIM_MSG_DEST_BROADCAST		3
 
-/*
+/**
  * struct slim_controller: Controls every instance of SLIMbus
  *				(similar to 'master' on SPI)
  *	'Manager device' is responsible for  device management, bandwidth
@@ -165,13 +166,14 @@ struct slim_controller {
 	struct workqueue_struct *wq;
 	struct completion	dev_released;
 	int			(*set_laddr)(struct slim_controller *ctrl,
-				struct slim_eaddr *ea, u8 laddr);
+					     struct slim_eaddr *ea, u8 laddr);
 	int			(*get_laddr)(struct slim_controller *ctrl,
-				struct slim_eaddr *ea, u8 *laddr);
+					     struct slim_eaddr *ea, u8 *laddr);
 };
+
 #define to_slim_controller(d) container_of(d, struct slim_controller, dev)
 
-/*
+/**
  * struct slim_driver: Slimbus 'generic device' (slave) device driver
  *				(similar to 'spi_device' on SPI)
  * @probe: Binds this driver to a slimbus device.
@@ -196,7 +198,7 @@ struct slim_driver {
 	int				(*remove)(struct slim_device *sl);
 	void				(*shutdown)(struct slim_device *sl);
 	int				(*suspend)(struct slim_device *sl,
-					pm_message_t pmesg);
+						   pm_message_t pmesg);
 	int				(*resume)(struct slim_device *sl);
 	int				(*device_up)(struct slim_device *sl);
 	int				(*device_down)(struct slim_device *sl);
@@ -205,9 +207,10 @@ struct slim_driver {
 	struct device_driver		driver;
 	const struct slim_device_id	*id_table;
 };
+
 #define to_slim_driver(d) container_of(d, struct slim_driver, driver)
 
-/*
+/**
  * Client/device handle (struct slim_device):
  * ------------------------------------------
  *  This is the client/device handle returned when a slimbus
@@ -242,9 +245,10 @@ struct slim_device {
 	struct list_head	dev_list;
 	struct work_struct	wd;
 };
+
 #define to_slim_device(d) container_of(d, struct slim_device, dev)
 
-/*
+/**
  * struct slim_boardinfo: Declare board info for Slimbus device bringup.
  * @bus_num: Controller number (bus) on which this device will sit.
  * @slim_slave: Device to be registered with slimbus.
@@ -256,7 +260,7 @@ struct slim_boardinfo {
 
 /* Manager's logical address is set to 0xFF per spec */
 #define SLIM_LA_MANAGER 0xFF
-/*
+/**
  * slim_get_logical_addr: Return the logical address of a slimbus device.
  * @sb: client handle requesting the adddress.
  * @e_addr: Enumeration address of the device.
@@ -266,51 +270,53 @@ struct slim_boardinfo {
  *  the device with this elemental address is not found.
  */
 
-extern int slim_get_logical_addr(struct slim_device *sb,
-				struct slim_eaddr *e_addr, u8 *laddr);
+int slim_get_logical_addr(struct slim_device *sb,
+			  struct slim_eaddr *e_addr, u8 *laddr);
 
-/*
+/**
  * slim_driver_register: Client driver registration with slimbus
  * @drv:Client driver to be associated with client-device.
  * This API will register the client driver with the slimbus
  * It is called from the driver's module-init function.
  */
-extern int slim_driver_register(struct slim_driver *drv);
+int slim_driver_register(struct slim_driver *drv);
 
 /*
  * slim_driver_unregister: Undo effects of slim_driver_register
  * @drv: Client driver to be unregistered
  */
-extern void slim_driver_unregister(struct slim_driver *drv);
+void slim_driver_unregister(struct slim_driver *drv);
 
-/*
+/**
  * slim_add_numbered_controller: Controller bring-up.
  * @ctrl: Controller to be registered.
  * A controller is registered with the framework using this API. ctrl->nr is the
  * desired number with which slimbus framework registers the controller.
  * Function will return -EBUSY if the number is in use.
  */
-extern int slim_add_numbered_controller(struct slim_controller *ctrl);
+int slim_add_numbered_controller(struct slim_controller *ctrl);
 
 /*
  * slim_del_controller: Controller tear-down.
+ * @ctrl: Controller to be torn-down.
  * Controller added with the above API is teared down using this API.
  */
-extern int slim_del_controller(struct slim_controller *ctrl);
+int slim_del_controller(struct slim_controller *ctrl);
 
-/*
+/**
  * slim_add_device: Add a new device without register board info.
  * @ctrl: Controller to which this device is to be added to.
+ * @sbdev: slim_device to add
  * Called when device doesn't have an explicit client-driver to be probed, or
  * the client-driver is a module installed dynamically.
  */
-extern int slim_add_device(struct slim_controller *ctrl,
-			struct slim_device *sbdev);
+int slim_add_device(struct slim_controller *ctrl,
+		    struct slim_device *sbdev);
 
 /* slim_remove_device: Remove the effect of slim_add_device() */
-extern void slim_remove_device(struct slim_device *sbdev);
+void slim_remove_device(struct slim_device *sbdev);
 
-/*
+/**
  * slim_assign_laddr: Assign logical address to a device enumerated.
  * @ctrl: Controller with which device is enumerated.
  * @e_addr: Enumeration address of the device.
@@ -323,10 +329,10 @@ extern void slim_remove_device(struct slim_device *sbdev);
  * Function returns -EXFULL to indicate that all logical addresses are already
  * taken.
  */
-extern int slim_assign_laddr(struct slim_controller *ctrl,
-			struct slim_eaddr *e_addr, u8 *laddr, bool valid);
+int slim_assign_laddr(struct slim_controller *ctrl,
+		      struct slim_eaddr *e_addr, u8 *laddr, bool valid);
 
-/*
+/**
  * slim_report_absent: Controller calls this function when a device
  *	reports absent, OR when the device cannot be communicated with
  * @sbdev: Device that cannot be reached, or that sent report absent
@@ -343,16 +349,16 @@ void slim_report_absent(struct slim_device *sbdev);
  */
 void slim_framer_booted(struct slim_controller *ctrl);
 
-/*
+/**
  * slim_ctrl_add_boarddevs: Add devices registered by board-info
  * @ctrl: Controller to which these devices are to be added to.
  * This API is called by controller when it is up and running.
  * If devices on a controller were registered before controller,
  * this will make sure that they get probed when controller is up
  */
-extern void slim_ctrl_add_boarddevs(struct slim_controller *ctrl);
+void slim_ctrl_add_boarddevs(struct slim_controller *ctrl);
 
-/*
+/**
  * slim_register_board_info: Board-initialization routine.
  * @info: List of all devices on all controllers present on the board.
  * @n: number of entries.
@@ -360,11 +366,11 @@ extern void slim_ctrl_add_boarddevs(struct slim_controller *ctrl);
  * Called from board-init function.
  */
 #ifdef CONFIG_SLIMBUS
-extern int slim_register_board_info(struct slim_boardinfo const *info,
-					unsigned n);
+int slim_register_board_info(struct slim_boardinfo const *info,
+			     unsigned n);
 #else
 static inline int slim_register_board_info(struct slim_boardinfo const *info,
-					unsigned n)
+					   unsigned n)
 {
 	return 0;
 }