From: Damon Ding <damon.ding@rock-chips.com>
To: Luca Ceresoli <luca.ceresoli@bootlin.com>,
Jun Yan <jerrysteve1101@gmail.com>,
Andrzej Hajda <andrzej.hajda@intel.com>,
Neil Armstrong <neil.armstrong@linaro.org>,
Robert Foss <rfoss@kernel.org>,
Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
Jonas Karlman <jonas@kwiboo.se>,
Jernej Skrabec <jernej.skrabec@gmail.com>,
Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Maxime Ripard <mripard@kernel.org>,
Thomas Zimmermann <tzimmermann@suse.de>,
David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
Heiko Stuebner <heiko@sntech.de>
Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
Subject: Re: [PATCH v3] drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY
Date: Fri, 13 Nov 2026 09:15:36 +0800 [thread overview]
Message-ID: <385a67f0-e5cd-4dba-9b9c-5b766462c6c0@rock-chips.com> (raw)
In-Reply-To: <DMTCFJVH99S4.27AWO8PN3ZPK7@bootlin.com>
On 11/12/2026 11:39 PM, Luca Ceresoli wrote:
> Hello,
>
> On Tue Nov 10, 2026 at 3:37 AM CET, 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.
>>
>> Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
>> Reviewed-by: Damon Ding <damon.ding@rock-chips.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);
>
> The original code ignbores -ENOSYS, devm_phy_optional_get() does not. So,
> either -ENOSYS was just impossible, or this patch potentially changes the
> behaviour.
>
> Can you elaborate on that?
>
>
You are right, this is really a valid concern.
If CONFIG_GENERIC_PHY is disabled, the old code could still proceed and
keep the driver probing, while the new version would return an error
directly.
It should be better to keep the -ENOSYS handling to preserve the
original behavior.
Best regards,
Damon
prev parent reply other threads:[~2026-11-13 1:15 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-10 2:37 Jun Yan
2026-11-12 15:39 ` Luca Ceresoli
2026-11-13 1:15 ` Damon Ding [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=385a67f0-e5cd-4dba-9b9c-5b766462c6c0@rock-chips.com \
--to=damon.ding@rock-chips.com \
--cc=Laurent.pinchart@ideasonboard.com \
--cc=airlied@gmail.com \
--cc=andrzej.hajda@intel.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=heiko@sntech.de \
--cc=jernej.skrabec@gmail.com \
--cc=jerrysteve1101@gmail.com \
--cc=jonas@kwiboo.se \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=luca.ceresoli@bootlin.com \
--cc=maarten.lankhorst@linux.intel.com \
--cc=mripard@kernel.org \
--cc=neil.armstrong@linaro.org \
--cc=rfoss@kernel.org \
--cc=simona@ffwll.ch \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®