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
