diff mbox series

[net-next,v27,10/13] rtase: Implement ethtool function

Message ID 20240812063539.575865-11-justinlai0215@realtek.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add Realtek automotive PCIe driver | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 7 this patch: 7
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 5 of 5 maintainers
netdev/build_clang success Errors and warnings before: 7 this patch: 7
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 7 this patch: 7
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 101 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-08-13--12-00 (tests: 706)

Commit Message

Justin Lai Aug. 12, 2024, 6:35 a.m. UTC
Implement the ethtool function to support users to obtain network card
information, including obtaining various device settings, Report whether
physical link is up, Report pause parameters, Set pause parameters,
Return extended statistics about the device.

Signed-off-by: Justin Lai <justinlai0215@realtek.com>
---
 .../net/ethernet/realtek/rtase/rtase_main.c   | 92 +++++++++++++++++++
 1 file changed, 92 insertions(+)

Comments

Jakub Kicinski Aug. 16, 2024, 1:46 a.m. UTC | #1
On Mon, 12 Aug 2024 14:35:36 +0800 Justin Lai wrote:
> +static void rtase_get_drvinfo(struct net_device *dev,
> +			      struct ethtool_drvinfo *info)
> +{
> +	const struct rtase_private *tp = netdev_priv(dev);
> +
> +	strscpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> +	strscpy(info->bus_info, pci_name(tp->pdev), sizeof(info->bus_info));
> +}

This shouldn't be necessary, can you delete this function from the
driver and check if the output of ethtool -i changes?
ethtool_get_drvinfo() should fill this in for you.

