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] [day] [month] [year] [list]

Message-ID: <d591574c-80e5-494d-8b67-7b85594fd821@huawei.com>
Date: Mon, 30 Jun 2025 22:58:35 +0800
From: Jijie Shao <shaojijie@...wei.com>
To: Larysa Zaremba <larysa.zaremba@...el.com>
CC: <shaojijie@...wei.com>, <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 2025/6/30 16:38, Larysa Zaremba wrote:
> 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

Thank you, if I need to send v4 for other reasons, I will modify it together.

>> 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.


Because the macro name is much shorter than the above,
there is a tab between the numbers and the name in these two lines.

Thanks,
Jijie Shao

>
>>   
>>   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
>>
>>