[PATCH v2] usb: dwc3: qcom: Use bulk clock API and devres

[PATCH v2] usb: dwc3: qcom: Use bulk clock API and devres

[Bjorn Andersson]

The Qualcomm DWC3 glue driver duplicates the logic of the bulk clock
API to acquire, prepare, and unprepare the controller's clocks. It also
manages the life cycle of these handled explicitly.

Transition to the bulk clock API and manage the resources using devres,
to clean up the code. The resource acquisition is moved above the
initial reset pulse, to handle resource issues before the state is
touched - other than this, this no functional change.

Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
---
Changes in v2:
- Actually call clk_bulk_prepare_enable()...
- Moved the resource acquisition above the initial reset pulse (but
  keeping the prepare in it's current place)
- Link to v1: https://lore.kernel.org/r/20250422-dwc3-clk-bulk-v1-1-37c7c941330f@oss.qualcomm.com
---
 drivers/usb/dwc3/dwc3-qcom.c | 90 ++++++++------------------------------------
 1 file changed, 15 insertions(+), 75 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 20c00ba3bc3d29dfe1e11e38dedea0c94aaa6a81..7334de85ad10c7f680a794bd7818f1802b130440 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -73,7 +73,7 @@ struct dwc3_qcom {
 	struct device		*dev;
 	void __iomem		*qscratch_base;
 	struct dwc3		dwc;
-	struct clk		**clks;
+	struct clk_bulk_data	*clks;
 	int			num_clocks;
 	struct reset_control	*resets;
 	struct dwc3_qcom_port	ports[DWC3_QCOM_MAX_PORTS];
@@ -431,9 +431,7 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
 		if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
 			dev_err(qcom->dev, "port-%d HS-PHY not in L2\n", i + 1);
 	}
-
-	for (i = qcom->num_clocks - 1; i >= 0; i--)
-		clk_disable_unprepare(qcom->clks[i]);
+	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
 
 	ret = dwc3_qcom_interconnect_disable(qcom);
 	if (ret)
@@ -465,14 +463,9 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
 	if (dwc3_qcom_is_host(qcom) && wakeup)
 		dwc3_qcom_disable_interrupts(qcom);
 
-	for (i = 0; i < qcom->num_clocks; i++) {
-		ret = clk_prepare_enable(qcom->clks[i]);
-		if (ret < 0) {
-			while (--i >= 0)
-				clk_disable_unprepare(qcom->clks[i]);
-			return ret;
-		}
-	}
+	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
+	if (ret < 0)
+		return ret;
 
 	ret = dwc3_qcom_interconnect_enable(qcom);
 	if (ret)
@@ -648,62 +641,14 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
 	return 0;
 }
 
-static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
-{
-	struct device		*dev = qcom->dev;
-	struct device_node	*np = dev->of_node;
-	int			i;
-
-	if (!np || !count)
-		return 0;
-
-	if (count < 0)
-		return count;
-
-	qcom->num_clocks = count;
-
-	qcom->clks = devm_kcalloc(dev, qcom->num_clocks,
-				  sizeof(struct clk *), GFP_KERNEL);
-	if (!qcom->clks)
-		return -ENOMEM;
-
-	for (i = 0; i < qcom->num_clocks; i++) {
-		struct clk	*clk;
-		int		ret;
-
-		clk = of_clk_get(np, i);
-		if (IS_ERR(clk)) {
-			while (--i >= 0)
-				clk_put(qcom->clks[i]);
-			return PTR_ERR(clk);
-		}
-
-		ret = clk_prepare_enable(clk);
-		if (ret < 0) {
-			while (--i >= 0) {
-				clk_disable_unprepare(qcom->clks[i]);
-				clk_put(qcom->clks[i]);
-			}
-			clk_put(clk);
-
-			return ret;
-		}
-
-		qcom->clks[i] = clk;
-	}
-
-	return 0;
-}
-
 static int dwc3_qcom_probe(struct platform_device *pdev)
 {
 	struct dwc3_probe_data	probe_data = {};
-	struct device_node	*np = pdev->dev.of_node;
 	struct device		*dev = &pdev->dev;
 	struct dwc3_qcom	*qcom;
 	struct resource		res;
 	struct resource		*r;
-	int			ret, i;
+	int			ret;
 	bool			ignore_pipe_clk;
 	bool			wakeup_source;
 
@@ -719,6 +664,11 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 				     "failed to get resets\n");
 	}
 