> +static void rtase_get_pauseparam(struct net_device *dev,
> +				 struct ethtool_pauseparam *pause)
> +{
> +	const struct rtase_private *tp = netdev_priv(dev);
> +	u16 value = rtase_r16(tp, RTASE_CPLUS_CMD);
> +
> +	pause->autoneg = AUTONEG_DISABLE;
> +
> +	if ((value & (RTASE_FORCE_TXFLOW_EN | RTASE_FORCE_RXFLOW_EN)) ==
> +	    (RTASE_FORCE_TXFLOW_EN | RTASE_FORCE_RXFLOW_EN)) {
> +		pause->rx_pause = 1;
> +		pause->tx_pause = 1;
> +	} else if (value & RTASE_FORCE_TXFLOW_EN) {
> +		pause->tx_pause = 1;
> +	} else if (value & RTASE_FORCE_RXFLOW_EN) {
> +		pause->rx_pause = 1;
> +	}

This 3 if statements can be replaced with just two lines:

	pause->rx_pause = !!(value & RTASE_FORCE_RXFLOW_EN);
	pause->tx_pause = !!(value & RTASE_FORCE_TXFLOW_EN);
Justin Lai Aug. 16, 2024, 6:44 a.m. UTC | #2
> On Mon, 12 Aug 2024 14:35:36 +0800 Justin Lai wrote:
> > +static void rtase_get_drvinfo(struct net_device *dev,
> > +                           struct ethtool_drvinfo *info) {
> > +     const struct rtase_private *tp = netdev_priv(dev);
> > +
> > +     strscpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> > +     strscpy(info->bus_info, pci_name(tp->pdev),
> > +sizeof(info->bus_info)); }
> 
> This shouldn't be necessary, can you delete this function from the driver and
> check if the output of ethtool -i changes?
> ethtool_get_drvinfo() should fill this in for you.

Thank you for your response. As you mentioned, ethtool_get_drvinfo()
will indeed populate the relevant information. I will remove
rtase_get_drvinfo().

> 
> > +static void rtase_get_pauseparam(struct net_device *dev,
> > +                              struct ethtool_pauseparam *pause) {
> > +     const struct rtase_private *tp = netdev_priv(dev);
> > +     u16 value = rtase_r16(tp, RTASE_CPLUS_CMD);
> > +
> > +     pause->autoneg = AUTONEG_DISABLE;
> > +
> > +     if ((value & (RTASE_FORCE_TXFLOW_EN |
> RTASE_FORCE_RXFLOW_EN)) ==
> > +         (RTASE_FORCE_TXFLOW_EN | RTASE_FORCE_RXFLOW_EN)) {
> > +             pause->rx_pause = 1;
> > +             pause->tx_pause = 1;
> > +     } else if (value & RTASE_FORCE_TXFLOW_EN) {
> > +             pause->tx_pause = 1;
> > +     } else if (value & RTASE_FORCE_RXFLOW_EN) {
> > +             pause->rx_pause = 1;
> > +     }
> 
> This 3 if statements can be replaced with just two lines:
> 
>         pause->rx_pause = !!(value & RTASE_FORCE_RXFLOW_EN);
>         pause->tx_pause = !!(value & RTASE_FORCE_TXFLOW_EN);

Ok, I will modify it.

Thanks
Justin
diff mbox series

Patch

diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
index bd2a05c96b1c..9a07252c3663 100644
--- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
+++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
@@ -1717,9 +1717,101 @@  static void rtase_get_mac_address(struct net_device *dev)
 	rtase_rar_set(tp, dev->dev_addr);
 }
 
+static void rtase_get_drvinfo(struct net_device *dev,
+			      struct ethtool_drvinfo *info)
+{
+	const struct rtase_private *tp = netdev_priv(dev);
+
+	strscpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
+	strscpy(info->bus_info, pci_name(tp->pdev), sizeof(info->bus_info));
+}
+
+static int rtase_get_settings(struct net_device *dev,
+			      struct ethtool_link_ksettings *cmd)
+{
+	u32 supported = SUPPORTED_MII | SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+
+	ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
+						supported);
+	cmd->base.speed = SPEED_5000;
+	cmd->base.duplex = DUPLEX_FULL;
+	cmd->base.port = PORT_MII;
+	cmd->base.autoneg = AUTONEG_DISABLE;
+
+	return 0;
+}
+
+static void rtase_get_pauseparam(struct net_device *dev,
+				 struct ethtool_pauseparam *pause)
+{
+	const struct rtase_private *tp = netdev_priv(dev);
+	u16 value = rtase_r16(tp, RTASE_CPLUS_CMD);
+
+	pause->autoneg = AUTONEG_DISABLE;
+
+	if ((value & (RTASE_FORCE_TXFLOW_EN | RTASE_FORCE_RXFLOW_EN)) ==
+	    (RTASE_FORCE_TXFLOW_EN | RTASE_FORCE_RXFLOW_EN)) {
+		pause->rx_pause = 1;
+		pause->tx_pause = 1;
+	} else if (value & RTASE_FORCE_TXFLOW_EN) {
+		pause->tx_pause = 1;
+	} else if (value & RTASE_FORCE_RXFLOW_EN) {
+		pause->rx_pause = 1;
+	}
+}
+
+static int rtase_set_pauseparam(struct net_device *dev,
+				struct ethtool_pauseparam *pause)
+{
+	const struct rtase_private *tp = netdev_priv(dev);
+	u16 value = rtase_r16(tp, RTASE_CPLUS_CMD);
+
+	if (pause->autoneg)
+		return -EOPNOTSUPP;
+
+	value &= ~(RTASE_FORCE_TXFLOW_EN | RTASE_FORCE_RXFLOW_EN);
+
+	if (pause->tx_pause)
+		value |= RTASE_FORCE_TXFLOW_EN;
+
+	if (pause->rx_pause)
+		value |= RTASE_FORCE_RXFLOW_EN;
+
+	rtase_w16(tp, RTASE_CPLUS_CMD, value);
+	return 0;
+}
+
+static void rtase_get_eth_mac_stats(struct net_device *dev,
+				    struct ethtool_eth_mac_stats *stats)
+{
+	struct rtase_private *tp = netdev_priv(dev);
+	const struct rtase_counters *counters;
+
+	counters = tp->tally_vaddr;
+
+	rtase_dump_tally_counter(tp);
+
+	stats->FramesTransmittedOK = le64_to_cpu(counters->tx_packets);
+	stats->FramesReceivedOK = le64_to_cpu(counters->rx_packets);
+	stats->FramesLostDueToIntMACXmitError =
+		le64_to_cpu(counters->tx_errors);
+	stats->BroadcastFramesReceivedOK = le64_to_cpu(counters->rx_broadcast);
+}
+
+static const struct ethtool_ops rtase_ethtool_ops = {
+	.get_drvinfo = rtase_get_drvinfo,
+	.get_link = ethtool_op_get_link,
+	.get_link_ksettings = rtase_get_settings,
+	.get_pauseparam = rtase_get_pauseparam,
+	.set_pauseparam = rtase_set_pauseparam,
+	.get_eth_mac_stats = rtase_get_eth_mac_stats,
+	.get_ts_info = ethtool_op_get_ts_info,
+};
+
 static void rtase_init_netdev_ops(struct net_device *dev)
 {
 	dev->netdev_ops = &rtase_netdev_ops;
+	dev->ethtool_ops = &rtase_ethtool_ops;
 }
 
 static void rtase_reset_interrupt(struct pci_dev *pdev,