[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fb5174e6b774e4ccda1fe274f01601661201e324.camel@denx.de>
Date: Mon, 08 May 2023 15:11:19 +0200
From: Harald Seiler <hws@...x.de>
To: Marek Vasut <marex@...x.de>, netdev@...r.kernel.org
Cc: "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>, 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
Hi Marek,
On Sun, 2023-05-07 at 01:58 +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>
Tested on STM32MP157 with KSZ9131RNX at 1000Mb/s. This patch makes the
network as reliable with EEE active as it is with EEE disabled. So
Tested-by: Harald Seiler <hws@...x.de>
> ---
> NOTE: The hint that this might be related to LPI timer misconfiguration
> came from sending large fragmented ICMP request, i.e.
> ping -4 -c 1 -s 4096 -I eth1 192.168.1.1
> The received packets consistently missed the 1st fragment, because
> the LPI exit time was too short and the first packet was likely
> pushed out of the MAC while the PHY was still not ready for it.
> 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
> NOTE: STM32MP15xx sets 'ethmac' clock to 266.5 MHz, so this patch likely
> helps there as well. The default value of MAC_ONEUS_TIC_COUNTER on
> this platform is also 0x63, i.e. expected 100 MHz CSR clock. I can
> not test this with KSZ9131RNX as I do not have any STM32MP15xx
> board with this PHY. Harald, can you please test ?
> ---
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Francesco Dolcini <francesco.dolcini@...adex.com>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@...com>
> Cc: Harald Seiler <hws@...x.de>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Jose Abreu <joabreu@...opsys.com>
> Cc: Marcel Ziswiler <marcel.ziswiler@...adex.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@...il.com>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-stm32@...md-mailman.stormreply.com
> Cc: netdev@...r.kernel.org
> ---
> drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 1 +
> drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 5 +++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> index 4538f334df576..d3c5306f1c41f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
> @@ -181,6 +181,7 @@ enum power_event {
> #define GMAC4_LPI_CTRL_STATUS 0xd0
> #define GMAC4_LPI_TIMER_CTRL 0xd4
> #define GMAC4_LPI_ENTRY_TIMER 0xd8
> +#define GMAC4_MAC_ONEUS_TIC_COUNTER 0xdc
>
> /* LPI control and status defines */
> #define GMAC4_LPI_CTRL_STATUS_LPITCSE BIT(21) /* LPI Tx Clock Stop Enable */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> index afaec3fb9ab66..03b1c5a97826e 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
> @@ -25,6 +25,7 @@ static void dwmac4_core_init(struct mac_device_info *hw,
> struct stmmac_priv *priv = netdev_priv(dev);
> void __iomem *ioaddr = hw->pcsr;
> u32 value = readl(ioaddr + GMAC_CONFIG);
> + u32 clk_rate;
>
> value |= GMAC_CORE_INIT;
>
> @@ -47,6 +48,10 @@ static void dwmac4_core_init(struct mac_device_info *hw,
>
> writel(value, ioaddr + GMAC_CONFIG);
>
> + /* Configure LPI 1us counter to number of CSR clock ticks in 1us - 1 */
> + clk_rate = clk_get_rate(priv->plat->stmmac_clk);
> + writel((clk_rate / 1000000) - 1, ioaddr + GMAC4_MAC_ONEUS_TIC_COUNTER);
> +
> /* Enable GMAC interrupts */
> value = GMAC_INT_DEFAULT_ENABLE;
>
Powered by blists - more mailing lists