andy git 06/15 suspend/resume observations

Mike (mwester) mwester at dls.net
Thu Jun 19 23:01:43 CEST 2008


This patch that cleans up the GSM suspend/resume handling code.

Specifically, this patch removes:
- the delayed GSM flow-control release code (work queue)
- the code that enabled/disabled suspended flow-control via user-space
  manipulation of the RTS modem control signal via the tty driver
- the GTA01-specific code to read the first data after resume by polling

The patch adds some paranoid tests to ensure that none of the remaining
GSM suspend/resume handling code dereferences pointers without testing
for NULL.

This leaves the existing sysfs mechanism to forcibly enable/disable RTS
in place as the preferred means for user-space to manage putting the GSM
"to bed" during suspend, with the fall-back of having the neo1973_pm_gsm
driver forcibly enable and disable RTS in the event that user-space has
failed to do so.

No special code exists for the GTA01 any longer as this special code
merely made overruns less likely to occur, but did not actually fix
the problem.

Signed-off-by: Mike Westerhof (mwester at dls.net)

 arch/arm/plat-s3c24xx/neo1973_pm_gsm.c |   52 +-------
 drivers/serial/s3c2410.c               |  204 +++-----------------------------
 2 files changed, 24 insertions(+), 232 deletions(-)

diff --git a/arch/arm/plat-s3c24xx/neo1973_pm_gsm.c b/arch/arm/plat-s3c24xx/neo1973_pm_gsm.c
index 3d086bc..7443d48 100644
--- a/arch/arm/plat-s3c24xx/neo1973_pm_gsm.c
+++ b/arch/arm/plat-s3c24xx/neo1973_pm_gsm.c
@@ -31,19 +31,10 @@
 #endif
 
 #include <linux/neospy.h>
-#include <linux/workqueue.h>
-#include <linux/delay.h>
-
-extern void s3c24xx_fake_rx_interrupt(int l);
-
-static struct work_struct gsmwork;
 
 /* flag set if we flow-controlled the GSM in our suspend routine */
 int gsm_auto_flowcontrolled;
 
-/* msecs to delay the auto-unlock after resume to minimize overruns */
-int gsm_autounlock_delay = 750;
-
 struct neospy_logdata neospy;
 EXPORT_SYMBOL(neospy);
 int nspy_showctrl;
@@ -103,8 +94,6 @@ static ssize_t gsm_read(struct device *dev, struct device_attribute *attr,
 		}
 	} else if (!strcmp(attr->attr.name, "gsm_interrupts")) {
 		return snprintf(buf, PAGE_SIZE, "%d\n", gta_gsm_interrupts);
-	} else if (!strcmp(attr->attr.name, "autounlock_delay")) {
-		return snprintf(buf, PAGE_SIZE, "%d\n", gsm_autounlock_delay);
 	} else if (!strcmp(attr->attr.name, "flowcontrolled")) {
 		if (s3c2410_gpio_getcfg(S3C2410_GPH1) == S3C2410_GPIO_OUTPUT)
 			goto out_1;
@@ -361,13 +350,6 @@ static ssize_t gsm_write(struct device *dev, struct device_attribute *attr,
 		}
 	} else if (!strcmp(attr->attr.name, "gsm_interrupts")) {
 		gta_gsm_interrupts = 0;
-	} else if (!strcmp(attr->attr.name, "autounlock_delay")) {
-		sscanf(buf, "%d", &gsm_autounlock_delay);
-		/* constrain to within reasonable bounds */
-		if ((gsm_autounlock_delay > 0) && (gsm_autounlock_delay < 250))
-			gsm_autounlock_delay = 250;
-		else if (gsm_autounlock_delay > 5000)
-			gsm_autounlock_delay = 5000;
 	} else if (!strcmp(attr->attr.name, "flowcontrolled")) {
 		if (on) {
 			gta_gsm_interrupts = 0;
@@ -405,7 +387,6 @@ static DEVICE_ATTR(power_on, 0644, gsm_read, gsm_write);
 static DEVICE_ATTR(reset, 0644, gsm_read, gsm_write);
 static DEVICE_ATTR(download, 0644, gsm_read, gsm_write);
 static DEVICE_ATTR(gsm_interrupts, 0644, gsm_read, gsm_write);
-static DEVICE_ATTR(autounlock_delay, 0644, gsm_read, gsm_write);
 static DEVICE_ATTR(flowcontrolled, 0644, gsm_read, gsm_write);
 static DEVICE_ATTR(nspy_enable, 0644, gsm_read, gsm_write);
 static DEVICE_ATTR(nspy_buffer, 0644, gsm_read, gsm_write);
@@ -414,23 +395,6 @@ static DEVICE_ATTR(nspy_linewrap, 0644, gsm_read, gsm_write);
 static DEVICE_ATTR(nspy_jifdelta, 0644, gsm_read, gsm_write);
 
 #ifdef CONFIG_PM
-static void gsm_resume_work(struct work_struct *w)
-{
-	printk(KERN_INFO "%s: waiting...\n", __FUNCTION__);
-	nspy_add(NSPY_TYPE_RESUME, 'W', jiffies);
-	if (gsm_autounlock_delay)
-		msleep(gsm_autounlock_delay);
-	if (gsm_auto_flowcontrolled) {
-		nspy_add(NSPY_TYPE_SPECIAL, '+', jiffies);
-		if (machine_is_neo1973_gta01())
-			s3c24xx_fake_rx_interrupt(10000);
-		s3c2410_gpio_cfgpin(S3C2410_GPH1, S3C2410_GPH1_nRTS0);
-		gsm_auto_flowcontrolled = 0;
-	}
-	nspy_add(NSPY_TYPE_RESUME, 'Z', jiffies);
-	printk(KERN_INFO "%s: done.\n", __FUNCTION__);
-}
-
 static int gta01_gsm_suspend(struct platform_device *pdev, pm_message_t state)
 {
 	/* GPIO state is saved/restored by S3C2410 core GPIO driver, so we
@@ -499,12 +463,14 @@ static int gta01_gsm_resume(struct platform_device *pdev)
 	if (machine_is_neo1973_gta02())
 		s3c2410_gpio_setpin(GTA02_GPIO_nDL_GSM, gta01_gsm.gpio_ndl_gsm);
 
-	/* We must defer the auto flowcontrol because we resume before
-	 * the serial driver */
-	if (!schedule_work(&gsmwork))
-		dev_err(&pdev->dev,
-			"Unable to schedule GSM wakeup work\n");
+	/* If we forced flow-control on the GSM, we should also release it */
+	if (gsm_auto_flowcontrolled) {
+		nspy_add(NSPY_TYPE_SPECIAL, '+', jiffies);
+		s3c2410_gpio_cfgpin(S3C2410_GPH1, S3C2410_GPH1_nRTS0);
+		gsm_auto_flowcontrolled = 0;
+	}
 
+	nspy_add(NSPY_TYPE_RESUME, 'Z', jiffies);
 	return 0;
 }
 #else
@@ -518,7 +484,6 @@ static struct attribute *gta01_gsm_sysfs_entries[] = {
 	&dev_attr_reset.attr,
 	&dev_attr_download.attr,
 	&dev_attr_gsm_interrupts.attr,
-	&dev_attr_autounlock_delay.attr,
 	&dev_attr_flowcontrolled.attr,
 	&dev_attr_nspy_enable.attr,
 	&dev_attr_nspy_buffer.attr,
@@ -612,9 +577,6 @@ static struct platform_driver gta01_gsm_driver = {
 static int __devinit gta01_gsm_init(void)
 {
 	nspy_init();
-	INIT_WORK(&gsmwork, gsm_resume_work);
-	if (machine_is_neo1973_gta02())
-		gsm_autounlock_delay = 0;
 	return platform_driver_register(&gta01_gsm_driver);
 }
 
diff --git a/drivers/serial/s3c2410.c b/drivers/serial/s3c2410.c
index 3315fca..4fe1325 100644
--- a/drivers/serial/s3c2410.c
+++ b/drivers/serial/s3c2410.c
@@ -83,6 +83,7 @@
 
 #include <linux/neospy.h>
 extern struct neospy_logdata neospy;
+#define GSM_PORT 0
 
 extern int gta_gsm_interrupts;
 
@@ -372,7 +373,7 @@ s3c24xx_serial_rx_chars(int irq, void *dev_id)
 			/* check for break */
 			if (uerstat & S3C2410_UERSTAT_BREAK) {
 				dbg("break!\n");
-				if (cfg->hwport == 0)
+				if (cfg && (cfg->hwport == GSM_PORT))
 					nspy_add(NSPY_TYPE_CERROR,
 						 'B', jiffies);
 				port->icount.brk++;
@@ -386,13 +387,13 @@ s3c24xx_serial_rx_chars(int irq, void *dev_id)
 				port->icount.overrun++;
 
 			if ((uerstat & S3C2410_UERSTAT_FRAME) &&
-			    (cfg->hwport == 0))
+			    cfg && (cfg->hwport == GSM_PORT))
 				nspy_add(NSPY_TYPE_CERROR, 'F', jiffies);
 			if ((uerstat & S3C2410_UERSTAT_OVERRUN) &&
-			    (cfg->hwport == 0))
+			    cfg && (cfg->hwport == GSM_PORT))
 				nspy_add(NSPY_TYPE_CERROR, 'O', jiffies);
 			if ((uerstat & S3C2410_UERSTAT_PARITY) &&
-			    (cfg->hwport == 0))
+			    cfg && (cfg->hwport == GSM_PORT))
 				nspy_add(NSPY_TYPE_CERROR, 'P', jiffies);
 
 			uerstat &= port->read_status_mask;
@@ -411,7 +412,7 @@ s3c24xx_serial_rx_chars(int irq, void *dev_id)
 
 		uart_insert_char(port, uerstat, S3C2410_UERSTAT_OVERRUN, ch,
 									  flag);
-		if (cfg->hwport == 0) {
+		if (cfg && (cfg->hwport == GSM_PORT)) {
 			if (afc_is_enabled)
 				nspy_add(NSPY_TYPE_CIN, ch, jiffies);
 			else
@@ -427,126 +428,6 @@ s3c24xx_serial_rx_chars(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-struct s3c24xx_uart_port *gta01_devptr;
-#define FAKE_BSIZE	64
-void s3c24xx_fake_rx_interrupt(int l)
-{
-	struct s3c24xx_uart_port *ourport;
-	struct uart_port *port;
-	struct tty_struct *tty;
-	struct s3c2410_uartcfg *cfg;
-	unsigned int ulcon, ucon, ufcon, umcon, ufstat, umstat;
-	unsigned int ch, uerstat, flag, u;
-	int n, c, d;
-	unsigned int cbuf[FAKE_BSIZE], ubuf[FAKE_BSIZE];
-	int i, top, lctr;
-
-	ourport = gta01_devptr;
-	if (!ourport)
-		return;
-
-	port = &ourport->port;
-	if (!port)
-		return;
-
-	tty = port->info->tty;
-	if (!tty)
-		return;
-
-	cfg = s3c24xx_port_to_cfg(port);
-	if (!cfg)
-		return;
-
-	disable_irq(RX_IRQ(port));
-
-	ulcon  = rd_regl(port, S3C2410_ULCON);
-	ucon   = rd_regl(port, S3C2410_UCON),
-	ufcon  = rd_regl(port, S3C2410_UFCON);
-	umcon  = rd_regl(port, S3C2410_UMCON);
-	ufstat = rd_regl(port, S3C2410_UFSTAT);
-	umstat = rd_regl(port, S3C2410_UMSTAT);
-
-	printk(KERN_ERR "%s @ entry: ulcon=0x%08x, ucon=0x%08x, ufcon=0x%08x\n",
-	       __FUNCTION__, ulcon, ucon, ufcon);
-	printk(KERN_ERR
-	       "%s @ entry: umcon=0x%08x, ufstat=0x%08x, umstat=0x%08x\n",
-	       __FUNCTION__, umcon, ufstat, umstat);
-
-	n = s3c24xx_serial_rx_fifocnt(ourport, ufstat);
-	if (ufstat & 0x100)
-		printk(KERN_ERR "%s @ entry: FIFO full...\n", __FUNCTION__);
-	else
-		printk(KERN_ERR "%s @ entry: %d characters await...\n",
-		       __FUNCTION__, (ufstat & 0xf));
-
-	nspy_add(NSPY_TYPE_SPECIAL, '[', jiffies);
-
-	if (l < 100)
-		l = 10000;
-
-	top = 0;
-	d = FAKE_BSIZE;
-	lctr = 0;
-	if (!(umcon & S3C2410_UMCOM_AFC)) {
-		wr_regl(port, S3C2410_UMCON, (umcon | S3C2410_UMCOM_AFC));
-		printk(KERN_ERR "%s @ entry: set AFC.\n", __FUNCTION__);
-	}
-	s3c2410_gpio_cfgpin(S3C2410_GPH1, S3C2410_GPH1_nRTS0);
-	while (d-- > 0) {
-
-		u = rd_regl(port, S3C2410_UFSTAT) & 0x10f;
-		if (u == 0) {
-			c = l;
-			while ((c-- > 0) && (u == 0))
-				u = rd_regl(port, S3C2410_UFSTAT) & 0x10f;
-
-			if (u) {
-				i = l - c;
-				if (i > lctr)
-					lctr = i;
-			}
-		}
-
-		if (u & 0x100)
-			n = 16;
-		else
-			n = u & 0xf;
-
-		while ((top < FAKE_BSIZE) && (n-- > 0)) {
-			ubuf[top] = rd_regl(port, S3C2410_UERSTAT);
-			cbuf[top] = rd_regb(port, S3C2410_URXH);
-			top++;
-		}
-	}
-
-	for (i = 0; i < top; i++) {
-		uerstat = ubuf[i];
-		ch = cbuf[i];
-		flag = TTY_NORMAL;
-		port->icount.rx++;
-		if (uerstat & S3C2410_UERSTAT_OVERRUN) {
-			port->icount.overrun++;
-			nspy_add(NSPY_TYPE_CERROR, 'O', jiffies);
-			uerstat &= port->read_status_mask;
-			if (uerstat & S3C2410_UERSTAT_OVERRUN)
-				flag = TTY_FRAME;
-		}
-		uart_insert_char(port, uerstat, S3C2410_UERSTAT_OVERRUN, ch,
-									  flag);
-		if (umcon & S3C2410_UMCOM_AFC)
-			nspy_add(NSPY_TYPE_CIN, ch, jiffies);
-		else
-			nspy_add(NSPY_TYPE_CIN_NOAFC, ch, jiffies);
-	}
-	tty_flip_buffer_push(tty);
-
-	nspy_add(NSPY_TYPE_SPECIAL, ']', jiffies);
-	enable_irq(RX_IRQ(port));
-	printk(KERN_ERR "%s @ exit: max loop counter was %d...\n", __FUNCTION__,
-									  lctr);
-}
-EXPORT_SYMBOL(s3c24xx_fake_rx_interrupt);
-
 static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 {
 	struct s3c24xx_uart_port *ourport = id;
@@ -557,7 +438,7 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 
 	if (port->x_char) {
 		wr_regb(port, S3C2410_UTXH, port->x_char);
-		if (cfg->hwport == 0)
+		if (cfg && (cfg->hwport == GSM_PORT))
 			nspy_add(NSPY_TYPE_COUT, port->x_char, jiffies);
 		port->icount.tx++;
 		port->x_char = 0;
@@ -580,7 +461,7 @@ static irqreturn_t s3c24xx_serial_tx_chars(int irq, void *id)
 			break;
 
 		wr_regb(port, S3C2410_UTXH, xmit->buf[xmit->tail]);
-		if (cfg->hwport == 0)
+		if (cfg && (cfg->hwport == GSM_PORT))
 			nspy_add(NSPY_TYPE_COUT, xmit->buf[xmit->tail],
 								       jiffies);
 		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
@@ -625,51 +506,9 @@ static unsigned int s3c24xx_serial_get_mctrl(struct uart_port *port)
 		return TIOCM_CAR | TIOCM_DSR;
 }
 
-/* set during suspend/resume to ensure only user-space changes GSM mctrl */
-static int doing_serial_suspend_resume;
-/* flag to ensure that we only re-assert if, in fact, we did the de-assert */
-static int gsm_mctrl_flowcontrolled;
-
 static void s3c24xx_serial_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
-	struct s3c2410_uartcfg *cfg = s3c24xx_port_to_cfg(port);
-	if (cfg == NULL)
-		return;
-
-	/* Special handling is only on port 0 of the GTA0x devices (GSM port) */
-	if (cfg->hwport != 0)
-		return;
-
-	printk(KERN_ERR "s3c24xx_serial_set_mctrl: GSM mctrl=0x%08x\n", mctrl);
-	nspy_add(NSPY_TYPE_SPECIAL, 'R', jiffies);
-
-	/* Ignore the request if this is a side-effect of suspend/resume */
-	if (doing_serial_suspend_resume)
-		return;
-
-	if (mctrl & TIOCM_RTS) {
-		/* Return RTS to the UART - but only if
-		 * we de-asserted it originally
-		 */
-		if (gsm_mctrl_flowcontrolled) {
-			nspy_add(NSPY_TYPE_SPECIAL, '+', jiffies);
-			s3c2410_gpio_cfgpin(S3C2410_GPH1, S3C2410_GPH1_nRTS0);
-			gsm_mctrl_flowcontrolled = 0;
-		}
-	} else {
-		/* Hold RTS off by turning it into a GPIO,
-		 * if not already done
-		 */
-		if (s3c2410_gpio_getcfg(S3C2410_GPH1) == S3C2410_GPIO_OUTPUT)
-			gsm_mctrl_flowcontrolled = 0;
-		else {
-			gta_gsm_interrupts = 0;
-			s3c2410_gpio_setpin(S3C2410_GPH1, 1);
-			s3c2410_gpio_cfgpin(S3C2410_GPH1, S3C2410_GPH1_OUTP);
-			gsm_mctrl_flowcontrolled = 1;
-			nspy_add(NSPY_TYPE_SPECIAL, '-', jiffies);
-		}
-	}
+	/* todo - possibly remove AFC and do manual CTS */
 }
 
 static void s3c24xx_serial_break_ctl(struct uart_port *port, int break_state)
@@ -1005,7 +844,7 @@ static void s3c24xx_serial_set_termios(struct uart_port *port,
 
 	umcon = (termios->c_cflag & CRTSCTS) ? S3C2410_UMCOM_AFC : 0;
 
-	if (cfg->hwport == 0) {
+	if (cfg && (cfg->hwport == GSM_PORT)) {
 		if (umcon & S3C2410_UMCOM_AFC)
 			nspy_add(NSPY_TYPE_SPECIAL, 'M', jiffies);
 		else
@@ -1337,14 +1176,13 @@ static int s3c24xx_serial_suspend(struct platform_device *dev,
 
 	if (port) {
 		cfg = s3c24xx_port_to_cfg(port);
-		if (cfg->hwport == 0) {
+		if (cfg && (cfg->hwport == GSM_PORT)) {
 			nspy_add(NSPY_TYPE_SUSPEND, 'S', jiffies);
 			if ((s3c2410_gpio_getcfg(S3C2410_GPH1) ==
 			     S3C2410_GPIO_OUTPUT) && gta_gsm_interrupts) {
 				nspy_add(NSPY_TYPE_SUSPEND, '!', jiffies);
 				goto busy;
 			}
-			doing_serial_suspend_resume = 1;
 			nspy_add(NSPY_TYPE_SUSPEND, '{', jiffies);
 			umcon = rd_regl(port, S3C2410_UMCON);
 			if (umcon & S3C2410_UMCOM_AFC)
@@ -1353,10 +1191,8 @@ static int s3c24xx_serial_suspend(struct platform_device *dev,
 				nspy_add(NSPY_TYPE_SUSPEND, 'i', jiffies);
 		}
 		uart_suspend_port(&s3c24xx_uart_drv, port);
-		if (cfg->hwport == 0) {
+		if (cfg && (cfg->hwport == GSM_PORT))
 			nspy_add(NSPY_TYPE_SUSPEND, '}', jiffies);
-			doing_serial_suspend_resume = 0;
-		}
 	}
 
 	return 0;
@@ -1374,8 +1210,7 @@ static int s3c24xx_serial_resume(struct platform_device *dev)
 
 	if (port) {
 		cfg = s3c24xx_port_to_cfg(port);
-		if (cfg->hwport == 0) {
-			doing_serial_suspend_resume = 1;
+		if (cfg && (cfg->hwport == GSM_PORT)) {
 			nspy_add(NSPY_TYPE_RESUME, 'S', jiffies);
 			nspy_add(NSPY_TYPE_RESUME, '{', jiffies);
 			umcon = rd_regl(port, S3C2410_UMCON);
@@ -1390,18 +1225,13 @@ static int s3c24xx_serial_resume(struct platform_device *dev)
 
 		uart_resume_port(&s3c24xx_uart_drv, port);
 
-		if (cfg->hwport == 0) {
+		if (cfg && (cfg->hwport == GSM_PORT)) {
 			umcon = rd_regl(port, S3C2410_UMCON);
 			if (umcon & S3C2410_UMCOM_AFC)
 				nspy_add(NSPY_TYPE_RESUME, 'J', jiffies);
 			else
 				nspy_add(NSPY_TYPE_RESUME, 'j', jiffies);
 			nspy_add(NSPY_TYPE_RESUME, '}', jiffies);
-			/* stash away the port info
-			 * for the fake RX IRQ routine
-			 */
-			gta01_devptr = ourport;
-			doing_serial_suspend_resume = 0;
 		}
 	}
 
@@ -1551,7 +1381,7 @@ static int s3c2410_serial_resetport(struct uart_port *port,
 				    struct s3c2410_uartcfg *cfg)
 {
 	unsigned int umcon = rd_regl(port, S3C2410_UMCON);
-	if (cfg->hwport == 0) {
+	if (cfg && (cfg->hwport == GSM_PORT)) {
 		if (umcon & S3C2410_UMCOM_AFC)
 			nspy_add(NSPY_TYPE_SPECIAL, 'R', jiffies);
 		else
@@ -1716,7 +1546,7 @@ static int s3c2440_serial_resetport(struct uart_port *port,
 {
 	unsigned long ucon = rd_regl(port, S3C2410_UCON);
 	unsigned int umcon = rd_regl(port, S3C2410_UMCON);
-	if (cfg->hwport == 0) {
+	if (cfg && (cfg->hwport == GSM_PORT)) {
 		if (umcon & S3C2410_UMCOM_AFC)
 			nspy_add(NSPY_TYPE_SPECIAL, 'R', jiffies);
 		else
@@ -2015,7 +1845,7 @@ s3c24xx_serial_console_putchar(struct uart_port *port, int ch)
 	while (!s3c24xx_serial_console_txrdy(port, ufcon))
 		barrier();
 	wr_regb(cons_uart, S3C2410_UTXH, ch);
-	if (cfg->hwport == 0)
+	if (cfg && (cfg->hwport == GSM_PORT))
 		nspy_add(NSPY_TYPE_CONSOLE, ch, jiffies);
 
 	if (umcon & S3C2410_UMCOM_AFC)

 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: cleanup_GSM_flowcontrol.patch
Url: http://lists.openmoko.org/pipermail/openmoko-kernel/attachments/20080619/97d6b8a7/attachment.txt 


More information about the openmoko-kernel mailing list