Re: [PATCH v5] xen/char: implement suspend/resume calls for SCIF driver

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

@Stefano Stabellini @Michal Orzel @Julien Grall @Bertrand Marquis ping

On Thu, Aug 7, 2025 at 8:16 AM Mykola Kvach <xakep.amatop@xxxxxxxxx> wrote:
>
> From: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
>
> Implement suspend and resume callbacks for the SCIF UART driver,
> enabled when CONFIG_SYSTEM_SUSPEND is set. This allows proper
> handling of UART state across system suspend/resume cycles.
>
> Tested on Renesas R-Car H3 Starter Kit.
>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
> In patch v5, there are no changes at all;
> it was done just to trigger a review.
>
> In patch v4, enhance commit message, no functional changes
>
> 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
>