| Message ID | 20250306203858.1677595-1-jonas@kwiboo.se (mailing list archive) |
|---|---|
| Headers | show |
| Series | Use DELAY_ENABLE macro for RK3328, RK3566/RK3568 and RK3588 | expand |
Hello Jonas, On 2025-03-06 21:38, Jonas Karlman wrote: > Almost all Rockchip GMAC variants use the DELAY_ENABLE macro to help > enable or disable use of MAC rx/tx delay. However, RK3328, > RK3566/RK3568 > and RK3588 GMAC driver does not. > > Use of the DELAY_ENABLE macro help ensure the MAC rx/tx delay is > disabled, instead of being enabled and using a zero delay, when > RGMII_ID/RXID/TXID is used. > > RK3328 driver was merged around the same time as when DELAY_ENABLE was > introduced so it is understandable why it was missed. Both > RK3566/RK3568 > and RK3588 support were introduced much later yet they also missed > using > the DELAY_ENABLE macro (so did vendor kernel at that time). > > This series fixes all these cases to unify how GMAC delay feature is > enabled or disabled across the different GMAC variants. > > Jonas Karlman (3): > net: stmmac: dwmac-rk: Use DELAY_ENABLE macro for RK3328 > net: stmmac: dwmac-rk: Use DELAY_ENABLE macro for RK3566/RK3568 > net: stmmac: dwmac-rk: Use DELAY_ENABLE macro for RK3588 > > .../net/ethernet/stmicro/stmmac/dwmac-rk.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) As far as I can tell, the RV1126 GMAC should also be converted to use the DELAY_ENABLE macro, which the vendor kernel already does. [*] Perhaps that could be performed in new patch 4/4 in this series? BTW, it would be quite neat to introduce the DELAY_VALUE macro, which makes the function calls a bit more compact. [*] [*] https://raw.githubusercontent.com/rockchip-linux/kernel/refs/heads/develop-5.10/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
Hi Dragan, On 2025-03-06 22:41, Dragan Simic wrote: > Hello Jonas, > > On 2025-03-06 21:38, Jonas Karlman wrote: >> Almost all Rockchip GMAC variants use the DELAY_ENABLE macro to help >> enable or disable use of MAC rx/tx delay. However, RK3328, >> RK3566/RK3568 >> and RK3588 GMAC driver does not. >> >> Use of the DELAY_ENABLE macro help ensure the MAC rx/tx delay is >> disabled, instead of being enabled and using a zero delay, when >> RGMII_ID/RXID/TXID is used. >> >> RK3328 driver was merged around the same time as when DELAY_ENABLE was >> introduced so it is understandable why it was missed. Both >> RK3566/RK3568 >> and RK3588 support were introduced much later yet they also missed >> using >> the DELAY_ENABLE macro (so did vendor kernel at that time). >> >> This series fixes all these cases to unify how GMAC delay feature is >> enabled or disabled across the different GMAC variants. >> >> Jonas Karlman (3): >> net: stmmac: dwmac-rk: Use DELAY_ENABLE macro for RK3328 >> net: stmmac: dwmac-rk: Use DELAY_ENABLE macro for RK3566/RK3568 >> net: stmmac: dwmac-rk: Use DELAY_ENABLE macro for RK3588 >> >> .../net/ethernet/stmicro/stmmac/dwmac-rk.c | 19 ++++++++++--------- >> 1 file changed, 10 insertions(+), 9 deletions(-) > > As far as I can tell, the RV1126 GMAC should also be converted to use > the DELAY_ENABLE macro, which the vendor kernel already does. [*] > Perhaps > that could be performed in new patch 4/4 in this series? Good catch, the RV1126 seem to use a slight different M0/M1 variant and unfortunately I do not have access to any RV1126 board to do any runtime testing. For RK3328, RK356x and RK3588 I could at least do runtime testing. I can add an untested patch for RV1126 in a v2 if needed :-) > > BTW, it would be quite neat to introduce the DELAY_VALUE macro, which > makes the function calls a bit more compact. [*] Personally, I do not see a real need for the DELAY_VALUE macro, current use of the soc_GMAC_CLK_yX_DL_CFG() macro is readable enough. The ENABLE_DELAY at least helps remove an otherwise more complex enable or disable conditional handling. Regards, Jonas > > [*] > https://raw.githubusercontent.com/rockchip-linux/kernel/refs/heads/develop-5.10/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c