| Message ID | 20241009145110.16847-4-johan+linaro@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | serial: qcom-geni: fix receiver enable | expand |
Hi, On Wed, Oct 9, 2024 at 7:51 AM Johan Hovold <johan+linaro@kernel.org> wrote: > > A commit adding back the stopping of tx on port shutdown failed to add > back the locking which had also been removed by commit e83766334f96 > ("tty: serial: qcom_geni_serial: No need to stop tx/rx on UART > shutdown"). > > Holding the port lock is needed to serialise against the console code, > which may update the interrupt enable register and access the port > state. > > Fixes: d8aca2f96813 ("tty: serial: qcom-geni-serial: stop operations in progress at shutdown") > Fixes: 947cc4ecc06c ("serial: qcom-geni: fix soft lockup on sw flow control and suspend") > Cc: stable@vger.kernel.org # 6.3 > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/tty/serial/qcom_geni_serial.c | 2 ++ > 1 file changed, 2 insertions(+) Though this doesn't fix the preexisting bug I talked about [1] that we'll need to touch the same code to fix: Reviewed-by: Douglas Anderson <dianders@chromium.org> [1] https://lore.kernel.org/r/CAD=FV=UZtZ1-0SkN2sOMp6YdU02em_RnK85Heg5z0jkH4U30eQ@mail.gmail.com
On Thu, Oct 10, 2024 at 03:36:30PM -0700, Doug Anderson wrote: > On Wed, Oct 9, 2024 at 7:51 AM Johan Hovold <johan+linaro@kernel.org> wrote: > > > > A commit adding back the stopping of tx on port shutdown failed to add > > back the locking which had also been removed by commit e83766334f96 > > ("tty: serial: qcom_geni_serial: No need to stop tx/rx on UART > > shutdown"). > > > > Holding the port lock is needed to serialise against the console code, > > which may update the interrupt enable register and access the port > > state. > > > > Fixes: d8aca2f96813 ("tty: serial: qcom-geni-serial: stop operations in progress at shutdown") > > Fixes: 947cc4ecc06c ("serial: qcom-geni: fix soft lockup on sw flow control and suspend") > > Cc: stable@vger.kernel.org # 6.3 > > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > > --- > > drivers/tty/serial/qcom_geni_serial.c | 2 ++ > > 1 file changed, 2 insertions(+) > > Though this doesn't fix the preexisting bug I talked about [1] that > we'll need to touch the same code to fix: > > Reviewed-by: Douglas Anderson <dianders@chromium.org> Yeah, let's address that separately. Thanks for reviewing! Johan > [1] https://lore.kernel.org/r/CAD=FV=UZtZ1-0SkN2sOMp6YdU02em_RnK85Heg5z0jkH4U30eQ@mail.gmail.com
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 2e4a5361f137..87cd974b76bf 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -1114,10 +1114,12 @@ static void qcom_geni_serial_shutdown(struct uart_port *uport) { disable_irq(uport->irq); + uart_port_lock_irq(uport); qcom_geni_serial_stop_tx(uport); qcom_geni_serial_stop_rx(uport); qcom_geni_serial_cancel_tx_cmd(uport); + uart_port_unlock_irq(uport); } static void qcom_geni_serial_flush_buffer(struct uart_port *uport)
A commit adding back the stopping of tx on port shutdown failed to add back the locking which had also been removed by commit e83766334f96 ("tty: serial: qcom_geni_serial: No need to stop tx/rx on UART shutdown"). Holding the port lock is needed to serialise against the console code, which may update the interrupt enable register and access the port state. Fixes: d8aca2f96813 ("tty: serial: qcom-geni-serial: stop operations in progress at shutdown") Fixes: 947cc4ecc06c ("serial: qcom-geni: fix soft lockup on sw flow control and suspend") Cc: stable@vger.kernel.org # 6.3 Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Signed-off-by: Johan Hovold <johan+linaro@kernel.org> --- drivers/tty/serial/qcom_geni_serial.c | 2 ++ 1 file changed, 2 insertions(+)