On Fri, Jan 8, 2021 at 05:27 PM, Jeremy Linton wrote:<br />
<blockquote>Hi,<br /><br />On 1/8/21 2:28 AM, Ard Biesheuvel wrote:<br />
<blockquote>On 1/8/21 7:16 AM, Jeremy Linton wrote:<br />
<blockquote>The primarly problem with the rpi Arasan controller working<br />with a default SDHCI driver is the lack of a meaningful<br />capabilities register. As such if we add a _DSD entry<br />to provide that information, we can then bind it to<br />the linux sdhci_iproc driver which already<br />hardcodes the remaining controller bugs.<br /><br />Further we have gotten BRCME88C approved as the HID<br />for the newer eMMC2 controller. So lets define an<br />ACPI object to describe it.<br /><br />Of course both devices are sharing an interrupt so<br />we should also indicate that in the table as well.<br /><br />Signed-off-by: Jeremy Linton &lt;jeremy.linton@arm.com&gt;<br />---<br />Platform/RaspberryPi/AcpiTables/Dsdt.asl | 2 +<br />Platform/RaspberryPi/AcpiTables/Sdhc.asl | 86 +++++++++++++++++++++++++++++++-<br />2 files changed, 86 insertions(+), 2 deletions(-)<br /><br />diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl<br />index d116f965e1..cca08c0539 100644<br />--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl<br />+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl<br />@@ -150,6 +150,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)<br />QWORDMEMORYBUF(23)<br />QWORDMEMORYBUF(24)<br />QWORDMEMORYBUF(25)<br />+ QWORDMEMORYBUF(26)<br />})<br /><br />// USB<br />@@ -196,6 +197,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)<br />// SDC<br />QWORDMEMORYSET(24, MMCHS1_OFFSET, MMCHS1_LENGTH)<br />QWORDMEMORYSET(25, SDHOST_OFFSET, SDHOST_LENGTH)<br />+ QWORDMEMORYSET(26, MMCHS2_OFFSET, MMCHS2_LENGTH)<br /><br />Return (RBUF)<br />}<br />diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl b/Platform/RaspberryPi/AcpiTables/Sdhc.asl<br />index 0ab1ba27f2..a7ac831066 100644<br />--- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl<br />+++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl<br />@@ -19,7 +19,7 @@<br />// Note: UEFI can use either SDHost or Arasan. We expose both to the OS.<br />//<br /><br />-// ArasanSD 3.0 SD Host Controller.<br />+// ArasanSD 3.0 SD Host Controller. (brcm,bcm2835-sdhci)<br />Device (SDC1)<br />{<br />Name (_HID, "BCM2847")<br />@@ -37,7 +37,7 @@ Device (SDC1)<br />Name (RBUF, ResourceTemplate ()<br />{<br />MEMORY32FIXED (ReadWrite, 0, MMCHS1_LENGTH, RMEM)<br />- Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_MMCHS1_INTERRUPT }<br />+ Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_MMCHS1_INTERRUPT }<br />})<br />Method (_CRS, 0x0, Serialized)<br />{<br />@@ -45,6 +45,86 @@ Device (SDC1)<br />Return (^RBUF)<br />}<br /><br />+ // The standard CAPs registers on this controller<br />+ // appear to be 0, lets set some minimal defaults<br />+ // Since this cap doesn't indicate DMA capability<br />+ // we don't need a _DMA()<br />+ Name (_DSD, Package () {<br />+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),<br />+ Package () {<br />+ Package () { "sdhci-caps", 0x0100fa81 },<br />+ }<br />+ })<br />+<br />+<br />+ //<br />+ // A child device that represents the<br />+ // sd card, which is marked as non-removable.<br />+ //<br />+ Device (SDMM)<br />+ {<br />+ Method (_ADR)<br />+ {<br />+ Return (0)<br />+ }<br />+ Method (_RMV) // Is removable<br />+ {<br />+ Return (0) // 0 - fixed<br />+ }<br />+ }<br />+}<br />+<br />+#if (RPI_MODEL == 4)<br />+// emmc2 Host Controller. (brcm,bcm2711-emmc2)<br />+Device (SDC3)<br />+{<br />+ Name (_HID, "BRCME88C")<br />+ Name (_UID, 0x1)<br />+ Name (_CCA, 0x0)<br />+ Name (_S1D, 0x1)<br />+ Name (_S2D, 0x1)<br />+ Name (_S3D, 0x1)<br />+ Name (_S4D, 0x1)<br />+ Method (_STA)<br />+ {<br />+ Return(0xf)<br />+ }<br />+ Name (RBUF, ResourceTemplate ()<br />+ {<br />+ MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)<br />+ Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_MMCHS1_INTERRUPT }<br />+ })<br />+ Method (_CRS, 0x0, Serialized)<br />+ {<br />+ MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)<br />+ Return (^RBUF)<br />+ }<br />+<br />+ // forceably limit to 1G<br />+ Name (_DMA, ResourceTemplate() {<br />+ QWordMemory (ResourceConsumer,<br />+ ,<br />+ MinFixed,<br />+ MaxFixed,<br />+ NonCacheable,<br />+ ReadWrite,<br />+ 0x0,<br />+ 0x0, // MIN<br />+ 0x3fffffff, // MAX<br />+ 0x0, // TRA<br />+ 0x40000000, // LEN<br />+ ,<br />+ ,<br />+ )<br />+ })<br />+</blockquote>
Shouldn't the _DMA method be on a container object instead of on the<br />device object itself?</blockquote>
Pained cough... There is a _DMA on the gpu devs container of which this <br />device belongs. That doesn't make the DMA work. The spec says that _DMA <br />is to exist on the bus device, and I think what is happening here is <br />that the "device" is the SDMM (which isn't directly visible because of <br />the way diff broke this patch up) contained in the "SDC3 mmc bus".</blockquote>
The Linux device tree has the following comment:<br /><br />
<div>
<div>&nbsp; &nbsp; &nbsp; &nbsp; /*</div>
<div>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;* emmc2 has different DMA constraints based on SoC revisions. It was</div>
<div>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;* moved into its own bus, so as for RPi4's firmware to update them.</div>
<div>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;* The firmware will find whether the emmc2bus alias is defined, and if</div>
<div>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;* so, it'll edit the dma-ranges property below accordingly.</div>
<div>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp;*<br /><br />And the dma-ranges property it references is:</div>
</div>
<div><br />&nbsp; &nbsp; dma-ranges = &lt;0x0 0xc0000000&nbsp; 0x0 0x00000000&nbsp; 0x40000000&gt;;<br /><br />Not sure how the firmware will adjust this, but it does imply that there is actual translation<br />going on here and it doesn't match the _DMA property you added.</div>
<br />
<blockquote>
<div>Frankly, I could be wrong, but what I do know is that without that bit <br />of ugliness DMA doesn't work in linux on this controller. I just <br />stripped it out again to verify the failure with 4.11. It does the same <br />thing, the controller itself seems to be working but the block device <br />associated with it can't be read. There are few other possibilities <br />(like maybe this controller shouldn't be a gpudevs child) and i'm fixing <br />a bug I created..I had kernel instrumentation in 5.9 tracking the dma <br />mask propagation, and I was fairly confident in this a couple months <br />ago, less so now.</div>
</blockquote>
One question I have is whether _DMA methods should be applied recursively or<br />not. If you have a bus that provides a translating _DMA method that is behind a<br />bus that also has a _DMA method, should the translation specified in that first _DMA<br />method be applied or not? The text in the ACPI spec is far for clear her...<br /><br />If I read the Linux code in drivers/acpi/scan.c correctly, Linux would not apply<br />the translation in this case. Since the _DMA method you add does not specify any<br />translation, I think that means the required address translation isn't happening<br />which means that DMA transactions would fail. I suspect the block device behind<br />the controller would still be detected, but any actual I/O would fail since the right<br />memory isn't addressed.<br /><br />So I think the best thing to do would be to have a separate ACPI0004 container<br />with a translating _DMA method and but the emmc2 device below that.<br /><br />
<blockquote>
<blockquote>
<blockquote>+ Name (_DSD, Package () {<br />+ ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),<br />+ Package () {<br />+ Package () { "sdhci-caps-mask", 0x0000000000080000 },<br />+ }<br />+ })<br />+<br />//<br />// A child device that represents the<br />// sd card, which is marked as non-removable.<br />@@ -62,6 +142,7 @@ Device (SDC1)<br />}<br />}<br /><br />+#else // !RPI4<br /><br />// Broadcom SDHost 2.0 SD Host Controller<br />Device (SDC2)<br />@@ -105,3 +186,4 @@ Device (SDC2)<br />}<br />}<br />}<br />+#endif // !RPI4</blockquote>
</blockquote>
</blockquote>