linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups
@ 2025-05-23  8:48 Johan Hovold
  2025-05-23  8:48 ` [PATCH 1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit() Johan Hovold
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Johan Hovold @ 2025-05-23  8:48 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel, Johan Hovold

Here are a couple of fixes and some related cleanups to the recently
reworked and renamed phy-snps-eusb2 driver.

The clock and repeater imbalance fixes are not marked for stable as the
first issue was introduced in the recent rework which is queued for
6.16-rc1.

The repeater imbalance has been there for a few years and
could be backported even if this is now complicated by the
rework/rename. Since it only affects a resource leak in an error path I
decided to not mark this one for stable for now.

Ideally, these could go in along with the reworked driver for rc1.

Johan


Johan Hovold (7):
  phy: phy-snps-eusb2: fix clock imbalance on phy_exit()
  phy: phy-snps-eusb2: fix repeater imbalance on phy_init() failure
  phy: phy-snps-eusb2: rename phy_init() clock error label
  phy: phy-snps-eusb2: clean up error messages
  phy: phy-snps-eusb2: fix optional phy lookup parameter
  phy: phy-snps-eusb2: drop unnecessary loop index declarations
  phy: phy-snps-eusb2: clean up id table sentinel

 drivers/phy/phy-snps-eusb2.c | 39 +++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 18 deletions(-)

-- 
2.49.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit()
  2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
@ 2025-05-23  8:48 ` Johan Hovold
  2025-05-25 13:17   ` Ivaylo Ivanov
  2025-05-26  7:49   ` neil.armstrong
  2025-05-23  8:48 ` [PATCH 2/7] phy: phy-snps-eusb2: fix repeater imbalance on phy_init() failure Johan Hovold
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 17+ messages in thread
From: Johan Hovold @ 2025-05-23  8:48 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel, Johan Hovold

Make sure to disable all clocks enabled at phy_init() also on
phy_exit().

Fixes: c4098f3e6134 ("phy: phy-snps-eusb2: add support for exynos2200")
Cc: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/phy/phy-snps-eusb2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index b73a1d7e57b3..19af3f99692c 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -504,7 +504,7 @@ static int snps_eusb2_hsphy_exit(struct phy *p)
 {
 	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
 
-	clk_disable_unprepare(phy->ref_clk);
+	clk_bulk_disable_unprepare(phy->data->num_clks, phy->clks);
 
 	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
 
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/7] phy: phy-snps-eusb2: fix repeater imbalance on phy_init() failure
  2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
  2025-05-23  8:48 ` [PATCH 1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit() Johan Hovold
@ 2025-05-23  8:48 ` Johan Hovold
  2025-05-26  7:49   ` Neil Armstrong
  2025-05-23  8:48 ` [PATCH 3/7] phy: phy-snps-eusb2: rename phy_init() clock error label Johan Hovold
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Johan Hovold @ 2025-05-23  8:48 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel, Johan Hovold, Neil Armstrong

Make sure to disable the repeater PHY also on phy_init() failure.

Fixes: 3584f6392f09 ("phy: qcom: phy-qcom-snps-eusb2: Add support for eUSB2 repeater")
Cc: Abel Vesa <abel.vesa@linaro.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/phy/phy-snps-eusb2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index 19af3f99692c..a799c3d2bcfb 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -468,7 +468,7 @@ static int snps_eusb2_hsphy_init(struct phy *p)
 	ret = clk_bulk_prepare_enable(phy->data->num_clks, phy->clks);
 	if (ret) {
 		dev_err(&p->dev, "failed to enable ref clock, %d\n", ret);
-		goto disable_vreg;
+		goto exit_repeater;
 	}
 
 	ret = reset_control_assert(phy->phy_reset);
@@ -493,7 +493,8 @@ static int snps_eusb2_hsphy_init(struct phy *p)
 
 disable_ref_clk:
 	clk_bulk_disable_unprepare(phy->data->num_clks, phy->clks);
-
+exit_repeater:
+	phy_exit(phy->repeater);
 disable_vreg:
 	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
 
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 3/7] phy: phy-snps-eusb2: rename phy_init() clock error label
  2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
  2025-05-23  8:48 ` [PATCH 1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit() Johan Hovold
  2025-05-23  8:48 ` [PATCH 2/7] phy: phy-snps-eusb2: fix repeater imbalance on phy_init() failure Johan Hovold
@ 2025-05-23  8:48 ` Johan Hovold
  2025-05-26  7:49   ` neil.armstrong
  2025-05-23  8:48 ` [PATCH 4/7] phy: phy-snps-eusb2: clean up error messages Johan Hovold
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Johan Hovold @ 2025-05-23  8:48 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel, Johan Hovold

Rename the clock error label which is now used to disable all clocks and
not just the ref clock on phy_init() errors.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/phy/phy-snps-eusb2.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index a799c3d2bcfb..c8f14f8fb9ac 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -474,7 +474,7 @@ static int snps_eusb2_hsphy_init(struct phy *p)
 	ret = reset_control_assert(phy->phy_reset);
 	if (ret) {
 		dev_err(&p->dev, "failed to assert phy_reset, %d\n", ret);
-		goto disable_ref_clk;
+		goto disable_clks;
 	}
 
 	usleep_range(100, 150);
@@ -482,16 +482,16 @@ static int snps_eusb2_hsphy_init(struct phy *p)
 	ret = reset_control_deassert(phy->phy_reset);
 	if (ret) {
 		dev_err(&p->dev, "failed to de-assert phy_reset, %d\n", ret);
-		goto disable_ref_clk;
+		goto disable_clks;
 	}
 
 	ret = phy->data->phy_init(p);
 	if (ret)
-		goto disable_ref_clk;
+		goto disable_clks;
 
 	return 0;
 
-disable_ref_clk:
+disable_clks:
 	clk_bulk_disable_unprepare(phy->data->num_clks, phy->clks);
 exit_repeater:
 	phy_exit(phy->repeater);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 4/7] phy: phy-snps-eusb2: clean up error messages
  2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
                   ` (2 preceding siblings ...)
  2025-05-23  8:48 ` [PATCH 3/7] phy: phy-snps-eusb2: rename phy_init() clock error label Johan Hovold
@ 2025-05-23  8:48 ` Johan Hovold
  2025-05-26  7:50   ` neil.armstrong
  2025-05-23  8:48 ` [PATCH 5/7] phy: phy-snps-eusb2: fix optional phy lookup parameter Johan Hovold
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Johan Hovold @ 2025-05-23  8:48 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel, Johan Hovold

Clean up the error messages by using a consistent format with a colon
followed by a space and an errno (or unsupported frequency).

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/phy/phy-snps-eusb2.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index c8f14f8fb9ac..232c5e8a554f 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -256,7 +256,7 @@ static int exynos_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
 	}
 
 	if (!config) {
-		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
+		dev_err(&phy->phy->dev, "unsupported ref_clk_freq: %lu\n", ref_clk_freq);
 		return -EINVAL;
 	}
 
@@ -293,7 +293,7 @@ static int qcom_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
 	}
 
 	if (!config) {
-		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
+		dev_err(&phy->phy->dev, "unsupported ref_clk_freq: %lu\n", ref_clk_freq);
 		return -EINVAL;
 	}
 
@@ -461,19 +461,19 @@ static int snps_eusb2_hsphy_init(struct phy *p)
 
 	ret = phy_init(phy->repeater);
 	if (ret) {
-		dev_err(&p->dev, "repeater init failed. %d\n", ret);
+		dev_err(&p->dev, "repeater init failed: %d\n", ret);
 		goto disable_vreg;
 	}
 
 	ret = clk_bulk_prepare_enable(phy->data->num_clks, phy->clks);
 	if (ret) {
-		dev_err(&p->dev, "failed to enable ref clock, %d\n", ret);
+		dev_err(&p->dev, "failed to enable ref clock: %d\n", ret);
 		goto exit_repeater;
 	}
 
 	ret = reset_control_assert(phy->phy_reset);
 	if (ret) {
-		dev_err(&p->dev, "failed to assert phy_reset, %d\n", ret);
+		dev_err(&p->dev, "failed to assert phy_reset: %d\n", ret);
 		goto disable_clks;
 	}
 
@@ -481,7 +481,7 @@ static int snps_eusb2_hsphy_init(struct phy *p)
 
 	ret = reset_control_deassert(phy->phy_reset);
 	if (ret) {
-		dev_err(&p->dev, "failed to de-assert phy_reset, %d\n", ret);
+		dev_err(&p->dev, "failed to de-assert phy_reset: %d\n", ret);
 		goto disable_clks;
 	}
 
@@ -588,7 +588,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
 
 	generic_phy = devm_phy_create(dev, NULL, &snps_eusb2_hsphy_ops);
 	if (IS_ERR(generic_phy)) {
-		dev_err(dev, "failed to create phy %d\n", ret);
+		dev_err(dev, "failed to create phy: %d\n", ret);
 		return PTR_ERR(generic_phy);
 	}
 
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 5/7] phy: phy-snps-eusb2: fix optional phy lookup parameter
  2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
                   ` (3 preceding siblings ...)
  2025-05-23  8:48 ` [PATCH 4/7] phy: phy-snps-eusb2: clean up error messages Johan Hovold
@ 2025-05-23  8:48 ` Johan Hovold
  2025-05-26  8:27   ` neil.armstrong
  2025-05-23  8:48 ` [PATCH 6/7] phy: phy-snps-eusb2: drop unnecessary loop index declarations Johan Hovold
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Johan Hovold @ 2025-05-23  8:48 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel, Johan Hovold

The devm_of_phy_optional_get() takes an optional name argument as its
third parameter and not an index like the recently replaced
devm_of_phy_get_by_index().

Replace 0 with an explicit NULL for consistency and readability.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/phy/phy-snps-eusb2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index 232c5e8a554f..328e67ebfe03 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -581,7 +581,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret,
 				     "failed to get regulator supplies\n");
 
