All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Jun Yan" <jerrysteve1101@gmail.com>, <damon.ding@rock-chips.com>
Cc: <Laurent.pinchart@ideasonboard.com>, <airlied@gmail.com>,
	<andrzej.hajda@intel.com>, <dri-devel@lists.freedesktop.org>,
	<heiko@sntech.de>, <jernej.skrabec@gmail.com>, <jonas@kwiboo.se>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-rockchip@lists.infradead.org>, <luca.ceresoli@bootlin.com>,
	<maarten.lankhorst@linux.intel.com>, <mripard@kernel.org>,
	<neil.armstrong@linaro.org>, <rfoss@kernel.org>,
	<simona@ffwll.ch>, <tzimmermann@suse.de>
Subject: Re: [PATCH v3] drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY
Date: Fri, 13 Nov 2026 16:41:56 +0100	[thread overview]
Message-ID: <DMU749TMB523.3OCGPYVI4NZDU@bootlin.com> (raw)
In-Reply-To: <20261113131248.2804840-1-jerrysteve1101@gmail.com>

Hello,

On Fri Nov 13, 2026 at 2:12 PM CET, Jun Yan wrote:
> Hi Damon,
>
>> 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.
>
> -ENOSYS can only come from the !CONFIG_GENERIC_PHY stubs in
> include/linux/phy/phy.h; the real implementation in drivers/phy/phy-core.c
> never returns it. And the stub for the optional variant returns NULL, not
> -ENOSYS:
>
>     static inline struct phy *devm_phy_optional_get(struct device *dev,
>                                                     const char *string)
>     {
>             return NULL;
>     }
>
> So with CONFIG_GENERIC_PHY disabled the PHY is still treated as absent
> (NULL), exactly as the old -ENOSYS branch did. With CONFIG_GENERIC_PHY
> enabled, -ENOSYS cannot occur, devm_phy_optional_get() turns -ENODEV into
> NULL, and any other error is still propagated - same as the open-coded
> check.
>
>> It should be better to keep the -ENOSYS handling to preserve the
>> original behavior.
>
> Overall the behaviour is unchanged in both configurations, so no separate
> handling is needed.

Thanks for the analysis! Cna you send a v4 with that explained in the
commit message, possibly in a more concise way, maybe with bullets for the
various CONFIG_GENERIC_PHY on/off cases or so, for readability?

Reading it now, this v3 commit message kind of said the same things. but it
misled both me and Damon, so it's worth a clear explanation.

Thanks!

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


WARNING: multiple messages have this Message-ID (diff)
From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Jun Yan" <jerrysteve1101@gmail.com>, <damon.ding@rock-chips.com>
Cc: <Laurent.pinchart@ideasonboard.com>, <airlied@gmail.com>,
	<andrzej.hajda@intel.com>, <dri-devel@lists.freedesktop.org>,
	<heiko@sntech.de>, <jernej.skrabec@gmail.com>, <jonas@kwiboo.se>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>,
	<linux-rockchip@lists.infradead.org>, <luca.ceresoli@bootlin.com>,
	<maarten.lankhorst@linux.intel.com>, <mripard@kernel.org>,
	<neil.armstrong@linaro.org>, <rfoss@kernel.org>,
	<simona@ffwll.ch>, <tzimmermann@suse.de>
Subject: Re: [PATCH v3] drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY
Date: Fri, 13 Nov 2026 16:41:56 +0100	[thread overview]
Message-ID: <DMU749TMB523.3OCGPYVI4NZDU@bootlin.com> (raw)
In-Reply-To: <20261113131248.2804840-1-jerrysteve1101@gmail.com>

Hello,

On Fri Nov 13, 2026 at 2:12 PM CET, Jun Yan wrote:
> Hi Damon,
>
>> 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.
>
> -ENOSYS can only come from the !CONFIG_GENERIC_PHY stubs in
> include/linux/phy/phy.h; the real implementation in drivers/phy/phy-core.c
> never returns it. And the stub for the optional variant returns NULL, not
> -ENOSYS:
>
>     static inline struct phy *devm_phy_optional_get(struct device *dev,
>                                                     const char *string)
>     {
>             return NULL;
>     }
>
> So with CONFIG_GENERIC_PHY disabled the PHY is still treated as absent
> (NULL), exactly as the old -ENOSYS branch did. With CONFIG_GENERIC_PHY
> enabled, -ENOSYS cannot occur, devm_phy_optional_get() turns -ENODEV into
> NULL, and any other error is still propagated - same as the open-coded
> check.
>
>> It should be better to keep the -ENOSYS handling to preserve the
>> original behavior.
>
> Overall the behaviour is unchanged in both configurations, so no separate
> handling is needed.

Thanks for the analysis! Cna you send a v4 with that explained in the
commit message, possibly in a more concise way, maybe with bullets for the
various CONFIG_GENERIC_PHY on/off cases or so, for readability?

Reading it now, this v3 commit message kind of said the same things. but it
misled both me and Damon, so it's worth a clear explanation.

Thanks!

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

  reply	other threads:[~2026-11-13 15:42 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-10  2:37 [PATCH v3] drm/bridge: analogix_dp: use devm_phy_optional_get() for the DP PHY Jun Yan
2026-11-10  2:37 ` Jun Yan
2026-11-12 15:39 ` Luca Ceresoli
2026-11-12 15:39   ` Luca Ceresoli
2026-11-13  1:15   ` Damon Ding
2026-11-13  1:15     ` Damon Ding
2026-11-13 13:12     ` Jun Yan
2026-11-13 13:12       ` Jun Yan
2026-11-13 15:41       ` Luca Ceresoli [this message]
2026-11-13 15:41         ` Luca Ceresoli

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=DMU749TMB523.3OCGPYVI4NZDU@bootlin.com \
    --to=luca.ceresoli@bootlin.com \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=airlied@gmail.com \
    --cc=andrzej.hajda@intel.com \
    --cc=damon.ding@rock-chips.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=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.