2023-06-13 14:16:11

by Mehta, Piyush

[permalink] [raw]
Subject: [PATCH V2 0/2] phy: xilinx: phy-zynqmp: dynamic clock support

This patch of the series adds the following supports:
- Runtime PM support:
Added runtime power management support to power saving during the
suspend and resume calls.

- Dynamic clock support:
Current zynqmp-phy driver, by default all the clock enabled for all
the lanes, that consumes power even PHY is active or idle.
So, the dynamic clock feature will be enabled clock only for active
PHY in the phy_init and disabled in phy_exit. Clock enabling is not
required at multiple times. Activation of PHY depends on the peripheral
DT node status (status = "okay";).

---
Changes in V2:
- Addressed Vinod Koul review comments:
- Added runtime PM support - "DEFINE_RUNTIME_DEV_PM_OPS" MACRO used.
- Updated commit message for dynamic clock support.

Link: https://lore.kernel.org/all/MN2PR12MB43339B1089E1CF552152DA2D881A9@MN2PR12MB4333.namprd12.prod.outlook.com/#t
---

Piyush Mehta (2):
phy: xilinx: add runtime PM support
phy: xilinx: phy-zynqmp: dynamic clock support for power-save

drivers/phy/xilinx/phy-zynqmp.c | 90 +++++++++++++++------------------
1 file changed, 40 insertions(+), 50 deletions(-)

--
2.25.1



2023-06-13 14:35:17

by Mehta, Piyush

[permalink] [raw]
Subject: [PATCH V2 1/2] phy: xilinx: add runtime PM support

Added Runtime power management support to the xilinx phy driver and using
DEFINE_RUNTIME_DEV_PM_OPS new macros allows the compiler to remove the
unused dev_pm_ops structure and related functions if !CONFIG_PM without
the need to mark the functions __maybe_unused.

Signed-off-by: Piyush Mehta <[email protected]>
---
drivers/phy/xilinx/phy-zynqmp.c | 35 ++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index 8833680923a1..aada18123c19 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -20,6 +20,7 @@
#include <linux/of.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>

#include <dt-bindings/phy/phy.h>
@@ -820,7 +821,7 @@ static struct phy *xpsgtr_xlate(struct device *dev,
* Power Management
*/

-static int __maybe_unused xpsgtr_suspend(struct device *dev)
+static int xpsgtr_runtime_suspend(struct device *dev)
{
struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
unsigned int i;
@@ -835,7 +836,7 @@ static int __maybe_unused xpsgtr_suspend(struct device *dev)
return 0;
}

-static int __maybe_unused xpsgtr_resume(struct device *dev)
+static int xpsgtr_runtime_resume(struct device *dev)
{
struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
unsigned int icm_cfg0, icm_cfg1;
@@ -876,10 +877,8 @@ static int __maybe_unused xpsgtr_resume(struct device *dev)
return err;
}

-static const struct dev_pm_ops xpsgtr_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(xpsgtr_suspend, xpsgtr_resume)
-};
-
+static DEFINE_RUNTIME_DEV_PM_OPS(xpsgtr_pm_ops, xpsgtr_runtime_suspend,
+ xpsgtr_runtime_resume, NULL);
/*
* Probe & Platform Driver
*/
@@ -1005,6 +1004,16 @@ static int xpsgtr_probe(struct platform_device *pdev)
ret = PTR_ERR(provider);
goto err_clk_put;
}
+
+ pm_runtime_set_active(gtr_dev->dev);
+ pm_runtime_enable(gtr_dev->dev);
+
+ ret = pm_runtime_resume_and_get(gtr_dev->dev);
+ if (ret < 0) {
+ pm_runtime_disable(gtr_dev->dev);
+ goto err_clk_put;
+ }
+
return 0;

err_clk_put:
@@ -1014,6 +1023,17 @@ static int xpsgtr_probe(struct platform_device *pdev)
return ret;
}

+static int xpsgtr_remove(struct platform_device *pdev)
+{
+ struct xpsgtr_dev *gtr_dev = platform_get_drvdata(pdev);
+
+ pm_runtime_disable(gtr_dev->dev);
+ pm_runtime_put_noidle(gtr_dev->dev);
+ pm_runtime_set_suspended(gtr_dev->dev);
+
+ return 0;
+}
+
static const struct of_device_id xpsgtr_of_match[] = {
{ .compatible = "xlnx,zynqmp-psgtr", },
{ .compatible = "xlnx,zynqmp-psgtr-v1.1", },
@@ -1023,10 +1043,11 @@ MODULE_DEVICE_TABLE(of, xpsgtr_of_match);

static struct platform_driver xpsgtr_driver = {
.probe = xpsgtr_probe,
+ .remove = xpsgtr_remove,
.driver = {
.name = "xilinx-psgtr",
.of_match_table = xpsgtr_of_match,
- .pm = &xpsgtr_pm_ops,
+ .pm = pm_ptr(&xpsgtr_pm_ops),
},
};

--
2.25.1


2023-06-13 14:40:38

by Mehta, Piyush

[permalink] [raw]
Subject: [PATCH V2 2/2] phy: xilinx: phy-zynqmp: dynamic clock support for power-save

Enabling clock for all the lanes consumes power even PHY is active or
inactive. To resolve this, enable/disable clocks in phy_init/phy_exit.

By default clock is disabled for all the lanes. Whenever phy_init called
from USB, SATA, or display driver, etc. It enabled the required clock
for requested lane. On phy_exit cycle, it disabled clock for the active
PHYs.

During the suspend/resume cycle, each USB/ SATA/ display driver called
phy_exit/phy_init individually. It disabled clock on exit, and enabled
on initialization for the active PHYs.

Signed-off-by: Piyush Mehta <[email protected]>
---
drivers/phy/xilinx/phy-zynqmp.c | 61 ++++++++-------------------------
1 file changed, 15 insertions(+), 46 deletions(-)

diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index aada18123c19..b0c2045b886e 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -572,6 +572,10 @@ static int xpsgtr_phy_init(struct phy *phy)

mutex_lock(&gtr_dev->gtr_mutex);

+ /* Configure and enable the clock when peripheral phy_init call */
+ if (clk_prepare_enable(gtr_dev->clk[gtr_phy->lane]))
+ goto out;
+
/* Skip initialization if not required. */
if (!xpsgtr_phy_init_required(gtr_phy))
goto out;
@@ -616,9 +620,13 @@ static int xpsgtr_phy_init(struct phy *phy)
static int xpsgtr_phy_exit(struct phy *phy)
{
struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy);
+ struct xpsgtr_dev *gtr_dev = gtr_phy->dev;

gtr_phy->skip_phy_init = false;

+ /* Ensure that disable clock only, which configure for lane */
+ clk_disable_unprepare(gtr_dev->clk[gtr_phy->lane]);
+
return 0;
}

@@ -824,15 +832,11 @@ static struct phy *xpsgtr_xlate(struct device *dev,
static int xpsgtr_runtime_suspend(struct device *dev)
{
struct xpsgtr_dev *gtr_dev = dev_get_drvdata(dev);
- unsigned int i;

/* Save the snapshot ICM_CFG registers. */
gtr_dev->saved_icm_cfg0 = xpsgtr_read(gtr_dev, ICM_CFG0);
gtr_dev->saved_icm_cfg1 = xpsgtr_read(gtr_dev, ICM_CFG1);

- for (i = 0; i < ARRAY_SIZE(gtr_dev->clk); i++)
- clk_disable_unprepare(gtr_dev->clk[i]);
-
return 0;
}

@@ -842,13 +846,6 @@ static int xpsgtr_runtime_resume(struct device *dev)
unsigned int icm_cfg0, icm_cfg1;
unsigned int i;
bool skip_phy_init;
- int err;
-
- for (i = 0; i < ARRAY_SIZE(gtr_dev->clk); i++) {
- err = clk_prepare_enable(gtr_dev->clk[i]);
- if (err)
- goto err_clk_put;
- }

icm_cfg0 = xpsgtr_read(gtr_dev, ICM_CFG0);
icm_cfg1 = xpsgtr_read(gtr_dev, ICM_CFG1);
@@ -869,12 +866,6 @@ static int xpsgtr_runtime_resume(struct device *dev)
gtr_dev->phys[i].skip_phy_init = skip_phy_init;

return 0;
-
-err_clk_put:
- while (i--)
- clk_disable_unprepare(gtr_dev->clk[i]);
-
- return err;
}

static DEFINE_RUNTIME_DEV_PM_OPS(xpsgtr_pm_ops, xpsgtr_runtime_suspend,
@@ -886,7 +877,6 @@ static DEFINE_RUNTIME_DEV_PM_OPS(xpsgtr_pm_ops, xpsgtr_runtime_suspend,
static int xpsgtr_get_ref_clocks(struct xpsgtr_dev *gtr_dev)
{
unsigned int refclk;
- int ret;

for (refclk = 0; refclk < ARRAY_SIZE(gtr_dev->refclk_sscs); ++refclk) {
unsigned long rate;
@@ -897,19 +887,14 @@ static int xpsgtr_get_ref_clocks(struct xpsgtr_dev *gtr_dev)
snprintf(name, sizeof(name), "ref%u", refclk);
clk = devm_clk_get_optional(gtr_dev->dev, name);
if (IS_ERR(clk)) {
- ret = dev_err_probe(gtr_dev->dev, PTR_ERR(clk),
- "Failed to get reference clock %u\n",
- refclk);
- goto err_clk_put;
+ return dev_err_probe(gtr_dev->dev, PTR_ERR(clk),
+ "Failed to get ref clock %u\n",
+ refclk);
}

if (!clk)
continue;

- ret = clk_prepare_enable(clk);
- if (ret)
- goto err_clk_put;
-
gtr_dev->clk[refclk] = clk;

/*
@@ -929,18 +914,11 @@ static int xpsgtr_get_ref_clocks(struct xpsgtr_dev *gtr_dev)
dev_err(gtr_dev->dev,
"Invalid rate %lu for reference clock %u\n",
rate, refclk);
- ret = -EINVAL;
- goto err_clk_put;
+ return -EINVAL;
}
}

return 0;
-
-err_clk_put:
- while (refclk--)
- clk_disable_unprepare(gtr_dev->clk[refclk]);
-
- return ret;
}

static int xpsgtr_probe(struct platform_device *pdev)
@@ -949,7 +927,6 @@ static int xpsgtr_probe(struct platform_device *pdev)
struct xpsgtr_dev *gtr_dev;
struct phy_provider *provider;
unsigned int port;
- unsigned int i;
int ret;

gtr_dev = devm_kzalloc(&pdev->dev, sizeof(*gtr_dev), GFP_KERNEL);
@@ -989,8 +966,7 @@ static int xpsgtr_probe(struct platform_device *pdev)
phy = devm_phy_create(&pdev->dev, np, &xpsgtr_phyops);
if (IS_ERR(phy)) {
dev_err(&pdev->dev, "failed to create PHY\n");
- ret = PTR_ERR(phy);
- goto err_clk_put;
+ return PTR_ERR(phy);
}

gtr_phy->phy = phy;
@@ -1001,8 +977,7 @@ static int xpsgtr_probe(struct platform_device *pdev)
provider = devm_of_phy_provider_register(&pdev->dev, xpsgtr_xlate);
if (IS_ERR(provider)) {
dev_err(&pdev->dev, "registering provider failed\n");
- ret = PTR_ERR(provider);
- goto err_clk_put;
+ return PTR_ERR(provider);
}

pm_runtime_set_active(gtr_dev->dev);
@@ -1011,16 +986,10 @@ static int xpsgtr_probe(struct platform_device *pdev)
ret = pm_runtime_resume_and_get(gtr_dev->dev);
if (ret < 0) {
pm_runtime_disable(gtr_dev->dev);
- goto err_clk_put;
+ return ret;
}

return 0;
-
-err_clk_put:
- for (i = 0; i < ARRAY_SIZE(gtr_dev->clk); i++)
- clk_disable_unprepare(gtr_dev->clk[i]);
-
- return ret;
}

static int xpsgtr_remove(struct platform_device *pdev)
--
2.25.1


2023-07-11 08:00:04

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH V2 0/2] phy: xilinx: phy-zynqmp: dynamic clock support


On Tue, 13 Jun 2023 19:32:48 +0530, Piyush Mehta wrote:
> This patch of the series adds the following supports:
> - Runtime PM support:
> Added runtime power management support to power saving during the
> suspend and resume calls.
>
> - Dynamic clock support:
> Current zynqmp-phy driver, by default all the clock enabled for all
> the lanes, that consumes power even PHY is active or idle.
> So, the dynamic clock feature will be enabled clock only for active
> PHY in the phy_init and disabled in phy_exit. Clock enabling is not
> required at multiple times. Activation of PHY depends on the peripheral
> DT node status (status = "okay";).
>
> [...]

Applied, thanks!

[1/2] phy: xilinx: add runtime PM support
commit: 1414920a5fd570e67bb7d367eda08f8851b2e378
[2/2] phy: xilinx: phy-zynqmp: dynamic clock support for power-save
commit: e4f01e75cdcf003f07ca5f4ba61823b687fd941c

Best regards,
--
~Vinod