+	ret = devm_clk_bulk_get_all(&pdev->dev, &qcom->clks);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "failed to get clocks\n");
+	qcom->num_clocks = ret;
+
 	ret = reset_control_assert(qcom->resets);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to assert resets, err=%d\n", ret);
@@ -733,11 +683,9 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 		goto reset_assert;
 	}
 
-	ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
-	if (ret) {
-		dev_err_probe(dev, ret, "failed to get clocks\n");
+	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
+	if (ret < 0)
 		goto reset_assert;
-	}
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!r) {
@@ -806,10 +754,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 remove_core:
 	dwc3_core_remove(&qcom->dwc);
 clk_disable:
-	for (i = qcom->num_clocks - 1; i >= 0; i--) {
-		clk_disable_unprepare(qcom->clks[i]);
-		clk_put(qcom->clks[i]);
-	}
+	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
 reset_assert:
 	reset_control_assert(qcom->resets);
 
@@ -820,15 +765,10 @@ static void dwc3_qcom_remove(struct platform_device *pdev)
 {
 	struct dwc3 *dwc = platform_get_drvdata(pdev);
 	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
-	int i;
 
 	dwc3_core_remove(&qcom->dwc);
 
-	for (i = qcom->num_clocks - 1; i >= 0; i--) {
-		clk_disable_unprepare(qcom->clks[i]);
-		clk_put(qcom->clks[i]);
-	}
-	qcom->num_clocks = 0;
+	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
 
 	dwc3_qcom_interconnect_exit(qcom);
 	reset_control_assert(qcom->resets);

---
base-commit: f48887a98b78880b7711aca311fbbbcaad6c4e3b
change-id: 20250422-dwc3-clk-bulk-960a91ed4ab8

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>

Re: [PATCH v2] usb: dwc3: qcom: Use bulk clock API and devres

[Thinh Nguyen]

On Thu, May 08, 2025, Bjorn Andersson wrote:
> The Qualcomm DWC3 glue driver duplicates the logic of the bulk clock
> API to acquire, prepare, and unprepare the controller's clocks. It also
> manages the life cycle of these handled explicitly.
> 
> Transition to the bulk clock API and manage the resources using devres,
> to clean up the code. The resource acquisition is moved above the
> initial reset pulse, to handle resource issues before the state is
> touched - other than this, this no functional change.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> ---
> Changes in v2:
> - Actually call clk_bulk_prepare_enable()...
> - Moved the resource acquisition above the initial reset pulse (but
>   keeping the prepare in it's current place)
> - Link to v1: https://urldefense.com/v3/__https://lore.kernel.org/r/20250422-dwc3-clk-bulk-v1-1-37c7c941330f@oss.qualcomm.com__;!!A4F2R9G_pg!YdtVrttLCllV2yEx69CQRP5hENylo9Wn9GbB2byRmItCl-n3loIJ87094hdb8T2lxvCAnk6w_LUlOW8gVJ-1PGVcNVwmOSZ2CSWt$ 
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 90 ++++++++------------------------------------
>  1 file changed, 15 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 20c00ba3bc3d29dfe1e11e38dedea0c94aaa6a81..7334de85ad10c7f680a794bd7818f1802b130440 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -73,7 +73,7 @@ struct dwc3_qcom {
>  	struct device		*dev;
>  	void __iomem		*qscratch_base;
>  	struct dwc3		dwc;
> -	struct clk		**clks;
> +	struct clk_bulk_data	*clks;
>  	int			num_clocks;
>  	struct reset_control	*resets;
>  	struct dwc3_qcom_port	ports[DWC3_QCOM_MAX_PORTS];
> @@ -431,9 +431,7 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
>  		if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
>  			dev_err(qcom->dev, "port-%d HS-PHY not in L2\n", i + 1);
>  	}
> -
> -	for (i = qcom->num_clocks - 1; i >= 0; i--)
> -		clk_disable_unprepare(qcom->clks[i]);
> +	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>  
>  	ret = dwc3_qcom_interconnect_disable(qcom);
>  	if (ret)
> @@ -465,14 +463,9 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
>  	if (dwc3_qcom_is_host(qcom) && wakeup)
>  		dwc3_qcom_disable_interrupts(qcom);
>  
> -	for (i = 0; i < qcom->num_clocks; i++) {
> -		ret = clk_prepare_enable(qcom->clks[i]);
> -		if (ret < 0) {
> -			while (--i >= 0)
> -				clk_disable_unprepare(qcom->clks[i]);
> -			return ret;
> -		}
> -	}
> +	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
> +	if (ret < 0)
> +		return ret;
>  
>  	ret = dwc3_qcom_interconnect_enable(qcom);
>  	if (ret)
> @@ -648,62 +641,14 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
>  	return 0;
>  }
>  
> -static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
> -{
> -	struct device		*dev = qcom->dev;
> -	struct device_node	*np = dev->of_node;
> -	int			i;
> -
> -	if (!np || !count)
> -		return 0;
> -
> -	if (count < 0)
> -		return count;
> -
> -	qcom->num_clocks = count;
> -
> -	qcom->clks = devm_kcalloc(dev, qcom->num_clocks,
> -				  sizeof(struct clk *), GFP_KERNEL);
> -	if (!qcom->clks)
> -		return -ENOMEM;
> -
> -	for (i = 0; i < qcom->num_clocks; i++) {
> -		struct clk	*clk;
> -		int		ret;
> -
> -		clk = of_clk_get(np, i);
> -		if (IS_ERR(clk)) {
> -			while (--i >= 0)
> -				clk_put(qcom->clks[i]);
> -			return PTR_ERR(clk);
> -		}
> -
> -		ret = clk_prepare_enable(clk);
> -		if (ret < 0) {
> -			while (--i >= 0) {
> -				clk_disable_unprepare(qcom->clks[i]);
> -				clk_put(qcom->clks[i]);
> -			}
> -			clk_put(clk);
> -
> -			return ret;
> -		}
> -
> -		qcom->clks[i] = clk;
> -	}
> -
> -	return 0;
> -}
> -
>  static int dwc3_qcom_probe(struct platform_device *pdev)
>  {
>  	struct dwc3_probe_data	probe_data = {};
> -	struct device_node	*np = pdev->dev.of_node;
>  	struct device		*dev = &pdev->dev;
>  	struct dwc3_qcom	*qcom;
>  	struct resource		res;
>  	struct resource		*r;
> -	int			ret, i;
> +	int			ret;
>  	bool			ignore_pipe_clk;
>  	bool			wakeup_source;
>  
> @@ -719,6 +664,11 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  				     "failed to get resets\n");
>  	}
>  
> +	ret = devm_clk_bulk_get_all(&pdev->dev, &qcom->clks);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret, "failed to get clocks\n");
> +	qcom->num_clocks = ret;
> +
>  	ret = reset_control_assert(qcom->resets);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to assert resets, err=%d\n", ret);
> @@ -733,11 +683,9 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  		goto reset_assert;
>  	}
>  
> -	ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
> -	if (ret) {
> -		dev_err_probe(dev, ret, "failed to get clocks\n");
> +	ret = clk_bulk_prepare_enable(qcom->num_clocks, qcom->clks);
> +	if (ret < 0)
>  		goto reset_assert;
> -	}
>  
>  	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (!r) {
> @@ -806,10 +754,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  remove_core:
>  	dwc3_core_remove(&qcom->dwc);
>  clk_disable:
> -	for (i = qcom->num_clocks - 1; i >= 0; i--) {
> -		clk_disable_unprepare(qcom->clks[i]);
> -		clk_put(qcom->clks[i]);
> -	}
> +	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>  reset_assert:
>  	reset_control_assert(qcom->resets);
>  
> @@ -820,15 +765,10 @@ static void dwc3_qcom_remove(struct platform_device *pdev)
>  {
>  	struct dwc3 *dwc = platform_get_drvdata(pdev);
>  	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
> -	int i;
>  
>  	dwc3_core_remove(&qcom->dwc);
>  
> -	for (i = qcom->num_clocks - 1; i >= 0; i--) {
> -		clk_disable_unprepare(qcom->clks[i]);
> -		clk_put(qcom->clks[i]);
> -	}
> -	qcom->num_clocks = 0;
> +	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>  
>  	dwc3_qcom_interconnect_exit(qcom);
>  	reset_control_assert(qcom->resets);
> 
> ---
> base-commit: f48887a98b78880b7711aca311fbbbcaad6c4e3b
> change-id: 20250422-dwc3-clk-bulk-960a91ed4ab8
> 
> Best regards,
> -- 
> Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh