| Message ID | 20201109095953.7f810239@xhacker.debian |
|---|---|
| State | New |
| Headers | show |
| Series | [usb-next] usb: dwc3: Use devm_of_platform_populate | expand |
On Mon, Nov 09, 2020 at 09:59:53AM +0800, Jisheng Zhang wrote: > diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c > index 417e05381b5d..83015bb7b926 100644 > --- a/drivers/usb/dwc3/dwc3-meson-g12a.c > +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c > @@ -702,7 +702,6 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev) > { > struct dwc3_meson_g12a *priv; > struct device *dev = &pdev->dev; > - struct device_node *np = dev->of_node; > void __iomem *base; > int ret, i; > > @@ -794,7 +793,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev) > goto err_phys_power; > } > > - ret = of_platform_populate(np, NULL, NULL, dev); > + ret = devm_of_platform_populate(dev); > if (ret) > goto err_phys_power; > > @@ -832,8 +831,6 @@ static int dwc3_meson_g12a_remove(struct platform_device *pdev) > if (priv->drvdata->otg_switch_supported) > usb_role_switch_unregister(priv->role_switch); > > - of_platform_depopulate(dev); > - > for (i = 0 ; i < PHY_COUNT ; ++i) { > phy_power_off(priv->phys[i]); > phy_exit(priv->phys[i]); Does it matter that the order that things happen in dwc3_meson_g12a_remove() is changed as a result of your patch? Was the code relying on the platform devices being depopulated before powering off the PHYs? > diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c > index e62ecd22b3ed..f1c267e39d62 100644 > --- a/drivers/usb/dwc3/dwc3-of-simple.c > +++ b/drivers/usb/dwc3/dwc3-of-simple.c > @@ -73,7 +73,7 @@ static int dwc3_of_simple_probe(struct platform_device *pdev) > if (ret) > goto err_resetc_assert; > > - ret = of_platform_populate(np, NULL, NULL, dev); > + ret = devm_of_platform_populate(dev); > if (ret) > goto err_clk_put; > > @@ -97,8 +97,6 @@ static int dwc3_of_simple_probe(struct platform_device *pdev) > > static void __dwc3_of_simple_teardown(struct dwc3_of_simple *simple) > { > - of_platform_depopulate(simple->dev); > - > clk_bulk_disable_unprepare(simple->num_clocks, simple->clks); > clk_bulk_put_all(simple->num_clocks, simple->clks); > simple->num_clocks = 0; Same here... and for anywhere else in this patch that you're deleting a of_platform_depopulate(). You effectively are moving the call to of_platform_depopulate() *after* the driver's .remove function has been called.
On Mon, 9 Nov 2020 10:34:14 +0000 Russell King - ARM Linux admin wrote: > > > On Mon, Nov 09, 2020 at 09:59:53AM +0800, Jisheng Zhang wrote: > > diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c > > index 417e05381b5d..83015bb7b926 100644 > > --- a/drivers/usb/dwc3/dwc3-meson-g12a.c > > +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c > > @@ -702,7 +702,6 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev) > > { > > struct dwc3_meson_g12a *priv; > > struct device *dev = &pdev->dev; > > - struct device_node *np = dev->of_node; > > void __iomem *base; > > int ret, i; > > > > @@ -794,7 +793,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev) > > goto err_phys_power; > > } > > > > - ret = of_platform_populate(np, NULL, NULL, dev); > > + ret = devm_of_platform_populate(dev); > > if (ret) > > goto err_phys_power; > > > > @@ -832,8 +831,6 @@ static int dwc3_meson_g12a_remove(struct platform_device *pdev) > > if (priv->drvdata->otg_switch_supported) > > usb_role_switch_unregister(priv->role_switch); > > > > - of_platform_depopulate(dev); > > - > > for (i = 0 ; i < PHY_COUNT ; ++i) { > > phy_power_off(priv->phys[i]); > > phy_exit(priv->phys[i]); > > Does it matter that the order that things happen in > dwc3_meson_g12a_remove() is changed as a result of your patch? Was > the code relying on the platform devices being depopulated before > powering off the PHYs? ... > > Same here... and for anywhere else in this patch that you're deleting > a of_platform_depopulate(). oops, good question! I believe it's not safe to move the depopulate after the phy_* APIs. So please ignore this patch. > > You effectively are moving the call to of_platform_depopulate() *after* > the driver's .remove function has been called. > > -- >
diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c index 90bb022737da..f567d1e63c05 100644 --- a/drivers/usb/dwc3/dwc3-exynos.c +++ b/drivers/usb/dwc3/dwc3-exynos.c @@ -110,7 +110,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev) } if (node) { - ret = of_platform_populate(node, NULL, NULL, dev); + ret = devm_of_platform_populate(dev); if (ret) { dev_err(dev, "failed to add dwc3 core\n"); goto populate_err; diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c index 9a99253d5ba3..626b16c46688 100644 --- a/drivers/usb/dwc3/dwc3-keystone.c +++ b/drivers/usb/dwc3/dwc3-keystone.c @@ -157,7 +157,7 @@ static int kdwc3_probe(struct platform_device *pdev) kdwc3_enable_irqs(kdwc); skip_irq: - error = of_platform_populate(node, NULL, NULL, dev); + error = devm_of_platform_populate(dev); if (error) { dev_err(&pdev->dev, "failed to create dwc3 core\n"); goto err_core; diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c index 417e05381b5d..83015bb7b926 100644 --- a/drivers/usb/dwc3/dwc3-meson-g12a.c +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c @@ -702,7 +702,6 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev) { struct dwc3_meson_g12a *priv; struct device *dev = &pdev->dev; - struct device_node *np = dev->of_node; void __iomem *base; int ret, i; @@ -794,7 +793,7 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev) goto err_phys_power; } - ret = of_platform_populate(np, NULL, NULL, dev); + ret = devm_of_platform_populate(dev); if (ret) goto err_phys_power; @@ -832,8 +831,6 @@ static int dwc3_meson_g12a_remove(struct platform_device *pdev) if (priv->drvdata->otg_switch_supported) usb_role_switch_unregister(priv->role_switch); - of_platform_depopulate(dev); - for (i = 0 ; i < PHY_COUNT ; ++i) { phy_power_off(priv->phys[i]); phy_exit(priv->phys[i]); diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c index e62ecd22b3ed..f1c267e39d62 100644 --- a/drivers/usb/dwc3/dwc3-of-simple.c +++ b/drivers/usb/dwc3/dwc3-of-simple.c @@ -73,7 +73,7 @@ static int dwc3_of_simple_probe(struct platform_device *pdev) if (ret) goto err_resetc_assert; - ret = of_platform_populate(np, NULL, NULL, dev); + ret = devm_of_platform_populate(dev); if (ret) goto err_clk_put; @@ -97,8 +97,6 @@ static int dwc3_of_simple_probe(struct platform_device *pdev) static void __dwc3_of_simple_teardown(struct dwc3_of_simple *simple) { - of_platform_depopulate(simple->dev); - clk_bulk_disable_unprepare(simple->num_clocks, simple->clks); clk_bulk_put_all(simple->num_clocks, simple->clks); simple->num_clocks = 0; diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 3db17806e92e..46e7a1dd7c50 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -505,7 +505,7 @@ static int dwc3_omap_probe(struct platform_device *pdev) if (ret < 0) goto err1; - ret = of_platform_populate(node, NULL, NULL, dev); + ret = devm_of_platform_populate(dev); if (ret) { dev_err(&pdev->dev, "failed to create dwc3 core\n"); goto err1; @@ -535,7 +535,6 @@ static int dwc3_omap_remove(struct platform_device *pdev) dwc3_omap_disable_irqs(omap); disable_irq(omap->irq); - of_platform_depopulate(omap->dev); pm_runtime_put_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c index c703d552bbcf..8f5714c3d379 100644 --- a/drivers/usb/dwc3/dwc3-qcom.c +++ b/drivers/usb/dwc3/dwc3-qcom.c @@ -636,7 +636,7 @@ static int dwc3_qcom_of_register_core(struct platform_device *pdev) return -ENODEV; } - ret = of_platform_populate(np, NULL, NULL, dev); + ret = devm_of_platform_populate(dev); if (ret) { dev_err(dev, "failed to register dwc3 core - %d\n", ret); return ret; @@ -775,9 +775,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev) interconnect_exit: dwc3_qcom_interconnect_exit(qcom); depopulate: - if (np) - of_platform_depopulate(&pdev->dev); - else + if (!np) platform_device_put(pdev); clk_disable: for (i = qcom->num_clocks - 1; i >= 0; i--) { @@ -796,8 +794,6 @@ static int dwc3_qcom_remove(struct platform_device *pdev) struct device *dev = &pdev->dev; int i; - of_platform_depopulate(dev); - for (i = qcom->num_clocks - 1; i >= 0; i--) { clk_disable_unprepare(qcom->clks[i]); clk_put(qcom->clks[i]); diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c index e733be840545..9ec2ecf8d81f 100644 --- a/drivers/usb/dwc3/dwc3-st.c +++ b/drivers/usb/dwc3/dwc3-st.c @@ -259,7 +259,7 @@ static int st_dwc3_probe(struct platform_device *pdev) } /* Allocate and initialize the core */ - ret = of_platform_populate(node, NULL, NULL, dev); + ret = devm_of_platform_populate(dev); if (ret) { dev_err(dev, "failed to add dwc3 core\n"); goto err_node_put; @@ -309,8 +309,6 @@ static int st_dwc3_remove(struct platform_device *pdev) { struct st_dwc3 *dwc3_data = platform_get_drvdata(pdev); - of_platform_depopulate(&pdev->dev); - reset_control_assert(dwc3_data->rstc_pwrdn); reset_control_assert(dwc3_data->rstc_rst);
Use managed API devm_of_platform_populate() to simplify error and exit code path. Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com> --- drivers/usb/dwc3/dwc3-exynos.c | 2 +- drivers/usb/dwc3/dwc3-keystone.c | 2 +- drivers/usb/dwc3/dwc3-meson-g12a.c | 5 +---- drivers/usb/dwc3/dwc3-of-simple.c | 4 +--- drivers/usb/dwc3/dwc3-omap.c | 3 +-- drivers/usb/dwc3/dwc3-qcom.c | 8 ++------ drivers/usb/dwc3/dwc3-st.c | 4 +--- 7 files changed, 8 insertions(+), 20 deletions(-)