[U-Boot-Users] [PATCH] add config options for VSC8601 RGMII PHY
The Vitesse VSC8601 RGMII PHY has internal delay for both Rx and Tx clock lines. They are configured using 2 bits in extended register 0x17. Therefore CFG_VSC8601_SKEW_TX and CFG_VSC8601_SKEW_RX have been introduced with valid values 0-3 giving 0.0, 1.4,1.7 and 2.0ns delay.
Signed-off-by: Andre Schwarz andre.schwarz@matrix-vision.de --
drivers/net/tsec.c | 6 ++++++ drivers/net/tsec.h | 4 ++++ 2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c index 9d22aa3..06250ae 100644 --- a/drivers/net/tsec.c +++ b/drivers/net/tsec.c @@ -1277,6 +1277,12 @@ struct phy_info phy_info_VSC8601 = { {MIIM_CONTROL, MIIM_CONTROL_INIT, &mii_cr_init}, #ifdef CFG_VSC8601_SKEWFIX
{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL}, +#if defined(CFG_VSC8601_SKEW_TX) && defined(CFG_VSC8601_SKEW_RX) + {MIIM_EXT_PAGE_ACCESS,1,NULL}, +#define VSC8101_SKEW (CFG_VSC8601_SKEW_TX<<14)|(CFG_VSC8601_SKEW_RX<<12) + {MIIM_VSC8601_SKEW_CTRL,VSC8101_SKEW,NULL}, + {MIIM_EXT_PAGE_ACCESS,0,NULL}, +#endif #endif {miim_end,} }, diff --git a/drivers/net/tsec.h b/drivers/net/tsec.h index cfa7d1a..e8776b0 100644 --- a/drivers/net/tsec.h +++ b/drivers/net/tsec.h @@ -112,6 +112,8 @@ #define MIIM_GBIT_CONTROL 0x9 #define MIIM_GBIT_CONTROL_INIT 0xe00
+#define MIIM_EXT_PAGE_ACCESS 0x1f + /* Broadcom BCM54xx -- taken from linux sungem_phy */ #define MIIM_BCM54xx_AUXSTATUS 0x19 #define MIIM_BCM54xx_AUXSTATUS_LINKMODE_MASK 0x0700 @@ -163,6 +165,8 @@ /* Vitesse VSC8601 Extended PHY Control Register 1 */ #define MIIM_VSC8601_EPHY_CON 0x17 #define MIIM_VSC8601_EPHY_CON_INIT_SKEW 0x1120 +#define MIIM_VSC8601_SKEW_CTRL 0x1c +#define MIIM_VSC8601_EPHY_CON_INIT_SKEW 0x1120
/* 88E1011 PHY Status Register */ #define MIIM_88E1011_PHY_STATUS 0x11
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
On Thu, Apr 24, 2008 at 9:45 AM, Andre Schwarz andre.schwarz@matrix-vision.de wrote:
{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL}, +#if defined(CFG_VSC8601_SKEW_TX) && defined(CFG_VSC8601_SKEW_RX)
{MIIM_EXT_PAGE_ACCESS,1,NULL},+#define VSC8101_SKEW (CFG_VSC8601_SKEW_TX<<14)|(CFG_VSC8601_SKEW_RX<<12)
{MIIM_VSC8601_SKEW_CTRL,VSC8101_SKEW,NULL},{MIIM_EXT_PAGE_ACCESS,0,NULL},+#endif #endif
I'm not sure this is the best solution to this. It seems like it wouldn't scale well. Either we need to set a bit somewhere that the phy driver can read (and thereby determine how to configure the skew), or we need to set the value to write in the board config file. I'm partial to the first solution, as it encapsulates the information inside the code that deals with it.
[...]
/* Broadcom BCM54xx -- taken from linux sungem_phy */ #define MIIM_BCM54xx_AUXSTATUS 0x19 #define MIIM_BCM54xx_AUXSTATUS_LINKMODE_MASK 0x0700 @@ -163,6 +165,8 @@ /* Vitesse VSC8601 Extended PHY Control Register 1 */ #define MIIM_VSC8601_EPHY_CON 0x17 #define MIIM_VSC8601_EPHY_CON_INIT_SKEW 0x1120 +#define MIIM_VSC8601_SKEW_CTRL 0x1c +#define MIIM_VSC8601_EPHY_CON_INIT_SKEW 0x1120
Am I crazy, or did you just doubly define MIIM_VSC8601_EPHY_CON_INIT_SKEW?
Andy,
thanks for your comments.
Andy Fleming schrieb:
On Thu, Apr 24, 2008 at 9:45 AM, Andre Schwarz andre.schwarz@matrix-vision.de wrote:
{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL}, +#if defined(CFG_VSC8601_SKEW_TX) && defined(CFG_VSC8601_SKEW_RX)
{MIIM_EXT_PAGE_ACCESS,1,NULL},+#define VSC8101_SKEW (CFG_VSC8601_SKEW_TX<<14)|(CFG_VSC8601_SKEW_RX<<12)
{MIIM_VSC8601_SKEW_CTRL,VSC8101_SKEW,NULL},{MIIM_EXT_PAGE_ACCESS,0,NULL},+#endif #endif
I'm not sure this is the best solution to this. It seems like it wouldn't scale well. Either we need to set a bit somewhere that the phy driver can read (and thereby determine how to configure the skew), or we need to set the value to write in the board config file. I'm partial to the first solution, as it encapsulates the information inside the code that deals with it.
[...]
I don't understand "scale well". What should be scalable ?
Of course using a function would be better. I silently assumed that the other bits are set to zero which is true after reset. There are only two other bits : Packet size and 10M preamble mode. Both should be left untouched, i.e. "0".
/* Broadcom BCM54xx -- taken from linux sungem_phy */ #define MIIM_BCM54xx_AUXSTATUS 0x19 #define MIIM_BCM54xx_AUXSTATUS_LINKMODE_MASK 0x0700 @@ -163,6 +165,8 @@ /* Vitesse VSC8601 Extended PHY Control Register 1 */ #define MIIM_VSC8601_EPHY_CON 0x17 #define MIIM_VSC8601_EPHY_CON_INIT_SKEW 0x1120 +#define MIIM_VSC8601_SKEW_CTRL 0x1c +#define MIIM_VSC8601_EPHY_CON_INIT_SKEW 0x1120
Am I crazy, or did you just doubly define MIIM_VSC8601_EPHY_CON_INIT_SKEW?
This obviously is a mistake - sorry.
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
On Tue, Apr 29, 2008 at 1:58 AM, Andre Schwarz andre.schwarz@matrix-vision.de wrote:
Andy,
thanks for your comments.
Andy Fleming schrieb:
On Thu, Apr 24, 2008 at 9:45 AM, Andre Schwarz andre.schwarz@matrix-vision.de wrote:
{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL}, +#if defined(CFG_VSC8601_SKEW_TX) && defined(CFG_VSC8601_SKEW_RX)
{MIIM_EXT_PAGE_ACCESS,1,NULL},+#define VSC8101_SKEW (CFG_VSC8601_SKEW_TX<<14)|(CFG_VSC8601_SKEW_RX<<12)
{MIIM_VSC8601_SKEW_CTRL,VSC8101_SKEW,NULL},{MIIM_EXT_PAGE_ACCESS,0,NULL},+#endif #endif
I'm not sure this is the best solution to this. It seems like it wouldn't scale well. Either we need to set a bit somewhere that the phy driver can read (and thereby determine how to configure the skew), or we need to set the value to write in the board config file. I'm partial to the first solution, as it encapsulates the information inside the code that deals with it.
[...]
I don't understand "scale well". What should be scalable ?
Of course using a function would be better. I silently assumed that the other bits are set to zero which is true after reset. There are only two other bits : Packet size and 10M preamble mode. Both should be left untouched, i.e. "0".
Sorry, I should have been more explicit. In this case I'm referring to how well the solution scales as we deal with more configuration options. Your solution isn't a terrible example of this, but #ifdefs that seem straightforward when there is one of them quickly can become unmanageable. I'll admit, though, this is the sort of problem that is best solved by rearchitecting the PHY code in tsec.c, which is, I believe, under way by Ben.
In light of the fact that the #ifdefs here are feature-based, and that the PHY code is probably changing soon, anyway, I'd be ok with letting this patch through (with the duplicate #define removed, of course).
Andy
Andy,
it's no problem to re-send a "more suitable" patch as soon as Ben has finished the re-work. I'll wait for him and send an updated version to the list.
Cheers, André
Andy Fleming schrieb:
On Tue, Apr 29, 2008 at 1:58 AM, Andre Schwarz andre.schwarz@matrix-vision.de wrote:
Andy,
thanks for your comments.
Andy Fleming schrieb:
On Thu, Apr 24, 2008 at 9:45 AM, Andre Schwarz andre.schwarz@matrix-vision.de wrote:
{MIIM_VSC8601_EPHY_CON,MIIM_VSC8601_EPHY_CON_INIT_SKEW,NULL}, +#if defined(CFG_VSC8601_SKEW_TX) && defined(CFG_VSC8601_SKEW_RX)
{MIIM_EXT_PAGE_ACCESS,1,NULL},+#define VSC8101_SKEW (CFG_VSC8601_SKEW_TX<<14)|(CFG_VSC8601_SKEW_RX<<12)
{MIIM_VSC8601_SKEW_CTRL,VSC8101_SKEW,NULL},{MIIM_EXT_PAGE_ACCESS,0,NULL},+#endif #endif
I'm not sure this is the best solution to this. It seems like it wouldn't scale well. Either we need to set a bit somewhere that the phy driver can read (and thereby determine how to configure the skew), or we need to set the value to write in the board config file. I'm partial to the first solution, as it encapsulates the information inside the code that deals with it.
[...]
I don't understand "scale well". What should be scalable ?
Of course using a function would be better. I silently assumed that the other bits are set to zero which is true after reset. There are only two other bits : Packet size and 10M preamble mode. Both should be left untouched, i.e. "0".
Sorry, I should have been more explicit. In this case I'm referring to how well the solution scales as we deal with more configuration options. Your solution isn't a terrible example of this, but #ifdefs that seem straightforward when there is one of them quickly can become unmanageable. I'll admit, though, this is the sort of problem that is best solved by rearchitecting the PHY code in tsec.c, which is, I believe, under way by Ben.
In light of the fact that the #ifdefs here are feature-based, and that the PHY code is probably changing soon, anyway, I'd be ok with letting this patch through (with the duplicate #define removed, of course).
Andy
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
Ben Warren schrieb:
André Schwarz wrote:
Andy,
it's no problem to re-send a "more suitable" patch as soon as Ben has finished the re-work. I'll wait for him and send an updated version to the list.
It's better to get this in now if you don't mind.
regards, Ben
I don't mind and will send it a a new patch.
Cheers, Andre
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner
participants (4)
-
Andre Schwarz -
André Schwarz
-
Andy Fleming -
Ben Warren