Re: [PATCH net-next 2/2] net: pse-pd: pd692x0: Add sysfs interface for configuration save/reset

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

Hello Andrew,

Le Fri, 22 Aug 2025 19:17:55 +0200,
Andrew Lunn <andrew@lunn.ch> a écrit :

> On Fri, Aug 22, 2025 at 05:37:02PM +0200, Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com>
> > 
> > Add sysfs attributes save_conf and reset_conf to enable userspace
> > management of the PSE's permanent configuration stored in EEPROM.
> > 
> > The save_conf attribute allows saving the current configuration to
> > EEPROM by writing '1'. The reset_conf attribute restores factory
> > defaults and reinitializes the port matrix configuration.  
> 
> I'm not sure sysfs is the correct interface for this.
> 
> Lets take a step back.
> 
> I assume ethtool will report the correct state after a reboot when the
> EEPROM has content? The driver does not hold configuration state which
> cannot be represented in the EEPROM?

In fact I assumed it is an EEPROM but it is described as non volatile memory
so I don't know which type it is.

Yes ethtool report the current configuration which match the saved one if it has
been saved before. No the driver doesn't hold any state that can not be
represented in the non-volatile memory.

> Is the EEPROM mandatory, or optional? Is it built into the controller?

It is built into the controller. It seem there are version of this
controller that does not support it : "This command is not supported by
PD69200M."

> How fast is it to store the settings?

2 i2c messages and a 50 ms wait as described in the datasheet.
 
> I'm wondering if rather than having this sysfs parameter, you just
> store every configuration change? That could be more intuitive.

I have not thought of it. I don't know if it is a good idea. We may need
feedback from people that actually use PSE on field. Kyle any idea on this?
In any case we still need a way to reset the configuration through sysfs or
whatever other way.

> I've not looked at the sysfs documentation. Are there other examples
> of such a property?

Not sure for that particular save/reset configuration case.
Have you another implementation idea in mind?

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