On Mon, Jun 19, 2017 at 4:37 AM, Marek Vasut <ma...@denx.de> wrote:
> On 06/13/2017 06:28 PM, Joe Hershberger wrote:
>> On Tue, Jun 13, 2017 at 4:24 AM, Marek Vasut <ma...@denx.de> wrote:
>>> On 06/12/2017 10:20 PM, Joe Hershberger wrote:
>>>> Don't wait forever, Pass errors back, etc.
>>>>
>>>> Signed-off-by: Joe Hershberger <joe.hershber...@ni.com>
>>>>
>>>> ---
>>>> This is a pass at improving the code quality.
>>>> This has not been tested in any way.
>>>>
>>>> drivers/net/ag7xxx.c | 63
>>>> +++++++++++++++++++++++++++++++++++++++++-----------
>>>> 1 file changed, 50 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ag7xxx.c b/drivers/net/ag7xxx.c
>>>> index cf60d11..c8352d1 100644
>>>> --- a/drivers/net/ag7xxx.c
>>>> +++ b/drivers/net/ag7xxx.c
>>
>> [...] SNIP
>>
>>>> @@ -723,10 +764,13 @@ static int ag933x_phy_setup_common(struct udevice
>>>> *dev)
>>>> return ret;
>>>>
>>>> /* Read out link status */
>>>> - ret = ag7xxx_mdio_read(priv->bus, phymax, 0, MII_MIPSCR);
>>>> + ret = ag7xxx_mdio_read(priv->bus, phymax, 0,
>>>> AG7XXX_PHY_PSSR);
>>>> if (ret < 0)
>>>> return ret;
>>>>
>>>> + if (!(ret & AG7XXX_PHY_PSSR_LINK_UP))
>>>> + return -ENOLINK;
>>>
>>> Are you sure about this ?
>>
>> It seems reasonable to me, but I don't have the HW to test against as
>> noted above.
>
> CCing Wills . I wouldn't be surprised if the hardware was somehow
> magically screwed up, so I'd prefer to stick with equivalent changes and
> maybe changes of the logic in a separate patch.
OK.
>>>> return 0;
>>>> }
>>>>
>>>> @@ -743,13 +787,6 @@ static int ag933x_phy_setup_common(struct udevice
>>>> *dev)
>>>> return ret;
>>>> }
>>>>
>>>> - for (i = 0; i < phymax; i++) {
>>>> - /* Read out link status */
>>>> - ret = ag7xxx_mdio_read(priv->bus, i, 0, MII_MIPSCR);
>>>> - if (ret < 0)
>>>> - return ret;
>>>> - }
>>>
>>> And this ?
>>
>> This was based on your comment: "Actually, I think this is only for
>> the switch ports, so we don't care about the link status."
>
> Just so I understand it correctly, the code reads link status and does
> nothing with it ?
That's how I read it.
>
> --
> Best regards,
> Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
