Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] ARM: mvebu: dt64 for v7.2 (#1)
@ 2026-06-05 15:20 Gregory CLEMENT
  2026-06-09 16:11 ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Gregory CLEMENT @ 2026-06-05 15:20 UTC (permalink / raw)
  To: Arnd Bergmann, arm, soc
  Cc: Andrew Lunn, Sebastian Hesselbarth, linux-arm-kernel

Hi,

Here is the first pull request for dt64 for mvebu for v7.2.

Gregory

The following changes since commit 254f49634ee16a731174d2ae34bc50bd5f45e731:

  Linux 7.1-rc1 (2026-04-26 14:19:00 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gclement/mvebu.git tags/mvebu-dt64-7.2-1

for you to fetch changes up to 5dd98e0389545b302b0c46cef2264b5b09c01677:

  arm64: dts: marvell: armada-37xx: mark EIP97 as dma-coherent (2026-06-01 10:04:36 +0200)

----------------------------------------------------------------
mvebu dt64 for 7.2 (part 1)

Mark EIP97 as dma-coherent for Armada 3720

----------------------------------------------------------------
Aleksander Jan Bajkowski (1):
      arm64: dts: marvell: armada-37xx: mark EIP97 as dma-coherent

 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 1 +
 1 file changed, 1 insertion(+)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] ARM: mvebu: dt64 for v7.2 (#1)
  2026-06-05 15:20 [GIT PULL] ARM: mvebu: dt64 for v7.2 (#1) Gregory CLEMENT
@ 2026-06-09 16:11 ` Arnd Bergmann
  2026-06-09 17:35   ` Aleksander Jan Bajkowski
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2026-06-09 16:11 UTC (permalink / raw)
  To: Gregory Clement, arm, soc
  Cc: Andrew Lunn, Sebastian Hesselbarth, linux-arm-kernel,
	Aleksander Jan Bajkowski

On Fri, Jun 5, 2026, at 17:20, Gregory CLEMENT wrote:
>
> ----------------------------------------------------------------
> mvebu dt64 for 7.2 (part 1)
>
> Mark EIP97 as dma-coherent for Armada 3720
>
> ----------------------------------------------------------------
> Aleksander Jan Bajkowski (1):
>       arm64: dts: marvell: armada-37xx: mark EIP97 as dma-coherent

Hi Gregory and Aleksander,

I'm a bit surprised by this oneline change. Since you successfully tested
this, I assume the change is correct, but I have two questions that
I would like to have an answer for before I pull it.

- I would expect a missing 'dma-coherent' property to cause data
  corruption, as the DMA master may write directly into the L2
  cache, which is then invalidated before the CPU accesses it.
  Do you have any idea how this one ends up working even when
  the property is missing?

- I see that the Product Brief for Armada 37xx mentions that it
  has a "High-bandwidth, low-latency IO Cache Coherency" interconnect,
  which also indicates that the patch is correct. However I don't
  see why it's only the crypto engine that needs it. What about
  the other high-speed DMA masters (neta, xhci, pcie, sata, ...)?

       Arnd


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] ARM: mvebu: dt64 for v7.2 (#1)
  2026-06-09 16:11 ` Arnd Bergmann
@ 2026-06-09 17:35   ` Aleksander Jan Bajkowski
  2026-06-09 19:29     ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Aleksander Jan Bajkowski @ 2026-06-09 17:35 UTC (permalink / raw)
  To: Arnd Bergmann, Gregory Clement, arm, soc
  Cc: Andrew Lunn, Sebastian Hesselbarth, linux-arm-kernel

Hi Arnd,

On 09/06/2026 18:11, Arnd Bergmann wrote:
> On Fri, Jun 5, 2026, at 17:20, Gregory CLEMENT wrote:
>> ----------------------------------------------------------------
>> mvebu dt64 for 7.2 (part 1)
>>
>> Mark EIP97 as dma-coherent for Armada 3720
>>
>> ----------------------------------------------------------------
>> Aleksander Jan Bajkowski (1):
>>        arm64: dts: marvell: armada-37xx: mark EIP97 as dma-coherent
> Hi Gregory and Aleksander,
>
> I'm a bit surprised by this oneline change. Since you successfully tested
> this, I assume the change is correct, but I have two questions that
> I would like to have an answer for before I pull it.
By the way, the upstream safexcel driver works correctly only on coherent
platforms. On non-coherent platforms (MediaTek), the SHA-384 and SHA-512
selftests fail. Since the selftests pass on Armada's SoC, I assume I'm 
right.
I have a plan to send a patch upstream, which has long been maintained
downstream in OpenWRT[1]. But I need to think a bit more about how to do
this properly.
[1] 
https://github.com/openwrt/openwrt/blob/main/target/linux/mediatek/patches-6.18/401-crypto-fix-eip97-cache-incoherent.patch 

>
> - I would expect a missing 'dma-coherent' property to cause data
>    corruption, as the DMA master may write directly into the L2
>    cache, which is then invalidated before the CPU accesses it.
>    Do you have any idea how this one ends up working even when
>    the property is missing?
No idea. Don't have access the Armada SoC TRM. Maybe the folks at
Marvel will be able to explain it.
>
> - I see that the Product Brief for Armada 37xx mentions that it
>    has a "High-bandwidth, low-latency IO Cache Coherency" interconnect,
>    which also indicates that the patch is correct. However I don't
>    see why it's only the crypto engine that needs it. What about
>    the other high-speed DMA masters (neta, xhci, pcie, sata, ...)?
I didn't test to determine whether the other DMA masters are coherent.
But I'm assuming you're correct and they are also coherent. My recent
work has been focused on improving the Rambus/Verimatrix/Safenet crypto
drivers :)

Best regards,
Aleksander


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] ARM: mvebu: dt64 for v7.2 (#1)
  2026-06-09 17:35   ` Aleksander Jan Bajkowski
@ 2026-06-09 19:29     ` Arnd Bergmann
  2026-06-11 13:30       ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2026-06-09 19:29 UTC (permalink / raw)
  To: Aleksander Jan Bajkowski, Gregory Clement, arm, soc
  Cc: Andrew Lunn, Sebastian Hesselbarth, linux-arm-kernel

On Tue, Jun 9, 2026, at 19:35, Aleksander Jan Bajkowski wrote:
> On 09/06/2026 18:11, Arnd Bergmann wrote:
>> I'm a bit surprised by this oneline change. Since you successfully tested
>> this, I assume the change is correct, but I have two questions that
>> I would like to have an answer for before I pull it.
> By the way, the upstream safexcel driver works correctly only on 
> coherent
> platforms. On non-coherent platforms (MediaTek), the SHA-384 and SHA-512
> selftests fail. Since the selftests pass on Armada's SoC, I assume I'm 
> right.

It's not necessarily proof that this is correct, but it is quite likely.

After checking the datasheet some more and finding that this should
indeed be coherent everywhere, I remembered that even the old
32-bit Armada 370 had a coherency manager. At the time, we used a hack
in  arch/arm/mach-mvebu/coherency.c to mark all device nodes as coherent,
since the original DTB did not contain the correct annotations.

I suspect that the Armada 37xx started out with a copy of the
old DT files and also never had the annotation, but then never
had the same hack because arch/arm64 does not have platform
specific code.

> I have a plan to send a patch upstream, which has long been maintained
> downstream in OpenWRT[1]. But I need to think a bit more about how to do
> this properly.
> [1] 
> https://github.com/openwrt/openwrt/blob/main/target/linux/mediatek/patches-6.18/401-crypto-fix-eip97-cache-incoherent.patch 

The patch is basically correct, I think you should just change two
details:

- instead of defining your own SYSTEM_CACHELINE_SIZE macro, just use
  the existing CRYPTO_DMA_ALIGN macro that is used in crypto_dma_align

- move the 'state[]' and 'cache[]' arrays to the beginning of
  safexcel_ahash_req so you don't have to manually align them.
  
>> - I would expect a missing 'dma-coherent' property to cause data
>>    corruption, as the DMA master may write directly into the L2
>>    cache, which is then invalidated before the CPU accesses it.
>>    Do you have any idea how this one ends up working even when
>>    the property is missing?
> No idea. Don't have access the Armada SoC TRM. Maybe the folks at
> Marvel will be able to explain it.

ok

        Arnd


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] ARM: mvebu: dt64 for v7.2 (#1)
  2026-06-09 19:29     ` Arnd Bergmann
@ 2026-06-11 13:30       ` Arnd Bergmann
  2026-06-12 13:45         ` Gregory CLEMENT
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2026-06-11 13:30 UTC (permalink / raw)
  To: Aleksander Jan Bajkowski, Gregory Clement, arm, soc
  Cc: Andrew Lunn, Sebastian Hesselbarth, linux-arm-kernel

On Tue, Jun 9, 2026, at 21:29, Arnd Bergmann wrote:
> On Tue, Jun 9, 2026, at 19:35, Aleksander Jan Bajkowski wrote:
>> On 09/06/2026 18:11, Arnd Bergmann wrote:
>>> I'm a bit surprised by this oneline change. Since you successfully tested
>>> this, I assume the change is correct, but I have two questions that
>>> I would like to have an answer for before I pull it.
>> By the way, the upstream safexcel driver works correctly only on 
>> coherent
>> platforms. On non-coherent platforms (MediaTek), the SHA-384 and SHA-512
>> selftests fail. Since the selftests pass on Armada's SoC, I assume I'm 
>> right.
>
> It's not necessarily proof that this is correct, but it is quite likely.
>
> After checking the datasheet some more and finding that this should
> indeed be coherent everywhere, I remembered that even the old
> 32-bit Armada 370 had a coherency manager. At the time, we used a hack
> in  arch/arm/mach-mvebu/coherency.c to mark all device nodes as coherent,
> since the original DTB did not contain the correct annotations.
>
> I suspect that the Armada 37xx started out with a copy of the
> old DT files and also never had the annotation, but then never
> had the same hack because arch/arm64 does not have platform
> specific code.

After investigating a little more, I think the correct fix here
will be to mark all DMA masters in this SoC as dma-coherent.
I thought there was a way to do this for an entire system,
but I could not find that, so this likely has to be done
for each DMA master separately.

Not sure who still has the hardware and has time to
test this properly. Given that the incorrect DT has
existed for over 10 years now, I assume this is not
urgent and I will skip the pull request for 7.2.

     Arnd


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] ARM: mvebu: dt64 for v7.2 (#1)
  2026-06-11 13:30       ` Arnd Bergmann
@ 2026-06-12 13:45         ` Gregory CLEMENT
  0 siblings, 0 replies; 6+ messages in thread
From: Gregory CLEMENT @ 2026-06-12 13:45 UTC (permalink / raw)
  To: Arnd Bergmann, Aleksander Jan Bajkowski, arm, soc
  Cc: Andrew Lunn, Sebastian Hesselbarth, linux-arm-kernel

Hello Arnd,

> On Tue, Jun 9, 2026, at 21:29, Arnd Bergmann wrote:
>> On Tue, Jun 9, 2026, at 19:35, Aleksander Jan Bajkowski wrote:
>>> On 09/06/2026 18:11, Arnd Bergmann wrote:
>>>> I'm a bit surprised by this oneline change. Since you successfully tested
>>>> this, I assume the change is correct, but I have two questions that
>>>> I would like to have an answer for before I pull it.
>>> By the way, the upstream safexcel driver works correctly only on 
>>> coherent
>>> platforms. On non-coherent platforms (MediaTek), the SHA-384 and SHA-512
>>> selftests fail. Since the selftests pass on Armada's SoC, I assume I'm 
>>> right.
>>
>> It's not necessarily proof that this is correct, but it is quite likely.
>>
>> After checking the datasheet some more and finding that this should
>> indeed be coherent everywhere, I remembered that even the old
>> 32-bit Armada 370 had a coherency manager. At the time, we used a hack
>> in  arch/arm/mach-mvebu/coherency.c to mark all device nodes as coherent,
>> since the original DTB did not contain the correct annotations.
>>
>> I suspect that the Armada 37xx started out with a copy of the
>> old DT files and also never had the annotation, but then never
>> had the same hack because arch/arm64 does not have platform
>> specific code.
>
> After investigating a little more, I think the correct fix here
> will be to mark all DMA masters in this SoC as dma-coherent.
> I thought there was a way to do this for an entire system,
> but I could not find that, so this likely has to be done
> for each DMA master separately.

Thanks for doing this research. I also checked the data in the datasheet
before applying the patch, and it appears the platform is coherent. I
was surprised that when we initially submitted the support request,
Marvel didn't mention this; usually, SoC vendors like to have good
performance numbers. However, they also didn't say anything about a
coherence issue. The fact that the test succeeded is a good indicator
that the SoC is indeed coherent.
>
> Not sure who still has the hardware and has time to
> test this properly. Given that the incorrect DT has
> existed for over 10 years now, I assume this is not
> urgent and I will skip the pull request for 7.2.

I believe I still have a board based on an Armada 3700, and I should be
able to find time to do some tests.

Gregory

>
>      Arnd

-- 
Grégory CLEMENT, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-12 13:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-05 15:20 [GIT PULL] ARM: mvebu: dt64 for v7.2 (#1) Gregory CLEMENT
2026-06-09 16:11 ` Arnd Bergmann
2026-06-09 17:35   ` Aleksander Jan Bajkowski
2026-06-09 19:29     ` Arnd Bergmann
2026-06-11 13:30       ` Arnd Bergmann
2026-06-12 13:45         ` Gregory CLEMENT

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox