On 12/13/2018 03:33 PM, Andrew Lunn wrote:
> On Thu, Dec 13, 2018 at 02:27:51PM +0100, Marek Vasut wrote:
>> On 12/13/2018 09:38 AM, Andrew Lunn wrote:
>>>> +static int tja11xx_config_aneg(struct phy_device *phydev)
>>>> +{
>>>> + phydev->autoneg = 0;
>>>> + phydev->speed = SPEED_100;
>>>> + phydev->duplex = DUPLEX_FULL;
>>>> + phydev->pause = 0;
>>>> + phydev->asym_pause = 0;
>>>> +
>>>> + return 0;
>>>> +}
>>>
>>> Hi Marek
>>
>> Hi,
>>
>>> That is, err, interesting. Are you saying the PHY cannot do auto-neg?
>>
>> There isn't much to auto-negotiate and some of the bits in the standard
>> registers behave weirdly.
>
> Hi Marek
Hi,
> Hopefully the config_aneg callback is not called if you don't list
> autoneg to the .features. The microchip_t1 driver just uses
> genphy_config_aneg, but if a NULL works, i would prefer that.
Without the custom config_aneg which sets speed and duplex, I get a
report claiming the link is at 10/Half , while the link is at 100/Full.
If I force this in the custom config_aneg, the communication works fine.
Do you have a hint for me ?
>>> Does it happen to be a 100T1 device for automotive?
>>
>> Yes, that's what the commit message says, why ?
>
> Sorry, i missed it in the commit message. We now have enough bits in
> the link_mode that we should explicitly say this is 100BaseT2_Full,
> not 100BaseT_Full. So please add a new member to
> ethtool_link_mode_bit_indices ...
Oh, nice. Shouldn't this be basic_t1 , which is already supported ?
>>>> +
>>>> +static int tja11xx_config_init(struct phy_device *phydev)
>>>> +{
>>>> + u32 features = SUPPORTED_TP | SUPPORTED_MII | SUPPORTED_100baseT_Full;
>
> And make it part of the .features. Well, you need to add a new
> PHY_BASIC_T2_FEATURES macro, which uses it, and use that in the
> phy_driver structure. There should not be any need to modify it in the
> config_init call.
Right
> We also need to think about what we do with the PHY_BASIC_T1_FEATURES
> macro. Ideally we want to swap that to also make use of a new
> ethtool_link_mode_bit_indices, but i've no idea at the moment if that
> will break something.
Do you mind if I skip this part for now , until I get the driver into
better shape ?
--
Best regards,
Marek Vasut
