[PATCH v4] drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY

[PATCH v4] drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY

[Jun Yan]

The DP PHY is optional: the Rockchip RK3288/RK3399 eDP controllers use the
built-in PHY of the Analogix DP controller, which is programmed through
the GRF registers, so their DT nodes have no "phys"/"phy-names"
properties. devm_phy_get() therefore returns -ENODEV and the driver logs
a misleading "no DP phy configured" error before falling back to a NULL
phy.

Use devm_phy_optional_get() instead, which returns NULL when the PHY is
absent and propagates any other error, and drop the open-coded -ENODEV/
-ENOSYS handling and the spurious error message.

The behaviour is unchanged for both CONFIG_GENERIC_PHY settings, because
the real implementation never returns -ENOSYS:

- CONFIG_GENERIC_PHY=n: devm_phy_optional_get() is a stub returning NULL
  (devm_phy_get() returns -ENOSYS, which the old code mapped to NULL).
- CONFIG_GENERIC_PHY=y, no PHY described in DT: devm_phy_optional_get()
  converts -ENODEV to NULL, exactly like the old code, which mapped
  -ENODEV to NULL in its open-coded check.
- CONFIG_GENERIC_PHY=y, any other error: still propagated, as before.

Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Reviewed-by: Damon Ding <damon.ding@rock-chips.com>
---
Changes in v4:
  - Rework the commit message to explain why the behaviour is unchanged
    for both CONFIG_GENERIC_PHY settings, with one bullet per case, as
    requested by Luca Ceresoli.
  - Link to v3: https://lore.kernel.org/dri-devel/20261110023721.2014061-1-jerrysteve1101@gmail.com/

Changes in v3:
  - Add missing changelog entries and reference links
  - Link to v2: https://lore.kernel.org/dri-devel/20261013134659.335829-1-jerrysteve1101@gmail.com/

Changes in v2:
  - Added comments to explain the changes made.
  - Link to v1: https://lore.kernel.org/dri-devel/20260924020417.868588-1-jerrysteve1101@gmail.com/

 .../drm/bridge/analogix/analogix_dp_core.c    | 22 ++++++-------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 6df6963740d2..637b1bff0f6c 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1426,21 +1426,13 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
 	if (ret)
 		return ERR_PTR(ret);
 
-	dp->phy = devm_phy_get(dp->dev, "dp");
-	if (IS_ERR(dp->phy)) {
-		dev_err(dp->dev, "no DP phy configured\n");
-		ret = PTR_ERR(dp->phy);
-		if (ret) {
-			/*
-			 * phy itself is not enabled, so we can move forward
-			 * assigning NULL to phy pointer.
-			 */
-			if (ret == -ENOSYS || ret == -ENODEV)
-				dp->phy = NULL;
-			else
-				return ERR_PTR(ret);
-		}
-	}
+	/*
+	 * RK3288/RK3399 use Analogix DP controller built-in PHY,
+	 * no extra PHY configuration is required.
+	 */
+	dp->phy = devm_phy_optional_get(dp->dev, "dp");
+	if (IS_ERR(dp->phy))
+		return ERR_CAST(dp->phy);
 
 	dp->clock = devm_clk_get(&pdev->dev, "dp");
 	if (IS_ERR(dp->clock)) {
-- 
2.56.0

[PATCH v4] drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY

[Jun Yan]

The DP PHY is optional: the Rockchip RK3288/RK3399 eDP controllers use the
built-in PHY of the Analogix DP controller, which is programmed through
the GRF registers, so their DT nodes have no "phys"/"phy-names"
properties. devm_phy_get() therefore returns -ENODEV and the driver logs
a misleading "no DP phy configured" error before falling back to a NULL
phy.

Use devm_phy_optional_get() instead, which returns NULL when the PHY is
absent and propagates any other error, and drop the open-coded -ENODEV/
-ENOSYS handling and the spurious error message.

The behaviour is unchanged for both CONFIG_GENERIC_PHY settings, because
the real implementation never returns -ENOSYS:

- CONFIG_GENERIC_PHY=n: devm_phy_optional_get() is a stub returning NULL
  (devm_phy_get() returns -ENOSYS, which the old code mapped to NULL).
- CONFIG_GENERIC_PHY=y, no PHY described in DT: devm_phy_optional_get()
  converts -ENODEV to NULL, exactly like the old code, which mapped
  -ENODEV to NULL in its open-coded check.
- CONFIG_GENERIC_PHY=y, any other error: still propagated, as before.

Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
Reviewed-by: Damon Ding <damon.ding@rock-chips.com>
---
Changes in v4:
  - Rework the commit message to explain why the behaviour is unchanged
    for both CONFIG_GENERIC_PHY settings, with one bullet per case, as
    requested by Luca Ceresoli.
  - Link to v3: https://lore.kernel.org/dri-devel/20261110023721.2014061-1-jerrysteve1101@gmail.com/

Changes in v3:
  - Add missing changelog entries and reference links
  - Link to v2: https://lore.kernel.org/dri-devel/20261013134659.335829-1-jerrysteve1101@gmail.com/

Changes in v2:
  - Added comments to explain the changes made.
  - Link to v1: https://lore.kernel.org/dri-devel/20260924020417.868588-1-jerrysteve1101@gmail.com/

 .../drm/bridge/analogix/analogix_dp_core.c    | 22 ++++++-------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 6df6963740d2..637b1bff0f6c 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1426,21 +1426,13 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
 	if (ret)
 		return ERR_PTR(ret);
 
-	dp->phy = devm_phy_get(dp->dev, "dp");
-	if (IS_ERR(dp->phy)) {
-		dev_err(dp->dev, "no DP phy configured\n");
-		ret = PTR_ERR(dp->phy);
-		if (ret) {
-			/*
-			 * phy itself is not enabled, so we can move forward
-			 * assigning NULL to phy pointer.
-			 */
-			if (ret == -ENOSYS || ret == -ENODEV)
-				dp->phy = NULL;
-			else
-				return ERR_PTR(ret);
-		}
-	}
+	/*
+	 * RK3288/RK3399 use Analogix DP controller built-in PHY,
+	 * no extra PHY configuration is required.
+	 */
+	dp->phy = devm_phy_optional_get(dp->dev, "dp");
+	if (IS_ERR(dp->phy))
+		return ERR_CAST(dp->phy);
 
 	dp->clock = devm_clk_get(&pdev->dev, "dp");
 	if (IS_ERR(dp->clock)) {
-- 
2.56.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH v4] drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] drm/bridge: analogix_dp: inaccurate RK3288 PHY comment in
  analogix_dp_probe()
