lkml.org 
[lkml]   [2015]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH net-next v1 2/7] net: phy: extend link mode support to 48 bits
From
Date
On Sun, 2015-01-04 at 12:56 -0800, David Decotigny wrote:
> From: David Decotigny <decot@googlers.com>
>
> Signed-off-by: David Decotigny <decot@googlers.com>
> ---
> drivers/net/phy/phy.c | 29 ++++++++++++++---------------
> drivers/net/phy/phy_device.c | 4 ++--
> include/linux/phy.h | 10 +++++-----
> 3 files changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 767cd11..e9c8499 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
[...]
> @@ -245,7 +246,7 @@ static inline unsigned int phy_find_valid(unsigned int idx, u32 features)
> */
> static void phy_sanitize_settings(struct phy_device *phydev)
> {
> - u32 features = phydev->supported;
> + ethtool_link_mode_mask_t features = phydev->supported;
> unsigned int idx;
>
> /* Sanitize settings based on PHY capabilities */
> @@ -279,13 +280,13 @@ int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd)
> return -EINVAL;
>
> /* We make sure that we don't pass unsupported values in to the PHY */
> - cmd->advertising &= phydev->supported;
> + phydev->advertising = ethtool_cmd_advertising(cmd) & phydev->supported;

phydev->advertising should not be changed until after the following
validation. Use a local variable for this.

> /* Verify the settings we care about. */
> if (cmd->autoneg != AUTONEG_ENABLE && cmd->autoneg != AUTONEG_DISABLE)
> return -EINVAL;
>
> - if (cmd->autoneg == AUTONEG_ENABLE && cmd->advertising == 0)
> + if (cmd->autoneg == AUTONEG_ENABLE && phydev->advertising == 0)
> return -EINVAL;
>
> if (cmd->autoneg == AUTONEG_DISABLE &&
> @@ -300,8 +301,6 @@ int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd)
>
> phydev->speed = speed;
>
> - phydev->advertising = cmd->advertising;
> -
> if (AUTONEG_ENABLE == cmd->autoneg)
> phydev->advertising |= ADVERTISED_Autoneg;
> else
[...]

The assignment to phydev->advertising should probably remain here.

Ben.

--
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2015-01-04 23:21    [W:0.205 / U:2.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site