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

Murali Karicheri
2014-03-12T19:04:04Z

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