[PATCH v2 edk2-platforms 0/3] Re-enable the second SATA controller on Overdrive

[PATCH v2 edk2-platforms 0/3] Re-enable the second SATA controller on Overdrive

[Alan Ott]

Without the PCD for the second SATA Controller being specified, the boot
will hang. These patches fix it.

Alan Ott (3):
  Silicon/AMD/Styx: Make PcdSataPortMode 32 bits
  Silicon/AMD/Styx: Use PcdSataPortMode properly for two controllers
  Platform/AMD/OverdriveBoard: Re-enable the second SATA Controller

 Platform/AMD/OverdriveBoard/OverdriveBoard.dsc              | 10 +++-------
 Silicon/AMD/Styx/AmdStyx.dec                                |  2 +-
 .../AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c   | 13 +++++++++----
 3 files changed, 13 insertions(+), 12 deletions(-)

-- 
2.9.3

[PATCH v2 edk2-platforms 1/3] Silicon/AMD/Styx: Make PcdSataPortMode 32 bits

[Alan Ott]

Extra bits are needed to accomodate all 14 SATA ports

Signed-off-by: Alan Ott <alan@softiron.com>
Contributed-under: TianoCore Contribution Agreement 1.0
---
 Silicon/AMD/Styx/AmdStyx.dec                                  | 2 +-
 Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Silicon/AMD/Styx/AmdStyx.dec b/Silicon/AMD/Styx/AmdStyx.dec
index ddd5bf4..c6eebe6 100644
--- a/Silicon/AMD/Styx/AmdStyx.dec
+++ b/Silicon/AMD/Styx/AmdStyx.dec
@@ -54,7 +54,7 @@
   gAmdStyxTokenSpaceGuid.PcdSata0CtrlAxiSlvPort|0xE0300000|UINT32|0x00020000
   gAmdStyxTokenSpaceGuid.PcdSata0PortCount|8|UINT8|0x00020001
   gAmdStyxTokenSpaceGuid.PcdSataPi|0xFF|UINT32|0x00020002
-  gAmdStyxTokenSpaceGuid.PcdSataPortMode|0|UINT16|0x00020003
+  gAmdStyxTokenSpaceGuid.PcdSataPortMode|0|UINT32|0x00020003
   gAmdStyxTokenSpaceGuid.PcdSataPortMpsp|TRUE|BOOLEAN|0x00020004
   gAmdStyxTokenSpaceGuid.PcdSataSmpsSupport|FALSE|BOOLEAN|0x00020005
   gAmdStyxTokenSpaceGuid.PcdSataSssSupport|TRUE|BOOLEAN|0x00020006
diff --git a/Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c b/Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c
index 1958d91..78c6819 100644
--- a/Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c
+++ b/Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c
@@ -110,8 +110,8 @@ InitializeSataController (
   SataChPerSerdes = FixedPcdGet8 (PcdSataNumChPerSerdes);
 
   for (PortNum = 0; PortNum < SataPortCount; PortNum += SataChPerSerdes) {
-    EvenPort = (UINT32)(FixedPcdGet16 (PcdSataPortMode) >> (PortNum * 2)) & 3;
-    OddPort = (UINT32)(FixedPcdGet16 (PcdSataPortMode) >> ((PortNum+1) * 2)) & 3;
+    EvenPort = (UINT32)(FixedPcdGet32 (PcdSataPortMode) >> (PortNum * 2)) & 3;
+    OddPort = (UINT32)(FixedPcdGet32 (PcdSataPortMode) >> ((PortNum+1) * 2)) & 3;
     SataPhyInit ((StartPort + PortNum) / SataChPerSerdes, EvenPort, OddPort);
   }
 
-- 
2.9.3

[PATCH v2 edk2-platforms 2/3] Silicon/AMD/Styx: Use PcdSataPortMode properly for two controllers

[Alan Ott]

The previous implementation used only the lower bits for both the first
and second SATA controller, when the upper bits should have been used for
the second SATA controller.

Also ASSERT that SataChPerSerdes is 2, because the even/odd logic doesn't
work if it's not.

Signed-off-by: Alan Ott <alan@softiron.com>
Contributed-under: TianoCore Contribution Agreement 1.0
---
 Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c b/Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c
index 78c6819..ea49cae 100644
--- a/Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c
+++ b/Silicon/AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c
@@ -109,10 +109,15 @@ InitializeSataController (
 
   SataChPerSerdes = FixedPcdGet8 (PcdSataNumChPerSerdes);
 
-  for (PortNum = 0; PortNum < SataPortCount; PortNum += SataChPerSerdes) {
+  //
+  // SataChPerSerdes must be 2 for the Even/Odd logic in the loop below
+  //
+  ASSERT(SataChPerSerdes == 2);
+
+  for (PortNum = StartPort; PortNum < SataPortCount + StartPort; PortNum += SataChPerSerdes) {
     EvenPort = (UINT32)(FixedPcdGet32 (PcdSataPortMode) >> (PortNum * 2)) & 3;
     OddPort = (UINT32)(FixedPcdGet32 (PcdSataPortMode) >> ((PortNum+1) * 2)) & 3;
-    SataPhyInit ((StartPort + PortNum) / SataChPerSerdes, EvenPort, OddPort);
+    SataPhyInit (PortNum / SataChPerSerdes, EvenPort, OddPort);
   }
 
   //
-- 
2.9.3

[PATCH v2 edk2-platforms 3/3] Platform/AMD/OverdriveBoard: Re-enable the second SATA Controller

[Alan Ott]

The comment indicating that only the first SATA controller is operational
on SoftIron-branded OverDrive 3000 boards is incorrect. Re-enable the
second SATA controller.

Signed-off-by: Alan Ott <alan@softiron.com>
Contributed-under: TianoCore Contribution Agreement 1.0
---
 Platform/AMD/OverdriveBoard/OverdriveBoard.dsc | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
index f256ffb..2881de3 100644
--- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
+++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
@@ -409,14 +409,10 @@ DEFINE DO_FLASHER   = FALSE
   gArmTokenSpaceGuid.PcdGicDistributorBase|0xE1110000
   gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0xE112F000
 
-  #
-  # AMD's B1 based Overdrive has 14 SATA ports across 2 controllers. However,
-  # it appears that Softiron's Overdrive 3000, which is also B1 based, does
-  # not have the second SATA controller enabled, and any attempts to use it
-  # will crash the firmware. So use the first controller only.
-  #
+  # SATA Ports
   gAmdStyxTokenSpaceGuid.PcdSata0PortCount|8
-  gAmdStyxTokenSpaceGuid.PcdSataPortMode|0xffff
+  gAmdStyxTokenSpaceGuid.PcdSata1PortCount|6
+  gAmdStyxTokenSpaceGuid.PcdSataPortMode|0x0fffffff
 
   # PCIe Support
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xF0000000
-- 
2.9.3

Re: [PATCH v2 edk2-platforms 0/3] Re-enable the second SATA controller on Overdrive

[Ard Biesheuvel]

On 20 August 2017 at 20:33, Alan Ott <alan@softiron.com> wrote:
> Without the PCD for the second SATA Controller being specified, the boot
> will hang. These patches fix it.
>
> Alan Ott (3):
>   Silicon/AMD/Styx: Make PcdSataPortMode 32 bits
>   Silicon/AMD/Styx: Use PcdSataPortMode properly for two controllers
>   Platform/AMD/OverdriveBoard: Re-enable the second SATA Controller
>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Re: [PATCH v2 edk2-platforms 0/3] Re-enable the second SATA controller on Overdrive

[Leif Lindholm]

On Sun, Aug 20, 2017 at 03:33:32PM -0400, Alan Ott wrote:
> Without the PCD for the second SATA Controller being specified, the boot
> will hang. These patches fix it.

Given Ard's RB, I'm happy for this to go in.
However, would you be happy to contribute these under TianoCore
Contribution Agreement 1.1 rather than 1.0?

If so, please confirm here and I can fold in the change before
pushing.

/
    Leif

> Alan Ott (3):
>   Silicon/AMD/Styx: Make PcdSataPortMode 32 bits
>   Silicon/AMD/Styx: Use PcdSataPortMode properly for two controllers
>   Platform/AMD/OverdriveBoard: Re-enable the second SATA Controller
> 
>  Platform/AMD/OverdriveBoard/OverdriveBoard.dsc              | 10 +++-------
>  Silicon/AMD/Styx/AmdStyx.dec                                |  2 +-
>  .../AMD/Styx/Drivers/StyxSataPlatformDxe/InitController.c   | 13 +++++++++----
>  3 files changed, 13 insertions(+), 12 deletions(-)
> 
> -- 
> 2.9.3
> 

Re: [PATCH v2 edk2-platforms 0/3] Re-enable the second SATA controller on Overdrive

[Alan Ott]

On 08/21/2017 10:12 AM, Leif Lindholm wrote:
> On Sun, Aug 20, 2017 at 03:33:32PM -0400, Alan Ott wrote:
>> Without the PCD for the second SATA Controller being specified, the boot
>> will hang. These patches fix it.
>
> Given Ard's RB, I'm happy for this to go in.
> However, would you be happy to contribute these under TianoCore
> Contribution Agreement 1.1 rather than 1.0?
>
> If so, please confirm here and I can fold in the change before
> pushing.

Contributed-under: TianoCore Contribution Agreement 1.1

Thanks Leif!

Re: [PATCH v2 edk2-platforms 0/3] Re-enable the second SATA controller on Overdrive

[Leif Lindholm]

On Mon, Aug 21, 2017 at 10:20:11AM -0400, Alan Ott wrote:
> On 08/21/2017 10:12 AM, Leif Lindholm wrote:
> > On Sun, Aug 20, 2017 at 03:33:32PM -0400, Alan Ott wrote:
> > > Without the PCD for the second SATA Controller being specified, the boot
> > > will hang. These patches fix it.
> > 
> > Given Ard's RB, I'm happy for this to go in.
> > However, would you be happy to contribute these under TianoCore
> > Contribution Agreement 1.1 rather than 1.0?
> > 
> > If so, please confirm here and I can fold in the change before
> > pushing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> 
> Thanks Leif!

Thanks, Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Series pushed as 7d9c49468..b263c30e5.

/
    Leif