Message-ID: <ZyMpkJRHZWYsszh2@pengutronix.de>
Date: Thu, 31 Oct 2024 07:54:08 +0100
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Kory Maincent <kory.maincent@...tlin.com>
Cc: Andrew Lunn <andrew@...n.ch>, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Jonathan Corbet <corbet@....net>,
	Donald Hunter <donald.hunter@...il.com>,
	Rob Herring <robh@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
	Simon Horman <horms@...nel.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	linux-doc@...r.kernel.org, Kyle Swenson <kyle.swenson@....tech>,
	Dent Project <dentproject@...uxfoundation.org>,
	kernel@...gutronix.de,
	Maxime Chevallier <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH RFC net-next v2 15/18] net: pse-pd: Add support for
 getting and setting port priority

>  struct pse_control_status {
>  	u32 pse_id;
> @@ -74,6 +83,10 @@ struct pse_control_status {
>  	u32 c33_avail_pw_limit;
>  	struct ethtool_c33_pse_pw_limit_range *c33_pw_limit_ranges;
>  	u32 c33_pw_limit_nb_ranges;
> +	u32 c33_prio_supp_modes;
> +	enum pse_port_prio_modes c33_prio_mode;
> +	u32 c33_prio_max;
> +	u32 c33_prio;
>  };
>  
>  /**
> @@ -93,6 +106,8 @@ struct pse_control_status {
>   *			  set_current_limit regulator callback.
>   *			  Should not return an error in case of MAX_PI_CURRENT
>   *			  current value set.
> + * @pi_set_prio: Configure the PSE PI priority.
> + * @pi_get_pw_req: Get the power requested by a PD before enabling the PSE PI
>   */
>  struct pse_controller_ops {
>  	int (*ethtool_get_status)(struct pse_controller_dev *pcdev,
> @@ -107,6 +122,9 @@ struct pse_controller_ops {
>  				    int id);
>  	int (*pi_set_current_limit)(struct pse_controller_dev *pcdev,
>  				    int id, int max_uA);
> +	int (*pi_set_prio)(struct pse_controller_dev *pcdev, int id,
> +			   unsigned int prio);
> +	int (*pi_get_pw_req)(struct pse_controller_dev *pcdev, int id);
>  };
>  
>  struct module;
> @@ -141,6 +159,10 @@ struct pse_pi_pairset {
>   * @rdev: regulator represented by the PSE PI
>   * @admin_state_enabled: PI enabled state
>   * @pw_d: Power domain of the PSE PI
> + * @prio: Priority of the PSE PI. Used in static port priority mode
> + * @pw_enabled: PSE PI power status in static port priority mode
> + * @pw_allocated: Power allocated to a PSE PI to manage power budget in
> + *	static port priority mode
>   */
>  struct pse_pi {
>  	struct pse_pi_pairset pairset[2];
> @@ -148,6 +170,9 @@ struct pse_pi {
>  	struct regulator_dev *rdev;
>  	bool admin_state_enabled;
>  	struct pse_power_domain *pw_d;
> +	int prio;
> +	bool pw_enabled;
> +	int pw_allocated;
>  };
>  
>  /**
> @@ -165,6 +190,9 @@ struct pse_pi {
>   * @pi: table of PSE PIs described in this controller device
>   * @no_of_pse_pi: flag set if the pse_pis devicetree node is not used
>   * @id: Index of the PSE
> + * @pis_prio_max: Maximum value allowed for the PSE PIs priority
> + * @port_prio_supp_modes: Bitfield of port priority mode supported by the PSE
> + * @port_prio_mode: Current port priority mode of the PSE
>   */
>  struct pse_controller_dev {
>  	const struct pse_controller_ops *ops;
> @@ -179,6 +207,9 @@ struct pse_controller_dev {
>  	struct pse_pi *pi;
>  	bool no_of_pse_pi;
>  	int id;
> +	unsigned int pis_prio_max;
> +	u32 port_prio_supp_modes;
> +	enum pse_port_prio_modes port_prio_mode;
>  };

> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index a1ad257b1ec1..22664b1ea4a2 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -1002,11 +1002,35 @@ enum ethtool_c33_pse_pw_d_status {
>   * enum ethtool_c33_pse_events - event list of the C33 PSE controller.
>   * @ETHTOOL_C33_PSE_EVENT_OVER_CURRENT: PSE output current is too high.
>   * @ETHTOOL_C33_PSE_EVENT_OVER_TEMP: PSE in over temperature state.
> + * @ETHTOOL_C33_PSE_EVENT_CONNECTED: PD detected on the PSE.
> + * @ETHTOOL_C33_PSE_EVENT_DISCONNECTED: PD has been disconnected on the PSE.
> + * @ETHTOOL_C33_PSE_EVENT_PORT_PRIO_STATIC_ERROR: PSE faced an error in static
> + *	port priority management mode.
>   */
>  
>  enum ethtool_c33_pse_events {
> -	ETHTOOL_C33_PSE_EVENT_OVER_CURRENT =	1 << 0,
> -	ETHTOOL_C33_PSE_EVENT_OVER_TEMP =	1 << 1,
> +	ETHTOOL_C33_PSE_EVENT_OVER_CURRENT =		1 << 0,
> +	ETHTOOL_C33_PSE_EVENT_OVER_TEMP =		1 << 1,
> +	ETHTOOL_C33_PSE_EVENT_CONNECTED =		1 << 2,
> +	ETHTOOL_C33_PSE_EVENT_DISCONNECTED =		1 << 3,
> +	ETHTOOL_C33_PSE_EVENT_PORT_PRIO_STATIC_ERROR =	1 << 4,
> +};

Same here, priority concept is not part of the spec, so the C33 prefix
should be removed.

> +
> +/**
> + * enum pse_port_prio_modes - PSE port priority modes.
> + * @ETHTOOL_PSE_PORT_PRIO_DISABLED: Port priority disabled.
> + * @ETHTOOL_PSE_PORT_PRIO_STATIC: PSE static port priority. Port priority
> + *	based on the power requested during PD classification. This mode
> + *	is managed by the PSE core.
> + * @ETHTOOL_PSE_PORT_PRIO_DYNAMIC: PSE dynamic port priority. Port priority
> + *	based on the current consumption per ports compared to the total
> + *	power budget. This mode is managed by the PSE controller.
> + */

This part will need some clarification about behavior with mixed port
configurations. Here is my proposal:

 * Expected behaviors in mixed port priority configurations:
 * - When ports are configured with a mix of disabled, static, and dynamic
 *   priority modes, the following behaviors are expected:
 *     - Ports with priority disabled (ETHTOOL_PSE_PORT_PRIO_DISABLED) are
 *       treated with lowest priority, receiving power only if the budget
 *       remains after static and dynamic ports have been served.
 *     - Static-priority ports are allocated power up to their requested
 *       levels during PD classification, provided the budget allows.
 *     - Dynamic-priority ports receive power based on real-time consumption,
 *       as monitored by the PSE controller, relative to the remaining budget
 *       after static ports.
 *
 * Handling scenarios where power budget is exceeded:
 * - Hot-plug behavior: If a new device is added that causes the total power
 *   demand to exceed the PSE budget, the newly added device is de-prioritized
 *   and shut down to maintain stability for previously connected devices.
 *   This behavior ensures that existing connections are not disrupted, though
 *   it may lead to inconsistent behavior if the device is disconnected and
 *   reconnected (hot-plugged).
 *
 * - Startup behavior (boot): When the system initializes with attached devices,
 *   the PSE allocates power based on a predefined order (e.g., by port index)
 *   until the budget is exhausted. Devices connected later in this order may
 *   not be enabled if they would exceed the power budget, resulting in consistent
 *   behavior during startup but potentially differing from runtime behavior
 *   (hot-plug).
 *
 * - Consistency challenge: These two scenarios—hot-plug vs. system boot—may lead
 *   to different handling of devices. During system boot, power is allocated
 *   sequentially, potentially leaving out high-priority devices added later due to
 *   a first-come-first-serve approach. In contrast, hot-plug behavior favors the
 *   status quo, maintaining stability for initially connected devices, which
 *   might not align with the system's prioritization policy.
 *

> +enum pse_port_prio_modes {
> +	ETHTOOL_PSE_PORT_PRIO_DISABLED,
> +	ETHTOOL_PSE_PORT_PRIO_STATIC,
> +	ETHTOOL_PSE_PORT_PRIO_DYNAMIC,
>  };
 

-- 
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 |

Powered by blists - more mailing lists