net: pse-pd: add poll path and LED trigger support
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v5 Latest2 patches 6 repliesIn review
- v42 patches 7 repliesSuperseded
- v33 patches 2 repliesSuperseded
Earlier versions · 2
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v2 | 3 patches | 4 replies | Superseded | |
| v1 | 1 patch | 8 replies | Superseded |
Changes in v5 from the cover letter
- Rebased on top of "net: pse-pd: fix out-of-bounds bitmap access in pse_isr() on 32-bit" (5099807f335c, merged via net). Extended the same bitmap-pointer pattern to the new pse_handle_events() and pse_poll_worker() code paths so for_each_set_bit() does not read past a single unsigned long when nr_lines > BITS_PER_LONG (Jakub, Kory)
- Cancel the poll work explicitly in pse_controller_unregister(), next to the existing disable_irq() for the IRQ path, instead of relying on devm_add_action_or_reset() LIFO ordering. Makes the helper safe even when a driver registers it in the standard order (helper before devm_pse_controller_register()) (Jakub)
- struct pse_pi_led_triggers no longer wrapped in
#if IS_ENABLED(CONFIG_LEDS_TRIGGERS); only the function bodies remain ifdef'd, so reference sites no longer need their own ifdefs (Jakub)
- Added .activate callbacks for both LED triggers. Without these, an LED bound to a trigger after pse_controller_register() (e.g. via sysfs) would stay dark until the next hardware event toggled state (Jakub)
- Run the post-registration initial-state pass under pcdev->lock, matching pse_led_update()'s documented locking contract and avoiding races with concurrent regulator_enable() that share last_delivering / last_enabled and the hardware ops (Jakub)
- On partial pse_led_triggers_register() failure, NULL out pcdev->pi_led_trigs so the existing early-return guard in pse_led_update() short-circuits any later calls onto partially-registered triggers (Jakub)
Patches in v5 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/2 | net: pse-pd: add devm_pse_poll_helper()drivers/ include/ | +140 | 3 replies | |
| 2/2 | net: pse-pd: add LED trigger support via notification pathdrivers/ include/ | +217 | 3 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v5 yet.
Cover letter [PATCH net-next v5 0/2] net: pse-pd: add poll path and LED trigger support
Thanks to Kory, Oleksij, Krzysztof, Andrew and Jakub for all the helpful feedback on earlier versions:-) I really appreciate the time you put into reviewing this. I hope this is now the last round ;-)
This series adds poll-based event detection and LED trigger support to the PSE core subsystem.
Patch 1 introduces the poll path independently of LED support, so it can be tested in isolation on boards with and without IRQ configured.
Patch 2 adds LED triggers that hook into the shared event handling path introduced by patch 1.
Note: while reworking the poll teardown for v5, I noticed pse_release_pis() (which kfree()s the pi[] array) runs before disable_irq() / cancel_delayed_work_sync() in pse_controller_unregister(). A concurrent IRQ or poll worker that fires in that window would read through a freed pi[]. This is pre-existing for the IRQ path (since fc0e6db30941, "Add support for reporting events"); the v5 poll cancel inherits the same placement for symmetry. Does it make sense that I send a separate fix that disables both async sources before pse_release_pis() or am I wrong here?
Changes since v4:
- Rebased on top of "net: pse-pd: fix out-of-bounds bitmap access in pse_isr() on 32-bit" (5099807f335c, merged via net). Extended the same bitmap-pointer pattern to the new pse_handle_events() and pse_poll_worker() code paths so for_each_set_bit() does not read past a single unsigned long when nr_lines > BITS_PER_LONG (Jakub, Kory)
- Cancel the poll work explicitly in pse_controller_unregister(), next to the existing disable_irq() for the IRQ path, instead of relying on devm_add_action_or_reset() LIFO ordering. Makes the helper safe even when a driver registers it in the standard order (helper before devm_pse_controller_register()) (Jakub)
- struct pse_pi_led_triggers no longer wrapped in
#if IS_ENABLED(CONFIG_LEDS_TRIGGERS); only the function bodies remain ifdef'd, so reference sites no longer need their own ifdefs (Jakub)
- Added .activate callbacks for both LED triggers. Without these, an LED bound to a trigger after pse_controller_register() (e.g. via sysfs) would stay dark until the next hardware event toggled state (Jakub)
- Run the post-registration initial-state pass under pcdev->lock, matching pse_led_update()'s documented locking contract and avoiding races with concurrent regulator_enable() that share last_delivering / last_enabled and the hardware ops (Jakub)
- On partial pse_led_triggers_register() failure, NULL out pcdev->pi_led_trigs so the existing early-return guard in pse_led_update() short-circuits any later calls onto partially-registered triggers (Jakub)
Changes since v3:
- Dropped the dt-bindings poll-interval-ms patch: the poll interval is a driver decision, not a hardware property (Krzysztof)
- Removed of_property_read_u32() for poll-interval-ms from devm_pse_poll_helper(); the 500ms default is now hardcoded but drivers can override pcdev->poll_interval_ms before calling the helper
- Rebased on net-next/main
Changes since v2:
- Based on net-next/main, added net-next subject prefix
- Added --base tree information
- Added CC for devicetree list and DT maintainers
- Collected Reviewed-by from Kory Maincent on patch 1/3
- Fixed build error when CONFIG_LEDS_TRIGGERS is disabled:
moved LED registration before list_add(), removing the pcdev->pi_led_trigs = NULL assignment on conditionally compiled struct member (reported by kernel test robot)
- Fixed use-after-free on device unbind: poll work is now cancelled via devm_add_action_or_reset() to ensure correct devres teardown ordering (poll_work cancelled before poll_notifs is freed)
- Used system_freezable_wq for poll worker to prevent hardware access during system suspend
- Added PoDL power status and admin state checks to LED triggers so they work for both C33 and PoDL controller types
- Used dev_name(dev) for LED trigger names to ensure uniqueness across multiple PSE controllers (of_node->name can be generic)
- Added initial LED state query at registration so already-active ports are reflected immediately
- Added pse_led_update() calls in regulator enable/disable paths so ethtool admin state changes are reflected in LEDs
- Moved LED trigger registration before list_add() to prevent race where IRQ/poll could invoke pse_led_update() on partially initialized triggers
Changes since v1:
- Split single patch into 3 separate patches
- Extracted pse_handle_events() and devm_pse_poll_helper() as a standalone poll path (patches 1-2), testable without LED code
- Added DT binding for poll-interval-ms as a separate patch
- Renamed led-poll-interval-ms to poll-interval-ms for generic use
- Fire LED triggers from the notification path rather than a separate poll loop
Tested on Realtek RTL9303 with HS104 PoE chip, poll path only (without IRQ configured). Verified PD connect/disconnect notifications and LED trigger state changes.
Carlo Szelinsky (2):
net: pse-pd: add devm_pse_poll_helper()
net: pse-pd: add LED trigger support via notification path
drivers/net/pse-pd/pse_core.c | 355 +++++++++++++++++++++++++++++++--- include/linux/pse-pd/pse.h | 32 +++ 2 files changed, 356 insertions(+), 31 deletions(-)
base-commit: 09942ddedcb960f9e78fd817ec33f501d1040c5b --
2.43.0