[PATCH 00/11] usb: dwc3: various cleanups

[PATCH 00/11] usb: dwc3: various cleanups

[Krzysztof Kozlowski]

Hi,

The first ST patch depends on my fix:
https://lore.kernel.org/all/20240814093957.37940-2-krzysztof.kozlowski@linaro.org/

Series makes some code simplifications and cleanups.

Best regards,
Krzysztof

---
Krzysztof Kozlowski (11):
      usb: dwc3: st: use scoped device node handling to simplify error paths
      usb: dwc3: st: simplify with dev_err_probe
      usb: dwc3: st: simplify pdev->dev usage
      usb: dwc3: imx8mp: simplify with devm_clk_get_enabled
      usb: dwc3: imx8mp: simplify with dev_err_probe
      usb: dwc3: imx8mp: use scoped device node handling to simplify error paths
      usb: dwc3: qcom: use scoped device node handling to simplify error paths
      usb: dwc3: qcom: simplify with devm_platform_ioremap_resource
      usb: dwc3: rtk: use scoped device node handling to simplify error paths
      usb: dwc3: rtk: return directly and simplify with devm_platform_ioremap_resource
      usb: dwc3: xilinx: simplify with dev_err_probe

 drivers/usb/dwc3/dwc3-imx8mp.c | 66 ++++++++++++------------------------------
 drivers/usb/dwc3/dwc3-qcom.c   | 16 ++++------
 drivers/usb/dwc3/dwc3-rtk.c    | 52 ++++++++++-----------------------
 drivers/usb/dwc3/dwc3-st.c     | 38 +++++++++++-------------
 drivers/usb/dwc3/dwc3-xilinx.c |  7 ++---
 5 files changed, 58 insertions(+), 121 deletions(-)
---
base-commit: 64b429eaf21be888cc83e9013e25897d5fb03a75
change-id: 20240814-b4-cleanup-h-of-node-put-usb-93fadfc77d33

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

[PATCH 01/11] usb: dwc3: st: use scoped device node handling to simplify error paths

[Krzysztof Kozlowski]

Obtain the device node reference with scoped/cleanup.h to reduce error
handling and make the code a bit simpler.  Scoped/cleanup.h coding style
expects variable declaration with initialization, so the
of_get_compatible_child() call has to be moved earlier, before any goto
jumps happen.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

This depends on my earlier fix:
https://lore.kernel.org/all/20240814093957.37940-2-krzysztof.kozlowski@linaro.org/
---
 drivers/usb/dwc3/dwc3-st.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index c8c7cd0c1796..98f43d7082d7 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -14,6 +14,7 @@
  * Inspired by dwc3-omap.c and dwc3-exynos.c.
  */
 
+#include <linux/cleanup.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -197,7 +198,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	struct st_dwc3 *dwc3_data;
 	struct resource *res;
 	struct device *dev = &pdev->dev;
-	struct device_node *node = dev->of_node, *child;
+	struct device_node *node = dev->of_node;
 	struct platform_device *child_pdev;
 	struct regmap *regmap;
 	int ret;
@@ -227,6 +228,13 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	dev_vdbg(&pdev->dev, "glue-logic addr 0x%pK, syscfg-reg offset 0x%x\n",
 		 dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
 
+	struct device_node *child __free(device_node) = of_get_compatible_child(node,
+										"snps,dwc3");
+	if (!child) {
+		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
+		return -ENODEV;
+	}
+
 	dwc3_data->rstc_pwrdn =
 		devm_reset_control_get_exclusive(dev, "powerdown");
 	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
@@ -248,18 +256,11 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	/* Manage SoftReset */
 	reset_control_deassert(dwc3_data->rstc_rst);
 
-	child = of_get_compatible_child(node, "snps,dwc3");
-	if (!child) {
-		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
-		ret = -ENODEV;
-		goto err_node_put;
-	}
-
 	/* Allocate and initialize the core */
 	ret = of_platform_populate(node, NULL, NULL, dev);
 	if (ret) {
 		dev_err(dev, "failed to add dwc3 core\n");
-		goto err_node_put;
+		goto undo_softreset;
 	}
 
 	child_pdev = of_find_device_by_node(child);
@@ -270,7 +271,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	}
 
 	dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
-	of_node_put(child);
 	platform_device_put(child_pdev);
 
 	/*
@@ -282,8 +282,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	ret = st_dwc3_drd_init(dwc3_data);
 	if (ret) {
 		dev_err(dev, "drd initialisation failed\n");
-		of_platform_depopulate(dev);
-		goto undo_softreset;
+		goto depopulate;
 	}
 
 	/* ST glue logic init */
@@ -294,8 +293,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
 
 depopulate:
 	of_platform_depopulate(dev);
-err_node_put:
-	of_node_put(child);
 undo_softreset:
 	reset_control_assert(dwc3_data->rstc_rst);
 undo_powerdown:

-- 
2.43.0

[PATCH 02/11] usb: dwc3: st: simplify with dev_err_probe

[Krzysztof Kozlowski]

Use dev_err_probe() to make the error paths a bit simpler.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-st.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index 98f43d7082d7..7a0b1821768a 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -237,10 +237,9 @@ static int st_dwc3_probe(struct platform_device *pdev)
 
 	dwc3_data->rstc_pwrdn =
 		devm_reset_control_get_exclusive(dev, "powerdown");
-	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
-		dev_err(&pdev->dev, "could not get power controller\n");
-		return PTR_ERR(dwc3_data->rstc_pwrdn);
-	}
+	if (IS_ERR(dwc3_data->rstc_pwrdn))
+		return dev_err_probe(dev, PTR_ERR(dwc3_data->rstc_pwrdn),
+				     "could not get power controller\n");
 
 	/* Manage PowerDown */
 	reset_control_deassert(dwc3_data->rstc_pwrdn);
@@ -248,8 +247,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	dwc3_data->rstc_rst =
 		devm_reset_control_get_shared(dev, "softreset");
 	if (IS_ERR(dwc3_data->rstc_rst)) {
-		dev_err(&pdev->dev, "could not get reset controller\n");
-		ret = PTR_ERR(dwc3_data->rstc_rst);
+		ret = dev_err_probe(dev, PTR_ERR(dwc3_data->rstc_rst),
+				    "could not get reset controller\n");
 		goto undo_powerdown;
 	}
 

-- 
2.43.0

[PATCH 03/11] usb: dwc3: st: simplify pdev->dev usage

[Krzysztof Kozlowski]

The probe() function already stores '&pdev->dev' in local 'dev'
variable.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-st.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index 7a0b1821768a..2841021f3557 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -225,13 +225,13 @@ static int st_dwc3_probe(struct platform_device *pdev)
 
 	dwc3_data->syscfg_reg_off = res->start;
 
-	dev_vdbg(&pdev->dev, "glue-logic addr 0x%pK, syscfg-reg offset 0x%x\n",
+	dev_vdbg(dev, "glue-logic addr 0x%pK, syscfg-reg offset 0x%x\n",
 		 dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
 
 	struct device_node *child __free(device_node) = of_get_compatible_child(node,
 										"snps,dwc3");
 	if (!child) {
-		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
+		dev_err(dev, "failed to find dwc3 core node\n");
 		return -ENODEV;
 	}
 

-- 
2.43.0

[PATCH 04/11] usb: dwc3: imx8mp: simplify with devm_clk_get_enabled

[Krzysztof Kozlowski]

Use devm_clk_get_enabled() to drop clock preparing and handling from
error and remove paths.  This makes the code a bit simpler.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-imx8mp.c | 32 +++++---------------------------
 1 file changed, 5 insertions(+), 27 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
index 392fa1232788..8c91e595d3b9 100644
--- a/drivers/usb/dwc3/dwc3-imx8mp.c
+++ b/drivers/usb/dwc3/dwc3-imx8mp.c
@@ -178,37 +178,23 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 			return PTR_ERR(dwc3_imx->glue_base);
 	}
 
-	dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio");
+	dwc3_imx->hsio_clk = devm_clk_get_enabled(dev, "hsio");
 	if (IS_ERR(dwc3_imx->hsio_clk)) {
 		err = PTR_ERR(dwc3_imx->hsio_clk);
 		dev_err(dev, "Failed to get hsio clk, err=%d\n", err);
 		return err;
 	}
 
-	err = clk_prepare_enable(dwc3_imx->hsio_clk);
-	if (err) {
-		dev_err(dev, "Failed to enable hsio clk, err=%d\n", err);
-		return err;
-	}
-
-	dwc3_imx->suspend_clk = devm_clk_get(dev, "suspend");
+	dwc3_imx->suspend_clk = devm_clk_get_enabled(dev, "suspend");
 	if (IS_ERR(dwc3_imx->suspend_clk)) {
 		err = PTR_ERR(dwc3_imx->suspend_clk);
 		dev_err(dev, "Failed to get suspend clk, err=%d\n", err);
-		goto disable_hsio_clk;
-	}
-
-	err = clk_prepare_enable(dwc3_imx->suspend_clk);
-	if (err) {
-		dev_err(dev, "Failed to enable suspend clk, err=%d\n", err);
-		goto disable_hsio_clk;
+		return err;
 	}
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		err = irq;
-		goto disable_clks;
-	}
+	if (irq < 0)
+		return irq;
 	dwc3_imx->irq = irq;
 
 	imx8mp_configure_glue(dwc3_imx);
@@ -259,25 +245,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 disable_rpm:
 	pm_runtime_disable(dev);
 	pm_runtime_put_noidle(dev);
-disable_clks:
-	clk_disable_unprepare(dwc3_imx->suspend_clk);
-disable_hsio_clk:
-	clk_disable_unprepare(dwc3_imx->hsio_clk);
 
 	return err;
 }
 
 static void dwc3_imx8mp_remove(struct platform_device *pdev)
 {
-	struct dwc3_imx8mp *dwc3_imx = platform_get_drvdata(pdev);
 	struct device *dev = &pdev->dev;
 
 	pm_runtime_get_sync(dev);
 	of_platform_depopulate(dev);
 
-	clk_disable_unprepare(dwc3_imx->suspend_clk);
-	clk_disable_unprepare(dwc3_imx->hsio_clk);
-
 	pm_runtime_disable(dev);
 	pm_runtime_put_noidle(dev);
 }

-- 
2.43.0

[PATCH 05/11] usb: dwc3: imx8mp: simplify with dev_err_probe

[Krzysztof Kozlowski]

Use dev_err_probe() to make the error paths a bit simpler.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-imx8mp.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
index 8c91e595d3b9..869b5e7c15ed 100644
--- a/drivers/usb/dwc3/dwc3-imx8mp.c
+++ b/drivers/usb/dwc3/dwc3-imx8mp.c
@@ -179,18 +179,14 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 	}
 
 	dwc3_imx->hsio_clk = devm_clk_get_enabled(dev, "hsio");
-	if (IS_ERR(dwc3_imx->hsio_clk)) {
-		err = PTR_ERR(dwc3_imx->hsio_clk);
-		dev_err(dev, "Failed to get hsio clk, err=%d\n", err);
-		return err;
-	}
+	if (IS_ERR(dwc3_imx->hsio_clk))
+		return dev_err_probe(dev, PTR_ERR(dwc3_imx->hsio_clk),
+				     "Failed to get hsio clk\n");
 
 	dwc3_imx->suspend_clk = devm_clk_get_enabled(dev, "suspend");
-	if (IS_ERR(dwc3_imx->suspend_clk)) {
-		err = PTR_ERR(dwc3_imx->suspend_clk);
-		dev_err(dev, "Failed to get suspend clk, err=%d\n", err);
-		return err;
-	}
+	if (IS_ERR(dwc3_imx->suspend_clk))
+		return dev_err_probe(dev, PTR_ERR(dwc3_imx->suspend_clk),
+				     "Failed to get suspend clk\n");
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)

-- 
2.43.0

[PATCH 06/11] usb: dwc3: imx8mp: use scoped device node handling to simplify error paths

[Krzysztof Kozlowski]

Obtain the device node reference with scoped/cleanup.h to reduce error
handling and make the code a bit simpler.  Scoped/cleanup.h coding style
expects variable declaration with initialization, so the
of_get_compatible_child() call has to be moved earlier, before any goto
jumps happen.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-imx8mp.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
index 869b5e7c15ed..8b88649b569f 100644
--- a/drivers/usb/dwc3/dwc3-imx8mp.c
+++ b/drivers/usb/dwc3/dwc3-imx8mp.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2020 NXP.
  */
 
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -147,7 +148,7 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
 static int dwc3_imx8mp_probe(struct platform_device *pdev)
 {
 	struct device		*dev = &pdev->dev;
-	struct device_node	*dwc3_np, *node = dev->of_node;
+	struct device_node	*node = dev->of_node;
 	struct dwc3_imx8mp	*dwc3_imx;
 	struct resource		*res;
 	int			err, irq;
@@ -193,6 +194,11 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 		return irq;
 	dwc3_imx->irq = irq;
 
+	struct device_node *dwc3_np __free(device_node) = of_get_compatible_child(node,
+										  "snps,dwc3");
+	if (!dwc3_np)
+		return dev_err_probe(dev, -ENODEV, "failed to find dwc3 core child\n");
+
 	imx8mp_configure_glue(dwc3_imx);
 
 	pm_runtime_set_active(dev);
@@ -201,17 +207,10 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto disable_rpm;
 
-	dwc3_np = of_get_compatible_child(node, "snps,dwc3");
-	if (!dwc3_np) {
-		err = -ENODEV;
-		dev_err(dev, "failed to find dwc3 core child\n");
-		goto disable_rpm;
-	}
-
 	err = of_platform_populate(node, NULL, NULL, dev);
 	if (err) {
 		dev_err(&pdev->dev, "failed to create dwc3 core\n");
-		goto err_node_put;
+		goto disable_rpm;
 	}
 
 	dwc3_imx->dwc3 = of_find_device_by_node(dwc3_np);
@@ -220,7 +219,6 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 		err = -ENODEV;
 		goto depopulate;
 	}
-	of_node_put(dwc3_np);
 
 	err = devm_request_threaded_irq(dev, irq, NULL, dwc3_imx8mp_interrupt,
 					IRQF_ONESHOT, dev_name(dev), dwc3_imx);
@@ -236,8 +234,6 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
 
 depopulate:
 	of_platform_depopulate(dev);
-err_node_put:
-	of_node_put(dwc3_np);
 disable_rpm:
 	pm_runtime_disable(dev);
 	pm_runtime_put_noidle(dev);

-- 
2.43.0

[PATCH 07/11] usb: dwc3: qcom: use scoped device node handling to simplify error paths

[Krzysztof Kozlowski]

Obtain the device node reference with scoped/cleanup.h to reduce error
handling and make the code a bit simpler.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-qcom.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 88fb6706a18d..03a8f080078e 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -4,6 +4,7 @@
  * Inspired by dwc3-of-simple.c
  */
 
+#include <linux/cleanup.h>
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/clk.h>
@@ -702,11 +703,12 @@ static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
 static int dwc3_qcom_of_register_core(struct platform_device *pdev)
 {
 	struct dwc3_qcom	*qcom = platform_get_drvdata(pdev);
-	struct device_node	*np = pdev->dev.of_node, *dwc3_np;
+	struct device_node	*np = pdev->dev.of_node;
 	struct device		*dev = &pdev->dev;
 	int			ret;
 
-	dwc3_np = of_get_compatible_child(np, "snps,dwc3");
+	struct device_node *dwc3_np __free(device_node) = of_get_compatible_child(np,
+										  "snps,dwc3");
 	if (!dwc3_np) {
 		dev_err(dev, "failed to find dwc3 core child\n");
 		return -ENODEV;
@@ -715,7 +717,7 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)
 	ret = of_platform_populate(np, NULL, NULL, dev);
 	if (ret) {
 		dev_err(dev, "failed to register dwc3 core - %d\n", ret);
-		goto node_put;
+		return ret;
 	}
 
 	qcom->dwc3 = of_find_device_by_node(dwc3_np);
@@ -725,9 +727,6 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)
 		of_platform_depopulate(dev);
 	}
 
-node_put:
-	of_node_put(dwc3_np);
-
 	return ret;
 }
 

-- 
2.43.0

[PATCH 08/11] usb: dwc3: qcom: simplify with devm_platform_ioremap_resource

[Krzysztof Kozlowski]

Use devm_platform_ioremap_resource() wrapper instead of two calls.  This
allows also dropping local 'res' variable.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-qcom.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 03a8f080078e..c1d4b52f25b0 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -735,7 +735,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	struct device_node	*np = pdev->dev.of_node;
 	struct device		*dev = &pdev->dev;
 	struct dwc3_qcom	*qcom;
-	struct resource		*res;
 	int			ret, i;
 	bool			ignore_pipe_clk;
 	bool			wakeup_source;
