[U-Boot] [PATCH 1/1 v3] ppc4xx: Add support for GPCS, SGMII and M88E1112 PHY

[Ben Warren]

Hi Stefan and Victor,

Stefan Roese wrote:
> On Wednesday 03 September 2008, Victor Gallardo wrote:
>   
>> This patch adds GPCS, SGMII and M88E1112 PHY support
>> for the AMCC PPC460GT/EX processors.
>>
>> Signed-off-by: Victor Gallardo <vgallardo at amcc.com>
>> ---
>>     
>
> A good idea is to keep a history of what changed in the patch revisions here 
> in this area (after the "---"). Something like:
>
> v2:
> - Added comments to GPCS PHY setup
> - Minor coding style cleanup
>
> v3: 
> - Generalized the PHY-less configuration even more
>
> Please find some more comments below.
>
>   
>>  cpu/ppc4xx/4xx_enet.c |  162
>> ++++++++++++++++++++++++++++++++++++++++++++++++- cpu/ppc4xx/miiphy.c   |  
>> 41 ++++++++++++-
>>  include/ppc4xx_enet.h |    3 +
>>  3 files changed, 201 insertions(+), 5 deletions(-)
>>
>> diff --git a/cpu/ppc4xx/4xx_enet.c b/cpu/ppc4xx/4xx_enet.c
>> index 8a38335..e137bac 100644
>> --- a/cpu/ppc4xx/4xx_enet.c
>> +++ b/cpu/ppc4xx/4xx_enet.c
>> @@ -198,6 +198,7 @@
>>  #define BI_PHYMODE_RMII  8
>>  #endif
>>  #endif
>> +#define BI_PHYMODE_SGMII 9
>>
>>  #if defined(CONFIG_440SP) || defined(CONFIG_440SPE) || \
>>      defined(CONFIG_440EPX) || defined(CONFIG_440GRX) || \
>> @@ -216,6 +217,52 @@
>>  #define MAL_RX_CHAN_MUL	1
>>  #endif
>>
>> +/*--------------------------------------------------------------------+
>> + * PHY-less support for Ethernet Ports.
>> + *--------------------------------------------------------------------*/
>> +
>> +/*
>> + * Some boards do not have a PHY for each ethernet port.
>> + * For example on Arches board (2 CPU system) eth0 does not have
>> + * a PHY, both CPU's are wired directly together (AC coupled)
>> + * using SGMII0.
>> + *
>> + * In these cases :
>> + *    1) set the appropriate CONFIG_PHY_ADDR equal to CONFIG_PHY_LESS
>> + *       to detect that the specified ethernet port does not have a PHY.
>> + *    2) Then define CFG_PHY_LESS_PORT and CFG_PHY_LESS_PORTS in board
>> + *       configuration file. For example on the Arches board we would do
>> + *       the following.
>> + *         #define CFG_PHY_LESS_PORT(devnum,speed,duplex) \
>> + *                 { devnum, speed, duplex},
>> + *         #define CFG_PHY_LESS_PORTS \
>> + *                 CFG_PHY_LESS_PORT(0,1000,FULL)
>> + */
>> +#if !defined(CONFIG_PHY_LESS)
>> +#define CONFIG_PHY_LESS	0xFFFFFFFF /* PHY-less mode */
>> +#endif
>>     
>
> If we agree that this is a good generic approach for this PHY-less handling, 
> then we should probably move this to a common header so that other ethernet 
> driver can use this too.
>
> Ben, what do you think?
>
> And the description should be moved to a common place too. Either the toplevel 
> README, or a new README.xxx in the doc directory.
>
>   
I like the idea very much, but am not sure about the implementation.  
This problem has been around for a while (just search the archives for 
people wondering how to deal with a switch chip connected via rvMII or 
whatever).  The trickiest part of this is how to get the information to 
the driver.  I've always thought that the best way would be for board 
code to initialize each controller through proper C code (i.e. not 
CONFIG macros).  But there's definitely something to be said for doing 
it all through macros, since that's how Kconfig works.  Please have a 
look at the code that Andy Fleming recently submitted for the TSEC 
driver (it's in the main branch now).  He passes a tsec_info_struct into 
each call of tsec_initialize(), allowing all type of custom information 
to go in.  In my mind, that could be generalized to something that more 
than just TSEC, but let's take baby steps.

Incidentally, the term "Fixed PHY" has already been coined for what 
you're calling "PHY-less".  I suggest we standardize.

Anyway, I have to go to bed.  Eyes are starting to close and brain's 
sloowwwiing doowwn.

cheers,
Ben