-	phy->repeater = devm_of_phy_optional_get(dev, np, 0);
+	phy->repeater = devm_of_phy_optional_get(dev, np, NULL);
 	if (IS_ERR(phy->repeater))
 		return dev_err_probe(dev, PTR_ERR(phy->repeater),
 				     "failed to get repeater\n");
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 6/7] phy: phy-snps-eusb2: drop unnecessary loop index declarations
  2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
                   ` (4 preceding siblings ...)
  2025-05-23  8:48 ` [PATCH 5/7] phy: phy-snps-eusb2: fix optional phy lookup parameter Johan Hovold
@ 2025-05-23  8:48 ` Johan Hovold
  2025-05-26  8:27   ` neil.armstrong
  2025-05-23  8:48 ` [PATCH 7/7] phy: phy-snps-eusb2: clean up id table sentinel Johan Hovold
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Johan Hovold @ 2025-05-23  8:48 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel, Johan Hovold

There is already a loop index variable declared at function scope so
drop the unnecessary overloaded loop declarations.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/phy/phy-snps-eusb2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index 328e67ebfe03..c67712bd8bba 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -552,7 +552,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
 	if (!phy->clks)
 		return -ENOMEM;
 
-	for (int i = 0; i < phy->data->num_clks; ++i)
+	for (i = 0; i < phy->data->num_clks; ++i)
 		phy->clks[i].id = phy->data->clk_names[i];
 
 	ret = devm_clk_bulk_get(dev, phy->data->num_clks, phy->clks);
@@ -561,7 +561,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
 				     "failed to get phy clock(s)\n");
 
 	phy->ref_clk = NULL;
-	for (int i = 0; i < phy->data->num_clks; ++i) {
+	for (i = 0; i < phy->data->num_clks; ++i) {
 		if (!strcmp(phy->clks[i].id, "ref")) {
 			phy->ref_clk = phy->clks[i].clk;
 			break;
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 7/7] phy: phy-snps-eusb2: clean up id table sentinel
  2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
                   ` (5 preceding siblings ...)
  2025-05-23  8:48 ` [PATCH 6/7] phy: phy-snps-eusb2: drop unnecessary loop index declarations Johan Hovold
@ 2025-05-23  8:48 ` Johan Hovold
  2025-05-23  8:57 ` [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Abel Vesa
  2025-06-15 16:53 ` Vinod Koul
  8 siblings, 0 replies; 17+ messages in thread
