diff mbox series

[v3,03/12] clk: Unconditionally recursively en-/dis-able clocks

Message ID 33db3690-e3ce-f25b-80c5-741d5e1d4773@gmail.com
State Superseded
Delegated to: Andes
Headers show
Series riscv: Add Sipeed Maix support | expand

Commit Message

Sean Anderson Feb. 2, 2020, 7:59 p.m. UTC 
For clocks not in the CCF, their parents will not have UCLASS_CLK, so we just
enable them as normal. The enable count is local to the struct clk, but this
will never result in the actual en-/dis-able op being called (unless the same
struct clk is enabled twice).

For clocks in the CCF, we always traverse up the tree when enabling. Previously,
CCF clocks without id set would be skipped, stopping the traversal too early.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---
  Changes for v3:
  - New.

 drivers/clk/clk-uclass.c | 58 +++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 33 deletions(-)

Comments

Lukasz Majewski Feb. 6, 2020, 9:45 p.m. UTC | #1 
Hi Sean,

> For clocks not in the CCF, their parents will not have UCLASS_CLK, so
> we just enable them as normal. The enable count is local to the
> struct clk, but this will never result in the actual en-/dis-able op
> being called (unless the same struct clk is enabled twice).
> 
> For clocks in the CCF, we always traverse up the tree when enabling.
> Previously, CCF clocks without id set would be skipped, stopping the
> traversal too early.
> 
> Signed-off-by: Sean Anderson <seanga2@gmail.com>

Acked-by: Lukasz Majewski <lukma@denx.de>

> ---
>   Changes for v3:
>   - New.
> 
>  drivers/clk/clk-uclass.c | 58
> +++++++++++++++++----------------------- 1 file changed, 25
> insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> index 9aa8537004..87d101aab4 100644
> --- a/drivers/clk/clk-uclass.c
> +++ b/drivers/clk/clk-uclass.c
> @@ -490,7 +490,6 @@ int clk_set_parent(struct clk *clk, struct clk
> *parent) int clk_enable(struct clk *clk)
>  {
>  	const struct clk_ops *ops;
> -	struct clk *clkp = NULL;
>  	int ret;
>  
>  	debug("%s(clk=%p)\n", __func__, clk);
> @@ -499,32 +498,28 @@ int clk_enable(struct clk *clk)
>  	ops = clk_dev_ops(clk->dev);
>  
>  	if (CONFIG_IS_ENABLED(CLK_CCF)) {
> -		/* Take id 0 as a non-valid clk, such as dummy */
> -		if (clk->id && !clk_get_by_id(clk->id, &clkp)) {
> -			if (clkp->enable_count) {
> -				clkp->enable_count++;
> -				return 0;
> -			}
> -			if (clkp->dev->parent &&
> -			    device_get_uclass_id(clkp->dev) ==
> UCLASS_CLK) {
> -				ret =
> clk_enable(dev_get_clk_ptr(clkp->dev->parent));
> -				if (ret) {
> -					printf("Enable %s failed\n",
> -
> clkp->dev->parent->name);
> -					return ret;
> -				}
> +		if (clk->enable_count) {
> +			clk->enable_count++;
> +			return 0;
> +		}
> +		if (clk->dev->parent &&
> +		    device_get_uclass_id(clk->dev->parent) ==
> UCLASS_CLK) {
> +			ret =
> clk_enable(dev_get_clk_ptr(clk->dev->parent));
> +			if (ret) {
> +				printf("Enable %s failed\n",
> +				       clk->dev->parent->name);
> +				return ret;
>  			}
>  		}
>  
>  		if (ops->enable) {
>  			ret = ops->enable(clk);
>  			if (ret) {
> -				printf("Enable %s failed\n",
> clk->dev->name);
> +				printf("Enable %s failed (error
> %d)\n", clk->dev->name, ret); return ret;
>  			}
>  		}
> -		if (clkp)
> -			clkp->enable_count++;
> +		clk->enable_count++;
>  	} else {
>  		if (!ops->enable)
>  			return -ENOSYS;
> @@ -550,7 +545,6 @@ int clk_enable_bulk(struct clk_bulk *bulk)
>  int clk_disable(struct clk *clk)
>  {
>  	const struct clk_ops *ops;
> -	struct clk *clkp = NULL;
>  	int ret;
>  
>  	debug("%s(clk=%p)\n", __func__, clk);
> @@ -559,29 +553,27 @@ int clk_disable(struct clk *clk)
>  	ops = clk_dev_ops(clk->dev);
>  
>  	if (CONFIG_IS_ENABLED(CLK_CCF)) {
> -		if (clk->id && !clk_get_by_id(clk->id, &clkp)) {
> -			if (clkp->enable_count == 0) {
> -				printf("clk %s already disabled\n",
> -				       clkp->dev->name);
> -				return 0;
> -			}
> -
> -			if (--clkp->enable_count > 0)
> -				return 0;
> +		if (clk->enable_count == 0) {
> +			printf("clk %s already disabled\n",
> +			       clk->dev->name);
> +			return 0;
>  		}
>  
> +		if (--clk->enable_count > 0)
> +			return 0;
> +
>  		if (ops->disable) {
>  			ret = ops->disable(clk);
>  			if (ret)
>  				return ret;
>  		}
>  
> -		if (clkp && clkp->dev->parent &&
> -		    device_get_uclass_id(clkp->dev) == UCLASS_CLK) {
> -			ret =
> clk_disable(dev_get_clk_ptr(clkp->dev->parent));
> +		if (clk->dev->parent &&
> +		    device_get_uclass_id(clk->dev) == UCLASS_CLK) {
> +			ret =
> clk_disable(dev_get_clk_ptr(clk->dev->parent)); if (ret) {
> -				printf("Disable %s failed\n",
> -				       clkp->dev->parent->name);
> +				printf("Disable %s failed (error
> %d)\n",
> +				       clk->dev->parent->name, ret);
>  				return ret;
>  			}
>  		}




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff mbox series

Patch

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 9aa8537004..87d101aab4 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -490,7 +490,6 @@  int clk_set_parent(struct clk *clk, struct clk *parent)
 int clk_enable(struct clk *clk)
 {
 	const struct clk_ops *ops;
-	struct clk *clkp = NULL;
 	int ret;
 
 	debug("%s(clk=%p)\n", __func__, clk);
@@ -499,32 +498,28 @@  int clk_enable(struct clk *clk)
 	ops = clk_dev_ops(clk->dev);
 
 	if (CONFIG_IS_ENABLED(CLK_CCF)) {
-		/* Take id 0 as a non-valid clk, such as dummy */
-		if (clk->id && !clk_get_by_id(clk->id, &clkp)) {
-			if (clkp->enable_count) {
-				clkp->enable_count++;
-				return 0;
-			}
-			if (clkp->dev->parent &&
-			    device_get_uclass_id(clkp->dev) == UCLASS_CLK) {
-				ret = clk_enable(dev_get_clk_ptr(clkp->dev->parent));
-				if (ret) {
-					printf("Enable %s failed\n",
-					       clkp->dev->parent->name);
-					return ret;
-				}
+		if (clk->enable_count) {
+			clk->enable_count++;
+			return 0;
+		}
+		if (clk->dev->parent &&
+		    device_get_uclass_id(clk->dev->parent) == UCLASS_CLK) {
+			ret = clk_enable(dev_get_clk_ptr(clk->dev->parent));
+			if (ret) {
+				printf("Enable %s failed\n",
+				       clk->dev->parent->name);
+				return ret;
 			}
 		}
 
 		if (ops->enable) {
 			ret = ops->enable(clk);
 			if (ret) {
-				printf("Enable %s failed\n", clk->dev->name);
+				printf("Enable %s failed (error %d)\n", clk->dev->name, ret);
 				return ret;
 			}
 		}
-		if (clkp)
-			clkp->enable_count++;
+		clk->enable_count++;
 	} else {
 		if (!ops->enable)
 			return -ENOSYS;
@@ -550,7 +545,6 @@  int clk_enable_bulk(struct clk_bulk *bulk)
 int clk_disable(struct clk *clk)
 {
 	const struct clk_ops *ops;
-	struct clk *clkp = NULL;
 	int ret;
 
 	debug("%s(clk=%p)\n", __func__, clk);
@@ -559,29 +553,27 @@  int clk_disable(struct clk *clk)
 	ops = clk_dev_ops(clk->dev);
 
 	if (CONFIG_IS_ENABLED(CLK_CCF)) {
-		if (clk->id && !clk_get_by_id(clk->id, &clkp)) {
-			if (clkp->enable_count == 0) {
-				printf("clk %s already disabled\n",
-				       clkp->dev->name);
-				return 0;
-			}
-
-			if (--clkp->enable_count > 0)
-				return 0;
+		if (clk->enable_count == 0) {
+			printf("clk %s already disabled\n",
+			       clk->dev->name);
+			return 0;
 		}
 
+		if (--clk->enable_count > 0)
+			return 0;
+
 		if (ops->disable) {
 			ret = ops->disable(clk);
 			if (ret)
 				return ret;
 		}
 
-		if (clkp && clkp->dev->parent &&
-		    device_get_uclass_id(clkp->dev) == UCLASS_CLK) {
-			ret = clk_disable(dev_get_clk_ptr(clkp->dev->parent));
+		if (clk->dev->parent &&
+		    device_get_uclass_id(clk->dev) == UCLASS_CLK) {
+			ret = clk_disable(dev_get_clk_ptr(clk->dev->parent));
 			if (ret) {
-				printf("Disable %s failed\n",
-				       clkp->dev->parent->name);
+				printf("Disable %s failed (error %d)\n",
+				       clk->dev->parent->name, ret);
 				return ret;
 			}
 		}