Re: [CFT 5/8] net: dpaa2-mac: use resolved link config in mac_link_up()

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Tue, Feb 18, 2020 at 10:42:41AM +0000, Ioana Ciornei wrote:
> > Subject: Re: [CFT 5/8] net: dpaa2-mac: use resolved link config in mac_link_up()
> > 
> > It would really help if MAINTAINERS were updated with the correct information
> > for this driver:
> > 
> > DPAA2 ETHERNET DRIVER
> > M:      Ioana Radulescu <ruxandra.radulescu@nxp.com>
> > 
> > This address bounces.  Given what I find in the git history, is the correct person is
> > now:
> > 
> > Ioana Ciornei <ioana.ciornei@nxp.com>
> > 
> > Please submit a patch updating MAINTAINERS.  Thanks.
> 
> Sure thing.  I'll update the MAINTAINERS file and list myself instead of Ioana Radulescu.

Any comments on the patch itself?

> > On Mon, Feb 17, 2020 at 05:24:16PM +0000, Russell King wrote:
> > > Convert the DPAA2 ethernet driver to use the finalised link parameters
> > > in mac_link_up() rather than the parameters in mac_config(), which are
> > > more suited to the needs of the DPAA2 MC firmware than those available
> > > via mac_config().
> > >
> > > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > > ---
> > >  .../net/ethernet/freescale/dpaa2/dpaa2-mac.c  | 54
> > > +++++++++++--------  .../net/ethernet/freescale/dpaa2/dpaa2-mac.h  |
> > > 1 +
> > >  2 files changed, 33 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > > index 3a75c5b58f95..3ee236c5fc37 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
> > > @@ -123,35 +123,16 @@ static void dpaa2_mac_config(struct phylink_config
> > *config, unsigned int mode,
> > >  	struct dpmac_link_state *dpmac_state = &mac->state;
> > >  	int err;
> > >
> > > -	if (state->speed != SPEED_UNKNOWN)
> > > -		dpmac_state->rate = state->speed;
> > > -
> > > -	if (state->duplex != DUPLEX_UNKNOWN) {
> > > -		if (!state->duplex)
> > > -			dpmac_state->options |=
> > DPMAC_LINK_OPT_HALF_DUPLEX;
> > > -		else
> > > -			dpmac_state->options &=
> > ~DPMAC_LINK_OPT_HALF_DUPLEX;
> > > -	}
> > > -
> > >  	if (state->an_enabled)
> > >  		dpmac_state->options |= DPMAC_LINK_OPT_AUTONEG;
> > >  	else
> > >  		dpmac_state->options &= ~DPMAC_LINK_OPT_AUTONEG;
> > >
> > > -	if (state->pause & MLO_PAUSE_RX)
> > > -		dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> > > -	else
> > > -		dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> > > -
> > > -	if (!!(state->pause & MLO_PAUSE_RX) ^ !!(state->pause &
> > MLO_PAUSE_TX))
> > > -		dpmac_state->options |= DPMAC_LINK_OPT_ASYM_PAUSE;
> > > -	else
> > > -		dpmac_state->options &= ~DPMAC_LINK_OPT_ASYM_PAUSE;
> > > -
> > >  	err = dpmac_set_link_state(mac->mc_io, 0,
> > >  				   mac->mc_dev->mc_handle, dpmac_state);
> > >  	if (err)
> > > -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n",
> > err);
> > > +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =
> > %d\n",
> > > +			   __func__, err);
> > >  }
> > >
> > >  static void dpaa2_mac_link_up(struct phylink_config *config, @@
> > > -165,10 +146,37 @@ static void dpaa2_mac_link_up(struct phylink_config
> > *config,
> > >  	int err;
> > >
> > >  	dpmac_state->up = 1;
> > > +
> > > +	if (mac->if_link_type == DPMAC_LINK_TYPE_PHY) {
> > > +		/* If the DPMAC is configured for PHY mode, we need
> > > +		 * to pass the link parameters to the MC firmware.
> > > +		 */
> > > +		dpmac_state->rate = speed;
> > > +
> > > +		if (duplex == DUPLEX_HALF)
> > > +			dpmac_state->options |=
> > DPMAC_LINK_OPT_HALF_DUPLEX;
> > > +		else if (duplex == DUPLEX_FULL)
> > > +			dpmac_state->options &=
> > ~DPMAC_LINK_OPT_HALF_DUPLEX;
> > > +
> > > +		/* This is lossy; the firmware really should take the pause
> > > +		 * enablement status rather than pause/asym pause status.
> > > +		 */
> > > +		if (rx_pause)
> > > +			dpmac_state->options |= DPMAC_LINK_OPT_PAUSE;
> > > +		else
> > > +			dpmac_state->options &= ~DPMAC_LINK_OPT_PAUSE;
> > > +
> > > +		if (rx_pause ^ tx_pause)
> > > +			dpmac_state->options |=
> > DPMAC_LINK_OPT_ASYM_PAUSE;
> > > +		else
> > > +			dpmac_state->options &=
> > ~DPMAC_LINK_OPT_ASYM_PAUSE;
> > > +	}
> > > +
> > >  	err = dpmac_set_link_state(mac->mc_io, 0,
> > >  				   mac->mc_dev->mc_handle, dpmac_state);
> > >  	if (err)
> > > -		netdev_err(mac->net_dev, "dpmac_set_link_state() = %d\n",
> > err);
> > > +		netdev_err(mac->net_dev, "%s: dpmac_set_link_state() =
> > %d\n",
> > > +			   __func__, err);
> > >  }
> > >
> > >  static void dpaa2_mac_link_down(struct phylink_config *config, @@
> > > -241,6 +249,8 @@ int dpaa2_mac_connect(struct dpaa2_mac *mac)
> > >  		goto err_close_dpmac;
> > >  	}
> > >
> > > +	mac->if_link_type = attr.link_type;
> > > +
> > >  	dpmac_node = dpaa2_mac_get_node(attr.id);
> > >  	if (!dpmac_node) {
> > >  		netdev_err(net_dev, "No dpmac@%d node found.\n", attr.id);
> > diff
> > > --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > > b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > > index 4da8079b9155..2130d9c7d40e 100644
> > > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.h
> > > @@ -20,6 +20,7 @@ struct dpaa2_mac {
> > >  	struct phylink_config phylink_config;
> > >  	struct phylink *phylink;
> > >  	phy_interface_t if_mode;
> > > +	enum dpmac_link_type if_link_type;
> > >  };
> > >
> > >  bool dpaa2_mac_is_type_fixed(struct fsl_mc_device *dpmac_dev,
> > > --
> > > 2.20.1
> > >
> > >
> > 
> > --
> > RMK's Patch system:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ar
> > mlinux.org.uk%2Fdeveloper%2Fpatches%2F&amp;data=02%7C01%7Cioana.cior
> > nei%40nxp.com%7C09d0167191914135433808d7b45e15fd%7C686ea1d3bc2b4
> > c6fa92cd99c5c301635%7C0%7C0%7C637176188497544105&amp;sdata=t0%2B
> > OzkoqRM180UHGBrW6FYAvHsIelx4CaP4oC3QcP1k%3D&amp;reserved=0
> > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> > According to speedtest.net: 11.9Mbps down 500kbps up
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up