* Adding PWM commands @ 2023-05-29 11:06 Marc Reilly 2023-05-29 11:06 ` [PATCH v3 1/3] commands: add pwm manipulation command Marc Reilly ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Marc Reilly @ 2023-05-29 11:06 UTC (permalink / raw) To: barebox Thanks again all for comments so far. Changes in v3: - fix minor typos - remove unused 'error' var in pwm_get Changes in v2: - remove '-n' option to specify device by number - fix freeing resources on all return paths - add pwm_get command ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/3] commands: add pwm manipulation command 2023-05-29 11:06 Adding PWM commands Marc Reilly @ 2023-05-29 11:06 ` Marc Reilly 2023-05-30 9:27 ` Sascha Hauer 2023-05-29 11:06 ` [PATCH v3 2/3] commands: pwm: add pwm_get command Marc Reilly 2023-05-29 11:06 ` [PATCH v3 3/3] include: pwm: minor function doc fix for pwm_set_relative_duty_cycle() Marc Reilly 2 siblings, 1 reply; 7+ messages in thread From: Marc Reilly @ 2023-05-29 11:06 UTC (permalink / raw) To: barebox 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 | 143 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 commands/pwm.c diff --git a/commands/Kconfig b/commands/Kconfig index 4d3ff631a8..cc7585bbb2 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 [dDPfwis] + + 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..94bbbaf1be --- /dev/null +++ b/commands/pwm.c @@ -0,0 +1,143 @@ +// 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 *devname = NULL; + int duty = -1, period = -1; + int freq = -1, width = -1; + int invert_polarity = -1, stop = -1; + int opt; + + while ((opt = getopt(argc, argv, "d:D:P:f:w:is")) > 0) { + switch (opt) { + 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': + invert_polarity = 1; + break; + case 's': + stop = 1; + break; + } + } + + if (!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 (width > 100) { + printf(" width (%% duty cycle) can't be more than 100%%\n"); + return COMMAND_ERROR_USAGE; + } + + pwm = pwm_request(devname); + if (!pwm) { + printf(" pwm device %s not found\n", devname); + 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 duty/period\n"); + pwm_free(pwm); + return COMMAND_ERROR_USAGE; + } + + if (invert_polarity >= 0) + state.polarity = invert_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) { + 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 by device name") +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 - except when setting frequency, in which case") +BAREBOX_CMD_HELP_TEXT(" the 'width' will default to 50%.") +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 state, set the duty to 0.") +BAREBOX_CMD_HELP_TEXT("Options:") +BAREBOX_CMD_HELP_OPT("-d string", "device name (eg 'pwm0')") +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 (%) - the on 'width' of each cycle") +BAREBOX_CMD_HELP_OPT("-i\t", "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("[-dDPfwis]") + BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP) + BAREBOX_CMD_HELP(cmd_pwm_set_help) +BAREBOX_CMD_END -- 2.35.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] commands: add pwm manipulation command 2023-05-29 11:06 ` [PATCH v3 1/3] commands: add pwm manipulation command Marc Reilly @ 2023-05-30 9:27 ` Sascha Hauer 0 siblings, 0 replies; 7+ messages in thread From: Sascha Hauer @ 2023-05-30 9:27 UTC (permalink / raw) To: Marc Reilly; +Cc: barebox On Mon, May 29, 2023 at 09:06:25PM +1000, Marc Reilly wrote: > 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 | 143 ++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 155 insertions(+) > create mode 100644 commands/pwm.c > > diff --git a/commands/Kconfig b/commands/Kconfig > index 4d3ff631a8..cc7585bbb2 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 [dDPfwis] > + > + 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..94bbbaf1be > --- /dev/null > +++ b/commands/pwm.c > @@ -0,0 +1,143 @@ > +// 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 *devname = NULL; > + int duty = -1, period = -1; > + int freq = -1, width = -1; > + int invert_polarity = -1, stop = -1; better bool invert_polarity = false, stop = false. > + int opt; > + > + while ((opt = getopt(argc, argv, "d:D:P:f:w:is")) > 0) { > + switch (opt) { > + 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': > + invert_polarity = 1; > + break; > + case 's': > + stop = 1; > + break; > + } > + } > + > + if (!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; > + } You should also check if both freq and period are given: if (freq >= 0 && period >= 0) printf("Specify period or frequency, not both\n"); likewise width and duty: if (width >= 0 && duty >= 0) printf("Specify width or duty cycle, not both\n"); > + if (width > 100) { > + printf(" width (%% duty cycle) can't be more than 100%%\n"); > + return COMMAND_ERROR_USAGE; > + } > + > + pwm = pwm_request(devname); > + if (!pwm) { > + printf(" pwm device %s not found\n", devname); > + return -ENODEV; > + } > + > + pwm_get_state(pwm, &state); > + > + if ((state.period_ns == 0) > + && (freq < 0) && (duty < 0) && (period < 0)) { It's already an error when: state.period_ns == 0 && freq < 0 && period < 0 No need to check for duty here. > + printf(" need to know some timing info; freq or duty/period\n"); > + pwm_free(pwm); > + return COMMAND_ERROR_USAGE; > + } Maybe just print the current PWM state when no actions are given? > + > + if (invert_polarity >= 0) > + state.polarity = invert_polarity; > + > + /* period */ > + if (freq > 0) { > + state.p_enable = true; > + state.period_ns = HZ_TO_NANOSECONDS(freq); > + if (width < 0) { > + width = 50; > + } I think this shouldn't be here, at least I don't see why it shouldn't be possible to configure a PWM with -f x -D y. > + } else if (period > 0) { > + state.p_enable = true; > + state.period_ns = period; > + } > + > + /* duty */ > + if (width >= 0) { > + 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); > + } No need to call pwm_apply_state() twice when the PWM shall be stopped. This should do it: if (stop > 0) state.p_enable = false; error = pwm_apply_state(pwm, &state); 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 | ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] commands: pwm: add pwm_get command 2023-05-29 11:06 Adding PWM commands Marc Reilly 2023-05-29 11:06 ` [PATCH v3 1/3] commands: add pwm manipulation command Marc Reilly @ 2023-05-29 11:06 ` Marc Reilly 2023-05-30 9:32 ` Sascha Hauer 2023-05-29 11:06 ` [PATCH v3 3/3] include: pwm: minor function doc fix for pwm_set_relative_duty_cycle() Marc Reilly 2 siblings, 1 reply; 7+ messages in thread From: Marc Reilly @ 2023-05-29 11:06 UTC (permalink / raw) To: barebox Signed-off-by: Marc Reilly <marc@cpdesign.com.au> --- commands/Kconfig | 7 ++++++ commands/pwm.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/commands/Kconfig b/commands/Kconfig index cc7585bbb2..f2a6d22f69 100644 --- a/commands/Kconfig +++ b/commands/Kconfig @@ -1940,6 +1940,13 @@ config CMD_PWM Controls the pwm values such as period and duty cycle + + pwm_get - get pwm state + + Usage: pwm_get [d] + + Displays the pwm values such as period and duty cycle + config CMD_LED bool depends on LED diff --git a/commands/pwm.c b/commands/pwm.c index 94bbbaf1be..18f65b0a59 100644 --- a/commands/pwm.c +++ b/commands/pwm.c @@ -141,3 +141,59 @@ BAREBOX_CMD_START(pwm_set) BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP) BAREBOX_CMD_HELP(cmd_pwm_set_help) BAREBOX_CMD_END + +#define HZ_FROM_NANOSECONDS(x) (1000000000UL/(x)) + +static int do_pwm_get(int argc, char *argv[]) +{ + struct pwm_device *pwm = NULL; + struct pwm_state state; + char *devname = NULL; + int opt; + + while ((opt = getopt(argc, argv, "d:")) > 0) { + switch (opt) { + case 'd': + devname = optarg; + break; + } + } + + if (!devname) { + printf(" need to specify a device\n"); + return COMMAND_ERROR_USAGE; + } + + pwm = pwm_request(devname); + if (!pwm) { + printf(" pwm device %s not found\n", devname); + return -ENODEV; + } + + pwm_get_state(pwm, &state); + + printf(" pwm params for '%s':\n", devname); + printf(" period : %u (ns)\n", state.period_ns); + printf(" duty : %u (ns)\n", state.duty_ns); + printf(" enabled : %d\n", state.p_enable); + printf(" polarity : %s\n", state.polarity == 0 ? "Normal" : "Inverted"); + printf(" freq : %lu (Hz)\n", HZ_FROM_NANOSECONDS(state.period_ns)); + + pwm_free(pwm); + return 0; +} + +BAREBOX_CMD_HELP_START(pwm_get) +BAREBOX_CMD_HELP_TEXT("Gets a pwm device parameters.") +BAREBOX_CMD_HELP_TEXT(" Specify the device by device name") +BAREBOX_CMD_HELP_TEXT("Options:") +BAREBOX_CMD_HELP_OPT("-d string", "device name (eg 'pwm0')") +BAREBOX_CMD_HELP_END + +BAREBOX_CMD_START(pwm_get) + .cmd = do_pwm_get, + BAREBOX_CMD_DESC("get pwm") + BAREBOX_CMD_OPTS("[-d]") + BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP) + BAREBOX_CMD_HELP(cmd_pwm_get_help) +BAREBOX_CMD_END -- 2.35.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/3] commands: pwm: add pwm_get command 2023-05-29 11:06 ` [PATCH v3 2/3] commands: pwm: add pwm_get command Marc Reilly @ 2023-05-30 9:32 ` Sascha Hauer 0 siblings, 0 replies; 7+ messages in thread From: Sascha Hauer @ 2023-05-30 9:32 UTC (permalink / raw) To: Marc Reilly; +Cc: barebox On Mon, May 29, 2023 at 09:06:26PM +1000, Marc Reilly wrote: > Signed-off-by: Marc Reilly <marc@cpdesign.com.au> > --- > commands/Kconfig | 7 ++++++ > commands/pwm.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 63 insertions(+) > > diff --git a/commands/Kconfig b/commands/Kconfig > index cc7585bbb2..f2a6d22f69 100644 > --- a/commands/Kconfig > +++ b/commands/Kconfig > @@ -1940,6 +1940,13 @@ config CMD_PWM > > Controls the pwm values such as period and duty cycle > > + > + pwm_get - get pwm state > + > + Usage: pwm_get [d] > + > + Displays the pwm values such as period and duty cycle > + > config CMD_LED > bool > depends on LED > diff --git a/commands/pwm.c b/commands/pwm.c > index 94bbbaf1be..18f65b0a59 100644 > --- a/commands/pwm.c > +++ b/commands/pwm.c > @@ -141,3 +141,59 @@ BAREBOX_CMD_START(pwm_set) > BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP) > BAREBOX_CMD_HELP(cmd_pwm_set_help) > BAREBOX_CMD_END > + > +#define HZ_FROM_NANOSECONDS(x) (1000000000UL/(x)) > + > +static int do_pwm_get(int argc, char *argv[]) > +{ > + struct pwm_device *pwm = NULL; > + struct pwm_state state; > + char *devname = NULL; > + int opt; > + > + while ((opt = getopt(argc, argv, "d:")) > 0) { > + switch (opt) { > + case 'd': > + devname = optarg; > + break; > + } > + } > + > + if (!devname) { > + printf(" need to specify a device\n"); > + return COMMAND_ERROR_USAGE; > + } > + > + pwm = pwm_request(devname); > + if (!pwm) { > + printf(" pwm device %s not found\n", devname); > + return -ENODEV; > + } > + > + pwm_get_state(pwm, &state); > + > + printf(" pwm params for '%s':\n", devname); > + printf(" period : %u (ns)\n", state.period_ns); > + printf(" duty : %u (ns)\n", state.duty_ns); > + printf(" enabled : %d\n", state.p_enable); > + printf(" polarity : %s\n", state.polarity == 0 ? "Normal" : "Inverted"); > + printf(" freq : %lu (Hz)\n", HZ_FROM_NANOSECONDS(state.period_ns)); Not sure if this deserves an extra command. You could print the information along with the pwm_set command when no action is given or when an additional -v (for verbose) is given. When doint this the command could simply be named 'pwm' instead of 'pwm_set' 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 | ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] include: pwm: minor function doc fix for pwm_set_relative_duty_cycle() 2023-05-29 11:06 Adding PWM commands Marc Reilly 2023-05-29 11:06 ` [PATCH v3 1/3] commands: add pwm manipulation command Marc Reilly 2023-05-29 11:06 ` [PATCH v3 2/3] commands: pwm: add pwm_get command Marc Reilly @ 2023-05-29 11:06 ` Marc Reilly 2 siblings, 0 replies; 7+ messages in thread From: Marc Reilly @ 2023-05-29 11:06 UTC (permalink / raw) To: barebox Signed-off-by: Marc Reilly <marc@cpdesign.com.au> --- include/pwm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pwm.h b/include/pwm.h index 643ef4d7ee..02af7d0a5f 100644 --- a/include/pwm.h +++ b/include/pwm.h @@ -71,7 +71,7 @@ unsigned int pwm_get_period(struct pwm_device *pwm); * @scale: scale in which @duty_cycle is expressed * * This functions converts a relative into an absolute duty cycle (expressed - * in nanoseconds), and puts the result in state->duty_cycle. + * in nanoseconds), and puts the result in state->duty_ns. * * For example if you want to configure a 50% duty cycle, call: * -- 2.35.3 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Adding PWM Commands @ 2023-05-29 0:13 Marc Reilly 0 siblings, 0 replies; 7+ messages in thread From: Marc Reilly @ 2023-05-29 0:13 UTC (permalink / raw) To: barebox Hi, Thanks all for comments so far. Changes in v2: - remove '-n' option to specify device by number - fix freeing resources on all return paths - add pwm_get command Cheers Marc ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-05-30 9:33 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-29 11:06 Adding PWM commands Marc Reilly 2023-05-29 11:06 ` [PATCH v3 1/3] commands: add pwm manipulation command Marc Reilly 2023-05-30 9:27 ` Sascha Hauer 2023-05-29 11:06 ` [PATCH v3 2/3] commands: pwm: add pwm_get command Marc Reilly 2023-05-30 9:32 ` Sascha Hauer 2023-05-29 11:06 ` [PATCH v3 3/3] include: pwm: minor function doc fix for pwm_set_relative_duty_cycle() Marc Reilly -- strict thread matches above, loose matches on Subject: below -- 2023-05-29 0:13 Adding PWM Commands Marc Reilly
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox