[v4,1/4] serial: sh-sci: Update the suspend/resume support

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

The Renesas RZ/G3S supports a power saving mode where power to most of the
SoC components is turned off. When returning from this power saving mode,
SoC components need to be re-configured.

The SCIFs on the Renesas RZ/G3S need to be re-configured as well when
returning from this power saving mode. The sh-sci code already configures
the SCIF clocks, power domain and registers by calling uart_resume_port()
in sci_resume(). On suspend path the SCIF UART ports are suspended
accordingly (by calling uart_suspend_port() in sci_suspend()). The only
missing setting is the reset signal. For this assert/de-assert the reset
signal on driver suspend/resume.

In case the no_console_suspend is specified by the user, the registers need
to be saved on suspend path and restore on resume path. To do this the
sci_console_setup() function was added. There is no need to cache/restore
the status or FIFO registers. Only the control registers. To differentiate
b/w these, the struct sci_port_params::regs was updated with a new member
that specifies if the register needs to be chached on suspend. Only the
RZ_SCIFA instances were updated with this new support as the hardware for
the rest of variants was missing for testing.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v4:
- none

Changes in v3:
- none

Changes in v2:
- rebased on top of the update version of patch 2/8 from
  this series

 drivers/tty/serial/sh-sci.c | 53 ++++++++++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 9 deletions(-)

Hi Claudiu,

Thanks for your patch!

On Mon, Jan 20, 2025 at 2:09 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The Renesas RZ/G3S supports a power saving mode where power to most of the
> SoC components is turned off. When returning from this power saving mode,
> SoC components need to be re-configured.
>
> The SCIFs on the Renesas RZ/G3S need to be re-configured as well when
> returning from this power saving mode. The sh-sci code already configures
> the SCIF clocks, power domain and registers by calling uart_resume_port()
> in sci_resume(). On suspend path the SCIF UART ports are suspended
> accordingly (by calling uart_suspend_port() in sci_suspend()). The only
> missing setting is the reset signal. For this assert/de-assert the reset
> signal on driver suspend/resume.
>
> In case the no_console_suspend is specified by the user, the registers need
> to be saved on suspend path and restore on resume path. To do this the
> sci_console_setup() function was added. There is no need to cache/restore
> the status or FIFO registers. Only the control registers. To differentiate
> b/w these, the struct sci_port_params::regs was updated with a new member
> that specifies if the register needs to be chached on suspend. Only the

cached

> RZ_SCIFA instances were updated with this new support as the hardware for
> the rest of variants was missing for testing.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -101,7 +101,7 @@ enum SCI_CLKS {
>                 if ((_port)->sampling_rate_mask & SCI_SR((_sr)))
>
>  struct plat_sci_reg {
> -       u8 offset, size;
> +       u8 offset, size, suspend_cacheable;

This increases the size of sci_port_params[] by 300 bytes.
Using bitfields would mitigate that:

    struct plat_sci_reg {
            u16 offset:8;
            u16 size:5;
            u16 suspend_cacheable:1;
    };

(if we ever need more bits, the size member can store an enum value
 instead of the actual size (8 or 16 bits) of the register).

>  };
>
>  struct sci_port_params {
> @@ -134,6 +134,8 @@ struct sci_port {
>         struct dma_chan                 *chan_tx;
>         struct dma_chan                 *chan_rx;
>
> +       struct reset_control            *rstc;
> +
>  #ifdef CONFIG_SERIAL_SH_SCI_DMA
>         struct dma_chan                 *chan_tx_saved;
>         struct dma_chan                 *chan_rx_saved;
> @@ -153,6 +155,7 @@ struct sci_port {
>         int                             rx_trigger;
>         struct timer_list               rx_fifo_timer;
>         int                             rx_fifo_timeout;
> +       unsigned int                    console_cached_regs[SCIx_NR_REGS];

u16, as all registers are 8 or 16 bit wide.

We reserve space for 20 registers, but at most 6 will be used.
This has a rather big impact on the size of sci_ports[], as
CONFIG_SERIAL_SH_SCI_NR_UARTS defaults to 18.

Also, this space is used/needed only if:
  - CONFIG_PM_SLEEP=y,
  - CONFIG_SERIAL_CORE_CONSOLE=y (see uart_console()),
  - The port is actually used as a console (unfortunately the user
    can specify multiple console=ttySC<N> command line parameters, in
    addition to chosen/stdout-path).

>         u16                             hscif_tot;
>
>         bool has_rtscts;
> @@ -300,17 +303,17 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = {
>          */
>         [SCIx_RZ_SCIFA_REGTYPE] = {
>                 .regs = {
> -                       [SCSMR]         = { 0x00, 16 },
> -                       [SCBRR]         = { 0x02,  8 },
> -                       [SCSCR]         = { 0x04, 16 },
> +                       [SCSMR]         = { 0x00, 16, 1 },
> +                       [SCBRR]         = { 0x02,  8, 1 },
> +                       [SCSCR]         = { 0x04, 16, 1 },
>                         [SCxTDR]        = { 0x06,  8 },
>                         [SCxSR]         = { 0x08, 16 },
>                         [SCxRDR]        = { 0x0A,  8 },
> -                       [SCFCR]         = { 0x0C, 16 },
> +                       [SCFCR]         = { 0x0C, 16, 1 },
>                         [SCFDR]         = { 0x0E, 16 },
> -                       [SCSPTR]        = { 0x10, 16 },
> +                       [SCSPTR]        = { 0x10, 16, 1 },
>                         [SCLSR]         = { 0x12, 16 },
> -                       [SEMR]          = { 0x14, 8 },
> +                       [SEMR]          = { 0x14, 8, 1 },

Note that the driver always writes zero to SEMR.

>                 },
>                 .fifosize = 16,
>                 .overrun_reg = SCLSR,
> @@ -3374,6 +3377,7 @@ static struct plat_sci_port *sci_parse_dt(struct platform_device *pdev,
>         }
>
>         sp = &sci_ports[id];
> +       sp->rstc = rstc;
>         *dev_id = id;
>
>         p->type = SCI_OF_TYPE(data);
> @@ -3546,13 +3550,34 @@ static int sci_probe(struct platform_device *dev)
>         return 0;
>  }
>
> +static void sci_console_setup(struct sci_port *s, bool save)

sci_console_save_restore()?

> +{
> +       for (u16 i = 0; i < SCIx_NR_REGS; i++) {

unsigned int

> +               struct uart_port *port = &s->port;
> +
> +               if (!s->params->regs[i].suspend_cacheable)
> +                       continue;
> +
> +               if (save)
> +                       s->console_cached_regs[i] = sci_serial_in(port, i);
> +               else
> +                       sci_serial_out(port, i, s->console_cached_regs[i]);
> +       }
> +}

Gr{oetje,eeting}s,

                        Geert