2024-01-15 16:16:51

by Thomas Richard

[permalink] [raw]
Subject: [PATCH 05/14] phy: ti: phy-j721e-wiz: make wiz_clock_init callable multiple times

For suspend and resume support, wiz_clock_init needs to be called
multiple times.
Add a parameter to wiz_clock_init to be able to skip clocks
registration.

Based on the work of Théo Lebrun <[email protected]>

Signed-off-by: Thomas Richard <[email protected]>
---
drivers/phy/ti/phy-j721e-wiz.c | 60 +++++++++++++++++++++++++-----------------
1 file changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
index fc3cd98c60ff..09f7edf16562 100644
--- a/drivers/phy/ti/phy-j721e-wiz.c
+++ b/drivers/phy/ti/phy-j721e-wiz.c
@@ -1076,7 +1076,7 @@ static int wiz_clock_register(struct wiz *wiz)
return ret;
}

-static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
+static int wiz_clock_init(struct wiz *wiz, struct device_node *node, bool probe)
{
const struct wiz_clk_mux_sel *clk_mux_sel = wiz->clk_mux_sel;
struct device *dev = wiz->dev;
@@ -1087,14 +1087,36 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
int ret;
int i;

- clk = devm_clk_get(dev, "core_ref_clk");
- if (IS_ERR(clk)) {
- dev_err(dev, "core_ref_clk clock not found\n");
- ret = PTR_ERR(clk);
- return ret;
+ if (probe) {
+ clk = devm_clk_get(dev, "core_ref_clk");
+ if (IS_ERR(clk)) {
+ dev_err(dev, "core_ref_clk clock not found\n");
+ ret = PTR_ERR(clk);
+ return ret;
+ }
+ wiz->input_clks[WIZ_CORE_REFCLK] = clk;
+
+ if (wiz->data->pma_cmn_refclk1_int_mode) {
+ clk = devm_clk_get(dev, "core_ref1_clk");
+ if (IS_ERR(clk)) {
+ dev_err(dev, "core_ref1_clk clock not found\n");
+ ret = PTR_ERR(clk);
+ return ret;
+ }
+ wiz->input_clks[WIZ_CORE_REFCLK1] = clk;
+ }
+
+ clk = devm_clk_get(dev, "ext_ref_clk");
+ if (IS_ERR(clk)) {
+ dev_err(dev, "ext_ref_clk clock not found\n");
+ ret = PTR_ERR(clk);
+ return ret;
+ }
+ wiz->input_clks[WIZ_EXT_REFCLK] = clk;
}
- wiz->input_clks[WIZ_CORE_REFCLK] = clk;

+
+ clk = wiz->input_clks[WIZ_CORE_REFCLK];
rate = clk_get_rate(clk);
if (rate >= 100000000)
regmap_field_write(wiz->pma_cmn_refclk_int_mode, 0x1);
@@ -1121,14 +1143,7 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
}

if (wiz->data->pma_cmn_refclk1_int_mode) {
- clk = devm_clk_get(dev, "core_ref1_clk");
- if (IS_ERR(clk)) {
- dev_err(dev, "core_ref1_clk clock not found\n");
- ret = PTR_ERR(clk);
- return ret;
- }
- wiz->input_clks[WIZ_CORE_REFCLK1] = clk;
-
+ clk = wiz->input_clks[WIZ_CORE_REFCLK1];
rate = clk_get_rate(clk);
if (rate >= 100000000)
regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x1);
@@ -1136,20 +1151,17 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x3);
}

- clk = devm_clk_get(dev, "ext_ref_clk");
- if (IS_ERR(clk)) {
- dev_err(dev, "ext_ref_clk clock not found\n");
- ret = PTR_ERR(clk);
- return ret;
- }
- wiz->input_clks[WIZ_EXT_REFCLK] = clk;
-
+ clk = wiz->input_clks[WIZ_EXT_REFCLK];
rate = clk_get_rate(clk);
if (rate >= 100000000)
regmap_field_write(wiz->pma_cmn_refclk_mode, 0x0);
else
regmap_field_write(wiz->pma_cmn_refclk_mode, 0x2);

+ /* What follows is about registering clocks. */
+ if (!probe)
+ return 0;
+
switch (wiz->type) {
case AM64_WIZ_10G:
case J7200_WIZ_10G:
@@ -1592,7 +1604,7 @@ static int wiz_probe(struct platform_device *pdev)
goto err_get_sync;
}

- ret = wiz_clock_init(wiz, node);
+ ret = wiz_clock_init(wiz, node, true);
if (ret < 0) {
dev_warn(dev, "Failed to initialize clocks\n");
goto err_get_sync;

--
2.39.2



2024-01-23 07:19:06

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 05/14] phy: ti: phy-j721e-wiz: make wiz_clock_init callable multiple times