From: Johan Hovold @ 2025-05-23  8:48 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel, Johan Hovold

Use a more common format for the id table sentinel for consistency and
symmetry.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/phy/phy-snps-eusb2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
index c67712bd8bba..cf62f2221366 100644
--- a/drivers/phy/phy-snps-eusb2.c
+++ b/drivers/phy/phy-snps-eusb2.c
@@ -611,7 +611,9 @@ static const struct of_device_id snps_eusb2_hsphy_of_match_table[] = {
 	}, {
 		.compatible = "samsung,exynos2200-eusb2-phy",
 		.data = &exynos2200_snps_eusb2_phy,
-	}, { },
+	}, {
+		/* sentinel */
+	}
 };
 MODULE_DEVICE_TABLE(of, snps_eusb2_hsphy_of_match_table);
 
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups
  2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
                   ` (6 preceding siblings ...)
  2025-05-23  8:48 ` [PATCH 7/7] phy: phy-snps-eusb2: clean up id table sentinel Johan Hovold
@ 2025-05-23  8:57 ` Abel Vesa
  2025-06-15 16:53 ` Vinod Koul
  8 siblings, 0 replies; 17+ messages in thread
From: Abel Vesa @ 2025-05-23  8:57 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Vinod Koul, Kishon Vijay Abraham I, Ivaylo Ivanov, linux-phy,
	linux-kernel

