diff mbox series

cmd: sound: fix help text

Message ID 20240610-cmd-sound-help-v1-1-0c1e13a49ec4@cherry.de
State Accepted
Commit ca6a992e09441d6cca73439c63c3735f86b36ea4
Delegated to: Tom Rini
Headers show
Series cmd: sound: fix help text | expand

Commit Message

Quentin Schulz June 10, 2024, 4:11 p.m. UTC
From: Quentin Schulz <quentin.schulz@cherry.de>

There's never been a -q or -s argument handled in the command, so let's
remove it. This was highlighted during review[1] but somehow still got
through.

While at it, slightly "reword" in the help text how the len + freq
arguments are defined. Indeed, len and freq work in pair, it is possible
to define none of either, n of both, or n - 1 of freq if there are n
len, in which case the freq that goes with the last len would be the n -
1 (and not the default of 400Hz if neither len nor freq is passed). I
assume this isn't what's expected but leaving it for another patch if
need be to fix what happens in that very odd scenario.

[1] https://lore.kernel.org/u-boot/CAPnjgZ0QWNqVFZfEWHxRcFOA3E3gRAZCYs77nGUXKL0pLp+JLQ@mail.gmail.com/

Fixes: ea58b9a404d4 ("cmd: allow sound command to play multiple sounds")
Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 cmd/sound.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: 4f836fb324ba500ecabdba4146c3ca9e1600cdf5
change-id: 20240610-cmd-sound-help-cfef07dcc95a

Best regards,

Comments

Simon Glass June 11, 2024, 6:52 p.m. UTC | #1
On Mon, 10 Jun 2024 at 10:11, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@cherry.de>
>
> There's never been a -q or -s argument handled in the command, so let's
> remove it. This was highlighted during review[1] but somehow still got
> through.
>
> While at it, slightly "reword" in the help text how the len + freq
> arguments are defined. Indeed, len and freq work in pair, it is possible
> to define none of either, n of both, or n - 1 of freq if there are n
> len, in which case the freq that goes with the last len would be the n -
> 1 (and not the default of 400Hz if neither len nor freq is passed). I
> assume this isn't what's expected but leaving it for another patch if
> need be to fix what happens in that very odd scenario.
>
> [1] https://lore.kernel.org/u-boot/CAPnjgZ0QWNqVFZfEWHxRcFOA3E3gRAZCYs77nGUXKL0pLp+JLQ@mail.gmail.com/
>
> Fixes: ea58b9a404d4 ("cmd: allow sound command to play multiple sounds")
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  cmd/sound.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tom Rini June 13, 2024, 3:33 p.m. UTC | #2
On Mon, 10 Jun 2024 18:11:44 +0200, Quentin Schulz wrote:

> There's never been a -q or -s argument handled in the command, so let's
> remove it. This was highlighted during review[1] but somehow still got
> through.
> 
> While at it, slightly "reword" in the help text how the len + freq
> arguments are defined. Indeed, len and freq work in pair, it is possible
> to define none of either, n of both, or n - 1 of freq if there are n
> len, in which case the freq that goes with the last len would be the n -
> 1 (and not the default of 400Hz if neither len nor freq is passed). I
> assume this isn't what's expected but leaving it for another patch if
> need be to fix what happens in that very odd scenario.
> 
> [...]

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/cmd/sound.c b/cmd/sound.c
index 08bf74112f1..8f67cbd96e1 100644
--- a/cmd/sound.c
+++ b/cmd/sound.c
@@ -98,7 +98,7 @@  U_BOOT_CMD(
 	sound, INT_MAX, 1, do_sound,
 	"sound sub-system",
 	"init - initialise the sound driver\n"
-	"sound play [[[-q|-s] len [freq]] ...] - play sounds\n"
+	"sound play [len [freq [len [freq ...]]]] - play sounds\n"
 	"  len - duration in ms\n"
 	"  freq - frequency in Hz\n"
 );