| Message ID | 6E913AD9617D9BC9+20230724092544.73531-2-mengyuanlou@net-swift.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Wangxun ngbe nics nsci support | expand |
On Mon, 24 Jul 2023 17:24:58 +0800 Mengyuan Lou wrote:
> + netdev->ncsi_enabled = wx->ncsi_hw_supported;
I don't think that enabled and supported are the same thing.
If server has multiple NICs or a NIC with multiple ports and
BMC only uses one, or even none, we shouldn't keep the PHY up.
By that logic 99% of server NICs should report NCSI as enabled.
> 2023年7月26日 07:22，Jakub Kicinski <kuba@kernel.org> 写道： > > On Mon, 24 Jul 2023 17:24:58 +0800 Mengyuan Lou wrote: >> + netdev->ncsi_enabled = wx->ncsi_hw_supported; > > I don't think that enabled and supported are the same thing. > If server has multiple NICs or a NIC with multiple ports and > BMC only uses one, or even none, we shouldn't keep the PHY up. > By that logic 99% of server NICs should report NCSI as enabled. > > For a NIC with multiple ports, BMC switch connection for port0 to port1 online, Drivers can not know port1 should keep up, if do not set ncsi_enabled before.
On Wed, 26 Jul 2023 09:59:15 +0800 mengyuanlou@net-swift.com wrote: > > 2023年7月26日 07:22，Jakub Kicinski <kuba@kernel.org> 写道： > > On Mon, 24 Jul 2023 17:24:58 +0800 Mengyuan Lou wrote: > >> + netdev->ncsi_enabled = wx->ncsi_hw_supported; > > > > I don't think that enabled and supported are the same thing. > > If server has multiple NICs or a NIC with multiple ports and > > BMC only uses one, or even none, we shouldn't keep the PHY up. > > By that logic 99% of server NICs should report NCSI as enabled. > > For a NIC with multiple ports, BMC switch connection for port0 to port1 online, > Drivers can not know port1 should keep up, if do not set ncsi_enabled before. I'm not crystal clear on what you're saying. But BMC sends a enable command to the NIC to enable a channel (or some such). This is all handled by FW. The FW can tell the host that the NCSI is now active on port1 and not port0.
> 2023年7月26日 10:44，Jakub Kicinski <kuba@kernel.org> 写道： > > On Wed, 26 Jul 2023 09:59:15 +0800 mengyuanlou@net-swift.com wrote: >>> 2023年7月26日 07:22，Jakub Kicinski <kuba@kernel.org> 写道： >>> On Mon, 24 Jul 2023 17:24:58 +0800 Mengyuan Lou wrote: >>>> + netdev->ncsi_enabled = wx->ncsi_hw_supported; >>> >>> I don't think that enabled and supported are the same thing. >>> If server has multiple NICs or a NIC with multiple ports and >>> BMC only uses one, or even none, we shouldn't keep the PHY up. >>> By that logic 99% of server NICs should report NCSI as enabled. >> >> For a NIC with multiple ports, BMC switch connection for port0 to port1 online, >> Drivers can not know port1 should keep up, if do not set ncsi_enabled before. > > I'm not crystal clear on what you're saying. But BMC sends a enable > command to the NIC to enable a channel (or some such). This is all > handled by FW. The FW can tell the host that the NCSI is now active > on port1 and not port0. > > Ok, I think I understand. Thanks. Another question. Then, after drivers know that portx is using for BMC, it is necessary to let phy to know this port should not be suspended? I mean this patch 2/2 is useful.
On Wed, 26 Jul 2023 11:12:41 +0800 mengyuanlou@net-swift.com wrote: > Another question. > Then, after drivers know that portx is using for BMC, it is necessary to > let phy to know this port should not be suspended? > I mean this patch 2/2 is useful. Right, I think being more selective about which port sets netdev->ncsi_enabled is independent from patch 2. Some form of patch 2 is still needed, but how exactly it should look is up to the PHYLIB maintainers.
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h index 1de88a33a698..1b932e66044e 100644 --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h @@ -851,7 +851,7 @@ struct wx { struct phy_device *phydev; bool wol_hw_supported; - bool ncsi_enabled; + bool ncsi_hw_supported; bool gpio_ctrl; raw_spinlock_t gpio_lock; diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c index 2b431db6085a..e42e4dd26700 100644 --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c @@ -63,8 +63,8 @@ static void ngbe_init_type_code(struct wx *wx) em_mac_type_mdi; wx->wol_hw_supported = (wol_mask == NGBE_WOL_SUP) ? 1 : 0; - wx->ncsi_enabled = (ncsi_mask == NGBE_NCSI_MASK || - type_mask == NGBE_SUBID_OCP_CARD) ? 1 : 0; + wx->ncsi_hw_supported = (ncsi_mask == NGBE_NCSI_MASK || + type_mask == NGBE_SUBID_OCP_CARD) ? 1 : 0; switch (type_mask) { case NGBE_SUBID_LY_YT8521S_SFP: @@ -639,6 +639,7 @@ static int ngbe_probe(struct pci_dev *pdev, netdev->wol_enabled = !!(wx->wol); wr32(wx, NGBE_PSR_WKUP_CTL, wx->wol); device_set_wakeup_enable(&pdev->dev, wx->wol); + netdev->ncsi_enabled = wx->ncsi_hw_supported; /* Save off EEPROM version number and Option Rom version which * together make a unique identify for the eeprom diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index b828c7a75be2..dfa14e4c8e95 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -2024,6 +2024,8 @@ enum netdev_ml_priv_type { * * @wol_enabled: Wake-on-LAN is enabled * + * @ncsi_enabled: NCSI is enabled. + * * @threaded: napi threaded mode is enabled * * @net_notifier_list: List of per-net netdev notifier block @@ -2393,6 +2395,7 @@ struct net_device { struct lock_class_key *qdisc_tx_busylock; bool proto_down; unsigned wol_enabled:1; + unsigned ncsi_enabled:1; unsigned threaded:1; struct list_head net_notifier_list;
Add ncsi_enabled flag to struct netdev to indicate wangxun nics which support NCSI. Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com> --- drivers/net/ethernet/wangxun/libwx/wx_type.h | 2 +- drivers/net/ethernet/wangxun/ngbe/ngbe_main.c | 5 +++-- include/linux/netdevice.h | 3 +++ 3 files changed, 7 insertions(+), 3 deletions(-)