Use new helper function for clock and PM macro

PATCH In review Anand Moon 2 participants 8 messages Apr 21, 2024 – May 7, 2024

Subsystems (5)

USB Arm Samsung Exynos DesignWare USB3 DRD IP driver USB ehci driver +1 more subsystems

lore

Share & tools

lore

Anand Moon

author

2024-04-21 13:48 UTC #0 lore raw

v4 cover Use new helper function for clock and PM macro

using new helper functions for dev resources for clock and PM to simplify the code changes.

Addressed some review comments.

v4: Add Reviewed-by: Alan Stern For patches 1 - 4:
Add Acked-by: Thinh Nguyen For patch 5 Fix rephase commit messaeg for patch 2 and 4 Fix some typo in the commit messagee

v3 changes:
1 Use new DEFINE_SIMPLE_DEV_PM_OPS macro for PM operations
Thanks to Thinh Nguyen for your inputs.
so I have to update the $subject and commit messagee with using new macro.

2 Drop the dev_err_probe in return to simplify the error for clocks.

3 Dop the devm_regulator_bulk_get_enable patch.

Tests on Odroid XU4 and Odroid U3.
found no regression with suspend resume functionality.

Previous changes:
V3: https://lore.kernel.org/all/[email protected]/

V2: https://lore.kernel.org/all/[email protected]/

V1: https://patchwork.kernel.org/project/linux-samsung-soc/patch/[email protected]/

Thanks
-Anand

Anand Moon (5):
usb: ehci-exynos: Use devm_clk_get_enabled() helpers
usb: ehci-exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions
usb: ohci-exynos: Use devm_clk_get_enabled() helpers
usb: ohci-exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions
usb: dwc3: exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions

 drivers/usb/dwc3/dwc3-exynos.c | 13 +++----------
 drivers/usb/host/ehci-exynos.c | 27 ++++++---------------------
 drivers/usb/host/ohci-exynos.c | 27 ++++++---------------------
 3 files changed, 15 insertions(+), 52 deletions(-)

Signature · 1 line

2.44.0

Anand Moon

author

2024-04-21 13:48 UTC #1 lore raw

PATCH v4 1/5 usb: ehci-exynos: Use devm_clk_get_enabled() helpers

The devm_clk_get_enabled() helpers:
- call devm_clk_get()
- call clk_prepare_enable() and register what is needed in order to call clk_disable_unprepare() when needed, as a managed resource.

This simplifies the code and avoids the calls to clk_disable_unprepare().

Reviewed-by Alan Stern <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
v4: Add Rb Alan Stern

v3: drop dev_err_probe to simplify the error handle.

v2: drop the clk_disable_unprepare in suspend/resume functions
fix the usb_put_hcd return before the devm_clk_get_enabled

---
 drivers/usb/host/ehci-exynos.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

drivers/usb/host/ehci-exynos.c +3 ~~-11~~

index f644b131cc0b..e2303757bc0f 100644
@@ -159,20 +159,16 @@ static int exynos_ehci_probe(struct platform_device *pdev)
 
 	err = exynos_ehci_get_phy(&pdev->dev, exynos_ehci);
 	if (err)
-		goto fail_clk;
+		goto fail_io;
 
-	exynos_ehci->clk = devm_clk_get(&pdev->dev, "usbhost");
+	exynos_ehci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost");
 
 	if (IS_ERR(exynos_ehci->clk)) {
 		dev_err(&pdev->dev, "Failed to get usbhost clock\n");
 		err = PTR_ERR(exynos_ehci->clk);
-		goto fail_clk;
+		goto fail_io;
 	}
 
-	err = clk_prepare_enable(exynos_ehci->clk);
-	if (err)
-		goto fail_clk;
-
 	hcd->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
 	if (IS_ERR(hcd->regs)) {
 		err = PTR_ERR(hcd->regs);

@@ -223,8 +219,6 @@ static int exynos_ehci_probe(struct platform_device *pdev)
 	exynos_ehci_phy_disable(&pdev->dev);
 	pdev->dev.of_node = exynos_ehci->of_node;
 fail_io:
-	clk_disable_unprepare(exynos_ehci->clk);
-fail_clk:
 	usb_put_hcd(hcd);
 	return err;
 }

