[PATCH 1/4] cmd: clk: test the number of argument in setfreq command

[PATCH 1/4] cmd: clk: test the number of argument in setfreq command

[Patrick Delaunay]

Test the number of argument in setfreq command to avoid a crash when
the command setfreq is called without argument:

  STM32MP> clk setfreq
  data abort
  pc : [<ddba3f18>]	   lr : [<ddba3f89>]
  reloc pc : [<c018ff18>]	   lr : [<c018ff89>]
  sp : dbaf45b8  ip : ddb1d859	 fp : 00000002
  r10: dbb3fd80  r9 : dbb11e90	 r8 : ddbf38cc
  r7 : ddb39725  r6 : 00000000	 r5 : 00000000  r4 : dbb3fd84
  r3 : dbb3fd84  r2 : 0000000a	 r1 : dbaf45bc  r0 : 00000011
  Flags: nzCv  IRQs off  FIQs off  Mode SVC_32 (T)
  Code: 4dd3 1062 85a3 ddbd (7803) 2b30
  Resetting CPU ...

Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline")
Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---

 cmd/clk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/cmd/clk.c b/cmd/clk.c
index dbbdc31b35..52237791cf 100644
--- a/cmd/clk.c
+++ b/cmd/clk.c
@@ -120,6 +120,9 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
 	s32 freq;
 	struct udevice *dev;
 
+	if (argc != 3)
+		return CMD_RET_USAGE;
+
 	freq = dectoul(argv[2], NULL);
 
 	dev = clk_lookup(argv[1]);
-- 
2.25.1

[PATCH 2/4] cmd: clk: replace clk_lookup by uclass_get_device_by_name

[Patrick Delaunay]

The function clk_lookup can be replaced by a direct call
to uclass_get_device_by_name for UCLASS_CLK.

This patch removes duplicated codes by the generic DM API and avoids
issue in clk_lookup because result of uclass_get_device wasn't tested;
when ret < 0, dev = NULL and dev->name is invalid, the next function
call strcmp(name, dev->name) causes a crash.

Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---

 cmd/clk.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/cmd/clk.c b/cmd/clk.c
index 52237791cf..d615f14a84 100644
--- a/cmd/clk.c
+++ b/cmd/clk.c
@@ -99,20 +99,6 @@ static int do_clk_dump(struct cmd_tbl *cmdtp, int flag, int argc,
 }
 
 #if CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(CLK)
-struct udevice *clk_lookup(const char *name)
-{
-	int i = 0;
-	struct udevice *dev;
-
-	do {
-		uclass_get_device(UCLASS_CLK, i++, &dev);
-		if (!strcmp(name, dev->name))
-			return dev;
-	} while (dev);
-
-	return NULL;
-}
-
 static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
 			  char *const argv[])
 {
@@ -125,9 +111,7 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	freq = dectoul(argv[2], NULL);
 
-	dev = clk_lookup(argv[1]);
-
-	if (dev)
+	if (!uclass_get_device_by_name(UCLASS_CLK, argv[1], &dev))
 		clk = dev_get_clk_ptr(dev);
 
 	if (!clk) {
-- 
2.25.1

[PATCH 3/4] cmd: clk: update result of do_clk_setfreq

[Patrick Delaunay]

Update the result of do_clk_setfreq and always returns a CMD_RET_ value
(-EINVAL was a possible result).

This patch avoid the CLI output "exit not allowed from main input shell."

Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline")
Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---

 cmd/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/clk.c b/cmd/clk.c
index d615f14a84..73dea5e746 100644
--- a/cmd/clk.c
+++ b/cmd/clk.c
@@ -116,7 +116,7 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	if (!clk) {
 		printf("clock '%s' not found.\n", argv[1]);
-		return -EINVAL;
+		return CMD_RET_FAILURE;
 	}
 
 	freq = clk_set_rate(clk, freq);
-- 
2.25.1

[PATCH 4/4] cmd: clk: fix long help message

[Patrick Delaunay]

Fix the long help message for "clk setfreq" command

Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline")
Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
---

 cmd/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/clk.c b/cmd/clk.c
index 73dea5e746..a483fd8981 100644
--- a/cmd/clk.c
+++ b/cmd/clk.c
@@ -160,7 +160,7 @@ static int do_clk(struct cmd_tbl *cmdtp, int flag, int argc,
 #ifdef CONFIG_SYS_LONGHELP
 static char clk_help_text[] =
 	"dump - Print clock frequencies\n"
-	"setfreq [clk] [freq] - Set clock frequency";
+	"clk setfreq [clk] [freq] - Set clock frequency";
 #endif
 
 U_BOOT_CMD(clk, 4, 1, do_clk, "CLK sub-system", clk_help_text);
-- 
2.25.1

Re: [PATCH 1/4] cmd: clk: test the number of argument in setfreq command

[Sean Anderson]

On 1/31/22 11:21 AM, Patrick Delaunay wrote:
> Test the number of argument in setfreq command to avoid a crash when
> the command setfreq is called without argument:
> 
>    STM32MP> clk setfreq
>    data abort
>    pc : [<ddba3f18>]	   lr : [<ddba3f89>]
>    reloc pc : [<c018ff18>]	   lr : [<c018ff89>]
>    sp : dbaf45b8  ip : ddb1d859	 fp : 00000002
>    r10: dbb3fd80  r9 : dbb11e90	 r8 : ddbf38cc
>    r7 : ddb39725  r6 : 00000000	 r5 : 00000000  r4 : dbb3fd84
>    r3 : dbb3fd84  r2 : 0000000a	 r1 : dbaf45bc  r0 : 00000011
>    Flags: nzCv  IRQs off  FIQs off  Mode SVC_32 (T)
>    Code: 4dd3 1062 85a3 ddbd (7803) 2b30
>    Resetting CPU ...
> 
> Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline")
> Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
> 
>   cmd/clk.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/cmd/clk.c b/cmd/clk.c
> index dbbdc31b35..52237791cf 100644
> --- a/cmd/clk.c
> +++ b/cmd/clk.c
> @@ -120,6 +120,9 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
>   	s32 freq;
>   	struct udevice *dev;
>   
> +	if (argc != 3)
> +		return CMD_RET_USAGE;
> +
>   	freq = dectoul(argv[2], NULL);
>   
>   	dev = clk_lookup(argv[1]);
> 

Reviewed-by: Sean Anderson <seanga2@gmail.com>

Re: [PATCH 2/4] cmd: clk: replace clk_lookup by uclass_get_device_by_name

[Sean Anderson]

On 1/31/22 11:21 AM, Patrick Delaunay wrote:
> The function clk_lookup can be replaced by a direct call
> to uclass_get_device_by_name for UCLASS_CLK.
> 
> This patch removes duplicated codes by the generic DM API and avoids
> issue in clk_lookup because result of uclass_get_device wasn't tested;
> when ret < 0, dev = NULL and dev->name is invalid, the next function
> call strcmp(name, dev->name) causes a crash.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
> 
>   cmd/clk.c | 18 +-----------------
>   1 file changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/cmd/clk.c b/cmd/clk.c
> index 52237791cf..d615f14a84 100644
> --- a/cmd/clk.c
> +++ b/cmd/clk.c
> @@ -99,20 +99,6 @@ static int do_clk_dump(struct cmd_tbl *cmdtp, int flag, int argc,
>   }
>   
>   #if CONFIG_IS_ENABLED(DM) && CONFIG_IS_ENABLED(CLK)
> -struct udevice *clk_lookup(const char *name)
> -{
> -	int i = 0;
> -	struct udevice *dev;
> -
> -	do {
> -		uclass_get_device(UCLASS_CLK, i++, &dev);
> -		if (!strcmp(name, dev->name))
> -			return dev;
> -	} while (dev);
> -
> -	return NULL;
> -}
> -
>   static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
>   			  char *const argv[])
>   {
> @@ -125,9 +111,7 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
>   
>   	freq = dectoul(argv[2], NULL);
>   
> -	dev = clk_lookup(argv[1]);
> -
> -	if (dev)
> +	if (!uclass_get_device_by_name(UCLASS_CLK, argv[1], &dev))
>   		clk = dev_get_clk_ptr(dev);
>   
>   	if (!clk) {
> 

Reviewed-by: Sean Anderson

(This is such a strange command, since it can only handle CCF clocks)

Re: [PATCH 3/4] cmd: clk: update result of do_clk_setfreq

[Sean Anderson]

On 1/31/22 11:21 AM, Patrick Delaunay wrote:
> Update the result of do_clk_setfreq and always returns a CMD_RET_ value
> (-EINVAL was a possible result).
> 
> This patch avoid the CLI output "exit not allowed from main input shell."
> 
> Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline")
> Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
> 
>   cmd/clk.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmd/clk.c b/cmd/clk.c
> index d615f14a84..73dea5e746 100644
> --- a/cmd/clk.c
> +++ b/cmd/clk.c
> @@ -116,7 +116,7 @@ static int do_clk_setfreq(struct cmd_tbl *cmdtp, int flag, int argc,
>   
>   	if (!clk) {
>   		printf("clock '%s' not found.\n", argv[1]);
> -		return -EINVAL;
> +		return CMD_RET_FAILURE;
>   	}
>   
>   	freq = clk_set_rate(clk, freq);
> 

Reviewed-by: Sean Anderson <seanga2@gmail.com>

Re: [PATCH 4/4] cmd: clk: fix long help message

[Sean Anderson]

On 1/31/22 11:21 AM, Patrick Delaunay wrote:
> Fix the long help message for "clk setfreq" command
> 
> Fixes: 7ab418fbe612 ("clk: add support for setting clk rate from cmdline")
> Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
> ---
> 
>   cmd/clk.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmd/clk.c b/cmd/clk.c
> index 73dea5e746..a483fd8981 100644
> --- a/cmd/clk.c
> +++ b/cmd/clk.c
> @@ -160,7 +160,7 @@ static int do_clk(struct cmd_tbl *cmdtp, int flag, int argc,
>   #ifdef CONFIG_SYS_LONGHELP
>   static char clk_help_text[] =
>   	"dump - Print clock frequencies\n"
> -	"setfreq [clk] [freq] - Set clock frequency";
> +	"clk setfreq [clk] [freq] - Set clock frequency";
>   #endif
>   
>   U_BOOT_CMD(clk, 4, 1, do_clk, "CLK sub-system", clk_help_text);
> 

Reviewed-by: Sean Anderson <seanga2@gmail.com>

Re: [PATCH 1/4] cmd: clk: test the number of argument in setfreq command

[Sean Anderson]

On Mon, 31 Jan 2022 17:21:37 +0100, Patrick Delaunay wrote:
> Test the number of argument in setfreq command to avoid a crash when
> the command setfreq is called without argument:
> 
>   STM32MP> clk setfreq
>   data abort
>   pc : [<ddba3f18>]	   lr : [<ddba3f89>]
>   reloc pc : [<c018ff18>]	   lr : [<c018ff89>]
>   sp : dbaf45b8  ip : ddb1d859	 fp : 00000002
>   r10: dbb3fd80  r9 : dbb11e90	 r8 : ddbf38cc
>   r7 : ddb39725  r6 : 00000000	 r5 : 00000000  r4 : dbb3fd84
>   r3 : dbb3fd84  r2 : 0000000a	 r1 : dbaf45bc  r0 : 00000011
>   Flags: nzCv  IRQs off  FIQs off  Mode SVC_32 (T)
>   Code: 4dd3 1062 85a3 ddbd (7803) 2b30
>   Resetting CPU ...
> 
> [...]

Applied, thanks!

[1/4] cmd: clk: test the number of argument in setfreq command
      commit: 3386fb1e485da7f206488ed206a61ae811885d90
[2/4] cmd: clk: replace clk_lookup by uclass_get_device_by_name
      commit: afcc26140bc6bff7c23ce02dbba7882c97d2c14a
[3/4] cmd: clk: update result of do_clk_setfreq
      commit: 534859ac6b2fecb631457736e77e9d0df1e57616
[4/4] cmd: clk: fix long help message
      commit: 92a1bba85761e4dd5c0647c48048423bceef4749

Best regards,
-- 
Sean Anderson <seanga2@gmail.com>