Re: [PATCH v2] net: phy: Fix return value when !CONFIG_PHYLIB

[Heiner Kallweit <hkallweit1@gmail.com>]

On 13.04.2025 15:37, hhtracer@gmail.com wrote:
> From: huhai <huhai@kylinos.cn>
> 
> Many call sites of get_phy_device() and fwnode_get_phy_node(), such as
> sfp_sm_probe_phy(), phylink_fwnode_phy_connect(), etc., rely on IS_ERR()
> to check for errors in the returned pointer.
> 
> Furthermore, the implementations of get_phy_device() and
> fwnode_get_phy_node() themselves use ERR_PTR() to return error codes.
> 
> Therefore, when CONFIG_PHYLIB is disabled, returning NULL is incorrect,
> as this would bypass IS_ERR() checks and may lead to NULL pointer
> dereference.
> 
Is there actually any call site which doesn't select PHYLIB directly or
indirectly? When briefly checking I didn't find one.
So my question would be rather: Do we need/want stubs for the following
functions at all?

fwnode_get_phy_id
fwnode_mdio_find_device
fwnode_phy_find_device
device_phy_find_device
fwnode_get_phy_node
get_phy_device
phy_device_register


And a formal remark:
Your v2 has no change log, and please allow 24h before sending a new version.

> Returning ERR_PTR(-ENXIO) is the correct and consistent way to indicate
> that PHY support is not available, and it avoids such issues.
> 
> Signed-off-by: huhai <huhai@kylinos.cn>
> ---
>  include/linux/phy.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index a2bfae80c449..be299c572d73 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1787,13 +1787,13 @@ static inline struct phy_device *device_phy_find_device(struct device *dev)
>  static inline
>  struct fwnode_handle *fwnode_get_phy_node(struct fwnode_handle *fwnode)
>  {
> -	return NULL;
> +	return ERR_PTR(-ENXIO);
>  }
>  
>  static inline
>  struct phy_device *get_phy_device(struct mii_bus *bus, int addr, bool is_c45)
>  {
> -	return NULL;
> +	return ERR_PTR(-ENXIO);
>  }
>  
>  static inline int phy_device_register(struct phy_device *phy)