netdev - Re: [PATCH 0/2] net: stmmac: add DT parameter to keep RX

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:   Tue, 24 Jan 2023 11:49:06 +0300
From:   Andrey Konovalov <andrey.konovalov@...aro.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, alexandre.torgue@...s.st.com,
        peppe.cavallaro@...com, joabreu@...opsys.com,
        mcoquelin.stm32@...il.com, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH 0/2] net: stmmac: add DT parameter to keep RX_CLK running
 in LPI state

Hi Andrew,

On 24.01.2023 04:04, Andrew Lunn wrote:
> On Mon, Jan 23, 2023 at 04:37:45PM +0300, Andrey Konovalov wrote:
>> On my qcs404 based board the ethernet MAC has issues with handling
>> Rx LPI exit / Rx LPI entry interrupts.
>>
>> When in LPI mode the "refresh transmission" is received, the driver may
>> see both "Rx LPI exit", and "Rx LPI entry" bits set in the single read from
>> GMAC4_LPI_CTRL_STATUS register (vs "Rx LPI exit" first, and "Rx LPI entry"
>> then). In this case an interrupt storm happens: the LPI interrupt is
>> triggered every few microseconds - with all the status bits in the
>> GMAC4_LPI_CTRL_STATUS register being read as zeros. This interrupt storm
>> continues until a normal non-zero status is read from GMAC4_LPI_CTRL_STATUS
>> register (single "Rx LPI exit", or "Tx LPI exit").
>>
>> The reason seems to be in the hardware not being able to properly clear
>> the "Rx LPI exit" interrupt if GMAC4_LPI_CTRL_STATUS register is read
>> after Rx LPI mode is entered again.
>>
>> The current driver unconditionally sets the "Clock-stop enable" bit
>> (bit 10 in PHY's PCS Control 1 register) when calling phy_init_eee().
>> Not setting this bit - so that the PHY continues to provide RX_CLK
>> to the ethernet controller during Rx LPI state - prevents the LPI
>> interrupt storm.
>>
>> This patch set adds a new parameter to the stmmac DT:
>> snps,rx-clk-runs-in-lpi.
>> If this parameter is present in the device tree, the driver configures
>> the PHY not to stop RX_CLK after entering Rx LPI state.
> 
> Do we really need yet another device tree parameter?

Indeed, there are quite a lot of them already (as this is complex and 
highly configurable device).

> Could
> dwmac-qcom-ethqos.c just do this unconditionally?

Never stopping RX_CLK in Rx LPI state would always work, but the power 
consumption would somewhat increase (in Rx LPI state). Some people do 
care about it.

> Is the interrupt
> controller part of the licensed IP, or is it from QCOM? If it is part
> of the licensed IP, it is probably broken for other devices as well,
> so maybe it should be a quirk for all devices of a particular version
> of the IP?

Most probably this is the part of the ethernet MAC IP. And this is quite 
possible that the issue is specific for particular versions of the IP.
Unfortunately I don't have the documentation related to this particular 
issue. And don't have the test results for different IP versions. It 
looks like testing Energy Efficient Ethernet (EEE) support isn't very 
common (yes, it is enabled by default in the stmmac driver, but if the 
ethernet switch the device is connected to doesn't support EEE then the 
issue wouldn't reveal).

Thanks,
Andrey

>     Andrew