* [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 4:34 ` dillon.minfei
0 siblings, 0 replies; 21+ messages in thread
From: dillon.minfei @ 2021-04-12 4:34 UTC (permalink / raw)
To: gregkh, jirislaby, mcoquelin.stm32, alexandre.torgue
Cc: linux-serial, linux-stm32, linux-arm-kernel, linux-kernel, dillon min
From: dillon min <dillon.minfei@gmail.com>
To avoid potential deadlock in spin_lock usage, change to use
spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
spin_lock(), spin_unlock() under handler context.
remove unused local_irq_save/restore call.
Signed-off-by: dillon min <dillon.minfei@gmail.com>
---
Was verified on stm32f469-disco board. need more test on stm32mp platform.
drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index b3675cf25a69..c4c859b34367 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
struct tty_port *tport = &port->state->port;
struct stm32_port *stm32_port = to_stm32_port(port);
const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
- unsigned long c;
+ unsigned long c, flags;
u32 sr;
char flag;
@@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
uart_insert_char(port, sr, USART_SR_ORE, c, flag);
}
- spin_unlock(&port->lock);
+ if (threaded)
+ spin_unlock_irqrestore(&port->lock, flags);
+ else
+ spin_unlock(&port->lock);
+
tty_flip_buffer_push(tport);
- spin_lock(&port->lock);
+
+ if (threaded)
+ spin_lock_irqsave(&port->lock, flags);
+ else
+ spin_lock(&port->lock);
}
static void stm32_usart_tx_dma_complete(void *arg)
@@ -489,13 +497,14 @@ static irqreturn_t stm32_usart_threaded_interrupt(int irq, void *ptr)
{
struct uart_port *port = ptr;
struct stm32_port *stm32_port = to_stm32_port(port);
+ unsigned long flags;
- spin_lock(&port->lock);
+ spin_lock_irqsave(&port->lock, flags);
if (stm32_port->rx_ch)
stm32_usart_receive_chars(port, true);
- spin_unlock(&port->lock);
+ spin_unlock_irqrestore(&port->lock, flags);
return IRQ_HANDLED;
}
@@ -1354,13 +1363,12 @@ static void stm32_usart_console_write(struct console *co, const char *s,
u32 old_cr1, new_cr1;
int locked = 1;
- local_irq_save(flags);
if (port->sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock(&port->lock);
+ locked = spin_trylock_irqsave(&port->lock, flags);
else
- spin_lock(&port->lock);
+ spin_lock_irqsave(&port->lock, flags);
/* Save and disable interrupts, enable the transmitter */
old_cr1 = readl_relaxed(port->membase + ofs->cr1);
@@ -1374,8 +1382,7 @@ static void stm32_usart_console_write(struct console *co, const char *s,
writel_relaxed(old_cr1, port->membase + ofs->cr1);
if (locked)
- spin_unlock(&port->lock);
- local_irq_restore(flags);
+ spin_unlock_irqrestore(&port->lock, flags);
}
static int stm32_usart_console_setup(struct console *co, char *options)
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 4:34 ` dillon.minfei
0 siblings, 0 replies; 21+ messages in thread
From: dillon.minfei @ 2021-04-12 4:34 UTC (permalink / raw)
To: gregkh, jirislaby, mcoquelin.stm32, alexandre.torgue
Cc: linux-serial, linux-stm32, linux-arm-kernel, linux-kernel, dillon min
From: dillon min <dillon.minfei@gmail.com>
To avoid potential deadlock in spin_lock usage, change to use
spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
spin_lock(), spin_unlock() under handler context.
remove unused local_irq_save/restore call.
Signed-off-by: dillon min <dillon.minfei@gmail.com>
---
Was verified on stm32f469-disco board. need more test on stm32mp platform.
drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index b3675cf25a69..c4c859b34367 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
struct tty_port *tport = &port->state->port;
struct stm32_port *stm32_port = to_stm32_port(port);
const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
- unsigned long c;
+ unsigned long c, flags;
u32 sr;
char flag;
@@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
uart_insert_char(port, sr, USART_SR_ORE, c, flag);
}
- spin_unlock(&port->lock);
+ if (threaded)
+ spin_unlock_irqrestore(&port->lock, flags);
+ else
+ spin_unlock(&port->lock);
+
tty_flip_buffer_push(tport);
- spin_lock(&port->lock);
+
+ if (threaded)
+ spin_lock_irqsave(&port->lock, flags);
+ else
+ spin_lock(&port->lock);
}
static void stm32_usart_tx_dma_complete(void *arg)
@@ -489,13 +497,14 @@ static irqreturn_t stm32_usart_threaded_interrupt(int irq, void *ptr)
{
struct uart_port *port = ptr;
struct stm32_port *stm32_port = to_stm32_port(port);
+ unsigned long flags;
- spin_lock(&port->lock);
+ spin_lock_irqsave(&port->lock, flags);
if (stm32_port->rx_ch)
stm32_usart_receive_chars(port, true);
- spin_unlock(&port->lock);
+ spin_unlock_irqrestore(&port->lock, flags);
return IRQ_HANDLED;
}
@@ -1354,13 +1363,12 @@ static void stm32_usart_console_write(struct console *co, const char *s,
u32 old_cr1, new_cr1;
int locked = 1;
- local_irq_save(flags);
if (port->sysrq)
locked = 0;
else if (oops_in_progress)
- locked = spin_trylock(&port->lock);
+ locked = spin_trylock_irqsave(&port->lock, flags);
else
- spin_lock(&port->lock);
+ spin_lock_irqsave(&port->lock, flags);
/* Save and disable interrupts, enable the transmitter */
old_cr1 = readl_relaxed(port->membase + ofs->cr1);
@@ -1374,8 +1382,7 @@ static void stm32_usart_console_write(struct console *co, const char *s,
writel_relaxed(old_cr1, port->membase + ofs->cr1);
if (locked)
- spin_unlock(&port->lock);
- local_irq_restore(flags);
+ spin_unlock_irqrestore(&port->lock, flags);
}
static int stm32_usart_console_setup(struct console *co, char *options)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
2021-04-12 4:34 ` dillon.minfei
@ 2021-04-12 5:52 ` Greg KH
-1 siblings, 0 replies; 21+ messages in thread
From: Greg KH @ 2021-04-12 5:52 UTC (permalink / raw)
To: dillon.minfei
Cc: jirislaby, mcoquelin.stm32, alexandre.torgue, linux-serial,
linux-stm32, linux-arm-kernel, linux-kernel
On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
> From: dillon min <dillon.minfei@gmail.com>
>
> To avoid potential deadlock in spin_lock usage, change to use
> spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
> spin_lock(), spin_unlock() under handler context.
>
> remove unused local_irq_save/restore call.
>
> Signed-off-by: dillon min <dillon.minfei@gmail.com>
> ---
> Was verified on stm32f469-disco board. need more test on stm32mp platform.
>
> drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> index b3675cf25a69..c4c859b34367 100644
> --- a/drivers/tty/serial/stm32-usart.c
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> struct tty_port *tport = &port->state->port;
> struct stm32_port *stm32_port = to_stm32_port(port);
> const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> - unsigned long c;
> + unsigned long c, flags;
> u32 sr;
> char flag;
>
> @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> }
>
> - spin_unlock(&port->lock);
> + if (threaded)
> + spin_unlock_irqrestore(&port->lock, flags);
> + else
> + spin_unlock(&port->lock);
You shouldn't have to check for this, see the other patches on the list
recently that fixed this up to not be an issue for irq handlers.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 5:52 ` Greg KH
0 siblings, 0 replies; 21+ messages in thread
From: Greg KH @ 2021-04-12 5:52 UTC (permalink / raw)
To: dillon.minfei
Cc: jirislaby, mcoquelin.stm32, alexandre.torgue, linux-serial,
linux-stm32, linux-arm-kernel, linux-kernel
On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
> From: dillon min <dillon.minfei@gmail.com>
>
> To avoid potential deadlock in spin_lock usage, change to use
> spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
> spin_lock(), spin_unlock() under handler context.
>
> remove unused local_irq_save/restore call.
>
> Signed-off-by: dillon min <dillon.minfei@gmail.com>
> ---
> Was verified on stm32f469-disco board. need more test on stm32mp platform.
>
> drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> index b3675cf25a69..c4c859b34367 100644
> --- a/drivers/tty/serial/stm32-usart.c
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> struct tty_port *tport = &port->state->port;
> struct stm32_port *stm32_port = to_stm32_port(port);
> const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> - unsigned long c;
> + unsigned long c, flags;
> u32 sr;
> char flag;
>
> @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> }
>
> - spin_unlock(&port->lock);
> + if (threaded)
> + spin_unlock_irqrestore(&port->lock, flags);
> + else
> + spin_unlock(&port->lock);
You shouldn't have to check for this, see the other patches on the list
recently that fixed this up to not be an issue for irq handlers.
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
2021-04-12 5:52 ` Greg KH
@ 2021-04-12 6:50 ` dillon min
-1 siblings, 0 replies; 21+ messages in thread
From: dillon min @ 2021-04-12 6:50 UTC (permalink / raw)
To: Greg KH
Cc: jirislaby, Maxime Coquelin, Alexandre TORGUE, linux-serial,
linux-stm32, Linux ARM, Linux Kernel Mailing List
Hi Greg,
Thanks for the quick response, please ignore the last private mail.
On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
> > From: dillon min <dillon.minfei@gmail.com>
> >
> > To avoid potential deadlock in spin_lock usage, change to use
> > spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
> > spin_lock(), spin_unlock() under handler context.
> >
> > remove unused local_irq_save/restore call.
> >
> > Signed-off-by: dillon min <dillon.minfei@gmail.com>
> > ---
> > Was verified on stm32f469-disco board. need more test on stm32mp platform.
> >
> > drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
> > 1 file changed, 17 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> > index b3675cf25a69..c4c859b34367 100644
> > --- a/drivers/tty/serial/stm32-usart.c
> > +++ b/drivers/tty/serial/stm32-usart.c
> > @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > struct tty_port *tport = &port->state->port;
> > struct stm32_port *stm32_port = to_stm32_port(port);
> > const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> > - unsigned long c;
> > + unsigned long c, flags;
> > u32 sr;
> > char flag;
> >
> > @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> > }
> >
> > - spin_unlock(&port->lock);
> > + if (threaded)
> > + spin_unlock_irqrestore(&port->lock, flags);
> > + else
> > + spin_unlock(&port->lock);
>
> You shouldn't have to check for this, see the other patches on the list
> recently that fixed this up to not be an issue for irq handlers.
Can you help to give more hints, or the commit id of the patch which
fixed this. thanks.
I'm still confused with this.
The stm32_usart_threaded_interrupt() is a kthread context, once
port->lock holds by this function, another serial interrupts raised,
such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
there will be a deadlock. isn't it?
So, shouldn't I use spin_lock{_irqsave} according to the caller's context ?
There is a bug in this patch, the variable flags should be a global variable.
Thanks.
Dillon,
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 6:50 ` dillon min
0 siblings, 0 replies; 21+ messages in thread
From: dillon min @ 2021-04-12 6:50 UTC (permalink / raw)
To: Greg KH
Cc: jirislaby, Maxime Coquelin, Alexandre TORGUE, linux-serial,
linux-stm32, Linux ARM, Linux Kernel Mailing List
Hi Greg,
Thanks for the quick response, please ignore the last private mail.
On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
> > From: dillon min <dillon.minfei@gmail.com>
> >
> > To avoid potential deadlock in spin_lock usage, change to use
> > spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
> > spin_lock(), spin_unlock() under handler context.
> >
> > remove unused local_irq_save/restore call.
> >
> > Signed-off-by: dillon min <dillon.minfei@gmail.com>
> > ---
> > Was verified on stm32f469-disco board. need more test on stm32mp platform.
> >
> > drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
> > 1 file changed, 17 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> > index b3675cf25a69..c4c859b34367 100644
> > --- a/drivers/tty/serial/stm32-usart.c
> > +++ b/drivers/tty/serial/stm32-usart.c
> > @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > struct tty_port *tport = &port->state->port;
> > struct stm32_port *stm32_port = to_stm32_port(port);
> > const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> > - unsigned long c;
> > + unsigned long c, flags;
> > u32 sr;
> > char flag;
> >
> > @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> > }
> >
> > - spin_unlock(&port->lock);
> > + if (threaded)
> > + spin_unlock_irqrestore(&port->lock, flags);
> > + else
> > + spin_unlock(&port->lock);
>
> You shouldn't have to check for this, see the other patches on the list
> recently that fixed this up to not be an issue for irq handlers.
Can you help to give more hints, or the commit id of the patch which
fixed this. thanks.
I'm still confused with this.
The stm32_usart_threaded_interrupt() is a kthread context, once
port->lock holds by this function, another serial interrupts raised,
such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
there will be a deadlock. isn't it?
So, shouldn't I use spin_lock{_irqsave} according to the caller's context ?
There is a bug in this patch, the variable flags should be a global variable.
Thanks.
Dillon,
>
> thanks,
>
> greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
2021-04-12 4:34 ` dillon.minfei
(?)
@ 2021-04-12 7:23 ` kernel test robot
-1 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2021-04-12 7:23 UTC (permalink / raw)
To: dillon.minfei, gregkh, jirislaby, mcoquelin.stm32, alexandre.torgue
Cc: kbuild-all, clang-built-linux, linux-serial, linux-stm32,
linux-arm-kernel, linux-kernel, dillon min
[-- Attachment #1: Type: text/plain, Size: 5016 bytes --]
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on stm32/stm32-next]
[also build test WARNING on usb/usb-testing v5.12-rc7]
[cannot apply to tty/tty-testing next-20210409]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
config: riscv-randconfig-r034-20210412 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9829f5e6b1bca9b61efc629770d28bb9014dec45)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/a0e81ae10c46f768437d61cd3a3dfd4d1250b375
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
git checkout a0e81ae10c46f768437d61cd3a3dfd4d1250b375
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/tty/serial/stm32-usart.c:280:39: warning: variable 'flags' is uninitialized when used here [-Wuninitialized]
spin_unlock_irqrestore(&port->lock, flags);
^~~~~
drivers/tty/serial/stm32-usart.c:217:24: note: initialize the variable 'flags' to silence this warning
unsigned long c, flags;
^
= 0
1 warning generated.
vim +/flags +280 drivers/tty/serial/stm32-usart.c
211
212 static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
213 {
214 struct tty_port *tport = &port->state->port;
215 struct stm32_port *stm32_port = to_stm32_port(port);
216 const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
217 unsigned long c, flags;
218 u32 sr;
219 char flag;
220
221 if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
222 pm_wakeup_event(tport->tty->dev, 0);
223
224 while (stm32_usart_pending_rx(port, &sr, &stm32_port->last_res,
225 threaded)) {
226 sr |= USART_SR_DUMMY_RX;
227 flag = TTY_NORMAL;
228
229 /*
230 * Status bits has to be cleared before reading the RDR:
231 * In FIFO mode, reading the RDR will pop the next data
232 * (if any) along with its status bits into the SR.
233 * Not doing so leads to misalignement between RDR and SR,
234 * and clear status bits of the next rx data.
235 *
236 * Clear errors flags for stm32f7 and stm32h7 compatible
237 * devices. On stm32f4 compatible devices, the error bit is
238 * cleared by the sequence [read SR - read DR].
239 */
240 if ((sr & USART_SR_ERR_MASK) && ofs->icr != UNDEF_REG)
241 writel_relaxed(sr & USART_SR_ERR_MASK,
242 port->membase + ofs->icr);
243
244 c = stm32_usart_get_char(port, &sr, &stm32_port->last_res);
245 port->icount.rx++;
246 if (sr & USART_SR_ERR_MASK) {
247 if (sr & USART_SR_ORE) {
248 port->icount.overrun++;
249 } else if (sr & USART_SR_PE) {
250 port->icount.parity++;
251 } else if (sr & USART_SR_FE) {
252 /* Break detection if character is null */
253 if (!c) {
254 port->icount.brk++;
255 if (uart_handle_break(port))
256 continue;
257 } else {
258 port->icount.frame++;
259 }
260 }
261
262 sr &= port->read_status_mask;
263
264 if (sr & USART_SR_PE) {
265 flag = TTY_PARITY;
266 } else if (sr & USART_SR_FE) {
267 if (!c)
268 flag = TTY_BREAK;
269 else
270 flag = TTY_FRAME;
271 }
272 }
273
274 if (uart_handle_sysrq_char(port, c))
275 continue;
276 uart_insert_char(port, sr, USART_SR_ORE, c, flag);
277 }
278
279 if (threaded)
> 280 spin_unlock_irqrestore(&port->lock, flags);
281 else
282 spin_unlock(&port->lock);
283
284 tty_flip_buffer_push(tport);
285
286 if (threaded)
287 spin_lock_irqsave(&port->lock, flags);
288 else
289 spin_lock(&port->lock);
290 }
291
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36623 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 7:23 ` kernel test robot
0 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2021-04-12 7:23 UTC (permalink / raw)
To: dillon.minfei, gregkh, jirislaby, mcoquelin.stm32, alexandre.torgue
Cc: kbuild-all, clang-built-linux, linux-serial, linux-stm32,
linux-arm-kernel, linux-kernel, dillon min
[-- Attachment #1: Type: text/plain, Size: 5016 bytes --]
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on stm32/stm32-next]
[also build test WARNING on usb/usb-testing v5.12-rc7]
[cannot apply to tty/tty-testing next-20210409]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
config: riscv-randconfig-r034-20210412 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9829f5e6b1bca9b61efc629770d28bb9014dec45)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/a0e81ae10c46f768437d61cd3a3dfd4d1250b375
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
git checkout a0e81ae10c46f768437d61cd3a3dfd4d1250b375
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/tty/serial/stm32-usart.c:280:39: warning: variable 'flags' is uninitialized when used here [-Wuninitialized]
spin_unlock_irqrestore(&port->lock, flags);
^~~~~
drivers/tty/serial/stm32-usart.c:217:24: note: initialize the variable 'flags' to silence this warning
unsigned long c, flags;
^
= 0
1 warning generated.
vim +/flags +280 drivers/tty/serial/stm32-usart.c
211
212 static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
213 {
214 struct tty_port *tport = &port->state->port;
215 struct stm32_port *stm32_port = to_stm32_port(port);
216 const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
217 unsigned long c, flags;
218 u32 sr;
219 char flag;
220
221 if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
222 pm_wakeup_event(tport->tty->dev, 0);
223
224 while (stm32_usart_pending_rx(port, &sr, &stm32_port->last_res,
225 threaded)) {
226 sr |= USART_SR_DUMMY_RX;
227 flag = TTY_NORMAL;
228
229 /*
230 * Status bits has to be cleared before reading the RDR:
231 * In FIFO mode, reading the RDR will pop the next data
232 * (if any) along with its status bits into the SR.
233 * Not doing so leads to misalignement between RDR and SR,
234 * and clear status bits of the next rx data.
235 *
236 * Clear errors flags for stm32f7 and stm32h7 compatible
237 * devices. On stm32f4 compatible devices, the error bit is
238 * cleared by the sequence [read SR - read DR].
239 */
240 if ((sr & USART_SR_ERR_MASK) && ofs->icr != UNDEF_REG)
241 writel_relaxed(sr & USART_SR_ERR_MASK,
242 port->membase + ofs->icr);
243
244 c = stm32_usart_get_char(port, &sr, &stm32_port->last_res);
245 port->icount.rx++;
246 if (sr & USART_SR_ERR_MASK) {
247 if (sr & USART_SR_ORE) {
248 port->icount.overrun++;
249 } else if (sr & USART_SR_PE) {
250 port->icount.parity++;
251 } else if (sr & USART_SR_FE) {
252 /* Break detection if character is null */
253 if (!c) {
254 port->icount.brk++;
255 if (uart_handle_break(port))
256 continue;
257 } else {
258 port->icount.frame++;
259 }
260 }
261
262 sr &= port->read_status_mask;
263
264 if (sr & USART_SR_PE) {
265 flag = TTY_PARITY;
266 } else if (sr & USART_SR_FE) {
267 if (!c)
268 flag = TTY_BREAK;
269 else
270 flag = TTY_FRAME;
271 }
272 }
273
274 if (uart_handle_sysrq_char(port, c))
275 continue;
276 uart_insert_char(port, sr, USART_SR_ORE, c, flag);
277 }
278
279 if (threaded)
> 280 spin_unlock_irqrestore(&port->lock, flags);
281 else
282 spin_unlock(&port->lock);
283
284 tty_flip_buffer_push(tport);
285
286 if (threaded)
287 spin_lock_irqsave(&port->lock, flags);
288 else
289 spin_lock(&port->lock);
290 }
291
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36623 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 7:23 ` kernel test robot
0 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2021-04-12 7:23 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 5147 bytes --]
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on stm32/stm32-next]
[also build test WARNING on usb/usb-testing v5.12-rc7]
[cannot apply to tty/tty-testing next-20210409]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
config: riscv-randconfig-r034-20210412 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9829f5e6b1bca9b61efc629770d28bb9014dec45)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/a0e81ae10c46f768437d61cd3a3dfd4d1250b375
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
git checkout a0e81ae10c46f768437d61cd3a3dfd4d1250b375
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/tty/serial/stm32-usart.c:280:39: warning: variable 'flags' is uninitialized when used here [-Wuninitialized]
spin_unlock_irqrestore(&port->lock, flags);
^~~~~
drivers/tty/serial/stm32-usart.c:217:24: note: initialize the variable 'flags' to silence this warning
unsigned long c, flags;
^
= 0
1 warning generated.
vim +/flags +280 drivers/tty/serial/stm32-usart.c
211
212 static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
213 {
214 struct tty_port *tport = &port->state->port;
215 struct stm32_port *stm32_port = to_stm32_port(port);
216 const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
217 unsigned long c, flags;
218 u32 sr;
219 char flag;
220
221 if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
222 pm_wakeup_event(tport->tty->dev, 0);
223
224 while (stm32_usart_pending_rx(port, &sr, &stm32_port->last_res,
225 threaded)) {
226 sr |= USART_SR_DUMMY_RX;
227 flag = TTY_NORMAL;
228
229 /*
230 * Status bits has to be cleared before reading the RDR:
231 * In FIFO mode, reading the RDR will pop the next data
232 * (if any) along with its status bits into the SR.
233 * Not doing so leads to misalignement between RDR and SR,
234 * and clear status bits of the next rx data.
235 *
236 * Clear errors flags for stm32f7 and stm32h7 compatible
237 * devices. On stm32f4 compatible devices, the error bit is
238 * cleared by the sequence [read SR - read DR].
239 */
240 if ((sr & USART_SR_ERR_MASK) && ofs->icr != UNDEF_REG)
241 writel_relaxed(sr & USART_SR_ERR_MASK,
242 port->membase + ofs->icr);
243
244 c = stm32_usart_get_char(port, &sr, &stm32_port->last_res);
245 port->icount.rx++;
246 if (sr & USART_SR_ERR_MASK) {
247 if (sr & USART_SR_ORE) {
248 port->icount.overrun++;
249 } else if (sr & USART_SR_PE) {
250 port->icount.parity++;
251 } else if (sr & USART_SR_FE) {
252 /* Break detection if character is null */
253 if (!c) {
254 port->icount.brk++;
255 if (uart_handle_break(port))
256 continue;
257 } else {
258 port->icount.frame++;
259 }
260 }
261
262 sr &= port->read_status_mask;
263
264 if (sr & USART_SR_PE) {
265 flag = TTY_PARITY;
266 } else if (sr & USART_SR_FE) {
267 if (!c)
268 flag = TTY_BREAK;
269 else
270 flag = TTY_FRAME;
271 }
272 }
273
274 if (uart_handle_sysrq_char(port, c))
275 continue;
276 uart_insert_char(port, sr, USART_SR_ORE, c, flag);
277 }
278
279 if (threaded)
> 280 spin_unlock_irqrestore(&port->lock, flags);
281 else
282 spin_unlock(&port->lock);
283
284 tty_flip_buffer_push(tport);
285
286 if (threaded)
287 spin_lock_irqsave(&port->lock, flags);
288 else
289 spin_lock(&port->lock);
290 }
291
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36623 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
2021-04-12 7:23 ` kernel test robot
(?)
@ 2021-04-12 7:29 ` dillon min
-1 siblings, 0 replies; 21+ messages in thread
From: dillon min @ 2021-04-12 7:29 UTC (permalink / raw)
To: kernel test robot
Cc: Greg KH, jirislaby, Maxime Coquelin, Alexandre TORGUE,
kbuild-all, clang-built-linux, linux-serial, linux-stm32,
Linux ARM, Linux Kernel Mailing List
Hi lkp,
Thanks for the reminder.
I just noticed this from greg's mail. still waiting for his response
about the proper solution for this patch.
After that i will update version 2 for it.
Your test is really efficient, thanks.
Best regards,
Dillon.
On Mon, Apr 12, 2021 at 3:25 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on stm32/stm32-next]
> [also build test WARNING on usb/usb-testing v5.12-rc7]
> [cannot apply to tty/tty-testing next-20210409]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
> base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
> config: riscv-randconfig-r034-20210412 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9829f5e6b1bca9b61efc629770d28bb9014dec45)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install riscv cross compiling tool for clang build
> # apt-get install binutils-riscv64-linux-gnu
> # https://github.com/0day-ci/linux/commit/a0e81ae10c46f768437d61cd3a3dfd4d1250b375
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
> git checkout a0e81ae10c46f768437d61cd3a3dfd4d1250b375
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/tty/serial/stm32-usart.c:280:39: warning: variable 'flags' is uninitialized when used here [-Wuninitialized]
> spin_unlock_irqrestore(&port->lock, flags);
> ^~~~~
> drivers/tty/serial/stm32-usart.c:217:24: note: initialize the variable 'flags' to silence this warning
> unsigned long c, flags;
> ^
> = 0
> 1 warning generated.
>
>
> vim +/flags +280 drivers/tty/serial/stm32-usart.c
>
> 211
> 212 static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> 213 {
> 214 struct tty_port *tport = &port->state->port;
> 215 struct stm32_port *stm32_port = to_stm32_port(port);
> 216 const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> 217 unsigned long c, flags;
> 218 u32 sr;
> 219 char flag;
> 220
> 221 if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
> 222 pm_wakeup_event(tport->tty->dev, 0);
> 223
> 224 while (stm32_usart_pending_rx(port, &sr, &stm32_port->last_res,
> 225 threaded)) {
> 226 sr |= USART_SR_DUMMY_RX;
> 227 flag = TTY_NORMAL;
> 228
> 229 /*
> 230 * Status bits has to be cleared before reading the RDR:
> 231 * In FIFO mode, reading the RDR will pop the next data
> 232 * (if any) along with its status bits into the SR.
> 233 * Not doing so leads to misalignement between RDR and SR,
> 234 * and clear status bits of the next rx data.
> 235 *
> 236 * Clear errors flags for stm32f7 and stm32h7 compatible
> 237 * devices. On stm32f4 compatible devices, the error bit is
> 238 * cleared by the sequence [read SR - read DR].
> 239 */
> 240 if ((sr & USART_SR_ERR_MASK) && ofs->icr != UNDEF_REG)
> 241 writel_relaxed(sr & USART_SR_ERR_MASK,
> 242 port->membase + ofs->icr);
> 243
> 244 c = stm32_usart_get_char(port, &sr, &stm32_port->last_res);
> 245 port->icount.rx++;
> 246 if (sr & USART_SR_ERR_MASK) {
> 247 if (sr & USART_SR_ORE) {
> 248 port->icount.overrun++;
> 249 } else if (sr & USART_SR_PE) {
> 250 port->icount.parity++;
> 251 } else if (sr & USART_SR_FE) {
> 252 /* Break detection if character is null */
> 253 if (!c) {
> 254 port->icount.brk++;
> 255 if (uart_handle_break(port))
> 256 continue;
> 257 } else {
> 258 port->icount.frame++;
> 259 }
> 260 }
> 261
> 262 sr &= port->read_status_mask;
> 263
> 264 if (sr & USART_SR_PE) {
> 265 flag = TTY_PARITY;
> 266 } else if (sr & USART_SR_FE) {
> 267 if (!c)
> 268 flag = TTY_BREAK;
> 269 else
> 270 flag = TTY_FRAME;
> 271 }
> 272 }
> 273
> 274 if (uart_handle_sysrq_char(port, c))
> 275 continue;
> 276 uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> 277 }
> 278
> 279 if (threaded)
> > 280 spin_unlock_irqrestore(&port->lock, flags);
> 281 else
> 282 spin_unlock(&port->lock);
> 283
> 284 tty_flip_buffer_push(tport);
> 285
> 286 if (threaded)
> 287 spin_lock_irqsave(&port->lock, flags);
> 288 else
> 289 spin_lock(&port->lock);
> 290 }
> 291
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 7:29 ` dillon min
0 siblings, 0 replies; 21+ messages in thread
From: dillon min @ 2021-04-12 7:29 UTC (permalink / raw)
To: kernel test robot
Cc: Greg KH, jirislaby, Maxime Coquelin, Alexandre TORGUE,
kbuild-all, clang-built-linux, linux-serial, linux-stm32,
Linux ARM, Linux Kernel Mailing List
Hi lkp,
Thanks for the reminder.
I just noticed this from greg's mail. still waiting for his response
about the proper solution for this patch.
After that i will update version 2 for it.
Your test is really efficient, thanks.
Best regards,
Dillon.
On Mon, Apr 12, 2021 at 3:25 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on stm32/stm32-next]
> [also build test WARNING on usb/usb-testing v5.12-rc7]
> [cannot apply to tty/tty-testing next-20210409]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
> base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
> config: riscv-randconfig-r034-20210412 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9829f5e6b1bca9b61efc629770d28bb9014dec45)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install riscv cross compiling tool for clang build
> # apt-get install binutils-riscv64-linux-gnu
> # https://github.com/0day-ci/linux/commit/a0e81ae10c46f768437d61cd3a3dfd4d1250b375
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
> git checkout a0e81ae10c46f768437d61cd3a3dfd4d1250b375
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/tty/serial/stm32-usart.c:280:39: warning: variable 'flags' is uninitialized when used here [-Wuninitialized]
> spin_unlock_irqrestore(&port->lock, flags);
> ^~~~~
> drivers/tty/serial/stm32-usart.c:217:24: note: initialize the variable 'flags' to silence this warning
> unsigned long c, flags;
> ^
> = 0
> 1 warning generated.
>
>
> vim +/flags +280 drivers/tty/serial/stm32-usart.c
>
> 211
> 212 static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> 213 {
> 214 struct tty_port *tport = &port->state->port;
> 215 struct stm32_port *stm32_port = to_stm32_port(port);
> 216 const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> 217 unsigned long c, flags;
> 218 u32 sr;
> 219 char flag;
> 220
> 221 if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
> 222 pm_wakeup_event(tport->tty->dev, 0);
> 223
> 224 while (stm32_usart_pending_rx(port, &sr, &stm32_port->last_res,
> 225 threaded)) {
> 226 sr |= USART_SR_DUMMY_RX;
> 227 flag = TTY_NORMAL;
> 228
> 229 /*
> 230 * Status bits has to be cleared before reading the RDR:
> 231 * In FIFO mode, reading the RDR will pop the next data
> 232 * (if any) along with its status bits into the SR.
> 233 * Not doing so leads to misalignement between RDR and SR,
> 234 * and clear status bits of the next rx data.
> 235 *
> 236 * Clear errors flags for stm32f7 and stm32h7 compatible
> 237 * devices. On stm32f4 compatible devices, the error bit is
> 238 * cleared by the sequence [read SR - read DR].
> 239 */
> 240 if ((sr & USART_SR_ERR_MASK) && ofs->icr != UNDEF_REG)
> 241 writel_relaxed(sr & USART_SR_ERR_MASK,
> 242 port->membase + ofs->icr);
> 243
> 244 c = stm32_usart_get_char(port, &sr, &stm32_port->last_res);
> 245 port->icount.rx++;
> 246 if (sr & USART_SR_ERR_MASK) {
> 247 if (sr & USART_SR_ORE) {
> 248 port->icount.overrun++;
> 249 } else if (sr & USART_SR_PE) {
> 250 port->icount.parity++;
> 251 } else if (sr & USART_SR_FE) {
> 252 /* Break detection if character is null */
> 253 if (!c) {
> 254 port->icount.brk++;
> 255 if (uart_handle_break(port))
> 256 continue;
> 257 } else {
> 258 port->icount.frame++;
> 259 }
> 260 }
> 261
> 262 sr &= port->read_status_mask;
> 263
> 264 if (sr & USART_SR_PE) {
> 265 flag = TTY_PARITY;
> 266 } else if (sr & USART_SR_FE) {
> 267 if (!c)
> 268 flag = TTY_BREAK;
> 269 else
> 270 flag = TTY_FRAME;
> 271 }
> 272 }
> 273
> 274 if (uart_handle_sysrq_char(port, c))
> 275 continue;
> 276 uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> 277 }
> 278
> 279 if (threaded)
> > 280 spin_unlock_irqrestore(&port->lock, flags);
> 281 else
> 282 spin_unlock(&port->lock);
> 283
> 284 tty_flip_buffer_push(tport);
> 285
> 286 if (threaded)
> 287 spin_lock_irqsave(&port->lock, flags);
> 288 else
> 289 spin_lock(&port->lock);
> 290 }
> 291
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 7:29 ` dillon min
0 siblings, 0 replies; 21+ messages in thread
From: dillon min @ 2021-04-12 7:29 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 6975 bytes --]
Hi lkp,
Thanks for the reminder.
I just noticed this from greg's mail. still waiting for his response
about the proper solution for this patch.
After that i will update version 2 for it.
Your test is really efficient, thanks.
Best regards,
Dillon.
On Mon, Apr 12, 2021 at 3:25 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on stm32/stm32-next]
> [also build test WARNING on usb/usb-testing v5.12-rc7]
> [cannot apply to tty/tty-testing next-20210409]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
> base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
> config: riscv-randconfig-r034-20210412 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 9829f5e6b1bca9b61efc629770d28bb9014dec45)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install riscv cross compiling tool for clang build
> # apt-get install binutils-riscv64-linux-gnu
> # https://github.com/0day-ci/linux/commit/a0e81ae10c46f768437d61cd3a3dfd4d1250b375
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
> git checkout a0e81ae10c46f768437d61cd3a3dfd4d1250b375
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/tty/serial/stm32-usart.c:280:39: warning: variable 'flags' is uninitialized when used here [-Wuninitialized]
> spin_unlock_irqrestore(&port->lock, flags);
> ^~~~~
> drivers/tty/serial/stm32-usart.c:217:24: note: initialize the variable 'flags' to silence this warning
> unsigned long c, flags;
> ^
> = 0
> 1 warning generated.
>
>
> vim +/flags +280 drivers/tty/serial/stm32-usart.c
>
> 211
> 212 static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> 213 {
> 214 struct tty_port *tport = &port->state->port;
> 215 struct stm32_port *stm32_port = to_stm32_port(port);
> 216 const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> 217 unsigned long c, flags;
> 218 u32 sr;
> 219 char flag;
> 220
> 221 if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
> 222 pm_wakeup_event(tport->tty->dev, 0);
> 223
> 224 while (stm32_usart_pending_rx(port, &sr, &stm32_port->last_res,
> 225 threaded)) {
> 226 sr |= USART_SR_DUMMY_RX;
> 227 flag = TTY_NORMAL;
> 228
> 229 /*
> 230 * Status bits has to be cleared before reading the RDR:
> 231 * In FIFO mode, reading the RDR will pop the next data
> 232 * (if any) along with its status bits into the SR.
> 233 * Not doing so leads to misalignement between RDR and SR,
> 234 * and clear status bits of the next rx data.
> 235 *
> 236 * Clear errors flags for stm32f7 and stm32h7 compatible
> 237 * devices. On stm32f4 compatible devices, the error bit is
> 238 * cleared by the sequence [read SR - read DR].
> 239 */
> 240 if ((sr & USART_SR_ERR_MASK) && ofs->icr != UNDEF_REG)
> 241 writel_relaxed(sr & USART_SR_ERR_MASK,
> 242 port->membase + ofs->icr);
> 243
> 244 c = stm32_usart_get_char(port, &sr, &stm32_port->last_res);
> 245 port->icount.rx++;
> 246 if (sr & USART_SR_ERR_MASK) {
> 247 if (sr & USART_SR_ORE) {
> 248 port->icount.overrun++;
> 249 } else if (sr & USART_SR_PE) {
> 250 port->icount.parity++;
> 251 } else if (sr & USART_SR_FE) {
> 252 /* Break detection if character is null */
> 253 if (!c) {
> 254 port->icount.brk++;
> 255 if (uart_handle_break(port))
> 256 continue;
> 257 } else {
> 258 port->icount.frame++;
> 259 }
> 260 }
> 261
> 262 sr &= port->read_status_mask;
> 263
> 264 if (sr & USART_SR_PE) {
> 265 flag = TTY_PARITY;
> 266 } else if (sr & USART_SR_FE) {
> 267 if (!c)
> 268 flag = TTY_BREAK;
> 269 else
> 270 flag = TTY_FRAME;
> 271 }
> 272 }
> 273
> 274 if (uart_handle_sysrq_char(port, c))
> 275 continue;
> 276 uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> 277 }
> 278
> 279 if (threaded)
> > 280 spin_unlock_irqrestore(&port->lock, flags);
> 281 else
> 282 spin_unlock(&port->lock);
> 283
> 284 tty_flip_buffer_push(tport);
> 285
> 286 if (threaded)
> 287 spin_lock_irqsave(&port->lock, flags);
> 288 else
> 289 spin_lock(&port->lock);
> 290 }
> 291
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
2021-04-12 6:50 ` dillon min
@ 2021-04-12 8:25 ` Greg KH
-1 siblings, 0 replies; 21+ messages in thread
From: Greg KH @ 2021-04-12 8:25 UTC (permalink / raw)
To: dillon min
Cc: jirislaby, Maxime Coquelin, Alexandre TORGUE, linux-serial,
linux-stm32, Linux ARM, Linux Kernel Mailing List
On Mon, Apr 12, 2021 at 02:50:20PM +0800, dillon min wrote:
> Hi Greg,
>
> Thanks for the quick response, please ignore the last private mail.
>
> On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
> > > From: dillon min <dillon.minfei@gmail.com>
> > >
> > > To avoid potential deadlock in spin_lock usage, change to use
> > > spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
> > > spin_lock(), spin_unlock() under handler context.
> > >
> > > remove unused local_irq_save/restore call.
> > >
> > > Signed-off-by: dillon min <dillon.minfei@gmail.com>
> > > ---
> > > Was verified on stm32f469-disco board. need more test on stm32mp platform.
> > >
> > > drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
> > > 1 file changed, 17 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> > > index b3675cf25a69..c4c859b34367 100644
> > > --- a/drivers/tty/serial/stm32-usart.c
> > > +++ b/drivers/tty/serial/stm32-usart.c
> > > @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > > struct tty_port *tport = &port->state->port;
> > > struct stm32_port *stm32_port = to_stm32_port(port);
> > > const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> > > - unsigned long c;
> > > + unsigned long c, flags;
> > > u32 sr;
> > > char flag;
> > >
> > > @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > > uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> > > }
> > >
> > > - spin_unlock(&port->lock);
> > > + if (threaded)
> > > + spin_unlock_irqrestore(&port->lock, flags);
> > > + else
> > > + spin_unlock(&port->lock);
> >
> > You shouldn't have to check for this, see the other patches on the list
> > recently that fixed this up to not be an issue for irq handlers.
> Can you help to give more hints, or the commit id of the patch which
> fixed this. thanks.
>
> I'm still confused with this.
>
> The stm32_usart_threaded_interrupt() is a kthread context, once
> port->lock holds by this function, another serial interrupts raised,
> such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
> there will be a deadlock. isn't it?
>
> So, shouldn't I use spin_lock{_irqsave} according to the caller's context ?
Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
handlers") for when threaded irq handlers have irqs disabled, isn't that
the case you are trying to "protect" from here?
Why is the "threaded" flag used at all? The driver should not care.
Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
threading") in linux-next for an example of how this was fixed up in a
serial driver.
does that help?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 8:25 ` Greg KH
0 siblings, 0 replies; 21+ messages in thread
From: Greg KH @ 2021-04-12 8:25 UTC (permalink / raw)
To: dillon min
Cc: jirislaby, Maxime Coquelin, Alexandre TORGUE, linux-serial,
linux-stm32, Linux ARM, Linux Kernel Mailing List
On Mon, Apr 12, 2021 at 02:50:20PM +0800, dillon min wrote:
> Hi Greg,
>
> Thanks for the quick response, please ignore the last private mail.
>
> On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
> > > From: dillon min <dillon.minfei@gmail.com>
> > >
> > > To avoid potential deadlock in spin_lock usage, change to use
> > > spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
> > > spin_lock(), spin_unlock() under handler context.
> > >
> > > remove unused local_irq_save/restore call.
> > >
> > > Signed-off-by: dillon min <dillon.minfei@gmail.com>
> > > ---
> > > Was verified on stm32f469-disco board. need more test on stm32mp platform.
> > >
> > > drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
> > > 1 file changed, 17 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> > > index b3675cf25a69..c4c859b34367 100644
> > > --- a/drivers/tty/serial/stm32-usart.c
> > > +++ b/drivers/tty/serial/stm32-usart.c
> > > @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > > struct tty_port *tport = &port->state->port;
> > > struct stm32_port *stm32_port = to_stm32_port(port);
> > > const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> > > - unsigned long c;
> > > + unsigned long c, flags;
> > > u32 sr;
> > > char flag;
> > >
> > > @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > > uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> > > }
> > >
> > > - spin_unlock(&port->lock);
> > > + if (threaded)
> > > + spin_unlock_irqrestore(&port->lock, flags);
> > > + else
> > > + spin_unlock(&port->lock);
> >
> > You shouldn't have to check for this, see the other patches on the list
> > recently that fixed this up to not be an issue for irq handlers.
> Can you help to give more hints, or the commit id of the patch which
> fixed this. thanks.
>
> I'm still confused with this.
>
> The stm32_usart_threaded_interrupt() is a kthread context, once
> port->lock holds by this function, another serial interrupts raised,
> such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
> there will be a deadlock. isn't it?
>
> So, shouldn't I use spin_lock{_irqsave} according to the caller's context ?
Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
handlers") for when threaded irq handlers have irqs disabled, isn't that
the case you are trying to "protect" from here?
Why is the "threaded" flag used at all? The driver should not care.
Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
threading") in linux-next for an example of how this was fixed up in a
serial driver.
does that help?
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
2021-04-12 8:25 ` Greg KH
@ 2021-04-12 8:54 ` dillon min
-1 siblings, 0 replies; 21+ messages in thread
From: dillon min @ 2021-04-12 8:54 UTC (permalink / raw)
To: Greg KH
Cc: jirislaby, Maxime Coquelin, Alexandre TORGUE, linux-serial,
linux-stm32, Linux ARM, Linux Kernel Mailing List
Hi Greg,
On Mon, Apr 12, 2021 at 4:25 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Apr 12, 2021 at 02:50:20PM +0800, dillon min wrote:
> > Hi Greg,
> >
> > Thanks for the quick response, please ignore the last private mail.
> >
> > On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
> > > > From: dillon min <dillon.minfei@gmail.com>
> > > >
> > > > To avoid potential deadlock in spin_lock usage, change to use
> > > > spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
> > > > spin_lock(), spin_unlock() under handler context.
> > > >
> > > > remove unused local_irq_save/restore call.
> > > >
> > > > Signed-off-by: dillon min <dillon.minfei@gmail.com>
> > > > ---
> > > > Was verified on stm32f469-disco board. need more test on stm32mp platform.
> > > >
> > > > drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
> > > > 1 file changed, 17 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> > > > index b3675cf25a69..c4c859b34367 100644
> > > > --- a/drivers/tty/serial/stm32-usart.c
> > > > +++ b/drivers/tty/serial/stm32-usart.c
> > > > @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > > > struct tty_port *tport = &port->state->port;
> > > > struct stm32_port *stm32_port = to_stm32_port(port);
> > > > const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> > > > - unsigned long c;
> > > > + unsigned long c, flags;
> > > > u32 sr;
> > > > char flag;
> > > >
> > > > @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > > > uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> > > > }
> > > >
> > > > - spin_unlock(&port->lock);
> > > > + if (threaded)
> > > > + spin_unlock_irqrestore(&port->lock, flags);
> > > > + else
> > > > + spin_unlock(&port->lock);
> > >
> > > You shouldn't have to check for this, see the other patches on the list
> > > recently that fixed this up to not be an issue for irq handlers.
> > Can you help to give more hints, or the commit id of the patch which
> > fixed this. thanks.
> >
> > I'm still confused with this.
> >
> > The stm32_usart_threaded_interrupt() is a kthread context, once
> > port->lock holds by this function, another serial interrupts raised,
> > such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
> > there will be a deadlock. isn't it?
> >
> > So, shouldn't I use spin_lock{_irqsave} according to the caller's context ?
>
> Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
> handlers") for when threaded irq handlers have irqs disabled, isn't that
> the case you are trying to "protect" from here?
>
> Why is the "threaded" flag used at all? The driver should not care.
>
> Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
> threading") in linux-next for an example of how this was fixed up in a
> serial driver.
>
> does that help?
>
Yes, it's really helpful. and 81e2073c175b should be highlighted in a doc.
In my past knowledge, we should care about hard irq & thread_fn lock conflict.
This patch has totally avoided patching code in the separate driver side.
thanks.
I will just keep the changes in stm32_usart_console_write(), remove
these code in
thread_fn. update version 2 for you.
thanks.
Dillon,
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 8:54 ` dillon min
0 siblings, 0 replies; 21+ messages in thread
From: dillon min @ 2021-04-12 8:54 UTC (permalink / raw)
To: Greg KH
Cc: jirislaby, Maxime Coquelin, Alexandre TORGUE, linux-serial,
linux-stm32, Linux ARM, Linux Kernel Mailing List
Hi Greg,
On Mon, Apr 12, 2021 at 4:25 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Mon, Apr 12, 2021 at 02:50:20PM +0800, dillon min wrote:
> > Hi Greg,
> >
> > Thanks for the quick response, please ignore the last private mail.
> >
> > On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
> > > > From: dillon min <dillon.minfei@gmail.com>
> > > >
> > > > To avoid potential deadlock in spin_lock usage, change to use
> > > > spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
> > > > spin_lock(), spin_unlock() under handler context.
> > > >
> > > > remove unused local_irq_save/restore call.
> > > >
> > > > Signed-off-by: dillon min <dillon.minfei@gmail.com>
> > > > ---
> > > > Was verified on stm32f469-disco board. need more test on stm32mp platform.
> > > >
> > > > drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
> > > > 1 file changed, 17 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> > > > index b3675cf25a69..c4c859b34367 100644
> > > > --- a/drivers/tty/serial/stm32-usart.c
> > > > +++ b/drivers/tty/serial/stm32-usart.c
> > > > @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > > > struct tty_port *tport = &port->state->port;
> > > > struct stm32_port *stm32_port = to_stm32_port(port);
> > > > const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
> > > > - unsigned long c;
> > > > + unsigned long c, flags;
> > > > u32 sr;
> > > > char flag;
> > > >
> > > > @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
> > > > uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> > > > }
> > > >
> > > > - spin_unlock(&port->lock);
> > > > + if (threaded)
> > > > + spin_unlock_irqrestore(&port->lock, flags);
> > > > + else
> > > > + spin_unlock(&port->lock);
> > >
> > > You shouldn't have to check for this, see the other patches on the list
> > > recently that fixed this up to not be an issue for irq handlers.
> > Can you help to give more hints, or the commit id of the patch which
> > fixed this. thanks.
> >
> > I'm still confused with this.
> >
> > The stm32_usart_threaded_interrupt() is a kthread context, once
> > port->lock holds by this function, another serial interrupts raised,
> > such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
> > there will be a deadlock. isn't it?
> >
> > So, shouldn't I use spin_lock{_irqsave} according to the caller's context ?
>
> Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
> handlers") for when threaded irq handlers have irqs disabled, isn't that
> the case you are trying to "protect" from here?
>
> Why is the "threaded" flag used at all? The driver should not care.
>
> Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
> threading") in linux-next for an example of how this was fixed up in a
> serial driver.
>
> does that help?
>
Yes, it's really helpful. and 81e2073c175b should be highlighted in a doc.
In my past knowledge, we should care about hard irq & thread_fn lock conflict.
This patch has totally avoided patching code in the separate driver side.
thanks.
I will just keep the changes in stm32_usart_console_write(), remove
these code in
thread_fn. update version 2 for you.
thanks.
Dillon,
> thanks,
>
> greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Linux-stm32] [PATCH] serial: stm32: optimize spin lock usage
2021-04-12 8:54 ` dillon min
@ 2021-04-12 13:19 ` Erwan LE RAY
-1 siblings, 0 replies; 21+ messages in thread
From: Erwan LE RAY @ 2021-04-12 13:19 UTC (permalink / raw)
To: dillon min, Greg KH
Cc: linux-serial, Alexandre TORGUE, Linux Kernel Mailing List,
Maxime Coquelin, jirislaby, linux-stm32, Linux ARM,
Fabrice Gasnier, Valentin Caron
Hi Dillon,
Thanks for your patch.
Could you please elaborate the use case in your commit message ?
Best Regards, Erwan.
On 4/12/21 10:54 AM, dillon min wrote:
> Hi Greg,
>
> On Mon, Apr 12, 2021 at 4:25 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> On Mon, Apr 12, 2021 at 02:50:20PM +0800, dillon min wrote:
>>> Hi Greg,
>>>
>>> Thanks for the quick response, please ignore the last private mail.
>>>
>>> On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>>>
>>>> On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
>>>>> From: dillon min <dillon.minfei@gmail.com>
>>>>>
>>>>> To avoid potential deadlock in spin_lock usage, change to use
>>>>> spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
>>>>> spin_lock(), spin_unlock() under handler context.
>>>>>
>>>>> remove unused local_irq_save/restore call.
>>>>>
>>>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>
>>>>> ---
>>>>> Was verified on stm32f469-disco board. need more test on stm32mp platform.
>>>>>
>>>>> drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
>>>>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
>>>>> index b3675cf25a69..c4c859b34367 100644
>>>>> --- a/drivers/tty/serial/stm32-usart.c
>>>>> +++ b/drivers/tty/serial/stm32-usart.c
>>>>> @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
>>>>> struct tty_port *tport = &port->state->port;
>>>>> struct stm32_port *stm32_port = to_stm32_port(port);
>>>>> const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
>>>>> - unsigned long c;
>>>>> + unsigned long c, flags;
>>>>> u32 sr;
>>>>> char flag;
>>>>>
>>>>> @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
>>>>> uart_insert_char(port, sr, USART_SR_ORE, c, flag);
>>>>> }
>>>>>
>>>>> - spin_unlock(&port->lock);
>>>>> + if (threaded)
>>>>> + spin_unlock_irqrestore(&port->lock, flags);
>>>>> + else
>>>>> + spin_unlock(&port->lock);
>>>>
>>>> You shouldn't have to check for this, see the other patches on the list
>>>> recently that fixed this up to not be an issue for irq handlers.
>>> Can you help to give more hints, or the commit id of the patch which
>>> fixed this. thanks.
>>>
>>> I'm still confused with this.
>>>
>>> The stm32_usart_threaded_interrupt() is a kthread context, once
>>> port->lock holds by this function, another serial interrupts raised,
>>> such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
>>> there will be a deadlock. isn't it?
>>>
>>> So, shouldn't I use spin_lock{_irqsave} according to the caller's context ?
>>
>> Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
>> handlers") for when threaded irq handlers have irqs disabled, isn't that
>> the case you are trying to "protect" from here?
>>
>> Why is the "threaded" flag used at all? The driver should not care.
>>
>> Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
>> threading") in linux-next for an example of how this was fixed up in a
>> serial driver.
>>
>> does that help?
>>
> Yes, it's really helpful. and 81e2073c175b should be highlighted in a doc.
> In my past knowledge, we should care about hard irq & thread_fn lock conflict.
> This patch has totally avoided patching code in the separate driver side.
> thanks.
>
> I will just keep the changes in stm32_usart_console_write(), remove
> these code in
> thread_fn. update version 2 for you.
>
> thanks.
>
> Dillon,
>> thanks,
>>
>> greg k-h
> _______________________________________________
> Linux-stm32 mailing list
> Linux-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Linux-stm32] [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 13:19 ` Erwan LE RAY
0 siblings, 0 replies; 21+ messages in thread
From: Erwan LE RAY @ 2021-04-12 13:19 UTC (permalink / raw)
To: dillon min, Greg KH
Cc: linux-serial, Alexandre TORGUE, Linux Kernel Mailing List,
Maxime Coquelin, jirislaby, linux-stm32, Linux ARM,
Fabrice Gasnier, Valentin Caron
Hi Dillon,
Thanks for your patch.
Could you please elaborate the use case in your commit message ?
Best Regards, Erwan.
On 4/12/21 10:54 AM, dillon min wrote:
> Hi Greg,
>
> On Mon, Apr 12, 2021 at 4:25 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> On Mon, Apr 12, 2021 at 02:50:20PM +0800, dillon min wrote:
>>> Hi Greg,
>>>
>>> Thanks for the quick response, please ignore the last private mail.
>>>
>>> On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>>>
>>>> On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com wrote:
>>>>> From: dillon min <dillon.minfei@gmail.com>
>>>>>
>>>>> To avoid potential deadlock in spin_lock usage, change to use
>>>>> spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn) context.
>>>>> spin_lock(), spin_unlock() under handler context.
>>>>>
>>>>> remove unused local_irq_save/restore call.
>>>>>
>>>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>
>>>>> ---
>>>>> Was verified on stm32f469-disco board. need more test on stm32mp platform.
>>>>>
>>>>> drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
>>>>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
>>>>> index b3675cf25a69..c4c859b34367 100644
>>>>> --- a/drivers/tty/serial/stm32-usart.c
>>>>> +++ b/drivers/tty/serial/stm32-usart.c
>>>>> @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
>>>>> struct tty_port *tport = &port->state->port;
>>>>> struct stm32_port *stm32_port = to_stm32_port(port);
>>>>> const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
>>>>> - unsigned long c;
>>>>> + unsigned long c, flags;
>>>>> u32 sr;
>>>>> char flag;
>>>>>
>>>>> @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
>>>>> uart_insert_char(port, sr, USART_SR_ORE, c, flag);
>>>>> }
>>>>>
>>>>> - spin_unlock(&port->lock);
>>>>> + if (threaded)
>>>>> + spin_unlock_irqrestore(&port->lock, flags);
>>>>> + else
>>>>> + spin_unlock(&port->lock);
>>>>
>>>> You shouldn't have to check for this, see the other patches on the list
>>>> recently that fixed this up to not be an issue for irq handlers.
>>> Can you help to give more hints, or the commit id of the patch which
>>> fixed this. thanks.
>>>
>>> I'm still confused with this.
>>>
>>> The stm32_usart_threaded_interrupt() is a kthread context, once
>>> port->lock holds by this function, another serial interrupts raised,
>>> such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
>>> there will be a deadlock. isn't it?
>>>
>>> So, shouldn't I use spin_lock{_irqsave} according to the caller's context ?
>>
>> Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
>> handlers") for when threaded irq handlers have irqs disabled, isn't that
>> the case you are trying to "protect" from here?
>>
>> Why is the "threaded" flag used at all? The driver should not care.
>>
>> Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
>> threading") in linux-next for an example of how this was fixed up in a
>> serial driver.
>>
>> does that help?
>>
> Yes, it's really helpful. and 81e2073c175b should be highlighted in a doc.
> In my past knowledge, we should care about hard irq & thread_fn lock conflict.
> This patch has totally avoided patching code in the separate driver side.
> thanks.
>
> I will just keep the changes in stm32_usart_console_write(), remove
> these code in
> thread_fn. update version 2 for you.
>
> thanks.
>
> Dillon,
>> thanks,
>>
>> greg k-h
> _______________________________________________
> Linux-stm32 mailing list
> Linux-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Linux-stm32] [PATCH] serial: stm32: optimize spin lock usage
2021-04-12 13:19 ` Erwan LE RAY
@ 2021-04-12 13:41 ` dillon min
-1 siblings, 0 replies; 21+ messages in thread
From: dillon min @ 2021-04-12 13:41 UTC (permalink / raw)
To: Erwan LE RAY
Cc: Greg KH, linux-serial, Alexandre TORGUE,
Linux Kernel Mailing List, Maxime Coquelin, jirislaby,
linux-stm32, Linux ARM, Fabrice Gasnier, Valentin Caron
On 4/12/21, Erwan LE RAY <erwan.leray@foss.st.com> wrote:
> Hi Dillon,
>
> Thanks for your patch.
>
> Could you please elaborate the use case in your commit message ?
Sorry, local_irq_save() plus spin_lock() same to spin_lock_irqsave()
There is no deadlock . Please ignore this patch.
Thanks
Dillon
>
> Best Regards, Erwan.
>
> On 4/12/21 10:54 AM, dillon min wrote:
>> Hi Greg,
>>
>> On Mon, Apr 12, 2021 at 4:25 PM Greg KH <gregkh@linuxfoundation.org>
>> wrote:
>>>
>>> On Mon, Apr 12, 2021 at 02:50:20PM +0800, dillon min wrote:
>>>> Hi Greg,
>>>>
>>>> Thanks for the quick response, please ignore the last private mail.
>>>>
>>>> On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org>
>>>> wrote:
>>>>>
>>>>> On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com
>>>>> wrote:
>>>>>> From: dillon min <dillon.minfei@gmail.com>
>>>>>>
>>>>>> To avoid potential deadlock in spin_lock usage, change to use
>>>>>> spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn)
>>>>>> context.
>>>>>> spin_lock(), spin_unlock() under handler context.
>>>>>>
>>>>>> remove unused local_irq_save/restore call.
>>>>>>
>>>>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>
>>>>>> ---
>>>>>> Was verified on stm32f469-disco board. need more test on stm32mp
>>>>>> platform.
>>>>>>
>>>>>> drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
>>>>>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/tty/serial/stm32-usart.c
>>>>>> b/drivers/tty/serial/stm32-usart.c
>>>>>> index b3675cf25a69..c4c859b34367 100644
>>>>>> --- a/drivers/tty/serial/stm32-usart.c
>>>>>> +++ b/drivers/tty/serial/stm32-usart.c
>>>>>> @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct
>>>>>> uart_port *port, bool threaded)
>>>>>> struct tty_port *tport = &port->state->port;
>>>>>> struct stm32_port *stm32_port = to_stm32_port(port);
>>>>>> const struct stm32_usart_offsets *ofs =
>>>>>> &stm32_port->info->ofs;
>>>>>> - unsigned long c;
>>>>>> + unsigned long c, flags;
>>>>>> u32 sr;
>>>>>> char flag;
>>>>>>
>>>>>> @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct
>>>>>> uart_port *port, bool threaded)
>>>>>> uart_insert_char(port, sr, USART_SR_ORE, c, flag);
>>>>>> }
>>>>>>
>>>>>> - spin_unlock(&port->lock);
>>>>>> + if (threaded)
>>>>>> + spin_unlock_irqrestore(&port->lock, flags);
>>>>>> + else
>>>>>> + spin_unlock(&port->lock);
>>>>>
>>>>> You shouldn't have to check for this, see the other patches on the
>>>>> list
>>>>> recently that fixed this up to not be an issue for irq handlers.
>>>> Can you help to give more hints, or the commit id of the patch which
>>>> fixed this. thanks.
>>>>
>>>> I'm still confused with this.
>>>>
>>>> The stm32_usart_threaded_interrupt() is a kthread context, once
>>>> port->lock holds by this function, another serial interrupts raised,
>>>> such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
>>>> there will be a deadlock. isn't it?
>>>>
>>>> So, shouldn't I use spin_lock{_irqsave} according to the caller's
>>>> context ?
>>>
>>> Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
>>> handlers") for when threaded irq handlers have irqs disabled, isn't that
>>> the case you are trying to "protect" from here?
>>>
>>> Why is the "threaded" flag used at all? The driver should not care.
>>>
>>> Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
>>> threading") in linux-next for an example of how this was fixed up in a
>>> serial driver.
>>>
>>> does that help?
>>>
>> Yes, it's really helpful. and 81e2073c175b should be highlighted in a
>> doc.
>> In my past knowledge, we should care about hard irq & thread_fn lock
>> conflict.
>> This patch has totally avoided patching code in the separate driver side.
>> thanks.
>>
>> I will just keep the changes in stm32_usart_console_write(), remove
>> these code in
>> thread_fn. update version 2 for you.
>>
>> thanks.
>>
>> Dillon,
>>> thanks,
>>>
>>> greg k-h
>> _______________________________________________
>> Linux-stm32 mailing list
>> Linux-stm32@st-md-mailman.stormreply.com
>> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Linux-stm32] [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 13:41 ` dillon min
0 siblings, 0 replies; 21+ messages in thread
From: dillon min @ 2021-04-12 13:41 UTC (permalink / raw)
To: Erwan LE RAY
Cc: Greg KH, linux-serial, Alexandre TORGUE,
Linux Kernel Mailing List, Maxime Coquelin, jirislaby,
linux-stm32, Linux ARM, Fabrice Gasnier, Valentin Caron
On 4/12/21, Erwan LE RAY <erwan.leray@foss.st.com> wrote:
> Hi Dillon,
>
> Thanks for your patch.
>
> Could you please elaborate the use case in your commit message ?
Sorry, local_irq_save() plus spin_lock() same to spin_lock_irqsave()
There is no deadlock . Please ignore this patch.
Thanks
Dillon
>
> Best Regards, Erwan.
>
> On 4/12/21 10:54 AM, dillon min wrote:
>> Hi Greg,
>>
>> On Mon, Apr 12, 2021 at 4:25 PM Greg KH <gregkh@linuxfoundation.org>
>> wrote:
>>>
>>> On Mon, Apr 12, 2021 at 02:50:20PM +0800, dillon min wrote:
>>>> Hi Greg,
>>>>
>>>> Thanks for the quick response, please ignore the last private mail.
>>>>
>>>> On Mon, Apr 12, 2021 at 1:52 PM Greg KH <gregkh@linuxfoundation.org>
>>>> wrote:
>>>>>
>>>>> On Mon, Apr 12, 2021 at 12:34:21PM +0800, dillon.minfei@gmail.com
>>>>> wrote:
>>>>>> From: dillon min <dillon.minfei@gmail.com>
>>>>>>
>>>>>> To avoid potential deadlock in spin_lock usage, change to use
>>>>>> spin_lock_irqsave(), spin_unlock_irqrestore() in process(thread_fn)
>>>>>> context.
>>>>>> spin_lock(), spin_unlock() under handler context.
>>>>>>
>>>>>> remove unused local_irq_save/restore call.
>>>>>>
>>>>>> Signed-off-by: dillon min <dillon.minfei@gmail.com>
>>>>>> ---
>>>>>> Was verified on stm32f469-disco board. need more test on stm32mp
>>>>>> platform.
>>>>>>
>>>>>> drivers/tty/serial/stm32-usart.c | 27 +++++++++++++++++----------
>>>>>> 1 file changed, 17 insertions(+), 10 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/tty/serial/stm32-usart.c
>>>>>> b/drivers/tty/serial/stm32-usart.c
>>>>>> index b3675cf25a69..c4c859b34367 100644
>>>>>> --- a/drivers/tty/serial/stm32-usart.c
>>>>>> +++ b/drivers/tty/serial/stm32-usart.c
>>>>>> @@ -214,7 +214,7 @@ static void stm32_usart_receive_chars(struct
>>>>>> uart_port *port, bool threaded)
>>>>>> struct tty_port *tport = &port->state->port;
>>>>>> struct stm32_port *stm32_port = to_stm32_port(port);
>>>>>> const struct stm32_usart_offsets *ofs =
>>>>>> &stm32_port->info->ofs;
>>>>>> - unsigned long c;
>>>>>> + unsigned long c, flags;
>>>>>> u32 sr;
>>>>>> char flag;
>>>>>>
>>>>>> @@ -276,9 +276,17 @@ static void stm32_usart_receive_chars(struct
>>>>>> uart_port *port, bool threaded)
>>>>>> uart_insert_char(port, sr, USART_SR_ORE, c, flag);
>>>>>> }
>>>>>>
>>>>>> - spin_unlock(&port->lock);
>>>>>> + if (threaded)
>>>>>> + spin_unlock_irqrestore(&port->lock, flags);
>>>>>> + else
>>>>>> + spin_unlock(&port->lock);
>>>>>
>>>>> You shouldn't have to check for this, see the other patches on the
>>>>> list
>>>>> recently that fixed this up to not be an issue for irq handlers.
>>>> Can you help to give more hints, or the commit id of the patch which
>>>> fixed this. thanks.
>>>>
>>>> I'm still confused with this.
>>>>
>>>> The stm32_usart_threaded_interrupt() is a kthread context, once
>>>> port->lock holds by this function, another serial interrupts raised,
>>>> such as USART_SR_TXE,stm32_usart_interrupt() can't get the lock,
>>>> there will be a deadlock. isn't it?
>>>>
>>>> So, shouldn't I use spin_lock{_irqsave} according to the caller's
>>>> context ?
>>>
>>> Please see 81e2073c175b ("genirq: Disable interrupts for force threaded
>>> handlers") for when threaded irq handlers have irqs disabled, isn't that
>>> the case you are trying to "protect" from here?
>>>
>>> Why is the "threaded" flag used at all? The driver should not care.
>>>
>>> Also see 9baedb7baeda ("serial: imx: drop workaround for forced irq
>>> threading") in linux-next for an example of how this was fixed up in a
>>> serial driver.
>>>
>>> does that help?
>>>
>> Yes, it's really helpful. and 81e2073c175b should be highlighted in a
>> doc.
>> In my past knowledge, we should care about hard irq & thread_fn lock
>> conflict.
>> This patch has totally avoided patching code in the separate driver side.
>> thanks.
>>
>> I will just keep the changes in stm32_usart_console_write(), remove
>> these code in
>> thread_fn. update version 2 for you.
>>
>> thanks.
>>
>> Dillon,
>>> thanks,
>>>
>>> greg k-h
>> _______________________________________________
>> Linux-stm32 mailing list
>> Linux-stm32@st-md-mailman.stormreply.com
>> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] serial: stm32: optimize spin lock usage
@ 2021-04-12 10:50 kernel test robot
0 siblings, 0 replies; 21+ messages in thread
From: kernel test robot @ 2021-04-12 10:50 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 7953 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <1618202061-8243-1-git-send-email-dillon.minfei@gmail.com>
References: <1618202061-8243-1-git-send-email-dillon.minfei@gmail.com>
TO: dillon.minfei(a)gmail.com
TO: gregkh(a)linuxfoundation.org
TO: jirislaby(a)kernel.org
TO: mcoquelin.stm32(a)gmail.com
TO: alexandre.torgue(a)foss.st.com
CC: linux-serial(a)vger.kernel.org
CC: linux-stm32(a)st-md-mailman.stormreply.com
CC: linux-arm-kernel(a)lists.infradead.org
CC: linux-kernel(a)vger.kernel.org
CC: dillon min <dillon.minfei@gmail.com>
Hi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on stm32/stm32-next]
[also build test WARNING on usb/usb-testing v5.12-rc7]
[cannot apply to tty/tty-testing next-20210409]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/dillon-minfei-gmail-com/serial-stm32-optimize-spin-lock-usage/20210412-123607
base: https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git stm32-next
:::::: branch date: 6 hours ago
:::::: commit date: 6 hours ago
config: m68k-randconfig-m031-20210412 (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/tty/serial/stm32-usart.c:280 stm32_usart_receive_chars() error: uninitialized symbol 'flags'.
vim +/flags +280 drivers/tty/serial/stm32-usart.c
3489187204eb75 Alexandre TORGUE 2016-09-15 211
56f9a76c27b51b Erwan Le Ray 2021-01-06 212 static void stm32_usart_receive_chars(struct uart_port *port, bool threaded)
48a6092fb41fab Maxime Coquelin 2015-06-10 213 {
48a6092fb41fab Maxime Coquelin 2015-06-10 214 struct tty_port *tport = &port->state->port;
ada8618ff3bfe1 Alexandre TORGUE 2016-09-15 215 struct stm32_port *stm32_port = to_stm32_port(port);
d825f0bea20f49 Stephen Boyd 2021-01-22 216 const struct stm32_usart_offsets *ofs = &stm32_port->info->ofs;
a0e81ae10c46f7 dillon min 2021-04-12 217 unsigned long c, flags;
48a6092fb41fab Maxime Coquelin 2015-06-10 218 u32 sr;
48a6092fb41fab Maxime Coquelin 2015-06-10 219 char flag;
48a6092fb41fab Maxime Coquelin 2015-06-10 220
29d60981db522c Andy Shevchenko 2017-08-13 221 if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
48a6092fb41fab Maxime Coquelin 2015-06-10 222 pm_wakeup_event(tport->tty->dev, 0);
48a6092fb41fab Maxime Coquelin 2015-06-10 223
56f9a76c27b51b Erwan Le Ray 2021-01-06 224 while (stm32_usart_pending_rx(port, &sr, &stm32_port->last_res,
56f9a76c27b51b Erwan Le Ray 2021-01-06 225 threaded)) {
48a6092fb41fab Maxime Coquelin 2015-06-10 226 sr |= USART_SR_DUMMY_RX;
48a6092fb41fab Maxime Coquelin 2015-06-10 227 flag = TTY_NORMAL;
48a6092fb41fab Maxime Coquelin 2015-06-10 228
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 229 /*
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 230 * Status bits has to be cleared before reading the RDR:
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 231 * In FIFO mode, reading the RDR will pop the next data
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 232 * (if any) along with its status bits into the SR.
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 233 * Not doing so leads to misalignement between RDR and SR,
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 234 * and clear status bits of the next rx data.
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 235 *
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 236 * Clear errors flags for stm32f7 and stm32h7 compatible
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 237 * devices. On stm32f4 compatible devices, the error bit is
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 238 * cleared by the sequence [read SR - read DR].
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 239 */
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 240 if ((sr & USART_SR_ERR_MASK) && ofs->icr != UNDEF_REG)
1250ed7114a977 Fabrice Gasnier 2019-11-21 241 writel_relaxed(sr & USART_SR_ERR_MASK,
1250ed7114a977 Fabrice Gasnier 2019-11-21 242 port->membase + ofs->icr);
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 243
56f9a76c27b51b Erwan Le Ray 2021-01-06 244 c = stm32_usart_get_char(port, &sr, &stm32_port->last_res);
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 245 port->icount.rx++;
48a6092fb41fab Maxime Coquelin 2015-06-10 246 if (sr & USART_SR_ERR_MASK) {
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 247 if (sr & USART_SR_ORE) {
48a6092fb41fab Maxime Coquelin 2015-06-10 248 port->icount.overrun++;
48a6092fb41fab Maxime Coquelin 2015-06-10 249 } else if (sr & USART_SR_PE) {
48a6092fb41fab Maxime Coquelin 2015-06-10 250 port->icount.parity++;
48a6092fb41fab Maxime Coquelin 2015-06-10 251 } else if (sr & USART_SR_FE) {
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 252 /* Break detection if character is null */
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 253 if (!c) {
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 254 port->icount.brk++;
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 255 if (uart_handle_break(port))
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 256 continue;
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 257 } else {
48a6092fb41fab Maxime Coquelin 2015-06-10 258 port->icount.frame++;
48a6092fb41fab Maxime Coquelin 2015-06-10 259 }
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 260 }
48a6092fb41fab Maxime Coquelin 2015-06-10 261
48a6092fb41fab Maxime Coquelin 2015-06-10 262 sr &= port->read_status_mask;
48a6092fb41fab Maxime Coquelin 2015-06-10 263
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 264 if (sr & USART_SR_PE) {
48a6092fb41fab Maxime Coquelin 2015-06-10 265 flag = TTY_PARITY;
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 266 } else if (sr & USART_SR_FE) {
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 267 if (!c)
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 268 flag = TTY_BREAK;
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 269 else
48a6092fb41fab Maxime Coquelin 2015-06-10 270 flag = TTY_FRAME;
48a6092fb41fab Maxime Coquelin 2015-06-10 271 }
4f01d833fdcdd6 Erwan Le Ray 2019-05-21 272 }
48a6092fb41fab Maxime Coquelin 2015-06-10 273
48a6092fb41fab Maxime Coquelin 2015-06-10 274 if (uart_handle_sysrq_char(port, c))
48a6092fb41fab Maxime Coquelin 2015-06-10 275 continue;
48a6092fb41fab Maxime Coquelin 2015-06-10 276 uart_insert_char(port, sr, USART_SR_ORE, c, flag);
48a6092fb41fab Maxime Coquelin 2015-06-10 277 }
48a6092fb41fab Maxime Coquelin 2015-06-10 278
a0e81ae10c46f7 dillon min 2021-04-12 279 if (threaded)
a0e81ae10c46f7 dillon min 2021-04-12 @280 spin_unlock_irqrestore(&port->lock, flags);
a0e81ae10c46f7 dillon min 2021-04-12 281 else
48a6092fb41fab Maxime Coquelin 2015-06-10 282 spin_unlock(&port->lock);
a0e81ae10c46f7 dillon min 2021-04-12 283
48a6092fb41fab Maxime Coquelin 2015-06-10 284 tty_flip_buffer_push(tport);
a0e81ae10c46f7 dillon min 2021-04-12 285
a0e81ae10c46f7 dillon min 2021-04-12 286 if (threaded)
a0e81ae10c46f7 dillon min 2021-04-12 287 spin_lock_irqsave(&port->lock, flags);
a0e81ae10c46f7 dillon min 2021-04-12 288 else
48a6092fb41fab Maxime Coquelin 2015-06-10 289 spin_lock(&port->lock);
48a6092fb41fab Maxime Coquelin 2015-06-10 290 }
48a6092fb41fab Maxime Coquelin 2015-06-10 291
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30026 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2021-04-12 13:43 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 4:34 [PATCH] serial: stm32: optimize spin lock usage dillon.minfei
2021-04-12 4:34 ` dillon.minfei
2021-04-12 5:52 ` Greg KH
2021-04-12 5:52 ` Greg KH
2021-04-12 6:50 ` dillon min
2021-04-12 6:50 ` dillon min
2021-04-12 8:25 ` Greg KH
2021-04-12 8:25 ` Greg KH
2021-04-12 8:54 ` dillon min
2021-04-12 8:54 ` dillon min
2021-04-12 13:19 ` [Linux-stm32] " Erwan LE RAY
2021-04-12 13:19 ` Erwan LE RAY
2021-04-12 13:41 ` dillon min
2021-04-12 13:41 ` dillon min
2021-04-12 7:23 ` kernel test robot
2021-04-12 7:23 ` kernel test robot
2021-04-12 7:23 ` kernel test robot
2021-04-12 7:29 ` dillon min
2021-04-12 7:29 ` dillon min
2021-04-12 7:29 ` dillon min
2021-04-12 10:50 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.