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

[André Schwarz]

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


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