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

2023-03-02 Thread Michael Ellerman 
Jiaxun Yang  writes:
>> 2023年3月1日 13:06，Christoph Hellwig  写道：
>> 
>>> - 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

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

2023-03-01 Thread Jiaxun Yang 



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

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

2023-03-01 Thread Christoph Hellwig 
> - 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.

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

2023-02-23 Thread Jiaxun Yang 
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 
---
 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_CHUNKif PPC64
select NEED_SG_DMA_LENGTH
select OF
-   select OF_DMA_DEFAULT_COHERENT  if !NOT_COHERENT_CACHE
select OF_EARLY_FLATTREE
select OLD_SIGACTIONif 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)