--

commit 720fa3eab5052c038874ca588f7e1541886e6915
Author: Jun Yan <jerrysteve1101@gmail.com>

drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY

This commit replaces devm_phy_get() and its open-coded error handling with
devm_phy_optional_get() in analogix_dp_probe() when acquiring the optional
DP PHY.

> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 8bb4c48aa3b48..09171c6605d0b 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1427,21 +1427,13 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data)
>  	if (ret)
>  		return ERR_PTR(ret);
>  
> -	dp->phy = devm_phy_get(dp->dev, "dp");
> -	if (IS_ERR(dp->phy)) {
> -		dev_err(dp->dev, "no DP phy configured\n");
> -		ret = PTR_ERR(dp->phy);
> -		if (ret) {
> -			/*
> -			 * phy itself is not enabled, so we can move forward
> -			 * assigning NULL to phy pointer.
> -			 */
> -			if (ret == -ENOSYS || ret == -ENODEV)
> -				dp->phy = NULL;
> -			else
> -				return ERR_PTR(ret);
> -		}
> -	}
> +	/*
> +	 * RK3288/RK3399 use Analogix DP controller built-in PHY,
> +	 * no extra PHY configuration is required.
> +	 */
> +	dp->phy = devm_phy_optional_get(dp->dev, "dp");
> +	if (IS_ERR(dp->phy))
> +		return ERR_CAST(dp->phy);

[Severity: Low]
Are this comment and the commit message accurate about RK3288?

In arch/arm/boot/dts/rockchip/rk3288.dtsi and
Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml,
the RK3288 eDP node defines phys = <&edp_phy> and phy-names = "dp" for its
rockchip,rk3288-dp-phy device, which drivers/phy/rockchip/phy-rockchip-dp.c
programs via GRF_SOC_CON12 when phy_power_on() and phy_power_off() are
called on dp->phy.

RK3399 and RK3568 are the variants that omit the phys property and configure
the built-in PHY directly through the controller MMIO registers rather than
through a separate PHY device or GRF.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115044755.395888-1-jerrysteve1101@gmail.com?part=1

Re: [PATCH v4] drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY

[Jun Yan]

Hi,

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Low] drm/bridge: analogix_dp: inaccurate RK3288 PHY comment in
>   analogix_dp_probe()

> [Severity: Low]
> Are this comment and the commit message accurate about RK3288?
>
> In arch/arm/boot/dts/rockchip/rk3288.dtsi and
> Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml,
> the RK3288 eDP node defines phys = <&edp_phy> and phy-names = "dp" for its
> rockchip,rk3288-dp-phy device, which drivers/phy/rockchip/phy-rockchip-dp.c
> programs via GRF_SOC_CON12 when phy_power_on() and phy_power_off() are
> called on dp->phy.

This is indeed inaccurate. Good catch. I will remove it in v5.

Do you have any opinions on this issue, Damon?

> RK3399 and RK3568 are the variants that omit the phys property and configure
> the built-in PHY directly through the controller MMIO registers rather than
> through a separate PHY device or GRF.

Yes, you're right. RK3399 doesn't configure PHY through GRF; MMIO registers
are the correct way. (RK3568 eDP is not supported in mainline yet.)

I'll send a v5 with the comment removed and the commit message reworded.

Thanks,
Jun