[U-Boot-Users] [PATCH] add config options for VSC8601 RGMII PHY

[Andy Fleming]

On Tue, Apr 29, 2008 at 1:58 AM, Andre Schwarz
<andre.schwarz at 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 at 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