[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Tor Krill]

Add phy_info for Vitesse VSC8601.
Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.

Signed-off-by: Tor Krill <tor@excito.com>
---
 drivers/net/tsec.c |   30 ++++++++++++++++++++++++++++++
 drivers/net/tsec.h |    5 +++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index 431a8d2..9d22aa3 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -1267,6 +1267,35 @@ struct phy_info phy_info_VSC8244 = {
 			   },
 };
 
+struct phy_info phy_info_VSC8601 = {
+		0x00007042,
+		"Vitesse VSC8601",
+		4,
+		(struct phy_cmd[]){     /* config */
+				/* Override PHY config settings */
+				/* Configure some basic stuff */
+				{MIIM_CONTROL, MIIM_CONTROL_INIT, &mii_cr_init},
+#ifdef CFG_VSC8601_SKEWFIX
+				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
+#endif
+				{miim_end,}
+				 },
+		(struct phy_cmd[]){     /* startup */
+				/* Read the Status (2x to make sure link is right) */
+				{MIIM_STATUS, miim_read, NULL},
+				/* Auto-negotiate */
+				{MIIM_STATUS, miim_read, &mii_parse_sr},
+				/* Read the status */
+				{MIIM_VSC8244_AUX_CONSTAT, miim_read,
+						&mii_parse_vsc8244},
+				{miim_end,}
+				},
+		(struct phy_cmd[]){     /* shutdown */
+				{miim_end,}
+				},
+};
+
+
 struct phy_info phy_info_dm9161 = {
 	0x0181b88,
 	"Davicom DM9161E",
@@ -1462,6 +1491,7 @@ struct phy_info *phy_info[] = {
 	&phy_info_dm9161,
 	&phy_info_lxt971,
 	&phy_info_VSC8244,
+	&phy_info_VSC8601,
 	&phy_info_dp83865,
 	&phy_info_rtl8211b,
 	&phy_info_generic,
diff --git a/drivers/net/tsec.h b/drivers/net/tsec.h
index d4dc15a..cfa7d1a 100644
--- a/drivers/net/tsec.h
+++ b/drivers/net/tsec.h
@@ -159,6 +159,11 @@
 #define MIIM_VSC8244_LED_CON            0x1b
 #define MIIM_VSC8244_LEDCON_INIT        0xF011
 
+/* Entry for Vitesse VSC8601 regs starts here (Not complete) */
+/* Vitesse VSC8601 Extended PHY Control Register 1 */
+#define MIIM_VSC8601_EPHY_CON			0x17
+#define MIIM_VSC8601_EPHY_CON_INIT_SKEW	0x1120
+
 /* 88E1011 PHY Status Register */
 #define MIIM_88E1011_PHY_STATUS         0x11
 #define MIIM_88E1011_PHYSTAT_SPEED      0xc000
-- 
1.5.4.4

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[André Schwarz]

Tor,

what a pitty - I've been working on this for 2 days now.
I did not implement the skew since I configured it by hardware strapping.

Very interesting PHY : low power, self calibrating ...... and cheap :-)

Is yor PHY already up and running including data ?

regards,
Andr? Schwarz
Matrix Vision

Tor Krill wrote:
> Add phy_info for Vitesse VSC8601.
> Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.
>
> Signed-off-by: Tor Krill <tor@excito.com>
> ---
>  drivers/net/tsec.c |   30 ++++++++++++++++++++++++++++++
>  drivers/net/tsec.h |    5 +++++
>  2 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> index 431a8d2..9d22aa3 100644
> --- a/drivers/net/tsec.c
> +++ b/drivers/net/tsec.c
> @@ -1267,6 +1267,35 @@ struct phy_info phy_info_VSC8244 = {
>  			   },
>  };
>  
> +struct phy_info phy_info_VSC8601 = {
> +		0x00007042,
> +		"Vitesse VSC8601",
> +		4,
> +		(struct phy_cmd[]){     /* config */
> +				/* Override PHY config settings */
> +				/* Configure some basic stuff */
> +				{MIIM_CONTROL, MIIM_CONTROL_INIT, &mii_cr_init},
> +#ifdef CFG_VSC8601_SKEWFIX
> +				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
> +#endif
> +				{miim_end,}
> +				 },
> +		(struct phy_cmd[]){     /* startup */
> +				/* Read the Status (2x to make sure link is right) */
> +				{MIIM_STATUS, miim_read, NULL},
> +				/* Auto-negotiate */
> +				{MIIM_STATUS, miim_read, &mii_parse_sr},
> +				/* Read the status */
> +				{MIIM_VSC8244_AUX_CONSTAT, miim_read,
> +						&mii_parse_vsc8244},
> +				{miim_end,}
> +				},
> +		(struct phy_cmd[]){     /* shutdown */
> +				{miim_end,}
> +				},
> +};
> +
> +
>  struct phy_info phy_info_dm9161 = {
>  	0x0181b88,
>  	"Davicom DM9161E",
> @@ -1462,6 +1491,7 @@ struct phy_info *phy_info[] = {
>  	&phy_info_dm9161,
>  	&phy_info_lxt971,
>  	&phy_info_VSC8244,
> +	&phy_info_VSC8601,
>  	&phy_info_dp83865,
>  	&phy_info_rtl8211b,
>  	&phy_info_generic,
> diff --git a/drivers/net/tsec.h b/drivers/net/tsec.h
> index d4dc15a..cfa7d1a 100644
> --- a/drivers/net/tsec.h
> +++ b/drivers/net/tsec.h
> @@ -159,6 +159,11 @@
>  #define MIIM_VSC8244_LED_CON            0x1b
>  #define MIIM_VSC8244_LEDCON_INIT        0xF011
>  
> +/* Entry for Vitesse VSC8601 regs starts here (Not complete) */
> +/* Vitesse VSC8601 Extended PHY Control Register 1 */
> +#define MIIM_VSC8601_EPHY_CON			0x17
> +#define MIIM_VSC8601_EPHY_CON_INIT_SKEW	0x1120
> +
>  /* 88E1011 PHY Status Register */
>  #define MIIM_88E1011_PHY_STATUS         0x11
>  #define MIIM_88E1011_PHYSTAT_SPEED      0xc000
>   


MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Tor Krill]

Ben regarding your call for network related patches. Im not sure if i
should have sent this one to you? Got no reactions on it.

/Tor

On 3/28/2008, "Tor Krill" <tor@excito.com> wrote:

>Add phy_info for Vitesse VSC8601.
>Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.
>
>Signed-off-by: Tor Krill <tor@excito.com>
>---
> drivers/net/tsec.c |   30 ++++++++++++++++++++++++++++++
> drivers/net/tsec.h |    5 +++++
> 2 files changed, 35 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
>index 431a8d2..9d22aa3 100644
>--- a/drivers/net/tsec.c
>+++ b/drivers/net/tsec.c
>@@ -1267,6 +1267,35 @@ struct phy_info phy_info_VSC8244 = {
> 			   },
> };
>
>+struct phy_info phy_info_VSC8601 = {
>+		0x00007042,
>+		"Vitesse VSC8601",
>+		4,
>+		(struct phy_cmd[]){     /* config */
>+				/* Override PHY config settings */
>+				/* Configure some basic stuff */
>+				{MIIM_CONTROL, MIIM_CONTROL_INIT, &mii_cr_init},
>+#ifdef CFG_VSC8601_SKEWFIX
>+				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
>+#endif
>+				{miim_end,}
>+				 },
>+		(struct phy_cmd[]){     /* startup */
>+				/* Read the Status (2x to make sure link is right) */
>+				{MIIM_STATUS, miim_read, NULL},
>+				/* Auto-negotiate */
>+				{MIIM_STATUS, miim_read, &mii_parse_sr},
>+				/* Read the status */
>+				{MIIM_VSC8244_AUX_CONSTAT, miim_read,
>+						&mii_parse_vsc8244},
>+				{miim_end,}
>+				},
>+		(struct phy_cmd[]){     /* shutdown */
>+				{miim_end,}
>+				},
>+};
>+
>+
> struct phy_info phy_info_dm9161 = {
> 	0x0181b88,
> 	"Davicom DM9161E",
>@@ -1462,6 +1491,7 @@ struct phy_info *phy_info[] = {
> 	&phy_info_dm9161,
> 	&phy_info_lxt971,
> 	&phy_info_VSC8244,
>+	&phy_info_VSC8601,
> 	&phy_info_dp83865,
> 	&phy_info_rtl8211b,
> 	&phy_info_generic,
>diff --git a/drivers/net/tsec.h b/drivers/net/tsec.h
>index d4dc15a..cfa7d1a 100644
>--- a/drivers/net/tsec.h
>+++ b/drivers/net/tsec.h
>@@ -159,6 +159,11 @@
> #define MIIM_VSC8244_LED_CON            0x1b
> #define MIIM_VSC8244_LEDCON_INIT        0xF011
>
>+/* Entry for Vitesse VSC8601 regs starts here (Not complete) */
>+/* Vitesse VSC8601 Extended PHY Control Register 1 */
>+#define MIIM_VSC8601_EPHY_CON			0x17
>+#define MIIM_VSC8601_EPHY_CON_INIT_SKEW	0x1120
>+
> /* 88E1011 PHY Status Register */
> #define MIIM_88E1011_PHY_STATUS         0x11
> #define MIIM_88E1011_PHYSTAT_SPEED      0xc000
>--
>1.5.4.4
>

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Ben Warren]

Tor Krill wrote:
> Add phy_info for Vitesse VSC8601.
> Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.
>
> Signed-off-by: Tor Krill <tor@excito.com>
>   
Acked-by: Ben Warren <biggerbadderben@gmail.com>
> ---
>  drivers/net/tsec.c |   30 ++++++++++++++++++++++++++++++
>  drivers/net/tsec.h |    5 +++++
>  2 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> index 431a8d2..9d22aa3 100644
> --- a/drivers/net/tsec.c
> +++ b/drivers/net/tsec.c
> @@ -1267,6 +1267,35 @@ struct phy_info phy_info_VSC8244 = {
>  			   },
>  };
>  
> +struct phy_info phy_info_VSC8601 = {
> +		0x00007042,
> +		"Vitesse VSC8601",
> +		4,
> +		(struct phy_cmd[]){     /* config */
> +				/* Override PHY config settings */
> +				/* Configure some basic stuff */
> +				{MIIM_CONTROL, MIIM_CONTROL_INIT, &mii_cr_init},
> +#ifdef CFG_VSC8601_SKEWFIX
> +				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
> +#endif
> +				{miim_end,}
> +				 },
> +		(struct phy_cmd[]){     /* startup */
> +				/* Read the Status (2x to make sure link is right) */
> +				{MIIM_STATUS, miim_read, NULL},
> +				/* Auto-negotiate */
> +				{MIIM_STATUS, miim_read, &mii_parse_sr},
> +				/* Read the status */
> +				{MIIM_VSC8244_AUX_CONSTAT, miim_read,
> +						&mii_parse_vsc8244},
> +				{miim_end,}
> +				},
> +		(struct phy_cmd[]){     /* shutdown */
> +				{miim_end,}
> +				},
> +};
> +
> +
>  struct phy_info phy_info_dm9161 = {
>  	0x0181b88,
>  	"Davicom DM9161E",
> @@ -1462,6 +1491,7 @@ struct phy_info *phy_info[] = {
>  	&phy_info_dm9161,
>  	&phy_info_lxt971,
>  	&phy_info_VSC8244,
> +	&phy_info_VSC8601,
>  	&phy_info_dp83865,
>  	&phy_info_rtl8211b,
>  	&phy_info_generic,
> diff --git a/drivers/net/tsec.h b/drivers/net/tsec.h
> index d4dc15a..cfa7d1a 100644
> --- a/drivers/net/tsec.h
> +++ b/drivers/net/tsec.h
> @@ -159,6 +159,11 @@
>  #define MIIM_VSC8244_LED_CON            0x1b
>  #define MIIM_VSC8244_LEDCON_INIT        0xF011
>  
> +/* Entry for Vitesse VSC8601 regs starts here (Not complete) */
> +/* Vitesse VSC8601 Extended PHY Control Register 1 */
> +#define MIIM_VSC8601_EPHY_CON			0x17
> +#define MIIM_VSC8601_EPHY_CON_INIT_SKEW	0x1120
> +
>  /* 88E1011 PHY Status Register */
>  #define MIIM_88E1011_PHY_STATUS         0x11
>  #define MIIM_88E1011_PHYSTAT_SPEED      0xc000
>   

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Ben Warren]

Tor Krill wrote:
> Ben regarding your call for network related patches. Im not sure if i
> should have sent this one to you? Got no reactions on it.
>
> /Tor
>   
No problem.  If either Kim Phillips or Andy Fleming blesses it, I'll 
pull it in today.

regards,
Ben

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Kim Phillips]

On Mon, 31 Mar 2008 10:01:34 -0400
Ben Warren <biggerbadderben@gmail.com> wrote:

> Tor Krill wrote:
> > Add phy_info for Vitesse VSC8601.
> > Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.
> >
> > Signed-off-by: Tor Krill <tor@excito.com>
> >   
> Acked-by: Ben Warren <biggerbadderben@gmail.com>

I don't have a Vitesse 8601, so technically I can't ack it, but I can:

Reviewed-by: Kim Phillips <kim.phillips@freescale.com>

minor nit: it would be nice if the following:

> > +#ifdef CFG_VSC8601_SKEWFIX
> > +				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
> > +#endif

were made to have spaces after commas, and flow onto a separate
line so as to not be a 96 char line..

Kim

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Andre Schwarz]

Kim,

would it help if we donate one or two boards to the custodians ?
Or do you have no time at all for specific testing ?

After bring-up + validation I could offer 1-2 MPC8343B based boards with 
2x VSC8601.
The system also has a miniPCI Slot, USB-A and 512MB DDR-II Micron memory.


Please let me know if you're interested.


regards,
Andre Schwarz
Matrix Vision



Kim Phillips schrieb:
> On Mon, 31 Mar 2008 10:01:34 -0400
> Ben Warren <biggerbadderben@gmail.com> wrote:
>
>   
>> Tor Krill wrote:
>>     
>>> Add phy_info for Vitesse VSC8601.
>>> Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.
>>>
>>> Signed-off-by: Tor Krill <tor@excito.com>
>>>   
>>>       
>> Acked-by: Ben Warren <biggerbadderben@gmail.com>
>>     
>
> I don't have a Vitesse 8601, so technically I can't ack it, but I can:
>
> Reviewed-by: Kim Phillips <kim.phillips@freescale.com>
>
> minor nit: it would be nice if the following:
>
>   
>>> +#ifdef CFG_VSC8601_SKEWFIX
>>> +				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
>>> +#endif
>>>       
>
> were made to have spaces after commas, and flow onto a separate
> line so as to not be a 96 char line..
>
> Kim
>
> -------------------------------------------------------------------------
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for
> just about anything Open Source.
> http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>   



MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20080401/7495f4a5/attachment.htm 

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Andre Schwarz]

Tor,

after investigating the tsec code I'm wondering how your PHY  works in 
RGMII mode ...

I think that there are some things missing, e.g. taking RGMII into 
account during tsec_init.

/* Init ECNTRL */
        regs->ecntrl = ECNTRL_INIT_SETTINGS;

This will clear bit 27 which indicates RGMII as set up by the HRCW.


I would expect something like

        if ( priv->flags & TSEC_REDUCED )
                regs->ecntrl |= ECNTRL_RPM;

afterwards.



Before I'll start doing double work again : Have you some unposted 
patches regarding RGMII on TSEC ?

Am I missing something ?


@Kim : Did you ever run a MPC834x with a RGMII  PHY ? Is it known to work ?


regards,
Andre



Tor Krill schrieb:
> Add phy_info for Vitesse VSC8601.
> Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.
>
> Signed-off-by: Tor Krill <tor@excito.com>
> ---
>  drivers/net/tsec.c |   30 ++++++++++++++++++++++++++++++
>  drivers/net/tsec.h |    5 +++++
>  2 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> index 431a8d2..9d22aa3 100644
> --- a/drivers/net/tsec.c
> +++ b/drivers/net/tsec.c
> @@ -1267,6 +1267,35 @@ struct phy_info phy_info_VSC8244 = {
>  			   },
>  };
>  
> +struct phy_info phy_info_VSC8601 = {
> +		0x00007042,
> +		"Vitesse VSC8601",
> +		4,
> +		(struct phy_cmd[]){     /* config */
> +				/* Override PHY config settings */
> +				/* Configure some basic stuff */
> +				{MIIM_CONTROL, MIIM_CONTROL_INIT, &mii_cr_init},
> +#ifdef CFG_VSC8601_SKEWFIX
> +				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
> +#endif
> +				{miim_end,}
> +				 },
> +		(struct phy_cmd[]){     /* startup */
> +				/* Read the Status (2x to make sure link is right) */
> +				{MIIM_STATUS, miim_read, NULL},
> +				/* Auto-negotiate */
> +				{MIIM_STATUS, miim_read, &mii_parse_sr},
> +				/* Read the status */
> +				{MIIM_VSC8244_AUX_CONSTAT, miim_read,
> +						&mii_parse_vsc8244},
> +				{miim_end,}
> +				},
> +		(struct phy_cmd[]){     /* shutdown */
> +				{miim_end,}
> +				},
> +};
> +
> +
>  struct phy_info phy_info_dm9161 = {
>  	0x0181b88,
>  	"Davicom DM9161E",
> @@ -1462,6 +1491,7 @@ struct phy_info *phy_info[] = {
>  	&phy_info_dm9161,
>  	&phy_info_lxt971,
>  	&phy_info_VSC8244,
> +	&phy_info_VSC8601,
>  	&phy_info_dp83865,
>  	&phy_info_rtl8211b,
>  	&phy_info_generic,
> diff --git a/drivers/net/tsec.h b/drivers/net/tsec.h
> index d4dc15a..cfa7d1a 100644
> --- a/drivers/net/tsec.h
> +++ b/drivers/net/tsec.h
> @@ -159,6 +159,11 @@
>  #define MIIM_VSC8244_LED_CON            0x1b
>  #define MIIM_VSC8244_LEDCON_INIT        0xF011
>  
> +/* Entry for Vitesse VSC8601 regs starts here (Not complete) */
> +/* Vitesse VSC8601 Extended PHY Control Register 1 */
> +#define MIIM_VSC8601_EPHY_CON			0x17
> +#define MIIM_VSC8601_EPHY_CON_INIT_SKEW	0x1120
> +
>  /* 88E1011 PHY Status Register */
>  #define MIIM_88E1011_PHY_STATUS         0x11
>  #define MIIM_88E1011_PHYSTAT_SPEED      0xc000
>   


MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Andy Fleming]

On Tue, Apr 1, 2008 at 8:33 AM, Andre Schwarz
<andre.schwarz@matrix-vision.de> wrote:
> Tor,
>
>  after investigating the tsec code I'm wondering how your PHY  works in
>  RGMII mode ...
>
>  I think that there are some things missing, e.g. taking RGMII into
>  account during tsec_init.
>
>  /* Init ECNTRL */
>         regs->ecntrl = ECNTRL_INIT_SETTINGS;

If you look carefully, you'll notice that ecntrl's RPM bit is
read-only.  Those bits are configured by POR pin strappings.

You may be more familiar with the UEC, which doesn't automatically
detect the link type, but is otherwise fairly similar to the tsec.

Andy

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Andre Schwarz]

