Skip to content

Conversation

@hujun260
Copy link
Contributor

Summary

use small lock in following files

arch/xtensa/src/esp32s3/esp32s3_rtc_lowerhalf.c
arch/z16/src/z16f/z16f_serial.c
arch/z80/src/z8/z8_serial.c
boards/arm/max326xx/max32660-evsys/src/max326_button.c boards/arm/stm32/stm32f4discovery/src/stm32_gs2200m.c drivers/i2c/i2c_bitbang.c
drivers/power/supply/act8945a.c
drivers/serial/uart_pci_16550.c
drivers/serial/uart_pl011.c
drivers/wireless/bluetooth/bt_uart_shim.c

Impact

arch/xtensa/src/esp32s3/esp32s3_rtc_lowerhalf.c
arch/z16/src/z16f/z16f_serial.c
arch/z80/src/z8/z8_serial.c
boards/arm/max326xx/max32660-evsys/src/max326_button.c boards/arm/stm32/stm32f4discovery/src/stm32_gs2200m.c drivers/i2c/i2c_bitbang.c
drivers/power/supply/act8945a.c
drivers/serial/uart_pci_16550.c
drivers/serial/uart_pl011.c
drivers/wireless/bluetooth/bt_uart_shim.c

Testing

ci

@github-actions github-actions bot added Area: Bluetooth Arch: xtensa Issues related to the Xtensa architecture Arch: z16 Issues related to the Z16 architecture Arch: z80 Issues related to the Z80 architecture Area: Drivers Drivers issues Board: arm Size: M The size of the change in this PR is medium labels Dec 20, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 20, 2024

[Experimental Bot, please feedback here]

This PR appears to partially meet the NuttX requirements, but needs more information.

Missing/Insufficient Information:

  • Summary: "use small lock" is vague. Why are these changes necessary? What problem does it solve? What kind of "small lock" is being used (e.g., a spinlock, mutex, semaphore)? Explain the rationale behind using this lock. Provide a high-level overview of how the code is changed (e.g., "Replaced mutex with spinlock in interrupt handler for performance reasons"). Include relevant NuttX issue numbers if applicable.
  • Impact: Just listing the files modified is not enough. Address each impact question specifically. For example:
    • User Impact: Likely NO, but explain why. Does this change behavior in a way the user might notice?
    • Build Impact: Likely NO, but confirm. Are any new dependencies introduced?
    • Hardware Impact: Specify affected architectures and boards. Does this change driver behavior or add support for new hardware?
    • Documentation Impact: Do any documentation updates need to be made to reflect these changes?
    • Security Impact: Does using a different locking mechanism have security implications? Justify your answer.
    • Compatibility Impact: Are there any backward compatibility concerns? Does this affect interoperability with other systems or software?
  • Testing: "ci" is not sufficient. Provide details about your local setup (OS, compiler, etc.) and the target hardware used for testing. Include actual testing logs (even if brief) showing the behavior before and after the change. Demonstrate that the changes work as intended and don't introduce regressions. Ideally, the tests should exercise the code paths where the locking mechanisms are used.

Example of Improved Impact Section:

  • Impact:
    • User Impact: NO. This change is internal and should not affect user-visible behavior.
    • Build Impact: NO. No changes to the build system are required.
    • Hardware Impact: YES. This change affects the following:
      • ARM: max326xx/max32660-evsys, stm32/stm32f4discovery
      • Xtensa: esp32s3
      • Z16: z16f
      • Z80: z8
      • PCI 16550 UART
      • PL011 UART
    • Documentation Impact: NO. No documentation updates are required for this internal change.
    • Security Impact: Potentially YES. Switching to a spinlock could lead to priority inversion if not handled carefully. However, we have ensured that... [explain mitigation].
    • Compatibility Impact: NO. This change maintains backward compatibility.

By providing more detail and addressing all the required points, the PR will be much easier to review and merge.

@hujun260 hujun260 force-pushed the apache_43 branch 2 times, most recently from ff0f564 to e8b2aea Compare December 20, 2024 12:52
arch/xtensa/src/esp32s3/esp32s3_rtc_lowerhalf.c
arch/z16/src/z16f/z16f_serial.c
arch/z80/src/z8/z8_serial.c
boards/arm/max326xx/max32660-evsys/src/max326_button.c
boards/arm/stm32/stm32f4discovery/src/stm32_gs2200m.c
drivers/i2c/i2c_bitbang.c
drivers/power/supply/act8945a.c
drivers/serial/uart_pci_16550.c
drivers/serial/uart_pl011.c
drivers/wireless/bluetooth/bt_uart_shim.c

