Ok, patch v2 sent, only with the interrupt bug fix, and without renaming all registers.
> Also this change should go upstream. If you don't want to handle it, I > can do as well. Yes, please make the upstream work. Regards 2016-12-18 12:54 GMT+01:00 Jonas Gorski <[email protected]>: > Hi, > > On 18 December 2016 at 02:19, Daniel Gonzalez Cabanelas > <[email protected]> wrote: >> The internal phy is using wrong registers for the config interrupt function, >> causing incorrect behavior when detecting the link activity. Fix it. >> >> We cannot use the bcm_phy_config_intr function from the bcm-phy-lib.c >> because it uses different registers from brcm63xx. We need to use >> our own function, which matches with the one used by the >> "Broadcom PHY driver" (brcm_fet_config_intr at broadcom.c). >> >> brcm63xx internal phy uses the same registers as the ones defined in >> brcmphy.h for fast ethernet, use them instead. >> >> Signed-off-by: Daniel Gonzalez Cabanelas <[email protected]> > > Good find. > >> diff --git >> a/target/linux/brcm63xx/patches-4.4/409-bcm63xx_net_phy-fix-registers.patch >> b/target/linux/brcm63xx/patches-4.4/409-bcm63xx_net_phy-fix-registers.patch >> new file mode 100644 >> index 0000000..c53d464 >> --- /dev/null >> +++ >> b/target/linux/brcm63xx/patches-4.4/409-bcm63xx_net_phy-fix-registers.patch >> @@ -0,0 +1,102 @@ >> +--- a/drivers/net/phy/bcm63xx.c >> ++++ b/drivers/net/phy/bcm63xx.c >> +@@ -6,44 +6,55 @@ >> + * as published by the Free Software Foundation; either version >> + * 2 of the License, or (at your option) any later version. >> + */ >> + #include "bcm-phy-lib.h" >> + #include <linux/module.h> >> + #include <linux/phy.h> >> +- >> +-#define MII_BCM63XX_IR 0x1a /* interrupt register */ >> +-#define MII_BCM63XX_IR_EN 0x4000 /* global interrupt enable */ >> +-#define MII_BCM63XX_IR_DUPLEX 0x0800 /* duplex changed */ >> +-#define MII_BCM63XX_IR_SPEED 0x0400 /* speed changed */ >> +-#define MII_BCM63XX_IR_LINK 0x0200 /* link changed */ >> +-#define MII_BCM63XX_IR_GMASK 0x0100 /* global interrupt mask */ >> ++#include <linux/brcmphy.h> >> + >> + MODULE_DESCRIPTION("Broadcom 63xx internal PHY driver"); >> + MODULE_AUTHOR("Maxime Bizon <[email protected]>"); >> + MODULE_LICENSE("GPL"); >> + >> + static int bcm63xx_config_init(struct phy_device *phydev) >> + { >> + int reg, err; >> + >> +- reg = phy_read(phydev, MII_BCM63XX_IR); >> ++ reg = phy_read(phydev, MII_BRCM_FET_INTREG); >> + if (reg < 0) >> + return reg; >> + >> + /* Mask interrupts globally. */ >> +- reg |= MII_BCM63XX_IR_GMASK; >> +- err = phy_write(phydev, MII_BCM63XX_IR, reg); >> ++ reg |= MII_BRCM_FET_IR_MASK; >> ++ err = phy_write(phydev, MII_BRCM_FET_INTREG, reg); >> + if (err < 0) >> + return err; >> + >> + /* Unmask events we are interested in */ >> +- reg = ~(MII_BCM63XX_IR_DUPLEX | >> +- MII_BCM63XX_IR_SPEED | >> +- MII_BCM63XX_IR_LINK) | >> +- MII_BCM63XX_IR_EN; >> +- return phy_write(phydev, MII_BCM63XX_IR, reg); >> ++ reg = ~(MII_BRCM_FET_IR_DUPLEX_EN | >> ++ MII_BRCM_FET_IR_SPEED_EN | >> ++ MII_BRCM_FET_IR_LINK_EN) | >> ++ MII_BRCM_FET_IR_ENABLE; >> ++ return phy_write(phydev, MII_BRCM_FET_INTREG, reg); >> ++} > > This whole change has nothing to do with the issue and is just > cosmetic, please drop it. It makes finding the actual changes hard to > follow. The relevant part is: > >> ++ >> ++static int brcm_fet_config_intr(struct phy_device *phydev) > > Please call it bcm63xx_config_intr to match the other functions in this > driver. > >> ++{ >> ++ int reg, err; >> ++ >> ++ reg = phy_read(phydev, MII_BRCM_FET_INTREG); >> ++ if (reg < 0) >> ++ return reg; >> ++ >> ++ if (phydev->interrupts == PHY_INTERRUPT_ENABLED) >> ++ reg &= ~MII_BRCM_FET_IR_MASK; >> ++ else >> ++ reg |= MII_BRCM_FET_IR_MASK; >> ++ >> ++ err = phy_write(phydev, MII_BRCM_FET_INTREG, reg); >> ++ return err; > > You can just do "return phy_write()" here, and then you can drop the > err variable. > > *looks at the breaking commit* > > Ah, I see what you did there, that's basically the old function. So it > can probably stay. > > > > > Regards > Jonas -- Daniel _______________________________________________ Lede-dev mailing list [email protected] http://lists.infradead.org/mailman/listinfo/lede-dev
