| Message ID | 20250219142454.2761556-1-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | serial: sh-sci: Save and restore SCDL and SCCKS | expand |
Hi, Geert, On 19.02.2025 16:24, Geert Uytterhoeven wrote: > On (H)SCIF with a Baud Rate Generator for External Clock (BRG), there > are multiple ways to configure the requested serial speed. If firmware > uses a different method than Linux, and if any debug info is printed > after the Bit Rate Register (SCBRR) is restored, but before termios is > reconfigured (which configures the alternative method), the system may > lock-up during resume. > > Fix this by saving and restoring the contents of the Frequency Division > (DL) and Clock Select (CKS) registers as well. Keeping the thinks RZ/G3S focused (as proposed in the first versions of this support), I missed there might be other registers on other IP variants. Reviewing the full list of registers from [1], maybe the HSSRR and SCPCR should be saved/restored as well? Thank you, Claudiu [1] https://elixir.bootlin.com/linux/v6.13.3/source/drivers/tty/serial/sh-sci.h#L14 > > Fixes: 22a6984c5b5df8ea ("serial: sh-sci: Update the suspend/resume support") > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> For this patch: Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > This can be reproduced on e.g. Salvator-X(S) by enabling the debug > print in sci_brg_calc(), and using s2ram with no_console_suspend. > --- > drivers/tty/serial/sh-sci.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > index df6512c9c0ff28db..70f34b8a93888eb9 100644 > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -109,6 +109,8 @@ struct sci_suspend_regs { > u16 scscr; > u16 scfcr; > u16 scsptr; > + u16 scdl; > + u16 sccks; > u8 scbrr; > u8 semr; > }; > @@ -3571,6 +3573,10 @@ static void sci_console_save(struct sci_port *s) > regs->scfcr = sci_serial_in(port, SCFCR); > if (sci_getreg(port, SCSPTR)->size) > regs->scsptr = sci_serial_in(port, SCSPTR); > + if (sci_getreg(port, SCDL)->size) > + regs->scdl = sci_serial_in(port, SCDL); > + if (sci_getreg(port, SCCKS)->size) > + regs->sccks = sci_serial_in(port, SCCKS); > if (sci_getreg(port, SCBRR)->size) > regs->scbrr = sci_serial_in(port, SCBRR); > if (sci_getreg(port, SEMR)->size) > @@ -3590,6 +3596,10 @@ static void sci_console_restore(struct sci_port *s) > sci_serial_out(port, SCFCR, regs->scfcr); > if (sci_getreg(port, SCSPTR)->size) > sci_serial_out(port, SCSPTR, regs->scsptr); > + if (sci_getreg(port, SCDL)->size) > + sci_serial_out(port, SCDL, regs->scdl); > + if (sci_getreg(port, SCCKS)->size) > + sci_serial_out(port, SCCKS, regs->sccks); > if (sci_getreg(port, SCBRR)->size) > sci_serial_out(port, SCBRR, regs->scbrr); > if (sci_getreg(port, SEMR)->size)
Hi Claudiu, On Wed, 19 Feb 2025 at 16:04, Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote: > On 19.02.2025 16:24, Geert Uytterhoeven wrote: > > On (H)SCIF with a Baud Rate Generator for External Clock (BRG), there > > are multiple ways to configure the requested serial speed. If firmware > > uses a different method than Linux, and if any debug info is printed > > after the Bit Rate Register (SCBRR) is restored, but before termios is > > reconfigured (which configures the alternative method), the system may > > lock-up during resume. > > > > Fix this by saving and restoring the contents of the Frequency Division > > (DL) and Clock Select (CKS) registers as well. > > Keeping the thinks RZ/G3S focused (as proposed in the first versions of > this support), I missed there might be other registers on other IP variants. Indeed. I forgot to mention I didn't check other registers... > Reviewing the full list of registers from [1], maybe the HSSRR and > SCPCR should be saved/restored as well? Agreed. Late restoring these is probably never fatal, though. > [1] > https://elixir.bootlin.com/linux/v6.13.3/source/drivers/tty/serial/sh-sci.h#L14 > > > > > Fixes: 22a6984c5b5df8ea ("serial: sh-sci: Update the suspend/resume support") > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > For this patch: > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Thanks! Gr{oetje,eeting}s, Geert
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index df6512c9c0ff28db..70f34b8a93888eb9 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -109,6 +109,8 @@ struct sci_suspend_regs { u16 scscr; u16 scfcr; u16 scsptr; + u16 scdl; + u16 sccks; u8 scbrr; u8 semr; }; @@ -3571,6 +3573,10 @@ static void sci_console_save(struct sci_port *s) regs->scfcr = sci_serial_in(port, SCFCR); if (sci_getreg(port, SCSPTR)->size) regs->scsptr = sci_serial_in(port, SCSPTR); + if (sci_getreg(port, SCDL)->size) + regs->scdl = sci_serial_in(port, SCDL); + if (sci_getreg(port, SCCKS)->size) + regs->sccks = sci_serial_in(port, SCCKS); if (sci_getreg(port, SCBRR)->size) regs->scbrr = sci_serial_in(port, SCBRR); if (sci_getreg(port, SEMR)->size) @@ -3590,6 +3596,10 @@ static void sci_console_restore(struct sci_port *s) sci_serial_out(port, SCFCR, regs->scfcr); if (sci_getreg(port, SCSPTR)->size) sci_serial_out(port, SCSPTR, regs->scsptr); + if (sci_getreg(port, SCDL)->size) + sci_serial_out(port, SCDL, regs->scdl); + if (sci_getreg(port, SCCKS)->size) + sci_serial_out(port, SCCKS, regs->sccks); if (sci_getreg(port, SCBRR)->size) sci_serial_out(port, SCBRR, regs->scbrr); if (sci_getreg(port, SEMR)->size)
On (H)SCIF with a Baud Rate Generator for External Clock (BRG), there are multiple ways to configure the requested serial speed. If firmware uses a different method than Linux, and if any debug info is printed after the Bit Rate Register (SCBRR) is restored, but before termios is reconfigured (which configures the alternative method), the system may lock-up during resume. Fix this by saving and restoring the contents of the Frequency Division (DL) and Clock Select (CKS) registers as well. Fixes: 22a6984c5b5df8ea ("serial: sh-sci: Update the suspend/resume support") Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- This can be reproduced on e.g. Salvator-X(S) by enabling the debug print in sci_brg_calc(), and using s2ram with no_console_suspend. --- drivers/tty/serial/sh-sci.c | 10 ++++++++++ 1 file changed, 10 insertions(+)