[PATCH net] smsc95xx: Add implementation for set_pauseparam for enabling to pause RX path.

[PATCH net] smsc95xx: Add implementation for set_pauseparam for enabling to pause RX path.

[Liel Harel]

Enable userspace applications to pause RX path by IOCTL.
The function write to MAC control and status register for pausing RX path.

Signed-off-by: Liel Harel <liel.harel@gmail.com>
---
 drivers/net/usb/smsc95xx.c | 79 ++++++++++++++++++++++++++------------
 1 file changed, 55 insertions(+), 24 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 8e82184be..98afdf635 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -137,7 +137,8 @@ static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
 }
 
 /* Loop until the read is completed with timeout
- * called with phy_mutex held */
+ * called with phy_mutex held
+ */
 static int __must_check smsc95xx_phy_wait_not_busy(struct usbnet *dev)
 {
 	unsigned long start_time = jiffies;
@@ -470,7 +471,8 @@ static int __must_check smsc95xx_write_reg_async(struct usbnet *dev, u16 index,
 
 /* returns hash bit number for given MAC address
  * example:
- * 01 00 5E 00 00 01 -> returns bit number 31 */
+ * 01 00 5E 00 00 01 -> returns bit number 31
+ */
 static unsigned int smsc95xx_hash(char addr[ETH_ALEN])
 {
 	return (ether_crc(ETH_ALEN, addr) >> 26) & 0x3f;
@@ -772,6 +774,45 @@ static int smsc95xx_ethtool_get_sset_count(struct net_device *ndev, int sset)
 	}
 }
 
+/* Starts the Receive path */
+static int smsc95xx_start_rx_path(struct usbnet *dev)
+{
+	struct smsc95xx_priv *pdata = dev->driver_priv;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
+	pdata->mac_cr |= MAC_CR_RXEN_;
+	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
+
+	return smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
+}
+
+/* Stops the Receive path */
+static int smsc95xx_stop_rx_path(struct usbnet *dev)
+{
+	struct smsc95xx_priv *pdata = dev->driver_priv;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
+	pdata->mac_cr &= ~MAC_CR_RXEN_;
+	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
+
+	return smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
+}
+
+static int smsc95xx_ethtool_set_pauseparam(struct net_device *netdev,
+									struct ethtool_pauseparam *pause)
+{
+	struct usbnet *dev = netdev_priv(netdev);
+
+	if (!pause->tx_pause || !pause->autoneg)
+		return -EINVAL;
+
+	if (pause->rx_pause)
+		return smsc95xx_start_rx_path(dev);
+	return smsc95xx_stop_rx_path(dev);
+}
+
 static const struct ethtool_ops smsc95xx_ethtool_ops = {
 	.get_link	= smsc95xx_get_link,
 	.nway_reset	= phy_ethtool_nway_reset,
@@ -791,6 +832,7 @@ static const struct ethtool_ops smsc95xx_ethtool_ops = {
 	.self_test	= net_selftest,
 	.get_strings	= smsc95xx_ethtool_get_strings,
 	.get_sset_count	= smsc95xx_ethtool_get_sset_count,
+	.set_pauseparam = smsc95xx_ethtool_set_pauseparam,
 };
 
 static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
@@ -863,26 +905,13 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
 	return smsc95xx_write_reg(dev, TX_CFG, TX_CFG_ON_);
 }
 
-/* Starts the Receive path */
-static int smsc95xx_start_rx_path(struct usbnet *dev)
-{
-	struct smsc95xx_priv *pdata = dev->driver_priv;
-	unsigned long flags;
-
-	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
-	pdata->mac_cr |= MAC_CR_RXEN_;
-	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
-
-	return smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
-}
-
 static int smsc95xx_reset(struct usbnet *dev)
 {
 	struct smsc95xx_priv *pdata = dev->driver_priv;
 	u32 read_buf, burst_cap;
 	int ret = 0, timeout;
 
-	netif_dbg(dev, ifup, dev->net, "entering smsc95xx_reset\n");
+	netif_dbg(dev, ifup, dev->net, "entering %s\n", __func__);
 
 	ret = smsc95xx_write_reg(dev, HW_CFG, HW_CFG_LRST_);
 	if (ret < 0)
@@ -1065,7 +1094,7 @@ static int smsc95xx_reset(struct usbnet *dev)
 		return ret;
 	}
 
-	netif_dbg(dev, ifup, dev->net, "smsc95xx_reset, return 0\n");
+	netif_dbg(dev, ifup, dev->net, "%s, return 0\n", __func__);
 	return 0;
 }
 