On 15-01-24, 17:14, Thomas Richard wrote:
> For suspend and resume support, wiz_clock_init needs to be called
> multiple times.
> Add a parameter to wiz_clock_init to be able to skip clocks
> registration.
>
> Based on the work of Th?o Lebrun <[email protected]>
>
> Signed-off-by: Thomas Richard <[email protected]>
> ---
> drivers/phy/ti/phy-j721e-wiz.c | 60 +++++++++++++++++++++++++-----------------
> 1 file changed, 36 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
> index fc3cd98c60ff..09f7edf16562 100644
> --- a/drivers/phy/ti/phy-j721e-wiz.c
> +++ b/drivers/phy/ti/phy-j721e-wiz.c
> @@ -1076,7 +1076,7 @@ static int wiz_clock_register(struct wiz *wiz)
> return ret;
> }
>
> -static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
> +static int wiz_clock_init(struct wiz *wiz, struct device_node *node, bool probe)
> {
> const struct wiz_clk_mux_sel *clk_mux_sel = wiz->clk_mux_sel;
> struct device *dev = wiz->dev;
> @@ -1087,14 +1087,36 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
> int ret;
> int i;
>
> - clk = devm_clk_get(dev, "core_ref_clk");
> - if (IS_ERR(clk)) {
> - dev_err(dev, "core_ref_clk clock not found\n");
> - ret = PTR_ERR(clk);
> - return ret;
> + if (probe) {
> + clk = devm_clk_get(dev, "core_ref_clk");
> + if (IS_ERR(clk)) {
> + dev_err(dev, "core_ref_clk clock not found\n");
> + ret = PTR_ERR(clk);
> + return ret;
> + }
> + wiz->input_clks[WIZ_CORE_REFCLK] = clk;
> +
> + if (wiz->data->pma_cmn_refclk1_int_mode) {
> + clk = devm_clk_get(dev, "core_ref1_clk");
> + if (IS_ERR(clk)) {
> + dev_err(dev, "core_ref1_clk clock not found\n");
> + ret = PTR_ERR(clk);
> + return ret;
> + }
> + wiz->input_clks[WIZ_CORE_REFCLK1] = clk;
> + }
> +
> + clk = devm_clk_get(dev, "ext_ref_clk");
> + if (IS_ERR(clk)) {
> + dev_err(dev, "ext_ref_clk clock not found\n");
> + ret = PTR_ERR(clk);
> + return ret;
> + }
> + wiz->input_clks[WIZ_EXT_REFCLK] = clk;
> }
> - wiz->input_clks[WIZ_CORE_REFCLK] = clk;
>
> +
> + clk = wiz->input_clks[WIZ_CORE_REFCLK];
> rate = clk_get_rate(clk);
> if (rate >= 100000000)
> regmap_field_write(wiz->pma_cmn_refclk_int_mode, 0x1);
> @@ -1121,14 +1143,7 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
> }
>
> if (wiz->data->pma_cmn_refclk1_int_mode) {
> - clk = devm_clk_get(dev, "core_ref1_clk");
> - if (IS_ERR(clk)) {
> - dev_err(dev, "core_ref1_clk clock not found\n");
> - ret = PTR_ERR(clk);
> - return ret;
> - }
> - wiz->input_clks[WIZ_CORE_REFCLK1] = clk;
> -
> + clk = wiz->input_clks[WIZ_CORE_REFCLK1];
> rate = clk_get_rate(clk);
> if (rate >= 100000000)
> regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x1);
> @@ -1136,20 +1151,17 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
> regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x3);
> }
>
> - clk = devm_clk_get(dev, "ext_ref_clk");
> - if (IS_ERR(clk)) {
> - dev_err(dev, "ext_ref_clk clock not found\n");
> - ret = PTR_ERR(clk);
> - return ret;
> - }
> - wiz->input_clks[WIZ_EXT_REFCLK] = clk;
> -
> + clk = wiz->input_clks[WIZ_EXT_REFCLK];
> rate = clk_get_rate(clk);
> if (rate >= 100000000)
> regmap_field_write(wiz->pma_cmn_refclk_mode, 0x0);
> else
> regmap_field_write(wiz->pma_cmn_refclk_mode, 0x2);
>
> + /* What follows is about registering clocks. */
> + if (!probe)
> + return 0;
> +
> switch (wiz->type) {
> case AM64_WIZ_10G:
> case J7200_WIZ_10G:
> @@ -1592,7 +1604,7 @@ static int wiz_probe(struct platform_device *pdev)
> goto err_get_sync;
> }
>
> - ret = wiz_clock_init(wiz, node);
> + ret = wiz_clock_init(wiz, node, true);

You are calling it one once? So what am I missing

--
~Vinod

2024-01-23 08:23:43

by Thomas Richard

[permalink] [raw]
Subject: Re: [PATCH 05/14] phy: ti: phy-j721e-wiz: make wiz_clock_init callable multiple times

>>
>> - ret = wiz_clock_init(wiz, node);
>> + ret = wiz_clock_init(wiz, node, true);
>
> You are calling it one once? So what am I missing

In patch 6/14, wiz_clock_init is called in resume_noirq callback:

ret = wiz_clock_init(wiz, node, false);

Regards

--
Thomas Richard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


2024-01-23 14:03:11

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 05/14] phy: ti: phy-j721e-wiz: make wiz_clock_init callable multiple times

On 23-01-24, 09:20, Thomas Richard wrote:
> >>
> >> - ret = wiz_clock_init(wiz, node);
> >> + ret = wiz_clock_init(wiz, node, true);
> >
> > You are calling it one once? So what am I missing
>
> In patch 6/14, wiz_clock_init is called in resume_noirq callback:
>
> ret = wiz_clock_init(wiz, node, false);

noticed later, I think this would be useful to mention in changelog

--
~Vinod