Re: [PATCH v2] cmd: Add a pwm command
Hi Pragnesh, On Tue, 1 Dec 2020 at 00:05, Pragnesh Patel wrote: > > Hi Simon, > > >-Original Message- > >From: U-Boot On Behalf Of Pragnesh Patel > >Sent: 01 December 2020 11:17 > >To: Simon Glass > >Cc: U-Boot Mailing List ; Atish Patra > >; Palmer Dabbelt ; Bin > >Meng ; Paul Walmsley ( Sifive) > >; Anup Patel ; Sagar Kadam > >; rick ; Naoki Hayama > >; Marek Vasut ; > >Patrick Delaunay ; Adam Ford > >; Thomas Hebb ; Ramon Fried > >; Heinrich Schuchardt ; Bin Meng > >; Sam Protsenko ; Miquel > >Raynal ; Philippe Reynes > >; Frédéric Danis > >; Patrice Chotard ; > >Vladimir Olovyannikov > >Subject: RE: [PATCH v2] cmd: Add a pwm command > > > >Hi Simon, > > > >>-Original Message- > >>From: Simon Glass > >>Sent: 01 December 2020 01:42 > >>To: Pragnesh Patel > >>Cc: U-Boot Mailing List ; Atish Patra > >>; Palmer Dabbelt ; Bin > >>Meng ; Paul Walmsley ( Sifive) > >>; Anup Patel ; Sagar > >>Kadam ; rick ; Naoki > >>Hayama ; Marek Vasut > >>; Patrick Delaunay > >>; Adam Ford ; Thomas Hebb > >>; Ramon Fried ; Heinrich > >>Schuchardt ; Bin Meng ; > >Sam > >>Protsenko ; Miquel Raynal > >>; Philippe Reynes > >>; Frédéric Danis > >>; Patrice Chotard > >>; Vladimir Olovyannikov > >> > >>Subject: Re: [PATCH v2] 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 Thu, 26 Nov 2020 at 03:48, Pragnesh Patel > >> > >>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 > >>> --- > >>> > >>> Changes in v2: > >>> - Add test for pwm command > >>> > >>> > >>> README| 1 + > >>> cmd/Kconfig | 6 ++ > >>> cmd/Makefile | 1 + > >>> cmd/pwm.c | 120 ++ > >>> configs/sandbox_defconfig | 1 + > >>> test/cmd/Makefile | 1 + > >>> test/cmd/pwm.c| 54 + > >>> 7 files changed, 184 insertions(+) > >>> create mode 100644 cmd/pwm.c > >>> create mode 100644 test/cmd/pwm.c > >> > >>Reviewed-by: Simon Glass > >> > >>Minor nits below > >> > >>> > >>> 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 > &
RE: [PATCH v2] cmd: Add a pwm command
Hi Simon,
>-Original Message-
>From: U-Boot On Behalf Of Pragnesh Patel
>Sent: 01 December 2020 11:17
>To: Simon Glass
>Cc: U-Boot Mailing List ; Atish Patra
>; Palmer Dabbelt ; Bin
>Meng ; Paul Walmsley ( Sifive)
>; Anup Patel ; Sagar Kadam
>; rick ; Naoki Hayama
>; Marek Vasut ;
>Patrick Delaunay ; Adam Ford
>; Thomas Hebb ; Ramon Fried
>; Heinrich Schuchardt ; Bin Meng
>; Sam Protsenko ; Miquel
>Raynal ; Philippe Reynes
>; Frédéric Danis
>; Patrice Chotard ;
>Vladimir Olovyannikov
>Subject: RE: [PATCH v2] cmd: Add a pwm command
>
>Hi Simon,
>
>>-Original Message-
>>From: Simon Glass
>>Sent: 01 December 2020 01:42
>>To: Pragnesh Patel
>>Cc: U-Boot Mailing List ; Atish Patra
>>; Palmer Dabbelt ; Bin
>>Meng ; Paul Walmsley ( Sifive)
>>; Anup Patel ; Sagar
>>Kadam ; rick ; Naoki
>>Hayama ; Marek Vasut
>>; Patrick Delaunay
>>; Adam Ford ; Thomas Hebb
>>; Ramon Fried ; Heinrich
>>Schuchardt ; Bin Meng ;
>Sam
>>Protsenko ; Miquel Raynal
>>; Philippe Reynes
>>; Frédéric Danis
>>; Patrice Chotard
>>; Vladimir Olovyannikov
>>
>>Subject: Re: [PATCH v2] 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 Thu, 26 Nov 2020 at 03:48, Pragnesh Patel
>>
>>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
>>> ---
>>>
>>> Changes in v2:
>>> - Add test for pwm command
>>>
>>>
>>> README| 1 +
>>> cmd/Kconfig | 6 ++
>>> cmd/Makefile | 1 +
>>> cmd/pwm.c | 120 ++
>>> configs/sandbox_defconfig | 1 +
>>> test/cmd/Makefile | 1 +
>>> test/cmd/pwm.c| 54 +
>>> 7 files changed, 184 insertions(+)
>>> create mode 100644 cmd/pwm.c
>>> create mode 100644 test/cmd/pwm.c
>>
>>Reviewed-by: Simon Glass
>>
>>Minor nits below
>>
>>>
>>> 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 00..f704c7a755
>>> --- /dev/null
>>> +++ b/cmd/pwm.c
>>> @@ -0,0 +1,120 @@
>>> +// SPDX-License-Identifier: GPL-2.0+
>>> +/*
>>> + * Control PWM channels
>>> + *
>>> + * Copyright (c) 2020 SiFive, Inc
>>> + * author: Pragnesh Patel */
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +enum pwm_cmd {
>>> + PWM_SET_INVERT
RE: [PATCH v2] cmd: Add a pwm command
Hi Simon,
>-Original Message-
>From: Simon Glass
>Sent: 01 December 2020 01:42
>To: Pragnesh Patel
>Cc: U-Boot Mailing List ; Atish Patra
>; Palmer Dabbelt ; Bin
>Meng ; Paul Walmsley ( Sifive)
>; Anup Patel ; Sagar Kadam
>; rick ; Naoki Hayama
>; Marek Vasut ;
>Patrick Delaunay ; Adam Ford
>; Thomas Hebb ; Ramon Fried
>; Heinrich Schuchardt ; Bin Meng
>; Sam Protsenko ; Miquel
>Raynal ; Philippe Reynes
>; Frédéric Danis
>; Patrice Chotard ;
>Vladimir Olovyannikov
>Subject: Re: [PATCH v2] 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 Thu, 26 Nov 2020 at 03:48, Pragnesh Patel
>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
>> ---
>>
>> Changes in v2:
>> - Add test for pwm command
>>
>>
>> README| 1 +
>> cmd/Kconfig | 6 ++
>> cmd/Makefile | 1 +
>> cmd/pwm.c | 120 ++
>> configs/sandbox_defconfig | 1 +
>> test/cmd/Makefile | 1 +
>> test/cmd/pwm.c| 54 +
>> 7 files changed, 184 insertions(+)
>> create mode 100644 cmd/pwm.c
>> create mode 100644 test/cmd/pwm.c
>
>Reviewed-by: Simon Glass
>
>Minor nits below
>
>>
>> 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 00..f704c7a755
>> --- /dev/null
>> +++ b/cmd/pwm.c
>> @@ -0,0 +1,120 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Control PWM channels
>> + *
>> + * Copyright (c) 2020 SiFive, Inc
>> + * author: Pragnesh Patel */
>> +
>> +#include
>> +#include
>> +#include
>> +
>> +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)
>> + show_usage:
>> + return CMD_RET_USAGE;
>> +
>> + str_cmd = argv[1];
>> + argc -= 2;
>> + argv += 2;
>> +
>> + if (argc > 0) {
>> + str_pwm = *argv;
>> + argc--;
>> + argv++;
>> + }
>> +
>> + if (!str_pwm)
>> + goto show_usage;
>> +
>> + switch (*str_cmd) {
>&
Re: [PATCH v2] cmd: Add a pwm command
Hi Pragnesh,
On Thu, 26 Nov 2020 at 03:48, Pragnesh Patel 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
> ---
>
> Changes in v2:
> - Add test for pwm command
>
>
> README| 1 +
> cmd/Kconfig | 6 ++
> cmd/Makefile | 1 +
> cmd/pwm.c | 120 ++
> configs/sandbox_defconfig | 1 +
> test/cmd/Makefile | 1 +
> test/cmd/pwm.c| 54 +
> 7 files changed, 184 insertions(+)
> create mode 100644 cmd/pwm.c
> create mode 100644 test/cmd/pwm.c
Reviewed-by: Simon Glass
Minor nits below
>
> 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 00..f704c7a755
> --- /dev/null
> +++ b/cmd/pwm.c
> @@ -0,0 +1,120 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Control PWM channels
> + *
> + * Copyright (c) 2020 SiFive, Inc
> + * author: Pragnesh Patel
> + */
> +
> +#include
> +#include
> +#include
> +
> +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)
> + show_usage:
> + return CMD_RET_USAGE;
> +
> + str_cmd = argv[1];
> + argc -= 2;
> + argv += 2;
> +
> + if (argc > 0) {
> + str_pwm = *argv;
> + argc--;
> + argv++;
> + }
> +
> + if (!str_pwm)
> + goto show_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:
> + goto show_usage;
I think it is better to use 'return CMD_RET_USAGE' at each place
rather than use a goto.
> + }
> +
> + if (IS_ENABLED(CONFIG_DM_PWM)) {
You don't need this because the command is only enabled if DM_PWM
> + 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);
printf("pwm: ...
> + return cmd_process_error(cmdtp, ret);
> + }
> + }
> +
> + if (argc > 0) {
> + str_channel = *argv;
> + channel = simple_strtoul(str_channel, NULL, 10);
> + argc--;
> + argv++;
> + } else {
> + goto show_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--;
> +