@@ -1076,7 +1105,7 @@ static const struct net_device_ops smsc95xx_netdev_ops = {
 	.ndo_tx_timeout		= usbnet_tx_timeout,
 	.ndo_change_mtu		= usbnet_change_mtu,
 	.ndo_get_stats64	= dev_get_tstats64,
-	.ndo_set_mac_address 	= eth_mac_addr,
+	.ndo_set_mac_address = eth_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_eth_ioctl		= smsc95xx_ioctl,
 	.ndo_set_rx_mode	= smsc95xx_set_multicast,
@@ -1471,7 +1500,8 @@ static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
 		/* link is down so enter EDPD mode, but only if device can
 		 * reliably resume from it.  This check should be redundant
 		 * as current FEATURE_REMOTE_WAKEUP parts also support
-		 * FEATURE_PHY_NLP_CROSSOVER but it's included for clarity */
+		 * FEATURE_PHY_NLP_CROSSOVER but it's included for clarity
+		 */
 		if (!(pdata->features & FEATURE_PHY_NLP_CROSSOVER)) {
 			netdev_warn(dev->net, "EDPD not supported\n");
 			return -EBUSY;
@@ -1922,11 +1952,11 @@ static u32 smsc95xx_calc_csum_preamble(struct sk_buff *skb)
  */
 static bool smsc95xx_can_tx_checksum(struct sk_buff *skb)
 {
-       unsigned int len = skb->len - skb_checksum_start_offset(skb);
+	unsigned int len = skb->len - skb_checksum_start_offset(skb);
 
-       if (skb->len <= 45)
-	       return false;
-       return skb->csum_offset < (len - (4 + 1));
+	if (skb->len <= 45)
+		return false;
+	return skb->csum_offset < (len - (4 + 1));
 }
 
 static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
@@ -1955,7 +1985,8 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 	if (csum) {
 		if (!smsc95xx_can_tx_checksum(skb)) {
 			/* workaround - hardware tx checksum does not work
-			 * properly with extremely small packets */
+			 * properly with extremely small packets
+			 */
 			long csstart = skb_checksum_start_offset(skb);
 			__wsum calc = csum_partial(skb->data + csstart,
 				skb->len - csstart, 0);
-- 
2.25.1

Re: [PATCH net] smsc95xx: Add implementation for set_pauseparam for enabling to pause RX path.

[Andrew Lunn]

On Fri, Oct 04, 2024 at 02:20:00PM +0300, Liel Harel wrote:
> Enable userspace applications to pause RX path by IOCTL.

The legacy API probably does still work, but netlink is used now a
days for pause.

>  
>  /* Loop until the read is completed with timeout
> - * called with phy_mutex held */
> + * called with phy_mutex held
> + */

Please don't mix checkpatch style changes with functional changes in
one patch. Please break this up into a patchset.

>  static int __must_check smsc95xx_phy_wait_not_busy(struct usbnet *dev)
>  {
>  	unsigned long start_time = jiffies;
> @@ -470,7 +471,8 @@ static int __must_check smsc95xx_write_reg_async(struct usbnet *dev, u16 index,
>  
>  /* returns hash bit number for given MAC address
>   * example:
> - * 01 00 5E 00 00 01 -> returns bit number 31 */
> + * 01 00 5E 00 00 01 -> returns bit number 31
> + */
>  static unsigned int smsc95xx_hash(char addr[ETH_ALEN])
>  {
>  	return (ether_crc(ETH_ALEN, addr) >> 26) & 0x3f;
> @@ -772,6 +774,45 @@ static int smsc95xx_ethtool_get_sset_count(struct net_device *ndev, int sset)
>  	}
>  }
>  
> +/* Starts the Receive path */
> +static int smsc95xx_start_rx_path(struct usbnet *dev)
> +{
> +	struct smsc95xx_priv *pdata = dev->driver_priv;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
> +	pdata->mac_cr |= MAC_CR_RXEN_;
> +	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
> +
> +	return smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
> +}
> +
> +/* Stops the Receive path */
> +static int smsc95xx_stop_rx_path(struct usbnet *dev)
> +{
> +	struct smsc95xx_priv *pdata = dev->driver_priv;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
> +	pdata->mac_cr &= ~MAC_CR_RXEN_;
> +	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
> +
> +	return smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
> +}
> +
> +static int smsc95xx_ethtool_set_pauseparam(struct net_device *netdev,
> +									struct ethtool_pauseparam *pause)

indentation is wrong here.

> +{
> +	struct usbnet *dev = netdev_priv(netdev);
> +
> +	if (!pause->tx_pause || !pause->autoneg)
> +		return -EINVAL;
> +
> +	if (pause->rx_pause)
> +		return smsc95xx_start_rx_path(dev);
> +	return smsc95xx_stop_rx_path(dev);

This does not make much sense to me. What does pause mean to you?

> +}
> +
>  static const struct ethtool_ops smsc95xx_ethtool_ops = {
>  	.get_link	= smsc95xx_get_link,
>  	.nway_reset	= phy_ethtool_nway_reset,
> @@ -791,6 +832,7 @@ static const struct ethtool_ops smsc95xx_ethtool_ops = {
>  	.self_test	= net_selftest,
>  	.get_strings	= smsc95xx_ethtool_get_strings,
>  	.get_sset_count	= smsc95xx_ethtool_get_sset_count,
> +	.set_pauseparam = smsc95xx_ethtool_set_pauseparam,
>  };
>  
>  static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
> @@ -863,26 +905,13 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
>  	return smsc95xx_write_reg(dev, TX_CFG, TX_CFG_ON_);
>  }
>  
> -/* Starts the Receive path */
> -static int smsc95xx_start_rx_path(struct usbnet *dev)
> -{
> -	struct smsc95xx_priv *pdata = dev->driver_priv;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&pdata->mac_cr_lock, flags);
> -	pdata->mac_cr |= MAC_CR_RXEN_;
> -	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
> -
> -	return smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
> -}

If you need to move a function earlier in the code, please do that as
a separate patch, making it clear in the commit message that all it is
doing is moving code.

You want lots of small patches which are obviously correct.

> @@ -1076,7 +1105,7 @@ static const struct net_device_ops smsc95xx_netdev_ops = {
>  	.ndo_tx_timeout		= usbnet_tx_timeout,
>  	.ndo_change_mtu		= usbnet_change_mtu,
>  	.ndo_get_stats64	= dev_get_tstats64,
> -	.ndo_set_mac_address 	= eth_mac_addr,
> +	.ndo_set_mac_address = eth_mac_addr,

This hunk looks wrong.

	Andrew