The rzn1-gmac instance is connected to a PCS (MIIC). In order to use
this pcs, add support in the sttmac driver to set a generic phylink pcs
device instead of the xpcs only. Moreover, it adds support to provide
a phylink pcs device from the stmmac platform data and use it with the
driver. It also adds the bindings and the new rzn1-gmac driver that
retrieve this pcs from the device-tree.
Clément Léger (6):
net: stmmac: add support to use a generic phylink_pcs as PCS
net: stmmac: add support to provide pcs from platform data
net: stmmac: start phylink before setting up hardware
dt-bindings: net: renesas,rzn1-gmac: Document RZ/N1 GMAC support
net: stmmac: add support for RZ/N1 GMAC
ARM: dts: r9a06g032: describe GMAC1
.../bindings/net/renesas,rzn1-gmac.yaml | 71 +++++++++++
arch/arm/boot/dts/r9a06g032.dtsi | 18 +++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 ++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
drivers/net/ethernet/stmicro/stmmac/common.h | 2 +
.../net/ethernet/stmicro/stmmac/dwmac-rzn1.c | 113 ++++++++++++++++++
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 15 ++-
.../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 1 +
include/linux/stmmac.h | 1 +
9 files changed, 228 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c
--
2.39.0
The stmmac on the Renesas RZ/N1 platform is connected to the PCS which
must be configured to provide a correct RGMII RX clock to the stmmac IP.
Without the RX clock, the driver will fail to initialize the hardware
(more specifically, the driver will report it fails to reset DMA). In
order to fix that, start phylink mecanism before setting up hardware.
Signed-off-by: Clément Léger <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f2247b8cf0a3..88c941003855 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3818,6 +3818,12 @@ static int __stmmac_open(struct net_device *dev,
}
}
+ /* We need to setup the phy & PCS before accessing the stmmac registers
+ * because in some cases (RZ/N1), if the stmmac IP is not clocked by the
+ * PCS, hardware init will fail because it lacks a RGMII RX clock.
+ */
+ phylink_start(priv->phylink);
+
ret = stmmac_hw_setup(dev, true);
if (ret < 0) {
netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
@@ -3826,7 +3832,6 @@ static int __stmmac_open(struct net_device *dev,
stmmac_init_coalesce(priv);
- phylink_start(priv->phylink);
/* We may have called phylink_speed_down before */
phylink_speed_up(priv->phylink);
--
2.39.0
On Mon, Jan 16, 2023 at 11:39:23AM +0100, Cl?ment L?ger wrote:
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index f2247b8cf0a3..88c941003855 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3818,6 +3818,12 @@ static int __stmmac_open(struct net_device *dev,
> }
> }
>
> + /* We need to setup the phy & PCS before accessing the stmmac registers
> + * because in some cases (RZ/N1), if the stmmac IP is not clocked by the
> + * PCS, hardware init will fail because it lacks a RGMII RX clock.
> + */
> + phylink_start(priv->phylink);
So what happens if you end up with the mac_link_up method being called
at this point in the driver, before the hardware has been setup ?
If you use a fixed-link, that's a real possibility.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Currently, the PCS is set based on the presence of the xpcs field. In
order to allow supporting other PCS, add a phylink_pcs pcs field to struct
mac_device_info which is used in stmmac_mac_select_pcs() to select the
correct PCS.
Signed-off-by: Clément Léger <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 2 ++
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 5 +----
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 1 +
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 6b5d96bced47..79fd67e8ab90 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -15,6 +15,7 @@
#include <linux/netdevice.h>
#include <linux/stmmac.h>
#include <linux/phy.h>
+#include <linux/phylink.h>
#include <linux/pcs/pcs-xpcs.h>
#include <linux/module.h>
#if IS_ENABLED(CONFIG_VLAN_8021Q)
@@ -518,6 +519,7 @@ struct mac_device_info {
const struct stmmac_tc_ops *tc;
const struct stmmac_mmc_ops *mmc;
struct dw_xpcs *xpcs;
+ struct phylink_pcs *phylink_pcs;
struct mii_regs mii; /* MII register Addresses */
struct mac_link link;
void __iomem *pcsr; /* vpointer to device CSRs */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c6951c976f5d..19459ef15a35 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -937,10 +937,7 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config,
{
struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
- if (!priv->hw->xpcs)
- return NULL;
-
- return &priv->hw->xpcs->pcs;
+ return priv->hw->phylink_pcs;
}
static void stmmac_mac_config(struct phylink_config *config, unsigned int mode,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 5f177ea80725..3e875d4664c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -413,6 +413,7 @@ int stmmac_xpcs_setup(struct mii_bus *bus)
}
priv->hw->xpcs = xpcs;
+ priv->hw->phylink_pcs = &xpcs->pcs;
break;
}
--
2.39.0
Add "renesas,rzn1-gmac" binding documention which is compatible which
"snps,dwmac" compatible driver but uses a custom PCS to communicate
with the phy.
Signed-off-by: Clément Léger <[email protected]>
---
.../bindings/net/renesas,rzn1-gmac.yaml | 71 +++++++++++++++++++
1 file changed, 71 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
diff --git a/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml b/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
new file mode 100644
index 000000000000..effb9a312832
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/renesas,rzn1-gmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas GMAC1 Device Tree Bindings
+
+maintainers:
+ - Clément Léger <[email protected]>
+
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - renesas,r9a06g032-gmac
+ - renesas,rzn1-gmac
+ required:
+ - compatible
+
+allOf:
+ - $ref: "snps,dwmac.yaml#"
+
+properties:
+ compatible:
+ additionalItems: true
+ maxItems: 3
+ items:
+ - enum:
+ - renesas,r9a06g032-gmac
+ - renesas,rzn1-gmac
+ contains:
+ enum:
+ - snps,dwmac
+
+ pcs-handle:
+ description:
+ phandle pointing to a PCS sub-node compatible with
+ renesas,rzn1-miic.yaml#
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+required:
+ - compatible
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/r9a06g032-sysctrl.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ ethernet@44000000 {
+ compatible = "renesas,rzn1-gmac";
+ reg = <0x44000000 0x2000>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+ clock-names = "stmmaceth";
+ clocks = <&sysctrl R9A06G032_HCLK_GMAC0>;
+ snps,multicast-filter-bins = <256>;
+ snps,perfect-filter-entries = <128>;
+ tx-fifo-depth = <2048>;
+ rx-fifo-depth = <4096>;
+ pcs-handle = <&mii_conv1>;
+ phy-mode = "mii";
+ };
+
+...
--
2.39.0
Add a pcs field in platform_data to allow providing platform data. This is
gonig to be used by the renesas,rzn1-gmac compatible driver which can make
use of a PCS.
Signed-off-by: Clément Léger <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
include/linux/stmmac.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 19459ef15a35..f2247b8cf0a3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7287,6 +7287,9 @@ int stmmac_dvr_probe(struct device *device,
goto error_xpcs_setup;
}
+ if (priv->plat->pcs)
+ priv->hw->phylink_pcs = priv->plat->pcs;
+
ret = stmmac_phy_setup(priv);
if (ret) {
netdev_err(ndev, "failed to setup phy (%d)\n", ret);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index 83ca2e8eb6b5..af09d5e0ca4b 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -272,5 +272,6 @@ struct plat_stmmacenet_data {
bool use_phy_wol;
bool sph_disable;
bool serdes_up_after_phy_linkup;
+ struct phylink_pcs *pcs;
};
#endif
--
2.39.0
Hi Clément,
Thanks for your patch!
On Mon, Jan 16, 2023 at 11:37 AM Clément Léger
<[email protected]> wrote:
> Add "renesas,rzn1-gmac" binding documention which is compatible which
documentation
> "snps,dwmac" compatible driver but uses a custom PCS to communicate
> with the phy.
>
> Signed-off-by: Clément Léger <[email protected]>
> ---
> .../bindings/net/renesas,rzn1-gmac.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml b/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
> new file mode 100644
> index 000000000000..effb9a312832
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/renesas,rzn1-gmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas GMAC1 Device Tree Bindings
> +
> +maintainers:
> + - Clément Léger <[email protected]>
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - renesas,r9a06g032-gmac
> + - renesas,rzn1-gmac
> + required:
> + - compatible
> +
> +allOf:
> + - $ref: "snps,dwmac.yaml#"
> +
> +properties:
> + compatible:
> + additionalItems: true
> + maxItems: 3
> + items:
> + - enum:
> + - renesas,r9a06g032-gmac
> + - renesas,rzn1-gmac
> + contains:
> + enum:
> + - snps,dwmac
Why not just
items:
- enum:
- renesas,r9a06g032-gmac
- renesas,rzn1-gmac
- snps,dwmac
?
> +examples:
> + - |
> + #include <dt-bindings/clock/r9a06g032-sysctrl.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + ethernet@44000000 {
> + compatible = "renesas,rzn1-gmac";
Documentation/devicetree/bindings/net/renesas,rzn1-gmac.example.dtb:
ethernet@44000000: compatible: ['renesas,rzn1-gmac'] does not contain
items matching the given schema
> + reg = <0x44000000 0x2000>;
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> + clock-names = "stmmaceth";
> + clocks = <&sysctrl R9A06G032_HCLK_GMAC0>;
> + snps,multicast-filter-bins = <256>;
> + snps,perfect-filter-entries = <128>;
> + tx-fifo-depth = <2048>;
> + rx-fifo-depth = <4096>;
> + pcs-handle = <&mii_conv1>;
> + phy-mode = "mii";
> + };
> +
> +...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Mon, 16 Jan 2023 11:39:24 +0100, Clément Léger wrote:
> Add "renesas,rzn1-gmac" binding documention which is compatible which
> "snps,dwmac" compatible driver but uses a custom PCS to communicate
> with the phy.
>
> Signed-off-by: Clément Léger <[email protected]>
> ---
> .../bindings/net/renesas,rzn1-gmac.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.example.dtb: ethernet@44000000: compatible: ['renesas,rzn1-gmac'] does not contain items matching the given schema
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On 16/01/2023 11:39, Clément Léger wrote:
> Add "renesas,rzn1-gmac" binding documention which is compatible which
> "snps,dwmac" compatible driver but uses a custom PCS to communicate
> with the phy.
>
> Signed-off-by: Clément Léger <[email protected]>
> ---
> .../bindings/net/renesas,rzn1-gmac.yaml | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml b/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
> new file mode 100644
> index 000000000000..effb9a312832
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/renesas,rzn1-gmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas GMAC1 Device Tree Bindings
Drop Device Tree Bindings.
> +
> +maintainers:
> + - Clément Léger <[email protected]>
> +
> +select:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - renesas,r9a06g032-gmac
> + - renesas,rzn1-gmac
> + required:
> + - compatible
> +
> +allOf:
> + - $ref: "snps,dwmac.yaml#"
Drop quotes.
> +
> +properties:
> + compatible:
> + additionalItems: true
No. Drop.
> + maxItems: 3
No.
> + items:
> + - enum:
> + - renesas,r9a06g032-gmac
> + - renesas,rzn1-gmac
> + contains:
> + enum:
> + - snps,dwmac
No, please list possibilities
> +
> + pcs-handle:
> + description:
> + phandle pointing to a PCS sub-node compatible with
> + renesas,rzn1-miic.yaml#
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> +required:
> + - compatible
> +
> +unevaluatedProperties: false
> +
Best regards,
Krzysztof
Le Mon, 16 Jan 2023 10:53:49 +0000,
"Russell King (Oracle)" <[email protected]> a écrit :
> On Mon, Jan 16, 2023 at 11:39:23AM +0100, Clément Léger wrote:
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index f2247b8cf0a3..88c941003855 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -3818,6 +3818,12 @@ static int __stmmac_open(struct net_device *dev,
> > }
> > }
> >
> > + /* We need to setup the phy & PCS before accessing the stmmac registers
> > + * because in some cases (RZ/N1), if the stmmac IP is not clocked by the
> > + * PCS, hardware init will fail because it lacks a RGMII RX clock.
> > + */
> > + phylink_start(priv->phylink);
>
> So what happens if you end up with the mac_link_up method being called
> at this point in the driver, before the hardware has been setup ?
>
> If you use a fixed-link, that's a real possibility.
I actually have this setup. On the board, one GMAC is connected to a
DSA switch using a fixed-link and the other using the PCS such as added
by this series.
From what I see, indeed, the mac_link_up() function is called before
stmmac_hw_setup(). This does not seems to have any effect on my setup
(except making it working of course) but I agree this is clearly not
ideal.
What I could do is adding a function in the miic pcs driver that could
be called from my rzn1 stmmac probe function to actually configure the
PCS at probe time based on the detected "phy-mode". Does that seems
better to you ?
Thanks,
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com
On Tue, Feb 07, 2023 at 03:41:35PM +0100, Cl?ment L?ger wrote:
> Le Mon, 16 Jan 2023 10:53:49 +0000,
> "Russell King (Oracle)" <[email protected]> a ?crit :
>
> > On Mon, Jan 16, 2023 at 11:39:23AM +0100, Cl?ment L?ger wrote:
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > index f2247b8cf0a3..88c941003855 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > @@ -3818,6 +3818,12 @@ static int __stmmac_open(struct net_device *dev,
> > > }
> > > }
> > >
> > > + /* We need to setup the phy & PCS before accessing the stmmac registers
> > > + * because in some cases (RZ/N1), if the stmmac IP is not clocked by the
> > > + * PCS, hardware init will fail because it lacks a RGMII RX clock.
> > > + */
> > > + phylink_start(priv->phylink);
> >
> > So what happens if you end up with the mac_link_up method being called
> > at this point in the driver, before the hardware has been setup ?
> >
> > If you use a fixed-link, that's a real possibility.
>
> I actually have this setup. On the board, one GMAC is connected to a
> DSA switch using a fixed-link and the other using the PCS such as added
> by this series.
>
> From what I see, indeed, the mac_link_up() function is called before
> stmmac_hw_setup(). This does not seems to have any effect on my setup
> (except making it working of course) but I agree this is clearly not
> ideal.
>
> What I could do is adding a function in the miic pcs driver that could
> be called from my rzn1 stmmac probe function to actually configure the
> PCS at probe time based on the detected "phy-mode". Does that seems
> better to you ?
I think Clark Wang is also working on addressing a very similar problem
with stmmac. Please can you check out his work first, he's adding a new
function to phylink to bring the PHY up early in the resume path.
I would like you both to work together to address what seems to be the
same issue.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Le Fri, 10 Feb 2023 11:03:34 +0000,
"Russell King (Oracle)" <[email protected]> a écrit :
> On Tue, Feb 07, 2023 at 03:41:35PM +0100, Clément Léger wrote:
> > Le Mon, 16 Jan 2023 10:53:49 +0000,
> > "Russell King (Oracle)" <[email protected]> a écrit :
> >
> > > On Mon, Jan 16, 2023 at 11:39:23AM +0100, Clément Léger wrote:
> > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > index f2247b8cf0a3..88c941003855 100644
> > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > > > @@ -3818,6 +3818,12 @@ static int __stmmac_open(struct net_device *dev,
> > > > }
> > > > }
> > > >
> > > > + /* We need to setup the phy & PCS before accessing the stmmac registers
> > > > + * because in some cases (RZ/N1), if the stmmac IP is not clocked by the
> > > > + * PCS, hardware init will fail because it lacks a RGMII RX clock.
> > > > + */
> > > > + phylink_start(priv->phylink);
> > >
> > > So what happens if you end up with the mac_link_up method being called
> > > at this point in the driver, before the hardware has been setup ?
> > >
> > > If you use a fixed-link, that's a real possibility.
> >
> > I actually have this setup. On the board, one GMAC is connected to a
> > DSA switch using a fixed-link and the other using the PCS such as added
> > by this series.
> >
> > From what I see, indeed, the mac_link_up() function is called before
> > stmmac_hw_setup(). This does not seems to have any effect on my setup
> > (except making it working of course) but I agree this is clearly not
> > ideal.
> >
> > What I could do is adding a function in the miic pcs driver that could
> > be called from my rzn1 stmmac probe function to actually configure the
> > PCS at probe time based on the detected "phy-mode". Does that seems
> > better to you ?
>
> I think Clark Wang is also working on addressing a very similar problem
> with stmmac. Please can you check out his work first, he's adding a new
> function to phylink to bring the PHY up early in the resume path.
>
> I would like you both to work together to address what seems to be the
> same issue.
>
Acked
--
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com