| Message ID | 20241121-feature_poe_port_prio-v3-3-83299fa6967c@bootlin.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add support for PSE port priority | expand |
Hi Kory, On Thu, Nov 21, 2024 at 03:42:29PM +0100, Kory Maincent wrote: > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com> > > Setting the max_uA constraint in the regulator API imposes a current > limit during the regulator registration process. This behavior conflicts > with preserving the maximum PI power budget configuration across reboots. > > Instead, compare the desired current limit to MAX_PI_CURRENT in the > pse_pi_set_current_limit() function to ensure proper handling of the > power budget. Not enough coffee :) I still didn't correctly understood the problem. MAX_PI_CURRENT is the hard limit according to the standard, so it is the intial limit anyway. Why it is bad to set it on registration? It feels still better compared to have no limit on init. Or do i'm missing things? Regards, Oleksij
Hello Oleksij, On Sat, 23 Nov 2024 07:29:29 +0100 Oleksij Rempel <o.rempel@pengutronix.de> wrote: > Hi Kory, > > On Thu, Nov 21, 2024 at 03:42:29PM +0100, Kory Maincent wrote: > > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com> > > > > Setting the max_uA constraint in the regulator API imposes a current > > limit during the regulator registration process. This behavior conflicts > > with preserving the maximum PI power budget configuration across reboots. > > > > Instead, compare the desired current limit to MAX_PI_CURRENT in the > > pse_pi_set_current_limit() function to ensure proper handling of the > > power budget. > > Not enough coffee :) I still didn't correctly understood the problem. Don't worry, if you don't understand as a PSE Maintainer no one will. The cause is the commit message. > MAX_PI_CURRENT is the hard limit according to the standard, so it is the > intial limit anyway. Why it is bad to set it on registration? It feels > still better compared to have no limit on init. Or do i'm missing > things? Using constraints.max_uA to set the max current limit will cause regulator core to call the set_current_limit callback when registering a regulator. This call will be done using MAX_PI_CURRENT (1,92A) limit. Is see two issues to this: - There is a chance the power budget of the supply will be not sufficient for all the PIs configured to MAX_PI_CURRENT. On that case the power budget of the supply is exceeded and the next PI of the PSE power domain can't probe with success. It will have -ERANGE error returned from devm_regulator_register() call. So the PSE driver can't probe with success. - The PI current limit is reset at each boot. The next patch series will tackle permanent configuration feature of the PD692x0. This will conflict as we want to keep the PI current limit saved. Another solution would be to add a no_apply_max_uA_constraint flag. Regards,
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c index 2906ce173f66..9fee4dd53515 100644 --- a/drivers/net/pse-pd/pse_core.c +++ b/drivers/net/pse-pd/pse_core.c @@ -357,6 +357,9 @@ static int pse_pi_set_current_limit(struct regulator_dev *rdev, int min_uA, if (!ops->pi_set_current_limit) return -EOPNOTSUPP; + if (max_uA > MAX_PI_CURRENT) + return -ERANGE; + id = rdev_get_id(rdev); mutex_lock(&pcdev->lock); ret = ops->pi_set_current_limit(pcdev, id, max_uA); @@ -403,11 +406,9 @@ devm_pse_pi_regulator_register(struct pse_controller_dev *pcdev, rinit_data->constraints.valid_ops_mask = REGULATOR_CHANGE_STATUS; - if (pcdev->ops->pi_set_current_limit) { + if (pcdev->ops->pi_set_current_limit) rinit_data->constraints.valid_ops_mask |= REGULATOR_CHANGE_CURRENT; - rinit_data->constraints.max_uA = MAX_PI_CURRENT; - } rinit_data->supply_regulator = "vpwr";