linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] add wakeup_irq for in-band wakeup support
@ 2020-05-06  7:23 Claire Chang
  2020-05-06  7:23 ` [PATCH 1/3] serdev: ttyport: add devt for tty port Claire Chang
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Claire Chang @ 2020-05-06  7:23 UTC (permalink / raw)
  To: robh, gregkh, jslaby, long.cheng, changqi.hu
  Cc: linux-serial, linux-mediatek, linux-kernel, Claire Chang

Since some uart controllers may be off in S3, add additional wakeup_irq
to support in-band wakeup.

Claire Chang (3):
  serdev: ttyport: add devt for tty port
  tty: serial_core: add wakeup_irq to support in-band wakeup
  uart: mediatek: move the in-band wakeup logic to core

 drivers/tty/serdev/serdev-ttyport.c |  2 ++
 drivers/tty/serial/8250/8250_core.c |  1 +
 drivers/tty/serial/8250/8250_mtk.c  | 24 +++---------------------
 drivers/tty/serial/serial_core.c    |  8 +++++---
 include/linux/serial_core.h         |  1 +
 5 files changed, 12 insertions(+), 24 deletions(-)

-- 
2.26.2.526.g744177e7f7-goog


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

* [PATCH 1/3] serdev: ttyport: add devt for tty port
  2020-05-06  7:23 [PATCH 0/3] add wakeup_irq for in-band wakeup support Claire Chang
@ 2020-05-06  7:23 ` Claire Chang
  2020-05-15 12:46   ` Greg KH
  2020-05-18 14:56   ` Johan Hovold
  2020-05-06  7:23 ` [PATCH 2/3] tty: serial_core: add wakeup_irq to support in-band wakeup Claire Chang
  2020-05-06  7:23 ` [PATCH 3/3] uart: mediatek: move the in-band wakeup logic to core Claire Chang
  2 siblings, 2 replies; 7+ messages in thread
From: Claire Chang @ 2020-05-06  7:23 UTC (permalink / raw)
  To: robh, gregkh, jslaby, long.cheng, changqi.hu
  Cc: linux-serial, linux-mediatek, linux-kernel, Claire Chang

serial_match_port() uses devt to match devices. However, when serdev
registers a tty port, devt has never been set. This makes
device_find_child() always return NULL.

Assign devt in serdev_tty_port_register() to fix this.

Signed-off-by: Claire Chang <tientzu@chromium.org>
---
 drivers/tty/serdev/serdev-ttyport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index d367803e2044f..9238119173a47 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -267,6 +267,7 @@ struct device *serdev_tty_port_register(struct tty_port *port,
 {
 	struct serdev_controller *ctrl;
 	struct serport *serport;
+	dev_t devt = MKDEV(drv->major, drv->minor_start) + idx;
 	int ret;
 
 	if (!port || !drv || !parent)
@@ -282,6 +283,7 @@ struct device *serdev_tty_port_register(struct tty_port *port,
 	serport->tty_drv = drv;
 
 	ctrl->ops = &ctrl_ops;
+	ctrl->dev.devt = devt;
 
 	port->client_ops = &client_ops;
 	port->client_data = ctrl;
-- 
2.26.2.526.g744177e7f7-goog


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

* [PATCH 2/3] tty: serial_core: add wakeup_irq to support in-band wakeup
  2020-05-06  7:23 [PATCH 0/3] add wakeup_irq for in-band wakeup support Claire Chang
  2020-05-06  7:23 ` [PATCH 1/3] serdev: ttyport: add devt for tty port Claire Chang
@ 2020-05-06  7:23 ` Claire Chang
  2020-05-06  7:23 ` [PATCH 3/3] uart: mediatek: move the in-band wakeup logic to core Claire Chang
  2 siblings, 0 replies; 7+ messages in thread
From: Claire Chang @ 2020-05-06  7:23 UTC (permalink / raw)
  To: robh, gregkh, jslaby, long.cheng, changqi.hu
  Cc: linux-serial, linux-mediatek, linux-kernel, Claire Chang

Since some uart controllers may be off in S3, we won't be able to use
the normal in-band wakeup.

Take 8250_mtk.c as an example. The driver needs to allocate an edge
sensitive interrupt as the wakeup_irq and use an addtional pinctrl to
reconfigure Rx pin to normal GPIO in sleep state. Once host detects Rx
falling, an interrupt is triggered, and the system leaves sleep state.

Add the wakeup_irq logic in core to simplify and make the code more
generic. Also, we can align with the original wakeup behavior and
power/wakeup node.

Signed-off-by: Claire Chang <tientzu@chromium.org>
---
 drivers/tty/serial/8250/8250_core.c | 1 +
 drivers/tty/serial/serial_core.c    | 8 +++++---
 include/linux/serial_core.h         | 1 +
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 45d9117cab680..06214e9fdc8ff 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -1001,6 +1001,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
 		uart->port.membase      = up->port.membase;
 		uart->port.irq          = up->port.irq;
 		uart->port.irqflags     = up->port.irqflags;
+		uart->port.wakeup_irq	= up->port.wakeup_irq;
 		uart->port.uartclk      = up->port.uartclk;
 		uart->port.fifosize     = up->port.fifosize;
 		uart->port.regshift     = up->port.regshift;
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 66a5e2faf57ea..1796a33986613 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2165,12 +2165,13 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 	struct tty_port *port = &state->port;
 	struct device *tty_dev;
 	struct uart_match match = {uport, drv};
+	int irq = uport->wakeup_irq > 0 ? uport->wakeup_irq : uport->irq;
 
 	mutex_lock(&port->mutex);
 
 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (tty_dev && device_may_wakeup(tty_dev)) {
-		enable_irq_wake(uport->irq);
+		enable_irq_wake(irq);
 		put_device(tty_dev);
 		mutex_unlock(&port->mutex);
 		return 0;
@@ -2228,13 +2229,14 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
 	struct device *tty_dev;
 	struct uart_match match = {uport, drv};
 	struct ktermios termios;
+	int irq = uport->wakeup_irq > 0 ? uport->wakeup_irq : uport->irq;
 
 	mutex_lock(&port->mutex);
 
 	tty_dev = device_find_child(uport->dev, &match, serial_match_port);
 	if (!uport->suspended && device_may_wakeup(tty_dev)) {
-		if (irqd_is_wakeup_set(irq_get_irq_data((uport->irq))))
-			disable_irq_wake(uport->irq);
+		if (irqd_is_wakeup_set(irq_get_irq_data((irq))))
+			disable_irq_wake(irq);
 		put_device(tty_dev);
 		mutex_unlock(&port->mutex);
 		return 0;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 92f5eba860528..5764687b90a36 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -136,6 +136,7 @@ struct uart_port {
 						  struct serial_iso7816 *iso7816);
 	unsigned int		irq;			/* irq number */
 	unsigned long		irqflags;		/* irq flags  */
+	unsigned int		wakeup_irq;		/* wakeup irq number */
 	unsigned int		uartclk;		/* base uart clock */
 	unsigned int		fifosize;		/* tx fifo size */
 	unsigned char		x_char;			/* xon/xoff char */
-- 
2.26.2.526.g744177e7f7-goog


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

* [PATCH 3/3] uart: mediatek: move the in-band wakeup logic to core
  2020-05-06  7:23 [PATCH 0/3] add wakeup_irq for in-band wakeup support Claire Chang
  2020-05-06  7:23 ` [PATCH 1/3] serdev: ttyport: add devt for tty port Claire Chang
  2020-05-06  7:23 ` [PATCH 2/3] tty: serial_core: add wakeup_irq to support in-band wakeup Claire Chang
@ 2020-05-06  7:23 ` Claire Chang
  2 siblings, 0 replies; 7+ messages in thread
From: Claire Chang @ 2020-05-06  7:23 UTC (permalink / raw)
  To: robh, gregkh, jslaby, long.cheng, changqi.hu
  Cc: linux-serial, linux-mediatek, linux-kernel, Claire Chang

Move the in-band wakeup logic to core so that we can control the wakeup
behavior by serdev controller's power/wakeup node and align with other
serial drivers.

Signed-off-by: Claire Chang <tientzu@chromium.org>
---
 drivers/tty/serial/8250/8250_mtk.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c
index f839380c2f4c1..52cb41e4e493d 100644
--- a/drivers/tty/serial/8250/8250_mtk.c
+++ b/drivers/tty/serial/8250/8250_mtk.c
@@ -71,7 +71,6 @@ struct mtk8250_data {
 #ifdef CONFIG_SERIAL_8250_DMA
 	enum dma_rx_status	rx_status;
 #endif
-	int			rx_wakeup_irq;
 };
 
 /* flow control mode */
@@ -496,6 +495,8 @@ static int mtk8250_probe(struct platform_device *pdev)
 	struct uart_8250_port uart = {};
 	struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	struct resource *wakeup_irq =
+		platform_get_resource(pdev, IORESOURCE_IRQ, 1);
 	struct mtk8250_data *data;
 	int err;
 
@@ -525,6 +526,7 @@ static int mtk8250_probe(struct platform_device *pdev)
 	spin_lock_init(&uart.port.lock);
 	uart.port.mapbase = regs->start;
 	uart.port.irq = irq->start;
+	uart.port.wakeup_irq = wakeup_irq ? wakeup_irq->start : -ENXIO;
 	uart.port.pm = mtk8250_do_pm;
 	uart.port.type = PORT_16550;
 	uart.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT;
@@ -556,8 +558,6 @@ static int mtk8250_probe(struct platform_device *pdev)
 	if (data->line < 0)
 		return data->line;
 
-	data->rx_wakeup_irq = platform_get_irq_optional(pdev, 1);
-
 	return 0;
 }
 
@@ -581,23 +581,9 @@ static int mtk8250_remove(struct platform_device *pdev)
 static int __maybe_unused mtk8250_suspend(struct device *dev)
 {
 	struct mtk8250_data *data = dev_get_drvdata(dev);
-	int irq = data->rx_wakeup_irq;
-	int err;
 
 	serial8250_suspend_port(data->line);
-
 	pinctrl_pm_select_sleep_state(dev);
-	if (irq >= 0) {
-		err = enable_irq_wake(irq);
-		if (err) {
-			dev_err(dev,
-				"failed to enable irq wake on IRQ %d: %d\n",
-				irq, err);
-			pinctrl_pm_select_default_state(dev);
-			serial8250_resume_port(data->line);
-			return err;
-		}
-	}
 
 	return 0;
 }
@@ -605,12 +591,8 @@ static int __maybe_unused mtk8250_suspend(struct device *dev)
 static int __maybe_unused mtk8250_resume(struct device *dev)
 {
 	struct mtk8250_data *data = dev_get_drvdata(dev);
-	int irq = data->rx_wakeup_irq;
 
-	if (irq >= 0)
-		disable_irq_wake(irq);
 	pinctrl_pm_select_default_state(dev);
-
 	serial8250_resume_port(data->line);
 
 	return 0;
-- 
2.26.2.526.g744177e7f7-goog


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

* Re: [PATCH 1/3] serdev: ttyport: add devt for tty port
  2020-05-06  7:23 ` [PATCH 1/3] serdev: ttyport: add devt for tty port Claire Chang
@ 2020-05-15 12:46   ` Greg KH
  2020-05-18 10:04     ` Claire Chang
  2020-05-18 14:56   ` Johan Hovold
  1 sibling, 1 reply; 7+ messages in thread
From: Greg KH @ 2020-05-15 12:46 UTC (permalink / raw)
  To: Claire Chang
  Cc: robh, jslaby, long.cheng, changqi.hu, linux-serial,
	linux-mediatek, linux-kernel

On Wed, May 06, 2020 at 03:23:12PM +0800, Claire Chang wrote:
> serial_match_port() uses devt to match devices. However, when serdev
> registers a tty port, devt has never been set. This makes
> device_find_child() always return NULL.
> 
> Assign devt in serdev_tty_port_register() to fix this.
> 
> Signed-off-by: Claire Chang <tientzu@chromium.org>
> ---
>  drivers/tty/serdev/serdev-ttyport.c | 2 ++
>  1 file changed, 2 insertions(+)

So is existing code broken because of this?  Or does no one ever call
device_find_child() on this?  Who needs/uses this?

thanks,

greg k-h

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

* Re: [PATCH 1/3] serdev: ttyport: add devt for tty port
  2020-05-15 12:46   ` Greg KH
@ 2020-05-18 10:04     ` Claire Chang
  0 siblings, 0 replies; 7+ messages in thread
From: Claire Chang @ 2020-05-18 10:04 UTC (permalink / raw)
  To: Greg KH
  Cc: robh, jslaby, long.cheng, changqi.hu, linux-serial,
	moderated list:ARM/Mediatek SoC support, lkml

On Fri, May 15, 2020 at 8:46 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, May 06, 2020 at 03:23:12PM +0800, Claire Chang wrote:
> > serial_match_port() uses devt to match devices. However, when serdev
> > registers a tty port, devt has never been set. This makes
> > device_find_child() always return NULL.
> >
> > Assign devt in serdev_tty_port_register() to fix this.
> >
> > Signed-off-by: Claire Chang <tientzu@chromium.org>
> > ---
> >  drivers/tty/serdev/serdev-ttyport.c | 2 ++
> >  1 file changed, 2 insertions(+)
>
> So is existing code broken because of this?  Or does no one ever call
> device_find_child() on this?  Who needs/uses this?
>
> thanks,
>
> greg k-h

I'm not sure. Our use case is to control the wake on bluetooth
behavior by the power/wakeup node.

`readlink -f /sys/class/bluetooth/hci0`
/sys/devices/platform/soc/11003000.serial/serial0/serial0-0/bluetooth/hci0

and we'd like to use
`/sys/devices/platform/soc/11003000.serial/serial0/power/wakeup` to
decide whether to enable the in-band wakeup on uart host side.

Thanks,
Claire

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

* Re: [PATCH 1/3] serdev: ttyport: add devt for tty port
  2020-05-06  7:23 ` [PATCH 1/3] serdev: ttyport: add devt for tty port Claire Chang
  2020-05-15 12:46   ` Greg KH
@ 2020-05-18 14:56   ` Johan Hovold
  1 sibling, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2020-05-18 14:56 UTC (permalink / raw)
  To: Claire Chang
  Cc: robh, gregkh, jslaby, long.cheng, changqi.hu, linux-serial,
	linux-mediatek, linux-kernel

On Wed, May 06, 2020 at 03:23:12PM +0800, Claire Chang wrote:
> serial_match_port() uses devt to match devices. However, when serdev
> registers a tty port, devt has never been set. This makes
> device_find_child() always return NULL.
> 
> Assign devt in serdev_tty_port_register() to fix this.
> 
> Signed-off-by: Claire Chang <tientzu@chromium.org>
> ---
>  drivers/tty/serdev/serdev-ttyport.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
> index d367803e2044f..9238119173a47 100644
> --- a/drivers/tty/serdev/serdev-ttyport.c
> +++ b/drivers/tty/serdev/serdev-ttyport.c
> @@ -267,6 +267,7 @@ struct device *serdev_tty_port_register(struct tty_port *port,
>  {
>  	struct serdev_controller *ctrl;
>  	struct serport *serport;
> +	dev_t devt = MKDEV(drv->major, drv->minor_start) + idx;
>  	int ret;
>  
>  	if (!port || !drv || !parent)
> @@ -282,6 +283,7 @@ struct device *serdev_tty_port_register(struct tty_port *port,
>  	serport->tty_drv = drv;
>  
>  	ctrl->ops = &ctrl_ops;
> +	ctrl->dev.devt = devt;

This is conceptually wrong. A serdev controller is not a tty class
device with a corresponding character device.

It seems you need to rethink how serial core should handle the wakeup
flags with respect to serdev.

>  
>  	port->client_ops = &client_ops;
>  	port->client_data = ctrl;

Johan

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

end of thread, other threads:[~2020-05-18 14:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06  7:23 [PATCH 0/3] add wakeup_irq for in-band wakeup support Claire Chang
2020-05-06  7:23 ` [PATCH 1/3] serdev: ttyport: add devt for tty port Claire Chang
2020-05-15 12:46   ` Greg KH
2020-05-18 10:04     ` Claire Chang
2020-05-18 14:56   ` Johan Hovold
2020-05-06  7:23 ` [PATCH 2/3] tty: serial_core: add wakeup_irq to support in-band wakeup Claire Chang
2020-05-06  7:23 ` [PATCH 3/3] uart: mediatek: move the in-band wakeup logic to core Claire Chang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).