diff mbox series

[net-next,4/7] net: dsa: felix: set proper pause frame timers based on link speed

Message ID 20200625152331.3784018-5-olteanv@gmail.com
State Changes Requested
Delegated to: David Miller
Headers show
Series PHYLINK integration improvements for Felix DSA driver | expand

Commit Message

Vladimir Oltean June 25, 2020, 3:23 p.m. UTC
From: Vladimir Oltean <vladimir.oltean@nxp.com>

state->speed holds a value of 10, 100, 1000 or 2500, but
SYS_MAC_FC_CFG_FC_LINK_SPEED expects a value in the range 0, 1, 2 or 3.

So set the correct speed encoding into this register.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/ocelot/felix.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

Comments

Russell King - ARM Linux admin June 25, 2020, 4:37 p.m. UTC | #1
On Thu, Jun 25, 2020 at 06:23:28PM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> state->speed holds a value of 10, 100, 1000 or 2500, but
> SYS_MAC_FC_CFG_FC_LINK_SPEED expects a value in the range 0, 1, 2 or 3.
> 
> So set the correct speed encoding into this register.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/dsa/ocelot/felix.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> index d229cb5d5f9e..da337c63e7ca 100644
> --- a/drivers/net/dsa/ocelot/felix.c
> +++ b/drivers/net/dsa/ocelot/felix.c
> @@ -250,10 +250,25 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
>  			   DEV_CLOCK_CFG_LINK_SPEED(OCELOT_SPEED_1000),
>  			   DEV_CLOCK_CFG);
>  
> -	/* Flow control. Link speed is only used here to evaluate the time
> -	 * specification in incoming pause frames.
> -	 */
> -	mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(state->speed);
> +	switch (state->speed) {
> +	case SPEED_10:
> +		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(3);
> +		break;
> +	case SPEED_100:
> +		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(2);
> +		break;
> +	case SPEED_1000:
> +	case SPEED_2500:
> +		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(1);
> +		break;
> +	case SPEED_UNKNOWN:
> +		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(0);
> +		break;
> +	default:
> +		dev_err(ocelot->dev, "Unsupported speed on port %d: %d\n",
> +			port, state->speed);
> +		return;
> +	}
>  
>  	/* handle Rx pause in all cases, with 2500base-X this is used for rate
>  	 * adaptation.
> @@ -265,6 +280,10 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
>  			      SYS_MAC_FC_CFG_PAUSE_VAL_CFG(0xffff) |
>  			      SYS_MAC_FC_CFG_FC_LATENCY_CFG(0x7) |
>  			      SYS_MAC_FC_CFG_ZERO_PAUSE_ENA;
> +
> +	/* Flow control. Link speed is only used here to evaluate the time
> +	 * specification in incoming pause frames.
> +	 */
>  	ocelot_write_rix(ocelot, mac_fc_cfg, SYS_MAC_FC_CFG, port);

I'm not that happy with the persistence of felix using the legacy
method; I can bring a horse to water but can't make it drink comes
to mind.  I'm at the point where I don't care _if_ my phylink changes
break code that I've asked people to convert and they haven't yet,
and I'm planning to send the series that /may/ cause breakage in
that regard pretty soon, so the lynx PCS changes can move forward.

I even tried to move felix forward by sending a patch for it, and I
just gave up with that approach based on your comment.

Shrug.
Vladimir Oltean June 25, 2020, 4:48 p.m. UTC | #2
On Thu, 25 Jun 2020 at 19:37, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Thu, Jun 25, 2020 at 06:23:28PM +0300, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > state->speed holds a value of 10, 100, 1000 or 2500, but
> > SYS_MAC_FC_CFG_FC_LINK_SPEED expects a value in the range 0, 1, 2 or 3.
> >
> > So set the correct speed encoding into this register.
> >
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> >  drivers/net/dsa/ocelot/felix.c | 27 +++++++++++++++++++++++----
> >  1 file changed, 23 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> > index d229cb5d5f9e..da337c63e7ca 100644
> > --- a/drivers/net/dsa/ocelot/felix.c
> > +++ b/drivers/net/dsa/ocelot/felix.c
> > @@ -250,10 +250,25 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
> >                          DEV_CLOCK_CFG_LINK_SPEED(OCELOT_SPEED_1000),
> >                          DEV_CLOCK_CFG);
> >
> > -     /* Flow control. Link speed is only used here to evaluate the time
> > -      * specification in incoming pause frames.
> > -      */
> > -     mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(state->speed);
> > +     switch (state->speed) {
> > +     case SPEED_10:
> > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(3);
> > +             break;
> > +     case SPEED_100:
> > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(2);
> > +             break;
> > +     case SPEED_1000:
> > +     case SPEED_2500:
> > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(1);
> > +             break;
> > +     case SPEED_UNKNOWN:
> > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(0);
> > +             break;
> > +     default:
> > +             dev_err(ocelot->dev, "Unsupported speed on port %d: %d\n",
> > +                     port, state->speed);
> > +             return;
> > +     }
> >
> >       /* handle Rx pause in all cases, with 2500base-X this is used for rate
> >        * adaptation.
> > @@ -265,6 +280,10 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
> >                             SYS_MAC_FC_CFG_PAUSE_VAL_CFG(0xffff) |
> >                             SYS_MAC_FC_CFG_FC_LATENCY_CFG(0x7) |
> >                             SYS_MAC_FC_CFG_ZERO_PAUSE_ENA;
> > +
> > +     /* Flow control. Link speed is only used here to evaluate the time
> > +      * specification in incoming pause frames.
> > +      */
> >       ocelot_write_rix(ocelot, mac_fc_cfg, SYS_MAC_FC_CFG, port);
>
> I'm not that happy with the persistence of felix using the legacy
> method; I can bring a horse to water but can't make it drink comes
> to mind.  I'm at the point where I don't care _if_ my phylink changes
> break code that I've asked people to convert and they haven't yet,
> and I'm planning to send the series that /may/ cause breakage in
> that regard pretty soon, so the lynx PCS changes can move forward.
>
> I even tried to move felix forward by sending a patch for it, and I
> just gave up with that approach based on your comment.
>
> Shrug.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

The callbacks clearly work the way they are, given the current
structure of phylink, as proven by the users of this driver. Code that
isn't there yet simply isn't there yet.

What you consider "useless churn" and what I consider "useless churn"
are pretty different things.
To me, patch 7/7 is the useless churn, that's why it's at the end.
Patches 1-6 are structured in a way that is compatible with
backporting to a stable 5.4 tree. Patch 7, not so much.

The fact that you have such an attitude and you make the effort to
actually send an email complaining about me using state->speed in
.mac_config(), when literally 3 patches later I'm moving this
configuration where you want it to be, is pretty annoying.
Russell King - ARM Linux admin June 25, 2020, 4:58 p.m. UTC | #3
On Thu, Jun 25, 2020 at 07:48:23PM +0300, Vladimir Oltean wrote:
> On Thu, 25 Jun 2020 at 19:37, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Thu, Jun 25, 2020 at 06:23:28PM +0300, Vladimir Oltean wrote:
> > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > >
> > > state->speed holds a value of 10, 100, 1000 or 2500, but
> > > SYS_MAC_FC_CFG_FC_LINK_SPEED expects a value in the range 0, 1, 2 or 3.
> > >
> > > So set the correct speed encoding into this register.
> > >
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > ---
> > >  drivers/net/dsa/ocelot/felix.c | 27 +++++++++++++++++++++++----
> > >  1 file changed, 23 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> > > index d229cb5d5f9e..da337c63e7ca 100644
> > > --- a/drivers/net/dsa/ocelot/felix.c
> > > +++ b/drivers/net/dsa/ocelot/felix.c
> > > @@ -250,10 +250,25 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
> > >                          DEV_CLOCK_CFG_LINK_SPEED(OCELOT_SPEED_1000),
> > >                          DEV_CLOCK_CFG);
> > >
> > > -     /* Flow control. Link speed is only used here to evaluate the time
> > > -      * specification in incoming pause frames.
> > > -      */
> > > -     mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(state->speed);
> > > +     switch (state->speed) {
> > > +     case SPEED_10:
> > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(3);
> > > +             break;
> > > +     case SPEED_100:
> > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(2);
> > > +             break;
> > > +     case SPEED_1000:
> > > +     case SPEED_2500:
> > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(1);
> > > +             break;
> > > +     case SPEED_UNKNOWN:
> > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(0);
> > > +             break;
> > > +     default:
> > > +             dev_err(ocelot->dev, "Unsupported speed on port %d: %d\n",
> > > +                     port, state->speed);
> > > +             return;
> > > +     }
> > >
> > >       /* handle Rx pause in all cases, with 2500base-X this is used for rate
> > >        * adaptation.
> > > @@ -265,6 +280,10 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
> > >                             SYS_MAC_FC_CFG_PAUSE_VAL_CFG(0xffff) |
> > >                             SYS_MAC_FC_CFG_FC_LATENCY_CFG(0x7) |
> > >                             SYS_MAC_FC_CFG_ZERO_PAUSE_ENA;
> > > +
> > > +     /* Flow control. Link speed is only used here to evaluate the time
> > > +      * specification in incoming pause frames.
> > > +      */
> > >       ocelot_write_rix(ocelot, mac_fc_cfg, SYS_MAC_FC_CFG, port);
> >
> > I'm not that happy with the persistence of felix using the legacy
> > method; I can bring a horse to water but can't make it drink comes
> > to mind.  I'm at the point where I don't care _if_ my phylink changes
> > break code that I've asked people to convert and they haven't yet,
> > and I'm planning to send the series that /may/ cause breakage in
> > that regard pretty soon, so the lynx PCS changes can move forward.
> >
> > I even tried to move felix forward by sending a patch for it, and I
> > just gave up with that approach based on your comment.
> >
> > Shrug.
> >
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> 
> The callbacks clearly work the way they are, given the current
> structure of phylink, as proven by the users of this driver. Code that
> isn't there yet simply isn't there yet.
> 
> What you consider "useless churn" and what I consider "useless churn"
> are pretty different things.
> To me, patch 7/7 is the useless churn, that's why it's at the end.
> Patches 1-6 are structured in a way that is compatible with
> backporting to a stable 5.4 tree. Patch 7, not so much.
> 
> The fact that you have such an attitude and you make the effort to
> actually send an email complaining about me using state->speed in
> .mac_config(), when literally 3 patches later I'm moving this
> configuration where you want it to be, is pretty annoying.

I have asked you to update felix _prior_ to my patch update, because
I forsee the possibility for there to be breakage from pushing the
PCS support further forward.  In other words, I see there to be a
dependency between moving forward with PCS support and drivers that
still use the legacy method.

You don't see that, so you constantly bitch and moan.

Please stop being a problem.
Vladimir Oltean June 25, 2020, 5:06 p.m. UTC | #4
On Thu, 25 Jun 2020 at 19:58, Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Thu, Jun 25, 2020 at 07:48:23PM +0300, Vladimir Oltean wrote:
> > On Thu, 25 Jun 2020 at 19:37, Russell King - ARM Linux admin
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Thu, Jun 25, 2020 at 06:23:28PM +0300, Vladimir Oltean wrote:
> > > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > >
> > > > state->speed holds a value of 10, 100, 1000 or 2500, but
> > > > SYS_MAC_FC_CFG_FC_LINK_SPEED expects a value in the range 0, 1, 2 or 3.
> > > >
> > > > So set the correct speed encoding into this register.
> > > >
> > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > ---
> > > >  drivers/net/dsa/ocelot/felix.c | 27 +++++++++++++++++++++++----
> > > >  1 file changed, 23 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> > > > index d229cb5d5f9e..da337c63e7ca 100644
> > > > --- a/drivers/net/dsa/ocelot/felix.c
> > > > +++ b/drivers/net/dsa/ocelot/felix.c
> > > > @@ -250,10 +250,25 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
> > > >                          DEV_CLOCK_CFG_LINK_SPEED(OCELOT_SPEED_1000),
> > > >                          DEV_CLOCK_CFG);
> > > >
> > > > -     /* Flow control. Link speed is only used here to evaluate the time
> > > > -      * specification in incoming pause frames.
> > > > -      */
> > > > -     mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(state->speed);
> > > > +     switch (state->speed) {
> > > > +     case SPEED_10:
> > > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(3);
> > > > +             break;
> > > > +     case SPEED_100:
> > > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(2);
> > > > +             break;
> > > > +     case SPEED_1000:
> > > > +     case SPEED_2500:
> > > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(1);
> > > > +             break;
> > > > +     case SPEED_UNKNOWN:
> > > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(0);
> > > > +             break;
> > > > +     default:
> > > > +             dev_err(ocelot->dev, "Unsupported speed on port %d: %d\n",
> > > > +                     port, state->speed);
> > > > +             return;
> > > > +     }
> > > >
> > > >       /* handle Rx pause in all cases, with 2500base-X this is used for rate
> > > >        * adaptation.
> > > > @@ -265,6 +280,10 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
> > > >                             SYS_MAC_FC_CFG_PAUSE_VAL_CFG(0xffff) |
> > > >                             SYS_MAC_FC_CFG_FC_LATENCY_CFG(0x7) |
> > > >                             SYS_MAC_FC_CFG_ZERO_PAUSE_ENA;
> > > > +
> > > > +     /* Flow control. Link speed is only used here to evaluate the time
> > > > +      * specification in incoming pause frames.
> > > > +      */
> > > >       ocelot_write_rix(ocelot, mac_fc_cfg, SYS_MAC_FC_CFG, port);
> > >
> > > I'm not that happy with the persistence of felix using the legacy
> > > method; I can bring a horse to water but can't make it drink comes
> > > to mind.  I'm at the point where I don't care _if_ my phylink changes
> > > break code that I've asked people to convert and they haven't yet,
> > > and I'm planning to send the series that /may/ cause breakage in
> > > that regard pretty soon, so the lynx PCS changes can move forward.
> > >
> > > I even tried to move felix forward by sending a patch for it, and I
> > > just gave up with that approach based on your comment.
> > >
> > > Shrug.
> > >
> > > --
> > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> >
> > The callbacks clearly work the way they are, given the current
> > structure of phylink, as proven by the users of this driver. Code that
> > isn't there yet simply isn't there yet.
> >
> > What you consider "useless churn" and what I consider "useless churn"
> > are pretty different things.
> > To me, patch 7/7 is the useless churn, that's why it's at the end.
> > Patches 1-6 are structured in a way that is compatible with
> > backporting to a stable 5.4 tree. Patch 7, not so much.
> >
> > The fact that you have such an attitude and you make the effort to
> > actually send an email complaining about me using state->speed in
> > .mac_config(), when literally 3 patches later I'm moving this
> > configuration where you want it to be, is pretty annoying.
>
> I have asked you to update felix _prior_ to my patch update, because
> I forsee the possibility for there to be breakage from pushing the
> PCS support further forward.  In other words, I see there to be a
> dependency between moving forward with PCS support and drivers that
> still use the legacy method.
>
> You don't see that, so you constantly bitch and moan.
>

And that's what this patch series is, no?
Some cleanup needed to be done, and it needed to be done _before_ the
mac_link_up() conversion. And for things to go quicker, the cleanup
and the conversion are part of the same series.

> Please stop being a problem.
>

Come again, please?
Russell King - ARM Linux admin June 25, 2020, 5:53 p.m. UTC | #5
On Thu, Jun 25, 2020 at 08:06:08PM +0300, Vladimir Oltean wrote:
> On Thu, 25 Jun 2020 at 19:58, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Thu, Jun 25, 2020 at 07:48:23PM +0300, Vladimir Oltean wrote:
> > > On Thu, 25 Jun 2020 at 19:37, Russell King - ARM Linux admin
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Thu, Jun 25, 2020 at 06:23:28PM +0300, Vladimir Oltean wrote:
> > > > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > >
> > > > > state->speed holds a value of 10, 100, 1000 or 2500, but
> > > > > SYS_MAC_FC_CFG_FC_LINK_SPEED expects a value in the range 0, 1, 2 or 3.
> > > > >
> > > > > So set the correct speed encoding into this register.
> > > > >
> > > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > > > ---
> > > > >  drivers/net/dsa/ocelot/felix.c | 27 +++++++++++++++++++++++----
> > > > >  1 file changed, 23 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
> > > > > index d229cb5d5f9e..da337c63e7ca 100644
> > > > > --- a/drivers/net/dsa/ocelot/felix.c
> > > > > +++ b/drivers/net/dsa/ocelot/felix.c
> > > > > @@ -250,10 +250,25 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
> > > > >                          DEV_CLOCK_CFG_LINK_SPEED(OCELOT_SPEED_1000),
> > > > >                          DEV_CLOCK_CFG);
> > > > >
> > > > > -     /* Flow control. Link speed is only used here to evaluate the time
> > > > > -      * specification in incoming pause frames.
> > > > > -      */
> > > > > -     mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(state->speed);
> > > > > +     switch (state->speed) {
> > > > > +     case SPEED_10:
> > > > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(3);
> > > > > +             break;
> > > > > +     case SPEED_100:
> > > > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(2);
> > > > > +             break;
> > > > > +     case SPEED_1000:
> > > > > +     case SPEED_2500:
> > > > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(1);
> > > > > +             break;
> > > > > +     case SPEED_UNKNOWN:
> > > > > +             mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(0);
> > > > > +             break;
> > > > > +     default:
> > > > > +             dev_err(ocelot->dev, "Unsupported speed on port %d: %d\n",
> > > > > +                     port, state->speed);
> > > > > +             return;
> > > > > +     }
> > > > >
> > > > >       /* handle Rx pause in all cases, with 2500base-X this is used for rate
> > > > >        * adaptation.
> > > > > @@ -265,6 +280,10 @@ static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
> > > > >                             SYS_MAC_FC_CFG_PAUSE_VAL_CFG(0xffff) |
> > > > >                             SYS_MAC_FC_CFG_FC_LATENCY_CFG(0x7) |
> > > > >                             SYS_MAC_FC_CFG_ZERO_PAUSE_ENA;
> > > > > +
> > > > > +     /* Flow control. Link speed is only used here to evaluate the time
> > > > > +      * specification in incoming pause frames.
> > > > > +      */
> > > > >       ocelot_write_rix(ocelot, mac_fc_cfg, SYS_MAC_FC_CFG, port);
> > > >
> > > > I'm not that happy with the persistence of felix using the legacy
> > > > method; I can bring a horse to water but can't make it drink comes
> > > > to mind.  I'm at the point where I don't care _if_ my phylink changes
> > > > break code that I've asked people to convert and they haven't yet,
> > > > and I'm planning to send the series that /may/ cause breakage in
> > > > that regard pretty soon, so the lynx PCS changes can move forward.
> > > >
> > > > I even tried to move felix forward by sending a patch for it, and I
> > > > just gave up with that approach based on your comment.
> > > >
> > > > Shrug.
> > > >
> > > > --
> > > > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > > > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> > >
> > > The callbacks clearly work the way they are, given the current
> > > structure of phylink, as proven by the users of this driver. Code that
> > > isn't there yet simply isn't there yet.
> > >
> > > What you consider "useless churn" and what I consider "useless churn"
> > > are pretty different things.
> > > To me, patch 7/7 is the useless churn, that's why it's at the end.
> > > Patches 1-6 are structured in a way that is compatible with
> > > backporting to a stable 5.4 tree. Patch 7, not so much.
> > >
> > > The fact that you have such an attitude and you make the effort to
> > > actually send an email complaining about me using state->speed in
> > > .mac_config(), when literally 3 patches later I'm moving this
> > > configuration where you want it to be, is pretty annoying.
> >
> > I have asked you to update felix _prior_ to my patch update, because
> > I forsee the possibility for there to be breakage from pushing the
> > PCS support further forward.  In other words, I see there to be a
> > dependency between moving forward with PCS support and drivers that
> > still use the legacy method.
> >
> > You don't see that, so you constantly bitch and moan.
> >
> 
> And that's what this patch series is, no?
> Some cleanup needed to be done, and it needed to be done _before_ the
> mac_link_up() conversion. And for things to go quicker, the cleanup
> and the conversion are part of the same series.