@@ -773,9 +772,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 		goto reset_assert;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-
-	qcom->qscratch_base = devm_ioremap_resource(dev, res);
+	qcom->qscratch_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(qcom->qscratch_base)) {
 		ret = PTR_ERR(qcom->qscratch_base);
 		goto clk_disable;

-- 
2.43.0

[PATCH 09/11] usb: dwc3: rtk: use scoped device node handling to simplify error paths

[Krzysztof Kozlowski]

Obtain the device node reference with scoped/cleanup.h to reduce error
handling and make the code a bit simpler.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-rtk.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-rtk.c b/drivers/usb/dwc3/dwc3-rtk.c
index 3cd6b184551c..1e3ec2084286 100644
--- a/drivers/usb/dwc3/dwc3-rtk.c
+++ b/drivers/usb/dwc3/dwc3-rtk.c
@@ -6,6 +6,7 @@
  *
  */
 
+#include <linux/cleanup.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
@@ -173,23 +174,20 @@ static const char *const speed_names[] = {
 
 static enum usb_device_speed __get_dwc3_maximum_speed(struct device_node *np)
 {
-	struct device_node *dwc3_np;
 	const char *maximum_speed;
 	int ret;
 
-	dwc3_np = of_get_compatible_child(np, "snps,dwc3");
+	struct device_node *dwc3_np __free(device_node) = of_get_compatible_child(np,
+										  "snps,dwc3");
 	if (!dwc3_np)
 		return USB_SPEED_UNKNOWN;
 
 	ret = of_property_read_string(dwc3_np, "maximum-speed", &maximum_speed);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	ret = match_string(speed_names, ARRAY_SIZE(speed_names), maximum_speed);
 
-out:
-	of_node_put(dwc3_np);
-
 	return (ret < 0) ? USB_SPEED_UNKNOWN : ret;
 }
 
@@ -276,7 +274,6 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
 	struct device_node *node = dev->of_node;
 	struct platform_device *dwc3_pdev;
 	struct device *dwc3_dev;
-	struct device_node *dwc3_node;
 	enum usb_dr_mode dr_mode;
 	int ret = 0;
 
@@ -290,7 +287,8 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
 		return ret;
 	}
 
-	dwc3_node = of_get_compatible_child(node, "snps,dwc3");
+	struct device_node *dwc3_node __free(device_node) = of_get_compatible_child(node,
+										    "snps,dwc3");
 	if (!dwc3_node) {
 		dev_err(dev, "failed to find dwc3 core node\n");
 		ret = -ENODEV;
@@ -301,7 +299,7 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
 	if (!dwc3_pdev) {
 		dev_err(dev, "failed to find dwc3 core platform_device\n");
 		ret = -ENODEV;
-		goto err_node_put;
+		goto depopulate;
 	}
 
 	dwc3_dev = &dwc3_pdev->dev;
@@ -343,14 +341,11 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
 	switch_usb2_role(rtk, rtk->cur_role);
 
 	platform_device_put(dwc3_pdev);
-	of_node_put(dwc3_node);
 
 	return 0;
 
 err_pdev_put:
 	platform_device_put(dwc3_pdev);
-err_node_put:
-	of_node_put(dwc3_node);
 depopulate:
 	of_platform_depopulate(dev);
 

-- 
2.43.0

[PATCH 10/11] usb: dwc3: rtk: return directly and simplify with devm_platform_ioremap_resource

[Krzysztof Kozlowski]

Use devm_platform_ioremap_resource() wrapper instead of two calls, which
together with returning directly instead of useless goto, allows to
nicely simplify the probe() function.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-rtk.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-rtk.c b/drivers/usb/dwc3/dwc3-rtk.c
index 1e3ec2084286..b3db5cd9906e 100644
--- a/drivers/usb/dwc3/dwc3-rtk.c
+++ b/drivers/usb/dwc3/dwc3-rtk.c
@@ -358,30 +358,18 @@ static int dwc3_rtk_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	void __iomem *regs;
-	int ret = 0;
 
 	rtk = devm_kzalloc(dev, sizeof(*rtk), GFP_KERNEL);
-	if (!rtk) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!rtk)
+		return -ENOMEM;
 
 	platform_set_drvdata(pdev, rtk);
 
 	rtk->dev = dev;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(dev, "missing memory resource\n");
-		ret = -ENODEV;
-		goto out;
-	}
-
-	regs = devm_ioremap_resource(dev, res);
-	if (IS_ERR(regs)) {
-		ret = PTR_ERR(regs);
-		goto out;
-	}
+	regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
 
 	rtk->regs = regs;
 	rtk->regs_size = resource_size(res);
@@ -389,16 +377,11 @@ static int dwc3_rtk_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	if (res) {
 		rtk->pm_base = devm_ioremap_resource(dev, res);
-		if (IS_ERR(rtk->pm_base)) {
-			ret = PTR_ERR(rtk->pm_base);
-			goto out;
-		}
+		if (IS_ERR(rtk->pm_base))
+			return PTR_ERR(rtk->pm_base);
 	}
 
-	ret = dwc3_rtk_probe_dwc3_core(rtk);
-
-out:
-	return ret;
+	return dwc3_rtk_probe_dwc3_core(rtk);
 }
 
 static void dwc3_rtk_remove(struct platform_device *pdev)

-- 
2.43.0

[PATCH 11/11] usb: dwc3: xilinx: simplify with dev_err_probe

[Krzysztof Kozlowski]

Use dev_err_probe() to make the error paths a bit simpler.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-xilinx.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
index b7613a106da6..eb535733ce91 100644
--- a/drivers/usb/dwc3/dwc3-xilinx.c
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -285,11 +285,8 @@ static int dwc3_xlnx_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(regs)) {
-		ret = PTR_ERR(regs);
-		dev_err_probe(dev, ret, "failed to map registers\n");
-		return ret;
-	}
+	if (IS_ERR(regs))
+		return dev_err_probe(dev, PTR_ERR(regs), "failed to map registers\n");
 
 	match = of_match_node(dwc3_xlnx_of_match, pdev->dev.of_node);
 

-- 
2.43.0

Re: [PATCH 01/11] usb: dwc3: st: use scoped device node handling to simplify error paths

[Patrice CHOTARD]

On 8/14/24 12:35, Krzysztof Kozlowski wrote:
> Obtain the device node reference with scoped/cleanup.h to reduce error
> handling and make the code a bit simpler.  Scoped/cleanup.h coding style
> expects variable declaration with initialization, so the
> of_get_compatible_child() call has to be moved earlier, before any goto
> jumps happen.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> This depends on my earlier fix:
> https://lore.kernel.org/all/20240814093957.37940-2-krzysztof.kozlowski@linaro.org/
> ---
>  drivers/usb/dwc3/dwc3-st.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index c8c7cd0c1796..98f43d7082d7 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -14,6 +14,7 @@
>   * Inspired by dwc3-omap.c and dwc3-exynos.c.
>   */
>  
> +#include <linux/cleanup.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -197,7 +198,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	struct st_dwc3 *dwc3_data;
>  	struct resource *res;
>  	struct device *dev = &pdev->dev;
> -	struct device_node *node = dev->of_node, *child;
> +	struct device_node *node = dev->of_node;
>  	struct platform_device *child_pdev;
>  	struct regmap *regmap;
>  	int ret;
> @@ -227,6 +228,13 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	dev_vdbg(&pdev->dev, "glue-logic addr 0x%pK, syscfg-reg offset 0x%x\n",
>  		 dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
>  
> +	struct device_node *child __free(device_node) = of_get_compatible_child(node,
> +										"snps,dwc3");
> +	if (!child) {
> +		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
> +		return -ENODEV;
> +	}
> +
>  	dwc3_data->rstc_pwrdn =
>  		devm_reset_control_get_exclusive(dev, "powerdown");
>  	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
> @@ -248,18 +256,11 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	/* Manage SoftReset */
>  	reset_control_deassert(dwc3_data->rstc_rst);
>  
> -	child = of_get_compatible_child(node, "snps,dwc3");
> -	if (!child) {
> -		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
> -		ret = -ENODEV;
> -		goto err_node_put;
> -	}
> -
>  	/* Allocate and initialize the core */
>  	ret = of_platform_populate(node, NULL, NULL, dev);
>  	if (ret) {
>  		dev_err(dev, "failed to add dwc3 core\n");
> -		goto err_node_put;
> +		goto undo_softreset;
>  	}
>  
>  	child_pdev = of_find_device_by_node(child);
> @@ -270,7 +271,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	}
>  
>  	dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
> -	of_node_put(child);
>  	platform_device_put(child_pdev);
>  
>  	/*
> @@ -282,8 +282,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	ret = st_dwc3_drd_init(dwc3_data);
>  	if (ret) {
>  		dev_err(dev, "drd initialisation failed\n");
> -		of_platform_depopulate(dev);
> -		goto undo_softreset;
> +		goto depopulate;
>  	}
>  
>  	/* ST glue logic init */
> @@ -294,8 +293,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  
>  depopulate:
>  	of_platform_depopulate(dev);
> -err_node_put:
> -	of_node_put(child);
>  undo_softreset:
>  	reset_control_assert(dwc3_data->rstc_rst);
>  undo_powerdown:
> 


Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

Re: [PATCH 06/11] usb: dwc3: imx8mp: use scoped device node handling to simplify error paths

[Frank Li]

On Wed, Aug 14, 2024 at 12:35:42PM +0200, Krzysztof Kozlowski wrote:
> Obtain the device node reference with scoped/cleanup.h to reduce error
> handling and make the code a bit simpler.  Scoped/cleanup.h coding style
> expects variable declaration with initialization, so the
> of_get_compatible_child() call has to be moved earlier, before any goto
> jumps happen.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> index 869b5e7c15ed..8b88649b569f 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2020 NXP.
>   */
>
> +#include <linux/cleanup.h>
>  #include <linux/clk.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -147,7 +148,7 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
>  static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  {
>  	struct device		*dev = &pdev->dev;
> -	struct device_node	*dwc3_np, *node = dev->of_node;
> +	struct device_node	*node = dev->of_node;
>  	struct dwc3_imx8mp	*dwc3_imx;
>  	struct resource		*res;
>  	int			err, irq;
> @@ -193,6 +194,11 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  		return irq;
>  	dwc3_imx->irq = irq;
>
> +	struct device_node *dwc3_np __free(device_node) = of_get_compatible_child(node,
> +										  "snps,dwc3");
> +	if (!dwc3_np)
> +		return dev_err_probe(dev, -ENODEV, "failed to find dwc3 core child\n");
> +
>  	imx8mp_configure_glue(dwc3_imx);
>
>  	pm_runtime_set_active(dev);
> @@ -201,17 +207,10 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto disable_rpm;
>
> -	dwc3_np = of_get_compatible_child(node, "snps,dwc3");
> -	if (!dwc3_np) {
> -		err = -ENODEV;
> -		dev_err(dev, "failed to find dwc3 core child\n");
> -		goto disable_rpm;
> -	}
> -
>  	err = of_platform_populate(node, NULL, NULL, dev);
>  	if (err) {
>  		dev_err(&pdev->dev, "failed to create dwc3 core\n");
> -		goto err_node_put;
> +		goto disable_rpm;
>  	}
>
>  	dwc3_imx->dwc3 = of_find_device_by_node(dwc3_np);
> @@ -220,7 +219,6 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  		err = -ENODEV;
>  		goto depopulate;
>  	}
> -	of_node_put(dwc3_np);
>
>  	err = devm_request_threaded_irq(dev, irq, NULL, dwc3_imx8mp_interrupt,
>  					IRQF_ONESHOT, dev_name(dev), dwc3_imx);
> @@ -236,8 +234,6 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>
>  depopulate:
>  	of_platform_depopulate(dev);
> -err_node_put:
> -	of_node_put(dwc3_np);
>  disable_rpm:
>  	pm_runtime_disable(dev);
>  	pm_runtime_put_noidle(dev);
>
> --
> 2.43.0
>

Re: [PATCH 05/11] usb: dwc3: imx8mp: simplify with dev_err_probe

[Frank Li]

On Wed, Aug 14, 2024 at 12:35:41PM +0200, Krzysztof Kozlowski wrote:
> Use dev_err_probe() to make the error paths a bit simpler.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> index 8c91e595d3b9..869b5e7c15ed 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -179,18 +179,14 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  	}
>
>  	dwc3_imx->hsio_clk = devm_clk_get_enabled(dev, "hsio");
> -	if (IS_ERR(dwc3_imx->hsio_clk)) {
> -		err = PTR_ERR(dwc3_imx->hsio_clk);
> -		dev_err(dev, "Failed to get hsio clk, err=%d\n", err);
> -		return err;
> -	}
> +	if (IS_ERR(dwc3_imx->hsio_clk))
> +		return dev_err_probe(dev, PTR_ERR(dwc3_imx->hsio_clk),
> +				     "Failed to get hsio clk\n");
>
>  	dwc3_imx->suspend_clk = devm_clk_get_enabled(dev, "suspend");
> -	if (IS_ERR(dwc3_imx->suspend_clk)) {
> -		err = PTR_ERR(dwc3_imx->suspend_clk);
> -		dev_err(dev, "Failed to get suspend clk, err=%d\n", err);
> -		return err;
> -	}
> +	if (IS_ERR(dwc3_imx->suspend_clk))
> +		return dev_err_probe(dev, PTR_ERR(dwc3_imx->suspend_clk),
> +				     "Failed to get suspend clk\n");
>
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0)
>
> --
> 2.43.0
>

Re: [PATCH 02/11] usb: dwc3: st: simplify with dev_err_probe

[Patrice CHOTARD]

On 8/14/24 12:35, Krzysztof Kozlowski wrote:
> Use dev_err_probe() to make the error paths a bit simpler.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-st.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index 98f43d7082d7..7a0b1821768a 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -237,10 +237,9 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  
>  	dwc3_data->rstc_pwrdn =
>  		devm_reset_control_get_exclusive(dev, "powerdown");
> -	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
> -		dev_err(&pdev->dev, "could not get power controller\n");
> -		return PTR_ERR(dwc3_data->rstc_pwrdn);
> -	}
> +	if (IS_ERR(dwc3_data->rstc_pwrdn))
> +		return dev_err_probe(dev, PTR_ERR(dwc3_data->rstc_pwrdn),
> +				     "could not get power controller\n");
>  
>  	/* Manage PowerDown */
>  	reset_control_deassert(dwc3_data->rstc_pwrdn);
> @@ -248,8 +247,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	dwc3_data->rstc_rst =
>  		devm_reset_control_get_shared(dev, "softreset");
>  	if (IS_ERR(dwc3_data->rstc_rst)) {
> -		dev_err(&pdev->dev, "could not get reset controller\n");
> -		ret = PTR_ERR(dwc3_data->rstc_rst);
> +		ret = dev_err_probe(dev, PTR_ERR(dwc3_data->rstc_rst),
> +				    "could not get reset controller\n");
>  		goto undo_powerdown;
>  	}
>  
> 


Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

Re: [PATCH 03/11] usb: dwc3: st: simplify pdev->dev usage

[Patrice CHOTARD]

On 8/14/24 12:35, Krzysztof Kozlowski wrote:
> The probe() function already stores '&pdev->dev' in local 'dev'
> variable.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-st.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index 7a0b1821768a..2841021f3557 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -225,13 +225,13 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  
>  	dwc3_data->syscfg_reg_off = res->start;
>  
> -	dev_vdbg(&pdev->dev, "glue-logic addr 0x%pK, syscfg-reg offset 0x%x\n",
> +	dev_vdbg(dev, "glue-logic addr 0x%pK, syscfg-reg offset 0x%x\n",
>  		 dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
>  
>  	struct device_node *child __free(device_node) = of_get_compatible_child(node,
>  										"snps,dwc3");
>  	if (!child) {
> -		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
> +		dev_err(dev, "failed to find dwc3 core node\n");
>  		return -ENODEV;
>  	}
>  
> 

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

Re: [PATCH 04/11] usb: dwc3: imx8mp: simplify with devm_clk_get_enabled

[Frank Li]

On Wed, Aug 14, 2024 at 12:35:40PM +0200, Krzysztof Kozlowski wrote:
> Use devm_clk_get_enabled() to drop clock preparing and handling from
> error and remove paths.  This makes the code a bit simpler.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 32 +++++---------------------------
>  1 file changed, 5 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> index 392fa1232788..8c91e595d3b9 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -178,37 +178,23 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  			return PTR_ERR(dwc3_imx->glue_base);
>  	}
>
> -	dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio");
> +	dwc3_imx->hsio_clk = devm_clk_get_enabled(dev, "hsio");
>  	if (IS_ERR(dwc3_imx->hsio_clk)) {
>  		err = PTR_ERR(dwc3_imx->hsio_clk);
>  		dev_err(dev, "Failed to get hsio clk, err=%d\n", err);
>  		return err;
>  	}
>
> -	err = clk_prepare_enable(dwc3_imx->hsio_clk);
> -	if (err) {
> -		dev_err(dev, "Failed to enable hsio clk, err=%d\n", err);
> -		return err;
> -	}
> -
> -	dwc3_imx->suspend_clk = devm_clk_get(dev, "suspend");
> +	dwc3_imx->suspend_clk = devm_clk_get_enabled(dev, "suspend");
>  	if (IS_ERR(dwc3_imx->suspend_clk)) {
>  		err = PTR_ERR(dwc3_imx->suspend_clk);
>  		dev_err(dev, "Failed to get suspend clk, err=%d\n", err);
> -		goto disable_hsio_clk;
> -	}
> -
> -	err = clk_prepare_enable(dwc3_imx->suspend_clk);
> -	if (err) {
> -		dev_err(dev, "Failed to enable suspend clk, err=%d\n", err);
> -		goto disable_hsio_clk;
> +		return err;
>  	}
>
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		err = irq;
> -		goto disable_clks;
> -	}
> +	if (irq < 0)
> +		return irq;
>  	dwc3_imx->irq = irq;
>
>  	imx8mp_configure_glue(dwc3_imx);
> @@ -259,25 +245,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  disable_rpm:
>  	pm_runtime_disable(dev);
>  	pm_runtime_put_noidle(dev);
> -disable_clks:
> -	clk_disable_unprepare(dwc3_imx->suspend_clk);
> -disable_hsio_clk:
> -	clk_disable_unprepare(dwc3_imx->hsio_clk);
>
>  	return err;
>  }
>
>  static void dwc3_imx8mp_remove(struct platform_device *pdev)
>  {
> -	struct dwc3_imx8mp *dwc3_imx = platform_get_drvdata(pdev);
>  	struct device *dev = &pdev->dev;
>
>  	pm_runtime_get_sync(dev);
>  	of_platform_depopulate(dev);
>
> -	clk_disable_unprepare(dwc3_imx->suspend_clk);
> -	clk_disable_unprepare(dwc3_imx->hsio_clk);
> -
>  	pm_runtime_disable(dev);
>  	pm_runtime_put_noidle(dev);
>  }
>
> --
> 2.43.0
>

Re: [PATCH 01/11] usb: dwc3: st: use scoped device node handling to simplify error paths

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Obtain the device node reference with scoped/cleanup.h to reduce error
> handling and make the code a bit simpler.  Scoped/cleanup.h coding style
> expects variable declaration with initialization, so the
> of_get_compatible_child() call has to be moved earlier, before any goto
> jumps happen.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> This depends on my earlier fix:
> https://urldefense.com/v3/__https://lore.kernel.org/all/20240814093957.37940-2-krzysztof.kozlowski@linaro.org/__;!!A4F2R9G_pg!exVYjnFIRkiBgaYBTEIW3apFnDM4Mojp3r5-ayi3seBrI7oszldsFK-Yo1uzpLWjrg69Za02vLy_SlMQE76EyRC3hn2xaV5j$ 


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

Thanks,
Thinh

Re: [PATCH 02/11] usb: dwc3: st: simplify with dev_err_probe

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Use dev_err_probe() to make the error paths a bit simpler.

I think it makes more sense to note that this helps with cases of
-EPROBE_DEFER than making this simpler. Regardless, this is an
improvement.

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

Thanks,
Thinh

> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-st.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index 98f43d7082d7..7a0b1821768a 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -237,10 +237,9 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  
>  	dwc3_data->rstc_pwrdn =
>  		devm_reset_control_get_exclusive(dev, "powerdown");
> -	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
> -		dev_err(&pdev->dev, "could not get power controller\n");
> -		return PTR_ERR(dwc3_data->rstc_pwrdn);
> -	}
> +	if (IS_ERR(dwc3_data->rstc_pwrdn))
> +		return dev_err_probe(dev, PTR_ERR(dwc3_data->rstc_pwrdn),
> +				     "could not get power controller\n");
>  
>  	/* Manage PowerDown */
>  	reset_control_deassert(dwc3_data->rstc_pwrdn);
> @@ -248,8 +247,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	dwc3_data->rstc_rst =
>  		devm_reset_control_get_shared(dev, "softreset");
>  	if (IS_ERR(dwc3_data->rstc_rst)) {
> -		dev_err(&pdev->dev, "could not get reset controller\n");
> -		ret = PTR_ERR(dwc3_data->rstc_rst);
> +		ret = dev_err_probe(dev, PTR_ERR(dwc3_data->rstc_rst),
> +				    "could not get reset controller\n");
>  		goto undo_powerdown;
>  	}
>  
> 
> -- 
> 2.43.0
> 

Re: [PATCH 03/11] usb: dwc3: st: simplify pdev->dev usage

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> The probe() function already stores '&pdev->dev' in local 'dev'
> variable.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-st.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index 7a0b1821768a..2841021f3557 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -225,13 +225,13 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  
>  	dwc3_data->syscfg_reg_off = res->start;
>  
> -	dev_vdbg(&pdev->dev, "glue-logic addr 0x%pK, syscfg-reg offset 0x%x\n",
> +	dev_vdbg(dev, "glue-logic addr 0x%pK, syscfg-reg offset 0x%x\n",
>  		 dwc3_data->glue_base, dwc3_data->syscfg_reg_off);
>  
>  	struct device_node *child __free(device_node) = of_get_compatible_child(node,
>  										"snps,dwc3");
>  	if (!child) {
> -		dev_err(&pdev->dev, "failed to find dwc3 core node\n");
> +		dev_err(dev, "failed to find dwc3 core node\n");
>  		return -ENODEV;
>  	}
>  
> 
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

Re: [PATCH 04/11] usb: dwc3: imx8mp: simplify with devm_clk_get_enabled

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Use devm_clk_get_enabled() to drop clock preparing and handling from
> error and remove paths.  This makes the code a bit simpler.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 32 +++++---------------------------
>  1 file changed, 5 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> index 392fa1232788..8c91e595d3b9 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -178,37 +178,23 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  			return PTR_ERR(dwc3_imx->glue_base);
>  	}
>  
> -	dwc3_imx->hsio_clk = devm_clk_get(dev, "hsio");
> +	dwc3_imx->hsio_clk = devm_clk_get_enabled(dev, "hsio");
>  	if (IS_ERR(dwc3_imx->hsio_clk)) {
>  		err = PTR_ERR(dwc3_imx->hsio_clk);
>  		dev_err(dev, "Failed to get hsio clk, err=%d\n", err);
>  		return err;
>  	}
>  
> -	err = clk_prepare_enable(dwc3_imx->hsio_clk);
> -	if (err) {
> -		dev_err(dev, "Failed to enable hsio clk, err=%d\n", err);
> -		return err;
> -	}
> -
> -	dwc3_imx->suspend_clk = devm_clk_get(dev, "suspend");
> +	dwc3_imx->suspend_clk = devm_clk_get_enabled(dev, "suspend");
>  	if (IS_ERR(dwc3_imx->suspend_clk)) {
>  		err = PTR_ERR(dwc3_imx->suspend_clk);
>  		dev_err(dev, "Failed to get suspend clk, err=%d\n", err);
> -		goto disable_hsio_clk;
> -	}
> -
> -	err = clk_prepare_enable(dwc3_imx->suspend_clk);
> -	if (err) {
> -		dev_err(dev, "Failed to enable suspend clk, err=%d\n", err);
> -		goto disable_hsio_clk;
> +		return err;
>  	}
>  
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		err = irq;
> -		goto disable_clks;
> -	}
> +	if (irq < 0)
> +		return irq;
>  	dwc3_imx->irq = irq;
>  
>  	imx8mp_configure_glue(dwc3_imx);
> @@ -259,25 +245,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  disable_rpm:
>  	pm_runtime_disable(dev);
>  	pm_runtime_put_noidle(dev);
> -disable_clks:
> -	clk_disable_unprepare(dwc3_imx->suspend_clk);
> -disable_hsio_clk:
> -	clk_disable_unprepare(dwc3_imx->hsio_clk);
>  
>  	return err;
>  }
>  
>  static void dwc3_imx8mp_remove(struct platform_device *pdev)
>  {
> -	struct dwc3_imx8mp *dwc3_imx = platform_get_drvdata(pdev);
>  	struct device *dev = &pdev->dev;
>  
>  	pm_runtime_get_sync(dev);
>  	of_platform_depopulate(dev);
>  
> -	clk_disable_unprepare(dwc3_imx->suspend_clk);
> -	clk_disable_unprepare(dwc3_imx->hsio_clk);
> -
>  	pm_runtime_disable(dev);
>  	pm_runtime_put_noidle(dev);
>  }
> 
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

Re: [PATCH 05/11] usb: dwc3: imx8mp: simplify with dev_err_probe

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Use dev_err_probe() to make the error paths a bit simpler.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> index 8c91e595d3b9..869b5e7c15ed 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -179,18 +179,14 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  	}
>  
>  	dwc3_imx->hsio_clk = devm_clk_get_enabled(dev, "hsio");
> -	if (IS_ERR(dwc3_imx->hsio_clk)) {
> -		err = PTR_ERR(dwc3_imx->hsio_clk);
> -		dev_err(dev, "Failed to get hsio clk, err=%d\n", err);
> -		return err;
> -	}
> +	if (IS_ERR(dwc3_imx->hsio_clk))
> +		return dev_err_probe(dev, PTR_ERR(dwc3_imx->hsio_clk),
> +				     "Failed to get hsio clk\n");
>  
>  	dwc3_imx->suspend_clk = devm_clk_get_enabled(dev, "suspend");
> -	if (IS_ERR(dwc3_imx->suspend_clk)) {
> -		err = PTR_ERR(dwc3_imx->suspend_clk);
> -		dev_err(dev, "Failed to get suspend clk, err=%d\n", err);
> -		return err;
> -	}
> +	if (IS_ERR(dwc3_imx->suspend_clk))
> +		return dev_err_probe(dev, PTR_ERR(dwc3_imx->suspend_clk),
> +				     "Failed to get suspend clk\n");
>  
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0)
> 
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

Re: [PATCH 06/11] usb: dwc3: imx8mp: use scoped device node handling to simplify error paths

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Obtain the device node reference with scoped/cleanup.h to reduce error
> handling and make the code a bit simpler.  Scoped/cleanup.h coding style
> expects variable declaration with initialization, so the
> of_get_compatible_child() call has to be moved earlier, before any goto
> jumps happen.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-imx8mp.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> index 869b5e7c15ed..8b88649b569f 100644
> --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2020 NXP.
>   */
>  
> +#include <linux/cleanup.h>
>  #include <linux/clk.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -147,7 +148,7 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
>  static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  {
>  	struct device		*dev = &pdev->dev;
> -	struct device_node	*dwc3_np, *node = dev->of_node;
> +	struct device_node	*node = dev->of_node;
>  	struct dwc3_imx8mp	*dwc3_imx;
>  	struct resource		*res;
>  	int			err, irq;
> @@ -193,6 +194,11 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  		return irq;
>  	dwc3_imx->irq = irq;
>  
> +	struct device_node *dwc3_np __free(device_node) = of_get_compatible_child(node,
> +										  "snps,dwc3");
> +	if (!dwc3_np)
> +		return dev_err_probe(dev, -ENODEV, "failed to find dwc3 core child\n");
> +
>  	imx8mp_configure_glue(dwc3_imx);
>  
>  	pm_runtime_set_active(dev);
> @@ -201,17 +207,10 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto disable_rpm;
>  
> -	dwc3_np = of_get_compatible_child(node, "snps,dwc3");
> -	if (!dwc3_np) {
> -		err = -ENODEV;
> -		dev_err(dev, "failed to find dwc3 core child\n");
> -		goto disable_rpm;
> -	}
> -
>  	err = of_platform_populate(node, NULL, NULL, dev);
>  	if (err) {
>  		dev_err(&pdev->dev, "failed to create dwc3 core\n");
> -		goto err_node_put;
> +		goto disable_rpm;
>  	}
>  
>  	dwc3_imx->dwc3 = of_find_device_by_node(dwc3_np);
> @@ -220,7 +219,6 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  		err = -ENODEV;
>  		goto depopulate;
>  	}
> -	of_node_put(dwc3_np);
>  
>  	err = devm_request_threaded_irq(dev, irq, NULL, dwc3_imx8mp_interrupt,
>  					IRQF_ONESHOT, dev_name(dev), dwc3_imx);
> @@ -236,8 +234,6 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
>  
>  depopulate:
>  	of_platform_depopulate(dev);
> -err_node_put:
> -	of_node_put(dwc3_np);
>  disable_rpm:
>  	pm_runtime_disable(dev);
>  	pm_runtime_put_noidle(dev);
> 
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

Re: [PATCH 07/11] usb: dwc3: qcom: use scoped device node handling to simplify error paths

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Obtain the device node reference with scoped/cleanup.h to reduce error
> handling and make the code a bit simpler.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 88fb6706a18d..03a8f080078e 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -4,6 +4,7 @@
>   * Inspired by dwc3-of-simple.c
>   */
>  
> +#include <linux/cleanup.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
>  #include <linux/clk.h>
> @@ -702,11 +703,12 @@ static int dwc3_qcom_clk_init(struct dwc3_qcom *qcom, int count)
>  static int dwc3_qcom_of_register_core(struct platform_device *pdev)
>  {
>  	struct dwc3_qcom	*qcom = platform_get_drvdata(pdev);
> -	struct device_node	*np = pdev->dev.of_node, *dwc3_np;
> +	struct device_node	*np = pdev->dev.of_node;
>  	struct device		*dev = &pdev->dev;
>  	int			ret;
>  
> -	dwc3_np = of_get_compatible_child(np, "snps,dwc3");
> +	struct device_node *dwc3_np __free(device_node) = of_get_compatible_child(np,
> +										  "snps,dwc3");
>  	if (!dwc3_np) {
>  		dev_err(dev, "failed to find dwc3 core child\n");
>  		return -ENODEV;
> @@ -715,7 +717,7 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)
>  	ret = of_platform_populate(np, NULL, NULL, dev);
>  	if (ret) {
>  		dev_err(dev, "failed to register dwc3 core - %d\n", ret);
> -		goto node_put;
> +		return ret;
>  	}
>  
>  	qcom->dwc3 = of_find_device_by_node(dwc3_np);
> @@ -725,9 +727,6 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev)
>  		of_platform_depopulate(dev);
>  	}
>  
> -node_put:
> -	of_node_put(dwc3_np);
> -
>  	return ret;
>  }
>  
> 
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

