On 5/28/19 8:29 PM, Heiner Kallweit wrote: > On 28.05.2019 20:16, Marek Vasut wrote: >> Add support for handling the TJA11xx PHY IRQ signal. >> >> Signed-off-by: Marek Vasut <[email protected]> >> Cc: Andrew Lunn <[email protected]> >> Cc: Florian Fainelli <[email protected]> >> Cc: Guenter Roeck <[email protected]> >> Cc: Heiner Kallweit <[email protected]> >> Cc: Jean Delvare <[email protected]> >> Cc: [email protected] >> --- >> drivers/net/phy/nxp-tja11xx.c | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c >> index b705d0bd798b..0be9fe9a9604 100644 >> --- a/drivers/net/phy/nxp-tja11xx.c >> +++ b/drivers/net/phy/nxp-tja11xx.c >> @@ -40,6 +40,8 @@ >> #define MII_INTSRC_TEMP_ERR BIT(1) >> #define MII_INTSRC_UV_ERR BIT(3) >> >> +#define MII_INTEN 22 >> + >> #define MII_COMMSTAT 23 >> #define MII_COMMSTAT_LINK_UP BIT(15) >> >> @@ -239,6 +241,30 @@ static int tja11xx_read_status(struct phy_device >> *phydev) >> return 0; >> } >> >> +static int tja11xx_config_intr(struct phy_device *phydev) >> +{ >> + int ret; >> + >> + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) >> + ret = phy_write(phydev, MII_INTEN, 0xcfef); > > As Florian commented already, such magic numbers are not nice. > Better add a constant for each bit representing an interrupt > source. Please note that phylib is interested in the link > change event only. Therefore typically only one bit is set.
With this PHY, it seems all of those bits have impact on the link. >> + else >> + ret = phy_write(phydev, MII_INTEN, 0); >> + >> + if (ret < 0) >> + return ret; >> + >> + ret = phy_read(phydev, MII_INTSRC); >> + > > This IRQ ACK can be removed. It's done by phylib, see > phy_enable_interrupts(). OK -- Best regards, Marek Vasut
