linux-kernel - Re: [PATCH v3 2/8] serial: sh-sci: Check if TX data was written to device in .tx

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2024112128-faceted-moonstone-027f@gregkh>
Date: Thu, 21 Nov 2024 22:32:19 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Claudiu <claudiu.beznea@...on.dev>
Cc: geert+renesas@...der.be, magnus.damm@...il.com, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, mturquette@...libre.com,
	sboyd@...nel.org, jirislaby@...nel.org, p.zabel@...gutronix.de,
	lethal@...ux-sh.org, g.liakhovetski@....de,
	linux-renesas-soc@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
	linux-serial@...r.kernel.org,
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>,
	stable@...r.kernel.org
Subject: Re: [PATCH v3 2/8] serial: sh-sci: Check if TX data was written to
 device in .tx_empty()

On Fri, Nov 15, 2024 at 03:43:55PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> 
> On the Renesas RZ/G3S, when doing suspend to RAM, the uart_suspend_port()
> is called. The uart_suspend_port() calls 3 times the
> struct uart_port::ops::tx_empty() before shutting down the port.
> 
> According to the documentation, the struct uart_port::ops::tx_empty()
> API tests whether the transmitter FIFO and shifter for the port is
> empty.
> 
> The Renesas RZ/G3S SCIFA IP reports the number of data units stored in the
> transmit FIFO through the FDR (FIFO Data Count Register). The data units
> in the FIFOs are written in the shift register and transmitted from there.
> The TEND bit in the Serial Status Register reports if the data was
> transmitted from the shift register.
> 
> In the previous code, in the tx_empty() API implemented by the sh-sci
> driver, it is considered that the TX is empty if the hardware reports the
> TEND bit set and the number of data units in the FIFO is zero.
> 
> According to the HW manual, the TEND bit has the following meaning:
> 
> 0: Transmission is in the waiting state or in progress.
> 1: Transmission is completed.
> 
> It has been noticed that when opening the serial device w/o using it and
> then switch to a power saving mode, the tx_empty() call in the
> uart_port_suspend() function fails, leading to the "Unable to drain
> transmitter" message being printed on the console. This is because the
> TEND=0 if nothing has been transmitted and the FIFOs are empty. As the
> TEND=0 has double meaning (waiting state, in progress) we can't
> determined the scenario described above.
> 
> Add a software workaround for this. This sets a variable if any data has
> been sent on the serial console (when using PIO) or if the DMA callback has
> been called (meaning something has been transmitted). In the tx_empty()
> API the status of the DMA transaction is also checked and if it is
> completed or in progress the code falls back in checking the hardware
> registers instead of relying on the software variable.
> 
> Fixes: 73a19e4c0301 ("serial: sh-sci: Add DMA support.")
> Cc: stable@...r.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>

Why is this bug/regression fix burried in a long series?  It should be
sent individually so that it could be applied on its own as it is not
related to the other ones, right?

Or are you ok with waiting for this to show up in 6.14-rc1?

thanks,

greg k-h