Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

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

Posted by Jeremy Linton 3 years, 2 months ago

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/AcpiTables.inf     |   1 +
 Platform/RaspberryPi/AcpiTables/Emmc.asl           | 131 +++++++++++++++++++++
 Platform/RaspberryPi/AcpiTables/Sdhc.asl           |  18 ++-
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |   6 +
 4 files changed, 153 insertions(+), 3 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/Emmc.asl

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
index d2cce074e5..743261afcf 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
@@ -35,6 +35,7 @@
   Spcr.aslc
   Pptt.aslc
   SsdtThermal.asl
+  Emmc.asl
 
 [Packages]
   ArmPkg/ArmPkg.dec
diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl b/Platform/RaspberryPi/AcpiTables/Emmc.asl
new file mode 100644
index 0000000000..f089068556
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
@@ -0,0 +1,131 @@
+/** @file
+ *
+ *  Copyright (c) 2021 Arm. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include <IndustryStandard/Bcm2836SdHost.h>
+#include <IndustryStandard/Bcm2836Sdio.h>
+
+#include "AcpiTables.h"
+
+DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI4EMMC", 2)
+{
+  Scope (\_SB_)
+  {
+#if (RPI_MODEL == 4)
+    Device (GDV1) {
+      Name (_HID, "ACPI0004")
+      Name (_UID, 0x0)
+      Name (_CCA, 0x0)
+
+      Name (RBUF, ResourceTemplate ()
+      {
+          MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)
+      })
+      Method (_CRS, 0x0, Serialized)
+      {
+        MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)
+        Return (^RBUF)
+      }
+
+      Name (_DMA, ResourceTemplate() {
+        /*
+         * XHC0 is limited to DMA to first 3GB. Note this
+         * only applies to PCIe, not GENET or other devices
+         * next to the A72.
+         */
+        QWordMemory (ResourceConsumer,
+            ,
+            MinFixed,
+            MaxFixed,
+            NonCacheable,
+            ReadWrite,
+            0x0,
+            0x00000000C0000000, // MIN
+            0x00000000FFFFFFFF, // MAX
+            0xFFFFFFFF40000000, // TRA
+            0x0000000040000000, // LEN
+            ,
+            ,
+            )
+      })
+      
+      // 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)
+	Name (SDMA, 0x2)
+	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)
+	}
+
+        // Unfortunatly this controller doesn't honor the
+        // standard sdhci voltage control registers
+        // (or at least linux's standard code can't 
+        // lower the voltage) So, UHS mode is disabled with caps
+        Name (DSD1, Package () {
+            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+            Package () {
+               Package () { "sdhci-caps-mask", 0x0000000500080000 },
+	    }
+        })
+	// We also disable both SDMA and ADMA2 until the linux
+	// _DMA() mask/translate works properly
+        Name (DSD2, Package () {
+            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+            Package () {
+               Package () { "sdhci-caps-mask", 0x0000000504480000 },
+	    }
+        })
+	Method (_DSD, 0x0, Serialized)
+        {
+          if (SDMA == 0) 
+          {
+            return (^DSD2)
+          } 
+          else 
+          {
+            return (^DSD1)
+          }
+        }
+
+        //
+        // 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
+          }
+        }
+      } //SDC3
+    } //GDV1
+#endif
+  } //\SB
+}
diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
index 0ab1ba27f2..0430ab7d2d 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,17 @@ 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.
@@ -62,7 +73,7 @@ Device (SDC1)
   }
 }
 
-
+#if (RPI_MODEL < 4)
 // Broadcom SDHost 2.0 SD Host Controller
 Device (SDC2)
 {
@@ -105,3 +116,4 @@ Device (SDC2)
     }
   }
 }
+#endif // !RPI4
diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index ca7533cbee..7f26f7b4be 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -738,6 +738,12 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] = {
     SsdtNameOpReplace
   },
   {
+    SIGNATURE_64 ('R', 'P', 'I', '4', 'E', 'M', 'M', 'C'),
+    0,
+    PcdToken(PcdSdIsArasan),
+    NULL
+  },
+  {
     SIGNATURE_64 ('R', 'P', 'I', 0, 0, 0, 0, 0),
     0,
     0,
-- 
2.13.7



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71035): https://edk2.groups.io/g/devel/message/71035
Mute This Topic: https://groups.io/mt/80300533/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Andrei Warkentin 3 years, 2 months ago

[with all of Pete's comments]

Reviewed-by: Andrei Warkentin <awarkentin@vmware.com>
________________________________
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Monday, February 1, 2021 4:53 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: pete@akeo.ie <pete@akeo.ie>; Andrei Warkentin <awarkentin@vmware.com>; samer.el-haj-mahmoud@arm.com <samer.el-haj-mahmoud@arm.com>; leif@nuviainc.com <leif@nuviainc.com>; ardb+tianocore@kernel.org <ardb+tianocore@kernel.org>; Jeremy Linton <jeremy.linton@arm.com>
Subject: [PATCH] Platform/RaspberryPi/Acpitables: Add eMMC2 device and tweak Arasan

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/AcpiTables.inf     |   1 +
 Platform/RaspberryPi/AcpiTables/Emmc.asl           | 131 +++++++++++++++++++++
 Platform/RaspberryPi/AcpiTables/Sdhc.asl           |  18 ++-
 Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |   6 +
 4 files changed, 153 insertions(+), 3 deletions(-)
 create mode 100644 Platform/RaspberryPi/AcpiTables/Emmc.asl

diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
index d2cce074e5..743261afcf 100644
--- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
+++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
@@ -35,6 +35,7 @@
   Spcr.aslc

   Pptt.aslc

   SsdtThermal.asl

+  Emmc.asl



 [Packages]

   ArmPkg/ArmPkg.dec

diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl b/Platform/RaspberryPi/AcpiTables/Emmc.asl
new file mode 100644
index 0000000000..f089068556
--- /dev/null
+++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
@@ -0,0 +1,131 @@
+/** @file
+ *
+ *  Copyright (c) 2021 Arm. All rights reserved.
+ *
+ *  SPDX-License-Identifier: BSD-2-Clause-Patent
+ *
+ **/
+
+#include <IndustryStandard/Bcm2836SdHost.h>
+#include <IndustryStandard/Bcm2836Sdio.h>
+
+#include "AcpiTables.h"
+
+DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI4EMMC", 2)
+{
+  Scope (\_SB_)
+  {
+#if (RPI_MODEL == 4)
+    Device (GDV1) {
+      Name (_HID, "ACPI0004")
+      Name (_UID, 0x0)
+      Name (_CCA, 0x0)
+
+      Name (RBUF, ResourceTemplate ()
+      {
+          MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)
+      })
+      Method (_CRS, 0x0, Serialized)
+      {
+        MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)
+        Return (^RBUF)
+      }
+
+      Name (_DMA, ResourceTemplate() {
+        /*
+         * XHC0 is limited to DMA to first 3GB. Note this
+         * only applies to PCIe, not GENET or other devices
+         * next to the A72.
+         */
+        QWordMemory (ResourceConsumer,
+            ,
+            MinFixed,
+            MaxFixed,
+            NonCacheable,
+            ReadWrite,
+            0x0,
+            0x00000000C0000000, // MIN
+            0x00000000FFFFFFFF, // MAX
+            0xFFFFFFFF40000000, // TRA
+            0x0000000040000000, // LEN
+            ,
+            ,
+            )
+      })
+
+      // 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)
+       Name (SDMA, 0x2)
+       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)
+       }
+
+        // Unfortunatly this controller doesn't honor the
+        // standard sdhci voltage control registers
+        // (or at least linux's standard code can't
+        // lower the voltage) So, UHS mode is disabled with caps
+        Name (DSD1, Package () {
+            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+            Package () {
+               Package () { "sdhci-caps-mask", 0x0000000500080000 },
+           }
+        })
+       // We also disable both SDMA and ADMA2 until the linux
+       // _DMA() mask/translate works properly
+        Name (DSD2, Package () {
+            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+            Package () {
+               Package () { "sdhci-caps-mask", 0x0000000504480000 },
+           }
+        })
+       Method (_DSD, 0x0, Serialized)
+        {
+          if (SDMA == 0)
+          {
+            return (^DSD2)
+          }
+          else
+          {
+            return (^DSD1)
+          }
+        }
+
+        //
+        // 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
+          }
+        }
+      } //SDC3
+    } //GDV1
+#endif
+  } //\SB
+}
diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
index 0ab1ba27f2..0430ab7d2d 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,17 @@ 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.

@@ -62,7 +73,7 @@ Device (SDC1)
   }

 }



-

+#if (RPI_MODEL < 4)

 // Broadcom SDHost 2.0 SD Host Controller

 Device (SDC2)

 {

@@ -105,3 +116,4 @@ Device (SDC2)
     }

   }

 }

+#endif // !RPI4

diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
index ca7533cbee..7f26f7b4be 100644
--- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
+++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
@@ -738,6 +738,12 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] = {
     SsdtNameOpReplace

   },

   {

+    SIGNATURE_64 ('R', 'P', 'I', '4', 'E', 'M', 'M', 'C'),

+    0,

+    PcdToken(PcdSdIsArasan),

+    NULL

+  },

+  {

     SIGNATURE_64 ('R', 'P', 'I', 0, 0, 0, 0, 0),

     0,

     0,

--
2.13.7



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71477): https://edk2.groups.io/g/devel/message/71477
Mute This Topic: https://groups.io/mt/80300533/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

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

Posted by Pete Batard 3 years, 2 months ago

On 2021.02.01 22:53, Jeremy Linton wrote:
> The primarly problem with the rpi Arasan controller working

Small typo: primarly -> primary

> 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/AcpiTables.inf     |   1 +
>   Platform/RaspberryPi/AcpiTables/Emmc.asl           | 131 +++++++++++++++++++++
>   Platform/RaspberryPi/AcpiTables/Sdhc.asl           |  18 ++-
>   Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c |   6 +
>   4 files changed, 153 insertions(+), 3 deletions(-)
>   create mode 100644 Platform/RaspberryPi/AcpiTables/Emmc.asl
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> index d2cce074e5..743261afcf 100644
> --- a/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> +++ b/Platform/RaspberryPi/AcpiTables/AcpiTables.inf
> @@ -35,6 +35,7 @@
>     Spcr.aslc
> 
>     Pptt.aslc
> 
>     SsdtThermal.asl
> 
> +  Emmc.asl
> 
>   
> 
>   [Packages]
> 
>     ArmPkg/ArmPkg.dec
> 
> diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl b/Platform/RaspberryPi/AcpiTables/Emmc.asl
> new file mode 100644
> index 0000000000..f089068556
> --- /dev/null
> +++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
> @@ -0,0 +1,131 @@
> +/** @file
> + *
> + *  Copyright (c) 2021 Arm. All rights reserved.
> + *
> + *  SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + **/
> +
> +#include <IndustryStandard/Bcm2836SdHost.h>
> +#include <IndustryStandard/Bcm2836Sdio.h>
> +
> +#include "AcpiTables.h"
> +
> +DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI4EMMC", 2)
> +{
> +  Scope (\_SB_)
> +  {
> +#if (RPI_MODEL == 4)
> +    Device (GDV1) {
> +      Name (_HID, "ACPI0004")
> +      Name (_UID, 0x0)
> +      Name (_CCA, 0x0)
> +
> +      Name (RBUF, ResourceTemplate ()
> +      {
> +          MEMORY32FIXED (ReadWrite, 0, MMCHS2_LENGTH, RMEM)
> +      })
> +      Method (_CRS, 0x0, Serialized)
> +      {
> +        MEMORY32SETBASE (RBUF, RMEM, RBAS, MMCHS2_OFFSET)
> +        Return (^RBUF)
> +      }
> +
> +      Name (_DMA, ResourceTemplate() {
> +        /*
> +         * XHC0 is limited to DMA to first 3GB. Note this
> +         * only applies to PCIe, not GENET or other devices
> +         * next to the A72.
> +         */

I don't think this comment, that appears to have been lifted from a 
copy/paste of 
https://github.com/tianocore/edk2-platforms/blob/master/Platform/RaspberryPi/AcpiTables/Xhci.asl#L62-L82, 
applies here, since you are no longer describing a [0x00000000, 
0xbfffffff] range here (or dealing with the XHC0 ACPI device) but 
applying a translation.

I would either remove the comment or make it explain why the translation 
is needed.

> +        QWordMemory (ResourceConsumer,
> +            ,
> +            MinFixed,
> +            MaxFixed,
> +            NonCacheable,
> +            ReadWrite,
> +            0x0,
> +            0x00000000C0000000, // MIN
> +            0x00000000FFFFFFFF, // MAX
> +            0xFFFFFFFF40000000, // TRA
> +            0x0000000040000000, // LEN
> +            ,
> +            ,
> +            )
> +      })
> +
> +      // emmc2 Host Controller. (brcm,bcm2711-emmc2)
> +      Device (SDC3)
> +      {
> +        Name (_HID, "BRCME88C")
> +        Name (_UID, 0x1)

vvvvvvvvvvvv

> +	Name (_CCA, 0x0)
> +	Name (_S1D, 0x1)
> +	Name (_S2D, 0x1)
> +	Name (_S3D, 0x1)
> +	Name (_S4D, 0x1)
> +	Name (SDMA, 0x2)
> +	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)
> +	}

^^^^^^^^^^^^^
Section above uses tabs instead of spaces.
I believe edk2/BaseTools/Scripts/PatchCheck.py, when run, should warn 
you about this.

> +
> +        // Unfortunatly this controller doesn't honor the

Small typo: Unfortunatly -> Unfortunately

> +        // standard sdhci voltage control registers
> +        // (or at least linux's standard code can't
> +        // lower the voltage) So, UHS mode is disabled with caps
> +        Name (DSD1, Package () {
> +            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +            Package () {
> +               Package () { "sdhci-caps-mask", 0x0000000500080000 },
> +	    }
> +        })

vvvvvvvvvvvv

> +	// We also disable both SDMA and ADMA2 until the linux
> +	// _DMA() mask/translate works properly

^^^^^^^^^^^^^
Another tabbed section

Also, the "also" above seems to imply that DSD1 applies on top of DSD2, 
whereas, per the code further down, we only apply one of DSD1 or DSD2 
according to SDMA. Maybe, for clarity, this should be mentioned more 
explicitly in the comments above?

> +        Name (DSD2, Package () {
> +            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> +            Package () {
> +               Package () { "sdhci-caps-mask", 0x0000000504480000 },
> +	    }

^^^^^^^^^^^^^
Another tabbed line

> +        })
> +	Method (_DSD, 0x0, Serialized)

^^^^^^^^^^^^^
Another tabbed line

> +        {
> +          if (SDMA == 0)
> +          {
> +            return (^DSD2)
> +          }
> +          else
> +          {
> +            return (^DSD1)
> +          }
> +        }
> +
> +        //
> +        // 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
> +          }
> +        }
> +      } //SDC3
> +    } //GDV1
> +#endif
> +  } //\SB
> +}
> diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> index 0ab1ba27f2..0430ab7d2d 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,17 @@ 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.
> 
> @@ -62,7 +73,7 @@ Device (SDC1)
>     }
> 
>   }
> 
>   
> 
> -
> 
> +#if (RPI_MODEL < 4)
> 
>   // Broadcom SDHost 2.0 SD Host Controller
> 
>   Device (SDC2)
> 
>   {
> 
> @@ -105,3 +116,4 @@ Device (SDC2)
>       }
> 
>     }
> 
>   }
> 
> +#endif // !RPI4
> 
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index ca7533cbee..7f26f7b4be 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -738,6 +738,12 @@ STATIC CONST NAMESPACE_TABLES SdtTables[] = {
>       SsdtNameOpReplace
> 
>     },
> 
>     {
> 
> +    SIGNATURE_64 ('R', 'P', 'I', '4', 'E', 'M', 'M', 'C'),
> 
> +    0,
> 
> +    PcdToken(PcdSdIsArasan),
> 
> +    NULL
> 
> +  },
> 
> +  {
> 
>       SIGNATURE_64 ('R', 'P', 'I', 0, 0, 0, 0, 0),
> 
>       0,
> 
>       0,
> 

With all of the above being formatting/comment/typos issues:
Reviewed-by: Pete Batard <pete@akeo.ie>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71473): https://edk2.groups.io/g/devel/message/71473
Mute This Topic: https://groups.io/mt/80300533/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-