linux-kernel - Re: [PATCH net v4 3/3] net: stmmac: Specify hardware capability value when FIFO size isn't specified

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: <f343c126-fed9-4209-a18d-61a4e604db2f@arm.com>
Date: Fri, 31 Jan 2025 14:33:28 +0000
From: Steven Price <steven.price@....com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Jose Abreu <joabreu@...opsys.com>, 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>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Russell King <linux@...linux.org.uk>, Yanteng Si <si.yanteng@...ux.dev>,
 Furong Xu <0x1207@...il.com>, Joao Pinto <Joao.Pinto@...opsys.com>,
 netdev@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v4 3/3] net: stmmac: Specify hardware capability value
 when FIFO size isn't specified

On 31/01/2025 14:15, Andrew Lunn wrote:
> On Fri, Jan 31, 2025 at 09:46:41AM +0000, Steven Price wrote:
>> On 27/01/2025 01:38, Kunihiko Hayashi wrote:
>>> When Tx/Rx FIFO size is not specified in advance, the driver checks if
>>> the value is zero and sets the hardware capability value in functions
>>> where that value is used.
>>>
>>> Consolidate the check and settings into function stmmac_hw_init() and
>>> remove redundant other statements.
>>>
>>> If FIFO size is zero and the hardware capability also doesn't have upper
>>> limit values, return with an error message.
>>
>> This patch breaks my Firefly RK3288 board. It appears that all of the 
>> following are true:
>>
>>  * priv->plat->rx_fifo_size == 0
>>  * priv->dma_cap.rx_fifo_size == 0
>>  * priv->plat->tx_fifo_size == 0
>>  * priv->dma_cap.tx_fifo_size == 0
>>
>> Simply removing the "return -ENODEV" lines gets this platform working 
>> again (and AFAICT matches the behaviour before this patch was applied).
>> I'm not sure whether this points to another bug causing these to 
>> all be zero or if this is just an oversight. The below patch gets my 
>> board working:
> 
> Thanks for the quick report of the problem.
> 
> Your 'fix' basically just reverts the patch. Let first try to
> understand what is going on, and fix the patch. We can do a revert
> later if we cannot find a better solution.

Sure thing - I wasn't entirely sure if the patch was just to 'tidy up'
the code (in which case my code keeps the consolidation). I'm not
familiar with this area so I'll let you figure out if there's a better
solution.

> I'm guessing, but in your setup, i assume the value is never written
> to a register, hence 0 is O.K. e.g. dwmac1000_dma_operation_mode_rx(),
> the fifosz value is used to determine if flow control can be used, but
> is otherwise ignored.

I haven't traced the code, but that fits my assumptions too.

> We should determine which versions of stmmac actually need values, and
> limit the test to those versions.

If you want me to try out a patch or do any more investigations then
just let me know.

Thanks,

Steve