Date: Thu, 8 Oct 2020 14:32:09 -0500 From: Kyle Evans <kevans@freebsd.org> To: Mark Millard <marklmi@yahoo.com> Cc: freebsd-arm <freebsd-arm@freebsd.org> Subject: Re: RPi4B: emmc2bus dma-range handling does not track the boot-time-FDT (u-boot based booting) Message-ID: <CACNAnaHwAyw=6tiOC12WH9BBcj5FDZzDGxoh0UJw9KL0HE206g@mail.gmail.com> In-Reply-To: <32A8A046-56DF-4998-9C3D-8630ACCE4951@yahoo.com> References: <D8BDF95A-D6A8-4E95-A0CE-D53068E8355B.ref@yahoo.com> <D8BDF95A-D6A8-4E95-A0CE-D53068E8355B@yahoo.com> <CACNAnaG3CKTiXdXNUO1Jgr34=XGF4wYRuUnuiJRhNb1J9XaGbw@mail.gmail.com> <CACNAnaEZP=Gxs4cbvCOFzM8MZB6-2Ny0N7KEW-nYoDw22_Jzwg@mail.gmail.com> <CACNAnaFXpJcA5Xyw%2BPR6iem3U2nvpog53hsnCFKiGWiO6OgKKA@mail.gmail.com> <32A8A046-56DF-4998-9C3D-8630ACCE4951@yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Oct 8, 2020 at 2:28 PM Mark Millard <marklmi@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@freebsd.org> wrote: > >> > >> On Thu, Oct 8, 2020 at 11:33 AM Kyle Evans <kevans@freebsd.org> wrote: > >>> > >>> On Thu, Oct 8, 2020 at 4:01 AM Mark Millard via freebsd-arm > >>> <freebsd-arm@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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACNAnaHwAyw=6tiOC12WH9BBcj5FDZzDGxoh0UJw9KL0HE206g>