On 25-05-23 10:48:32, Johan Hovold wrote:
> Here are a couple of fixes and some related cleanups to the recently
> reworked and renamed phy-snps-eusb2 driver.
> 
> The clock and repeater imbalance fixes are not marked for stable as the
> first issue was introduced in the recent rework which is queued for
> 6.16-rc1.
> 
> The repeater imbalance has been there for a few years and
> could be backported even if this is now complicated by the
> rework/rename. Since it only affects a resource leak in an error path I
> decided to not mark this one for stable for now.
> 
> Ideally, these could go in along with the reworked driver for rc1.
> 
> Johan
> 
> 
> Johan Hovold (7):
>   phy: phy-snps-eusb2: fix clock imbalance on phy_exit()
>   phy: phy-snps-eusb2: fix repeater imbalance on phy_init() failure
>   phy: phy-snps-eusb2: rename phy_init() clock error label
>   phy: phy-snps-eusb2: clean up error messages
>   phy: phy-snps-eusb2: fix optional phy lookup parameter
>   phy: phy-snps-eusb2: drop unnecessary loop index declarations
>   phy: phy-snps-eusb2: clean up id table sentinel

Really straightforward. So for the entire series:

Reviewed-by: Abel Vesa <abel.vesa@linaro.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit()
  2025-05-23  8:48 ` [PATCH 1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit() Johan Hovold
@ 2025-05-25 13:17   ` Ivaylo Ivanov
  2025-05-26  7:49   ` neil.armstrong
  1 sibling, 0 replies; 17+ messages in thread
From: Ivaylo Ivanov @ 2025-05-25 13:17 UTC (permalink / raw)
  To: Johan Hovold, Vinod Koul
  Cc: Kishon Vijay Abraham I, Abel Vesa, linux-phy, linux-kernel

On 5/23/25 11:48, Johan Hovold wrote:
> Make sure to disable all clocks enabled at phy_init() also on
> phy_exit().
>
> Fixes: c4098f3e6134 ("phy: phy-snps-eusb2: add support for exynos2200")
> Cc: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/phy/phy-snps-eusb2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index b73a1d7e57b3..19af3f99692c 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -504,7 +504,7 @@ static int snps_eusb2_hsphy_exit(struct phy *p)
>  {
>  	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
>  
> -	clk_disable_unprepare(phy->ref_clk);
> +	clk_bulk_disable_unprepare(phy->data->num_clks, phy->clks);

This shouldn't affect functionality for Exynos 2200, as gates are not
handled manually in the clocks driver.

Anyways, this is good to have for other/future supported SoCs.

Acked-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>

Best regards,
Ivaylo

>  
>  	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
>  


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit()
  2025-05-23  8:48 ` [PATCH 1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit() Johan Hovold
  2025-05-25 13:17   ` Ivaylo Ivanov
@ 2025-05-26  7:49   ` neil.armstrong
  1 sibling, 0 replies; 17+ messages in thread
From: neil.armstrong @ 2025-05-26  7:49 UTC (permalink / raw)
  To: Johan Hovold, Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel

On 23/05/2025 10:48, Johan Hovold wrote:
> Make sure to disable all clocks enabled at phy_init() also on
> phy_exit().
> 
> Fixes: c4098f3e6134 ("phy: phy-snps-eusb2: add support for exynos2200")
> Cc: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/phy/phy-snps-eusb2.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index b73a1d7e57b3..19af3f99692c 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -504,7 +504,7 @@ static int snps_eusb2_hsphy_exit(struct phy *p)
>   {
>   	struct snps_eusb2_hsphy *phy = phy_get_drvdata(p);
>   
> -	clk_disable_unprepare(phy->ref_clk);
> +	clk_bulk_disable_unprepare(phy->data->num_clks, phy->clks);
>   
>   	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
>   

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/7] phy: phy-snps-eusb2: fix repeater imbalance on phy_init() failure
  2025-05-23  8:48 ` [PATCH 2/7] phy: phy-snps-eusb2: fix repeater imbalance on phy_init() failure Johan Hovold
@ 2025-05-26  7:49   ` Neil Armstrong
  0 siblings, 0 replies; 17+ messages in thread
From: Neil Armstrong @ 2025-05-26  7:49 UTC (permalink / raw)
  To: Johan Hovold, Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel

On 23/05/2025 10:48, Johan Hovold wrote:
> Make sure to disable the repeater PHY also on phy_init() failure.
> 
> Fixes: 3584f6392f09 ("phy: qcom: phy-qcom-snps-eusb2: Add support for eUSB2 repeater")
> Cc: Abel Vesa <abel.vesa@linaro.org>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/phy/phy-snps-eusb2.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index 19af3f99692c..a799c3d2bcfb 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -468,7 +468,7 @@ static int snps_eusb2_hsphy_init(struct phy *p)
>   	ret = clk_bulk_prepare_enable(phy->data->num_clks, phy->clks);
>   	if (ret) {
>   		dev_err(&p->dev, "failed to enable ref clock, %d\n", ret);
> -		goto disable_vreg;
> +		goto exit_repeater;
>   	}
>   
>   	ret = reset_control_assert(phy->phy_reset);
> @@ -493,7 +493,8 @@ static int snps_eusb2_hsphy_init(struct phy *p)
>   
>   disable_ref_clk:
>   	clk_bulk_disable_unprepare(phy->data->num_clks, phy->clks);
> -
> +exit_repeater:
> +	phy_exit(phy->repeater);
>   disable_vreg:
>   	regulator_bulk_disable(ARRAY_SIZE(phy->vregs), phy->vregs);
>   

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/7] phy: phy-snps-eusb2: rename phy_init() clock error label
  2025-05-23  8:48 ` [PATCH 3/7] phy: phy-snps-eusb2: rename phy_init() clock error label Johan Hovold
@ 2025-05-26  7:49   ` neil.armstrong
  0 siblings, 0 replies; 17+ messages in thread
