From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiner Kallweit Subject: Re: [PATCH] r8169: remove TBI 1000BaseX support Date: Fri, 29 Jun 2018 08:08:41 +0200 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "netdev@vger.kernel.org" To: David Miller , Realtek linux nic maintainers Return-path: Received: from mail-wr0-f196.google.com ([209.85.128.196]:36618 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753260AbeF2GIq (ORCPT ); Fri, 29 Jun 2018 02:08:46 -0400 Received: by mail-wr0-f196.google.com with SMTP id f16-v6so7618381wrm.3 for ; Thu, 28 Jun 2018 23:08:45 -0700 (PDT) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 29.06.2018 08:07, Heiner Kallweit wrote: > The very first version of RTL8169 from 2002 (and only this one) has > support for a TBI 1000BaseX fiber interface. The TBI support in the > driver makes switching to phylib tricky, so best would be to get > rid of it. I found no report from anybody using a device with RTL8169 > and fiber interface, also the vendor driver doesn't support this mode > (any longer). > So remove TBI support and bail out with a message if a card with > activated TBI is detected. If there really should be any user of it > out there, we could add a stripped-down version of the driver > supporting chip version 01 and TBI only (and maybe move it to > staging). > > Signed-off-by: Heiner Kallweit Sorry, missed flagging it as net-next. > --- > drivers/net/ethernet/realtek/r8169.c | 156 ++++----------------------- > 1 file changed, 20 insertions(+), 136 deletions(-) > > diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c > index 21ffaf10..72a7778b 100644 > --- a/drivers/net/ethernet/realtek/r8169.c > +++ b/drivers/net/ethernet/realtek/r8169.c > @@ -384,12 +384,6 @@ enum rtl_registers { > FuncForceEvent = 0xfc, > }; > > -enum rtl8110_registers { > - TBICSR = 0x64, > - TBI_ANAR = 0x68, > - TBI_LPAR = 0x6a, > -}; > - > enum rtl8168_8101_registers { > CSIDR = 0x64, > CSIAR = 0x68, > @@ -556,14 +550,6 @@ enum rtl_register_content { > PMEStatus = (1 << 0), /* PME status can be reset by PCI RST# */ > ASPM_en = (1 << 0), /* ASPM enable */ > > - /* TBICSR p.28 */ > - TBIReset = 0x80000000, > - TBILoopback = 0x40000000, > - TBINwEnable = 0x20000000, > - TBINwRestart = 0x10000000, > - TBILinkOk = 0x02000000, > - TBINwComplete = 0x01000000, > - > /* CPlusCmd p.31 */ > EnableBist = (1 << 15), // 8168 8101 > Mac_dbgo_oe = (1 << 14), // 8168 8101 > @@ -761,14 +747,7 @@ struct rtl8169_private { > void (*disable)(struct rtl8169_private *); > } jumbo_ops; > > - int (*set_speed)(struct net_device *, u8 aneg, u16 sp, u8 dpx, u32 adv); > - int (*get_link_ksettings)(struct net_device *, > - struct ethtool_link_ksettings *); > - void (*phy_reset_enable)(struct rtl8169_private *tp); > void (*hw_start)(struct rtl8169_private *tp); > - unsigned int (*phy_reset_pending)(struct rtl8169_private *tp); > - unsigned int (*link_ok)(struct rtl8169_private *tp); > - int (*do_ioctl)(struct rtl8169_private *tp, struct mii_ioctl_data *data, int cmd); > bool (*tso_csum)(struct rtl8169_private *, struct sk_buff *, u32 *); > > struct { > @@ -1463,31 +1442,16 @@ static void rtl8169_irq_mask_and_ack(struct rtl8169_private *tp) > RTL_R8(tp, ChipCmd); > } > > -static unsigned int rtl8169_tbi_reset_pending(struct rtl8169_private *tp) > -{ > - return RTL_R32(tp, TBICSR) & TBIReset; > -} > - > static unsigned int rtl8169_xmii_reset_pending(struct rtl8169_private *tp) > { > return rtl_readphy(tp, MII_BMCR) & BMCR_RESET; > } > > -static unsigned int rtl8169_tbi_link_ok(struct rtl8169_private *tp) > -{ > - return RTL_R32(tp, TBICSR) & TBILinkOk; > -} > - > static unsigned int rtl8169_xmii_link_ok(struct rtl8169_private *tp) > { > return RTL_R8(tp, PHYstatus) & LinkStatus; > } > > -static void rtl8169_tbi_reset_enable(struct rtl8169_private *tp) > -{ > - RTL_W32(tp, TBICSR, RTL_R32(tp, TBICSR) | TBIReset); > -} > - > static void rtl8169_xmii_reset_enable(struct rtl8169_private *tp) > { > unsigned int val; > @@ -1557,7 +1521,7 @@ static void rtl8169_check_link_status(struct net_device *dev, > { > struct device *d = tp_to_dev(tp); > > - if (tp->link_ok(tp)) { > + if (rtl8169_xmii_link_ok(tp)) { > rtl_link_chg_patch(tp); > /* This is to cancel a scheduled suspend if there's one. */ > pm_request_resume(d); > @@ -1744,28 +1708,6 @@ static int rtl8169_get_regs_len(struct net_device *dev) > return R8169_REGS_SIZE; > } > > -static int rtl8169_set_speed_tbi(struct net_device *dev, > - u8 autoneg, u16 speed, u8 duplex, u32 ignored) > -{ > - struct rtl8169_private *tp = netdev_priv(dev); > - int ret = 0; > - u32 reg; > - > - reg = RTL_R32(tp, TBICSR); > - if ((autoneg == AUTONEG_DISABLE) && (speed == SPEED_1000) && > - (duplex == DUPLEX_FULL)) { > - RTL_W32(tp, TBICSR, reg & ~(TBINwEnable | TBINwRestart)); > - } else if (autoneg == AUTONEG_ENABLE) > - RTL_W32(tp, TBICSR, reg | TBINwEnable | TBINwRestart); > - else { > - netif_warn(tp, link, dev, > - "incorrect speed setting refused in TBI mode\n"); > - ret = -EOPNOTSUPP; > - } > - > - return ret; > -} > - > static int rtl8169_set_speed_xmii(struct net_device *dev, > u8 autoneg, u16 speed, u8 duplex, u32 adv) > { > @@ -1849,7 +1791,7 @@ static int rtl8169_set_speed(struct net_device *dev, > struct rtl8169_private *tp = netdev_priv(dev); > int ret; > > - ret = tp->set_speed(dev, autoneg, speed, duplex, advertising); > + ret = rtl8169_set_speed_xmii(dev, autoneg, speed, duplex, advertising); > if (ret < 0) > goto out; > > @@ -1925,53 +1867,14 @@ static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb) > __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), swab16(opts2 & 0xffff)); > } > > -static int rtl8169_get_link_ksettings_tbi(struct net_device *dev, > - struct ethtool_link_ksettings *cmd) > -{ > - struct rtl8169_private *tp = netdev_priv(dev); > - u32 status; > - u32 supported, advertising; > - > - supported = > - SUPPORTED_1000baseT_Full | SUPPORTED_Autoneg | SUPPORTED_FIBRE; > - cmd->base.port = PORT_FIBRE; > - > - status = RTL_R32(tp, TBICSR); > - advertising = (status & TBINwEnable) ? ADVERTISED_Autoneg : 0; > - cmd->base.autoneg = !!(status & TBINwEnable); > - > - cmd->base.speed = SPEED_1000; > - cmd->base.duplex = DUPLEX_FULL; /* Always set */ > - > - ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported, > - supported); > - ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.advertising, > - advertising); > - > - return 0; > -} > - > -static int rtl8169_get_link_ksettings_xmii(struct net_device *dev, > - struct ethtool_link_ksettings *cmd) > -{ > - struct rtl8169_private *tp = netdev_priv(dev); > - > - mii_ethtool_get_link_ksettings(&tp->mii, cmd); > - > - return 0; > -} > - > static int rtl8169_get_link_ksettings(struct net_device *dev, > struct ethtool_link_ksettings *cmd) > { > struct rtl8169_private *tp = netdev_priv(dev); > - int rc; > > - rtl_lock_work(tp); > - rc = tp->get_link_ksettings(dev, cmd); > - rtl_unlock_work(tp); > + mii_ethtool_get_link_ksettings(&tp->mii, cmd); > > - return rc; > + return 0; > } > > static int rtl8169_set_link_ksettings(struct net_device *dev, > @@ -4395,7 +4298,7 @@ static void rtl_phy_work(struct rtl8169_private *tp) > struct timer_list *timer = &tp->timer; > unsigned long timeout = RTL8169_PHY_TIMEOUT; > > - if (tp->phy_reset_pending(tp)) { > + if (rtl8169_xmii_reset_pending(tp)) { > /* > * A busy loop could burn quite a few cycles on nowadays CPU. > * Let's delay the execution of the timer for a few ticks. > @@ -4404,12 +4307,12 @@ static void rtl_phy_work(struct rtl8169_private *tp) > goto out_mod_timer; > } > > - if (tp->link_ok(tp)) > + if (rtl8169_xmii_link_ok(tp)) > return; > > netif_dbg(tp, link, tp->dev, "PHY reset until link up\n"); > > - tp->phy_reset_enable(tp); > + rtl8169_xmii_reset_enable(tp); > > out_mod_timer: > mod_timer(timer, jiffies + timeout); > @@ -4430,20 +4333,20 @@ static void rtl8169_phy_timer(struct timer_list *t) > > DECLARE_RTL_COND(rtl_phy_reset_cond) > { > - return tp->phy_reset_pending(tp); > + return rtl8169_xmii_reset_pending(tp); > } > > static void rtl8169_phy_reset(struct net_device *dev, > struct rtl8169_private *tp) > { > - tp->phy_reset_enable(tp); > + rtl8169_xmii_reset_enable(tp); > rtl_msleep_loop_wait_low(tp, &rtl_phy_reset_cond, 1, 100); > } > > static bool rtl_tbi_enabled(struct rtl8169_private *tp) > { > return (tp->mac_version == RTL_GIGA_MAC_VER_01) && > - (RTL_R8(tp, PHYstatus) & TBI_Enable); > + (RTL_R8(tp, PHYstatus) & TBI_Enable); > } > > static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp) > @@ -4478,9 +4381,6 @@ static void rtl8169_init_phy(struct net_device *dev, struct rtl8169_private *tp) > (tp->mii.supports_gmii ? > ADVERTISED_1000baseT_Half | > ADVERTISED_1000baseT_Full : 0)); > - > - if (rtl_tbi_enabled(tp)) > - netif_info(tp, link, dev, "TBI auto-negotiating\n"); > } > > static void rtl_rar_set(struct rtl8169_private *tp, u8 *addr) > @@ -4523,14 +4423,6 @@ static int rtl_set_mac_address(struct net_device *dev, void *p) > return 0; > } > > -static int rtl8169_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) > -{ > - struct rtl8169_private *tp = netdev_priv(dev); > - struct mii_ioctl_data *data = if_mii(ifr); > - > - return netif_running(dev) ? tp->do_ioctl(tp, data, cmd) : -ENODEV; > -} > - > static int rtl_xmii_ioctl(struct rtl8169_private *tp, > struct mii_ioctl_data *data, int cmd) > { > @@ -4550,9 +4442,12 @@ static int rtl_xmii_ioctl(struct rtl8169_private *tp, > return -EOPNOTSUPP; > } > > -static int rtl_tbi_ioctl(struct rtl8169_private *tp, struct mii_ioctl_data *data, int cmd) > +static int rtl8169_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) > { > - return -EOPNOTSUPP; > + struct rtl8169_private *tp = netdev_priv(dev); > + struct mii_ioctl_data *data = if_mii(ifr); > + > + return netif_running(dev) ? rtl_xmii_ioctl(tp, data, cmd) : -ENODEV; > } > > static void rtl_init_mdio_ops(struct rtl8169_private *tp) > @@ -7676,6 +7571,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > /* Identify chip attached to board */ > rtl8169_get_mac_version(tp, cfg->default_ver); > > + if (rtl_tbi_enabled(tp)) { > + dev_err(&pdev->dev, "TBI fiber mode not supported\n"); > + return -ENODEV; > + } > + > tp->cp_cmd = RTL_R16(tp, CPlusCmd); > > if ((sizeof(dma_addr_t) > 4) && > @@ -7724,22 +7624,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) > /* override BIOS settings, use userspace tools to enable WOL */ > __rtl8169_set_wol(tp, 0); > > - if (rtl_tbi_enabled(tp)) { > - tp->set_speed = rtl8169_set_speed_tbi; > - tp->get_link_ksettings = rtl8169_get_link_ksettings_tbi; > - tp->phy_reset_enable = rtl8169_tbi_reset_enable; > - tp->phy_reset_pending = rtl8169_tbi_reset_pending; > - tp->link_ok = rtl8169_tbi_link_ok; > - tp->do_ioctl = rtl_tbi_ioctl; > - } else { > - tp->set_speed = rtl8169_set_speed_xmii; > - tp->get_link_ksettings = rtl8169_get_link_ksettings_xmii; > - tp->phy_reset_enable = rtl8169_xmii_reset_enable; > - tp->phy_reset_pending = rtl8169_xmii_reset_pending; > - tp->link_ok = rtl8169_xmii_link_ok; > - tp->do_ioctl = rtl_xmii_ioctl; > - } > - > mutex_init(&tp->wk.mutex); > u64_stats_init(&tp->rx_stats.syncp); > u64_stats_init(&tp->tx_stats.syncp); >