Signed-off-by: hujun5 <hujun5@xiaomi.com>
@xiaoxiang781216 xiaoxiang781216 merged commit 8411a97 into apache:master Dec 22, 2024
27 checks passed
hujun260 added a commit to hujun260/nuttx that referenced this pull request Dec 24, 2024
0  up_testset (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/arch/spinlock.h:96
1  spin_lock_wo_note (lock=<optimized out>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:207
2  spin_lock_irqsave_wo_note (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:467
3  spin_lock_irqsave (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:521
4  pl011_txint (dev=0x404240b0 <g_uart1port>, enable=false) at serial/uart_pl011.c:746
5  0x00000000402a3f1c in uart_xmitchars (dev=0x404240b0 <g_uart1port>) at serial/serial_io.c:118
6  0x00000000402a10f8 in pl011_txint (dev=<optimized out>, enable=<optimized out>) at serial/uart_pl011.c:756
7  0x00000000402a2ca0 in uart_write (filep=<optimized out>, buffer=<optimized out>, buflen=0) at serial/serial.c:1493
8  0x000000004028c464 in file_writev_compat (filep=0x4046cda0, uio=<optimized out>) at vfs/fs_write.c:81
9  0x000000004028c588 in file_writev (filep=<optimized out>, uio=uio@entry=0x40470dc0) at vfs/fs_write.c:161
10 0x000000004028c5fc in nx_writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:257
11 0x000000004028c660 in writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:356
12 0x000000004028c6dc in write (fd=<optimized out>, buf=buf@entry=0x404090b2 <g_nshgreeting>, nbytes=<optimized out>) at vfs/fs_write.c:421
13 0x00000000402adb10 in nsh_session (pstate=pstate@entry=0x40471080, login=login@entry=1, argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_session.c:108
14 0x00000000402ad94c in nsh_consolemain (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_consolemain.c:75
15 0x00000000402ad894 in nsh_main (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_main.c:74
16 0x00000000402a5880 in nxtask_startup (entrypt=0x402ad7fc <nsh_main>, argc=1, argv=0x4046cf40) at sched/task_startup.c:72
17 0x000000004029d444 in nxtask_start () at task/task_start.c:116
18 0x0000000000000000 in ?? ()

fix regresion from apache#15301

Signed-off-by: hujun5 <hujun5@xiaomi.com>
@hujun260 hujun260 mentioned this pull request Dec 24, 2024
hujun260 added a commit to hujun260/nuttx that referenced this pull request Dec 25, 2024
0  up_testset (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/arch/spinlock.h:96
1  spin_lock_wo_note (lock=<optimized out>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:207
2  spin_lock_irqsave_wo_note (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:467
3  spin_lock_irqsave (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:521
4  pl011_txint (dev=0x404240b0 <g_uart1port>, enable=false) at serial/uart_pl011.c:746
5  0x00000000402a3f1c in uart_xmitchars (dev=0x404240b0 <g_uart1port>) at serial/serial_io.c:118
6  0x00000000402a10f8 in pl011_txint (dev=<optimized out>, enable=<optimized out>) at serial/uart_pl011.c:756
7  0x00000000402a2ca0 in uart_write (filep=<optimized out>, buffer=<optimized out>, buflen=0) at serial/serial.c:1493
8  0x000000004028c464 in file_writev_compat (filep=0x4046cda0, uio=<optimized out>) at vfs/fs_write.c:81
9  0x000000004028c588 in file_writev (filep=<optimized out>, uio=uio@entry=0x40470dc0) at vfs/fs_write.c:161
10 0x000000004028c5fc in nx_writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:257
11 0x000000004028c660 in writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:356
12 0x000000004028c6dc in write (fd=<optimized out>, buf=buf@entry=0x404090b2 <g_nshgreeting>, nbytes=<optimized out>) at vfs/fs_write.c:421
13 0x00000000402adb10 in nsh_session (pstate=pstate@entry=0x40471080, login=login@entry=1, argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_session.c:108
14 0x00000000402ad94c in nsh_consolemain (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_consolemain.c:75
15 0x00000000402ad894 in nsh_main (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_main.c:74
16 0x00000000402a5880 in nxtask_startup (entrypt=0x402ad7fc <nsh_main>, argc=1, argv=0x4046cf40) at sched/task_startup.c:72
17 0x000000004029d444 in nxtask_start () at task/task_start.c:116
18 0x0000000000000000 in ?? ()

fix regresion from apache#15301

Signed-off-by: hujun5 <hujun5@xiaomi.com>
acassis pushed a commit that referenced this pull request Dec 25, 2024
0  up_testset (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/arch/spinlock.h:96
1  spin_lock_wo_note (lock=<optimized out>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:207
2  spin_lock_irqsave_wo_note (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:467
3  spin_lock_irqsave (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:521
4  pl011_txint (dev=0x404240b0 <g_uart1port>, enable=false) at serial/uart_pl011.c:746
5  0x00000000402a3f1c in uart_xmitchars (dev=0x404240b0 <g_uart1port>) at serial/serial_io.c:118
6  0x00000000402a10f8 in pl011_txint (dev=<optimized out>, enable=<optimized out>) at serial/uart_pl011.c:756
7  0x00000000402a2ca0 in uart_write (filep=<optimized out>, buffer=<optimized out>, buflen=0) at serial/serial.c:1493
8  0x000000004028c464 in file_writev_compat (filep=0x4046cda0, uio=<optimized out>) at vfs/fs_write.c:81
9  0x000000004028c588 in file_writev (filep=<optimized out>, uio=uio@entry=0x40470dc0) at vfs/fs_write.c:161
10 0x000000004028c5fc in nx_writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:257
11 0x000000004028c660 in writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:356
12 0x000000004028c6dc in write (fd=<optimized out>, buf=buf@entry=0x404090b2 <g_nshgreeting>, nbytes=<optimized out>) at vfs/fs_write.c:421
13 0x00000000402adb10 in nsh_session (pstate=pstate@entry=0x40471080, login=login@entry=1, argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_session.c:108
14 0x00000000402ad94c in nsh_consolemain (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_consolemain.c:75
15 0x00000000402ad894 in nsh_main (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_main.c:74
16 0x00000000402a5880 in nxtask_startup (entrypt=0x402ad7fc <nsh_main>, argc=1, argv=0x4046cf40) at sched/task_startup.c:72
17 0x000000004029d444 in nxtask_start () at task/task_start.c:116
18 0x0000000000000000 in ?? ()

fix regresion from #15301

Signed-off-by: hujun5 <hujun5@xiaomi.com>
jerpelea pushed a commit to jerpelea/nuttx that referenced this pull request Dec 27, 2024
0  up_testset (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/arch/spinlock.h:96
1  spin_lock_wo_note (lock=<optimized out>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:207
2  spin_lock_irqsave_wo_note (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:467
3  spin_lock_irqsave (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:521
4  pl011_txint (dev=0x404240b0 <g_uart1port>, enable=false) at serial/uart_pl011.c:746
5  0x00000000402a3f1c in uart_xmitchars (dev=0x404240b0 <g_uart1port>) at serial/serial_io.c:118
6  0x00000000402a10f8 in pl011_txint (dev=<optimized out>, enable=<optimized out>) at serial/uart_pl011.c:756
7  0x00000000402a2ca0 in uart_write (filep=<optimized out>, buffer=<optimized out>, buflen=0) at serial/serial.c:1493
8  0x000000004028c464 in file_writev_compat (filep=0x4046cda0, uio=<optimized out>) at vfs/fs_write.c:81
9  0x000000004028c588 in file_writev (filep=<optimized out>, uio=uio@entry=0x40470dc0) at vfs/fs_write.c:161
10 0x000000004028c5fc in nx_writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:257
11 0x000000004028c660 in writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:356
12 0x000000004028c6dc in write (fd=<optimized out>, buf=buf@entry=0x404090b2 <g_nshgreeting>, nbytes=<optimized out>) at vfs/fs_write.c:421
13 0x00000000402adb10 in nsh_session (pstate=pstate@entry=0x40471080, login=login@entry=1, argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_session.c:108
14 0x00000000402ad94c in nsh_consolemain (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_consolemain.c:75
15 0x00000000402ad894 in nsh_main (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_main.c:74
16 0x00000000402a5880 in nxtask_startup (entrypt=0x402ad7fc <nsh_main>, argc=1, argv=0x4046cf40) at sched/task_startup.c:72
17 0x000000004029d444 in nxtask_start () at task/task_start.c:116
18 0x0000000000000000 in ?? ()

fix regresion from apache#15301

Signed-off-by: hujun5 <hujun5@xiaomi.com>
xiaoxiang781216 pushed a commit that referenced this pull request Dec 28, 2024
0  up_testset (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/arch/spinlock.h:96
1  spin_lock_wo_note (lock=<optimized out>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:207
2  spin_lock_irqsave_wo_note (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:467
3  spin_lock_irqsave (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:521
4  pl011_txint (dev=0x404240b0 <g_uart1port>, enable=false) at serial/uart_pl011.c:746
5  0x00000000402a3f1c in uart_xmitchars (dev=0x404240b0 <g_uart1port>) at serial/serial_io.c:118
6  0x00000000402a10f8 in pl011_txint (dev=<optimized out>, enable=<optimized out>) at serial/uart_pl011.c:756
7  0x00000000402a2ca0 in uart_write (filep=<optimized out>, buffer=<optimized out>, buflen=0) at serial/serial.c:1493
8  0x000000004028c464 in file_writev_compat (filep=0x4046cda0, uio=<optimized out>) at vfs/fs_write.c:81
9  0x000000004028c588 in file_writev (filep=<optimized out>, uio=uio@entry=0x40470dc0) at vfs/fs_write.c:161
10 0x000000004028c5fc in nx_writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:257
11 0x000000004028c660 in writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:356
12 0x000000004028c6dc in write (fd=<optimized out>, buf=buf@entry=0x404090b2 <g_nshgreeting>, nbytes=<optimized out>) at vfs/fs_write.c:421
13 0x00000000402adb10 in nsh_session (pstate=pstate@entry=0x40471080, login=login@entry=1, argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_session.c:108
14 0x00000000402ad94c in nsh_consolemain (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_consolemain.c:75
15 0x00000000402ad894 in nsh_main (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_main.c:74
16 0x00000000402a5880 in nxtask_startup (entrypt=0x402ad7fc <nsh_main>, argc=1, argv=0x4046cf40) at sched/task_startup.c:72
17 0x000000004029d444 in nxtask_start () at task/task_start.c:116
18 0x0000000000000000 in ?? ()

fix regresion from #15301

Signed-off-by: hujun5 <hujun5@xiaomi.com>
linguini1 pushed a commit to CarletonURocketry/nuttx that referenced this pull request Jan 15, 2025
0  up_testset (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/arch/spinlock.h:96
1  spin_lock_wo_note (lock=<optimized out>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:207
2  spin_lock_irqsave_wo_note (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:467
3  spin_lock_irqsave (lock=0x404241c0 <g_uart1priv+32>) at /home/hujun5/下载/vela_sim/nuttx/include/nuttx/spinlock.h:521
4  pl011_txint (dev=0x404240b0 <g_uart1port>, enable=false) at serial/uart_pl011.c:746
5  0x00000000402a3f1c in uart_xmitchars (dev=0x404240b0 <g_uart1port>) at serial/serial_io.c:118
6  0x00000000402a10f8 in pl011_txint (dev=<optimized out>, enable=<optimized out>) at serial/uart_pl011.c:756
7  0x00000000402a2ca0 in uart_write (filep=<optimized out>, buffer=<optimized out>, buflen=0) at serial/serial.c:1493
8  0x000000004028c464 in file_writev_compat (filep=0x4046cda0, uio=<optimized out>) at vfs/fs_write.c:81
9  0x000000004028c588 in file_writev (filep=<optimized out>, uio=uio@entry=0x40470dc0) at vfs/fs_write.c:161
10 0x000000004028c5fc in nx_writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:257
11 0x000000004028c660 in writev (fd=<optimized out>, iov=iov@entry=0x40470e10, iovcnt=iovcnt@entry=1) at vfs/fs_write.c:356
12 0x000000004028c6dc in write (fd=<optimized out>, buf=buf@entry=0x404090b2 <g_nshgreeting>, nbytes=<optimized out>) at vfs/fs_write.c:421
13 0x00000000402adb10 in nsh_session (pstate=pstate@entry=0x40471080, login=login@entry=1, argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_session.c:108
14 0x00000000402ad94c in nsh_consolemain (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_consolemain.c:75
15 0x00000000402ad894 in nsh_main (argc=argc@entry=1, argv=argv@entry=0x4046cf40) at nsh_main.c:74
16 0x00000000402a5880 in nxtask_startup (entrypt=0x402ad7fc <nsh_main>, argc=1, argv=0x4046cf40) at sched/task_startup.c:72
17 0x000000004029d444 in nxtask_start () at task/task_start.c:116
18 0x0000000000000000 in ?? ()

fix regresion from apache#15301

Signed-off-by: hujun5 <hujun5@xiaomi.com>
@hujun260 hujun260 deleted the apache_43 branch February 5, 2025 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: xtensa Issues related to the Xtensa architecture Arch: z16 Issues related to the Z16 architecture Arch: z80 Issues related to the Z80 architecture Area: Bluetooth Area: Drivers Drivers issues Board: arm Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants