[PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[Heiner Kallweit]

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(+)

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;
 }
-- 
2.15.0

Re: [PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[Andrew Lunn]

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

Re: [PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[Jerome Brunet]

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

Re: [PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[Andrew Lunn]

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

Re: [PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[Jerome Brunet]

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

Re: [PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[Florian Fainelli]

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;
>  }
> 

-- 
Florian

Re: [PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[Jerome Brunet]

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

Re: [PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[Jerome Brunet]

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>

Re: [PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[David Miller]

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.

Re: [PATCH] net: phy: realtek: fix RTL8211F interrupt mode

[Martin Blumenstingl]

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