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 linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <0188dd19-7fcb-4bfe-945d-6cb5b57ae80a@lunn.ch> Date: Sat, 12 Aug 2023 00:16:22 +0200 From: Andrew Lunn <andrew@...n.ch> To: Giulio Benetti <giulio.benetti@...ettiengineering.com> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, Heiner Kallweit <hkallweit1@...il.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Florian Fainelli <florian.fainelli@...adcom.com>, Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Jim Reinhart <jimr@...vox.com>, James Autry <jautry@...vox.com>, Matthew Maron <matthewm@...vox.com> Subject: Re: [PATCH] net: phy: broadcom: add support for BCM5221 phy On Fri, Aug 11, 2023 at 11:53:22PM +0200, Giulio Benetti wrote: > This patch adds the BCM5221 PHY support by reusing > brcm_fet_config_intr() and brcm_fet_handle_interrupt() and > implementing config_init()/suspend()/resume(). > > Sponsored by: Tekvox Inc. That is a new tag. Maybe you should update Documentation/process/submitting-patches.rst ? > +static int bcm5221_config_init(struct phy_device *phydev) > +{ > + int reg, err, err2, brcmtest; > + > + /* Reset the PHY to bring it to a known state. */ > + err = phy_write(phydev, MII_BMCR, BMCR_RESET); > + if (err < 0) > + return err; > + > + /* The datasheet indicates the PHY needs up to 1us to complete a reset, > + * build some slack here. > + */ > + usleep_range(1000, 2000); > + > + /* The PHY requires 65 MDC clock cycles to complete a write operation > + * and turnaround the line properly. > + * > + * We ignore -EIO here as the MDIO controller (e.g.: mdio-bcm-unimac) > + * may flag the lack of turn-around as a read failure. This is > + * particularly true with this combination since the MDIO controller > + * only used 64 MDC cycles. This is not a critical failure in this > + * specific case and it has no functional impact otherwise, so we let > + * that one go through. If there is a genuine bus error, the next read > + * of MII_BRCM_FET_INTREG will error out. > + */ > + err = phy_read(phydev, MII_BMCR); > + if (err < 0 && err != -EIO) > + return err; It is pretty normal to check the value of MII_BMCR and ensure that BMCR_RESET has cleared. See phy_poll_reset(). It might not be needed, if you trust the datasheet, but 802.3 C22 says it should clear. > + /* Enable auto MDIX */ > + err = phy_clear_bits(phydev, BCM5221_AEGSR, BCM5221_AEGSR_MDIX_DIS); > + if (err < 0) > + return err; It is better to set it based on phydev->mdix_ctrl. > @@ -1288,6 +1431,7 @@ static struct mdio_device_id __maybe_unused broadcom_tbl[] = { > { PHY_ID_BCM53125, 0xfffffff0 }, > { PHY_ID_BCM53128, 0xfffffff0 }, > { PHY_ID_BCM89610, 0xfffffff0 }, > + { PHY_ID_BCM5221, 0xfffffff0 }, This table has some sort of sorting. I would put this new entry before PHY_ID_BCM5241. > #define PHY_ID_BCM50610 0x0143bd60 > #define PHY_ID_BCM50610M 0x0143bd70 > #define PHY_ID_BCM5241 0x0143bc30 > +#define PHY_ID_BCM5221 0x004061e0 The value looks odd. Is the OUI correct? Is that a broadcom OUI? Andrew --- pw-bot: cr
Powered by blists - more mailing lists