Right, which is what I find when I eventually get to patch 7, the patch
that you called above "useless churn."  I'm not going to review patch 7
tonight because of this fiasco.  To pissed off to do a decent job, so
you're just going to have to wait.
diff mbox series

Patch

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index d229cb5d5f9e..da337c63e7ca 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -250,10 +250,25 @@  static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
 			   DEV_CLOCK_CFG_LINK_SPEED(OCELOT_SPEED_1000),
 			   DEV_CLOCK_CFG);
 
-	/* Flow control. Link speed is only used here to evaluate the time
-	 * specification in incoming pause frames.
-	 */
-	mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(state->speed);
+	switch (state->speed) {
+	case SPEED_10:
+		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(3);
+		break;
+	case SPEED_100:
+		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(2);
+		break;
+	case SPEED_1000:
+	case SPEED_2500:
+		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(1);
+		break;
+	case SPEED_UNKNOWN:
+		mac_fc_cfg = SYS_MAC_FC_CFG_FC_LINK_SPEED(0);
+		break;
+	default:
+		dev_err(ocelot->dev, "Unsupported speed on port %d: %d\n",
+			port, state->speed);
+		return;
+	}
 
 	/* handle Rx pause in all cases, with 2500base-X this is used for rate
 	 * adaptation.
@@ -265,6 +280,10 @@  static void felix_phylink_mac_config(struct dsa_switch *ds, int port,
 			      SYS_MAC_FC_CFG_PAUSE_VAL_CFG(0xffff) |
 			      SYS_MAC_FC_CFG_FC_LATENCY_CFG(0x7) |
 			      SYS_MAC_FC_CFG_ZERO_PAUSE_ENA;
+
+	/* Flow control. Link speed is only used here to evaluate the time
+	 * specification in incoming pause frames.
+	 */
 	ocelot_write_rix(ocelot, mac_fc_cfg, SYS_MAC_FC_CFG, port);
 
 	ocelot_write_rix(ocelot, 0, ANA_POL_FLOWC, port);