andy git 06/15 suspend/resume observations
Sean McNeil
sean at mcneil.com
Thu Jun 19 23:39:32 CEST 2008
Please excuse my ignorance (once again), but I'm unclear about the
manipulation required for GSM in the serial driver. Is this because of
multiplexing? Isn't the GSM simply a uart with hw flow control and RTS
causing an interrupt when in suspend? I noticed several places that test
for GSM_PORT and thought this might be able to be generic across all of
the ports.
Mike (mwester) wrote:
> 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(>a01_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)
>
>
>
More information about the openmoko-kernel
mailing list