Re: [PATCH 08/11] usb: dwc3: qcom: simplify with devm_platform_ioremap_resource

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Use devm_platform_ioremap_resource() wrapper instead of two calls.  This
> allows also dropping local 'res' variable.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 03a8f080078e..c1d4b52f25b0 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -735,7 +735,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	struct device_node	*np = pdev->dev.of_node;
>  	struct device		*dev = &pdev->dev;
>  	struct dwc3_qcom	*qcom;
> -	struct resource		*res;
>  	int			ret, i;
>  	bool			ignore_pipe_clk;
>  	bool			wakeup_source;
> @@ -773,9 +772,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  		goto reset_assert;
>  	}
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -
> -	qcom->qscratch_base = devm_ioremap_resource(dev, res);
> +	qcom->qscratch_base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(qcom->qscratch_base)) {
>  		ret = PTR_ERR(qcom->qscratch_base);
>  		goto clk_disable;
> 
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

Re: [PATCH 09/11] usb: dwc3: rtk: use scoped device node handling to simplify error paths

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Obtain the device node reference with scoped/cleanup.h to reduce error
> handling and make the code a bit simpler.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-rtk.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-rtk.c b/drivers/usb/dwc3/dwc3-rtk.c
> index 3cd6b184551c..1e3ec2084286 100644
> --- a/drivers/usb/dwc3/dwc3-rtk.c
> +++ b/drivers/usb/dwc3/dwc3-rtk.c
> @@ -6,6 +6,7 @@
>   *
>   */
>  
> +#include <linux/cleanup.h>
>  #include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
> @@ -173,23 +174,20 @@ static const char *const speed_names[] = {
>  
>  static enum usb_device_speed __get_dwc3_maximum_speed(struct device_node *np)
>  {
> -	struct device_node *dwc3_np;
>  	const char *maximum_speed;
>  	int ret;
>  
> -	dwc3_np = of_get_compatible_child(np, "snps,dwc3");
> +	struct device_node *dwc3_np __free(device_node) = of_get_compatible_child(np,
> +										  "snps,dwc3");
>  	if (!dwc3_np)
>  		return USB_SPEED_UNKNOWN;
>  
>  	ret = of_property_read_string(dwc3_np, "maximum-speed", &maximum_speed);
>  	if (ret < 0)
> -		goto out;
> +		return ret;
>  
>  	ret = match_string(speed_names, ARRAY_SIZE(speed_names), maximum_speed);
>  
> -out:
> -	of_node_put(dwc3_np);
> -
>  	return (ret < 0) ? USB_SPEED_UNKNOWN : ret;
>  }
>  
> @@ -276,7 +274,6 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
>  	struct device_node *node = dev->of_node;
>  	struct platform_device *dwc3_pdev;
>  	struct device *dwc3_dev;
> -	struct device_node *dwc3_node;
>  	enum usb_dr_mode dr_mode;
>  	int ret = 0;
>  
> @@ -290,7 +287,8 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
>  		return ret;
>  	}
>  
> -	dwc3_node = of_get_compatible_child(node, "snps,dwc3");
> +	struct device_node *dwc3_node __free(device_node) = of_get_compatible_child(node,
> +										    "snps,dwc3");
>  	if (!dwc3_node) {
>  		dev_err(dev, "failed to find dwc3 core node\n");
>  		ret = -ENODEV;
> @@ -301,7 +299,7 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
>  	if (!dwc3_pdev) {
>  		dev_err(dev, "failed to find dwc3 core platform_device\n");
>  		ret = -ENODEV;
> -		goto err_node_put;
> +		goto depopulate;
>  	}
>  
>  	dwc3_dev = &dwc3_pdev->dev;
> @@ -343,14 +341,11 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
>  	switch_usb2_role(rtk, rtk->cur_role);
>  
>  	platform_device_put(dwc3_pdev);
> -	of_node_put(dwc3_node);
>  
>  	return 0;
>  
>  err_pdev_put:
>  	platform_device_put(dwc3_pdev);
> -err_node_put:
> -	of_node_put(dwc3_node);
>  depopulate:
>  	of_platform_depopulate(dev);
>  
> 
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

Re: [PATCH 10/11] usb: dwc3: rtk: return directly and simplify with devm_platform_ioremap_resource

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Use devm_platform_ioremap_resource() wrapper instead of two calls, which
> together with returning directly instead of useless goto, allows to
> nicely simplify the probe() function.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-rtk.c | 33 ++++++++-------------------------
>  1 file changed, 8 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-rtk.c b/drivers/usb/dwc3/dwc3-rtk.c
> index 1e3ec2084286..b3db5cd9906e 100644
> --- a/drivers/usb/dwc3/dwc3-rtk.c
> +++ b/drivers/usb/dwc3/dwc3-rtk.c
> @@ -358,30 +358,18 @@ static int dwc3_rtk_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct resource *res;
>  	void __iomem *regs;
> -	int ret = 0;
>  
>  	rtk = devm_kzalloc(dev, sizeof(*rtk), GFP_KERNEL);
> -	if (!rtk) {
> -		ret = -ENOMEM;
> -		goto out;
> -	}
> +	if (!rtk)
> +		return -ENOMEM;
>  
>  	platform_set_drvdata(pdev, rtk);
>  
>  	rtk->dev = dev;
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		dev_err(dev, "missing memory resource\n");
> -		ret = -ENODEV;
> -		goto out;
> -	}
> -
> -	regs = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(regs)) {
> -		ret = PTR_ERR(regs);
> -		goto out;
> -	}
> +	regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
> +	if (IS_ERR(regs))
> +		return PTR_ERR(regs);
>  
>  	rtk->regs = regs;
>  	rtk->regs_size = resource_size(res);
> @@ -389,16 +377,11 @@ static int dwc3_rtk_probe(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	if (res) {
>  		rtk->pm_base = devm_ioremap_resource(dev, res);
> -		if (IS_ERR(rtk->pm_base)) {
> -			ret = PTR_ERR(rtk->pm_base);
> -			goto out;
> -		}
> +		if (IS_ERR(rtk->pm_base))
> +			return PTR_ERR(rtk->pm_base);
>  	}
>  
> -	ret = dwc3_rtk_probe_dwc3_core(rtk);
> -
> -out:
> -	return ret;
> +	return dwc3_rtk_probe_dwc3_core(rtk);
>  }
>  
>  static void dwc3_rtk_remove(struct platform_device *pdev)
> 
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

Re: [PATCH 11/11] usb: dwc3: xilinx: simplify with dev_err_probe

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Use dev_err_probe() to make the error paths a bit simpler.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-xilinx.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
> index b7613a106da6..eb535733ce91 100644
> --- a/drivers/usb/dwc3/dwc3-xilinx.c
> +++ b/drivers/usb/dwc3/dwc3-xilinx.c
> @@ -285,11 +285,8 @@ static int dwc3_xlnx_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	regs = devm_platform_ioremap_resource(pdev, 0);
> -	if (IS_ERR(regs)) {
> -		ret = PTR_ERR(regs);
> -		dev_err_probe(dev, ret, "failed to map registers\n");
> -		return ret;
> -	}
> +	if (IS_ERR(regs))
> +		return dev_err_probe(dev, PTR_ERR(regs), "failed to map registers\n");
>  
>  	match = of_match_node(dwc3_xlnx_of_match, pdev->dev.of_node);
>  
> 
> -- 
> 2.43.0
> 

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

Thanks,
Thinh

Re: [PATCH 00/11] usb: dwc3: various cleanups

[Thinh Nguyen]

Hi Krzysztof,

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Hi,
> 
> The first ST patch depends on my fix:
> https://urldefense.com/v3/__https://lore.kernel.org/all/20240814093957.37940-2-krzysztof.kozlowski@linaro.org/__;!!A4F2R9G_pg!eFzt54pYSEl8wrvrH7yQSwFRzbojnRSyelnYjxlY8RZaU6oZCVeui2f-DHQ0bk8Fdy6gvJoSWLeAPz2w_3F9ownOFLGUoyHd$ 
> 
> Series makes some code simplifications and cleanups.
> 
> Best regards,
> Krzysztof
> 
> ---
> Krzysztof Kozlowski (11):
>       usb: dwc3: st: use scoped device node handling to simplify error paths
>       usb: dwc3: st: simplify with dev_err_probe
>       usb: dwc3: st: simplify pdev->dev usage
>       usb: dwc3: imx8mp: simplify with devm_clk_get_enabled
>       usb: dwc3: imx8mp: simplify with dev_err_probe
>       usb: dwc3: imx8mp: use scoped device node handling to simplify error paths
>       usb: dwc3: qcom: use scoped device node handling to simplify error paths
>       usb: dwc3: qcom: simplify with devm_platform_ioremap_resource
>       usb: dwc3: rtk: use scoped device node handling to simplify error paths
>       usb: dwc3: rtk: return directly and simplify with devm_platform_ioremap_resource
>       usb: dwc3: xilinx: simplify with dev_err_probe
> 
>  drivers/usb/dwc3/dwc3-imx8mp.c | 66 ++++++++++++------------------------------
>  drivers/usb/dwc3/dwc3-qcom.c   | 16 ++++------
>  drivers/usb/dwc3/dwc3-rtk.c    | 52 ++++++++++-----------------------
>  drivers/usb/dwc3/dwc3-st.c     | 38 +++++++++++-------------
>  drivers/usb/dwc3/dwc3-xilinx.c |  7 ++---
>  5 files changed, 58 insertions(+), 121 deletions(-)
> ---
> base-commit: 64b429eaf21be888cc83e9013e25897d5fb03a75
> change-id: 20240814-b4-cleanup-h-of-node-put-usb-93fadfc77d33
> 
> Best regards,
> -- 
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 

Thanks for the cleanup!

I wish the mixed declarations in between statements for some of the
scoped device node handling can be changed. Bugs me a little with how
I'm used to parse the old standard with my eyes, but it's not a big
issue.

BR,
Thinh

Re: [PATCH 02/11] usb: dwc3: st: simplify with dev_err_probe

[Krzysztof Kozlowski]

On 27/08/2024 03:19, Thinh Nguyen wrote:
> On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
>> Use dev_err_probe() to make the error paths a bit simpler.
> 
> I think it makes more sense to note that this helps with cases of
> -EPROBE_DEFER than making this simpler. Regardless, this is an
> improvement.
> 
> Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Yeah, I forgot about this argument. Getting resets can defer, so this
actually solves the dmesg flood for deferred probes.

Best regards,
Krzysztof

Re: [PATCH 00/11] usb: dwc3: various cleanups

[Krzysztof Kozlowski]

On 27/08/2024 03:59, Thinh Nguyen wrote:
> Hi Krzysztof,
> 
> On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
>> Hi,
>>
>> The first ST patch depends on my fix:
>> https://urldefense.com/v3/__https://lore.kernel.org/all/20240814093957.37940-2-krzysztof.kozlowski@linaro.org/__;!!A4F2R9G_pg!eFzt54pYSEl8wrvrH7yQSwFRzbojnRSyelnYjxlY8RZaU6oZCVeui2f-DHQ0bk8Fdy6gvJoSWLeAPz2w_3F9ownOFLGUoyHd$ 
>>
>> Series makes some code simplifications and cleanups.
>>
>> Best regards,
>> Krzysztof
>>
>> ---
>> Krzysztof Kozlowski (11):
>>       usb: dwc3: st: use scoped device node handling to simplify error paths
>>       usb: dwc3: st: simplify with dev_err_probe
>>       usb: dwc3: st: simplify pdev->dev usage
>>       usb: dwc3: imx8mp: simplify with devm_clk_get_enabled
>>       usb: dwc3: imx8mp: simplify with dev_err_probe
>>       usb: dwc3: imx8mp: use scoped device node handling to simplify error paths
>>       usb: dwc3: qcom: use scoped device node handling to simplify error paths
>>       usb: dwc3: qcom: simplify with devm_platform_ioremap_resource
>>       usb: dwc3: rtk: use scoped device node handling to simplify error paths
>>       usb: dwc3: rtk: return directly and simplify with devm_platform_ioremap_resource
>>       usb: dwc3: xilinx: simplify with dev_err_probe
>>
>>  drivers/usb/dwc3/dwc3-imx8mp.c | 66 ++++++++++++------------------------------
>>  drivers/usb/dwc3/dwc3-qcom.c   | 16 ++++------
>>  drivers/usb/dwc3/dwc3-rtk.c    | 52 ++++++++++-----------------------
>>  drivers/usb/dwc3/dwc3-st.c     | 38 +++++++++++-------------
>>  drivers/usb/dwc3/dwc3-xilinx.c |  7 ++---
>>  5 files changed, 58 insertions(+), 121 deletions(-)
>> ---
>> base-commit: 64b429eaf21be888cc83e9013e25897d5fb03a75
>> change-id: 20240814-b4-cleanup-h-of-node-put-usb-93fadfc77d33
>>
>> Best regards,
>> -- 
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
> 
> Thanks for the cleanup!
> 
> I wish the mixed declarations in between statements for some of the
> scoped device node handling can be changed. Bugs me a little with how
> I'm used to parse the old standard with my eyes, but it's not a big
> issue.

Maybe this will be helpful:


https://lore.kernel.org/all/CAHk-=wicfvWPuRVDG5R1mZSxD8Xg=-0nLOiHay2T_UJ0yDX42g@mail.gmail.com/

https://lore.kernel.org/all/CAHk-=wgRHiV5VSxtfXA4S6aLUmcQYEuB67u3BJPJPtuESs1JyA@mail.gmail.com/

https://lore.kernel.org/all/CAHk-=whvOGL3aNhtps0YksGtzvaob_bvZpbaTcVEqGwNMxB6xg@mail.gmail.com/

and finally it will reach documentation (although it focuses on
unwinding process to be specific - "When the unwind order ..."):
https://lore.kernel.org/all/171175585714.2192972.12661675876300167762.stgit@dwillia2-xfh.jf.intel.com/

Best regards,
Krzysztof

Re: [PATCH 00/11] usb: dwc3: various cleanups

[Thinh Nguyen]

On Tue, Aug 27, 2024, Krzysztof Kozlowski wrote:
> On 27/08/2024 03:59, Thinh Nguyen wrote:
> > Hi Krzysztof,
> > 
> > On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> >> Hi,
> >>
> >> The first ST patch depends on my fix:
> >> https://urldefense.com/v3/__https://lore.kernel.org/all/20240814093957.37940-2-krzysztof.kozlowski@linaro.org/__;!!A4F2R9G_pg!eFzt54pYSEl8wrvrH7yQSwFRzbojnRSyelnYjxlY8RZaU6oZCVeui2f-DHQ0bk8Fdy6gvJoSWLeAPz2w_3F9ownOFLGUoyHd$ 
> >>
> >> Series makes some code simplifications and cleanups.
> >>
> >> Best regards,
> >> Krzysztof
> >>
> >> ---
> >> Krzysztof Kozlowski (11):
> >>       usb: dwc3: st: use scoped device node handling to simplify error paths
> >>       usb: dwc3: st: simplify with dev_err_probe
> >>       usb: dwc3: st: simplify pdev->dev usage
> >>       usb: dwc3: imx8mp: simplify with devm_clk_get_enabled
> >>       usb: dwc3: imx8mp: simplify with dev_err_probe
> >>       usb: dwc3: imx8mp: use scoped device node handling to simplify error paths
> >>       usb: dwc3: qcom: use scoped device node handling to simplify error paths
> >>       usb: dwc3: qcom: simplify with devm_platform_ioremap_resource
> >>       usb: dwc3: rtk: use scoped device node handling to simplify error paths
> >>       usb: dwc3: rtk: return directly and simplify with devm_platform_ioremap_resource
> >>       usb: dwc3: xilinx: simplify with dev_err_probe
> >>
> >>  drivers/usb/dwc3/dwc3-imx8mp.c | 66 ++++++++++++------------------------------
> >>  drivers/usb/dwc3/dwc3-qcom.c   | 16 ++++------
> >>  drivers/usb/dwc3/dwc3-rtk.c    | 52 ++++++++++-----------------------
> >>  drivers/usb/dwc3/dwc3-st.c     | 38 +++++++++++-------------
> >>  drivers/usb/dwc3/dwc3-xilinx.c |  7 ++---
> >>  5 files changed, 58 insertions(+), 121 deletions(-)
> >> ---
> >> base-commit: 64b429eaf21be888cc83e9013e25897d5fb03a75
> >> change-id: 20240814-b4-cleanup-h-of-node-put-usb-93fadfc77d33
> >>
> >> Best regards,
> >> -- 
> >> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>
> > 
> > Thanks for the cleanup!
> > 
> > I wish the mixed declarations in between statements for some of the
> > scoped device node handling can be changed. Bugs me a little with how
> > I'm used to parse the old standard with my eyes, but it's not a big
> > issue.
> 
> Maybe this will be helpful:
> 
> 
> https://urldefense.com/v3/__https://lore.kernel.org/all/CAHk-=wicfvWPuRVDG5R1mZSxD8Xg=-0nLOiHay2T_UJ0yDX42g@mail.gmail.com/__;!!A4F2R9G_pg!YvWQyaH3G00Drln91_0r1t-Uz3rbturjHMQZCCITwC6fpBh3C_PZaROxmqbsZDiGRCddw2Qo8ZuAKQaadWBkyN7Now0EO78z$ 
> 
> https://urldefense.com/v3/__https://lore.kernel.org/all/CAHk-=wgRHiV5VSxtfXA4S6aLUmcQYEuB67u3BJPJPtuESs1JyA@mail.gmail.com/__;!!A4F2R9G_pg!YvWQyaH3G00Drln91_0r1t-Uz3rbturjHMQZCCITwC6fpBh3C_PZaROxmqbsZDiGRCddw2Qo8ZuAKQaadWBkyN7No5Q5NOqo$ 
> 
> https://urldefense.com/v3/__https://lore.kernel.org/all/CAHk-=whvOGL3aNhtps0YksGtzvaob_bvZpbaTcVEqGwNMxB6xg@mail.gmail.com/__;!!A4F2R9G_pg!YvWQyaH3G00Drln91_0r1t-Uz3rbturjHMQZCCITwC6fpBh3C_PZaROxmqbsZDiGRCddw2Qo8ZuAKQaadWBkyN7NozMgso_e$ 
> 
> and finally it will reach documentation (although it focuses on
> unwinding process to be specific - "When the unwind order ..."):
> https://urldefense.com/v3/__https://lore.kernel.org/all/171175585714.2192972.12661675876300167762.stgit@dwillia2-xfh.jf.intel.com/__;!!A4F2R9G_pg!YvWQyaH3G00Drln91_0r1t-Uz3rbturjHMQZCCITwC6fpBh3C_PZaROxmqbsZDiGRCddw2Qo8ZuAKQaadWBkyN7No9_dTm9A$ 
> 

Thanks for the links. That's good to know.

BR,
Thinh