Resend: Request for review and addition of PTI implementation into kernel

Resend: Request for review and addition of PTI implementation into kernel

[james_p_freyensee]

I apologize if this winds up being a duplicate; I believe yesterday's email 
attempt never made it out completely.

This is a request for review and acceptance into the mainline kernel
for the Intel-Atom implementation of the MIPI P1149.7 compact JTAG
standard for mobile devices.  Complete documentation and background
has been provided in PATCH 02/12, "Kernel documentation for the PTI
feature".  PATCH 01/12 is a kernel fix to allow the PTI driver
to compile modularly; this is a fix Greg KH, Alan Cox, and Arjan
van de Ven and I talked about a while ago.  It's not techincally
related to PTI, but it is it's own separate patch so it can be
applied independent of any possible problems with the PTI submission.

Thanks for your help and time.  Regards,
J Freyensee

[PATCH 01/12] export kernel call get_task_comm()

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This allows drivers who call this function to be compiled modularly.
Otherwise, a driver who is interested in this type of functionality
has to implement their own.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 fs/exec.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index c62efcb..7b209f9 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -998,6 +998,7 @@ char *get_task_comm(char *buf, struct task_struct *tsk)
 	task_unlock(tsk);
 	return buf;
 }
+EXPORT_SYMBOL_GPL(get_task_comm);
 
 void set_task_comm(struct task_struct *tsk, char *buf)
 {
-- 
1.6.6.1

[PATCH 02/12] Kernel documentation for the PTI feature.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This provides Kernel documentation for the PTI
feature for Linux mobile solutions.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 Documentation/pti/pti_intel_mid.txt |   89 +++++++++++++++++++++++++++++++++++
 1 files changed, 89 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/pti/pti_intel_mid.txt

diff --git a/Documentation/pti/pti_intel_mid.txt b/Documentation/pti/pti_intel_mid.txt
new file mode 100644
index 0000000..b9c3f3a
--- /dev/null
+++ b/Documentation/pti/pti_intel_mid.txt
@@ -0,0 +1,89 @@
+The Intel MID PTI project is HW implemented in Intel Atom
+system-on-a-chip designs based on the Parallel Trace
+Interface for MIPI P1149.7 cJTAG standard.  The kernel solution
+for this platform involves the following files:
+
+./include/linux/pti.h
+./include/linux/n_tracesink.h
+./drivers/.../n_tracerouter.c
+./drivers/.../n_tracesink.c
+./drivers/.../pti.c
+
+pti.c is the driver that enables various debugging features
+popular on certain mobile manufacturers.  n_tracerouter.c
+and n_tracesink.c allow extra system information to be
+collected and routed to the pti driver, such as trace
+debugging data from a modem.  Altough n_tracerouter
+and n_tracesink are a part of the complete PTI solution,
+these two line disciplines can work separate from
+pti.c and route any data stream from one /dev/tty node
+to another /dev/tty node via kernel-space.  This provides
+a stable, reliable connection that will not break unless
+the user-space application shuts down (plus avoids
+kernel->user->kernel context switch overheads of routing
+data).
+
+An example debugging usage for this driver system:
+   *Hook /dev/ttyPTI0 to syslogd.  Opening this port will also start
+    a console device to further capture debugging messages to PTI.
+   *Hook /dev/ttyPTI1 to modem debugging data to write to PTI HW.
+    This is where n_tracerouter and n_tracesink are used.
+   *Hook /dev/pti to a user-level debugging application for writing
+    to PTI HW.
+   *Use mipi_* Kernel Driver API in other device drivers for
+    debugging to PTI by first requesting a PTI write address via
+    mipi_request_masterchannel(1).
+
+Example 'privileged' (normal user privileges are not enough)
+user-space code on how to setup the n_tracerouter and n_tracesink
+ldisc drivers (note: n_tracerouter depends on n_tracesink):
+
+/////////// To hook up n_tracerouter and n_tracesink /////////
+
+#include <errno.h>
+#define ONE_TTY "/dev/ttyOne"
+#define TWO_TTY "/dev/ttyTwo"
+
+// needed global to hand onto ldisc connection
+static int g_fd_source = -1;
+static int g_fd_sink  = -1;
+
+// grab LDISC values from loaded ldisc drivers from /proc/tty/ldiscs
+int source_ldisc_num, sink_ldisc_num = -1;
+int retval;
+
+g_fd_source = open(ONE_TTY, O_RDWR); // must be R/W
+g_fd_sink   = open(TWO_TTY, O_RDWR); // must be R/W
+
+if (g_fd_source <= 0) || (g_fd_sink <= 0) {
+   // doubt you'll want to use these exact error lines of code
+   printf("Error on open(). errno: %d\n",errno);
+   return errno;
+}
+
+retval = ioctl(g_fd_sink, TIOCSETD, &sink_ldisc_num);
+if (retval < 0) {
+   printf("Error on ioctl().  errno: %d\n", errno);
+   return errno;
+}
+
+retval = ioctl(g_fd_source, TIOCSETD, &source_ldisc_num);
+if (retval < 0) {
+   printf("Error on ioctl().  errno: %d\n", errno);
+   return errno;
+}
+
+/////////// To disconnect n_tracerouter and n_tracesink ////////
+
+// First make sure data through the ldiscs has stopped.
+
+// Second, disconnect ldiscs.  This provides a
+// little cleaner shutdown on tty stack.
+sink_ldisc_num = 0;
+source_ldisc_num = 0;
+ioctl(g_fd_uart, TIOCSETD, &sink_ldisc_num);
+ioctl(g_fd_gadget, TIOCSETD, &source_ldisc_num);
+
+// Three, program closes connection:
+close(g_fd_uart);
+close(g_fd_gadget);
-- 
1.6.6.1

[PATCH 03/12] Intel PTI implementaiton of 1149.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This driver is the implementation of MIPI P1149.7, compact JTAG
standard for Intel Atom mobile devices.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 drivers/misc/pti.c |  891 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 891 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/pti.c

diff --git a/drivers/misc/pti.c b/drivers/misc/pti.c
new file mode 100644
index 0000000..c88b1e5
--- /dev/null
+++ b/drivers/misc/pti.c
@@ -0,0 +1,891 @@
+/*
+ *  pti.c - PTI driver for cJTAG data extration
+ *
+ *  Copyright (C) Intel 2010
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * 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., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
+ * USA
+ */
+
+
+/*
+ * The PTI (Parallel Trace Interface) driver directs trace data routed from
+ * various parts in the system out through the Intel Penwell PTI port and
+ * out of the mobile device for analysis with a debugging tool
+ * (Lauterbach, Fido). This is part of a solution for the MIPI P1149.7,
+ * compact JTAG, standard.
+ */
+
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/interrupt.h>
+#include <linux/console.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/tty.h>
+#include <linux/tty_driver.h>
+#include <linux/pci.h>
+#include <linux/mutex.h>
+#include <linux/miscdevice.h>
+#include <linux/pti.h>
+
+#define DRIVERNAME		"pti"
+#define PCINAME			"pciPTI"
+#define TTYNAME			"ttyPTI"
+#define CHARNAME		"pti"
+#define PTITTY_MINOR_START	0
+#define PTITTY_MINOR_NUM	2
+#define MAX_APP_IDS		16   /* 128 channel ids / u8 bit size */
+#define MAX_OS_IDS		16   /* 128 channel ids / u8 bit size */
+#define MAX_MODEM_IDS		16   /* 128 channel ids / u8 bit size */
+#define MODEM_BASE_ID		71   /* modem master ID address    */
+#define CONTROL_ID		72   /* control master ID address  */
+#define CONSOLE_ID		73   /* console master ID address  */
+#define OS_BASE_ID		74   /* base OS master ID address  */
+#define APP_BASE_ID		80   /* base App master ID address */
+#define CONTROL_FRAME_LEN	32   /* PTI control frame maximum size */
+#define USER_COPY_SIZE		8192 /* 8Kb buffer for user space copy */
+
+struct pti_tty {
+	struct masterchannel *mc;
+};
+
+struct pti_dev {
+	struct tty_port port;
+	unsigned long pti_addr;
+	unsigned long aperture_base;
+	void __iomem *pti_ioaddr;
+	unsigned long pti_iolen;
+	u8 IA_App[MAX_APP_IDS];
+	u8 IA_OS[MAX_OS_IDS];
+	u8 IA_Modem[MAX_MODEM_IDS];
+};
+
+
+static DEFINE_MUTEX(alloclock);
+
+static struct pci_device_id pci_ids[] __devinitconst = {
+		{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x82B) },
+		{0}
+};
+
+static struct tty_driver *pti_tty_driver;
+
+static struct pti_dev *drv_data;
+
+static unsigned int pti_console_channel;
+static unsigned int pti_control_channel;
+
+#define DTS 0x30		/* offset for last dword of a PTI message */
+
+/**
+ *  pti_write_to_aperture() - THE private write function to PTI HW.
+ *  @mc: The 'aperture'. It's part of a write address that holds
+ *       a master and channel ID.
+ *  @buf: Data being written to the HW that will ultimately be seen
+ *        in a debugging tool (Fido, Lauterbach).
+ *  @len: Size of buffer.
+ *
+ *  Since each aperture is specified by a unique
+ *  master/channel ID, no two processes will be writing
+ *  to the same aperture at the same time so no lock is required. The
+ *  PTI-Output agent will send these out in the order that they arrived, and
+ *  thus, it will intermix these messages. The debug tool can then later
+ *  regroup the appropriate message segments together reconstituting each
+ *  message.
+ */
+static void pti_write_to_aperture(struct masterchannel *mc, u8 *buf, int len)
+{
+	int dwordcnt, final, i;
+	u32 ptiword;
+	u8 *p;
+	u32 __iomem *aperture;
+
+	p = buf;
+
+	/*
+	   calculate the aperture offset from the base using the master and
+	   channel id's.
+	*/
+	aperture = drv_data->pti_ioaddr + (mc->master << 15)
+		+ (mc->channel << 8);
+
+	dwordcnt = len >> 2;
+	final = len - (dwordcnt << 2);		/* final = trailing bytes */
+	if (final == 0 && dwordcnt != 0) {	/* always have a final dword */
+		final += 4;
+		dwordcnt--;
+	}
+
+	for (i = 0; i < dwordcnt; i++) {
+		ptiword = be32_to_cpu(*(u32 *)p);
+		p += 4;
+		iowrite32(ptiword, aperture);
+	}
+
+	aperture += DTS;		/* adding DTS signals that is EOM */
+
+	ptiword = 0;
+	for (i = 0; i < final; i++)
+		ptiword |= *p++ << (24-(8*i));
+
+	iowrite32(ptiword, aperture);
+	return;
+}
+
+/**
+ *  pti_control_frame_built_and_sent() - control frame build and send function.
+ *  @mc: The master / channel structure on which the function built a control
+ *  frame.
+ *
+ *  To be able to post process the PTI contents on host side, a control frame
+ *  is added before sending any PTI content. So the host side knows on
+ *  each PTI frame the name of the thread using a dedicated master / channel.
+ *  This function builds this frame and sends it to a master ID CONTROL_ID.
+ *  The overhead is only 32 bytes since the driver only writes to HW
+ *  in 32 byte chunks.
+ */
+
+static void pti_control_frame_built_and_sent(struct masterchannel *mc)
+{
+	struct masterchannel mccontrol = {.master = CONTROL_ID, .channel = 0};
+	const char *control_format = "%3d %3d %s";
+
+	char comm[sizeof(current->comm) + 1];
+	u8 control_frame[CONTROL_FRAME_LEN];
+
+	if (!in_interrupt())
+		get_task_comm(comm, current);
+	else
+		strcpy(comm, "Interrupt");
+
+	/* Ensure our buffer is zero terminated */
+	comm[sizeof(current->comm)] = 0;
+
+	mccontrol.channel = pti_control_channel;
+	pti_control_channel = (pti_control_channel + 1) & 0x7f;
+
+	snprintf(control_frame, CONTROL_FRAME_LEN, control_format, mc->master,
+		mc->channel, comm);
+	pti_write_to_aperture(&mccontrol, control_frame, strlen(control_frame));
+}
+
+/**
+ *  pti_write_full_frame_to_aperture() - high level function to write to PTI
+ *  @mc: The 'aperture'. It's part of a write address that holds
+ *       a master and channel ID.
+ *  @buf: Data being written to the HW that will ultimately be seen
+ *        in a debugging tool (Fido, Lauterbach).
+ *  @len: Size of buffer.
+ *
+ *  All threads sending data (either console, user space application, ...)
+ *  are calling the high level function to write to PTI meaning that it is
+ *  possible to add a control frame before sending the content.
+ */
+static void pti_write_full_frame_to_aperture(struct masterchannel *mc,
+						const unsigned char *buf,
+						int len)
+{
+	pti_control_frame_built_and_sent(mc);
+	pti_write_to_aperture(mc, (u8 *)buf, len);
+}
+
+
+/**
+ * getID(): Allocate a master and channel ID.
+ *
+ * @IDarray:
+ * @max_IDS: The max amount of available write IDs to use.
+ * @baseID:  The starting SW channel ID, based on the Intel
+ *           PTI arch.
+ *
+ * @return: masterchannel struct containing master, channel ID address,
+ * or 0 for error.
+ *
+ * Each bit in the arrays IA_App and IA_OS correspond to a master and
+ * channel id. The bit is one if the id is taken and 0 if free. For
+ * every master there are 128 channel id's.
+ */
+static struct masterchannel *getID(u8 *IDarray, int max_IDS, int baseID)
+{
+	struct masterchannel *mc;
+	int i, j, mask;
+
+	mc = kmalloc(sizeof(struct masterchannel), GFP_KERNEL);
+	if (mc == NULL)
+		return NULL;
+
+	/* look for a byte with a free bit */
+	for (i = 0; i < max_IDS; i++)
+		if (IDarray[i] != 0xff)
+			break;
+	if (i == max_IDS) {
+		kfree(mc);
+		return NULL;
+	}
+	/* find the bit in the 128 possible channel opportunities */
+	mask = 0x80;
+	for (j = 0; j < 8; j++) {
+		if ((IDarray[i] & mask) == 0)
+			break;
+		mask >>= 1;
+	}
+
+	/* grab it */
+	IDarray[i] |= mask;
+	mc->master  = baseID;
+	mc->channel = ((i & 0xf)<<3) + j;
+	/* write new master Id / channel Id allocation to channel control */
+	pti_control_frame_built_and_sent(mc);
+	return mc;
+}
+
+/*
+	The following three functions:
+	mipi_request_mastercahannel(), mipi_release masterchannel()
+	and mipi_write_data() are an API for other kernel drivers to
+	access PTI.
+*/
+
+/**
+ * mipi_request_masterchannel() - Kernel API function used to allocate
+ *                                a master, channel ID address to write to
+ *                                PTI HW.
+ * @type: 0- request Application  master, channel aperture ID write address.
+ *        1- request OS master, channel aperture ID write address.
+ *        2- request Modem master, channel aperture ID write
+ *           address.
+ *        Other values, error.
+ * @return: masterchannel struct or 0 for error.
+ *
+ */
+struct masterchannel *mipi_request_masterchannel(u8 type)
+{
+	struct masterchannel *mc;
+
+	mutex_lock(&alloclock);
+
+	switch (type) {
+
+	case 0:
+		mc = getID(drv_data->IA_App, MAX_APP_IDS, APP_BASE_ID);
+		break;
+
+	case 1:
+		mc = getID(drv_data->IA_OS, MAX_OS_IDS, OS_BASE_ID);
+		break;
+
+	case 2:
+		mc = getID(drv_data->IA_Modem, MAX_MODEM_IDS, MODEM_BASE_ID);
+		break;
+	default:
+		mc = NULL;
+	}
+
+	mutex_unlock(&alloclock);
+	return mc;
+}
+EXPORT_SYMBOL_GPL(mipi_request_masterchannel);
+
+/**
+ * mipi_release_masterchannel() - Kernel API function used to release
+ *                                a master, channel ID address
+ *                                used to write to PTI HW.
+ * @mc: master, channel apeture ID address to be released.
+ *
+ */
+void mipi_release_masterchannel(struct masterchannel *mc)
+{
+	u8 master, channel, i;
+
+	mutex_lock(&alloclock);
+
+	if (mc) {
+		master = mc->master;
+		channel = mc->channel;
+
+		if (master == APP_BASE_ID) {
+			i = channel >> 3;
+			drv_data->IA_App[i] &=  ~(0x80>>(channel & 0x7));
+		} else if (master == OS_BASE_ID) {
+			i = channel >> 3;
+			drv_data->IA_OS[i] &= ~(0x80>>(channel & 0x7));
+		} else {
+			i = channel >> 3;
+			drv_data->IA_Modem[i] &= ~(0x80>>(channel & 0x7));
+		}
+
+		kfree(mc);
+	}
+
+	mutex_unlock(&alloclock);
+}
+EXPORT_SYMBOL_GPL(mipi_release_masterchannel);
+
+/**
+ * mipi_pti_writedata() - Kernel API function used to write trace
+ *                        debugging data to PTI HW.
+ *
+ * @mc:    Master, channel aperture ID address to write to.
+ *         Null value will return with no write occurring.
+ * @buf:   Trace debuging data to write to the PTI HW.
+ *         Null value will return with no write occurring.
+ * @count: Size of buf. Value of 0 or a negative number will
+ *         retrn with no write occuring.
+ */
+void mipi_pti_writedata(struct masterchannel *mc, u8 *buf, int count)
+{
+	/*
+	   since this function is exported, this is treated like an
+	   API function, thus, all parameters should
+	   be checked for validity.
+	*/
+	if ((mc != NULL) && (buf != NULL) && (count > 0))
+		pti_write_to_aperture(mc, buf, count);
+	return;
+}
+EXPORT_SYMBOL_GPL(mipi_pti_writedata);
+
+static void __devexit pti_pci_remove(struct pci_dev *pdev)
+{
+	struct pti_dev *drv_data;
+
+	drv_data = pci_get_drvdata(pdev);
+	if (drv_data != NULL) {
+		pci_iounmap(pdev, drv_data->pti_ioaddr);
+		pci_set_drvdata(pdev, NULL);
+		kfree(drv_data);
+		pci_release_region(pdev, 1);
+		pci_disable_device(pdev);
+	}
+}
+
+/*
+   for the tty_driver_*() basic function descriptions, see tty_driver.h.
+   Specific header comments made for PTI-related specifics.
+*/
+
+/**
+ * pti_tty_driver_open()- Open an Application master, channel aperture
+ * ID to the PTI device via tty device.
+ *
+ * @param tty: tty interface.
+ * @param filp: filp interface pased to tty_port_open() call.
+ *
+ * @return int : Success = 0, otherwise fail.
+ *
+ * The main purpose of using the tty device interface is for
+ * each tty port to have a unique PTI write aperture.  In an
+ * example use case, ttyPTI0 gets syslogd and an APP aperture
+ * ID and ttyPTI1 is where the n_tracesink ldisc hooks to route
+ * modem messages into PTI.  Modem trace data does not have to
+ * go to ttyPTI1, but ttyPTI0 and ttyPTI1 do need to be distinct
+ * master IDs.  These messages go through the PTI HW and out of
+ * the handheld platform and to the Fido/Lauterbach device.
+ */
+static int pti_tty_driver_open(struct tty_struct *tty, struct file *filp)
+{
+	int ret = 0;
+
+	/*
+	   we actually want to allocate a new channel per open, per
+	   system arch.  HW gives more than plenty channels for a single
+	   system task to have its own channel to write trace data. This
+	   also removes a locking requirement for the actual write
+	   procedure.
+	*/
+	ret = tty_port_open(&drv_data->port, tty, filp);
+
+	return ret;
+}
+
+/**
+ * pti_tty_driver_close()- close tty device and release Application
+ * master, channel aperture ID to the PTI device via tty device.
+ *
+ * @param tty: tty interface.
+ * @param filp: filp interface pased to tty_port_close() call.
+ *
+ * The main purpose of using the tty device interface is to route
+ * syslog daemon messages to the PTI HW and out of the handheld platform
+ * and to the Fido/Lauterbach device.
+ */
+static void pti_tty_driver_close(struct tty_struct *tty, struct file *filp)
+{
+	tty_port_close(&drv_data->port, tty, filp);
+
+	return;
+}
+
+static int pti_tty_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+	int idx = tty->index;
+	struct pti_tty *pti_tty_data;
+	struct masterchannel *mc;
+	int ret = tty_init_termios(tty);
+
+	if (ret == 0) {
+		tty_driver_kref_get(driver);
+		tty->count++;
+		driver->ttys[idx] = tty;
+
+		pti_tty_data = kmalloc(sizeof(struct pti_tty), GFP_KERNEL);
+		if (pti_tty_data == NULL)
+			return -ENOMEM;
+
+		tty->driver_data = pti_tty_data;
+
+		if (idx == PTITTY_MINOR_START)
+			mc = mipi_request_masterchannel(0);
+		else
+			mc = mipi_request_masterchannel(2);
+
+		if (mc == NULL)
+			return -ENXIO;
+
+		pti_tty_data->mc = mc;
+	}
+
+	return ret;
+}
+
+static void pti_tty_cleanup(struct tty_struct *tty)
+{
+	struct pti_tty *pti_tty_data;
+	struct masterchannel *mc;
+
+	pti_tty_data = tty->driver_data;
+
+	if (pti_tty_data != NULL) {
+		mc = pti_tty_data->mc;
+		mipi_release_masterchannel(mc);
+		pti_tty_data->mc = NULL;
+	}
+
+	if (pti_tty_data != NULL)
+		kfree(pti_tty_data);
+
+	tty->driver_data = NULL;
+}
+
+/**
+ * pti_tty_driver_write():  Write trace debugging data through the char
+ * interface to the PTI HW.  Part of the misc device implementation.
+ *
+ * @param filp: Contains private data which is used to obtain
+ *              master, channel write ID.
+ * @param data: trace data to be written.
+ * @param len:  # of byte to write.
+ * @return int : # of bytes written, or error.
+ */
+static int pti_tty_driver_write(struct tty_struct *tty,
+	const unsigned char *buf, int len)
+{
+	struct masterchannel *mc;
+	struct pti_tty *pti_tty_data;
+
+	pti_tty_data = tty->driver_data;
+	mc = pti_tty_data->mc;
+	pti_write_to_aperture(mc, (u8 *)buf, len);
+
+	return len;
+}
+
+static int pti_tty_write_room(struct tty_struct *tty)
+{
+	return 2048;
+}
+
+/**
+ * pti_char_open()- Open an Application master, channel aperture
+ * ID to the PTI device. Part of the misc device implementation.
+ *
+ * @param inode: not used.
+ * @param filp: Output- will have a masterchannel struct set containing
+ * the allocated application PTI aperture write address.
+ *
+ * @return int : Success = 0, otherwise fail.  As of right now,
+ *         it is not sure what needs to really be initialized
+ *         for open(), so it always returns 0.
+ */
+static int pti_char_open(struct inode *inode, struct file *filp)
+{
+	struct masterchannel *mc;
+
+	mc = mipi_request_masterchannel(0);
+	if (mc == NULL)
+		return -ENOMEM;
+	filp->private_data = mc;
+	return 0;
+}
+
+/**
+ * pti_char_release()-  Close a char channel to the PTI device. Part
+ * of the misc device implementation.
+ *
+ * @param inode: Not used in this implementaiton.
+ * @param filp: Contains private_data that contains the master, channel
+ * ID to be released by the PTI device.
+ *
+ * @return int : Success = 0
+ */
+static int pti_char_release(struct inode *inode, struct file *filp)
+{
+	mipi_release_masterchannel(filp->private_data);
+
+	return 0;
+}
+
+/**
+ * pti_char_write():  Write trace debugging data through the char
+ * interface to the PTI HW.  Part of the misc device implementation.
+ *
+ * @param filp: Contains private data which is used to obtain
+ *              master, channel write ID.
+ * @param data: trace data to be written.
+ * @param len:  # of byte to write.
+ * @param ppose: Not used in this function implementation.
+ * @return int : # of bytes written, or error.
+ */
+static ssize_t pti_char_write(struct file *filp, const char __user *data,
+			      size_t len, loff_t *ppose)
+{
+	struct masterchannel *mc;
+	void *kbuf;
+	const char __user *tmp;
+	size_t size = USER_COPY_SIZE, n = 0;
+
+	tmp = data;
+	mc = filp->private_data;
+
+	kbuf = kmalloc(size, GFP_KERNEL);
+	if (kbuf == NULL)  {
+		pr_err("%s(%d): buf allocation failed\n",
+			__func__, __LINE__);
+		return 0;
+	}
+
+	do {
+		if (len - n > USER_COPY_SIZE)
+			size = USER_COPY_SIZE;
+		else
+			size = len - n;
+
+		if (copy_from_user(kbuf, tmp, size)) {
+			kfree(kbuf);
+			return n ? n : -EFAULT;
+		}
+
+		pti_write_to_aperture(mc, kbuf, size);
+		n  += size;
+		tmp += size;
+
+	} while (len > n);
+
+	kfree(kbuf);
+	kbuf = NULL;
+
+	return len;
+}
+
+static const struct tty_operations pti_tty_driver_ops = {
+	.open		= pti_tty_driver_open,
+	.close		= pti_tty_driver_close,
+	.write		= pti_tty_driver_write,
+	.write_room	= pti_tty_write_room,
+	.install	= pti_tty_install,
+	.cleanup	= pti_tty_cleanup
+};
+
+static const struct file_operations pti_char_driver_ops = {
+	.owner		= THIS_MODULE,
+	.write		= pti_char_write,
+	.open		= pti_char_open,
+	.release	= pti_char_release,
+};
+
+static struct miscdevice pti_char_driver = {
+	.minor		= MISC_DYNAMIC_MINOR,
+	.name		= CHARNAME,
+	.fops		= &pti_char_driver_ops
+};
+
+static void pti_console_write(struct console *c, const char *buf, unsigned len)
+{
+	static struct masterchannel mc = {.master = CONSOLE_ID, .channel = 0};
+
+	mc.channel = pti_console_channel;
+	pti_console_channel = (pti_console_channel + 1) & 0x7f;
+
+	pti_write_full_frame_to_aperture(&mc, buf, len);
+}
+
+static struct tty_driver *pti_console_device(struct console *c, int *index)
+{
+	*index = c->index;
+	return pti_tty_driver;
+}
+
+static int pti_console_setup(struct console *c, char *opts)
+{
+	pti_console_channel = 0;
+	pti_control_channel = 0;
+	return 0;
+}
+
+/* pti_console struct, used to capture OS printk()'s and shift
+ * out to the PTI device for debugging.  This cannot be
+ * enabled upon boot because of the possibility of eating
+ * any serial console printk's (race condition discovered).
+ * The console should be enabled upon when the tty port is
+ * used for the first time.  Since the primary purpose for
+ * the tty port is to hook up syslog to it, the tty port
+ * will be open for a really long time.
+ */
+static struct console pti_console = {
+	.name		= TTYNAME,
+	.write		= pti_console_write,
+	.device		= pti_console_device,
+	.setup		= pti_console_setup,
+	.flags		= CON_PRINTBUFFER,
+	.index		= 0,
+};
+
+/**
+ * pti_port_activate(): Used to start/initialize any items upon
+ * first opening of tty_port().
+ *
+ * @param port- The tty port number of the PTI device.
+ * @param tty-  The tty struct associated with this device.
+ *
+ * @return int - Always returns 0.
+ *
+ * Notes: The primary purpose of the PTI tty port 0 is to hook
+ * the syslog daemon to it; thus this port will be open for a
+ * very long time.
+ */
+static int pti_port_activate(struct tty_port *port, struct tty_struct *tty)
+{
+	if (port->tty->index == PTITTY_MINOR_START)
+		console_start(&pti_console);
+	return 0;
+}
+
+/**
+ * pti_port_shutdown(): Used to stop/shutdown any items upon the
+ * last tty port close.
+ *
+ * @param port- The tty port number of the PTI device.
+ *
+ * Notes: The primary purpose of the PTI tty port 0 is to hook
+ * the syslog daemon to it; thus this port will be open for a
+ * very long time.
+ */
+static void pti_port_shutdown(struct tty_port *port)
+{
+	if (port->tty->index == PTITTY_MINOR_START)
+		console_stop(&pti_console);
+}
+
+static const struct tty_port_operations tty_port_ops = {
+	.activate = pti_port_activate,
+	.shutdown = pti_port_shutdown,
+};
+
+/*
+   Note the _probe() call sets everything up and ties the char and tty
+   to successfully detecting the PTI device on the pci bus.
+*/
+
+static int __devinit pti_pci_probe(struct pci_dev *pdev,
+		const struct pci_device_id *ent)
+{
+	int retval = -EINVAL;
+	int pci_bar = 1;
+
+	dev_dbg(&pdev->dev, "%s %s(%d): PTI PCI ID %04x:%04x\n", __FILE__,
+			__func__, __LINE__, pdev->vendor, pdev->device);
+
+	retval = misc_register(&pti_char_driver);
+	if (retval) {
+		pr_err("%s(%d): CHAR registration failed of pti driver\n",
+			__func__, __LINE__);
+		pr_err("%s(%d): Error value returned: %d\n",
+			__func__, __LINE__, retval);
+		return retval;
+	}
+
+	retval = pci_enable_device(pdev);
+	if (retval != 0) {
+		dev_err(&pdev->dev,
+			"%s: pci_enable_device() returned error %d\n",
+			__func__, retval);
+		return retval;
+	}
+
+	drv_data = kzalloc(sizeof(*drv_data), GFP_KERNEL);
+
+	if (drv_data == NULL) {
+		retval = -ENOMEM;
+		dev_err(&pdev->dev,
+			"%s(%d): kmalloc() returned NULL memory.\n",
+			__func__, __LINE__);
+		return retval;
+	}
+	drv_data->pti_addr = pci_resource_start(pdev, pci_bar);
+
+	retval = pci_request_region(pdev, pci_bar, dev_name(&pdev->dev));
+	if (retval != 0) {
+		dev_err(&pdev->dev,
+			"%s(%d): pci_request_region() returned error %d\n",
+			__func__, __LINE__, retval);
+		kfree(drv_data);
+		return retval;
+	}
+	drv_data->pti_iolen = pci_resource_len(pdev, pci_bar);
+	drv_data->aperture_base = drv_data->pti_addr+APERTURE_14;
+	drv_data->pti_ioaddr =
+		ioremap_nocache((u32)drv_data->aperture_base,
+		APERTURE_LEN);
+	if (!drv_data->pti_ioaddr) {
+		pci_release_region(pdev, pci_bar);
+		retval = -ENOMEM;
+		kfree(drv_data);
+		return retval;
+	}
+
+	pci_set_drvdata(pdev, drv_data);
+
+	tty_port_init(&drv_data->port);
+	drv_data->port.ops = &tty_port_ops;
+
+	tty_register_device(pti_tty_driver, 0, &pdev->dev);
+	tty_register_device(pti_tty_driver, 1, &pdev->dev);
+
+	register_console(&pti_console);
+
+	return retval;
+}
+
+static struct pci_driver pti_pci_driver = {
+	.name		= PCINAME,
+	.id_table	= pci_ids,
+	.probe		= pti_pci_probe,
+	.remove		= pti_pci_remove,
+};
+
+/**
+ *
+ * pti_init():
+ *
+ * @return int __init: 0 for success, any other value error.
+ *
+ */
+static int __init pti_init(void)
+{
+	int retval = -EINVAL;
+
+	/* First register module as tty device */
+
+	pti_tty_driver = alloc_tty_driver(1);
+	if (pti_tty_driver == NULL) {
+		pr_err("%s(%d): Memory allocation failed for ptiTTY driver\n",
+			__func__, __LINE__);
+		return -ENOMEM;
+	}
+
+	pti_tty_driver->owner			= THIS_MODULE;
+	pti_tty_driver->magic			= TTY_DRIVER_MAGIC;
+	pti_tty_driver->driver_name		= DRIVERNAME;
+	pti_tty_driver->name			= TTYNAME;
+	pti_tty_driver->major			= 0;
+	pti_tty_driver->minor_start		= PTITTY_MINOR_START;
+	pti_tty_driver->minor_num		= PTITTY_MINOR_NUM;
+	pti_tty_driver->num			= PTITTY_MINOR_NUM;
+	pti_tty_driver->type			= TTY_DRIVER_TYPE_SYSTEM;
+	pti_tty_driver->subtype			= SYSTEM_TYPE_SYSCONS;
+	pti_tty_driver->flags			= TTY_DRIVER_REAL_RAW |
+						  TTY_DRIVER_DYNAMIC_DEV;
+	pti_tty_driver->init_termios		= tty_std_termios;
+
+	tty_set_operations(pti_tty_driver, &pti_tty_driver_ops);
+
+	retval = tty_register_driver(pti_tty_driver);
+	if (retval) {
+		pr_err("%s(%d): TTY registration failed of pti driver\n",
+			__func__, __LINE__);
+		pr_err("%s(%d): Error value returned: %d\n",
+			__func__, __LINE__, retval);
+
+		pti_tty_driver = NULL;
+		return retval;
+	}
+
+	retval = pci_register_driver(&pti_pci_driver);
+
+	if (retval) {
+		pr_err("%s(%d): PCI registration failed of pti driver\n",
+			__func__, __LINE__);
+		pr_err("%s(%d): Error value returned: %d\n",
+			__func__, __LINE__, retval);
+
+		tty_unregister_driver(pti_tty_driver);
+		pr_err("%s(%d): Unregistering TTY part of pti driver\n",
+			__func__, __LINE__);
+		pti_tty_driver = NULL;
+		return retval;
+	}
+
+	return retval;
+}
+
+/**
+ * pti_exit(): Unregisters this module as a tty and pci driver.
+ */
+static void __exit pti_exit(void)
+{
+	int retval;
+
+	tty_unregister_device(pti_tty_driver, 0);
+	tty_unregister_device(pti_tty_driver, 1);
+
+	retval = tty_unregister_driver(pti_tty_driver);
+	if (retval) {
+		pr_err("%s(%d): TTY unregistration failed of pti driver\n",
+			__func__, __LINE__);
+		pr_err("%s(%d): Error value returned: %d\n",
+			__func__, __LINE__, retval);
+	}
+
+	pci_unregister_driver(&pti_pci_driver);
+
+	retval = misc_deregister(&pti_char_driver);
+	if (retval) {
+		pr_err("%s(%d): CHAR unregistration failed of pti driver\n",
+			__func__, __LINE__);
+		pr_err("%s(%d): Error value returned: %d\n",
+			__func__, __LINE__, retval);
+	}
+
+	unregister_console(&pti_console);
+	return;
+}
+
+module_init(pti_init);
+module_exit(pti_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Ken Mills, Jay Freyensee");
+MODULE_DESCRIPTION("PTI Driver");
+
-- 
1.6.6.1

[PATCH 04/12] PTI Kconfig change in misc.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This adds the Intel PTI Kconfig option.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 drivers/misc/Kconfig |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 4d073f1..f8076f1 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -136,6 +136,18 @@ config PHANTOM
 	  If you choose to build module, its name will be phantom. If unsure,
 	  say N here.
 
+config INTEL_MID_PTI
+        tristate "Parallel Trace Interface for MIPI P1149.7 cJTAG standard"
+        help
+          The PTI (Parallel Trace Interface) driver directs
+          trace data routed from various parts in the system out
+          through an Intel Penwell PTI port and out of the mobile
+          device for analysis with a debugging tool (Lauterbach or Fido).
+
+          You should select this driver if the target kernel is meant for
+          an Intel Atom (non-netbook) mobile device containing a MIPI
+          P1149.7 standard implementation.
+
 config SGI_IOC4
 	tristate "SGI IOC4 Base IO support"
 	depends on PCI
-- 
1.6.6.1

[PATCH 05/12] PTI misc Makefile addition.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This allows the Intel implementation of the PTI standard to be compiled.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 drivers/misc/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 98009cc..cae0463 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_IBM_ASM)		+= ibmasm/
 obj-$(CONFIG_AD525X_DPOT)	+= ad525x_dpot.o
 obj-$(CONFIG_AD525X_DPOT_I2C)	+= ad525x_dpot-i2c.o
 obj-$(CONFIG_AD525X_DPOT_SPI)	+= ad525x_dpot-spi.o
+obj-$(CONFIG_INTEL_MID_PTI)     += pti.o
 obj-$(CONFIG_ATMEL_PWM)		+= atmel_pwm.o
 obj-$(CONFIG_ATMEL_SSC)		+= atmel-ssc.o
 obj-$(CONFIG_ATMEL_TCLIB)	+= atmel_tclib.o
-- 
1.6.6.1

[PATCH 06/12] PTI header file.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This adds PTI header information for the PTI project.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 include/linux/pti.h |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/pti.h

diff --git a/include/linux/pti.h b/include/linux/pti.h
new file mode 100644
index 0000000..4b1c9f6
--- /dev/null
+++ b/include/linux/pti.h
@@ -0,0 +1,38 @@
+/*
+ *  Copyright (C) Intel 2010
+ *  Ken Mills <ken.k.mills@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * 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., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
+ * USA
+ *
+ */
+
+#ifndef PTI_H_
+#define PTI_H_
+
+/* basic structure used as a write address to the PTI HW */
+struct masterchannel {
+	u8 master;
+	u8 channel;
+};
+
+/* the following functions are defined in misc/pti.c */
+void mipi_pti_writedata(struct masterchannel *mc, u8 *cp, int count);
+struct masterchannel *mipi_request_masterchannel(u8 kerneluser);
+void mipi_release_masterchannel(struct masterchannel *mc);
+
+#define APERTURE_14 0x3800000
+#define APERTURE_LEN 0x400000
+
+#endif /*PTI_H_*/
-- 
1.6.6.1

[PATCH 07/12] n_tracerouter and n_tracesink additions.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This patch adds line discipline numbers to n_tracerouter and
n_tracesink line disciplines for the Intel-Atom PTI implementation
for mobile devices.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 include/linux/tty.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/tty.h b/include/linux/tty.h
index 54e4eaa..c507f53 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -50,6 +50,8 @@
 #define N_CAIF		20      /* CAIF protocol for talking to modems */
 #define N_GSM0710	21	/* GSM 0710 Mux */
 #define N_TI_WL		22	/* for TI's WL BT, FM, GPS combo chips */
+#define N_TRACESINK	23	/* Trace data routing for MIPI P1149.7 */
+#define N_TRACEROUTER	24	/* Trace data routing for MIPI P1149.7 */
 
 /*
  * This character is the same as _POSIX_VDISABLE: it cannot be used as
-- 
1.6.6.1

[PATCH 08/12] n_tracesink ldisc addition.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This patch adds n_tracesink line discipline driver, which is
part of the Intel-Atom PTI implementation solution.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 drivers/tty/n_tracesink.c |  253 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 253 insertions(+), 0 deletions(-)
 create mode 100644 drivers/tty/n_tracesink.c

diff --git a/drivers/tty/n_tracesink.c b/drivers/tty/n_tracesink.c
new file mode 100644
index 0000000..0d072e3
--- /dev/null
+++ b/drivers/tty/n_tracesink.c
@@ -0,0 +1,253 @@
+/*
+ *  n_tracesink.c - Trace data router and sink path through tty space.
+ *
+ *  Copyright (C) Intel 2010
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  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., 51 Franklin Street, Fifth Floor, Boston, MA
+ *  02110-1301, USA
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * The trace sink uses the Linux line discipline framework to receive
+ * trace data coming from the PTI source line discipline driver
+ * to a user-desired tty port, like USB.
+ * This is to provide a way to extract modem trace data on
+ * devices that do not have a PTI HW module, or just need modem
+ * trace data to come out of a different HW output port.
+ * This is part of a solution for the P1149.7, compact JTAG, standard.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/ioctl.h>
+#include <linux/tty.h>
+#include <linux/tty_ldisc.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <asm-generic/bug.h>
+#include <linux/n_tracesink.h>
+
+/* Other ldisc drivers use 65536 which basically means,
+ * 'I can always accept 64k' and flow control is off.
+ * This number is deemed appropriate for this driver.
+ */
+#define RECEIVE_ROOM	65536
+#define DRIVERNAME	"n_tracesink"
+
+/* there is a quirk with this ldisc is he can write data
+ * to a tty from anyone calling his kernel API, which
+ * meets customer requirements in the drivers/misc/pti.c
+ * project.  So he needs to know when he can and cannot write when
+ * the API is called. In theory, the API can be called
+ * after an init() but before a successful open() which
+ * would crash the system if tty is not checked.
+ */
+static struct tty_struct *this_tty;
+static DEFINE_MUTEX(writelock);
+
+/**
+ * n_tracesink_open() - Called when a tty is opened by a SW entity.
+ * @tty: terminal device to the ldisc.
+ *
+ * Return:
+ *      0 for success,
+ *      -EFAULT = couldn't get a tty kref n_tracesink will sit
+ *       on top of
+ *      -EEXIST = open() called successfully once and it cannot
+ *      be called again.
+ *
+ * Caveats: open() should only be successful the first time a
+ * SW entity calls it.
+ */
+static int n_tracesink_open(struct tty_struct *tty)
+{
+	int retval = -EEXIST;
+
+	mutex_lock(&writelock);
+	if (this_tty == NULL) {
+
+		this_tty = tty_kref_get(tty);
+
+		if (this_tty == NULL)
+			retval = -EFAULT;
+		else {
+			tty->disc_data = this_tty;
+			tty_driver_flush_buffer(tty);
+			retval = 0;
+		}
+	}
+	mutex_unlock(&writelock);
+
+	return retval;
+}
+
+/**
+ * n_tracesink_close() - close connection
+ * @tty: terminal device to the ldisc.
+ *
+ * Called when a software entity wants to close a connection.
+ */
+static void n_tracesink_close(struct tty_struct *tty)
+{
+
+	tty_driver_flush_buffer(tty);
+
+	mutex_lock(&writelock);
+	tty_kref_put(this_tty);
+	this_tty = NULL;
+	mutex_unlock(&writelock);
+
+	tty->disc_data = NULL;
+}
+
+/**
+ * n_tracesink_read() - read request from user space
+ * @tty:  terminal device passed into the ldisc.
+ * @file: pointer to open file object.
+ * @buf:  pointer to the data buffer that gets eventually returned.
+ * @nr:   number of bytes of the data buffer that is returned.
+ *
+ * function that allows read() functionality in userspace. By default if this
+ * is not implemented it returns -EIO. This module is functioning like a
+ * router via n_tracesink_receivebuf(), and there is no real requirement
+ * to implement this function. However, an error return value other than
+ * -EIO should be used just to show that there was an intent not to have
+ * this function implemented.  Return value based on read() man pages.
+ *
+ * Return:
+ *	 -EINVAL
+ */
+static ssize_t n_tracesink_read(struct tty_struct *tty, struct file *file,
+				unsigned char __user *buf, size_t nr) {
+	return -EINVAL;
+}
+
+/**
+ * n_tracesink_write() - Function that allows write() in userspace.
+ * @tty:  terminal device passed into the ldisc.
+ * @file: pointer to open file object.
+ * @buf:  pointer to the data buffer that gets eventually returned.
+ * @nr:   number of bytes of the data buffer that is returned.
+ *
+ * By default if this is not implemented, it returns -EIO.
+ * This should not be implemented, ever, because
+ * 1. this driver is functioning like a router via
+ *    n_tracesink_receivebuf()
+ * 2. No writes to HW will ever go through this line discpline driver.
+ * However, an error return value other than -EIO should be used
+ * just to show that there was an intent not to have this function
+ * implemented.  Return value based on write() man pages.
+ *
+ * Return:
+ *	-EINVAL
+ */
+static ssize_t n_tracesink_write(struct tty_struct *tty, struct file *file,
+				 const unsigned char *buf, size_t nr) {
+	return -EINVAL;
+}
+
+/**
+ *  mipi_pti_sinkdata() - Kernel API function used to route
+ *			trace debugging data to user-defined
+ *			port like USB.
+ *
+ * @buf:   Trace debuging data buffer to write to tty target
+ *         port. Null value will return with no write occurring.
+ * @count: Size of buf. Value of 0 or a negative number will
+ *         return with no write occuring.
+ *
+ * Caveat: If this line discipline does not set the tty it sits
+ * on top of via an open() call, this API function will not
+ * call the tty's write() call because it will have no pointer
+ * to call the write().
+ */
+
+void n_tracesink_datadrain(u8 *cp, int count)
+{
+	mutex_lock(&writelock);
+
+	if ((cp != NULL) && (count > 0) && (this_tty != NULL))
+		this_tty->ops->write(this_tty, cp, count);
+
+	mutex_unlock(&writelock);
+}
+EXPORT_SYMBOL_GPL(n_tracesink_datadrain);
+
+/* Flush buffer is not impelemented as the ldisc has no internal buffering
+ * so the tty_driver_flush_buffer() is sufficient for this driver's needs.
+ */
+
+/*
+ * tty_ldisc function operations for this driver.
+ */
+static struct tty_ldisc_ops tty_n_tracesink = {
+	.owner		= THIS_MODULE,
+	.magic		= TTY_LDISC_MAGIC,
+	.name		= DRIVERNAME,
+	.open		= n_tracesink_open,
+	.close		= n_tracesink_close,
+	.read		= n_tracesink_read,
+	.write		= n_tracesink_write
+};
+
+/**
+ * n_tracesink_init-	module initialisation
+ *
+ * Registers this module as a line discipline driver.
+ *
+ * Return:
+ *	0 for success, any other value error.
+ */
+static int __init n_tracesink_init(void)
+{
+	int retval;
+
+	/* Note N_TRACESINK is defined in linux/tty.h */
+	retval = tty_register_ldisc(N_TRACESINK, &tty_n_tracesink);
+
+	if (retval < 0)
+		pr_err("%s: Registration failed: %d\n",
+					__func__, retval);
+
+	return retval;
+}
+
+/**
+ * n_tracesink_exit -	module unload
+ *
+ * Removes this module as a line discipline driver.
+ */
+static void __exit n_tracesink_exit(void)
+{
+	int retval;
+
+	retval = tty_unregister_ldisc(N_TRACESINK);
+
+	if (retval < 0)
+		pr_err("%s: Unregistration failed: %d\n",
+					__func__,  retval);
+}
+
+module_init(n_tracesink_init);
+module_exit(n_tracesink_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jay Freyensee");
+MODULE_ALIAS_LDISC(N_TRACESINK);
+MODULE_DESCRIPTION("Trace sink ldisc driver");
-- 
1.6.6.1

[PATCH 09/12] n_tracesink header file.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This header file allows the n_tracerouter to send it's information
to the n_tracesink ldisc driver.  It's part of the Intel-Atom
PTI implementation solution.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 include/linux/n_tracesink.h |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/n_tracesink.h

diff --git a/include/linux/n_tracesink.h b/include/linux/n_tracesink.h
new file mode 100644
index 0000000..b1a3ad8
--- /dev/null
+++ b/include/linux/n_tracesink.h
@@ -0,0 +1,32 @@
+/*
+ *  n_tracesink.h - Kernel driver API to route trace data in kernel space.
+ *
+ *  Copyright (C) Intel 2010
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  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., 51 Franklin Street, Fifth Floor, Boston, MA
+ *  02110-1301, USA
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ * The implementation of the function seen in this file can be found in
+ * char/n_tracesink.c
+ */
+
+#ifndef N_TRACESINK_H_
+#define N_TRACESINK_H_
+
+void n_tracesink_datadrain(u8 *cp, int count);
+
+#endif
-- 
1.6.6.1

[PATCH 10/12] n_tracerouter ldisc driver.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This patch adds the n_tracerouter ldisc driver.  It is part
of the Intel-Atom PTI implementation solution.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 drivers/tty/n_tracerouter.c |  264 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 264 insertions(+), 0 deletions(-)
 create mode 100644 drivers/tty/n_tracerouter.c

diff --git a/drivers/tty/n_tracerouter.c b/drivers/tty/n_tracerouter.c
new file mode 100644
index 0000000..1bb8c9e
--- /dev/null
+++ b/drivers/tty/n_tracerouter.c
@@ -0,0 +1,264 @@
+/*
+ *  n_tracerouter.c - Trace data router through tty space
+ *
+ *  Copyright (C) Intel 2010
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  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., 51 Franklin Street, Fifth Floor, Boston, MA
+ *  02110-1301, USA
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This trace router uses the Linux line discipline framework to route
+ * trace data coming from a HW Modem to a PTI (Parallel Trace Module) port.
+ * The solution is not specific to a HW modem and this line disciple can
+ * be used to route any stream of data in kernel space.
+ * This is part of a solution for the P1149.7, compact JTAG, standard.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/ioctl.h>
+#include <linux/tty.h>
+#include <linux/tty_ldisc.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <asm-generic/bug.h>
+#include <linux/n_tracesink.h>
+
+/* Other ldisc drivers use 65536 which basically means,
+ * 'I can always accept 64k' and flow control is off.
+ * This number is deemed appropriate for this driver.
+ */
+
+#define RECEIVE_ROOM	65536
+#define DRIVERNAME	"n_tracerouter"
+
+/* struct to hold private configuration data for this ldisc.
+ * opencalled is used to hold if this ldisc has been opened.
+ * kref_tty holds the tty reference the ldisc sits on top of.
+ */
+struct tracerouter_data {
+	u8 opencalled;
+	struct tty_struct *kref_tty;
+};
+static struct tracerouter_data *tr_data;
+
+/* lock for ioctl() setting of tracerouter_data values */
+static DEFINE_MUTEX(routelock);
+
+/**
+ * tracerouter_alloc
+ *
+ * Allocates the structure needed for this ldisc.
+ */
+static struct tracerouter_data *tracerouter_alloc(void)
+{
+	struct tracerouter_data *tptr = kzalloc(
+					sizeof(struct tracerouter_data),
+					GFP_KERNEL);
+	if (tptr == NULL)
+		return NULL;
+	tptr->opencalled = 0;
+	return tptr;
+}
+
+/**
+ * n_tracerouter_open() - Called when a tty is opened by a SW entity.
+ * @tty: terminal device to the ldisc.
+ *
+ * Return:
+ *      0 for success.
+ *
+ * Caveats: This should only be opened one time per SW entity.
+ */
+static int n_tracerouter_open(struct tty_struct *tty)
+{
+	int retval = -EEXIST;
+
+	mutex_lock(&routelock);
+	if (tr_data->opencalled == 0) {
+
+		tr_data->kref_tty = tty_kref_get(tty);
+		if (tr_data->kref_tty == NULL)
+			retval = -EFAULT;
+		else {
+			tr_data->opencalled = 1;
+			tty->disc_data      = tr_data;
+			tty->receive_room   = RECEIVE_ROOM;
+			tty_driver_flush_buffer(tty);
+			retval = 0;
+		}
+	}
+	mutex_unlock(&routelock);
+	return retval;
+}
+
+/**
+ * n_tracerouter_close() - close connection
+ * @tty: terminal device to the ldisc.
+ *
+ * Called when a software entity wants to close a connection.
+ */
+static void n_tracerouter_close(struct tty_struct *tty)
+{
+	struct tracerouter_data *tptr = tty->disc_data;
+
+	WARN_ON(tptr->kref_tty != tr_data->kref_tty);
+	tty_driver_flush_buffer(tty);
+	mutex_lock(&routelock);
+	tty_kref_put(tr_data->kref_tty);
+	tr_data->kref_tty = NULL;
+	tr_data->opencalled = 0;
+	tty->disc_data = NULL;
+	mutex_unlock(&routelock);
+}
+
+/**
+ * n_tracerouter_read() - read request from user space
+ * @tty:  terminal device passed into the ldisc.
+ * @file: pointer to open file object.
+ * @buf:  pointer to the data buffer that gets eventually returned.
+ * @nr:   number of bytes of the data buffer that is returned.
+ *
+ * function that allows read() functionality in userspace. By default if this
+ * is not implemented it returns -EIO. This module is functioning like a
+ * router via n_tracerouter_receivebuf(), and there is no real requirement
+ * to implement this function. However, an error return value other than
+ * -EIO should be used just to show that there was an intent not to have
+ * this function implemented.  Return value based on read() man pages.
+ *
+ * Return:
+ *	 -EINVAL
+ */
+static ssize_t n_tracerouter_read(struct tty_struct *tty, struct file *file,
+				  unsigned char __user *buf, size_t nr) {
+	return -EINVAL;
+}
+
+/**
+ * n_tracerouter_write() - Function that allows write() in userspace.
+ * @tty:  terminal device passed into the ldisc.
+ * @file: pointer to open file object.
+ * @buf:  pointer to the data buffer that gets eventually returned.
+ * @nr:   number of bytes of the data buffer that is returned.
+ *
+ * By default if this is not implemented, it returns -EIO.
+ * This should not be implemented, ever, because
+ * 1. this driver is functioning like a router via
+ *    n_tracerouter_receivebuf()
+ * 2. No writes to HW will ever go through this line discpline driver.
+ * However, an error return value other than -EIO should be used
+ * just to show that there was an intent not to have this function
+ * implemented.  Return value based on write() man pages.
+ *
+ * Return:
+ *	-EINVAL
+ */
+static ssize_t n_tracerouter_write(struct tty_struct *tty, struct file *file,
+				   const unsigned char *buf, size_t nr) {
+	return -EINVAL;
+}
+
+/**
+ * n_tracerouter_receivebuf() - Routing function for driver.
+ * @tty: terminal device passed into the ldisc.  It's assumed
+ *       tty will never be NULL.
+ * @cp:  buffer, block of characters to be eventually read by
+ *       someone, somewhere (user read() call or some kernel function).
+ * @fp:  flag buffer.
+ * @count: number of characters (aka, bytes) in cp.
+ *
+ * This function takes the input buffer, cp, and passes it to
+ * an external API function for processing.
+ */
+static void n_tracerouter_receivebuf(struct tty_struct *tty,
+					const unsigned char *cp,
+					char *fp, int count)
+{
+	mutex_lock(&routelock);
+	n_tracesink_datadrain((u8 *) cp, count);
+	mutex_unlock(&routelock);
+}
+
+/* Flush buffer is not impelemented as the ldisc has no internal buffering
+ * so the tty_driver_flush_buffer() is sufficient for this driver's needs.
+ */
+
+static struct tty_ldisc_ops tty_ptirouter_ldisc = {
+	.owner		= THIS_MODULE,
+	.magic		= TTY_LDISC_MAGIC,
+	.name		= DRIVERNAME,
+	.open		= n_tracerouter_open,
+	.close		= n_tracerouter_close,
+	.read		= n_tracerouter_read,
+	.write		= n_tracerouter_write,
+	.receive_buf	= n_tracerouter_receivebuf
+};
+
+/**
+ * n_tracerouter_init		-	module initialisation
+ *
+ * Registers this module as a line discipline driver.
+ *
+ * Return:
+ *	0 for success, any other value error.
+ */
+static int __init n_tracerouter_init(void)
+{
+	int retval;
+
+	tr_data = tracerouter_alloc();
+	if (tr_data == NULL)
+		return -ENOMEM;
+
+	/* Note N_TRACEROUTER is defined in linux/tty.h */
+	retval = tty_register_ldisc(N_TRACEROUTER, &tty_ptirouter_ldisc);
+	if (retval < 0) {
+		pr_err("%s: Registration failed: %d\n",
+					__func__, retval);
+		kfree(tr_data);
+	}
+	return retval;
+}
+
+/**
+ * n_tracerouter_exit -	-	module unload
+ *
+ * Removes this module as a line discipline driver.
+ */
+static void __exit n_tracerouter_exit(void)
+{
+	int retval;
+
+	kfree(tr_data);
+	retval = tty_unregister_ldisc(N_TRACEROUTER);
+	if (retval < 0)
+		pr_err("%s: Unregistration failed: %d\n",
+					__func__,  retval);
+}
+
+module_init(n_tracerouter_init);
+module_exit(n_tracerouter_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jay Freyensee");
+MODULE_ALIAS_LDISC(N_TRACEROUTER);
+MODULE_DESCRIPTION("Trace router ldisc driver");
-- 
1.6.6.1

[PATCH 11/12] n_tracerouter and n_tracesink Kconfig.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This patch allows n_tracerouter and n_tracesink ldisc drivers
to be configured in menuconfig.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 drivers/char/Kconfig |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 43d3395..02b9be2 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -289,6 +289,38 @@ config N_GSM
 	  This line discipline provides support for the GSM MUX protocol and
 	  presents the mux as a set of 61 individual tty devices.
 
+config TRACE_ROUTER
+        tristate "Trace data router for MIPI P1149.7 cJTAG standard"
+                depends on TRACE_SINK
+                default Y
+        ---help---
+          The trace router uses the Linux tty line discipline framework to
+          route trace data coming from a tty port (say UART for example) to
+           the trace sink line discipline driver and to another tty port(say USB).
+           This is part of a solution for the MIPI P1149.7, compact JTAG,
+           standard, which is for debugging mobile devices. The PTI driver in
+           drivers/misc/pti.c defines the majority of this MIPI solution.
+
+          You should select this driver if the target kernel is meant for
+          a mobile device containing a modem.  Then you will need to select
+          "Trace data sink for MIPI P1149.7 cJTAG standard" line discipline
+          driver.
+
+config TRACE_SINK
+        tristate "Trace data sink for MIPI P1149.7 cJTAG standard"
+                default Y
+        ---help---
+          The trace sink uses the Linux line discipline framework to receive
+          trace data coming from the trace router line discipline driver
+          to a user-defined tty port target, like USB.
+          This is to provide a way to extract modem trace data on
+          devices that do not have a PTI HW module, or just need modem
+          trace data to come out of a different HW output port.
+          This is part of a solution for the P1149.7, compact JTAG, standard.
+
+          If you select this option, you need to select
+          "Trace data router for MIPI P1149.7 cJTAG standard".
+
 config RISCOM8
 	tristate "SDL RISCom/8 card support"
 	depends on SERIAL_NONSTANDARD
-- 
1.6.6.1

[PATCH 12/12] n_tracerouter and n_tracesink Makefile addition.

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This allows n_tracerouter and n_tracesink to be compiled in the
Linux kernel.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 drivers/tty/Makefile |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
index c43ef48..8c56ffd 100644
--- a/drivers/tty/Makefile
+++ b/drivers/tty/Makefile
@@ -7,5 +7,7 @@ obj-$(CONFIG_MAGIC_SYSRQ)	+= sysrq.o
 obj-$(CONFIG_N_HDLC)		+= n_hdlc.o
 obj-$(CONFIG_N_GSM)		+= n_gsm.o
 obj-$(CONFIG_R3964)		+= n_r3964.o
+obj-$(CONFIG_TRACE_ROUTER)      += n_tracerouter.o
+obj-$(CONFIG_TRACE_SINK)        += n_tracesink.o
 
 obj-y				+= vt/
-- 
1.6.6.1

Re: [PATCH 11/12] n_tracerouter and n_tracesink Kconfig.

[Randy Dunlap]

On 2/8/2011 11:34 AM, james_p_freyensee@linux.intel.com wrote:
> From: J Freyensee<james_p_freyensee@linux.intel.com>
>
> This patch allows n_tracerouter and n_tracesink ldisc drivers
> to be configured in menuconfig.

s/menuconfig/kconfig tools/


> Signed-off-by: J Freyensee<james_p_freyensee@linux.intel.com>
> ---
>   drivers/char/Kconfig |   32 ++++++++++++++++++++++++++++++++
>   1 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> index 43d3395..02b9be2 100644
> --- a/drivers/char/Kconfig
> +++ b/drivers/char/Kconfig
> @@ -289,6 +289,38 @@ config N_GSM
>   	  This line discipline provides support for the GSM MUX protocol and
>   	  presents the mux as a set of 61 individual tty devices.
>
> +config TRACE_ROUTER
> +        tristate "Trace data router for MIPI P1149.7 cJTAG standard"
> +                depends on TRACE_SINK
> +                default Y

Indentation above and on the "default" below looks flaky.
Should line up with "tristate".

> +        ---help---
> +          The trace router uses the Linux tty line discipline framework to
> +          route trace data coming from a tty port (say UART for example) to
> +           the trace sink line discipline driver and to another tty port(say USB).
> +           This is part of a solution for the MIPI P1149.7, compact JTAG,
> +           standard, which is for debugging mobile devices. The PTI driver in
> +           drivers/misc/pti.c defines the majority of this MIPI solution.
> +
> +          You should select this driver if the target kernel is meant for
> +          a mobile device containing a modem.  Then you will need to select
> +          "Trace data sink for MIPI P1149.7 cJTAG standard" line discipline
> +          driver.
> +
> +config TRACE_SINK
> +        tristate "Trace data sink for MIPI P1149.7 cJTAG standard"
> +                default Y
> +        ---help---
> +          The trace sink uses the Linux line discipline framework to receive
> +          trace data coming from the trace router line discipline driver
> +          to a user-defined tty port target, like USB.
> +          This is to provide a way to extract modem trace data on
> +          devices that do not have a PTI HW module, or just need modem
> +          trace data to come out of a different HW output port.
> +          This is part of a solution for the P1149.7, compact JTAG, standard.
> +
> +          If you select this option, you need to select
> +          "Trace data router for MIPI P1149.7 cJTAG standard".
> +
>   config RISCOM8
>   	tristate "SDL RISCom/8 card support"
>   	depends on SERIAL_NONSTANDARD

Re: [PATCH 09/12] n_tracesink header file.

[Greg KH]

On Tue, Feb 08, 2011 at 11:34:54AM -0800, james_p_freyensee@linux.intel.com wrote:
> From: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> This header file allows the n_tracerouter to send it's information
> to the n_tracesink ldisc driver.  It's part of the Intel-Atom
> PTI implementation solution.
> 
> Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> ---
>  include/linux/n_tracesink.h |   32 ++++++++++++++++++++++++++++++++

Why is this in include/linux/ ?

thanks,

greg k-h

Re: [PATCH 09/12] n_tracesink header file.

[J Freyensee]

On Thu, 2011-02-17 at 11:23 -0800, Greg KH wrote:
> On Tue, Feb 08, 2011 at 11:34:54AM -0800, james_p_freyensee@linux.intel.com wrote:
> > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > 
> > This header file allows the n_tracerouter to send it's information
> > to the n_tracesink ldisc driver.  It's part of the Intel-Atom
> > PTI implementation solution.
> > 
> > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > ---
> >  include/linux/n_tracesink.h |   32 ++++++++++++++++++++++++++++++++
> 
> Why is this in include/linux/ ?
> 

I thought this was the best place to stick this .h file.  I'd be happy
to change locations based off of your suggestion.

Thanks,
jay

> thanks,
> 
> greg k-h

Re: [PATCH 09/12] n_tracesink header file.

[Greg KH]

On Thu, Feb 17, 2011 at 11:45:15AM -0800, J Freyensee wrote:
> On Thu, 2011-02-17 at 11:23 -0800, Greg KH wrote:
> > On Tue, Feb 08, 2011 at 11:34:54AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > 
> > > This header file allows the n_tracerouter to send it's information
> > > to the n_tracesink ldisc driver.  It's part of the Intel-Atom
> > > PTI implementation solution.
> > > 
> > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > ---
> > >  include/linux/n_tracesink.h |   32 ++++++++++++++++++++++++++++++++
> > 
> > Why is this in include/linux/ ?
> > 
> 
> I thought this was the best place to stick this .h file.  I'd be happy
> to change locations based off of your suggestion.

Why do you need a .h file at all?  Who is using it?

thanks,

greg k-h

Re: [PATCH 09/12] n_tracesink header file.

[J Freyensee]

On Thu, 2011-02-17 at 11:54 -0800, Greg KH wrote:
> On Thu, Feb 17, 2011 at 11:45:15AM -0800, J Freyensee wrote:
> > On Thu, 2011-02-17 at 11:23 -0800, Greg KH wrote:
> > > On Tue, Feb 08, 2011 at 11:34:54AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > 
> > > > This header file allows the n_tracerouter to send it's information
> > > > to the n_tracesink ldisc driver.  It's part of the Intel-Atom
> > > > PTI implementation solution.
> > > > 
> > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > ---
> > > >  include/linux/n_tracesink.h |   32 ++++++++++++++++++++++++++++++++
> > > 
> > > Why is this in include/linux/ ?
> > > 
> > 
> > I thought this was the best place to stick this .h file.  I'd be happy
> > to change locations based off of your suggestion.
> 
> Why do you need a .h file at all?  Who is using it?

The only current module that really uses it is n_tracerouter; however, I
thought sticking this in a .h file made things slightly more modular and
more re-usable, especially for future device driver writers that could
want something like this.

If you would like me to remove this though, I can, just let me know.
Thanks.

> 
> thanks,
> 
> greg k-h

Re: [PATCH 09/12] n_tracesink header file.

[Greg KH]

On Thu, Feb 17, 2011 at 12:10:47PM -0800, J Freyensee wrote:
> On Thu, 2011-02-17 at 11:54 -0800, Greg KH wrote:
> > On Thu, Feb 17, 2011 at 11:45:15AM -0800, J Freyensee wrote:
> > > On Thu, 2011-02-17 at 11:23 -0800, Greg KH wrote:
> > > > On Tue, Feb 08, 2011 at 11:34:54AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > 
> > > > > This header file allows the n_tracerouter to send it's information
> > > > > to the n_tracesink ldisc driver.  It's part of the Intel-Atom
> > > > > PTI implementation solution.
> > > > > 
> > > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > ---
> > > > >  include/linux/n_tracesink.h |   32 ++++++++++++++++++++++++++++++++
> > > > 
> > > > Why is this in include/linux/ ?
> > > > 
> > > 
> > > I thought this was the best place to stick this .h file.  I'd be happy
> > > to change locations based off of your suggestion.
> > 
> > Why do you need a .h file at all?  Who is using it?
> 
> The only current module that really uses it is n_tracerouter; however, I
> thought sticking this in a .h file made things slightly more modular and
> more re-usable, especially for future device driver writers that could
> want something like this.
> 
> If you would like me to remove this though, I can, just let me know.

Yes, please remove it.

thanks,

greg k-h

Re: [PATCH 09/12] n_tracesink header file.

[J Freyensee]

On Thu, 2011-02-17 at 13:43 -0800, Greg KH wrote:
> On Thu, Feb 17, 2011 at 12:10:47PM -0800, J Freyensee wrote:
> > On Thu, 2011-02-17 at 11:54 -0800, Greg KH wrote:
> > > On Thu, Feb 17, 2011 at 11:45:15AM -0800, J Freyensee wrote:
> > > > On Thu, 2011-02-17 at 11:23 -0800, Greg KH wrote:
> > > > > On Tue, Feb 08, 2011 at 11:34:54AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > > 
> > > > > > This header file allows the n_tracerouter to send it's information
> > > > > > to the n_tracesink ldisc driver.  It's part of the Intel-Atom
> > > > > > PTI implementation solution.
> > > > > > 
> > > > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > > ---
> > > > > >  include/linux/n_tracesink.h |   32 ++++++++++++++++++++++++++++++++
> > > > > 
> > > > > Why is this in include/linux/ ?
> > > > > 
> > > > 
> > > > I thought this was the best place to stick this .h file.  I'd be happy
> > > > to change locations based off of your suggestion.
> > > 
> > > Why do you need a .h file at all?  Who is using it?
> > 
> > The only current module that really uses it is n_tracerouter; however, I
> > thought sticking this in a .h file made things slightly more modular and
> > more re-usable, especially for future device driver writers that could
> > want something like this.
> > 
> > If you would like me to remove this though, I can, just let me know.
> 
> Yes, please remove it.
> 
Greg,

If I remove this file I need to add the function header
n_tracesink_datadrain() that is in this file to n_tracerouter.c.

If I add that function header to n_tracerouter.c then checkpatch.pl
complains that extern should be avoided in .c files.

If I don't have this function header in n_tracerouter.c then it won't
compile at all.

Do you have an alternative suggestion on what I should do here?  It
appears the best solution is for me to keep this .h file considering the
constraints.

Thanks,
Jay

> thanks,
> 
> greg k-h

Re: [PATCH 09/12] n_tracesink header file.

[Greg KH]

On Tue, Feb 22, 2011 at 12:22:58PM -0800, J Freyensee wrote:
> On Thu, 2011-02-17 at 13:43 -0800, Greg KH wrote:
> > On Thu, Feb 17, 2011 at 12:10:47PM -0800, J Freyensee wrote:
> > > On Thu, 2011-02-17 at 11:54 -0800, Greg KH wrote:
> > > > On Thu, Feb 17, 2011 at 11:45:15AM -0800, J Freyensee wrote:
> > > > > On Thu, 2011-02-17 at 11:23 -0800, Greg KH wrote:
> > > > > > On Tue, Feb 08, 2011 at 11:34:54AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > > > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > > > 
> > > > > > > This header file allows the n_tracerouter to send it's information
> > > > > > > to the n_tracesink ldisc driver.  It's part of the Intel-Atom
> > > > > > > PTI implementation solution.
> > > > > > > 
> > > > > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > > > ---
> > > > > > >  include/linux/n_tracesink.h |   32 ++++++++++++++++++++++++++++++++
> > > > > > 
> > > > > > Why is this in include/linux/ ?
> > > > > > 
> > > > > 
> > > > > I thought this was the best place to stick this .h file.  I'd be happy
> > > > > to change locations based off of your suggestion.
> > > > 
> > > > Why do you need a .h file at all?  Who is using it?
> > > 
> > > The only current module that really uses it is n_tracerouter; however, I
> > > thought sticking this in a .h file made things slightly more modular and
> > > more re-usable, especially for future device driver writers that could
> > > want something like this.
> > > 
> > > If you would like me to remove this though, I can, just let me know.
> > 
> > Yes, please remove it.
> > 
> Greg,
> 
> If I remove this file I need to add the function header
> n_tracesink_datadrain() that is in this file to n_tracerouter.c.
> 
> If I add that function header to n_tracerouter.c then checkpatch.pl
> complains that extern should be avoided in .c files.
> 
> If I don't have this function header in n_tracerouter.c then it won't
> compile at all.
> 
> Do you have an alternative suggestion on what I should do here?  It
> appears the best solution is for me to keep this .h file considering the
> constraints.

Make it a "local" .h file, in the same directory as these two .c files,
no need to make it global for the whole of the kernel tree, right?

thanks,

greg k-h

Re: [PATCH 09/12] n_tracesink header file.

[J Freyensee]

On Tue, 2011-02-22 at 12:26 -0800, Greg KH wrote:
> On Tue, Feb 22, 2011 at 12:22:58PM -0800, J Freyensee wrote:
> > On Thu, 2011-02-17 at 13:43 -0800, Greg KH wrote:
> > > On Thu, Feb 17, 2011 at 12:10:47PM -0800, J Freyensee wrote:
> > > > On Thu, 2011-02-17 at 11:54 -0800, Greg KH wrote:
> > > > > On Thu, Feb 17, 2011 at 11:45:15AM -0800, J Freyensee wrote:
> > > > > > On Thu, 2011-02-17 at 11:23 -0800, Greg KH wrote:
> > > > > > > On Tue, Feb 08, 2011 at 11:34:54AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > > > > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > > > > 
> > > > > > > > This header file allows the n_tracerouter to send it's information
> > > > > > > > to the n_tracesink ldisc driver.  It's part of the Intel-Atom
> > > > > > > > PTI implementation solution.
> > > > > > > > 
> > > > > > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > > > > ---
> > > > > > > >  include/linux/n_tracesink.h |   32 ++++++++++++++++++++++++++++++++
> > > > > > > 
> > > > > > > Why is this in include/linux/ ?
> > > > > > > 
> > > > > > 
> > > > > > I thought this was the best place to stick this .h file.  I'd be happy
> > > > > > to change locations based off of your suggestion.
> > > > > 
> > > > > Why do you need a .h file at all?  Who is using it?
> > > > 
> > > > The only current module that really uses it is n_tracerouter; however, I
> > > > thought sticking this in a .h file made things slightly more modular and
> > > > more re-usable, especially for future device driver writers that could
> > > > want something like this.
> > > > 
> > > > If you would like me to remove this though, I can, just let me know.
> > > 
> > > Yes, please remove it.
> > > 
> > Greg,
> > 
> > If I remove this file I need to add the function header
> > n_tracesink_datadrain() that is in this file to n_tracerouter.c.
> > 
> > If I add that function header to n_tracerouter.c then checkpatch.pl
> > complains that extern should be avoided in .c files.
> > 
> > If I don't have this function header in n_tracerouter.c then it won't
> > compile at all.
> > 
> > Do you have an alternative suggestion on what I should do here?  It
> > appears the best solution is for me to keep this .h file considering the
> > constraints.
> 
> Make it a "local" .h file, in the same directory as these two .c files,
> no need to make it global for the whole of the kernel tree, right?
> 

yes, you are right, I don't think it needs to be global for the whole
tree since the two .c files are in the same place and they are the only
ones that care about it.

Jay

> thanks,
> 
> greg k-h

Re: [PATCH 07/12] n_tracerouter and n_tracesink additions.

[Greg KH]

On Tue, Feb 08, 2011 at 11:34:52AM -0800, james_p_freyensee@linux.intel.com wrote:
> From: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> This patch adds line discipline numbers to n_tracerouter and
> n_tracesink line disciplines for the Intel-Atom PTI implementation
> for mobile devices.
> 
> Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> ---
>  include/linux/tty.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index 54e4eaa..c507f53 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -50,6 +50,8 @@
>  #define N_CAIF		20      /* CAIF protocol for talking to modems */
>  #define N_GSM0710	21	/* GSM 0710 Mux */
>  #define N_TI_WL		22	/* for TI's WL BT, FM, GPS combo chips */
> +#define N_TRACESINK	23	/* Trace data routing for MIPI P1149.7 */
> +#define N_TRACEROUTER	24	/* Trace data routing for MIPI P1149.7 */

Two new ones?  Really?  Alan, no objection from you on this?

thanks,

greg k-h

Re: [PATCH 07/12] n_tracerouter and n_tracesink additions.

[J Freyensee]

On Thu, 2011-02-17 at 11:21 -0800, Greg KH wrote:
> On Tue, Feb 08, 2011 at 11:34:52AM -0800, james_p_freyensee@linux.intel.com wrote:
> > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > 
> > This patch adds line discipline numbers to n_tracerouter and
> > n_tracesink line disciplines for the Intel-Atom PTI implementation
> > for mobile devices.
> > 
> > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > ---
> >  include/linux/tty.h |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/linux/tty.h b/include/linux/tty.h
> > index 54e4eaa..c507f53 100644
> > --- a/include/linux/tty.h
> > +++ b/include/linux/tty.h
> > @@ -50,6 +50,8 @@
> >  #define N_CAIF		20      /* CAIF protocol for talking to modems */
> >  #define N_GSM0710	21	/* GSM 0710 Mux */
> >  #define N_TI_WL		22	/* for TI's WL BT, FM, GPS combo chips */
> > +#define N_TRACESINK	23	/* Trace data routing for MIPI P1149.7 */
> > +#define N_TRACEROUTER	24	/* Trace data routing for MIPI P1149.7 */
> 
> Two new ones?  Really?  Alan, no objection from you on this?

I'll let Alan chime in, but I did have Alan review this code before I
submitted here.

Jay

> thanks,
> 
> greg k-h

Re: [PATCH 07/12] n_tracerouter and n_tracesink additions.

[Greg KH]

On Thu, Feb 17, 2011 at 11:43:26AM -0800, J Freyensee wrote:
> On Thu, 2011-02-17 at 11:21 -0800, Greg KH wrote:
> > On Tue, Feb 08, 2011 at 11:34:52AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > 
> > > This patch adds line discipline numbers to n_tracerouter and
> > > n_tracesink line disciplines for the Intel-Atom PTI implementation
> > > for mobile devices.
> > > 
> > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > ---
> > >  include/linux/tty.h |    2 ++
> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/include/linux/tty.h b/include/linux/tty.h
> > > index 54e4eaa..c507f53 100644
> > > --- a/include/linux/tty.h
> > > +++ b/include/linux/tty.h
> > > @@ -50,6 +50,8 @@
> > >  #define N_CAIF		20      /* CAIF protocol for talking to modems */
> > >  #define N_GSM0710	21	/* GSM 0710 Mux */
> > >  #define N_TI_WL		22	/* for TI's WL BT, FM, GPS combo chips */
> > > +#define N_TRACESINK	23	/* Trace data routing for MIPI P1149.7 */
> > > +#define N_TRACEROUTER	24	/* Trace data routing for MIPI P1149.7 */
> > 
> > Two new ones?  Really?  Alan, no objection from you on this?
> 
> I'll let Alan chime in, but I did have Alan review this code before I
> submitted here.

Hm, I don't see a "Reviewed-by:" line on the patch...

thanks,

greg k-h

Re: [PATCH 07/12] n_tracerouter and n_tracesink additions.

[J Freyensee]

On Thu, 2011-02-17 at 11:54 -0800, Greg KH wrote:
> On Thu, Feb 17, 2011 at 11:43:26AM -0800, J Freyensee wrote:
> > On Thu, 2011-02-17 at 11:21 -0800, Greg KH wrote:
> > > On Tue, Feb 08, 2011 at 11:34:52AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > 
> > > > This patch adds line discipline numbers to n_tracerouter and
> > > > n_tracesink line disciplines for the Intel-Atom PTI implementation
> > > > for mobile devices.
> > > > 
> > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > ---
> > > >  include/linux/tty.h |    2 ++
> > > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/include/linux/tty.h b/include/linux/tty.h
> > > > index 54e4eaa..c507f53 100644
> > > > --- a/include/linux/tty.h
> > > > +++ b/include/linux/tty.h
> > > > @@ -50,6 +50,8 @@
> > > >  #define N_CAIF		20      /* CAIF protocol for talking to modems */
> > > >  #define N_GSM0710	21	/* GSM 0710 Mux */
> > > >  #define N_TI_WL		22	/* for TI's WL BT, FM, GPS combo chips */
> > > > +#define N_TRACESINK	23	/* Trace data routing for MIPI P1149.7 */
> > > > +#define N_TRACEROUTER	24	/* Trace data routing for MIPI P1149.7 */
> > > 
> > > Two new ones?  Really?  Alan, no objection from you on this?
> > 
> > I'll let Alan chime in, but I did have Alan review this code before I
> > submitted here.
> 
> Hm, I don't see a "Reviewed-by:" line on the patch...

Good point...I can add this to all patches as Alan reviewed all this
code before-hand.

> 
> thanks,
> 
> greg k-h

Re: [PATCH 07/12] n_tracerouter and n_tracesink additions.

[Alan Cox]

> > > +#define N_TRACESINK	23	/* Trace data routing for MIPI P1149.7 */
> > > +#define N_TRACEROUTER	24	/* Trace data routing for MIPI P1149.7 */
> > 
> > Two new ones?  Really?  Alan, no objection from you on this?
> 
> I'll let Alan chime in, but I did have Alan review this code before I
> submitted here.

There isn't really an elegant way of avoiding that. What probably does
want doing for a final merge is to make them more generic. So that
anything can be a source or a sink that meets a basic interface.

Then you'd be able to do

	set ldisc to tracesink
	ioctl (something or other) 		"attach to sink of
						 name 'modem/mipi/p1149.7'

or similar.

That would need a struct of some sort and a register_sink/unregister_sink
method. Ditto for tracerouter. Obviously for now only the current drivers
would support it but the interface is then there for anything else

Alan

Re: [PATCH 06/12] PTI header file.

[Greg KH]

On Tue, Feb 08, 2011 at 11:34:51AM -0800, james_p_freyensee@linux.intel.com wrote:
> From: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> This adds PTI header information for the PTI project.
> 
> Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> ---
>  include/linux/pti.h |   38 ++++++++++++++++++++++++++++++++++++++

Why does this have to be in include/linux/?  What other files use it?

>  1 files changed, 38 insertions(+), 0 deletions(-)
>  create mode 100644 include/linux/pti.h
> 
> diff --git a/include/linux/pti.h b/include/linux/pti.h
> new file mode 100644
> index 0000000..4b1c9f6
> --- /dev/null
> +++ b/include/linux/pti.h
> @@ -0,0 +1,38 @@
> +/*
> + *  Copyright (C) Intel 2010
> + *  Ken Mills <ken.k.mills@intel.com>

That's nice, where is Ken's signed-off-by?  We need that please.

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * 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., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
> + * USA

This paragraph is not needed, please remove it unless you are willing to
keep up with the FSF's office moves for the next 40+ years.

Same goes for this paragraph on other files in this patchset, please
remove it.

> + *
> + */
> +
> +#ifndef PTI_H_
> +#define PTI_H_
> +
> +/* basic structure used as a write address to the PTI HW */
> +struct masterchannel {

Pretty generic structure name, please prefix it with "pti_"

> +	u8 master;
> +	u8 channel;
> +};
> +
> +/* the following functions are defined in misc/pti.c */
> +void mipi_pti_writedata(struct masterchannel *mc, u8 *cp, int count);

"cp"?

> +struct masterchannel *mipi_request_masterchannel(u8 kerneluser);

kerneluser?

> +void mipi_release_masterchannel(struct masterchannel *mc);

"Mipi"?  What's that?

> +
> +#define APERTURE_14 0x3800000
> +#define APERTURE_LEN 0x400000

What are these for?

> +
> +#endif /*PTI_H_*/
> -- 
> 1.6.6.1

Re: [PATCH 06/12] PTI header file.

[J Freyensee]

On Thu, 2011-02-17 at 11:20 -0800, Greg KH wrote:
> On Tue, Feb 08, 2011 at 11:34:51AM -0800, james_p_freyensee@linux.intel.com wrote:
> > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > 
> > This adds PTI header information for the PTI project.
> > 
> > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > ---
> >  include/linux/pti.h |   38 ++++++++++++++++++++++++++++++++++++++
> 
> Why does this have to be in include/linux/?  What other files use it?
> 
> >  1 files changed, 38 insertions(+), 0 deletions(-)
> >  create mode 100644 include/linux/pti.h
> > 
> > diff --git a/include/linux/pti.h b/include/linux/pti.h
> > new file mode 100644
> > index 0000000..4b1c9f6
> > --- /dev/null
> > +++ b/include/linux/pti.h
> > @@ -0,0 +1,38 @@
> > +/*
> > + *  Copyright (C) Intel 2010
> > + *  Ken Mills <ken.k.mills@intel.com>
> 
> That's nice, where is Ken's signed-off-by?  We need that please.
> 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * 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., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
> > + * USA
> 
> This paragraph is not needed, please remove it unless you are willing to
> keep up with the FSF's office moves for the next 40+ years.
> 
> Same goes for this paragraph on other files in this patchset, please
> remove it.
> 
> > + *
> > + */
> > +
> > +#ifndef PTI_H_
> > +#define PTI_H_
> > +
> > +/* basic structure used as a write address to the PTI HW */
> > +struct masterchannel {
> 
> Pretty generic structure name, please prefix it with "pti_"
> 

no problem, I'll fix all these.

> > +	u8 master;
> > +	u8 channel;
> > +};
> > +
> > +/* the following functions are defined in misc/pti.c */
> > +void mipi_pti_writedata(struct masterchannel *mc, u8 *cp, int count);
> 
> "cp"?
> 

All of the questions concerning these variables are described in pti.c.
Should have the function header comments been copied/moved here??

> > +struct masterchannel *mipi_request_masterchannel(u8 kerneluser);
> 
> kerneluser?
> 
> > +void mipi_release_masterchannel(struct masterchannel *mc);
> 
> "Mipi"?  What's that?

'mipi' is the standard body for the PTI standard.  It's an acronymn that
does not stand for anything other than the Organization controlling the
standard: http://www.mipi.org/about-mipi/frequently-asked-questions#q2

> 
> > +
> > +#define APERTURE_14 0x3800000
> > +#define APERTURE_LEN 0x400000

> What are these for?
> 

Write address and write length for the PTI but I can add that.

Thanks,
jay

> > +
> > +#endif /*PTI_H_*/
> > -- 
> > 1.6.6.1

Re: [PATCH 06/12] PTI header file.

[Greg KH]

On Thu, Feb 17, 2011 at 11:41:55AM -0800, J Freyensee wrote:
> On Thu, 2011-02-17 at 11:20 -0800, Greg KH wrote:
> > On Tue, Feb 08, 2011 at 11:34:51AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > 
> > > This adds PTI header information for the PTI project.
> > > 
> > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > ---
> > >  include/linux/pti.h |   38 ++++++++++++++++++++++++++++++++++++++
> > 
> > Why does this have to be in include/linux/?  What other files use it?
> > 
> > >  1 files changed, 38 insertions(+), 0 deletions(-)
> > >  create mode 100644 include/linux/pti.h
> > > 
> > > diff --git a/include/linux/pti.h b/include/linux/pti.h
> > > new file mode 100644
> > > index 0000000..4b1c9f6
> > > --- /dev/null
> > > +++ b/include/linux/pti.h
> > > @@ -0,0 +1,38 @@
> > > +/*
> > > + *  Copyright (C) Intel 2010
> > > + *  Ken Mills <ken.k.mills@intel.com>
> > 
> > That's nice, where is Ken's signed-off-by?  We need that please.
> > 
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + *
> > > + * 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., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
> > > + * USA
> > 
> > This paragraph is not needed, please remove it unless you are willing to
> > keep up with the FSF's office moves for the next 40+ years.
> > 
> > Same goes for this paragraph on other files in this patchset, please
> > remove it.
> > 
> > > + *
> > > + */
> > > +
> > > +#ifndef PTI_H_
> > > +#define PTI_H_
> > > +
> > > +/* basic structure used as a write address to the PTI HW */
> > > +struct masterchannel {
> > 
> > Pretty generic structure name, please prefix it with "pti_"
> > 
> 
> no problem, I'll fix all these.
> 
> > > +	u8 master;
> > > +	u8 channel;
> > > +};
> > > +
> > > +/* the following functions are defined in misc/pti.c */
> > > +void mipi_pti_writedata(struct masterchannel *mc, u8 *cp, int count);
> > 
> > "cp"?
> > 
> 
> All of the questions concerning these variables are described in pti.c.
> Should have the function header comments been copied/moved here??

No, you are right, as long as you are using the proper kerneldoc
structure for the functions there, all is fine.

> > > +struct masterchannel *mipi_request_masterchannel(u8 kerneluser);
> > 
> > kerneluser?
> > 
> > > +void mipi_release_masterchannel(struct masterchannel *mc);
> > 
> > "Mipi"?  What's that?
> 
> 'mipi' is the standard body for the PTI standard.  It's an acronymn that
> does not stand for anything other than the Organization controlling the
> standard: http://www.mipi.org/about-mipi/frequently-asked-questions#q2

Why not just use "pti" as that's what you are dealing with here.
Otherwise people are going to be confused.

thanks,

greg k-h

Re: [PATCH 06/12] PTI header file.

[J Freyensee]

On Thu, 2011-02-17 at 11:55 -0800, Greg KH wrote:
> On Thu, Feb 17, 2011 at 11:41:55AM -0800, J Freyensee wrote:
> > On Thu, 2011-02-17 at 11:20 -0800, Greg KH wrote:
> > > On Tue, Feb 08, 2011 at 11:34:51AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > 
> > > > This adds PTI header information for the PTI project.
> > > > 
> > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > ---
> > > >  include/linux/pti.h |   38 ++++++++++++++++++++++++++++++++++++++
> > > 
> > > Why does this have to be in include/linux/?  What other files use it?
> > > 
> > > >  1 files changed, 38 insertions(+), 0 deletions(-)
> > > >  create mode 100644 include/linux/pti.h
> > > > 
> > > > diff --git a/include/linux/pti.h b/include/linux/pti.h
> > > > new file mode 100644
> > > > index 0000000..4b1c9f6
> > > > --- /dev/null
> > > > +++ b/include/linux/pti.h
> > > > @@ -0,0 +1,38 @@
> > > > +/*
> > > > + *  Copyright (C) Intel 2010
> > > > + *  Ken Mills <ken.k.mills@intel.com>
> > > 
> > > That's nice, where is Ken's signed-off-by?  We need that please.
> > > 
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License version 2 as
> > > > + * published by the Free Software Foundation.
> > > > + *
> > > > + * This program is distributed in the hope that it will be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > > + * GNU General Public License for more details.
> > > > + *
> > > > + * 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., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
> > > > + * USA
> > > 
> > > This paragraph is not needed, please remove it unless you are willing to
> > > keep up with the FSF's office moves for the next 40+ years.
> > > 
> > > Same goes for this paragraph on other files in this patchset, please
> > > remove it.
> > > 
> > > > + *
> > > > + */
> > > > +
> > > > +#ifndef PTI_H_
> > > > +#define PTI_H_
> > > > +
> > > > +/* basic structure used as a write address to the PTI HW */
> > > > +struct masterchannel {
> > > 
> > > Pretty generic structure name, please prefix it with "pti_"
> > > 
> > 
> > no problem, I'll fix all these.
> > 
> > > > +	u8 master;
> > > > +	u8 channel;
> > > > +};
> > > > +
> > > > +/* the following functions are defined in misc/pti.c */
> > > > +void mipi_pti_writedata(struct masterchannel *mc, u8 *cp, int count);
> > > 
> > > "cp"?
> > > 
> > 
> > All of the questions concerning these variables are described in pti.c.
> > Should have the function header comments been copied/moved here??
> 
> No, you are right, as long as you are using the proper kerneldoc
> structure for the functions there, all is fine.
> 
> > > > +struct masterchannel *mipi_request_masterchannel(u8 kerneluser);
> > > 
> > > kerneluser?
> > > 
> > > > +void mipi_release_masterchannel(struct masterchannel *mc);
> > > 
> > > "Mipi"?  What's that?
> > 
> > 'mipi' is the standard body for the PTI standard.  It's an acronymn that
> > does not stand for anything other than the Organization controlling the
> > standard: http://www.mipi.org/about-mipi/frequently-asked-questions#q2
> 
> Why not just use "pti" as that's what you are dealing with here.
> Otherwise people are going to be confused.

Good idea, I can change this and re-submit.

Thanks for the review,
Jay

> 
> thanks,
> 
> greg k-h

Re: [PATCH 04/12] PTI Kconfig change in misc.

[Greg KH]

On Tue, Feb 08, 2011 at 11:34:49AM -0800, james_p_freyensee@linux.intel.com wrote:
> From: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> This adds the Intel PTI Kconfig option.
> 
> Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> ---
>  drivers/misc/Kconfig |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 4d073f1..f8076f1 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -136,6 +136,18 @@ config PHANTOM
>  	  If you choose to build module, its name will be phantom. If unsure,
>  	  say N here.
>  
> +config INTEL_MID_PTI

Don't add new Kconfig options when there is no code for it yet to use
it.  Merge this with the Makefile patch.

thanks,

greg k-h

Re: [PATCH 04/12] PTI Kconfig change in misc.

[J Freyensee]

On Thu, 2011-02-17 at 11:20 -0800, Greg KH wrote:
> On Tue, Feb 08, 2011 at 11:34:49AM -0800, james_p_freyensee@linux.intel.com wrote:
> > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > 
> > This adds the Intel PTI Kconfig option.
> > 
> > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > ---
> >  drivers/misc/Kconfig |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > index 4d073f1..f8076f1 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -136,6 +136,18 @@ config PHANTOM
> >  	  If you choose to build module, its name will be phantom. If unsure,
> >  	  say N here.
> >  
> > +config INTEL_MID_PTI
> 
> Don't add new Kconfig options when there is no code for it yet to use
> it.  Merge this with the Makefile patch.
> 

I do not quite understand the problem.  The code for this Kconfig is
~/drivers/misc/pti.c and it was patch 3/12.  I tested that in 'make
menuconfig' the kernel turns the compile option on and off fine.

What needs to be fixed here?

> thanks,
> 
> greg k-h

Re: [PATCH 04/12] PTI Kconfig change in misc.

[Greg KH]

On Thu, Feb 17, 2011 at 11:35:51AM -0800, J Freyensee wrote:
> On Thu, 2011-02-17 at 11:20 -0800, Greg KH wrote:
> > On Tue, Feb 08, 2011 at 11:34:49AM -0800, james_p_freyensee@linux.intel.com wrote:
> > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > 
> > > This adds the Intel PTI Kconfig option.
> > > 
> > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > ---
> > >  drivers/misc/Kconfig |   12 ++++++++++++
> > >  1 files changed, 12 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > > index 4d073f1..f8076f1 100644
> > > --- a/drivers/misc/Kconfig
> > > +++ b/drivers/misc/Kconfig
> > > @@ -136,6 +136,18 @@ config PHANTOM
> > >  	  If you choose to build module, its name will be phantom. If unsure,
> > >  	  say N here.
> > >  
> > > +config INTEL_MID_PTI
> > 
> > Don't add new Kconfig options when there is no code for it yet to use
> > it.  Merge this with the Makefile patch.
> > 
> 
> I do not quite understand the problem.  The code for this Kconfig is
> ~/drivers/misc/pti.c and it was patch 3/12.  I tested that in 'make
> menuconfig' the kernel turns the compile option on and off fine.
> 
> What needs to be fixed here?

Merge it with your Makefile patch, it doesn't need to be separate.

thanks,

greg k-h

Re: [PATCH 02/12] Kernel documentation for the PTI feature.

[Randy Dunlap]

On 2/8/2011 11:34 AM, james_p_freyensee@linux.intel.com wrote:
> From: J Freyensee<james_p_freyensee@linux.intel.com>
>
> This provides Kernel documentation for the PTI
> feature for Linux mobile solutions.
>
> Signed-off-by: J Freyensee<james_p_freyensee@linux.intel.com>
> ---
>   Documentation/pti/pti_intel_mid.txt |   89 +++++++++++++++++++++++++++++++++++
>   1 files changed, 89 insertions(+), 0 deletions(-)
>   create mode 100644 Documentation/pti/pti_intel_mid.txt
>
> diff --git a/Documentation/pti/pti_intel_mid.txt b/Documentation/pti/pti_intel_mid.txt
> new file mode 100644
> index 0000000..b9c3f3a
> --- /dev/null
> +++ b/Documentation/pti/pti_intel_mid.txt
> @@ -0,0 +1,89 @@
> +The Intel MID PTI project is HW implemented in Intel Atom
> +system-on-a-chip designs based on the Parallel Trace
> +Interface for MIPI P1149.7 cJTAG standard.  The kernel solution
> +for this platform involves the following files:
> +
> +./include/linux/pti.h
> +./include/linux/n_tracesink.h
> +./drivers/.../n_tracerouter.c
> +./drivers/.../n_tracesink.c
> +./drivers/.../pti.c
> +
> +pti.c is the driver that enables various debugging features
> +popular on certain mobile manufacturers.  n_tracerouter.c
> +and n_tracesink.c allow extra system information to be
> +collected and routed to the pti driver, such as trace
> +debugging data from a modem.  Altough n_tracerouter

                                  Although

> +and n_tracesink are a part of the complete PTI solution,
> +these two line disciplines can work separate from

                                        separately

> +pti.c and route any data stream from one /dev/tty node
> +to another /dev/tty node via kernel-space.  This provides
> +a stable, reliable connection that will not break unless
> +the user-space application shuts down (plus avoids
> +kernel->user->kernel context switch overheads of routing
> +data).

~Randy

Re: [PATCH 01/12] export kernel call get_task_comm()

[Christoph Hellwig]

On Tue, Feb 08, 2011 at 11:34:46AM -0800, james_p_freyensee@linux.intel.com wrote:
> From: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> This allows drivers who call this function to be compiled modularly.
> Otherwise, a driver who is interested in this type of functionality
> has to implement their own.

NAK.

>From current you can just dereference current->comm, and access from
other threads from drivers is a bad idea.

Re: [PATCH 01/12] export kernel call get_task_comm()

[james_p_freyensee]

> On Tue, Feb 08, 2011 at 11:34:46AM -0800,
> james_p_freyensee@linux.intel.com wrote:
>> From: J Freyensee <james_p_freyensee@linux.intel.com>
>>
>> This allows drivers who call this function to be compiled modularly.
>> Otherwise, a driver who is interested in this type of functionality
>> has to implement their own.
>
> NAK.
>
> From current you can just dereference current->comm, and access from
> other threads from drivers is a bad idea.
>
>

That is originally what I did, but after talking with Greg KH, Arjan vdv,
and Alan Cox I made this patch instead.

Re: [PATCH 01/12] export kernel call get_task_comm()

[Christoph Hellwig]

On Tue, Feb 08, 2011 at 11:44:16AM -0800, james_p_freyensee@linux.intel.com wrote:
> That is originally what I did, but after talking with Greg KH, Arjan vdv,
> and Alan Cox I made this patch instead.

Care to explain why exactly?

Re: [PATCH 01/12] export kernel call get_task_comm()

[james_p_freyensee]

> On Tue, Feb 08, 2011 at 11:44:16AM -0800,
> james_p_freyensee@linux.intel.com wrote:
>> That is originally what I did, but after talking with Greg KH, Arjan
>> vdv,
>> and Alan Cox I made this patch instead.
>
> Care to explain why exactly?
>
>

If I can jog my own memory as this was a while ago...

Well for starters, Arjan van de Ven first suggested it, and I trust his
judgement because of his Linux kernel experience.

But looking at the code, the kernel function get_task_comm() provides a
level of safety with a lock, so the global resource 'current' can be used
without someone else trying to use it at the same time.  And using this
function facilitates code reuse which is something Greg KH was trying to
point out to me at the time:

http://lists.meego.com/pipermail/meego-kernel/2010-October/000823.html

In fact, I remember finding existing, accepted drivers in the kernel that
wrote their own locks before accessing current (I quickly just now found
block/dfdb/drbd_main.c as an example), so Greg is making a valid point
about code reuse.  So not exporting get_task_comm() encourages code
locking re-write of every driver that wants to access 'current'.

In that email, Greg clearly reminded me my goal is to get my work into the
kernel.  Thus, I listened to their advice and made this patch.

Re: [PATCH 01/12] export kernel call get_task_comm()

[Alan Cox]

On Tue, 8 Feb 2011 11:44:16 -0800 (PST)
> > From current you can just dereference current->comm, and access from
> > other threads from drivers is a bad idea.

You need to take the locks for it - which get_task_comm encapsulates.

Re: [PATCH 01/12] export kernel call get_task_comm()

[Christoph Hellwig]

On Tue, Feb 08, 2011 at 08:28:52PM +0000, Alan Cox wrote:
> On Tue, 8 Feb 2011 11:44:16 -0800 (PST)
> > > From current you can just dereference current->comm, and access from
> > > other threads from drivers is a bad idea.
> 
> You need to take the locks for it - which get_task_comm encapsulates.

Not for current->comm, which is the only thing it cares about.

Re: [PATCH 01/12] export kernel call get_task_comm()

[Alan Cox]

On Tue, 8 Feb 2011 15:28:56 -0500
Christoph Hellwig <hch@infradead.org> wrote:

> On Tue, Feb 08, 2011 at 08:28:52PM +0000, Alan Cox wrote:
> > On Tue, 8 Feb 2011 11:44:16 -0800 (PST)
> > > > From current you can just dereference current->comm, and access from
> > > > other threads from drivers is a bad idea.
> > 
> > You need to take the locks for it - which get_task_comm encapsulates.
> 
> Not for current->comm, which is the only thing it cares about.

Which is a detail you really don't want to leak out of the proper
abstraction use. I really don't see why you are advocating bad
programming practice - everything else uses the abstraction properly,
this just happens to be the first modular case that wants to behave
properly.

Re: [PATCH 01/12] export kernel call get_task_comm()

[Christoph Hellwig]

On Tue, Feb 08, 2011 at 08:36:02PM +0000, Alan Cox wrote:
> Which is a detail you really don't want to leak out of the proper
> abstraction use. I really don't see why you are advocating bad
> programming practice - everything else uses the abstraction properly,
> this just happens to be the first modular case that wants to behave
> properly.

Just grep for current->comm uses and get_task_comm uses and see that
this is not true.

Re: [PATCH 01/12] export kernel call get_task_comm()

[J Freyensee]

Greg:

Is this a patch you will or have already take/taken?  Or do you want me
to re-submit this with the rest of the patches like last time?  

My default is I will re-send you all of these patches with the changes
we talked about last week.

thanks,
jay

On Tue, 2011-02-08 at 11:34 -0800, james_p_freyensee@linux.intel.com
wrote:
> From: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> This allows drivers who call this function to be compiled modularly.
> Otherwise, a driver who is interested in this type of functionality
> has to implement their own.
> 
> Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> ---
>  fs/exec.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index c62efcb..7b209f9 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -998,6 +998,7 @@ char *get_task_comm(char *buf, struct task_struct *tsk)
>  	task_unlock(tsk);
>  	return buf;
>  }
> +EXPORT_SYMBOL_GPL(get_task_comm);
>  
>  void set_task_comm(struct task_struct *tsk, char *buf)
>  {

Re: [PATCH 01/12] export kernel call get_task_comm()

[Greg KH]

On Tue, Feb 22, 2011 at 10:38:06AM -0800, J Freyensee wrote:
> Greg:
> 
> Is this a patch you will or have already take/taken?  Or do you want me
> to re-submit this with the rest of the patches like last time?  
> 
> My default is I will re-send you all of these patches with the changes
> we talked about last week.

I have not taken any of this patch series, so yes, if you want it
applied, you need to resend it.

thanks,

greg k-h