linux-kernel - Re: [PATCH v4 1/4] serial: sh-sci: Update the suspend/resume support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMuHMdVTpffbK8Nv_b45o=b76ageQtfH693CW=ob4hF28NBsBw@mail.gmail.com>
Date: Mon, 27 Jan 2025 17:44:48 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Claudiu Beznea <claudiu.beznea@...on.dev>
Cc: magnus.damm@...il.com, robh@...nel.org, krzk+dt@...nel.org, 
	conor+dt@...nel.org, gregkh@...uxfoundation.org, jirislaby@...nel.org, 
	p.zabel@...gutronix.de, claudiu.beznea.uj@...renesas.com, 
	wsa+renesas@...g-engineering.com, prabhakar.mahadev-lad.rj@...renesas.com, 
	linux-renesas-soc@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH v4 1/4] serial: sh-sci: Update the suspend/resume support

Hi Claudiu,

On Mon, 27 Jan 2025 at 13:23, Claudiu Beznea <claudiu.beznea@...on.dev> wrote:
> On 27.01.2025 11:19, Geert Uytterhoeven wrote:
> > On Mon, 27 Jan 2025 at 09:44, Claudiu Beznea <claudiu.beznea@...on.dev> wrote:
> >> On 24.01.2025 12:53, Geert Uytterhoeven wrote:
> >>> On Mon, Jan 20, 2025 at 2:09 PM Claudiu <claudiu.beznea@...on.dev> wrote:
> >>>> From: Claudiu Beznea <claudiu.beznea.uj@...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@...renesas.com>

> > While storing the suspend_cacheable flag wouldn't cost any storage
> > space anymore using bitfields, I am wondering if it would be worthwhile
> > to have explicit code to save/restore registers, instead of looping
> > over all of them and checking the flag. I.e.
> >
> >     u16 saved_scmsr;
> >     u16 saved_scscr;
> >     u8 saved_scbrr;
> >     ...
> >     u8 saved_semr;
> >
> >     /* Save omnipresent registers */
> >     s->saved_scmsr = sci_serial_in(port, SCSMR);
> >     ...
> >     /* Save optional registers */
> >     if (sci_getreg(port, SEMR)->size)
> >             s->saved_semr = sci_serial_in(port, SEMR);
> >
> > That would make it apply to all SCI variants, not just for SCIFA,
> > while increasing sci_port by only 10 bytes/port. And 10 bytes/port is
> > probably not worth to be protected by an #ifdef...
>
> That was the other approach I thought about when working on this patch. I
> chose the one proposed in this patch as it looked to me simpler to extend
> to other registers, if needed (just enable proper flag in
> sci_port_params[]). And needed less changes for the code saving/restoring
> the registers.
>
> If you prefer your version let me know and I'll switch to it.

As this driver is also used on smaller systems (e.g. SH), I'd go for
the solution that leads to the smallest increase of code size and
memory consumption.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds