Re: [PATCH 11/14] net: ravb: Add RZ/G2L Support

On 10/24/24 5:24 PM, Paul Barker wrote:

The Renesas R9A07G044L (RZ/G2L) SoC includes two Gigabit Ethernet interfaces which can be supported using the ravb driver. Some RZ/G2L specific steps need to be taken during initialization due to differences between this SoC and previously supported SoCs. We also need to ensure that the module reset is de-asserted after the module clock is enabled but before any Ethernet register reads/writes take place.

Signed-off-by: Paul Barker paul.barker.ct@bp.renesas.com

arch/arm/mach-renesas/Kconfig | 1 + drivers/net/Kconfig | 2 + drivers/net/ravb.c | 183 ++++++++++++++++++++++++++++++++-- 3 files changed, 176 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-renesas/Kconfig b/arch/arm/mach-renesas/Kconfig index aeb55da609bd..d373ab56ce91 100644 --- a/arch/arm/mach-renesas/Kconfig +++ b/arch/arm/mach-renesas/Kconfig @@ -76,6 +76,7 @@ config RZG2L imply MULTI_DTB_FIT imply MULTI_DTB_FIT_USER_DEFINED_AREA imply PINCTRL_RZG2L

• imply RENESAS_RAVB imply RENESAS_SDHI imply RZG2L_GPIO imply SCIF_CONSOLE

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 89f7411bdf33..d009acdcd94f 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -822,6 +822,8 @@ config RENESAS_RAVB depends on RCAR_64 select PHYLIB select PHY_ETHERNET_ID

• select BITBANGMII
• select BITBANGMII_MULTI

Keep the list sorted.

help This driver implements support for the Ethernet AVB block in Renesas M3 and H3 SoCs. diff --git a/drivers/net/ravb.c b/drivers/net/ravb.c index fb869cd0872e..e2ab929858c8 100644 --- a/drivers/net/ravb.c +++ b/drivers/net/ravb.c

[...]

@@ -108,6 +122,16 @@

#define RAVB_TX_TIMEOUT_MS 1000

+#define RAVB_RCV_BUFF_MAX 8192

+struct ravb_device_ops {

• int (*mac_init)(struct udevice *dev);
• int (*dmac_init)(struct udevice *dev);
• int (*config)(struct udevice *dev);
• int (*reset_deassert)(struct udevice *dev);
• void (*reset_assert)(struct udevice *dev);

+};

[...]

+static int ravb_reset_deassert_rcar(struct udevice *dev) +{

The callsites should check if a callback is assigned or NULL and only call the callback if it is assigned. Then you won't need empty callbacks like this.

Basically add if (ops->reset_deassert) ops->reset_deassert() and remove this empty function.

• return 0;

+}

+static void ravb_reset_assert_rzg2l(struct udevice *dev) +{

• struct ravb_priv *eth = dev_get_priv(dev);
• reset_assert(&eth->rst);
• reset_free(&eth->rst);

+}

A bit of a design question -- would it make sense to have ravb-rcar.c and ravb-rzg2l.c to contain the differences between the ravb variants, and keep common code only in ravb.c ?

[...]

@@ -684,9 +821,35 @@ int ravb_of_to_plat(struct udevice *dev) return 0; }

+static const struct ravb_device_ops ravb_device_ops_rzg2l = {

Keep the list sorted, rzg is below rcar (Z is after C) .

• .mac_init = ravb_mac_init_rzg2l,
• .dmac_init = ravb_dmac_init_rzg2l,
• .config = ravb_config_rzg2l,
• .reset_deassert = ravb_reset_deassert_rzg2l,
• .reset_assert = ravb_reset_assert_rzg2l,

+};

+static const struct ravb_device_ops ravb_device_ops_rcar = {

• .mac_init = ravb_mac_init_rcar,
• .dmac_init = ravb_dmac_init_rcar,
• .config = ravb_config_rcar,
• .reset_deassert = ravb_reset_deassert_rcar,
• .reset_assert = ravb_reset_assert_rcar,

+};

[...]