| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250513001334.r4bomm5s2jbzlfoh@synopsys.com>
Date: Tue, 13 May 2025 00:13:40 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Bjorn Andersson <bjorn.andersson@....qualcomm.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] usb: dwc3: qcom: Use bulk clock API and devres
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@....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@....qualcomm.com>
>
Acked-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Thanks,
Thinh
Powered by blists - more mailing lists