[PATCH 1/1] commands: add pwm manipulation command

[PATCH 1/1] commands: add pwm manipulation command

[Marc Reilly]

This introduces a command to set parameters for a pwm device.

Signed-off-by: Marc Reilly <marc@cpdesign.com.au>
---
 commands/Kconfig  |  11 ++++
 commands/Makefile |   1 +
 commands/pwm.c    | 149 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 commands/pwm.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 4d3ff631a8..f90111bd42 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -1929,6 +1929,17 @@ config CMD_I2C
 		  -w		use word (16 bit) wide access
 		  -v		verbose
 
+config CMD_PWM
+	bool
+	depends on PWM
+	prompt "PWM commands: pwm_set"
+	help
+	  pwm_set - set pwm state
+
+	  Usage: pwm_set [ndDPfwis]
+
+ 	  Controls the pwm values such as period and duty cycle
+
 config CMD_LED
 	bool
 	depends on LED
diff --git a/commands/Makefile b/commands/Makefile
index 98625a0373..011ae59427 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_CMD_GPIO)		+= gpio.o
 obj-$(CONFIG_CMD_UNCOMPRESS)	+= uncompress.o
 obj-$(CONFIG_CMD_I2C)		+= i2c.o
 obj-$(CONFIG_CMD_SPI)		+= spi.o
+obj-$(CONFIG_CMD_PWM)		+= pwm.o
 obj-$(CONFIG_CMD_MIPI_DBI)	+= mipi_dbi.o
 obj-$(CONFIG_CMD_UBI)		+= ubi.o
 obj-$(CONFIG_CMD_UBIFORMAT)	+= ubiformat.o
diff --git a/commands/pwm.c b/commands/pwm.c
new file mode 100644
index 0000000000..f638ea60a4
--- /dev/null
+++ b/commands/pwm.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// SPDX-FileCopyrightText: © 2023 Marc Reilly <marc@cpdesign.com.au>
+
+/* pwm - pwm commands */
+
+#include <common.h>
+#include <command.h>
+#include <errno.h>
+#include <malloc.h>
+#include <getopt.h>
+#include <pwm.h>
+
+#define HZ_TO_NANOSECONDS(x) (1000000000UL/(x))
+
+static int do_pwm_set(int argc, char *argv[])
+{
+	struct pwm_device *pwm = NULL;
+	struct pwm_state state;
+	int error = 0;
+	char namebuf[128];
+	char *devname = NULL;
+	int n = -1, polarity = -1, duty = -1, period = -1, freq = -1, width = -1, opt;
+	int stop = -1;
+
+	while ((opt = getopt(argc, argv, "n:d:D:P:f:w:i:s")) > 0) {
+		switch (opt) {
+		case 'n':
+			n = simple_strtol(optarg, NULL, 0);
+			break;
+		case 'd':
+			devname = optarg;
+			break;
+		case 'D':
+			duty = simple_strtol(optarg, NULL, 0);
+			break;
+		case 'P':
+			period = simple_strtol(optarg, NULL, 0);
+			break;
+		case 'f':
+			freq = simple_strtol(optarg, NULL, 0);
+			break;
+		case 'w':
+			width = simple_strtol(optarg, NULL, 0);
+			break;
+		case 'i':
+			polarity = simple_strtol(optarg, NULL, 0);
+			break;
+		case 's':
+			stop = 1;
+			break;
+		}
+	}
+
+	if ((n < 0) && !devname) {
+		printf(" need to specify a device\n");
+		return COMMAND_ERROR_USAGE;
+	}
+	if ((freq == 0) || (period == 0)) {
+		printf(" period or freqency needs to be non-zero\n");
+		return COMMAND_ERROR_USAGE;
+	}
+
+	if (!devname) {
+		sprintf(namebuf, "pwm%d", n);
+	} else {
+		strcpy(namebuf, devname);
+	}
+
+	pwm = pwm_request(namebuf);
+	if (!pwm) {
+		printf("pwm device %s not found\n", namebuf);
+		return -ENODEV;
+	}
+
+	pwm_get_state(pwm, &state);
+
+	if ((state.period_ns == 0)
+			&& (freq < 0) && (duty < 0) && (period < 0)) {
+		printf(" need to know some timing info; freq or dury/period\n");
+		return COMMAND_ERROR_USAGE;
+	}
+
+	if (polarity >= 0)
+		state.polarity = polarity;
+
+	/* period */
+	if (freq > 0) {
+		state.p_enable = true;
+		state.period_ns = HZ_TO_NANOSECONDS(freq);
+		if (width < 0) {
+			width = 50;
+		}
+	} else if (period > 0) {
+		state.p_enable = true;
+		state.period_ns = period;
+	}
+
+	/* duty */
+	if (width >= 0) {
+		if (width > 100)
+			width = 100;
+
+		pwm_set_relative_duty_cycle(&state, width, 100);
+	} else if (duty >= 0) {
+		if (duty > state.period_ns)
+			printf(" warning: duty_ns is greater than period\n");
+
+		state.duty_ns = duty;
+	}
+
+	error = pwm_apply_state(pwm, &state);
+
+	if (stop > 0) {
+		state.p_enable = false;
+		error = pwm_apply_state(pwm, &state);
+	}
+
+	pwm_free(pwm);
+
+	return error;
+}
+
+BAREBOX_CMD_HELP_START(pwm_set)
+BAREBOX_CMD_HELP_TEXT("Sets a pwm device parameters.")
+BAREBOX_CMD_HELP_TEXT(" Specify the device either by device name or pwm number")
+BAREBOX_CMD_HELP_TEXT(" Timings can either be specified via period + duty (on) duration,")
+BAREBOX_CMD_HELP_TEXT("  or via frequency. Duty can be given either as a percentage or time")
+BAREBOX_CMD_HELP_TEXT(" If a parameter is not specified, the current value will be used")
+BAREBOX_CMD_HELP_TEXT("  where possible.")
+BAREBOX_CMD_HELP_TEXT(" Stopping the device does not necessarily set the output to inactive.")
+BAREBOX_CMD_HELP_TEXT(" To set an output to inactive use, set the duty to 0.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT("-d number", "device name (eg 'pwm0')")
+BAREBOX_CMD_HELP_OPT("-n number", "pwm chip number (default 0, assumes device has 'pwmX' name)")
+BAREBOX_CMD_HELP_OPT("-D number", "duty cycle (ns)")
+BAREBOX_CMD_HELP_OPT("-P number", "period (ns)")
+BAREBOX_CMD_HELP_OPT("-f number", "frequency (Hz)")
+BAREBOX_CMD_HELP_OPT("-w number", "duty cycle (%) - on 'width' of each cycle")
+BAREBOX_CMD_HELP_OPT("-i number", "line inverted polarity")
+BAREBOX_CMD_HELP_OPT("-s\t", "stop (disable) the pwm device")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(pwm_set)
+	.cmd		= do_pwm_set,
+	BAREBOX_CMD_DESC("set pwm")
+	BAREBOX_CMD_OPTS("[-dnDPfwis]")
+	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
+	BAREBOX_CMD_HELP(cmd_pwm_set_help)
+BAREBOX_CMD_END
-- 
2.35.3

Re: [PATCH 1/1] commands: add pwm manipulation command

[Jookia]

Hi, not really a Barebox developer but I figured I might chime in.

On Sun, May 28, 2023 at 09:24:09AM +1000, Marc Reilly wrote:
> This introduces a command to set parameters for a pwm device.
> 
> Signed-off-by: Marc Reilly <marc@cpdesign.com.au>
> ---

...

> +	if ((n < 0) && !devname) {
> +		printf(" need to specify a device\n");
> +		return COMMAND_ERROR_USAGE;
> +	}
> +	if ((freq == 0) || (period == 0)) {
> +		printf(" period or freqency needs to be non-zero\n");
> +		return COMMAND_ERROR_USAGE;
> +	}
> +
> +	if (!devname) {
> +		sprintf(namebuf, "pwm%d", n);
> +	} else {
> +		strcpy(namebuf, devname);
> +	}

Is devname capped to namebuf length? It might be better refer to devname
instead of namebuf and point devname to namebuf when you use namebuf, otherwise
point it to the the optarg.

Is it even worth supporting refering by number instead of just having scripts
type pwmX?

> +	pwm = pwm_request(namebuf);
> +	if (!pwm) {
> +		printf("pwm device %s not found\n", namebuf);

No space at the start?

> +		return -ENODEV;
> +	}
> +
> +	pwm_get_state(pwm, &state);
> +
> +	if ((state.period_ns == 0)
> +			&& (freq < 0) && (duty < 0) && (period < 0)) {
> +		printf(" need to know some timing info; freq or dury/period\n");

No pwm_free?

> +		return COMMAND_ERROR_USAGE;
> +	}
> +
> +	if (polarity >= 0)
> +		state.polarity = polarity;
> +
> +	/* period */
> +	if (freq > 0) {
> +		state.p_enable = true;
> +		state.period_ns = HZ_TO_NANOSECONDS(freq);
> +		if (width < 0) {
> +			width = 50;
> +		}
> +	} else if (period > 0) {
> +		state.p_enable = true;
> +		state.period_ns = period;
> +	}
> +
> +	/* duty */
> +	if (width >= 0) {
> +		if (width > 100)
> +			width = 100;
> +
> +		pwm_set_relative_duty_cycle(&state, width, 100);
> +	} else if (duty >= 0) {
> +		if (duty > state.period_ns)
> +			printf(" warning: duty_ns is greater than period\n");
> +
> +		state.duty_ns = duty;
> +	}

It might be worth moving the width and duty checks to up with the opt parsing
and make a width > 100 and error.

Jookia.