Re: [PATCH] serial: stm32: optimize spin lock usage
From: dillon min
Date: Mon Apr 12 2021 - 03:31:27 EST
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@xxxxxxxxx> 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@xxxxxxxxx>
>
> 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@xxxxxxxxxxxx