[RFC v1 net-next 0/7] add support for ocelot external ports

[RFC v1 net-next 0/7] add support for ocelot external ports

[Colin Foster]

This is the start of part 3 of what is hopefully a 3-part series to add
Ethernet switching support to Ocelot chips.

Part 1 of the series (A New Chip) added general support for Ocelot chips
that were controlled externally via SPI.
https://lore.kernel.org/all/20220815005553.1450359-1-colin.foster@in-advantage.com/

Part 2 of the series (The Ethernet Strikes Back) added DSA Ethernet
support for ports 0-3, which are the four copper ports that are internal
to the chip.
https://lore.kernel.org/all/20230127193559.1001051-1-colin.foster@in-advantage.com/

Part 3 will, at a minimum, add support for ports 4-7, which are
configured to use QSGMII to an external phy (Return Of The QSGMII). With
any luck, and some guidance, support for SGMII, SFPs, etc. will also be
part of this series.


This patch series is absolutely an RFC at this point. While all 8 copper
ports on the VSC7512 are currently functional, I recognize there are a
couple empty function callbacks in the last patch that likely need to be
implemented.

Aside from that, there is feedback I'd greatly appreciate. Specifically
patch 6 ("net: dsa: felix: allow external parsing of port nodes") and
whether that is an acceptable way to solve the problem at hand.

Also, with patch 7 ("net: dsa: ocelot_ext: add support for external phys")
my basis was the function mscc_ocelot_init_ports(), but there were several
changes I had to make for DSA / Phylink. Are my implementations of
ocelot_ext_parse_port_node() and ocelot_ext_phylink_create() barking up
the right tree?


For reference, a boot log in case it is useful:

[    3.222208] pinctrl-ocelot ocelot-pinctrl.0.auto: DMA mask not set
[    3.231781] pinctrl-ocelot ocelot-pinctrl.0.auto: driver registered
[    3.241747] pinctrl-microchip-sgpio ocelot-sgpio.1.auto: DMA mask not set
[    3.260366] mscc-miim ocelot-miim0.2.auto: DMA mask not set
[    3.787309] mscc-miim ocelot-miim1.3.auto: DMA mask not set
[    3.822367] mscc,ocelot-serdes ocelot-serdes.4.auto: DMA mask not set
[    3.837637] ocelot-switch ocelot-switch.5.auto: DMA mask not set
[    5.368119] ocelot-switch ocelot-switch.5.auto: PHY [ocelot-miim1.3.auto-mii:04] driver [Microsemi GE VSC8514 SyncE] (irq=POLL)
[    5.668093] ocelot-switch ocelot-switch.5.auto: PHY [ocelot-miim1.3.auto-mii:05] driver [Microsemi GE VSC8514 SyncE] (irq=POLL)
[    5.968342] ocelot-switch ocelot-switch.5.auto: PHY [ocelot-miim1.3.auto-mii:06] driver [Microsemi GE VSC8514 SyncE] (irq=POLL)
[    6.288098] ocelot-switch ocelot-switch.5.auto: PHY [ocelot-miim1.3.auto-mii:07] driver [Microsemi GE VSC8514 SyncE] (irq=POLL)
[    6.364114] ocelot-switch ocelot-switch.5.auto: PHY [ocelot-miim0.2.auto-mii:00] driver [Generic PHY] (irq=POLL)
[    6.375061] ocelot-switch ocelot-switch.5.auto: configuring for phy/internal link mode
[    6.391843] ocelot-switch ocelot-switch.5.auto swp1 (uninitialized): PHY [ocelot-miim0.2.auto-mii:01] driver [Generic PHY] (irq=POLL)
[    6.410033] ocelot-switch ocelot-switch.5.auto swp2 (uninitialized): PHY [ocelot-miim0.2.auto-mii:02] driver [Generic PHY] (irq=POLL)
[    6.427761] ocelot-switch ocelot-switch.5.auto swp3 (uninitialized): PHY [ocelot-miim0.2.auto-mii:03] driver [Generic PHY] (irq=POLL)
[    6.598305] ocelot-switch ocelot-switch.5.auto swp4 (uninitialized): PHY [ocelot-miim1.3.auto-mii:04] driver [Microsemi GE VSC8514 SyncE] (irq=POLL)
[    6.778331] ocelot-switch ocelot-switch.5.auto swp5 (uninitialized): PHY [ocelot-miim1.3.auto-mii:05] driver [Microsemi GE VSC8514 SyncE] (irq=POLL)
[    6.958337] ocelot-switch ocelot-switch.5.auto swp6 (uninitialized): PHY [ocelot-miim1.3.auto-mii:06] driver [Microsemi GE VSC8514 SyncE] (irq=POLL)
[    7.138379] ocelot-switch ocelot-switch.5.auto swp7 (uninitialized): PHY [ocelot-miim1.3.auto-mii:07] driver [Microsemi GE VSC8514 SyncE] (irq=POLL)
[    7.156375] device eth0 entered promiscuous mode
[    7.161140] DSA: tree 0 setup


And a couple device tree snippets of the ports:

&spi0 {
        #address-cells = <1>;
        #size-cells = <0>;
        status = "okay";

        soc@0 {
                compatible = "mscc,vsc7512";
                spi-max-frequency = <2500000>;
                reg = <0 0>;
                #address-cells = <1>;
                #size-cells = <1>;

                ethernet-switch@71010000 {
                        compatible = "mscc,vsc7512-switch";
                        ...

                        ports {
                                ...
                                port@4 {
                                       reg = <4>;
                                       label = "swp4";
                                       status = "okay";
                                       phy-handle = <&sw_phy4>;
                                       phy-mode = "qsgmii";
                                       phys = <&serdes 4 SERDES6G(0)>;
                               };
                               ...
                        };
                        ...
                };

                mdio@710700c0 {
                        compatible = "mscc,ocelot-miim";
                        pinctrl-names = "default";
                        pinctrl-0 = <&miim1>;
                        #address-cells = <1>;
                        #size-cells = <0>;
                        reg = <0x710700c0 0x24>;

                        sw_phy4: ethernet-phy@4 {
                                reg = <0x4>;
                        };
                        ...
                };
                ...

                serdes: serdes {
                        compatible = "mscc,vsc7514-serdes";
                        #phy-cells = <2>;
                };
        };
};



Colin Foster (7):
  phy: phy-ocelot-serdes: add ability to be used in a non-syscon
    configuration
  mfd: ocelot: add ocelot-serdes capability
  net: mscc: ocelot: expose ocelot_pll5_init routine
  net: mscc: ocelot: expose generic phylink_mac_config routine
  net: dsa: felix: attempt to initialize internal hsio plls
  net: dsa: felix: allow external parsing of port nodes
  net: dsa: ocelot_ext: add support for external phys

 drivers/mfd/ocelot-core.c                  |  13 +
 drivers/net/dsa/ocelot/felix.c             |  57 +++-
 drivers/net/dsa/ocelot/felix.h             |   6 +
 drivers/net/dsa/ocelot/ocelot_ext.c        | 319 ++++++++++++++++++++-
 drivers/net/ethernet/mscc/ocelot.c         |  57 ++++
 drivers/net/ethernet/mscc/ocelot_net.c     |  21 +-
 drivers/net/ethernet/mscc/ocelot_vsc7514.c |  30 --
 drivers/phy/mscc/phy-ocelot-serdes.c       |   9 +
 include/soc/mscc/ocelot.h                  |   5 +
 9 files changed, 440 insertions(+), 77 deletions(-)

-- 
2.25.1

[RFC v1 net-next 1/7] phy: phy-ocelot-serdes: add ability to be used in a non-syscon configuration

[Colin Foster]

The phy-ocelot-serdes module has exclusively been used in a syscon setup,
from an internal CPU. The addition of external control of ocelot switches
via an existing MFD implementation means that syscon is no longer the only
interface that phy-ocelot-serdes will see.

In the MFD configuration, an IORESOURCE_REG resource will exist for the
device. Utilize this resource to be able to function in both syscon and
non-syscon configurations.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/phy/mscc/phy-ocelot-serdes.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/phy/mscc/phy-ocelot-serdes.c b/drivers/phy/mscc/phy-ocelot-serdes.c
index 76f596365176..d9443e865a78 100644
--- a/drivers/phy/mscc/phy-ocelot-serdes.c
+++ b/drivers/phy/mscc/phy-ocelot-serdes.c
@@ -494,6 +494,7 @@ static int serdes_probe(struct platform_device *pdev)
 {
 	struct phy_provider *provider;
 	struct serdes_ctrl *ctrl;
+	struct resource *res;
 	unsigned int i;
 	int ret;
 
@@ -503,6 +504,14 @@ static int serdes_probe(struct platform_device *pdev)
 
 	ctrl->dev = &pdev->dev;
 	ctrl->regs = syscon_node_to_regmap(pdev->dev.parent->of_node);
+	if (IS_ERR(ctrl->regs)) {
+		/* Fall back to using IORESOURCE_REG, if possible */
+		res = platform_get_resource(pdev, IORESOURCE_REG, 0);
+		if (res)
+			ctrl->regs = dev_get_regmap(ctrl->dev->parent,
+						    res->name);
+	}
+
 	if (IS_ERR(ctrl->regs))
 		return PTR_ERR(ctrl->regs);
 
-- 
2.25.1

[RFC v1 net-next 2/7] mfd: ocelot: add ocelot-serdes capability

[Colin Foster]

Add support for the Ocelot SERDES module to support functionality of all
non-internal phy ports.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/mfd/ocelot-core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c
index b0ff05c1759f..c2224f8a16c0 100644
--- a/drivers/mfd/ocelot-core.c
+++ b/drivers/mfd/ocelot-core.c
@@ -45,6 +45,9 @@
 #define VSC7512_SIO_CTRL_RES_START	0x710700f8
 #define VSC7512_SIO_CTRL_RES_SIZE	0x00000100
 
+#define VSC7512_HSIO_RES_START		0x710d0000
+#define VSC7512_HSIO_RES_SIZE		0x00000128
+
 #define VSC7512_ANA_RES_START		0x71880000
 #define VSC7512_ANA_RES_SIZE		0x00010000
 
@@ -129,8 +132,13 @@ static const struct resource vsc7512_sgpio_resources[] = {
 	DEFINE_RES_REG_NAMED(VSC7512_SIO_CTRL_RES_START, VSC7512_SIO_CTRL_RES_SIZE, "gcb_sio"),
 };
 
+static const struct resource vsc7512_serdes_resources[] = {
+	DEFINE_RES_REG_NAMED(VSC7512_HSIO_RES_START, VSC7512_HSIO_RES_SIZE, "hsio"),
+};
+
 static const struct resource vsc7512_switch_resources[] = {
 	DEFINE_RES_REG_NAMED(VSC7512_ANA_RES_START, VSC7512_ANA_RES_SIZE, "ana"),
+	DEFINE_RES_REG_NAMED(VSC7512_HSIO_RES_START, VSC7512_HSIO_RES_SIZE, "hsio"),
 	DEFINE_RES_REG_NAMED(VSC7512_QS_RES_START, VSC7512_QS_RES_SIZE, "qs"),
 	DEFINE_RES_REG_NAMED(VSC7512_QSYS_RES_START, VSC7512_QSYS_RES_SIZE, "qsys"),
 	DEFINE_RES_REG_NAMED(VSC7512_REW_RES_START, VSC7512_REW_RES_SIZE, "rew"),
@@ -176,6 +184,11 @@ static const struct mfd_cell vsc7512_devs[] = {
 		.use_of_reg = true,
 		.num_resources = ARRAY_SIZE(vsc7512_miim1_resources),
 		.resources = vsc7512_miim1_resources,
+	}, {
+		.name = "ocelot-serdes",
+		.of_compatible = "mscc,vsc7514-serdes",
+		.num_resources = ARRAY_SIZE(vsc7512_serdes_resources),
+		.resources = vsc7512_serdes_resources,
 	}, {
 		.name = "ocelot-switch",
 		.of_compatible = "mscc,vsc7512-switch",
-- 
2.25.1

Re: [RFC v1 net-next 2/7] mfd: ocelot: add ocelot-serdes capability

[Lee Jones]

On Wed, 15 Feb 2023, Colin Foster wrote:

> Add support for the Ocelot SERDES module to support functionality of all
> non-internal phy ports.

Looks non-controversial.

Please provide some explanation of what SERDES means / is.
 
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> ---
>  drivers/mfd/ocelot-core.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

I'd expect this to go in via MFD once it comes out of RFC.

> diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c
> index b0ff05c1759f..c2224f8a16c0 100644
> --- a/drivers/mfd/ocelot-core.c
> +++ b/drivers/mfd/ocelot-core.c
> @@ -45,6 +45,9 @@
>  #define VSC7512_SIO_CTRL_RES_START	0x710700f8
>  #define VSC7512_SIO_CTRL_RES_SIZE	0x00000100
>  
> +#define VSC7512_HSIO_RES_START		0x710d0000
> +#define VSC7512_HSIO_RES_SIZE		0x00000128
> +
>  #define VSC7512_ANA_RES_START		0x71880000
>  #define VSC7512_ANA_RES_SIZE		0x00010000
>  
> @@ -129,8 +132,13 @@ static const struct resource vsc7512_sgpio_resources[] = {
>  	DEFINE_RES_REG_NAMED(VSC7512_SIO_CTRL_RES_START, VSC7512_SIO_CTRL_RES_SIZE, "gcb_sio"),
>  };
>  
> +static const struct resource vsc7512_serdes_resources[] = {
> +	DEFINE_RES_REG_NAMED(VSC7512_HSIO_RES_START, VSC7512_HSIO_RES_SIZE, "hsio"),
> +};
> +
>  static const struct resource vsc7512_switch_resources[] = {
>  	DEFINE_RES_REG_NAMED(VSC7512_ANA_RES_START, VSC7512_ANA_RES_SIZE, "ana"),
> +	DEFINE_RES_REG_NAMED(VSC7512_HSIO_RES_START, VSC7512_HSIO_RES_SIZE, "hsio"),
>  	DEFINE_RES_REG_NAMED(VSC7512_QS_RES_START, VSC7512_QS_RES_SIZE, "qs"),
>  	DEFINE_RES_REG_NAMED(VSC7512_QSYS_RES_START, VSC7512_QSYS_RES_SIZE, "qsys"),
>  	DEFINE_RES_REG_NAMED(VSC7512_REW_RES_START, VSC7512_REW_RES_SIZE, "rew"),
> @@ -176,6 +184,11 @@ static const struct mfd_cell vsc7512_devs[] = {
>  		.use_of_reg = true,
>  		.num_resources = ARRAY_SIZE(vsc7512_miim1_resources),
>  		.resources = vsc7512_miim1_resources,
> +	}, {
> +		.name = "ocelot-serdes",
> +		.of_compatible = "mscc,vsc7514-serdes",
> +		.num_resources = ARRAY_SIZE(vsc7512_serdes_resources),
> +		.resources = vsc7512_serdes_resources,
>  	}, {
>  		.name = "ocelot-switch",
>  		.of_compatible = "mscc,vsc7512-switch",
> -- 
> 2.25.1
> 

-- 
Lee Jones [李琼斯]

Re: [RFC v1 net-next 2/7] mfd: ocelot: add ocelot-serdes capability

[Colin Foster]

On Fri, Mar 03, 2023 at 10:48:07AM +0000, Lee Jones wrote:
> On Wed, 15 Feb 2023, Colin Foster wrote:
> 
> > Add support for the Ocelot SERDES module to support functionality of all
> > non-internal phy ports.
> 
> Looks non-controversial.
> 
> Please provide some explanation of what SERDES means / is.

Will do.

>  
> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> > ---
> >  drivers/mfd/ocelot-core.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> 
> I'd expect this to go in via MFD once it comes out of RFC.

Understood. I'll be sure to make it clear that some sync will be needed
between MFD and net-next in the cover letter this time.

Thanks Lee!

Re: [RFC v1 net-next 2/7] mfd: ocelot: add ocelot-serdes capability

[Lee Jones]

On Fri, 03 Mar 2023, Colin Foster wrote:

> On Fri, Mar 03, 2023 at 10:48:07AM +0000, Lee Jones wrote:
> > On Wed, 15 Feb 2023, Colin Foster wrote:
> > 
> > > Add support for the Ocelot SERDES module to support functionality of all
> > > non-internal phy ports.
> > 
> > Looks non-controversial.
> > 
> > Please provide some explanation of what SERDES means / is.
> 
> Will do.
> 
> >  
> > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> > > ---
> > >  drivers/mfd/ocelot-core.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > 
> > I'd expect this to go in via MFD once it comes out of RFC.
> 
> Understood. I'll be sure to make it clear that some sync will be needed
> between MFD and net-next in the cover letter this time.

To be honest, I don't think that's required this time.

This patch looks orthogonal, unless there's something I'm missing.
 
> Thanks Lee!

NP

-- 
Lee Jones [李琼斯]

[RFC v1 net-next 3/7] net: mscc: ocelot: expose ocelot_pll5_init routine

[Colin Foster]

Ocelot chips have an internal PLL that must be used when communicating
through external phys. Expose the init routine, so it can be used by other
drivers.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/net/ethernet/mscc/ocelot.c         | 31 ++++++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot_vsc7514.c | 30 ---------------------
 include/soc/mscc/ocelot.h                  |  2 ++
 3 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 08acb7b89086..9b8403e29445 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -7,6 +7,7 @@
 #include <linux/dsa/ocelot.h>
 #include <linux/if_bridge.h>
 #include <linux/iopoll.h>
+#include <soc/mscc/ocelot_hsio.h>
 #include <soc/mscc/ocelot_vcap.h>
 #include "ocelot.h"
 #include "ocelot_vcap.h"
@@ -211,6 +212,36 @@ static void ocelot_mact_init(struct ocelot *ocelot)
 	ocelot_write(ocelot, MACACCESS_CMD_INIT, ANA_TABLES_MACACCESS);
 }
 
+void ocelot_pll5_init(struct ocelot *ocelot)
+{
+	/* Configure PLL5. This will need a proper CCF driver
+	 * The values are coming from the VTSS API for Ocelot
+	 */
+	regmap_write(ocelot->targets[HSIO], HSIO_PLL5G_CFG4,
+		     HSIO_PLL5G_CFG4_IB_CTRL(0x7600) |
+		     HSIO_PLL5G_CFG4_IB_BIAS_CTRL(0x8));
+	regmap_write(ocelot->targets[HSIO], HSIO_PLL5G_CFG0,
+		     HSIO_PLL5G_CFG0_CORE_CLK_DIV(0x11) |
+		     HSIO_PLL5G_CFG0_CPU_CLK_DIV(2) |
+		     HSIO_PLL5G_CFG0_ENA_BIAS |
+		     HSIO_PLL5G_CFG0_ENA_VCO_BUF |
+		     HSIO_PLL5G_CFG0_ENA_CP1 |
+		     HSIO_PLL5G_CFG0_SELCPI(2) |
+		     HSIO_PLL5G_CFG0_LOOP_BW_RES(0xe) |
+		     HSIO_PLL5G_CFG0_SELBGV820(4) |
+		     HSIO_PLL5G_CFG0_DIV4 |
+		     HSIO_PLL5G_CFG0_ENA_CLKTREE |
+		     HSIO_PLL5G_CFG0_ENA_LANE);
+	regmap_write(ocelot->targets[HSIO], HSIO_PLL5G_CFG2,
+		     HSIO_PLL5G_CFG2_EN_RESET_FRQ_DET |
+		     HSIO_PLL5G_CFG2_EN_RESET_OVERRUN |
+		     HSIO_PLL5G_CFG2_GAIN_TEST(0x8) |
+		     HSIO_PLL5G_CFG2_ENA_AMPCTRL |
+		     HSIO_PLL5G_CFG2_PWD_AMPCTRL_N |
+		     HSIO_PLL5G_CFG2_AMPC_SEL(0x10));
+}
+EXPORT_SYMBOL(ocelot_pll5_init);
+
 static void ocelot_vcap_enable(struct ocelot *ocelot, int port)
 {
 	ocelot_write_gix(ocelot, ANA_PORT_VCAP_S2_CFG_S2_ENA |
diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
index 7388c3b0535c..97e90e2869d4 100644
--- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
+++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
@@ -18,7 +18,6 @@
 
 #include <soc/mscc/ocelot.h>
 #include <soc/mscc/ocelot_vcap.h>
-#include <soc/mscc/ocelot_hsio.h>
 #include <soc/mscc/vsc7514_regs.h>
 #include "ocelot_fdma.h"
 #include "ocelot.h"
@@ -26,35 +25,6 @@
 #define VSC7514_VCAP_POLICER_BASE			128
 #define VSC7514_VCAP_POLICER_MAX			191
 
-static void ocelot_pll5_init(struct ocelot *ocelot)
-{
-	/* Configure PLL5. This will need a proper CCF driver
-	 * The values are coming from the VTSS API for Ocelot
-	 */
-	regmap_write(ocelot->targets[HSIO], HSIO_PLL5G_CFG4,
-		     HSIO_PLL5G_CFG4_IB_CTRL(0x7600) |
-		     HSIO_PLL5G_CFG4_IB_BIAS_CTRL(0x8));
-	regmap_write(ocelot->targets[HSIO], HSIO_PLL5G_CFG0,
-		     HSIO_PLL5G_CFG0_CORE_CLK_DIV(0x11) |
-		     HSIO_PLL5G_CFG0_CPU_CLK_DIV(2) |
-		     HSIO_PLL5G_CFG0_ENA_BIAS |
-		     HSIO_PLL5G_CFG0_ENA_VCO_BUF |
-		     HSIO_PLL5G_CFG0_ENA_CP1 |
-		     HSIO_PLL5G_CFG0_SELCPI(2) |
-		     HSIO_PLL5G_CFG0_LOOP_BW_RES(0xe) |
-		     HSIO_PLL5G_CFG0_SELBGV820(4) |
-		     HSIO_PLL5G_CFG0_DIV4 |
-		     HSIO_PLL5G_CFG0_ENA_CLKTREE |
-		     HSIO_PLL5G_CFG0_ENA_LANE);
-	regmap_write(ocelot->targets[HSIO], HSIO_PLL5G_CFG2,
-		     HSIO_PLL5G_CFG2_EN_RESET_FRQ_DET |
-		     HSIO_PLL5G_CFG2_EN_RESET_OVERRUN |
-		     HSIO_PLL5G_CFG2_GAIN_TEST(0x8) |
-		     HSIO_PLL5G_CFG2_ENA_AMPCTRL |
-		     HSIO_PLL5G_CFG2_PWD_AMPCTRL_N |
-		     HSIO_PLL5G_CFG2_AMPC_SEL(0x10));
-}
-
 static int ocelot_chip_init(struct ocelot *ocelot, const struct ocelot_ops *ops)
 {
 	int ret;
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 2080879e4134..751d9b250615 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -1183,4 +1183,6 @@ ocelot_mrp_del_ring_role(struct ocelot *ocelot, int port,
 }
 #endif
 
+void ocelot_pll5_init(struct ocelot *ocelot);
+
 #endif
-- 
2.25.1

[RFC v1 net-next 4/7] net: mscc: ocelot: expose generic phylink_mac_config routine

[Colin Foster]

The ocelot-switch driver can utilize the phylink_mac_config routine. Move
this to the ocelot library location and export the symbol to make this
possible.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/net/ethernet/mscc/ocelot.c     | 26 ++++++++++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot_net.c | 21 +++------------------
 include/soc/mscc/ocelot.h              |  3 +++
 3 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 9b8403e29445..8292e93a3782 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -809,6 +809,32 @@ static int ocelot_port_flush(struct ocelot *ocelot, int port)
 	return err;
 }
 
+void ocelot_phylink_mac_config(struct ocelot *ocelot, int port,
+			       unsigned int link_an_mode,
+			       const struct phylink_link_state *state)
+{
+	struct ocelot_port *ocelot_port = ocelot->ports[port];
+
+	/* Disable HDX fast control */
+	ocelot_port_writel(ocelot_port, DEV_PORT_MISC_HDX_FAST_DIS,
+			   DEV_PORT_MISC);
+
+	/* SGMII only for now */
+	ocelot_port_writel(ocelot_port, PCS1G_MODE_CFG_SGMII_MODE_ENA,
+			   PCS1G_MODE_CFG);
+	ocelot_port_writel(ocelot_port, PCS1G_SD_CFG_SD_SEL, PCS1G_SD_CFG);
+
+	/* Enable PCS */
+	ocelot_port_writel(ocelot_port, PCS1G_CFG_PCS_ENA, PCS1G_CFG);
+
+	/* No aneg on SGMII */
+	ocelot_port_writel(ocelot_port, 0, PCS1G_ANEG_CFG);
+
+	/* No loopback */
+	ocelot_port_writel(ocelot_port, 0, PCS1G_LB_CFG);
+}
+EXPORT_SYMBOL_GPL(ocelot_phylink_mac_config);
+
 void ocelot_phylink_mac_link_down(struct ocelot *ocelot, int port,
 				  unsigned int link_an_mode,
 				  phy_interface_t interface,
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index ca4bde861397..590a2b2816ad 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -1675,25 +1675,10 @@ static void vsc7514_phylink_mac_config(struct phylink_config *config,
 {
 	struct net_device *ndev = to_net_dev(config->dev);
 	struct ocelot_port_private *priv = netdev_priv(ndev);
-	struct ocelot_port *ocelot_port = &priv->port;
-
-	/* Disable HDX fast control */
-	ocelot_port_writel(ocelot_port, DEV_PORT_MISC_HDX_FAST_DIS,
-			   DEV_PORT_MISC);
-
-	/* SGMII only for now */
-	ocelot_port_writel(ocelot_port, PCS1G_MODE_CFG_SGMII_MODE_ENA,
-			   PCS1G_MODE_CFG);
-	ocelot_port_writel(ocelot_port, PCS1G_SD_CFG_SD_SEL, PCS1G_SD_CFG);
-
-	/* Enable PCS */
-	ocelot_port_writel(ocelot_port, PCS1G_CFG_PCS_ENA, PCS1G_CFG);
-
-	/* No aneg on SGMII */
-	ocelot_port_writel(ocelot_port, 0, PCS1G_ANEG_CFG);
+	struct ocelot *ocelot = priv->port.ocelot;
+	int port = priv->port.index;
 
-	/* No loopback */
-	ocelot_port_writel(ocelot_port, 0, PCS1G_LB_CFG);
+	ocelot_phylink_mac_config(ocelot, port, link_an_mode, state);
 }
 
 static void vsc7514_phylink_mac_link_down(struct phylink_config *config,
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 751d9b250615..87ade87d3540 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -1111,6 +1111,9 @@ int ocelot_sb_occ_tc_port_bind_get(struct ocelot *ocelot, int port,
 				   enum devlink_sb_pool_type pool_type,
 				   u32 *p_cur, u32 *p_max);
 
+void ocelot_phylink_mac_config(struct ocelot *ocelot, int port,
+			       unsigned int link_an_mode,
+			       const struct phylink_link_state *state);
 void ocelot_phylink_mac_link_down(struct ocelot *ocelot, int port,
 				  unsigned int link_an_mode,
 				  phy_interface_t interface,
-- 
2.25.1

[RFC v1 net-next 5/7] net: dsa: felix: attempt to initialize internal hsio plls

[Colin Foster]

The VSC7512 and VSC7514 have internal PLLs that can be used to control
different peripherals. Initialize these high speed I/O (HSIO) PLLs when
they exist, so that dependent peripherals like QSGMII can function.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/net/dsa/ocelot/felix.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index d4cc9e60f369..21dcb9cadc12 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1555,6 +1555,9 @@ static int felix_setup(struct dsa_switch *ds)
 	if (err)
 		return err;
 
+	if (ocelot->targets[HSIO])
+		ocelot_pll5_init(ocelot);
+
 	err = ocelot_init(ocelot);
 	if (err)
 		goto out_mdiobus_free;
-- 
2.25.1

[RFC v1 net-next 6/7] net: dsa: felix: allow external parsing of port nodes

[Colin Foster]

When the felix driver parses the device tree, it does so in such a way that
every node is parsed in a loop. This is done in the felix_init_structs()
function. After this is done, a separate loop will invoke
ocelot_init_port() on each port.

This causes problems if a user of the felix driver needs to retain some
information from the device tree during port initialization. A driver
might, for example, need to create call phylink_create() during
ocelot_init_port(), which requires a reference to the fwnode_handle.

Add a hook from felix into the sub-drivers, where they can optionally grab
references to the device_node when needed.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/net/dsa/ocelot/felix.c | 54 +++++++++++++++++++++++++---------
 drivers/net/dsa/ocelot/felix.h |  5 ++++
 2 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 21dcb9cadc12..b6e3a88addb8 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1245,7 +1245,8 @@ static int felix_parse_ports_node(struct felix *felix,
 				  struct device_node *ports_node,
 				  phy_interface_t *port_phy_modes)
 {
-	struct device *dev = felix->ocelot.dev;
+	struct ocelot *ocelot = &felix->ocelot;
+	struct device *dev = ocelot->dev;
 	struct device_node *child;
 
 	for_each_available_child_of_node(ports_node, child) {
@@ -1285,6 +1286,19 @@ static int felix_parse_ports_node(struct felix *felix,
 		}
 
 		port_phy_modes[port] = phy_mode;
+
+		if (dsa_is_cpu_port(felix->ds, port) ||
+		    !felix->info->parse_port_node)
+			continue;
+
+		err = felix->info->parse_port_node(ocelot, child, phy_mode,
+						   port);
+		if (err < 0) {
+			dev_err(dev, "Unable to create etherdev for port %d\n",
+				port);
+			of_node_put(child);
+			return err;
+		}
 	}
 
 	return 0;
@@ -1396,8 +1410,7 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports)
 
 	err = felix_parse_dt(felix, port_phy_modes);
 	if (err) {
-		kfree(port_phy_modes);
-		return err;
+		goto free_port_modes;
 	}
 
 	for (i = 0; i < TARGET_MAX; i++) {
@@ -1406,8 +1419,8 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports)
 			dev_err(ocelot->dev,
 				"Failed to map device memory space: %pe\n",
 				target);
-			kfree(port_phy_modes);
-			return PTR_ERR(target);
+			err = PTR_ERR(target);
+			goto free_port_modes;
 		}
 
 		ocelot->targets[i] = target;
@@ -1416,8 +1429,7 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports)
 	err = ocelot_regfields_init(ocelot, felix->info->regfields);
 	if (err) {
 		dev_err(ocelot->dev, "failed to init reg fields map\n");
-		kfree(port_phy_modes);
-		return err;
+		goto free_port_modes;
 	}
 
 	for (port = 0; port < num_phys_ports; port++) {
@@ -1429,8 +1441,8 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports)
 		if (!ocelot_port) {
 			dev_err(ocelot->dev,
 				"failed to allocate port memory\n");
-			kfree(port_phy_modes);
-			return -ENOMEM;
+			err = -ENOMEM;
+			goto free_port_modes;
 		}
 
 		target = felix_request_port_regmap(felix, port);
@@ -1438,8 +1450,8 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports)
 			dev_err(ocelot->dev,
 				"Failed to map memory space for port %d: %pe\n",
 				port, target);
-			kfree(port_phy_modes);
-			return PTR_ERR(target);
+			err = PTR_ERR(target);
+			goto free_port_modes;
 		}
 
 		ocelot_port->phy_mode = port_phy_modes[port];
@@ -1449,15 +1461,21 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports)
 		ocelot->ports[port] = ocelot_port;
 	}
 
-	kfree(port_phy_modes);
-
 	if (felix->info->mdio_bus_alloc) {
 		err = felix->info->mdio_bus_alloc(ocelot);
 		if (err < 0)
-			return err;
+			goto free_port_modes;
 	}
 
+	kfree(port_phy_modes);
+
 	return 0;
+
+free_port_modes:
+	if (felix->info->phylink_of_cleanup)
+		felix->info->phylink_of_cleanup(ocelot);
+	kfree(port_phy_modes);
+	return err;
 }
 
 static void ocelot_port_purge_txtstamp_skb(struct ocelot *ocelot, int port,
@@ -1574,12 +1592,18 @@ static int felix_setup(struct dsa_switch *ds)
 	dsa_switch_for_each_available_port(dp, ds) {
 		ocelot_init_port(ocelot, dp->index);
 
+		if (felix->info->phylink_create)
+			felix->info->phylink_create(ocelot, dp->index);
+
 		/* Set the default QoS Classification based on PCP and DEI
 		 * bits of vlan tag.
 		 */
 		felix_port_qos_map_init(ocelot, dp->index);
 	}
 
+	if (felix->info->phylink_of_cleanup)
+		felix->info->phylink_of_cleanup(ocelot);
+
 	err = ocelot_devlink_sb_register(ocelot);
 	if (err)
 		goto out_deinit_ports;
@@ -1604,6 +1628,8 @@ static int felix_setup(struct dsa_switch *ds)
 	ocelot_deinit(ocelot);
 
 out_mdiobus_free:
+	if (felix->info->phylink_of_cleanup)
+		felix->info->phylink_of_cleanup(ocelot);
 	if (felix->info->mdio_bus_free)
 		felix->info->mdio_bus_free(ocelot);
 
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index d5d0b30c0b75..ffb60bcf1817 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -58,6 +58,11 @@ struct felix_info {
 	void	(*tas_guard_bands_update)(struct ocelot *ocelot, int port);
 	void	(*port_sched_speed_set)(struct ocelot *ocelot, int port,
 					u32 speed);
+	int	(*parse_port_node)(struct ocelot *ocelot,
+				   struct device_node *ports_node,
+				   phy_interface_t phy_mode, int port);
+	int	(*phylink_create)(struct ocelot *ocelot, int port);
+	void	(*phylink_of_cleanup)(struct ocelot *ocelot);
 };
 
 /* Methods for initializing the hardware resources specific to a tagging
-- 
2.25.1

[RFC v1 net-next 7/7] net: dsa: ocelot_ext: add support for external phys

[Colin Foster]

The VSC7512 has four internal copper ports, and can be configured to work
in various configurations with up to six additional ports. Support for the
initial four ports was added in commit 3d7316ac81ac ("net: dsa: ocelot: add
external ocelot switch control"). This patch adds support for the
additional ports.

The specific hardware configuration for this development uses a QSGMII link
between a VSC7512 and a VSC8514. The VSC8514 offers connection to four RJ45
ports, all of which are verified functional.

Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
 drivers/net/dsa/ocelot/felix.h      |   1 +
 drivers/net/dsa/ocelot/ocelot_ext.c | 319 ++++++++++++++++++++++++++--
 2 files changed, 305 insertions(+), 15 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index ffb60bcf1817..fdd402305925 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h
@@ -6,6 +6,7 @@
 
 #define ocelot_to_felix(o)		container_of((o), struct felix, ocelot)
 #define FELIX_MAC_QUIRKS		OCELOT_QUIRK_PCS_PERFORMS_RATE_ADAPTATION
+#define OCELOT_EXT_MAC_QUIRKS		OCELOT_QUIRK_QSGMII_PORTS_MUST_BE_UP
 
 #define OCELOT_PORT_MODE_NONE		0
 #define OCELOT_PORT_MODE_INTERNAL	BIT(0)
diff --git a/drivers/net/dsa/ocelot/ocelot_ext.c b/drivers/net/dsa/ocelot/ocelot_ext.c
index 14efa6387bd7..f10271b973b2 100644
--- a/drivers/net/dsa/ocelot/ocelot_ext.c
+++ b/drivers/net/dsa/ocelot/ocelot_ext.c
@@ -4,10 +4,13 @@
  */
 
 #include <linux/mfd/ocelot.h>
+#include <linux/of_net.h>
+#include <linux/phy/phy.h>
 #include <linux/phylink.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <soc/mscc/ocelot.h>
+#include <soc/mscc/ocelot_dev.h>
 #include <soc/mscc/vsc7514_regs.h>
 #include "felix.h"
 
@@ -16,20 +19,283 @@
 #define OCELOT_PORT_MODE_SERDES		(OCELOT_PORT_MODE_SGMII | \
 					 OCELOT_PORT_MODE_QSGMII)
 
+#define phylink_config_to_ocelot_port(config) \
+	container_of(config, struct ocelot_ext_port_priv, phylink_config)
+#define phylink_pcs_to_ocelot_port(pl_pcs) \
+	container_of(pl_pcs, struct ocelot_ext_port_priv, pcs)
+
+struct ocelot_ext_port_priv {
+	struct device_node *node;
+	struct phylink_config phylink_config;
+	struct phylink *phylink;
+	struct ocelot *ocelot;
+	int chip_port;
+	struct phylink_pcs pcs;
+};
+
+struct ocelot_ext_priv {
+	struct felix felix;
+	struct ocelot_ext_port_priv *port_priv[VSC7514_NUM_PORTS];
+};
+
+static struct ocelot_ext_priv *felix_to_ocelot_ext_priv(struct felix *felix)
+{
+	return container_of(felix, struct ocelot_ext_priv, felix);
+}
+
 static const u32 vsc7512_port_modes[VSC7514_NUM_PORTS] = {
 	OCELOT_PORT_MODE_INTERNAL,
 	OCELOT_PORT_MODE_INTERNAL,
 	OCELOT_PORT_MODE_INTERNAL,
 	OCELOT_PORT_MODE_INTERNAL,
-	OCELOT_PORT_MODE_NONE,
-	OCELOT_PORT_MODE_NONE,
-	OCELOT_PORT_MODE_NONE,
-	OCELOT_PORT_MODE_NONE,
-	OCELOT_PORT_MODE_NONE,
-	OCELOT_PORT_MODE_NONE,
-	OCELOT_PORT_MODE_NONE,
+	OCELOT_PORT_MODE_SERDES,
+	OCELOT_PORT_MODE_SERDES,
+	OCELOT_PORT_MODE_SERDES,
+	OCELOT_PORT_MODE_SERDES,
+	OCELOT_PORT_MODE_SERDES,
+	OCELOT_PORT_MODE_SGMII,
+	OCELOT_PORT_MODE_SERDES,
+};
+
+static void ocelot_ext_phylink_of_cleanup(struct ocelot *ocelot)
+{
+	struct felix *felix = ocelot_to_felix(ocelot);
+	struct ocelot_ext_priv *ocelot_ext_priv;
+	int i;
+
+	ocelot_ext_priv = felix_to_ocelot_ext_priv(felix);
+	for (i = 0; i < VSC7514_NUM_PORTS; i++) {
+		struct ocelot_ext_port_priv *port_priv;
+
+		port_priv = ocelot_ext_priv->port_priv[i];
+		if (port_priv && port_priv->node)
+			of_node_put(port_priv->node);
+	}
+}
+
+static void ocelot_ext_phylink_mac_config(struct phylink_config *config,
+					  unsigned int link_an_mode,
+					  const struct phylink_link_state *state)
+{
+	struct ocelot_ext_port_priv *priv =
+		phylink_config_to_ocelot_port(config);
+	struct ocelot *ocelot = priv->ocelot;
+	int port = priv->chip_port;
+
+	ocelot_phylink_mac_config(ocelot, port, link_an_mode, state);
+}
+
+static void ocelot_ext_phylink_mac_link_down(struct phylink_config *config,
+					     unsigned int link_an_mode,
+					     phy_interface_t interface)
+{
+	struct ocelot_ext_port_priv *priv =
+		phylink_config_to_ocelot_port(config);
+	struct ocelot *ocelot = priv->ocelot;
+	struct felix *felix = ocelot_to_felix(ocelot);
+	int port = priv->chip_port;
+
+	ocelot_phylink_mac_link_down(ocelot, port, link_an_mode, interface,
+				     felix->info->quirks);
+}
+
+static void ocelot_ext_phylink_mac_link_up(struct phylink_config *config,
+					   struct phy_device *phydev,
+					   unsigned int link_an_mode,
+					   phy_interface_t interface,
+					   int speed, int duplex,
+					   bool tx_pause, bool rx_pause)
+{
+	struct ocelot_ext_port_priv *priv =
+		phylink_config_to_ocelot_port(config);
+	struct ocelot *ocelot = priv->ocelot;
+	struct felix *felix = ocelot_to_felix(ocelot);
+	int port = priv->chip_port;
+
+	ocelot_phylink_mac_link_up(ocelot, port, phydev, link_an_mode,
+				   interface, speed, duplex, tx_pause, rx_pause,
+				   felix->info->quirks);
+}
+
+static const struct phylink_mac_ops ocelot_ext_phylink_ops = {
+	.validate		= phylink_generic_validate,
+	.mac_config		= ocelot_ext_phylink_mac_config,
+	.mac_link_down		= ocelot_ext_phylink_mac_link_down,
+	.mac_link_up		= ocelot_ext_phylink_mac_link_up,
+};
+
+static void ocelot_ext_pcs_get_state(struct phylink_pcs *pcs,
+				     struct phylink_link_state *state)
+{
+	struct ocelot_ext_port_priv *port_priv =
+		phylink_pcs_to_ocelot_port(pcs);
+
+	/* TODO: Determine state from hardware? */
+}
+
+static int ocelot_ext_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
+				 phy_interface_t interface,
+				 const unsigned long *advertising,
+				 bool permit_pause_to_mac)
+{
+	struct ocelot_ext_port_priv *port_priv =
+		phylink_pcs_to_ocelot_port(pcs);
+
+	switch (interface) {
+	case PHY_INTERFACE_MODE_QSGMII:
+		ocelot_ext_phylink_mac_config(&port_priv->phylink_config, mode,
+					      NULL);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static void ocelot_ext_pcs_an_restart(struct phylink_pcs *pcs)
+{
+	/* TODO: Restart autonegotiaion process */
+}
+
+static void ocelot_ext_pcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
+				   phy_interface_t interface, int speed,
+				   int duplex)
+{
+	struct ocelot_ext_port_priv *port_priv =
+		phylink_pcs_to_ocelot_port(pcs);
+
+	ocelot_ext_phylink_mac_link_up(&port_priv->phylink_config, NULL, mode,
+				       interface, speed, duplex, false, false);
+}
+
+static const struct phylink_pcs_ops ocelot_ext_pcs_ops = {
+	.pcs_get_state = ocelot_ext_pcs_get_state,
+	.pcs_config = ocelot_ext_pcs_config,
+	.pcs_an_restart = ocelot_ext_pcs_an_restart,
+	.pcs_link_up = ocelot_ext_pcs_link_up,
 };
 
+static int ocelot_ext_parse_port_node(struct ocelot *ocelot,
+				      struct device_node *ports_node,
+				      phy_interface_t phy_mode, int port)
+{
+	struct ocelot_ext_port_priv *ocelot_ext_port_priv;
+	struct felix *felix = ocelot_to_felix(ocelot);
+	struct ocelot_ext_priv *ocelot_ext_priv;
+
+	ocelot_ext_priv = felix_to_ocelot_ext_priv(felix);
+
+	ocelot_ext_port_priv = devm_kzalloc(ocelot->dev,
+					    sizeof(*ocelot_ext_port_priv),
+					    GFP_KERNEL);
+	if (!ocelot_ext_port_priv)
+		return -ENOMEM;
+
+	ocelot_ext_port_priv->ocelot = ocelot;
+	ocelot_ext_port_priv->chip_port = port;
+	ocelot_ext_port_priv->pcs.ops = &ocelot_ext_pcs_ops;
+
+	if (!felix->pcs)
+		felix->pcs = devm_kcalloc(ocelot->dev, felix->info->num_ports,
+					  sizeof(struct phylink_pcs *),
+					  GFP_KERNEL);
+
+	if (!felix->pcs)
+		return -ENOMEM;
+
+	felix->pcs[port] = &ocelot_ext_port_priv->pcs;
+
+	ocelot_ext_priv->port_priv[port] = ocelot_ext_port_priv;
+
+	ocelot_ext_port_priv->node = of_node_get(ports_node);
+
+	return 0;
+}
+
+static int ocelot_ext_phylink_create(struct ocelot *ocelot, int port)
+{
+	struct ocelot_ext_port_priv *ocelot_ext_port_priv;
+	struct felix *felix = ocelot_to_felix(ocelot);
+	struct ocelot_ext_priv *ocelot_ext_priv;
+	struct device *dev = ocelot->dev;
+	struct ocelot_port *ocelot_port;
+	struct device_node *portnp;
+	phy_interface_t phy_mode;
+	struct phylink *phylink;
+	int err;
+
+	ocelot_ext_priv = felix_to_ocelot_ext_priv(felix);
+	ocelot_port = ocelot->ports[port];
+	ocelot_ext_port_priv = ocelot_ext_priv->port_priv[port];
+
+	if (!ocelot_ext_port_priv)
+		return 0;
+
+	portnp = ocelot_ext_port_priv->node;
+	phy_mode = ocelot_port->phy_mode;
+
+	/* Break out early if we're internal...? */
+	if (phy_mode == PHY_INTERFACE_MODE_INTERNAL)
+		return 0;
+
+	if (phy_mode == PHY_INTERFACE_MODE_QSGMII)
+		ocelot_port_rmwl(ocelot_port, 0,
+				 DEV_CLOCK_CFG_MAC_TX_RST |
+				 DEV_CLOCK_CFG_MAC_RX_RST,
+				 DEV_CLOCK_CFG);
+
+	if (phy_mode != PHY_INTERFACE_MODE_INTERNAL) {
+		struct phy *serdes = of_phy_get(portnp, NULL);
+
+		if (IS_ERR(serdes)) {
+			err = PTR_ERR(serdes);
+			dev_err_probe(dev, err,
+				      "missing SerDes phys for port %d\n",
+				      port);
+			return err;
+		}
+
+		err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET, phy_mode);
+		of_phy_put(serdes);
+		if (err) {
+			dev_err(dev,
+				"Could not set SerDes mode on port %d: %pe\n",
+				port, ERR_PTR(err));
+			return err;
+		}
+	}
+
+	ocelot_ext_port_priv->phylink_config.dev = dev;
+	ocelot_ext_port_priv->phylink_config.type = PHYLINK_DEV;
+	ocelot_ext_port_priv->phylink_config.mac_capabilities = MAC_ASYM_PAUSE |
+		MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000FD | MAC_2500FD;
+
+	__set_bit(ocelot_port->phy_mode,
+		  ocelot_ext_port_priv->phylink_config.supported_interfaces);
+
+	phylink = phylink_create(&ocelot_ext_port_priv->phylink_config,
+				 of_fwnode_handle(portnp),
+				 phy_mode, &ocelot_ext_phylink_ops);
+	if (IS_ERR(phylink)) {
+		err = PTR_ERR(phylink);
+		dev_err(dev, "Could not create phylink (%pe)\n", phylink);
+		return err;
+	}
+
+	ocelot_ext_port_priv->phylink = phylink;
+
+	err = phylink_of_phy_connect(phylink, portnp, 0);
+	if (err) {
+		dev_err(dev, "Could not connect to PHY: %pe\n", ERR_PTR(err));
+		phylink_destroy(phylink);
+		ocelot_ext_port_priv->phylink = NULL;
+		return err;
+	}
+
+	return 0;
+}
+
 static const struct ocelot_ops ocelot_ext_ops = {
 	.reset		= ocelot_reset,
 	.wm_enc		= ocelot_wm_enc,
@@ -48,6 +314,7 @@ static const char * const vsc7512_resource_names[TARGET_MAX] = {
 	[QS] = "qs",
 	[QSYS] = "qsys",
 	[ANA] = "ana",
+	[HSIO] = "hsio",
 };
 
 static const struct felix_info vsc7512_info = {
@@ -56,25 +323,32 @@ static const struct felix_info vsc7512_info = {
 	.map				= vsc7514_regmap,
 	.ops				= &ocelot_ext_ops,
 	.vcap				= vsc7514_vcap_props,
+	.quirks				= OCELOT_EXT_MAC_QUIRKS,
 	.num_mact_rows			= 1024,
 	.num_ports			= VSC7514_NUM_PORTS,
 	.num_tx_queues			= OCELOT_NUM_TC,
 	.port_modes			= vsc7512_port_modes,
+	.parse_port_node		= ocelot_ext_parse_port_node,
+	.phylink_create			= ocelot_ext_phylink_create,
+	.phylink_of_cleanup		= ocelot_ext_phylink_of_cleanup,
 };
 
 static int ocelot_ext_probe(struct platform_device *pdev)
 {
+	struct ocelot_ext_priv *ocelot_ext_priv;
 	struct device *dev = &pdev->dev;
 	struct dsa_switch *ds;
 	struct ocelot *ocelot;
 	struct felix *felix;
 	int err;
 
-	felix = kzalloc(sizeof(*felix), GFP_KERNEL);
-	if (!felix)
+	ocelot_ext_priv = kzalloc(sizeof(*ocelot_ext_priv), GFP_KERNEL);
+	if (!ocelot_ext_priv)
 		return -ENOMEM;
 
-	dev_set_drvdata(dev, felix);
+	dev_set_drvdata(dev, ocelot_ext_priv);
+
+	felix = &ocelot_ext_priv->felix;
 
 	ocelot = &felix->ocelot;
 	ocelot->dev = dev;
@@ -116,28 +390,43 @@ static int ocelot_ext_probe(struct platform_device *pdev)
 
 static int ocelot_ext_remove(struct platform_device *pdev)
 {
-	struct felix *felix = dev_get_drvdata(&pdev->dev);
+	struct ocelot_ext_priv *ocelot_ext_priv = dev_get_drvdata(&pdev->dev);
+	struct felix *felix;
 
-	if (!felix)
+	if (!ocelot_ext_priv)
 		return 0;
 
+	felix = &ocelot_ext_priv->felix;
+
 	dsa_unregister_switch(felix->ds);
 
 	kfree(felix->ds);
-	kfree(felix);
+	kfree(ocelot_ext_priv);
 
 	return 0;
 }
 
 static void ocelot_ext_shutdown(struct platform_device *pdev)
 {
-	struct felix *felix = dev_get_drvdata(&pdev->dev);
+	struct ocelot_ext_priv *ocelot_ext_priv = dev_get_drvdata(&pdev->dev);
+	struct ocelot_ext_port_priv *port_priv;
+	struct felix *felix;
+	int i;
 
-	if (!felix)
+	if (!ocelot_ext_priv)
 		return;
 
+	felix = &ocelot_ext_priv->felix;
+
 	dsa_switch_shutdown(felix->ds);
 
+	for (i = 0; i < felix->info->num_ports; i++) {
+		port_priv = ocelot_ext_priv->port_priv[i];
+
+		if (port_priv && port_priv->phylink)
+			phylink_destroy(port_priv->phylink);
+	}
+
 	dev_set_drvdata(&pdev->dev, NULL);
 }
 
-- 
2.25.1

Re: [RFC v1 net-next 7/7] net: dsa: ocelot_ext: add support for external phys

[Russell King (Oracle)]

Hi Colin,

On Wed, Feb 15, 2023 at 11:53:21PM -0800, Colin Foster wrote:
> +static const struct phylink_mac_ops ocelot_ext_phylink_ops = {
> +	.validate		= phylink_generic_validate,

There is no need to set this anymore.

> +	.mac_config		= ocelot_ext_phylink_mac_config,
> +	.mac_link_down		= ocelot_ext_phylink_mac_link_down,
> +	.mac_link_up		= ocelot_ext_phylink_mac_link_up,
> +};
> +
> +static void ocelot_ext_pcs_get_state(struct phylink_pcs *pcs,
> +				     struct phylink_link_state *state)
> +{
> +	struct ocelot_ext_port_priv *port_priv =
> +		phylink_pcs_to_ocelot_port(pcs);
> +
> +	/* TODO: Determine state from hardware? */
> +}
> +
> +static int ocelot_ext_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
> +				 phy_interface_t interface,
> +				 const unsigned long *advertising,
> +				 bool permit_pause_to_mac)
> +{
> +	struct ocelot_ext_port_priv *port_priv =
> +		phylink_pcs_to_ocelot_port(pcs);
> +
> +	switch (interface) {
> +	case PHY_INTERFACE_MODE_QSGMII:
> +		ocelot_ext_phylink_mac_config(&port_priv->phylink_config, mode,
> +					      NULL);

Why are you calling a "mac" operation from a "pcs" operation? If this
PCS is attached to the same phylink instance as the MAC, you'll get
the .mac_config method called along with the .pcs_config, so calling
one from the other really isn't necessary.

> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static void ocelot_ext_pcs_an_restart(struct phylink_pcs *pcs)
> +{
> +	/* TODO: Restart autonegotiaion process */
> +}
> +
> +static void ocelot_ext_pcs_link_up(struct phylink_pcs *pcs, unsigned int mode,
> +				   phy_interface_t interface, int speed,
> +				   int duplex)
> +{
> +	struct ocelot_ext_port_priv *port_priv =
> +		phylink_pcs_to_ocelot_port(pcs);
> +
> +	ocelot_ext_phylink_mac_link_up(&port_priv->phylink_config, NULL, mode,
> +				       interface, speed, duplex, false, false);

Same here... and I fail to see why any of these need to be implemented
or what the purpose of providing this pcs code is.

> +}
> +
> +static const struct phylink_pcs_ops ocelot_ext_pcs_ops = {
> +	.pcs_get_state = ocelot_ext_pcs_get_state,
> +	.pcs_config = ocelot_ext_pcs_config,
> +	.pcs_an_restart = ocelot_ext_pcs_an_restart,
> +	.pcs_link_up = ocelot_ext_pcs_link_up,
>  };
>  
> +static int ocelot_ext_parse_port_node(struct ocelot *ocelot,
> +				      struct device_node *ports_node,
> +				      phy_interface_t phy_mode, int port)
> +{
> +	struct ocelot_ext_port_priv *ocelot_ext_port_priv;
> +	struct felix *felix = ocelot_to_felix(ocelot);
> +	struct ocelot_ext_priv *ocelot_ext_priv;
> +
> +	ocelot_ext_priv = felix_to_ocelot_ext_priv(felix);
> +
> +	ocelot_ext_port_priv = devm_kzalloc(ocelot->dev,
> +					    sizeof(*ocelot_ext_port_priv),
> +					    GFP_KERNEL);
> +	if (!ocelot_ext_port_priv)
> +		return -ENOMEM;
> +
> +	ocelot_ext_port_priv->ocelot = ocelot;
> +	ocelot_ext_port_priv->chip_port = port;
> +	ocelot_ext_port_priv->pcs.ops = &ocelot_ext_pcs_ops;
> +
> +	if (!felix->pcs)
> +		felix->pcs = devm_kcalloc(ocelot->dev, felix->info->num_ports,
> +					  sizeof(struct phylink_pcs *),
> +					  GFP_KERNEL);
> +
> +	if (!felix->pcs)
> +		return -ENOMEM;
> +
> +	felix->pcs[port] = &ocelot_ext_port_priv->pcs;
> +
> +	ocelot_ext_priv->port_priv[port] = ocelot_ext_port_priv;
> +
> +	ocelot_ext_port_priv->node = of_node_get(ports_node);
> +
> +	return 0;
> +}
> +
> +static int ocelot_ext_phylink_create(struct ocelot *ocelot, int port)
> +{
> +	struct ocelot_ext_port_priv *ocelot_ext_port_priv;
> +	struct felix *felix = ocelot_to_felix(ocelot);
> +	struct ocelot_ext_priv *ocelot_ext_priv;
> +	struct device *dev = ocelot->dev;
> +	struct ocelot_port *ocelot_port;
> +	struct device_node *portnp;
> +	phy_interface_t phy_mode;
> +	struct phylink *phylink;
> +	int err;
> +
> +	ocelot_ext_priv = felix_to_ocelot_ext_priv(felix);
> +	ocelot_port = ocelot->ports[port];
> +	ocelot_ext_port_priv = ocelot_ext_priv->port_priv[port];
> +
> +	if (!ocelot_ext_port_priv)
> +		return 0;
> +
> +	portnp = ocelot_ext_port_priv->node;
> +	phy_mode = ocelot_port->phy_mode;
> +
> +	/* Break out early if we're internal...? */
> +	if (phy_mode == PHY_INTERFACE_MODE_INTERNAL)
> +		return 0;
> +
> +	if (phy_mode == PHY_INTERFACE_MODE_QSGMII)
> +		ocelot_port_rmwl(ocelot_port, 0,
> +				 DEV_CLOCK_CFG_MAC_TX_RST |
> +				 DEV_CLOCK_CFG_MAC_RX_RST,
> +				 DEV_CLOCK_CFG);
> +
> +	if (phy_mode != PHY_INTERFACE_MODE_INTERNAL) {
> +		struct phy *serdes = of_phy_get(portnp, NULL);
> +
> +		if (IS_ERR(serdes)) {
> +			err = PTR_ERR(serdes);
> +			dev_err_probe(dev, err,
> +				      "missing SerDes phys for port %d\n",
> +				      port);
> +			return err;
> +		}
> +
> +		err = phy_set_mode_ext(serdes, PHY_MODE_ETHERNET, phy_mode);
> +		of_phy_put(serdes);
> +		if (err) {
> +			dev_err(dev,
> +				"Could not set SerDes mode on port %d: %pe\n",
> +				port, ERR_PTR(err));
> +			return err;
> +		}
> +	}
> +
> +	ocelot_ext_port_priv->phylink_config.dev = dev;
> +	ocelot_ext_port_priv->phylink_config.type = PHYLINK_DEV;
> +	ocelot_ext_port_priv->phylink_config.mac_capabilities = MAC_ASYM_PAUSE |
> +		MAC_SYM_PAUSE | MAC_10 | MAC_100 | MAC_1000FD | MAC_2500FD;
> +
> +	__set_bit(ocelot_port->phy_mode,
> +		  ocelot_ext_port_priv->phylink_config.supported_interfaces);
> +
> +	phylink = phylink_create(&ocelot_ext_port_priv->phylink_config,
> +				 of_fwnode_handle(portnp),
> +				 phy_mode, &ocelot_ext_phylink_ops);

I'm confused. DSA already sets up a phylink instance per port, so why
do you need another one?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC v1 net-next 7/7] net: dsa: ocelot_ext: add support for external phys

[Colin Foster]

On Thu, Feb 16, 2023 at 11:17:48AM +0000, Russell King (Oracle) wrote:
> Hi Colin,
> 
> On Wed, Feb 15, 2023 at 11:53:21PM -0800, Colin Foster wrote:
> > +static const struct phylink_mac_ops ocelot_ext_phylink_ops = {
> > +	.validate		= phylink_generic_validate,
> 
> There is no need to set this anymore.

I'll remove. Thanks.

> > +static int ocelot_ext_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
> > +				 phy_interface_t interface,
> > +				 const unsigned long *advertising,
> > +				 bool permit_pause_to_mac)
> > +{
> > +	struct ocelot_ext_port_priv *port_priv =
> > +		phylink_pcs_to_ocelot_port(pcs);
> > +
> > +	switch (interface) {
> > +	case PHY_INTERFACE_MODE_QSGMII:
> > +		ocelot_ext_phylink_mac_config(&port_priv->phylink_config, mode,
> > +					      NULL);
> 
> Why are you calling a "mac" operation from a "pcs" operation? If this
> PCS is attached to the same phylink instance as the MAC, you'll get
> the .mac_config method called along with the .pcs_config, so calling
> one from the other really isn't necessary.

Per the other email, it was my misunderstanding - probably from the
unnecessary phylink_create(). V2 will be cleaned up.

...

> > +
> > +	phylink = phylink_create(&ocelot_ext_port_priv->phylink_config,
> > +				 of_fwnode_handle(portnp),
> > +				 phy_mode, &ocelot_ext_phylink_ops);
> 
> I'm confused. DSA already sets up a phylink instance per port, so why
> do you need another one?

Also in the other email, it is definitely my confusion. I'll get things
straighened out for V2, as these patches seem more complicated than they
need to be.


Thanks again!

> 
> Thanks.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC v1 net-next 0/7] add support for ocelot external ports

[Russell King (Oracle)]

On Wed, Feb 15, 2023 at 11:53:14PM -0800, Colin Foster wrote:
> Part 3 will, at a minimum, add support for ports 4-7, which are
> configured to use QSGMII to an external phy (Return Of The QSGMII). With
> any luck, and some guidance, support for SGMII, SFPs, etc. will also be
> part of this series.
> 
> 
> This patch series is absolutely an RFC at this point. While all 8 copper
> ports on the VSC7512 are currently functional, I recognize there are a
> couple empty function callbacks in the last patch that likely need to be
> implemented.
> 
...
> 
> Also, with patch 7 ("net: dsa: ocelot_ext: add support for external phys")
> my basis was the function mscc_ocelot_init_ports(), but there were several
> changes I had to make for DSA / Phylink. Are my implementations of
> ocelot_ext_parse_port_node() and ocelot_ext_phylink_create() barking up
> the right tree?

DSA already creates phylink instances per DSA port, and provides many
of the phylink MAC operations to the DSA driver via the .phylink_*
operations in the dsa_switch_ops structure, and this phylink instance
should be used for managing the status and configuring the port
according to phylink's callbacks. The core felix code already makes
use of this, implementing the mac_link_down() and mac_link_up()
operations to handle when the link comes up or goes down.

I don't see why one would need to create a separate phylink instance
to support external PHYs, SFPs, etc on a DSA switch. The phylink
instance created by DSA is there for the DSA driver to make use of
for the port, and should be sufficient for this.

I think if you use the DSA-created phylink instance, then you don't
need any of patch 6. I'm not yet convinced that you need anything
from patch 7, but maybe you could explain what patch 7 provides that
the existing felix phylink implementation doesn't already provide.

I do get the impression that the use of the PCS instance in patch 7
is an attempt to work around the use of a private instance,
redirecting the pcs_config and pcs_link_up methods to the
corresponding MAC operations as a workaround for having the private
instance.

It looks like you need to hook into the mac_config(), mac_link_up()
and mac_link_down() methods at the core felix layer, so I would
suggest looking at the felix_info structure, adding methods there for
each of these, and arranging for the core felix code to forward these
calls down to the implementation as required.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC v1 net-next 0/7] add support for ocelot external ports

[Colin Foster]

Hi Russell,

On Thu, Feb 16, 2023 at 12:14:38PM +0000, Russell King (Oracle) wrote:
> On Wed, Feb 15, 2023 at 11:53:14PM -0800, Colin Foster wrote:
> > Part 3 will, at a minimum, add support for ports 4-7, which are
> > configured to use QSGMII to an external phy (Return Of The QSGMII). With
> > any luck, and some guidance, support for SGMII, SFPs, etc. will also be
> > part of this series.
> > 
> > 
> > This patch series is absolutely an RFC at this point. While all 8 copper
> > ports on the VSC7512 are currently functional, I recognize there are a
> > couple empty function callbacks in the last patch that likely need to be
> > implemented.
> > 
> ...
> > 
> > Also, with patch 7 ("net: dsa: ocelot_ext: add support for external phys")
> > my basis was the function mscc_ocelot_init_ports(), but there were several
> > changes I had to make for DSA / Phylink. Are my implementations of
> > ocelot_ext_parse_port_node() and ocelot_ext_phylink_create() barking up
> > the right tree?
> 
> DSA already creates phylink instances per DSA port, and provides many
> of the phylink MAC operations to the DSA driver via the .phylink_*
> operations in the dsa_switch_ops structure, and this phylink instance
> should be used for managing the status and configuring the port
> according to phylink's callbacks. The core felix code already makes
> use of this, implementing the mac_link_down() and mac_link_up()
> operations to handle when the link comes up or goes down.
> 
> I don't see why one would need to create a separate phylink instance
> to support external PHYs, SFPs, etc on a DSA switch. The phylink
> instance created by DSA is there for the DSA driver to make use of
> for the port, and should be sufficient for this.

This is essentially the feedback I was looking for. "This looks wrong"
which means I'll take a step back.

> 
> I think if you use the DSA-created phylink instance, then you don't
> need any of patch 6. I'm not yet convinced that you need anything
> from patch 7, but maybe you could explain what patch 7 provides that
> the existing felix phylink implementation doesn't already provide.

I'll have to go through it again to remember exactly what I was up
against - it was a while ago now. All of the logic was based on the
logic in ocelot_port_phylink_create() - which is part of the vsc7514
switchdev implementation (a chip that is essentially identical, except
for an internal MIPS instead of external SPI control)

I believe the main gotcha was that control over the phy itself, by way
of phy_set_mode_ext(). That needed the 'struct device_node *portnp'


.... Keeps looking ....

Ahh, yes. Regmaps and regfields aren't initialized at the time of
dt parsing in felix. And the MDIO bus isn't allocated until after that.
That's the reason for patch 6 parse_port_node() - I need the tree node
to get MDIO access to the phy, which I don't have until I'm done parsing
the tree...

There might be a cleaner way for me to do that. I'm tiptoeing a little
bit to avoid any regressions with the felix_vsc9959 or seville_vsc9953.

> I do get the impression that the use of the PCS instance in patch 7
> is an attempt to work around the use of a private instance,
> redirecting the pcs_config and pcs_link_up methods to the
> corresponding MAC operations as a workaround for having the private
> instance.

I'm not convinced I don't need PCS here, and just have things working
wrong.

The configuration looks like this:


		|------------------------------------------------|
		| CPU                                            |
		|------------------------------------------------|
            |
           SPI
            |
		|------------------------------------------------|
		| VSC7512                                        |
		|------------------------------------------------|
            ||        ||         ||         ||      |
		|-------|  |-------|  |-------|  |-------|  |
		| port4 |  | port5 |  | port6 |  | port7 |  |
		|-------|  |-------|  |-------|  |-------|  |
                      ||                            |
                    QSGMII                         MDIO
                      ||                            |
		|------------------------------------------------|
		| VSC8512                                        |
		|------------------------------------------------|
            ||        ||         ||         ||
		|-------|  |-------|  |-------|  |-------|
		| sw0p4 |  | sw0p5 |  | sw0p6 |  | sw0p7 |
		|-------|  |-------|  |-------|  |-------|


Would phylink_pcs need to get involved in the QSGMII link at all, or
should the phylnk from dsa_port_phylink_create() be all that's needed?


> 
> It looks like you need to hook into the mac_config(), mac_link_up()
> and mac_link_down() methods at the core felix layer, so I would
> suggest looking at the felix_info structure, adding methods there for
> each of these, and arranging for the core felix code to forward these
> calls down to the implementation as required.

Yes, I'll look at how I can clean things up.

I greatly appreciate your feedback!

> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC v1 net-next 0/7] add support for ocelot external ports

[Vladimir Oltean]

On Thu, Feb 16, 2023 at 04:42:06PM -0800, Colin Foster wrote:
> I believe the main gotcha was that control over the phy itself, by way
> of phy_set_mode_ext(). That needed the 'struct device_node *portnp'

DT parsing in felix_parse_dt() is not the only DT parsing that is done,
and certainly nothing depends on it in the way you describe.

dsa_switch_parse_of() also parses the device tree. felix_parse_dt() only
exists because the SERDES/PCS drivers from NXP LS1028A do not support
dynamic reconfiguration of the SERDES protocol. So we parse the device
tree to set the initial ocelot_port->phy_mode, and then (with the
current phylink API) we populate phylink's config->supported_interfaces
with just that one bit set, to prevent SERDES protocol changes.

Do not get too hung up on this parsing (unless you believe you could
simplify the code by removing it; case in which I'd be interested if you
had patches in this area). Each port's device_node is also available in
struct dsa_port :: dn.

> 
> .... Keeps looking ....
> 
> Ahh, yes. Regmaps and regfields aren't initialized at the time of
> dt parsing in felix. And the MDIO bus isn't allocated until after that.
> That's the reason for patch 6 parse_port_node() - I need the tree node
> to get MDIO access to the phy, which I don't have until I'm done parsing
> the tree...

Nope. Device tree parsing in DSA is done from dsa_register_switch(), and
dsa_switch_ops :: setup() (aka felix_setup()) is the first callback in
which the information is reliably available.

You can *easily* call phy_set_mode_ext() from the "setup()" callback.
In fact, you're already doing that. Not sure what the problem seems to be.
It doesn't seem to be an ordering problem between phy_set_mode_ext() and
phylink_create() either, because DSA calls phylink_create() after both
the dsa_switch_ops :: setup() as well as port_setup() callbacks.
So there should be plenty of opportunity for you to prepare.

> There might be a cleaner way for me to do that. I'm tiptoeing a little
> bit to avoid any regressions with the felix_vsc9959 or seville_vsc9953.

I can test those. I'd much prefer if you made an initial effort to keep
a relatively consistent code structure.

Re: [RFC v1 net-next 0/7] add support for ocelot external ports

[Colin Foster]

On Fri, Feb 17, 2023 at 03:11:55AM +0200, Vladimir Oltean wrote:
> On Thu, Feb 16, 2023 at 04:42:06PM -0800, Colin Foster wrote:
> > I believe the main gotcha was that control over the phy itself, by way
> > of phy_set_mode_ext(). That needed the 'struct device_node *portnp'
> 
> DT parsing in felix_parse_dt() is not the only DT parsing that is done,
> and certainly nothing depends on it in the way you describe.
> 
> dsa_switch_parse_of() also parses the device tree. felix_parse_dt() only
> exists because the SERDES/PCS drivers from NXP LS1028A do not support
> dynamic reconfiguration of the SERDES protocol. So we parse the device
> tree to set the initial ocelot_port->phy_mode, and then (with the
> current phylink API) we populate phylink's config->supported_interfaces
> with just that one bit set, to prevent SERDES protocol changes.
> 
> Do not get too hung up on this parsing (unless you believe you could
> simplify the code by removing it; case in which I'd be interested if you
> had patches in this area). Each port's device_node is also available in
> struct dsa_port :: dn.
> 
> > 
> > .... Keeps looking ....
> > 
> > Ahh, yes. Regmaps and regfields aren't initialized at the time of
> > dt parsing in felix. And the MDIO bus isn't allocated until after that.
> > That's the reason for patch 6 parse_port_node() - I need the tree node
> > to get MDIO access to the phy, which I don't have until I'm done parsing
> > the tree...
> 
> Nope. Device tree parsing in DSA is done from dsa_register_switch(), and
> dsa_switch_ops :: setup() (aka felix_setup()) is the first callback in
> which the information is reliably available.
> 
> You can *easily* call phy_set_mode_ext() from the "setup()" callback.
> In fact, you're already doing that. Not sure what the problem seems to be.
> It doesn't seem to be an ordering problem between phy_set_mode_ext() and
> phylink_create() either, because DSA calls phylink_create() after both
> the dsa_switch_ops :: setup() as well as port_setup() callbacks.
> So there should be plenty of opportunity for you to prepare.

Thank you Vladimir and Russel for the nudge in the right direction! I
made this a lot harder on myself than it needed to be.

Yes, I can drop patches 6 and 7 (they felt pretty wrong as I was writing
them), implement phylink_mac_config() in felix_switch_ops, and do the
phy configuration using the dsa port's dp->dn node.

I'll probably send out another RFC in the next couple weeks with the
much better implementation. Then hopefully it'll be ready to be upgraded
to patch status after the merge window. (I hope I don't regret saying
that)