RPi4B: emmc2bus dma-range handling does not track the boot-time-FDT (u-boot based booting)

[Mark Millard]

On 2020-Oct-8, at 11:34, Kyle Evans <kevans at freebsd.org> wrote:

> On Thu, Oct 8, 2020 at 12:38 PM Kyle Evans <kevans at freebsd.org> wrote:
>> 
>> On Thu, Oct 8, 2020 at 11:33 AM Kyle Evans <kevans at freebsd.org> wrote:
>>> 
>>> On Thu, Oct 8, 2020 at 4:01 AM Mark Millard via freebsd-arm
>>> <freebsd-arm at freebsd.org> wrote:
>>>> 
>>>> sys/gnu/dts/arm/bcm2711.dtsi reports:
>>>> 
>>>>        /*
>>>>         * emmc2 has different DMA constraints based on SoC revisions. It was
>>>>         * moved into its own bus, so as for RPi4's firmware to update them.
>>>>         * The firmware will find whether the emmc2bus alias is defined, and if
>>>>         * so, it'll edit the dma-ranges property below accordingly.
>>>>         */
>>>> [... snip ...]
>>> 
>>> I have no words for how annoying this is.
>>> 
>> 
>> For a slightly more helpful response:
>> 
>> We can fix this, and it ends up being much cleaner than my current
>> hack. Basically, in bcm2835_vcbus.c, we should eradicate the
>> busdma_lowaddr from bcm283x_memory_soc_cfg.
>> 
>> bcm283x_dmabus_peripheral_lowaddr should instead take a device_t and
>> grab the bus's dma-ranges. It /looks/ to be valid on all the DTS I see
>> for the RPi boards we support, so we can just unconditionally use that
>> and things will just work for the newer RPi4 models.
>> 
>> From my discussion (with an assist Ian on address interpretation) on
>> IRC, so I don't forget:
>> 
>> dma-ranges is three-value: <dma_addr cpu_addr max_len>

Note: For the below I looked at 3 separate RPi4B
examples, using u-boot print fdt / when I could
or translating the dtb to a dts otherwise. One
of the examples is from one of ubuntu 2020.04.1's
RPi4B specific builds. The others I use with FreeBSD,
one for u-boot and one for uefi/ACPI.

(

cpu_addr is sized via the global:

/  {

        #address-cells = <0x2>;

and the dma_addr and max_len by more local definitions,
such as in:

                #address-cells = <0x1>;
                #size-cells = <0x1>;
                compatible = "simple-bus";
                dma-ranges = <0xc0000000 0x0 0x0 0x40000000>;

and:

                #address-cells = <0x2>;
                #size-cells = <0x2>;
                compatible = "simple-bus";
                dma-ranges = <0x0 0x0 0x0 0x0 0x4 0x0>;

Note that the above has #size-cells varying when:
4 <= number of cells in dma-range . There will be
worse cases later, below.

and:

                        #address-cells = <0x3>;
                        #interrupt-cells = <0x1>;
                        #size-cells = <0x2>;
                        . . .
                        dma-ranges = <0x2000000 0x0 0x0 0x0 0x0 0x0 0xc0000000>;

(The #address-cells being 3 indicates a bit mask as the first of the 3,
the bit mask indicating extra information about the context.)

and:

                #address-cells = <0x2>;
                #size-cells = <0x1>;
                compatible = "simple-bus";
                dma-ranges = <0x0 0xc0000000 0x0 0x0 0x40000000>;

and:

                #address-cells = <0x1>;
                #size-cells = <0x2>;
                compatible = "simple-bus";
                dma-ranges = <0x0 0x0 0x0 0x4 0x0>;

Note that for the last 2 examples above the number of cells
in the dma-range (5) is not sufficient to indicate the value
for #size-cells or #(dma-addr-cells) without presuming some
other context to disambiguate.


There is also an example of just:

                        dma-ranges;

(in firmware { . . . }).

>> We'll see 4 and 5 value variants of this because 64-bit addresses are
>> described with pairs of 32-bit values.
>> 
>> 4-value variant: dma_addr will be 32-bit, cpu_addr will be 64-bit
>> 5-value variant: both are 64-bit

There is an example shown above with 5-value having #size_cells
being 1 (32-bit) [and dma-addr being 64 bit (cells 2)] instead of
#size_cells being 2 (64-bit) [and dma_addr being 32-bit (cells 1)].

>> Note that bcm283x_dmabus_peripheral_lowaddr() will be returning
>> cpu_addr + (max_len - 1)
>> 
>> This won't match perfectly with what we currently return, but it will
>> be more accurate.
> 
> Here's a patch that I hacked out and can't test for quite a while yet,
> feel free to give it a shot:
> https://people.freebsd.org/~kevans/bcm2835_vcbus.diff  -- the best
> guarantee I can give you is that it builds. We'll need to test it on
> both RPi4 models with the separate bus and the original RPi4s, as well
> as an RPi3 and RPi2/0w.

See above about trying to use the number of cells in a dma-ranges
to figure out the sizes of #size-cells or #(dma-addr-cells).

===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)