As for now all arches have dma_default_coherent matched with default
DMA coherency for of devices, so there is no need to have a standalone
config option.

Note for PowerPC: CONFIG_OF_DMA_DEFUALT_COHERENT was only selected when
CONFIG_NOT_COHERENT_CACHE is false, in this case dma_default_coherent will
be true, so it still matches present behavior.

Note for RISCV: dma_default_coherent is set to true in this series.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/powerpc/Kconfig |  1 -
 arch/riscv/Kconfig   |  1 -
 drivers/of/Kconfig   |  4 ----
 drivers/of/address.c | 10 +---------
 4 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2c9cdf1d8761..c67e5da714f7 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -272,7 +272,6 @@ config PPC
 	select NEED_PER_CPU_PAGE_FIRST_CHUNK	if PPC64
 	select NEED_SG_DMA_LENGTH
 	select OF
-	select OF_DMA_DEFAULT_COHERENT		if !NOT_COHERENT_CACHE
 	select OF_EARLY_FLATTREE
 	select OLD_SIGACTION			if PPC32
 	select OLD_SIGSUSPEND
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b71ce992c0c0..a79663191228 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -119,7 +119,6 @@ config RISCV
 	select MODULES_USE_ELF_RELA if MODULES
 	select MODULE_SECTIONS if MODULES
 	select OF
-	select OF_DMA_DEFAULT_COHERENT
 	select OF_EARLY_FLATTREE
 	select OF_IRQ
 	select PCI_DOMAINS_GENERIC if PCI
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 644386833a7b..e40f10bf2ba4 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -102,8 +102,4 @@ config OF_OVERLAY
 config OF_NUMA
 	bool
 
-config OF_DMA_DEFAULT_COHERENT
-	# arches should select this if DMA is coherent by default for OF devices
-	bool
-
 endif # OF
diff --git a/drivers/of/address.c b/drivers/of/address.c
index c105d66a1fa4..23ade4919853 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -1103,15 +1103,7 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
 bool of_dma_is_coherent(struct device_node *np)
 {
 	struct device_node *node;
-	bool is_coherent = IS_ENABLED(CONFIG_OF_DMA_DEFAULT_COHERENT);
-
-	/*
-	 * DT-based MIPS doesn't use OF_DMA_DEFAULT_COHERENT, but
-	 * might override the system-wide default at runtime.
-	 */
-#if defined(CONFIG_MIPS) && defined(CONFIG_DMA_NONCOHERENT)
-	is_coherent = dma_default_coherent;
-#endif
+	bool is_coherent = dma_default_coherent;
 
 	node = of_node_get(np);
 
-- 
2.37.1 (Apple Git-137.1)

> -	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?

In fact I wonder if adding CONFIG_ARCH_DMA_DEFAULT_COHERENT and removing
OF_DMA_DEFAULT_COHERENT should be one patch, as that seems to bring
over the intent a little better I'd say.

> 2023年3月1日 13:06，Christoph Hellwig <hch@lst.de> 写道：
> 
>> - 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….

> 
> In fact I wonder if adding CONFIG_ARCH_DMA_DEFAULT_COHERENT and removing
> OF_DMA_DEFAULT_COHERENT should be one patch, as that seems to bring
> over the intent a little better I'd say.

Jiaxun Yang <jiaxun.yang@flygoat.com> writes:
>> 2023年3月1日 13:06，Christoph Hellwig <hch@lst.de> 写道：
>> 
>>> - 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