On 24.05.2019 21:55, Maxim Uvarov wrote:
> пт, 24 мая 2019 г. в 20:24, Heiner Kallweit <hkallwe...@gmail.com>:
>>
>> On 24.05.2019 12:25, Max Uvarov wrote:
>>> After reset SGMII Autoneg timer is set to 2us (bits 6 and 5 are 01).
>>> That us not enough to finalize autonegatiation on some devices.
>>> Increase this timer duration to maximum supported 16ms.
>>>
>>> Signed-off-by: Max Uvarov <muva...@gmail.com>
>>> ---
>>> drivers/net/phy/dp83867.c | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
>>> index afd31c516cc7..66b0a09ad094 100644
>>> --- a/drivers/net/phy/dp83867.c
>>> +++ b/drivers/net/phy/dp83867.c
>>> @@ -297,6 +297,19 @@ static int dp83867_config_init(struct phy_device
>>> *phydev)
>>> WARN_ONCE(1, "dp83867: err DP83867_10M_SGMII_CFG\n");
>>> return ret;
>>> }
>>> +
>>> + /* After reset SGMII Autoneg timer is set to 2us (bits 6 and 5
>>> + * are 01). That us not enough to finalize autoneg on some
>>> + * devices. Increase this timer duration to maximum 16ms.
>>> + */
>> In the public datasheet the bits are described as reserved. However, based on
>> the value, I suppose it's not a timer value but the timer resolution.
>
> No, it's public:
> http://www.ti.com/lit/ds/symlink/dp83867e.pdf page 72.
>
I just searched for Dp83867 and found this one:
http://www.ti.com/lit/ds/symlink/dp83867cr.pdf
This PHY seems to have the same ID, but the timer bits are at least not
documented.
> SGMII Auto-Negotiation Timer Duration.
>
I see, we talk about SGMII aneg, not PHY aneg. My bad.
>>
>>> + val = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_CFG4);
>>> + val &= ~(BIT(5) | BIT(6));
>>> + ret = phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_CFG4,
>>> val);
>>> + if (ret) {
>>> + WARN_ONCE(1, "dp83867: error config sgmii auto-neg
>>> timer\n");
>>> + return ret;
>>
>> Same comment as for patch 1.
>
> Yes, the same answer. I want to capture hardware error then silently
> return error and then debug it.
If you have hardware with buggy MDIO support, then the problem could occur
in any MDIO access. I think then the WARN should be in the MDIO bus ops.
> WARN is more informative then some random "phy not detected" things.
>
> Max.
>
>>
>>> + }
>>> +
>>> }
>>>
>>> /* Enable Interrupt output INT_OE in CFG3 register */
>>>
>>
>
