Intel PTI implementaiton of MIPI 1149.7.

drivers/misc/pti.c new file +64 -0

new file mode 100644
index 0000000..f3a2980
@@ -0,0 +1,898 @@
+/*
+ *  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.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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 */
+#define APERTURE_14		0x3800000 /* offset to first OS write addr */
+#define APERTURE_LEN		0x400000  /* address length */
+
+struct pti_tty {
+	struct pti_masterchannel *mc;
+};
+
+struct pti_dev {
+	struct tty_port port;
+	unsigned long pti_addr;
+	unsigned long aperture_base;
+	void __iomem *pti_ioaddr;

drivers/misc/pti.c new file +8 -0

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

drivers/misc/pti.c new file +19 -0

+ *  @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 pti_masterchannel *mc,
+				  u8 *buf,
+				  int len)
+{
+	int dwordcnt, final, i;

drivers/misc/pti.c new file +77 -0

+	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 pti_masterchannel *mc)
+{
+	struct pti_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 pti_masterchannel *mc,

drivers/misc/pti.c new file +36 -0

+ * 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 pti_masterchannel *getID(u8 *IDarray, int max_IDS, int baseID)
+{
+	struct pti_masterchannel *mc;
+	int i, j, mask;
+
+	mc = kmalloc(sizeof(struct pti_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 */

drivers/misc/pti.c new file +65 -0

+ *
+ */
+struct pti_masterchannel *pti_request_masterchannel(u8 type)
+{
+	struct pti_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(pti_request_masterchannel);
+
+/**
+ * pti_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 pti_release_masterchannel(struct pti_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(pti_release_masterchannel);
+

drivers/misc/pti.c new file +14 -0

+	if ((mc != NULL) && (buf != NULL) && (count > 0))
+		pti_write_to_aperture(mc, buf, count);
+	return;
+}
+EXPORT_SYMBOL_GPL(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);

drivers/misc/pti.c new file +11 -0

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

drivers/misc/pti.c new file +55 -0

+ *
+ * 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 pti_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 = pti_request_masterchannel(0);
+		else
+			mc = pti_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 pti_masterchannel *mc;
+
+	pti_tty_data = tty->driver_data;
+
+	if (pti_tty_data != NULL) {
+		mc = pti_tty_data->mc;
+		pti_release_masterchannel(mc);
+		pti_tty_data->mc = NULL;

drivers/misc/pti.c new file +20 -0

+ */
+static int pti_tty_driver_write(struct tty_struct *tty,
+	const unsigned char *buf, int len)
+{
+	struct pti_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;
+}
+
+/**

drivers/misc/pti.c new file +11 -0

+ */
+static int pti_char_open(struct inode *inode, struct file *filp)
+{
+	struct pti_masterchannel *mc;
+
+	mc = pti_request_masterchannel(0);
+	if (mc == NULL)
+		return -ENOMEM;
+	filp->private_data = mc;
+	return 0;
+}

drivers/misc/pti.c new file +93 -0

+ *
+ * Notes: From side discussions with Alan Cox and experimenting
+ * with PTI debug HW like Nokia's Fido box and Lauterbach
+ * devices, 8192 byte write buffer used by USER_COPY_SIZE was
+ * deemed an appropriate size for this type of usage with
+ * debugging HW.
+ */
+static ssize_t pti_char_write(struct file *filp, const char __user *data,
+			      size_t len, loff_t *ppose)
+{
+	struct pti_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 pti_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;
+}

drivers/misc/pti.c new file +4 -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.

drivers/misc/pti.c new file +10 -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 void pti_port_shutdown(struct tty_port *port)
+{
+	if (port->tty->index == PTITTY_MINOR_START)
+		console_stop(&pti_console);
+}

drivers/misc/pti.c new file +70 -0

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

drivers/misc/pti.c new file +52 -0

+ *
+ */
+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",

drivers/misc/pti.c new file +26 -0

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

Quoted 72 lines

drivers/misc/pti.c new file +64 -0

new file mode 100644
index 0000000..f3a2980
@@ -0,0 +1,898 @@
+/*
+ *  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.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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 */
+#define APERTURE_14		0x3800000 /* offset to first OS write addr */
+#define APERTURE_LEN		0x400000  /* address length */
+
+struct pti_tty {
+	struct pti_masterchannel *mc;
+};
+
+struct pti_dev {
+	struct tty_port port;
+	unsigned long pti_addr;
+	unsigned long aperture_base;
+	void __iomem *pti_ioaddr;

drivers/misc/pti.c line 74

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

+	u8 ia_app[MAX_APP_IDS];
+	u8 ia_os[MAX_OS_IDS];
+	u8 ia_modem[MAX_MODEM_IDS];
+};
+
+/*
+   This protects access to ia_app, ia_os, and ia_modem,
+   which keeps track of channels allocated in
+   an aperture write id.
+*/

Use normal multi-line comment format:

/*
* line1
* line2
*/

Quoted 8 lines

drivers/misc/pti.c new file +8 -0

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

drivers/misc/pti.c line 92

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

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

THE ?

Quoted 19 lines

drivers/misc/pti.c new file +19 -0

+ *  @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 pti_masterchannel *mc,
+				  u8 *buf,
+				  int len)
+{
+	int dwordcnt, final, i;

drivers/misc/pti.c line 121

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

+	u32 ptiword;
+	u8 *p;
+	u32 __iomem *aperture;
+
+	p = buf;
+
+	/*
+	   calculate the aperture offset from the base using the master and
+	   channel id's.
+	*/

comment style (see Documentation/CodingStyle, section 8)

Quoted 77 lines

drivers/misc/pti.c new file +77 -0

+	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 pti_masterchannel *mc)
+{
+	struct pti_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 pti_masterchannel *mc,

drivers/misc/pti.c line 208

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

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

* getID() - Allocate a master and channel ID

drivers/misc/pti.c line 210

Quoted 2 lines

drivers/misc/pti.c new file +2 -0

+ *
+ * @IDarray:

needs explanatory text

drivers/misc/pti.c line 215

Quoted 5 lines

drivers/misc/pti.c new file +5 -0

+ * @max_IDS: The max amount of available write IDs to use.
+ * @baseID:  The starting SW channel ID, based on the Intel
+ *           PTI arch.
+ *
+ * @return: pti_masterchannel struct containing master, channel ID address,

No '@' on "return".

Quoted 38 lines

drivers/misc/pti.c new file +36 -0

+ * 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 pti_masterchannel *getID(u8 *IDarray, int max_IDS, int baseID)
+{
+	struct pti_masterchannel *mc;
+	int i, j, mask;
+
+	mc = kmalloc(sizeof(struct pti_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 */

drivers/misc/pti.c line 261

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

+	pti_control_frame_built_and_sent(mc);
+	return mc;
+}
+
+/*
+	The following three functions:
+	pti_request_mastercahannel(), mipi_release_masterchannel()
+	and pti_writedata() are an API for other kernel drivers to
+	access PTI.
+*/

multi-line comment style.

drivers/misc/pti.c line 272

Quoted 11 lines

drivers/misc/pti.c new file +11 -0

+
+/**
+ * pti_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: pti_masterchannel struct or 0 for error.

No '@' on "return".

Quoted 66 lines

drivers/misc/pti.c new file +65 -0

+ *
+ */
+struct pti_masterchannel *pti_request_masterchannel(u8 type)
+{
+	struct pti_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(pti_request_masterchannel);
+
+/**
+ * pti_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 pti_release_masterchannel(struct pti_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(pti_release_masterchannel);
+

drivers/misc/pti.c line 347

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

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

return with no write occurring.

or maybe the @count/@len parameters should be size_t so that they cannot be negative.

drivers/misc/pti.c line 355

Quoted 9 lines

drivers/misc/pti.c new file +8 -0

+ */
+void pti_writedata(struct pti_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.
+	*/

comment style.

Quoted 14 lines

drivers/misc/pti.c new file +14 -0

+	if ((mc != NULL) && (buf != NULL) && (count > 0))
+		pti_write_to_aperture(mc, buf, count);
+	return;
+}
+EXPORT_SYMBOL_GPL(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);

drivers/misc/pti.c line 379

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

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

comment style.

drivers/misc/pti.c line 386

Quoted 7 lines

drivers/misc/pti.c new file +7 -0

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

drop "param". Just use:

* @tty: <text>
* @filp: <text>

drivers/misc/pti.c line 388

Quoted 3 lines

drivers/misc/pti.c new file +2 -0

+ *
+ * @return int : Success = 0, otherwise fail.

No '@' on "return".

Quoted 13 lines

drivers/misc/pti.c new file +11 -0

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

drivers/misc/pti.c line 409

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

+{
+	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.
+	*/

comment style.

drivers/misc/pti.c line 420

Quoted 11 lines

drivers/misc/pti.c new file +11 -0

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

Incorrect kernel-doc notation. See Documentation/kernel-doc-nano-HOWTO.txt.

Quoted 55 lines

drivers/misc/pti.c new file +55 -0

+ *
+ * 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 pti_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 = pti_request_masterchannel(0);
+		else
+			mc = pti_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 pti_masterchannel *mc;
+
+	pti_tty_data = tty->driver_data;
+
+	if (pti_tty_data != NULL) {
+		mc = pti_tty_data->mc;
+		pti_release_masterchannel(mc);
+		pti_tty_data->mc = NULL;

drivers/misc/pti.c line 485

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

+	}
+
+	if (pti_tty_data != NULL)
+		kfree(pti_tty_data);
+
+	tty->driver_data = NULL;
+}
+
+/**
+ * pti_tty_driver_write():  Write trace debugging data through the char

* pti_tty_driver_write() - <text>

drivers/misc/pti.c line 492

Quoted 7 lines

drivers/misc/pti.c new file +7 -0

+ * 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.

Fix kernel-doc notation. again. :(

Quoted 20 lines

drivers/misc/pti.c new file +20 -0

+ */
+static int pti_tty_driver_write(struct tty_struct *tty,
+	const unsigned char *buf, int len)
+{
+	struct pti_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;
+}
+
+/**

drivers/misc/pti.c line 522

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

+ * 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.

Fix kernel-doc notation.

Quoted 11 lines

drivers/misc/pti.c new file +11 -0

+ */
+static int pti_char_open(struct inode *inode, struct file *filp)
+{
+	struct pti_masterchannel *mc;
+
+	mc = pti_request_masterchannel(0);
+	if (mc == NULL)
+		return -ENOMEM;
+	filp->private_data = mc;
+	return 0;
+}

drivers/misc/pti.c line 543

Quoted 10 lines

drivers/misc/pti.c new file +10 -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

fix kernel-doc notation.

drivers/misc/pti.c line 552

Quoted 9 lines

drivers/misc/pti.c new file +9 -0

+ */
+static int pti_char_release(struct inode *inode, struct file *filp)
+{
+	pti_release_masterchannel(filp->private_data);
+	return 0;
+}
+
+/**
+ * pti_char_write():  Write trace debugging data through the char

use '-' instead of ':'

drivers/misc/pti.c line 560

Quoted 8 lines

drivers/misc/pti.c new file +8 -0

+ * 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.

Fix kernel-doc notation.

Quoted 93 lines

drivers/misc/pti.c new file +93 -0

+ *
+ * Notes: From side discussions with Alan Cox and experimenting
+ * with PTI debug HW like Nokia's Fido box and Lauterbach
+ * devices, 8192 byte write buffer used by USER_COPY_SIZE was
+ * deemed an appropriate size for this type of usage with
+ * debugging HW.
+ */
+static ssize_t pti_char_write(struct file *filp, const char __user *data,
+			      size_t len, loff_t *ppose)
+{
+	struct pti_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 pti_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;
+}

drivers/misc/pti.c line 663

Quoted 10 lines

drivers/misc/pti.c new file +10 -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.
+ */

fix long comment style.

drivers/misc/pti.c line 674

Quoted 11 lines

drivers/misc/pti.c new file +11 -0

+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

use '-' instead of ':'

drivers/misc/pti.c line 680

Quoted 6 lines

drivers/misc/pti.c new file +6 -0

+ * 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.

fix kernel-doc notation.

Quoted 4 lines

drivers/misc/pti.c new file +4 -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.

drivers/misc/pti.c line 694

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

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

use - instead of :

drivers/misc/pti.c line 697

Quoted 3 lines

drivers/misc/pti.c new file +3 -0

+ * last tty port close.
+ *
+ * @param port- The tty port number of the PTI device.

fix kernel-doc notation.

Quoted 10 lines

drivers/misc/pti.c new file +10 -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 void pti_port_shutdown(struct tty_port *port)
+{
+	if (port->tty->index == PTITTY_MINOR_START)
+		console_stop(&pti_console);
+}

drivers/misc/pti.c line 717

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

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

fix long comment style.

Quoted 70 lines

drivers/misc/pti.c new file +70 -0

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

drivers/misc/pti.c line 797

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

+static struct pci_driver pti_pci_driver = {
+	.name		= PCINAME,
+	.id_table	= pci_ids,
+	.probe		= pti_pci_probe,
+	.remove		= pti_pci_remove,
+};
+
+/**
+ *
+ * pti_init():

Needs short function summary on line above.

drivers/misc/pti.c line 799

Quoted 2 lines

drivers/misc/pti.c new file +2 -0

+ *
+ * @return int __init: 0 for success, any other value error.

fix kernel-doc notation.

Quoted 52 lines

drivers/misc/pti.c new file +52 -0

+ *
+ */
+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",

drivers/misc/pti.c line 861

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

+			__func__, __LINE__);
+		pti_tty_driver = NULL;
+		return retval;
+	}
+
+	return retval;
+}
+
+/**
+ * pti_exit(): Unregisters this module as a tty and pci driver.

use - instead of :

Quoted 26 lines

drivers/misc/pti.c new file +26 -0

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

drivers/misc/pti.c line 897

Quoted 10 lines

drivers/misc/pti.c new file +10 -0

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

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

On Tue, 2011-04-19 at 16:15 -0700, Randy Dunlap wrote:

A couple more comments below.

Quoted 9 lines · On Tue, 19 Apr 2011 15:58:08 -0700 [email protected] wrote:

Quoted 7 lines

+ * @max_IDS: The max amount of available write IDs to use.
+ * @baseID: The starting SW channel ID, based on the Intel
+ * PTI arch.
+ *
+ * @return: pti_masterchannel struct containing master, channel ID address,

Quoted 1 line

No '@' on "return".

Why no '@' on 'return' when just by doing a 'grep -Ri "@return" drivers/ | wc -l' I count 369 examples of '@return' being used already in the kernel? It looks like an acceptable format to me.

Quoted 21 lines

Quoted 20 lines

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

drivers/misc/pti.c new file +12 -0

+static struct pti_masterchannel *getID(u8 *IDarray, int max_IDS, int baseID)
+{
 
 
+/**
+ * pti_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: pti_masterchannel struct or 0 for error.

Quoted 1 line

No '@' on "return".

Same reason here.

Quoted 4 lines

Quoted 2 lines

+ *
+ * @return int : Success = 0, otherwise fail.

No '@' on "return".

Same explanation as above.

Quoted 15 lines · On Wed, 20 Apr 2011 16:05:00 -0700 J Freyensee wrote:

On Tue, 2011-04-19 at 16:15 -0700, Randy Dunlap wrote:

A couple more comments below.

Quoted 10 lines · On Tue, 19 Apr 2011 15:58:08 -0700 [email protected] wrote:

Quoted 7 lines

+ * @max_IDS: The max amount of available write IDs to use.
+ * @baseID: The starting SW channel ID, based on the Intel
+ * PTI arch.
+ *
+ * @return: pti_masterchannel struct containing master, channel ID address,

No '@' on "return".

Quoted 3 lines

Why no '@' on 'return' when just by doing a 'grep -Ri "@return" drivers/ | wc -l' I count 369 examples of '@return' being used already in the kernel? It looks like an acceptable format to me.

It's not. See Documentation/kernel-doc-nano-HOWTO.txt. '@' goes on function parameters (or struct members). Not on return values. Those other places should be fixed, but it's just not a high priority thing to do.

Quoted 22 lines

Quoted 22 lines

Quoted 21 lines

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

drivers/misc/pti.c new file +12 -0

+static struct pti_masterchannel *getID(u8 *IDarray, int max_IDS, int baseID)
+{
 
 
+/**
+ * pti_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: pti_masterchannel struct or 0 for error.

Quoted 3 lines

Quoted 1 line

No '@' on "return".

Same reason here.

Same answer here.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

+ *
+ * @return int : Success = 0, otherwise fail.

No '@' on "return".

Same explanation as above.

Same reply also.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Quoted 21 lines · Randy Dunlap

Quoted 18 lines · On Wed, 20 Apr 2011 16:05:00 -0700 J Freyensee wrote:

On Tue, 2011-04-19 at 16:15 -0700, Randy Dunlap wrote:

A couple more comments below.

Quoted 10 lines · On Tue, 19 Apr 2011 15:58:08 -0700 [email protected] wrote:

Quoted 7 lines

+ * @max_IDS: The max amount of available write IDs to use.
+ * @baseID: The starting SW channel ID, based on the Intel
+ * PTI arch.
+ *
+ * @return: pti_masterchannel struct containing master, channel ID address,

No '@' on "return".

Why no '@' on 'return' when just by doing a 'grep -Ri "@return" drivers/ | wc -l' I count 369 examples of '@return' being used already in the kernel? It looks like an acceptable format to me.

Quoted 5 lines

It's not. See Documentation/kernel-doc-nano-HOWTO.txt. '@' goes on function parameters (or struct members). Not on return values. Those other places should be fixed, but it's just not a high priority thing to do.

How should I document return values of functions? I would like them documented somehow.

kernel-doc-nano-HOWTO.txt does not say other than give examples of what I don't want to do and to 'Take a look around the source tree for examples'.

So one example I found that documents return values does not seem to follow kernel-doc-nano-HOWTO.txt:

(acpi/acpica/dsutils.c)
/******************************************************************************* *
* FUNCTION: acpi_ds_clear_implicit_return
*
* PARAMETERS: walk_state - Current State
*
* RETURN: None.
*

then another driver, net/wireless/libertas/tx.c, does exactly what I do that I'm being advised against (minus the 's' at the end of 'return'):

/**
* @brief This function sends to the host the last transmitted packet,
* filling the radiotap headers with transmission information. *

 *  @param priv     A pointer to struct lbs_private structure
 *  @param status   A 32 bit value containing transmission status.

*
* @returns void
*/

Quoted 29 lines

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

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

drivers/misc/pti.c new file +12 -0

+static struct pti_masterchannel *getID(u8 *IDarray, int max_IDS, int baseID)
+{
 
 
+/**
+ * pti_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: pti_masterchannel struct or 0 for error.

Quoted 3 lines

Quoted 1 line

No '@' on "return".

Same reason here.

Same answer here.

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

+ *
+ * @return int : Success = 0, otherwise fail.

No '@' on "return".

Same explanation as above.

Same reply also.

drivers/misc/pti.c new file +7 -0

+static void pti_control_frame_built_and_sent(struct pti_masterchannel *mc)
+{
+	struct pti_masterchannel mccontrol = {.master = CONTROL_ID,
+					      .channel = 0};
+	const char *control_format = "%3d %3d %s";
+
+	char comm[sizeof(current->comm) + 1];

is small enough that it shouldn't be an issue. Otherwise there's nothing protecting sizeof(current->comm) from changing without holding task_lock(current).

Quoted 3 lines

is small enough that it shouldn't be an issue. Otherwise there's nothing protecting sizeof(current->comm) from changing without holding task_lock(current).

The C language definition for one - sizeof() is a constant at compile time

Quoted 15 lines · On Tue, 19 Apr 2011, [email protected] wrote:

drivers/misc/pti.c new file +15 -0

+static void pti_control_frame_built_and_sent(struct pti_masterchannel *mc)
+{
+	struct pti_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 */

Review

Quoted 2 lines

(quoted diff) +2 -0

+	comm[sizeof(current->comm)] = 0;
+

You definitely need to use get_task_comm() here, but that means you can't allocate char comm[] on the stack with anything but TASK_COMM_LEN, which is small enough that it shouldn't be an issue. Otherwise there's nothing protecting sizeof(current->comm) from changing without holding task_lock(current).

+ comm[sizeof(current->comm)] = 0;