@@ -240,8 +234,6 @@ static void exynos_ehci_remove(struct platform_device *pdev)
 
 	exynos_ehci_phy_disable(&pdev->dev);
 
-	clk_disable_unprepare(exynos_ehci->clk);
-
 	usb_put_hcd(hcd);
 }

Signature · 1 line

2.44.0

Anand Moon

author

2024-04-21 13:48 UTC #2 lore raw

PATCH v4 2/5 usb: ehci-exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions

In reply to Anand Moon

Use DEFINE_SIMPLE_DEV_PM_OPS macro to progressively switch from a code model where PM callbacks are all protected behind CONFIG_PM guards, to a code model where the PM callbacks are always seen by the compiler, but discarded if not used.

Reviewed-by Alan Stern <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
v4: rephrase commit message
add reviewed by RB Alan Stern

v3: fix using new DEFINE_SIMPLE_DEV_PM_OPS PM macro hence
change the $subject and the commit message

v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM
is disabled.

---
 drivers/usb/host/ehci-exynos.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

drivers/usb/host/ehci-exynos.c +3 ~~-10~~

index e2303757bc0f..f40bc2a7a124 100644
@@ -237,7 +237,6 @@ static void exynos_ehci_remove(struct platform_device *pdev)
 	usb_put_hcd(hcd);
 }
 
-#ifdef CONFIG_PM
 static int exynos_ehci_suspend(struct device *dev)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);

@@ -280,15 +279,9 @@ static int exynos_ehci_resume(struct device *dev)
 	ehci_resume(hcd, false);
 	return 0;
 }
-#else
-#define exynos_ehci_suspend	NULL
-#define exynos_ehci_resume	NULL
-#endif
 
-static const struct dev_pm_ops exynos_ehci_pm_ops = {
-	.suspend	= exynos_ehci_suspend,
-	.resume		= exynos_ehci_resume,
-};
+static DEFINE_SIMPLE_DEV_PM_OPS(exynos_ehci_pm_ops,
+				exynos_ehci_suspend, exynos_ehci_resume);
 
 #ifdef CONFIG_OF
 static const struct of_device_id exynos_ehci_match[] = {

@@ -304,7 +297,7 @@ static struct platform_driver exynos_ehci_driver = {
 	.shutdown	= usb_hcd_platform_shutdown,
 	.driver = {
 		.name	= "exynos-ehci",
-		.pm	= &exynos_ehci_pm_ops,
+		.pm	= pm_ptr(&exynos_ehci_pm_ops),
 		.of_match_table = of_match_ptr(exynos_ehci_match),
 	}
 };

Signature · 1 line

2.44.0

Anand Moon

author

2024-04-21 13:48 UTC #3 lore raw

PATCH v4 3/5 usb: ohci-exynos: Use devm_clk_get_enabled() helpers

In reply to Anand Moon

The devm_clk_get_enabled() helpers:
- call devm_clk_get()
- call clk_prepare_enable() and register what is needed in order to call clk_disable_unprepare() when needed, as a managed resource.

This simplifies the code and avoids the calls to clk_disable_unprepare().

Reviewed-by Alan Stern <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
v4 : Add reviewed by Alan Stern

v3: drop dev_err_probe to simplify the error handle.

v2: new patch in this series

---
 drivers/usb/host/ohci-exynos.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

drivers/usb/host/ohci-exynos.c +3 ~~-11~~

index 20e26a474591..89e6587c089b 100644
@@ -135,20 +135,16 @@ static int exynos_ohci_probe(struct platform_device *pdev)
 
 	err = exynos_ohci_get_phy(&pdev->dev, exynos_ohci);
 	if (err)
-		goto fail_clk;
+		goto fail_io;
 
-	exynos_ohci->clk = devm_clk_get(&pdev->dev, "usbhost");
+	exynos_ohci->clk = devm_clk_get_enabled(&pdev->dev, "usbhost");
 
 	if (IS_ERR(exynos_ohci->clk)) {
 		dev_err(&pdev->dev, "Failed to get usbhost clock\n");
 		err = PTR_ERR(exynos_ohci->clk);
-		goto fail_clk;
+		goto fail_io;
 	}
 
-	err = clk_prepare_enable(exynos_ohci->clk);
-	if (err)
-		goto fail_clk;
-
 	hcd->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
 	if (IS_ERR(hcd->regs)) {
 		err = PTR_ERR(hcd->regs);

@@ -191,8 +187,6 @@ static int exynos_ohci_probe(struct platform_device *pdev)
 	exynos_ohci_phy_disable(&pdev->dev);
 	pdev->dev.of_node = exynos_ohci->of_node;
 fail_io:
-	clk_disable_unprepare(exynos_ohci->clk);
-fail_clk:
 	usb_put_hcd(hcd);
 	return err;
 }

@@ -208,8 +202,6 @@ static void exynos_ohci_remove(struct platform_device *pdev)
 
 	exynos_ohci_phy_disable(&pdev->dev);
 
-	clk_disable_unprepare(exynos_ohci->clk);
-
 	usb_put_hcd(hcd);
 }

