On 3/24/25 11:12 AM, Paul Barker wrote:
On 24/03/2025 01:11, Marek Vasut wrote:
On 3/19/25 1:03 PM, Paul Barker wrote:
[...]
+++ b/drivers/net/Kconfig
@@ -864,7 +864,7 @@ config RENESAS_RAVB
select PHY_ETHERNET_ID
help
This driver implements support for the Ethernet AVB block in
- Renesas M3 and H3 SoCs.
+ several Renesas R-Car and RZ SoCs.
RZ/G instead of RZ , right ?
This will also be used for RZ/V2L.
Ahh, thank you for clarifying.
[...]
+static void ravb_mac_init_rzg2l(struct udevice *dev)
+{
+ struct ravb_priv *eth = dev_get_priv(dev);
+
+ setbits_32(eth->iobase + RAVB_REG_ECMR,
+ ECMR_PRM | ECMR_RXF | ECMR_TXF | ECMR_RCPT |
+ ECMR_TE | ECMR_RE | ECMR_RZPF |
+ (eth->phydev->duplex ? ECMR_DM : 0));
Can you configure the ECMR extras in ravb_config() just before
writel(mask, eth->iobase + RAVB_REG_ECMR); based on some private data
flag, like '.is_rzg2l' , instead ?
ravb_config() has been split into ravb_config_rcar() &
ravb_config_rzg2l() by this patch series. So there is no longer a common
write to RAVB_REG_ECMR.
+}
+
/* AVB-DMAC init function */
static int ravb_dmac_init(struct udevice *dev)
{
@@ -459,6 +481,14 @@ static void ravb_dmac_init_rcar(struct udevice *dev)
writel(mode, eth->iobase + RAVB_REG_APSR);
}
+static void ravb_dmac_init_rzg2l(struct udevice *dev)
+{
+ struct ravb_priv *eth = dev_get_priv(dev);
+
+ /* Set Max Frame Length (RTC) */
+ writel(0x7ffc0000 | RFLR_RFL_MIN, eth->iobase + RAVB_REG_RTC);
I assume this register is actually RZ/G2L specific ?
This register also exists on RZ/G2{H,M,N,E}, but in the Linux kernel
ravb driver it is only modified for RZ/G2L.
Is this deliberate or is that a bug ?
+CC Geert.
+}
+
static int ravb_config(struct udevice *dev)
{
struct ravb_device_ops *device_ops =
@@ -501,6 +531,22 @@ static void ravb_config_rcar(struct udevice *dev)
writel(mask, eth->iobase + RAVB_REG_ECMR);
}
+static void ravb_config_rzg2l(struct udevice *dev)
+{
+ struct ravb_priv *eth = dev_get_priv(dev);
+ struct phy_device *phy = eth->phydev;
+
+ writel(CSR0_TPE | CSR0_RPE, eth->iobase + RAVB_REG_CSR0);
+
+ /* Set the transfer speed */
+ if (phy->speed == 10)
+ writel(GECMR_SPEED_10M, eth->iobase + RAVB_REG_GECMR);
Since this patch adds .has_reset flag, wouldn't it be possible to add
another flag called something like .has_10 flag and simply do:
if (eth->has_10 && phy->speed == 10)
writel(GECMR_SPEED_10M, eth->iobase + RAVB_REG_GECMR);
?
The layout of RAVB_REG_GECMR differs between RZ/G2L and RZ/G2{H,M,N,E},
so we can't use the same constants for all platforms. We also need the
additional write to RAVB_REG_CSR0 for RZ/G2L so it's worth having the
ravb_config_rzg2l() function instead of a couple of flags.
Ah, thank you for clarifying. This register layout bit is what I was
missing. The CSR0 part could have been isolated in some if (..), but the
register layout is a bit more unpleasant to deal with.
