[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190802172226.307fd56b@bootlin.com>
Date: Fri, 2 Aug 2019 17:22:26 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Matt Pelland <mpelland@...rry.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net,
antoine.tenart@...tlin.com
Subject: Re: [PATCH 2/2] net: mvpp2: support multiple comphy lanes
Hello Matt,
On Thu, 1 Aug 2019 16:45:23 -0400
Matt Pelland <mpelland@...rry.com> wrote:
>mvpp 2.2 supports RXAUI which requires a pair of serdes lanes instead of
>the usual single lane required by other interface modes. This patch
>expands the number of lanes that can be associated to a port so that
>both lanes are correctly configured at the appropriate times when RXAUI
>is in use.
Thanks for the patch, it's nice to see RXAUI support moving forward.
I'll give your patches a test on my setup, although I never really got
something stable, it might simply be due to the particular hardware I
have access to.
I do have some comments below :
>Signed-off-by: Matt Pelland <mpelland@...rry.com>
>---
> drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 7 +-
> .../net/ethernet/marvell/mvpp2/mvpp2_main.c | 66 +++++++++++++------
> 2 files changed, 53 insertions(+), 20 deletions(-)
>
>diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
>index 256e7c796631..9ae2b3d9d0c7 100644
>--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
>+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
>@@ -655,6 +655,11 @@
> #define MVPP2_F_LOOPBACK BIT(0)
> #define MVPP2_F_DT_COMPAT BIT(1)
>
>+/* MVPP22 supports RXAUI which requires two comphy lanes, all other modes
>+ * require one.
>+ */
>+#define MVPP22_MAX_COMPHYS 2
This driver is "supposed" to support XAUI too, at least it appears in
the list of modes we handle. XAUI uses 4 lanes, maybe we could bump the
max number of lanes to 4.
> /* Marvell tag types */
> enum mvpp2_tag_type {
> MVPP2_TAG_TYPE_NONE = 0,
>@@ -935,7 +940,7 @@ struct mvpp2_port {
> phy_interface_t phy_interface;
> struct phylink *phylink;
> struct phylink_config phylink_config;
>- struct phy *comphy;
>+ struct phy *comphys[MVPP22_MAX_COMPHYS];
>
> struct mvpp2_bm_pool *pool_long;
> struct mvpp2_bm_pool *pool_short;
>diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
>index 8b633af4a684..97dfe2e71b03 100644
>--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
>+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
>@@ -1186,17 +1186,40 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
> */
> static int mvpp22_comphy_init(struct mvpp2_port *port)
> {
>- int ret;
>+ int i, ret;
>
>- if (!port->comphy)
>- return 0;
>+ for (i = 0; i < ARRAY_SIZE(port->comphys); i++) {
>+ if (!port->comphys[i])
>+ return 0;
>
>- ret = phy_set_mode_ext(port->comphy, PHY_MODE_ETHERNET,
>- port->phy_interface);
>- if (ret)
>- return ret;
>+ ret = phy_set_mode_ext(port->comphys[i],
>+ PHY_MODE_ETHERNET,
>+ port->phy_interface);
>+ if (ret)
>+ return ret;
>
>- return phy_power_on(port->comphy);
>+ ret = phy_power_on(port->comphys[i]);
>+ if (ret)
>+ return ret;
>+ }
>+
>+ return 0;
>+}
It could be good to add some sanity check, to make sure we do have 2
comphy lanes available when configuring RXAUI mode (and 4 for XAUI).
>+static int mvpp22_comphy_deinit(struct mvpp2_port *port)
>+{
>+ int i, ret;
>+
>+ for (i = 0; i < ARRAY_SIZE(port->comphys); i++) {
>+ if (!port->comphys[i])
>+ return 0;
>+
>+ ret = phy_power_off(port->comphys[i]);
>+ if (ret)
>+ return ret;
>+ }
>+
>+ return 0;
> }
>
> static void mvpp2_port_enable(struct mvpp2_port *port)
>@@ -3375,7 +3398,9 @@ static void mvpp2_stop_dev(struct mvpp2_port *port)
>
> if (port->phylink)
> phylink_stop(port->phylink);
>- phy_power_off(port->comphy);
>+
>+ if (port->priv->hw_version == MVPP22)
>+ mvpp22_comphy_deinit(port);
> }
>
> static int mvpp2_check_ringparam_valid(struct net_device *dev,
>@@ -4947,7 +4972,7 @@ static void mvpp2_mac_config(struct phylink_config *config, unsigned int mode,
> port->phy_interface = state->interface;
>
> /* Reconfigure the serdes lanes */
>- phy_power_off(port->comphy);
>+ mvpp22_comphy_deinit(port);
> mvpp22_mode_reconfigure(port);
> }
>
>@@ -5038,7 +5063,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> struct fwnode_handle *port_fwnode,
> struct mvpp2 *priv)
> {
>- struct phy *comphy = NULL;
> struct mvpp2_port *port;
> struct mvpp2_port_pcpu *port_pcpu;
> struct device_node *port_node = to_of_node(port_fwnode);
>@@ -5085,14 +5109,20 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> goto err_free_netdev;
> }
>
>+ port = netdev_priv(dev);
>+
> if (port_node) {
>- comphy = devm_of_phy_get(&pdev->dev, port_node, NULL);
>- if (IS_ERR(comphy)) {
>- if (PTR_ERR(comphy) == -EPROBE_DEFER) {
>- err = -EPROBE_DEFER;
>- goto err_free_netdev;
>+ for (i = 0; i < ARRAY_SIZE(port->comphys); i++) {
>+ port->comphys[i] = devm_of_phy_get_by_index(&pdev->dev,
>+ port_node,
>+ i);
>+ if (IS_ERR(port->comphys[i])) {
>+ err = PTR_ERR(port->comphys[i]);
>+ port->comphys[i] = NULL;
>+ if (err == -EPROBE_DEFER)
>+ goto err_free_netdev;
>+ err = 0;
> }
>- comphy = NULL;
> }
> }
>
>@@ -5107,7 +5137,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> dev->netdev_ops = &mvpp2_netdev_ops;
> dev->ethtool_ops = &mvpp2_eth_tool_ops;
>
>- port = netdev_priv(dev);
> port->dev = dev;
> port->fwnode = port_fwnode;
> port->has_phy = !!of_find_property(port_node, "phy", NULL);
>@@ -5144,7 +5173,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
>
> port->of_node = port_node;
> port->phy_interface = phy_mode;
>- port->comphy = comphy;
>
> if (priv->hw_version == MVPP21) {
> port->base = devm_platform_ioremap_resource(pdev, 2 + id);
--
Maxime Chevallier, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Powered by blists - more mailing lists