Signature · 1 line

2.44.0

Anand Moon

author

2024-04-21 13:48 UTC #4 lore raw

PATCH v4 4/5 usb: ohci-exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions

In reply to Anand Moon

Reviewed-by Alan Stern <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
v4: rephrase commit message
add reviewed by RB Alan Stern

v3: fix using new DEFINE_SIMPLE_DEV_PM_OPS PM macro hence
change the $subject and the commit message

v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM
is disabled.

---
 drivers/usb/host/ohci-exynos.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

drivers/usb/host/ohci-exynos.c +3 ~~-10~~

index 89e6587c089b..3c4d68fd5c33 100644
@@ -213,7 +213,6 @@ static void exynos_ohci_shutdown(struct platform_device *pdev)
 		hcd->driver->shutdown(hcd);
 }
 
-#ifdef CONFIG_PM
 static int exynos_ohci_suspend(struct device *dev)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);

@@ -250,19 +249,13 @@ static int exynos_ohci_resume(struct device *dev)
 
 	return 0;
 }
-#else
-#define exynos_ohci_suspend	NULL
-#define exynos_ohci_resume	NULL
-#endif
 
 static const struct ohci_driver_overrides exynos_overrides __initconst = {
 	.extra_priv_size =	sizeof(struct exynos_ohci_hcd),
 };
 
