Re: [PATCH net-next v2 4/4] net: pse-pd: pd692x0: Add devlink interface for configuration save/reset

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Wed, Sep 03, 2025 at 11:10:25AM +0200, Kory Maincent wrote:
> On Wed, 3 Sep 2025 09:10:28 +0200
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > On Tue, Sep 02, 2025 at 01:48:44PM -0700, Jakub Kicinski wrote:
> > > On Tue, 2 Sep 2025 13:42:12 -0700 Jakub Kicinski wrote:  
> > > > On Tue, 2 Sep 2025 16:43:14 +0200 Kory Maincent wrote:  
> > > > > > Sorry for not offering a clear alternative, but I'm not aware of any
> > > > > > precedent for treating devlink params as action triggers. devlink
> > > > > > params should be values that can be set and read, which is clearly not
> > > > > > the case here:      
> > > > > 
> > > > > Ok.
> > > > > We could save the configuration for every config change and add a
> > > > > reset-conf action to devlink reload uAPI? The drawback it that it will
> > > > > bring a bit of latency (about 110ms) for every config change.
> > > > > 
> > > > > Or adding a new devlink uAPI like a devlink conf but maybe we don't
> > > > > have enough cases to add such generic new uAPI.
> > > > > Or get back to the first proposition to use sysfs. 
> > > > > 
> > > > > What do you think?    
> > > > 
> > > > If you are asking for my real preference, abstracting away whether it's
> > > > doable and justifiable amount of effort for you -- I'd explore using
> > > > flags in the ethtool header to control whether setting is written to
> > > > the flash.  
> > > 
> > > PS. failing that the less uAPI the better. Tho, given that the whole
> > > point here is giving user the ability to write the flash -- asking for
> > > uAPI-light approach feels contradictory.
> > > 
> > > Taking a step back -- the "save to flash" is something that OEM FW
> > > often supports. But for Linux-based control the "save to flash" should
> > > really be equivalent to updating some user space config. When user
> > > configures interfaces in OpenWRT we're not flashing them into the
> > > device tree... Could you perhaps explain what makes updating the
> > > in-flash config a high-priority requirement for PoE?
> > >   
> > 
> > I think the main use case question is: what happens if the application
> > CPU reboots?
> > Do we go back to “safe defaults”? But what are safe defaults - that can
> > vary a lot between systems.
> 
> In case of CPU reboot, the port matrix will be flashed, which means the
> controller is restarted and the ports get disconnected.
> Therefore indeed we will go back to default settings.
>  
> > In many setups, if the CPU reboots it also means the bridge is down, so
> > there is no packet forwarding. In that case, does it even make sense to
> > keep providing PoE power if the networking part is non-functional?
> 
> It depends, we might not want to reboot the Powered Devices if the switch
> reboot. I don't currently have specific case in mind which could need this
> behavior.
> Mainly, the Dent Project final aim was to support mainline all the features
> supported in their poed tool.
> https://github.com/dentproject/poed/blob/main/dentos-poe-agent/opt/poeagent/docs/Userguide
> 
> > Another angle: does it make sense to overwrite the hardware power-on
> > defaults each time the system starts? Or should we rather be able to
> > read back the stored defaults from the hardware into the driver and work
> > with them?
> 
> Yes that is one of the design proposition, but we will still need a way to
> reset the conf as said before.
> 
> > Does anyone here have field experience with similar devices? How are
> > these topics usually handled outside of my bubble?
> 
> Kyle any field experience on this?

I can confirm a field case from industrial/medical gear. Closed system,
several modules on SPE, PoDL for power. Requirement: power the PDs as
early as possible, even before Linux. The box boots faster if power-up
and Linux init run in parallel. In this setup the power-on state is
pre-designed by the product team and should not be changed by Linux at
runtime.

So the question is how to communicate and control this:

Option A - Vendor tool writes a fixed config to NVM (EEPROM)
Pro: matches "pre-designed, don't touch" model; PDs come up early
without Linux.
Con: needs extra vendor tooling; hard to keep in sync with what
userspace shows; Linux may not know what is in NVM unless we
read/reflect it.

Option B - Do all changes in RAM, then one explicit "commit to NVM"
Pro: one write; predictable latency hit only on commit; maps to a
"transaction/commit" model.
Con: what if the controller discarded some changes during the session?
We would need a clear commit status and a way to report which settings
actually stuck.

Option C - Write-through: every change also goes to NVM
Pro: if the system resets, config is always up to date.
Con: adds about 50-110 ms per change on this hardware; may be too slow
for interactive tools or batch reconfig.

From API side, if write-through is possible on this hardware, we can
likely make this per-port and per-setting:

ethtool per-port setters can take a "persist=1" flag. Driver applies the
change and also writes it to NVM for that port.

If a particular setting (bit/field) cannot be persisted by the
controller/NVM, the driver returns an error for the whole request. Userspace
then knows persistence is not supported for that item.

Factory reset:
If hardware supports per-port defaults in NVM, provide a per-port
factory_reset op.

Do PD692x0 supports per-port save/restore functionality?

Regards,
Oleksij

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |