lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 11 Dec 2022 15:35:04 +0800 From: "mengyuanlou@...-swift.com" <mengyuanlou@...-swift.com> To: Andrew Lunn <andrew@...n.ch> Cc: netdev@...r.kernel.org, jiawenwu@...stnetic.com Subject: Re: [PATCH net-next v2] net: ngbe: Add ngbe mdio bus driver. > 2022年12月10日 18:47,Andrew Lunn <andrew@...n.ch> 写道: > >> int ngbe_reset_hw(struct ngbe_hw *hw) >> { >> struct wx_hw *wxhw = &hw->wxhw; >> - int status = 0; >> - u32 reset = 0; >> + u32 val = 0; >> + int ret = 0; >> >> /* Call adapter stop to disable tx/rx and clear interrupts */ >> - status = wx_stop_adapter(wxhw); >> - if (status != 0) >> - return status; >> - reset = WX_MIS_RST_LAN_RST(wxhw->bus.func); >> - wr32(wxhw, WX_MIS_RST, reset | rd32(wxhw, WX_MIS_RST)); >> + ret = wx_stop_adapter(wxhw); >> + if (ret != 0) >> + return ret; >> + >> + if (hw->mac_type != ngbe_mac_type_mdi) { >> + val = WX_MIS_RST_LAN_RST(wxhw->bus.func); >> + wr32(wxhw, WX_MIS_RST, val | rd32(wxhw, WX_MIS_RST)); >> + >> + ret = read_poll_timeout(rd32, val, >> + !(val & (BIT(9) << wxhw->bus.func)), 1000, >> + 100000, false, wxhw, 0x10028); >> + if (ret) >> + wx_dbg(wxhw, "Lan reset exceed s maximum times.\n"); > > This is a real error. Return it up the call stack so the user gets to > know. This should be netdev_err(), since it is an error. > >> +static void ngbe_up(struct ngbe_adapter *adapter) >> +{ >> + struct ngbe_hw *hw = &adapter->hw; >> + >> + pci_set_master(adapter->pdev); >> + if (hw->gpio_ctrl) >> + /* gpio0 is used to power on control*/ >> + wr32(&hw->wxhw, NGBE_GPIO_DR, 0); > > Control of what? Control for sfp modules power down/up. The chip has not i2c interface, so I do not use phylink. > >> +static int ngbe_phy_read_reg_mdi(struct mii_bus *bus, int phy_addr, int regnum) >> +{ >> + u32 command = 0, device_type = 0; >> + struct ngbe_hw *hw = bus->priv; >> + struct wx_hw *wxhw = &hw->wxhw; >> + u32 phy_data = 0; >> + u32 val = 0; >> + int ret = 0; >> + >> + if (regnum & MII_ADDR_C45) { >> + wr32(wxhw, NGBE_MDIO_CLAUSE_SELECT, 0x0); >> + /* setup and write the address cycle command */ >> + command = NGBE_MSCA_RA(mdiobus_c45_regad(regnum)) | >> + NGBE_MSCA_PA(phy_addr) | >> + NGBE_MSCA_DA(mdiobus_c45_devad(regnum)); >> + } else { >> + wr32(wxhw, NGBE_MDIO_CLAUSE_SELECT, 0xF); >> + /* setup and write the address cycle command */ >> + command = NGBE_MSCA_RA(regnum) | >> + NGBE_MSCA_PA(phy_addr) | >> + NGBE_MSCA_DA(device_type); >> + } >> + wr32(wxhw, NGBE_MSCA, command); >> + command = NGBE_MSCC_CMD(NGBE_MSCA_CMD_READ) | >> + NGBE_MSCC_BUSY | >> + NGBE_MDIO_CLK(6); >> + wr32(wxhw, NGBE_MSCC, command); >> + >> + /* wait to complete */ >> + ret = read_poll_timeout(rd32, val, !(val & NGBE_MSCC_BUSY), 1000, >> + 100000, false, wxhw, NGBE_MSCC); >> + >> + if (ret) >> + wx_dbg(wxhw, "PHY address command did not complete.\n"); > > netdev_err() and return the error code. > >> +static int ngbe_phy_write_reg_mdi(struct mii_bus *bus, int phy_addr, int regnum, u16 value) >> +{ >> + u32 command = 0, device_type = 0; >> + struct ngbe_hw *hw = bus->priv; >> + struct wx_hw *wxhw = &hw->wxhw; >> + int ret = 0; >> + u16 val = 0; >> + >> + if (regnum & MII_ADDR_C45) { >> + wr32(wxhw, NGBE_MDIO_CLAUSE_SELECT, 0x0); >> + /* setup and write the address cycle command */ >> + command = NGBE_MSCA_RA(mdiobus_c45_regad(regnum)) | >> + NGBE_MSCA_PA(phy_addr) | >> + NGBE_MSCA_DA(mdiobus_c45_devad(regnum)); >> + } else { >> + wr32(wxhw, NGBE_MDIO_CLAUSE_SELECT, 0xF); >> + /* setup and write the address cycle command */ >> + command = NGBE_MSCA_RA(regnum) | >> + NGBE_MSCA_PA(phy_addr) | >> + NGBE_MSCA_DA(device_type); >> + } >> + wr32(wxhw, NGBE_MSCA, command); >> + command = value | >> + NGBE_MSCC_CMD(NGBE_MSCA_CMD_WRITE) | >> + NGBE_MSCC_BUSY | >> + NGBE_MDIO_CLK(6); >> + wr32(wxhw, NGBE_MSCC, command); >> + >> + /* wait to complete */ >> + ret = read_poll_timeout(rd32, val, !(val & NGBE_MSCC_BUSY), 1000, >> + 100000, false, wxhw, NGBE_MSCC); >> + >> + if (ret) >> + wx_dbg(wxhw, "PHY address command did not complete.\n"); > > dev_error(). > >> + >> + return ret; >> +} >> + >> +static int ngbe_phy_read_reg(struct mii_bus *bus, int phy_addr, int regnum) >> +{ >> + struct ngbe_hw *hw = bus->priv; >> + u16 phy_data = 0; >> + >> + if (hw->mac_type == ngbe_mac_type_mdi) >> + phy_data = ngbe_phy_read_reg_internal(bus, phy_addr, regnum); >> + else if (hw->mac_type == ngbe_mac_type_rgmii) >> + phy_data = ngbe_phy_read_reg_mdi(bus, phy_addr, regnum); >> + >> + return phy_data; > > You return 0 if it is neither MDI or RGMII. It would be better to > either return -EINVAL, or 0xffff. A read on an MDIO bus for an address > without a device returns 0xffff due to the pull up resisters. That > will let Linux know there is no device there. > >> +static void ngbe_handle_link_change(struct net_device *dev) >> +{ >> + struct ngbe_adapter *adapter = netdev_priv(dev); >> + struct ngbe_hw *hw = &adapter->hw; >> + struct wx_hw *wxhw = &hw->wxhw; >> + u32 lan_speed = 2, reg; >> + bool changed = false; >> + >> + struct phy_device *phydev = hw->phydev; >> + >> + if (hw->link != phydev->link || >> + hw->speed != phydev->speed || >> + hw->duplex != phydev->duplex) { >> + changed = true; >> + hw->link = phydev->link; >> + hw->speed = phydev->speed; >> + hw->duplex = phydev->duplex; >> + } >> + >> + if (!changed) >> + return; >> + >> + switch (phydev->speed) { >> + case SPEED_1000: >> + lan_speed = 2; >> + break; >> + case SPEED_100: >> + lan_speed = 1; >> + break; >> + case SPEED_10: >> + lan_speed = 0; >> + break; >> + default: >> + break; >> + } >> + wr32m(wxhw, NGBE_CFG_LAN_SPEED, 0x3, lan_speed); >> + >> + if (phydev->link) { >> + if (phydev->speed & (SPEED_1000 | SPEED_100 | SPEED_10)) { > > You logically OR 10, 100 and 1000? These SPEED_FOO macros are not > bits. > > Andrew >
Powered by blists - more mailing lists