Hello everyone,
This is version four of my series that adds support for a Gigabit Ethernet
controller featured in the Renesas r9a06g032 SoC, of the RZ/N1 family. This
GMAC device is based on a Synopsys IP and is compatible with the stmmac driver.
My former colleague Clément Léger originally sent a series for this driver,
but an issue in bringing up the PCS clock had blocked the upstreaming
process. This issue has since been resolved by the following series:
https://lore.kernel.org/all/[email protected]/
This series consists of a devicetree binding describing the RZN1 GMAC
controller IP, a node for the GMAC1 device in the r9a06g032 SoC device
tree, and the GMAC driver itself which is a glue layer in stmmac.
There are also two patches by Russell that improve pcs initialization handling
in stmmac.
Best Regards,
Romain Gantois
---
Changes in v4:
- Removed the second parameters of the new pcs_init/exit() callbacks
- Removed unnecessary interrupt-parent reference in gmac1 device node
- Link to v3: https://lore.kernel.org/r/[email protected]
Changes in v3:
- Fixed a typo in the socfpga patch
- Link to v2: https://lore.kernel.org/r/[email protected]
Changes in v2:
- Add pcs_init/exit callbacks in stmmac to solve race condition
- Use pcs_init/exit callbacks in dwmac_socfpga glue layer
- Miscellaneous device tree binding corrections
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Clément Léger (3):
dt-bindings: net: renesas,rzn1-gmac: Document RZ/N1 GMAC support
net: stmmac: add support for RZ/N1 GMAC
ARM: dts: r9a06g032: describe GMAC1
Russell King (Oracle) (2):
net: stmmac: introduce pcs_init/pcs_exit stmmac operations
net: stmmac: dwmac-socfpga: use pcs_init/pcs_exit
.../devicetree/bindings/net/renesas,rzn1-gmac.yaml | 66 +++++++++++++
MAINTAINERS | 6 ++
arch/arm/boot/dts/renesas/r9a06g032.dtsi | 18 ++++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 +++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c | 86 +++++++++++++++++
.../net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 107 ++++++++++-----------
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 +++
include/linux/stmmac.h | 2 +
9 files changed, 258 insertions(+), 54 deletions(-)
---
base-commit: 1c04b46cbdddc7882eeb671521035ea884245b9f
change-id: 20240402-rzn1-gmac1-685cf8793d0e
Best regards,
--
Romain Gantois <[email protected]>
From: "Russell King (Oracle)" <[email protected]>
Introduce a mechanism whereby platforms can create their PCS instances
prior to the network device being published to userspace, but after
some of the core stmmac initialisation has been completed. This means
that the data structures that platforms need will be available.
Signed-off-by: Russell King (Oracle) <[email protected]>
Reviewed-by: Maxime Chevallier <[email protected]>
[rgantois: removed second parameters of new callbacks]
Signed-off-by: Romain Gantois <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++++++++
include/linux/stmmac.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 59bf83904b62d..bee9c9ab31a88 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7200,6 +7200,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
if (ret)
return ret;
+ if (priv->plat->pcs_init) {
+ ret = priv->plat->pcs_init(priv);
+ if (ret)
+ return ret;
+ }
+
/* Get the HW capability (new GMAC newer than 3.50a) */
priv->hw_cap_support = stmmac_get_hw_features(priv);
if (priv->hw_cap_support) {
@@ -7282,6 +7288,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
return 0;
}
+static void stmmac_hw_exit(struct stmmac_priv *priv)
+{
+ if (priv->plat->pcs_exit)
+ priv->plat->pcs_exit(priv);
+}
+
static void stmmac_napi_add(struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
@@ -7795,6 +7807,7 @@ int stmmac_dvr_probe(struct device *device,
priv->hw->pcs != STMMAC_PCS_RTBI)
stmmac_mdio_unregister(ndev);
error_mdio_register:
+ stmmac_hw_exit(priv);
stmmac_napi_del(ndev);
error_hw_init:
destroy_workqueue(priv->wq);
@@ -7835,6 +7848,7 @@ void stmmac_dvr_remove(struct device *dev)
if (priv->hw->pcs != STMMAC_PCS_TBI &&
priv->hw->pcs != STMMAC_PCS_RTBI)
stmmac_mdio_unregister(ndev);
+ stmmac_hw_exit(priv);
destroy_workqueue(priv->wq);
mutex_destroy(&priv->lock);
bitmap_free(priv->af_xdp_zc_qps);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index dfa1828cd756a..4a24a246c617d 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -285,6 +285,8 @@ struct plat_stmmacenet_data {
int (*crosststamp)(ktime_t *device, struct system_counterval_t *system,
void *ctx);
void (*dump_debug_regs)(void *priv);
+ int (*pcs_init)(struct stmmac_priv *priv);
+ void (*pcs_exit)(struct stmmac_priv *priv);
void *bsp_priv;
struct clk *stmmac_clk;
struct clk *pclk;
--
2.44.0
Hi Romain
On Wed, Apr 24, 2024 at 11:06:20AM +0200, Romain Gantois wrote:
> From: "Russell King (Oracle)" <[email protected]>
>
> Introduce a mechanism whereby platforms can create their PCS instances
> prior to the network device being published to userspace, but after
> some of the core stmmac initialisation has been completed. This means
> that the data structures that platforms need will be available.
>
> Signed-off-by: Russell King (Oracle) <[email protected]>
> Reviewed-by: Maxime Chevallier <[email protected]>
> [rgantois: removed second parameters of new callbacks]
> Signed-off-by: Romain Gantois <[email protected]>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++++++++
> include/linux/stmmac.h | 2 ++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 59bf83904b62d..bee9c9ab31a88 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -7200,6 +7200,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
> if (ret)
> return ret;
>
> + if (priv->plat->pcs_init) {
> + ret = priv->plat->pcs_init(priv);
> + if (ret)
> + return ret;
> + }
> +
Once again. There is a ready-to-use stmmac_xpcs_setup() method. Which
is currently intended for the XPCS setups. Let's collect all the
PCS-related stuff in a single place there. That will make code cleaner
and easier to read. This was discussed on v3:
https://lore.kernel.org/netdev/42chuecdt7dpgm6fcrtt2crifvv5hflmtnmdrw5fvk3r7pwjgu@hlcv56dbeosf/
You agreed to do that, but just ignored in result. I'll repeat what I
said in v3:
On Tue, 16 Apr 2024 16:41:33 +0300, Serge Semin wrote:
> I am currently working on my Memory-mapped DW XPCS patchset cooking:
> https://lore.kernel.org/netdev/[email protected]/
> The changes in this series seems to intersect to what is/will be
> introduced in my patchset. In particular as before I am going to
> use the "pcs-handle" property for getting the XPCS node. If so what
> about collecting PCS-related things in a single place. Like this:
>
> int stmmac_xpcs_setup(struct net_device *ndev)
> {
> ...
>
> if (priv->plat->pcs_init) {
> return priv->plat->pcs_init(priv); /* Romain' part */
> } else if (fwnode_property_present(priv->plat->port_node, "pcs-handle")) {
> /* My DW XPCS part */
> } else if (priv->plat->mdio_bus_data && priv->plat->mdio_bus_data->has_xpcs) {
> /* Currently implemented procedure */
> }
>
> ...
> }
>
> void stmmac_xpcs_clean(struct net_device *ndev)
> {
> ...
>
> if (priv->plat->pcs_exit) {
> priv->plat->pcs_exit(priv);
> return;
>
> }
>
> xpcs_destroy(priv->hw->xpcs);
> priv->hw->xpcs = NULL;
> }
>
> Please see the last two patches in my series:
> https://lore.kernel.org/netdev/[email protected]/
> https://lore.kernel.org/netdev/[email protected]/
> as a reference of how the changes could be provided.
You replied it was a good idea, but the function names should be
renamed. That's not a problem. Just create a pre-requisite patch which
does that. So the patch in the subject could be replaced with four
subsequent patches:
1. Move the conditional XPCS-setup execution into the
stmmac_xpcs_setup() method. This change is partly implemented here
https://lore.kernel.org/netdev/[email protected]/
2. Rename stmmac_xpcs_setup() method to just stmmac_pcs_setup() as a
preparation before adding the platform-specific PCS init()/exit()
callbacks.
3. Introduce the PCS-cleanup method. You can pick it up from here, but
use the stmmac_pcs_clean() name:
https://lore.kernel.org/netdev/[email protected]/
4. Add pcc_init()/pcs_exit() callbacks as it's done in this patch but
call them in the stmmac_pcs_setup()/stmmac_pcs_clean() methods
instead of open-coding in the more generic
stmmac_hw_init()/stmmac_hw_exit() functions.
It doesn't look as that much hard thing to do, but will cause having a
better readable code by providing a single coherent function for all
PCS'es.
-Serge(y)
> /* Get the HW capability (new GMAC newer than 3.50a) */
> priv->hw_cap_support = stmmac_get_hw_features(priv);
> if (priv->hw_cap_support) {
> @@ -7282,6 +7288,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
> return 0;
> }
>
> +static void stmmac_hw_exit(struct stmmac_priv *priv)
> +{
> + if (priv->plat->pcs_exit)
> + priv->plat->pcs_exit(priv);
> +}
> +
>
> [...]
Hi Serge,
On Wed, 24 Apr 2024, Serge Semin wrote:
> Once again. There is a ready-to-use stmmac_xpcs_setup() method. Which
> is currently intended for the XPCS setups. Let's collect all the
> PCS-related stuff in a single place there. That will make code cleaner
> and easier to read. This was discussed on v3:
>
> https://lore.kernel.org/netdev/42chuecdt7dpgm6fcrtt2crifvv5hflmtnmdrw5fvk3r7pwjgu@hlcv56dbeosf/
>
> You agreed to do that, but just ignored in result. I'll repeat what I
> said in v3:
Yeah sorry I took a quick look at your merged patches and thought that
stmmac_xpcs_setup() had been repurposed in the meantime, but it seems like I was
just confused about that.
> It doesn't look as that much hard thing to do, but will cause having a
> better readable code by providing a single coherent function for all
> PCS'es.
Sure, I'll get to it in v5.
Thanks,
--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Wed, Apr 24, 2024 at 06:33:30PM +0200, Romain Gantois wrote:
> Hi Serge,
>
> On Wed, 24 Apr 2024, Serge Semin wrote:
>
> > Once again. There is a ready-to-use stmmac_xpcs_setup() method. Which
> > is currently intended for the XPCS setups. Let's collect all the
> > PCS-related stuff in a single place there. That will make code cleaner
> > and easier to read. This was discussed on v3:
> >
> > https://lore.kernel.org/netdev/42chuecdt7dpgm6fcrtt2crifvv5hflmtnmdrw5fvk3r7pwjgu@hlcv56dbeosf/
> >
> > You agreed to do that, but just ignored in result. I'll repeat what I
> > said in v3:
>
> Yeah sorry I took a quick look at your merged patches and thought that
> stmmac_xpcs_setup() had been repurposed in the meantime, but it seems like I was
> just confused about that.
>
> > It doesn't look as that much hard thing to do, but will cause having a
> > better readable code by providing a single coherent function for all
> > PCS'es.
>
> Sure, I'll get to it in v5.
Awesome! Thanks.
-Serge(y)
>
> Thanks,
>
> --
> Romain Gantois, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com