Re: [PATCH] commands: regulator: add support for enabling/disabling regulators

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 05.01.22 10:14, Sascha Hauer wrote:
> On Mon, Jan 03, 2022 at 01:03:36PM +0100, Ahmad Fatoum wrote:
>> For testing regulator drivers, it can be handy to enable/disable them
>> from the shell prompt. Extend the regulator command to support this.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  commands/regulator.c     | 38 +++++++++++++++++++++++++++++++++++---
>>  drivers/regulator/core.c |  8 ++++++++
>>  include/regulator.h      |  1 +
>>  3 files changed, 44 insertions(+), 3 deletions(-)
>>
>> diff --git a/commands/regulator.c b/commands/regulator.c
>> index 3e2595f8bfc1..e6b2f4852db4 100644
>> --- a/commands/regulator.c
>> +++ b/commands/regulator.c
>> @@ -6,16 +6,48 @@
>>  #include <common.h>
>>  #include <command.h>
>>  #include <regulator.h>
>> +#include <getopt.h>
>>  
>>  static int do_regulator(int argc, char *argv[])
>>  {
>> -	regulators_print();
>> +	struct regulator *chosen;
>> +	int opt, ret;
>> +
>> +	while ((opt = getopt(argc, argv, "e:d:")) > 0) {
>> +		switch (opt) {
>> +		case 'e':
>> +		case 'd':
>> +			chosen = regulator_get_name(optarg);
>> +			if (IS_ERR_OR_NULL(chosen)) {
>> +				printf("regulator not found\n");
>> +				return COMMAND_ERROR;
>> +			}
>> +
>> +			ret = opt == 'e' ? regulator_enable(chosen)
>> +				         : regulator_disable(chosen);
>> +			regulator_put(chosen);
>> +			return ret;
> 
> The barebox regulator core distinguishes between struct regulator and
> struct regulator_internal. regulator_internal represents the physical
> regulator whereas regulator is allocated for each consumer. If the
> regulator core were a bit more sophisticated then a regulator would
> have it's own enable count and you would be warned when a regulators
> enable count goes below zero.
> 
> I agree that controlling regulators on the command line would be useful,
> but I also don't want to block extending the regulator core in such a
> way.

Should I move the command implementation then into drivers/regulator/core.c?

That's what I did here[1], but I seem to recall that you objected to
moving the command into drivers/ to access internals, when the API
should suffice/be extended. I can't find the mail right now though or
perhaps my recollection is erroneous.

So how to proceed?

[1]: https://lore.barebox.org/barebox/20191106094459.w32tgsl22ty34vhe@pengutronix.de/#t

Cheers,
Ahmad

> 
> Sascha
> 


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

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox