diff mbox series

clk: at91: remove unnecessary conditions

Message ID 7782b4f1-deed-49dc-8207-b6ea06d7602f@moroto.mountain (mailing list archive)
State Accepted, archived
Headers show
Series clk: at91: remove unnecessary conditions | expand

Commit Message

Dan Carpenter Oct. 17, 2023, 2:06 p.m. UTC
This code checks "if (parent_hw)" is non-NULL, but then it has more
checks if parent_hw is non-NULL on the lines inside the if statement.
It is a bit confusing.

For the else statement, keep in mind that at the start of the function
we checked:

	if (!(parent_name || parent_hw))
		return ERR_PTR(-EINVAL);

That check ensures that if parent_hw is NULL that means that parent_name
is non-NULL.  At least one must always be non-NULL.  So here again, the
checks inside the if statement can be removed.

In the original code, it was a bit confusing and you could easily get
the impression that "init.num_parents" could be zero.  When we remove
the unnecessary checking it's more obvious that it's always set to 1.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/clk/at91/clk-utmi.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

Comments

Claudiu Oct. 18, 2023, 6:26 a.m. UTC | #1
On 17.10.2023 17:06, Dan Carpenter wrote:
> This code checks "if (parent_hw)" is non-NULL, but then it has more
> checks if parent_hw is non-NULL on the lines inside the if statement.
> It is a bit confusing.
> 
> For the else statement, keep in mind that at the start of the function
> we checked:
> 
> 	if (!(parent_name || parent_hw))
> 		return ERR_PTR(-EINVAL);
> 
> That check ensures that if parent_hw is NULL that means that parent_name
> is non-NULL.  At least one must always be non-NULL.  So here again, the
> checks inside the if statement can be removed.
> 
> In the original code, it was a bit confusing and you could easily get
> the impression that "init.num_parents" could be zero.  When we remove
> the unnecessary checking it's more obvious that it's always set to 1.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

I had something similar in my queue to submit, but didn't manage to do it
yet. Thanks for taking care of it.

Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>

> ---
>  drivers/clk/at91/clk-utmi.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/at91/clk-utmi.c b/drivers/clk/at91/clk-utmi.c
> index 40c84f5af5e8..b991180beea1 100644
> --- a/drivers/clk/at91/clk-utmi.c
> +++ b/drivers/clk/at91/clk-utmi.c
> @@ -161,13 +161,11 @@ at91_clk_register_utmi_internal(struct regmap *regmap_pmc,
>  
>  	init.name = name;
>  	init.ops = ops;
> -	if (parent_hw) {
> -		init.parent_hws = parent_hw ? (const struct clk_hw **)&parent_hw : NULL;
> -		init.num_parents = parent_hw ? 1 : 0;
> -	} else {
> -		init.parent_names = parent_name ? &parent_name : NULL;
> -		init.num_parents = parent_name ? 1 : 0;
> -	}
> +	if (parent_hw)
> +		init.parent_hws = (const struct clk_hw **)&parent_hw;
> +	else
> +		init.parent_names = &parent_name;
> +	init.num_parents = 1;
>  	init.flags = flags;
>  
>  	utmi->hw.init = &init;
Stephen Boyd Oct. 19, 2023, 12:56 a.m. UTC | #2
Quoting Dan Carpenter (2023-10-17 07:06:53)
> This code checks "if (parent_hw)" is non-NULL, but then it has more
> checks if parent_hw is non-NULL on the lines inside the if statement.
> It is a bit confusing.
> 
> For the else statement, keep in mind that at the start of the function
> we checked:
> 
>         if (!(parent_name || parent_hw))
>                 return ERR_PTR(-EINVAL);
> 
> That check ensures that if parent_hw is NULL that means that parent_name
> is non-NULL.  At least one must always be non-NULL.  So here again, the
> checks inside the if statement can be removed.
> 
> In the original code, it was a bit confusing and you could easily get
> the impression that "init.num_parents" could be zero.  When we remove
> the unnecessary checking it's more obvious that it's always set to 1.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---

Applied to clk-next
diff mbox series

Patch

diff --git a/drivers/clk/at91/clk-utmi.c b/drivers/clk/at91/clk-utmi.c
index 40c84f5af5e8..b991180beea1 100644
--- a/drivers/clk/at91/clk-utmi.c
+++ b/drivers/clk/at91/clk-utmi.c
@@ -161,13 +161,11 @@  at91_clk_register_utmi_internal(struct regmap *regmap_pmc,
 
 	init.name = name;
 	init.ops = ops;
-	if (parent_hw) {
-		init.parent_hws = parent_hw ? (const struct clk_hw **)&parent_hw : NULL;
-		init.num_parents = parent_hw ? 1 : 0;
-	} else {
-		init.parent_names = parent_name ? &parent_name : NULL;
-		init.num_parents = parent_name ? 1 : 0;
-	}
+	if (parent_hw)
+		init.parent_hws = (const struct clk_hw **)&parent_hw;
+	else
+		init.parent_names = &parent_name;
+	init.num_parents = 1;
 	init.flags = flags;
 
 	utmi->hw.init = &init;