[U-Boot] [PATCH 2/4] net: fec_mxc: add PHYLIB support

[Andy Fleming]

On Mon, Jan 30, 2012 at 8:13 PM, Troy Kisky
<troy.kisky at boundarydevices.com> wrote:
> On 1/29/2012 7:04 PM, Andy Fleming wrote:
>>
>> On Thu, Jan 26, 2012 at 4:21 PM, Troy Kisky
>> <troy.kisky at boundarydevices.com>  wrote:
>>>
>>> Signed-off-by: Troy Kisky<troy.kisky at boundarydevices.com>
>>> ---
>>>  drivers/net/fec_mxc.c |   35 ++++++++++++++++++++++++++++++-----
>>>  drivers/net/fec_mxc.h |    1 +
>>>  2 files changed, 31 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
>>> index 3fffe79..4d7a38d 100644
>>> --- a/drivers/net/fec_mxc.c
>>> +++ b/drivers/net/fec_mxc.c
>>> @@ -371,6 +371,20 @@ static int fec_set_hwaddr(struct eth_device *dev)
>>>        return 0;
>>>  }
>>>
>>> +static void fec_eth_phy_config(struct eth_device *dev)
>>> +{
>>> +#ifdef CONFIG_PHYLIB
>>> +       struct fec_priv *fec = (struct fec_priv *)dev->priv;
>>> +       struct phy_device *phydev;
>>> +
>>> +       phydev = phy_connect(miiphy_get_dev_by_name(dev->name),
>>> +                       fec->phy_id, dev, PHY_INTERFACE_MODE_RGMII);
>>> +       fec->phydev = phydev;
>>> +       if (phydev)
>>> +               phy_config(phydev);
>>> +#endif
>>> +}
>>> +
>>>  /**
>>>  * Start the FEC engine
>>>  * @param[in] dev Our device to handle
>>> @@ -427,9 +441,19 @@ static int fec_open(struct eth_device *edev)
>>>        }
>>>  #endif
>>>
>>> -       miiphy_wait_aneg(edev);
>>> -       speed = miiphy_speed(edev->name, fec->phy_id);
>>> -       miiphy_duplex(edev->name, fec->phy_id);
>>> +       if (fec->phydev) {
>>> +               /* Start up the PHY */
>>> +#ifdef CONFIG_PHYLIB
>>> +               phy_startup(fec->phydev);
>>> +#endif
>>
>>
>> The old miiphy code is not truly compatible with the new PHY Lib code.
>> Please implement full support, rather than relying on the
>> compatibility shim. It should be straightforward, just convert all of
>> the miiphy-style calls into the newer mdio/phy calls.
>>
>> Andy
>>
> How can I do this without running the risk of breaking existing boards?
> Surely, there should be an intermediate stage where both are supported
> until all boards are converted. Then, my "shim" can be safely removed.
>
> Or am I entirely missing you point???
> Sorry if I am. Please elaborate on how not to break existing boards which
> I cannot test.

Well, it wouldn't take *too* much work to fix it for all boards (Looks
like none of the boards do board-specific PHY things. All of the
board-specific PHY code is contained, evilly, in the ethernet driver).
However, I'm not going to suggest that you have to port the driver
fully now. But you need to make the two separate. The proper PHY Lib
functions may, at some point, assume that there's proper bus
infrastructure behind the bus transactions, and that might cause the
redirection through the miiphy shim to do weird things. So allow
miiphy to exist, but add the proper mdio initialization code in
parallel. The config options will pick which type is used.

The problem is that the shim was meant to allow all existing code to
continue working while not duplicating too much code. It wasn't meant
to allow new code to use the old mechanisms (miiphy_read/miiphy_write,
etc). The PHY Lib functions ignore how the miiphy code works, and the
miiphy code is unaware of how the PHY Lib code works, so there's
potential for things to break horribly. In practice, it's probably
fine, but I strongly prefer avoiding hybrid solutions (I think this is
the third driver that has tried this, which prompted a patch to mark
the miiphy code as deprecated).

Andy