| Message ID | 1517934852-23255-2-git-send-email-pdeschrijver@nvidia.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote: > * PGP Signed by an unknown key > > On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote: > > From: Laxman Dewangan <ldewangan@nvidia.com> > > > > Add API to get min/max rail voltage configured from platform for > > given rails. > > because... of the next patch where this is used to retrieve the minimum rail voltage. Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 07, 2018 at 10:47:44AM +0200, Peter De Schrijver wrote: > On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote: > > On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote: > > > Add API to get min/max rail voltage configured from platform for > > > given rails. > > because... > of the next patch where this is used to retrieve the minimum rail voltage. And that in turn is needed for...?
On Wed, Feb 07, 2018 at 10:43:51AM +0000, Mark Brown wrote: > On Wed, Feb 07, 2018 at 10:47:44AM +0200, Peter De Schrijver wrote: > > On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote: > > > On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote: > > > > > Add API to get min/max rail voltage configured from platform for > > > > given rails. > > > > because... > > > of the next patch where this is used to retrieve the minimum rail voltage. > > And that in turn is needed for...? To calculate the required voltage for each frequency. Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 07, 2018 at 02:37:51PM +0200, Peter De Schrijver wrote: > On Wed, Feb 07, 2018 at 10:43:51AM +0000, Mark Brown wrote: > > On Wed, Feb 07, 2018 at 10:47:44AM +0200, Peter De Schrijver wrote: > > > On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote: > > > > On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote: > > > > > Add API to get min/max rail voltage configured from platform for > > > > > given rails. > > > > because... > > > of the next patch where this is used to retrieve the minimum rail voltage. > > And that in turn is needed for...? > To calculate the required voltage for each frequency. You're going to have to provide a much better explanation of what this is doing - right now it seems like an abuse of constraints. Client drivers can already determine if a particular voltage they want to set is available via regulator_list_voltage() and so on, that's what constraints are there to set. It sounds like you're trying to use them for something else but you're really not explaining your use case clearly.
On Wed, Feb 07, 2018 at 02:18:46PM +0000, Mark Brown wrote: > On Wed, Feb 07, 2018 at 02:37:51PM +0200, Peter De Schrijver wrote: > > On Wed, Feb 07, 2018 at 10:43:51AM +0000, Mark Brown wrote: > > > On Wed, Feb 07, 2018 at 10:47:44AM +0200, Peter De Schrijver wrote: > > > > On Tue, Feb 06, 2018 at 04:35:44PM +0000, Mark Brown wrote: > > > > > On Tue, Feb 06, 2018 at 06:34:02PM +0200, Peter De Schrijver wrote: > > > > > > > Add API to get min/max rail voltage configured from platform for > > > > > > given rails. > > > > > > because... > > > > > of the next patch where this is used to retrieve the minimum rail voltage. > > > > And that in turn is needed for...? > > > To calculate the required voltage for each frequency. > > You're going to have to provide a much better explanation of what this > is doing - right now it seems like an abuse of constraints. Client > drivers can already determine if a particular voltage they want to set > is available via regulator_list_voltage() and so on, that's what > constraints are there to set. It sounds like you're trying to use them > for something else but you're really not explaining your use case > clearly. There is no way to query what voltage I will actually get for a given input voltage. If you read drivers/clk/tegra/cvb. (you did do that right?), you will see that there is a minimum and maximum voltage defined by charaterization which needs to be capped to the regulator generated voltages for those points. Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 07, 2018 at 04:32:13PM +0200, Peter De Schrijver wrote: > On Wed, Feb 07, 2018 at 02:18:46PM +0000, Mark Brown wrote: > > You're going to have to provide a much better explanation of what this > > is doing - right now it seems like an abuse of constraints. Client > > drivers can already determine if a particular voltage they want to set > > is available via regulator_list_voltage() and so on, that's what > > constraints are there to set. It sounds like you're trying to use them > > for something else but you're really not explaining your use case > > clearly. > There is no way to query what voltage I will actually get for a given input I looked at patch 2. It looked like an abuse of what constraints do, and had zero explanation of why it was doing what it was doing. In any case we need the regulator code and changelog to be clear about what the interface is for and why it should be used, that's not happening here. > voltage. If you read drivers/clk/tegra/cvb. (you did do that right?), you > will see that there is a minimum and maximum voltage defined by > charaterization which needs to be capped to the regulator generated voltages > for those points. I can't really tell what you're saying here. If the driver needs to know if it can set the a given voltage there's already an API for doing that as I said. If you're trying to convey this minimum and maximum voltage via the constraints that sounds like an abuse of the constraints.
On Wed, Feb 07, 2018 at 03:01:55PM +0000, Mark Brown wrote: > On Wed, Feb 07, 2018 at 04:32:13PM +0200, Peter De Schrijver wrote: > > On Wed, Feb 07, 2018 at 02:18:46PM +0000, Mark Brown wrote: > > > > You're going to have to provide a much better explanation of what this > > > is doing - right now it seems like an abuse of constraints. Client > > > drivers can already determine if a particular voltage they want to set > > > is available via regulator_list_voltage() and so on, that's what > > > constraints are there to set. It sounds like you're trying to use them > > > for something else but you're really not explaining your use case > > > clearly. > > > There is no way to query what voltage I will actually get for a given input > > I looked at patch 2. It looked like an abuse of what constraints do, > and had zero explanation of why it was doing what it was doing. In any > case we need the regulator code and changelog to be clear about what the > interface is for and why it should be used, that's not happening here. > > > voltage. If you read drivers/clk/tegra/cvb. (you did do that right?), you > > will see that there is a minimum and maximum voltage defined by > > charaterization which needs to be capped to the regulator generated voltages > > for those points. > > I can't really tell what you're saying here. If the driver needs to > know if it can set the a given voltage there's already an API for doing > that as I said. If you're trying to convey this minimum and maximum > voltage via the constraints that sounds like an abuse of the constraints. No, what I want is the voltage which the regulator will output for a given regulator_set_voltage request taking constraints, regulator step etc into account. Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 07, 2018 at 05:20:45PM +0200, Peter De Schrijver wrote: > On Wed, Feb 07, 2018 at 03:01:55PM +0000, Mark Brown wrote: > > I can't really tell what you're saying here. If the driver needs to > > know if it can set the a given voltage there's already an API for doing > > that as I said. If you're trying to convey this minimum and maximum > > voltage via the constraints that sounds like an abuse of the constraints. > No, what I want is the voltage which the regulator will output for a given > regulator_set_voltage request taking constraints, regulator step etc into > account. Knowing the range of the constaints is going to tell you nothing useful about that, it has zero information on steps or anything. The way to find out what voltages can be set is to enumerate the voltages that can be set through the existing API and then if you want to set a specific voltage that you've confirmed is available you can set exactly that voltage via the normal voltage setting interface, no need to provide a range.
On Wednesday 07 February 2018 09:07 PM, Mark Brown wrote: > On Wed, Feb 07, 2018 at 05:20:45PM +0200, Peter De Schrijver wrote: >> On Wed, Feb 07, 2018 at 03:01:55PM +0000, Mark Brown wrote: >>> I can't really tell what you're saying here. If the driver needs to >>> know if it can set the a given voltage there's already an API for doing >>> that as I said. If you're trying to convey this minimum and maximum >>> voltage via the constraints that sounds like an abuse of the constraints. >> No, what I want is the voltage which the regulator will output for a given >> regulator_set_voltage request taking constraints, regulator step etc into >> account. > Knowing the range of the constaints is going to tell you nothing useful > about that, it has zero information on steps or anything. The way to > find out what voltages can be set is to enumerate the voltages that can > be set through the existing API and then if you want to set a specific > voltage that you've confirmed is available you can set exactly that > voltage via the normal voltage setting interface, no need to provide a > range. Hi, I added this API in downstream for the purpose of finding whether rail output can be changed or not(fixed) based on constraints. If min and max is same then it can not change. I used this information to set the pad voltage of SOC automatically. I dont know other usage when I added this API. May be Peter is needed here. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 08, 2018 at 03:34:56PM +0530, Laxman Dewangan wrote: > I added this API in downstream for the purpose of finding whether rail > output can be changed or not(fixed) based on constraints. > If min and max is same then it can not change. I used this information to > set the pad voltage of SOC automatically. That's a reasonable thing to want to know. The way the API wants you to ask that question at the minute is to check if you can set the voltages you might want to set and then if there's only one possible voltage you can conclude that change is not possible. I appreciate that that's a bit of a long way round to do it, the thinking was to try to keep the client drivers more robust in cases you can change the voltage but not to all the values the driver wants. I'd consider relaxing that and having a direct API to ask about variability (just as a boolean) since it does occur to me that this can be a useful speedup during init but I need to think. > I dont know other usage when I added this API. May be Peter is needed here.
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index be767dd..c498774 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -3246,6 +3246,37 @@ int regulator_get_voltage(struct regulator *regulator) EXPORT_SYMBOL_GPL(regulator_get_voltage); /** + * regulator_get_constraint_voltages - get platform specific constraint voltage, + * @regulator: regulator source + * @min_uV: Minimum microvolts. + * @max_uV: Maximum microvolts. + * + * This returns the current regulator voltage in uV. + * + * NOTE: If the regulator is disabled it will return the voltage value. This + * function should not be used to determine regulator state. + */ + +int regulator_get_constraint_voltages(struct regulator *regulator, + int *min_uV, int *max_uV) +{ + struct regulator_dev *rdev = regulator->rdev; + + if (rdev->desc && rdev->desc->fixed_uV && rdev->desc->n_voltages == 1) { + *min_uV = rdev->desc->fixed_uV; + *max_uV = rdev->desc->fixed_uV; + return 0; + } + if (rdev->constraints) { + *min_uV = rdev->constraints->min_uV; + *max_uV = rdev->constraints->max_uV; + return 0; + } + return -EINVAL; +} +EXPORT_SYMBOL_GPL(regulator_get_constraint_voltages); + +/** * regulator_set_current_limit - set regulator output current limit * @regulator: regulator source * @min_uA: Minimum supported current in uA diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h index df176d7..d3f495a 100644 --- a/include/linux/regulator/consumer.h +++ b/include/linux/regulator/consumer.h @@ -250,6 +250,8 @@ int regulator_is_supported_voltage(struct regulator *regulator, int regulator_set_voltage_time(struct regulator *regulator, int old_uV, int new_uV); int regulator_get_voltage(struct regulator *regulator); +int regulator_get_constraint_voltages(struct regulator *regulator, + int *min_uV, int *max_uV); int regulator_sync_voltage(struct regulator *regulator); int regulator_set_current_limit(struct regulator *regulator, int min_uA, int max_uA);