[PATCH 0/3] Rpi: SD/Wifi Acpi updates

[PATCH 0/3] Rpi: SD/Wifi Acpi updates

[Jeremy Linton]

The existing rpi3 acpi entries for the Arasan
and eMMC2 controllers needs to updated to work
with the rpi4. This is done by adding a caps
override for the legacy Arasan controller and
then adding an entirely new entry for the newer
eMMC2 controller.

Then we flip the default routing to make the eMMC2
the default for the SD card, so that the wifi can
start working on the Arasan.

Jeremy Linton (3):
  Platform/RaspberryPi/Acpitables: Further lower the DMA zone
  Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan
  Platform/RaspberryPi; Invert default Arasan, Emmc2 routing

 Platform/RaspberryPi/AcpiTables/Dsdt.asl  |  2 +
 Platform/RaspberryPi/AcpiTables/Iort.aslc |  2 +-
 Platform/RaspberryPi/AcpiTables/Sdhc.asl  | 86 ++++++++++++++++++++++++++++++-
 Platform/RaspberryPi/RPi4/RPi4.dsc        |  2 +-
 Platform/RaspberryPi/RPi4/Readme.md       |  2 +-
 5 files changed, 89 insertions(+), 5 deletions(-)

-- 
2.13.7

[PATCH 1/3] Platform/RaspberryPi/Acpitables: Further lower the DMA zone

[Jeremy Linton]

We are going to add a _DMA() for the new controller, but
it won't work right with the linux driver unless we lower
the DMA zone again.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 Platform/RaspberryPi/AcpiTables/Iort.aslc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Iort.aslc b/Platform/RaspberryPi/AcpiTables/Iort.aslc
index 00720194bb..f576fa3b04 100644
--- a/Platform/RaspberryPi/AcpiTables/Iort.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Iort.aslc
@@ -46,7 +46,7 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
       0x0,                                          // AllocationHints
       0x0,                                          // Reserved
       0x0,                                          // MemoryAccessFlags
-      31,                                           // AddressSizeLimit
+      30,                                           // AddressSizeLimit
     }, {
       "\\_SB_.SCB0.XHC0"                            // ObjectName
     }
-- 
2.13.7

[PATCH 2/3] Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

[Jeremy Linton]

The primarly problem with the rpi Arasan controller working
with a default SDHCI driver is the lack of a meaningful
capabilities register. As such if we add a _DSD entry
to provide that information, we can then bind it to
the linux sdhci_iproc driver which already
hardcodes the remaining controller bugs.

Further we have gotten BRCME88C approved as the HID
for the newer eMMC2 controller. So lets define an
ACPI object to describe it.

Of course both devices are sharing an interrupt so
we should also indicate that in the table as well.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 Platform/RaspberryPi/AcpiTables/Dsdt.asl |  2 +
 Platform/RaspberryPi/AcpiTables/Sdhc.asl | 86 +++++++++++++++++++++++++++++++-
 2 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
index d116f965e1..cca08c0539 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -150,6 +150,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
           QWORDMEMORYBUF(23)
           QWORDMEMORYBUF(24)
           QWORDMEMORYBUF(25)
+          QWORDMEMORYBUF(26)
         })
 
         // USB
@@ -196,6 +197,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
         // SDC
         QWORDMEMORYSET(24, MMCHS1_OFFSET, MMCHS1_LENGTH)
         QWORDMEMORYSET(25, SDHOST_OFFSET, SDHOST_LENGTH)
+        QWORDMEMORYSET(26, MMCHS2_OFFSET, MMCHS2_LENGTH)
 
         Return (RBUF)
       }
diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
index 0ab1ba27f2..a7ac831066 100644
--- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
+++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
@@ -19,7 +19,7 @@
 // Note: UEFI can use either SDHost or Arasan. We expose both to the OS.
 //
 
-// ArasanSD 3.0 SD Host Controller.
+// ArasanSD 3.0 SD Host Controller. (brcm,bcm2835-sdhci)
 Device (SDC1)
 {
   Name (_HID, "BCM2847")
@@ -37,7 +37,7 @@ Device (SDC1)
   Name (RBUF, ResourceTemplate ()
   {
     MEMORY32FIXED (ReadWrite, 0, MMCHS1_LENGTH, RMEM)
-    Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_MMCHS1_INTERRUPT }
+    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_MMCHS1_INTERRUPT }
   })
   Method (_CRS, 0x0, Serialized)
   {
@@ -45,6 +45,86 @@ Device (SDC1)
     Return (^RBUF)
   }
 
+  // The standard CAPs registers on this controller
+  // appear to be 0, lets set some minimal defaults
+  // Since this cap doesn't indicate DMA capability
+  // we don't need a _DMA()
+  Name (_DSD, Package () {
+    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+    Package () {
+      Package () { "sdhci-caps", 0x0100fa81 },
+    }
+  })
+
+
+  //
+  // A child device that represents the
+  // sd card, which is marked as non-removable.
+  //
+  Device (SDMM)
+  {
+    Method (_ADR)
+    {
+      Return (0)
+    }
+    Method (_RMV) // Is removable
+    {
+      Return (0) // 0 - fixed
+    }
+  }
+}
+
+#if (RPI_MODEL == 4)
+// emmc2 Host Controller. (brcm,bcm2711-emmc2)
+Device (SDC3)
+{
+  Name (_HID, "BRCME88C")
+  Name (_UID, 0x1)
+  Name (_CCA, 0x0)
+  Name (_S1D, 0x1)
+  Name (_S2D, 0x1)
+  Name (_S3D, 0x1)
+  Name (_S4D, 0x1)
+  Method (_STA)
+  {
+    Return(0xf)
+  }
+  Name (RBUF, ResourceTemplate ()
+  {
+    MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)
+    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_MMCHS1_INTERRUPT }
+  })
+  Method (_CRS, 0x0, Serialized)
+  {
+    MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)
+    Return (^RBUF)
+  }
+
+  // forceably limit to 1G
+  Name (_DMA, ResourceTemplate() {
+    QWordMemory (ResourceConsumer,
+      ,
+      MinFixed,
+      MaxFixed,
+      NonCacheable,
+      ReadWrite,
+      0x0,
+      0x0,        // MIN
+      0x3fffffff, // MAX
+      0x0,        // TRA
+      0x40000000, // LEN
+      ,
+      ,
+    )
+  })
+
+  Name (_DSD, Package () {
+    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+    Package () {
+      Package () { "sdhci-caps-mask", 0x0000000000080000 },
+    }
+  })
+
   //
   // A child device that represents the
   // sd card, which is marked as non-removable.
@@ -62,6 +142,7 @@ Device (SDC1)
   }
 }
 
+#else // !RPI4
 
 // Broadcom SDHost 2.0 SD Host Controller
 Device (SDC2)
@@ -105,3 +186,4 @@ Device (SDC2)
     }
   }
 }
+#endif // !RPI4
-- 
2.13.7

[PATCH 3/3] Platform/RaspberryPi; Invert default Arasan, Emmc2 routing

[Jeremy Linton]

In order for the wifi to work, and the SD to run at full
speed we need to bind the sd slot to the eMMC2 controller.

Since we now have a driver for the eMMC2 controller
there isn't any reason to leave the SD card bound
to the older Arasan controller.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 Platform/RaspberryPi/RPi4/RPi4.dsc  | 2 +-
 Platform/RaspberryPi/RPi4/Readme.md | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index ddf4dd6a41..f6abe90e9d 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -475,7 +475,7 @@
   # SD-related.
   #
 
-  gRaspberryPiTokenSpaceGuid.PcdSdIsArasan|L"SdIsArasan"|gConfigDxeFormSetGuid|0x0|1
+  gRaspberryPiTokenSpaceGuid.PcdSdIsArasan|L"SdIsArasan"|gConfigDxeFormSetGuid|0x0|0
   gRaspberryPiTokenSpaceGuid.PcdMmcForce1Bit|L"MmcForce1Bit"|gConfigDxeFormSetGuid|0x0|0
   gRaspberryPiTokenSpaceGuid.PcdMmcForceDefaultSpeed|L"MmcForceDefaultSpeed"|gConfigDxeFormSetGuid|0x0|0
   gRaspberryPiTokenSpaceGuid.PcdMmcSdDefaultSpeedMHz|L"MmcSdDefaultSpeedMHz"|gConfigDxeFormSetGuid|0x0|25
diff --git a/Platform/RaspberryPi/RPi4/Readme.md b/Platform/RaspberryPi/RPi4/Readme.md
index 3b2ed44e3c..80899f4ca4 100644
--- a/Platform/RaspberryPi/RPi4/Readme.md
+++ b/Platform/RaspberryPi/RPi4/Readme.md
@@ -181,7 +181,7 @@ Limit RAM to 3 GB            | `RamLimitTo3GB` | Disable = `0x00000000` <br> Ena
 System Table Selection       | `SystemTableMode`| ACPI = `0x00000000` (default)<br> ACPI + Devicetree = `0x00000001` <br> Devicetree = `0x00000002`
 Asset Tag                    | `AssetTag` | String, 32 characters or less (e.g. `L"ABCD123"`)<br> (default `L""`)
 **SD/MMC Configuration**     |
-uSD/eMMC Routing             | `SdIsArasan` | Arasan SDHC = `0x00000001` (default) <br> eMMC2 SDHCI = `0x00000000`
+uSD/eMMC Routing             | `SdIsArasan` | Arasan SDHC = `0x00000001` <br> eMMC2 SDHCI = `0x00000000` (default)
 Multi-Block Support          | `MmcDisableMulti` | Multi-block transfers = `0x00000000` (default)<br> Single block transfers = `0x00000001`
 uSD Max Bus Width            | `MmcForce1Bit` | 4-bit Mode = `0x00000000`  (default)<br> 1-bit Mode = `0x00000001`
 uSD Force Default Speed      | `MmcForceDefaultSpeed` | Allow High Speed = `0x00000000` (default)<br> Force Default Speed = `0x00000001`
-- 
2.13.7

Re: [PATCH 2/3] Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

[Ard Biesheuvel]

On 1/8/21 7:16 AM, Jeremy Linton wrote:
> The primarly problem with the rpi Arasan controller working
> with a default SDHCI driver is the lack of a meaningful
> capabilities register. As such if we add a _DSD entry
> to provide that information, we can then bind it to
> the linux sdhci_iproc driver which already
> hardcodes the remaining controller bugs.
> 
> Further we have gotten BRCME88C approved as the HID
> for the newer eMMC2 controller. So lets define an
> ACPI object to describe it.
> 
> Of course both devices are sharing an interrupt so
> we should also indicate that in the table as well.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  Platform/RaspberryPi/AcpiTables/Dsdt.asl |  2 +
>  Platform/RaspberryPi/AcpiTables/Sdhc.asl | 86 +++++++++++++++++++++++++++++++-
>  2 files changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
> index d116f965e1..cca08c0539 100644
> --- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
> @@ -150,6 +150,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
>            QWORDMEMORYBUF(23)
>            QWORDMEMORYBUF(24)
>            QWORDMEMORYBUF(25)
> +          QWORDMEMORYBUF(26)
>          })
>  
>          // USB
> @@ -196,6 +197,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
>          // SDC
>          QWORDMEMORYSET(24, MMCHS1_OFFSET, MMCHS1_LENGTH)
>          QWORDMEMORYSET(25, SDHOST_OFFSET, SDHOST_LENGTH)
> +        QWORDMEMORYSET(26, MMCHS2_OFFSET, MMCHS2_LENGTH)
>  
>          Return (RBUF)
>        }
> diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> index 0ab1ba27f2..a7ac831066 100644
> --- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> @@ -19,7 +19,7 @@
>  // Note: UEFI can use either SDHost or Arasan. We expose both to the OS.
>  //
>  
> -// ArasanSD 3.0 SD Host Controller.
> +// ArasanSD 3.0 SD Host Controller. (brcm,bcm2835-sdhci)
>  Device (SDC1)
>  {
>    Name (_HID, "BCM2847")
> @@ -37,7 +37,7 @@ Device (SDC1)
>    Name (RBUF, ResourceTemplate ()
>    {
>      MEMORY32FIXED (ReadWrite, 0, MMCHS1_LENGTH, RMEM)
> -    Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_MMCHS1_INTERRUPT }
> +    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_MMCHS1_INTERRUPT }
>    })
>    Method (_CRS, 0x0, Serialized)
>    {
> @@ -45,6 +45,86 @@ Device (SDC1)
>      Return (^RBUF)
>    }
>  
> +  // The standard CAPs registers on this controller
> +  // appear to be 0, lets set some minimal defaults
> +  // Since this cap doesn't indicate DMA capability
> +  // we don't need a _DMA()
> +  Name (_DSD, Package () {
> +    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +    Package () {
> +      Package () { "sdhci-caps", 0x0100fa81 },
> +    }
> +  })
> +
> +
> +  //
> +  // A child device that represents the
> +  // sd card, which is marked as non-removable.
> +  //
> +  Device (SDMM)
> +  {
> +    Method (_ADR)
> +    {
> +      Return (0)
> +    }
> +    Method (_RMV) // Is removable
> +    {
> +      Return (0) // 0 - fixed
> +    }
> +  }
> +}
> +
> +#if (RPI_MODEL == 4)
> +// emmc2 Host Controller. (brcm,bcm2711-emmc2)
> +Device (SDC3)
> +{
> +  Name (_HID, "BRCME88C")
> +  Name (_UID, 0x1)
> +  Name (_CCA, 0x0)
> +  Name (_S1D, 0x1)
> +  Name (_S2D, 0x1)
> +  Name (_S3D, 0x1)
> +  Name (_S4D, 0x1)
> +  Method (_STA)
> +  {
> +    Return(0xf)
> +  }
> +  Name (RBUF, ResourceTemplate ()
> +  {
> +    MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)
> +    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_MMCHS1_INTERRUPT }
> +  })
> +  Method (_CRS, 0x0, Serialized)
> +  {
> +    MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)
> +    Return (^RBUF)
> +  }
> +
> +  // forceably limit to 1G
> +  Name (_DMA, ResourceTemplate() {
> +    QWordMemory (ResourceConsumer,
> +      ,
> +      MinFixed,
> +      MaxFixed,
> +      NonCacheable,
> +      ReadWrite,
> +      0x0,
> +      0x0,        // MIN
> +      0x3fffffff, // MAX
> +      0x0,        // TRA
> +      0x40000000, // LEN
> +      ,
> +      ,
> +    )
> +  })
> +

Shouldn't the _DMA method be on a container object instead of on the
device object itself?


> +  Name (_DSD, Package () {
> +    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +    Package () {
> +      Package () { "sdhci-caps-mask", 0x0000000000080000 },
> +    }
> +  })
> +
>    //
>    // A child device that represents the
>    // sd card, which is marked as non-removable.
> @@ -62,6 +142,7 @@ Device (SDC1)
>    }
>  }
>  
> +#else // !RPI4
>  
>  // Broadcom SDHost 2.0 SD Host Controller
>  Device (SDC2)
> @@ -105,3 +186,4 @@ Device (SDC2)
>      }
>    }
>  }
> +#endif // !RPI4
> 

Re: [PATCH 2/3] Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

[Jeremy Linton]

Hi,

On 1/8/21 2:28 AM, Ard Biesheuvel wrote:
> On 1/8/21 7:16 AM, Jeremy Linton wrote:
>> The primarly problem with the rpi Arasan controller working
>> with a default SDHCI driver is the lack of a meaningful
>> capabilities register. As such if we add a _DSD entry
>> to provide that information, we can then bind it to
>> the linux sdhci_iproc driver which already
>> hardcodes the remaining controller bugs.
>>
>> Further we have gotten BRCME88C approved as the HID
>> for the newer eMMC2 controller. So lets define an
>> ACPI object to describe it.
>>
>> Of course both devices are sharing an interrupt so
>> we should also indicate that in the table as well.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>>   Platform/RaspberryPi/AcpiTables/Dsdt.asl |  2 +
>>   Platform/RaspberryPi/AcpiTables/Sdhc.asl | 86 +++++++++++++++++++++++++++++++-
>>   2 files changed, 86 insertions(+), 2 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>> index d116f965e1..cca08c0539 100644
>> --- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>> +++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>> @@ -150,6 +150,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
>>             QWORDMEMORYBUF(23)
>>             QWORDMEMORYBUF(24)
>>             QWORDMEMORYBUF(25)
>> +          QWORDMEMORYBUF(26)
>>           })
>>   
>>           // USB
>> @@ -196,6 +197,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
>>           // SDC
>>           QWORDMEMORYSET(24, MMCHS1_OFFSET, MMCHS1_LENGTH)
>>           QWORDMEMORYSET(25, SDHOST_OFFSET, SDHOST_LENGTH)
>> +        QWORDMEMORYSET(26, MMCHS2_OFFSET, MMCHS2_LENGTH)
>>   
>>           Return (RBUF)
>>         }
>> diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>> index 0ab1ba27f2..a7ac831066 100644
>> --- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>> +++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>> @@ -19,7 +19,7 @@
>>   // Note: UEFI can use either SDHost or Arasan. We expose both to the OS.
>>   //
>>   
>> -// ArasanSD 3.0 SD Host Controller.
>> +// ArasanSD 3.0 SD Host Controller. (brcm,bcm2835-sdhci)
>>   Device (SDC1)
>>   {
>>     Name (_HID, "BCM2847")
>> @@ -37,7 +37,7 @@ Device (SDC1)
>>     Name (RBUF, ResourceTemplate ()
>>     {
>>       MEMORY32FIXED (ReadWrite, 0, MMCHS1_LENGTH, RMEM)
>> -    Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) { BCM2836_MMCHS1_INTERRUPT }
>> +    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_MMCHS1_INTERRUPT }
>>     })
>>     Method (_CRS, 0x0, Serialized)
>>     {
>> @@ -45,6 +45,86 @@ Device (SDC1)
>>       Return (^RBUF)
>>     }
>>   
>> +  // The standard CAPs registers on this controller
>> +  // appear to be 0, lets set some minimal defaults
>> +  // Since this cap doesn't indicate DMA capability
>> +  // we don't need a _DMA()
>> +  Name (_DSD, Package () {
>> +    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>> +    Package () {
>> +      Package () { "sdhci-caps", 0x0100fa81 },
>> +    }
>> +  })
>> +
>> +
>> +  //
>> +  // A child device that represents the
>> +  // sd card, which is marked as non-removable.
>> +  //
>> +  Device (SDMM)
>> +  {
>> +    Method (_ADR)
>> +    {
>> +      Return (0)
>> +    }
>> +    Method (_RMV) // Is removable
>> +    {
>> +      Return (0) // 0 - fixed
>> +    }
>> +  }
>> +}
>> +
>> +#if (RPI_MODEL == 4)
>> +// emmc2 Host Controller. (brcm,bcm2711-emmc2)
>> +Device (SDC3)
>> +{
>> +  Name (_HID, "BRCME88C")
>> +  Name (_UID, 0x1)
>> +  Name (_CCA, 0x0)
>> +  Name (_S1D, 0x1)
>> +  Name (_S2D, 0x1)
>> +  Name (_S3D, 0x1)
>> +  Name (_S4D, 0x1)
>> +  Method (_STA)
>> +  {
>> +    Return(0xf)
>> +  }
>> +  Name (RBUF, ResourceTemplate ()
>> +  {
>> +    MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)
>> +    Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) { BCM2836_MMCHS1_INTERRUPT }
>> +  })
>> +  Method (_CRS, 0x0, Serialized)
>> +  {
>> +    MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)
>> +    Return (^RBUF)
>> +  }
>> +
>> +  // forceably limit to 1G
>> +  Name (_DMA, ResourceTemplate() {
>> +    QWordMemory (ResourceConsumer,
>> +      ,
>> +      MinFixed,
>> +      MaxFixed,
>> +      NonCacheable,
>> +      ReadWrite,
>> +      0x0,
>> +      0x0,        // MIN
>> +      0x3fffffff, // MAX
>> +      0x0,        // TRA
>> +      0x40000000, // LEN
>> +      ,
>> +      ,
>> +    )
>> +  })
>> +
> 
> Shouldn't the _DMA method be on a container object instead of on the
> device object itself?

Pained cough... There is a _DMA on the gpu devs container of which this 
device belongs. That doesn't make the DMA work. The spec says that _DMA 
is to exist on the bus device, and I think what is happening here is 
that the "device" is the SDMM (which isn't directly visible because of 
the way diff broke this patch up) contained in the "SDC3 mmc bus".

Frankly, I could be wrong, but what I do know is that without that bit 
of ugliness DMA doesn't work in linux on this controller. I just 
stripped it out again to verify the failure with 4.11. It does the same 
thing, the controller itself seems to be working but the block device 
associated with it can't be read. There are few other possibilities 
(like maybe this controller shouldn't be a gpudevs child) and i'm fixing 
a bug I created..I had kernel instrumentation in 5.9 tracking the dma 
mask propagation, and I was fairly confident in this a couple months 
ago, less so now.



> 
> 
>> +  Name (_DSD, Package () {
>> +    ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>> +    Package () {
>> +      Package () { "sdhci-caps-mask", 0x0000000000080000 },
>> +    }
>> +  })
>> +
>>     //
>>     // A child device that represents the
>>     // sd card, which is marked as non-removable.
>> @@ -62,6 +142,7 @@ Device (SDC1)
>>     }
>>   }
>>   
>> +#else // !RPI4
>>   
>>   // Broadcom SDHost 2.0 SD Host Controller
>>   Device (SDC2)
>> @@ -105,3 +186,4 @@ Device (SDC2)
>>       }
>>     }
>>   }
>> +#endif // !RPI4
>>
> 

Re: [edk2-devel] [PATCH 2/3] Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

[Mark Kettenis]

[-- Attachment #1: Type: text/plain, Size: 7707 bytes --]

On Fri, Jan 8, 2021 at 05:27 PM, Jeremy Linton wrote:

> 
> Hi,
> 
> On 1/8/21 2:28 AM, Ard Biesheuvel wrote:
> 
>> On 1/8/21 7:16 AM, Jeremy Linton wrote:
>> 
>>> The primarly problem with the rpi Arasan controller working
>>> with a default SDHCI driver is the lack of a meaningful
>>> capabilities register. As such if we add a _DSD entry
>>> to provide that information, we can then bind it to
>>> the linux sdhci_iproc driver which already
>>> hardcodes the remaining controller bugs.
>>> 
>>> Further we have gotten BRCME88C approved as the HID
>>> for the newer eMMC2 controller. So lets define an
>>> ACPI object to describe it.
>>> 
>>> Of course both devices are sharing an interrupt so
>>> we should also indicate that in the table as well.
>>> 
>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>> ---
>>> Platform/RaspberryPi/AcpiTables/Dsdt.asl | 2 +
>>> Platform/RaspberryPi/AcpiTables/Sdhc.asl | 86
>>> +++++++++++++++++++++++++++++++-
>>> 2 files changed, 86 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>> b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>> index d116f965e1..cca08c0539 100644
>>> --- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>> +++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>> @@ -150,6 +150,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN",
>>> "RPI", 2)
>>> QWORDMEMORYBUF(23)
>>> QWORDMEMORYBUF(24)
>>> QWORDMEMORYBUF(25)
>>> + QWORDMEMORYBUF(26)
>>> })
>>> 
>>> // USB
>>> @@ -196,6 +197,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN",
>>> "RPI", 2)
>>> // SDC
>>> QWORDMEMORYSET(24, MMCHS1_OFFSET, MMCHS1_LENGTH)
>>> QWORDMEMORYSET(25, SDHOST_OFFSET, SDHOST_LENGTH)
>>> + QWORDMEMORYSET(26, MMCHS2_OFFSET, MMCHS2_LENGTH)
>>> 
>>> Return (RBUF)
>>> }
>>> diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>> b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>> index 0ab1ba27f2..a7ac831066 100644
>>> --- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>> +++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>> @@ -19,7 +19,7 @@
>>> // Note: UEFI can use either SDHost or Arasan. We expose both to the OS.
>>> //
>>> 
>>> -// ArasanSD 3.0 SD Host Controller.
>>> +// ArasanSD 3.0 SD Host Controller. (brcm,bcm2835-sdhci)
>>> Device (SDC1)
>>> {
>>> Name (_HID, "BCM2847")
>>> @@ -37,7 +37,7 @@ Device (SDC1)
>>> Name (RBUF, ResourceTemplate ()
>>> {
>>> MEMORY32FIXED (ReadWrite, 0, MMCHS1_LENGTH, RMEM)
>>> - Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
>>> BCM2836_MMCHS1_INTERRUPT }
>>> + Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) {
>>> BCM2836_MMCHS1_INTERRUPT }
>>> })
>>> Method (_CRS, 0x0, Serialized)
>>> {
>>> @@ -45,6 +45,86 @@ Device (SDC1)
>>> Return (^RBUF)
>>> }
>>> 
>>> + // The standard CAPs registers on this controller
>>> + // appear to be 0, lets set some minimal defaults
>>> + // Since this cap doesn't indicate DMA capability
>>> + // we don't need a _DMA()
>>> + Name (_DSD, Package () {
>>> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>> + Package () {
>>> + Package () { "sdhci-caps", 0x0100fa81 },
>>> + }
>>> + })
>>> +
>>> +
>>> + //
>>> + // A child device that represents the
>>> + // sd card, which is marked as non-removable.
>>> + //
>>> + Device (SDMM)
>>> + {
>>> + Method (_ADR)
>>> + {
>>> + Return (0)
>>> + }
>>> + Method (_RMV) // Is removable
>>> + {
>>> + Return (0) // 0 - fixed
>>> + }
>>> + }
>>> +}
>>> +
>>> +#if (RPI_MODEL == 4)
>>> +// emmc2 Host Controller. (brcm,bcm2711-emmc2)
>>> +Device (SDC3)
>>> +{
>>> + Name (_HID, "BRCME88C")
>>> + Name (_UID, 0x1)
>>> + Name (_CCA, 0x0)
>>> + Name (_S1D, 0x1)
>>> + Name (_S2D, 0x1)
>>> + Name (_S3D, 0x1)
>>> + Name (_S4D, 0x1)
>>> + Method (_STA)
>>> + {
>>> + Return(0xf)
>>> + }
>>> + Name (RBUF, ResourceTemplate ()
>>> + {
>>> + MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)
>>> + Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) {
>>> BCM2836_MMCHS1_INTERRUPT }
>>> + })
>>> + Method (_CRS, 0x0, Serialized)
>>> + {
>>> + MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)
>>> + Return (^RBUF)
>>> + }
>>> +
>>> + // forceably limit to 1G
>>> + Name (_DMA, ResourceTemplate() {
>>> + QWordMemory (ResourceConsumer,
>>> + ,
>>> + MinFixed,
>>> + MaxFixed,
>>> + NonCacheable,
>>> + ReadWrite,
>>> + 0x0,
>>> + 0x0, // MIN
>>> + 0x3fffffff, // MAX
>>> + 0x0, // TRA
>>> + 0x40000000, // LEN
>>> + ,
>>> + ,
>>> + )
>>> + })
>>> +
>> 
>> Shouldn't the _DMA method be on a container object instead of on the
>> device object itself?
> 
> Pained cough... There is a _DMA on the gpu devs container of which this
> device belongs. That doesn't make the DMA work. The spec says that _DMA
> is to exist on the bus device, and I think what is happening here is
> that the "device" is the SDMM (which isn't directly visible because of
> the way diff broke this patch up) contained in the "SDC3 mmc bus".

