All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH stable 4.9 v2] net: dsa: bcm_sf2: force pause link settings
@ 2022-07-06 19:24 Florian Fainelli
  2022-07-06 19:24 ` [PATCH stable 4.14 " Florian Fainelli
  2022-07-06 20:31 ` [PATCH stable 4.9 " Vladimir Oltean
  0 siblings, 2 replies; 5+ messages in thread
From: Florian Fainelli @ 2022-07-06 19:24 UTC (permalink / raw)
  To: stable
  Cc: Doug Berger, Florian Fainelli, Jakub Kicinski,
	Greg Kroah-Hartman, Sasha Levin, netdev, olteanv, andrew

From: Doug Berger <opendmb@gmail.com>

commit 7c97bc0128b2eecc703106112679a69d446d1a12 upstream

The pause settings reported by the PHY should also be applied to the
GMII port status override otherwise the switch will not generate pause
frames towards the link partner despite the advertisement saying
otherwise.

Fixes: 246d7f773c13 ("net: dsa: add Broadcom SF2 switch driver")
Signed-off-by: Doug Berger <opendmb@gmail.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/20220623030204.1966851-1-f.fainelli@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
Changes in v2:

- use both local and remote advertisement to determine when to apply
  flow control settings

 drivers/net/dsa/bcm_sf2.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 40b3adf7ad99..562b5eb23d90 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -600,7 +600,9 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	struct ethtool_eee *p = &priv->port_sts[port].eee;
 	u32 id_mode_dis = 0, port_mode;
+	u16 lcl_adv = 0, rmt_adv = 0;
 	const char *str = NULL;
+	u8 flowctrl = 0;
 	u32 reg;
 
 	switch (phydev->interface) {
@@ -667,10 +669,24 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
 		break;
 	}
 
+	if (phydev->pause)
+		rmt_adv = LPA_PAUSE_CAP;
+	if (phydev->asym_pause)
+		rmt_adv |= LPA_PAUSE_ASYM;
+	if (phydev->advertising & ADVERTISED_Pause)
+		lcl_adv = ADVERTISE_PAUSE_CAP;
+	if (phydev->advertising & ADVERTISED_Asym_Pause)
+		lcl_adv |= ADVERTISE_PAUSE_ASYM;
+	flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
+
 	if (phydev->link)
 		reg |= LINK_STS;
 	if (phydev->duplex == DUPLEX_FULL)
 		reg |= DUPLX_MODE;
+	if (flowctrl & FLOW_CTRL_TX)
+		reg |= TXFLOW_CNTL;
+	if (flowctrl & FLOW_CTRL_RX)
+		reg |= RXFLOW_CNTL;
 
 	core_writel(priv, reg, CORE_STS_OVERRIDE_GMIIP_PORT(port));
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH stable 4.14 v2] net: dsa: bcm_sf2: force pause link settings
  2022-07-06 19:24 [PATCH stable 4.9 v2] net: dsa: bcm_sf2: force pause link settings Florian Fainelli
@ 2022-07-06 19:24 ` Florian Fainelli
  2022-07-06 20:31 ` [PATCH stable 4.9 " Vladimir Oltean
  1 sibling, 0 replies; 5+ messages in thread
From: Florian Fainelli @ 2022-07-06 19:24 UTC (permalink / raw)
  To: stable
  Cc: Doug Berger, Florian Fainelli, Jakub Kicinski,
	Greg Kroah-Hartman, Sasha Levin, netdev, olteanv, andrew

From: Doug Berger <opendmb@gmail.com>

commit 7c97bc0128b2eecc703106112679a69d446d1a12 upstream

The pause settings reported by the PHY should also be applied to the
GMII port status override otherwise the switch will not generate pause
frames towards the link partner despite the advertisement saying
otherwise.

Fixes: 246d7f773c13 ("net: dsa: add Broadcom SF2 switch driver")
Signed-off-by: Doug Berger <opendmb@gmail.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/20220623030204.1966851-1-f.fainelli@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
Changes in v2:

- use both local and remote advertisement to determine when to apply
  flow control settings

 drivers/net/dsa/bcm_sf2.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 11a72c4cbb92..42b4f49c9f47 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -625,7 +625,9 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
 	struct ethtool_eee *p = &priv->port_sts[port].eee;
 	u32 id_mode_dis = 0, port_mode;
+	u16 lcl_adv = 0, rmt_adv = 0;
 	const char *str = NULL;
+	u8 flowctrl = 0;
 	u32 reg, offset;
 
 	if (priv->type == BCM7445_DEVICE_ID)
@@ -697,10 +699,24 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
 		break;
 	}
 
+	if (phydev->pause)
+		rmt_adv = LPA_PAUSE_CAP;
+	if (phydev->asym_pause)
+		rmt_adv |= LPA_PAUSE_ASYM;
+	if (phydev->advertising & ADVERTISED_Pause)
+		lcl_adv = ADVERTISE_PAUSE_CAP;
+	if (phydev->advertising & ADVERTISED_Asym_Pause)
+		lcl_adv |= ADVERTISE_PAUSE_ASYM;
+	flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
+
 	if (phydev->link)
 		reg |= LINK_STS;
 	if (phydev->duplex == DUPLEX_FULL)
 		reg |= DUPLX_MODE;
+	if (flowctrl & FLOW_CTRL_TX)
+		reg |= TXFLOW_CNTL;
+	if (flowctrl & FLOW_CTRL_RX)
+		reg |= RXFLOW_CNTL;
 
 	core_writel(priv, reg, offset);
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH stable 4.9 v2] net: dsa: bcm_sf2: force pause link settings
  2022-07-06 19:24 [PATCH stable 4.9 v2] net: dsa: bcm_sf2: force pause link settings Florian Fainelli
  2022-07-06 19:24 ` [PATCH stable 4.14 " Florian Fainelli
@ 2022-07-06 20:31 ` Vladimir Oltean
  2022-07-07 16:53   ` Florian Fainelli
  1 sibling, 1 reply; 5+ messages in thread
From: Vladimir Oltean @ 2022-07-06 20:31 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: stable, Doug Berger, Jakub Kicinski, Greg Kroah-Hartman,
	Sasha Levin, netdev, andrew

Hi Florian,

On Wed, Jul 06, 2022 at 12:24:54PM -0700, Florian Fainelli wrote:
> From: Doug Berger <opendmb@gmail.com>
> 
> commit 7c97bc0128b2eecc703106112679a69d446d1a12 upstream
> 
> The pause settings reported by the PHY should also be applied to the
> GMII port status override otherwise the switch will not generate pause
> frames towards the link partner despite the advertisement saying
> otherwise.
> 
> Fixes: 246d7f773c13 ("net: dsa: add Broadcom SF2 switch driver")
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> Link: https://lore.kernel.org/r/20220623030204.1966851-1-f.fainelli@gmail.com
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> Changes in v2:
> 
> - use both local and remote advertisement to determine when to apply
>   flow control settings
> 
>  drivers/net/dsa/bcm_sf2.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
> index 40b3adf7ad99..562b5eb23d90 100644
> --- a/drivers/net/dsa/bcm_sf2.c
> +++ b/drivers/net/dsa/bcm_sf2.c
> @@ -600,7 +600,9 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
>  	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
>  	struct ethtool_eee *p = &priv->port_sts[port].eee;
>  	u32 id_mode_dis = 0, port_mode;
> +	u16 lcl_adv = 0, rmt_adv = 0;
>  	const char *str = NULL;
> +	u8 flowctrl = 0;
>  	u32 reg;
>  
>  	switch (phydev->interface) {
> @@ -667,10 +669,24 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
>  		break;
>  	}
>  
> +	if (phydev->pause)
> +		rmt_adv = LPA_PAUSE_CAP;
> +	if (phydev->asym_pause)
> +		rmt_adv |= LPA_PAUSE_ASYM;
> +	if (phydev->advertising & ADVERTISED_Pause)
> +		lcl_adv = ADVERTISE_PAUSE_CAP;
> +	if (phydev->advertising & ADVERTISED_Asym_Pause)
> +		lcl_adv |= ADVERTISE_PAUSE_ASYM;
> +	flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);

IEEE 802.3 says "The PAUSE function shall be enabled according to Table
28B–3 only if the highest common denominator is a full duplex technology."

> +
>  	if (phydev->link)
>  		reg |= LINK_STS;
>  	if (phydev->duplex == DUPLEX_FULL)
>  		reg |= DUPLX_MODE;
> +	if (flowctrl & FLOW_CTRL_TX)
> +		reg |= TXFLOW_CNTL;
> +	if (flowctrl & FLOW_CTRL_RX)
> +		reg |= RXFLOW_CNTL;
>  
>  	core_writel(priv, reg, CORE_STS_OVERRIDE_GMIIP_PORT(port));
>  
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH stable 4.9 v2] net: dsa: bcm_sf2: force pause link settings
  2022-07-06 20:31 ` [PATCH stable 4.9 " Vladimir Oltean
@ 2022-07-07 16:53   ` Florian Fainelli
  2022-07-07 22:15     ` Vladimir Oltean
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2022-07-07 16:53 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: stable, Doug Berger, Jakub Kicinski, Greg Kroah-Hartman,
	Sasha Levin, netdev, andrew

On 7/6/22 13:31, Vladimir Oltean wrote:
> Hi Florian,
> 
> On Wed, Jul 06, 2022 at 12:24:54PM -0700, Florian Fainelli wrote:
>> From: Doug Berger <opendmb@gmail.com>
>>
>> commit 7c97bc0128b2eecc703106112679a69d446d1a12 upstream
>>
>> The pause settings reported by the PHY should also be applied to the
>> GMII port status override otherwise the switch will not generate pause
>> frames towards the link partner despite the advertisement saying
>> otherwise.
>>
>> Fixes: 246d7f773c13 ("net: dsa: add Broadcom SF2 switch driver")
>> Signed-off-by: Doug Berger <opendmb@gmail.com>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> Link: https://lore.kernel.org/r/20220623030204.1966851-1-f.fainelli@gmail.com
>> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
>> ---
>> Changes in v2:
>>
>> - use both local and remote advertisement to determine when to apply
>>    flow control settings
>>
>>   drivers/net/dsa/bcm_sf2.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
>> index 40b3adf7ad99..562b5eb23d90 100644
>> --- a/drivers/net/dsa/bcm_sf2.c
>> +++ b/drivers/net/dsa/bcm_sf2.c
>> @@ -600,7 +600,9 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
>>   	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
>>   	struct ethtool_eee *p = &priv->port_sts[port].eee;
>>   	u32 id_mode_dis = 0, port_mode;
>> +	u16 lcl_adv = 0, rmt_adv = 0;
>>   	const char *str = NULL;
>> +	u8 flowctrl = 0;
>>   	u32 reg;
>>   
>>   	switch (phydev->interface) {
>> @@ -667,10 +669,24 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
>>   		break;
>>   	}
>>   
>> +	if (phydev->pause)
>> +		rmt_adv = LPA_PAUSE_CAP;
>> +	if (phydev->asym_pause)
>> +		rmt_adv |= LPA_PAUSE_ASYM;
>> +	if (phydev->advertising & ADVERTISED_Pause)
>> +		lcl_adv = ADVERTISE_PAUSE_CAP;
>> +	if (phydev->advertising & ADVERTISED_Asym_Pause)
>> +		lcl_adv |= ADVERTISE_PAUSE_ASYM;
>> +	flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
> 
> IEEE 802.3 says "The PAUSE function shall be enabled according to Table
> 28B–3 only if the highest common denominator is a full duplex technology."

Indeed, sorry about that, so I suppose the incremental patch would do, 
since we do not support changing pause frame auto-negotiation settings 
in 4.9:

diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 562b5eb23d90..f3d61f2bb0f7 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -669,15 +669,18 @@ static void bcm_sf2_sw_adjust_link(struct 
dsa_switch *ds, int port,
                 break;
         }

-       if (phydev->pause)
-               rmt_adv = LPA_PAUSE_CAP;
-       if (phydev->asym_pause)
-               rmt_adv |= LPA_PAUSE_ASYM;
-       if (phydev->advertising & ADVERTISED_Pause)
-               lcl_adv = ADVERTISE_PAUSE_CAP;
-       if (phydev->advertising & ADVERTISED_Asym_Pause)
-               lcl_adv |= ADVERTISE_PAUSE_ASYM;
-       flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
+       if (phydev->duplex == DUPLEX_FULL &&
+           phydev->autoneg == AUTONEG_ENABLE) {
+               if (phydev->pause)
+                       rmt_adv = LPA_PAUSE_CAP;
+               if (phydev->asym_pause)
+                       rmt_adv |= LPA_PAUSE_ASYM;
+               if (phydev->advertising & ADVERTISED_Pause)
+                       lcl_adv = ADVERTISE_PAUSE_CAP;
+               if (phydev->advertising & ADVERTISED_Asym_Pause)
+                       lcl_adv |= ADVERTISE_PAUSE_ASYM;
+               flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
+       }

         if (phydev->link)
                 reg |= LINK_STS;
-- 
Florian

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH stable 4.9 v2] net: dsa: bcm_sf2: force pause link settings
  2022-07-07 16:53   ` Florian Fainelli
@ 2022-07-07 22:15     ` Vladimir Oltean
  0 siblings, 0 replies; 5+ messages in thread
