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

[Kyle Evans]

On Thu, Oct 8, 2020 at 3:16 PM Mark Millard <marklmi at yahoo.com> wrote:
>
>
>
> On 2020-Oct-8, at 12:32, Kyle Evans <kevans at freebsd.org> wrote:
>
> > On Thu, Oct 8, 2020 at 2:28 PM Mark Millard <marklmi at yahoo.com> wrote:
> >>
> >>
> >>
> >> On 2020-Oct-8, at 11:34, Kyle Evans <kevans at freebsd.org> wrote:
> >>
> >> 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).
> >>
> >
> > Yeah, Ian pointed out just a little bit ago the way this should be
> > done correctly. The hacky patch should at least get it correct enough
> > for now, then I can write a more generic version of dma-ranges parsing
> > that does it correctly.
>
> Okay.
>
> Another type of overall issue (that may not apply to
> the specific context here) is dtb content like:
>
>         scb {
>                 compatible = "simple-bus";
>                 #address-cells = <0x00000002>;
>                 #size-cells = <0x00000002>;
>                 ranges = * 0x0000000007ef8a18 [0x00000060];
>                 dma-ranges = <0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0xfc000000 0x00000001 0x00000000 0x00000001 0x00000000 0x00000001 0x00000000>;
>                 phandle = <0x000000d0>;
>                 pcie at 7d500000 {
>                         compatible = "brcm,bcm2711-pcie";
>                         . . .
>                         #address-cells = <0x00000003>;
>                         #interrupt-cells = <0x00000001>;
>                         #size-cells = <0x00000002>;
>                         . . .
>                         dma-ranges = <0x02000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0xc0000000>;
>                         . . .
>                 };
>                 . . .
>
> where the inner most dma-ranges is not from the just-surrounding
> context (parent) but is strictly local (despite the parent
> potentially also having dma-ranges). I think that only pcie at ...
> is that way so far for the RPi4 --but notationally it seems to
> be generally allowed.
>

This is irrelevant for the time being as the PCI driver doesn't cross
paths with the snippet the patch touches.

Thanks,

Kyle Evans