[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY.

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY.

[Marc Kleine-Budde]

Adding alkml and Shawn on Cc.

Marc

On 02/25/2015 08:23 PM, Andrey Panov wrote:
> RTL8211E Ethernet PHY found in Embedsky E9 board is need
> an additional initialization.
> Taken from vendor's binary-only module.
> 
> Signed-off-by: Andrey Panov <rockford@yandex.ru>
> ---
>  arch/arm/mach-imx/mach-imx6q.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index 4ad6e47..41708f5 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -166,6 +166,18 @@ static int ar8035_phy_fixup(struct phy_device *dev)
>  
>  #define PHY_ID_AR8035 0x004dd072
>  
> +static int rtl8211e_phy_fixup(struct phy_device *dev)
> +{
> +	phy_write(dev, 0x00, 0x3140);
> +	mdelay(10);
> +	phy_write(dev, 0x00, 0x3340);
> +	mdelay(10);
> +
> +	return 0;
> +}
> +
> +#define PHY_ID_RTL8211E 0x001cc915
> +
>  static void __init imx6q_enet_phy_init(void)
>  {
>  	if (IS_BUILTIN(CONFIG_PHYLIB)) {
> @@ -177,6 +189,8 @@ static void __init imx6q_enet_phy_init(void)
>  				ar8031_phy_fixup);
>  		phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,
>  				ar8035_phy_fixup);
> +		phy_register_fixup_for_uid(PHY_ID_RTL8211E, 0xffffffff,
> +				rtl8211e_phy_fixup);
>  	}
>  }
>  
> 


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150225/f34208c3/attachment.sig>

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY.

[Shawn Guo]

On Wed, Feb 25, 2015 at 08:34:24PM +0100, Marc Kleine-Budde wrote:
> Adding alkml and Shawn on Cc.

@Andrey, can you please resend the patch to LAKML and copy me?

Shawn

> 
> Marc
> 
> On 02/25/2015 08:23 PM, Andrey Panov wrote:
> > RTL8211E Ethernet PHY found in Embedsky E9 board is need
> > an additional initialization.
> > Taken from vendor's binary-only module.
> > 
> > Signed-off-by: Andrey Panov <rockford@yandex.ru>
> > ---
> >  arch/arm/mach-imx/mach-imx6q.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> > index 4ad6e47..41708f5 100644
> > --- a/arch/arm/mach-imx/mach-imx6q.c
> > +++ b/arch/arm/mach-imx/mach-imx6q.c
> > @@ -166,6 +166,18 @@ static int ar8035_phy_fixup(struct phy_device *dev)
> >  
> >  #define PHY_ID_AR8035 0x004dd072
> >  
> > +static int rtl8211e_phy_fixup(struct phy_device *dev)
> > +{
> > +	phy_write(dev, 0x00, 0x3140);
> > +	mdelay(10);
> > +	phy_write(dev, 0x00, 0x3340);
> > +	mdelay(10);
> > +
> > +	return 0;
> > +}
> > +
> > +#define PHY_ID_RTL8211E 0x001cc915
> > +
> >  static void __init imx6q_enet_phy_init(void)
> >  {
> >  	if (IS_BUILTIN(CONFIG_PHYLIB)) {
> > @@ -177,6 +189,8 @@ static void __init imx6q_enet_phy_init(void)
> >  				ar8031_phy_fixup);
> >  		phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,
> >  				ar8035_phy_fixup);
> > +		phy_register_fixup_for_uid(PHY_ID_RTL8211E, 0xffffffff,
> > +				rtl8211e_phy_fixup);
> >  	}
> >  }
> >  
> > 
> 
> 
> -- 
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY

[Andrey Panov]

RTL8211E Ethernet PHY found in Embedsky E9 board is need
an additional initialization.
Taken from vendor's binary-only module.

Signed-off-by: Andrey Panov <rockford@yandex.ru>
---
 arch/arm/mach-imx/mach-imx6q.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 4ad6e47..41708f5 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -166,6 +166,18 @@ static int ar8035_phy_fixup(struct phy_device *dev)
 
 #define PHY_ID_AR8035 0x004dd072
 
+static int rtl8211e_phy_fixup(struct phy_device *dev)
+{
+	phy_write(dev, 0x00, 0x3140);
+	mdelay(10);
+	phy_write(dev, 0x00, 0x3340);
+	mdelay(10);
+
+	return 0;
+}
+
+#define PHY_ID_RTL8211E 0x001cc915
+
 static void __init imx6q_enet_phy_init(void)
 {
 	if (IS_BUILTIN(CONFIG_PHYLIB)) {
@@ -177,6 +189,8 @@ static void __init imx6q_enet_phy_init(void)
 				ar8031_phy_fixup);
 		phy_register_fixup_for_uid(PHY_ID_AR8035, 0xffffffef,
 				ar8035_phy_fixup);
+		phy_register_fixup_for_uid(PHY_ID_RTL8211E, 0xffffffff,
+				rtl8211e_phy_fixup);
 	}
 }
 
-- 
2.1.4

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY

[Arnd Bergmann]

On Tuesday 03 March 2015 20:42:59 Andrey Panov wrote:
> +static int rtl8211e_phy_fixup(struct phy_device *dev)
> +{
> +       phy_write(dev, 0x00, 0x3140);
> +       mdelay(10);
> +       phy_write(dev, 0x00, 0x3340);
> +       mdelay(10);
> +
> +       return 0;
> +}
> 

mdelay(10) is rather evil, can you use msleep(10) here instead?

> +               phy_register_fixup_for_uid(PHY_ID_RTL8211E, 0xffffffff,
> +                               rtl8211e_phy_fixup);

How do you ensure that this fixup is only applied on the boards that need
it, rather than all machines that happen to have this phy?

	Arnd

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY

[Панов Андрей]

Hello!

04.03.2015, 02:29, "Arnd Bergmann" <arnd@arndb.de>:
> On Tuesday 03 March 2015 20:42:59 Andrey Panov wrote:
>> ?+static int rtl8211e_phy_fixup(struct phy_device *dev)
>> ?+{
>> ?+ ??????phy_write(dev, 0x00, 0x3140);
>> ?+ ??????mdelay(10);
>> ?+ ??????phy_write(dev, 0x00, 0x3340);
>> ?+ ??????mdelay(10);
>> ?+
>> ?+ ??????return 0;
>> ?+}
>
> mdelay(10) is rather evil, can you use msleep(10) here instead?

Yes

>> ?+ ??????????????phy_register_fixup_for_uid(PHY_ID_RTL8211E, 0xffffffff,
>> ?+ ??????????????????????????????rtl8211e_phy_fixup);
>
> How do you ensure that this fixup is only applied on the boards that need
> it, rather than all machines that happen to have this phy?

I've thought that if there no other fixups in code here checking a particular 
board where they run, this isn't necessary.
This SoC has only one ethernet MAC and if it is connected to this PHY, 
it should be initalized this way independently of board, like other code here does, I think.

This code just does forcibly restart autonegotiation.
I can wrap it in if(!of_machine_is_compatible(...)){}, if it is need.

And I see, mask is wrong - I should use 0x001fffff.

I'll resend a patch, thanks.

--
 ??????

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY

[Arnd Bergmann]

On Wednesday 04 March 2015 22:08:42 ????? ?????? wrote:
> 
> >>  +               phy_register_fixup_for_uid(PHY_ID_RTL8211E, 0xffffffff,
> >>  +                               rtl8211e_phy_fixup);
> >
> > How do you ensure that this fixup is only applied on the boards that need
> > it, rather than all machines that happen to have this phy?
> 
> I've thought that if there no other fixups in code here checking a particular 
> board where they run, this isn't necessary.
> This SoC has only one ethernet MAC and if it is connected to this PHY, 
> it should be initalized this way independently of board, like other code here does, I think.
> 
> This code just does forcibly restart autonegotiation.
> I can wrap it in if(!of_machine_is_compatible(...)){}, if it is need.

Your explanation makes sense to me, though I wonder what the property of this
SoC is that requires the PHY fixup. Is this something we could or should be doing
in a more general way using the PHY API, by having the device driver call a
phy API function to restart autoneg independent of the PHY?

> And I see, mask is wrong - I should use 0x001fffff.
> 
> I'll resend a patch, thanks.

Ok.

	Arnd

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY

[Панов Андрей]

04.03.2015, 22:44, "Arnd Bergmann" <arnd@arndb.de>:
> On Wednesday 04 March 2015 22:08:42 ????? ?????? wrote:
>>>> ??+ ??????????????phy_register_fixup_for_uid(PHY_ID_RTL8211E, 0xffffffff,
>>>> ??+ ??????????????????????????????rtl8211e_phy_fixup);
>>> ?How do you ensure that this fixup is only applied on the boards that need
>>> ?it, rather than all machines that happen to have this phy?
>> ?I've thought that if there no other fixups in code here checking a particular
>> ?board where they run, this isn't necessary.
>> ?This SoC has only one ethernet MAC and if it is connected to this PHY,
>> ?it should be initalized this way independently of board, like other code here does, I think.
>>
>> ?This code just does forcibly restart autonegotiation.
>> ?I can wrap it in if(!of_machine_is_compatible(...)){}, if it is need.
>
> Your explanation makes sense to me, though I wonder what the property of this
> SoC is that requires the PHY fixup. Is this something we could or should be doing
> in a more general way using the PHY API, by having the device driver call a
> phy API function to restart autoneg independent of the PHY?

Actually, I don't know. I've discovered this when I wanted to boot a more recent kernel
on this board, and network wasn't working. So I've had to disassembly a vendor's binary only module
to discover how they handle this situation.

--
 ??????

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY

[Fabio Estevam]

On Wed, Mar 4, 2015 at 5:05 PM, ????? ?????? <rockford@yandex.ru> wrote:

> Actually, I don't know. I've discovered this when I wanted to boot a more recent kernel
> on this board, and network wasn't working. So I've had to disassembly a vendor's binary only module
> to discover how they handle this situation.

Do you use the generic PHY driver or do you have CONFIG_REALTEK_PHY enabled?

In case you use the realtek PHY driver and still see the issue, can't
you fix this in drivers/net/phy/realtek.c instead?

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY

[Панов Андрей]

04.03.2015, 23:18, "Fabio Estevam" <festevam@gmail.com>:
> On Wed, Mar 4, 2015 at 5:05 PM, ????? ?????? <rockford@yandex.ru> wrote:
>> ?Actually, I don't know. I've discovered this when I wanted to boot a more recent kernel
>> ?on this board, and network wasn't working. So I've had to disassembly a vendor's binary only module
>> ?to discover how they handle this situation.
>
> Do you use the generic PHY driver or do you have CONFIG_REALTEK_PHY enabled?

CONFIG_REALTEK_PHY=y
CONFIG_FIXED_PHY=y

> In case you use the realtek PHY driver and still see the issue, can't
> you fix this in drivers/net/phy/realtek.c instead?

I think there it will require a more work with board detection than here and will
populate a driver with unnecessary for most users code.


--
 ??????

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY

[Arnd Bergmann]

On Wednesday 04 March 2015 23:32:38 ????? ?????? wrote:
> 04.03.2015, 23:18, "Fabio Estevam" <festevam@gmail.com>:
> > On Wed, Mar 4, 2015 at 5:05 PM, ????? ?????? <rockford@yandex.ru> wrote:
> >>  Actually, I don't know. I've discovered this when I wanted to boot a more recent kernel
> >>  on this board, and network wasn't working. So I've had to disassembly a vendor's binary only module
> >>  to discover how they handle this situation.
> >
> > Do you use the generic PHY driver or do you have CONFIG_REALTEK_PHY enabled?
> 
> CONFIG_REALTEK_PHY=y
> CONFIG_FIXED_PHY=y
> 
> > In case you use the realtek PHY driver and still see the issue, can't
> > you fix this in drivers/net/phy/realtek.c instead?
> 
> I think there it will require a more work with board detection than here and will
> populate a driver with unnecessary for most users code.

But you said earlier that it's board independent and a property of the SoC
instead. Can't the ethernet driver call phy_start_aneg() for the same effect,
like a lot of other ethernet drivers do?

	Arnd

[PATCH] ARM: i.MX6Q: Add fixup for RTL8211E Gigabit Ethernet PHY

[Панов Андрей]

05.03.2015, 00:32, "Arnd Bergmann" <arnd@arndb.de>:
> On Wednesday 04 March 2015 23:32:38 ????? ?????? wrote:
>> ?04.03.2015, 23:18, "Fabio Estevam" <festevam@gmail.com>:
>>> ?On Wed, Mar 4, 2015 at 5:05 PM, ????? ?????? <rockford@yandex.ru> wrote:
>>>> ??Actually, I don't know. I've discovered this when I wanted to boot a more recent kernel
>>>> ??on this board, and network wasn't working. So I've had to disassembly a vendor's binary only module
>>>> ??to discover how they handle this situation.
>>> ?Do you use the generic PHY driver or do you have CONFIG_REALTEK_PHY enabled?
>> ?CONFIG_REALTEK_PHY=y
>> ?CONFIG_FIXED_PHY=y
>>> ?In case you use the realtek PHY driver and still see the issue, can't
>>> ?you fix this in drivers/net/phy/realtek.c instead?
>> ?I think there it will require a more work with board detection than here and will
>> ?populate a driver with unnecessary for most users code.
>
> But you said earlier that it's board independent and a property of the SoC
> instead. Can't the ethernet driver call phy_start_aneg() for the same effect,
> like a lot of other ethernet drivers do?

Of course. I'll try to make it this way.

--
 ??????