linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 0/6] Introduce framework for SLIMbus device drivers
@ 2015-06-17  1:45 Sagar Dharia
  2015-06-17  1:45 ` [PATCH V2 1/6] SLIMbus: Device management on SLIMbus Sagar Dharia
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Sagar Dharia @ 2015-06-17  1:45 UTC (permalink / raw)
  To: gregkh, bp, poeschel, sdharia, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, joe, davem, james.hogan,
	michael.opdenacker, daniel.thompson, linux-kernel
  Cc: nkaje, kheitke, mlocke, agross, linux-arm-msm

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, and clock-pause feature for entering/exiting low-power mode 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.


Changes from V1 to V2:
* Addressed inline-code review comments from Joe and Mark Brown.
* Added initial binding document for slimbus
* Added initial version of Qualcomm controller driver and initial
  binding document for it.
* Added clock-pause feature for entering/exiting low power mode
* Added runtime-pm to Qualcomm controller driver

Sagar Dharia (6):
  SLIMbus: Device management on SLIMbus
  of/slimbus: OF helper for SLIMbus
  slimbus: Add messaging APIs to slimbus framework
  slim: qcom: Add Qualcomm Slimbus controller driver
  slimbus: Add support for 'clock-pause' feature
  slim: qcom: Add runtime-pm support using clock-pause feature

 Documentation/devicetree/bindings/slimbus/bus.txt  |   34 +
 .../devicetree/bindings/slimbus/slim-qcom-ctrl.txt |   45 +
 drivers/Kconfig                                    |    2 +
 drivers/Makefile                                   |    1 +
 drivers/slimbus/Kconfig                            |   19 +
 drivers/slimbus/Makefile                           |    6 +
 drivers/slimbus/slim-qcom-ctrl.c                   |  852 ++++++++++++++
 drivers/slimbus/slim-qcom.c                        |   96 ++
 drivers/slimbus/slim-qcom.h                        |   90 ++
 drivers/slimbus/slimbus.c                          | 1174 ++++++++++++++++++++
 include/linux/mod_devicetable.h                    |   13 +
 include/linux/slimbus.h                            |  589 ++++++++++
 12 files changed, 2921 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/slimbus/bus.txt
 create mode 100644 Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt
 create mode 100644 drivers/slimbus/Kconfig
 create mode 100644 drivers/slimbus/Makefile
 create mode 100644 drivers/slimbus/slim-qcom-ctrl.c
 create mode 100644 drivers/slimbus/slim-qcom.c
 create mode 100644 drivers/slimbus/slim-qcom.h
 create mode 100644 drivers/slimbus/slimbus.c
 create mode 100644 include/linux/slimbus.h

-- 
1.8.2.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH V2 1/6] SLIMbus: Device management on SLIMbus
  2015-06-17  1:45 [PATCH V2 0/6] Introduce framework for SLIMbus device drivers Sagar Dharia
@ 2015-06-17  1:45 ` Sagar Dharia
  2015-06-17  3:38   ` Joe Perches
                     ` (3 more replies)
  2015-06-17  1:46 ` [PATCH V2 2/6] of/slimbus: OF helper for SLIMbus Sagar Dharia
                   ` (4 subsequent siblings)
  5 siblings, 4 replies; 16+ messages in thread
From: Sagar Dharia @ 2015-06-17  1:45 UTC (permalink / raw)
  To: gregkh, bp, poeschel, sdharia, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, joe, davem, james.hogan,
	michael.opdenacker, daniel.thompson, linux-kernel
  Cc: nkaje, kheitke, mlocke, agross, linux-arm-msm

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       | 714 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mod_devicetable.h |  13 +
 include/linux/slimbus.h         | 396 ++++++++++++++++++++++
 7 files changed, 1139 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..2baf43a
--- /dev/null
+++ b/drivers/slimbus/slimbus.c
@@ -0,0 +1,714 @@
+/* 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 bool slim_eaddr_equal(struct slim_eaddr *a, struct slim_eaddr *b)
+{
+	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)
+{
+	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)
+		return 0;
+
+	slim_dev = to_slim_device(dev);
+	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)
+		return -ENXIO;
+
+	slim_dev = to_slim_device(dev);
+	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)
+		return -ENXIO;
+
+	slim_dev = to_slim_device(dev);
+	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;
+}
+
+struct bus_type slimbus_type = {
+	.name		= "slimbus",
+	.match		= slim_device_match,
+	.probe		= slim_device_probe,
+	.remove		= slim_device_remove,
+};
+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\n", 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\n",
+			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_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_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.
+ * @ctrl: Controller to tear-down.
+ */
+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_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).
+ * @ctrl: Controller associated with this framer
+ * 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->boot_device)
+				sbdrv->boot_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_eaddr_equal(&bi->board_info.slim_slave->e_addr,
+				     e_addr)) {
+			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_eaddr_equal(&slim->e_addr, e_addr)) {
+			mutex_unlock(&ctrl->m_ctrl);
+			return slim;
+		}
+	}
+	mutex_unlock(&ctrl->m_ctrl);
+
+	slim = kzalloc(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_eaddr_equal(&ctrl->addrt[i].eaddr, eaddr)) {
+			*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\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
+	 */
+	slim = slim_query_device(ctrl, e_addr);
+	if (!slim) {
+		ret = -ENOMEM;
+	} else {
+		struct slim_driver *sbdrv;
+
+		slim->laddr = *laddr;
+		slim->reported = true;
+		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..6522d4c
--- /dev/null
+++ b/include/linux/slimbus.h
@@ -0,0 +1,396 @@
+/* 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
+ * @boot_device: This callback is called after framer is booted.
+ *		Driver should do the needful to boot 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				(*boot_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.
+ * -ENXIO is returned if the device with this elemental address is not found.
+ */
+
+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.
+ */
+int slim_driver_register(struct slim_driver *drv);
+
+/**
+ * slim_driver_unregister: Undo effects of slim_driver_register
+ * @drv: Client driver to be unregistered
+ */
+void slim_driver_unregister(struct slim_driver *drv);
+
+/**
+ * slim_del_controller: Controller tear-down.
+ * @ctrl: Controller to be torn-down.
+ */
+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 be added
+ * 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);
+
+/* slim_remove_device: Remove the effect of slim_add_device() */
+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.
+ */
+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_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);
+
+/**
+ * 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);
+
+/**
+ * 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
+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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 2/6] of/slimbus: OF helper for SLIMbus
  2015-06-17  1:45 [PATCH V2 0/6] Introduce framework for SLIMbus device drivers Sagar Dharia
  2015-06-17  1:45 ` [PATCH V2 1/6] SLIMbus: Device management on SLIMbus Sagar Dharia
@ 2015-06-17  1:46 ` Sagar Dharia
  2015-06-17 13:09   ` Mark Brown
  2015-06-17  1:46 ` [PATCH V2 3/6] slimbus: Add messaging APIs to slimbus framework Sagar Dharia
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Sagar Dharia @ 2015-06-17  1:46 UTC (permalink / raw)
  To: gregkh, bp, poeschel, sdharia, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, joe, davem, james.hogan,
	michael.opdenacker, daniel.thompson, linux-kernel
  Cc: nkaje, kheitke, mlocke, agross, linux-arm-msm

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>
---
 Documentation/devicetree/bindings/slimbus/bus.txt | 34 ++++++++++
 drivers/slimbus/slimbus.c                         | 76 +++++++++++++++++++++++
 2 files changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/slimbus/bus.txt

diff --git a/Documentation/devicetree/bindings/slimbus/bus.txt b/Documentation/devicetree/bindings/slimbus/bus.txt
new file mode 100644
index 0000000..a7a36aa
--- /dev/null
+++ b/Documentation/devicetree/bindings/slimbus/bus.txt
@@ -0,0 +1,34 @@
+SLIM(Serial Low Power Interchip Media Bus) bus
+
+SLIMbus is a 2-wire bus, and is used to communicate with peripheral
+components like audio-codec.
+
+Required property for SLIMbus controller node:
+- compatible	- name of SLIMbus controller.
+
+No other properties are required in the SLIMbus controller bus node.
+
+Child nodes:
+
+Every SLIMbus controller node can contain zero or more child nodes
+representing slave devices on the bus. Every SLIMbus slave device is
+uniquely determined by the 6 byte enumeration address.
+
+Required property for SLIMbus child node:
+enumeration-addr	- 6 byte enumeration address of the slave
+
+SLIMbus example for Qualcomm's slimbus manager compoent:
+
+	slim@28080000 {
+		compatible = "qcom,slim-msm";
+		reg = <0x28080000 0x2000>,
+		reg-names = "slimbus_physical";
+		interrupts = <0 33 0>;
+		interrupt-names = "slimbus_irq";
+		clocks = <&lcc SLIMBUS_SRC>, <&lcc AUDIO_SLIMBUS_CLK>;
+		clock-names = "iface_clk", "core_clk";
+
+		slim_codec_slave {
+			enumeration-addr = [00 01 60 00 17 02];
+		};
+	};
diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
index 2baf43a..0295a06 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);
@@ -270,6 +271,80 @@ static LIST_HEAD(board_list);
 static LIST_HEAD(slim_ctrl_list);
 static DEFINE_MUTEX(board_lock);
 
+#if IS_ENABLED(CONFIG_OF)
+/* OF helpers for SLIMbus */
+static void of_register_slim_devices(struct slim_controller *ctrl)
+{
+	struct device_node *node;
+	struct slim_boardinfo *temp, *binfo = NULL;
+	int ret, n = 0;
+
+	if (!ctrl->dev.of_node)
+		return;
+
+	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\n");
+			continue;
+		}
+		ea = (u8 *)prop->value;
+		name = kcalloc(SLIMBUS_NAME_SIZE, sizeof(char), GFP_KERNEL);
+		if (!name)
+			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 = kzalloc(sizeof(struct slim_device), GFP_KERNEL);
+		if (!slim) {
+			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++;
+	}
+	slim_register_board_info(binfo, n);
+	return;
+
+of_slim_err:
+	n--;
+	while (n >= 0) {
+		kfree(binfo[n].slim_slave->name);
+		kfree(binfo[n].slim_slave);
+	}
+	kfree(binfo);
+}
+#else
+static void of_register_slim_devices(struct slim_controller *ctrl) { }
+#endif
+
 /* 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)
@@ -326,6 +401,7 @@ void slim_ctrl_add_boarddevs(struct slim_controller *ctrl)
 {
 	struct sbi_boardinfo *bi;
 
+	of_register_slim_devices(ctrl);
 	mutex_lock(&board_lock);
 	list_add_tail(&ctrl->list, &slim_ctrl_list);
 	list_for_each_entry(bi, &board_list, list)
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 3/6] slimbus: Add messaging APIs to slimbus framework
  2015-06-17  1:45 [PATCH V2 0/6] Introduce framework for SLIMbus device drivers Sagar Dharia
  2015-06-17  1:45 ` [PATCH V2 1/6] SLIMbus: Device management on SLIMbus Sagar Dharia
  2015-06-17  1:46 ` [PATCH V2 2/6] of/slimbus: OF helper for SLIMbus Sagar Dharia
@ 2015-06-17  1:46 ` Sagar Dharia
  2015-06-17  1:46 ` [PATCH V2 4/6] slim: qcom: Add Qualcomm Slimbus controller driver Sagar Dharia
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Sagar Dharia @ 2015-06-17  1:46 UTC (permalink / raw)
  To: gregkh, bp, poeschel, sdharia, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, joe, davem, james.hogan,
	michael.opdenacker, daniel.thompson, linux-kernel
  Cc: nkaje, kheitke, mlocke, agross, linux-arm-msm

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 | 275 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/slimbus.h   | 128 +++++++++++++++++++++
 2 files changed, 403 insertions(+)

diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
index 0295a06..d74dfec 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 bool slim_eaddr_equal(struct slim_eaddr *a, struct slim_eaddr *b)
 {
 	return (a->manf_id == b->manf_id &&
@@ -784,6 +792,273 @@ 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\n",
+				tid, len);
+		return;
+	}
+	memcpy(msg->rbuf, reply, len);
+	ctrl->txnt[tid] = NULL;
+	if (msg->comp_cb)
+		msg->comp_cb(msg->ctx, 0);
+	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;
+	unsigned long flags;
+
+	if (slim_tid_txn(txn->mt, txn->mc)) {
+		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);
+	}
+
+	return ctrl->xfer_msg(ctrl, txn);
+}
+
+static int slim_val_inf_sanity(struct slim_controller *ctrl,
+			       struct slim_val_inf *msg, u8 mc)
+{
+	if (!msg || msg->num_bytes > 16 ||
+	    (msg->start_offset + msg->num_bytes) > 0xC00)
+		goto reterr;
+	switch (mc) {
+	case SLIM_MSG_MC_REQUEST_VALUE:
+	case SLIM_MSG_MC_REQUEST_INFORMATION:
+		if (msg->rbuf != NULL)
+			return 0;
+		break;
+	case SLIM_MSG_MC_CHANGE_VALUE:
+	case SLIM_MSG_MC_CLEAR_INFORMATION:
+		if (msg->wbuf != NULL)
+			return 0;
+		break;
+	case SLIM_MSG_MC_REQUEST_CHANGE_VALUE:
+	case SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION:
+		if (msg->rbuf != NULL && msg->wbuf != NULL)
+			return 0;
+		break;
+	default:
+		break;
+	}
+reterr:
+	dev_err(&ctrl->dev, "Sanity check failed:msg:offset:0x%x, mc:%d\n",
+		msg->start_offset, mc);
+	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, int err)
+{
+	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(ctrl, msg, mc);
+	if (ret)
+		return ret;
+
+	tid_txn = slim_tid_txn(txn->mt, 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) {
+		/* Fine-tune calculation after bandwidth management */
+		unsigned long ms = txn->rl + 100;
+
+		ret = wait_for_completion_timeout(&complete,
+						  msecs_to_jiffies(ms));
+		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)
+		dev_err(&ctrl->dev, "xfer err:%d:offset:0x%x, MC:%d\n", 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);
+
 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 6522d4c..924cdd3 100644
--- a/include/linux/slimbus.h
+++ b/include/linux/slimbus.h
@@ -34,6 +34,9 @@ extern struct bus_type slimbus_type;
 #define SLIM_FRM_SLOTS_PER_SUPERFRAME	16
 #define SLIM_GDE_SLOTS_PER_SUPERFRAME	2
 
+/* MAX in-flight transactions neededing transaction ID (8-bit, per spec) */
+#define SLIM_MAX_TXNS			256
+
 struct slim_controller;
 struct slim_device;
 
@@ -98,12 +101,68 @@ 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, int err);
+	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
@@ -138,6 +197,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
@@ -164,7 +226,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,
@@ -393,4 +460,65 @@ 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.
+ */
+int slim_request_val_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+int slim_change_val_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+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
+ */
+
+int slim_request_inf_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+int slim_clear_inf_element(struct slim_device *sb,
+					struct slim_val_inf *msg);
+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
+ */
+void slim_msg_response(struct slim_controller *ctrl, u8 *reply, u8 tid,
+				u8 len);
+
+static inline bool slim_tid_txn(u8 mt, u8 mc)
+{
+	return (mt == SLIM_MSG_MT_CORE &&
+		(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));
+}
+/* end of message apis */
+
 #endif /* _LINUX_SLIMBUS_H */
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 4/6] slim: qcom: Add Qualcomm Slimbus controller driver
  2015-06-17  1:45 [PATCH V2 0/6] Introduce framework for SLIMbus device drivers Sagar Dharia
                   ` (2 preceding siblings ...)
  2015-06-17  1:46 ` [PATCH V2 3/6] slimbus: Add messaging APIs to slimbus framework Sagar Dharia
@ 2015-06-17  1:46 ` Sagar Dharia
  2015-06-17 13:53   ` Mark Brown
  2015-06-17  1:46 ` [PATCH V2 5/6] slimbus: Add support for 'clock-pause' feature Sagar Dharia
  2015-06-17  1:46 ` [PATCH V2 6/6] slim: qcom: Add runtime-pm support using clock-pause feature Sagar Dharia
  5 siblings, 1 reply; 16+ messages in thread
From: Sagar Dharia @ 2015-06-17  1:46 UTC (permalink / raw)
  To: gregkh, bp, poeschel, sdharia, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, joe, davem, james.hogan,
	michael.opdenacker, daniel.thompson, linux-kernel
  Cc: nkaje, kheitke, mlocke, agross, linux-arm-msm

This controller driver programs manager, interface, and framer
devices for Qualcomm's slimbus HW block.
Manager component currently implements logical address setting,
and messaging interface.
Interface device reports bus synchronization information, and framer
device clocks the bus from the time it's woken up, until clock-pause
is executed by the manager device.

Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
---
 .../devicetree/bindings/slimbus/slim-qcom-ctrl.txt |  45 ++
 drivers/slimbus/Kconfig                            |  10 +
 drivers/slimbus/Makefile                           |   2 +
 drivers/slimbus/slim-qcom-ctrl.c                   | 723 +++++++++++++++++++++
 drivers/slimbus/slim-qcom.c                        |  96 +++
 drivers/slimbus/slim-qcom.h                        |  89 +++
 6 files changed, 965 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt
 create mode 100644 drivers/slimbus/slim-qcom-ctrl.c
 create mode 100644 drivers/slimbus/slim-qcom.c
 create mode 100644 drivers/slimbus/slim-qcom.h

diff --git a/Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt b/Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt
new file mode 100644
index 0000000..25c9609
--- /dev/null
+++ b/Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt
@@ -0,0 +1,45 @@
+Qualcomm SLIMBUS controller
+"qcom,slim-msm": This controller is used if applications processor
+	driver is controlling slimbus master component. This driver is
+	responsible for communicating with slave HW directly using
+	messaging interface, and doing data channel management.
+
+Required properties:
+
+ - reg : Offset and length of the register region(s) for the device
+ - reg-names : Register region name(s) referenced in reg above
+	 Required register resource entries are:
+	 "slimbus_physical": Physical adderss of controller register blocks
+	 "slimbus_bam_physical": Physical address of Bus Access Module (BAM)
+				 for this controller
+ - compatible : should be "qcom,slim-msm" if this is master component driver
+ - cell-index : SLIMBUS number used for this controller
+ - interrupts : Interrupt numbers used by this controller
+ - interrupt-names : Required interrupt resource entries are:
+	"slimbus_irq" : Interrupt for SLIMBUS core
+ - clocks : Interface and core clocks used by this slimbus controller
+ - clock-names : Required clock-name entries are:
+	"iface_clk" : Interface clock for this controller
+	"core_clk" : Interrupt for controller core's BAM
+
+Optional property:
+ - reg entry for slew rate : If slew rate control register is provided, this
+	entry should be used.
+ - reg-name for slew rate: "slimbus_slew_reg"
+ - dmaengine, and pipes used to communicate between controller and memory if
+	sps-BAM HW is used
+
+Example:
+	slim@28080000 {
+		compatible = "qcom,slim-msm";
+		reg = <0x28080000 0x2000>,
+		reg-names = "slimbus_physical";
+		interrupts = <0 33 0>;
+		interrupt-names = "slimbus_irq";
+		clocks = <&lcc SLIMBUS_SRC>, <&lcc AUDIO_SLIMBUS_CLK>;
+		clock-names = "iface_clk", "core_clk";
+
+		slim_codec_slave {
+			enumeration-addr = [00 01 60 00 17 02];
+		};
+	};
diff --git a/drivers/slimbus/Kconfig b/drivers/slimbus/Kconfig
index fb30497..ce988c2 100644
--- a/drivers/slimbus/Kconfig
+++ b/drivers/slimbus/Kconfig
@@ -7,3 +7,13 @@ menuconfig SLIMBUS
 	  Slimbus is standard interface between baseband and audio codec,
 	  and other peripheral components in mobile terminals.
 
+if SLIMBUS
+config SLIM_QCOM_CTRL
+	tristate "Qualcomm Slimbus Manager Component"
+	depends on SLIMBUS
+	default n
+	help
+	  Select driver if Qualcomm's Slimbus Manager Component is
+	  programmed using Linux kernel.
+
+endif
diff --git a/drivers/slimbus/Makefile b/drivers/slimbus/Makefile
index 05f53bc..efc3db4 100644
--- a/drivers/slimbus/Makefile
+++ b/drivers/slimbus/Makefile
@@ -2,3 +2,5 @@
 # Makefile for kernel slimbus framework.
 #
 obj-$(CONFIG_SLIMBUS)			+= slimbus.o
