Netdev List
 help / color / mirror / Atom feed
From: Kory Maincent <kory.maincent@bootlin.com>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Carlo Szelinsky <github@szelinsky.de>,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: pse-pd: add LED trigger support
Date: Mon, 16 Mar 2026 15:44:33 +0100	[thread overview]
Message-ID: <20260316154433.032346e5@kmaincent-XPS-13-7390> (raw)
In-Reply-To: <abblHRq6Q85j5lXA@pengutronix.de>

Hello Carlo,

On Sun, 15 Mar 2026 17:58:05 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Hi Carlo,

Nice to see you trying upstreaming the PSE LED work.
 
> On Sun, Mar 15, 2026 at 12:59:16AM +0100, Carlo Szelinsky wrote:
> > Add LED trigger registration and polling into the PSE core subsystem.
> > Per-PI "delivering" and "enabled" triggers are registered for each PSE
> > controller, with a configurable poll interval via the DT property
> > "led-poll-interval-ms".  
> 
> Nice work. However, this needs an architectural shift.
> 
> Since the hardware lacks interrupts, we need a core polling mechanism.
> However, the PSE core already has an event notification framework. The
> new polling should integrate with it instead of being LED-specific.
> 
> Please consider this approach:
> 
> - Add a generic polling loop in the PSE core. It should simulate the IRQ
>   handler pse_isr() by detecting state changes and pushing standard
>   events into the existing ntf_fifo to be processed by
>   pse_send_ntf_worker()

Careful with pse_isr() there is a tricky case between software and hardware
managed power control. And the irq support was mainly designed to support the
software managed power control case.

Adding a generic polling loop should be indeed similar to what happen in the
interrupt process. This mean we need a polling_handler to report the events
from the driver similarly to the irq one:
https://elixir.bootlin.com/linux/v6.19.6/source/drivers/net/pse-pd/tps23881.c#L1348
And either modify the devm_pse_irq_helper() supporting the case when the irq is
null or adding a new devm_pse_poll_helper() helper. So either the core decide
to use polling instead of irq either this choice comes from the driver.

We have two cases, interrupt not supported by the controller or interrupt
supported but not wired and the polling case should comply with both.

Note: You may also need to modify pse_pw_d_is_sw_pw_control() accordingly.

> - Do not poll inside the LED code. The core state tracker should trigger
>   LED events as a reaction to state changes.

+1

> - Please add a define for the default polling interval. Include a
>   comment explaining why this specific value is chosen.

+1

> @Köry, How should we decide when to enable polling?  Should we check if
> no IRQ is registered? Or add a flag if the controller lacks IRQ support?

Replied above.

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

  parent reply	other threads:[~2026-03-16 14:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-14 23:59 [PATCH] net: pse-pd: add LED trigger support Carlo Szelinsky
2026-03-15 16:58 ` Oleksij Rempel
2026-03-15 21:12   ` Carlo Szelinsky
2026-03-16  6:12     ` Oleksij Rempel
2026-03-16 14:44   ` Kory Maincent [this message]
2026-03-18 20:55 ` Andrew Lunn
2026-03-21 17:55   ` Carlo Szelinsky
2026-03-23 10:30     ` Kory Maincent
2026-03-23 20:27   ` Carlo Szelinsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260316154433.032346e5@kmaincent-XPS-13-7390 \
    --to=kory.maincent@bootlin.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=github@szelinsky.de \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox