Hi Jonas,
On 2/10/25 12:27 AM, Jonas Karlman wrote:
When rgmii-rxid/txid/id phy-mode is used the MAC should not add RX
and/or TX delay. Currently RX/TX delay is configured as enabled using
zero as delay value for the rgmii-rxid/txid/id modes.
Change to disable RX and/or TX delay and using zero as delay value.
Signed-off-by: Jonas Karlman <jo...@kwiboo.se>
---
drivers/net/dwc_eth_qos_rockchip.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dwc_eth_qos_rockchip.c
b/drivers/net/dwc_eth_qos_rockchip.c
index 9fc8c686b889..3e10e07403c1 100644
--- a/drivers/net/dwc_eth_qos_rockchip.c
+++ b/drivers/net/dwc_eth_qos_rockchip.c
@@ -46,6 +46,10 @@ struct rockchip_platform_data {
#define GRF_BIT(nr) (BIT(nr) | BIT((nr) + 16))
#define GRF_CLR_BIT(nr) (BIT((nr) + 16))
+#define DELAY_ENABLE(soc, tx, rx) \
+ (((tx) ? soc##_GMAC_TXCLK_DLY_ENABLE : soc##_GMAC_TXCLK_DLY_DISABLE) | \
+ ((rx) ? soc##_GMAC_RXCLK_DLY_ENABLE : soc##_GMAC_RXCLK_DLY_DISABLE))
+
Considering this won't be used for RK3588 in the next patch and that I
found it obfuscating the constants that are used, can't we simply do...
#define RK3568_GRF_GMAC0_CON0 0x0380
#define RK3568_GRF_GMAC0_CON1 0x0384
#define RK3568_GRF_GMAC1_CON0 0x0388
@@ -85,8 +89,7 @@ static int rk3568_set_to_rgmii(struct udevice *dev,
regmap_write(data->grf, con1,
RK3568_GMAC_PHY_INTF_SEL_RGMII |
- RK3568_GMAC_RXCLK_DLY_ENABLE |
- RK3568_GMAC_TXCLK_DLY_ENABLE);
+ DELAY_ENABLE(RK3568, tx_delay, rx_delay));
(tx_delay ? RK3568_GMAC_TXCLK_DLY_ENABLE : RK3568_GMAC_TXCLK_DLY_DISABLE) |
(rx_delay ? RK3568_GMAC_RXCLK_DLY_ENABLE : RK3568_GMAC_RXCLK_DLY_DISABLE));
instead?
Also, if tx_delay or rx_delay is 0, does it actually make sense to write
the delay config in there? (so the regmap_write before this one).
Note that the config is set before the enable on rk3568, but the
opposite on rk3588 which I find quite odd, you usually want to configure
something before it's enabled?
The change itself looks fine though!
Cheers,
Quentin
