mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] commands: regulator: add support for enabling/disabling regulators
@ 2022-01-03 12:03 Ahmad Fatoum
  2022-01-05  9:14 ` Sascha Hauer
  0 siblings, 1 reply; 3+ messages in thread
From: Ahmad Fatoum @ 2022-01-03 12:03 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

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;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
 
+	regulators_print();
 	return 0;
 }
 
+BAREBOX_CMD_HELP_START(regulator)
+	BAREBOX_CMD_HELP_TEXT("List and control regulators.")
+	BAREBOX_CMD_HELP_TEXT("Without options, displays regulator info")
+	BAREBOX_CMD_HELP_TEXT("Options:")
+	BAREBOX_CMD_HELP_OPT("-e REGULATOR\t", "enable REGULATOR")
+	BAREBOX_CMD_HELP_OPT("-d REGULATOR\t", "disable REGULATOR")
+BAREBOX_CMD_HELP_END
+
 BAREBOX_CMD_START(regulator)
 	.cmd		= do_regulator,
-	BAREBOX_CMD_DESC("list regulators")
-	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
+	BAREBOX_CMD_DESC("list and control regulators")
+	BAREBOX_CMD_OPTS("[-ed] [REGULATOR]")
+	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
+	BAREBOX_CMD_HELP(cmd_regulator_help)
 BAREBOX_CMD_END
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b2e5f8caa2ca..35567100c5af 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -355,6 +355,14 @@ struct regulator *regulator_get(struct device_d *dev, const char *supply)
 	return r;
 }
 
+void regulator_put(struct regulator *r)
+{
+	if (IS_ERR_OR_NULL(r))
+		return;
+	list_del(&r->list);
+	free(r);
+}
+
 static struct regulator_internal *regulator_by_name(const char *name)
 {
 	struct regulator_internal *ri;
diff --git a/include/regulator.h b/include/regulator.h
index dfdfbf033262..f4cf6726c2ad 100644
--- a/include/regulator.h
+++ b/include/regulator.h
@@ -148,6 +148,7 @@ void regulators_print(void);
 #ifdef CONFIG_REGULATOR
 
 struct regulator *regulator_get(struct device_d *, const char *);
+void regulator_put(struct regulator *r);
 struct regulator *regulator_get_name(const char *name);
 int regulator_enable(struct regulator *);
 int regulator_disable(struct regulator *);
-- 
2.30.2


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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] commands: regulator: add support for enabling/disabling regulators
  2022-01-03 12:03 [PATCH] commands: regulator: add support for enabling/disabling regulators Ahmad Fatoum
@ 2022-01-05  9:14 ` Sascha Hauer
  2022-01-05  9:21   ` Ahmad Fatoum
  0 siblings, 1 reply; 3+ messages in thread
From: Sascha Hauer @ 2022-01-05  9:14 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

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.

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] commands: regulator: add support for enabling/disabling regulators
  2022-01-05  9:14 ` Sascha Hauer
@ 2022-01-05  9:21   ` Ahmad Fatoum
  0 siblings, 0 replies; 3+ messages in thread
From: Ahmad Fatoum @ 2022-01-05  9:21 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-01-05  9:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-03 12:03 [PATCH] commands: regulator: add support for enabling/disabling regulators Ahmad Fatoum
2022-01-05  9:14 ` Sascha Hauer
2022-01-05  9:21   ` Ahmad Fatoum

mail archive of the barebox mailing list

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lore.barebox.org/barebox/0 barebox/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 barebox barebox/ https://lore.barebox.org/barebox \
		barebox@lists.infradead.org barebox@lists.infradead.org
	public-inbox-index barebox

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git