On 06/19/2017 05:55 PM, Joe Hershberger wrote:
> 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.
Sigh, well ... if you could split the patch in two, that'd be nice. I
will try to test it as soon as I have a chance. If it's broken, I'll try
to bisect it then.
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
