Skip to content

Review: PR #754 - Invert USB wakeup_on_write default to prevent DWC2 lockup#1

Closed
Copilot wants to merge 1 commit intofix/usb-hid-wakeup-disconnectfrom
copilot/review-changes-in-pull-754
Closed

Review: PR #754 - Invert USB wakeup_on_write default to prevent DWC2 lockup#1
Copilot wants to merge 1 commit intofix/usb-hid-wakeup-disconnectfrom
copilot/review-changes-in-pull-754

Conversation

Copy link
Copy Markdown

Copilot AI commented Feb 13, 2026

Review Summary

Status: APPROVED ✅

PR sipeed#754 fixes USB HID lockup during host reboot by inverting wakeup_on_write from opt-out to opt-in.

Problem

DWC2 controller spams USB remote wakeup signals during host restart/BIOS. Host rejects these, causing:

Changes

Logic inversion in 2 files, 6 lines total:

# Before (unsafe default)
if [ ! -e /boot/usb.notwakeup ]; then
    echo 1 > functions/hid.GS*/wakeup_on_write
fi

# After (safe default)
if [ -e /boot/usb.wakeup ]; then
    echo 1 > functions/hid.GS*/wakeup_on_write
fi

Applied to keyboard/mouse/touchpad in:

  • kvmapp/system/init.d/S03usbdev
  • kvmapp/system/init.d/S03usbhid

Review Findings

  • ✅ Logic correct, consistently applied
  • ✅ No syntax errors, valid POSIX sh
  • ✅ No security vulnerabilities
  • ✅ Minimal change scope (6 lines)
  • ⚠️ Minor breaking change: users needing remote wakeup must create /boot/usb.wakeup

Recommendation

Merge. Acceptable trade-off—current default causes crashes for many users. USB HID functions correctly without remote wakeup.

Optional: Document /boot/usb.wakeup flag in follow-up.

Original prompt

Pull request: https://github.com/sipeed/NanoKVM/pull/754/files

review the changes


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI assigned Copilot and 7tg Feb 13, 2026
Copilot AI changed the title [WIP] Review changes in pull request 754 Review: PR #754 - Invert USB wakeup_on_write default to prevent DWC2 lockup Feb 13, 2026
Copilot AI requested a review from 7tg February 13, 2026 12:11
@7tg 7tg closed this Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants