| Message ID | 20170105204807.25990-1-grygorii.strashko@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/05/2017 12:48 PM, Grygorii Strashko wrote: > For proper IRQ generation by DP83867 phy the INT/PWDN pin has to be > programmed as an interrupt output instead of a Powerdown input in > Configuration Register 3 (CFG3), Address 0x001E, bit 7 INT_OE = 1. The > current driver doesn't do this and as result IRQs will not be generated by > DP83867 phy even if they are properly configured in DT. > > Hence, fix IRQ generation by properly configuring CFG3.INT_OE bit and > ensure that Link Status Change (LINK_STATUS_CHNG_INT) and Auto-Negotiation > Complete (AUTONEG_COMP_INT) interrupt are enabled. After this the DP83867 > driver will work properly in interrupt enabled mode. > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> > --- > drivers/net/phy/dp83867.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c > index 1b63924..e84ae08 100644 > --- a/drivers/net/phy/dp83867.c > +++ b/drivers/net/phy/dp83867.c > @@ -29,6 +29,7 @@ > #define MII_DP83867_MICR 0x12 > #define MII_DP83867_ISR 0x13 > #define DP83867_CTRL 0x1f > +#define DP83867_CFG3 0x1e > > /* Extended Registers */ > #define DP83867_RGMIICTL 0x0032 > @@ -98,6 +99,8 @@ static int dp83867_config_intr(struct phy_device *phydev) > micr_status |= > (MII_DP83867_MICR_AN_ERR_INT_EN | > MII_DP83867_MICR_SPEED_CHNG_INT_EN | > + MII_DP83867_MICR_AUTONEG_COMP_INT_EN | > + MII_DP83867_MICR_LINK_STS_CHNG_INT_EN | > MII_DP83867_MICR_DUP_MODE_CHNG_INT_EN | > MII_DP83867_MICR_SLEEP_MODE_CHNG_INT_EN); > > @@ -214,6 +217,13 @@ static int dp83867_config_init(struct phy_device *phydev) > } > } > > + /* Enable Interrupt output INT_OE in CFG3 register */ > + if (phy_interrupt_is_valid(phydev)) { > + val = phy_read(phydev, DP83867_CFG3); > + val |= BIT(7); > + phy_write(phydev, DP83867_CFG3, val); > + } Don't you need to clear that bit in the case phy_interrupt_is_valid() returns false? Other than that: Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
On 01/05/2017 04:10 PM, Florian Fainelli wrote: > On 01/05/2017 12:48 PM, Grygorii Strashko wrote: >> For proper IRQ generation by DP83867 phy the INT/PWDN pin has to be >> programmed as an interrupt output instead of a Powerdown input in >> Configuration Register 3 (CFG3), Address 0x001E, bit 7 INT_OE = 1. The >> current driver doesn't do this and as result IRQs will not be generated by >> DP83867 phy even if they are properly configured in DT. >> >> Hence, fix IRQ generation by properly configuring CFG3.INT_OE bit and >> ensure that Link Status Change (LINK_STATUS_CHNG_INT) and Auto-Negotiation >> Complete (AUTONEG_COMP_INT) interrupt are enabled. After this the DP83867 >> driver will work properly in interrupt enabled mode. >> >> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> >> --- >> drivers/net/phy/dp83867.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c >> index 1b63924..e84ae08 100644 >> --- a/drivers/net/phy/dp83867.c >> +++ b/drivers/net/phy/dp83867.c >> @@ -29,6 +29,7 @@ >> #define MII_DP83867_MICR 0x12 >> #define MII_DP83867_ISR 0x13 >> #define DP83867_CTRL 0x1f >> +#define DP83867_CFG3 0x1e >> >> /* Extended Registers */ >> #define DP83867_RGMIICTL 0x0032 >> @@ -98,6 +99,8 @@ static int dp83867_config_intr(struct phy_device *phydev) >> micr_status |= >> (MII_DP83867_MICR_AN_ERR_INT_EN | >> MII_DP83867_MICR_SPEED_CHNG_INT_EN | >> + MII_DP83867_MICR_AUTONEG_COMP_INT_EN | >> + MII_DP83867_MICR_LINK_STS_CHNG_INT_EN | >> MII_DP83867_MICR_DUP_MODE_CHNG_INT_EN | >> MII_DP83867_MICR_SLEEP_MODE_CHNG_INT_EN); >> >> @@ -214,6 +217,13 @@ static int dp83867_config_init(struct phy_device *phydev) >> } >> } >> >> + /* Enable Interrupt output INT_OE in CFG3 register */ >> + if (phy_interrupt_is_valid(phydev)) { >> + val = phy_read(phydev, DP83867_CFG3); >> + val |= BIT(7); >> + phy_write(phydev, DP83867_CFG3, val); >> + } > > Don't you need to clear that bit in the case phy_interrupt_is_valid() > returns false? Not sure I need to touch it in this case - default value is 0 and Linux will not configure any IRQ. > > Other than that: > > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> >
From: Grygorii Strashko <grygorii.strashko@ti.com> Date: Thu, 5 Jan 2017 14:48:07 -0600 > For proper IRQ generation by DP83867 phy the INT/PWDN pin has to be > programmed as an interrupt output instead of a Powerdown input in > Configuration Register 3 (CFG3), Address 0x001E, bit 7 INT_OE = 1. The > current driver doesn't do this and as result IRQs will not be generated by > DP83867 phy even if they are properly configured in DT. > > Hence, fix IRQ generation by properly configuring CFG3.INT_OE bit and > ensure that Link Status Change (LINK_STATUS_CHNG_INT) and Auto-Negotiation > Complete (AUTONEG_COMP_INT) interrupt are enabled. After this the DP83867 > driver will work properly in interrupt enabled mode. > > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> Applied.
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c index 1b63924..e84ae08 100644 --- a/drivers/net/phy/dp83867.c +++ b/drivers/net/phy/dp83867.c @@ -29,6 +29,7 @@ #define MII_DP83867_MICR 0x12 #define MII_DP83867_ISR 0x13 #define DP83867_CTRL 0x1f +#define DP83867_CFG3 0x1e /* Extended Registers */ #define DP83867_RGMIICTL 0x0032 @@ -98,6 +99,8 @@ static int dp83867_config_intr(struct phy_device *phydev) micr_status |= (MII_DP83867_MICR_AN_ERR_INT_EN | MII_DP83867_MICR_SPEED_CHNG_INT_EN | + MII_DP83867_MICR_AUTONEG_COMP_INT_EN | + MII_DP83867_MICR_LINK_STS_CHNG_INT_EN | MII_DP83867_MICR_DUP_MODE_CHNG_INT_EN | MII_DP83867_MICR_SLEEP_MODE_CHNG_INT_EN); @@ -214,6 +217,13 @@ static int dp83867_config_init(struct phy_device *phydev) } } + /* Enable Interrupt output INT_OE in CFG3 register */ + if (phy_interrupt_is_valid(phydev)) { + val = phy_read(phydev, DP83867_CFG3); + val |= BIT(7); + phy_write(phydev, DP83867_CFG3, val); + } + return 0; }
For proper IRQ generation by DP83867 phy the INT/PWDN pin has to be programmed as an interrupt output instead of a Powerdown input in Configuration Register 3 (CFG3), Address 0x001E, bit 7 INT_OE = 1. The current driver doesn't do this and as result IRQs will not be generated by DP83867 phy even if they are properly configured in DT. Hence, fix IRQ generation by properly configuring CFG3.INT_OE bit and ensure that Link Status Change (LINK_STATUS_CHNG_INT) and Auto-Negotiation Complete (AUTONEG_COMP_INT) interrupt are enabled. After this the DP83867 driver will work properly in interrupt enabled mode. Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> --- drivers/net/phy/dp83867.c | 10 ++++++++++ 1 file changed, 10 insertions(+)