diff mbox series

[v2,net-next,7/8] net: ethernet: xgbe: expand PHY_GBIT_FEAUTRES

Message ID 1538255056-15114-8-git-send-email-andrew@lunn.ch
State Accepted, archived
Delegated to: David Miller
Headers show
Series Continue towards using linkmode in phylib | expand

Commit Message

Andrew Lunn Sept. 29, 2018, 9:04 p.m. UTC
The macro PHY_GBIT_FEAUTRES needs to change into a bitmap in order to
support link_modes. Remove its use from xgde by replacing it with its
definition.

Probably, the current behavior is wrong. It probably should be
ANDing not assigning.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
v2
Remove unneeded ()
---
 drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Sergei Shtylyov Sept. 30, 2018, 8:41 a.m. UTC | #1
Hello!

On 9/30/2018 12:04 AM, Andrew Lunn wrote:

> The macro PHY_GBIT_FEAUTRES needs to change into a bitmap in order to
> support link_modes. Remove its use from xgde by replacing it with its
> definition.
> 
> Probably, the current behavior is wrong. It probably should be
> ANDing not assigning.

    ORing, maybe?

> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> v2
> Remove unneeded ()

    Really? :-)

> ---
>   drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> index a7e03e3ecc93..151bdb629e8a 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> @@ -878,8 +878,9 @@ static bool xgbe_phy_finisar_phy_quirks(struct xgbe_prv_data *pdata)
>   	phy_write(phy_data->phydev, 0x04, 0x0d01);
>   	phy_write(phy_data->phydev, 0x00, 0x9140);
>   
> -	phy_data->phydev->supported = PHY_GBIT_FEATURES;
> -	phy_data->phydev->advertising = phy_data->phydev->supported;
> +	phy_data->phydev->supported = PHY_10BT_FEATURES |
> +				      PHY_100BT_FEATURES |
> +				      PHY_1000BT_FEATURES;
>   	phy_support_asym_pause(phy_data->phydev);
>   
>   	netif_dbg(pdata, drv, pdata->netdev,
> @@ -950,8 +951,9 @@ static bool xgbe_phy_belfuse_phy_quirks(struct xgbe_prv_data *pdata)
>   	reg = phy_read(phy_data->phydev, 0x00);
>   	phy_write(phy_data->phydev, 0x00, reg & ~0x00800);
>   
> -	phy_data->phydev->supported = PHY_GBIT_FEATURES;
> -	phy_data->phydev->advertising = phy_data->phydev->supported;
> +	phy_data->phydev->supported = (PHY_10BT_FEATURES |
> +				       PHY_100BT_FEATURES |
> +				       PHY_1000BT_FEATURES);

    First time w/o parens and 2nd time with them doesn't look very consistent...

>   	phy_support_asym_pause(phy_data->phydev);
>   
>   	netif_dbg(pdata, drv, pdata->netdev,

MBR, Sergei
Andrew Lunn Oct. 1, 2018, 11:23 p.m. UTC | #2
On Sun, Sep 30, 2018 at 11:41:00AM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 9/30/2018 12:04 AM, Andrew Lunn wrote:
> 
> >The macro PHY_GBIT_FEAUTRES needs to change into a bitmap in order to
> >support link_modes. Remove its use from xgde by replacing it with its
> >definition.
> >
> >Probably, the current behavior is wrong. It probably should be
> >ANDing not assigning.
> 
>    ORing, maybe?

Hi Sergei

It is hard to know what was intended here.

By assigning these speeds, if the PHY does not actually support 1Gbps,
that information is going to be overwritten. So it should really be
ANDing with that the MAC supports. ORing would have the same problem.
This assignment is also clearing out an TP, AUI, BNC bits which might
be set.

Since i don't really know what the intention is here, i'm just going
to leave it alone.

> 
> >Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> >---
> >v2
> >Remove unneeded ()
> 
>    Really? :-)

I did not say all unneeded :-)

I will remove some more.

  Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
index a7e03e3ecc93..151bdb629e8a 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
@@ -878,8 +878,9 @@  static bool xgbe_phy_finisar_phy_quirks(struct xgbe_prv_data *pdata)
 	phy_write(phy_data->phydev, 0x04, 0x0d01);
 	phy_write(phy_data->phydev, 0x00, 0x9140);
 
-	phy_data->phydev->supported = PHY_GBIT_FEATURES;
-	phy_data->phydev->advertising = phy_data->phydev->supported;
+	phy_data->phydev->supported = PHY_10BT_FEATURES |
+				      PHY_100BT_FEATURES |
+				      PHY_1000BT_FEATURES;
 	phy_support_asym_pause(phy_data->phydev);
 
 	netif_dbg(pdata, drv, pdata->netdev,
@@ -950,8 +951,9 @@  static bool xgbe_phy_belfuse_phy_quirks(struct xgbe_prv_data *pdata)
 	reg = phy_read(phy_data->phydev, 0x00);
 	phy_write(phy_data->phydev, 0x00, reg & ~0x00800);
 
-	phy_data->phydev->supported = PHY_GBIT_FEATURES;
-	phy_data->phydev->advertising = phy_data->phydev->supported;
+	phy_data->phydev->supported = (PHY_10BT_FEATURES |
+				       PHY_100BT_FEATURES |
+				       PHY_1000BT_FEATURES);
 	phy_support_asym_pause(phy_data->phydev);
 
 	netif_dbg(pdata, drv, pdata->netdev,