The Linux device tree has the following comment:

/*
* 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.
*

And the dma-ranges property it references is:

dma-ranges = <0x0 0xc0000000  0x0 0x00000000  0x40000000>;

Not sure how the firmware will adjust this, but it does imply that there is actual translation
going on here and it doesn't match the _DMA property you added.

> 
> Frankly, I could be wrong, but what I do know is that without that bit
> of ugliness DMA doesn't work in linux on this controller. I just
> stripped it out again to verify the failure with 4.11. It does the same
> thing, the controller itself seems to be working but the block device
> associated with it can't be read. There are few other possibilities
> (like maybe this controller shouldn't be a gpudevs child) and i'm fixing
> a bug I created..I had kernel instrumentation in 5.9 tracking the dma
> mask propagation, and I was fairly confident in this a couple months
> ago, less so now.
> 

One question I have is whether _DMA methods should be applied recursively or
not. If you have a bus that provides a translating _DMA method that is behind a
bus that also has a _DMA method, should the translation specified in that first _DMA
method be applied or not? The text in the ACPI spec is far for clear her...

If I read the Linux code in drivers/acpi/scan.c correctly, Linux would not apply
the translation in this case. Since the _DMA method you add does not specify any
translation, I think that means the required address translation isn't happening
which means that DMA transactions would fail. I suspect the block device behind
the controller would still be detected, but any actual I/O would fail since the right
memory isn't addressed.

So I think the best thing to do would be to have a separate ACPI0004 container
with a translating _DMA method and but the emmc2 device below that.

> 
> 
>> 
>>> + Name (_DSD, Package () {
>>> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>> + Package () {
>>> + Package () { "sdhci-caps-mask", 0x0000000000080000 },
>>> + }
>>> + })
>>> +
>>> //
>>> // A child device that represents the
>>> // sd card, which is marked as non-removable.
>>> @@ -62,6 +142,7 @@ Device (SDC1)
>>> }
>>> }
>>> 
>>> +#else // !RPI4
>>> 
>>> // Broadcom SDHost 2.0 SD Host Controller
>>> Device (SDC2)
>>> @@ -105,3 +186,4 @@ Device (SDC2)
>>> }
>>> }
>>> }
>>> +#endif // !RPI4
>> 
>> 
> 
>

[-- Attachment #2: Type: text/html, Size: 8264 bytes --]

Re: [edk2-devel] [PATCH 2/3] Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

[Mark Kettenis]

[-- Attachment #1: Type: text/plain, Size: 8254 bytes --]

On Sat, Jan 9, 2021 at 03:09 PM, Mark Kettenis wrote:

> 
> On Fri, Jan 8, 2021 at 05:27 PM, Jeremy Linton wrote:
> 
>> Hi,
>> 
>> On 1/8/21 2:28 AM, Ard Biesheuvel wrote:
>> 
>>> On 1/8/21 7:16 AM, Jeremy Linton wrote:
>>> 
>>>> The primarly problem with the rpi Arasan controller working
>>>> with a default SDHCI driver is the lack of a meaningful
>>>> capabilities register. As such if we add a _DSD entry
>>>> to provide that information, we can then bind it to
>>>> the linux sdhci_iproc driver which already
>>>> hardcodes the remaining controller bugs.
>>>> 
>>>> Further we have gotten BRCME88C approved as the HID
>>>> for the newer eMMC2 controller. So lets define an
>>>> ACPI object to describe it.
>>>> 
>>>> Of course both devices are sharing an interrupt so
>>>> we should also indicate that in the table as well.
>>>> 
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>> ---
>>>> Platform/RaspberryPi/AcpiTables/Dsdt.asl | 2 +
>>>> Platform/RaspberryPi/AcpiTables/Sdhc.asl | 86
>>>> +++++++++++++++++++++++++++++++-
>>>> 2 files changed, 86 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> index d116f965e1..cca08c0539 100644
>>>> --- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> +++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> @@ -150,6 +150,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN",
>>>> "RPI", 2)
>>>> QWORDMEMORYBUF(23)
>>>> QWORDMEMORYBUF(24)
>>>> QWORDMEMORYBUF(25)
>>>> + QWORDMEMORYBUF(26)
>>>> })
>>>> 
>>>> // USB
>>>> @@ -196,6 +197,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN",
>>>> "RPI", 2)
>>>> // SDC
>>>> QWORDMEMORYSET(24, MMCHS1_OFFSET, MMCHS1_LENGTH)
>>>> QWORDMEMORYSET(25, SDHOST_OFFSET, SDHOST_LENGTH)
>>>> + QWORDMEMORYSET(26, MMCHS2_OFFSET, MMCHS2_LENGTH)
>>>> 
>>>> Return (RBUF)
>>>> }
>>>> diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>>> b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>>> index 0ab1ba27f2..a7ac831066 100644
>>>> --- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>>> +++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>>> @@ -19,7 +19,7 @@
>>>> // Note: UEFI can use either SDHost or Arasan. We expose both to the OS.
>>>> //
>>>> 
>>>> -// ArasanSD 3.0 SD Host Controller.
>>>> +// ArasanSD 3.0 SD Host Controller. (brcm,bcm2835-sdhci)
>>>> Device (SDC1)
>>>> {
>>>> Name (_HID, "BCM2847")
>>>> @@ -37,7 +37,7 @@ Device (SDC1)
>>>> Name (RBUF, ResourceTemplate ()
>>>> {
>>>> MEMORY32FIXED (ReadWrite, 0, MMCHS1_LENGTH, RMEM)
>>>> - Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
>>>> BCM2836_MMCHS1_INTERRUPT }
>>>> + Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) {
>>>> BCM2836_MMCHS1_INTERRUPT }
>>>> })
>>>> Method (_CRS, 0x0, Serialized)
>>>> {
>>>> @@ -45,6 +45,86 @@ Device (SDC1)
>>>> Return (^RBUF)
>>>> }
>>>> 
>>>> + // The standard CAPs registers on this controller
>>>> + // appear to be 0, lets set some minimal defaults
>>>> + // Since this cap doesn't indicate DMA capability
>>>> + // we don't need a _DMA()
>>>> + Name (_DSD, Package () {
>>>> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>>> + Package () {
>>>> + Package () { "sdhci-caps", 0x0100fa81 },
>>>> + }
>>>> + })
>>>> +
>>>> +
>>>> + //
>>>> + // A child device that represents the
>>>> + // sd card, which is marked as non-removable.
>>>> + //
>>>> + Device (SDMM)
>>>> + {
>>>> + Method (_ADR)
>>>> + {
>>>> + Return (0)
>>>> + }
>>>> + Method (_RMV) // Is removable
>>>> + {
>>>> + Return (0) // 0 - fixed
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> +#if (RPI_MODEL == 4)
>>>> +// emmc2 Host Controller. (brcm,bcm2711-emmc2)
>>>> +Device (SDC3)
>>>> +{
>>>> + Name (_HID, "BRCME88C")
>>>> + Name (_UID, 0x1)
>>>> + Name (_CCA, 0x0)
>>>> + Name (_S1D, 0x1)
>>>> + Name (_S2D, 0x1)
>>>> + Name (_S3D, 0x1)
>>>> + Name (_S4D, 0x1)
>>>> + Method (_STA)
>>>> + {
>>>> + Return(0xf)
>>>> + }
>>>> + Name (RBUF, ResourceTemplate ()
>>>> + {
>>>> + MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)
>>>> + Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) {
>>>> BCM2836_MMCHS1_INTERRUPT }
>>>> + })
>>>> + Method (_CRS, 0x0, Serialized)
>>>> + {
>>>> + MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)
>>>> + Return (^RBUF)
>>>> + }
>>>> +
>>>> + // forceably limit to 1G
>>>> + Name (_DMA, ResourceTemplate() {
>>>> + QWordMemory (ResourceConsumer,
>>>> + ,
>>>> + MinFixed,
>>>> + MaxFixed,
>>>> + NonCacheable,
>>>> + ReadWrite,
>>>> + 0x0,
>>>> + 0x0, // MIN
>>>> + 0x3fffffff, // MAX
>>>> + 0x0, // TRA
>>>> + 0x40000000, // LEN
>>>> + ,
>>>> + ,
>>>> + )
>>>> + })
>>>> +
>>> 
>>> Shouldn't the _DMA method be on a container object instead of on the
>>> device object itself?
>> 
>> Pained cough... There is a _DMA on the gpu devs container of which this
>> device belongs. That doesn't make the DMA work. The spec says that _DMA
>> is to exist on the bus device, and I think what is happening here is
>> that the "device" is the SDMM (which isn't directly visible because of
>> the way diff broke this patch up) contained in the "SDC3 mmc bus".
> 
> The Linux device tree has the following comment:
> 
> /*
> * 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.
> *
> 
> And the dma-ranges property it references is:
> 
> dma-ranges = <0x0 0xc0000000  0x0 0x00000000  0x40000000>;
> 
> Not sure how the firmware will adjust this, but it does imply that there
> is actual translation
> going on here and it doesn't match the _DMA property you added.
> 

This is what I get on my Pi4:

dma-ranges: 00000000.c0000000.00000000.00000000.3c000000

So we don't have get the full 1G to play with on some revisions of the SoC.

> 
> 
>> Frankly, I could be wrong, but what I do know is that without that bit
>> of ugliness DMA doesn't work in linux on this controller. I just
>> stripped it out again to verify the failure with 4.11. It does the same
>> thing, the controller itself seems to be working but the block device
>> associated with it can't be read. There are few other possibilities
>> (like maybe this controller shouldn't be a gpudevs child) and i'm fixing
>> a bug I created..I had kernel instrumentation in 5.9 tracking the dma
>> mask propagation, and I was fairly confident in this a couple months
>> ago, less so now.
>> 
> 
> One question I have is whether _DMA methods should be applied recursively
> or
> not. If you have a bus that provides a translating _DMA method that is
> behind a
> bus that also has a _DMA method, should the translation specified in that
> first _DMA
> method be applied or not? The text in the ACPI spec is far for clear
> her...
> 
> If I read the Linux code in drivers/acpi/scan.c correctly, Linux would not
> apply
> the translation in this case. Since the _DMA method you add does not
> specify any
> translation, I think that means the required address translation isn't
> happening
> which means that DMA transactions would fail. I suspect the block device
> behind
> the controller would still be detected, but any actual I/O would fail
> since the right
> memory isn't addressed.
> 
> So I think the best thing to do would be to have a separate ACPI0004
> container
> with a translating _DMA method and but the emmc2 device below that.
> 
> 
>> 
>>> 
>>>> + Name (_DSD, Package () {
>>>> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>>> + Package () {
>>>> + Package () { "sdhci-caps-mask", 0x0000000000080000 },
>>>> + }
>>>> + })
>>>> +
>>>> //
>>>> // A child device that represents the
>>>> // sd card, which is marked as non-removable.
>>>> @@ -62,6 +142,7 @@ Device (SDC1)
>>>> }
>>>> }
>>>> 
>>>> +#else // !RPI4
>>>> 
>>>> // Broadcom SDHost 2.0 SD Host Controller
>>>> Device (SDC2)
>>>> @@ -105,3 +186,4 @@ Device (SDC2)
>>>> }
>>>> }
>>>> }
>>>> +#endif // !RPI4
>>> 
>>> 
>> 
>> 
> 
>

[-- Attachment #2: Type: text/html, Size: 8598 bytes --]

Re: [edk2-devel] [PATCH 2/3] Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

[Jeremy Linton]

Hi,

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


First, thanks for looking at this!

The way I understand the spec, one does have to honor nested translations.

> 
> If I read the Linux code in drivers/acpi/scan.c correctly, Linux would not apply
> the translation in this case. Since the _DMA method you add does not specify any
> translation, I think that means the required address translation isn't happening
> which means that DMA transactions would fail. I suspect the block device behind
> the controller would still be detected, but any actual I/O would fail since the right
> memory isn't addressed.
> 
> So I think the best thing to do would be to have a separate ACPI0004 container
> with a translating _DMA method and but the emmc2 device below that.

I think if you look at the rpi's Dsdt, you will see an ACPI0004 
container, which includes the GpuDevs, of which Sdhc.asl is child. That 
container has a _DMA method which specifies the 1G translation+limit 
which matches what you would expect based on the DT.

So this further _DMA should be layering on it and re-enforcing only the 
DMA limit, not a new translation.

> 
>>
>>
>>>
>>>> + Name (_DSD, Package () {
>>>> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>>> + Package () {
>>>> + Package () { "sdhci-caps-mask", 0x0000000000080000 },
>>>> + }
>>>> + })
>>>> +
>>>> //
>>>> // A child device that represents the
>>>> // sd card, which is marked as non-removable.
>>>> @@ -62,6 +142,7 @@ Device (SDC1)
>>>> }
>>>> }
>>>>
>>>> +#else // !RPI4
>>>>
>>>> // Broadcom SDHost 2.0 SD Host Controller
>>>> Device (SDC2)
>>>> @@ -105,3 +186,4 @@ Device (SDC2)
>>>> }
>>>> }
>>>> }
>>>> +#endif // !RPI4
>>>
>>>
>>
>>
> 
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 2/3] Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

[Jeremy Linton]

Hi,


On 1/9/21 8:09 AM, Mark Kettenis via groups.io wrote:
> On Fri, Jan 8, 2021 at 05:27 PM, Jeremy Linton wrote:
> 
>>
>> Hi,
>>
>> On 1/8/21 2:28 AM, Ard Biesheuvel wrote:
>>
>>> On 1/8/21 7:16 AM, Jeremy Linton wrote:
>>>
>>>> The primarly problem with the rpi Arasan controller working
>>>> with a default SDHCI driver is the lack of a meaningful
>>>> capabilities register. As such if we add a _DSD entry
>>>> to provide that information, we can then bind it to
>>>> the linux sdhci_iproc driver which already
>>>> hardcodes the remaining controller bugs.
>>>>
>>>> Further we have gotten BRCME88C approved as the HID
>>>> for the newer eMMC2 controller. So lets define an
>>>> ACPI object to describe it.
>>>>
>>>> Of course both devices are sharing an interrupt so
>>>> we should also indicate that in the table as well.
>>>>
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>> ---
>>>> Platform/RaspberryPi/AcpiTables/Dsdt.asl | 2 +
>>>> Platform/RaspberryPi/AcpiTables/Sdhc.asl | 86
>>>> +++++++++++++++++++++++++++++++-
>>>> 2 files changed, 86 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> index d116f965e1..cca08c0539 100644
>>>> --- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> +++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
>>>> @@ -150,6 +150,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN",
>>>> "RPI", 2)
>>>> QWORDMEMORYBUF(23)
>>>> QWORDMEMORYBUF(24)
>>>> QWORDMEMORYBUF(25)
>>>> + QWORDMEMORYBUF(26)
>>>> })
>>>>
>>>> // USB
>>>> @@ -196,6 +197,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN",
>>>> "RPI", 2)
>>>> // SDC
>>>> QWORDMEMORYSET(24, MMCHS1_OFFSET, MMCHS1_LENGTH)
>>>> QWORDMEMORYSET(25, SDHOST_OFFSET, SDHOST_LENGTH)
>>>> + QWORDMEMORYSET(26, MMCHS2_OFFSET, MMCHS2_LENGTH)
>>>>
>>>> Return (RBUF)
>>>> }
>>>> diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>>> b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>>> index 0ab1ba27f2..a7ac831066 100644
>>>> --- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>>> +++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
>>>> @@ -19,7 +19,7 @@
>>>> // Note: UEFI can use either SDHost or Arasan. We expose both to the OS.
>>>> //
>>>>
>>>> -// ArasanSD 3.0 SD Host Controller.
>>>> +// ArasanSD 3.0 SD Host Controller. (brcm,bcm2835-sdhci)
>>>> Device (SDC1)
>>>> {
>>>> Name (_HID, "BCM2847")
>>>> @@ -37,7 +37,7 @@ Device (SDC1)
>>>> Name (RBUF, ResourceTemplate ()
>>>> {
>>>> MEMORY32FIXED (ReadWrite, 0, MMCHS1_LENGTH, RMEM)
>>>> - Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive) {
>>>> BCM2836_MMCHS1_INTERRUPT }
>>>> + Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) {
>>>> BCM2836_MMCHS1_INTERRUPT }
>>>> })
>>>> Method (_CRS, 0x0, Serialized)
>>>> {
>>>> @@ -45,6 +45,86 @@ Device (SDC1)
>>>> Return (^RBUF)
>>>> }
>>>>
>>>> + // The standard CAPs registers on this controller
>>>> + // appear to be 0, lets set some minimal defaults
>>>> + // Since this cap doesn't indicate DMA capability
>>>> + // we don't need a _DMA()
>>>> + Name (_DSD, Package () {
>>>> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>>> + Package () {
>>>> + Package () { "sdhci-caps", 0x0100fa81 },
>>>> + }
>>>> + })
>>>> +
>>>> +
>>>> + //
>>>> + // A child device that represents the
>>>> + // sd card, which is marked as non-removable.
>>>> + //
>>>> + Device (SDMM)
>>>> + {
>>>> + Method (_ADR)
>>>> + {
>>>> + Return (0)
>>>> + }
>>>> + Method (_RMV) // Is removable
>>>> + {
>>>> + Return (0) // 0 - fixed
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> +#if (RPI_MODEL == 4)
>>>> +// emmc2 Host Controller. (brcm,bcm2711-emmc2)
>>>> +Device (SDC3)
>>>> +{
>>>> + Name (_HID, "BRCME88C")
>>>> + Name (_UID, 0x1)
>>>> + Name (_CCA, 0x0)
>>>> + Name (_S1D, 0x1)
>>>> + Name (_S2D, 0x1)
>>>> + Name (_S3D, 0x1)
>>>> + Name (_S4D, 0x1)
>>>> + Method (_STA)
>>>> + {
>>>> + Return(0xf)
>>>> + }
>>>> + Name (RBUF, ResourceTemplate ()
>>>> + {
>>>> + MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)
>>>> + Interrupt (ResourceConsumer, Level, ActiveHigh, Shared) {
>>>> BCM2836_MMCHS1_INTERRUPT }
>>>> + })
>>>> + Method (_CRS, 0x0, Serialized)
>>>> + {
>>>> + MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)
>>>> + Return (^RBUF)
>>>> + }
>>>> +
>>>> + // forceably limit to 1G
>>>> + Name (_DMA, ResourceTemplate() {
>>>> + QWordMemory (ResourceConsumer,
>>>> + ,
>>>> + MinFixed,
>>>> + MaxFixed,
>>>> + NonCacheable,
>>>> + ReadWrite,
>>>> + 0x0,
>>>> + 0x0, // MIN
>>>> + 0x3fffffff, // MAX
>>>> + 0x0, // TRA
>>>> + 0x40000000, // LEN
>>>> + ,
>>>> + ,
>>>> + )
>>>> + })
>>>> +
>>>
>>> Shouldn't the _DMA method be on a container object instead of on the
>>> device object itself?
>>
>> Pained cough... There is a _DMA on the gpu devs container of which this
>> device belongs. That doesn't make the DMA work. The spec says that _DMA
>> is to exist on the bus device, and I think what is happening here is
>> that the "device" is the SDMM (which isn't directly visible because of
>> the way diff broke this patch up) contained in the "SDC3 mmc bus".
> 
> The Linux device tree has the following comment:
> 
> /*
> * 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.
> *
> 
> And the dma-ranges property it references is:
> 
> dma-ranges = <0x0 0xc0000000  0x0 0x00000000  0x40000000>;
> 
> Not sure how the firmware will adjust this, but it does imply that there is actual translation
> going on here and it doesn't match the _DMA property you added.

Just to add to this, its quite possible this shouldn't be a gpudev, as 
I mentioned above. I need to test a couple of the rpi4's i've got to see 
what the variation they mention above is.

AKA, the DT you feed into the videocore isn't always the DT you get out 
of it, so you can't depend on reading the raw DT. That is what is 
happening with the PCIe now, the firmware is modifying the mappings 
based on SOC revision.


> 
>>
>> Frankly, I could be wrong, but what I do know is that without that bit
>> of ugliness DMA doesn't work in linux on this controller. I just
>> stripped it out again to verify the failure with 4.11. It does the same
>> thing, the controller itself seems to be working but the block device
>> associated with it can't be read. There are few other possibilities
>> (like maybe this controller shouldn't be a gpudevs child) and i'm fixing
>> a bug I created..I had kernel instrumentation in 5.9 tracking the dma
>> mask propagation, and I was fairly confident in this a couple months
>> ago, less so now.
>>
> 
> One question I have is whether _DMA methods should be applied recursively or
> not. If you have a bus that provides a translating _DMA method that is behind a
> bus that also has a _DMA method, should the translation specified in that first _DMA
> method be applied or not? The text in the ACPI spec is far for clear her...
> 
> If I read the Linux code in drivers/acpi/scan.c correctly, Linux would not apply
> the translation in this case. Since the _DMA method you add does not specify any
> translation, I think that means the required address translation isn't happening
> which means that DMA transactions would fail. I suspect the block device behind
> the controller would still be detected, but any actual I/O would fail since the right
> memory isn't addressed.
> 
> So I think the best thing to do would be to have a separate ACPI0004 container
> with a translating _DMA method and but the emmc2 device below that.
> 
>>
>>
>>>
>>>> + Name (_DSD, Package () {
>>>> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>>>> + Package () {
>>>> + Package () { "sdhci-caps-mask", 0x0000000000080000 },
>>>> + }
>>>> + })
>>>> +
>>>> //
>>>> // A child device that represents the
>>>> // sd card, which is marked as non-removable.
>>>> @@ -62,6 +142,7 @@ Device (SDC1)
>>>> }
>>>> }
>>>>
>>>> +#else // !RPI4
>>>>
>>>> // Broadcom SDHost 2.0 SD Host Controller
>>>> Device (SDC2)
>>>> @@ -105,3 +186,4 @@ Device (SDC2)
>>>> }
>>>> }
>>>> }
>>>> +#endif // !RPI4
>>>
>>>
>>
>>
> 
> 
> 
> 
> 
> 

Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Further lower the DMA zone

[Ard Biesheuvel]

On 1/8/21 7:16 AM, Jeremy Linton wrote:
> We are going to add a _DMA() for the new controller, but
> it won't work right with the linux driver unless we lower
> the DMA zone again.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  Platform/RaspberryPi/AcpiTables/Iort.aslc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Iort.aslc b/Platform/RaspberryPi/AcpiTables/Iort.aslc
> index 00720194bb..f576fa3b04 100644
> --- a/Platform/RaspberryPi/AcpiTables/Iort.aslc
> +++ b/Platform/RaspberryPi/AcpiTables/Iort.aslc
> @@ -46,7 +46,7 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
>        0x0,                                          // AllocationHints
>        0x0,                                          // Reserved
>        0x0,                                          // MemoryAccessFlags
> -      31,                                           // AddressSizeLimit
> +      30,                                           // AddressSizeLimit
>      }, {
>        "\\_SB_.SCB0.XHC0"                            // ObjectName
>      }
> 

If you create a new named component node here for the device(s) that
actually need this lower limit, you can avoid the _DMA method altogether.

Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Further lower the DMA zone

[Andrei Warkentin]

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

Hi Ard and Jeremy,

Let's keep _DMA regardless. Not all OSes support the IORT mechanism for DMA restrictions....

A


---
Отправлено из Workspace ONE Boxer<https://whatisworkspaceone.com/boxer>

11 января 2021 г. в 2:50:34 AM GMT-6 Ard Biesheuvel <ard.biesheuvel@arm.com> пишет:
On 1/8/21 7:16 AM, Jeremy Linton wrote:
> We are going to add a _DMA() for the new controller, but
> it won't work right with the linux driver unless we lower
> the DMA zone again.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  Platform/RaspberryPi/AcpiTables/Iort.aslc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/AcpiTables/Iort.aslc b/Platform/RaspberryPi/AcpiTables/Iort.aslc
> index 00720194bb..f576fa3b04 100644
> --- a/Platform/RaspberryPi/AcpiTables/Iort.aslc
> +++ b/Platform/RaspberryPi/AcpiTables/Iort.aslc
> @@ -46,7 +46,7 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
>        0x0,                                          // AllocationHints
>        0x0,                                          // Reserved
>        0x0,                                          // MemoryAccessFlags
> -      31,                                           // AddressSizeLimit
> +      30,                                           // AddressSizeLimit
>      }, {
>        "\\_SB_.SCB0.XHC0"                            // ObjectName
>      }
>

If you create a new named component node here for the device(s) that
actually need this lower limit, you can avoid the _DMA method altogether.



[-- Attachment #2: Type: text/html, Size: 4425 bytes --]

Re: [edk2-devel] [PATCH 2/3] Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

[Mark Kettenis]

[-- Attachment #1: Type: text/plain, Size: 8757 bytes --]

On Mon, Jan 11, 2021 at 02:45 AM, Jeremy Linton wrote:

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

As I wrote upthread, I suspect the Linux kernel isn't handling this properly.
The code walks up the ACPI device tree until it finds a _DMA method and uses
the information from that instance to calculate the translation offset and the
mask for the DMA limit. That would result in a translation offset of 0 for the emmc2
device which wouldn't work.

Given that this 2nd _DMA method isn't really necessary, I think it would be
better to drop it.

[-- Attachment #2: Type: text/html, Size: 8619 bytes --]