[U-Boot] [U-Boot PATCH v2 10/12] keystone2: net: add keystone ethernet driver

[Murali Karicheri]

On 2/25/2014 5:11 PM, Rini, Tom wrote:
> On Thu, Feb 20, 2014 at 12:55:12PM -0500, Murali Karicheri wrote:
>
>> From: Vitaly Andrianov <vitalya at ti.com>
>>
>> Ethernet driver configures the CPSW, SGMI and Phy and uses
>> the the Navigator APIs. The driver supports 4 Ethernet ports and
>> can work with only one port at a time.
> First, can we just use things in a "dumb" mode and use the existing CPSW
> driver?  Or are there use cases we need to worry about here with a
> bigger / more robust network driver?

First of all we believe the CPSW driver in u-boot is for  tnetv107x CPSW 
hardware and
keystone CPSW is different and more complex. We need to support 
different modes
such as MAC to PHY, MAC to MAC and MAC to MAC forced. Also we are using 
a SGMII
with up to 8 ports in the newer K2 devices. So we don't think we can 
re-use the CPSW
driver.
>> +++ b/drivers/net/keystone_net.c
> Generic problem, please use phylib.

Vitaly has done some prototyping with phy lib and  found that this adds 
tons of code and
huge time to change the active port.  It doesn't give any advantage of 
using phylib as the
driver make only very few reads from standard generic phy registers.

keystone driver has to support phy-less mode (different modes listed 
above) and using the
phylib just unnecessary complicates the driver.  So

Does phylib based drivers have similar scenario as in keystone? How does 
other platforms
that uses phylib solves the above issues? Mainly delayed tftp and 
handling multiple modes?

We have taken care of all of comments here. So can we go with the 
current version of the
driver and then switch to phylib implementation if the above issues can 
be solved on a
phylib based implementation?

>> +void sgmii_serdes_setup_156p25mhz()
>> +{
>> +	unsigned int cnt;
>> +
>> +	reg_rmw(0x0232a000, 0x00800000, 0xffff0000);
> Please comment on what we're doing here, and why we aren't using a
> struct to describe whatever is at 0x0232a000 ?
>
>> +void sgmii_serdes_shutdown()
>> +{
>> +	reg_rmw(0x0232bfe0, 0, 3 << 29 | 3 << 13);
> Same.

We will add comment here as we are using third party SERDES IP and the 
register
names are not published by the vendor.
> Thanks!
>