From: neil.armstrong @ 2025-05-26  7:49 UTC (permalink / raw)
  To: Johan Hovold, Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel

On 23/05/2025 10:48, Johan Hovold wrote:
> Rename the clock error label which is now used to disable all clocks and
> not just the ref clock on phy_init() errors.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/phy/phy-snps-eusb2.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index a799c3d2bcfb..c8f14f8fb9ac 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -474,7 +474,7 @@ static int snps_eusb2_hsphy_init(struct phy *p)
>   	ret = reset_control_assert(phy->phy_reset);
>   	if (ret) {
>   		dev_err(&p->dev, "failed to assert phy_reset, %d\n", ret);
> -		goto disable_ref_clk;
> +		goto disable_clks;
>   	}
>   
>   	usleep_range(100, 150);
> @@ -482,16 +482,16 @@ static int snps_eusb2_hsphy_init(struct phy *p)
>   	ret = reset_control_deassert(phy->phy_reset);
>   	if (ret) {
>   		dev_err(&p->dev, "failed to de-assert phy_reset, %d\n", ret);
> -		goto disable_ref_clk;
> +		goto disable_clks;
>   	}
>   
>   	ret = phy->data->phy_init(p);
>   	if (ret)
> -		goto disable_ref_clk;
> +		goto disable_clks;
>   
>   	return 0;
>   
> -disable_ref_clk:
> +disable_clks:
>   	clk_bulk_disable_unprepare(phy->data->num_clks, phy->clks);
>   exit_repeater:
>   	phy_exit(phy->repeater);

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 4/7] phy: phy-snps-eusb2: clean up error messages
  2025-05-23  8:48 ` [PATCH 4/7] phy: phy-snps-eusb2: clean up error messages Johan Hovold
@ 2025-05-26  7:50   ` neil.armstrong
  0 siblings, 0 replies; 17+ messages in thread
