marfrit/bes2600-dkms
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bes2600: Patch H — bh.c hygiene cleanup (drop fossil blocks, dead stubs) #10

Merged
marfrit merged 1 commits from bes2600/bh-c-fossil-cleanup into cleanups 2026-05-08 06:30:40 +00:00
Conversation 1 Commits 1 Files Changed 3
+28 -564
claude-noether commented 2026-05-08 06:23:55 +00:00
Collaborator
Copy Link
Copy Source

Per Opus structural critique §4.1 (#if 0 graveyard), §4.3 (asm volatile("nop") placeholder), §4.4 (BUG_ON in steady-state hot path). Pure source-tree cleanup, no functional change.

Diff scope

  • bes2600/bh.c −578/+27 (mostly deletions)
  • bes2600/hwio.c −7/+7 (stub function → comment block)
  • bes2600/sbus.h −2/+1 (declaration → comment)
  • net: −578/+28 across 3 files

What got removed

location size rationale
1 bh.c 319-395 76 lines dead cw1200 helpers (bes2600_bh_read_ctrl_reg, bes2600_get_skb, bes2600_put_skb, bes2600_device_wakeup) under #if 0
2 bh.c 405-873 + 1659 468 lines cw1200-ancestor bes2600_bh() function body preserved verbatim alongside the active impl. Same function name. Same goto labels. Maintenance hazard.
3 bh.c done: body 5 lines __bes2600_irq_enable(1) placeholder (commented out) + asm volatile("nop") filler
4 bh.c post-loop block 4 lines sbus lock + __bes2600_irq_enable(0) + sbus unlock — stub call wrapped in lock/unlock ceremony
5 hwio.c 327-330 3 lines __bes2600_irq_enable() stub returning 0
6 sbus.h 99 1 line stub forward declaration

What got replaced

site change
7 bh.c steady-state BUG_ON(hw_bufs_used > numInpChBufs) → WARN_ON_ONCE — runs every bh-loop iteration; locking up the kernel on a bookkeeping bug is the wrong response

Why __bes2600_irq_enable was a stub

cw1200 has the same-named function (drivers/net/wireless/st/cw1200/hwio.c:267) that does real work — reads ST90TDS_CONFIG_REG_ID and toggles the ST90TDS_CONF_IRQ_RDY_ENABLE bit. bes2600 inherited the function name + signature when forked, but the bes2600 chip's IRQ enable is managed by sdio_claim_irq + chip-side firmware, not by a driver-side enable register. Bestechnic kept the function as a no-op stub (return 0).

Build verification

Deferred — ohm offline. Pure-deletion change, no semantic risk: the deleted code was either #if 0-gated (never compiled) or stub-implementations (always returned 0). When ohm is back, single sandbox build + smoke-test will confirm.

Test plan

  • Build on ohm sandbox once ohm is back online
  • Smoke-test module load + association
  • No Phase 7 stress ramp needed (no hot-path semantic change)
  • Fallback rollback: any prior /var/tmp/bes2600.patch*.rollback.ko file
Per Opus structural critique §4.1 (#if 0 graveyard), §4.3 (`asm volatile("nop")` placeholder), §4.4 (BUG_ON in steady-state hot path). Pure source-tree cleanup, no functional change. ## Diff scope - `bes2600/bh.c` −578/+27 (mostly deletions) - `bes2600/hwio.c` −7/+7 (stub function → comment block) - `bes2600/sbus.h` −2/+1 (declaration → comment) - net: **−578/+28** across 3 files ## What got removed | | location | size | rationale | |---|---|---:|---| | 1 | bh.c 319-395 | 76 lines | dead cw1200 helpers (bes2600_bh_read_ctrl_reg, bes2600_get_skb, bes2600_put_skb, bes2600_device_wakeup) under #if 0 | | 2 | bh.c 405-873 + 1659 | 468 lines | cw1200-ancestor bes2600_bh() function body preserved verbatim alongside the active impl. Same function name. Same goto labels. Maintenance hazard. | | 3 | bh.c done: body | 5 lines | `__bes2600_irq_enable(1)` placeholder (commented out) + `asm volatile("nop")` filler | | 4 | bh.c post-loop block | 4 lines | sbus lock + __bes2600_irq_enable(0) + sbus unlock — stub call wrapped in lock/unlock ceremony | | 5 | hwio.c 327-330 | 3 lines | `__bes2600_irq_enable()` stub returning 0 | | 6 | sbus.h 99 | 1 line | stub forward declaration | ## What got replaced | | site | change | |---|---|---| | 7 | bh.c steady-state BUG_ON(hw_bufs_used > numInpChBufs) | → WARN_ON_ONCE — runs every bh-loop iteration; locking up the kernel on a bookkeeping bug is the wrong response | ## Why `__bes2600_irq_enable` was a stub cw1200 has the same-named function (`drivers/net/wireless/st/cw1200/hwio.c:267`) that does real work — reads `ST90TDS_CONFIG_REG_ID` and toggles the `ST90TDS_CONF_IRQ_RDY_ENABLE` bit. bes2600 inherited the function name + signature when forked, but the bes2600 chip's IRQ enable is managed by `sdio_claim_irq` + chip-side firmware, not by a driver-side enable register. Bestechnic kept the function as a no-op stub (`return 0`). ## Build verification **Deferred — ohm offline.** Pure-deletion change, no semantic risk: the deleted code was either #if 0-gated (never compiled) or stub-implementations (always returned 0). When ohm is back, single sandbox build + smoke-test will confirm. ## Test plan - [ ] Build on ohm sandbox once ohm is back online - [ ] Smoke-test module load + association - [ ] No Phase 7 stress ramp needed (no hot-path semantic change) - [ ] Fallback rollback: any prior `/var/tmp/bes2600.patch*.rollback.ko` file
claude-noether added 1 commit 2026-05-08 06:23:55 +00:00
bes2600: Patch H — bh.c hygiene cleanup (drop fossil blocks, dead stubs) f68fd5530f 
Per Opus structural critique §4.1 (#if 0 graveyard), §4.3 (asm
volatile("nop") placeholder), §4.4 (BUG_ON in steady-state hot
path).  Pure source-tree cleanup, no functional change.

Removed:

  1. bh.c lines 319-395 (76-line #if 0 block) — dead helper
     functions inherited from cw1200 ancestor:
     bes2600_bh_read_ctrl_reg, bes2600_get_skb, bes2600_put_skb,
     bes2600_device_wakeup.  Compiled out for years.

  2. bh.c lines 405-873 + line 1659 (the outer #if 0 / #else /
     #endif) — 468-line cw1200-ancestor bes2600_bh() function body,
     preserved verbatim alongside the active impl.  Same function
     name, same goto labels.  Maintenance hazard removed.

  3. bh.c done: label body — `__bes2600_irq_enable(1)` placeholder
     (commented out) + `asm volatile ("nop")` filler.  Both
     no-ops on bes2600 silicon.

  4. bh.c post-loop "Explicitly disable device interrupts" block
     (sbus lock + __bes2600_irq_enable(0) + sbus unlock) — the
     stub call wrapped in lock/unlock ceremony.  Dead.

  5. hwio.c __bes2600_irq_enable() function definition —
     `int __bes2600_irq_enable(int enable) { return 0; }`.  Stub.
     Removed entirely.

  6. sbus.h __bes2600_irq_enable() forward declaration.

Replaced:

  7. bh.c bes2600_bh outer-loop BUG_ON(hw_bufs_used > numInpChBufs)
     -> WARN_ON_ONCE.  The BUG_ON ran every bh-loop iteration;
     tripping it on a bookkeeping bug locks the kernel up during
     normal operation — the wrong response to a (recoverable)
     accounting drift.  WARN_ON_ONCE surfaces the issue without
     taking the system down.

Why __bes2600_irq_enable was a stub on bes2600:

  cw1200 has the same-named function (drivers/net/wireless/st/cw1200/
  hwio.c:267) that does real work — reads ST90TDS_CONFIG_REG_ID and
  toggles the ST90TDS_CONF_IRQ_RDY_ENABLE bit.  bes2600 inherited
  the function name + signature when forked, but the bes2600 chip's
  IRQ enable is managed by sdio_claim_irq + chip-side firmware, not
  by a driver-side enable register.  Bestechnic kept the function as
  a no-op stub (return 0).  Patch H removes the dead infrastructure.

Diff scope:

  - bes2600/bh.c   -578/+27   (mostly deletions)
  - bes2600/hwio.c -7/+7     (stub function -> comment block)
  - bes2600/sbus.h -2/+1     (declaration -> comment)
  - net: -578/+28 across 3 files

Build verification deferred — ohm offline.  Pure-deletion change,
no semantic risk; the deleted code was either #if 0-gated
(never compiled) or stub-implementations (always returned 0).
marfrit merged commit 87a3d65960 into cleanups 2026-05-08 06:30:40 +00:00
claude-noether commented 2026-05-08 06:43:08 +00:00
Author
Collaborator
Copy Link
Copy Source

Build verified via vpn

Built on ohm sandbox via openvpn (ohm.vpn = fd96:cafe:cafe::1002, ~138ms RTT). Module srcversion 99052D756C6ED08252B0E68, 0 warnings, 0 errors, 14.78 MB unstripped.

Module is staged at /var/tmp/c6-sandbox/bes2600.ko. NOT installed (per the reboot-button rule; no wired fallback right now → cannot autonomously reboot). When ohm is back at home, swap-and-reboot to verify load + smoke-test.

Danctnix-flavor delta required (same as previous patches): bh.c from_timer → timer_container_of, del_timer_sync → timer_delete_sync. Already documented in feedback_bes2600_dual_tree_flavors memory.

## Build verified via vpn Built on ohm sandbox via openvpn (ohm.vpn = fd96:cafe:cafe::1002, ~138ms RTT). Module srcversion **99052D756C6ED08252B0E68**, 0 warnings, 0 errors, 14.78 MB unstripped. Module is staged at /var/tmp/c6-sandbox/bes2600.ko. NOT installed (per the reboot-button rule; no wired fallback right now → cannot autonomously reboot). When ohm is back at home, swap-and-reboot to verify load + smoke-test. Danctnix-flavor delta required (same as previous patches): bh.c from_timer → timer_container_of, del_timer_sync → timer_delete_sync. Already documented in feedback_bes2600_dual_tree_flavors memory.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claude-noether (Claude (noether)) marfrit (Markus Fritsche)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marfrit/bes2600-dkms#10
Delete Branch "bes2600/bh-c-fossil-cleanup"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?