Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1701223

Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support

Show all headers | View raw

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

>> >> The PHY's EEE settings are already accessed by the DSA layer through the
>> >> Marvell PHY driver and there is nothing to be done for switch's MACs.
>> >
>> > I'm confused, or missing something. Does not patch #1 mean that if the
>> > DSA driver does not have a set_eee function, we always return -ENODEV
>> > in slave.c?
>> 
>> If there is a PHY, phy_init_eee (if eee_enabled is true) and
>> phy_ethtool_set_eee is called. If there is a .set_eee op, it is
>> called. If both are absent, -ENODEV is returned.
>
> O.K, i don't think that is correct. EEE should only be enabled if both
> the MAC and the PHY supports it. We need some way for the MAC to
> indicate it does not support EEE.
>
> If set_eee is optional the meaning of a NULL pointer is that the MAC
> does support EEE. So for mv88e6060, lan9303, microchip and mt7530
> which currently don't support EEE, you need to add a set_eee which
> returns -ENODEV.
>
> Having to implement the op to say you don't implement the feature just
> seems wrong.

Agreed, above I simply described how this patchset currently behaves.

I suggested in the previous mail to define a DSA noop so that the driver
can indicate that its MACs supports EEE, even though there's nothing to
do (and the DSA layer can learn about that):

    static inline int dsa_set_mac_eee_noop(struct dsa_switch *ds,
                                           int port,
                                           struct ethtool_eee *e)
    {
        dev_dbg(ds->dev, "nothing to do for port %d's MAC\n", port);
        return 0;
    }

(and the respective dsa_get_mac_eee_noop() for sure.)

Second option is: we keep it KISS and let the driver define its noop,
but as I explain, it is confusion, especially for the get operation.


Thanks,

        Vivien

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH net-next 00/11] net: dsa: rework EEE support Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 00:30 +0200
  [PATCH net-next 03/11] net: dsa: qca8k: enable EEE once Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 00:30 +0200
  [PATCH net-next 02/11] net: dsa: qca8k: fix EEE init Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 00:30 +0200
  [PATCH net-next 11/11] net: dsa: rename switch EEE ops Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 00:30 +0200
  [PATCH net-next 09/11] net: dsa: remove PHY device argument from .set_eee Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 00:30 +0200
  [PATCH net-next 04/11] net: dsa: qca8k: do not cache unneeded EEE fields Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 00:30 +0200
  [PATCH net-next 01/11] net: dsa: make EEE ops optional Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 00:30 +0200
    Re: [PATCH net-next 01/11] net: dsa: make EEE ops optional Andrew Lunn <andrew@lunn.ch> - 2017-08-01 16:10 +0200
  [PATCH net-next 06/11] net: dsa: bcm_sf2: remove unneeded supported flags Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 00:30 +0200
  [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 00:30 +0200
    Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support Andrew Lunn <andrew@lunn.ch> - 2017-08-01 16:30 +0200
      Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 17:40 +0200
        Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support Andrew Lunn <andrew@lunn.ch> - 2017-08-01 18:10 +0200
          Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 18:40 +0200
            Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2017-08-01 22:20 +0200
          Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support Florian Fainelli <f.fainelli@gmail.com> - 2017-08-01 18:40 +0200
            Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support Andrew Lunn <andrew@lunn.ch> - 2017-08-01 19:30 +0200
              Re: [PATCH net-next 10/11] net: dsa: mv88e6xxx: remove EEE support Florian Fainelli <f.fainelli@gmail.com> - 2017-08-01 21:00 +0200
  Re: [PATCH net-next 00/11] net: dsa: rework EEE support Andrew Lunn <andrew@lunn.ch> - 2017-08-01 16:40 +0200

csiph-web