Re: [PATCH 4/4] octeon_ep: add ethtool support for Octeon PCI Endpoint NIC.
From: Andrew Lunn <andrew@lunn.ch>
Date: 2022-02-11 21:41:15
Also in:
lkml, netdev
quoted hunk ↗ jump to hunk
+static void octep_get_drvinfo(struct net_device *netdev, + struct ethtool_drvinfo *info) +{ + struct octep_device *oct = netdev_priv(netdev); + + strscpy(info->driver, OCTEP_DRV_NAME, sizeof(info->driver)); + strscpy(info->version, OCTEP_DRV_VERSION_STR, sizeof(info->version));
A driver version string is meaningless. If you don't set it, the core will fill in the kernel version, which is actually usable information.
quoted hunk ↗ jump to hunk
+static int octep_get_link_ksettings(struct net_device *netdev, + struct ethtool_link_ksettings *cmd) +{ + struct octep_device *oct = netdev_priv(netdev); + struct octep_iface_link_info *link_info; + u32 advertised, supported; + + ethtool_link_ksettings_zero_link_mode(cmd, supported); + ethtool_link_ksettings_zero_link_mode(cmd, advertising); + + octep_get_link_info(oct); + + advertised = oct->link_info.advertised_modes; + supported = oct->link_info.supported_modes; + link_info = &oct->link_info; + + if (supported & BIT(OCTEP_LINK_MODE_10GBASE_T)) + ethtool_link_ksettings_add_link_mode(cmd, supported, 10000baseT_Full); + if (supported & BIT(OCTEP_LINK_MODE_10GBASE_R)) + ethtool_link_ksettings_add_link_mode(cmd, supported, 10000baseR_FEC);
....
quoted hunk ↗ jump to hunk
+ + if (advertised & BIT(OCTEP_LINK_MODE_10GBASE_T)) + ethtool_link_ksettings_add_link_mode(cmd, advertising, 10000baseT_Full); + if (advertised & BIT(OCTEP_LINK_MODE_10GBASE_R)) + ethtool_link_ksettings_add_link_mode(cmd, advertising, 10000baseR_FEC);
It looks like you are doing the same thing twice, just different variables. Pull this out into a helper. Do you know what the link partner is advertising? It is useful debug information if your firmware will tell you.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c index 700852fd4c3a..00c6ca047332 100644 --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c@@ -827,7 +827,7 @@ static int octep_set_mac(struct net_device *netdev, void *p) return err; memcpy(oct->mac_addr, addr->sa_data, ETH_ALEN); - memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len); + eth_hw_addr_set(netdev, addr->sa_data); return 0; }@@ -1067,7 +1068,7 @@ static int octep_probe(struct pci_dev *pdev, const struct pci_device_id *ent) netdev->mtu = OCTEP_DEFAULT_MTU; octep_get_mac_addr(octep_dev, octep_dev->mac_addr); - memcpy(netdev->dev_addr, octep_dev->mac_addr, netdev->addr_len); + eth_hw_addr_set(netdev, octep_dev->mac_addr);
These two changes don't belong here.
Andrew