netdev - Re: [PATCH] net: stmmac: Initialize MAC_ONEUS_TIC

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]

Date: Wed, 10 May 2023 01:13:24 +0200
From: Marek Vasut <marex@...x.de>
To: Francesco Dolcini <francesco@...cini.it>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Eric Dumazet <edumazet@...gle.com>,
 Francesco Dolcini <francesco.dolcini@...adex.com>,
 Giuseppe Cavallaro <peppe.cavallaro@...com>, Harald Seiler <hws@...x.de>,
 Jakub Kicinski <kuba@...nel.org>, Jose Abreu <joabreu@...opsys.com>,
 Marcel Ziswiler <marcel.ziswiler@...adex.com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>, Paolo Abeni
 <pabeni@...hat.com>, linux-arm-kernel@...ts.infradead.org,
 linux-stm32@...md-mailman.stormreply.com
Subject: Re: [PATCH] net: stmmac: Initialize MAC_ONEUS_TIC_COUNTER register

On 5/9/23 20:00, Francesco Dolcini wrote:
> On Sun, May 07, 2023 at 01:58:45AM +0200, Marek Vasut wrote:
>> Initialize MAC_ONEUS_TIC_COUNTER register with correct value derived
>> from CSR clock, otherwise EEE is unstable on at least NXP i.MX8M Plus
>> and Micrel KSZ9131RNX PHY, to the point where not even ARP request can
>> be sent out.
>>
>> i.MX 8M Plus Applications Processor Reference Manual, Rev. 1, 06/2021
>> 11.7.6.1.34 One-microsecond Reference Timer (MAC_ONEUS_TIC_COUNTER)
>> defines this register as:
>> "
>> This register controls the generation of the Reference time (1 microsecond
>> tic) for all the LPI timers. This timer has to be programmed by the software
>> initially.
>> ...
>> The application must program this counter so that the number of clock cycles
>> of CSR clock is 1us. (Subtract 1 from the value before programming).
>> For example if the CSR clock is 100MHz then this field needs to be programmed
>> to value 100 - 1 = 99 (which is 0x63).
>> This is required to generate the 1US events that are used to update some of
>> the EEE related counters.
>> "
>>
>> The reset value is 0x63 on i.MX8M Plus, which means expected CSR clock are
>> 100 MHz. However, the i.MX8M Plus "enet_qos_root_clk" are 266 MHz instead,
>> which means the LPI timers reach their count much sooner on this platform.
>>
>> This is visible using a scope by monitoring e.g. exit from LPI mode on TX_CTL
>> line from MAC to PHY. This should take 30us per STMMAC_DEFAULT_TWT_LS setting,
>> during which the TX_CTL line transitions from tristate to low, and 30 us later
>> from low to high. On i.MX8M Plus, this transition takes 11 us, which matches
>> the 30us * 100/266 formula for misconfigured MAC_ONEUS_TIC_COUNTER register.
>>
>> Configure MAC_ONEUS_TIC_COUNTER based on CSR clock, so that the LPI timers
>> have correct 1us reference. This then fixes EEE on i.MX8M Plus with Micrel
>> KSZ9131RNX PHY.
>>
>> Signed-off-by: Marek Vasut <marex@...x.de>
> 
> Reviewed-by: Francesco Dolcini <francesco.dolcini@...adex.com>
> Tested-by: Francesco Dolcini <francesco.dolcini@...adex.com> # Toradex Verdin iMX8MP
> 
> I think this commit should have a fixes tag, what about
> 
> Fixes: 477286b53f55 ("stmmac: add GMAC4 core support")

Fine by me.

>> NOTE: I suspect this can help with Toradex ELB-3757, Marcel, can you please
>>        test this patch on i.MX8M Plus Verdin ?
>>        https://developer-archives.toradex.com/software/linux/release-details?module=Verdin+iMX8M+Plus&key=ELB-3757
> I think you are right, your patch clearly makes a difference here.

Thanks for testing !