Re: [PATCH] net: pse-pd: add LED trigger support

[Kory Maincent <kory.maincent@bootlin.com>]

Hello Carlo,

On Sat, 21 Mar 2026 18:55:46 +0100
Carlo Szelinsky <github@szelinsky.de> wrote:

> Hey Kory, Hey Oleksij,
> 
> Thanks again for taking the time to give detailed feedback. I am not really
> experienced with working on the kernel, so I took some time to process and
> get a clear picture. I will try to implement and test it asap... My action
> points would be the following: 1. Replace the LED specific polling with some
> generic devm_pse_poll_helper() that is based on the existing pse_isr() logic
> but in a timer instead of an IRQ - pushing events through ntf_fifo /
> pse_send_ntf_worker() like other IRQ-based controllers already do. 

Please add a patch to introduce the poll path before any LED support.
I will test this new path with my boards with and without the IRQ configured. 

> 2. Fire
> LED triggers from the notification path, not from a separate poll loop: LEDs
> react to state changes e.g. they don't drive their own polling. 3. Fix
> pse_pw_d_is_sw_pw_control() - it currently requires pcdev->irq to be set in
> the PSE_BUDGET_EVAL_STRAT_DISABLED path, so poll-only controllers like hs104
> would never enter software power control. Needs to also check for an active
> poll worker. 4. Add #define for the default poll interval (e.g. 500ms) with a
> comment explainin why.
> 
> Did I understand you correctly to not waste any time?

Yes that's it.

> Unclear is for me still:
> * Poll helper design: new devm_pse_poll_helper() vs extending
> devm_pse_irq_helper() with IRQ=0 fallback? I suggest a separate
> devm_pse_poll_helper() - it keeps the IRQ and poll paths clean and symmetric,
> and avoids overloading devm_pse_irq_helper() with conditional logic.
> * Who decides to poll? The driver explicitly calls the poll helper, or the
> core auto-detects missing IRQ? I suggest the driver decides explicitly - the
> driver knows its hardware best, and an explicit call is easier to review and
> reason about than auto-detection magic.
> * DT property: rename led-poll-interval-ms to poll-interval-ms since polling
> is now generic? I suggest yes - the polling is no longer LED-specific, so the
> property name should reflect that.
> * Kory mentioned two distinct cases: (a) controller has no IRQ support at all
> (like the hs104), (b) controller supports IRQ but it's not wired on the
> board. Should both cases be handled by the same poll helper, or does (b) need
> different treatment? I suggest the same devm_pse_poll_helper() handles both -
> from the core's perspective the situation is identical: no IRQ available,
> need to poll. The driver just calls the poll helper instead of the IRQ helper
> in either case.

I agreed with all your points.
Thank you!

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com