[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a863646c-adc0-4d16-aad3-158702dfef45@lunn.ch>
Date: Tue, 10 Sep 2024 14:34:12 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jijie Shao <shaojijie@...wei.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, shenjian15@...wei.com, wangpeiyang1@...wei.com,
liuyonglong@...wei.com, chenhao418@...wei.com,
sudongming1@...wei.com, xujunsheng@...wei.com,
shiyongbang@...wei.com, libaihan@...wei.com, jdamato@...tly.com,
horms@...nel.org, kalesh-anakkur.purayil@...adcom.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 V9 net-next 05/11] net: hibmcge: Implement some .ndo
functions
On Tue, Sep 10, 2024 at 03:59:36PM +0800, Jijie Shao wrote:
> Implement the .ndo_open() .ndo_stop() .ndo_set_mac_address()
> .ndo_change_mtu functions() and ndo.get_stats64()
> And .ndo_validate_addr calls the eth_validate_addr function directly
>
> Signed-off-by: Jijie Shao <shaojijie@...wei.com>
> ---
> ChangeLog:
> v8 -> v9:
> - Remove HBG_NIC_STATE_OPEN in ndo.open() and ndo.stop(),
> suggested by Kalesh and Andrew.
> - Use netif_running() instead of hbg_nic_is_open() in ndo.change_mtu(),
> suggested by Kalesh and Andrew
> v8: https://lore.kernel.org/all/20240909023141.3234567-1-shaojijie@huawei.com/
> v6 -> v7:
> - Add implement ndo.get_stats64(), suggested by Paolo.
> v6: https://lore.kernel.org/all/20240830121604.2250904-6-shaojijie@huawei.com/
> v5 -> v6:
> - Delete netif_carrier_off() in .ndo_open() and .ndo_stop(),
> suggested by Jakub and Andrew.
> v5: https://lore.kernel.org/all/20240827131455.2919051-1-shaojijie@huawei.com/
> v3 -> v4:
> - Delete INITED_STATE in priv, suggested by Andrew.
> - Delete unnecessary defensive code in hbg_phy_start()
> and hbg_phy_stop(), suggested by Andrew.
> v3: https://lore.kernel.org/all/20240822093334.1687011-1-shaojijie@huawei.com/
> RFC v1 -> RFC v2:
> - Delete validation for mtu in hbg_net_change_mtu(), suggested by Andrew.
> - Delete validation for mac address in hbg_net_set_mac_address(),
> suggested by Andrew.
> - Add a patch to add is_valid_ether_addr check in dev_set_mac_address,
> suggested by Andrew.
> RFC v1: https://lore.kernel.org/all/20240731094245.1967834-1-shaojijie@huawei.com/
> ---
> .../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 39 ++++++++
> .../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 3 +
> .../net/ethernet/hisilicon/hibmcge/hbg_main.c | 97 +++++++++++++++++++
> .../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 11 ++-
> 4 files changed, 149 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
> index 8e971e9f62a0..97fee714155a 100644
> --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
> +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
> @@ -15,6 +15,7 @@
> * ctrl means packet description, data means skb packet data
> */
> #define HBG_ENDIAN_CTRL_LE_DATA_BE 0x0
> +#define HBG_PCU_FRAME_LEN_PLUS 4
>
> static bool hbg_hw_spec_is_valid(struct hbg_priv *priv)
> {
> @@ -129,6 +130,44 @@ void hbg_hw_irq_enable(struct hbg_priv *priv, u32 mask, bool enable)
> hbg_reg_write(priv, HBG_REG_CF_INTRPT_MSK_ADDR, value);
> }
>
> +void hbg_hw_set_uc_addr(struct hbg_priv *priv, u64 mac_addr)
> +{
> + hbg_reg_write64(priv, HBG_REG_STATION_ADDR_LOW_2_ADDR, mac_addr);
> +}
> +
> @@ -88,6 +181,10 @@ static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> if (ret)
> return ret;
>
> + netdev->max_mtu = priv->dev_specs.max_mtu;
> + netdev->min_mtu = priv->dev_specs.min_mtu;
> + hbg_change_mtu(priv, HBG_DEFAULT_MTU_SIZE);
It does not help that you added added HBG_DEFAULT_MTU_SIZE in a
previous patch, but as far as i see, it is just ETH_DATA_LEN.
Please use the standard defines, rather than adding your own. It makes
the code a lot easier to understand, it is not using some special
jumbo size by default, it is just the plain, boring, normal 1500
bytes.
Andrew
---
pw-bot: cr
Powered by blists - more mailing lists