On Sun, 2015-05-31 at 17:19 -0700, David Miller wrote: > From: Ben Hutchings <b...@decadent.org.uk> > Date: Sun, 31 May 2015 20:54:06 +0100 > > > On Fri, 2015-05-22 at 16:15 -0400, David Miller wrote: > >> From: Arun Parameswaran <apara...@broadcom.com> > >> Date: Wed, 20 May 2015 14:35:30 -0700 > >> > >> > When trying to configure the settings for PHY1, using commands > >> > like 'ethtool -s eth0 phyad 1 speed 100', the 'ethtool' seems to > >> > modify other settings apart from the speed of the PHY1, in the > >> > above case. > >> > > >> > The ethtool seems to query the settings for PHY0, and use this > >> > as the base to apply the new settings to the PHY1. This is > >> > causing the other settings of the PHY 1 to be wrongly > >> > configured. > >> > > >> > The issue is caused by the '_ethtool_get_settings()' API, which > >> > gets called because of the 'ETHTOOL_GSET' command, is clearing > >> > the 'cmd' pointer (of type 'struct ethtool_cmd') by calling > >> > memset. This clears all the parameters (if any) passed for the > >> > 'ETHTOOL_GSET' cmd. So the driver's callback is always invoked > >> > with 'cmd->phy_address' as '0'. > >> > > >> > The '_ethtool_get_settings()' is called from other files in the > >> > 'net/core'. So the fix is applied to the 'ethtool_get_settings()' > >> > which is only called in the context of the 'ethtool'. > >> > > >> > Signed-off-by: Arun Parameswaran <apara...@broadcom.com> > >> > Reviewed-by: Ray Jui <r...@broadcom.com> > >> > Reviewed-by: Scott Branden <sbran...@broadcom.com> > >> > >> Applied and queued up for -stable, thanks. > > > > Please revert this. This is an incompatible API change, not a bug fix. > > The established semantics are that 'phyad' is filled in by the driver; > > it is not a parameter to the ETHTOOL_GSET command. > > But then how in the world can the user specify specific PHY ADs for > a device that will respond to more than one?
ETHTOOL_SSET sets the current PHY address and ETHTOOL_GSET gets it.
If multiple PHYs need to be configured for a single link then the driver
should configure them all at the same time rather than making it the
administrator's problem.
What we can't support with the current API are:
- multiple physical links behind a single net device (different
configuration possible for each link)
- multiple PHYs are needed for a single link, and the driver can't
automatically decide which to use (multiple addresses to set)
Ben.
--
Ben Hutchings
Power corrupts. Absolute power is kind of neat.
- John Lehman, Secretary of the US Navy 1981-1987
signature.asc
Description: This is a digitally signed message part
