Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API

[Daniel Santos <daniel.santos@pobox.com>]

On 6/25/19 3:41 PM, Andrew Lunn wrote:
> On Tue, Jun 25, 2019 at 02:27:55PM -0500, Daniel Santos wrote:
>> On 6/25/19 2:02 PM, Andrew Lunn wrote:
>>>> But will there still be a mechanism to ignore link partner's advertising
>>>> and force these parameters?
>>> >From man 1 ethtool:
>>>
>>>        -a --show-pause
>>>               Queries the specified Ethernet device for pause parameter information.
>>>
>>>        -A --pause
>>>               Changes the pause parameters of the specified Ethernet device.
>>>
>>>            autoneg on|off
>>>                   Specifies whether pause autonegotiation should be enabled.
>>>
>>>            rx on|off
>>>                   Specifies whether RX pause should be enabled.
>>>
>>>            tx on|off
>>>                   Specifies whether TX pause should be enabled.
>>>
>>> You need to check the driver to see if it actually implements this
>>> ethtool call, but that is how it should be configured.
>>>
>>> 	Andrew
>>>
>> Thank you Andrew,
>>
>> So in this context, my question is the difference between "enabling" and
>> "forcing".  Here's that register for the mt7620 (which has an mt7530 on
>> its die): https://imgur.com/a/pTk0668  I believe this is also what René
>> is seeking clarity on?
> Lets start with normal operation. If the MAC supports pause or asym
> pause, it calls phy_support_sym_pause() or phy_support_asym_pause().
> phylib will then configure the PHY to advertise pause as appropriate.
> Once auto-neg has completed, the results of the negotiation are set in
> phydev. So phdev->pause and phydev->asym_pause. The MAC callback is
> then used to tell the MAC about the autoneg results. The MAC should be
> programmed using the values in phdev->pause and phydev->asym_pause.
>
> For ethtool, the MAC driver needs to implement .get_pauseparam and
> .set_pauseparam. The set_pauseparam needs to validate the settings,
> using phy_validate_pause(). If valid, phy_set_asym_pause() is used to
> tell the PHY about the new configuration. This will trigger a new
> auto-neg if auto-neg is enabled, and the results will be passed back
> in the usual way. If auto-neg is disabled, or pause auto-neg is
> disabled, the MAC should configure pause directly based on the
> settings passed.
>
> Looking at the data sheet page, you want FORCE_MODE_Pn set. You never
> want the MAC directly talking to the PHY. Bad things will happen.

So what exactly do you mean by the MAC directly talking to the PHY?  Do
you mean setting speed, duplex, etc. via the MAC registers instead of
via MDIO to the MII registers of the PHY?

> Then use FORCE_RX_FC_Pn and FORCE_TX_Pn to reflect phydev->pause and
> phydev->asym_pause.
>
> The same idea applies when using phylink.
>
>     Andrew

You're help is greatly appreciated here.  Admittedly, I'm also trying to
get this working in the now deprecated swconfig for a 3.18 kernel that's
in production.  In my code, I had just set the appropriate bits in both
the MAC and mii registers -- did I just shoot myself in the foot or only
toe or two? :)  I should probably start a separate thread for this. 
(And probably attempt to wrestle an mt7530 programmer's guide out of
MediaTek!)

Thanks,
Daniel

PS: I found a rather humorous quote from the mt7621 datasheet regarding
the MAC registers (at 0x3000 for port 0, 0x3100 for port 1, etc.):

    2.4 Link Status

    You can find MAC control register put at 0x3500 for MAC 5, and
    0x3600 for MAC 6. You can change
    MAC ability at this register. We would suggest don’t use the
    register 0x3000 to 0x3400. It may not
    work.

I'm not sure if this only applies to something in between the mt7621 and
it's internal mt7530 or not.