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

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

Posted by Jun Yan 10 hours ago

The DP PHY is optional: the Rockchip RK3399 eDP controller (used as the
internal eDP port) has no separate PHY device and relies on its built-in
PHY, which is programmed directly through the GRF registers, so its DT
node carries no "phys"/"phy-names" property.

Requesting the PHY with devm_phy_get() therefore returns -ENODEV (or
-ENOSYS when CONFIG_GENERIC_PHY is disabled) and the driver logs a
misleading "no DP phy configured" error at probe time before falling back
to a NULL phy.

Use devm_phy_optional_get() instead, which returns NULL when the PHY is
absent and otherwise behaves identically, and drop the manual -ENODEV/
-ENOSYS handling. This also removes the spurious error message.

Assisted-by: Claude:deepseek-v4-flash
Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
---
 .../gpu/drm/bridge/analogix/analogix_dp_core.c | 18 +++---------------
 1 file changed, 3 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 18e76d1e78e1..28eb668d3e3f 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1427,21 +1427,9 @@ 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);
-		}
-	}
+	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.55.0

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

Posted by Damon Ding 9 hours ago

Hi Jun,

On 9/24/2026 10:04 AM, Jun Yan wrote:
> The DP PHY is optional: the Rockchip RK3399 eDP controller (used as the
> internal eDP port) has no separate PHY device and relies on its built-in
> PHY, which is programmed directly through the GRF registers, so its DT
> node carries no "phys"/"phy-names" property.
> 
> Requesting the PHY with devm_phy_get() therefore returns -ENODEV (or
> -ENOSYS when CONFIG_GENERIC_PHY is disabled) and the driver logs a
> misleading "no DP phy configured" error at probe time before falling back
> to a NULL phy.
> 
> Use devm_phy_optional_get() instead, which returns NULL when the PHY is
> absent and otherwise behaves identically, and drop the manual -ENODEV/
> -ENOSYS handling. This also removes the spurious error message.
> 
> Assisted-by: Claude:deepseek-v4-flash
> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> ---
>   .../gpu/drm/bridge/analogix/analogix_dp_core.c | 18 +++---------------
>   1 file changed, 3 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 18e76d1e78e1..28eb668d3e3f 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1427,21 +1427,9 @@ 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);
> -		}
> -	}

It should be better to add some comments here:

/*
  * Exynos5/RK3288/RK3399 use Analogix DP controller built-in PHY,
  * no separate PHY configuration is needed.
  */

> +	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)) {

With the comment added:
Reviewed-by: Damon Ding <damon.ding@rock-chips.com>

Best regards,
Damon

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

Posted by Damon Ding 9 hours ago

On 9/24/2026 11:24 AM, Damon Ding wrote:
> Hi Jun,
> 
> On 9/24/2026 10:04 AM, Jun Yan wrote:
>> The DP PHY is optional: the Rockchip RK3399 eDP controller (used as the
>> internal eDP port) has no separate PHY device and relies on its built-in
>> PHY, which is programmed directly through the GRF registers, so its DT
>> node carries no "phys"/"phy-names" property.
>>
>> Requesting the PHY with devm_phy_get() therefore returns -ENODEV (or
>> -ENOSYS when CONFIG_GENERIC_PHY is disabled) and the driver logs a
>> misleading "no DP phy configured" error at probe time before falling back
>> to a NULL phy.
>>
>> Use devm_phy_optional_get() instead, which returns NULL when the PHY is
>> absent and otherwise behaves identically, and drop the manual -ENODEV/
>> -ENOSYS handling. This also removes the spurious error message.
>>
>> Assisted-by: Claude:deepseek-v4-flash
>> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
>> ---
>>   .../gpu/drm/bridge/analogix/analogix_dp_core.c | 18 +++---------------
>>   1 file changed, 3 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 18e76d1e78e1..28eb668d3e3f 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -1427,21 +1427,9 @@ 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);
>> -        }
>> -    }
> 
> It should be better to add some comments here:
> 
> /*
>   * Exynos5/RK3288/RK3399 use Analogix DP controller built-in PHY,
>   * no separate PHY configuration is needed.
>   */

Well, I think it should be fixed to:

/*
  * RK3288/RK3399 use Analogix DP controller built-in PHY,
  * no extra PHY configuration is required.
  */

The Exynos5xxx case may be more complex.

> 
>> +    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)) {
> 
> With the comment added:
> Reviewed-by: Damon Ding <damon.ding@rock-chips.com>
> 
> Best regards,
> Damon
>

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

Posted by Jun Yan an hour ago

> On 9/24/2026 11:24 AM, Damon Ding wrote:
> > Hi Jun,
> > 
> > On 9/24/2026 10:04 AM, Jun Yan wrote:
> >> The DP PHY is optional: the Rockchip RK3399 eDP controller (used as the
> >> internal eDP port) has no separate PHY device and relies on its built-in
> >> PHY, which is programmed directly through the GRF registers, so its DT
> >> node carries no "phys"/"phy-names" property.
> >>
> >> Requesting the PHY with devm_phy_get() therefore returns -ENODEV (or
> >> -ENOSYS when CONFIG_GENERIC_PHY is disabled) and the driver logs a
> >> misleading "no DP phy configured" error at probe time before falling back
> >> to a NULL phy.
> >>
> >> Use devm_phy_optional_get() instead, which returns NULL when the PHY is
> >> absent and otherwise behaves identically, and drop the manual -ENODEV/
> >> -ENOSYS handling. This also removes the spurious error message.
> >>
> >> Assisted-by: Claude:deepseek-v4-flash
> >> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> >> ---
> >>   .../gpu/drm/bridge/analogix/analogix_dp_core.c | 18 +++---------------
> >>   1 file changed, 3 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 18e76d1e78e1..28eb668d3e3f 100644
> >> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> >> @@ -1427,21 +1427,9 @@ 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);
> >> -        }
> >> -    }
> > 
> > It should be better to add some comments here:
> > 
> > /*
> >   * Exynos5/RK3288/RK3399 use Analogix DP controller built-in PHY,
> >   * no separate PHY configuration is needed.
> >   */
> 
> Well, I think it should be fixed to:
> 
> /*
>   * RK3288/RK3399 use Analogix DP controller built-in PHY,
>   * no extra PHY configuration is required.
>   */
> 
> The Exynos5xxx case may be more complex.
> 

Thanks for the review. I will add the comments in the patch and send v2 soon.

> > 
> >> +    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)) {
> > 
> > With the comment added:
> > Reviewed-by: Damon Ding <damon.ding@rock-chips.com>
> > 
> > Best regards,
> > Damon
> >