mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [RFC] regulator: add runtime disable/enable support
@ 2015-11-22 19:28 Alexander Aring
  2015-11-23  7:35 ` Sascha Hauer
  0 siblings, 1 reply; 2+ messages in thread
From: Alexander Aring @ 2015-11-22 19:28 UTC (permalink / raw)
  To: barebox

This patch adds support to call disable/enable for a specific regulator
by name. This can be useful to test various linux boot tests in
different regulator states or adding workarounds inside the bootloader
when linux doesn't support to enable the regulator for probing.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 commands/regulator.c     | 73 ++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/regulator/core.c | 30 ++++++++++++++++++++
 include/regulator.h      | 12 ++++++++
 3 files changed, 112 insertions(+), 3 deletions(-)

diff --git a/commands/regulator.c b/commands/regulator.c
index e0b704f..1fb1559 100644
--- a/commands/regulator.c
+++ b/commands/regulator.c
@@ -18,17 +18,84 @@
  */
 #include <common.h>
 #include <command.h>
+#include <getopt.h>
 #include <regulator.h>
 
+static inline void regulator_setting_parse_error(int ret, const char *cmd)
+{
+	perror(cmd);
+	if (ret == -ENOENT)
+		printf("check \"%s -l\" for possible regulator names.\n", cmd);
+}
+
 static int do_regulator(int argc, char *argv[])
 {
-	regulators_print();
+	char *name = NULL, *setting = NULL;
+	int opt, ret, print = 0;
+
+	if (argc == 1)
+		return COMMAND_ERROR_USAGE;
+
+	while ((opt = getopt(argc, argv, "ln:s:")) > 0) {
+		switch (opt) {
+		case 'l':
+			print = 1;
+			break;
+		case 'n':
+			name = optarg;
+			break;
+		case 's':
+			setting = optarg;
+			break;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
 
-	return 0;
+	/* do nothing, if name or nothing was given only */
+	if ((name && !setting) || (!name && !setting))
+		goto out;
+
+	if (!name && setting) {
+		printf("no regulator name for setting given.\n");
+		return COMMAND_ERROR_USAGE;
+	}
+
+	if (!strcmp(setting, "enable")) {
+		ret = regulator_enable_by_name(name);
+		if (ret < 0) {
+			perror(argv[0]);
+			regulator_setting_parse_error(ret, argv[0]);
+			return COMMAND_ERROR;
+		}
+	} else if (!strcmp(setting, "disable")) {
+		ret = regulator_disable_by_name(name);
+		if (ret < 0) {
+			regulator_setting_parse_error(ret, argv[0]);
+			return COMMAND_ERROR;
+		}
+	} else {
+		printf("failed to parse setting argument.\n");
+		return COMMAND_ERROR_USAGE;
+	}
+
+out:
+	if (print)
+		regulators_print();
+
+	return COMMAND_SUCCESS;
 }
 
+BAREBOX_CMD_HELP_START(regulator)
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT("-l", "list regulators, after settings if given.")
+BAREBOX_CMD_HELP_OPT("-n REGULATOR_NAME", "REGULATOR_NAME which can be lookup by -l option.")
+BAREBOX_CMD_HELP_OPT("-s enable/disable", "force enable or disable regulator, requires -n argument.")
+BAREBOX_CMD_HELP_END
+
 BAREBOX_CMD_START(regulator)
 	.cmd		= do_regulator,
-	BAREBOX_CMD_DESC("list regulators")
+	BAREBOX_CMD_DESC("regulator settings")
+	BAREBOX_CMD_OPTS("[-lns]")
 	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
 BAREBOX_CMD_END
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 73f5c6e..de764f3 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -345,3 +345,33 @@ void regulators_print(void)
 	list_for_each_entry(ri, &regulator_list, list)
 		regulator_print_one(ri);
 }
+
+/*
+ * regulator_enable_by_name - enable regulator by internal regulator name
+ */
+int regulator_enable_by_name(const char *name)
+{
+	struct regulator_internal *ri;
+
+	list_for_each_entry(ri, &regulator_list, list) {
+		if (!strcmp(ri->name, name))
+			return regulator_enable_internal(ri);
+	}
+
+	return -ENOENT;
+}
+
+/*
+ * regulator_disable_by_name - disable regulator by internal regulator name
+ */
+int regulator_disable_by_name(const char *name)
+{
+	struct regulator_internal *ri;
+
+	list_for_each_entry(ri, &regulator_list, list) {
+		if (!strcmp(ri->name, name))
+			return regulator_disable_internal(ri);
+	}
+
+	return -ENOENT;
+}
diff --git a/include/regulator.h b/include/regulator.h
index 367e13f..cb899f8 100644
--- a/include/regulator.h
+++ b/include/regulator.h
@@ -35,6 +35,8 @@ void regulators_print(void);
 struct regulator *regulator_get(struct device_d *, const char *);
 int regulator_enable(struct regulator *);
 int regulator_disable(struct regulator *);
+int regulator_enable_by_name(const char *name);
+int regulator_disable_by_name(const char *name);
 
 #else
 
@@ -53,6 +55,16 @@ static inline int regulator_disable(struct regulator *r)
 	return 0;
 }
 
+static inline int regulator_enable_by_name(const char *name)
+{
+	return 0;
+}
+
+static inline int regulator_disable_by_name(const char *name)
+{
+	return 0;
+}
+
 #endif
 
 #endif /* __REGULATOR_H */
-- 
2.6.1


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

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

* Re: [RFC] regulator: add runtime disable/enable support
  2015-11-22 19:28 [RFC] regulator: add runtime disable/enable support Alexander Aring
@ 2015-11-23  7:35 ` Sascha Hauer
  0 siblings, 0 replies; 2+ messages in thread
From: Sascha Hauer @ 2015-11-23  7:35 UTC (permalink / raw)
  To: Alexander Aring; +Cc: barebox

On Sun, Nov 22, 2015 at 08:28:00PM +0100, Alexander Aring wrote:
> This patch adds support to call disable/enable for a specific regulator
> by name. This can be useful to test various linux boot tests in
> different regulator states or adding workarounds inside the bootloader
> when linux doesn't support to enable the regulator for probing.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  commands/regulator.c     | 73 ++++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/regulator/core.c | 30 ++++++++++++++++++++
>  include/regulator.h      | 12 ++++++++
>  3 files changed, 112 insertions(+), 3 deletions(-)
> 
> diff --git a/commands/regulator.c b/commands/regulator.c
> index e0b704f..1fb1559 100644
> --- a/commands/regulator.c
> +++ b/commands/regulator.c
> @@ -18,17 +18,84 @@
>   */
>  #include <common.h>
>  #include <command.h>
> +#include <getopt.h>
>  #include <regulator.h>
>  
> +static inline void regulator_setting_parse_error(int ret, const char *cmd)
> +{
> +	perror(cmd);
> +	if (ret == -ENOENT)
> +		printf("check \"%s -l\" for possible regulator names.\n", cmd);
> +}
> +
>  static int do_regulator(int argc, char *argv[])
>  {
> -	regulators_print();
> +	char *name = NULL, *setting = NULL;
> +	int opt, ret, print = 0;
> +
> +	if (argc == 1)
> +		return COMMAND_ERROR_USAGE;
> +
> +	while ((opt = getopt(argc, argv, "ln:s:")) > 0) {
> +		switch (opt) {
> +		case 'l':
> +			print = 1;
> +			break;
> +		case 'n':
> +			name = optarg;
> +			break;
> +		case 's':
> +			setting = optarg;
> +			break;
> +		default:
> +			return COMMAND_ERROR_USAGE;
> +		}
> +	}

The usage is not very intuitive. How about creating separate
regulator_enable / regulator_disable commands? These would be easy to
use then and wouldn't require clever option parsing.

> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 73f5c6e..de764f3 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -345,3 +345,33 @@ void regulators_print(void)
>  	list_for_each_entry(ri, &regulator_list, list)
>  		regulator_print_one(ri);
>  }
> +
> +/*
> + * regulator_enable_by_name - enable regulator by internal regulator name
> + */
> +int regulator_enable_by_name(const char *name)
> +{
> +	struct regulator_internal *ri;
> +
> +	list_for_each_entry(ri, &regulator_list, list) {
> +		if (!strcmp(ri->name, name))
> +			return regulator_enable_internal(ri);
> +	}
> +
> +	return -ENOENT;
> +}

regulator_by_name() would be a useful addition, but I think introducing
a regulator_*_by name set of functions is a bit too much, given that we
might also need set/get voltage and maybe other functions.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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] 2+ messages in thread

end of thread, other threads:[~2015-11-23  7:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-22 19:28 [RFC] regulator: add runtime disable/enable support Alexander Aring
2015-11-23  7:35 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox