Re: [PATCH v2 5/5] of: address: Always use dma_default_coherent for default coherency

Jiaxun Yang [off-list ref] writes:

quoted

2023年3月1日 13:06，Christoph Hellwig [off-list ref] 写道：

quoted

- select OF_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE

Doesn't powerpc need to select CONFIG_ARCH_DMA_DEFAULT_COHERENT now,
or even better should be doing that in the patch adding that
symbol?

If I read the code correctly for powerpc OF_DMA_DEFAULT_COHERENT is only selected
with !NOT_COHERENT_CACHE, which means non-coherent dma support is disabled….

I think you're right, but it's not easy to understand.

powerpc's NOT_COHERENT_CACHE selects:

  select ARCH_HAS_DMA_PREP_COHERENT
  select ARCH_HAS_SYNC_DMA_FOR_DEVICE
  select ARCH_HAS_SYNC_DMA_FOR_CPU


Then in your patch 3 you do:

 #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
	defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
	defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
-bool dma_default_coherent;
+bool dma_default_coherent = IS_ENABLED(CONFIG_ARCH_DMA_DEFAULT_COHERENT);
 #endif

So for powerpc if NOT_COHERENT_CACHE=n, then none of those ARCH_HAS
symbols are defined, and so CONFIG_ARCH_DMA_DEFAULT_COHERENT is never used.

But like I said it's not very obvious, and it also seems fragile to
future changes.

So it seems it would be more future proof, and self documenting for
powerpc to just have:

        select ARCH_DMA_DEFAULT_COHERENT if !NOT_COHERENT_CACHE


cheers