Andy Fleming schrieb:
> On Tue, Apr 1, 2008 at 8:33 AM, Andre Schwarz
> <andre.schwarz@matrix-vision.de> wrote:
>   
>> Tor,
>>
>>  after investigating the tsec code I'm wondering how your PHY  works in
>>  RGMII mode ...
>>
>>  I think that there are some things missing, e.g. taking RGMII into
>>  account during tsec_init.
>>
>>  /* Init ECNTRL */
>>         regs->ecntrl = ECNTRL_INIT_SETTINGS;
>>     
>
> If you look carefully, you'll notice that ecntrl's RPM bit is
> read-only.  Those bits are configured by POR pin strappings.
>
>   
sorry, my documentation (MPC8349EARM rev.1) declares this register 
read-write.
Of course it will be configured by the HRCW but can be overwritten 
afterwards.

If this is not true it's a documentation bug.

> You may be more familiar with the UEC, which doesn't automatically
> detect the link type, but is otherwise fairly similar to the tsec.
>
>   
What do you mean ?
I'm trying to get two VSC8601 RGMII PHYs running on a MPC8343B ...


> Andy
>   


regards,
Andre


MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20080401/7bad5047/attachment.htm 

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Andy Fleming]

On Tue, Apr 1, 2008 at 9:08 AM, Andre Schwarz
<andre.schwarz@matrix-vision.de> wrote:
>
>
>  Tor,
>
>  after investigating the tsec code I'm wondering how your PHY works in
>  RGMII mode ...
>
>  I think that there are some things missing, e.g. taking RGMII into
>  account during tsec_init.
>
>  /* Init ECNTRL */
>  regs->ecntrl = ECNTRL_INIT_SETTINGS;
>
>  If you look carefully, you'll notice that ecntrl's RPM bit is
> read-only. Those bits are configured by POR pin strappings.
>
>
>  sorry, my documentation (MPC8349EARM rev.1) declares this register
> read-write.
>  Of course it will be configured by the HRCW but can be overwritten
> afterwards.
>
>  If this is not true it's a documentation bug.


Thank you for bringing this to my attention.  It is almost certainly a
bug.  If you look at the 8349ERM (rather than the 8349EARM), you'll
see that the bits are read-only (except for R100, which tsec.c does
modify based on the link type).  I will file a bug with the docs
people.


>
>
>  You may be more familiar with the UEC, which doesn't automatically
> detect the link type, but is otherwise fairly similar to the tsec.
>
>
>  What do you mean ?
>  I'm trying to get two VSC8601 RGMII PHYs running on a MPC8343B ...

sorry, I forgot which ethernet controller you were using, and I'm not
as familiar with the 83xx family as the 85xx family.  Some of our
parts have the QUICC Engine, which has an ethernet controller with
some similar registers to the TSEC's.  I was guessing that was why you
thought those bits were writable, rather than a documentation bug, but
clearly I was wrong.  :)

Andy

Andy

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Andre Schwarz]

Andy Fleming schrieb:
> On Tue, Apr 1, 2008 at 9:08 AM, Andre Schwarz
> <andre.schwarz@matrix-vision.de> wrote:
>   
>>  Tor,
>>
>>  after investigating the tsec code I'm wondering how your PHY works in
>>  RGMII mode ...
>>
>>  I think that there are some things missing, e.g. taking RGMII into
>>  account during tsec_init.
>>
>>  /* Init ECNTRL */
>>  regs->ecntrl = ECNTRL_INIT_SETTINGS;
>>
>>  If you look carefully, you'll notice that ecntrl's RPM bit is
>> read-only. Those bits are configured by POR pin strappings.
>>
>>
>>  sorry, my documentation (MPC8349EARM rev.1) declares this register
>> read-write.
>>  Of course it will be configured by the HRCW but can be overwritten
>> afterwards.
>>
>>  If this is not true it's a documentation bug.
>>     
>
>
> Thank you for bringing this to my attention.  It is almost certainly a
> bug.  If you look at the 8349ERM (rather than the 8349EARM), you'll
> see that the bits are read-only (except for R100, which tsec.c does
> modify based on the link type).  I will file a bug with the docs
> people.
>
>
>   
I've x-checked and printed the register after init.
You're right - it's read only and the RPM bit is set.
But the ECNTL  register description @ 15.5.3.1.4 says "read/write" to 
all bits ...
>>  You may be more familiar with the UEC, which doesn't automatically
>> detect the link type, but is otherwise fairly similar to the tsec.
>>
>>
>>  What do you mean ?
>>  I'm trying to get two VSC8601 RGMII PHYs running on a MPC8343B ...
>>     
>
> sorry, I forgot which ethernet controller you were using, and I'm not
> as familiar with the 83xx family as the 85xx family.  Some of our
> parts have the QUICC Engine, which has an ethernet controller with
> some similar registers to the TSEC's.  I was guessing that was why you
> thought those bits were writable, rather than a documentation bug, but
> clearly I was wrong.  :)
>
>   
no problem.

Andre
> Andy
>
> Andy
>   



MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20080401/51718d44/attachment.htm 

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Kim Phillips]

On Tue, 01 Apr 2008 15:33:10 +0200
Andre Schwarz <andre.schwarz@matrix-vision.de> wrote:

> I would expect something like
> 
>         if ( priv->flags & TSEC_REDUCED )
>                 regs->ecntrl |= ECNTRL_RPM;

yeah, that register should be r/o..

> @Kim : Did you ever run a MPC834x with a RGMII  PHY ? Is it known to work ?

834x?  I haven't, but I also haven't a reason why it would not work :)

Kim

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[André Schwarz]

Kim Phillips schrieb:
> On Tue, 01 Apr 2008 15:33:10 +0200
> Andre Schwarz <andre.schwarz@matrix-vision.de> wrote:
> 
>> I would expect something like
>>
>>         if ( priv->flags & TSEC_REDUCED )
>>                 regs->ecntrl |= ECNTRL_RPM;
> 
> yeah, that register should be r/o..
> 
you're right.
The register actually _is_ r/o. But datasheet says "r/w" ...

>> @Kim : Did you ever run a MPC834x with a RGMII  PHY ? Is it known to work ?
> 
> 834x?  I haven't, but I also haven't a reason why it would not work :)

ok - I'll have a closer look into the hardware.
> 
> Kim

thanks,
Andr?


MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Andre Schwarz]

Tor,

after all my VSC8601 is up and running on MPC8343  :-)

I'm sorry to say that I don't find this patch ok after going through the 
manuals :

Register 0x17 is a very coarse setting. If the capabilities of the PHY 
should be taken into account and be configurable we should use the skew 
control in extended register 0x1c. This should be definable - 
CFG_VSC8601_SKEWFIX simply applies maximum skew ...

After all changing the bits in register 0x17 _require_ a soft reset by 
asserting bit 15 in register 0 before they are going to work.

Obviously this patch has no effect at all.
Do you reset the PHY manually after this configuration ?

This PHY definitely needs a proper setup function since there are quite 
interesting registers which need read-modify-write.


What do you think ?


regards,
Andre

Kim Phillips schrieb:
> On Mon, 31 Mar 2008 10:01:34 -0400
> Ben Warren <biggerbadderben@gmail.com> wrote:
>
>   
>> Tor Krill wrote:
>>     
>>> Add phy_info for Vitesse VSC8601.
>>> Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.
>>>
>>> Signed-off-by: Tor Krill <tor@excito.com>
>>>   
>>>       
>> Acked-by: Ben Warren <biggerbadderben@gmail.com>
>>     
>
> I don't have a Vitesse 8601, so technically I can't ack it, but I can:
>
> Reviewed-by: Kim Phillips <kim.phillips@freescale.com>
>
> minor nit: it would be nice if the following:
>
>   
>>> +#ifdef CFG_VSC8601_SKEWFIX
>>> +				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
>>> +#endif
>>>       
>
> were made to have spaces after commas, and flow onto a separate
> line so as to not be a 96 char line..
>
> Kim
>
> -------------------------------------------------------------------------
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for
> just about anything Open Source.
> http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>   



MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20080417/fa4537ba/attachment.htm 

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Tor Krill]

Hi,

On 4/17/2008, "Andre Schwarz" <andre.schwarz@matrix-vision.de> wrote:

>Tor,
>
>after all my VSC8601 is up and running on MPC8343  :-)
>
>I'm sorry to say that I don't find this patch ok after going through the
>manuals :
>
>Register 0x17 is a very coarse setting. If the capabilities of the PHY
>should be taken into account and be configurable we should use the skew
>control in extended register 0x1c. This should be definable -
>CFG_VSC8601_SKEWFIX simply applies maximum skew ...

Sure it certainly could have been done in a more configurable way. But
you have to decide what you need. For us this was an apropriate level
atm that scratched our itch. If we where to expose every setting from
the start we still would not have been done  ;)

>After all changing the bits in register 0x17 _require_ a soft reset by
>asserting bit 15 in register 0 before they are going to work.
>
>Obviously this patch has no effect at all.
>Do you reset the PHY manually after this configuration ?

According to our datasheet (dated july 2006) only changes of bit 12 in
this register needs a software reset to take. We don't reset the phy
after changing this and the change obviously work (we have tested and
verified that it won't work without the change).

>This PHY definitely needs a proper setup function since there are quite
>interesting registers which need read-modify-write.
>
>What do you think ?

Perhaps a patch to improve what you find missing?

/Tor

>
>Kim Phillips schrieb:
>> On Mon, 31 Mar 2008 10:01:34 -0400
>> Ben Warren <biggerbadderben@gmail.com> wrote:
>>
>>
>>> Tor Krill wrote:
>>>
>>>> Add phy_info for Vitesse VSC8601.
>>>> Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.
>>>>
>>>> Signed-off-by: Tor Krill <tor@excito.com>
>>>>
>>>>
>>> Acked-by: Ben Warren <biggerbadderben@gmail.com>
>>>
>>
>> I don't have a Vitesse 8601, so technically I can't ack it, but I can:
>>
>> Reviewed-by: Kim Phillips <kim.phillips@freescale.com>
>>
>> minor nit: it would be nice if the following:
>>
>>
>>>> +#ifdef CFG_VSC8601_SKEWFIX
>>>> +				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
>>>> +#endif
>>>>
>>
>> were made to have spaces after commas, and flow onto a separate
>> line so as to not be a 96 char line..
>>
>> Kim
>>
>> -------------------------------------------------------------------------
>> Check out the new SourceForge.net Marketplace.
>> It's the best place to buy or sell services for
>> just about anything Open Source.
>> http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
>> _______________________________________________
>> U-Boot-Users mailing list
>> U-Boot-Users at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>>
>
>
>
>MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
>Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

[U-Boot-Users] [PATCH] Add Vitesse 8601 support to TSEC driver

[Andre Schwarz]

Tor,

at last I could solve my "strange" behaviour.

Actually the PHY definitely uses both 2 bit definitions of Rx and Tx Skew at
"RGMII Skew Control Reg" 0x1c - which are set up by eeprom or strapping.

Due to noise or strapping resistor mismatch these setting are not always
like
I want them to be.

Setting bit 8 in "Extended PHY Control 1" @ 0x17 activates the delay lines.
Obviously your Skew values did match.

I'll send a patch to configure the delay.

Cheers,
Andre

Tor Krill schrieb:
> Hi,
>
> On 4/17/2008, "Andre Schwarz" <andre.schwarz@matrix-vision.de> wrote:
>
>   
>> Tor,
>>
>> after all my VSC8601 is up and running on MPC8343  :-)
>>
>> I'm sorry to say that I don't find this patch ok after going through the
>> manuals :
>>
>> Register 0x17 is a very coarse setting. If the capabilities of the PHY
>> should be taken into account and be configurable we should use the skew
>> control in extended register 0x1c. This should be definable -
>> CFG_VSC8601_SKEWFIX simply applies maximum skew ...
>>     
>
> Sure it certainly could have been done in a more configurable way. But
> you have to decide what you need. For us this was an apropriate level
> atm that scratched our itch. If we where to expose every setting from
> the start we still would not have been done  ;)
>
>   
>> After all changing the bits in register 0x17 _require_ a soft reset by
>> asserting bit 15 in register 0 before they are going to work.
>>
>> Obviously this patch has no effect at all.
>> Do you reset the PHY manually after this configuration ?
>>     
>
> According to our datasheet (dated july 2006) only changes of bit 12 in
> this register needs a software reset to take. We don't reset the phy
> after changing this and the change obviously work (we have tested and
> verified that it won't work without the change).
>
>   
>> This PHY definitely needs a proper setup function since there are quite
>> interesting registers which need read-modify-write.
>>
>> What do you think ?
>>     
>
> Perhaps a patch to improve what you find missing?
>
> /Tor
>
>   
>> Kim Phillips schrieb:
>>     
>>> On Mon, 31 Mar 2008 10:01:34 -0400
>>> Ben Warren <biggerbadderben@gmail.com> wrote:
>>>
>>>
>>>       
>>>> Tor Krill wrote:
>>>>
>>>>         
>>>>> Add phy_info for Vitesse VSC8601.
>>>>> Add config option, CFG_VSC8601_SKEWFIX, to enable RGMII skew timing compensation.
>>>>>
>>>>> Signed-off-by: Tor Krill <tor@excito.com>
>>>>>
>>>>>
>>>>>           
>>>> Acked-by: Ben Warren <biggerbadderben@gmail.com>
>>>>
>>>>         
>>> I don't have a Vitesse 8601, so technically I can't ack it, but I can:
>>>
>>> Reviewed-by: Kim Phillips <kim.phillips@freescale.com>
>>>
>>> minor nit: it would be nice if the following:
>>>
>>>
>>>       
>>>>> +#ifdef CFG_VSC8601_SKEWFIX
>>>>> +				{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL},
>>>>> +#endif
>>>>>
>>>>>           
>>> were made to have spaces after commas, and flow onto a separate
>>> line so as to not be a 96 char line..
>>>
>>> Kim
>>>
>>> -------------------------------------------------------------------------
>>> Check out the new SourceForge.net Marketplace.
>>> It's the best place to buy or sell services for
>>> just about anything Open Source.
>>> http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
>>> _______________________________________________
>>> U-Boot-Users mailing list
>>> U-Boot-Users at lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>>>
>>>       
>>
>> MATRIX VISION GmbH, Talstra??e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
>> Gesch??ftsf??hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
>>     


MATRIX VISION GmbH, Talstra?e 16, DE-71570 Oppenweiler  - Registergericht: Amtsgericht Stuttgart, HRB 271090
Gesch?ftsf?hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner