linux-kernel - Re: [PATCH v3 net-next 1/3] net: hibmcge: support scenario without PHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aGJNHsCMwVLqbAq0@soc-5CG4396X81.clients.intel.com>
Date: Mon, 30 Jun 2025 10:38:54 +0200
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: Jijie Shao <shaojijie@...wei.com>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <andrew+netdev@...n.ch>, <horms@...nel.org>,
	<shenjian15@...wei.com>, <liuyonglong@...wei.com>, <chenhao418@...wei.com>,
	<jonathan.cameron@...wei.com>, <shameerali.kolothum.thodi@...wei.com>,
	<salil.mehta@...wei.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 net-next 1/3] net: hibmcge: support scenario without
 PHY

On Thu, Jun 26, 2025 at 10:06:11AM +0800, Jijie Shao wrote:
> Currently, the driver uses phylib to operate PHY by default.
> 
> On some boards, the PHY device is separated from the MAC device.
> As a result, the hibmcge driver cannot operate the PHY device.
> 
> In this patch, the driver determines whether a PHY is available
> based on register configuration. If no PHY is available,
> the driver use fixed_phy to register fake phydev.

uses/will use

> 
> Signed-off-by: Jijie Shao <shaojijie@...wei.com>

Some minor cosmetic problems, but overall seems like you nicely incorporated v2 
feedback.

Reviewed-by: Larysa Zaremba <larysa.zaremba@...el.com>

> ---
> ChangeLog:
> v2 -> v3:
>   - Use fixed_phy to re-implement the no-phy scenario, suggested by Andrew Lunn
>   v2: https://lore.kernel.org/all/20250623034129.838246-1-shaojijie@huawei.com/
> ---
>  .../net/ethernet/hisilicon/hibmcge/hbg_mdio.c | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
> index 42b0083c9193..41558fe7770c 100644
> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_mdio.c
> @@ -2,6 +2,7 @@
>  // Copyright (c) 2024 Hisilicon Limited.
>  
>  #include <linux/phy.h>
> +#include <linux/phy_fixed.h>
>  #include <linux/rtnetlink.h>
>  #include "hbg_common.h"
>  #include "hbg_hw.h"
> @@ -19,6 +20,7 @@
>  #define HBG_MDIO_OP_INTERVAL_US		(5 * 1000)
>  
>  #define HBG_NP_LINK_FAIL_RETRY_TIMES	5
> +#define HBG_NO_PHY	0xFF

Number does not align with one in the previous line.

>  
>  static void hbg_mdio_set_command(struct hbg_mac *mac, u32 cmd)
>  {
> @@ -229,6 +231,39 @@ void hbg_phy_stop(struct hbg_priv *priv)
>  	phy_stop(priv->mac.phydev);
>  }
>  
> +static void hbg_fixed_phy_uninit(void *data)
> +{
> +	fixed_phy_unregister((struct phy_device *)data);
> +}
> +
> +static int hbg_fixed_phy_init(struct hbg_priv *priv)
> +{
> +	struct fixed_phy_status hbg_fixed_phy_status = {
> +		.link = 1,
> +		.speed = SPEED_1000,
> +		.duplex = DUPLEX_FULL,
> +		.pause = 1,
> +		.asym_pause = 1,
> +	};
> +	struct device *dev = &priv->pdev->dev;
> +	struct phy_device *phydev;
> +	int ret;
> +
> +	phydev = fixed_phy_register(&hbg_fixed_phy_status, NULL);
> +	if (IS_ERR(phydev)) {
> +		dev_err_probe(dev, IS_ERR(phydev),
> +			      "failed to register fixed PHY device\n");
> +		return IS_ERR(phydev);
> +	}
> +
> +	ret = devm_add_action_or_reset(dev, hbg_fixed_phy_uninit, phydev);
> +	if (ret)
> +		return ret;
> +
> +	priv->mac.phydev = phydev;
> +	return hbg_phy_connect(priv);
> +}
> +
>  int hbg_mdio_init(struct hbg_priv *priv)
>  {
>  	struct device *dev = &priv->pdev->dev;
> @@ -238,6 +273,9 @@ int hbg_mdio_init(struct hbg_priv *priv)
>  	int ret;
>  
>  	mac->phy_addr = priv->dev_specs.phy_addr;
> +	if (mac->phy_addr == HBG_NO_PHY)
> +		return hbg_fixed_phy_init(priv);
> +
>  	mdio_bus = devm_mdiobus_alloc(dev);
>  	if (!mdio_bus)
>  		return dev_err_probe(dev, -ENOMEM,
> -- 
> 2.33.0
> 
>