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

[Troy Kisky <troy.kisky@boundarydevices.com>]

On 1/31/2012 5:29 PM, Andy Fleming wrote:
> On Mon, Jan 30, 2012 at 8:13 PM, Troy Kisky
> <troy.kisky@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@boundarydevices.com>    wrote:
>>>> Signed-off-by: Troy Kisky<troy.kisky@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
>

I think I followed you that time.

Thanks
Troy