From: Vladimir Oltean @ 2022-07-07 22:15 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: stable, Doug Berger, Jakub Kicinski, Greg Kroah-Hartman,
	Sasha Levin, netdev, andrew

On Thu, Jul 07, 2022 at 09:53:04AM -0700, Florian Fainelli wrote:
> > IEEE 802.3 says "The PAUSE function shall be enabled according to Table
> > 28B–3 only if the highest common denominator is a full duplex technology."
> 
> Indeed, sorry about that, so I suppose the incremental patch would do, since
> we do not support changing pause frame auto-negotiation settings in 4.9:
> 
> diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
> index 562b5eb23d90..f3d61f2bb0f7 100644
> --- a/drivers/net/dsa/bcm_sf2.c
> +++ b/drivers/net/dsa/bcm_sf2.c
> @@ -669,15 +669,18 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch
> *ds, int port,
>                 break;
>         }
> 
> -       if (phydev->pause)
> -               rmt_adv = LPA_PAUSE_CAP;
> -       if (phydev->asym_pause)
> -               rmt_adv |= LPA_PAUSE_ASYM;
> -       if (phydev->advertising & ADVERTISED_Pause)
> -               lcl_adv = ADVERTISE_PAUSE_CAP;
> -       if (phydev->advertising & ADVERTISED_Asym_Pause)
> -               lcl_adv |= ADVERTISE_PAUSE_ASYM;
> -       flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
> +       if (phydev->duplex == DUPLEX_FULL &&
> +           phydev->autoneg == AUTONEG_ENABLE) {
> +               if (phydev->pause)
> +                       rmt_adv = LPA_PAUSE_CAP;
> +               if (phydev->asym_pause)
> +                       rmt_adv |= LPA_PAUSE_ASYM;
> +               if (phydev->advertising & ADVERTISED_Pause)
> +                       lcl_adv = ADVERTISE_PAUSE_CAP;
> +               if (phydev->advertising & ADVERTISED_Asym_Pause)
> +                       lcl_adv |= ADVERTISE_PAUSE_ASYM;
> +               flowctrl = mii_resolve_flowctrl_fdx(lcl_adv, rmt_adv);
> +       }
> 
>         if (phydev->link)
>                 reg |= LINK_STS;

Yes, this looks OK, thank you.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-07-07 22:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-06 19:24 [PATCH stable 4.9 v2] net: dsa: bcm_sf2: force pause link settings Florian Fainelli
2022-07-06 19:24 ` [PATCH stable 4.14 " Florian Fainelli
2022-07-06 20:31 ` [PATCH stable 4.9 " Vladimir Oltean
2022-07-07 16:53   ` Florian Fainelli
2022-07-07 22:15     ` Vladimir Oltean

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.