linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [2/3][PATCH][v2] TDM Framework
@ 2012-07-27 14:05 sandeep
  2012-07-27 14:05 ` [1/3][PATCH][v2] Adding documentation for TDM sandeep
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: sandeep @ 2012-07-27 14:05 UTC (permalink / raw)
  To: linuxppc-dev, linux-arm-kernel; +Cc: devel, Sandeep Singh, linux-kernel

From: Sandeep Singh <Sandeep@freescale.com>

TDM Framework is an attempt to provide a platform independent layer which can
offer a standard interface  for TDM access to different client modules.
Beneath, the framework layer can house different types of TDM drivers to handle
various TDM devices, the hardware intricacies of the devices being completely
taken care by TDM drivers.
This framework layer will allow any type of TDM device to hook with it.
For example Freescale controller as on MPC8315, UCC based TDM controller, etc

The main functions of this Framework are:
 - provides interface to TDM clients to access TDM functionalities.
 - provides standard interface for TDM drivers to hook with the framework.
 - handles various data handling stuff and buffer management.

In future this Framework will be extended to provide Interface for Line control devices also. For example SLIC, E1/T1 Framers etc.

Presently the framework supports only Single Port channelised mode.
Also the configurability options are limited which will be extended later on.
Only kernel mode TDM clients are supported currently. Support for User mode clients will be added later.

Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
Signed-off-by: Hemant Agrawal <hemant@freescale.com> 
Signed-off-by: Rajesh Gumasta <rajesh.gumasta@freescale.com> 
---
Based on: git://git.am.freescale.net/gitolite/mirrors/galak-powerpc.git
Branch: master
Checkpatch: passed

Changes since v1:
		Incorporated Timur's comments:
		- Removed unnecessary includes
		- Comment format and error prints are more homogenous now.
		- Removed unnecessary declarations.
		- Corrected return code value
		- Called missing module_put()

Changes since RFC:
	- Since all read/write operations are in TDM are channel based, polling
	on TDM channel for data instead of TDM port before going for read/write.
	- Also corrected a faulty error leg
	- Removed unused function tdm_port_get_wait_queue.

	Incorporated Scott's comments:
	- Removed TDM_CORE_DEBUG.
	- Added sysfs knob to change use_latest_tdm_data at runtime.
	- Works more silently now (lesser prints).
	- Cosmetic errors rectified.
	- Removed unused function tdm_init.
	- Removed unused variables.
	- Removed unnecessary typecast and NULL check.
	- Removed #include "device/tdm_fsl.h".

	Incorporated Timur's comments:
	- Handled errors.
	- Used dev_err instead of pr_err
	- Removed extern from function declaration.

 drivers/Kconfig                 |    1 +
 drivers/Makefile                |    1 +
 drivers/tdm/Kconfig             |   18 +
 drivers/tdm/Makefile            |    5 +
 drivers/tdm/tdm-core.c          | 1087 +++++++++++++++++++++++++++++++++++++++
 include/linux/mod_devicetable.h |   11 +
 include/linux/tdm.h             |  389 ++++++++++++++
 7 files changed, 1512 insertions(+), 0 deletions(-)
 create mode 100644 drivers/tdm/Kconfig
 create mode 100644 drivers/tdm/Makefile
 create mode 100644 drivers/tdm/tdm-core.c
 create mode 100644 include/linux/tdm.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 5afe5d1..abd6c83 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -136,4 +136,5 @@ source "drivers/virt/Kconfig"
 
 source "drivers/devfreq/Kconfig"
 
+source "drivers/tdm/Kconfig"
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 1b31421..7cb88e3 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -104,6 +104,7 @@ obj-$(CONFIG_INFINIBAND)	+= infiniband/
 obj-$(CONFIG_SGI_SN)		+= sn/
 obj-y				+= firmware/
 obj-$(CONFIG_CRYPTO)		+= crypto/
+obj-$(CONFIG_TDM)		+= tdm/
 obj-$(CONFIG_SUPERH)		+= sh/
 obj-$(CONFIG_ARCH_SHMOBILE)	+= sh/
 ifndef CONFIG_ARCH_USES_GETTIMEOFFSET
diff --git a/drivers/tdm/Kconfig b/drivers/tdm/Kconfig
new file mode 100644
index 0000000..0b0fda8
--- /dev/null
+++ b/drivers/tdm/Kconfig
@@ -0,0 +1,18 @@
+#
+# TDM subsystem configuration
+#
+
+menuconfig TDM
+	tristate "TDM support"
+	---help---
+	  More information is contained in the directory <file:Documentation/tdm/>,
+	  especially in the file called "summary" there.
+	  If you want TDM support, you should say Y here and also to the
+	  specific driver for your bus adapter(s) below.
+
+	  This TDM support can also be built as a module.  If so, the module
+	  will be called tdm-core.
+
+if TDM
+
+endif # TDM
diff --git a/drivers/tdm/Makefile b/drivers/tdm/Makefile
new file mode 100644
index 0000000..84e2cb9
--- /dev/null
+++ b/drivers/tdm/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the TDM core.
+#
+
+obj-$(CONFIG_TDM)		+= tdm-core.o
diff --git a/drivers/tdm/tdm-core.c b/drivers/tdm/tdm-core.c
new file mode 100644
index 0000000..c633190
--- /dev/null
+++ b/drivers/tdm/tdm-core.c
@@ -0,0 +1,1087 @@
+/*
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * TDM core is the interface between TDM clients and TDM devices.
+ * It is also intended to serve as an interface for line controlled
+ * devices later on.
+ *
+ * Author:Hemant Agrawal <hemant@freescale.com>
+ *	Rajesh Gumasta <rajesh.gumasta@freescale.com>
+ *
+ * Note that some parts of this code may have been derived from i2c subsystem.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the  GNU General Public License along
+ * with this program; if not, write  to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+
+#include <linux/slab.h>
+#include <linux/tdm.h>
+#include <linux/idr.h>
+
+static DEFINE_MUTEX(tdm_core_lock);
+static DEFINE_IDR(tdm_adapter_idr);
+/* List of TDM adapters registered with TDM framework */
+LIST_HEAD(adapter_list);
+
+/* List of TDM clients registered with TDM framework */
+LIST_HEAD(driver_list);
+
+/*
+ * In case the previous data is not fetched by the client driver, the
+ * de-interleaving function will  discard the old data and rewrite the
+ * new data
+ */
+
+static int use_latest_tdm_data = 1;
+
+/* Data structures required for sysfs */
+static struct tdm_sysfs attr = {
+	.attr.name = "use_latest_data",
+	.attr.mode = 0664,
+	.cmd_type = TDM_LATEST_DATA,
+};
+
+static struct attribute *tdm_attr[] = {
+	&attr.attr,
+	NULL
+};
+
+const struct sysfs_ops tdm_ops = {
+	.show = tdm_show_sysfs,
+	.store = tdm_store_sysfs,
+};
+
+static struct kobj_type tdm_type = {
+	.sysfs_ops = &tdm_ops,
+	.default_attrs = tdm_attr,
+};
+
+/* tries to match client driver with the adapter */
+static int tdm_device_match(struct tdm_driver *driver, struct tdm_adapter *adap)
+{
+	/* match on an id table if there is one */
+	if (driver->id_table && driver->id_table->name[0]) {
+		if (!(strcmp(driver->id_table->name, adap->name)))
+			return (int)driver->id_table;
+	}
+	return 0;
+}
+
+static int tdm_attach_driver_adap(struct tdm_driver *driver,
+		struct tdm_adapter *adap)
+{
+	int ret = 0;
+	/* if driver is already attached to any other adapter, return*/
+	if (driver->adapter && (driver->adapter != adap))
+		return 0;
+
+	driver->adapter = adap;
+
+	if (driver->attach_adapter) {
+		ret = driver->attach_adapter(adap);
+		if (ret < 0) {
+			pr_err("tdm: attach_adapter failed for driver [%s]"
+					"err:%d\n", driver->name, ret);
+			return ret;
+		}
+	}
+	adap->drv_count++;
+
+	if (!adap->tasklet_conf) {
+		tdm_sysfs_init();
+		tasklet_init(&adap->tdm_data_tasklet, tdm_data_tasklet_fn,
+				(unsigned long)adap);
+		adap->tasklet_conf = 1;
+	}
+
+	return ret;
+}
+
+/* Detach client driver and adapter */
+static int tdm_detach_driver_adap(struct tdm_driver *driver,
+		struct tdm_adapter *adap)
+{
+	int res = 0;
+
+	if (!driver->adapter || (driver->adapter != adap))
+		return 0;
+
+	adap->drv_count--;
+
+	/* If no more driver is registed with the adapter*/
+	if (!adap->drv_count && adap->tasklet_conf) {
+		tasklet_disable(&adap->tdm_data_tasklet);
+		tasklet_kill(&adap->tdm_data_tasklet);
+		adap->tasklet_conf = 0;
+	}
+
+	if (driver->detach_adapter) {
+		if (driver->detach_adapter(adap))
+			pr_err("tdm: detach_adapter failed for driver [%s]\n",
+					driver->name);
+	}
+
+	driver->adapter = NULL;
+	return res;
+}
+
+/* TDM adapter Registration/De-registration with TDM framework */
+
+static int tdm_register_adapter(struct tdm_adapter *adap)
+{
+	int res = 0;
+	struct tdm_driver *driver, *next;
+
+	mutex_init(&adap->adap_lock);
+	INIT_LIST_HEAD(&adap->myports);
+	spin_lock_init(&adap->portlist_lock);
+
+	adap->drv_count = 0;
+	adap->tasklet_conf = 0;
+
+	list_add_tail(&adap->list, &adapter_list);
+
+	/* initialization of driver by framework in default configuration */
+	init_config_adapter(adap);
+
+	/* Notify drivers */
+	pr_info("adapter [%s] registered\n", adap->name);
+	mutex_lock(&tdm_core_lock);
+	list_for_each_entry_safe(driver, next, &driver_list, list) {
+		if (tdm_device_match(driver, adap)) {
+			res = tdm_attach_driver_adap(driver, adap);
+			if (res == 0) {
+				pr_info("tdm: Driver(ID=%d) is "
+						"attached with Adapter %s(ID"
+						" = %d)\n", driver->id,
+						adap->name, adap->id);
+			} else {
+				pr_err("tdm: Driver(ID=%d) is unable "
+						"to attach with Adapter %s(ID = %d)\n",
+						driver->id, adap->name,
+						adap->id);
+			}
+		}
+	}
+	mutex_unlock(&tdm_core_lock);
+
+	return res;
+}
+
+/*
+ * tdm_add_adapter - declare tdm adapter, use dynamic device number
+ * @adapter: the adapter to add
+ * Context: can sleep
+ *
+ * This routine is used to declare a TDM adapter
+ * When this returns zero, a new device number will be allocated and stored
+ * in adap->id, and the specified adapter became available for the clients.
+ * Otherwise, a negative error number value is returned.
+ */
+int tdm_add_adapter(struct tdm_adapter *adapter)
+{
+	int id, res = 0;
+
+retry:
+	if (idr_pre_get(&tdm_adapter_idr, GFP_KERNEL) == 0)
+		return -ENOMEM;
+
+	mutex_lock(&tdm_core_lock);
+	res = idr_get_new(&tdm_adapter_idr, adapter, &id);
+	mutex_unlock(&tdm_core_lock);
+
+	if (res < 0) {
+		if (res == -EAGAIN)
+			goto retry;
+		return res;
+	}
+
+	adapter->id = id;
+	return tdm_register_adapter(adapter);
+}
+EXPORT_SYMBOL(tdm_add_adapter);
+
+
+/*
+ * tdm_del_adapter - unregister TDM adapter
+ * @adap: the adapter being unregistered
+ *
+ * This unregisters an TDM adapter which was previously registered
+ * by @tdm_add_adapter.
+ */
+int tdm_del_adapter(struct tdm_adapter *adap)
+{
+	int res = 0;
+	struct tdm_adapter *found;
+	struct tdm_driver *driver, *next;
+
+	/* First make sure that this adapter was ever added */
+	mutex_lock(&tdm_core_lock);
+	found = idr_find(&tdm_adapter_idr, adap->id);
+	mutex_unlock(&tdm_core_lock);
+	if (found != adap) {
+		pr_err("tdm: attempting to delete unregistered "
+				"adapter [%s]\n", adap->name);
+		return -EINVAL;
+	}
+
+	/* disable and kill the data processing tasklet */
+	if (adap->tasklet_conf) {
+		tasklet_disable(&adap->tdm_data_tasklet);
+		tasklet_kill(&adap->tdm_data_tasklet);
+		adap->tasklet_conf = 0;
+	}
+
+	/*
+	 * Detach any active ports. This can't fail, thus we do not
+	 * checking the returned value.
+	 */
+	mutex_lock(&tdm_core_lock);
+	list_for_each_entry_safe(driver, next, &driver_list, list) {
+		if (tdm_device_match(driver, adap)) {
+			tdm_detach_driver_adap(driver, adap);
+			pr_info(
+					"Driver(ID=%d) is detached from Adapter %s(ID = %d)\n",
+					driver->id, adap->name, adap->id);
+		}
+	}
+	idr_remove(&tdm_adapter_idr, adap->id);
+	mutex_unlock(&tdm_core_lock);
+
+	pr_debug("adapter [%s] unregistered\n", adap->name);
+
+	list_del(&adap->list);
+	/*
+	 * Clear the device structure in case this adapter is ever going to be
+	 * added again
+	 */
+	adap->parent = NULL;
+
+	return res;
+}
+EXPORT_SYMBOL(tdm_del_adapter);
+
+/* TDM Client Drivers Registration/De-registration Functions */
+int tdm_register_driver(struct tdm_driver *driver)
+{
+	int res = 0;
+	struct tdm_adapter *adap, *next;
+
+	list_add_tail(&driver->list, &driver_list);
+
+	mutex_lock(&tdm_core_lock);
+	/* Walk the adapters that are already present */
+	list_for_each_entry_safe(adap, next, &adapter_list, list) {
+		if (tdm_device_match(driver, adap)) {
+			res = tdm_attach_driver_adap(driver, adap);
+			if (res == 0) {
+				pr_info("TDM Driver(ID=%d)is attached with "
+						"Adapter%s(ID = %d) drv_count=%d",
+						driver->id, adap->name,
+						adap->id, adap->drv_count);
+			} else {
+				pr_err("TDM Driver(ID=%d) unable to attach "
+						"to Adapter%s(ID = %d) drv_count=%d",
+						driver->id, adap->name,
+						adap->id, adap->drv_count);
+			}
+			break;
+		}
+	}
+	mutex_unlock(&tdm_core_lock);
+
+	return res;
+}
+EXPORT_SYMBOL(tdm_register_driver);
+
+/*
+ * tdm_unregister_driver - unregister TDM client driver from TDM framework
+ * @driver: the driver being unregistered
+ */
+void tdm_unregister_driver(struct tdm_driver *driver)
+{
+	/*
+	 * A driver can register to only one adapter,
+	 * so no need to browse the list
+	 */
+	mutex_lock(&tdm_core_lock);
+	tdm_detach_driver_adap(driver, driver->adapter);
+	mutex_unlock(&tdm_core_lock);
+
+	list_del(&driver->list);
+
+	pr_debug("tdm-core: driver [%s] unregistered\n", driver->name);
+}
+EXPORT_SYMBOL(tdm_unregister_driver);
+
+/* Interface to the tdm device/adapter */
+
+/*
+ * tdm_adap_send - issue a TDM write
+ * @adap: Handle to TDM device
+ * @buf: Data that will be written to the TDM device
+ * @count: How many bytes to write
+ *
+ * Returns negative errno, or else the number of bytes written.
+ */
+int tdm_adap_send(struct tdm_adapter *adap, void **buf, int count)
+{
+	int res;
+
+	if (adap->algo->tdm_write)
+		res = adap->algo->tdm_write(adap, buf, count);
+	else {
+		pr_err("TDM level write not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	/*
+	 * If everything went ok (i.e. frame transmitted), return #bytes
+	 * transmitted, else error code.
+	 */
+	return (res == 1) ? count : res;
+}
+EXPORT_SYMBOL(tdm_adap_send);
+
+/*
+ * tdm_adap_recv - issue a TDM read
+ * @adap: Handle to TDM device
+ * @buf: Where to store data read from TDM device
+ *
+ * Returns negative errno, or else the number of bytes read.
+ */
+int tdm_adap_recv(struct tdm_adapter *adap, void **buf)
+{
+	int res;
+
+	if (adap->algo->tdm_read)
+		res = adap->algo->tdm_read(adap, (u16 **)buf);
+	else {
+		pr_err("TDM level read not supported\n");
+		return -EOPNOTSUPP;
+	}
+	/*
+	 * If everything went ok (i.e. frame received), return #bytes
+	 * transmitted, else error code.
+	 */
+	return res;
+}
+
+/*
+ * tdm_adap_get_write_buf - get next write TDM device buffer
+ * @adap: Handle to TDM device
+ * @buf: pointer to TDM device buffer
+ *
+ * Returns negative errno, or else size of the write buffer.
+ */
+int tdm_adap_get_write_buf(struct tdm_adapter *adap, void **buf)
+{
+	int res;
+
+	if (adap->algo->tdm_get_write_buf) {
+		res = adap->algo->tdm_get_write_buf(adap, (u16 **)buf);
+	} else {
+		pr_err("TDM level write buf get not supported\n");
+		return -EOPNOTSUPP;
+	}
+	/*
+	 * If everything went ok (i.e. 1 msg received), return #bytes
+	 * transmitted, else error code.
+	 */
+	return res;
+}
+EXPORT_SYMBOL(tdm_adap_get_write_buf);
+
+int tdm_adap_enable(struct tdm_driver *drv)
+{
+	int res;
+	struct tdm_adapter *adap;
+	adap = drv->adapter;
+
+	if (adap->algo->tdm_enable) {
+		res = adap->algo->tdm_enable(adap);
+	} else {
+		pr_err("TDM level enable not supported\n");
+		return -EOPNOTSUPP;
+	}
+	return res;
+}
+EXPORT_SYMBOL(tdm_adap_enable);
+
+int tdm_adap_disable(struct tdm_driver *drv)
+{
+	int res;
+	struct tdm_adapter *adap;
+	adap = drv->adapter;
+
+	if (adap->algo->tdm_disable) {
+		res = adap->algo->tdm_disable(adap);
+	} else {
+		pr_err("TDM level enable not supported\n");
+		return -EOPNOTSUPP;
+	}
+	return res;
+}
+EXPORT_SYMBOL(tdm_adap_disable);
+
+struct tdm_adapter *tdm_get_adapter(int id)
+{
+	struct tdm_adapter *adapter;
+
+	mutex_lock(&tdm_core_lock);
+	adapter = idr_find(&tdm_adapter_idr, id);
+	if (adapter && !try_module_get(adapter->owner))
+		adapter = NULL;
+
+	mutex_unlock(&tdm_core_lock);
+
+	return adapter;
+}
+EXPORT_SYMBOL(tdm_get_adapter);
+
+void tdm_put_adapter(struct tdm_adapter *adap)
+{
+	module_put(adap->owner);
+}
+EXPORT_SYMBOL(tdm_put_adapter);
+
+
+/* Port Level APIs of TDM Framework */
+int tdm_port_open(struct tdm_driver *driver, struct tdm_port **h_port)
+{
+	struct tdm_port *port;
+	struct tdm_adapter *adap;
+	unsigned long flags;
+	int res = 0;
+
+	/*
+	 * This creates an anonymous tdm_port, which may later be
+	 * pointed to some slot.
+	 */
+	port = kzalloc(sizeof(*port), GFP_KERNEL);
+	if (!port) {
+		res = -ENOMEM;
+		return res;
+	}
+
+	adap = tdm_get_adapter(driver->adapter->id);
+	if (!adap)
+		return -ENODEV;
+
+	port->rx_max_frames = NUM_SAMPLES_PER_FRAME;
+	port->port_cfg.port_mode = TDM_PORT_CHANNELIZED;
+
+	snprintf(driver->name, TDM_NAME_SIZE, "tdm-dev");
+	port->driver = driver;
+	port->adapter = adap;
+
+	spin_lock_irqsave(&adap->portlist_lock, flags);
+	list_add_tail(&port->list, &adap->myports);
+	spin_unlock_irqrestore(&adap->portlist_lock, flags);
+
+	INIT_LIST_HEAD(&port->mychannels);
+
+	*h_port = port;
+	return res;
+
+}
+EXPORT_SYMBOL(tdm_port_open);
+
+int tdm_port_close(struct tdm_port *h_port)
+{
+	struct tdm_adapter *adap;
+	struct tdm_driver *driver;
+	struct tdm_port *port;
+	struct tdm_channel *temp, *channel;
+	unsigned long flags;
+	int res = 0;
+	port = h_port;
+
+	driver =  port->driver;
+
+	list_for_each_entry_safe(channel, temp, &port->mychannels, list) {
+		if (channel)
+			if (channel->in_use) {
+				pr_err("tdm: Cannot close port. Channel in"
+						"use\n");
+				res = -ENXIO;
+				goto out;
+			}
+	}
+	adap = driver->adapter;
+	tdm_put_adapter(adap);
+
+	spin_lock_irqsave(&adap->portlist_lock, flags);
+	list_del(&port->list);
+	spin_unlock_irqrestore(&adap->portlist_lock, flags);
+
+	if (port->p_port_data != NULL) {
+		int i;
+		struct tdm_bd *ch_bd;
+
+		/*
+		 * If the tdm is in channelised mode,
+		 * de-allocate the channelised buffer
+		 */
+		ch_bd = &(port->p_port_data->rx_data_fifo[0]);
+		for (i = 0; ch_bd && i < TDM_CH_RX_BD_RING_SIZE; i++) {
+			ch_bd->flag = 0;
+			ch_bd++;
+		}
+		ch_bd = &(port->p_port_data->tx_data_fifo[0]);
+		for (i = 0; ch_bd && i < TDM_CH_TX_BD_RING_SIZE; i++) {
+			ch_bd->flag = 0;
+			ch_bd++;
+		}
+		kfree(port->p_port_data);
+	}
+	kfree(port);
+	return res;
+out:
+	if (port)
+		kfree(port->p_port_data);
+	kfree(port);
+	return res;
+}
+EXPORT_SYMBOL(tdm_port_close);
+
+int tdm_channel_read(struct tdm_port *h_port, struct tdm_channel *h_channel,
+		void *p_data, u16 *size)
+{
+	struct tdm_channel *channel;
+	struct tdm_bd *rx_bd;
+	unsigned long flags;
+	int res = 0;
+	unsigned short *buf, *buf1;
+	channel = h_channel;
+
+	if (!channel->p_ch_data || !channel->in_use)
+		return -EIO;
+
+	spin_lock_irqsave(&channel->p_ch_data->rx_channel_lock, flags);
+	rx_bd = channel->p_ch_data->rx_out_data;
+
+	if (rx_bd->flag) {
+		*size = rx_bd->length;
+		buf = (u16 *) p_data;
+		buf1 = (u16 *)rx_bd->p_data;
+		memcpy(buf1, buf, NUM_SAMPLES_PER_FRAME);
+		rx_bd->flag = 0;
+		rx_bd->offset = 0;
+		channel->p_ch_data->rx_out_data = (rx_bd->wrap) ?
+			channel->p_ch_data->rx_data_fifo : rx_bd + 1;
+
+	} else {
+		spin_unlock_irqrestore(&channel->p_ch_data->rx_channel_lock,
+				flags);
+		pr_debug("No Data Available");
+		return -EAGAIN;
+	}
+	spin_unlock_irqrestore(&channel->p_ch_data->rx_channel_lock, flags);
+
+	return res;
+}
+EXPORT_SYMBOL(tdm_channel_read);
+
+
+int tdm_channel_write(struct tdm_port *h_port, struct tdm_channel *h_channel,
+		void *p_data, u16 size)
+{
+	struct tdm_port *port;
+	struct tdm_channel *channel;
+	struct tdm_bd *tx_bd;
+	unsigned long flags;
+	int err = 0;
+	port = h_port;
+	channel = h_channel;
+#ifdef DEBUG
+	bool data_flag = 0;
+#endif
+
+	if (p_data == NULL) { /* invalid data*/
+		pr_err("tdm: Invalid Data");
+		return -EINVAL;
+	}
+
+	if (!channel->p_ch_data || !channel->in_use)
+		return -EIO;
+
+	spin_lock_irqsave(&channel->p_ch_data->tx_channel_lock, flags);
+	tx_bd = channel->p_ch_data->tx_in_data;
+
+	if (!tx_bd->flag) {
+		tx_bd->length = size;
+		memcpy(tx_bd->p_data, p_data,
+				size * port->adapter->adapt_cfg.slot_width);
+		tx_bd->flag = 1;
+		tx_bd->offset = 0;
+		channel->p_ch_data->tx_in_data = (tx_bd->wrap) ?
+			channel->p_ch_data->tx_data_fifo : tx_bd+1;
+		port->port_stat.tx_pkt_count++;
+#ifdef DEBUG
+		data_flag = 1;
+#endif
+	} else {
+		spin_unlock_irqrestore(&channel->p_ch_data->tx_channel_lock,
+				flags);
+		port->port_stat.tx_pkt_drop_count++;
+		pr_err("tdm: Transmit failed.");
+		return -ENOMEM;
+	}
+	spin_unlock_irqrestore(&channel->p_ch_data->tx_channel_lock, flags);
+
+#ifdef	DEBUG
+	if (data_flag) {
+		int k;
+		pr_info("\nTX port:%d - Write - Port TX-%d\n",
+				port->port_id, size);
+		for (k = 0; k < size; k++)
+			pr_info("%x", p_data[k]);
+		pr_info("\n");
+	}
+#endif
+	return err;
+}
+EXPORT_SYMBOL(tdm_channel_write);
+
+/*
+ * Driver Function for select and poll. Based on Channel, it sleeps on
+ * waitqueue
+ */
+int tdm_ch_poll(struct tdm_channel *h_channel, unsigned int wait_time)
+{
+	struct tdm_channel *channel;
+	channel = h_channel;
+
+	if (!channel->p_ch_data || !channel->in_use)
+		return -EIO;
+
+	if (channel->p_ch_data->rx_out_data->flag) {
+		pr_debug("Data Available");
+		return 0;
+	}
+	if (wait_time) {
+		unsigned long timeout = msecs_to_jiffies(wait_time);
+
+		wait_event_interruptible_timeout(channel->ch_wait_queue,
+				channel->p_ch_data->rx_out_data->flag,
+				timeout);
+
+		if (channel->p_ch_data->rx_out_data->flag) {
+			pr_debug("Data Available");
+			return 0;
+		}
+	}
+	return -EAGAIN;
+}
+EXPORT_SYMBOL(tdm_ch_poll);
+
+unsigned int tdm_port_get_stats(struct tdm_port *h_port,
+		struct tdm_port_stats *portstat)
+{
+	struct tdm_port *port;
+	int port_num;
+	port = h_port;
+
+	if (port == NULL || portstat == NULL) { /* invalid handle */
+		pr_err("tdm: Invalid Handle");
+		return -ENXIO;
+	}
+	port_num =  port->port_id;
+
+	memcpy(portstat, &port->port_stat, sizeof(struct tdm_port_stats));
+
+	pr_info("TDM Port %d Get Stats", port_num);
+
+	return 0;
+}
+EXPORT_SYMBOL(tdm_port_get_stats);
+
+/* Data handling functions */
+
+static int tdm_data_rx_deinterleave(struct tdm_adapter *adap)
+{
+	struct tdm_port *port, *next;
+	struct tdm_channel *channel, *temp;
+	struct tdm_bd	*ch_bd;
+
+	int i, buf_size, ch_data_len;
+	u16 *input_tdm_buffer;
+	u16 *pcm_buffer;
+	int slot_width;
+	int frame_ch_data_size;
+	bool ch_data;
+	int bytes_in_fifo_per_frame;
+	int bytes_slot_offset;
+
+	ch_data_len = NUM_SAMPLES_PER_FRAME;
+	frame_ch_data_size = NUM_SAMPLES_PER_FRAME;
+	ch_data = 0;
+
+	slot_width = adap->adapt_cfg.slot_width;
+	buf_size = tdm_adap_recv(adap, (void **)&input_tdm_buffer);
+	if (buf_size <= 0 || !input_tdm_buffer)
+		return -EINVAL;
+
+	bytes_in_fifo_per_frame = buf_size/frame_ch_data_size;
+	bytes_slot_offset = bytes_in_fifo_per_frame/slot_width;
+
+	/* de-interleaving for all ports*/
+	list_for_each_entry_safe(port, next, &adap->myports, list) {
+
+		list_for_each_entry_safe(channel, temp, &port->mychannels,
+				list) {
+			/* if the channel is not open */
+			if (!channel->in_use || !channel->p_ch_data)
+				continue;
+			ch_bd = channel->p_ch_data->rx_in_data;
+			spin_lock(&channel->p_ch_data->rx_channel_lock);
+			/*if old data is to be discarded */
+			if (use_latest_tdm_data && ch_bd->flag) {
+				ch_bd->flag = 0;
+				ch_bd->offset = 0;
+				if (ch_bd == channel->p_ch_data->rx_out_data)
+					channel->p_ch_data->rx_out_data =
+						ch_bd->wrap ?
+						channel->p_ch_data->rx_data_fifo
+						: ch_bd+1;
+				port->port_stat.rx_pkt_drop_count++;
+			}
+			/* if the bd is empty */
+			if (!ch_bd->flag) {
+				if (ch_bd->offset == 0)
+					ch_bd->length = port->rx_max_frames;
+
+				pcm_buffer = ch_bd->p_data + ch_bd->offset;
+				/* De-interleaving the data */
+				for (i = 0; i < ch_data_len; i++) {
+					pcm_buffer[i]
+						= input_tdm_buffer[i*
+						bytes_slot_offset +
+						channel->ch_id];
+				}
+				ch_bd->offset += ch_data_len * slot_width;
+
+				if (ch_bd->offset >=
+						(ch_bd->length -
+						frame_ch_data_size)*
+						(adap->adapt_cfg.slot_width)) {
+					ch_bd->flag = 1;
+					ch_bd->offset = 0;
+					channel->p_ch_data->rx_in_data =
+						ch_bd->wrap ?
+						channel->p_ch_data->rx_data_fifo
+						: ch_bd+1;
+					ch_data = 1;
+					wake_up_interruptible
+						(&channel->ch_wait_queue);
+				}
+			} else {
+				port->port_stat.rx_pkt_drop_count++;
+			}
+			spin_unlock(&channel->p_ch_data->rx_channel_lock);
+		}
+
+		if (ch_data) {
+			/* Wake up the Port Data Poll event */
+#ifdef	DEBUG
+			pr_info("Port RX-%d-%d\n", channel->ch_id, ch_data_len);
+			for (i = 0; i < ch_data_len; i++)
+				pr_info("%x", pcm_buffer[i]);
+			pr_info("\n");
+#endif
+			port->port_stat.rx_pkt_count++;
+			ch_data = 0;
+		}
+	}
+	return 0;
+}
+
+static int tdm_data_tx_interleave(struct tdm_adapter *adap)
+{
+	struct tdm_port *port, *next;
+	struct tdm_channel *channel, *temp;
+	struct tdm_bd	*ch_bd;
+	int i, buf_size, ch_data_len = NUM_SAMPLES_PER_FRAME;
+	bool last_data = 0;
+	u16 *output_tdm_buffer;
+	u16 *pcm_buffer;
+	int frame_ch_data_size = NUM_SAMPLES_PER_FRAME;
+	int bytes_in_fifo_per_frame;
+	int bytes_slot_offset;
+
+#ifdef DEBUG
+	u8	data_flag = 0;
+#endif
+
+	buf_size = tdm_adap_get_write_buf(adap, (void **)&output_tdm_buffer);
+	if (buf_size <= 0 || !output_tdm_buffer)
+		return -EINVAL;
+
+	bytes_in_fifo_per_frame = buf_size/frame_ch_data_size;
+	bytes_slot_offset = bytes_in_fifo_per_frame/adap->adapt_cfg.slot_width;
+
+
+	memset(output_tdm_buffer, 0, sizeof(buf_size));
+
+	list_for_each_entry_safe(port, next, &adap->myports, list) {
+
+		list_for_each_entry_safe(channel, temp, &port->mychannels,
+				list) {
+			pr_debug("TX-Tdm %d (slots-)", channel->ch_id);
+
+
+			/* if the channel is open */
+			if (!channel->in_use || !channel->p_ch_data)
+				continue;
+
+			spin_lock(&channel->p_ch_data->tx_channel_lock);
+			if (!channel->in_use || !channel->p_ch_data)
+				continue;
+			ch_bd = channel->p_ch_data->tx_out_data;
+			if (ch_bd->flag) {
+				pcm_buffer = (u16 *)((uint8_t *)ch_bd->p_data +
+						ch_bd->offset);
+				/* if the buffer has less frames than required*/
+				if (frame_ch_data_size >=
+						(ch_bd->length - ch_bd->offset/
+						 adap->adapt_cfg.slot_width)) {
+					ch_data_len =
+						ch_bd->length - ch_bd->offset/
+						adap->adapt_cfg.slot_width;
+					last_data = 1;
+				} else {
+					ch_data_len = frame_ch_data_size;
+				}
+				/* Interleaving the data */
+				for (i = 0; i < ch_data_len; i++) {
+					/*
+					 * TODO- need to be genric for any size
+					 *  assignment
+					 */
+					output_tdm_buffer[channel->ch_id +
+						bytes_slot_offset * i] =
+						pcm_buffer[i];
+				}
+				/*
+				 * If all the data of this buffer is
+				 * transmitted
+				 */
+				if (last_data) {
+					ch_bd->flag = 0;
+					ch_bd->offset = 0;
+					channel->p_ch_data->tx_out_data =
+						ch_bd->wrap ?
+						channel->p_ch_data->tx_data_fifo
+						: ch_bd+1;
+					port->port_stat.tx_pkt_conf_count++;
+				} else {
+					ch_bd->offset += ch_data_len *
+						(adap->adapt_cfg.slot_width);
+				}
+#ifdef	DEBUG
+				data_flag = 1;
+#endif
+			}
+			spin_unlock(&channel->p_ch_data->tx_channel_lock);
+		}
+	}
+
+#ifdef	DEBUG
+	if (data_flag) {
+		pr_info("TX-TDM Interleaved Data-\n");
+		for (i = 0; i < 64; i++)
+			pr_info("%x", output_tdm_buffer[i]);
+		pr_info("\n");
+	}
+#endif
+	return 0;
+}
+
+/* Channel Level APIs of TDM Framework */
+int tdm_channel_open(u16 chanid, u16 ch_width, struct tdm_port *port,
+		struct tdm_channel **h_channel)
+{
+	struct tdm_channel *channel, *temp;
+	unsigned long		flags;
+	struct tdm_ch_data	*p_ch_data;
+	int res = 0;
+
+	if (ch_width != 1) {
+		pr_err("tdm: Mode not supported\n");
+		return -EINVAL;
+	}
+
+	list_for_each_entry_safe(channel, temp, &port->mychannels, list) {
+		if (channel->ch_id == chanid) {
+			pr_err("tdm: Channel %d already open\n", chanid);
+			return -EINVAL;
+		}
+	}
+
+	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
+	if (!channel) {
+		res = -ENOMEM;
+		goto out;
+	}
+
+	init_waitqueue_head(&channel->ch_wait_queue);
+	p_ch_data = kzalloc(sizeof(struct tdm_ch_data), GFP_KERNEL);
+	if (!p_ch_data) {
+		res = -ENOMEM;
+		goto outdata;
+	}
+
+	p_ch_data->rx_data_fifo[TDM_CH_RX_BD_RING_SIZE-1].wrap = 1;
+	p_ch_data->tx_data_fifo[TDM_CH_TX_BD_RING_SIZE-1].wrap = 1;
+
+	p_ch_data->rx_in_data = p_ch_data->rx_data_fifo;
+	p_ch_data->rx_out_data = p_ch_data->rx_data_fifo;
+	p_ch_data->tx_in_data = p_ch_data->tx_data_fifo;
+	p_ch_data->tx_out_data = p_ch_data->tx_data_fifo;
+	spin_lock_init(&p_ch_data->rx_channel_lock);
+	spin_lock_init(&p_ch_data->tx_channel_lock);
+
+	channel->p_ch_data = p_ch_data;
+
+	channel->ch_id = chanid;
+	channel->ch_cfg.first_slot = chanid;
+	channel->ch_cfg.num_slots = 1;	/*
+					 * This is 1 for channelized mode and
+					 * configurable for other modes
+					 */
+	channel->port = port;
+	channel->in_use = 1;
+
+	spin_lock_irqsave(&port->ch_list_lock, flags);
+	list_add_tail(&channel->list, &port->mychannels);
+	spin_unlock_irqrestore(&port->ch_list_lock, flags);
+
+	*h_channel = channel;
+
+	return res;
+
+outdata:
+	kfree(channel);
+out:
+	return res;
+}
+EXPORT_SYMBOL(tdm_channel_open);
+
+int tdm_sysfs_init(void)
+{
+	struct kobject *tdm_kobj;
+	int err = 1;
+	tdm_kobj = kzalloc(sizeof(*tdm_kobj), GFP_KERNEL);
+	if (tdm_kobj) {
+		kobject_init(tdm_kobj, &tdm_type);
+		if (kobject_add(tdm_kobj, NULL, "%s", "tdm")) {
+			pr_err("tdm: Sysfs creation failed\n");
+			kobject_put(tdm_kobj);
+			err = -EINVAL;
+			goto out;
+		}
+	} else {
+		pr_err("tdm: Unable to allocate tdm_kobj\n");
+		err = -ENOMEM;
+		goto out;
+	}
+
+out:
+	return err;
+}
+
+int tdm_channel_close(u16 chanid, u16 ch_width, struct tdm_port *port,
+		struct tdm_channel *h_channel)
+{
+	struct tdm_channel *channel;
+	unsigned long		flags;
+	int res = 0;
+	channel = h_channel;
+
+	spin_lock_irqsave(&port->ch_list_lock, flags);
+	list_del(&channel->list);
+	spin_unlock_irqrestore(&port->ch_list_lock, flags);
+
+	if (channel)
+		kfree(channel->p_ch_data);
+	kfree(channel);
+	return res;
+}
+EXPORT_SYMBOL(tdm_channel_close);
+
+ssize_t tdm_show_sysfs(struct kobject *kobj,
+		struct attribute *attr, char *buf)
+{
+	int retval = 0;
+	struct tdm_sysfs *a = container_of(attr,
+			struct tdm_sysfs, attr);
+	switch (a->cmd_type) {
+	case TDM_LATEST_DATA:
+		pr_info("use_latest_tdm_data: %d\n", use_latest_tdm_data);
+		break;
+	default:
+		pr_info("Invalid cmd_type value\n");
+		return -EINVAL;
+	}
+	return retval;
+}
+
+ssize_t tdm_store_sysfs(struct kobject *kobj,
+		struct attribute *attr, const char *buf, size_t len)
+{
+	struct tdm_sysfs *a = container_of(attr,
+			struct tdm_sysfs, attr);
+
+	sscanf(buf, "%d", &a->data);
+	use_latest_tdm_data = a->data;
+	return strlen(buf);
+}
+
+void init_config_adapter(struct tdm_adapter *adap)
+{
+	struct fsl_tdm_adapt_cfg default_adapt_cfg = {
+		.loopback = TDM_PROCESS_NORMAL,
+		.num_ch = NUM_CHANNELS,
+		.ch_size_type = CHANNEL_16BIT_LIN,
+		.frame_len = NUM_SAMPLES_PER_FRAME,
+		.num_frames = NUM_SAMPLES_PER_FRAME,
+		.adap_mode = TDM_ADAPTER_MODE_NONE
+	};
+
+	default_adapt_cfg.slot_width = default_adapt_cfg.ch_size_type/3 + 1;
+
+	memcpy(&adap->adapt_cfg, &default_adapt_cfg,
+			sizeof(struct fsl_tdm_adapt_cfg));
+
+	return;
+}
+EXPORT_SYMBOL(init_config_adapter);
+
+void tdm_data_tasklet_fn(unsigned long data)
+{
+	struct tdm_adapter *adapter;
+	adapter = (struct tdm_adapter *)data;
+	if (adapter != NULL) {
+		tdm_data_tx_interleave(adapter);
+		tdm_data_rx_deinterleave(adapter);
+	}
+}
+
+
+MODULE_AUTHOR("Hemant Agrawal <hemant@freescale.com> and "
+	"Rajesh Gumasta <rajesh.gumasta@freescale.com>");
+MODULE_DESCRIPTION("TDM Driver Framework Core");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 83ac071..573ac40 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -425,6 +425,17 @@ struct i2c_device_id {
 			__attribute__((aligned(sizeof(kernel_ulong_t))));
 };
 
+/* tdm */
+
+#define TDM_NAME_SIZE   20
+#define TDM_MODULE_PREFIX "tdm:"
+
+struct tdm_device_id {
+	char name[TDM_NAME_SIZE];
+	kernel_ulong_t driver_data      /* Data private to the driver */
+			__attribute__((aligned(sizeof(kernel_ulong_t))));
+};
+
 /* spi */
 
 #define SPI_NAME_SIZE	32
diff --git a/include/linux/tdm.h b/include/linux/tdm.h
new file mode 100644
index 0000000..44cd8cf
--- /dev/null
+++ b/include/linux/tdm.h
@@ -0,0 +1,389 @@
+/* include/linux/tdm.h
+ *
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * tdm.h - definitions for the tdm-device framework interface
+ *
+ * Author:Hemant Agrawal <hemant@freescale.com>
+ *	Rajesh Gumasta <rajesh.gumasta@freescale.com>
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * 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.
+ *
+ * You should have received a copy of the  GNU General Public License along
+ * with this program; if not, write  to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+
+#ifndef _LINUX_TDM_H
+#define _LINUX_TDM_H
+
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/device.h>	/* for struct device */
+#include <linux/sched.h>	/* for completion */
+#include <linux/mutex.h>
+#include <linux/interrupt.h>
+#include <linux/sysfs.h>
+
+#define TDM_LATEST_DATA		1
+#define CHANNEL_8BIT_LIN	0	/* 8 bit linear */
+#define CHANNEL_8BIT_ULAW	1	/* 8 bit Mu-law */
+#define CHANNEL_8BIT_ALAW	2	/* 8 bit A-law */
+#define CHANNEL_16BIT_LIN	3	/* 16 bit Linear */
+
+/*
+ * Default adapter configuration. All the TDM adapters registered with
+ * framework will be configured with following default configuration.
+ */
+#define NUM_CHANNELS	16
+
+/*
+ * Default configuration for typical voice data sample. These parameters
+ * will generally not be required to be changed for voice type applications.
+ */
+
+/* 8 samples per milli sec per channel. Req for voice data */
+#define NUM_SAMPLES_PER_MS	8
+#define NUM_MS			10
+
+/* Number of samples for 1 client buffer */
+#define NUM_SAMPLES_PER_FRAME	(NUM_MS * NUM_SAMPLES_PER_MS)
+#define NUM_OF_TDM_BUF		3
+
+/* General options */
+
+struct tdm_adapt_algorithm;
+struct tdm_adapter;
+struct tdm_port;
+
+
+/*
+ * struct tdm_driver - represent an TDM device driver
+ * @class: What kind of tdm device we instantiate (for detect)
+ * @id:Driver id
+ * @name: Name of the driver
+ * @attach_adapter: Callback for device addition (for legacy drivers)
+ * @detach_adapter: Callback for device removal (for legacy drivers)
+ * @probe: Callback for device binding
+ * @remove: Callback for device unbinding
+ * @shutdown: Callback for device shutdown
+ * @suspend: Callback for device suspend
+ * @resume: Callback for device resume
+ * @command: Callback for sending commands to device
+ * @id_table: List of TDM devices supported by this driver
+ * @list: List of drivers created (for tdm-core use only)
+ */
+struct tdm_driver {
+	unsigned int class;
+	unsigned int id;
+	char name[TDM_NAME_SIZE];
+
+	int (*attach_adapter)(struct tdm_adapter *);
+	int (*detach_adapter)(struct tdm_adapter *);
+
+	/* Standard driver model interfaces */
+	int (*probe)(const struct tdm_device_id *);
+	int (*remove)(void);
+
+	/* driver model interfaces that don't relate to enumeration */
+	void (*shutdown)(void);
+	int (*suspend)(pm_message_t mesg);
+	int (*resume)(void);
+
+	const struct tdm_device_id *id_table;
+
+	/* The associated adapter for this driver */
+	struct tdm_adapter *adapter;
+	struct list_head list;
+};
+
+/*
+ * tdm per port statistics structure, used for providing and storing tdm port
+ * statistics.
+ */
+struct tdm_port_stats {
+	unsigned int rx_pkt_count;	/* Rx frame count per channel */
+	unsigned int rx_pkt_drop_count;	/*
+					 * Rx drop count per channel to
+					 * clean space for new buffer
+					 */
+	unsigned int tx_pkt_count;	/* Tx frame count per channel */
+	unsigned int tx_pkt_conf_count;	/*
+					 * Tx frame confirmation count per
+					 * channel
+					 */
+	unsigned int tx_pkt_drop_count;	/*
+					 * Tx drop count per channel due to
+					 * queue full
+					 */
+};
+
+
+/*
+ * tdm Buffer Descriptor, used for Creating Interleaved and De-interleaved
+ * FIFOs
+ */
+struct tdm_bd {
+	unsigned char flag;		/* BD is full or empty */
+	unsigned char wrap;		/* BD is last in the queue */
+	unsigned short length;		/* Length of data in BD */
+	/*TODO: use dyanmic memory */
+	unsigned short p_data[NUM_SAMPLES_PER_FRAME];	/* Data Pointer */
+	unsigned long offset;	/* Offset of the Data Pointer to be used */
+};
+
+#define TDM_CH_RX_BD_RING_SIZE	3
+#define TDM_CH_TX_BD_RING_SIZE	3
+
+/* tdm RX-TX Channelised Data */
+struct tdm_port_data {
+	struct tdm_bd rx_data_fifo[TDM_CH_RX_BD_RING_SIZE]; /*
+							     * Rx Channel Data
+							     * BD Ring
+							     */
+	struct tdm_bd *rx_in_data;	/*
+					 * Current Channel Rx BD to be filled by
+					 * de-interleave function
+					 */
+	struct tdm_bd *rx_out_data;	/*
+					 * Current Channel Rx BD to be
+					 * read by App
+					 */
+	struct tdm_bd tx_data_fifo[TDM_CH_TX_BD_RING_SIZE]; /*
+							     * Tx Channel Data
+							     *	BD Ring
+							     */
+	struct tdm_bd *tx_in_data;	/*
+					 * Current Channel Tx BD to be
+					 * filled by App
+					 */
+	struct tdm_bd *tx_out_data;	/*
+					 * Current Channel Tx BD to be read by
+					 * interleave function
+					 */
+	spinlock_t rx_channel_lock;	/* Spin Lock for Rx Channel */
+	spinlock_t tx_channel_lock;	/* Spin Lock for Tx Channel */
+};
+
+/* structure tdm_port_cfg - contains configuration params for a port */
+struct tdm_port_cfg {
+	unsigned short port_mode;
+};
+
+/* struct tdm_port - represent an TDM ports for a device */
+struct tdm_port {
+	unsigned short port_id;
+	unsigned short in_use;	/* Port is enabled? */
+	uint16_t rx_max_frames;	/*
+				 * Received port frames before allowing
+				 * read operation in Port Mode
+				 */
+
+	struct tdm_port_stats port_stat; /*
+					  * A structure parameter defining
+					  * TDM port statistics.
+					  */
+	struct tdm_port_data *p_port_data;	/*
+						 * a structure parameter
+						 * defining tdm channelised data
+						 */
+
+	struct tdm_driver *driver;	/* driver for this port */
+	struct tdm_adapter *adapter;	/* adapter for this port */
+	struct list_head list;		/* list of ports */
+	struct list_head mychannels;	/* list of channels, on this port*/
+	spinlock_t ch_list_lock;	/* Spin Lock for channel_list */
+	struct tdm_port_cfg port_cfg;	/*
+					 * A structure parameter defining
+					 * TDM port configuration.
+					 */
+};
+
+/* tdm RX-TX Channelised Data */
+struct tdm_ch_data {
+	struct tdm_bd rx_data_fifo[TDM_CH_RX_BD_RING_SIZE]; /*
+							     * Rx Port Data BD
+							     * Ring
+							     */
+	struct tdm_bd *rx_in_data;	/*
+					 * Current Port Rx BD to be filled by
+					 * de-interleave function
+					 */
+	struct tdm_bd *rx_out_data; /* Current Port Rx BD to be read by App */
+	struct tdm_bd tx_data_fifo[TDM_CH_TX_BD_RING_SIZE]; /*
+							     * Tx Port Data BD
+							     * Ring
+							     */
+	struct tdm_bd *tx_in_data;	/*
+					 * Current Port Tx BD to be filled by
+					 * App
+					 */
+	struct tdm_bd *tx_out_data;	/*
+					 * Current Port Tx BD to be read by
+					 * interleave function
+					 */
+	spinlock_t rx_channel_lock;	/* Spin Lock for Rx Port */
+	spinlock_t tx_channel_lock;	/* Spin Lock for Tx Port */
+};
+
+/* Channel config params */
+struct tdm_ch_cfg {
+	unsigned short num_slots;
+	unsigned short first_slot;
+};
+
+/* struct tdm_channel- represent a TDM channel for a port */
+struct tdm_channel {
+	u16 ch_id;			/* logical channel number */
+	struct list_head list;		/* list of channels in a port*/
+	struct tdm_port *port;		/* port for this channel */
+	u8 in_use;			/* channel is enabled? */
+	struct tdm_ch_cfg ch_cfg;	/* channel configuration */
+	struct tdm_ch_data *p_ch_data;	/* data storage space for channel */
+	wait_queue_head_t ch_wait_queue;/* waitQueue for RX Channel Data */
+};
+
+/* tdm_adapt_algorithm is for accessing the routines of device */
+struct tdm_adapt_algorithm {
+	int (*tdm_read)(struct tdm_adapter *, u16 **);
+	int (*tdm_get_write_buf)(struct tdm_adapter *, u16 **);
+	int (*tdm_write)(struct tdm_adapter *, void *, unsigned int len);
+	int (*tdm_enable)(struct tdm_adapter *);
+	int (*tdm_disable)(struct tdm_adapter *);
+};
+
+/* tdm_adapter_mode is to define in mode of the device */
+enum tdm_adapter_mode {
+	TDM_ADAPTER_MODE_NONE = 0x00,
+	TDM_ADAPTER_MODE_T1 = 0x01,
+	TDM_ADAPTER_MODE_E1 = 0x02,
+	TDM_ADAPTER_MODE_T1_RAW = 0x10,
+	TDM_ADAPTER_MODE_E1_RAW = 0x20,
+};
+
+/* tdm_port_mode defines the mode in which the port is configured to operate
+ * It can be channelized/full/fractional.
+ */
+enum tdm_port_mode {
+	TDM_PORT_CHANNELIZED = 0,	/* Channelized mode */
+	TDM_PORT_FULL = 1,		/* Full mode */
+	TDM_PORT_FRACTIONAL = 2		/* Fractional mode */
+};
+
+/* tdm_process_mode used for testing the tdm device in normal mode or internal
+ * loopback or external loopback
+ */
+enum tdm_process_mode {
+	TDM_PROCESS_NORMAL = 0,		/* Normal mode */
+	TDM_PROCESS_INT_LPB = 1,	/* Internal loop mode */
+	TDM_PROCESS_EXT_LPB = 2		/* External Loopback mode */
+};
+
+/* TDM configuration parameters */
+struct fsl_tdm_adapt_cfg {
+	u8 num_ch;		/* Number of channels in this adpater */
+	u8 ch_size_type;	/*
+				 * reciever/transmit channel
+				 * size for all channels
+				 */
+	u8 slot_width;		/* 1 or 2 Is defined by channel type */
+	u8 frame_len;		/* Length of frame in samples */
+	u32 num_frames;
+	u8 loopback;		/* loopback or normal */
+	u8 adap_mode;		/*
+				 * 0=None, 1= T1, 2= T1-FULL, 3=E1,
+				 * 4 = E1-FULL
+				 */
+	int max_timeslots;	/*
+				 * Max Number of timeslots that are
+				 * supported on this adapter
+				 */
+};
+
+/*
+ * tdm_adapter is the structure used to identify a physical tdm device along
+ * with the access algorithms necessary to access it.
+ */
+struct tdm_adapter {
+	struct module *owner;	/* owner of the adapter module */
+	unsigned int id;	/* Adapter Id */
+	unsigned int drv_count;	/*
+				 * Number of drivers associated with the
+				 * adapter
+				 */
+	const struct tdm_adapt_algorithm *algo;	/*
+						 * algorithm to access the
+						 * adapter
+						 */
+
+	char name[TDM_NAME_SIZE];	/* Name of Adapter */
+	struct mutex adap_lock;
+	struct device *parent;
+
+	struct tasklet_struct tdm_data_tasklet;	/*
+						 * tasklet handle to perform
+						 * data processing
+						 */
+	int tasklet_conf;	/* flag for tasklet configuration */
+	int tdm_rx_flag;
+
+	struct list_head myports;	/*
+					 * list of ports, created on this
+					 * adapter
+					 */
+	struct list_head list;
+	spinlock_t portlist_lock;
+	void *data;
+	struct fsl_tdm_adapt_cfg adapt_cfg;
+};
+
+struct tdm_sysfs {
+	struct attribute attr;
+	int data;
+	u32 cmd_type;
+};
+
+/* functions exported by tdm.o */
+
+int tdm_add_adapter(struct tdm_adapter *adpater);
+int tdm_del_adapter(struct tdm_adapter *adapter);
+int tdm_register_driver(struct tdm_driver *driver);
+void tdm_unregister_driver(struct tdm_driver *driver);
+void init_config_adapter(struct tdm_adapter *adapter);
+
+int tdm_port_open(struct tdm_driver *driver, struct tdm_port **h_port);
+int tdm_port_close(struct tdm_port *h_port);
+int tdm_channel_read(struct tdm_port *h_port, struct tdm_channel *h_channel,
+		void *p_data, u16 *size);
+int tdm_channel_write(struct tdm_port *h_port, struct tdm_channel *h_channel,
+		void *p_data, u16 size);
+int tdm_ch_poll(struct tdm_channel *h_channel, unsigned int wait_time);
+
+int tdm_channel_open(u16 chanid, u16 ch_width, struct tdm_port *port,
+		struct tdm_channel **h_channel);
+int tdm_channel_close(u16 chanid, u16 ch_width, struct tdm_port *port,
+		struct tdm_channel *h_channel);
+/* this tasklet is created for each adapter instance */
+void tdm_data_tasklet_fn(unsigned long);
+int tdm_sysfs_init(void);
+ssize_t tdm_show_sysfs(struct kobject *kobj,
+		struct attribute *attr, char *buf);
+ssize_t tdm_store_sysfs(struct kobject *kobj,
+		struct attribute *attr, const char *buf, size_t len);
+
+struct tdm_adapter *tdm_get_adapter(int id);
+void tdm_put_adapter(struct tdm_adapter *adap);
+
+#endif /* __KERNEL__ */
+
+#define TDM_E_OK 0
-- 
1.5.6.5

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

* [1/3][PATCH][v2] Adding documentation for TDM
  2012-07-27 14:05 [2/3][PATCH][v2] TDM Framework sandeep
@ 2012-07-27 14:05 ` sandeep
  2012-07-27 14:05   ` [3/3][PATCH][v2] Added TDM device support and Freescale Starlite driver sandeep
  2012-07-27 14:11 ` [2/3][PATCH][v2] TDM Framework John Stoffel
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: sandeep @ 2012-07-27 14:05 UTC (permalink / raw)
  To: linuxppc-dev, linux-arm-kernel; +Cc: devel, Sandeep Singh, linux-kernel

From: Sandeep Singh <Sandeep@freescale.com>

 tdm-summary.txt contains general description about TDM.
 tdm-framework.txt contains specific description of TDM framework.

Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
---
Changes since v1:
	Incorporated Laight's comments.
	-Removed reference to unused config.

 Documentation/tdm/tdm-framework.txt |  258 +++++++++++++++++++++++++++++++++++
 Documentation/tdm/tdm-summary.txt   |  103 ++++++++++++++
 2 files changed, 361 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/tdm/tdm-framework.txt
 create mode 100644 Documentation/tdm/tdm-summary.txt

diff --git a/Documentation/tdm/tdm-framework.txt b/Documentation/tdm/tdm-framework.txt
new file mode 100644
index 0000000..f96189a
--- /dev/null
+++ b/Documentation/tdm/tdm-framework.txt
@@ -0,0 +1,258 @@
+This document gives an overview of TDM framework and its interface with low
+level drivers and upper level users/clients.
+
+Terminology:
+============
+1. TDM: Time Division Multiplexing.
+2. TDM channel: The channel is the smallest entity on which all the TDM read/
+   write operations will occur. Technically each channel maps to a set of
+   consecutive time slots on the physical TDM frame. The channels will be
+   dynamically created and destroyed using tdm_open_channel and
+   tdm_close_channel.
+3. TDM adapter or Adapter: Refers to an instance of TDM controller/device on
+   the system.
+4. TDM frame: Is a set of TDM channels which is transmitted sequentially over
+   time. The frame start is identified by a frame sync signal that is briefly
+   asserted at the beginning of each frame.
+
+X----------TDM Frame 0-------------X------TDM Frame 1-----------------X
+|----|----|----|----|----|----|----|----|----|----|----|----|----|----|
+| 0  | 1  | 2  | 3  | 4  | ...|  n |  0 | 1  |  2 |  3 | 4  | ...| n  |...
+|----|----|----|----|----|----|----|----|----|----|----|----|----|----|
+<---->				   <---->
+ch 0				   ch 0
+
+4. TDM client: Application/driver which registers with TDM framework to use TDM
+   device.
+5. TDM port: It can be seen as a virtual device exposed to a client. At a time
+   TDM port can work in one of the follwing configurations:
+   full/fractional/E1/T1/raw.
+
+TDM modes
+========
+A TDM device can operate in one of the following modes:
+1. Single port full mode - Single user/no interleaving
+2. Single port channelised mode (raw, E1, T1)- many users using different
+   channels
+3. Single port fractional mode -
+4. Multi port mode - multiple users using different ports in different
+   configurations.
+
+All the above configurations differ in number of TDM client they support,
+number of TDM channels and number of TDM ports.
+
+Currently we are supporting only single port channelised mode. Hence all the
+explanations below refer to channelised mode of TDM. This framework can be
+easily extended to support other modes.
+
+Single port Channelised Mode
+==============================
+In single port channelised mode there can be only one port and each channel
+can have only one time slot.The number of active channels can be less than
+the maximum supported channels/slots.
+
+X----------TDM Frame 0-------------X------TDM Frame 1-----------------X
+|----|----|----|----|----|----|----|----|----|----|----|----|----|----|
+| 0  | 1  | 2  | 3  | 4  | ...|  n |  0 | 1  |  2 |  3 | 4  | ...| n  |...
+|----|----|----|----|----|----|----|----|----|----|----|----|----|----|
+<----><--->				   <---->
+ch 0	ch1			   ch 0
+client0 client1
+
+TDM Subsystem Overview
+========================
+
+	 |-----------------------|
+	 |user mode TDM clients  |
+	 |-----------------------|
+		||
+-------------------------------------------------------------------
+  tdm-dev.c     ||
+		||
+	        ||                    	     |------------------------|
+	    client register            	     | kernel mode TDM clients|
+	        ||                     	     |------------------------|
+	        ||					||
+	        || 					||
+		||		  		client register
+		||					||
+		\/		   		        \/
+      ______________________________________________________________
+      |                 					   |
+      |                 client interface                           |
+      |------------------------------------------------------------|
+      |             TDM Subsystem Framework         	           |
+      |                   (tdm-core.c)                             |
+      |                                                            |
+      | ->buffer handling                                          |
+      | ->interleaving/de-interleaving                             |
+      |                                                            |
+      |------------------------------------------------------------|
+      |    TDM interface                  Line control interface   |
+      |____________________________________________________________|
+          /\                                       /\
+          ||                                       ||
+    device register                          device register
+          ||                                       ||
+          ||                                       ||
+
+     fsl_tdm.c                 ucc_tdm.c         slic_zarlink.c framer.c
+--------------------------------------------------------------------------
+_______________________    _____________________  ________    ________
+|		      |	   |		       |  |	 |    |      |
+|[h/w] TDM controller |    |UCC TDM controller |  | SLIC |    |Framer|
+|_____________________|    |___________________|  |______|    |______|
+
+
+
+TDM Adapter Registration:
+=========================
+All the TDM adapter drivers will get registered as platform drivers to Linux.
+For every instance of the TDM adapter the relevant driver will be probed.
+As part of probe the driver will
+1. Do the basic initialization in terms of memory allocation for various
+   driver specific data structures, interrupts registration, etc.
+2. Initialize the TDM adapter and configure it in default configuration.
+   like operating mode, number of channels, channel type, etc.
+3. Add the TDM adapter to the TDM Framework via tdm_add_adapter() API.
+   As a result TDM framework will add this adapter to it's queue of
+   available adapters. As part of this adapter registration TDM framework
+   is also supplied a list of access algorithms for the particular TDM
+   adapter.
+4. Notifies the clients
+
+TDM Client Registration:
+========================
+Every TDM client gets itself registered with the TDM framework layer as
+a TDM driver using the API tdm_add_driver(). As part of this the TDM client
+supplies to the TDM framework the adapter with which it wants to hook and
+the function pointers of attach and detach functions which must be called
+as soon as the requested adapter is available.
+
+As a result the TDM framework keeps association of TDM adapters and TDM
+client drivers.
+As soon as this association gets established a tasklet is created for the
+adapter which is handled by tdm_data_tasklet_fn. The primary function of
+this tasklet acts as an interface to transfer the TDM data between the
+TDM adapter and the TDM client drivers.
+
+
+Currently TDM adaper can only be used in the default configuration.
+ie the configuration cannot be modified by TDM clients. This will
+be enhanced in future.
+
+Data handling:
+==============
+Some basic assumptions about data handling:
+
+1. As per standard voice rate of 8Khz or 8192Hz. Which means 8192 samples must
+be sent every second. So if there are multiple clients sending voice data
+over TDM interface the rate should be such that the individual samples
+sent by them must be transmitted at 8Kz.
+
+This is defined in the driver as
+
+	#define CH_1_MS_FRAMES		8
+
+2. Number of milliseconds at which TDM Rx interrupts occur
+This is basically the time for which the TDM data is sent in one Tx or Rx
+cycle of TDM controller hardware. In one DMA we send the data for 10ms.
+This gives enough time so that no buffer overflow or under-run occurs for
+transmit and receive respectively.
+
+	#define NUM_MS                  10
+
+3. TDM has programmable slot length (8 bits or 16 bits). It can be configured
+depending on the type of sample. For example the sample could be 16 bit linear
+or 8bit u-law encoded etc. Presently only word length of 16 is supported
+which is the default configuration.
+
+4. Number of channels means the total number of channels per TDM port.
+For example for E1 mode it will be 24, for T1 it will be 32, etc.
+There can also be raw mode, where the use case is not E1 or T1.
+Here the number of channels can be any number as per the use case.
+
+The whole framework follows a triple buffer approach to make sure that TDM data
+is played continuously at the desired rate.
+
+Buffers Involved:
+=================
+
+1.TDM driver or device buffers:
+These buffers are the device level buffers. They contain the TDM data which is
+transmitted/received on the TDM physical signals. As such these buffers must
+be allocated from driver layer so that all the hardware requirements are met.
+As an optimized design to remove extra memcopies, the client can pass the data
+in the same buffers. But this is only true for full mode of TDM. Where the
+user data can be straightaway passed to the hardware for transmission.
+Although in other cases memcopy cannot be avoided, because the framework layer
+will have to interleave the individual channels data to create the TDM frame
+data buffer.For channelised mode size of this buffer will be governed by:
+
+-	number of channels
+- 	number of slots per channel
+-	number of bytes per slot
+- 	number of frames per ms
+-	number of ms
+
+For a channelised mode with single port the size of the device level buffer
+will be:
+
+channels * slots per channel * bytes per slot * frames per ms *
+number of ms channels * NUM_BYTES_PER_SLOT * NUM_MS * CH_1_MS_FRAMES
+
+There will be 3 such buffers.
+
+2.Channel level buffers:
+In case the TDM device is configured for multiport/multichannel the Framework
+layer needs to maintain the data for each channel. Hence for each channel
+opened a Buffer Descriptor ring of 3 BDs(see note below) is allocated both for
+transmit and receive. The client reads from/writes to the buffers pointed by
+these BD rings.
+
+The framework layer maintains a Data Process Tasklet per TDM device which is
+scheduled from every Rx interrupt. The interrupt handling periodicity is
+governed by the TDM data buffer size configured in the above section. The data
+tasklet when scheduled, will do Rx and Tx processing to copy the data from/to
+the channel specific interleaved buffers. The TDM controller will DMA the
+data which is copied in the interleaved buffers or device level buffers.
+
+TDM framework provides the port level APIs and channel level APIs to the TDM
+client drivers to send and receive the respective data on different TDM slots.
+
+
+num of buffers = 3
+
+TDM client1           	TDM Client2
+
+buf0------->buf1	buf0------->buf1
+^            |		^            |
+|            V		|            V
+----buf2------		------buf2----
+     |				|
+     |				|
+     |				|
+     V				V
+-----------------------------------------
+|					|
+|    	 DATA Tasklet			|
+|					|
+-----------------------------------------
+		|
+		|
+		V
+-----------------------------------------
+|    	 TDM buffer interleaved * 3	|
+-----------------------------------------
+
+
+Not Implemented/Future work:
+============================
+1. TDM client will use the default configuration which is done at init time
+   and is not configurable. In future this should be made configurable as per
+   the needs of client.
+2. The TDM framework still needs to be enhanced to configure the ports and
+   their attributes. Currently only single port channelised mode is supported.
+3. Line control interface is not available in the framework presently.
+   Presently it offer very minimal kind of interface.
+4. SLIC interface will be enhanced as per Zarlink Open source APIs in future.
diff --git a/Documentation/tdm/tdm-summary.txt b/Documentation/tdm/tdm-summary.txt
new file mode 100644
index 0000000..f010f76
--- /dev/null
+++ b/Documentation/tdm/tdm-summary.txt
@@ -0,0 +1,103 @@
+Time Division Multiplexing (TDM)
+=================================
+
+TDM is a type of digital or analog multiplexing in which two or more bit
+streams or signals are transferred apparently simultaneously as sub-channels
+in one communication channel, but are physically taking turns on the channel.
+
+The time domain is divided into several recurrent timeslots of fixed duration.
+These timeslot are grouped together to form a channel. A sample byte or data
+block of channel 1 is transmitted during timeslots allocated to channel 1,
+channel 2 during timeslot for channel 2, etc.
+
+One TDM frame consists of multiple channels. After the last channel the cycle
+starts all over again with a new frame, starting with the second sample, byte
+or data block from channel 1, and so on.
+
+X----------TDM Frame 0-------------X------TDM Frame 1-----------------X
+|----|----|----|----|----|----|----|----|----|----|----|----|----|----|
+| 0  | 1  | 2  | 3  | 4  | ...|  n |  0 | 1  |  2 |  3 | 4  | ...| n  |...
+|----|----|----|----|----|----|----|----|----|----|----|----|----|----|
+<---->				   <---->
+channel 0			   channel 0
+-------------------------------------------------------------------->
+         Increasing Time
+
+Physical TDM interface
+=======================
+
+Physically TDM interface is a serial full duplex interface designed to
+communicate with variety of serial devices like industry standard framers,
+codecs, other DSPs, and microprocessors. It is typically used to transfer
+samples in a periodic manner. The TDM consists of independent transmitter and
+receiver sections with independent clock generation and frame synchronization.
+
+External TDM signals are:
+1. TDM_TCK: TDM Transmit clock
+2. TDM_RCK: TDM Receive clock
+3. TDM_TFS: TDM Tx frame sync to identify frame boundary
+4. TDM_RFS: TDM Rx Frame sync to identify frame boundary
+5. TDM_TD: TDM Tx data
+6. TDM_RD: TDM Rx data
+
+TDM is generally used to simultaneously transmit periodic data for multiple
+users. Common use cases would be to carry voice for various telephone
+subscribers in the telephone networks. It is widely used to carry telephonic
+data of various industry standards like E1/T1 data, etc.
+
+T1 Details
+==========
+T1 frame consists of 24 channels of 8 bits each plus one frame alignment bit.
+So a T1 frame has a total of 24x8 + 1 = 193 bits. Since each T1 frame contains
+one byte of voice data for each of 24 channels and the system needs to maintain
+a data rate of 8000 samples/sec. This would require 8000 frames/sec to be sent,
+yielding a data rate of 8000x193 bit/sec = 1.544 Mbps.
+
+E1 Details
+===========
+E1 frame consists of 32 channels each of 8 bits. Thus having a total frame
+length of 32x8 = 256 bits. Similar to the case of T1 it has to maintain a data
+rate of 8000 frames/sec. Thus having a data rate of 8000 x 256 bits/sec =
+2.048 Mbps.
+
+TDM use cases
+=============
+
+With SLIC kind of devices
+=========================
+SLIC stands for Subscriber Line Interface Card.
+Typically TDM systems consist of TDM controller and a line control device.
+
+The TDM controller interfaces to the line control device through TDM interface
+which is digital TDM multiplexed data.
+
+The Line controller has the functionality to interface with the TDM controller
+at one end and interface with the analog units at the other. For example if the
+line control device is a SLIC kind of device.
+The typical setup would be:
+
+|------------------|
+|		   |
+|		   | /-------\  |---------|
+|   TDM controller |/   TDM   \ |  SLIC   |<--------> s-ch0 analog phone 1
+|		   |\   data  / |         |<--------> s-ch1 analog phone 2
+|		   | \-------/  |---------|<--------> s-ch2 analog phone 3
+|		   |<----digital---->     <analog>
+|------------------|
+
+
+
+Another use case (VoIP):
+========================
+
+                             Voice packets on network
+     |--------|     |------|      _________      |------|     |------|
+>----|        |/---\|  TDM |     (         )     | TDM  |/---\|      |----->
+<----| SLIC   |\---/|      |     ( n/w     )     |      |\---/| SLIC |-----<
+>----|        |     |------|      ---------      |------|     |      |----->
+     |--------|      mux                          demux       |------|
+
+In the above figure analog phones are connected to the hosts via SLICs.
+The voice spoken on the phones is multiplexed converted into VoIP packets
+and sent over network. At the rendering end the multiplexed data
+is de-multiplexed and sent to respective listeners via SLIC.
-- 
1.5.6.5

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

* [3/3][PATCH][v2] Added TDM device support and Freescale Starlite driver
  2012-07-27 14:05 ` [1/3][PATCH][v2] Adding documentation for TDM sandeep
@ 2012-07-27 14:05   ` sandeep
  0 siblings, 0 replies; 22+ messages in thread
From: sandeep @ 2012-07-27 14:05 UTC (permalink / raw)
  To: linuxppc-dev, linux-arm-kernel; +Cc: devel, Sandeep Singh, linux-kernel

From: Sandeep Singh <Sandeep@freescale.com>

Freescale TDM controller consists of a TDM module supporting 128 channels
running at up to 50 Mbps with 8-bit and 16-bit word size. The TDM bus connects
gluelessly to most T1/E1 frames as well as to common buses such as the H.110,
SCAS, and MVIP. TDM also supports an I2S mode. The TDM module operates in
independent or shared mode when receiving or transmitting data.

This controller is available on MPC8315, P1010, P1020, P1022 and P1024 Freescale SOCs.

The driver registers itself with the TDM Framework & provides TDM functionality to the client modules.

In its present form this driver supports only channelised mode.

Signed-off-by: Sandeep Singh <Sandeep@freescale.com>
Signed-off-by: Poonam Aggrwal <poonam.aggrwal@freescale.com>
---

Based on: git://git.am.freescale.net/gitolite/mirrors/galak-powerpc.git
Branch: master
Checkpatch: passed

First patch version was RFC

Changes from RFC:
	- Enabling Tx FIFO for TDM
	- Removed unused variables.
	- PMUXCR has been removed as it is taken care by u-boot

	Incorporated Timur's comments:
	- Improved Copyright statement.
	- Removed unused function.
	- Introduced read after each write to register
	- Used spin_event_timeout for polling
	- Removed unused spinlock
	- Moved all macros and structures from header file to tdm_fsl.c
	- Rectified cosmetic problems.

 drivers/tdm/Kconfig          |    3 -
 drivers/tdm/Makefile         |    2 +-
 drivers/tdm/device/Kconfig   |   15 +
 drivers/tdm/device/Makefile  |    9 +
 drivers/tdm/device/tdm_fsl.c | 1186 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 1211 insertions(+), 4 deletions(-)
 create mode 100644 drivers/tdm/device/Kconfig
 create mode 100644 drivers/tdm/device/Makefile
 create mode 100644 drivers/tdm/device/tdm_fsl.c

diff --git a/drivers/tdm/Kconfig b/drivers/tdm/Kconfig
index 0b0fda8..69b8987 100644
--- a/drivers/tdm/Kconfig
+++ b/drivers/tdm/Kconfig
@@ -13,6 +13,3 @@ menuconfig TDM
 	  This TDM support can also be built as a module.  If so, the module
 	  will be called tdm-core.
 
-if TDM
-
-endif # TDM
diff --git a/drivers/tdm/Makefile b/drivers/tdm/Makefile
index 84e2cb9..a605b3d 100644
--- a/drivers/tdm/Makefile
+++ b/drivers/tdm/Makefile
@@ -2,4 +2,4 @@
 # Makefile for the TDM core.
 #
 
-obj-$(CONFIG_TDM)		+= tdm-core.o
+obj-$(CONFIG_TDM)		+= tdm-core.o device/
diff --git a/drivers/tdm/device/Kconfig b/drivers/tdm/device/Kconfig
new file mode 100644
index 0000000..9fd1b06
--- /dev/null
+++ b/drivers/tdm/device/Kconfig
@@ -0,0 +1,15 @@
+#
+# TDM device configuration
+#
+
+menu "TDM Device support"
+
+config TDM_FSL
+        tristate "Driver for Freescale TDM controller"
+        depends on FSL_SOC
+        ---help---
+          This is a driver for Freescale TDM controller. The controller
+          is found in various Freescale SOCs viz MPC8315, P1020. The TDM driver
+          basically multiplexes and demultiplexes data from different channels.
+          The TDM can interface SLIC kind of devices.
+endmenu
diff --git a/drivers/tdm/device/Makefile b/drivers/tdm/device/Makefile
new file mode 100644
index 0000000..4156d7f
--- /dev/null
+++ b/drivers/tdm/device/Makefile
@@ -0,0 +1,9 @@
+#
+# Makefile for the TDM device drivers.
+#
+
+obj-y	+= tdm_fsl.o
+
+#ifeq ($(CONFIG_TDM_DEBUG_BUS),y)
+#EXTRA_CFLAGS += -DDEBUG
+#endif
diff --git a/drivers/tdm/device/tdm_fsl.c b/drivers/tdm/device/tdm_fsl.c
new file mode 100644
index 0000000..040b0ea
--- /dev/null
+++ b/drivers/tdm/device/tdm_fsl.c
@@ -0,0 +1,1186 @@
+/*
+ * Copyright 2007-2012 Freescale Semiconductor, Inc, All rights reserved.
+ *
+ * TDM driver for Freescale TDM controller.
+ * This driver can interface with SLIC device to run VOIP kind of
+ * applications.
+ *
+ * Author: P. V. Suresh <pala@freescale.com>
+ *	Hemant Agrawal <hemant@freescale.com>
+ *	Rajesh Gumasta <rajesh.gumasta@freescale.com>
+ *
+ * Modifier: Sandeep Kr. Singh <sandeep@freescale.com>
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ *
+ * 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.
+ *
+ * You should have received a copy of the  GNU General Public License along
+ * with this program; if not, write  to the Free Software Foundation, Inc.,
+ * 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+/*
+ * Note that this is a complete rewrite of P.V. Suresh's driver code.
+ * But we have used so much of his original code and ideas that it seems
+ * only fair to recognize him as co-author -- Rajesh & Hemant
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/sched.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/of_platform.h>
+#include <linux/io.h>
+#include <linux/tdm.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/dma-mapping.h>
+#include <linux/spinlock.h>
+#include <sysdev/fsl_soc.h>
+
+#define DRV_DESC "Freescale TDM Driver Adapter"
+#define DRV_NAME "fsl_tdm"
+
+/* TDM data register offset */
+#define TDM_TDR_OFFSET 0x108
+#define TDM_RDR_OFFSET 0x100
+#define TDM_DATAREG_OFFSET 0x100
+#define TDM_CLKREG_OFFSET 0x180
+
+/* TCD params */
+#define SOFF_VAL	0x08
+#define DOFF_VAL	0x08
+#define NBYTES		0x08 /*Minor Bytes transfer count*/
+#define SLAST		0x00 /* last source addr adjustment*/
+#define SLAST_SGA	0x00
+#define DLAST_SGA	0x00
+
+/* RIR Params*/
+#define RIR_RFSD_VAL	0x01
+#define RIR_RFWM_VAL	0x00
+
+/* TIR Params*/
+#define TIR_RFSD_VAL	0x01
+#define TIR_RFWM_VAL	0x00
+
+/* TDMTCEN */
+#define NUM_TDMTCEN_REG		0x04
+#define TDMTCEN_REG_LEN		32
+
+
+#define DMAC_TX_INT 1
+#define DMAC_RX_INT 2
+
+/* DMA GPOR */
+#define DMAGPOR_SNOOP	0x00000040	/* Enable Snooping */
+
+/* DMA Control Register (DMACR) */
+#define DMACR_EMLM	0x00000080	/* Enable Minor loop Mapping */
+#define DMACR_CLM	0x00000040	/* Continuous link mode */
+#define DMACR_HALT	0x00000020	/* Halt DMA */
+#define DMACR_HOE	0x00000010	/* Halt on Error */
+#define DMACR_ERGA	0x00000008	/* Round robin among the groups */
+#define DMACR_ERCA	0x00000004	/* Round robin Port Arbitration */
+#define DMACR_EDBG	0x00000002	/* Debug */
+#define DMACR_EBW	0x00000001	/* Enable Buffer */
+
+/* DMA Error Status DMAES */
+#define DMAES_VLD	0x80000000	/* Logical OR of all DMA errors. */
+#define DMAES_ECX	0x00010000	/* Transfer cancelled */
+#define DMAES_GPE	0x00008000	/* Group priority error */
+#define DMAES_CPE	0x00004000	/* Channel priority error */
+/* errored/cancelled channel */
+#define DMAES_ERRCHN(errch)	(((errch) & 0x1F00) >> 8)
+#define DMAES_SAE	0x00000080	/* Source address error */
+#define DMAES_SOE	0x00000040	/* Source offset error */
+#define DMAES_DAE	0x00000020	/* Destination address error */
+#define DMAES_DOE	0x00000010	/* Destination offset error */
+#define DMAES_NCE	0x00000008	/* Nbytes citer error */
+#define DMAES_SGE	0x00000004	/* Scatter gather error */
+#define DMAES_SBE	0x00000002	/* Source bus error */
+#define DMAES_DBE	0x00000001	/* Destination bus error */
+
+/* DMA Enable Request (DMAERQH, DMAERQL) Enable/disable device
+	request for the channel */
+#define DMA_SET_ENABLE_REQUEST(regs, ch)	out_8(((regs)->dmasreq), ch)
+#define DMA_CLEAR_ENABLE_REQUEST(regs, ch)	out_8(((regs)->dmacerq), ch)
+
+/* DMA Enable Error Interrupt (DMAEEIH, DMAEEIL) Enable/disable
+	error interrupt for the channel */
+#define DMA_SET_ENABLE_ERROR_INT(regs, ch)	out_8(((regs)->dmaseei), ch)
+#define DMA_CLEAR_ENABLE_ERROR_INT(regs, ch)	out_8(((regs)->dmaceei), ch)
+
+/*   Clear interrupt/error for the channel */
+#define DMA_CLEAR_INTT_REQUEST(regs, ch)	out_8(((regs)->dmacint), ch)
+#define DMA_CLEAR_ERROR(regs, ch)	out_8(((regs)->dmacerr), ch)
+
+/* Clear done bit for the channel */
+#define DMA_CLEAR_DONE_BIT(regs, ch)	out_8(((regs)->dmacdne), ch)
+/* Set start bit for the channel */
+#define DMA_SET_START_BIT(regs, ch)	out_8(((regs)->dmassrt), ch)
+
+#define TDMTX_DMA_CH	0	/* TDM Tx uses DMA channel 0 HardWired */
+#define TDMRX_DMA_CH	1	/* TDM Rx uses DMA channel 1 Hardwired */
+#define TCD_SIZE 64	/* 64 byte buffer for TCD */
+
+/* Source address modulo */
+#define DMA_TCD1_SMOD(smod)   (((smod) & 0x1F) << 27)
+/* Source data transfer size */
+#define DMA_TCD1_SSIZE(ssize) (((ssize) & 0x7) << 24)
+
+/* Destination address modulo */
+#define DMA_TCD1_DMOD(dmod)   (((dmod) & 0x1F) << 19)
+/* data transfer size  */
+#define DMA_TCD1_DSIZE(dsize) (((dsize) & 0x7) << 16)
+
+/* Source address signed offset */
+#define DMA_TCD1_SOFF(soff)   ((soff) & 0xFFFF)
+
+/* Enable link to another channel on minor iteration completion. */
+#define DMA_TCD5_E_MINOR_LINK 0x80000000
+/* Link to this channel. */
+#define DMA_TCD5_LINK_CH(ch) (((ch) & 0x3F) << 25)
+/* Current iteration count when linking disnabled */
+#define DMA_TCD5_CITER_DISABLE_LINK(citer) (((citer) & 0x7FFF) << 16)
+/* Current iteration count when linking enabled */
+#define DMA_TCD5_CITER_ENABLE_LINK(citer) (((citer) & 0x00FF) << 16)
+/*  Destination address signed offset */
+#define DMA_TCD5_DOFF(doff) ((doff) & 0xFFFF)
+
+/* Beginning iteration count when linking disabled */
+#define DMA_TCD7_BITER_DISABLE_LINK(citer) (((citer) & 0x7FFF) << 16)
+/* Beginning iteration count when linking enabled */
+#define DMA_TCD7_BITER_ENABLE_LINK(citer) (((citer) & 0x00FF) << 16)
+#define DMA_TCD7_BWC(bw) (((bw)&0x3)<<14)	/* BandWidth Control. */
+/* Link channel number */
+#define DMA_TCD7_LINKCH(ch)   (((ch) & 0x1F) << 8)
+#define DMA_TCD7_DONE		0x00000080	/* Channel done  */
+#define DMA_TCD7_ACTIVE		0x00000040	/* Channel active */
+#define DMA_TCD7_E_MAJOR_LINK	0x00000020	/* channel to channel linking */
+#define DMA_TCD7_E_SG		0x00000010	/* Enable scatter gather */
+#define DMA_TCD7_D_REQ		0x00000008	/* Disable request */
+/* interrupt on half major counter */
+#define DMA_TCD7_INT_HALF	0x00000004
+#define DMA_TCD7_INT_MAJ	0x00000002	/* interrupt on major counter */
+#define DMA_TCD7_START		0x00000001	/* Channel start */
+
+/* Source data transfer size */
+#define SSIZE_08BITS		0x00
+#define SSIZE_16BITS		0x01
+#define SSIZE_32BITS		0x02
+#define SSIZE_64BITS		0x03
+
+/* max number of TDM-DMA channels */
+#define DMA_MAX_CHANNELS 4
+
+/* each DMA-ch contains 8 Transfer Control Discriptors  */
+#define MAX_TCD_WORD 8
+
+/* TDMGIR  General Interface Register */
+#define GIR_LPBK	0x00000004	/* loopback mode */
+#define GIR_CTS		0x00000002	/* Common TDM signals */
+#define GIR_RTS		0x00000001	/* Rx & Tx sharing */
+
+/* TDMRIR Recieve Interface Rgister */
+#define RIR_RFWM_MASK	0x00000003	/* Recieve FIFO Watermark */
+#define RIR_RFWM_SHIFT	16
+#define RIR_RFWM(x)     ((x & RIR_RFWM_MASK) << RIR_RFWM_SHIFT)
+#define RIR_RFEN	0x00008000	/* Recieve FIFO Enable */
+#define RIR_RWEN	0x00004000	/* Recieve Wide FIFO Enable */
+#define RIR_RDMA	0x00000040	/* Recieve DMA Enable */
+#define RIR_RFSD_SHIFT	0x00000004	/* Recieve Frame Sync Delay */
+#define RIR_RFSD_MASK	0x00000003
+#define RIR_RFSD(x)	((x & RIR_RFSD_MASK) << RIR_RFSD_SHIFT)
+#define RIR_RSO		0x00002000	/* Recieve sync Out */
+#define RIR_RSL		0x00000800	/* Recieve sync Out Length */
+#define RIR_RSOE	0x00000400	/* Recieve sync Out Edge */
+#define RIR_RCOE	0x00000200	/* Recieve Clock Output Enable */
+#define RIR_RSA		0x00000008	/* Recieve Sync Active */
+#define RIR_RDE		0x00000004	/* Recieve Data Edge */
+#define RIR_RFSE	0x00000002	/* Recieve Frame Sync Edge */
+#define RIR_RRDO	0x00000001	/* Revieve Reversed Data Order */
+
+/* TDMTIR  Transmit Interface Rgister */
+#define TIR_TFWM_MASK	0x00000003	/* Transmit FIFO Watermark */
+#define TIR_TFWM_SHIFT	16
+#define TIR_TFWM(x)	((x & TIR_TFWM_MASK) << TIR_TFWM_SHIFT)
+#define TIR_TFEN	0x00008000	/* Transmit FIFO Enable */
+#define TIR_TWEN	0x00004000	/* Transmit Wide FIFO Enable */
+#define TIR_TDMA	0x00000040	/* Transmit DMA Enable */
+#define TIR_TFSD_SHIFT	0x00000004	/* Transmit Frame Sync Delay */
+#define TIR_TFSD_MASK	0x00000003
+#define TIR_TFSD(x)	((x & TIR_TFSD_MASK) << TIR_TFSD_SHIFT)
+#define TIR_TSO		0x00002000	/* Transmit Sync Output */
+#define TIR_TSL		0x00000800	/* Transmit sync Out Length */
+#define TIR_TSOE	0x00000400	/* Transmit sync Out Edge */
+#define TIR_TCOE	0x00000200	/* Transmit Clock Output Enable */
+#define TIR_TSA		0x00000008	/* Transmit Sync Active */
+#define TIR_TDE		0x00000004	/* Transmit Data Edge */
+#define TIR_TFSE	0x00000002	/* Transmit Frame Sync Edge */
+#define TIR_TRDO	0x00000001	/* Transmit Reversed Data Order */
+
+/*TDMRFP  Revieve Frame Parameters */
+#define RFP_RNCF_SHIFT	0x00000010	/* Number of Channels in TDM Frame */
+#define RFP_RNCF_MASK	0x000000FF
+#define RFP_RNCF(x)	(((x - 1) & RFP_RNCF_MASK) << RFP_RNCF_SHIFT)
+#define RFP_RCS_SHIFT	0x00000004	/* Recieve Channel Size */
+#define RFP_RCS_MASK	0x00000003
+#define RFP_RCS(x)	((x & RFP_RCS_MASK) << RFP_RCS_SHIFT)
+#define RFP_RT1		0x00000002	/* Recieve T1 Frame */
+
+/*TDMTFP Transmit Frame Parameters */
+#define TFP_TNCF_SHIFT	0x00000010	/* Number of Channels in TDM Frame */
+#define TFP_TNCF_MASK	0x000000FF
+#define TFP_TNCF(x)	(((x - 1) & TFP_TNCF_MASK) << TFP_TNCF_SHIFT)
+#define TFP_TCS_SHIFT	0x00000004	/* Transmit Channel Size */
+#define TFP_TCS_MASK	0x00000003
+#define TFP_TCS(x)	((x & RFP_RCS_MASK) << RFP_RCS_SHIFT)
+#define TFP_TT1		0x00000002	/* Transmit T1 Frame */
+
+
+/* TDMRCR  Recieve Control Register */
+#define RCR_REN		0x00000001	/* Recieve Enable */
+/* TDMTCR  Transmit Control Register */
+#define TCR_TEN		0x00000001	/* Transmit Enable */
+
+/* TDMRIER receive interrupt enable register */
+#define RIER_RCEUE	0x00000100	/* Channel Enable Update Enable */
+#define RIER_RLCEE	0x00000080	/* Recieve Last Channel Event Enable */
+#define RIER_RFSEE	0x00000040	/* Recieve Frame Sync Event Enable */
+#define RIER_RFFEE	0x00000020	/* Recieve FIFO Full Event Enable */
+#define RIER_RDREE	0x00000010	/* Recieve Data Ready Event Enable */
+#define RIER_RSEEE	0x00000008	/* Recieve Sync Error Event Enable */
+#define RIER_ROEE	0x00000004	/* Recieve Overrun Event Enable */
+
+/* TDMTIER  transmit interrupt enable register */
+#define TIER_TCEUE	0x00000100	/* Channel Enable Update Enable */
+#define TIER_TLCEE	0x00000080	/* Transmit Last Channel Event */
+#define TIER_TFSEE	0x00000040	/* Transmit Frame Sync Event Enable */
+#define TIER_TFFEE	0x00000020	/* Transmit FIFO Full Event Enable */
+#define TIER_TDREE	0x00000010	/* Transmit Data Ready Event Enable */
+#define TIER_TSEEE	0x00000008	/* Transmit Sync Error Event Enable */
+#define TIER_TUEE	0x00000004	/* Transmit Overrun Event Enable */
+
+/* TDMRER  Recieve Event Register */
+#define RER_RCEU	0x00000100	/* Recieve Channel Enable Update */
+#define RER_RLCE	0x00000080	/* Recieve Last Channel Event */
+#define RER_RFSE	0x00000040	/* Recieve Frame Sync Event */
+#define RER_RFFE	0x00000020	/* Recieve FIFO Full Event */
+#define RER_RDRE	0x00000010	/* Recieve Data Ready Event */
+#define RER_RSEE	0x00000008	/* Recieve Sync Error Event */
+#define RER_ROE		0x00000004	/* Recieve Overrun Event */
+
+/* TDMTER  Transmit Event Register */
+#define TER_TCEU	0x00000100	/* Transmit Channel Enable Update */
+#define TER_TLCE	0x00000080	/* Transmit Last Channel Event */
+#define TER_TFSE	0x00000040	/* Transmit Frame Sync Event */
+#define TER_TFEE	0x00000020	/* Transmit FIFO Full Event */
+#define TER_TDRE	0x00000010	/* Transmit Data Ready Event */
+#define TER_TSEE	0x00000008	/* Transmit Sync Error Event */
+#define TER_TUE		0x00000004	/* Transmit Overrun Event */
+
+/* TDMRSR  Recieve Status Register */
+#define RSR_RFCNT	0x00000038	/* Recieve FIFO counter */
+#define RSSS_MASK	0x00000003	/* Recieve SYNC Status */
+#define RSR_RSSS_SHIFT  1
+#define RSR_RSSS(sss)	(((sss) >> (RSR_RSSS_SHIFT)) & (RSR_RSSS_MASK))
+#define RSR_RENS	0x00000001	/* Recieve Enable Status */
+
+/* TDMTSR  Transmit Status Register */
+#define TSR_TFCNT	0x00000038	/* Transmit FIFO counter */
+#define TSR_TSSS_MASK	0x00000003	/* Transmit SYNC Status */
+#define TSR_TSSS_SHIFT	1
+#define TSR_TSSS(sss)	(((sss) >> (TSR_TSSS_SHIFT)) & TSR_TSSS_MASK)
+#define TSR_TENS	0x00000001	/* Transmit Enable Status */
+
+
+/* channel parameters   */
+#define TDM_ENABLE_TIMEOUT	1000	/* time out for TDM rx, tx enable */
+#define NUM_OF_TDM_BUF		3	/* Number of tdm buffers for startlite
+					   DMA */
+#define ALIGNED_2_BYTES		0x02	/* 2-bytes alignment */
+#define ALIGNED_4_BYTES		0x04	/* 4-bytes alignment */
+#define ALIGNED_8_BYTES		0x08	/* 8-bytes alignment */
+#define ALIGNED_16_BYTES	0x10	/* 16-bytes alignment */
+#define ALIGNED_32_BYTES	0x20	/* 32-bytes alignment */
+#define ALIGNED_64_BYTES	0x40	/* 64-bytes alignment */
+
+static int tdmen = 1;
+
+module_param(tdmen, int, S_IRUSR);
+MODULE_PARM_DESC(tdmen, "Enable TDM: Enable=1, Disable=0(default)");
+
+/* DMAC TCD structure */
+struct tcd {
+	u32 tcd[MAX_TCD_WORD];
+};
+
+/* DMA Controllor */
+struct dmac_regs {
+	u32 dmacr;		/* DMA Control Register			*/
+	u32 dmaes;		/* DMA Error Status Register		*/
+	u32 dmaerqh;		/* DMA Enable Request			*/
+	u32 dmaerql;		/* DMA Enable Request			*/
+	u32 dmaeeih;		/* DMA Enable Error Interrupt		*/
+	u32 dmaeeil;		/* DMA Enable Error Interrupt		*/
+
+	u8 dmaserq;		/* DMA Set Enable Request		*/
+	u8 dmacerq;		/* DMA Clear Enable Request		*/
+	u8 dmaseei;		/* DMA Set Enable Error Interrupt	*/
+	u8 dmaceei;		/* DMA Clear Enable Error Interrupt	*/
+
+	u8 dmacint;		/* DMA Clear Interrupt Request		*/
+	u8 dmacerr;		/* DMA Clear Error			*/
+	u8 dmassrt;		/* DMA Set Start Bit			*/
+	u8 dmacdne;		/* DMA Clear Done Bit			*/
+
+	u32 dmainth;		/* DMA Interrupt Request High		*/
+	u32 dmaintl;		/* DMA Interrupt Request		*/
+	u32 dmaerrh;		/* DMA Error				*/
+	u32 dmaerrl;		/* DMA Error				*/
+	u32 dmahrsh;		/* DMA Hardware Request status		*/
+	u32 dmahrsl;		/* DMA HardWired Request status		*/
+	u32 dmagpor;		/* DMA General Purpose Register		*/
+	u8 reserved0[0xC4];
+	u8 dchpri[DMA_MAX_CHANNELS];	/* DMA Port Priority		*/
+	u8 reserved1[0xEFC];
+	struct tcd tcd[DMA_MAX_CHANNELS];	/*Transfer Control Descriptor */
+};
+
+/* TDM  Control Registers. */
+struct tdm_regs {
+	u32 gir;		/*  General Interface Register  */
+	u32 rir;		/*  Receive Interface Register  */
+	u32 tir;		/*  Transmit Interface Register */
+	u32 rfp;		/*  Receive Frame Parameters    */
+	u32 tfp;		/*  Transmit Frame Parameters   */
+	u8 reserved0[0xC];
+	u32 rcen[4];		/*  Recieve Channel Enabled     */
+	u8 reserved1[0x10];
+	u32 tcen[4];		/*  Transmit Channel Enabled    */
+	u8 reservedd2[0x10];
+	u32 tcma[4];		/*  Transmit Channel Mask       */
+	u8 reservederved3[0x10];
+	u32 rcr;		/*  Receiver Control Register           */
+	u32 tcr;		/*  Transmitter Control Register        */
+	u32 rier;		/*  Receive Interrupt Enable Register   */
+	u32 tier;		/*  Transmit Interrupt Enable Register  */
+	u8 reserved4[0x10];
+	u32 rer;		/*  Receive Event Register              */
+	u32 ter;		/*  Transmit Event Register             */
+	u32 rsr;		/*  Receive Status Register             */
+	u32 tsr;		/*  Transmit Status Register            */
+};
+
+struct tdm_data {
+	u64 rdr;		/* Receive Data Register */
+	u64 tdr;		/* Transmit Dataa Register */
+};
+
+struct tdm_clock {
+	u32 rx;			/* Transmit Dataa Register */
+	u32 tx;			/* Receive Data Register */
+};
+
+
+struct tdm_priv {
+	struct tdm_regs __iomem *tdm_regs;
+	struct tdm_data __iomem *data_regs;
+	struct dmac_regs __iomem *dmac_regs;
+	struct tdm_clock __iomem *clk_regs;
+	u32 ptdm_base;
+	u8 *tdm_input_data;
+	u8 *tdm_output_data;
+	dma_addr_t dma_input_paddr;	/* dma mapped buffer for TDM Rx */
+	dma_addr_t dma_output_paddr;	/* dma mapped buffer for TDM Tx */
+	void *dma_input_vaddr;
+	void *dma_output_vaddr;
+	u32 phase_rx;
+	u32 phase_tx;
+	struct tcd *dma_rx_tcd[NUM_OF_TDM_BUF];
+	struct tcd *dma_tx_tcd[NUM_OF_TDM_BUF];
+	dma_addr_t dma_rx_tcd_paddr;
+	dma_addr_t dma_tx_tcd_paddr;
+	void *dma_rx_tcd_vaddr;
+	void *dma_tx_tcd_vaddr;
+	u32 tdm_buffer_size;
+	u32 tdm_err_intr;
+	u32 dmac_err_intr;
+	u32 dmac_done_intr;
+	int tdm_active;
+	struct device *device;
+	spinlock_t tdmlock;
+	struct tdm_adapter *adap;
+};
+
+/* Extend a given size to make it alignable */
+static inline int ALIGNABLE_SIZE(u32 size, u32 alignment)
+{
+	return size + alignment - 1;
+}
+
+/* Align a given address */
+static inline void *ALIGN_ADDRESS(void *address, u32 alignment)
+{
+	return (void *)(((unsigned long) address + alignment - 1) &
+			(~(alignment - 1)));
+}
+
+/* Size of the buffer */
+static inline int TDM_1BUF_SIZE(u32 num_ch, u32 channel_size, u32 frame_size)
+{
+	return frame_size *
+		ALIGN(channel_size * num_ch, ALIGNED_8_BYTES);
+}
+
+/* Alignable size of the 3 buffers */
+static inline int TDM_3BUF_SIZE(u32 num_ch, u32 channel_size, u32 frame_size)
+{
+	return
+	    ALIGNABLE_SIZE((TDM_1BUF_SIZE(num_ch, channel_size, frame_size) *
+			    NUM_OF_TDM_BUF), ALIGNED_8_BYTES);
+}
+
+
+
+/* Initialize the Tx Transmit Control Descriptor parameters*/
+static void tx_tcd_init(struct tdm_priv *priv)
+{
+	int i;
+	u32 iter;
+	u32 offset;
+	dma_addr_t physaddr;
+	struct tdm_adapter *adap;
+	int bytes_in_fifo_per_frame;
+	adap = priv->adap;
+
+	if (!adap) {
+		dev_err(priv->device, "%s:Invalid handle\n", __func__);
+		return;
+	}
+	bytes_in_fifo_per_frame =
+		ALIGN(adap->adapt_cfg.num_ch * adap->adapt_cfg.slot_width, 8);
+
+	iter = (bytes_in_fifo_per_frame / NBYTES) * adap->adapt_cfg.num_frames;
+
+	for (i = 0; i < NUM_OF_TDM_BUF; i++) {
+		offset = i * adap->adapt_cfg.num_frames *
+			bytes_in_fifo_per_frame;
+		/* TCD word 0: source addr */
+		priv->dma_tx_tcd[i]->tcd[0] = (u32)priv->dma_output_paddr
+			+ offset;
+
+		/* TCD word 1: ssize=dsize=64bit, soff=8, smod=dmod=0 */
+		priv->dma_tx_tcd[i]->tcd[1] =
+			DMA_TCD1_SOFF(SOFF_VAL) | DMA_TCD1_SSIZE(SSIZE_64BITS) |
+			DMA_TCD1_DSIZE(SSIZE_64BITS);
+
+		/*
+		 * TCD word 2: number of bytes for minor loop, wide fifo
+		 *  8 bytes for dma
+		 */
+		priv->dma_tx_tcd[i]->tcd[2] = NBYTES;
+
+		/* TCD word 3: last source addr adjustment = 0 */
+		priv->dma_tx_tcd[i]->tcd[3] = SLAST;
+
+		/* TCD word 4: destination addr */
+		priv->dma_tx_tcd[i]->tcd[4] = TDM_TDR_OFFSET + priv->ptdm_base;
+
+		/*
+		 * channel to channel linking is disabled ,
+		 * destination offset is inc destination adr by 8,
+		 * current iteration(citer) = number of transfers for frame
+		 */
+		/* TCD word 5: citer count, dest addr offset */
+		priv->dma_tx_tcd[i]->tcd[5] = DMA_TCD5_CITER_DISABLE_LINK(iter);
+
+		/* TCD word 6: enable scater gather, interrupt on 1 Frame, */
+		priv->dma_tx_tcd[i]->tcd[6] = SLAST_SGA;
+
+		/*
+		 * TCD word 7: begining major iteration count(biter),
+		 * channel control/status
+		 */
+		priv->dma_tx_tcd[i]->tcd[7] =
+			DMA_TCD7_BITER_DISABLE_LINK(iter) | DMA_TCD7_E_SG;
+	}
+
+	/* Linking the TCDs togather for SG operation */
+	physaddr = priv->dma_tx_tcd_paddr;
+	priv->dma_tx_tcd[2]->tcd[6] = ALIGN(physaddr, ALIGNED_32_BYTES);
+	physaddr += TCD_SIZE;
+	priv->dma_tx_tcd[0]->tcd[6] = ALIGN(physaddr, ALIGNED_32_BYTES);
+	physaddr += TCD_SIZE;
+	priv->dma_tx_tcd[1]->tcd[6] = ALIGN(physaddr, ALIGNED_32_BYTES);
+}
+
+/* Initialize the Rx Transmit Control Discriptor parameters */
+static void rx_tcd_init(struct tdm_priv *priv)
+{
+	int i;
+	u32 iter;
+	u32 offset;
+	dma_addr_t physaddr;
+	struct tdm_adapter *adap;
+	int bytes_in_fifo_per_frame;
+	adap = priv->adap;
+	bytes_in_fifo_per_frame =
+		ALIGN(adap->adapt_cfg.num_ch * adap->adapt_cfg.slot_width, 8);
+
+	iter = (bytes_in_fifo_per_frame / NBYTES) * adap->adapt_cfg.num_frames;
+
+	for (i = 0; i < NUM_OF_TDM_BUF; i++) {
+		/* TCD word 0: source addr */
+		priv->dma_rx_tcd[i]->tcd[0] = TDM_RDR_OFFSET + priv->ptdm_base;
+
+		/* TCD word 1: ssize=dsize=64bit, soff=smod=dmod=0 */
+		priv->dma_rx_tcd[i]->tcd[1] =
+			DMA_TCD1_SSIZE(SSIZE_64BITS) |
+			DMA_TCD1_DSIZE(SSIZE_64BITS);
+
+		/*
+		 * TCD word 2: number of bytes for minor loop,
+		 * wide fifo 8 bytes for dma
+		 */
+		priv->dma_rx_tcd[i]->tcd[2] = NBYTES;
+
+		/* TCD word 3: last source addr adjustment = 0 */
+		priv->dma_rx_tcd[i]->tcd[3] = SLAST;
+
+		offset = i * adap->adapt_cfg.num_frames *
+			bytes_in_fifo_per_frame;
+
+		/* TCD word 4: destination addr */
+		priv->dma_rx_tcd[i]->tcd[4] = (u32)priv->dma_input_paddr
+			+ offset;
+
+		/*
+		 * channel to channel linking is disabled ,
+		 * destination offset is inc destination adr by 8,
+		 * current iteration(citer) = number of transfers for frame
+		 */
+		/* TCD word 5: citer count, dest addr offset */
+		priv->dma_rx_tcd[i]->tcd[5] =
+			DMA_TCD5_DOFF(DOFF_VAL) |
+			DMA_TCD5_CITER_DISABLE_LINK(iter);
+
+		/* TCD word 6: enable scater gather, interrupt on 1 Frame, */
+		priv->dma_rx_tcd[i]->tcd[6] = DLAST_SGA;
+
+		/*
+		 * TCD word 7: begining major iteration count(biter),
+		 * channel control/status
+		 */
+		priv->dma_rx_tcd[i]->tcd[7] =
+			DMA_TCD7_BITER_DISABLE_LINK(iter) | DMA_TCD7_E_SG |
+			DMA_TCD7_INT_MAJ;
+	}
+
+	/* Next TCD for SG operation */
+	physaddr = priv->dma_rx_tcd_paddr;
+	priv->dma_rx_tcd[2]->tcd[6] = ALIGN(physaddr, ALIGNED_32_BYTES);
+	physaddr += TCD_SIZE;
+	priv->dma_rx_tcd[0]->tcd[6] = ALIGN(physaddr, ALIGNED_32_BYTES);
+	physaddr += TCD_SIZE;
+	priv->dma_rx_tcd[1]->tcd[6] = ALIGN(physaddr, ALIGNED_32_BYTES);
+}
+
+static irqreturn_t dmac_done_isr(int irq, void *p)
+{
+	u32 ch;
+	int ret = IRQ_NONE;
+	struct tdm_priv *priv;
+
+	priv = p;
+
+	ch = in_be32(&priv->dmac_regs->dmaintl);
+
+	/* clear interrupt */
+	if (ch & DMAC_RX_INT) {
+		out_8(&priv->dmac_regs->dmacint, TDMRX_DMA_CH);
+		ret = IRQ_HANDLED;
+		/* track phases for Rx/Tx */
+		priv->phase_rx += 1;
+		if (priv->phase_rx == NUM_OF_TDM_BUF)
+			priv->phase_rx = 0;
+	}
+	if (ch & DMAC_TX_INT) {
+		out_8(&priv->dmac_regs->dmacint, TDMTX_DMA_CH);
+		ret = IRQ_HANDLED;
+	}
+
+	if (ret == IRQ_HANDLED) {
+		/* set the flag and wake up the thread */
+		priv->adap->tdm_rx_flag = 1;
+
+		/* schedule the tasklet */
+		if (priv->adap->tasklet_conf)
+			tasklet_schedule(&priv->adap->tdm_data_tasklet);
+	}
+	return ret;
+}
+
+static int init_tdm(struct tdm_priv *priv)
+{
+	u8 *buf;
+	int i;
+	int buf_size;
+	dma_addr_t physaddr = 0;
+	int ret = 0;
+	struct tdm_adapter *adap;
+
+
+	adap = priv->adap;
+
+	/*
+	 * Allocate memory for Rx/Tx buffer according to active time slots
+	 * BufferSize = NUM_OF_TDM_BUF * NUM_SAMPLES_PER_FRAME * slot_width *
+	 * num_ch
+	 */
+	/*Allocating Rx Buffer*/
+	buf_size = TDM_3BUF_SIZE(adap->adapt_cfg.num_ch,
+			adap->adapt_cfg.slot_width,
+			adap->adapt_cfg.num_frames);
+	buf = dma_alloc_coherent(priv->device, buf_size, &physaddr, GFP_KERNEL);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto err_alloc_ip;
+	}
+	priv->dma_input_paddr = physaddr;
+	priv->dma_input_vaddr = buf;
+	priv->tdm_input_data = ALIGN_ADDRESS(buf, ALIGNED_8_BYTES);
+
+	buf = dma_alloc_coherent(priv->device, buf_size, &physaddr, GFP_KERNEL);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto err_alloc_op;
+	}
+	priv->dma_output_paddr = physaddr;
+	priv->dma_output_vaddr = buf;
+	priv->tdm_output_data = ALIGN_ADDRESS(buf, ALIGNED_8_BYTES);
+
+	/* allocate memory for TCD buffer descriptors */
+	buf = dma_alloc_coherent(priv->device, NUM_OF_TDM_BUF * TCD_SIZE,
+			&physaddr, GFP_KERNEL);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto err_alloc_rx;
+	}
+
+	memset(buf, 0, NUM_OF_TDM_BUF * TCD_SIZE);
+	priv->dma_rx_tcd_paddr = physaddr;
+	priv->dma_rx_tcd_vaddr = buf;
+	for (i = 0; i < NUM_OF_TDM_BUF; i++) {
+		priv->dma_rx_tcd[i] = ALIGN_ADDRESS(buf, ALIGNED_32_BYTES);
+		buf += TCD_SIZE;
+	}
+
+	buf = dma_alloc_coherent(priv->device, 3 * TCD_SIZE, &physaddr,
+			GFP_KERNEL);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto err_alloc_tx;
+	}
+	memset(buf, 0, NUM_OF_TDM_BUF * TCD_SIZE);
+	priv->dma_tx_tcd_paddr = physaddr;
+	priv->dma_tx_tcd_vaddr = buf;
+	for (i = 0; i < NUM_OF_TDM_BUF; i++) {
+		priv->dma_tx_tcd[i] = ALIGN_ADDRESS(buf, ALIGNED_32_BYTES);
+		buf += TCD_SIZE;
+	}
+
+	priv->phase_rx = 0;
+	priv->phase_tx = 0;
+	return 0;
+
+err_alloc_tx:
+	dma_free_coherent(priv->device, NUM_OF_TDM_BUF * TCD_SIZE,
+			priv->dma_rx_tcd_vaddr, priv->dma_rx_tcd_paddr);
+err_alloc_rx:
+	dma_free_coherent(priv->device, buf_size, priv->dma_output_vaddr,
+			priv->dma_output_paddr);
+err_alloc_op:
+	dma_free_coherent(priv->device, buf_size, priv->dma_input_vaddr,
+			priv->dma_input_paddr);
+err_alloc_ip:
+	return ret;
+}
+
+/* TDM register programming */
+static int tdm_fsl_reg_init(struct tdm_priv *priv)
+{
+	int i;
+	int ch_size_type;
+	struct tdm_adapter *adap;
+
+	if (!priv) {
+		pr_err("%s: Invalid handle\n", __func__);
+		return -EINVAL;
+	}
+	adap = priv->adap;
+
+	/* channel/group round robin */
+	out_be32(&priv->dmac_regs->dmacr, DMACR_ERGA | DMACR_ERCA);
+	/* Enable error Interrupts for TDM Rx &Tx */
+	out_8(&priv->dmac_regs->dmaseei, TDMTX_DMA_CH);
+	in_8(&priv->dmac_regs->dmaseei);
+	out_8(&priv->dmac_regs->dmaseei, TDMRX_DMA_CH);
+	in_8(&priv->dmac_regs->dmaseei);
+	out_be32(&priv->dmac_regs->dmagpor, DMAGPOR_SNOOP);
+
+	tx_tcd_init(priv);
+	rx_tcd_init(priv);
+
+	/* TDM RD->TD loopback, Share T/R Fsync,Clock */
+	if (adap->adapt_cfg.loopback)
+		out_be32(&priv->tdm_regs->gir, GIR_LPBK | GIR_RTS);
+	else
+		out_be32(&priv->tdm_regs->gir, GIR_RTS);
+
+	/*
+	 *  Rx Water mark 0,  FIFO enable,  Wide fifo, DMA enable for RX,
+	 *  Receive Sync out, syncwidth = ch width, Rx clk out,zero sync,
+	 *  falling edge , data order
+	 */
+
+	out_be32(&priv->tdm_regs->rir,
+			RIR_RFWM(RIR_RFWM_VAL) | RIR_RFEN | RIR_RWEN |
+			RIR_RDMA | RIR_RSL | RIR_RSO | RIR_RCOE | RIR_RRDO |
+			RIR_RFSD(RIR_RFSD_VAL));
+	out_be32(&priv->tdm_regs->tir,
+			TIR_TFWM(TIR_RFWM_VAL) | TIR_TFEN | TIR_TWEN |
+			TIR_TDMA | TIR_TSL | TIR_TSO | TIR_TRDO |
+			TIR_TFSD(TIR_RFSD_VAL));
+
+	/* no of channels ,Channel size-coading */
+	switch (adap->adapt_cfg.ch_size_type) {
+	case CHANNEL_8BIT_LIN:
+		ch_size_type = CHANNEL_8BIT_LIN;
+		break;
+	case CHANNEL_8BIT_ULAW:
+		ch_size_type = CHANNEL_8BIT_ULAW;
+		break;
+	case CHANNEL_8BIT_ALAW:
+		ch_size_type = CHANNEL_8BIT_ALAW;
+		break;
+	case CHANNEL_16BIT_LIN:
+		ch_size_type = CHANNEL_16BIT_LIN;
+		break;
+	default:
+		dev_err(priv->device, "%s:Invalid channel size_type\n"
+				"Setting channel to default size: 16 bits",
+				__func__);
+		ch_size_type = CHANNEL_16BIT_LIN;
+	}
+	out_be32(&priv->tdm_regs->rfp,
+			RFP_RNCF(adap->adapt_cfg.num_ch) |
+			RFP_RCS(ch_size_type));
+	out_be32(&priv->tdm_regs->tfp,
+			TFP_TNCF(adap->adapt_cfg.num_ch) |
+			TFP_TCS(ch_size_type));
+
+	out_be32(&priv->tdm_regs->rier, 0);
+	out_be32(&priv->tdm_regs->tier, 0);
+
+	/* clear all receive and transmit chs */
+	for (i = 0; i < 4; i++) {
+		out_be32(&priv->tdm_regs->tcma[i], 0);
+		out_be32(&priv->tdm_regs->tcen[i], 0);
+		out_be32(&priv->tdm_regs->rcen[i], 0);
+	}
+
+	return 0;
+
+}
+
+static void tdm_fsl_stop(struct tdm_priv *priv)
+{
+	/* stop the Tx & Rx */
+	out_be32(&priv->tdm_regs->tcr, 0);
+	out_be32(&priv->tdm_regs->rcr, 0);
+
+	/* Clear DMA error Enable Request DMAEEIH/L */
+	out_8(&priv->dmac_regs->dmaceei, TDMTX_DMA_CH);
+	in_8(&priv->dmac_regs->dmaceei);
+	out_8(&priv->dmac_regs->dmaceei, TDMRX_DMA_CH);
+	in_8(&priv->dmac_regs->dmaceei);
+	out_8(&priv->dmac_regs->dmacint, TDMRX_DMA_CH);
+	in_8(&priv->dmac_regs->dmacint);
+	out_8(&priv->dmac_regs->dmacint, TDMTX_DMA_CH);
+	in_8(&priv->dmac_regs->dmacint);
+
+	/* disable the dma request */
+	out_8(&priv->dmac_regs->dmacerq, TDMRX_DMA_CH);
+	in_8(&priv->dmac_regs->dmacerq);
+	out_8(&priv->dmac_regs->dmacerq, TDMTX_DMA_CH);
+}
+
+static int tdm_fsl_disable(struct tdm_adapter *adap)
+{
+	struct tdm_priv *priv;
+
+	priv = adap->data;
+	if (priv->tdm_active == 0) {
+		dev_warn(priv->device, "already Disabled");
+		return 0;
+	}
+
+	spin_lock(&priv->tdmlock);
+	priv->tdm_active = 0;
+	spin_unlock(&priv->tdmlock);
+
+	return 0;
+}
+
+static int tdm_fsl_enable(struct tdm_adapter *adap)
+{
+	int i;
+	u32 ch_enab[NUM_TDMTCEN_REG] = {0};
+	unsigned long timeout;
+	struct tdm_priv *priv;
+	u32 ph;
+
+	priv = adap->data;
+	ph = priv->phase_tx;
+
+	if (priv->tdm_active == 1) {
+		dev_warn(priv->device, "already Enabled");
+		return 0;
+	}
+
+	/* enable the Channels required 0 to number of ch -1 */
+	for (i = 0; i < adap->adapt_cfg.num_ch; i++)
+		ch_enab[i / TDMTCEN_REG_LEN] |= (1 << (i & 0x1F));
+
+	for (i = 0; i < NUM_TDMTCEN_REG; i++) {
+		out_be32(&priv->tdm_regs->rcen[i], ch_enab[i]);
+		out_be32(&priv->tdm_regs->tcen[i], ch_enab[i]);
+	}
+
+	/* Clear the DONE bit */
+	out_8(&priv->dmac_regs->dmacdne, TDMRX_DMA_CH);
+	in_8(&priv->dmac_regs->dmacdne);
+	out_8(&priv->dmac_regs->dmacdne, TDMTX_DMA_CH);
+
+	/* Load the Tx  transfer control descriptors */
+	for (i = 0; i < MAX_TCD_WORD; i++)
+		out_be32(&priv->dmac_regs->tcd[TDMTX_DMA_CH].tcd[i],
+				priv->dma_tx_tcd[ph]->tcd[i]);
+
+	/* Load the Rx  transfer control descriptors */
+	for (i = 0; i < MAX_TCD_WORD; i++)
+		out_be32(&priv->dmac_regs->tcd[TDMRX_DMA_CH].tcd[i],
+				priv->dma_rx_tcd[ph]->tcd[i]);
+
+	/* enable the dma request */
+	out_8(&priv->dmac_regs->dmaserq, TDMRX_DMA_CH);
+	in_8(&priv->dmac_regs->dmaserq);
+	out_8(&priv->dmac_regs->dmaserq, TDMTX_DMA_CH);
+
+	/* Enable Receiver, transmitter */
+	timeout = jiffies + TDM_ENABLE_TIMEOUT;
+	out_be32(&priv->tdm_regs->tcr, TCR_TEN);
+	spin_event_timeout(!(in_be32(&priv->tdm_regs->tsr) & TSR_TENS),
+			timeout, 0);
+
+	timeout = jiffies + TDM_ENABLE_TIMEOUT;
+	out_be32(&priv->tdm_regs->rcr, RCR_REN);
+	spin_event_timeout(!(in_be32(&priv->tdm_regs->rsr) & RSR_RENS),
+			timeout, 0);
+
+	spin_lock(&priv->tdmlock);
+	priv->tdm_active = 1;
+	spin_unlock(&priv->tdmlock);
+
+	return 1;
+}
+
+static int tdm_fsl_read(struct tdm_adapter *adap,
+		u16 **input_tdm_buffer)
+{
+	struct tdm_priv *priv;
+	u32 phase_rx;
+	u32 buf_addr;
+	int bytes_in_fifo_per_frame, buf_size;
+
+	/* point to where to start for the current phase data processing */
+	bytes_in_fifo_per_frame =
+		ALIGN(adap->adapt_cfg.num_ch * adap->adapt_cfg.slot_width, 8);
+
+	priv = adap->data;
+	if (!priv) {
+		pr_err("%s: Invalid handle\n", __func__);
+		return -EINVAL;
+	}
+
+	if (priv->tdm_active == 0) {
+		dev_warn(priv->device, "TDM is not ready");
+		return 0;
+	}
+
+	if (priv->phase_rx == 0)
+		phase_rx = NUM_OF_TDM_BUF - 1;
+	else
+		phase_rx = priv->phase_rx - 1;
+
+	buf_size = bytes_in_fifo_per_frame * adap->adapt_cfg.num_frames;
+	buf_addr = buf_size * phase_rx;
+	*input_tdm_buffer = (u16 *)(priv->tdm_input_data + buf_addr);
+
+	return buf_size;
+}
+
+static int tdm_fsl_get_write_buf(struct tdm_adapter *adap,
+		u16 **output_tdm_buffer)
+{
+	struct tdm_priv *priv;
+	u32 tmp;
+	u32 phase_tx;
+	u32 buf_addr;
+	int bytes_in_fifo_per_frame, buf_size;
+
+	/* point to where to start for the current phase data processing */
+	bytes_in_fifo_per_frame =
+		ALIGN(adap->adapt_cfg.num_ch * adap->adapt_cfg.slot_width, 8);
+
+	priv = adap->data;
+	if (!priv) {
+		pr_err("%s: Invalid handle\n", __func__);
+		return -EINVAL;
+	}
+
+	if (priv->tdm_active == 0) {
+		dev_warn(priv->device, "TDM is not ready");
+		return 0;
+	}
+
+	tmp = in_be32(&priv->dmac_regs->tcd[TDMTX_DMA_CH].tcd[0]);
+
+	tmp -= priv->dma_tx_tcd[0]->tcd[0];
+
+	priv->phase_tx = tmp/(bytes_in_fifo_per_frame *
+			adap->adapt_cfg.num_frames);
+
+	if (priv->phase_tx == 0)
+		phase_tx = NUM_OF_TDM_BUF - 1;
+	else
+		phase_tx = priv->phase_tx - 1;
+
+	buf_size = bytes_in_fifo_per_frame * adap->adapt_cfg.num_frames;
+	buf_addr = buf_size * phase_tx;
+	*output_tdm_buffer = (u16 *)(priv->tdm_output_data + buf_addr);
+
+	return buf_size;
+}
+
+static const struct tdm_adapt_algorithm tdm_algo = {
+	.tdm_read = tdm_fsl_read,
+	.tdm_get_write_buf = tdm_fsl_get_write_buf,
+	.tdm_enable = tdm_fsl_enable,
+	.tdm_disable = tdm_fsl_disable,
+};
+
+static struct tdm_adapter tdm_fsl_ops = {
+	.owner = THIS_MODULE,
+	.name = "fsl_tdm",
+	.algo = &tdm_algo,
+};
+
+static int __devinit tdm_fsl_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct tdm_priv *priv;
+	struct resource res;
+
+	priv = kzalloc(sizeof(struct tdm_priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto err_alloc;
+	}
+
+	dev_set_drvdata(&pdev->dev, priv);
+	priv->device = &pdev->dev;
+	ret = of_address_to_resource(pdev->dev.of_node, 0, &res);
+	if (ret) {
+		ret = -EINVAL;
+		goto err_resource;
+	}
+
+	priv->ptdm_base = (u32)res.start;
+	priv->tdm_regs = of_iomap(pdev->dev.of_node, 0);
+	if (!priv->tdm_regs) {
+		ret = -ENOMEM;
+		goto err_tdmregs;
+	}
+
+	priv->dmac_regs = of_iomap(pdev->dev.of_node, 1);
+	if (!priv->dmac_regs) {
+		ret = -ENOMEM;
+		goto err_dmacreg;
+	}
+
+	priv->dmac_done_intr = irq_of_parse_and_map(pdev->dev.of_node, 0);
+	if (priv->dmac_done_intr ==  NO_IRQ) {
+		ret = -EINVAL;
+		goto err_dmacdone_irqmap;
+	}
+	ret = request_irq(priv->dmac_done_intr, dmac_done_isr, 0,
+			"dmac_done_isr", priv);
+	if (ret)
+		goto err_dmacdoneisr;
+
+	priv->adap = &tdm_fsl_ops;
+
+	/* Wait q initilization */
+	priv->adap->tdm_rx_flag = 0;
+	/* TODO - these should be configured by dts or init time */
+	priv->adap->data = priv;
+	priv->adap->parent = &pdev->dev;
+
+	ret = tdm_add_adapter(priv->adap);
+	if (ret < 0) {
+		dev_err(priv->device, "failed to add adapter\n");
+		goto fail_adapter;
+	}
+
+	/* Device does not supports 36 bit mode */
+	dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+
+	ret = init_tdm(priv);
+	if (ret)
+		goto err_tdminit;
+
+	tdm_fsl_reg_init(priv);
+
+	spin_lock_init(&priv->tdmlock);
+	priv->tdm_active = 0;
+
+	if (tdmen) {
+		ret = tdm_fsl_enable(priv->adap);
+		if (!ret)
+			goto err_tdminit;
+	}
+
+	return 0;
+
+err_tdminit:
+fail_adapter:
+	free_irq(priv->dmac_done_intr, priv);
+err_dmacdoneisr:
+	free_irq(priv->tdm_err_intr, priv);
+err_dmacdone_irqmap:
+	irq_dispose_mapping(priv->dmac_done_intr);
+err_dmacreg:
+	iounmap(priv->dmac_regs);
+err_tdmregs:
+err_resource:
+	dev_set_drvdata(&pdev->dev, NULL);
+	kfree(priv);
+err_alloc:
+	return ret;
+}
+
+static int __devexit tdm_fsl_remove(struct platform_device *pdev)
+{
+	struct tdm_priv *priv;
+	int buf_size;
+	struct tdm_adapter *adap;
+
+	if (!pdev) {
+		pr_err("%s: Invalid handle\n", __func__);
+		return -EINVAL;
+	}
+
+	priv = dev_get_drvdata(&pdev->dev);
+	adap = priv->adap;
+
+	tdm_fsl_disable(priv->adap);
+
+	tdm_fsl_stop(priv);
+
+	tdm_del_adapter(priv->adap);
+	dev_set_drvdata(&pdev->dev, NULL);
+
+	/* free the irqs and dispose their mapping */
+	free_irq(priv->tdm_err_intr, priv);
+	free_irq(priv->dmac_done_intr, priv);
+	irq_dispose_mapping(priv->tdm_err_intr);
+	irq_dispose_mapping(priv->dmac_done_intr);
+	iounmap(priv->tdm_regs);
+	iounmap(priv->dmac_regs);
+
+	/* free the buffers */
+	buf_size =
+		TDM_3BUF_SIZE(adap->adapt_cfg.num_ch,
+				adap->adapt_cfg.slot_width,
+				adap->adapt_cfg.num_frames);
+	dma_free_coherent(priv->device, buf_size, priv->dma_input_vaddr,
+			priv->dma_input_paddr);
+	dma_free_coherent(priv->device, buf_size, priv->dma_output_vaddr,
+			priv->dma_output_paddr);
+
+	/* free the TCDs */
+	dma_free_coherent(priv->device, NUM_OF_TDM_BUF * TCD_SIZE,
+			priv->dma_rx_tcd_vaddr,  priv->dma_rx_tcd_paddr);
+	dma_free_coherent(priv->device, NUM_OF_TDM_BUF * TCD_SIZE,
+			priv->dma_tx_tcd_vaddr,  priv->dma_tx_tcd_paddr);
+	dev_set_drvdata(&pdev->dev, NULL);
+	kfree(priv);
+	return 0;
+}
+
+static const struct of_device_id fsl_tdm_match[] = {
+	{
+		.compatible = "fsl,tdm1.0",
+	},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, fsl_tdm_match);
+
+static struct platform_driver tdm_fsl_driver = {
+	.driver		= {
+		.owner	= THIS_MODULE,
+		.name	= DRV_NAME,
+		.of_match_table	= fsl_tdm_match,
+
+	},
+	.probe		= tdm_fsl_probe,
+	.remove		= __devexit_p(tdm_fsl_remove),
+};
+
+static int __init tdm_fsl_init(void)
+{
+	int ret;
+	pr_info(DRV_NAME ": " DRV_DESC ":Init\n");
+	ret = platform_driver_register(&tdm_fsl_driver);
+	if (ret)
+		pr_err(DRV_NAME  "of_register_platform_driver failed (%i)\n",
+				ret);
+	return ret;
+}
+
+static void __exit tdm_fsl_exit(void)
+{
+	pr_info(DRV_NAME ": " DRV_DESC ":Exit\n");
+	platform_driver_unregister(&tdm_fsl_driver);
+}
+
+module_init(tdm_fsl_init);
+module_exit(tdm_fsl_exit);
+/*
+   module_platform_driver(tdm_fsl_driver);
+ */
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("P.V.Suresh, Freescale Semiconductor");
+MODULE_DESCRIPTION("Driver For Freescale TDM controller");
-- 
1.5.6.5

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-07-27 14:05 [2/3][PATCH][v2] TDM Framework sandeep
  2012-07-27 14:05 ` [1/3][PATCH][v2] Adding documentation for TDM sandeep
@ 2012-07-27 14:11 ` John Stoffel
  2012-07-30  9:29   ` Singh Sandeep-B37400
  2012-07-27 14:51 ` Russell King - ARM Linux
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: John Stoffel @ 2012-07-27 14:11 UTC (permalink / raw)
  To: sandeep; +Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel


> From: Sandeep Singh <Sandeep@freescale.com>
> TDM Framework is an attempt to provide a platform independent layer which can
> offer a standard interface  for TDM access to different client modules.

Please don't use TLAs (Three Letter Acronyms) like TDM without
explaining the clearly and up front.  It makes it hard for anyone else
who doens't know your code to look it over without having to spend
lots of time poking around to figure it out from either context or
somewhere else.

John

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-07-27 14:05 [2/3][PATCH][v2] TDM Framework sandeep
  2012-07-27 14:05 ` [1/3][PATCH][v2] Adding documentation for TDM sandeep
  2012-07-27 14:11 ` [2/3][PATCH][v2] TDM Framework John Stoffel
@ 2012-07-27 14:51 ` Russell King - ARM Linux
  2012-07-30 13:00   ` Singh Sandeep-B37400
  2012-07-27 15:25 ` Francois Romieu
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Russell King - ARM Linux @ 2012-07-27 14:51 UTC (permalink / raw)
  To: sandeep; +Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel

On Fri, Jul 27, 2012 at 07:35:38PM +0530, sandeep@freescale.com wrote:
> +static DEFINE_MUTEX(tdm_core_lock);
> +static DEFINE_IDR(tdm_adapter_idr);
> +/* List of TDM adapters registered with TDM framework */
> +LIST_HEAD(adapter_list);
> +
> +/* List of TDM clients registered with TDM framework */
> +LIST_HEAD(driver_list);

These two are far too generic to be public.  Have you checked your code
with sparse?  I think not.

> +
> +/*
> + * In case the previous data is not fetched by the client driver, the
> + * de-interleaving function will  discard the old data and rewrite the
> + * new data
> + */
> +
> +static int use_latest_tdm_data = 1;
> +
> +/* Data structures required for sysfs */
> +static struct tdm_sysfs attr = {
> +	.attr.name = "use_latest_data",
> +	.attr.mode = 0664,
> +	.cmd_type = TDM_LATEST_DATA,
> +};
> +
> +static struct attribute *tdm_attr[] = {
> +	&attr.attr,
> +	NULL
> +};
> +
> +const struct sysfs_ops tdm_ops = {
> +	.show = tdm_show_sysfs,
> +	.store = tdm_store_sysfs,
> +};

Again, lack of static.

> +
> +static struct kobj_type tdm_type = {
> +	.sysfs_ops = &tdm_ops,
> +	.default_attrs = tdm_attr,
> +};
> +
> +/* tries to match client driver with the adapter */
> +static int tdm_device_match(struct tdm_driver *driver, struct tdm_adapter *adap)
> +{
> +	/* match on an id table if there is one */
> +	if (driver->id_table && driver->id_table->name[0]) {
> +		if (!(strcmp(driver->id_table->name, adap->name)))
> +			return (int)driver->id_table;

Casting a pointer to 'int' is not a good thing to do.  Please fix this.

> +	}
> +	return 0;
> +}
> +
> +static int tdm_attach_driver_adap(struct tdm_driver *driver,
> +		struct tdm_adapter *adap)
> +{
> +	int ret = 0;
> +	/* if driver is already attached to any other adapter, return*/
> +	if (driver->adapter && (driver->adapter != adap))

Additional parens not required.

> +		return 0;
> +
> +	driver->adapter = adap;
> +
> +	if (driver->attach_adapter) {
> +		ret = driver->attach_adapter(adap);
> +		if (ret < 0) {
> +			pr_err("tdm: attach_adapter failed for driver [%s]"
> +					"err:%d\n", driver->name, ret);
> +			return ret;
> +		}
> +	}
> +	adap->drv_count++;
> +
> +	if (!adap->tasklet_conf) {
> +		tdm_sysfs_init();
> +		tasklet_init(&adap->tdm_data_tasklet, tdm_data_tasklet_fn,
> +				(unsigned long)adap);

Why not init this tasklet when the struct tdm_adapter is first created?
Why do you need to wait, and then have state tracking for this?

> +		adap->tasklet_conf = 1;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Detach client driver and adapter */
> +static int tdm_detach_driver_adap(struct tdm_driver *driver,
> +		struct tdm_adapter *adap)
> +{
> +	int res = 0;
> +
> +	if (!driver->adapter || (driver->adapter != adap))

Additional parens not required.

> +		return 0;
> +
> +	adap->drv_count--;
> +
> +	/* If no more driver is registed with the adapter*/
> +	if (!adap->drv_count && adap->tasklet_conf) {
> +		tasklet_disable(&adap->tdm_data_tasklet);
> +		tasklet_kill(&adap->tdm_data_tasklet);
> +		adap->tasklet_conf = 0;
> +	}
> +
> +	if (driver->detach_adapter) {
> +		if (driver->detach_adapter(adap))
> +			pr_err("tdm: detach_adapter failed for driver [%s]\n",
> +					driver->name);
> +	}
> +
> +	driver->adapter = NULL;
> +	return res;
> +}
> +
> +/* TDM adapter Registration/De-registration with TDM framework */
> +
> +static int tdm_register_adapter(struct tdm_adapter *adap)
> +{
> +	int res = 0;
> +	struct tdm_driver *driver, *next;
> +
> +	mutex_init(&adap->adap_lock);
> +	INIT_LIST_HEAD(&adap->myports);
> +	spin_lock_init(&adap->portlist_lock);
> +
> +	adap->drv_count = 0;
> +	adap->tasklet_conf = 0;
> +
> +	list_add_tail(&adap->list, &adapter_list);

What protects this list?

> +
> +	/* initialization of driver by framework in default configuration */
> +	init_config_adapter(adap);
> +
> +	/* Notify drivers */
> +	pr_info("adapter [%s] registered\n", adap->name);
> +	mutex_lock(&tdm_core_lock);
> +	list_for_each_entry_safe(driver, next, &driver_list, list) {
> +		if (tdm_device_match(driver, adap)) {
> +			res = tdm_attach_driver_adap(driver, adap);
> +			if (res == 0) {
> +				pr_info("tdm: Driver(ID=%d) is "
> +						"attached with Adapter %s(ID"
> +						" = %d)\n", driver->id,
> +						adap->name, adap->id);
> +			} else {
> +				pr_err("tdm: Driver(ID=%d) is unable "
> +						"to attach with Adapter %s(ID = %d)\n",
> +						driver->id, adap->name,
> +						adap->id);
> +			}
> +		}
> +	}
> +	mutex_unlock(&tdm_core_lock);
> +
> +	return res;
> +}
> +
> +/*
> + * tdm_add_adapter - declare tdm adapter, use dynamic device number
> + * @adapter: the adapter to add
> + * Context: can sleep
> + *
> + * This routine is used to declare a TDM adapter
> + * When this returns zero, a new device number will be allocated and stored
> + * in adap->id, and the specified adapter became available for the clients.
> + * Otherwise, a negative error number value is returned.
> + */
> +int tdm_add_adapter(struct tdm_adapter *adapter)
> +{
> +	int id, res = 0;
> +
> +retry:
> +	if (idr_pre_get(&tdm_adapter_idr, GFP_KERNEL) == 0)
> +		return -ENOMEM;
> +
> +	mutex_lock(&tdm_core_lock);
> +	res = idr_get_new(&tdm_adapter_idr, adapter, &id);
> +	mutex_unlock(&tdm_core_lock);
> +
> +	if (res < 0) {
> +		if (res == -EAGAIN)
> +			goto retry;
> +		return res;
> +	}
> +
> +	adapter->id = id;
> +	return tdm_register_adapter(adapter);
> +}
> +EXPORT_SYMBOL(tdm_add_adapter);
> +
> +
> +/*
> + * tdm_del_adapter - unregister TDM adapter
> + * @adap: the adapter being unregistered
> + *
> + * This unregisters an TDM adapter which was previously registered
> + * by @tdm_add_adapter.
> + */
> +int tdm_del_adapter(struct tdm_adapter *adap)
> +{
> +	int res = 0;
> +	struct tdm_adapter *found;
> +	struct tdm_driver *driver, *next;
> +
> +	/* First make sure that this adapter was ever added */
> +	mutex_lock(&tdm_core_lock);
> +	found = idr_find(&tdm_adapter_idr, adap->id);
> +	mutex_unlock(&tdm_core_lock);
> +	if (found != adap) {
> +		pr_err("tdm: attempting to delete unregistered "
> +				"adapter [%s]\n", adap->name);
> +		return -EINVAL;
> +	}
> +
> +	/* disable and kill the data processing tasklet */
> +	if (adap->tasklet_conf) {
> +		tasklet_disable(&adap->tdm_data_tasklet);
> +		tasklet_kill(&adap->tdm_data_tasklet);
> +		adap->tasklet_conf = 0;
> +	}
> +
> +	/*
> +	 * Detach any active ports. This can't fail, thus we do not
> +	 * checking the returned value.
> +	 */
> +	mutex_lock(&tdm_core_lock);
> +	list_for_each_entry_safe(driver, next, &driver_list, list) {
> +		if (tdm_device_match(driver, adap)) {
> +			tdm_detach_driver_adap(driver, adap);
> +			pr_info(
> +					"Driver(ID=%d) is detached from Adapter %s(ID = %d)\n",
> +					driver->id, adap->name, adap->id);
> +		}
> +	}
> +	idr_remove(&tdm_adapter_idr, adap->id);
> +	mutex_unlock(&tdm_core_lock);
> +
> +	pr_debug("adapter [%s] unregistered\n", adap->name);
> +
> +	list_del(&adap->list);

What protects this delete?

> +	/*
> +	 * Clear the device structure in case this adapter is ever going to be
> +	 * added again
> +	 */
> +	adap->parent = NULL;
> +
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_del_adapter);
> +
> +/* TDM Client Drivers Registration/De-registration Functions */
> +int tdm_register_driver(struct tdm_driver *driver)
> +{
> +	int res = 0;
> +	struct tdm_adapter *adap, *next;
> +
> +	list_add_tail(&driver->list, &driver_list);

What serializes this list?

> +
> +	mutex_lock(&tdm_core_lock);
> +	/* Walk the adapters that are already present */
> +	list_for_each_entry_safe(adap, next, &adapter_list, list) {
> +		if (tdm_device_match(driver, adap)) {
> +			res = tdm_attach_driver_adap(driver, adap);
> +			if (res == 0) {
> +				pr_info("TDM Driver(ID=%d)is attached with "
> +						"Adapter%s(ID = %d) drv_count=%d",
> +						driver->id, adap->name,
> +						adap->id, adap->drv_count);
> +			} else {
> +				pr_err("TDM Driver(ID=%d) unable to attach "
> +						"to Adapter%s(ID = %d) drv_count=%d",
> +						driver->id, adap->name,
> +						adap->id, adap->drv_count);
> +			}
> +			break;
> +		}
> +	}
> +	mutex_unlock(&tdm_core_lock);
> +
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_register_driver);
> +
> +/*
> + * tdm_unregister_driver - unregister TDM client driver from TDM framework
> + * @driver: the driver being unregistered
> + */
> +void tdm_unregister_driver(struct tdm_driver *driver)
> +{
> +	/*
> +	 * A driver can register to only one adapter,
> +	 * so no need to browse the list
> +	 */
> +	mutex_lock(&tdm_core_lock);
> +	tdm_detach_driver_adap(driver, driver->adapter);
> +	mutex_unlock(&tdm_core_lock);
> +
> +	list_del(&driver->list);

What serializes this delete?

> +
> +	pr_debug("tdm-core: driver [%s] unregistered\n", driver->name);
> +}
> +EXPORT_SYMBOL(tdm_unregister_driver);
> +
> +/* Interface to the tdm device/adapter */
> +
> +/*
> + * tdm_adap_send - issue a TDM write
> + * @adap: Handle to TDM device
> + * @buf: Data that will be written to the TDM device
> + * @count: How many bytes to write
> + *
> + * Returns negative errno, or else the number of bytes written.
> + */
> +int tdm_adap_send(struct tdm_adapter *adap, void **buf, int count)
> +{
> +	int res;
> +
> +	if (adap->algo->tdm_write)
> +		res = adap->algo->tdm_write(adap, buf, count);
> +	else {
> +		pr_err("TDM level write not supported\n");

Is there no associated struct device which could be used for these
error messages?  What if you have more than one TDM device?  It would
be useful to know which is producing errors.

> +		return -EOPNOTSUPP;
> +	}
> +
> +	/*
> +	 * If everything went ok (i.e. frame transmitted), return #bytes
> +	 * transmitted, else error code.
> +	 */
> +	return (res == 1) ? count : res;
> +}
> +EXPORT_SYMBOL(tdm_adap_send);
> +
> +/*
> + * tdm_adap_recv - issue a TDM read
> + * @adap: Handle to TDM device
> + * @buf: Where to store data read from TDM device
> + *
> + * Returns negative errno, or else the number of bytes read.
> + */
> +int tdm_adap_recv(struct tdm_adapter *adap, void **buf)
> +{
> +	int res;
> +
> +	if (adap->algo->tdm_read)
> +		res = adap->algo->tdm_read(adap, (u16 **)buf);
> +	else {
> +		pr_err("TDM level read not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +	/*
> +	 * If everything went ok (i.e. frame received), return #bytes
> +	 * transmitted, else error code.
> +	 */
> +	return res;
> +}
> +
> +/*
> + * tdm_adap_get_write_buf - get next write TDM device buffer
> + * @adap: Handle to TDM device
> + * @buf: pointer to TDM device buffer
> + *
> + * Returns negative errno, or else size of the write buffer.
> + */
> +int tdm_adap_get_write_buf(struct tdm_adapter *adap, void **buf)
> +{
> +	int res;
> +
> +	if (adap->algo->tdm_get_write_buf) {
> +		res = adap->algo->tdm_get_write_buf(adap, (u16 **)buf);
> +	} else {
> +		pr_err("TDM level write buf get not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +	/*
> +	 * If everything went ok (i.e. 1 msg received), return #bytes
> +	 * transmitted, else error code.
> +	 */
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_adap_get_write_buf);
> +
> +int tdm_adap_enable(struct tdm_driver *drv)
> +{
> +	int res;
> +	struct tdm_adapter *adap;
> +	adap = drv->adapter;

The above two lines can become one line.

> +
> +	if (adap->algo->tdm_enable) {
> +		res = adap->algo->tdm_enable(adap);
> +	} else {
> +		pr_err("TDM level enable not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_adap_enable);
> +
> +int tdm_adap_disable(struct tdm_driver *drv)
> +{
> +	int res;
> +	struct tdm_adapter *adap;
> +	adap = drv->adapter;

Ditto.

> +
> +	if (adap->algo->tdm_disable) {
> +		res = adap->algo->tdm_disable(adap);
> +	} else {
> +		pr_err("TDM level enable not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_adap_disable);
> +
> +struct tdm_adapter *tdm_get_adapter(int id)
> +{
> +	struct tdm_adapter *adapter;
> +
> +	mutex_lock(&tdm_core_lock);
> +	adapter = idr_find(&tdm_adapter_idr, id);
> +	if (adapter && !try_module_get(adapter->owner))
> +		adapter = NULL;
> +
> +	mutex_unlock(&tdm_core_lock);
> +
> +	return adapter;
> +}
> +EXPORT_SYMBOL(tdm_get_adapter);
> +
> +void tdm_put_adapter(struct tdm_adapter *adap)
> +{
> +	module_put(adap->owner);
> +}
> +EXPORT_SYMBOL(tdm_put_adapter);
> +
> +
> +/* Port Level APIs of TDM Framework */
> +int tdm_port_open(struct tdm_driver *driver, struct tdm_port **h_port)
> +{
> +	struct tdm_port *port;
> +	struct tdm_adapter *adap;
> +	unsigned long flags;
> +	int res = 0;
> +
> +	/*
> +	 * This creates an anonymous tdm_port, which may later be
> +	 * pointed to some slot.
> +	 */
> +	port = kzalloc(sizeof(*port), GFP_KERNEL);
> +	if (!port) {
> +		res = -ENOMEM;
> +		return res;
> +	}
> +
> +	adap = tdm_get_adapter(driver->adapter->id);
> +	if (!adap)
> +		return -ENODEV;
> +
> +	port->rx_max_frames = NUM_SAMPLES_PER_FRAME;
> +	port->port_cfg.port_mode = TDM_PORT_CHANNELIZED;
> +
> +	snprintf(driver->name, TDM_NAME_SIZE, "tdm-dev");
> +	port->driver = driver;
> +	port->adapter = adap;
> +
> +	spin_lock_irqsave(&adap->portlist_lock, flags);
> +	list_add_tail(&port->list, &adap->myports);
> +	spin_unlock_irqrestore(&adap->portlist_lock, flags);
> +
> +	INIT_LIST_HEAD(&port->mychannels);
> +
> +	*h_port = port;
> +	return res;
> +
> +}
> +EXPORT_SYMBOL(tdm_port_open);
> +
> +int tdm_port_close(struct tdm_port *h_port)
> +{
> +	struct tdm_adapter *adap;
> +	struct tdm_driver *driver;
> +	struct tdm_port *port;
> +	struct tdm_channel *temp, *channel;
> +	unsigned long flags;
> +	int res = 0;
> +	port = h_port;
> +
> +	driver =  port->driver;
> +
> +	list_for_each_entry_safe(channel, temp, &port->mychannels, list) {
> +		if (channel)
> +			if (channel->in_use) {

if (channel && channel->in_use) {

> +				pr_err("tdm: Cannot close port. Channel in"
> +						"use\n");

Don't wrap error messages.

> +				res = -ENXIO;
> +				goto out;
> +			}
> +	}
> +	adap = driver->adapter;
> +	tdm_put_adapter(adap);
> +
> +	spin_lock_irqsave(&adap->portlist_lock, flags);
> +	list_del(&port->list);
> +	spin_unlock_irqrestore(&adap->portlist_lock, flags);
> +
> +	if (port->p_port_data != NULL) {
> +		int i;
> +		struct tdm_bd *ch_bd;
> +
> +		/*
> +		 * If the tdm is in channelised mode,
> +		 * de-allocate the channelised buffer
> +		 */
> +		ch_bd = &(port->p_port_data->rx_data_fifo[0]);
> +		for (i = 0; ch_bd && i < TDM_CH_RX_BD_RING_SIZE; i++) {
> +			ch_bd->flag = 0;
> +			ch_bd++;
> +		}
> +		ch_bd = &(port->p_port_data->tx_data_fifo[0]);
> +		for (i = 0; ch_bd && i < TDM_CH_TX_BD_RING_SIZE; i++) {
> +			ch_bd->flag = 0;
> +			ch_bd++;
> +		}
> +		kfree(port->p_port_data);
> +	}
> +	kfree(port);
> +	return res;
> +out:
> +	if (port)
> +		kfree(port->p_port_data);
> +	kfree(port);
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_port_close);
> +
> +int tdm_channel_read(struct tdm_port *h_port, struct tdm_channel *h_channel,
> +		void *p_data, u16 *size)
> +{
> +	struct tdm_channel *channel;
> +	struct tdm_bd *rx_bd;
> +	unsigned long flags;
> +	int res = 0;
> +	unsigned short *buf, *buf1;
> +	channel = h_channel;
> +
> +	if (!channel->p_ch_data || !channel->in_use)
> +		return -EIO;
> +
> +	spin_lock_irqsave(&channel->p_ch_data->rx_channel_lock, flags);
> +	rx_bd = channel->p_ch_data->rx_out_data;
> +
> +	if (rx_bd->flag) {
> +		*size = rx_bd->length;
> +		buf = (u16 *) p_data;
> +		buf1 = (u16 *)rx_bd->p_data;
> +		memcpy(buf1, buf, NUM_SAMPLES_PER_FRAME);
> +		rx_bd->flag = 0;
> +		rx_bd->offset = 0;
> +		channel->p_ch_data->rx_out_data = (rx_bd->wrap) ?
> +			channel->p_ch_data->rx_data_fifo : rx_bd + 1;
> +
> +	} else {
> +		spin_unlock_irqrestore(&channel->p_ch_data->rx_channel_lock,
> +				flags);
> +		pr_debug("No Data Available");
> +		return -EAGAIN;
> +	}
> +	spin_unlock_irqrestore(&channel->p_ch_data->rx_channel_lock, flags);
> +
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_channel_read);
> +
> +
> +int tdm_channel_write(struct tdm_port *h_port, struct tdm_channel *h_channel,
> +		void *p_data, u16 size)
> +{
> +	struct tdm_port *port;
> +	struct tdm_channel *channel;
> +	struct tdm_bd *tx_bd;
> +	unsigned long flags;
> +	int err = 0;
> +	port = h_port;
> +	channel = h_channel;
> +#ifdef DEBUG
> +	bool data_flag = 0;
> +#endif
> +
> +	if (p_data == NULL) { /* invalid data*/
> +		pr_err("tdm: Invalid Data");
> +		return -EINVAL;
> +	}
> +
> +	if (!channel->p_ch_data || !channel->in_use)
> +		return -EIO;
> +
> +	spin_lock_irqsave(&channel->p_ch_data->tx_channel_lock, flags);
> +	tx_bd = channel->p_ch_data->tx_in_data;
> +
> +	if (!tx_bd->flag) {
> +		tx_bd->length = size;
> +		memcpy(tx_bd->p_data, p_data,
> +				size * port->adapter->adapt_cfg.slot_width);
> +		tx_bd->flag = 1;
> +		tx_bd->offset = 0;
> +		channel->p_ch_data->tx_in_data = (tx_bd->wrap) ?
> +			channel->p_ch_data->tx_data_fifo : tx_bd+1;
> +		port->port_stat.tx_pkt_count++;
> +#ifdef DEBUG
> +		data_flag = 1;
> +#endif
> +	} else {
> +		spin_unlock_irqrestore(&channel->p_ch_data->tx_channel_lock,
> +				flags);
> +		port->port_stat.tx_pkt_drop_count++;
> +		pr_err("tdm: Transmit failed.");
> +		return -ENOMEM;
> +	}
> +	spin_unlock_irqrestore(&channel->p_ch_data->tx_channel_lock, flags);
> +
> +#ifdef	DEBUG
> +	if (data_flag) {
> +		int k;
> +		pr_info("\nTX port:%d - Write - Port TX-%d\n",
> +				port->port_id, size);
> +		for (k = 0; k < size; k++)
> +			pr_info("%x", p_data[k]);
> +		pr_info("\n");
> +	}
> +#endif
> +	return err;
> +}
> +EXPORT_SYMBOL(tdm_channel_write);
> +
> +/*
> + * Driver Function for select and poll. Based on Channel, it sleeps on
> + * waitqueue
> + */
> +int tdm_ch_poll(struct tdm_channel *h_channel, unsigned int wait_time)
> +{
> +	struct tdm_channel *channel;
> +	channel = h_channel;
> +
> +	if (!channel->p_ch_data || !channel->in_use)
> +		return -EIO;
> +
> +	if (channel->p_ch_data->rx_out_data->flag) {
> +		pr_debug("Data Available");
> +		return 0;
> +	}
> +	if (wait_time) {
> +		unsigned long timeout = msecs_to_jiffies(wait_time);
> +
> +		wait_event_interruptible_timeout(channel->ch_wait_queue,
> +				channel->p_ch_data->rx_out_data->flag,
> +				timeout);
> +
> +		if (channel->p_ch_data->rx_out_data->flag) {
> +			pr_debug("Data Available");
> +			return 0;
> +		}
> +	}
> +	return -EAGAIN;

That is incorrect.  -EAGAIN is the wrong return code when
wait_event_interruptible_timeout() returns -ERESTARTSYS.

> +}
> +EXPORT_SYMBOL(tdm_ch_poll);
> +
> +unsigned int tdm_port_get_stats(struct tdm_port *h_port,
> +		struct tdm_port_stats *portstat)
> +{
> +	struct tdm_port *port;
> +	int port_num;
> +	port = h_port;
> +
> +	if (port == NULL || portstat == NULL) { /* invalid handle */
> +		pr_err("tdm: Invalid Handle");
> +		return -ENXIO;
> +	}
> +	port_num =  port->port_id;
> +
> +	memcpy(portstat, &port->port_stat, sizeof(struct tdm_port_stats));
> +
> +	pr_info("TDM Port %d Get Stats", port_num);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(tdm_port_get_stats);
> +
> +/* Data handling functions */
> +
> +static int tdm_data_rx_deinterleave(struct tdm_adapter *adap)
> +{
> +	struct tdm_port *port, *next;
> +	struct tdm_channel *channel, *temp;
> +	struct tdm_bd	*ch_bd;
> +
> +	int i, buf_size, ch_data_len;
> +	u16 *input_tdm_buffer;
> +	u16 *pcm_buffer;
> +	int slot_width;
> +	int frame_ch_data_size;
> +	bool ch_data;
> +	int bytes_in_fifo_per_frame;
> +	int bytes_slot_offset;
> +
> +	ch_data_len = NUM_SAMPLES_PER_FRAME;
> +	frame_ch_data_size = NUM_SAMPLES_PER_FRAME;
> +	ch_data = 0;
> +
> +	slot_width = adap->adapt_cfg.slot_width;
> +	buf_size = tdm_adap_recv(adap, (void **)&input_tdm_buffer);
> +	if (buf_size <= 0 || !input_tdm_buffer)
> +		return -EINVAL;
> +
> +	bytes_in_fifo_per_frame = buf_size/frame_ch_data_size;

Spaces around /

> +	bytes_slot_offset = bytes_in_fifo_per_frame/slot_width;

Spaces around /

> +
> +	/* de-interleaving for all ports*/
> +	list_for_each_entry_safe(port, next, &adap->myports, list) {
> +
> +		list_for_each_entry_safe(channel, temp, &port->mychannels,
> +				list) {

Why do you need the _safe variants here?  I can't see anything in this
code which manipulates the lists.

> +			/* if the channel is not open */
> +			if (!channel->in_use || !channel->p_ch_data)
> +				continue;
> +			ch_bd = channel->p_ch_data->rx_in_data;
> +			spin_lock(&channel->p_ch_data->rx_channel_lock);
> +			/*if old data is to be discarded */
> +			if (use_latest_tdm_data && ch_bd->flag) {
> +				ch_bd->flag = 0;
> +				ch_bd->offset = 0;
> +				if (ch_bd == channel->p_ch_data->rx_out_data)
> +					channel->p_ch_data->rx_out_data =
> +						ch_bd->wrap ?
> +						channel->p_ch_data->rx_data_fifo
> +						: ch_bd+1;
> +				port->port_stat.rx_pkt_drop_count++;
> +			}
> +			/* if the bd is empty */
> +			if (!ch_bd->flag) {
> +				if (ch_bd->offset == 0)
> +					ch_bd->length = port->rx_max_frames;
> +
> +				pcm_buffer = ch_bd->p_data + ch_bd->offset;
> +				/* De-interleaving the data */
> +				for (i = 0; i < ch_data_len; i++) {
> +					pcm_buffer[i]
> +						= input_tdm_buffer[i*
> +						bytes_slot_offset +
> +						channel->ch_id];
> +				}
> +				ch_bd->offset += ch_data_len * slot_width;
> +
> +				if (ch_bd->offset >=
> +						(ch_bd->length -
> +						frame_ch_data_size)*
> +						(adap->adapt_cfg.slot_width)) {
> +					ch_bd->flag = 1;
> +					ch_bd->offset = 0;
> +					channel->p_ch_data->rx_in_data =
> +						ch_bd->wrap ?
> +						channel->p_ch_data->rx_data_fifo
> +						: ch_bd+1;
> +					ch_data = 1;
> +					wake_up_interruptible
> +						(&channel->ch_wait_queue);
> +				}
> +			} else {
> +				port->port_stat.rx_pkt_drop_count++;
> +			}
> +			spin_unlock(&channel->p_ch_data->rx_channel_lock);
> +		}
> +
> +		if (ch_data) {
> +			/* Wake up the Port Data Poll event */
> +#ifdef	DEBUG
> +			pr_info("Port RX-%d-%d\n", channel->ch_id, ch_data_len);
> +			for (i = 0; i < ch_data_len; i++)
> +				pr_info("%x", pcm_buffer[i]);
> +			pr_info("\n");
> +#endif
> +			port->port_stat.rx_pkt_count++;
> +			ch_data = 0;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int tdm_data_tx_interleave(struct tdm_adapter *adap)
> +{
> +	struct tdm_port *port, *next;
> +	struct tdm_channel *channel, *temp;
> +	struct tdm_bd	*ch_bd;
> +	int i, buf_size, ch_data_len = NUM_SAMPLES_PER_FRAME;
> +	bool last_data = 0;
> +	u16 *output_tdm_buffer;
> +	u16 *pcm_buffer;
> +	int frame_ch_data_size = NUM_SAMPLES_PER_FRAME;
> +	int bytes_in_fifo_per_frame;
> +	int bytes_slot_offset;
> +
> +#ifdef DEBUG
> +	u8	data_flag = 0;
> +#endif
> +
> +	buf_size = tdm_adap_get_write_buf(adap, (void **)&output_tdm_buffer);
> +	if (buf_size <= 0 || !output_tdm_buffer)
> +		return -EINVAL;
> +
> +	bytes_in_fifo_per_frame = buf_size/frame_ch_data_size;
> +	bytes_slot_offset = bytes_in_fifo_per_frame/adap->adapt_cfg.slot_width;
> +
> +
> +	memset(output_tdm_buffer, 0, sizeof(buf_size));
> +
> +	list_for_each_entry_safe(port, next, &adap->myports, list) {
> +
> +		list_for_each_entry_safe(channel, temp, &port->mychannels,
> +				list) {

Why do you need the _safe variants here?  I can't see anything in this
code which manipulates the lists.

> +			pr_debug("TX-Tdm %d (slots-)", channel->ch_id);
> +
> +
> +			/* if the channel is open */
> +			if (!channel->in_use || !channel->p_ch_data)
> +				continue;
> +
> +			spin_lock(&channel->p_ch_data->tx_channel_lock);
> +			if (!channel->in_use || !channel->p_ch_data)
> +				continue;
> +			ch_bd = channel->p_ch_data->tx_out_data;
> +			if (ch_bd->flag) {
> +				pcm_buffer = (u16 *)((uint8_t *)ch_bd->p_data +
> +						ch_bd->offset);
> +				/* if the buffer has less frames than required*/

Space before */

> +				if (frame_ch_data_size >=
> +						(ch_bd->length - ch_bd->offset/

Space before /

> +						 adap->adapt_cfg.slot_width)) {
> +					ch_data_len =
> +						ch_bd->length - ch_bd->offset/

Space before /

> +						adap->adapt_cfg.slot_width;

Wrong indentation.  If you're finding the 80 column limit is causing
problems, you have too much code in this function (read
Documentation/CodingStyle again).

> +					last_data = 1;
> +				} else {
> +					ch_data_len = frame_ch_data_size;
> +				}
> +				/* Interleaving the data */
> +				for (i = 0; i < ch_data_len; i++) {
> +					/*
> +					 * TODO- need to be genric for any size
> +					 *  assignment
> +					 */
> +					output_tdm_buffer[channel->ch_id +
> +						bytes_slot_offset * i] =
> +						pcm_buffer[i];
> +				}
> +				/*
> +				 * If all the data of this buffer is
> +				 * transmitted
> +				 */
> +				if (last_data) {
> +					ch_bd->flag = 0;
> +					ch_bd->offset = 0;
> +					channel->p_ch_data->tx_out_data =
> +						ch_bd->wrap ?
> +						channel->p_ch_data->tx_data_fifo
> +						: ch_bd+1;

Spaces around +

> +					port->port_stat.tx_pkt_conf_count++;
> +				} else {
> +					ch_bd->offset += ch_data_len *
> +						(adap->adapt_cfg.slot_width);
> +				}
> +#ifdef	DEBUG
> +				data_flag = 1;
> +#endif
> +			}
> +			spin_unlock(&channel->p_ch_data->tx_channel_lock);
> +		}
> +	}
> +
> +#ifdef	DEBUG
> +	if (data_flag) {
> +		pr_info("TX-TDM Interleaved Data-\n");
> +		for (i = 0; i < 64; i++)
> +			pr_info("%x", output_tdm_buffer[i]);
> +		pr_info("\n");
> +	}
> +#endif
> +	return 0;
> +}
> +
> +/* Channel Level APIs of TDM Framework */
> +int tdm_channel_open(u16 chanid, u16 ch_width, struct tdm_port *port,
> +		struct tdm_channel **h_channel)
> +{
> +	struct tdm_channel *channel, *temp;
> +	unsigned long		flags;
> +	struct tdm_ch_data	*p_ch_data;
> +	int res = 0;
> +
> +	if (ch_width != 1) {
> +		pr_err("tdm: Mode not supported\n");
> +		return -EINVAL;
> +	}
> +
> +	list_for_each_entry_safe(channel, temp, &port->mychannels, list) {
> +		if (channel->ch_id == chanid) {
> +			pr_err("tdm: Channel %d already open\n", chanid);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
> +	if (!channel) {
> +		res = -ENOMEM;
> +		goto out;
> +	}
> +
> +	init_waitqueue_head(&channel->ch_wait_queue);
> +	p_ch_data = kzalloc(sizeof(struct tdm_ch_data), GFP_KERNEL);
> +	if (!p_ch_data) {
> +		res = -ENOMEM;
> +		goto outdata;
> +	}
> +
> +	p_ch_data->rx_data_fifo[TDM_CH_RX_BD_RING_SIZE-1].wrap = 1;
> +	p_ch_data->tx_data_fifo[TDM_CH_TX_BD_RING_SIZE-1].wrap = 1;
> +
> +	p_ch_data->rx_in_data = p_ch_data->rx_data_fifo;
> +	p_ch_data->rx_out_data = p_ch_data->rx_data_fifo;
> +	p_ch_data->tx_in_data = p_ch_data->tx_data_fifo;
> +	p_ch_data->tx_out_data = p_ch_data->tx_data_fifo;
> +	spin_lock_init(&p_ch_data->rx_channel_lock);
> +	spin_lock_init(&p_ch_data->tx_channel_lock);
> +
> +	channel->p_ch_data = p_ch_data;
> +
> +	channel->ch_id = chanid;
> +	channel->ch_cfg.first_slot = chanid;
> +	channel->ch_cfg.num_slots = 1;	/*
> +					 * This is 1 for channelized mode and
> +					 * configurable for other modes
> +					 */
> +	channel->port = port;
> +	channel->in_use = 1;
> +
> +	spin_lock_irqsave(&port->ch_list_lock, flags);
> +	list_add_tail(&channel->list, &port->mychannels);
> +	spin_unlock_irqrestore(&port->ch_list_lock, flags);
> +
> +	*h_channel = channel;
> +
> +	return res;
> +
> +outdata:
> +	kfree(channel);
> +out:
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_channel_open);
> +
> +int tdm_sysfs_init(void)

static?

> +{
> +	struct kobject *tdm_kobj;
> +	int err = 1;
> +	tdm_kobj = kzalloc(sizeof(*tdm_kobj), GFP_KERNEL);
> +	if (tdm_kobj) {
> +		kobject_init(tdm_kobj, &tdm_type);
> +		if (kobject_add(tdm_kobj, NULL, "%s", "tdm")) {
> +			pr_err("tdm: Sysfs creation failed\n");
> +			kobject_put(tdm_kobj);
> +			err = -EINVAL;
> +			goto out;
> +		}
> +	} else {
> +		pr_err("tdm: Unable to allocate tdm_kobj\n");
> +		err = -ENOMEM;
> +		goto out;
> +	}

What if this function gets called multiple times?  It looks like
kobject_add() will fail due to name conflicts.

Also, what's the point of the above?  I looks like the kobject is
never used.

> +
> +out:
> +	return err;
> +}
> +
> +int tdm_channel_close(u16 chanid, u16 ch_width, struct tdm_port *port,
> +		struct tdm_channel *h_channel)
> +{
> +	struct tdm_channel *channel;
> +	unsigned long		flags;
> +	int res = 0;
> +	channel = h_channel;
> +
> +	spin_lock_irqsave(&port->ch_list_lock, flags);
> +	list_del(&channel->list);
> +	spin_unlock_irqrestore(&port->ch_list_lock, flags);
> +
> +	if (channel)
> +		kfree(channel->p_ch_data);
> +	kfree(channel);
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_channel_close);
> +
> +ssize_t tdm_show_sysfs(struct kobject *kobj,
> +		struct attribute *attr, char *buf)
> +{
> +	int retval = 0;
> +	struct tdm_sysfs *a = container_of(attr,
> +			struct tdm_sysfs, attr);
> +	switch (a->cmd_type) {
> +	case TDM_LATEST_DATA:
> +		pr_info("use_latest_tdm_data: %d\n", use_latest_tdm_data);
> +		break;
> +	default:
> +		pr_info("Invalid cmd_type value\n");
> +		return -EINVAL;
> +	}
> +	return retval;
> +}
> +
> +ssize_t tdm_store_sysfs(struct kobject *kobj,
> +		struct attribute *attr, const char *buf, size_t len)
> +{
> +	struct tdm_sysfs *a = container_of(attr,
> +			struct tdm_sysfs, attr);
> +
> +	sscanf(buf, "%d", &a->data);
> +	use_latest_tdm_data = a->data;
> +	return strlen(buf);
> +}
> +
> +void init_config_adapter(struct tdm_adapter *adap)
> +{
> +	struct fsl_tdm_adapt_cfg default_adapt_cfg = {
> +		.loopback = TDM_PROCESS_NORMAL,
> +		.num_ch = NUM_CHANNELS,
> +		.ch_size_type = CHANNEL_16BIT_LIN,
> +		.frame_len = NUM_SAMPLES_PER_FRAME,
> +		.num_frames = NUM_SAMPLES_PER_FRAME,
> +		.adap_mode = TDM_ADAPTER_MODE_NONE
> +	};
> +
> +	default_adapt_cfg.slot_width = default_adapt_cfg.ch_size_type/3 + 1;

Spaces around /

> +
> +	memcpy(&adap->adapt_cfg, &default_adapt_cfg,
> +			sizeof(struct fsl_tdm_adapt_cfg));

If this is supposed to be a generic layer, why the fsl_ data in core code?

> +
> +	return;
> +}
> +EXPORT_SYMBOL(init_config_adapter);
> +
> +void tdm_data_tasklet_fn(unsigned long data)

static?

> +{
> +	struct tdm_adapter *adapter;
> +	adapter = (struct tdm_adapter *)data;
> +	if (adapter != NULL) {
> +		tdm_data_tx_interleave(adapter);
> +		tdm_data_rx_deinterleave(adapter);
> +	}
> +}

A general comment: it is better to place functions before their first use
where possible.

> +
> +
> +MODULE_AUTHOR("Hemant Agrawal <hemant@freescale.com> and "
> +	"Rajesh Gumasta <rajesh.gumasta@freescale.com>");
> +MODULE_DESCRIPTION("TDM Driver Framework Core");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 83ac071..573ac40 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -425,6 +425,17 @@ struct i2c_device_id {
>  			__attribute__((aligned(sizeof(kernel_ulong_t))));
>  };
>  
> +/* tdm */
> +
> +#define TDM_NAME_SIZE   20
> +#define TDM_MODULE_PREFIX "tdm:"
> +
> +struct tdm_device_id {
> +	char name[TDM_NAME_SIZE];
> +	kernel_ulong_t driver_data      /* Data private to the driver */
> +			__attribute__((aligned(sizeof(kernel_ulong_t))));
> +};
> +
>  /* spi */
>  
>  #define SPI_NAME_SIZE	32
> diff --git a/include/linux/tdm.h b/include/linux/tdm.h
> new file mode 100644
> index 0000000..44cd8cf
> --- /dev/null
> +++ b/include/linux/tdm.h
> @@ -0,0 +1,389 @@
> +/* include/linux/tdm.h
> + *
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + *
> + * tdm.h - definitions for the tdm-device framework interface
> + *
> + * Author:Hemant Agrawal <hemant@freescale.com>
> + *	Rajesh Gumasta <rajesh.gumasta@freescale.com>
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + *
> + * 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.
> + *
> + * You should have received a copy of the  GNU General Public License along
> + * with this program; if not, write  to the Free Software Foundation, Inc.,
> + * 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +
> +#ifndef _LINUX_TDM_H
> +#define _LINUX_TDM_H
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/device.h>	/* for struct device */
> +#include <linux/sched.h>	/* for completion */
> +#include <linux/mutex.h>
> +#include <linux/interrupt.h>
> +#include <linux/sysfs.h>
> +
> +#define TDM_LATEST_DATA		1
> +#define CHANNEL_8BIT_LIN	0	/* 8 bit linear */
> +#define CHANNEL_8BIT_ULAW	1	/* 8 bit Mu-law */
> +#define CHANNEL_8BIT_ALAW	2	/* 8 bit A-law */
> +#define CHANNEL_16BIT_LIN	3	/* 16 bit Linear */
> +
> +/*
> + * Default adapter configuration. All the TDM adapters registered with
> + * framework will be configured with following default configuration.
> + */
> +#define NUM_CHANNELS	16
> +
> +/*
> + * Default configuration for typical voice data sample. These parameters
> + * will generally not be required to be changed for voice type applications.
> + */
> +
> +/* 8 samples per milli sec per channel. Req for voice data */
> +#define NUM_SAMPLES_PER_MS	8
> +#define NUM_MS			10
> +
> +/* Number of samples for 1 client buffer */
> +#define NUM_SAMPLES_PER_FRAME	(NUM_MS * NUM_SAMPLES_PER_MS)
> +#define NUM_OF_TDM_BUF		3
> +
> +/* General options */
> +
> +struct tdm_adapt_algorithm;
> +struct tdm_adapter;
> +struct tdm_port;
> +
> +
> +/*
> + * struct tdm_driver - represent an TDM device driver
> + * @class: What kind of tdm device we instantiate (for detect)
> + * @id:Driver id
> + * @name: Name of the driver
> + * @attach_adapter: Callback for device addition (for legacy drivers)
> + * @detach_adapter: Callback for device removal (for legacy drivers)
> + * @probe: Callback for device binding
> + * @remove: Callback for device unbinding
> + * @shutdown: Callback for device shutdown
> + * @suspend: Callback for device suspend
> + * @resume: Callback for device resume
> + * @command: Callback for sending commands to device
> + * @id_table: List of TDM devices supported by this driver
> + * @list: List of drivers created (for tdm-core use only)
> + */
> +struct tdm_driver {
> +	unsigned int class;
> +	unsigned int id;
> +	char name[TDM_NAME_SIZE];
> +
> +	int (*attach_adapter)(struct tdm_adapter *);
> +	int (*detach_adapter)(struct tdm_adapter *);
> +
> +	/* Standard driver model interfaces */
> +	int (*probe)(const struct tdm_device_id *);
> +	int (*remove)(void);
> +
> +	/* driver model interfaces that don't relate to enumeration */
> +	void (*shutdown)(void);
> +	int (*suspend)(pm_message_t mesg);
> +	int (*resume)(void);

Far better to use the dev_pm_ops stuff now, don't propagate the old
PM interfaces.

> +
> +	const struct tdm_device_id *id_table;
> +
> +	/* The associated adapter for this driver */
> +	struct tdm_adapter *adapter;
> +	struct list_head list;
> +};
> +
> +/*
> + * tdm per port statistics structure, used for providing and storing tdm port
> + * statistics.
> + */
> +struct tdm_port_stats {
> +	unsigned int rx_pkt_count;	/* Rx frame count per channel */
> +	unsigned int rx_pkt_drop_count;	/*
> +					 * Rx drop count per channel to
> +					 * clean space for new buffer
> +					 */
> +	unsigned int tx_pkt_count;	/* Tx frame count per channel */
> +	unsigned int tx_pkt_conf_count;	/*
> +					 * Tx frame confirmation count per
> +					 * channel
> +					 */
> +	unsigned int tx_pkt_drop_count;	/*
> +					 * Tx drop count per channel due to
> +					 * queue full
> +					 */
> +};
> +
> +
> +/*
> + * tdm Buffer Descriptor, used for Creating Interleaved and De-interleaved
> + * FIFOs
> + */
> +struct tdm_bd {
> +	unsigned char flag;		/* BD is full or empty */
> +	unsigned char wrap;		/* BD is last in the queue */
> +	unsigned short length;		/* Length of data in BD */
> +	/*TODO: use dyanmic memory */
> +	unsigned short p_data[NUM_SAMPLES_PER_FRAME];	/* Data Pointer */
> +	unsigned long offset;	/* Offset of the Data Pointer to be used */
> +};
> +
> +#define TDM_CH_RX_BD_RING_SIZE	3
> +#define TDM_CH_TX_BD_RING_SIZE	3
> +
> +/* tdm RX-TX Channelised Data */
> +struct tdm_port_data {
> +	struct tdm_bd rx_data_fifo[TDM_CH_RX_BD_RING_SIZE]; /*
> +							     * Rx Channel Data
> +							     * BD Ring
> +							     */
> +	struct tdm_bd *rx_in_data;	/*
> +					 * Current Channel Rx BD to be filled by
> +					 * de-interleave function
> +					 */
> +	struct tdm_bd *rx_out_data;	/*
> +					 * Current Channel Rx BD to be
> +					 * read by App
> +					 */
> +	struct tdm_bd tx_data_fifo[TDM_CH_TX_BD_RING_SIZE]; /*
> +							     * Tx Channel Data
> +							     *	BD Ring
> +							     */
> +	struct tdm_bd *tx_in_data;	/*
> +					 * Current Channel Tx BD to be
> +					 * filled by App
> +					 */
> +	struct tdm_bd *tx_out_data;	/*
> +					 * Current Channel Tx BD to be read by
> +					 * interleave function
> +					 */
> +	spinlock_t rx_channel_lock;	/* Spin Lock for Rx Channel */
> +	spinlock_t tx_channel_lock;	/* Spin Lock for Tx Channel */
> +};
> +
> +/* structure tdm_port_cfg - contains configuration params for a port */
> +struct tdm_port_cfg {
> +	unsigned short port_mode;
> +};
> +
> +/* struct tdm_port - represent an TDM ports for a device */
> +struct tdm_port {
> +	unsigned short port_id;
> +	unsigned short in_use;	/* Port is enabled? */
> +	uint16_t rx_max_frames;	/*
> +				 * Received port frames before allowing
> +				 * read operation in Port Mode
> +				 */
> +
> +	struct tdm_port_stats port_stat; /*
> +					  * A structure parameter defining
> +					  * TDM port statistics.
> +					  */
> +	struct tdm_port_data *p_port_data;	/*
> +						 * a structure parameter
> +						 * defining tdm channelised data
> +						 */
> +
> +	struct tdm_driver *driver;	/* driver for this port */
> +	struct tdm_adapter *adapter;	/* adapter for this port */
> +	struct list_head list;		/* list of ports */
> +	struct list_head mychannels;	/* list of channels, on this port*/
> +	spinlock_t ch_list_lock;	/* Spin Lock for channel_list */
> +	struct tdm_port_cfg port_cfg;	/*
> +					 * A structure parameter defining
> +					 * TDM port configuration.
> +					 */
> +};
> +
> +/* tdm RX-TX Channelised Data */
> +struct tdm_ch_data {
> +	struct tdm_bd rx_data_fifo[TDM_CH_RX_BD_RING_SIZE]; /*
> +							     * Rx Port Data BD
> +							     * Ring
> +							     */
> +	struct tdm_bd *rx_in_data;	/*
> +					 * Current Port Rx BD to be filled by
> +					 * de-interleave function
> +					 */
> +	struct tdm_bd *rx_out_data; /* Current Port Rx BD to be read by App */
> +	struct tdm_bd tx_data_fifo[TDM_CH_TX_BD_RING_SIZE]; /*
> +							     * Tx Port Data BD
> +							     * Ring
> +							     */
> +	struct tdm_bd *tx_in_data;	/*
> +					 * Current Port Tx BD to be filled by
> +					 * App
> +					 */
> +	struct tdm_bd *tx_out_data;	/*
> +					 * Current Port Tx BD to be read by
> +					 * interleave function
> +					 */
> +	spinlock_t rx_channel_lock;	/* Spin Lock for Rx Port */
> +	spinlock_t tx_channel_lock;	/* Spin Lock for Tx Port */
> +};
> +
> +/* Channel config params */
> +struct tdm_ch_cfg {
> +	unsigned short num_slots;
> +	unsigned short first_slot;
> +};
> +
> +/* struct tdm_channel- represent a TDM channel for a port */
> +struct tdm_channel {
> +	u16 ch_id;			/* logical channel number */
> +	struct list_head list;		/* list of channels in a port*/
> +	struct tdm_port *port;		/* port for this channel */
> +	u8 in_use;			/* channel is enabled? */
> +	struct tdm_ch_cfg ch_cfg;	/* channel configuration */
> +	struct tdm_ch_data *p_ch_data;	/* data storage space for channel */
> +	wait_queue_head_t ch_wait_queue;/* waitQueue for RX Channel Data */
> +};
> +
> +/* tdm_adapt_algorithm is for accessing the routines of device */
> +struct tdm_adapt_algorithm {
> +	int (*tdm_read)(struct tdm_adapter *, u16 **);
> +	int (*tdm_get_write_buf)(struct tdm_adapter *, u16 **);
> +	int (*tdm_write)(struct tdm_adapter *, void *, unsigned int len);
> +	int (*tdm_enable)(struct tdm_adapter *);
> +	int (*tdm_disable)(struct tdm_adapter *);
> +};
> +
> +/* tdm_adapter_mode is to define in mode of the device */
> +enum tdm_adapter_mode {
> +	TDM_ADAPTER_MODE_NONE = 0x00,
> +	TDM_ADAPTER_MODE_T1 = 0x01,
> +	TDM_ADAPTER_MODE_E1 = 0x02,
> +	TDM_ADAPTER_MODE_T1_RAW = 0x10,
> +	TDM_ADAPTER_MODE_E1_RAW = 0x20,
> +};
> +
> +/* tdm_port_mode defines the mode in which the port is configured to operate
> + * It can be channelized/full/fractional.
> + */
> +enum tdm_port_mode {
> +	TDM_PORT_CHANNELIZED = 0,	/* Channelized mode */
> +	TDM_PORT_FULL = 1,		/* Full mode */
> +	TDM_PORT_FRACTIONAL = 2		/* Fractional mode */
> +};
> +
> +/* tdm_process_mode used for testing the tdm device in normal mode or internal
> + * loopback or external loopback
> + */
> +enum tdm_process_mode {
> +	TDM_PROCESS_NORMAL = 0,		/* Normal mode */
> +	TDM_PROCESS_INT_LPB = 1,	/* Internal loop mode */
> +	TDM_PROCESS_EXT_LPB = 2		/* External Loopback mode */
> +};
> +
> +/* TDM configuration parameters */
> +struct fsl_tdm_adapt_cfg {
> +	u8 num_ch;		/* Number of channels in this adpater */
> +	u8 ch_size_type;	/*
> +				 * reciever/transmit channel
> +				 * size for all channels
> +				 */
> +	u8 slot_width;		/* 1 or 2 Is defined by channel type */
> +	u8 frame_len;		/* Length of frame in samples */
> +	u32 num_frames;
> +	u8 loopback;		/* loopback or normal */
> +	u8 adap_mode;		/*
> +				 * 0=None, 1= T1, 2= T1-FULL, 3=E1,
> +				 * 4 = E1-FULL
> +				 */
> +	int max_timeslots;	/*
> +				 * Max Number of timeslots that are
> +				 * supported on this adapter
> +				 */
> +};
> +
> +/*
> + * tdm_adapter is the structure used to identify a physical tdm device along
> + * with the access algorithms necessary to access it.
> + */
> +struct tdm_adapter {
> +	struct module *owner;	/* owner of the adapter module */
> +	unsigned int id;	/* Adapter Id */
> +	unsigned int drv_count;	/*
> +				 * Number of drivers associated with the
> +				 * adapter
> +				 */
> +	const struct tdm_adapt_algorithm *algo;	/*
> +						 * algorithm to access the
> +						 * adapter
> +						 */
> +
> +	char name[TDM_NAME_SIZE];	/* Name of Adapter */
> +	struct mutex adap_lock;
> +	struct device *parent;
> +
> +	struct tasklet_struct tdm_data_tasklet;	/*
> +						 * tasklet handle to perform
> +						 * data processing
> +						 */
> +	int tasklet_conf;	/* flag for tasklet configuration */
> +	int tdm_rx_flag;
> +
> +	struct list_head myports;	/*
> +					 * list of ports, created on this
> +					 * adapter
> +					 */
> +	struct list_head list;
> +	spinlock_t portlist_lock;
> +	void *data;
> +	struct fsl_tdm_adapt_cfg adapt_cfg;
> +};
> +
> +struct tdm_sysfs {
> +	struct attribute attr;
> +	int data;
> +	u32 cmd_type;
> +};
> +
> +/* functions exported by tdm.o */
> +
> +int tdm_add_adapter(struct tdm_adapter *adpater);
> +int tdm_del_adapter(struct tdm_adapter *adapter);
> +int tdm_register_driver(struct tdm_driver *driver);
> +void tdm_unregister_driver(struct tdm_driver *driver);
> +void init_config_adapter(struct tdm_adapter *adapter);
> +
> +int tdm_port_open(struct tdm_driver *driver, struct tdm_port **h_port);
> +int tdm_port_close(struct tdm_port *h_port);
> +int tdm_channel_read(struct tdm_port *h_port, struct tdm_channel *h_channel,
> +		void *p_data, u16 *size);
> +int tdm_channel_write(struct tdm_port *h_port, struct tdm_channel *h_channel,
> +		void *p_data, u16 size);
> +int tdm_ch_poll(struct tdm_channel *h_channel, unsigned int wait_time);
> +
> +int tdm_channel_open(u16 chanid, u16 ch_width, struct tdm_port *port,
> +		struct tdm_channel **h_channel);
> +int tdm_channel_close(u16 chanid, u16 ch_width, struct tdm_port *port,
> +		struct tdm_channel *h_channel);
> +/* this tasklet is created for each adapter instance */
> +void tdm_data_tasklet_fn(unsigned long);
> +int tdm_sysfs_init(void);
> +ssize_t tdm_show_sysfs(struct kobject *kobj,
> +		struct attribute *attr, char *buf);
> +ssize_t tdm_store_sysfs(struct kobject *kobj,
> +		struct attribute *attr, const char *buf, size_t len);
> +
> +struct tdm_adapter *tdm_get_adapter(int id);
> +void tdm_put_adapter(struct tdm_adapter *adap);
> +
> +#endif /* __KERNEL__ */
> +
> +#define TDM_E_OK 0
> -- 
> 1.5.6.5
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-07-27 14:05 [2/3][PATCH][v2] TDM Framework sandeep
                   ` (2 preceding siblings ...)
  2012-07-27 14:51 ` Russell King - ARM Linux
@ 2012-07-27 15:25 ` Francois Romieu
  2012-07-30  9:50   ` Singh Sandeep-B37400
  2012-07-30 15:45   ` Mark Brown
  2012-07-27 17:59 ` Greg KH
  2012-07-27 18:12 ` Greg KH
  5 siblings, 2 replies; 22+ messages in thread
From: Francois Romieu @ 2012-07-27 15:25 UTC (permalink / raw)
  To: sandeep; +Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel

sandeep@freescale.com <sandeep@freescale.com> :
[...]
> The main functions of this Framework are:
>  - provides interface to TDM clients to access TDM functionalities.
>  - provides standard interface for TDM drivers to hook with the framework.
>  - handles various data handling stuff and buffer management.
> 
> In future this Framework will be extended to provide Interface for Line control devices also. For example SLIC, E1/T1 Framers etc.
> 
> Presently the framework supports only Single Port channelised mode.
> Also the configurability options are limited which will be extended later on.
> Only kernel mode TDM clients are supported currently. Support for User mode clients will be added later.

1. You should send some kernel mode TDM clients. Without those the framework
   is pretty useless.

2. It would probably make sense to Cc: netdev and serial. There may be
   some kernel client network integration from the start.

3. Where is the userspace configuration interface ?

[...]
> Based on: git://git.am.freescale.net/gitolite/mirrors/galak-powerpc.git

$ git clone git://git.am.freescale.net/gitolite/mirrors/galak-powerpc.git
Cloning into 'galak-powerpc'...
fatal: Unable to look up git.am.freescale.net (port 9418) (No address associated with hostname)

-- 
Ueimor

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-07-27 14:05 [2/3][PATCH][v2] TDM Framework sandeep
                   ` (3 preceding siblings ...)
  2012-07-27 15:25 ` Francois Romieu
