| Message ID | 20230818031532.15591-1-Wenhua.Lin@unisoc.com |
|---|---|
| State | New |
| Headers | show |
| Series | tty/serial: Cancel work queue before closing uart | expand |
On Fri, 18 Aug 2023, Wenhua Lin wrote: I've problems following your description below due to grammar errors. > When the system constantly sleeps and wankes up, plugging and unplugging wakes > the USB will probalility trigger a kernel crash problem. probalility is typoed and I cannot guess which of the words you meant, please fix. If there's a known crash you're fixing here, please quote the crash message in the changelog (and you should probably add Fixes: tag too in that case). > The reason is > that at this time, the system entered deep and turned off uart, and the "entered deep" lacks probably some word? > abnormal behavior of plugging and upplugging the USB triggered the read unplugging. Why call that abnormal behavior? Isn't USB expected to removed. > data process of uart, causing access to uart to hang. Are you saying a read was triggered while the UART was suspended or what? > The final solution > we came up with is to cancel the work queue before shutting down uart > , while ensuring that there is no uart business. ", while ensuring" -> to ensure "uart business" is too vague, you should replace it with something more concrete. Thanks. -- i. > > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com> > --- > drivers/tty/serial/sprd_serial.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c > index b58f51296ace..eddff4360155 100644 > --- a/drivers/tty/serial/sprd_serial.c > +++ b/drivers/tty/serial/sprd_serial.c > @@ -20,6 +20,7 @@ > #include <linux/slab.h> > #include <linux/tty.h> > #include <linux/tty_flip.h> > +#include "../tty.h" > > /* device name */ > #define UART_NR_MAX 8 > @@ -1221,7 +1222,10 @@ static int sprd_probe(struct platform_device *pdev) > static int sprd_suspend(struct device *dev) > { > struct sprd_uart_port *sup = dev_get_drvdata(dev); > + struct uart_port *uport = &sup->port; > + struct tty_port *tty = &uport->state->port; > > + tty_buffer_cancel_work(tty); > uart_suspend_port(&sprd_uart_driver, &sup->port); > > return 0; >
On Fri, Aug 18, 2023 at 11:15:32AM +0800, Wenhua Lin wrote: > When the system constantly sleeps and wankes up, plugging and unplugging > the USB will probalility trigger a kernel crash problem. The reason is > that at this time, the system entered deep and turned off uart, and the > abnormal behavior of plugging and upplugging the USB triggered the read > data process of uart, causing access to uart to hang. The final solution > we came up with is to cancel the work queue before shutting down uart > , while ensuring that there is no uart business. > > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com> > --- > drivers/tty/serial/sprd_serial.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c > index b58f51296ace..eddff4360155 100644 > --- a/drivers/tty/serial/sprd_serial.c > +++ b/drivers/tty/serial/sprd_serial.c > @@ -20,6 +20,7 @@ > #include <linux/slab.h> > #include <linux/tty.h> > #include <linux/tty_flip.h> > +#include "../tty.h" > > /* device name */ > #define UART_NR_MAX 8 > @@ -1221,7 +1222,10 @@ static int sprd_probe(struct platform_device *pdev) > static int sprd_suspend(struct device *dev) > { > struct sprd_uart_port *sup = dev_get_drvdata(dev); > + struct uart_port *uport = &sup->port; > + struct tty_port *tty = &uport->state->port; > > + tty_buffer_cancel_work(tty); What does this serial port have to do with the USB subsystem in your changelog text? And as the kernel bot said, this breaks the build, you can't do this within a serial driver, sorry. greg k-h
On Tue, Aug 22, 2023 at 9:27 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Fri, Aug 18, 2023 at 11:15:32AM +0800, Wenhua Lin wrote: > > When the system constantly sleeps and wankes up, plugging and unplugging > > the USB will probalility trigger a kernel crash problem. The reason is > > that at this time, the system entered deep and turned off uart, and the > > abnormal behavior of plugging and upplugging the USB triggered the read > > data process of uart, causing access to uart to hang. The final solution > > we came up with is to cancel the work queue before shutting down uart > > , while ensuring that there is no uart business. > > > > Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com> > > --- > > drivers/tty/serial/sprd_serial.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c > > index b58f51296ace..eddff4360155 100644 > > --- a/drivers/tty/serial/sprd_serial.c > > +++ b/drivers/tty/serial/sprd_serial.c > > @@ -20,6 +20,7 @@ > > #include <linux/slab.h> > > #include <linux/tty.h> > > #include <linux/tty_flip.h> > > +#include "../tty.h" > > > > /* device name */ > > #define UART_NR_MAX 8 > > @@ -1221,7 +1222,10 @@ static int sprd_probe(struct platform_device *pdev) > > static int sprd_suspend(struct device *dev) > > { > > struct sprd_uart_port *sup = dev_get_drvdata(dev); > > + struct uart_port *uport = &sup->port; > > + struct tty_port *tty = &uport->state->port; > > > > + tty_buffer_cancel_work(tty); > > What does this serial port have to do with the USB subsystem in your > changelog text? > > And as the kernel bot said, this breaks the build, you can't do this > within a serial driver, sorry. This modification is not directly related to the usb subsystem. I just described a problem scenario, such as the previous comment. There may be problems with this solution. I will make changes on patch v2, thank you for your review > > greg k-h
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c index b58f51296ace..eddff4360155 100644 --- a/drivers/tty/serial/sprd_serial.c +++ b/drivers/tty/serial/sprd_serial.c @@ -20,6 +20,7 @@ #include <linux/slab.h> #include <linux/tty.h> #include <linux/tty_flip.h> +#include "../tty.h" /* device name */ #define UART_NR_MAX 8 @@ -1221,7 +1222,10 @@ static int sprd_probe(struct platform_device *pdev) static int sprd_suspend(struct device *dev) { struct sprd_uart_port *sup = dev_get_drvdata(dev); + struct uart_port *uport = &sup->port; + struct tty_port *tty = &uport->state->port; + tty_buffer_cancel_work(tty); uart_suspend_port(&sprd_uart_driver, &sup->port); return 0;
When the system constantly sleeps and wankes up, plugging and unplugging the USB will probalility trigger a kernel crash problem. The reason is that at this time, the system entered deep and turned off uart, and the abnormal behavior of plugging and upplugging the USB triggered the read data process of uart, causing access to uart to hang. The final solution we came up with is to cancel the work queue before shutting down uart , while ensuring that there is no uart business. Signed-off-by: Wenhua Lin <Wenhua.Lin@unisoc.com> --- drivers/tty/serial/sprd_serial.c | 4 ++++ 1 file changed, 4 insertions(+)