# Re: \[PATCH net-next 4/8\] net/funeth: ethtool operations

- From: Dimitris Michailidis \<hidden\>
- Date: 2021-12-31 03:24:06 
- List: netdev
- Page: https://ratatoskr.run/netdev/2021/12/11219780
- Thread: https://ratatoskr.run/netdev/2021/12/11219713/t.md

```text
On Thu, Dec 30, 2021 at 6:24 PM Andrew Lunn [ref] wrote:
>
> On Thu, Dec 30, 2021 at 05:23:56PM -0800, Dimitris Michailidis wrote:
> > On Thu, Dec 30, 2021 at 2:26 PM Andrew Lunn [ref] wrote:
> > >
> > > > > I _think_ this is wrong. pause->autoneg means we are autoneg'ing
> > > > > pause, not that we are using auto-neg in general. The user can have
> > > > > autoneg turned on, but force pause by setting pause->autoneg to False.
> > > > > In that case, the pause->rx_pause and pause->tx_pause are given direct
> > > > > to the MAC, not auto negotiated.
> > > >
> > > > Having this mixed mode needs device FW support, which isn't there today.
> > >
> > > So if you are asked to set pause with pause->autoneg False, return
> > > -EOPNOTSUPP. And pause get should always return True. It is O.K, to
> > > support a subset of a feature, and say you don't support the
> > > rest. That is much better than wrongly implementing it until your
> > > firmware gets the needed support.
> >
> > include/uapi/linux/ethtool.h has this comment
> >
> >  * If the link is autonegotiated, drivers should use
> >  * mii_advertise_flowctrl() or similar code to set the advertised
> >  * pause frame capabilities based on the @rx_pause and @tx_pause flags,
> >  * even if @autoneg is zero. ...
> >
> > I read this as saying that pause->autoneg is ignored if AN is on and the
> > requested pause settings are fed to AN. I believe this is what the code
> > here implements.
> >
> > Whereas you are saying that pause->autoneg == 0 should force
> > despite AN. Right?
>
> Take a look at phylink_ethtool_set_pauseparam() and accompanying
> functions. This is a newish central implementation for any MAC/PHY
> with Linux controlling the hardware. There are ambiguities in the API
> description, so it would be better if your driver/firmware combo does
> the same a the core Linux code. When Russell wrote that code, there
> was quite a bit of discussion what the documentation actually means.

OK. If I understand correctly AN on and pause->autoneg != 0 means
negotiate requested settings and accept resolution result, while
AN on and pause->autoneg == 0 means negotiate settings and adopt
them as the resolution result. I'll mark the latter combination unsupported
for now.

>
>     Andrew
```