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

[Marek Vasut]

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 at 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,
> +};
[...]