On Thu, 2019-08-08 at 09:02 -0600, Rob Herring wrote:

On Tue, Aug 6, 2019 at 12:12 PM Nicolas Saenz Julienne
[off-list ref] wrote:

quoted

Hi Rob,

On Mon, 2019-08-05 at 13:23 -0600, Rob Herring wrote:

quoted

On Mon, Aug 5, 2019 at 10:03 AM Nicolas Saenz Julienne
[off-list ref] wrote:

quoted

Hi Rob,
Thanks for the review!

On Fri, 2019-08-02 at 11:17 -0600, Rob Herring wrote:

quoted

On Wed, Jul 31, 2019 at 9:48 AM Nicolas Saenz Julienne
[off-list ref] wrote:

quoted

Some SoCs might have multiple interconnects each with their own DMA
addressing limitations. This function parses the 'dma-ranges' on
each of
them and tries to guess the maximum SoC wide DMA addressable memory
size.

This is specially useful for arch code in order to properly setup
CMA
and memory zones.

We already have a way to setup CMA in reserved-memory, so why is this
needed for that?

Correct me if I'm wrong but I got the feeling you got the point of the
patch
later on.

No, for CMA I don't. Can't we already pass a size and location for CMA
region under /reserved-memory. The only advantage here is perhaps the
CMA range could be anywhere in the DMA zone vs. a fixed location.

Now I get it, sorry I wasn't aware of that interface.

Still, I'm not convinced it matches RPi's use case as this would hard-code
CMA's size. Most people won't care, but for the ones that do, it's nicer to
change the value from the kernel command line than editing the dtb.

Sure, I fully agree and am not a fan of the CMA DT overlays I've seen.

quoted

I get that
if you need to, for example, reserve some memory for the video to work, it's
silly not to hard-code it. Yet due to the board's nature and users base I
say
it's important to favor flexibility. It would also break compatibility with
earlier versions of the board and diverge from the downstream kernel
behaviour.
Which is a bigger issue than it seems as most users don't always understand
which kernel they are running and unknowingly copy configuration options
from
forums.

As I also need to know the DMA addressing limitations to properly configure
memory zones and dma-direct. Setting up the proper CMA constraints during
the
arch's init will be trivial anyway.

It was really just commentary on commit text as for CMA alone we have
a solution already. I agree on the need for zones.

Ok, understood :)

quoted

quoted

quoted

quoted

IMO, I'd just do:

if (of_fdt_machine_is_compatible(blob, "brcm,bcm2711"))
    dma_zone_size = XX;

2 lines of code is much easier to maintain than 10s of incomplete code
and is clearer who needs this. Maybe if we have dozens of SoCs with
this problem we should start parsing dma-ranges.

FYI that's what arm32 is doing at the moment and was my first instinct.
But
it
seems that arm64 has been able to survive so far without any machine
specific
code and I have the feeling Catalin and Will will not be happy about
this
solution. Am I wrong?

No doubt. I'm fine if the 2 lines live in drivers/of/.

Note that I'm trying to reduce the number of early_init_dt_scan_*
calls from arch code into the DT code so there's more commonality
across architectures in the early DT scans. So ideally, this can all
be handled under early_init_dt_scan() call.

How does this look? (I'll split it in two patches and add a comment
explaining
why dt_dma_zone_size is needed)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index f2444c61a136..1395be40b722 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c

@@ -30,6 +30,8 @@

 #include "of_private.h"

+u64 dt_dma_zone_size __ro_after_init;

Avoiding a call from arch code by just having a variable isn't really
better. I'd rather see a common, non DT specific variable that can be
adjusted. Something similar to initrd_start/end. Then the arch code
doesn't have to care what hardware description code adjusted the
value.

Way better, I'll update it.