Date:   Tue, 18 Feb 2020 10:42:41 +0000
From:   Ioana Ciornei <ioana.ciornei@....com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
CC:     "David S. Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>
Subject: RE: [CFT 5/8] net: dpaa2-mac: use resolved link config in
 mac_link_up()

> 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@....com>
> 
> This address bounces.  Given what I find in the git history, is the correct person is
> now:
> 
> Ioana Ciornei <ioana.ciornei@....com>
> 
> Please submit a patch updating MAINTAINERS.  Thanks.

Sure thing.  I'll update the MAINTAINERS file and list myself instead of Ioana Radulescu.

> 
> 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@...linux.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

Powered by blists - more mailing lists