linux-kernel - Re: [PATCH 0/3] Use DELAY_ENABLE macro for RK3328, RK3566/RK3568 and RK3588

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3ab13afd-0a9f-485e-bdf2-7b413d418065@kwiboo.se>
Date: Thu, 6 Mar 2025 23:07:23 +0100
From: Jonas Karlman <jonas@...boo.se>
To: Dragan Simic <dsimic@...jaro.org>
Cc: Heiko Stuebner <heiko@...ech.de>, Andrew Lunn <andrew+netdev@...n.ch>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
 linux-rockchip@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] Use DELAY_ENABLE macro for RK3328, RK3566/RK3568 and
 RK3588

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