Hi Simon, >-----Original Message----- >From: Simon Glass <s...@chromium.org> >Sent: 12 December 2020 21:05 >To: Pragnesh Patel <pragnesh.pa...@openfive.com> >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Atish Patra ><atish.pa...@wdc.com>; Palmer Dabbelt <palmerdabb...@google.com>; Bin >Meng <bmeng...@gmail.com>; Paul Walmsley ( Sifive) ><paul.walms...@sifive.com>; Anup Patel <anup.pa...@wdc.com>; Sagar Kadam ><sagar.ka...@openfive.com>; rick <r...@andestech.com>; Naoki Hayama ><naoki.hay...@lineo.co.jp>; Marek Vasut <marek.vasut+rene...@gmail.com>; >Patrick Delaunay <patrick.delau...@st.com>; Adam Ford ><aford...@gmail.com>; Thomas Hebb <tommyh...@gmail.com>; Ramon Fried ><rfried....@gmail.com>; Heinrich Schuchardt <xypron.g...@gmx.de>; Bin Meng ><bin.m...@windriver.com>; Sam Protsenko <joe.s...@gmail.com>; Miquel >Raynal <miquel.ray...@bootlin.com>; Frédéric Danis ><frederic.da...@collabora.com>; Philippe Reynes ><philippe.rey...@softathome.com>; Patrice Chotard <patrice.chot...@st.com>; >Baruch Siach <bar...@tkos.co.il>; Vladimir Olovyannikov ><vladimir.olovyanni...@broadcom.com> >Subject: Re: [RESEND,PATCH v3] cmd: Add a pwm command > >[External Email] Do not click links or attachments unless you recognize the >sender and know the content is safe > >Hi Pragnesh, > >On Wed, 2 Dec 2020 at 21:59, Pragnesh Patel <pragnesh.pa...@sifive.com> >wrote: >> >> Add the command "pwm" for controlling the pwm channels. This command >> provides pwm invert/config/enable/disable functionalities via PWM >> uclass drivers >> >> Signed-off-by: Pragnesh Patel <pragnesh.pa...@sifive.com> >> Reviewed-by: Simon Glass <s...@chromium.org> >> --- >> >> Changes in v3: >> - Replace goto with return >> - Print return value for error >> - Change the assert condition for success >> >> Changes in v2: >> - Add test for pwm command >> >> README | 1 + >> cmd/Kconfig | 6 ++ >> cmd/Makefile | 1 + >> cmd/pwm.c | 117 ++++++++++++++++++++++++++++++++++++++ >> configs/sandbox_defconfig | 1 + >> test/cmd/Makefile | 1 + >> test/cmd/pwm.c | 47 +++++++++++++++ >> 7 files changed, 174 insertions(+) >> create mode 100644 cmd/pwm.c >> create mode 100644 test/cmd/pwm.c >> >> diff --git a/README b/README >> index cb49aa15da..dab291e0d0 100644 >> --- a/README >> +++ b/README >> @@ -3160,6 +3160,7 @@ i2c - I2C sub-system >> sspi - SPI utility commands >> base - print or set address offset >> printenv- print environment variables >> +pwm - control pwm channels >> setenv - set environment variables >> saveenv - save environment variables to persistent storage protect - >> enable or disable FLASH write protection diff --git a/cmd/Kconfig >> b/cmd/Kconfig index 1595de999b..0d085108f4 100644 >> --- a/cmd/Kconfig >> +++ b/cmd/Kconfig >> @@ -918,6 +918,12 @@ config CMD_GPIO >> help >> GPIO support. >> >> +config CMD_PWM >> + bool "pwm" >> + depends on DM_PWM >> + help >> + Control PWM channels, this allows invert/config/enable/disable PWM >channels. >> + >> config CMD_GPT >> bool "GPT (GUID Partition Table) command" >> select EFI_PARTITION >> diff --git a/cmd/Makefile b/cmd/Makefile index dd86675bf2..75df3c136c >> 100644 >> --- a/cmd/Makefile >> +++ b/cmd/Makefile >> @@ -120,6 +120,7 @@ endif >> obj-$(CONFIG_CMD_PINMUX) += pinmux.o >> obj-$(CONFIG_CMD_PMC) += pmc.o >> obj-$(CONFIG_CMD_PSTORE) += pstore.o >> +obj-$(CONFIG_CMD_PWM) += pwm.o >> obj-$(CONFIG_CMD_PXE) += pxe.o pxe_utils.o >> obj-$(CONFIG_CMD_WOL) += wol.o >> obj-$(CONFIG_CMD_QFW) += qfw.o >> diff --git a/cmd/pwm.c b/cmd/pwm.c >> new file mode 100644 >> index 0000000000..5849fc57b6 >> --- /dev/null >> +++ b/cmd/pwm.c >> @@ -0,0 +1,117 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * Control PWM channels >> + * >> + * Copyright (c) 2020 SiFive, Inc >> + * author: Pragnesh Patel <pragnesh.pa...@sifive.com> */ >> + >> +#include <command.h> >> +#include <dm.h> >> +#include <pwm.h> >> + >> +enum pwm_cmd { >> + PWM_SET_INVERT, >> + PWM_SET_CONFIG, >> + PWM_SET_ENABLE, >> + PWM_SET_DISABLE, >> +}; >> + >> +static int do_pwm(struct cmd_tbl *cmdtp, int flag, int argc, >> + char *const argv[]) >> +{ >> + const char *str_cmd, *str_channel = NULL, *str_enable = NULL; >> + const char *str_pwm = NULL, *str_period = NULL, *str_duty = NULL; >> + enum pwm_cmd sub_cmd; >> + struct udevice *dev; >> + u32 channel, pwm_enable, pwm_dev, period_ns = 0, duty_ns = 0; >> + int ret; >> + >> + if (argc < 4) >> + return CMD_RET_USAGE; >> + >> + str_cmd = argv[1]; >> + argc -= 2; >> + argv += 2; >> + >> + if (argc > 0) { >> + str_pwm = *argv; >> + argc--; >> + argv++; >> + } >> + >> + if (!str_pwm) >> + return CMD_RET_USAGE; >> + >> + switch (*str_cmd) { >> + case 'i': >> + sub_cmd = PWM_SET_INVERT; >> + break; >> + case 'c': >> + sub_cmd = PWM_SET_CONFIG; >> + break; >> + case 'e': >> + sub_cmd = PWM_SET_ENABLE; >> + break; >> + case 'd': >> + sub_cmd = PWM_SET_DISABLE; >> + break; >> + default: >> + return CMD_RET_USAGE; >> + } >> + >> + pwm_dev = simple_strtoul(str_pwm, NULL, 10); >> + ret = uclass_get_device(UCLASS_PWM, pwm_dev, &dev); >> + if (ret) { >> + printf("pwm: '%s' not found\n", str_pwm); >> + return cmd_process_error(cmdtp, ret); >> + } >> + >> + if (argc > 0) { >> + str_channel = *argv; >> + channel = simple_strtoul(str_channel, NULL, 10); >> + argc--; >> + argv++; >> + } else { >> + return CMD_RET_USAGE; >> + } >> + >> + if (sub_cmd == PWM_SET_INVERT && argc > 0) { >> + str_enable = *argv; >> + pwm_enable = simple_strtoul(str_enable, NULL, 10); >> + ret = pwm_set_invert(dev, channel, pwm_enable); >> + } else if (sub_cmd == PWM_SET_CONFIG && argc == 2) { >> + str_period = *argv; >> + argc--; >> + argv++; >> + period_ns = simple_strtoul(str_period, NULL, 10); >> + >> + if (argc > 0) { >> + str_duty = *argv; >> + duty_ns = simple_strtoul(str_duty, NULL, 10); >> + } >> + >> + ret = pwm_set_config(dev, channel, period_ns, duty_ns); >> + } else if (sub_cmd == PWM_SET_ENABLE) { >> + ret = pwm_set_enable(dev, channel, 1); >> + } else if (sub_cmd == PWM_SET_DISABLE) { >> + ret = pwm_set_enable(dev, channel, 0); >> + } else { >> + printf("PWM arguments missing\n"); >> + return CMD_RET_FAILURE; >> + } >> + >> + if (ret) { >> + printf("error(%d)\n", ret); >> + return CMD_RET_FAILURE; >> + } >> + >> + return CMD_RET_SUCCESS; >> +} >> + >> +U_BOOT_CMD(pwm, 6, 0, do_pwm, >> + "control pwm channels", >> + "pwm <invert> <pwm_dev_num> <channel> <polarity>\n" >> + "pwm <config> <pwm_dev_num> <channel> <period_ns> <duty_ns>\n" >> + "pwm <enable/disable> <pwm_dev_num> <channel>\n" >> + "Note: All input values are in decimal"); >> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig >> index f2a767a4cd..7a16603461 100644 >> --- a/configs/sandbox_defconfig >> +++ b/configs/sandbox_defconfig >> @@ -58,6 +58,7 @@ CONFIG_CMD_LSBLK=y >> CONFIG_CMD_MUX=y >> CONFIG_CMD_OSD=y >> CONFIG_CMD_PCI=y >> +CONFIG_CMD_PWM=y >> CONFIG_CMD_READ=y >> CONFIG_CMD_REMOTEPROC=y >> CONFIG_CMD_SPI=y >> diff --git a/test/cmd/Makefile b/test/cmd/Makefile index >> 859dcda239..dde98dd371 100644 >> --- a/test/cmd/Makefile >> +++ b/test/cmd/Makefile >> @@ -4,3 +4,4 @@ >> >> obj-y += mem.o >> obj-$(CONFIG_CMD_MEM_SEARCH) += mem_search.o >> +obj-$(CONFIG_CMD_PWM) += pwm.o >> diff --git a/test/cmd/pwm.c b/test/cmd/pwm.c new file mode 100644 >> index 0000000000..3cc1aaffd4 >> --- /dev/null >> +++ b/test/cmd/pwm.c >> @@ -0,0 +1,47 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * Test for pwm command >> + * >> + * Copyright 2020 SiFive, Inc >> + * >> + * Authors: >> + * Pragnesh Patel <pragnesh.pa...@sifive.com> >> + */ >> + >> +#include <dm.h> >> +#include <dm/test.h> >> +#include <test/test.h> >> +#include <test/ut.h> >> + >> +/* Basic test of 'pwm' command */ >> +static int dm_test_pwm_cmd(struct unit_test_state *uts) { >> + struct udevice *dev; >> + >> + ut_assertok(uclass_get_device(UCLASS_PWM, 0, &dev)); >> + ut_assertnonnull(dev); >> + >> + ut_assertok(console_record_reset_enable()); >> + >> + /* pwm <invert> <pwm_dev_num> <channel> <polarity> */ >> + run_command("pwm invert 0 0 1", 0); > >These should have ut_assertok() also.
Will update in v4. > >> + ut_assert_console_end(); >> + >> + run_command("pwm invert 0 0 0", 0); >> + ut_assert_console_end(); >> + >> + /* pwm <config> <pwm_dev_num> <channel> <period_ns> <duty_ns> */ >> + run_command("pwm config 0 0 period_ns duty_ns", 0); >> + ut_assert_console_end(); >> + >> + /* pwm <enable/disable> <pwm_dev_num> <channel> */ >> + run_command("pwm enable 0 0", 0); >> + ut_assert_console_end(); >> + >> + run_command("pwm disable 0 0", 0); >> + ut_assert_console_end(); >> + >> + return 0; >> +} >> + >> +DM_TEST(dm_test_pwm_cmd, UT_TESTF_SCAN_PDATA | >UT_TESTF_SCAN_FDT | >> +UT_TESTF_CONSOLE_REC); >> -- >> 2.17.1 >> > >Regards, >Simon