@ 2012-07-27 17:59 ` Greg KH
  2012-07-30  9:10   ` Aggrwal Poonam-B10812
  2012-07-27 18:12 ` Greg KH
  5 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2012-07-27 17:59 UTC (permalink / raw)
  To: sandeep; +Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel

On Fri, Jul 27, 2012 at 07:35:38PM +0530, sandeep@freescale.com wrote:
> +/* Data structures required for sysfs */
> +static struct tdm_sysfs attr = {
> +	.attr.name = "use_latest_data",
> +	.attr.mode = 0664,
> +	.cmd_type = TDM_LATEST_DATA,
> +};

What is this for?

> +int tdm_sysfs_init(void)
> +{
> +	struct kobject *tdm_kobj;
> +	int err = 1;
> +	tdm_kobj = kzalloc(sizeof(*tdm_kobj), GFP_KERNEL);
> +	if (tdm_kobj) {
> +		kobject_init(tdm_kobj, &tdm_type);
> +		if (kobject_add(tdm_kobj, NULL, "%s", "tdm")) {
> +			pr_err("tdm: Sysfs creation failed\n");
> +			kobject_put(tdm_kobj);
> +			err = -EINVAL;
> +			goto out;
> +		}
> +	} else {
> +		pr_err("tdm: Unable to allocate tdm_kobj\n");
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
> +out:
> +	return err;
> +}

You just leaked memory, what are you trying to do here?

And why are you using "raw" kobjects?  That's a sure sign that something
is really wrong.

Your code doesn't look like it is tied into the driver model at all, why
not?  What are you trying to do here?

Also, when creating new sysfs entries, like you are attempting to do
here (unsuccessfully I might add), you must create Documentation/ABI/
files as well.

And, to top it all off, you do realize you are asking us to do code
review in the middle of the merge window, when we are all busy doing
other things?

greg k-h

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-07-27 14:05 [2/3][PATCH][v2] TDM Framework sandeep
                   ` (4 preceding siblings ...)
  2012-07-27 17:59 ` Greg KH
@ 2012-07-27 18:12 ` Greg KH
  2012-07-30  9:13   ` Aggrwal Poonam-B10812
  5 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2012-07-27 18:12 UTC (permalink / raw)
  To: sandeep; +Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel

On Fri, Jul 27, 2012 at 07:35:38PM +0530, sandeep@freescale.com wrote:
> +static struct kobj_type tdm_type = {
> +	.sysfs_ops = &tdm_ops,
> +	.default_attrs = tdm_attr,
> +};

Ah, also, as per the documentation in the kernel (go look, it's there),
I now get to publicly mock you for ignoring the error messages that
the kernel is giving you when you try to shut down your code path.

Well, to be fair, you are leaking memory like a sieve, so I doubt you
ever saw those error messages because you never cleaned up after
yourself, so perhaps I can forgive you, but your users can't, sorry.
They like to rely on the fact that Linux is a reliable operating system,
don't cause them to doubt that.

Please fix this code, it's horribly broken.  Read
Documentation/kobject.txt for why.  That file was written for a reason,
and not just because we like writing documentation (hint, we hate to...)

Ugh,

greg k-h

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

* RE: [2/3][PATCH][v2] TDM Framework
  2012-07-27 17:59 ` Greg KH
@ 2012-07-30  9:10   ` Aggrwal Poonam-B10812
  2012-07-30 16:04     ` Greg KH
  0 siblings, 1 reply; 22+ messages in thread
From: Aggrwal Poonam-B10812 @ 2012-07-30  9:10 UTC (permalink / raw)
  To: Greg KH, Singh Sandeep-B37400
  Cc: devel, linuxppc-dev, linux-kernel, linux-arm-kernel



> -----Original Message-----
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+poonam.aggrwal=3Dfreescale.com@lists.ozlabs.org] On Behalf Of Gre=
g
> KH
> Sent: Friday, July 27, 2012 11:30 PM
> To: Singh Sandeep-B37400
> Cc: devel@driverdev.osuosl.org; linuxppc-dev@lists.ozlabs.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [2/3][PATCH][v2] TDM Framework
>=20
> On Fri, Jul 27, 2012 at 07:35:38PM +0530, sandeep@freescale.com wrote:
> > +/* Data structures required for sysfs */ static struct tdm_sysfs attr
> > +=3D {
> > +	.attr.name =3D "use_latest_data",
> > +	.attr.mode =3D 0664,
> > +	.cmd_type =3D TDM_LATEST_DATA,
> > +};
>=20
> What is this for?
This knob is to control the behavior of the TDM framework with respect to h=
andling the received TDM frames.
The framework layer receives the TDM frames from the TDM adapter driver, de=
-interleaves the data and sends to respective client modules.
In the case when the TDM client module has not consumed the data and emptie=
d the Buffer, this flag decides whether to discard the un-fetched data, or =
discard the latest received data.

>=20
> > +int tdm_sysfs_init(void)
> > +{
> > +	struct kobject *tdm_kobj;
> > +	int err =3D 1;
> > +	tdm_kobj =3D kzalloc(sizeof(*tdm_kobj), GFP_KERNEL);
> > +	if (tdm_kobj) {
> > +		kobject_init(tdm_kobj, &tdm_type);
> > +		if (kobject_add(tdm_kobj, NULL, "%s", "tdm")) {
> > +			pr_err("tdm: Sysfs creation failed\n");
> > +			kobject_put(tdm_kobj);
> > +			err =3D -EINVAL;
> > +			goto out;
> > +		}
> > +	} else {
> > +		pr_err("tdm: Unable to allocate tdm_kobj\n");
> > +		err =3D -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +out:
> > +	return err;
> > +}
>=20
> You just leaked memory, what are you trying to do here?
>=20
> And why are you using "raw" kobjects?  That's a sure sign that something
> is really wrong.
Will refer the documentation. Not very experienced on this stuff. Thanks fo=
r the comment.
>=20
> Your code doesn't look like it is tied into the driver model at all, why
> not?  What are you trying to do here?
This is a framework layer, not associated to a particular device. TDM adapt=
er drivers will register to this framework.
We got this comment in internal freescale review list also.
>=20
> Also, when creating new sysfs entries, like you are attempting to do here
> (unsuccessfully I might add), you must create Documentation/ABI/ files as
> well.
Ok.
>=20
> And, to top it all off, you do realize you are asking us to do code
> review in the middle of the merge window, when we are all busy doing
> other things?
Apologize for asking a review in the merge window time frame.
Are there any guidelines when to send something for a review? We will be ca=
reful next time.

Regards
Poonam
>=20
> greg k-h
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

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

* RE: [2/3][PATCH][v2] TDM Framework
  2012-07-27 18:12 ` Greg KH
@ 2012-07-30  9:13   ` Aggrwal Poonam-B10812
  0 siblings, 0 replies; 22+ messages in thread
From: Aggrwal Poonam-B10812 @ 2012-07-30  9:13 UTC (permalink / raw)
  To: Greg KH, Singh Sandeep-B37400
  Cc: devel, linuxppc-dev, linux-kernel, linux-arm-kernel



> -----Original Message-----
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+poonam.aggrwal=3Dfreescale.com@lists.ozlabs.org] On Behalf Of Gre=
g
> KH
> Sent: Friday, July 27, 2012 11:42 PM
> To: Singh Sandeep-B37400
> Cc: devel@driverdev.osuosl.org; linuxppc-dev@lists.ozlabs.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [2/3][PATCH][v2] TDM Framework
>=20
> On Fri, Jul 27, 2012 at 07:35:38PM +0530, sandeep@freescale.com wrote:
> > +static struct kobj_type tdm_type =3D {
> > +	.sysfs_ops =3D &tdm_ops,
> > +	.default_attrs =3D tdm_attr,
> > +};
>=20
> Ah, also, as per the documentation in the kernel (go look, it's there), I
> now get to publicly mock you for ignoring the error messages that the
> kernel is giving you when you try to shut down your code path.
>=20
> Well, to be fair, you are leaking memory like a sieve, so I doubt you
> ever saw those error messages because you never cleaned up after
> yourself, so perhaps I can forgive you, but your users can't, sorry.
> They like to rely on the fact that Linux is a reliable operating system,
> don't cause them to doubt that.
>=20
> Please fix this code, it's horribly broken.  Read
> Documentation/kobject.txt for why.  That file was written for a reason,
> and not just because we like writing documentation (hint, we hate to...)
To be honest we are not sysfs experts. Thanks for pointing to the documenta=
tion.
We will rework the stuff.

Regards
Poonam
>=20
> Ugh,
>=20
> greg k-h
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

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

* RE: [2/3][PATCH][v2] TDM Framework
  2012-07-27 14:11 ` [2/3][PATCH][v2] TDM Framework John Stoffel
@ 2012-07-30  9:29   ` Singh Sandeep-B37400
  2012-07-30 14:10     ` John Stoffel
  0 siblings, 1 reply; 22+ messages in thread
From: Singh Sandeep-B37400 @ 2012-07-30  9:29 UTC (permalink / raw)
  To: John Stoffel; +Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel

-----Original Message-----
From: John Stoffel [mailto:john@stoffel.org]=20
Sent: 27 July 2012 19:42
To: Singh Sandeep-B37400
Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; ga=
lak@kernel.crashing.org; linux-kernel@vger.kernel.org; devel@driverdev.osuo=
sl.org
Subject: Re: [2/3][PATCH][v2] TDM Framework


> From: Sandeep Singh <Sandeep@freescale.com> TDM Framework is an=20
> attempt to provide a platform independent layer which can offer a=20
> standard interface  for TDM access to different client modules.

Please don't use TLAs (Three Letter Acronyms) like TDM without explaining t=
he clearly and up front.  It makes it hard for anyone else who doens't know=
 your code to look it over without having to spend lots of time poking arou=
nd to figure it out from either context or somewhere else.
[Sandeep] Patch for documentation for TDM is present in this patch set, whi=
ch explains TDM in detail. Should we do this in commit message too??
Link too documentation patch: http://patchwork.ozlabs.org/patch/173680/

John

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

* RE: [2/3][PATCH][v2] TDM Framework
  2012-07-27 15:25 ` Francois Romieu
@ 2012-07-30  9:50   ` Singh Sandeep-B37400
  2012-07-30 16:01     ` Greg KH
  2012-07-30 15:45   ` Mark Brown
  1 sibling, 1 reply; 22+ messages in thread
From: Singh Sandeep-B37400 @ 2012-07-30  9:50 UTC (permalink / raw)
  To: Francois Romieu; +Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel

Thanks for your comments. Please find replies inline.

Regards,
Sandeep

-----Original Message-----
From: Francois Romieu [mailto:romieu@fr.zoreil.com]=20
Sent: 27 July 2012 20:56
To: Singh Sandeep-B37400
Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; ga=
lak@kernel.crashing.org; linux-kernel@vger.kernel.org; devel@driverdev.osuo=
sl.org
Subject: Re: [2/3][PATCH][v2] TDM Framework

sandeep@freescale.com <sandeep@freescale.com> :
[...]
> The main functions of this Framework are:
>  - provides interface to TDM clients to access TDM functionalities.
>  - provides standard interface for TDM drivers to hook with the framework=
.
>  - handles various data handling stuff and buffer management.
>=20
> In future this Framework will be extended to provide Interface for Line c=
ontrol devices also. For example SLIC, E1/T1 Framers etc.
>=20
> Presently the framework supports only Single Port channelised mode.
> Also the configurability options are limited which will be extended later=
 on.
> Only kernel mode TDM clients are supported currently. Support for User mo=
de clients will be added later.

1. You should send some kernel mode TDM clients. Without those the framewor=
k
   is pretty useless.
[Sandeep] We do have a test client but not good enough to be pushed in open=
 source, should we add it to documentation??=20

2. It would probably make sense to Cc: netdev and serial. There may be
   some kernel client network integration from the start.
[Sandeep] Ok.=20

3. Where is the userspace configuration interface ?
[Sandeep] TDM framework right now supports only kernel mode clients. It has=
 been tested with the client module that I mentioned above. Both the framew=
ork and test client are a part of Freescale BSP.

[...]
> Based on: git://git.am.freescale.net/gitolite/mirrors/galak-powerpc.git
[Sandeep] Please try below mentioned link. The above one is Freescale's int=
ernal mirror of:
git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git=20

$ git clone git://git.am.freescale.net/gitolite/mirrors/galak-powerpc.git
Cloning into 'galak-powerpc'...
fatal: Unable to look up git.am.freescale.net (port 9418) (No address assoc=
iated with hostname)

--=20
Ueimor

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

* RE: [2/3][PATCH][v2] TDM Framework
  2012-07-27 14:51 ` Russell King - ARM Linux
@ 2012-07-30 13:00   ` Singh Sandeep-B37400
  0 siblings, 0 replies; 22+ messages in thread
From: Singh Sandeep-B37400 @ 2012-07-30 13:00 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel

Thanks for your comments.
Please find my response inline.

Regards,
Sandeep


-----Original Message-----
From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk]=20
Sent: Friday, July 27, 2012 8:22 PM
To: Singh Sandeep-B37400
Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; de=
vel@driverdev.osuosl.org; galak@kernel.crashing.org; linux-kernel@vger.kern=
el.org
Subject: Re: [2/3][PATCH][v2] TDM Framework

On Fri, Jul 27, 2012 at 07:35:38PM +0530, sandeep@freescale.com wrote:
> +static DEFINE_MUTEX(tdm_core_lock);
> +static DEFINE_IDR(tdm_adapter_idr);
> +/* List of TDM adapters registered with TDM framework */=20
> +LIST_HEAD(adapter_list);
> +
> +/* List of TDM clients registered with TDM framework */=20
> +LIST_HEAD(driver_list);

These two are far too generic to be public.  Have you checked your code wit=
h sparse?  I think not.
[Sandeep] Will changes the name to be more appropriate. Right, I haven't ch=
ecked with sparse.

> +
> +/*
> + * In case the previous data is not fetched by the client driver, the
> + * de-interleaving function will  discard the old data and rewrite=20
> +the
> + * new data
> + */
> +
> +static int use_latest_tdm_data =3D 1;
> +
> +/* Data structures required for sysfs */ static struct tdm_sysfs attr=20
> +=3D {
> +	.attr.name =3D "use_latest_data",
> +	.attr.mode =3D 0664,
> +	.cmd_type =3D TDM_LATEST_DATA,
> +};
> +
> +static struct attribute *tdm_attr[] =3D {
> +	&attr.attr,
> +	NULL
> +};
> +
> +const struct sysfs_ops tdm_ops =3D {
> +	.show =3D tdm_show_sysfs,
> +	.store =3D tdm_store_sysfs,
> +};

Again, lack of static.
[Sandeep] Ok

> +
> +static struct kobj_type tdm_type =3D {
> +	.sysfs_ops =3D &tdm_ops,
> +	.default_attrs =3D tdm_attr,
> +};
> +
> +/* tries to match client driver with the adapter */ static int=20
> +tdm_device_match(struct tdm_driver *driver, struct tdm_adapter *adap)=20
> +{
> +	/* match on an id table if there is one */
> +	if (driver->id_table && driver->id_table->name[0]) {
> +		if (!(strcmp(driver->id_table->name, adap->name)))
> +			return (int)driver->id_table;

Casting a pointer to 'int' is not a good thing to do.  Please fix this.
[Sandeep] Will fix this.

> +	}
> +	return 0;
> +}
> +
> +static int tdm_attach_driver_adap(struct tdm_driver *driver,
> +		struct tdm_adapter *adap)
> +{
> +	int ret =3D 0;
> +	/* if driver is already attached to any other adapter, return*/
> +	if (driver->adapter && (driver->adapter !=3D adap))

Additional parens not required.
[Sandeep] Ok

> +		return 0;
> +
> +	driver->adapter =3D adap;
> +
> +	if (driver->attach_adapter) {
> +		ret =3D driver->attach_adapter(adap);
> +		if (ret < 0) {
> +			pr_err("tdm: attach_adapter failed for driver [%s]"
> +					"err:%d\n", driver->name, ret);
> +			return ret;
> +		}
> +	}
> +	adap->drv_count++;
> +
> +	if (!adap->tasklet_conf) {
> +		tdm_sysfs_init();
> +		tasklet_init(&adap->tdm_data_tasklet, tdm_data_tasklet_fn,
> +				(unsigned long)adap);

Why not init this tasklet when the struct tdm_adapter is first created?
Why do you need to wait, and then have state tracking for this?
[Sandeep] Ok, will take care

> +		adap->tasklet_conf =3D 1;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Detach client driver and adapter */ static int=20
> +tdm_detach_driver_adap(struct tdm_driver *driver,
> +		struct tdm_adapter *adap)
> +{
> +	int res =3D 0;
> +
> +	if (!driver->adapter || (driver->adapter !=3D adap))

Additional parens not required.
[Sandeep] Ok.

> +		return 0;
> +
> +	adap->drv_count--;
> +
> +	/* If no more driver is registed with the adapter*/
> +	if (!adap->drv_count && adap->tasklet_conf) {
> +		tasklet_disable(&adap->tdm_data_tasklet);
> +		tasklet_kill(&adap->tdm_data_tasklet);
> +		adap->tasklet_conf =3D 0;
> +	}
> +
> +	if (driver->detach_adapter) {
> +		if (driver->detach_adapter(adap))
> +			pr_err("tdm: detach_adapter failed for driver [%s]\n",
> +					driver->name);
> +	}
> +
> +	driver->adapter =3D NULL;
> +	return res;
> +}
> +
> +/* TDM adapter Registration/De-registration with TDM framework */
> +
> +static int tdm_register_adapter(struct tdm_adapter *adap) {
> +	int res =3D 0;
> +	struct tdm_driver *driver, *next;
> +
> +	mutex_init(&adap->adap_lock);
> +	INIT_LIST_HEAD(&adap->myports);
> +	spin_lock_init(&adap->portlist_lock);
> +
> +	adap->drv_count =3D 0;
> +	adap->tasklet_conf =3D 0;
> +
> +	list_add_tail(&adap->list, &adapter_list);

What protects this list?
[Sandeep] Ok, will take care

> +
> +	/* initialization of driver by framework in default configuration */
> +	init_config_adapter(adap);
> +
> +	/* Notify drivers */
> +	pr_info("adapter [%s] registered\n", adap->name);
> +	mutex_lock(&tdm_core_lock);
> +	list_for_each_entry_safe(driver, next, &driver_list, list) {
> +		if (tdm_device_match(driver, adap)) {
> +			res =3D tdm_attach_driver_adap(driver, adap);
> +			if (res =3D=3D 0) {
> +				pr_info("tdm: Driver(ID=3D%d) is "
> +						"attached with Adapter %s(ID"
> +						" =3D %d)\n", driver->id,
> +						adap->name, adap->id);
> +			} else {
> +				pr_err("tdm: Driver(ID=3D%d) is unable "
> +						"to attach with Adapter %s(ID =3D %d)\n",
> +						driver->id, adap->name,
> +						adap->id);
> +			}
> +		}
> +	}
> +	mutex_unlock(&tdm_core_lock);
> +
> +	return res;
> +}
> +
> +/*
> + * tdm_add_adapter - declare tdm adapter, use dynamic device number
> + * @adapter: the adapter to add
> + * Context: can sleep
> + *
> + * This routine is used to declare a TDM adapter
> + * When this returns zero, a new device number will be allocated and=20
> +stored
> + * in adap->id, and the specified adapter became available for the clien=
ts.
> + * Otherwise, a negative error number value is returned.
> + */
> +int tdm_add_adapter(struct tdm_adapter *adapter) {
> +	int id, res =3D 0;
> +
> +retry:
> +	if (idr_pre_get(&tdm_adapter_idr, GFP_KERNEL) =3D=3D 0)
> +		return -ENOMEM;
> +
> +	mutex_lock(&tdm_core_lock);
> +	res =3D idr_get_new(&tdm_adapter_idr, adapter, &id);
> +	mutex_unlock(&tdm_core_lock);
> +
> +	if (res < 0) {
> +		if (res =3D=3D -EAGAIN)
> +			goto retry;
> +		return res;
> +	}
> +
> +	adapter->id =3D id;
> +	return tdm_register_adapter(adapter); }=20
> +EXPORT_SYMBOL(tdm_add_adapter);
> +
> +
> +/*
> + * tdm_del_adapter - unregister TDM adapter
> + * @adap: the adapter being unregistered
> + *
> + * This unregisters an TDM adapter which was previously registered
> + * by @tdm_add_adapter.
> + */
> +int tdm_del_adapter(struct tdm_adapter *adap) {
> +	int res =3D 0;
> +	struct tdm_adapter *found;
> +	struct tdm_driver *driver, *next;
> +
> +	/* First make sure that this adapter was ever added */
> +	mutex_lock(&tdm_core_lock);
> +	found =3D idr_find(&tdm_adapter_idr, adap->id);
> +	mutex_unlock(&tdm_core_lock);
> +	if (found !=3D adap) {
> +		pr_err("tdm: attempting to delete unregistered "
> +				"adapter [%s]\n", adap->name);
> +		return -EINVAL;
> +	}
> +
> +	/* disable and kill the data processing tasklet */
> +	if (adap->tasklet_conf) {
> +		tasklet_disable(&adap->tdm_data_tasklet);
> +		tasklet_kill(&adap->tdm_data_tasklet);
> +		adap->tasklet_conf =3D 0;
> +	}
> +
> +	/*
> +	 * Detach any active ports. This can't fail, thus we do not
> +	 * checking the returned value.
> +	 */
> +	mutex_lock(&tdm_core_lock);
> +	list_for_each_entry_safe(driver, next, &driver_list, list) {
> +		if (tdm_device_match(driver, adap)) {
> +			tdm_detach_driver_adap(driver, adap);
> +			pr_info(
> +					"Driver(ID=3D%d) is detached from Adapter %s(ID =3D %d)\n",
> +					driver->id, adap->name, adap->id);
> +		}
> +	}
> +	idr_remove(&tdm_adapter_idr, adap->id);
> +	mutex_unlock(&tdm_core_lock);
> +
> +	pr_debug("adapter [%s] unregistered\n", adap->name);
> +
> +	list_del(&adap->list);

What protects this delete?
[Sandeep] Ok, will take care

> +	/*
> +	 * Clear the device structure in case this adapter is ever going to be
> +	 * added again
> +	 */
> +	adap->parent =3D NULL;
> +
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_del_adapter);
> +
> +/* TDM Client Drivers Registration/De-registration Functions */ int=20
> +tdm_register_driver(struct tdm_driver *driver) {
> +	int res =3D 0;
> +	struct tdm_adapter *adap, *next;
> +
> +	list_add_tail(&driver->list, &driver_list);

What serializes this list?
[Sandeep] Ok, will take care

> +
> +	mutex_lock(&tdm_core_lock);
> +	/* Walk the adapters that are already present */
> +	list_for_each_entry_safe(adap, next, &adapter_list, list) {
> +		if (tdm_device_match(driver, adap)) {
> +			res =3D tdm_attach_driver_adap(driver, adap);
> +			if (res =3D=3D 0) {
> +				pr_info("TDM Driver(ID=3D%d)is attached with "
> +						"Adapter%s(ID =3D %d) drv_count=3D%d",
> +						driver->id, adap->name,
> +						adap->id, adap->drv_count);
> +			} else {
> +				pr_err("TDM Driver(ID=3D%d) unable to attach "
> +						"to Adapter%s(ID =3D %d) drv_count=3D%d",
> +						driver->id, adap->name,
> +						adap->id, adap->drv_count);
> +			}
> +			break;
> +		}
> +	}
> +	mutex_unlock(&tdm_core_lock);
> +
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_register_driver);
> +
> +/*
> + * tdm_unregister_driver - unregister TDM client driver from TDM=20
> +framework
> + * @driver: the driver being unregistered  */ void=20
> +tdm_unregister_driver(struct tdm_driver *driver) {
> +	/*
> +	 * A driver can register to only one adapter,
> +	 * so no need to browse the list
> +	 */
> +	mutex_lock(&tdm_core_lock);
> +	tdm_detach_driver_adap(driver, driver->adapter);
> +	mutex_unlock(&tdm_core_lock);
> +
> +	list_del(&driver->list);

What serializes this delete?
[Sandeep] Ok, will take care

> +
> +	pr_debug("tdm-core: driver [%s] unregistered\n", driver->name); }=20
> +EXPORT_SYMBOL(tdm_unregister_driver);
> +
> +/* Interface to the tdm device/adapter */
> +
> +/*
> + * tdm_adap_send - issue a TDM write
> + * @adap: Handle to TDM device
> + * @buf: Data that will be written to the TDM device
> + * @count: How many bytes to write
> + *
> + * Returns negative errno, or else the number of bytes written.
> + */
> +int tdm_adap_send(struct tdm_adapter *adap, void **buf, int count) {
> +	int res;
> +
> +	if (adap->algo->tdm_write)
> +		res =3D adap->algo->tdm_write(adap, buf, count);
> +	else {
> +		pr_err("TDM level write not supported\n");

Is there no associated struct device which could be used for these error me=
ssages?  What if you have more than one TDM device?  It would be useful to =
know which is producing errors.
[Sandeep] Since this is a framework, we do not have any struct device assoc=
iated. But we should work some way out to produce error messages which can =
be linked with specific adapter.

> +		return -EOPNOTSUPP;
> +	}
> +
> +	/*
> +	 * If everything went ok (i.e. frame transmitted), return #bytes
> +	 * transmitted, else error code.
> +	 */
> +	return (res =3D=3D 1) ? count : res;
> +}
> +EXPORT_SYMBOL(tdm_adap_send);
> +
> +/*
> + * tdm_adap_recv - issue a TDM read
> + * @adap: Handle to TDM device
> + * @buf: Where to store data read from TDM device
> + *
> + * Returns negative errno, or else the number of bytes read.
> + */
> +int tdm_adap_recv(struct tdm_adapter *adap, void **buf) {
> +	int res;
> +
> +	if (adap->algo->tdm_read)
> +		res =3D adap->algo->tdm_read(adap, (u16 **)buf);
> +	else {
> +		pr_err("TDM level read not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +	/*
> +	 * If everything went ok (i.e. frame received), return #bytes
> +	 * transmitted, else error code.
> +	 */
> +	return res;
> +}
> +
> +/*
> + * tdm_adap_get_write_buf - get next write TDM device buffer
> + * @adap: Handle to TDM device
> + * @buf: pointer to TDM device buffer
> + *
> + * Returns negative errno, or else size of the write buffer.
> + */
> +int tdm_adap_get_write_buf(struct tdm_adapter *adap, void **buf) {
> +	int res;
> +
> +	if (adap->algo->tdm_get_write_buf) {
> +		res =3D adap->algo->tdm_get_write_buf(adap, (u16 **)buf);
> +	} else {
> +		pr_err("TDM level write buf get not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +	/*
> +	 * If everything went ok (i.e. 1 msg received), return #bytes
> +	 * transmitted, else error code.
> +	 */
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_adap_get_write_buf);
> +
> +int tdm_adap_enable(struct tdm_driver *drv) {
> +	int res;
> +	struct tdm_adapter *adap;
> +	adap =3D drv->adapter;

The above two lines can become one line.
[Sandeep] Sure.=20

> +
> +	if (adap->algo->tdm_enable) {
> +		res =3D adap->algo->tdm_enable(adap);
> +	} else {
> +		pr_err("TDM level enable not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_adap_enable);
> +
> +int tdm_adap_disable(struct tdm_driver *drv) {
> +	int res;
> +	struct tdm_adapter *adap;
> +	adap =3D drv->adapter;

Ditto.
[Sandeep] Ok.

> +
> +	if (adap->algo->tdm_disable) {
> +		res =3D adap->algo->tdm_disable(adap);
> +	} else {
> +		pr_err("TDM level enable not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_adap_disable);
> +
> +struct tdm_adapter *tdm_get_adapter(int id) {
> +	struct tdm_adapter *adapter;
> +
> +	mutex_lock(&tdm_core_lock);
> +	adapter =3D idr_find(&tdm_adapter_idr, id);
> +	if (adapter && !try_module_get(adapter->owner))
> +		adapter =3D NULL;
> +
> +	mutex_unlock(&tdm_core_lock);
> +
> +	return adapter;
> +}
> +EXPORT_SYMBOL(tdm_get_adapter);
> +
> +void tdm_put_adapter(struct tdm_adapter *adap) {
> +	module_put(adap->owner);
> +}
> +EXPORT_SYMBOL(tdm_put_adapter);
> +
> +
> +/* Port Level APIs of TDM Framework */ int tdm_port_open(struct=20
> +tdm_driver *driver, struct tdm_port **h_port) {
> +	struct tdm_port *port;
> +	struct tdm_adapter *adap;
> +	unsigned long flags;
> +	int res =3D 0;
> +
> +	/*
> +	 * This creates an anonymous tdm_port, which may later be
> +	 * pointed to some slot.
> +	 */
> +	port =3D kzalloc(sizeof(*port), GFP_KERNEL);
> +	if (!port) {
> +		res =3D -ENOMEM;
> +		return res;
> +	}
> +
> +	adap =3D tdm_get_adapter(driver->adapter->id);
> +	if (!adap)
> +		return -ENODEV;
> +
> +	port->rx_max_frames =3D NUM_SAMPLES_PER_FRAME;
> +	port->port_cfg.port_mode =3D TDM_PORT_CHANNELIZED;
> +
> +	snprintf(driver->name, TDM_NAME_SIZE, "tdm-dev");
> +	port->driver =3D driver;
> +	port->adapter =3D adap;
> +
> +	spin_lock_irqsave(&adap->portlist_lock, flags);
> +	list_add_tail(&port->list, &adap->myports);
> +	spin_unlock_irqrestore(&adap->portlist_lock, flags);
> +
> +	INIT_LIST_HEAD(&port->mychannels);
> +
> +	*h_port =3D port;
> +	return res;
> +
> +}
> +EXPORT_SYMBOL(tdm_port_open);
> +
> +int tdm_port_close(struct tdm_port *h_port) {
> +	struct tdm_adapter *adap;
> +	struct tdm_driver *driver;
> +	struct tdm_port *port;
> +	struct tdm_channel *temp, *channel;
> +	unsigned long flags;
> +	int res =3D 0;
> +	port =3D h_port;
> +
> +	driver =3D  port->driver;
> +
> +	list_for_each_entry_safe(channel, temp, &port->mychannels, list) {
> +		if (channel)
> +			if (channel->in_use) {

if (channel && channel->in_use) {

> +				pr_err("tdm: Cannot close port. Channel in"
> +						"use\n");

Don't wrap error messages.
[Sandeep] How to handle them, it exceeds 80 character limit.

> +				res =3D -ENXIO;
> +				goto out;
> +			}
> +	}
> +	adap =3D driver->adapter;
> +	tdm_put_adapter(adap);
> +
> +	spin_lock_irqsave(&adap->portlist_lock, flags);
> +	list_del(&port->list);
> +	spin_unlock_irqrestore(&adap->portlist_lock, flags);
> +
> +	if (port->p_port_data !=3D NULL) {
> +		int i;
> +		struct tdm_bd *ch_bd;
> +
> +		/*
> +		 * If the tdm is in channelised mode,
> +		 * de-allocate the channelised buffer
> +		 */
> +		ch_bd =3D &(port->p_port_data->rx_data_fifo[0]);
> +		for (i =3D 0; ch_bd && i < TDM_CH_RX_BD_RING_SIZE; i++) {
> +			ch_bd->flag =3D 0;
> +			ch_bd++;
> +		}
> +		ch_bd =3D &(port->p_port_data->tx_data_fifo[0]);
> +		for (i =3D 0; ch_bd && i < TDM_CH_TX_BD_RING_SIZE; i++) {
> +			ch_bd->flag =3D 0;
> +			ch_bd++;
> +		}
> +		kfree(port->p_port_data);
> +	}
> +	kfree(port);
> +	return res;
> +out:
> +	if (port)
> +		kfree(port->p_port_data);
> +	kfree(port);
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_port_close);
> +
> +int tdm_channel_read(struct tdm_port *h_port, struct tdm_channel *h_chan=
nel,
> +		void *p_data, u16 *size)
> +{
> +	struct tdm_channel *channel;
> +	struct tdm_bd *rx_bd;
> +	unsigned long flags;
> +	int res =3D 0;
> +	unsigned short *buf, *buf1;
> +	channel =3D h_channel;
> +
> +	if (!channel->p_ch_data || !channel->in_use)
> +		return -EIO;
> +
> +	spin_lock_irqsave(&channel->p_ch_data->rx_channel_lock, flags);
> +	rx_bd =3D channel->p_ch_data->rx_out_data;
> +
> +	if (rx_bd->flag) {
> +		*size =3D rx_bd->length;
> +		buf =3D (u16 *) p_data;
> +		buf1 =3D (u16 *)rx_bd->p_data;
> +		memcpy(buf1, buf, NUM_SAMPLES_PER_FRAME);
> +		rx_bd->flag =3D 0;
> +		rx_bd->offset =3D 0;
> +		channel->p_ch_data->rx_out_data =3D (rx_bd->wrap) ?
> +			channel->p_ch_data->rx_data_fifo : rx_bd + 1;
> +
> +	} else {
> +		spin_unlock_irqrestore(&channel->p_ch_data->rx_channel_lock,
> +				flags);
> +		pr_debug("No Data Available");
> +		return -EAGAIN;
> +	}
> +	spin_unlock_irqrestore(&channel->p_ch_data->rx_channel_lock, flags);
> +
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_channel_read);
> +
> +
> +int tdm_channel_write(struct tdm_port *h_port, struct tdm_channel *h_cha=
nnel,
> +		void *p_data, u16 size)
> +{
> +	struct tdm_port *port;
> +	struct tdm_channel *channel;
> +	struct tdm_bd *tx_bd;
> +	unsigned long flags;
> +	int err =3D 0;
> +	port =3D h_port;
> +	channel =3D h_channel;
> +#ifdef DEBUG
> +	bool data_flag =3D 0;
> +#endif
> +
> +	if (p_data =3D=3D NULL) { /* invalid data*/
> +		pr_err("tdm: Invalid Data");
> +		return -EINVAL;
> +	}
> +
> +	if (!channel->p_ch_data || !channel->in_use)
> +		return -EIO;
> +
> +	spin_lock_irqsave(&channel->p_ch_data->tx_channel_lock, flags);
> +	tx_bd =3D channel->p_ch_data->tx_in_data;
> +
> +	if (!tx_bd->flag) {
> +		tx_bd->length =3D size;
> +		memcpy(tx_bd->p_data, p_data,
> +				size * port->adapter->adapt_cfg.slot_width);
> +		tx_bd->flag =3D 1;
> +		tx_bd->offset =3D 0;
> +		channel->p_ch_data->tx_in_data =3D (tx_bd->wrap) ?
> +			channel->p_ch_data->tx_data_fifo : tx_bd+1;
> +		port->port_stat.tx_pkt_count++;
> +#ifdef DEBUG
> +		data_flag =3D 1;
> +#endif
> +	} else {
> +		spin_unlock_irqrestore(&channel->p_ch_data->tx_channel_lock,
> +				flags);
> +		port->port_stat.tx_pkt_drop_count++;
> +		pr_err("tdm: Transmit failed.");
> +		return -ENOMEM;
> +	}
> +	spin_unlock_irqrestore(&channel->p_ch_data->tx_channel_lock, flags);
> +
> +#ifdef	DEBUG
> +	if (data_flag) {
> +		int k;
> +		pr_info("\nTX port:%d - Write - Port TX-%d\n",
> +				port->port_id, size);
> +		for (k =3D 0; k < size; k++)
> +			pr_info("%x", p_data[k]);
> +		pr_info("\n");
> +	}
> +#endif
> +	return err;
> +}
> +EXPORT_SYMBOL(tdm_channel_write);
> +
> +/*
> + * Driver Function for select and poll. Based on Channel, it sleeps=20
> +on
> + * waitqueue
> + */
> +int tdm_ch_poll(struct tdm_channel *h_channel, unsigned int=20
> +wait_time) {
> +	struct tdm_channel *channel;
> +	channel =3D h_channel;
> +
> +	if (!channel->p_ch_data || !channel->in_use)
> +		return -EIO;
> +
> +	if (channel->p_ch_data->rx_out_data->flag) {
> +		pr_debug("Data Available");
> +		return 0;
> +	}
> +	if (wait_time) {
> +		unsigned long timeout =3D msecs_to_jiffies(wait_time);
> +
> +		wait_event_interruptible_timeout(channel->ch_wait_queue,
> +				channel->p_ch_data->rx_out_data->flag,
> +				timeout);
> +
> +		if (channel->p_ch_data->rx_out_data->flag) {
> +			pr_debug("Data Available");
> +			return 0;
> +		}
> +	}
> +	return -EAGAIN;

That is incorrect.  -EAGAIN is the wrong return code when
wait_event_interruptible_timeout() returns -ERESTARTSYS.
[Sandeep] Ok

> +}
> +EXPORT_SYMBOL(tdm_ch_poll);
> +
> +unsigned int tdm_port_get_stats(struct tdm_port *h_port,
> +		struct tdm_port_stats *portstat)
> +{
> +	struct tdm_port *port;
> +	int port_num;
> +	port =3D h_port;
> +
> +	if (port =3D=3D NULL || portstat =3D=3D NULL) { /* invalid handle */
> +		pr_err("tdm: Invalid Handle");
> +		return -ENXIO;
> +	}
> +	port_num =3D  port->port_id;
> +
> +	memcpy(portstat, &port->port_stat, sizeof(struct tdm_port_stats));
> +
> +	pr_info("TDM Port %d Get Stats", port_num);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(tdm_port_get_stats);
> +
> +/* Data handling functions */
> +
> +static int tdm_data_rx_deinterleave(struct tdm_adapter *adap) {
> +	struct tdm_port *port, *next;
> +	struct tdm_channel *channel, *temp;
> +	struct tdm_bd	*ch_bd;
> +
> +	int i, buf_size, ch_data_len;
> +	u16 *input_tdm_buffer;
> +	u16 *pcm_buffer;
> +	int slot_width;
> +	int frame_ch_data_size;
> +	bool ch_data;
> +	int bytes_in_fifo_per_frame;
> +	int bytes_slot_offset;
> +
> +	ch_data_len =3D NUM_SAMPLES_PER_FRAME;
> +	frame_ch_data_size =3D NUM_SAMPLES_PER_FRAME;
> +	ch_data =3D 0;
> +
> +	slot_width =3D adap->adapt_cfg.slot_width;
> +	buf_size =3D tdm_adap_recv(adap, (void **)&input_tdm_buffer);
> +	if (buf_size <=3D 0 || !input_tdm_buffer)
> +		return -EINVAL;
> +
> +	bytes_in_fifo_per_frame =3D buf_size/frame_ch_data_size;

Spaces around /
[Sandeep] Ok.

> +	bytes_slot_offset =3D bytes_in_fifo_per_frame/slot_width;

Spaces around /
[Sandeep] Ok.

> +
> +	/* de-interleaving for all ports*/
> +	list_for_each_entry_safe(port, next, &adap->myports, list) {
> +
> +		list_for_each_entry_safe(channel, temp, &port->mychannels,
> +				list) {

Why do you need the _safe variants here?  I can't see anything in this code=
 which manipulates the lists.
[Sandeep] Will check.

> +			/* if the channel is not open */
> +			if (!channel->in_use || !channel->p_ch_data)
> +				continue;
> +			ch_bd =3D channel->p_ch_data->rx_in_data;
> +			spin_lock(&channel->p_ch_data->rx_channel_lock);
> +			/*if old data is to be discarded */
> +			if (use_latest_tdm_data && ch_bd->flag) {
> +				ch_bd->flag =3D 0;
> +				ch_bd->offset =3D 0;
> +				if (ch_bd =3D=3D channel->p_ch_data->rx_out_data)
> +					channel->p_ch_data->rx_out_data =3D
> +						ch_bd->wrap ?
> +						channel->p_ch_data->rx_data_fifo
> +						: ch_bd+1;
> +				port->port_stat.rx_pkt_drop_count++;
> +			}
> +			/* if the bd is empty */
> +			if (!ch_bd->flag) {
> +				if (ch_bd->offset =3D=3D 0)
> +					ch_bd->length =3D port->rx_max_frames;
> +
> +				pcm_buffer =3D ch_bd->p_data + ch_bd->offset;
> +				/* De-interleaving the data */
> +				for (i =3D 0; i < ch_data_len; i++) {
> +					pcm_buffer[i]
> +						=3D input_tdm_buffer[i*
> +						bytes_slot_offset +
> +						channel->ch_id];
> +				}
> +				ch_bd->offset +=3D ch_data_len * slot_width;
> +
> +				if (ch_bd->offset >=3D
> +						(ch_bd->length -
> +						frame_ch_data_size)*
> +						(adap->adapt_cfg.slot_width)) {
> +					ch_bd->flag =3D 1;
> +					ch_bd->offset =3D 0;
> +					channel->p_ch_data->rx_in_data =3D
> +						ch_bd->wrap ?
> +						channel->p_ch_data->rx_data_fifo
> +						: ch_bd+1;
> +					ch_data =3D 1;
> +					wake_up_interruptible
> +						(&channel->ch_wait_queue);
> +				}
> +			} else {
> +				port->port_stat.rx_pkt_drop_count++;
> +			}
> +			spin_unlock(&channel->p_ch_data->rx_channel_lock);
> +		}
> +
> +		if (ch_data) {
> +			/* Wake up the Port Data Poll event */
> +#ifdef	DEBUG
> +			pr_info("Port RX-%d-%d\n", channel->ch_id, ch_data_len);
> +			for (i =3D 0; i < ch_data_len; i++)
> +				pr_info("%x", pcm_buffer[i]);
> +			pr_info("\n");
> +#endif
> +			port->port_stat.rx_pkt_count++;
> +			ch_data =3D 0;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int tdm_data_tx_interleave(struct tdm_adapter *adap) {
> +	struct tdm_port *port, *next;
> +	struct tdm_channel *channel, *temp;
> +	struct tdm_bd	*ch_bd;
> +	int i, buf_size, ch_data_len =3D NUM_SAMPLES_PER_FRAME;
> +	bool last_data =3D 0;
> +	u16 *output_tdm_buffer;
> +	u16 *pcm_buffer;
> +	int frame_ch_data_size =3D NUM_SAMPLES_PER_FRAME;
> +	int bytes_in_fifo_per_frame;
> +	int bytes_slot_offset;
> +
> +#ifdef DEBUG
> +	u8	data_flag =3D 0;
> +#endif
> +
> +	buf_size =3D tdm_adap_get_write_buf(adap, (void **)&output_tdm_buffer);
> +	if (buf_size <=3D 0 || !output_tdm_buffer)
> +		return -EINVAL;
> +
> +	bytes_in_fifo_per_frame =3D buf_size/frame_ch_data_size;
> +	bytes_slot_offset =3D=20
> +bytes_in_fifo_per_frame/adap->adapt_cfg.slot_width;
> +
> +
> +	memset(output_tdm_buffer, 0, sizeof(buf_size));
> +
> +	list_for_each_entry_safe(port, next, &adap->myports, list) {
> +
> +		list_for_each_entry_safe(channel, temp, &port->mychannels,
> +				list) {

Why do you need the _safe variants here?  I can't see anything in this code=
 which manipulates the lists.
[Sandeep] Will check.

> +			pr_debug("TX-Tdm %d (slots-)", channel->ch_id);
> +
> +
> +			/* if the channel is open */
> +			if (!channel->in_use || !channel->p_ch_data)
> +				continue;
> +
> +			spin_lock(&channel->p_ch_data->tx_channel_lock);
> +			if (!channel->in_use || !channel->p_ch_data)
> +				continue;
> +			ch_bd =3D channel->p_ch_data->tx_out_data;
> +			if (ch_bd->flag) {
> +				pcm_buffer =3D (u16 *)((uint8_t *)ch_bd->p_data +
> +						ch_bd->offset);
> +				/* if the buffer has less frames than required*/

Space before */
[Sandeep] Ok

> +				if (frame_ch_data_size >=3D
> +						(ch_bd->length - ch_bd->offset/

Space before /
[Sandeep] Ok

> +						 adap->adapt_cfg.slot_width)) {
> +					ch_data_len =3D
> +						ch_bd->length - ch_bd->offset/

Space before /
[Sandeep] Ok

> +						adap->adapt_cfg.slot_width;

Wrong indentation.  If you're finding the 80 column limit is causing proble=
ms, you have too much code in this function (read Documentation/CodingStyle=
 again).
[Sandeep] Ok.

> +					last_data =3D 1;
> +				} else {
> +					ch_data_len =3D frame_ch_data_size;
> +				}
> +				/* Interleaving the data */
> +				for (i =3D 0; i < ch_data_len; i++) {
> +					/*
> +					 * TODO- need to be genric for any size
> +					 *  assignment
> +					 */
> +					output_tdm_buffer[channel->ch_id +
> +						bytes_slot_offset * i] =3D
> +						pcm_buffer[i];
> +				}
> +				/*
> +				 * If all the data of this buffer is
> +				 * transmitted
> +				 */
> +				if (last_data) {
> +					ch_bd->flag =3D 0;
> +					ch_bd->offset =3D 0;
> +					channel->p_ch_data->tx_out_data =3D
> +						ch_bd->wrap ?
> +						channel->p_ch_data->tx_data_fifo
> +						: ch_bd+1;

Spaces around +
[Sandeep] Ok

> +					port->port_stat.tx_pkt_conf_count++;
> +				} else {
> +					ch_bd->offset +=3D ch_data_len *
> +						(adap->adapt_cfg.slot_width);
> +				}
> +#ifdef	DEBUG
> +				data_flag =3D 1;
> +#endif
> +			}
> +			spin_unlock(&channel->p_ch_data->tx_channel_lock);
> +		}
> +	}
> +
> +#ifdef	DEBUG
> +	if (data_flag) {
> +		pr_info("TX-TDM Interleaved Data-\n");
> +		for (i =3D 0; i < 64; i++)
> +			pr_info("%x", output_tdm_buffer[i]);
> +		pr_info("\n");
> +	}
> +#endif
> +	return 0;
> +}
> +
> +/* Channel Level APIs of TDM Framework */ int tdm_channel_open(u16=20
> +chanid, u16 ch_width, struct tdm_port *port,
> +		struct tdm_channel **h_channel)
> +{
> +	struct tdm_channel *channel, *temp;
> +	unsigned long		flags;
> +	struct tdm_ch_data	*p_ch_data;
> +	int res =3D 0;
> +
> +	if (ch_width !=3D 1) {
> +		pr_err("tdm: Mode not supported\n");
> +		return -EINVAL;
> +	}
> +
> +	list_for_each_entry_safe(channel, temp, &port->mychannels, list) {
> +		if (channel->ch_id =3D=3D chanid) {
> +			pr_err("tdm: Channel %d already open\n", chanid);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	channel =3D kzalloc(sizeof(*channel), GFP_KERNEL);
> +	if (!channel) {
> +		res =3D -ENOMEM;
> +		goto out;
> +	}
> +
> +	init_waitqueue_head(&channel->ch_wait_queue);
> +	p_ch_data =3D kzalloc(sizeof(struct tdm_ch_data), GFP_KERNEL);
> +	if (!p_ch_data) {
> +		res =3D -ENOMEM;
> +		goto outdata;
> +	}
> +
> +	p_ch_data->rx_data_fifo[TDM_CH_RX_BD_RING_SIZE-1].wrap =3D 1;
> +	p_ch_data->tx_data_fifo[TDM_CH_TX_BD_RING_SIZE-1].wrap =3D 1;
> +
> +	p_ch_data->rx_in_data =3D p_ch_data->rx_data_fifo;
> +	p_ch_data->rx_out_data =3D p_ch_data->rx_data_fifo;
> +	p_ch_data->tx_in_data =3D p_ch_data->tx_data_fifo;
> +	p_ch_data->tx_out_data =3D p_ch_data->tx_data_fifo;
> +	spin_lock_init(&p_ch_data->rx_channel_lock);
> +	spin_lock_init(&p_ch_data->tx_channel_lock);
> +
> +	channel->p_ch_data =3D p_ch_data;
> +
> +	channel->ch_id =3D chanid;
> +	channel->ch_cfg.first_slot =3D chanid;
> +	channel->ch_cfg.num_slots =3D 1;	/*
> +					 * This is 1 for channelized mode and
> +					 * configurable for other modes
> +					 */
> +	channel->port =3D port;
> +	channel->in_use =3D 1;
> +
> +	spin_lock_irqsave(&port->ch_list_lock, flags);
> +	list_add_tail(&channel->list, &port->mychannels);
> +	spin_unlock_irqrestore(&port->ch_list_lock, flags);
> +
> +	*h_channel =3D channel;
> +
> +	return res;
> +
> +outdata:
> +	kfree(channel);
> +out:
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_channel_open);
> +
> +int tdm_sysfs_init(void)

static?
[Sandeep] Ok.

> +{
> +	struct kobject *tdm_kobj;
> +	int err =3D 1;
> +	tdm_kobj =3D kzalloc(sizeof(*tdm_kobj), GFP_KERNEL);
> +	if (tdm_kobj) {
> +		kobject_init(tdm_kobj, &tdm_type);
> +		if (kobject_add(tdm_kobj, NULL, "%s", "tdm")) {
> +			pr_err("tdm: Sysfs creation failed\n");
> +			kobject_put(tdm_kobj);
> +			err =3D -EINVAL;
> +			goto out;
> +		}
> +	} else {
> +		pr_err("tdm: Unable to allocate tdm_kobj\n");
> +		err =3D -ENOMEM;
> +		goto out;
> +	}

What if this function gets called multiple times?  It looks like
kobject_add() will fail due to name conflicts.

Also, what's the point of the above?  I looks like the kobject is never use=
d.
[Sandeep] Will check.

> +
> +out:
> +	return err;
> +}
> +
> +int tdm_channel_close(u16 chanid, u16 ch_width, struct tdm_port *port,
> +		struct tdm_channel *h_channel)
> +{
> +	struct tdm_channel *channel;
> +	unsigned long		flags;
> +	int res =3D 0;
> +	channel =3D h_channel;
> +
> +	spin_lock_irqsave(&port->ch_list_lock, flags);
> +	list_del(&channel->list);
> +	spin_unlock_irqrestore(&port->ch_list_lock, flags);
> +
> +	if (channel)
> +		kfree(channel->p_ch_data);
> +	kfree(channel);
> +	return res;
> +}
> +EXPORT_SYMBOL(tdm_channel_close);
> +
> +ssize_t tdm_show_sysfs(struct kobject *kobj,
> +		struct attribute *attr, char *buf)
> +{
> +	int retval =3D 0;
> +	struct tdm_sysfs *a =3D container_of(attr,
> +			struct tdm_sysfs, attr);
> +	switch (a->cmd_type) {
> +	case TDM_LATEST_DATA:
> +		pr_info("use_latest_tdm_data: %d\n", use_latest_tdm_data);
> +		break;
> +	default:
> +		pr_info("Invalid cmd_type value\n");
> +		return -EINVAL;
> +	}
> +	return retval;
> +}
> +
> +ssize_t tdm_store_sysfs(struct kobject *kobj,
> +		struct attribute *attr, const char *buf, size_t len) {
> +	struct tdm_sysfs *a =3D container_of(attr,
> +			struct tdm_sysfs, attr);
> +
> +	sscanf(buf, "%d", &a->data);
> +	use_latest_tdm_data =3D a->data;
> +	return strlen(buf);
> +}
> +
> +void init_config_adapter(struct tdm_adapter *adap) {
> +	struct fsl_tdm_adapt_cfg default_adapt_cfg =3D {
> +		.loopback =3D TDM_PROCESS_NORMAL,
> +		.num_ch =3D NUM_CHANNELS,
> +		.ch_size_type =3D CHANNEL_16BIT_LIN,
> +		.frame_len =3D NUM_SAMPLES_PER_FRAME,
> +		.num_frames =3D NUM_SAMPLES_PER_FRAME,
> +		.adap_mode =3D TDM_ADAPTER_MODE_NONE
> +	};
> +
> +	default_adapt_cfg.slot_width =3D default_adapt_cfg.ch_size_type/3 + 1;

Spaces around /
[Sandeep] Ok

> +
> +	memcpy(&adap->adapt_cfg, &default_adapt_cfg,
> +			sizeof(struct fsl_tdm_adapt_cfg));

If this is supposed to be a generic layer, why the fsl_ data in core code?
[Sandeep] It's because framework initializes adapter in a default configura=
tion and then passes it to the adapter.

> +
> +	return;
> +}
> +EXPORT_SYMBOL(init_config_adapter);
> +
> +void tdm_data_tasklet_fn(unsigned long data)

static?
[Sandeep] Ok

> +{
> +	struct tdm_adapter *adapter;
> +	adapter =3D (struct tdm_adapter *)data;
> +	if (adapter !=3D NULL) {
> +		tdm_data_tx_interleave(adapter);
> +		tdm_data_rx_deinterleave(adapter);
> +	}
> +}

A general comment: it is better to place functions before their first use w=
here possible.
[Sandeep] Alright.

> +
> +
> +MODULE_AUTHOR("Hemant Agrawal <hemant@freescale.com> and "
> +	"Rajesh Gumasta <rajesh.gumasta@freescale.com>");=20
> +MODULE_DESCRIPTION("TDM Driver Framework Core");=20
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mod_devicetable.h=20
> b/include/linux/mod_devicetable.h index 83ac071..573ac40 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -425,6 +425,17 @@ struct i2c_device_id {
>  			__attribute__((aligned(sizeof(kernel_ulong_t))));
>  };
> =20
> +/* tdm */
> +
> +#define TDM_NAME_SIZE   20
> +#define TDM_MODULE_PREFIX "tdm:"
> +
> +struct tdm_device_id {
> +	char name[TDM_NAME_SIZE];
> +	kernel_ulong_t driver_data      /* Data private to the driver */
> +			__attribute__((aligned(sizeof(kernel_ulong_t))));
> +};
> +
>  /* spi */
> =20
>  #define SPI_NAME_SIZE	32
> diff --git a/include/linux/tdm.h b/include/linux/tdm.h new file mode=20
> 100644 index 0000000..44cd8cf
> --- /dev/null
> +++ b/include/linux/tdm.h
> @@ -0,0 +1,389 @@
> +/* include/linux/tdm.h
> + *
> + * Copyright 2012 Freescale Semiconductor, Inc.
> + *
> + * tdm.h - definitions for the tdm-device framework interface
> + *
> + * Author:Hemant Agrawal <hemant@freescale.com>
> + *	Rajesh Gumasta <rajesh.gumasta@freescale.com>
> + *
> + * This program is free software; you can redistribute  it and/or=20
> +modify it
> + * under  the terms of  the GNU General  Public License as published=20
> +by the
> + * Free Software Foundation;  either version 2 of the  License, or=20
> +(at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,=20
> +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.
> + *
> + * You should have received a copy of the  GNU General Public License=20
> +along
> + * with this program; if not, write  to the Free Software Foundation,=20
> +Inc.,
> + * 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +
> +
> +#ifndef _LINUX_TDM_H
> +#define _LINUX_TDM_H
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/device.h>	/* for struct device */
> +#include <linux/sched.h>	/* for completion */
> +#include <linux/mutex.h>
> +#include <linux/interrupt.h>
> +#include <linux/sysfs.h>
> +
> +#define TDM_LATEST_DATA		1
> +#define CHANNEL_8BIT_LIN	0	/* 8 bit linear */
> +#define CHANNEL_8BIT_ULAW	1	/* 8 bit Mu-law */
> +#define CHANNEL_8BIT_ALAW	2	/* 8 bit A-law */
> +#define CHANNEL_16BIT_LIN	3	/* 16 bit Linear */
> +
> +/*
> + * Default adapter configuration. All the TDM adapters registered=20
> +with
> + * framework will be configured with following default configuration.
> + */
> +#define NUM_CHANNELS	16
> +
> +/*
> + * Default configuration for typical voice data sample. These=20
> +parameters
> + * will generally not be required to be changed for voice type applicati=
ons.
> + */
> +
> +/* 8 samples per milli sec per channel. Req for voice data */
> +#define NUM_SAMPLES_PER_MS	8
> +#define NUM_MS			10
> +
> +/* Number of samples for 1 client buffer */
> +#define NUM_SAMPLES_PER_FRAME	(NUM_MS * NUM_SAMPLES_PER_MS)
> +#define NUM_OF_TDM_BUF		3
> +
> +/* General options */
> +
> +struct tdm_adapt_algorithm;
> +struct tdm_adapter;
> +struct tdm_port;
> +
> +
> +/*
> + * struct tdm_driver - represent an TDM device driver
> + * @class: What kind of tdm device we instantiate (for detect)
> + * @id:Driver id
> + * @name: Name of the driver
> + * @attach_adapter: Callback for device addition (for legacy drivers)
> + * @detach_adapter: Callback for device removal (for legacy drivers)
> + * @probe: Callback for device binding
> + * @remove: Callback for device unbinding
> + * @shutdown: Callback for device shutdown
> + * @suspend: Callback for device suspend
> + * @resume: Callback for device resume
> + * @command: Callback for sending commands to device
> + * @id_table: List of TDM devices supported by this driver
> + * @list: List of drivers created (for tdm-core use only)  */ struct=20
> +tdm_driver {
> +	unsigned int class;
> +	unsigned int id;
> +	char name[TDM_NAME_SIZE];
> +
> +	int (*attach_adapter)(struct tdm_adapter *);
> +	int (*detach_adapter)(struct tdm_adapter *);
> +
> +	/* Standard driver model interfaces */
> +	int (*probe)(const struct tdm_device_id *);
> +	int (*remove)(void);
> +
> +	/* driver model interfaces that don't relate to enumeration */
> +	void (*shutdown)(void);
> +	int (*suspend)(pm_message_t mesg);
> +	int (*resume)(void);

Far better to use the dev_pm_ops stuff now, don't propagate the old PM inte=
rfaces.
[Sandeep] Ok.

> +
> +	const struct tdm_device_id *id_table;
> +
> +	/* The associated adapter for this driver */
> +	struct tdm_adapter *adapter;
> +	struct list_head list;
> +};
> +
> +/*
> + * tdm per port statistics structure, used for providing and storing=20
> +tdm port
> + * statistics.
> + */
> +struct tdm_port_stats {
> +	unsigned int rx_pkt_count;	/* Rx frame count per channel */
> +	unsigned int rx_pkt_drop_count;	/*
> +					 * Rx drop count per channel to
> +					 * clean space for new buffer
> +					 */
> +	unsigned int tx_pkt_count;	/* Tx frame count per channel */
> +	unsigned int tx_pkt_conf_count;	/*
> +					 * Tx frame confirmation count per
> +					 * channel
> +					 */
> +	unsigned int tx_pkt_drop_count;	/*
> +					 * Tx drop count per channel due to
> +					 * queue full
> +					 */
> +};
> +
> +
> +/*
> + * tdm Buffer Descriptor, used for Creating Interleaved and=20
> +De-interleaved
> + * FIFOs
> + */
> +struct tdm_bd {
> +	unsigned char flag;		/* BD is full or empty */
> +	unsigned char wrap;		/* BD is last in the queue */
> +	unsigned short length;		/* Length of data in BD */
> +	/*TODO: use dyanmic memory */
> +	unsigned short p_data[NUM_SAMPLES_PER_FRAME];	/* Data Pointer */
> +	unsigned long offset;	/* Offset of the Data Pointer to be used */
> +};
> +
> +#define TDM_CH_RX_BD_RING_SIZE	3
> +#define TDM_CH_TX_BD_RING_SIZE	3
> +
> +/* tdm RX-TX Channelised Data */
> +struct tdm_port_data {
> +	struct tdm_bd rx_data_fifo[TDM_CH_RX_BD_RING_SIZE]; /*
> +							     * Rx Channel Data
> +							     * BD Ring
> +							     */
> +	struct tdm_bd *rx_in_data;	/*
> +					 * Current Channel Rx BD to be filled by
> +					 * de-interleave function
> +					 */
> +	struct tdm_bd *rx_out_data;	/*
> +					 * Current Channel Rx BD to be
> +					 * read by App
> +					 */
> +	struct tdm_bd tx_data_fifo[TDM_CH_TX_BD_RING_SIZE]; /*
> +							     * Tx Channel Data
> +							     *	BD Ring
> +							     */
> +	struct tdm_bd *tx_in_data;	/*
> +					 * Current Channel Tx BD to be
> +					 * filled by App
> +					 */
> +	struct tdm_bd *tx_out_data;	/*
> +					 * Current Channel Tx BD to be read by
> +					 * interleave function
> +					 */
> +	spinlock_t rx_channel_lock;	/* Spin Lock for Rx Channel */
> +	spinlock_t tx_channel_lock;	/* Spin Lock for Tx Channel */
> +};
> +
> +/* structure tdm_port_cfg - contains configuration params for a port=20
> +*/ struct tdm_port_cfg {
> +	unsigned short port_mode;
> +};
> +
> +/* struct tdm_port - represent an TDM ports for a device */ struct=20
> +tdm_port {
> +	unsigned short port_id;
> +	unsigned short in_use;	/* Port is enabled? */
> +	uint16_t rx_max_frames;	/*
> +				 * Received port frames before allowing
> +				 * read operation in Port Mode
> +				 */
> +
> +	struct tdm_port_stats port_stat; /*
> +					  * A structure parameter defining
> +					  * TDM port statistics.
> +					  */
> +	struct tdm_port_data *p_port_data;	/*
> +						 * a structure parameter
> +						 * defining tdm channelised data
> +						 */
> +
> +	struct tdm_driver *driver;	/* driver for this port */
> +	struct tdm_adapter *adapter;	/* adapter for this port */
> +	struct list_head list;		/* list of ports */
> +	struct list_head mychannels;	/* list of channels, on this port*/
> +	spinlock_t ch_list_lock;	/* Spin Lock for channel_list */
> +	struct tdm_port_cfg port_cfg;	/*
> +					 * A structure parameter defining
> +					 * TDM port configuration.
> +					 */
> +};
> +
> +/* tdm RX-TX Channelised Data */
> +struct tdm_ch_data {
> +	struct tdm_bd rx_data_fifo[TDM_CH_RX_BD_RING_SIZE]; /*
> +							     * Rx Port Data BD
> +							     * Ring
> +							     */
> +	struct tdm_bd *rx_in_data;	/*
> +					 * Current Port Rx BD to be filled by
> +					 * de-interleave function
> +					 */
> +	struct tdm_bd *rx_out_data; /* Current Port Rx BD to be read by App */
> +	struct tdm_bd tx_data_fifo[TDM_CH_TX_BD_RING_SIZE]; /*
> +							     * Tx Port Data BD
> +							     * Ring
> +							     */
> +	struct tdm_bd *tx_in_data;	/*
> +					 * Current Port Tx BD to be filled by
> +					 * App
> +					 */
> +	struct tdm_bd *tx_out_data;	/*
> +					 * Current Port Tx BD to be read by
> +					 * interleave function
> +					 */
> +	spinlock_t rx_channel_lock;	/* Spin Lock for Rx Port */
> +	spinlock_t tx_channel_lock;	/* Spin Lock for Tx Port */
> +};
> +
> +/* Channel config params */
> +struct tdm_ch_cfg {
> +	unsigned short num_slots;
> +	unsigned short first_slot;
> +};
> +
> +/* struct tdm_channel- represent a TDM channel for a port */ struct=20
> +tdm_channel {
> +	u16 ch_id;			/* logical channel number */
> +	struct list_head list;		/* list of channels in a port*/
> +	struct tdm_port *port;		/* port for this channel */
> +	u8 in_use;			/* channel is enabled? */
> +	struct tdm_ch_cfg ch_cfg;	/* channel configuration */
> +	struct tdm_ch_data *p_ch_data;	/* data storage space for channel */
> +	wait_queue_head_t ch_wait_queue;/* waitQueue for RX Channel Data */=20
> +};
> +
> +/* tdm_adapt_algorithm is for accessing the routines of device */=20
> +struct tdm_adapt_algorithm {
> +	int (*tdm_read)(struct tdm_adapter *, u16 **);
> +	int (*tdm_get_write_buf)(struct tdm_adapter *, u16 **);
> +	int (*tdm_write)(struct tdm_adapter *, void *, unsigned int len);
> +	int (*tdm_enable)(struct tdm_adapter *);
> +	int (*tdm_disable)(struct tdm_adapter *); };
> +
> +/* tdm_adapter_mode is to define in mode of the device */ enum=20
> +tdm_adapter_mode {
> +	TDM_ADAPTER_MODE_NONE =3D 0x00,
> +	TDM_ADAPTER_MODE_T1 =3D 0x01,
> +	TDM_ADAPTER_MODE_E1 =3D 0x02,
> +	TDM_ADAPTER_MODE_T1_RAW =3D 0x10,
> +	TDM_ADAPTER_MODE_E1_RAW =3D 0x20,
> +};
> +
> +/* tdm_port_mode defines the mode in which the port is configured to=20
> +operate
> + * It can be channelized/full/fractional.
> + */
> +enum tdm_port_mode {
> +	TDM_PORT_CHANNELIZED =3D 0,	/* Channelized mode */
> +	TDM_PORT_FULL =3D 1,		/* Full mode */
> +	TDM_PORT_FRACTIONAL =3D 2		/* Fractional mode */
> +};
> +
> +/* tdm_process_mode used for testing the tdm device in normal mode or=20
> +internal
> + * loopback or external loopback
> + */
> +enum tdm_process_mode {
> +	TDM_PROCESS_NORMAL =3D 0,		/* Normal mode */
> +	TDM_PROCESS_INT_LPB =3D 1,	/* Internal loop mode */
> +	TDM_PROCESS_EXT_LPB =3D 2		/* External Loopback mode */
> +};
> +
> +/* TDM configuration parameters */
> +struct fsl_tdm_adapt_cfg {
> +	u8 num_ch;		/* Number of channels in this adpater */
> +	u8 ch_size_type;	/*
> +				 * reciever/transmit channel
> +				 * size for all channels
> +				 */
> +	u8 slot_width;		/* 1 or 2 Is defined by channel type */
> +	u8 frame_len;		/* Length of frame in samples */
> +	u32 num_frames;
> +	u8 loopback;		/* loopback or normal */
> +	u8 adap_mode;		/*
> +				 * 0=3DNone, 1=3D T1, 2=3D T1-FULL, 3=3DE1,
> +				 * 4 =3D E1-FULL
> +				 */
> +	int max_timeslots;	/*
> +				 * Max Number of timeslots that are
> +				 * supported on this adapter
> +				 */
> +};
> +
> +/*
> + * tdm_adapter is the structure used to identify a physical tdm=20
> +device along
> + * with the access algorithms necessary to access it.
> + */
> +struct tdm_adapter {
> +	struct module *owner;	/* owner of the adapter module */
> +	unsigned int id;	/* Adapter Id */
> +	unsigned int drv_count;	/*
> +				 * Number of drivers associated with the
> +				 * adapter
> +				 */
> +	const struct tdm_adapt_algorithm *algo;	/*
> +						 * algorithm to access the
> +						 * adapter
> +						 */
> +
> +	char name[TDM_NAME_SIZE];	/* Name of Adapter */
> +	struct mutex adap_lock;
> +	struct device *parent;
> +
> +	struct tasklet_struct tdm_data_tasklet;	/*
> +						 * tasklet handle to perform
> +						 * data processing
> +						 */
> +	int tasklet_conf;	/* flag for tasklet configuration */
> +	int tdm_rx_flag;
> +
> +	struct list_head myports;	/*
> +					 * list of ports, created on this
> +					 * adapter
> +					 */
> +	struct list_head list;
> +	spinlock_t portlist_lock;
> +	void *data;
> +	struct fsl_tdm_adapt_cfg adapt_cfg;
> +};
> +
> +struct tdm_sysfs {
> +	struct attribute attr;
> +	int data;
> +	u32 cmd_type;
> +};
> +
> +/* functions exported by tdm.o */
> +
> +int tdm_add_adapter(struct tdm_adapter *adpater); int=20
> +tdm_del_adapter(struct tdm_adapter *adapter); int=20
> +tdm_register_driver(struct tdm_driver *driver); void=20
> +tdm_unregister_driver(struct tdm_driver *driver); void=20
> +init_config_adapter(struct tdm_adapter *adapter);
> +
> +int tdm_port_open(struct tdm_driver *driver, struct tdm_port=20
> +**h_port); int tdm_port_close(struct tdm_port *h_port); int=20
> +tdm_channel_read(struct tdm_port *h_port, struct tdm_channel *h_channel,
> +		void *p_data, u16 *size);
> +int tdm_channel_write(struct tdm_port *h_port, struct tdm_channel *h_cha=
nnel,
> +		void *p_data, u16 size);
> +int tdm_ch_poll(struct tdm_channel *h_channel, unsigned int=20
> +wait_time);
> +
> +int tdm_channel_open(u16 chanid, u16 ch_width, struct tdm_port *port,
> +		struct tdm_channel **h_channel);
> +int tdm_channel_close(u16 chanid, u16 ch_width, struct tdm_port *port,
> +		struct tdm_channel *h_channel);
> +/* this tasklet is created for each adapter instance */ void=20
> +tdm_data_tasklet_fn(unsigned long); int tdm_sysfs_init(void); ssize_t=20
> +tdm_show_sysfs(struct kobject *kobj,
> +		struct attribute *attr, char *buf); ssize_t tdm_store_sysfs(struct=20
> +kobject *kobj,
> +		struct attribute *attr, const char *buf, size_t len);
> +
> +struct tdm_adapter *tdm_get_adapter(int id); void=20
> +tdm_put_adapter(struct tdm_adapter *adap);
> +
> +#endif /* __KERNEL__ */
> +
> +#define TDM_E_OK 0
> --
> 1.5.6.5
>=20
>=20
>=20
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [2/3][PATCH][v2] TDM Framework
  2012-07-30  9:29   ` Singh Sandeep-B37400
@ 2012-07-30 14:10     ` John Stoffel
  2012-07-31  6:40       ` Singh Sandeep-B37400
  0 siblings, 1 reply; 22+ messages in thread
From: John Stoffel @ 2012-07-30 14:10 UTC (permalink / raw)
  To: Singh Sandeep-B37400
  Cc: devel, John Stoffel, linux-kernel, linuxppc-dev, linux-arm-kernel

>>>>> "Singh" == Singh Sandeep-B37400 <B37400@freescale.com> writes:

Singh> -----Original Message-----
Singh> From: John Stoffel [mailto:john@stoffel.org] 
Singh> Sent: 27 July 2012 19:42
Singh> To: Singh Sandeep-B37400
Singh> Cc: linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; galak@kernel.crashing.org; linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org
Singh> Subject: Re: [2/3][PATCH][v2] TDM Framework


>> From: Sandeep Singh <Sandeep@freescale.com> TDM Framework is an 
>> attempt to provide a platform independent layer which can offer a 
>> standard interface  for TDM access to different client modules.

Singh> Please don't use TLAs (Three Letter Acronyms) like TDM without explaining the clearly and up front.  It makes it hard for anyone else who doens't know your code to look it over without having to spend lots of time poking around to figure it out from either context or somewhere else.

Singh> [Sandeep] Patch for documentation for TDM is present in this
Singh> patch set, which explains TDM in detail. Should we do this in
Singh> commit message too??  Link too documentation patch:
Singh> http://patchwork.ozlabs.org/patch/173680/

You should put the expansion of TDM into the initial commit message,
and also into the Kconfig text, so that someone configuring the kernel
has a clue what you're talking about.  

Try to approach this as a brandnew user who doesn't have your
knowledge of the software.  Write for the un-initiated if at all
possible.

John

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-07-27 15:25 ` Francois Romieu
  2012-07-30  9:50   ` Singh Sandeep-B37400
@ 2012-07-30 15:45   ` Mark Brown
  2012-07-31  6:41     ` Singh Sandeep-B37400
  1 sibling, 1 reply; 22+ messages in thread
From: Mark Brown @ 2012-07-30 15:45 UTC (permalink / raw)
  To: Francois Romieu
  Cc: devel, linux-kernel, sandeep, linuxppc-dev, linux-arm-kernel

On Fri, Jul 27, 2012 at 05:25:42PM +0200, Francois Romieu wrote:

> 2. It would probably make sense to Cc: netdev and serial. There may be
>    some kernel client network integration from the start.

Plus audio, quite a few of the buses mentioned as examples of use cases
for the hardware are audio ones.

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-07-30  9:50   ` Singh Sandeep-B37400
@ 2012-07-30 16:01     ` Greg KH
  2012-08-01 12:13       ` Singh Sandeep-B37400
  0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2012-07-30 16:01 UTC (permalink / raw)
  To: Singh Sandeep-B37400
  Cc: devel, linux-kernel, Francois Romieu, linuxppc-dev, linux-arm-kernel

On Mon, Jul 30, 2012 at 09:50:57AM +0000, Singh Sandeep-B37400 wrote:
> 1. You should send some kernel mode TDM clients. Without those the framework
>    is pretty useless.
> [Sandeep] We do have a test client but not good enough to be pushed in
> open source, should we add it to documentation?? 

Then how do you know if the framework is "correct" and good enough for
real clients?  We don't add frameworks, or apis, to the kernel without
users, so you will have to come up with some users before we can accept
it.

greg k-h

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-07-30  9:10   ` Aggrwal Poonam-B10812
@ 2012-07-30 16:04     ` Greg KH
  0 siblings, 0 replies; 22+ messages in thread
From: Greg KH @ 2012-07-30 16:04 UTC (permalink / raw)
  To: Aggrwal Poonam-B10812
  Cc: devel, linuxppc-dev, Singh Sandeep-B37400, linux-arm-kernel,
	linux-kernel

On Mon, Jul 30, 2012 at 09:10:48AM +0000, Aggrwal Poonam-B10812 wrote:
> 
> 
> > -----Original Message-----
> > From: Linuxppc-dev [mailto:linuxppc-dev-
> > bounces+poonam.aggrwal=freescale.com@lists.ozlabs.org] On Behalf Of Greg
> > KH
> > Sent: Friday, July 27, 2012 11:30 PM
> > To: Singh Sandeep-B37400
> > Cc: devel@driverdev.osuosl.org; linuxppc-dev@lists.ozlabs.org; linux-arm-
> > kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > Subject: Re: [2/3][PATCH][v2] TDM Framework
> > 
> > On Fri, Jul 27, 2012 at 07:35:38PM +0530, sandeep@freescale.com wrote:
> > > +/* Data structures required for sysfs */ static struct tdm_sysfs attr
> > > += {
> > > +	.attr.name = "use_latest_data",
> > > +	.attr.mode = 0664,
> > > +	.cmd_type = TDM_LATEST_DATA,
> > > +};
> > 
> > What is this for?
> This knob is to control the behavior of the TDM framework with respect
> to handling the received TDM frames.

How will userspace know how to use this?  Who will use this?

> The framework layer receives the TDM frames from the TDM adapter
> driver, de-interleaves the data and sends to respective client
> modules.

Why would userspace care about this then?

> In the case when the TDM client module has not consumed the data and
> emptied the Buffer, this flag decides whether to discard the
> un-fetched data, or discard the latest received data.

Again, why let userspace make this decision?  How will it know to do
this or not?

Don't add options for things that don't need options.

> > > +int tdm_sysfs_init(void)
> > > +{
> > > +	struct kobject *tdm_kobj;
> > > +	int err = 1;
> > > +	tdm_kobj = kzalloc(sizeof(*tdm_kobj), GFP_KERNEL);
> > > +	if (tdm_kobj) {
> > > +		kobject_init(tdm_kobj, &tdm_type);
> > > +		if (kobject_add(tdm_kobj, NULL, "%s", "tdm")) {
> > > +			pr_err("tdm: Sysfs creation failed\n");
> > > +			kobject_put(tdm_kobj);
> > > +			err = -EINVAL;
> > > +			goto out;
> > > +		}
> > > +	} else {
> > > +		pr_err("tdm: Unable to allocate tdm_kobj\n");
> > > +		err = -ENOMEM;
> > > +		goto out;
> > > +	}
> > > +
> > > +out:
> > > +	return err;
> > > +}
> > 
> > You just leaked memory, what are you trying to do here?
> > 
> > And why are you using "raw" kobjects?  That's a sure sign that something
> > is really wrong.
> Will refer the documentation. Not very experienced on this stuff. Thanks for the comment.
> > 
> > Your code doesn't look like it is tied into the driver model at all, why
> > not?  What are you trying to do here?
> This is a framework layer, not associated to a particular device.

Not true, you have a parent pointer already, so you are hooked up to the
device tree.

> TDM adapter drivers will register to this framework.

Then you had better be part of the kernel driver model.

> We got this comment in internal freescale review list also.

Why did you ignore that feedback and make us ask the same thing?

> > Also, when creating new sysfs entries, like you are attempting to do here
> > (unsuccessfully I might add), you must create Documentation/ABI/ files as
> > well.
> Ok.
> > 
> > And, to top it all off, you do realize you are asking us to do code
> > review in the middle of the merge window, when we are all busy doing
> > other things?
> Apologize for asking a review in the merge window time frame.
> Are there any guidelines when to send something for a review? We will
> be careful next time.

Anytime not in the merge window is usually good, also the week before
the merge window is usually busy as well.

greg k-h

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

* RE: [2/3][PATCH][v2] TDM Framework
  2012-07-30 14:10     ` John Stoffel
@ 2012-07-31  6:40       ` Singh Sandeep-B37400
  0 siblings, 0 replies; 22+ messages in thread
From: Singh Sandeep-B37400 @ 2012-07-31  6:40 UTC (permalink / raw)
  To: John Stoffel; +Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel


-----Original Message-----
From: John Stoffel [mailto:john@stoffel.org]=20
Sent: Monday, July 30, 2012 7:40 PM
To: Singh Sandeep-B37400
Cc: John Stoffel; linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.inf=
radead.org; galak@kernel.crashing.org; linux-kernel@vger.kernel.org; devel@=
driverdev.osuosl.org
Subject: RE: [2/3][PATCH][v2] TDM Framework

>>>>> "Singh" =3D=3D Singh Sandeep-B37400 <B37400@freescale.com> writes:

Singh> -----Original Message-----
Singh> From: John Stoffel [mailto:john@stoffel.org]
Singh> Sent: 27 July 2012 19:42
Singh> To: Singh Sandeep-B37400
Singh> Cc: linuxppc-dev@lists.ozlabs.org;=20
Singh> linux-arm-kernel@lists.infradead.org; galak@kernel.crashing.org;=20
Singh> linux-kernel@vger.kernel.org; devel@driverdev.osuosl.org
Singh> Subject: Re: [2/3][PATCH][v2] TDM Framework


>> From: Sandeep Singh <Sandeep@freescale.com> TDM Framework is an=20
>> attempt to provide a platform independent layer which can offer a=20
>> standard interface  for TDM access to different client modules.

Singh> Please don't use TLAs (Three Letter Acronyms) like TDM without expla=
ining the clearly and up front.  It makes it hard for anyone else who doens=
't know your code to look it over without having to spend lots of time poki=
ng around to figure it out from either context or somewhere else.

Singh> [Sandeep] Patch for documentation for TDM is present in this=20
Singh> patch set, which explains TDM in detail. Should we do this in=20
Singh> commit message too??  Link too documentation patch:
Singh> http://patchwork.ozlabs.org/patch/173680/

You should put the expansion of TDM into the initial commit message, and al=
so into the Kconfig text, so that someone configuring the kernel has a clue=
 what you're talking about. =20
[Sandeep] Thanks for suggestion. Will take care.

Try to approach this as a brandnew user who doesn't have your knowledge of =
the software.  Write for the un-initiated if at all possible.


John

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

* RE: [2/3][PATCH][v2] TDM Framework
  2012-07-30 15:45   ` Mark Brown
@ 2012-07-31  6:41     ` Singh Sandeep-B37400
  0 siblings, 0 replies; 22+ messages in thread
From: Singh Sandeep-B37400 @ 2012-07-31  6:41 UTC (permalink / raw)
  To: Mark Brown, Francois Romieu
  Cc: devel, linuxppc-dev, linux-arm-kernel, linux-kernel

-----Original Message-----
From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]=20
Sent: Monday, July 30, 2012 9:16 PM
To: Francois Romieu
Cc: Singh Sandeep-B37400; devel@driverdev.osuosl.org; linuxppc-dev@lists.oz=
labs.org; galak@kernel.crashing.org; linux-arm-kernel@lists.infradead.org; =
linux-kernel@vger.kernel.org
Subject: Re: [2/3][PATCH][v2] TDM Framework

On Fri, Jul 27, 2012 at 05:25:42PM +0200, Francois Romieu wrote:

> 2. It would probably make sense to Cc: netdev and serial. There may be
>    some kernel client network integration from the start.

Plus audio, quite a few of the buses mentioned as examples of use cases for=
 the hardware are audio ones.
[Sandeep] Ok

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

* RE: [2/3][PATCH][v2] TDM Framework
  2012-07-30 16:01     ` Greg KH
@ 2012-08-01 12:13       ` Singh Sandeep-B37400
  2012-08-01 12:37         ` Greg KH
  0 siblings, 1 reply; 22+ messages in thread
From: Singh Sandeep-B37400 @ 2012-08-01 12:13 UTC (permalink / raw)
  To: Greg KH
  Cc: devel, linux-kernel, Francois Romieu, linuxppc-dev, linux-arm-kernel




> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Monday, July 30, 2012 9:32 PM
> To: Singh Sandeep-B37400
> Cc: Francois Romieu; devel@driverdev.osuosl.org; linuxppc-
> dev@lists.ozlabs.org; galak@kernel.crashing.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [2/3][PATCH][v2] TDM Framework
>=20
> On Mon, Jul 30, 2012 at 09:50:57AM +0000, Singh Sandeep-B37400 wrote:
> > 1. You should send some kernel mode TDM clients. Without those the
> framework
> >    is pretty useless.
> > [Sandeep] We do have a test client but not good enough to be pushed in
> > open source, should we add it to documentation??
>=20
> Then how do you know if the framework is "correct" and good enough for
> real clients?  We don't add frameworks, or apis, to the kernel without
> users, so you will have to come up with some users before we can accept
> it.
We can only say that this framework is available in FSL BSPs and being used=
 by VoIP companies.
But running a complete voice stack itself is beyond the scope of Freescale.
So vendors integrate their solutions with FSL solution.
To test the framework we have a small application in our BSP (this is a ver=
y basic test client) which tests the TDM driver and the SLIC interface from=
 voice  transfer perspective.
We can get this added in the Linux codebase in some test directory. What co=
uld be a good place for this?

Regards
Sandeep

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-08-01 12:13       ` Singh Sandeep-B37400
@ 2012-08-01 12:37         ` Greg KH
  2012-08-21 14:13           ` Mark Brown
  0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2012-08-01 12:37 UTC (permalink / raw)
  To: Singh Sandeep-B37400
  Cc: devel, linux-kernel, Francois Romieu, linuxppc-dev, linux-arm-kernel

On Wed, Aug 01, 2012 at 12:13:19PM +0000, Singh Sandeep-B37400 wrote:
> > On Mon, Jul 30, 2012 at 09:50:57AM +0000, Singh Sandeep-B37400 wrote:
> > > 1. You should send some kernel mode TDM clients. Without those the
> > framework
> > >    is pretty useless.
> > > [Sandeep] We do have a test client but not good enough to be pushed in
> > > open source, should we add it to documentation??
> > 
> > Then how do you know if the framework is "correct" and good enough for
> > real clients?  We don't add frameworks, or apis, to the kernel without
> > users, so you will have to come up with some users before we can accept
> > it.
> We can only say that this framework is available in FSL BSPs and being used by VoIP companies.
> But running a complete voice stack itself is beyond the scope of Freescale.
> So vendors integrate their solutions with FSL solution.
> To test the framework we have a small application in our BSP (this is a very basic test client) which tests the TDM driver and the SLIC interface from voice  transfer perspective.
> We can get this added in the Linux codebase in some test directory. What could be a good place for this?

tools/ is a good place for that.

And sorry, I was thinking you had kernel drivers that attached to this
framework, not userspace programs.  Actually, what is the user/kernel
interface for this framework, I seem to have missed that entirely.  You
will have to document that quite well, and run it by the linux-api
mailing list.

thanks,

greg k-h

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

* Re: [2/3][PATCH][v2] TDM Framework
  2012-08-01 12:37         ` Greg KH
@ 2012-08-21 14:13           ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2012-08-21 14:13 UTC (permalink / raw)
  To: Greg KH
  Cc: devel, linux-kernel, Francois Romieu, linuxppc-dev,
	Singh Sandeep-B37400, linux-arm-kernel

On Wed, Aug 01, 2012 at 05:37:38AM -0700, Greg KH wrote:
> On Wed, Aug 01, 2012 at 12:13:19PM +0000, Singh Sandeep-B37400 wrote:

> > But running a complete voice stack itself is beyond the scope of Freescale.
> > So vendors integrate their solutions with FSL solution.

> And sorry, I was thinking you had kernel drivers that attached to this
> framework, not userspace programs.  Actually, what is the user/kernel
> interface for this framework, I seem to have missed that entirely.  You
> will have to document that quite well, and run it by the linux-api
> mailing list.

This does also sound like we ought to be playing nicer with ALSA rather
than just providing a binary only pipe...

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

end of thread, other threads:[~2012-08-21 14:13 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-27 14:05 [2/3][PATCH][v2] TDM Framework sandeep
2012-07-27 14:05 ` [1/3][PATCH][v2] Adding documentation for TDM sandeep
2012-07-27 14:05   ` [3/3][PATCH][v2] Added TDM device support and Freescale Starlite driver sandeep
2012-07-27 14:11 ` [2/3][PATCH][v2] TDM Framework John Stoffel
2012-07-30  9:29   ` Singh Sandeep-B37400
2012-07-30 14:10     ` John Stoffel
2012-07-31  6:40       ` Singh Sandeep-B37400
2012-07-27 14:51 ` Russell King - ARM Linux
2012-07-30 13:00   ` Singh Sandeep-B37400
2012-07-27 15:25 ` Francois Romieu
2012-07-30  9:50   ` Singh Sandeep-B37400
2012-07-30 16:01     ` Greg KH
2012-08-01 12:13       ` Singh Sandeep-B37400
2012-08-01 12:37         ` Greg KH
2012-08-21 14:13           ` Mark Brown
2012-07-30 15:45   ` Mark Brown
2012-07-31  6:41     ` Singh Sandeep-B37400
2012-07-27 17:59 ` Greg KH
2012-07-30  9:10   ` Aggrwal Poonam-B10812
2012-07-30 16:04     ` Greg KH
2012-07-27 18:12 ` Greg KH
2012-07-30  9:13   ` Aggrwal Poonam-B10812

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).