Implement the .ndo_open .ndo_stop .ndo_set_mac_address
and .ndo_change_mtu functions.
And .ndo_validate_addr calls the eth_validate_addr function directly
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
---
.../ethernet/hisilicon/hibmcge/hbg_common.h | 3 +
.../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 40 ++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 3 +
.../net/ethernet/hisilicon/hibmcge/hbg_main.c | 99 +++++++++++++++++++
.../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 11 ++-
5 files changed, 155 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
index e94ae2be5c4c..d11ef081f4da 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_common.h
@@ -17,8 +17,11 @@
enum hbg_nic_state {
HBG_NIC_STATE_EVENT_HANDLING = 0,
+ HBG_NIC_STATE_OPEN,
};
+#define hbg_nic_is_open(priv) test_bit(HBG_NIC_STATE_OPEN, &(priv)->state)
+
enum hbg_hw_event_type {
HBG_HW_EVENT_NONE = 0,
HBG_HW_EVENT_INIT, /* driver is loading */
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
index f7a7c8524546..04085a68c317 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c
@@ -125,6 +125,46 @@ 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);
+}
+
+static void hbg_hw_set_pcu_max_frame_len(struct hbg_priv *priv,
+ u16 max_frame_len)
+{
+#define HBG_PCU_FRAME_LEN_PLUS 4
+
+ max_frame_len = max_t(u32, max_frame_len, HBG_DEFAULT_MTU_SIZE);
+
+ /* lower two bits of value must be set to 0. Otherwise, the value is ignored */
+ max_frame_len = round_up(max_frame_len, HBG_PCU_FRAME_LEN_PLUS);
+
+ hbg_reg_write_field(priv, HBG_REG_MAX_FRAME_LEN_ADDR,
+ HBG_REG_MAX_FRAME_LEN_M, max_frame_len);
+}
+
+static void hbg_hw_set_mac_max_frame_len(struct hbg_priv *priv,
+ u16 max_frame_size)
+{
+ hbg_reg_write_field(priv, HBG_REG_MAX_FRAME_SIZE_ADDR,
+ HBG_REG_MAX_FRAME_LEN_M, max_frame_size);
+}
+
+void hbg_hw_set_mtu(struct hbg_priv *priv, u16 mtu)
+{
+ hbg_hw_set_pcu_max_frame_len(priv, mtu);
+ hbg_hw_set_mac_max_frame_len(priv, mtu);
+}
+
+void hbg_hw_mac_enable(struct hbg_priv *priv, u32 enable)
+{
+ hbg_reg_write_field(priv, HBG_REG_PORT_ENABLE_ADDR,
+ HBG_REG_PORT_ENABLE_TX_B, enable);
+ hbg_reg_write_field(priv, HBG_REG_PORT_ENABLE_ADDR,
+ HBG_REG_PORT_ENABLE_RX_B, enable);
+}
+
void hbg_hw_adjust_link(struct hbg_priv *priv, u32 speed, u32 duplex)
{
hbg_reg_write_field(priv, HBG_REG_PORT_MODE_ADDR,
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
index 09946c3966ff..ed72e1192b79 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h
@@ -47,5 +47,8 @@ u32 hbg_hw_get_irq_status(struct hbg_priv *priv);
void hbg_hw_irq_clear(struct hbg_priv *priv, u32 mask);
bool hbg_hw_irq_is_enabled(struct hbg_priv *priv, u32 mask);
void hbg_hw_irq_enable(struct hbg_priv *priv, u32 mask, bool enable);
+void hbg_hw_set_mtu(struct hbg_priv *priv, u16 mtu);
+void hbg_hw_mac_enable(struct hbg_priv *priv, u32 enable);
+void hbg_hw_set_uc_addr(struct hbg_priv *priv, u64 mac_addr);
#endif
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
index fb0e87c8fef7..b7248c830434 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c
@@ -2,6 +2,7 @@
// Copyright (c) 2024 Hisilicon Limited.
#include <linux/etherdevice.h>
+#include <linux/if_vlan.h>
#include <linux/netdevice.h>
#include <linux/pci.h>
#include "hbg_common.h"
@@ -9,6 +10,99 @@
#include "hbg_irq.h"
#include "hbg_mdio.h"
+static void hbg_all_irq_enable(struct hbg_priv *priv, bool enabled)
+{
+ struct hbg_irq_info *info;
+ u32 i;
+
+ for (i = 0; i < priv->vectors.info_array_len; i++) {
+ info = &priv->vectors.info_array[i];
+ hbg_hw_irq_enable(priv, info->mask, enabled);
+ }
+}
+
+static int hbg_net_open(struct net_device *dev)
+{
+ struct hbg_priv *priv = netdev_priv(dev);
+
+ if (test_and_set_bit(HBG_NIC_STATE_OPEN, &priv->state))
+ return 0;
+
+ netif_carrier_off(dev);
+ hbg_all_irq_enable(priv, true);
+ hbg_hw_mac_enable(priv, HBG_STATUS_ENABLE);
+ netif_start_queue(dev);
+ hbg_phy_start(priv);
+
+ return 0;
+}
+
+static int hbg_net_stop(struct net_device *dev)
+{
+ struct hbg_priv *priv = netdev_priv(dev);
+
+ if (!hbg_nic_is_open(priv))
+ return 0;
+
+ clear_bit(HBG_NIC_STATE_OPEN, &priv->state);
+ netif_carrier_off(dev);
+ hbg_phy_stop(priv);
+ netif_stop_queue(dev);
+ hbg_hw_mac_enable(priv, HBG_STATUS_DISABLE);
+ hbg_all_irq_enable(priv, false);
+
+ return 0;
+}
+
+static int hbg_net_set_mac_address(struct net_device *dev, void *addr)
+{
+ struct hbg_priv *priv = netdev_priv(dev);
+ u8 *mac_addr;
+
+ mac_addr = ((struct sockaddr *)addr)->sa_data;
+ if (!is_valid_ether_addr(mac_addr))
+ return -EADDRNOTAVAIL;
+
+ hbg_hw_set_uc_addr(priv, ether_addr_to_u64(mac_addr));
+ dev_addr_set(dev, mac_addr);
+
+ return 0;
+}
+
+static void hbg_change_mtu(struct hbg_priv *priv, int new_mtu)
+{
+ u32 frame_len;
+
+ frame_len = new_mtu + VLAN_HLEN * priv->dev_specs.vlan_layers +
+ ETH_HLEN + ETH_FCS_LEN;
+ hbg_hw_set_mtu(priv, frame_len);
+}
+
+static int hbg_net_change_mtu(struct net_device *dev, int new_mtu)
+{
+ struct hbg_priv *priv = netdev_priv(dev);
+ bool is_opened = hbg_nic_is_open(priv);
+
+ hbg_net_stop(dev);
+
+ hbg_change_mtu(priv, new_mtu);
+ WRITE_ONCE(dev->mtu, new_mtu);
+
+ dev_dbg(&priv->pdev->dev,
+ "change mtu from %u to %u\n", dev->mtu, new_mtu);
+ if (is_opened)
+ hbg_net_open(dev);
+ return 0;
+}
+
+static const struct net_device_ops hbg_netdev_ops = {
+ .ndo_open = hbg_net_open,
+ .ndo_stop = hbg_net_stop,
+ .ndo_validate_addr = eth_validate_addr,
+ .ndo_set_mac_address = hbg_net_set_mac_address,
+ .ndo_change_mtu = hbg_net_change_mtu,
+};
+
static int hbg_init(struct hbg_priv *priv)
{
int ret;
@@ -69,6 +163,7 @@ static int hbg_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
priv = netdev_priv(netdev);
priv->netdev = netdev;
priv->pdev = pdev;
+ netdev->netdev_ops = &hbg_netdev_ops;
ret = hbg_pci_init(pdev);
if (ret)
@@ -78,6 +173,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);
+ hbg_net_set_mac_address(priv->netdev, &priv->dev_specs.mac_addr);
ret = devm_register_netdev(dev, netdev);
if (ret)
return dev_err_probe(dev, ret, "failed to register netdev\n");
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
index b0991063ccba..63bb1bead8c0 100644
--- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
+++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h
@@ -37,18 +37,24 @@
#define HBG_REG_SGMII_BASE 0x10000
#define HBG_REG_DUPLEX_TYPE_ADDR (HBG_REG_SGMII_BASE + 0x0008)
#define HBG_REG_DUPLEX_B BIT(0)
+#define HBG_REG_MAX_FRAME_SIZE_ADDR (HBG_REG_SGMII_BASE + 0x003C)
#define HBG_REG_PORT_MODE_ADDR (HBG_REG_SGMII_BASE + 0x0040)
#define HBG_REG_PORT_MODE_M GENMASK(3, 0)
+#define HBG_REG_PORT_ENABLE_ADDR (HBG_REG_SGMII_BASE + 0x0044)
+#define HBG_REG_PORT_ENABLE_RX_B BIT(1)
+#define HBG_REG_PORT_ENABLE_TX_B BIT(2)
#define HBG_REG_TRANSMIT_CONTROL_ADDR (HBG_REG_SGMII_BASE + 0x0060)
#define HBG_REG_TRANSMIT_CONTROL_PAD_EN_B BIT(7)
#define HBG_REG_TRANSMIT_CONTROL_CRC_ADD_B BIT(6)
#define HBG_REG_TRANSMIT_CONTROL_AN_EN_B BIT(5)
#define HBG_REG_CF_CRC_STRIP_ADDR (HBG_REG_SGMII_BASE + 0x01B0)
-#define HBG_REG_CF_CRC_STRIP_B BIT(0)
+#define HBG_REG_CF_CRC_STRIP_B BIT(1)
#define HBG_REG_MODE_CHANGE_EN_ADDR (HBG_REG_SGMII_BASE + 0x01B4)
#define HBG_REG_MODE_CHANGE_EN_B BIT(0)
#define HBG_REG_RECV_CONTROL_ADDR (HBG_REG_SGMII_BASE + 0x01E0)
#define HBG_REG_RECV_CONTROL_STRIP_PAD_EN_B BIT(3)
+#define HBG_REG_STATION_ADDR_LOW_2_ADDR (HBG_REG_SGMII_BASE + 0x0210)
+#define HBG_REG_STATION_ADDR_HIGH_2_ADDR (HBG_REG_SGMII_BASE + 0x0214)
/* PCU */
#define HBG_REG_CF_INTRPT_MSK_ADDR (HBG_REG_SGMII_BASE + 0x042C)
@@ -72,6 +78,8 @@
#define HBG_INT_MSK_RX_B BIT(0) /* just used in driver */
#define HBG_REG_CF_INTRPT_STAT_ADDR (HBG_REG_SGMII_BASE + 0x0434)
#define HBG_REG_CF_INTRPT_CLR_ADDR (HBG_REG_SGMII_BASE + 0x0438)
+#define HBG_REG_MAX_FRAME_LEN_ADDR (HBG_REG_SGMII_BASE + 0x0444)
+#define HBG_REG_MAX_FRAME_LEN_M GENMASK(15, 0)
#define HBG_REG_RX_BUF_SIZE_ADDR (HBG_REG_SGMII_BASE + 0x04E4)
#define HBG_REG_RX_BUF_SIZE_M GENMASK(15, 0)
#define HBG_REG_BUS_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x04E8)
@@ -86,6 +94,7 @@
#define HBG_REG_RX_PKT_MODE_ADDR (HBG_REG_SGMII_BASE + 0x04F4)
#define HBG_REG_RX_PKT_MODE_PARSE_MODE_M GENMASK(22, 21)
#define HBG_REG_CF_IND_TXINT_MSK_ADDR (HBG_REG_SGMII_BASE + 0x0694)
+#define HBG_REG_IND_INTR_MASK_B BIT(0)
#define HBG_REG_CF_IND_TXINT_STAT_ADDR (HBG_REG_SGMII_BASE + 0x0698)
#define HBG_REG_CF_IND_TXINT_CLR_ADDR (HBG_REG_SGMII_BASE + 0x069C)
#define HBG_REG_CF_IND_RXINT_MSK_ADDR (HBG_REG_SGMII_BASE + 0x06a0)
--
2.33.0On Tue, 27 Aug 2024 21:14:49 +0800 Jijie Shao wrote:
> +static int hbg_net_open(struct net_device *dev)
> +{
> + struct hbg_priv *priv = netdev_priv(dev);
> +
> + if (test_and_set_bit(HBG_NIC_STATE_OPEN, &priv->state))
> + return 0;
> +
> + netif_carrier_off(dev);
Why clear the carrier during open? You should probably clear it once on
the probe path and then on stop.
> + hbg_all_irq_enable(priv, true);
> + hbg_hw_mac_enable(priv, HBG_STATUS_ENABLE);
> + netif_start_queue(dev);
> + hbg_phy_start(priv);
> +
> + return 0;
> +}
on 2024/8/29 9:39, Jakub Kicinski wrote:
> On Tue, 27 Aug 2024 21:14:49 +0800 Jijie Shao wrote:
>> +static int hbg_net_open(struct net_device *dev)
>> +{
>> + struct hbg_priv *priv = netdev_priv(dev);
>> +
>> + if (test_and_set_bit(HBG_NIC_STATE_OPEN, &priv->state))
>> + return 0;
>> +
>> + netif_carrier_off(dev);
> Why clear the carrier during open? You should probably clear it once on
> the probe path and then on stop.
In net_open(), the GMAC is not ready to receive or transmit packets.
Therefore, netif_carrier_off() is called.
Packets can be received or transmitted only after the PHY is linked.
Therefore, netif_carrier_on() should be called in adjust_link.
In net_stop() we also call netif_carrier_off()
Thanks,
Jijie Shao
On Thu, 29 Aug 2024 10:40:07 +0800 Jijie Shao wrote:
> on 2024/8/29 9:39, Jakub Kicinski wrote:
> > On Tue, 27 Aug 2024 21:14:49 +0800 Jijie Shao wrote:
> >> +static int hbg_net_open(struct net_device *dev)
> >> +{
> >> + struct hbg_priv *priv = netdev_priv(dev);
> >> +
> >> + if (test_and_set_bit(HBG_NIC_STATE_OPEN, &priv->state))
> >> + return 0;
> >> +
> >> + netif_carrier_off(dev);
> > Why clear the carrier during open? You should probably clear it once on
> > the probe path and then on stop.
>
> In net_open(), the GMAC is not ready to receive or transmit packets.
> Therefore, netif_carrier_off() is called.
>
> Packets can be received or transmitted only after the PHY is linked.
> Therefore, netif_carrier_on() should be called in adjust_link.
But why are you calling _off() during .ndo_open() ?
Surely the link is also off before ndo_open is called?
> In net_stop() we also call netif_carrier_off()
Exactly, so it should already be off.
On Thu, Aug 29, 2024 at 07:43:39AM -0700, Jakub Kicinski wrote:
> On Thu, 29 Aug 2024 10:40:07 +0800 Jijie Shao wrote:
> > on 2024/8/29 9:39, Jakub Kicinski wrote:
> > > On Tue, 27 Aug 2024 21:14:49 +0800 Jijie Shao wrote:
> > >> +static int hbg_net_open(struct net_device *dev)
> > >> +{
> > >> + struct hbg_priv *priv = netdev_priv(dev);
> > >> +
> > >> + if (test_and_set_bit(HBG_NIC_STATE_OPEN, &priv->state))
> > >> + return 0;
> > >> +
> > >> + netif_carrier_off(dev);
> > > Why clear the carrier during open? You should probably clear it once on
> > > the probe path and then on stop.
> >
> > In net_open(), the GMAC is not ready to receive or transmit packets.
> > Therefore, netif_carrier_off() is called.
> >
> > Packets can be received or transmitted only after the PHY is linked.
> > Therefore, netif_carrier_on() should be called in adjust_link.
>
> But why are you calling _off() during .ndo_open() ?
> Surely the link is also off before ndo_open is called?
I wounder what driver they copied?
The general trend is .probe() calls netif_carrier_off(). After than,
phylib/phylink is in control of the carrier and the MAC driver does
not touch it. in fact, when using phylink, if you try to change the
carrier, you will get SHOUTED at from Russell.
Andrew
on 2024/8/29 22:59, Andrew Lunn wrote:
> On Thu, Aug 29, 2024 at 07:43:39AM -0700, Jakub Kicinski wrote:
>> On Thu, 29 Aug 2024 10:40:07 +0800 Jijie Shao wrote:
>>> on 2024/8/29 9:39, Jakub Kicinski wrote:
>>>> On Tue, 27 Aug 2024 21:14:49 +0800 Jijie Shao wrote:
>>>>> +static int hbg_net_open(struct net_device *dev)
>>>>> +{
>>>>> + struct hbg_priv *priv = netdev_priv(dev);
>>>>> +
>>>>> + if (test_and_set_bit(HBG_NIC_STATE_OPEN, &priv->state))
>>>>> + return 0;
>>>>> +
>>>>> + netif_carrier_off(dev);
>>>> Why clear the carrier during open? You should probably clear it once on
>>>> the probe path and then on stop.
>>> In net_open(), the GMAC is not ready to receive or transmit packets.
>>> Therefore, netif_carrier_off() is called.
>>>
>>> Packets can be received or transmitted only after the PHY is linked.
>>> Therefore, netif_carrier_on() should be called in adjust_link.
>> But why are you calling _off() during .ndo_open() ?
>> Surely the link is also off before ndo_open is called?
> I wounder what driver they copied?
>
> The general trend is .probe() calls netif_carrier_off(). After than,
> phylib/phylink is in control of the carrier and the MAC driver does
> not touch it. in fact, when using phylink, if you try to change the
> carrier, you will get SHOUTED at from Russell.
>
> Andrew
Read the PHY driver code:
netif_carrier_on() or netif_carrier_off()
has been called in phy_link_change() based on the link status.
Therefore, the driver does not need to process it.
static void phy_link_change(struct phy_device *phydev, bool up)
{
struct net_device *netdev = phydev->attached_dev;
if (up)
netif_carrier_on(netdev);
else
netif_carrier_off(netdev);
phydev->adjust_link(netdev);
if (phydev->mii_ts && phydev->mii_ts->link_state)
phydev->mii_ts->link_state(phydev->mii_ts, phydev);
}
Thank you. I'll delete it in v6.
Jijie Shao
© 2016 - 2025 Red Hat, Inc.