| Message ID | 66991733-9d1a-dbd2-9857-bba1ffca1cf8@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: phy: realtek: fix RTL8211F interrupt mode | expand |
On Sun, Nov 12, 2017 at 04:16:04PM +0100, Heiner Kallweit wrote: > After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY > interrupt on some platforms" ethernet stopped working on my Odroid-C2 > which has a RTL8211F phy. Hi Jerome Please could you test this. I Just want to be sure we don't introduce a regression by breaking the boards you tested on. Thanks Andrew
Hi Heiner, On 11/12/2017 07:16 AM, Heiner Kallweit wrote: > After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY > interrupt on some platforms" ethernet stopped working on my Odroid-C2 > which has a RTL8211F phy. > > It turned out that no interrupts were triggered. Further analysis > showed the register INER can't be altered on page 0. > Because register INSR needs to be accessed via page 0xa43 I assumed > that register INER needs to be accessed via some page too. > Some brute force check resulted in page 0xa42 being the right one. > > With this patch the phy is working properly in interrupt mode. What would be the appropriate Fixes: tag for that patch? > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/phy/realtek.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c > index d4670ecdb..eda0a6e86 100644 > --- a/drivers/net/phy/realtek.c > +++ b/drivers/net/phy/realtek.c > @@ -115,11 +115,13 @@ static int rtl8211f_config_intr(struct phy_device *phydev) > { > int err; > > + phy_write(phydev, RTL821x_PAGE_SELECT, 0xa42); > if (phydev->interrupts == PHY_INTERRUPT_ENABLED) > err = phy_write(phydev, RTL821x_INER, > RTL8211F_INER_LINK_STATUS); > else > err = phy_write(phydev, RTL821x_INER, 0); > + phy_write(phydev, RTL821x_PAGE_SELECT, 0); > > return err; > } >
On Sun, 2017-11-12 at 19:25 +0100, Andrew Lunn wrote: > On Sun, Nov 12, 2017 at 04:16:04PM +0100, Heiner Kallweit wrote: > > After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY > > interrupt on some platforms" ethernet stopped working on my Odroid-C2 > > which has a RTL8211F phy. > > Hi Jerome > > Please could you test this. I Just want to be sure we don't introduce > a regression by breaking the boards you tested on. Sure I'll try it tomorrow. When I tested this, I was more focused on the SoC side of it (the interrupt controller itself) and whether the interrupt worked or not. The board (p200) I tested on used a micrel PHY and worked well ... I did not really expect that a problem would come from the phy side. This was clearly a mistake. > > Thanks > Andrew
On Sun, 2017-11-12 at 10:29 -0800, Florian Fainelli wrote: > Hi Heiner, > > On 11/12/2017 07:16 AM, Heiner Kallweit wrote: > > After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY > > interrupt on some platforms" ethernet stopped working on my Odroid-C2 > > which has a RTL8211F phy. > > > > It turned out that no interrupts were triggered. Further analysis > > showed the register INER can't be altered on page 0. > > Because register INSR needs to be accessed via page 0xa43 I assumed > > that register INER needs to be accessed via some page too. > > Some brute force check resulted in page 0xa42 being the right one. > > > > With this patch the phy is working properly in interrupt mode. > > What would be the appropriate Fixes: tag for that patch? Fixes: 3447cf2e9a11 ("net/phy: Add support for Realtek RTL8211F") Seems appropriate
On Sun, Nov 12, 2017 at 07:36:48PM +0100, Jerome Brunet wrote: > On Sun, 2017-11-12 at 19:25 +0100, Andrew Lunn wrote: > > On Sun, Nov 12, 2017 at 04:16:04PM +0100, Heiner Kallweit wrote: > > > After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY > > > interrupt on some platforms" ethernet stopped working on my Odroid-C2 > > > which has a RTL8211F phy. > > > > Hi Jerome > > > > Please could you test this. I Just want to be sure we don't introduce > > a regression by breaking the boards you tested on. > > Sure I'll try it tomorrow. > > When I tested this, I was more focused on the SoC side of it (the interrupt > controller itself) and whether the interrupt worked or not. The board (p200) I > tested on used a micrel PHY and worked well ... Hi Jerome Ah, O.K. Do you have a board with a RTL8211? If all your boards use a different PHY, there is no chance of a regression for you. Thanks Andrew
On Sun, 2017-11-12 at 21:06 +0100, Andrew Lunn wrote: > On Sun, Nov 12, 2017 at 07:36:48PM +0100, Jerome Brunet wrote: > > On Sun, 2017-11-12 at 19:25 +0100, Andrew Lunn wrote: > > > On Sun, Nov 12, 2017 at 04:16:04PM +0100, Heiner Kallweit wrote: > > > > After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY > > > > interrupt on some platforms" ethernet stopped working on my Odroid-C2 > > > > which has a RTL8211F phy. > > > > > > Hi Jerome > > > > > > Please could you test this. I Just want to be sure we don't introduce > > > a regression by breaking the boards you tested on. > > > > Sure I'll try it tomorrow. > > > > When I tested this, I was more focused on the SoC side of it (the interrupt > > controller itself) and whether the interrupt worked or not. The board (p200) > > I > > tested on used a micrel PHY and worked well ... > > Hi Jerome > > Ah, O.K. Do you have a board with a RTL8211? I do have some. I'll confirm Heiner's report and fix tomorrow. > If all your boards use a > different PHY, there is no chance of a regression for you. I'm not expecting any, quite the contrary actually > > Thanks > Andrew
On Sun, 2017-11-12 at 16:16 +0100, Heiner Kallweit wrote: > After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY > interrupt on some platforms" ethernet stopped working on my Odroid-C2 > which has a RTL8211F phy. > > It turned out that no interrupts were triggered. Further analysis > showed the register INER can't be altered on page 0. > Because register INSR needs to be accessed via page 0xa43 I assumed > that register INER needs to be accessed via some page too. > Some brute force check resulted in page 0xa42 being the right one. > > With this patch the phy is working properly in interrupt mode. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Tested-by: Jerome Brunet <jbrunet@baylibre.com>
From: Heiner Kallweit <hkallweit1@gmail.com> Date: Sun, 12 Nov 2017 16:16:04 +0100 > After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY > interrupt on some platforms" ethernet stopped working on my Odroid-C2 > which has a RTL8211F phy. > > It turned out that no interrupts were triggered. Further analysis > showed the register INER can't be altered on page 0. > Because register INSR needs to be accessed via page 0xa43 I assumed > that register INER needs to be accessed via some page too. > Some brute force check resulted in page 0xa42 being the right one. > > With this patch the phy is working properly in interrupt mode. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied.
Hi Heiner, On Sun, Nov 12, 2017 at 4:16 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote: > After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY > interrupt on some platforms" ethernet stopped working on my Odroid-C2 > which has a RTL8211F phy. > > It turned out that no interrupts were triggered. Further analysis > showed the register INER can't be altered on page 0. > Because register INSR needs to be accessed via page 0xa43 I assumed > that register INER needs to be accessed via some page too. > Some brute force check resulted in page 0xa42 being the right one. unfortunately there's no public datasheet for the RTL8211F. I contacted Realtek to see if we could get a datasheet. unfortunately an NDA is required for that however, they were kind enough to share some information from the RTL8211F datasheet with me RTL821x_INER is called INER (Interrupt Enable Register) in the datasheet. it is located at page 0xa42, address (the register after selecting the page) 0x12 (RTL821x_INER is also 0x12) in other words: your findings were correct! (I know that my mail is too late to make it into the commit message - but with this mail it's "documented" online now) RTL8211E also uses RTL821x_INER (0x12) register, but according to the information I got from Realtek it is located in page 0x0 (so no special page has to be selected before changing that register on RTL8211E) Regards Martin
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c index d4670ecdb..eda0a6e86 100644 --- a/drivers/net/phy/realtek.c +++ b/drivers/net/phy/realtek.c @@ -115,11 +115,13 @@ static int rtl8211f_config_intr(struct phy_device *phydev) { int err; + phy_write(phydev, RTL821x_PAGE_SELECT, 0xa42); if (phydev->interrupts == PHY_INTERRUPT_ENABLED) err = phy_write(phydev, RTL821x_INER, RTL8211F_INER_LINK_STATUS); else err = phy_write(phydev, RTL821x_INER, 0); + phy_write(phydev, RTL821x_PAGE_SELECT, 0); return err; }
After commit b94d22d94ad22 "ARM64: dts: meson-gx: add external PHY interrupt on some platforms" ethernet stopped working on my Odroid-C2 which has a RTL8211F phy. It turned out that no interrupts were triggered. Further analysis showed the register INER can't be altered on page 0. Because register INSR needs to be accessed via page 0xa43 I assumed that register INER needs to be accessed via some page too. Some brute force check resulted in page 0xa42 being the right one. With this patch the phy is working properly in interrupt mode. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/realtek.c | 2 ++ 1 file changed, 2 insertions(+)