1. Patchew
  2. Xen
  3. [PATCH v3 0/2] Add CONFIG_SYSTEM_SUSPEND and SCIF UART suspend/resume handlers
  4. View patch

[PATCH v3 2/2] xen/char: implement suspend/resume calls for SCIF driver

Mykola Kvach posted 2 patches 4 months, 3 weeks ago
[PATCH v3 2/2] xen/char: implement suspend/resume calls for SCIF driver
Posted by Mykola Kvach 4 months, 3 weeks ago 
From: Volodymyr Babchuk <volodymyr_babchuk@epam.com>

The changes have been tested only on the Renesas R-Car H3 Starter Kit board.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
In patch v2, I just added a CONFIG_SYSTEM_SUSPEND check around
the suspend/resume functions in the SCIF driver.
---
 xen/drivers/char/scif-uart.c | 40 ++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
index 757793ca45..888821a3b8 100644
--- a/xen/drivers/char/scif-uart.c
+++ b/xen/drivers/char/scif-uart.c
@@ -139,9 +139,8 @@ static void scif_uart_interrupt(int irq, void *data)
     }
 }
 
-static void __init scif_uart_init_preirq(struct serial_port *port)
+static void scif_uart_disable(struct scif_uart *uart)
 {
-    struct scif_uart *uart = port->uart;
     const struct port_params *params = uart->params;
 
     /*
@@ -155,6 +154,14 @@ static void __init scif_uart_init_preirq(struct serial_port *port)
 
     /* Reset TX/RX FIFOs */
     scif_writew(uart, SCIF_SCFCR, SCFCR_RFRST | SCFCR_TFRST);
+}
+
+static void scif_uart_init_preirq(struct serial_port *port)
+{
+    struct scif_uart *uart = port->uart;
+    const struct port_params *params = uart->params;
+
+    scif_uart_disable(uart);
 
     /* Clear all errors and flags */
     scif_readw(uart, params->status_reg);
@@ -271,6 +278,31 @@ static void scif_uart_stop_tx(struct serial_port *port)
     scif_writew(uart, SCIF_SCSCR, scif_readw(uart, SCIF_SCSCR) & ~SCSCR_TIE);
 }
 
+#ifdef CONFIG_SYSTEM_SUSPEND
+
+static void scif_uart_suspend(struct serial_port *port)
+{
+    struct scif_uart *uart = port->uart;
+
+    scif_uart_stop_tx(port);
+    scif_uart_disable(uart);
+}
+
+static void scif_uart_resume(struct serial_port *port)
+{
+    struct scif_uart *uart = port->uart;
+    const struct port_params *params = uart->params;
+    uint16_t ctrl;
+
+    scif_uart_init_preirq(port);
+
+    /* Enable TX/RX and Error Interrupts  */
+    ctrl = scif_readw(uart, SCIF_SCSCR);
+    scif_writew(uart, SCIF_SCSCR, ctrl | params->irq_flags);
+}
+
+#endif /* CONFIG_SYSTEM_SUSPEND */
+
 static struct uart_driver __read_mostly scif_uart_driver = {
     .init_preirq  = scif_uart_init_preirq,
     .init_postirq = scif_uart_init_postirq,
@@ -281,6 +313,10 @@ static struct uart_driver __read_mostly scif_uart_driver = {
     .start_tx     = scif_uart_start_tx,
     .stop_tx      = scif_uart_stop_tx,
     .vuart_info   = scif_vuart_info,
+#ifdef CONFIG_SYSTEM_SUSPEND
+    .suspend      = scif_uart_suspend,
+    .resume       = scif_uart_resume,
+#endif
 };
 
 static const struct dt_device_match scif_uart_dt_match[] __initconst =
-- 
2.48.1
Re: [PATCH v3 2/2] xen/char: implement suspend/resume calls for SCIF driver
Posted by Ayan Kumar Halder 4 months, 2 weeks ago 
Hi Mykola,

On 06/06/2025 11:11, Mykola Kvach wrote:
> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
>
>
> From: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
>
> The changes have been tested only on the Renesas R-Car H3 Starter Kit board.

The commit message need to explain what the change is and why it is needed.

Also ...

>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> In patch v2, I just added a CONFIG_SYSTEM_SUSPEND check around
> the suspend/resume functions in the SCIF driver.
> ---
>   xen/drivers/char/scif-uart.c | 40 ++++++++++++++++++++++++++++++++++--
>   1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
> index 757793ca45..888821a3b8 100644
> --- a/xen/drivers/char/scif-uart.c
> +++ b/xen/drivers/char/scif-uart.c
> @@ -139,9 +139,8 @@ static void scif_uart_interrupt(int irq, void *data)
>       }
>   }
>
> -static void __init scif_uart_init_preirq(struct serial_port *port)
> +static void scif_uart_disable(struct scif_uart *uart)
>   {
> -    struct scif_uart *uart = port->uart;
>       const struct port_params *params = uart->params;
>
>       /*
> @@ -155,6 +154,14 @@ static void __init scif_uart_init_preirq(struct serial_port *port)
>
>       /* Reset TX/RX FIFOs */
>       scif_writew(uart, SCIF_SCFCR, SCFCR_RFRST | SCFCR_TFRST);
> +}
> +
> +static void scif_uart_init_preirq(struct serial_port *port)
> +{
> +    struct scif_uart *uart = port->uart;
> +    const struct port_params *params = uart->params;
> +
> +    scif_uart_disable(uart);
>
>       /* Clear all errors and flags */
>       scif_readw(uart, params->status_reg);
> @@ -271,6 +278,31 @@ static void scif_uart_stop_tx(struct serial_port *port)
>       scif_writew(uart, SCIF_SCSCR, scif_readw(uart, SCIF_SCSCR) & ~SCSCR_TIE);
>   }
>
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +
> +static void scif_uart_suspend(struct serial_port *port)
> +{
> +    struct scif_uart *uart = port->uart;
> +
> +    scif_uart_stop_tx(port);
> +    scif_uart_disable(uart);
> +}
> +
> +static void scif_uart_resume(struct serial_port *port)
> +{
> +    struct scif_uart *uart = port->uart;
> +    const struct port_params *params = uart->params;
> +    uint16_t ctrl;
> +
> +    scif_uart_init_preirq(port);
> +
> +    /* Enable TX/RX and Error Interrupts  */
> +    ctrl = scif_readw(uart, SCIF_SCSCR);
> +    scif_writew(uart, SCIF_SCSCR, ctrl | params->irq_flags);

If you can give reference to a public doc which describe these 
registers, it will be great.

Otherwise, the changes look ok to me.

> +}
> +
> +#endif /* CONFIG_SYSTEM_SUSPEND */
> +
>   static struct uart_driver __read_mostly scif_uart_driver = {
>       .init_preirq  = scif_uart_init_preirq,
>       .init_postirq = scif_uart_init_postirq,
> @@ -281,6 +313,10 @@ static struct uart_driver __read_mostly scif_uart_driver = {
>       .start_tx     = scif_uart_start_tx,
>       .stop_tx      = scif_uart_stop_tx,
>       .vuart_info   = scif_vuart_info,
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +    .suspend      = scif_uart_suspend,
> +    .resume       = scif_uart_resume,
> +#endif
>   };
>
>   static const struct dt_device_match scif_uart_dt_match[] __initconst =
- Ayan
> --
> 2.48.1
>
>
Re: [PATCH v3 2/2] xen/char: implement suspend/resume calls for SCIF driver
Posted by Mykola Kvach 4 months, 2 weeks ago 
Hi Ayan

On Fri, Jun 13, 2025 at 5:36 PM Ayan Kumar Halder <ayankuma@amd.com> wrote:
>
> Hi Mykola,
>
> On 06/06/2025 11:11, Mykola Kvach wrote:
> > CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
> >
> >
> > From: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> >
> > The changes have been tested only on the Renesas R-Car H3 Starter Kit board.
>
> The commit message need to explain what the change is and why it is needed.

Thanks for the feedback. I thought the information from the commit
message title would be enough.

I will add a few more words to clarify what the change is and why it's
needed to the commit message body.

>
> Also ...
>
> >
> > Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> > In patch v2, I just added a CONFIG_SYSTEM_SUSPEND check around
> > the suspend/resume functions in the SCIF driver.
> > ---
> >   xen/drivers/char/scif-uart.c | 40 ++++++++++++++++++++++++++++++++++--
> >   1 file changed, 38 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
> > index 757793ca45..888821a3b8 100644
> > --- a/xen/drivers/char/scif-uart.c
> > +++ b/xen/drivers/char/scif-uart.c
> > @@ -139,9 +139,8 @@ static void scif_uart_interrupt(int irq, void *data)
> >       }
> >   }
> >
> > -static void __init scif_uart_init_preirq(struct serial_port *port)
> > +static void scif_uart_disable(struct scif_uart *uart)
> >   {
> > -    struct scif_uart *uart = port->uart;
> >       const struct port_params *params = uart->params;
> >
> >       /*
> > @@ -155,6 +154,14 @@ static void __init scif_uart_init_preirq(struct serial_port *port)
> >
> >       /* Reset TX/RX FIFOs */
> >       scif_writew(uart, SCIF_SCFCR, SCFCR_RFRST | SCFCR_TFRST);
> > +}
> > +
> > +static void scif_uart_init_preirq(struct serial_port *port)
> > +{
> > +    struct scif_uart *uart = port->uart;
> > +    const struct port_params *params = uart->params;
> > +
> > +    scif_uart_disable(uart);
> >
> >       /* Clear all errors and flags */
> >       scif_readw(uart, params->status_reg);
> > @@ -271,6 +278,31 @@ static void scif_uart_stop_tx(struct serial_port *port)
> >       scif_writew(uart, SCIF_SCSCR, scif_readw(uart, SCIF_SCSCR) & ~SCSCR_TIE);
> >   }
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +
> > +static void scif_uart_suspend(struct serial_port *port)
> > +{
> > +    struct scif_uart *uart = port->uart;
> > +
> > +    scif_uart_stop_tx(port);
> > +    scif_uart_disable(uart);
> > +}
> > +
> > +static void scif_uart_resume(struct serial_port *port)
> > +{
> > +    struct scif_uart *uart = port->uart;
> > +    const struct port_params *params = uart->params;
> > +    uint16_t ctrl;
> > +
> > +    scif_uart_init_preirq(port);
> > +
> > +    /* Enable TX/RX and Error Interrupts  */
> > +    ctrl = scif_readw(uart, SCIF_SCSCR);
> > +    scif_writew(uart, SCIF_SCSCR, ctrl | params->irq_flags);
>
> If you can give reference to a public doc which describe these
> registers, it will be great.

I don't think I can share documentation for the board I used for testing
this commit.

However, I searched for Linux boards that use the same driver and SCIF
register set with available documentation, and I found a few boards with
open documentation, for example, RZ/G1H or RZ/G1M:

https://www.renesas.com/en/products/microcontrollers-microprocessors/rz-mpus/rzg1m-ultra-high-performance-microprocessors-15ghz-dual-core-arm-cortex-a15-cpus-3d-graphics-and-video

>
> Otherwise, the changes look ok to me.
>
> > +}
> > +
> > +#endif /* CONFIG_SYSTEM_SUSPEND */
> > +
> >   static struct uart_driver __read_mostly scif_uart_driver = {
> >       .init_preirq  = scif_uart_init_preirq,
> >       .init_postirq = scif_uart_init_postirq,
> > @@ -281,6 +313,10 @@ static struct uart_driver __read_mostly scif_uart_driver = {
> >       .start_tx     = scif_uart_start_tx,
> >       .stop_tx      = scif_uart_stop_tx,
> >       .vuart_info   = scif_vuart_info,
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +    .suspend      = scif_uart_suspend,
> > +    .resume       = scif_uart_resume,
> > +#endif
> >   };
> >
> >   static const struct dt_device_match scif_uart_dt_match[] __initconst =
> - Ayan
> > --
> > 2.48.1
> >
> >

Best regards,
Mykola

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.