From: neil.armstrong @ 2025-05-26  7:50 UTC (permalink / raw)
  To: Johan Hovold, Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel

On 23/05/2025 10:48, Johan Hovold wrote:
> Clean up the error messages by using a consistent format with a colon
> followed by a space and an errno (or unsupported frequency).
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/phy/phy-snps-eusb2.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index c8f14f8fb9ac..232c5e8a554f 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -256,7 +256,7 @@ static int exynos_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
>   	}
>   
>   	if (!config) {
> -		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
> +		dev_err(&phy->phy->dev, "unsupported ref_clk_freq: %lu\n", ref_clk_freq);
>   		return -EINVAL;
>   	}
>   
> @@ -293,7 +293,7 @@ static int qcom_eusb2_ref_clk_init(struct snps_eusb2_hsphy *phy)
>   	}
>   
>   	if (!config) {
> -		dev_err(&phy->phy->dev, "unsupported ref_clk_freq:%lu\n", ref_clk_freq);
> +		dev_err(&phy->phy->dev, "unsupported ref_clk_freq: %lu\n", ref_clk_freq);
>   		return -EINVAL;
>   	}
>   
> @@ -461,19 +461,19 @@ static int snps_eusb2_hsphy_init(struct phy *p)
>   
>   	ret = phy_init(phy->repeater);
>   	if (ret) {
> -		dev_err(&p->dev, "repeater init failed. %d\n", ret);
> +		dev_err(&p->dev, "repeater init failed: %d\n", ret);
>   		goto disable_vreg;
>   	}
>   
>   	ret = clk_bulk_prepare_enable(phy->data->num_clks, phy->clks);
>   	if (ret) {
> -		dev_err(&p->dev, "failed to enable ref clock, %d\n", ret);
> +		dev_err(&p->dev, "failed to enable ref clock: %d\n", ret);
>   		goto exit_repeater;
>   	}
>   
>   	ret = reset_control_assert(phy->phy_reset);
>   	if (ret) {
> -		dev_err(&p->dev, "failed to assert phy_reset, %d\n", ret);
> +		dev_err(&p->dev, "failed to assert phy_reset: %d\n", ret);
>   		goto disable_clks;
>   	}
>   
> @@ -481,7 +481,7 @@ static int snps_eusb2_hsphy_init(struct phy *p)
>   
>   	ret = reset_control_deassert(phy->phy_reset);
>   	if (ret) {
> -		dev_err(&p->dev, "failed to de-assert phy_reset, %d\n", ret);
> +		dev_err(&p->dev, "failed to de-assert phy_reset: %d\n", ret);
>   		goto disable_clks;
>   	}
>   
> @@ -588,7 +588,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>   
>   	generic_phy = devm_phy_create(dev, NULL, &snps_eusb2_hsphy_ops);
>   	if (IS_ERR(generic_phy)) {
> -		dev_err(dev, "failed to create phy %d\n", ret);
> +		dev_err(dev, "failed to create phy: %d\n", ret);
>   		return PTR_ERR(generic_phy);
>   	}
>   

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 5/7] phy: phy-snps-eusb2: fix optional phy lookup parameter
  2025-05-23  8:48 ` [PATCH 5/7] phy: phy-snps-eusb2: fix optional phy lookup parameter Johan Hovold
@ 2025-05-26  8:27   ` neil.armstrong
  0 siblings, 0 replies; 17+ messages in thread
From: neil.armstrong @ 2025-05-26  8:27 UTC (permalink / raw)
  To: Johan Hovold, Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel

On 23/05/2025 10:48, Johan Hovold wrote:
> The devm_of_phy_optional_get() takes an optional name argument as its
> third parameter and not an index like the recently replaced
> devm_of_phy_get_by_index().
> 
> Replace 0 with an explicit NULL for consistency and readability.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/phy/phy-snps-eusb2.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index 232c5e8a554f..328e67ebfe03 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -581,7 +581,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>   		return dev_err_probe(dev, ret,
>   				     "failed to get regulator supplies\n");
>   
> -	phy->repeater = devm_of_phy_optional_get(dev, np, 0);
> +	phy->repeater = devm_of_phy_optional_get(dev, np, NULL);
>   	if (IS_ERR(phy->repeater))
>   		return dev_err_probe(dev, PTR_ERR(phy->repeater),
>   				     "failed to get repeater\n");

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 6/7] phy: phy-snps-eusb2: drop unnecessary loop index declarations
  2025-05-23  8:48 ` [PATCH 6/7] phy: phy-snps-eusb2: drop unnecessary loop index declarations Johan Hovold
@ 2025-05-26  8:27   ` neil.armstrong
  0 siblings, 0 replies; 17+ messages in thread
From: neil.armstrong @ 2025-05-26  8:27 UTC (permalink / raw)
  To: Johan Hovold, Vinod Koul
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel

On 23/05/2025 10:48, Johan Hovold wrote:
> There is already a loop index variable declared at function scope so
> drop the unnecessary overloaded loop declarations.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/phy/phy-snps-eusb2.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index 328e67ebfe03..c67712bd8bba 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -552,7 +552,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>   	if (!phy->clks)
>   		return -ENOMEM;
>   
> -	for (int i = 0; i < phy->data->num_clks; ++i)
> +	for (i = 0; i < phy->data->num_clks; ++i)
>   		phy->clks[i].id = phy->data->clk_names[i];
>   
>   	ret = devm_clk_bulk_get(dev, phy->data->num_clks, phy->clks);
> @@ -561,7 +561,7 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>   				     "failed to get phy clock(s)\n");
>   
>   	phy->ref_clk = NULL;
> -	for (int i = 0; i < phy->data->num_clks; ++i) {
> +	for (i = 0; i < phy->data->num_clks; ++i) {
>   		if (!strcmp(phy->clks[i].id, "ref")) {
>   			phy->ref_clk = phy->clks[i].clk;
>   			break;

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups
  2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
                   ` (7 preceding siblings ...)
  2025-05-23  8:57 ` [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Abel Vesa
@ 2025-06-15 16:53 ` Vinod Koul
  8 siblings, 0 replies; 17+ messages in thread
From: Vinod Koul @ 2025-06-15 16:53 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Kishon Vijay Abraham I, Ivaylo Ivanov, Abel Vesa, linux-phy,
	linux-kernel


On Fri, 23 May 2025 10:48:32 +0200, Johan Hovold wrote:
> Here are a couple of fixes and some related cleanups to the recently
> reworked and renamed phy-snps-eusb2 driver.
> 
> The clock and repeater imbalance fixes are not marked for stable as the
> first issue was introduced in the recent rework which is queued for
> 6.16-rc1.
> 
> [...]

Applied, thanks!

[1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit()
      commit: 641fa5b515a6900b1452cc92d30d1ab391e04414
[2/7] phy: phy-snps-eusb2: fix repeater imbalance on phy_init() failure
      commit: 3232a6b0d834569f71aa898401288af6b4ab781d
[3/7] phy: phy-snps-eusb2: rename phy_init() clock error label
      commit: 4f333990841e06059c3cd7251791017d4c9e9028
[4/7] phy: phy-snps-eusb2: clean up error messages
      commit: b7996f8e9473cf8a594af1fa1bb799f8f28c0670
[5/7] phy: phy-snps-eusb2: fix optional phy lookup parameter
      commit: f21b9bea6bc29de88b885cecd5e4f0ada60d4700
[6/7] phy: phy-snps-eusb2: drop unnecessary loop index declarations
      commit: d2d0ae723ba3fca2c54dfbc758b368d3009e79a7
[7/7] phy: phy-snps-eusb2: clean up id table sentinel
      commit: 47311eaa0a3be575f7835d99e3767f5ee5940b45

Best regards,
-- 
~Vinod



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-06-15 16:53 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-23  8:48 [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Johan Hovold
2025-05-23  8:48 ` [PATCH 1/7] phy: phy-snps-eusb2: fix clock imbalance on phy_exit() Johan Hovold
2025-05-25 13:17   ` Ivaylo Ivanov
2025-05-26  7:49   ` neil.armstrong
2025-05-23  8:48 ` [PATCH 2/7] phy: phy-snps-eusb2: fix repeater imbalance on phy_init() failure Johan Hovold
2025-05-26  7:49   ` Neil Armstrong
2025-05-23  8:48 ` [PATCH 3/7] phy: phy-snps-eusb2: rename phy_init() clock error label Johan Hovold
2025-05-26  7:49   ` neil.armstrong
2025-05-23  8:48 ` [PATCH 4/7] phy: phy-snps-eusb2: clean up error messages Johan Hovold
2025-05-26  7:50   ` neil.armstrong
2025-05-23  8:48 ` [PATCH 5/7] phy: phy-snps-eusb2: fix optional phy lookup parameter Johan Hovold
2025-05-26  8:27   ` neil.armstrong
2025-05-23  8:48 ` [PATCH 6/7] phy: phy-snps-eusb2: drop unnecessary loop index declarations Johan Hovold
2025-05-26  8:27   ` neil.armstrong
2025-05-23  8:48 ` [PATCH 7/7] phy: phy-snps-eusb2: clean up id table sentinel Johan Hovold
2025-05-23  8:57 ` [PATCH 0/7] phy: phy-snps-eusb2: fixes and cleanups Abel Vesa
2025-06-15 16:53 ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).