[net-next,v2,0/4] net: phy: EEE fixes

  changes v2:
- restore previous ethtool set logic for the case where advertisements
  are not provided by user space.
- use ethtool_convert_legacy_u32_to_link_mode() where possible
- genphy_c45_an_config_eee_aneg(): move adv initialization in to the if
  scope.

Different EEE related fixes.

Oleksij Rempel (4):
  net: phy: c45: use "supported_eee" instead of supported for access
    validation
  net: phy: c45: add genphy_c45_an_config_eee_aneg() function
  net: phy: do not force EEE support
  net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes

 drivers/net/phy/phy-c45.c    | 55 ++++++++++++++++++++++++++++--------
 drivers/net/phy/phy_device.c | 21 +++++++++++++-
 include/linux/phy.h          |  6 ++++
 3 files changed, 69 insertions(+), 13 deletions(-)
  

On Tue, Feb 21, 2023 at 06:03:30AM +0100, Oleksij Rempel wrote:
> changes v2:
> - restore previous ethtool set logic for the case where advertisements
>   are not provided by user space.

I don't think the _kernel_ should be doing this - this introduces a
different behaviour to the kernel. As I already said, setting the
default advertisement in the case of ethtool -s is done by userspace
not by the kernel.

In fact, the kernel explicitly rejects an attempt to have autoneg
enabled with a zero advertising mask:

        linkmode_copy(advertising, cmd->link_modes.advertising);
        linkmode_and(advertising, advertising, phydev->supported);
        if (autoneg == AUTONEG_ENABLE && linkmode_empty(advertising))
                return -EINVAL;

and I think we should have a uniform behaviour with the same API,
rather than different behaviours, as that becomes quite messy.
  

On Tue, Feb 21, 2023 at 09:36:35AM +0000, Russell King (Oracle) wrote:
> On Tue, Feb 21, 2023 at 06:03:30AM +0100, Oleksij Rempel wrote:
> > changes v2:
> > - restore previous ethtool set logic for the case where advertisements
> >   are not provided by user space.
> 
> I don't think the _kernel_ should be doing this - this introduces a
> different behaviour to the kernel. As I already said, setting the
> default advertisement in the case of ethtool -s is done by userspace
> not by the kernel.
> 
> In fact, the kernel explicitly rejects an attempt to have autoneg
> enabled with a zero advertising mask:
> 
>         linkmode_copy(advertising, cmd->link_modes.advertising);
>         linkmode_and(advertising, advertising, phydev->supported);
>         if (autoneg == AUTONEG_ENABLE && linkmode_empty(advertising))
>                 return -EINVAL;
> 
> and I think we should have a uniform behaviour with the same API,
> rather than different behaviours, as that becomes quite messy.

I decided to revert this part to not mix different issue. This logic
existed before my refactoring. So, it is better to handle it as
different patch. Current patch set should address only regressions.
  

On Tue, Feb 21, 2023 at 10:48:09AM +0100, Oleksij Rempel wrote:
> On Tue, Feb 21, 2023 at 09:36:35AM +0000, Russell King (Oracle) wrote:
> > On Tue, Feb 21, 2023 at 06:03:30AM +0100, Oleksij Rempel wrote:
> > > changes v2:
> > > - restore previous ethtool set logic for the case where advertisements
> > >   are not provided by user space.
> > 
> > I don't think the _kernel_ should be doing this - this introduces a
> > different behaviour to the kernel. As I already said, setting the
> > default advertisement in the case of ethtool -s is done by userspace
> > not by the kernel.
> > 
> > In fact, the kernel explicitly rejects an attempt to have autoneg
> > enabled with a zero advertising mask:
> > 
> >         linkmode_copy(advertising, cmd->link_modes.advertising);
> >         linkmode_and(advertising, advertising, phydev->supported);
> >         if (autoneg == AUTONEG_ENABLE && linkmode_empty(advertising))
> >                 return -EINVAL;
> > 
> > and I think we should have a uniform behaviour with the same API,
> > rather than different behaviours, as that becomes quite messy.
> 
> I decided to revert this part to not mix different issue. This logic
> existed before my refactoring. So, it is better to handle it as
> different patch. Current patch set should address only regressions.

Okay, let's keep it like that then. Thanks.
  

On Tue, 2023-02-21 at 06:03 +0100, Oleksij Rempel wrote:
> changes v2:
> - restore previous ethtool set logic for the case where advertisements
>   are not provided by user space.
> - use ethtool_convert_legacy_u32_to_link_mode() where possible
> - genphy_c45_an_config_eee_aneg(): move adv initialization in to the if
>   scope.
> 
> Different EEE related fixes.
> 
> Oleksij Rempel (4):
>   net: phy: c45: use "supported_eee" instead of supported for access
>     validation
>   net: phy: c45: add genphy_c45_an_config_eee_aneg() function
>   net: phy: do not force EEE support
>   net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes
> 
>  drivers/net/phy/phy-c45.c    | 55 ++++++++++++++++++++++++++++--------
>  drivers/net/phy/phy_device.c | 21 +++++++++++++-
>  include/linux/phy.h          |  6 ++++
>  3 files changed, 69 insertions(+), 13 deletions(-)
> 
# Form letter - net-next is closed

The merge window for v6.3 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.

Please repost when net-next reopens after Mar 6th.

RFC patches sent for review only are obviously welcome at any time.
  

On Tue, 2023-02-21 at 12:45 +0100, Paolo Abeni wrote:
> On Tue, 2023-02-21 at 06:03 +0100, Oleksij Rempel wrote:
> > changes v2:
> > - restore previous ethtool set logic for the case where advertisements
> >   are not provided by user space.
> > - use ethtool_convert_legacy_u32_to_link_mode() where possible
> > - genphy_c45_an_config_eee_aneg(): move adv initialization in to the if
> >   scope.
> > 
> > Different EEE related fixes.
> > 
> > Oleksij Rempel (4):
> >   net: phy: c45: use "supported_eee" instead of supported for access
> >     validation
> >   net: phy: c45: add genphy_c45_an_config_eee_aneg() function
> >   net: phy: do not force EEE support
> >   net: phy: c45: genphy_c45_ethtool_set_eee: validate EEE link modes
> > 
> >  drivers/net/phy/phy-c45.c    | 55 ++++++++++++++++++++++++++++--------
> >  drivers/net/phy/phy_device.c | 21 +++++++++++++-
> >  include/linux/phy.h          |  6 ++++
> >  3 files changed, 69 insertions(+), 13 deletions(-)
> > 
> # Form letter - net-next is closed
> 
> The merge window for v6.3 has begun and therefore net-next is closed
> for new drivers, features, code refactoring and optimizations.
> We are currently accepting bug fixes only.
> 
> Please repost when net-next reopens after Mar 6th.
> 
> RFC patches sent for review only are obviously welcome at any time.

It looks like I was a little too hasty here; these are fixes for code
currently only on net-next. As such you can re-post (for -net) after
that Linus's net-next pull and subsequent merge into -net.

Thanks,

Paolo