+
+obj-$(CONFIG_SLIM_QCOM_CTRL)		+= slim-qcom.o slim-qcom-ctrl.o
diff --git a/drivers/slimbus/slim-qcom-ctrl.c b/drivers/slimbus/slim-qcom-ctrl.c
new file mode 100644
index 0000000..feae9d1
--- /dev/null
+++ b/drivers/slimbus/slim-qcom-ctrl.c
@@ -0,0 +1,723 @@
+/* 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/irq.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/kthread.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/slimbus.h>
+#include "slim-qcom.h"
+
+#define MSM_SLIM_NAME	"msm_slim_ctrl"
+
+/* Manager registers */
+#define	MGR_CFG		0x200
+#define	MGR_STATUS	0x204
+#define	MGR_INT_EN	0x210
+#define	MGR_INT_STAT	0x214
+#define	MGR_INT_CLR	0x218
+#define	MGR_TX_MSG	0x230
+#define	MGR_RX_MSG	0x270
+#define	MGR_IE_STAT	0x2F0
+#define	MGR_VE_STAT	0x300
+#define	MGR_CFG_ENABLE	1
+
+/* Framer registers */
+#define	FRM_CFG		0x400
+#define	FRM_STAT	0x404
+#define	FRM_INT_EN	0x410
+#define	FRM_INT_STAT	0x414
+#define	FRM_INT_CLR	0x418
+#define	FRM_WAKEUP	0x41C
+#define	FRM_CLKCTL_DONE	0x420
+#define	FRM_IE_STAT	0x430
+#define	FRM_VE_STAT	0x440
+
+/* Interface registers */
+#define	INTF_CFG	0x600
+#define	INTF_STAT	0x604
+#define	INTF_INT_EN	0x610
+#define	INTF_INT_STAT	0x614
+#define	INTF_INT_CLR	0x618
+#define	INTF_IE_STAT	0x630
+#define	INTF_VE_STAT	0x640
+
+/* Interrupt status bits */
+#define	MGR_INT_TX_NACKED_2	BIT(25)
+#define	MGR_INT_MSG_BUF_CONTE	BIT(26)
+#define	MGR_INT_RX_MSG_RCVD	BIT(30)
+#define	MGR_INT_TX_MSG_SENT	BIT(31)
+
+/* Framer config register settings */
+#define	FRM_ACTIVE	1
+#define	CLK_GEAR	7
+#define	ROOT_FREQ	11
+#define	REF_CLK_GEAR	15
+#define	INTR_WAKE	19
+
+static irqreturn_t msm_slim_interrupt(int irq, void *d)
+{
+	struct msm_slim_ctrl *dev = d;
+	u32 stat = readl_relaxed(dev->base + MGR_INT_STAT);
+	int err = 0;
+
+	if (stat & MGR_INT_TX_MSG_SENT || stat & MGR_INT_TX_NACKED_2) {
+		if (stat & MGR_INT_TX_MSG_SENT)
+			writel_relaxed(MGR_INT_TX_MSG_SENT,
+				       dev->base + MGR_INT_CLR);
+		if (stat & MGR_INT_TX_NACKED_2) {
+			u32 mgr_stat = readl_relaxed(dev->base + MGR_STATUS);
+			u32 mgr_ie_stat = readl_relaxed(dev->base +
+							MGR_IE_STAT);
+			u32 frm_stat = readl_relaxed(dev->base + FRM_STAT);
+			u32 frm_cfg = readl_relaxed(dev->base + FRM_CFG);
+			u32 frm_intr_stat = readl_relaxed(dev->base +
+							  FRM_INT_STAT);
+			u32 frm_ie_stat = readl_relaxed(dev->base +
+							FRM_IE_STAT);
+			u32 intf_stat = readl_relaxed(dev->base + INTF_STAT);
+			u32 intf_intr_stat = readl_relaxed(dev->base +
+							   INTF_INT_STAT);
+			u32 intf_ie_stat = readl_relaxed(dev->base +
+							 INTF_IE_STAT);
+
+			writel_relaxed(MGR_INT_TX_NACKED_2, dev->base +
+				       MGR_INT_CLR);
+			dev_err(dev->dev, "TX Nack MGR:int:0x%x, stat:0x%x\n",
+				stat, mgr_stat);
+			dev_err(dev->dev, "TX Nack MGR:ie:0x%x\n", mgr_ie_stat);
+			dev_err(dev->dev, "TX Nack FRM:int:0x%x, stat:0x%x\n",
+				frm_intr_stat, frm_stat);
+			dev_err(dev->dev, "TX Nack FRM:cfg:0x%x, ie:0x%x\n",
+				frm_cfg, frm_ie_stat);
+			dev_err(dev->dev, "TX Nack INTF:intr:0x%x, stat:0x%x\n",
+				intf_intr_stat, intf_stat);
+			dev_err(dev->dev, "TX Nack INTF:ie:0x%x\n",
+				intf_ie_stat);
+			err = -ENOTCONN;
+		}
+		/**
+		 * Guarantee that interrupt clear bit write goes through before
+		 * signalling completion/exiting ISR
+		 */
+		mb();
+		msm_slim_put_tx(dev, err);
+	}
+	if (stat & MGR_INT_RX_MSG_RCVD) {
+		u8 mc, mt;
+		u8 len, i;
+		u32 *rx_buf, pkt[10];
+		bool notify_rx = false;
+
+		pkt[0] = readl_relaxed(dev->base + MGR_RX_MSG);
+		mt = (pkt[0] >> 5) & 0x7;
+		mc = (pkt[0] >> 8) & 0xff;
+		len = pkt[0] & 0x1F;
+		dev_dbg(dev->dev, "RX-IRQ: MC: %x, MT: %x\n", mc, mt);
+
+		if (mt == SLIM_MSG_MT_CORE &&
+			mc == SLIM_MSG_MC_REPORT_PRESENT)
+			rx_buf = (u32 *)msm_slim_get_rx(dev);
+		else
+			rx_buf = pkt;
+
+		if (rx_buf == NULL) {
+			dev_err(dev->dev, "dropping RX:0x%x due to RX full\n",
+						pkt[0]);
+			goto rx_ret_irq;
+		}
+
+		rx_buf[0] = pkt[0];
+		for (i = 1; i < ((len + 3) >> 2); i++) {
+			rx_buf[i] = readl_relaxed(dev->base + MGR_RX_MSG +
+						(4 * i));
+			dev_dbg(dev->dev, "reading data: %x\n", rx_buf[i]);
+		}
+
+		switch (mc) {
+			u8 *buf, la;
+			u16 ele;
+
+		case SLIM_MSG_MC_REPORT_PRESENT:
+			notify_rx = true;
+			break;
+		case SLIM_MSG_MC_REPLY_INFORMATION:
+		case SLIM_MSG_MC_REPLY_VALUE:
+			slim_msg_response(&dev->ctrl, (u8 *)(rx_buf + 1),
+					  (u8)(*rx_buf >> 24), (len - 4));
+			break;
+		case SLIM_MSG_MC_REPORT_INFORMATION:
+			buf = (u8 *)rx_buf;
+			la = buf[2];
+			ele = (u16)buf[4] << 4;
+
+			ele |= ((buf[3] & 0xf0) >> 4);
+			/**
+			 * report information is most likely loss of
+			 * sync or collision detected in data slots
+			 */
+			dev_err(dev->dev, "LA:%d report inf ele:0x%x\n",
+				la, ele);
+			for (i = 0; i < len - 5; i++)
+				dev_err(dev->dev, "bit-mask:%x\n",
+					buf[i+5]);
+			break;
+		default:
+			dev_err(dev->dev, "unsupported MC,%x MT:%x\n",
+				mc, mt);
+			break;
+		}
+rx_ret_irq:
+		writel_relaxed(MGR_INT_RX_MSG_RCVD, dev->base +
+			       MGR_INT_CLR);
+		/**
+		 * Guarantee that CLR bit write goes through
+		 * before exiting
+		 */
+		mb();
+		if (notify_rx)
+			complete(&dev->rx_msgq_notify);
+	}
+	return IRQ_HANDLED;
+}
+
+static void msm_slim_wait_retry(struct msm_slim_ctrl *dev)
+{
+	int msec_per_frm = 0;
+	int sfr_per_sec;
+
+	/* Wait for 1 superframe, or default time and then retry */
+	sfr_per_sec = dev->framer.superfreq /
+			(1 << (SLIM_MAX_CLK_GEAR - dev->ctrl.clkgear));
+	if (sfr_per_sec)
+		msec_per_frm = MSEC_PER_SEC / sfr_per_sec;
+	if (msec_per_frm < DEF_RETRY_MS)
+		msec_per_frm = DEF_RETRY_MS;
+	msleep(msec_per_frm);
+}
+
+static void msm_slim_cb(void *ctx, int err)
+{
+	if (err)
+		pr_err("MSM-slim completion reported err:%d\n", err);
+	else if (ctx)
+		complete(ctx);
+}
+
+static int msm_xfer_msg(struct slim_controller *ctrl, struct slim_msg_txn *txn)
+{
+	DECLARE_COMPLETION_ONSTACK(done);
+	struct msm_slim_ctrl *dev = slim_get_ctrldata(ctrl);
+	u32 *pbuf;
+	u8 *puc;
+	int timeout;
+	u8 la = txn->la;
+	struct msm_wr_cb wr_cb = {msm_slim_cb, (void *)&done};
+
+	/* No support to send this dest-type of message */
+	if (txn->dt == SLIM_MSG_DEST_ENUMADDR)
+		return -EPROTONOSUPPORT;
+
+	/* HW expects length field to be excluded */
+	txn->rl--;
+
+	mutex_lock(&dev->txn_lock);
+	pbuf = (u32 *)msm_slim_get_tx(dev, &wr_cb);
+	if (!pbuf) {
+		dev_err(dev->dev, "bus busy, can't get empty TX\n");
+		mutex_unlock(&dev->txn_lock);
+		return -EBUSY;
+	}
+
+	if (txn->dt == SLIM_MSG_DEST_LOGICALADDR)
+		*pbuf = SLIM_MSG_ASM_FIRST_WORD(txn->rl, txn->mt, txn->mc, 0,
+						la);
+	else
+		*pbuf = SLIM_MSG_ASM_FIRST_WORD(txn->rl, txn->mt, txn->mc, 1,
+						la);
+
+	if (txn->dt == SLIM_MSG_DEST_LOGICALADDR)
+		puc = ((u8 *)pbuf) + 3;
+	else
+		puc = ((u8 *)pbuf) + 2;
+	if (txn->mt == SLIM_MSG_MT_CORE && slim_tid_txn(txn->mt, txn->mc))
+		*(puc++) = txn->tid;
+	if ((txn->mt == SLIM_MSG_MT_CORE) &&
+		((txn->mc >= SLIM_MSG_MC_REQUEST_INFORMATION &&
+		txn->mc <= SLIM_MSG_MC_REPORT_INFORMATION) ||
+		(txn->mc >= SLIM_MSG_MC_REQUEST_VALUE &&
+		 txn->mc <= SLIM_MSG_MC_CHANGE_VALUE))) {
+		*(puc++) = (txn->ec & 0xFF);
+		*(puc++) = (txn->ec >> 8) & 0xFF;
+	}
+	if (txn->msg->wbuf)
+		memcpy(puc, txn->msg->wbuf, txn->msg->num_bytes);
+	msm_slim_queue_tx(dev, pbuf, txn->rl, MGR_TX_MSG);
+	timeout = wait_for_completion_timeout(&done,
+					      msecs_to_jiffies(txn->rl + 100));
+
+	if (!timeout)
+		dev_err(dev->dev, "TX timed out:MC:0x%x,mt:0x%x\n", txn->mc,
+			txn->mt);
+
+	mutex_unlock(&dev->txn_lock);
+	return timeout ? 0 : -ETIMEDOUT;
+}
+
+static int msm_set_laddr(struct slim_controller *ctrl,
+				struct slim_eaddr *ead, u8 laddr)
+{
+	struct msm_slim_ctrl *dev = slim_get_ctrldata(ctrl);
+	u8 ea[6];
+	struct completion done;
+	int timeout, ret, retries = 0;
+	u32 *buf;
+	struct msm_wr_cb wr_cb = {msm_slim_cb, (void *)&done};
+
+	ea[5] = (u8)(ead->manf_id >> 8);
+	ea[4] = (u8)(ead->manf_id & 0xFF);
+	ea[3] = (u8) (ead->prod_code >> 8);
+	ea[2] = (u8) (ead->prod_code & 0xFF);
+	ea[1] = ead->dev_index;
+	ea[0] = ead->instance;
+	mutex_lock(&dev->txn_lock);
+	/**
+	 * Retries are needed since bus may lose sync when multiple devices
+	 * are coming up and reporting present
+	 */
+retry_laddr:
+	init_completion(&done);
+	buf = (u32 *)msm_slim_get_tx(dev, &wr_cb);
+	if (buf == NULL) {
+		mutex_unlock(&dev->txn_lock);
+		return -ENOMEM;
+	}
+
+	buf[0] = SLIM_MSG_ASM_FIRST_WORD(9, SLIM_MSG_MT_CORE,
+					SLIM_MSG_MC_ASSIGN_LOGICAL_ADDRESS,
+					SLIM_MSG_DEST_LOGICALADDR,
+					ea[5] | ea[4] << 8);
+	buf[1] = ea[3] | (ea[2] << 8) | (ea[1] << 16) | (ea[0] << 24);
+	buf[2] = laddr;
+
+	ret = msm_slim_queue_tx(dev, buf, 9, MGR_TX_MSG);
+	timeout = wait_for_completion_timeout(&done, HZ);
+	if (!timeout)
+		ret = -ETIMEDOUT;
+
+	if (ret) {
+		dev_err(dev->dev, "set LA:0x%x failed:ret:%dretry:%d\n",
+			laddr, ret, retries);
+		if (retries < INIT_MX_RETRIES) {
+			msm_slim_wait_retry(dev);
+			retries++;
+			goto retry_laddr;
+		} else {
+			dev_err(dev->dev, "set LADDR failed:ret:%d\n", ret);
+		}
+	}
+	mutex_unlock(&dev->txn_lock);
+	return ret;
+}
+
+static void msm_slim_rxwq(struct msm_slim_ctrl *dev)
+{
+	u8 buf[40];
+	u8 mc, mt, len;
+	int i, ret;
+
+	if ((msm_slim_put_rx(dev, (u8 *)buf)) != -ENODATA) {
+		len = buf[0] & 0x1F;
+		mt = (buf[0] >> 5) & 0x7;
+		mc = buf[1];
+		if (mt == SLIM_MSG_MT_CORE &&
+			mc == SLIM_MSG_MC_REPORT_PRESENT) {
+			u8 laddr;
+			struct slim_eaddr ea;
+			u8 e_addr[6];
+
+			for (i = 0; i < 6; i++)
+				e_addr[i] = buf[7-i];
+
+			ea.manf_id = (u16)(e_addr[5] << 8) | e_addr[4];
+			ea.prod_code = (u16)(e_addr[3] << 8) | e_addr[2];
+			ea.dev_index = e_addr[1];
+			ea.instance = e_addr[0];
+			ret = slim_assign_laddr(&dev->ctrl, &ea, &laddr, false);
+			if (ret)
+				dev_err(dev->dev, "assign laddr failed:%d\n",
+					ret);
+		} else {
+			dev_err(dev->dev, "unexpected message:mc:%x, mt:%x\n",
+				mc, mt);
+
+		}
+
+	} else {
+		dev_err(dev->dev, "rxwq called and no RX buffer to consume\n");
+	}
+}
+
+static int msm_slim_rx_msgq_thread(void *data)
+{
+	struct msm_slim_ctrl *dev = (struct msm_slim_ctrl *)data;
+	struct completion *notify = &dev->rx_msgq_notify;
+	int ret;
+
+	while (!kthread_should_stop()) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		ret = wait_for_completion_interruptible(notify);
+
+		if (ret)
+			dev_err(dev->dev, "rx thread wait error:%d\n", ret);
+		else
+			msm_slim_rxwq(dev);
+	}
+
+	return 0;
+}
+
+static void msm_slim_prg_slew(struct platform_device *pdev,
+				struct msm_slim_ctrl *dev)
+{
+	void __iomem *slew_reg;
+
+	/* SLEW RATE register for this slimbus */
+	dev->slew_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						"slimbus_slew_reg");
+	if (!dev->slew_mem) {
+		dev_err(&pdev->dev, "no slimbus slew resource\n");
+		return;
+	}
+
+	slew_reg = ioremap(dev->slew_mem->start, resource_size(dev->slew_mem));
+	if (!slew_reg) {
+		dev_err(dev->dev, "slew register mapping failed");
+		release_mem_region(dev->slew_mem->start,
+					resource_size(dev->slew_mem));
+		dev->slew_mem = NULL;
+		return;
+	}
+	writel_relaxed(1, slew_reg);
+	/* Make sure slimbus-slew rate enabling goes through */
+	wmb();
+	/* 1 time setting, is not needed after probing */
+	iounmap(slew_reg);
+}
+
+static int msm_slim_probe(struct platform_device *pdev)
+{
+	struct msm_slim_ctrl *dev;
+	struct resource *slim_mem, *slim_io;
+	struct resource *irq;
+	struct clk *hclk, *rclk;
+	int ret;
+
+	hclk = clk_get(&pdev->dev, "iface_clk");
+	if (IS_ERR(hclk))
+		return PTR_ERR(hclk);
+
+	rclk = clk_get(&pdev->dev, "core_clk");
+	if (IS_ERR(rclk)) {
+		/* unlikely that this is probe-defer */
+		dev_err(&pdev->dev, "rclk get failed:%ld\n", PTR_ERR(rclk));
+		clk_put(hclk);
+		return PTR_ERR(rclk);
+	}
+
+	ret = clk_set_rate(rclk, SLIM_ROOT_FREQ);
+
+	slim_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						"slimbus_physical");
+	if (!slim_mem) {
+		dev_err(&pdev->dev, "no slimbus physical memory resource\n");
+		ret = ENODEV;
+		goto err_get_mem_failed;
+	}
+	slim_io = request_mem_region(slim_mem->start, resource_size(slim_mem),
+					pdev->name);
+	if (!slim_io) {
+		dev_err(&pdev->dev, "slimbus memory already claimed\n");
+		ret = EBUSY;
+		goto err_get_mem_failed;
+	}
+
+	irq = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
+						"slimbus_irq");
+	if (!irq) {
+		dev_err(&pdev->dev, "no slimbus IRQ resource\n");
+		ret = -ENODEV;
+		goto err_get_res_failed;
+	}
+
+	dev = kzalloc(sizeof(struct msm_slim_ctrl), GFP_KERNEL);
+	if (!dev) {
+		ret = -ENOMEM;
+		goto err_get_res_failed;
+	}
+	dev->hclk = hclk;
+	dev->rclk = rclk;
+
+	dev->pending_wr = kzalloc(sizeof(struct msm_wr_cb *) * MSM_TX_MSGS,
+					GFP_KERNEL);
+	if (!dev->pending_wr) {
+		ret = -ENOMEM;
+		goto err_pending_wr_alloc_failed;
+	}
+	dev->rx.base = kzalloc(MSM_RX_MSGS * SLIM_MSGQ_BUF_LEN, GFP_KERNEL);
+	if (!dev->rx.base) {
+		ret = -ENOMEM;
+		goto err_rx_alloc_failed;
+	}
+	dev->tx.base = kzalloc(MSM_TX_MSGS * SLIM_MSGQ_BUF_LEN, GFP_KERNEL);
+	if (!dev->tx.base) {
+		ret = -ENOMEM;
+		goto err_tx_alloc_failed;
+	}
+
+	dev->dev = &pdev->dev;
+	platform_set_drvdata(pdev, dev);
+	slim_set_ctrldata(&dev->ctrl, dev);
+	dev->base = ioremap(slim_mem->start, resource_size(slim_mem));
+	if (!dev->base) {
+		dev_err(&pdev->dev, "IOremap failed\n");
+		ret = -ENOMEM;
+		goto err_ioremap_failed;
+	}
+	if (pdev->dev.of_node) {
+
+		ret = of_property_read_u32(pdev->dev.of_node, "cell-index",
+					&dev->ctrl.nr);
+		if (ret) {
+			dev_err(&pdev->dev, "No cell index:%d\n", ret);
+			goto err_of_init_failed;
+		}
+	} else {
+		dev->ctrl.nr = pdev->id;
+	}
+	dev->ctrl.set_laddr = msm_set_laddr;
+	dev->ctrl.xfer_msg = msm_xfer_msg;
+
+	mutex_init(&dev->txn_lock);
+	init_completion(&dev->rx_msgq_notify);
+	spin_lock_init(&dev->rx.lock);
+	spin_lock_init(&dev->tx.lock);
+
+	dev->irq = irq->start;
+
+	/* Fire up the Rx message queue thread */
+	dev->rx_msgq_thread = kthread_run(msm_slim_rx_msgq_thread, dev,
+					MSM_SLIM_NAME "_rx_msgq_thread");
+	if (IS_ERR(dev->rx_msgq_thread)) {
+		ret = PTR_ERR(dev->rx_msgq_thread);
+		dev_err(dev->dev, "Failed to start Rx message queue thread\n");
+		goto err_thread_create_failed;
+	}
+
+	dev->framer.rootfreq = SLIM_ROOT_FREQ >> 3;
+	dev->framer.superfreq =
+		dev->framer.rootfreq / SLIM_CL_PER_SUPERFRAME_DIV8;
+	dev->ctrl.a_framer = &dev->framer;
+	dev->ctrl.clkgear = SLIM_MAX_CLK_GEAR;
+	dev->ctrl.dev.parent = &pdev->dev;
+	dev->ctrl.dev.of_node = pdev->dev.of_node;
+
+	msm_slim_prg_slew(pdev, dev);
+
+	ret = request_irq(dev->irq, msm_slim_interrupt,
+				IRQF_TRIGGER_HIGH, "msm_slim_irq", dev);
+	if (ret) {
+		dev_err(&pdev->dev, "request IRQ failed\n");
+		goto err_request_irq_failed;
+	}
+
+	ret = clk_prepare_enable(hclk);
+	if (ret)
+		goto err_hclk_enable_failed;
+
+	ret = clk_prepare_enable(rclk);
+	if (ret)
+		goto err_rclk_enable_failed;
+
+	/* Register with framework before enabling frame, clock */
+	ret = slim_add_numbered_controller(&dev->ctrl);
+	if (ret) {
+		dev_err(dev->dev, "error adding controller\n");
+		goto err_ctrl_failed;
+	}
+
+	dev->ver = readl_relaxed(dev->base);
+	/* Version info in 16 MSbits */
+	dev->ver >>= 16;
+	/* Component register initialization */
+	writel_relaxed(1, dev->base + CFG_PORT(COMP_CFG, dev->ver));
+	writel_relaxed((EE_MGR_RSC_GRP | EE_NGD_2 | EE_NGD_1),
+				dev->base + CFG_PORT(COMP_TRUST_CFG, dev->ver));
+
+	writel_relaxed((MGR_INT_TX_NACKED_2 |
+			MGR_INT_MSG_BUF_CONTE | MGR_INT_RX_MSG_RCVD |
+			MGR_INT_TX_MSG_SENT), dev->base + MGR_INT_EN);
+	writel_relaxed(1, dev->base + MGR_CFG);
+	/*
+	 * Framer registers are beyond 1K memory region after Manager and/or
+	 * component registers. Make sure those writes are ordered
+	 * before framer register writes
+	 */
+	wmb();
+
+	/* Framer register initialization */
+	writel_relaxed((1 << INTR_WAKE) | (0xA << REF_CLK_GEAR) |
+		(0xA << CLK_GEAR) | (1 << ROOT_FREQ) | (1 << FRM_ACTIVE) | 1,
+		dev->base + FRM_CFG);
+	/*
+	 * Make sure that framer wake-up and enabling writes go through
+	 * before any other component is enabled. Framer is responsible for
+	 * clocking the bus and enabling framer first will ensure that other
+	 * devices can report presence when they are enabled
+	 */
+	mb();
+
+	writel_relaxed(MGR_CFG_ENABLE, dev->base + MGR_CFG);
+	/*
+	 * Make sure that manager-enable is written through before interface
+	 * device is enabled
+	 */
+	mb();
+	writel_relaxed(1, dev->base + INTF_CFG);
+	/*
+	 * Make sure that interface-enable is written through before enabling
+	 * ported generic device inside MSM manager
+	 */
+	mb();
+
+	writel_relaxed(1, dev->base + CFG_PORT(COMP_CFG, dev->ver));
+	/*
+	 * Make sure that all writes have gone through before exiting this
+	 * function
+	 */
+	mb();
+
+	/* Add devices registered with board-info now that controller is up */
+	slim_ctrl_add_boarddevs(&dev->ctrl);
+
+	dev_dbg(dev->dev, "MSM SB controller is up:ver:0x%x!\n", dev->ver);
+	return 0;
+
+err_ctrl_failed:
+	clk_disable_unprepare(dev->rclk);
+err_rclk_enable_failed:
+	clk_disable_unprepare(dev->hclk);
+err_hclk_enable_failed:
+	free_irq(dev->irq, dev);
+err_request_irq_failed:
+	kthread_stop(dev->rx_msgq_thread);
+err_thread_create_failed:
+err_of_init_failed:
+	iounmap(dev->base);
+err_ioremap_failed:
+	kfree(dev->tx.base);
+err_tx_alloc_failed:
+	kfree(dev->rx.base);
+err_rx_alloc_failed:
+	kfree(dev->pending_wr);
+err_pending_wr_alloc_failed:
+	kfree(dev);
+err_get_res_failed:
+	release_mem_region(slim_mem->start, resource_size(slim_mem));
+err_get_mem_failed:
+	clk_put(rclk);
+	clk_put(hclk);
+	return ret;
+}
+
+static int msm_slim_remove(struct platform_device *pdev)
+{
+	struct msm_slim_ctrl *dev = platform_get_drvdata(pdev);
+	struct resource *slim_mem;
+	struct resource *slew_mem = dev->slew_mem;
+
+	free_irq(dev->irq, dev);
+	slim_del_controller(&dev->ctrl);
+	clk_put(dev->rclk);
+	if (dev->hclk)
+		clk_put(dev->hclk);
+	kthread_stop(dev->rx_msgq_thread);
+	kfree(dev->rx.base);
+	kfree(dev->tx.base);
+	kfree(dev->pending_wr);
+	iounmap(dev->base);
+	kfree(dev);
+	if (slew_mem)
+		release_mem_region(slew_mem->start, resource_size(slew_mem));
+	slim_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						"slimbus_physical");
+	if (slim_mem)
+		release_mem_region(slim_mem->start, resource_size(slim_mem));
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int msm_slim_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int msm_slim_resume(struct device *dev)
+{
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops msm_slim_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(msm_slim_suspend, msm_slim_resume)
+};
+
+static const struct of_device_id msm_slim_dt_match[] = {
+	{
+		.compatible = "qcom,slim-msm",
+	},
+	{}
+};
+
+static struct platform_driver msm_slim_driver = {
+	.probe = msm_slim_probe,
+	.remove = msm_slim_remove,
+	.driver	= {
+		.name = MSM_SLIM_NAME,
+		.owner = THIS_MODULE,
+		.pm = &msm_slim_dev_pm_ops,
+		.of_match_table = msm_slim_dt_match,
+	},
+};
+
+static int msm_slim_init(void)
+{
+	return platform_driver_register(&msm_slim_driver);
+}
+module_init(msm_slim_init);
+
+static void msm_slim_exit(void)
+{
+	platform_driver_unregister(&msm_slim_driver);
+}
+module_exit(msm_slim_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_VERSION("0.1");
+MODULE_DESCRIPTION("Qualcomm Slimbus controller");
+MODULE_ALIAS("platform:qcom-slim");
diff --git a/drivers/slimbus/slim-qcom.c b/drivers/slimbus/slim-qcom.c
new file mode 100644
index 0000000..2e59f99
--- /dev/null
+++ b/drivers/slimbus/slim-qcom.c
@@ -0,0 +1,96 @@
+/* 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/slab.h>
+#include <linux/slimbus.h>
+#include "slim-qcom.h"
+
+/* Helper functions to handle Slimbus Qualcomm controller's TX/RX */
+u8 *msm_slim_get_rx(struct msm_slim_ctrl *dev)
+{
+	unsigned long flags;
+	int idx;
+
+	spin_lock_irqsave(&dev->rx.lock, flags);
+	if ((dev->rx.tail + 1) % MSM_RX_MSGS == dev->rx.head) {
+		spin_unlock_irqrestore(&dev->rx.lock, flags);
+		dev_err(dev->dev, "RX QUEUE full!");
+		return NULL;
+	}
+	idx = dev->rx.tail;
+	dev->rx.tail = (dev->rx.tail + 1) % MSM_RX_MSGS;
+	spin_unlock_irqrestore(&dev->rx.lock, flags);
+	return dev->rx.base + (idx * SLIM_MSGQ_BUF_LEN);
+}
+
+int msm_slim_put_rx(struct msm_slim_ctrl *dev, u8 *buf)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->rx.lock, flags);
+	if (dev->rx.tail == dev->rx.head) {
+		spin_unlock_irqrestore(&dev->rx.lock, flags);
+		return -ENODATA;
+	}
+	memcpy(buf, dev->rx.base + (dev->rx.head * SLIM_MSGQ_BUF_LEN),
+	       SLIM_MSGQ_BUF_LEN);
+	dev->rx.head = (dev->rx.head + 1) % MSM_RX_MSGS;
+	spin_unlock_irqrestore(&dev->rx.lock, flags);
+	return 0;
+}
+
+void msm_slim_put_tx(struct msm_slim_ctrl *dev, int err)
+{
+	unsigned long flags;
+	int idx;
+	struct msm_wr_cb *cur;
+
+	spin_lock_irqsave(&dev->tx.lock, flags);
+	idx = dev->tx.head;
+	dev->tx.head = (dev->tx.head + 1) % MSM_TX_MSGS;
+	cur = dev->pending_wr[idx];
+	dev->pending_wr[idx] = NULL;
+	spin_unlock_irqrestore(&dev->tx.lock, flags);
+	if (!cur || !cur->cb)
+		dev_err(dev->dev, "NULL Transaction or completion");
+	else
+		cur->cb(cur->ctx, err);
+}
+
+int msm_slim_queue_tx(struct msm_slim_ctrl *dev, u32 *buf, u8 len, u32 tx_reg)
+{
+	int i;
+
+	for (i = 0; i < (len + 3) >> 2; i++) {
+		dev_dbg(dev->dev, "AHB TX data:0x%x\n", buf[i]);
+		writel_relaxed(buf[i], dev->base + tx_reg + (i * 4));
+	}
+	/* Guarantee that message is sent before returning */
+	mb();
+	return 0;
+}
+
+u8 *msm_slim_get_tx(struct msm_slim_ctrl *dev, struct msm_wr_cb *cur)
+{
+	unsigned long flags;
+	int idx;
+
+	spin_lock_irqsave(&dev->tx.lock, flags);
+	if (((dev->tx.head + 1) % MSM_TX_MSGS) == dev->tx.tail) {
+		spin_unlock_irqrestore(&dev->tx.lock, flags);
+		return NULL;
+	}
+	idx = dev->tx.tail;
+	dev->tx.tail = (dev->tx.tail + 1) % MSM_TX_MSGS;
+	spin_unlock_irqrestore(&dev->tx.lock, flags);
+	dev->pending_wr[idx] = cur;
+	return dev->tx.base + (idx * SLIM_MSGQ_BUF_LEN);
+}
diff --git a/drivers/slimbus/slim-qcom.h b/drivers/slimbus/slim-qcom.h
new file mode 100644
index 0000000..32073fc
--- /dev/null
+++ b/drivers/slimbus/slim-qcom.h
@@ -0,0 +1,89 @@
+/* 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 _SLIM_QCOM_H
+#define _SLIM_QCOM_H
+
+#include <linux/irq.h>
+#include <linux/kthread.h>
+
+#define QC_MFGID_LSB	0x2
+#define QC_MFGID_MSB	0x17
+
+#define SLIM_MSG_ASM_FIRST_WORD(l, mt, mc, dt, ad) \
+		((l) | ((mt) << 5) | ((mc) << 8) | ((dt) << 15) | ((ad) << 16))
+
+#define SLIM_ROOT_FREQ 24576000
+#define INIT_MX_RETRIES 10
+#define DEF_RETRY_MS	10
+
+/* MAX message size over control channel */
+#define SLIM_MSGQ_BUF_LEN	40
+#define MSM_TX_MSGS 2
+#define MSM_RX_MSGS	8
+
+#define CFG_PORT(r, v) ((v) ? CFG_PORT_V2(r) : CFG_PORT_V1(r))
+
+/* V2 Component registers */
+#define CFG_PORT_V2(r) ((r ## _V2))
+#define	COMP_CFG_V2		4
+#define	COMP_TRUST_CFG_V2	0x3000
+
+/* V1 Component registers */
+#define CFG_PORT_V1(r) ((r ## _V1))
+#define	COMP_CFG_V1		0
+#define	COMP_TRUST_CFG_V1	0x14
+
+/* Resource group info for manager, and non-ported generic device-components */
+#define EE_MGR_RSC_GRP	(1 << 10)
+#define EE_NGD_2	(2 << 6)
+#define EE_NGD_1	0
+
+struct msm_slim_addr {
+	u8		*base;
+	spinlock_t	lock;
+	int		head;
+	int		tail;
+};
+
+struct msm_wr_cb {
+	void (*cb)(void *ctx, int err);
+	void *ctx;
+};
+
+struct msm_slim_ctrl {
+	struct slim_controller  ctrl;
+	struct slim_framer	framer;
+	struct device		*dev;
+	void __iomem		*base;
+	struct msm_slim_addr	tx;
+	struct msm_slim_addr	rx;
+	struct resource		*slew_mem;
+	int			irq;
+	struct msm_wr_cb	**pending_wr;
+	struct mutex		txn_lock;
+	struct completion	rx_msgq_notify;
+	struct task_struct	*rx_msgq_thread;
+	struct clk		*rclk;
+	struct clk		*hclk;
+	u32			ver;
+};
+
+/* RX helpers */
+u8 *msm_slim_get_rx(struct msm_slim_ctrl *dev);
+int msm_slim_put_rx(struct msm_slim_ctrl *dev, u8 *buf);
+
+/* TX helpers */
+int msm_slim_queue_tx(struct msm_slim_ctrl *dev, u32 *buf, u8 len, u32 tx_reg);
+u8 *msm_slim_get_tx(struct msm_slim_ctrl *dev, struct msm_wr_cb *cur);
+void msm_slim_put_tx(struct msm_slim_ctrl *dev, int err);
+#endif
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 5/6] slimbus: Add support for 'clock-pause' feature
  2015-06-17  1:45 [PATCH V2 0/6] Introduce framework for SLIMbus device drivers Sagar Dharia
                   ` (3 preceding siblings ...)
  2015-06-17  1:46 ` [PATCH V2 4/6] slim: qcom: Add Qualcomm Slimbus controller driver Sagar Dharia
@ 2015-06-17  1:46 ` Sagar Dharia
  2015-06-17  1:46 ` [PATCH V2 6/6] slim: qcom: Add runtime-pm support using clock-pause feature Sagar Dharia
  5 siblings, 0 replies; 16+ messages in thread
From: Sagar Dharia @ 2015-06-17  1:46 UTC (permalink / raw)
  To: gregkh, bp, poeschel, sdharia, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, joe, davem, james.hogan,
	michael.opdenacker, daniel.thompson, linux-kernel
  Cc: nkaje, kheitke, mlocke, agross, linux-arm-msm

Per slimbus specification, a reconfiguration sequence known as
'clock pause' needs to be broadcast over the bus while entering low-
power mode. Clock-pause is initiated by the controller driver.
To exit clock-pause, controller typically wakes up the framer device.
Since wakeup precedure is controller-specific, framework calls it via
controller's function pointer to invoke it.

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

diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
index d74dfec..3cfa414 100644
--- a/drivers/slimbus/slimbus.c
+++ b/drivers/slimbus/slimbus.c
@@ -436,7 +436,9 @@ static int slim_register_controller(struct slim_controller *ctrl)
 		ctrl->min_cg = SLIM_MIN_CLK_GEAR;
 	if (!ctrl->max_cg)
 		ctrl->max_cg = SLIM_MAX_CLK_GEAR;
+	init_completion(&ctrl->sched.pause_comp);
 	mutex_init(&ctrl->m_ctrl);
+	mutex_init(&ctrl->sched.m_reconf);
 	ret = device_register(&ctrl->dev);
 	if (ret)
 		goto out_list;
@@ -1059,6 +1061,113 @@ int slim_request_clear_inf_element(struct slim_device *sb,
 }
 EXPORT_SYMBOL(slim_request_clear_inf_element);
 
+/**
+ * slim_ctrl_clk_pause: Called by slimbus controller to enter/exit 'clock pause'
+ * Slimbus specification needs this sequence to turn-off clocks for the bus.
+ * The sequence involves sending 3 broadcast messages (reconfiguration
+ * sequence) to inform all devices on the bus.
+ * To exit clock-pause, controller typically wakes up active framer device.
+ * @ctrl: controller requesting bus to be paused or woken up
+ * @wakeup: Wakeup this controller from clock pause.
+ * @restart: Restart time value per spec used for clock pause. This value
+ *	isn't used when controller is to be woken up.
+ * This API executes clock pause reconfiguration sequence if wakeup is false.
+ * If wakeup is true, controller's wakeup is called.
+ * For entering clock-pause, -EBUSY is returned if a message txn in pending.
+ */
+int slim_ctrl_clk_pause(struct slim_controller *ctrl, bool wakeup, u8 restart)
+{
+	int i, ret = 0;
+	unsigned long flags;
+	struct slim_sched *sched = &ctrl->sched;
+	struct slim_val_inf msg = {0, 0, NULL, NULL, NULL, NULL};
+
+	DEFINE_SLIM_BCAST_TXN(txn, SLIM_MSG_MC_BEGIN_RECONFIGURATION,
+				3, SLIM_LA_MANAGER, &msg);
+
+	if (wakeup == false && restart > SLIM_CLK_UNSPECIFIED)
+		return -EINVAL;
+	mutex_lock(&sched->m_reconf);
+	if (wakeup) {
+		if (sched->clk_state == SLIM_CLK_ACTIVE) {
+			mutex_unlock(&sched->m_reconf);
+			return 0;
+		}
+		/**
+		 * Fine-tune calculation based on clock gear,
+		 * message-bandwidth after bandwidth management
+		 */
+		ret = wait_for_completion_timeout(&sched->pause_comp,
+				msecs_to_jiffies(100));
+		if (!ret) {
+			mutex_unlock(&sched->m_reconf);
+			pr_err("Previous clock pause did not finish");
+			return -ETIMEDOUT;
+		}
+		ret = 0;
+		/**
+		 * Slimbus framework will call controller wakeup
+		 * Controller should make sure that it sets active framer
+		 * out of clock pause
+		 */
+		if (sched->clk_state == SLIM_CLK_PAUSED && ctrl->wakeup)
+			ret = ctrl->wakeup(ctrl);
+		if (!ret)
+			sched->clk_state = SLIM_CLK_ACTIVE;
+		mutex_unlock(&sched->m_reconf);
+		return ret;
+	}
+
+	/* already paused */
+	if (ctrl->sched.clk_state == SLIM_CLK_PAUSED) {
+		mutex_unlock(&sched->m_reconf);
+		return 0;
+	}
+
+	spin_lock_irqsave(&ctrl->txn_lock, flags);
+	for (i = 0; i < ctrl->last_tid; i++) {
+		/* Pending response for a message */
+		if (ctrl->txnt[i]) {
+			spin_unlock_irqrestore(&ctrl->txn_lock, flags);
+			mutex_unlock(&sched->m_reconf);
+			return -EBUSY;
+		}
+	}
+	spin_unlock_irqrestore(&ctrl->txn_lock, flags);
+
+	sched->clk_state = SLIM_CLK_ENTERING_PAUSE;
+
+	/* clock pause sequence */
+	ret = slim_processtxn(ctrl, &txn);
+	if (ret)
+		goto clk_pause_ret;
+
+	txn.mc = SLIM_MSG_MC_NEXT_PAUSE_CLOCK;
+	txn.rl = 4;
+	msg.num_bytes = 1;
+	msg.wbuf = &restart;
+	ret = slim_processtxn(ctrl, &txn);
+	if (ret)
+		goto clk_pause_ret;
+
+	txn.mc = SLIM_MSG_MC_RECONFIGURE_NOW;
+	txn.rl = 3;
+	msg.num_bytes = 1;
+	msg.wbuf = NULL;
+	ret = slim_processtxn(ctrl, &txn);
+
+clk_pause_ret:
+	if (ret) {
+		sched->clk_state = SLIM_CLK_ACTIVE;
+	} else {
+		sched->clk_state = SLIM_CLK_PAUSED;
+		complete(&sched->pause_comp);
+	}
+	mutex_unlock(&sched->m_reconf);
+	return ret;
+}
+EXPORT_SYMBOL(slim_ctrl_clk_pause);
+
 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 924cdd3..f1099c5 100644
--- a/include/linux/slimbus.h
+++ b/include/linux/slimbus.h
@@ -114,11 +114,21 @@ struct slim_addrt {
 #define SLIM_MSG_MC_REPLY_VALUE                  0x64
 #define SLIM_MSG_MC_CHANGE_VALUE                 0x68
 
+/* Clock pause Reconfiguration messages */
+#define SLIM_MSG_MC_BEGIN_RECONFIGURATION        0x40
+#define SLIM_MSG_MC_NEXT_PAUSE_CLOCK             0x4A
+#define SLIM_MSG_MC_RECONFIGURE_NOW              0x5F
+
 /* Destination type Values */
 #define SLIM_MSG_DEST_LOGICALADDR	0
 #define SLIM_MSG_DEST_ENUMADDR		1
 #define	SLIM_MSG_DEST_BROADCAST		3
 
+/* Clock pause values per slimbus spec */
+#define SLIM_CLK_FAST				0
+#define SLIM_CLK_CONST_PHASE			1
+#define SLIM_CLK_UNSPECIFIED			2
+
 /**
  * struct slim_val_inf: Slimbus value or information element
  * @start_offset: Specifies starting offset in information/value element map
@@ -163,6 +173,39 @@ struct slim_msg_txn {
 };
 
 /**
+ * enum slim_clk_state: Slimbus controller's clock state used internally for
+ *	maintaining current clock state.
+ * @SLIM_CLK_ACTIVE: Slimbus clock is active
+ * @SLIM_CLK_ENTERING_PAUSE: Slimbus clock pause sequence is being sent on the
+ *	bus. If this succeeds, state changes to SLIM_CLK_PAUSED. If the
+ *	transition fails, state changes back to SLIM_CLK_ACTIVE
+ * @SLIM_CLK_PAUSED: Slimbus controller clock has paused.
+ */
+enum slim_clk_state {
+	SLIM_CLK_ACTIVE,
+	SLIM_CLK_ENTERING_PAUSE,
+	SLIM_CLK_PAUSED,
+};
+
+/**
+ * struct slim_sched: Framework uses this structure internally for scheduling.
+ * @clk_state: Controller's clock state from enum slim_clk_state
+ * @pause_comp: Signals completion of clock pause sequence. This is useful when
+ *	client tries to call slimbus transaction when controller is entering
+ *	clock pause.
+ * @m_reconf: This mutex is held until current reconfiguration (data channel
+ *	scheduling, message bandwidth reservation) is done. Message APIs can
+ *	use the bus concurrently when this mutex is held since elemental access
+ *	messages can be sent on the bus when reconfiguration is in progress.
+ */
+struct slim_sched {
+	int			clkgear;
+	enum slim_clk_state	clk_state;
+	struct completion	pause_comp;
+	struct mutex		m_reconf;
+};
+
+/**
  * struct slim_controller: Controls every instance of SLIMbus
  *				(similar to 'master' on SPI)
  *	'Manager device' is responsible for  device management, bandwidth
@@ -200,6 +243,7 @@ struct slim_msg_txn {
  * @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)
+ * @sched: scheduler structure used by the controller
  * @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
@@ -211,6 +255,9 @@ struct slim_msg_txn {
  * @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.
+ * @wakeup: This function pointer implements controller-specific procedure
+ *	to wake it up from clock-pause. Framework will call this to bring
+ *	the controller out of clock pause.
  */
 struct slim_controller {
 	struct device		dev;
@@ -229,6 +276,7 @@ struct slim_controller {
 	struct slim_val_inf	*txnt[SLIM_MAX_TXNS];
 	u8			last_tid;
 	spinlock_t		txn_lock;
+	struct slim_sched	sched;
 	struct completion	dev_released;
 	int			(*xfer_msg)(struct slim_controller *ctrl,
 				struct slim_msg_txn *txn);
@@ -236,6 +284,7 @@ struct slim_controller {
 					     struct slim_eaddr *ea, u8 laddr);
 	int			(*get_laddr)(struct slim_controller *ctrl,
 					     struct slim_eaddr *ea, u8 *laddr);
+	int			(*wakeup)(struct slim_controller *ctrl);
 };
 
 #define to_slim_controller(d) container_of(d, struct slim_controller, dev)
@@ -521,4 +570,20 @@ static inline bool slim_tid_txn(u8 mt, u8 mc)
 }
 /* end of message apis */
 
+/**
+ * slim_ctrl_clk_pause: Called by slimbus controller to enter/exit 'clock pause'
+ * Slimbus specification needs this sequence to turn-off clocks for the bus.
+ * The sequence involves sending 3 broadcast messages (reconfiguration
+ * sequence) to inform all devices on the bus.
+ * To exit clock-pause, controller typically wakes up active framer device.
+ * @ctrl: controller requesting bus to be paused or woken up
+ * @wakeup: Wakeup this controller from clock pause.
+ * @restart: Restart time value per spec used for clock pause. This value
+ *	isn't used when controller is to be woken up.
+ * This API executes clock pause reconfiguration sequence if wakeup is false.
+ * If wakeup is true, controller's wakeup is called.
+ * For entering clock-pause, -EBUSY is returned if a message txn in pending.
+ */
+int slim_ctrl_clk_pause(struct slim_controller *ctrl, bool wakeup, u8 restart);
+
 #endif /* _LINUX_SLIMBUS_H */
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH V2 6/6] slim: qcom: Add runtime-pm support using clock-pause feature
  2015-06-17  1:45 [PATCH V2 0/6] Introduce framework for SLIMbus device drivers Sagar Dharia
                   ` (4 preceding siblings ...)
  2015-06-17  1:46 ` [PATCH V2 5/6] slimbus: Add support for 'clock-pause' feature Sagar Dharia
@ 2015-06-17  1:46 ` Sagar Dharia
  5 siblings, 0 replies; 16+ messages in thread
From: Sagar Dharia @ 2015-06-17  1:46 UTC (permalink / raw)
  To: gregkh, bp, poeschel, sdharia, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, joe, davem, james.hogan,
	michael.opdenacker, daniel.thompson, linux-kernel
  Cc: nkaje, kheitke, mlocke, agross, linux-arm-msm

Slimbus HW mandates that clock-pause sequence has to be executed
before disabling relevant interface and core clocks.
Runtime-PM's autosuspend feature is used here to enter/exit low
power mode for Qualcomm's Slimbus controller. Autosuspend feature
enables driver to avoid changing power-modes too frequently since
entering clock-pause is an expensive sequence

Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
---
 drivers/slimbus/slim-qcom-ctrl.c | 133 ++++++++++++++++++++++++++++++++++++++-
 drivers/slimbus/slim-qcom.h      |   1 +
 2 files changed, 132 insertions(+), 2 deletions(-)

diff --git a/drivers/slimbus/slim-qcom-ctrl.c b/drivers/slimbus/slim-qcom-ctrl.c
index feae9d1..ec8687d 100644
--- a/drivers/slimbus/slim-qcom-ctrl.c
+++ b/drivers/slimbus/slim-qcom-ctrl.c
@@ -21,6 +21,7 @@
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/slimbus.h>
+#include <linux/pm_runtime.h>
 #include "slim-qcom.h"
 
 #define MSM_SLIM_NAME	"msm_slim_ctrl"
@@ -193,6 +194,7 @@ rx_ret_irq:
 		if (notify_rx)
 			complete(&dev->rx_msgq_notify);
 	}
+	pm_runtime_mark_last_busy(dev->dev);
 	return IRQ_HANDLED;
 }
 
@@ -211,6 +213,28 @@ static void msm_slim_wait_retry(struct msm_slim_ctrl *dev)
 	msleep(msec_per_frm);
 }
 
+static int msm_clk_pause_wakeup(struct slim_controller *ctrl)
+{
+	struct msm_slim_ctrl *dev = slim_get_ctrldata(ctrl);
+
+	clk_prepare_enable(dev->hclk);
+	clk_prepare_enable(dev->rclk);
+	writel_relaxed(1, dev->base + FRM_WAKEUP);
+	/* Make sure framer wakeup write goes through before ISR fires */
+	mb();
+	/**
+	 * HW Workaround: Currently, slave is reporting lost-sync messages
+	 * after slimbus comes out of clock pause.
+	 * Transaction with slave fail before slave reports that message
+	 * Give some time for that report to come
+	 * Slimbus wakes up in clock gear 10 at 24.576MHz. With each superframe
+	 * being 250 usecs, we wait for 5-10 superframes here to ensure
+	 * we get the message
+	 */
+	usleep_range(1250, 2500);
+	return 0;
+}
+
 static void msm_slim_cb(void *ctx, int err)
 {
 	if (err)
@@ -225,14 +249,29 @@ static int msm_xfer_msg(struct slim_controller *ctrl, struct slim_msg_txn *txn)
 	struct msm_slim_ctrl *dev = slim_get_ctrldata(ctrl);
 	u32 *pbuf;
 	u8 *puc;
-	int timeout;
+	int ret, timeout;
 	u8 la = txn->la;
+	enum slim_clk_state cur_clk_state = ctrl->sched.clk_state;
 	struct msm_wr_cb wr_cb = {msm_slim_cb, (void *)&done};
 
 	/* No support to send this dest-type of message */
 	if (txn->dt == SLIM_MSG_DEST_ENUMADDR)
 		return -EPROTONOSUPPORT;
 
+	if (cur_clk_state == SLIM_CLK_ENTERING_PAUSE) {
+		if (txn->mc != SLIM_MSG_MC_BEGIN_RECONFIGURATION &&
+		    txn->mc != SLIM_MSG_MC_RECONFIGURE_NOW &&
+		    txn->mc != SLIM_MSG_MC_NEXT_PAUSE_CLOCK)
+			return -EBUSY;
+	} else {
+		ret = pm_runtime_get_sync(dev->dev);
+		if (ret < 0) {
+			pm_runtime_set_suspended(dev->dev);
+			dev_err(dev->dev, "runtime-pm vote failed:%d\n", ret);
+			return ret;
+		}
+	}
+
 	/* HW expects length field to be excluded */
 	txn->rl--;
 
@@ -276,6 +315,8 @@ static int msm_xfer_msg(struct slim_controller *ctrl, struct slim_msg_txn *txn)
 			txn->mt);
 
 	mutex_unlock(&dev->txn_lock);
+	if (cur_clk_state != SLIM_CLK_ENTERING_PAUSE)
+		pm_runtime_put(dev->dev);
 	return timeout ? 0 : -ETIMEDOUT;
 }
 
@@ -295,6 +336,13 @@ static int msm_set_laddr(struct slim_controller *ctrl,
 	ea[2] = (u8) (ead->prod_code & 0xFF);
 	ea[1] = ead->dev_index;
 	ea[0] = ead->instance;
+
+	ret = pm_runtime_get_sync(dev->dev);
+	if (ret < 0) {
+		pm_runtime_set_suspended(dev->dev);
+		dev_err(dev->dev, "runtime-pm vote failed:%d\n", ret);
+		return ret;
+	}
 	mutex_lock(&dev->txn_lock);
 	/**
 	 * Retries are needed since bus may lose sync when multiple devices
@@ -305,6 +353,7 @@ retry_laddr:
 	buf = (u32 *)msm_slim_get_tx(dev, &wr_cb);
 	if (buf == NULL) {
 		mutex_unlock(&dev->txn_lock);
+		pm_runtime_put(dev->dev);
 		return -ENOMEM;
 	}
 
@@ -332,6 +381,7 @@ retry_laddr:
 		}
 	}
 	mutex_unlock(&dev->txn_lock);
+	pm_runtime_put(dev->dev);
 	return ret;
 }
 
@@ -512,6 +562,7 @@ static int msm_slim_probe(struct platform_device *pdev)
 	}
 	dev->ctrl.set_laddr = msm_set_laddr;
 	dev->ctrl.xfer_msg = msm_xfer_msg;
+	dev->ctrl.wakeup =  msm_clk_pause_wakeup;
 
 	mutex_init(&dev->txn_lock);
 	init_completion(&dev->rx_msgq_notify);
@@ -615,6 +666,12 @@ static int msm_slim_probe(struct platform_device *pdev)
 	/* Add devices registered with board-info now that controller is up */
 	slim_ctrl_add_boarddevs(&dev->ctrl);
 
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, MSM_SLIM_AUTOSUSPEND);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
 	dev_dbg(dev->dev, "MSM SB controller is up:ver:0x%x!\n", dev->ver);
 	return 0;
 
@@ -651,6 +708,7 @@ static int msm_slim_remove(struct platform_device *pdev)
 	struct resource *slim_mem;
 	struct resource *slew_mem = dev->slew_mem;
 
+	pm_runtime_disable(&pdev->dev);
 	free_irq(dev->irq, dev);
 	slim_del_controller(&dev->ctrl);
 	clk_put(dev->rclk);
@@ -671,20 +729,91 @@ static int msm_slim_remove(struct platform_device *pdev)
 	return 0;
 }
 
+/**
+ * If PM_RUNTIME is not defined, these 2 functions become helper
+ * functions to be called from system suspend/resume.
+ */
+#ifdef CONFIG_PM
+static int msm_slim_runtime_suspend(struct device *device)
+{
+	struct platform_device *pdev = to_platform_device(device);
+	struct msm_slim_ctrl *dev = platform_get_drvdata(pdev);
+	int ret;
+
+	dev_dbg(device, "pm_runtime: suspending...\n");
+	ret = slim_ctrl_clk_pause(&dev->ctrl, false, SLIM_CLK_UNSPECIFIED);
+	if (ret)
+		dev_err(device, "clk pause not entered:%d", ret);
+	else {
+		clk_disable_unprepare(dev->hclk);
+		clk_disable_unprepare(dev->rclk);
+	}
+	return ret;
+}
+
+static int msm_slim_runtime_resume(struct device *device)
+{
+	struct platform_device *pdev = to_platform_device(device);
+	struct msm_slim_ctrl *dev = platform_get_drvdata(pdev);
+	int ret = 0;
+
+	dev_dbg(device, "pm_runtime: resuming...\n");
+	ret = slim_ctrl_clk_pause(&dev->ctrl, true, 0);
+	if (ret)
+		dev_err(device, "clk pause not exited:%d", ret);
+	return ret;
+}
+#endif
+
 #ifdef CONFIG_PM_SLEEP
 static int msm_slim_suspend(struct device *dev)
 {
-	return 0;
+	int ret = 0;
+
+	if (!pm_runtime_enabled(dev) ||
+		(!pm_runtime_suspended(dev))) {
+		dev_dbg(dev, "system suspend");
+		ret = msm_slim_runtime_suspend(dev);
+	}
+	if (ret == -EISCONN) {
+		/**
+		* If the clock pause failed due to active channels, there is
+		* a possibility that some audio stream is active during suspend
+		* We dont want to return suspend failure in that case so that
+		* display and relevant components can still go to suspend.
+		* If there is some other error, then it should prevent
+		* system level suspend
+		*/
+		ret = 0;
+	}
+	return ret;
 }
 
 static int msm_slim_resume(struct device *dev)
 {
+	if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
+		int ret;
+
+		dev_dbg(dev, "system resume");
+		ret = msm_slim_runtime_resume(dev);
+		if (!ret) {
+			pm_runtime_mark_last_busy(dev);
+			pm_request_autosuspend(dev);
+		}
+		return ret;
+
+	}
 	return 0;
 }
 #endif /* CONFIG_PM_SLEEP */
 
 static const struct dev_pm_ops msm_slim_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(msm_slim_suspend, msm_slim_resume)
+	SET_RUNTIME_PM_OPS(
+			   msm_slim_runtime_suspend,
+			   msm_slim_runtime_resume,
+			   NULL
+	)
 };
 
 static const struct of_device_id msm_slim_dt_match[] = {
diff --git a/drivers/slimbus/slim-qcom.h b/drivers/slimbus/slim-qcom.h
index 32073fc..f37a10a 100644
--- a/drivers/slimbus/slim-qcom.h
+++ b/drivers/slimbus/slim-qcom.h
@@ -25,6 +25,7 @@
 #define SLIM_ROOT_FREQ 24576000
 #define INIT_MX_RETRIES 10
 #define DEF_RETRY_MS	10
+#define MSM_SLIM_AUTOSUSPEND 1000
 
 /* MAX message size over control channel */
 #define SLIM_MSGQ_BUF_LEN	40
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus
  2015-06-17  1:45 ` [PATCH V2 1/6] SLIMbus: Device management on SLIMbus Sagar Dharia
@ 2015-06-17  3:38   ` Joe Perches
  2015-06-17 13:16   ` Mark Brown
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Joe Perches @ 2015-06-17  3:38 UTC (permalink / raw)
  To: Sagar Dharia
  Cc: gregkh, bp, poeschel, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, davem, james.hogan, michael.opdenacker,
	daniel.thompson, linux-kernel, nkaje, kheitke, mlocke, agross,
	linux-arm-msm

On Tue, 2015-06-16 at 19:45 -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.
[]
> diff --git a/drivers/slimbus/slimbus.c b/drivers/slimbus/slimbus.c
[]
> +static bool slim_eaddr_equal(struct slim_eaddr *a, struct slim_eaddr *b)
> +{
> +	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)
> +{
> +	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)
> +		return 0;
> +
> +	slim_dev = to_slim_device(dev);
> +	if (drv->id_table)
> +		return slim_match(drv->id_table, slim_dev) != NULL;
> +	return 0;
> +}

This should probably be a bool function return.

Maybe this:

static bool slim_device_match(struct device *dev, struct device_driver *driver)
{
	struct slim_driver *drv = to_slim_driver(driver);

	if (dev->type != &slim_dev_type || !drv->id_table)
		return false;

	return slim_match(drv->id_table, to_slim_device(dev));
}

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 2/6] of/slimbus: OF helper for SLIMbus
  2015-06-17  1:46 ` [PATCH V2 2/6] of/slimbus: OF helper for SLIMbus Sagar Dharia
@ 2015-06-17 13:09   ` Mark Brown
  2015-06-17 17:01     ` Sagar Dharia
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2015-06-17 13:09 UTC (permalink / raw)
  To: Sagar Dharia
  Cc: gregkh, bp, poeschel, treding, gong.chen, andreas.noever, alan,
	mathieu.poirier, daniel, oded.gabbay, jkosina, sharon.dvir1, joe,
	davem, james.hogan, michael.opdenacker, daniel.thompson,
	linux-kernel, nkaje, kheitke, mlocke, agross, linux-arm-msm

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

On Tue, Jun 16, 2015 at 07:46:00PM -0600, Sagar Dharia wrote:
> OF helper routine scans the SLIMbus DeviceTree, allocates resources,
> and creates slim_devices according to the hierarchy.

You've not CCed any of the DT maintainers on this, for a completely new
bus it seems like we really ought to get their input.

> @@ -0,0 +1,34 @@
> +SLIM(Serial Low Power Interchip Media Bus) bus
> +
> +SLIMbus is a 2-wire bus, and is used to communicate with peripheral
> +components like audio-codec.
> +
> +Required property for SLIMbus controller node:
> +- compatible	- name of SLIMbus controller.
> +
> +No other properties are required in the SLIMbus controller bus node.

It seems better to just say that the controller is a normal device using
the binding for whatever bus it is on and that the binding is for the
bus which is a child node of the controller device?  Also, do we need
#address-cells or #size-cells here?

> +Required property for SLIMbus child node:
> +enumeration-addr	- 6 byte enumeration address of the slave

The idiom for DT seems to be that we define the bus address using the
reg property.  Should we not be following that pattern here too?  I'd
also expect to see the ability to define a compatible for the slaves.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus
  2015-06-17  1:45 ` [PATCH V2 1/6] SLIMbus: Device management on SLIMbus Sagar Dharia
  2015-06-17  3:38   ` Joe Perches
@ 2015-06-17 13:16   ` Mark Brown
  2015-06-17 17:36     ` Sagar Dharia
  2015-06-18 21:39   ` Srinivas Kandagatla
  2015-06-19  8:22   ` Daniel Thompson
  3 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2015-06-17 13:16 UTC (permalink / raw)
  To: Sagar Dharia
  Cc: gregkh, bp, poeschel, treding, gong.chen, andreas.noever, alan,
	mathieu.poirier, daniel, oded.gabbay, jkosina, sharon.dvir1, joe,
	davem, james.hogan, michael.opdenacker, daniel.thompson,
	linux-kernel, nkaje, kheitke, mlocke, agross, linux-arm-msm

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

On Tue, Jun 16, 2015 at 07:45:59PM -0600, Sagar Dharia wrote:

> +	if (status) {
> +		slim_dev->driver = NULL;
> +	} else if (driver->device_up) {
> +		ctrl = slim_dev->ctrl;
> +		queue_work(ctrl->wq, &slim_dev->wd);
> +	}

Nothing ever cleans this work up if it didn't manage to run or
complete.

> +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;

So we just forget about the device if we don't have a driver for it?

> +	/* check if device-up or down needs to be called */
> +	if ((!sbdev->reported && !sbdev->notified) ||
> +	    (sbdev->reported && sbdev->notified))
> +		return;

No locking here?

> +/**
> + * 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)

My concerns about the split here still remain.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 4/6] slim: qcom: Add Qualcomm Slimbus controller driver
  2015-06-17  1:46 ` [PATCH V2 4/6] slim: qcom: Add Qualcomm Slimbus controller driver Sagar Dharia
@ 2015-06-17 13:53   ` Mark Brown
  2015-06-28 19:13     ` Sagar Dharia
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2015-06-17 13:53 UTC (permalink / raw)
  To: Sagar Dharia
  Cc: gregkh, bp, poeschel, treding, gong.chen, andreas.noever, alan,
	mathieu.poirier, daniel, oded.gabbay, jkosina, sharon.dvir1, joe,
	davem, james.hogan, michael.opdenacker, daniel.thompson,
	linux-kernel, nkaje, kheitke, mlocke, agross, linux-arm-msm

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

On Tue, Jun 16, 2015 at 07:46:02PM -0600, Sagar Dharia wrote:

> + - dmaengine, and pipes used to communicate between controller and memory if
> +	sps-BAM HW is used

This needs more detail.

> +		 */
> +		mb();
> +		if (notify_rx)
> +			complete(&dev->rx_msgq_notify);
> +	}
> +	return IRQ_HANDLED;

This interrupt handler unconditionally returns IRQ_HANDLED regardless of
if that's true or not.

> +static void msm_slim_wait_retry(struct msm_slim_ctrl *dev)
> +{
> +	int msec_per_frm = 0;
> +	int sfr_per_sec;
> +
> +	/* Wait for 1 superframe, or default time and then retry */
> +	sfr_per_sec = dev->framer.superfreq /
> +			(1 << (SLIM_MAX_CLK_GEAR - dev->ctrl.clkgear));
> +	if (sfr_per_sec)
> +		msec_per_frm = MSEC_PER_SEC / sfr_per_sec;
> +	if (msec_per_frm < DEF_RETRY_MS)
> +		msec_per_frm = DEF_RETRY_MS;
> +	msleep(msec_per_frm);
> +}

Is this device specific?

> +static void msm_slim_cb(void *ctx, int err)
> +{
> +	if (err)
> +		pr_err("MSM-slim completion reported err:%d\n", err);

dev_err()?

> +	else if (ctx)
> +		complete(ctx);
> +}

That's weird, if we get an error we don't signal whatever's waiting -
won't it just time out at best then?  Also, what happens if we get
neither an error nor context?

> +	if (txn->msg->wbuf)
> +		memcpy(puc, txn->msg->wbuf, txn->msg->num_bytes);
> +	msm_slim_queue_tx(dev, pbuf, txn->rl, MGR_TX_MSG);
> +	timeout = wait_for_completion_timeout(&done,
> +					      msecs_to_jiffies(txn->rl + 100));
> +
> +	if (!timeout)
> +		dev_err(dev->dev, "TX timed out:MC:0x%x,mt:0x%x\n", txn->mc,
> +			txn->mt);
> +
> +	mutex_unlock(&dev->txn_lock);
> +	return timeout ? 0 : -ETIMEDOUT;
> +}

Shouldn't we provide a route for error reports here (and might some of
this timeout stuff go into the core)?

> +
> +	if ((msm_slim_put_rx(dev, (u8 *)buf)) != -ENODATA) {
> +		len = buf[0] & 0x1F;
> +		mt = (buf[0] >> 5) & 0x7;
> +		mc = buf[1];
> +		if (mt == SLIM_MSG_MT_CORE &&
> +			mc == SLIM_MSG_MC_REPORT_PRESENT) {

Looks like a switch statement.

> +static int msm_slim_rx_msgq_thread(void *data)
> +{
> +	struct msm_slim_ctrl *dev = (struct msm_slim_ctrl *)data;
> +	struct completion *notify = &dev->rx_msgq_notify;
> +	int ret;
> +
> +	while (!kthread_should_stop()) {
> +		set_current_state(TASK_INTERRUPTIBLE);
> +		ret = wait_for_completion_interruptible(notify);
> +
> +		if (ret)
> +			dev_err(dev->dev, "rx thread wait error:%d\n", ret);
> +		else
> +			msm_slim_rxwq(dev);
> +	}
> +
> +	return 0;
> +}

It's not entirely clear to me why this is a kthread rather than a
workqueue or something.  I'm also unclear what happens if more than one
piece of work is queued prior to msm_slim_rxwq() running, it looks like
it only handles a single operation.

> +	/* SLEW RATE register for this slimbus */
> +	dev->slew_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +						"slimbus_slew_reg");
> +	if (!dev->slew_mem) {
> +		dev_err(&pdev->dev, "no slimbus slew resource\n");
> +		return;
> +	}

Warning not an error isn't it?

> +	hclk = clk_get(&pdev->dev, "iface_clk");
> +	if (IS_ERR(hclk))
> +		return PTR_ERR(hclk);

devm_clk_get()

> +	ret = clk_set_rate(rclk, SLIM_ROOT_FREQ);

You're ignoring this error in spite of assigning to ret.

> +	slim_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +						"slimbus_physical");
> +	if (!slim_mem) {
> +		dev_err(&pdev->dev, "no slimbus physical memory resource\n");
> +		ret = ENODEV;
> +		goto err_get_mem_failed;
> +	}
> +	slim_io = request_mem_region(slim_mem->start, resource_size(slim_mem),
> +					pdev->name);
> +	if (!slim_io) {
> +		dev_err(&pdev->dev, "slimbus memory already claimed\n");
> +		ret = EBUSY;
> +		goto err_get_mem_failed;
> +	}

devm_ioremap_resource() and a lot of this starts looking simpler.

> +	dev = kzalloc(sizeof(struct msm_slim_ctrl), GFP_KERNEL);

devm_kzalloc()

> +	if (pdev->dev.of_node) {
> +
> +		ret = of_property_read_u32(pdev->dev.of_node, "cell-index",
> +					&dev->ctrl.nr);

Was that in the binding?

> +	/* Register with framework before enabling frame, clock */
> +	ret = slim_add_numbered_controller(&dev->ctrl);
> +	if (ret) {
> +		dev_err(dev->dev, "error adding controller\n");
> +		goto err_ctrl_failed;
> +	}

This is suspicious - why are you adding a numbered controller with DT?
And looking at this interface why is it a separate call to add a
numbered controller, why not just register the controller using the same
API and handle any number that was set when doing that?

> +	dev->ver = readl_relaxed(dev->base);
> +	/* Version info in 16 MSbits */
> +	dev->ver >>= 16;
> +	/* Component register initialization */
> +	writel_relaxed(1, dev->base + CFG_PORT(COMP_CFG, dev->ver));
> +	writel_relaxed((EE_MGR_RSC_GRP | EE_NGD_2 | EE_NGD_1),
> +				dev->base + CFG_PORT(COMP_TRUST_CFG, dev->ver));

You've registered the device with the core prior to initialising the
hardware - I'd expect this means the generic code will start trying to
register slaves immediately.  The normal pattern would be to initialise
the hardware then register it.

> +#ifdef CONFIG_PM_SLEEP
> +static int msm_slim_suspend(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static int msm_slim_resume(struct device *dev)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_PM_SLEEP */

Omit empty functions.

> +static int msm_slim_init(void)
> +{
> +	return platform_driver_register(&msm_slim_driver);
> +}
> +module_init(msm_slim_init);

module_platform_driver().

> +u8 *msm_slim_get_tx(struct msm_slim_ctrl *dev, struct msm_wr_cb *cur)
> +{
> +	unsigned long flags;
> +	int idx;
> +
> +	spin_lock_irqsave(&dev->tx.lock, flags);
> +	if (((dev->tx.head + 1) % MSM_TX_MSGS) == dev->tx.tail) {
> +		spin_unlock_irqrestore(&dev->tx.lock, flags);
> +		return NULL;
> +	}
> +	idx = dev->tx.tail;
> +	dev->tx.tail = (dev->tx.tail + 1) % MSM_TX_MSGS;
> +	spin_unlock_irqrestore(&dev->tx.lock, flags);
> +	dev->pending_wr[idx] = cur;
> +	return dev->tx.base + (idx * SLIM_MSGQ_BUF_LEN);
> +}

Would just using a regular list hurt?

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 2/6] of/slimbus: OF helper for SLIMbus
  2015-06-17 13:09   ` Mark Brown
@ 2015-06-17 17:01     ` Sagar Dharia
  0 siblings, 0 replies; 16+ messages in thread
From: Sagar Dharia @ 2015-06-17 17:01 UTC (permalink / raw)
  To: Mark Brown
  Cc: gregkh, bp, poeschel, treding, gong.chen, andreas.noever, alan,
	mathieu.poirier, daniel, oded.gabbay, jkosina, sharon.dvir1, joe,
	davem, james.hogan, michael.opdenacker, daniel.thompson,
	linux-kernel, nkaje, kheitke, mlocke, agross, linux-arm-msm

On 6/17/2015 7:09 AM, Mark Brown wrote:
> On Tue, Jun 16, 2015 at 07:46:00PM -0600, Sagar Dharia wrote:
>> OF helper routine scans the SLIMbus DeviceTree, allocates resources,
>> and creates slim_devices according to the hierarchy.
> You've not CCed any of the DT maintainers on this, for a completely new
> bus it seems like we really ought to get their input.
Will do.
>
>> @@ -0,0 +1,34 @@
>> +SLIM(Serial Low Power Interchip Media Bus) bus
>> +
>> +SLIMbus is a 2-wire bus, and is used to communicate with peripheral
>> +components like audio-codec.
>> +
>> +Required property for SLIMbus controller node:
>> +- compatible	- name of SLIMbus controller.
>> +
>> +No other properties are required in the SLIMbus controller bus node.
> It seems better to just say that the controller is a normal device using
> the binding for whatever bus it is on and that the binding is for the
> bus which is a child node of the controller device?  Also, do we need
> #address-cells or #size-cells here?
It will be required if we use 'reg' property for enumeration address per 
my understanding (address-cells: 6, size-cells: 0)
I thought about using 'reg' and it seemed to work as well. Only reason 
for using 'enumeration-addr' was to be closer to what HW calls it.
Looking at other device-trees, (that 'reg' was used for other buses to 
represent slave address), I will document the 'reg' property and 
document it.
Thanks
Sagar
>
>> +Required property for SLIMbus child node:
>> +enumeration-addr	- 6 byte enumeration address of the slave
> The idiom for DT seems to be that we define the bus address using the
> reg property.  Should we not be following that pattern here too?  I'd
> also expect to see the ability to define a compatible for the slaves.


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus
  2015-06-17 13:16   ` Mark Brown
@ 2015-06-17 17:36     ` Sagar Dharia
  0 siblings, 0 replies; 16+ messages in thread
From: Sagar Dharia @ 2015-06-17 17:36 UTC (permalink / raw)
  To: Mark Brown
  Cc: gregkh, bp, poeschel, treding, gong.chen, andreas.noever, alan,
	mathieu.poirier, daniel, oded.gabbay, jkosina, sharon.dvir1, joe,
	davem, james.hogan, michael.opdenacker, daniel.thompson,
	linux-kernel, nkaje, kheitke, mlocke, agross, linux-arm-msm

On 6/17/2015 7:16 AM, Mark Brown wrote:
> On Tue, Jun 16, 2015 at 07:45:59PM -0600, Sagar Dharia wrote:
>
>> +	if (status) {
>> +		slim_dev->driver = NULL;
>> +	} else if (driver->device_up) {
>> +		ctrl = slim_dev->ctrl;
>> +		queue_work(ctrl->wq, &slim_dev->wd);
>> +	}
> Nothing ever cleans this work up if it didn't manage to run or
> complete.
>
>> +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;
> So we just forget about the device if we don't have a driver for it?
If device comes up before driver (e.g. dynamic module driver, or 
late-init driver), then to begin with: this doesn't do anything.
But once the driver-binding happens and probe succeeds, we queue 
device_up again.
>
>> +	/* check if device-up or down needs to be called */
>> +	if ((!sbdev->reported && !sbdev->notified) ||
>> +	    (sbdev->reported && sbdev->notified))
>> +		return;
> No locking here?
You are right, first I thought this was only touched in workqueue 
(singlethreaded), but this is also touched from logical-address 
assignment, driver removal etc.
I will add device-level lock for these.
Thanks
Sagar
>
>> +/**
>> + * 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)
> My concerns about the split here still remain.


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus
  2015-06-17  1:45 ` [PATCH V2 1/6] SLIMbus: Device management on SLIMbus Sagar Dharia
  2015-06-17  3:38   ` Joe Perches
  2015-06-17 13:16   ` Mark Brown
@ 2015-06-18 21:39   ` Srinivas Kandagatla
  2015-06-19  8:22   ` Daniel Thompson
  3 siblings, 0 replies; 16+ messages in thread
From: Srinivas Kandagatla @ 2015-06-18 21:39 UTC (permalink / raw)
  To: Sagar Dharia, gregkh, bp, poeschel, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, joe, davem, james.hogan,
	michael.opdenacker, daniel.thompson, linux-kernel
  Cc: nkaje, kheitke, mlocke, agross, linux-arm-msm



On 17/06/15 02:45, 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>

I think I was not as fast as you sent the v2 :-)
However my comments on v1 patcset mostly still applies to v2 as well.

--srini

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 1/6] SLIMbus: Device management on SLIMbus
  2015-06-17  1:45 ` [PATCH V2 1/6] SLIMbus: Device management on SLIMbus Sagar Dharia
                     ` (2 preceding siblings ...)
  2015-06-18 21:39   ` Srinivas Kandagatla
@ 2015-06-19  8:22   ` Daniel Thompson
  3 siblings, 0 replies; 16+ messages in thread
From: Daniel Thompson @ 2015-06-19  8:22 UTC (permalink / raw)
  To: Sagar Dharia, gregkh, bp, poeschel, treding, broonie, gong.chen,
	andreas.noever, alan, mathieu.poirier, daniel, oded.gabbay,
	jkosina, sharon.dvir1, joe, davem, james.hogan,
	michael.opdenacker, linux-kernel
  Cc: nkaje, kheitke, mlocke, agross, linux-arm-msm

On 17/06/15 02:45, Sagar Dharia wrote:
> --- /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.

Perhaps this is a fussy comment but this description of slimbus makes 
pretty heavy use of mobile phone jargon. Admittedly it is jargon from 
the slimbus spec which, as standards go, seems rather myopic about its 
potential user base.

However, even the standards own self-description doesn't limit its role 
to baseband <-> peripherals. What happened to the application 
processor/main-SoC?

Also would an "If unsure, choose N." be useful? It could be quite a long 
time before someone who doesn't know they need slimbus actually finds 
that they do.


Daniel.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH V2 4/6] slim: qcom: Add Qualcomm Slimbus controller driver
  2015-06-17 13:53   ` Mark Brown
@ 2015-06-28 19:13     ` Sagar Dharia
  0 siblings, 0 replies; 16+ messages in thread
From: Sagar Dharia @ 2015-06-28 19:13 UTC (permalink / raw)
  To: Mark Brown
  Cc: gregkh, bp, poeschel, treding, gong.chen, andreas.noever, alan,
	mathieu.poirier, daniel, oded.gabbay, jkosina, sharon.dvir1, joe,
	davem, james.hogan, michael.opdenacker, daniel.thompson,
	linux-kernel, nkaje, kheitke, mlocke, agross, linux-arm-msm

On 6/17/2015 7:53 AM, Mark Brown wrote:
> On Tue, Jun 16, 2015 at 07:46:02PM -0600, Sagar Dharia wrote:
>
>> + - dmaengine, and pipes used to communicate between controller and memory if
>> +	sps-BAM HW is used
> This needs more detail.
>> +		 */
>> +		mb();
>> +		if (notify_rx)
>> +			complete(&dev->rx_msgq_notify);
>> +	}
>> +	return IRQ_HANDLED;
> This interrupt handler unconditionally returns IRQ_HANDLED regardless of
> if that's true or not.

Some interrupt handling is done in ISR itself. RX-msgq thread 
notifier/workQ is only used for items that can sleep/operations that can 
take longer. (e.g. assignment of logical address based on report_present 
message)
That's the reason IRQ_HANDLED is always true here.
>
>> +static void msm_slim_wait_retry(struct msm_slim_ctrl *dev)
>> +{
>> +	int msec_per_frm = 0;
>> +	int sfr_per_sec;
>> +
>> +	/* Wait for 1 superframe, or default time and then retry */
>> +	sfr_per_sec = dev->framer.superfreq /
>> +			(1 << (SLIM_MAX_CLK_GEAR - dev->ctrl.clkgear));
>> +	if (sfr_per_sec)
>> +		msec_per_frm = MSEC_PER_SEC / sfr_per_sec;
>> +	if (msec_per_frm < DEF_RETRY_MS)
>> +		msec_per_frm = DEF_RETRY_MS;
>> +	msleep(msec_per_frm);
>> +}
> Is this device specific?

This is seen on most of the Qualcomm-controllers I've worked with, where 
retries in early initialization were needed to avoid issues due to noise.
>
>> +static void msm_slim_cb(void *ctx, int err)
>> +{
>> +	if (err)
>> +		pr_err("MSM-slim completion reported err:%d\n", err);
> dev_err()?
>
>> +	else if (ctx)
>> +		complete(ctx);
>> +}
> That's weird, if we get an error we don't signal whatever's waiting -
> won't it just time out at best then?  Also, what happens if we get
> neither an error nor context?
>
>> +	if (txn->msg->wbuf)
>> +		memcpy(puc, txn->msg->wbuf, txn->msg->num_bytes);
>> +	msm_slim_queue_tx(dev, pbuf, txn->rl, MGR_TX_MSG);
>> +	timeout = wait_for_completion_timeout(&done,
>> +					      msecs_to_jiffies(txn->rl + 100));
>> +
>> +	if (!timeout)
>> +		dev_err(dev->dev, "TX timed out:MC:0x%x,mt:0x%x\n", txn->mc,
>> +			txn->mt);
>> +
>> +	mutex_unlock(&dev->txn_lock);
>> +	return timeout ? 0 : -ETIMEDOUT;
>> +}
> Shouldn't we provide a route for error reports here (and might some of
> this timeout stuff go into the core)?

Sure, good point. If I move it to core, all controllers will be expected 
to have 'write-done' notification. If that's a fair assumption, I will 
move it to core.
That will also mean the above callback/completion will move to framework.
>
>> +
>> +	if ((msm_slim_put_rx(dev, (u8 *)buf)) != -ENODATA) {
>> +		len = buf[0] & 0x1F;
>> +		mt = (buf[0] >> 5) & 0x7;
>> +		mc = buf[1];
>> +		if (mt == SLIM_MSG_MT_CORE &&
>> +			mc == SLIM_MSG_MC_REPORT_PRESENT) {
> Looks like a switch statement.
>
>> +static int msm_slim_rx_msgq_thread(void *data)
>> +{
>> +	struct msm_slim_ctrl *dev = (struct msm_slim_ctrl *)data;
>> +	struct completion *notify = &dev->rx_msgq_notify;
>> +	int ret;
>> +
>> +	while (!kthread_should_stop()) {
>> +		set_current_state(TASK_INTERRUPTIBLE);
>> +		ret = wait_for_completion_interruptible(notify);
>> +
>> +		if (ret)
>> +			dev_err(dev->dev, "rx thread wait error:%d\n", ret);
>> +		else
>> +			msm_slim_rxwq(dev);
>> +	}
>> +
>> +	return 0;
>> +}
> It's not entirely clear to me why this is a kthread rather than a
> workqueue or something.  I'm also unclear what happens if more than one
> piece of work is queued prior to msm_slim_rxwq() running, it looks like
> it only handles a single operation.

Sure, I will change this to workqueue. That should not be a problem.
>
>> +	/* SLEW RATE register for this slimbus */
>> +	dev->slew_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> +						"slimbus_slew_reg");
>> +	if (!dev->slew_mem) {
>> +		dev_err(&pdev->dev, "no slimbus slew resource\n");
>> +		return;
>> +	}
> Warning not an error isn't it?
>
>> +	hclk = clk_get(&pdev->dev, "iface_clk");
>> +	if (IS_ERR(hclk))
>> +		return PTR_ERR(hclk);
> devm_clk_get()
>
>> +	ret = clk_set_rate(rclk, SLIM_ROOT_FREQ);
> You're ignoring this error in spite of assigning to ret.
>
>> +	slim_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
>> +						"slimbus_physical");
>> +	if (!slim_mem) {
>> +		dev_err(&pdev->dev, "no slimbus physical memory resource\n");
>> +		ret = ENODEV;
>> +		goto err_get_mem_failed;
>> +	}
>> +	slim_io = request_mem_region(slim_mem->start, resource_size(slim_mem),
>> +					pdev->name);
>> +	if (!slim_io) {
>> +		dev_err(&pdev->dev, "slimbus memory already claimed\n");
>> +		ret = EBUSY;
>> +		goto err_get_mem_failed;
>> +	}
> devm_ioremap_resource() and a lot of this starts looking simpler.
>
>> +	dev = kzalloc(sizeof(struct msm_slim_ctrl), GFP_KERNEL);
> devm_kzalloc()
>
>> +	if (pdev->dev.of_node) {
>> +
>> +		ret = of_property_read_u32(pdev->dev.of_node, "cell-index",
>> +					&dev->ctrl.nr);
> Was that in the binding?
>
>> +	/* Register with framework before enabling frame, clock */
>> +	ret = slim_add_numbered_controller(&dev->ctrl);
>> +	if (ret) {
>> +		dev_err(dev->dev, "error adding controller\n");
>> +		goto err_ctrl_failed;
>> +	}
> This is suspicious - why are you adding a numbered controller with DT?
> And looking at this interface why is it a separate call to add a
> numbered controller, why not just register the controller using the same
> API and handle any number that was set when doing that?

Agreed, I will change add_numbered_controller to register_controller. 
That will also mean the 'cell-index' property above is not needed.
>
>> +	dev->ver = readl_relaxed(dev->base);
>> +	/* Version info in 16 MSbits */
>> +	dev->ver >>= 16;
>> +	/* Component register initialization */
>> +	writel_relaxed(1, dev->base + CFG_PORT(COMP_CFG, dev->ver));
>> +	writel_relaxed((EE_MGR_RSC_GRP | EE_NGD_2 | EE_NGD_1),
>> +				dev->base + CFG_PORT(COMP_TRUST_CFG, dev->ver));
> You've registered the device with the core prior to initialising the
> hardware - I'd expect this means the generic code will start trying to
> register slaves immediately.  The normal pattern would be to initialise
> the hardware then register it.

This is for handling the logical-address assignment. That's only 
possible after registering controller.
The internal controller has 2 in-built slimbus devices (interface, 
framer) which will start to report-present right-away if I initialize 
the HW. Now if register_controller is not done, the logical-address 
assignment will not happen and as per spec, the devices will keep 
sending report_present until they get an assignment.

Registering slaves immediately is not an issue since that part will not 
be waiting on logical-address assignment (due to device_up callback)
>
>> +#ifdef CONFIG_PM_SLEEP
>> +static int msm_slim_suspend(struct device *dev)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int msm_slim_resume(struct device *dev)
>> +{
>> +	return 0;
>> +}
>> +#endif /* CONFIG_PM_SLEEP */
> Omit empty functions.
>
>> +static int msm_slim_init(void)
>> +{
>> +	return platform_driver_register(&msm_slim_driver);
>> +}
>> +module_init(msm_slim_init);
> module_platform_driver().
>
>> +u8 *msm_slim_get_tx(struct msm_slim_ctrl *dev, struct msm_wr_cb *cur)
>> +{
>> +	unsigned long flags;
>> +	int idx;
>> +
>> +	spin_lock_irqsave(&dev->tx.lock, flags);
>> +	if (((dev->tx.head + 1) % MSM_TX_MSGS) == dev->tx.tail) {
>> +		spin_unlock_irqrestore(&dev->tx.lock, flags);
>> +		return NULL;
>> +	}
>> +	idx = dev->tx.tail;
>> +	dev->tx.tail = (dev->tx.tail + 1) % MSM_TX_MSGS;
>> +	spin_unlock_irqrestore(&dev->tx.lock, flags);
>> +	dev->pending_wr[idx] = cur;
>> +	return dev->tx.base + (idx * SLIM_MSGQ_BUF_LEN);
>> +}
> Would just using a regular list hurt?

This provides O(1) circular buffer access rather than a list with O(n).
I will address other comments and upload patch later today/tomorrow.

Thanks for your comments
Sagar


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-06-28 19:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-17  1:45 [PATCH V2 0/6] Introduce framework for SLIMbus device drivers Sagar Dharia
2015-06-17  1:45 ` [PATCH V2 1/6] SLIMbus: Device management on SLIMbus Sagar Dharia
2015-06-17  3:38   ` Joe Perches
2015-06-17 13:16   ` Mark Brown
2015-06-17 17:36     ` Sagar Dharia
2015-06-18 21:39   ` Srinivas Kandagatla
2015-06-19  8:22   ` Daniel Thompson
2015-06-17  1:46 ` [PATCH V2 2/6] of/slimbus: OF helper for SLIMbus Sagar Dharia
2015-06-17 13:09   ` Mark Brown
2015-06-17 17:01     ` Sagar Dharia
2015-06-17  1:46 ` [PATCH V2 3/6] slimbus: Add messaging APIs to slimbus framework Sagar Dharia
2015-06-17  1:46 ` [PATCH V2 4/6] slim: qcom: Add Qualcomm Slimbus controller driver Sagar Dharia
2015-06-17 13:53   ` Mark Brown
2015-06-28 19:13     ` Sagar Dharia
2015-06-17  1:46 ` [PATCH V2 5/6] slimbus: Add support for 'clock-pause' feature Sagar Dharia
2015-06-17  1:46 ` [PATCH V2 6/6] slim: qcom: Add runtime-pm support using clock-pause feature Sagar Dharia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).