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: Sat, 10 Dec 2022 11:47:29 +0100 From: Andrew Lunn <andrew@...n.ch> To: Mengyuan Lou <mengyuanlou@...-swift.com> Cc: netdev@...r.kernel.org, jiawenwu@...stnetic.com Subject: Re: [PATCH net-next v2] net: ngbe: Add ngbe mdio bus driver. > 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? > +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