On 27/10/2024 16:29, Marek Vasut wrote: > 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...@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.
Will fix in v2. > >> 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. Will fix in v2. > >> + return 0; >> +} >> + >> +static void ravb_reset_assert_rzg2l(struct udevice *dev) >> +{ >> + struct ravb_priv *eth = dev_get_priv(dev); >> + >> + reset_assert(ð->rst); >> + reset_free(ð->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 ? That would probably be an improvement. I'll do that for v2. Thanks, -- Paul Barker
OpenPGP_0x27F4B3459F002257.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature