RPi4B: emmc2bus dma-range handling does not track the boot-time-FDT (u-boot based booting)
Mark Millard
marklmi at yahoo.com
Thu Oct 8 20:15:58 UTC 2020
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:
>>
>>> 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).
>>
>
> 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.
===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)
More information about the freebsd-arm
mailing list