-static const struct dev_pm_ops exynos_ohci_pm_ops = {
-	.suspend	= exynos_ohci_suspend,
-	.resume		= exynos_ohci_resume,
-};
+static DEFINE_SIMPLE_DEV_PM_OPS(exynos_ohci_pm_ops,
+				exynos_ohci_suspend, exynos_ohci_resume);
 
 #ifdef CONFIG_OF
 static const struct of_device_id exynos_ohci_match[] = {

@@ -278,7 +271,7 @@ static struct platform_driver exynos_ohci_driver = {
 	.shutdown	= exynos_ohci_shutdown,
 	.driver = {
 		.name	= "exynos-ohci",
-		.pm	= &exynos_ohci_pm_ops,
+		.pm	= pm_ptr(&exynos_ohci_pm_ops),
 		.of_match_table	= of_match_ptr(exynos_ohci_match),
 	}
 };

Signature · 1 line

2.44.0

Anand Moon

author

2024-04-21 13:48 UTC #5 lore raw

PATCH v4 5/5 usb: dwc3: exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions

In reply to Anand Moon

This macro has the advantage over SET_SYSTEM_SLEEP_PM_OPS that we don't have to care about when the functions are actually used.

Also make use of pm_sleep_ptr() to discard all PM_SLEEP related stuff if CONFIG_PM_SLEEP isn't enabled.

Acked-by Thinh Nguyen <[email protected]>

Signed-off-by Anand Moon <[email protected]>

---
v4 Fix typo in commit message SIMPLE_DEV_PM_OPS to
SET_SYSTEM_SLEEP_PM_OPS
Add Acked by Thinh Nguyen

v3: fix using new DEFINE_SIMPLE_DEV_PM_OPS PM macro hence
change the $subject and the commit message

v2: add __maybe_unused to suspend/resume functions in case CONFIG_PM
is disabled.

---
 drivers/usb/dwc3/dwc3-exynos.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

drivers/usb/dwc3/dwc3-exynos.c +3 ~~-10~~

index 5d365ca51771..3427522a7c6a 100644
@@ -187,7 +187,6 @@ static const struct of_device_id exynos_dwc3_match[] = {
 };
 MODULE_DEVICE_TABLE(of, exynos_dwc3_match);
 
-#ifdef CONFIG_PM_SLEEP
 static int dwc3_exynos_suspend(struct device *dev)
 {
 	struct dwc3_exynos *exynos = dev_get_drvdata(dev);

@@ -230,14 +229,8 @@ static int dwc3_exynos_resume(struct device *dev)
 	return 0;
 }
 
-static const struct dev_pm_ops dwc3_exynos_dev_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(dwc3_exynos_suspend, dwc3_exynos_resume)
-};
-
-#define DEV_PM_OPS	(&dwc3_exynos_dev_pm_ops)
-#else
-#define DEV_PM_OPS	NULL
-#endif /* CONFIG_PM_SLEEP */
+static DEFINE_SIMPLE_DEV_PM_OPS(dwc3_exynos_dev_pm_ops,
+				dwc3_exynos_suspend, dwc3_exynos_resume);
 
 static struct platform_driver dwc3_exynos_driver = {
 	.probe		= dwc3_exynos_probe,

@@ -245,7 +238,7 @@ static struct platform_driver dwc3_exynos_driver = {
 	.driver		= {
 		.name	= "exynos-dwc3",
 		.of_match_table = exynos_dwc3_match,
-		.pm	= DEV_PM_OPS,
+		.pm	= pm_sleep_ptr(&dwc3_exynos_dev_pm_ops),
 	},
 };

Signature · 1 line

2.44.0

Thinh Nguyen

maintainer

2024-04-24 01:21 UTC #6 lore raw

In reply to Anand Moon

Quoted 40 lines · Anand Moon

using new helper functions for dev resources for clock and PM to simplify the code changes.
Addressed some review comments.
v4: Add Reviewed-by: Alan Stern For patches 1 - 4:
Add Acked-by: Thinh Nguyen For patch 5 Fix rephase commit messaeg for patch 2 and 4 Fix some typo in the commit messagee
v3 changes:
1 Use new DEFINE_SIMPLE_DEV_PM_OPS macro for PM operations
Thanks to Thinh Nguyen for your inputs.
so I have to update the $subject and commit messagee with using new macro.
2 Drop the dev_err_probe in return to simplify the error for clocks.
3 Dop the devm_regulator_bulk_get_enable patch.
Tests on Odroid XU4 and Odroid U3.
found no regression with suspend resume functionality.
Previous changes:
V3: https://urldefense.com/v3/__https://lore.kernel.org/all/[email protected]/__;!!A4F2R9G_pg!b2EBzV-UTfuvwSmse-8kl4jsq8t4KgBPT_G--QCG_TDV4atcywVQgLHPFGmxGnPASOKOmp6r0xyI2DnUnRp_cur6Ew$
V2: https://urldefense.com/v3/__https://lore.kernel.org/all/[email protected]/__;!!A4F2R9G_pg!b2EBzV-UTfuvwSmse-8kl4jsq8t4KgBPT_G--QCG_TDV4atcywVQgLHPFGmxGnPASOKOmp6r0xyI2DnUnRqoIl7uEA$
V1: https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-samsung-soc/patch/[email protected]/__;!!A4F2R9G_pg!b2EBzV-UTfuvwSmse-8kl4jsq8t4KgBPT_G--QCG_TDV4atcywVQgLHPFGmxGnPASOKOmp6r0xyI2DnUnRpJKM0R2g$
Thanks
-Anand
Anand Moon (5):
usb: ehci-exynos: Use devm_clk_get_enabled() helpers
usb: ehci-exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions
usb: ohci-exynos: Use devm_clk_get_enabled() helpers
usb: ohci-exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions
usb: dwc3: exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions

Quoted 8 lines

 drivers/usb/dwc3/dwc3-exynos.c | 13 +++----------
 drivers/usb/host/ehci-exynos.c | 27 ++++++---------------------
 drivers/usb/host/ohci-exynos.c | 27 ++++++---------------------
 3 files changed, 15 insertions(+), 52 deletions(-)

Signature · 1 line

2.44.0

The v3 of this series is already pulled by Greg. Changes for v4 are not important. We can ignore v4.

Thanks,
Thinh

Anand Moon

author

2024-05-07 08:01 UTC #7 lore raw

Hi Thinh,

Quoted 47 lines · Thinh Nguyen

Quoted 45 lines · Anand Moon
using new helper functions for dev resources for clock and PM to simplify the code changes.
Addressed some review comments.
v4: Add Reviewed-by: Alan Stern For patches 1 - 4:
Add Acked-by: Thinh Nguyen For patch 5 Fix rephase commit messaeg for patch 2 and 4 Fix some typo in the commit messagee
v3 changes:
1 Use new DEFINE_SIMPLE_DEV_PM_OPS macro for PM operations
Thanks to Thinh Nguyen for your inputs.
so I have to update the $subject and commit messagee with using new macro.
2 Drop the dev_err_probe in return to simplify the error for clocks.
3 Dop the devm_regulator_bulk_get_enable patch.
Tests on Odroid XU4 and Odroid U3.
found no regression with suspend resume functionality.
Previous changes:
V3: https://urldefense.com/v3/__https://lore.kernel.org/all/[email protected]/__;!!A4F2R9G_pg!b2EBzV-UTfuvwSmse-8kl4jsq8t4KgBPT_G--QCG_TDV4atcywVQgLHPFGmxGnPASOKOmp6r0xyI2DnUnRp_cur6Ew$
V2: https://urldefense.com/v3/__https://lore.kernel.org/all/[email protected]/__;!!A4F2R9G_pg!b2EBzV-UTfuvwSmse-8kl4jsq8t4KgBPT_G--QCG_TDV4atcywVQgLHPFGmxGnPASOKOmp6r0xyI2DnUnRqoIl7uEA$
V1: https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-samsung-soc/patch/[email protected]/__;!!A4F2R9G_pg!b2EBzV-UTfuvwSmse-8kl4jsq8t4KgBPT_G--QCG_TDV4atcywVQgLHPFGmxGnPASOKOmp6r0xyI2DnUnRpJKM0R2g$
Thanks
-Anand
Anand Moon (5):
usb: ehci-exynos: Use devm_clk_get_enabled() helpers
usb: ehci-exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions
usb: ohci-exynos: Use devm_clk_get_enabled() helpers
usb: ohci-exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions
usb: dwc3: exynos: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions
 drivers/usb/dwc3/dwc3-exynos.c | 13 +++----------
 drivers/usb/host/ehci-exynos.c | 27 ++++++---------------------
 drivers/usb/host/ohci-exynos.c | 27 ++++++---------------------
 3 files changed, 15 insertions(+), 52 deletions(-)

Quoted 7 lines

Quoted 3 lines
--
2.44.0
The v3 of this series is already pulled by Greg. Changes for v4 are not important. We can ignore v4.

Anyway thanks for the input, I just tried to update the commit
message relevant to the changes.

Quoted 2 lines

Thanks,
Thinh

Thanks
-Anand