lkml.org 
[lkml]   [2004]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH r8169] ethtool support and sane speed selection/detection
	Hi Andy,

Andy Lutomirski wrote:
> +static void rtl8169_set_speed(struct net_device *dev,
> + u8 autoneg, u16 speed, u8 duplex)
> +{
> + struct rtl8169_private *tp = dev->priv;
> + void *ioaddr = tp->mmio_addr;
> + unsigned long flags;
> + u8 status;
> +
> + int auto_nego, giga_ctrl;
> +
> + spin_lock_irqsave(&tp->lock, flags);
> +
> + status = RTL_R8(PHYstatus);
> + if ((status & TBI_Enable) && autoneg == AUTONEG_DISABLE) {
> + autoneg = AUTONEG_ENABLE;
> + printk(KERN_WARNING PFX
> + "%s: ignoring request to force speed in TBI mode\n",
> + dev->name);
> + }
> +
> + auto_nego = mdio_read(ioaddr, PHY_AUTO_NEGO_REG);
> + auto_nego &= ~(PHY_Cap_10_Half | PHY_Cap_10_Full |
> + PHY_Cap_100_Half | PHY_Cap_100_Full);
> + giga_ctrl = mdio_read(ioaddr, PHY_1000_CTRL_REG);
> + giga_ctrl &= ~(PHY_Cap_1000_Full | PHY_Cap_Null);
> +
> + if (autoneg == AUTONEG_ENABLE) {
> + auto_nego |= (PHY_Cap_10_Half | PHY_Cap_10_Full |
> + PHY_Cap_100_Half | PHY_Cap_100_Full);
> + giga_ctrl |= PHY_Cap_1000_Full;
> + } else {
> + if (speed == SPEED_10)
> + auto_nego |= PHY_Cap_10_Half | PHY_Cap_10_Full;
> + else if (speed == SPEED_100)
> + auto_nego |= PHY_Cap_100_Half | PHY_Cap_100_Full;
> +
> + if (speed == SPEED_1000)
> + giga_ctrl |= PHY_Cap_1000_Full;
> + else
> + giga_ctrl |= PHY_Cap_Null;
> +
> + if (duplex == DUPLEX_HALF)
> + auto_nego &= ~(PHY_Cap_10_Full | PHY_Cap_100_Full);
> + }
> +
> + tp->phy_auto_nego_reg = auto_nego;
> + tp->phy_1000_ctrl_reg = giga_ctrl;
> +
> + if(!(status & TBI_Enable)) {
> + mdio_write(ioaddr, PHY_AUTO_NEGO_REG, auto_nego);
> + mdio_write(ioaddr, PHY_1000_CTRL_REG, giga_ctrl);
> + }
> +
> + mdio_write(ioaddr, PHY_CTRL_REG,
> + PHY_Enable_Auto_Nego | PHY_Restart_Auto_Nego);
> +
> + if (tp->if_up && (giga_ctrl & PHY_Cap_1000_Full))
> + mod_timer(&tp->timer, jiffies + RTL8169_PHY_TIMEOUT);
> +
> + spin_unlock_irqrestore(&tp->lock, flags);
> +}
> +

I think you can use the mii's interface here..

Please look 8139cp's way of doind this. Using that interface is much
cleaner and doesn't duplicate code.

> +static int rtl8169_get_settings(struct net_device *dev,
> + struct ethtool_cmd *cmd)
> +{
> + struct rtl8169_private *tp = dev->priv;
> + void *ioaddr = tp->mmio_addr;
> + u8 status = RTL_R8(PHYstatus);

IMHO you should hold the device lock here (tp->lock).

> + cmd->supported = SUPPORTED_10baseT_Half |
> + SUPPORTED_10baseT_Full |
> + SUPPORTED_100baseT_Half |
> + SUPPORTED_100baseT_Full |
> + SUPPORTED_1000baseT_Full |
> + SUPPORTED_Autoneg |
> + SUPPORTED_TP;
> +
> + cmd->autoneg = 1;
> + cmd->advertising = ADVERTISED_TP | ADVERTISED_Autoneg;
> + if (tp->phy_auto_nego_reg & PHY_Cap_10_Half)
> + cmd->advertising |= ADVERTISED_10baseT_Half;
> + if (tp->phy_auto_nego_reg & PHY_Cap_10_Full)
> + cmd->advertising |= ADVERTISED_10baseT_Full;
> + if (tp->phy_auto_nego_reg & PHY_Cap_100_Half)
> + cmd->advertising |= ADVERTISED_100baseT_Half;
> + if (tp->phy_auto_nego_reg & PHY_Cap_100_Full)
> + cmd->advertising |= ADVERTISED_100baseT_Full;
> + if (tp->phy_1000_ctrl_reg & PHY_Cap_1000_Full)
> + cmd->advertising |= ADVERTISED_1000baseT_Full;
> +
> + if (status & _1000bpsF) cmd->speed = SPEED_1000;
> + else if (status & _100bps) cmd->speed = SPEED_100;
> + else if (status & _10bps) cmd->speed = SPEED_10;
> +
> + if (status & _1000bpsF || status & FullDup)
> + cmd->duplex = DUPLEX_FULL;
> + else
> + cmd->duplex = DUPLEX_HALF;
> +
> + return 0;
> +}

You should use mii's interface on this. There's no need to duplicate
code. Please use mii_ethtool_gset here.

Also rtl8169_set_settings should use mii_ethtool_sset. If the card gets
a LinkChg interrupt you should treat with mii_check_media at
rtl8169_set_settings.

Cheers,

Felipe
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:02    [W:0.039 / U:0.180 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site