[Linaro-uefi,6/8] Platforms/AMD/Styx/AcpiTables: enable second SATA controller

Message ID CAKv+Gu-LO8eGVFoa2N7DqtDu_a-_EgoV0W+59UyfS_fBNxVeqw@mail.gmail.com
State New
Headers show

Commit Message

Ard Biesheuvel Nov. 1, 2016, 7:27 p.m.
On 1 November 2016 at 17:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Enable the second AHCI ACPI node when any ports are enabled on the
> second SATA controller.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

With the following hunk folded in, both SATA controllers are detected
correctly under Linux using ACPI as well as DT.

             })
         }



> ---
>  Platforms/AMD/Styx/AcpiTables/AcpiTables.inf | 1 +
>  Platforms/AMD/Styx/AcpiTables/Dsdt.c         | 9 ++-------
>  2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/Platforms/AMD/Styx/AcpiTables/AcpiTables.inf b/Platforms/AMD/Styx/AcpiTables/AcpiTables.inf
> index 72272aa0b85a..12e0444009ef 100644
> --- a/Platforms/AMD/Styx/AcpiTables/AcpiTables.inf
> +++ b/Platforms/AMD/Styx/AcpiTables/AcpiTables.inf
> @@ -83,6 +83,7 @@
>    gAmdStyxTokenSpaceGuid.PcdPsciOsSupport
>    gAmdStyxTokenSpaceGuid.PcdTrustedFWSupport
>    gAmdStyxTokenSpaceGuid.PcdParkingProtocolVersion
> +  gAmdStyxTokenSpaceGuid.PcdSata1PortCount
>
>  [Depex]
>    gAmdMpCoreInfoProtocolGuid
> diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.c b/Platforms/AMD/Styx/AcpiTables/Dsdt.c
> index 922d7214adf4..360a446f7631 100644
> --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.c
> +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.c
> @@ -174,15 +174,10 @@ DsdtHeader (
>      else if (AsciiStrCmp(Table->Pathname, "_SB_.ETH1._DSD") == 0) {
>        OverrideMacAddr ((UINT8 *)&AmlCode[Table->Offset], PcdGet64 (PcdEthMacB));
>      }
> -#if DO_SATA1
>      else if (AsciiStrCmp(Table->Pathname, "_SB_.AHC1._STA") == 0) {
> -      OverrideStatus ((UINT8 *)&AmlCode[Table->Offset], EnableOnB1);
> -    }
> -#else
> -    else if (AsciiStrCmp(Table->Pathname, "_SB_.AHC1._STA") == 0) {
> -      OverrideStatus ((UINT8 *)&AmlCode[Table->Offset], FALSE);
> +      OverrideStatus ((UINT8 *)&AmlCode[Table->Offset],
> +        EnableOnB1 && FixedPcdGet8(PcdSata1PortCount) > 0);
>      }
> -#endif
>      else if (AsciiStrCmp(Table->Pathname, "_SB_.GIO2._STA") == 0) {
>        OverrideStatus ((UINT8 *)&AmlCode[Table->Offset], EnableOnB1);
>      }
> --
> 2.7.4
>

Comments

Leif Lindholm Nov. 2, 2016, 4:55 p.m. | #1
On Tue, Nov 01, 2016 at 07:27:14PM +0000, Ard Biesheuvel wrote:
> On 1 November 2016 at 17:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > Enable the second AHCI ACPI node when any ports are enabled on the
> > second SATA controller.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> With the following hunk folded in, both SATA controllers are detected
> correctly under Linux using ACPI as well as DT.
> 
> diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
> b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
> index 4e80e4e59547..7edec3d1ec28 100644
> --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
> +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
> @@ -130,7 +130,7 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "AMDINC",
> "SEATTLE ", 3)
>                      0xE000007C,         // Address Base (SGPIO)
>                      0x00000001,         // Address Length
>                      )
> -                Interrupt (ResourceConsumer, Level, ActiveHigh,
> Exclusive, ,, ) {0x00000189, }
> +                Interrupt (ResourceConsumer, Level, ActiveHigh,
> Exclusive, ,, ) {0x00000182, }
>              })
>          }

Do you want to fold this into 6/8 or submit separately?
It looks like a bugfix on current tables that is independent of the
changes in the Pci/Ahci sets.

/
    Leif

> 
> 
> > ---
> >  Platforms/AMD/Styx/AcpiTables/AcpiTables.inf | 1 +
> >  Platforms/AMD/Styx/AcpiTables/Dsdt.c         | 9 ++-------
> >  2 files changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/Platforms/AMD/Styx/AcpiTables/AcpiTables.inf b/Platforms/AMD/Styx/AcpiTables/AcpiTables.inf
> > index 72272aa0b85a..12e0444009ef 100644
> > --- a/Platforms/AMD/Styx/AcpiTables/AcpiTables.inf
> > +++ b/Platforms/AMD/Styx/AcpiTables/AcpiTables.inf
> > @@ -83,6 +83,7 @@
> >    gAmdStyxTokenSpaceGuid.PcdPsciOsSupport
> >    gAmdStyxTokenSpaceGuid.PcdTrustedFWSupport
> >    gAmdStyxTokenSpaceGuid.PcdParkingProtocolVersion
> > +  gAmdStyxTokenSpaceGuid.PcdSata1PortCount
> >
> >  [Depex]
> >    gAmdMpCoreInfoProtocolGuid
> > diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.c b/Platforms/AMD/Styx/AcpiTables/Dsdt.c
> > index 922d7214adf4..360a446f7631 100644
> > --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.c
> > +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.c
> > @@ -174,15 +174,10 @@ DsdtHeader (
> >      else if (AsciiStrCmp(Table->Pathname, "_SB_.ETH1._DSD") == 0) {
> >        OverrideMacAddr ((UINT8 *)&AmlCode[Table->Offset], PcdGet64 (PcdEthMacB));
> >      }
> > -#if DO_SATA1
> >      else if (AsciiStrCmp(Table->Pathname, "_SB_.AHC1._STA") == 0) {
> > -      OverrideStatus ((UINT8 *)&AmlCode[Table->Offset], EnableOnB1);
> > -    }
> > -#else
> > -    else if (AsciiStrCmp(Table->Pathname, "_SB_.AHC1._STA") == 0) {
> > -      OverrideStatus ((UINT8 *)&AmlCode[Table->Offset], FALSE);
> > +      OverrideStatus ((UINT8 *)&AmlCode[Table->Offset],
> > +        EnableOnB1 && FixedPcdGet8(PcdSata1PortCount) > 0);
> >      }
> > -#endif
> >      else if (AsciiStrCmp(Table->Pathname, "_SB_.GIO2._STA") == 0) {
> >        OverrideStatus ((UINT8 *)&AmlCode[Table->Offset], EnableOnB1);
> >      }
> > --
> > 2.7.4
> >
Ard Biesheuvel Nov. 2, 2016, 4:57 p.m. | #2
On 2 November 2016 at 16:55, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Nov 01, 2016 at 07:27:14PM +0000, Ard Biesheuvel wrote:
>> On 1 November 2016 at 17:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > Enable the second AHCI ACPI node when any ports are enabled on the
>> > second SATA controller.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> With the following hunk folded in, both SATA controllers are detected
>> correctly under Linux using ACPI as well as DT.
>>
>> diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
>> b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
>> index 4e80e4e59547..7edec3d1ec28 100644
>> --- a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
>> +++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
>> @@ -130,7 +130,7 @@ DefinitionBlock ("DSDT.aml", "DSDT", 2, "AMDINC",
>> "SEATTLE ", 3)
>>                      0xE000007C,         // Address Base (SGPIO)
>>                      0x00000001,         // Address Length
>>                      )
>> -                Interrupt (ResourceConsumer, Level, ActiveHigh,
>> Exclusive, ,, ) {0x00000189, }
>> +                Interrupt (ResourceConsumer, Level, ActiveHigh,
>> Exclusive, ,, ) {0x00000182, }
>>              })
>>          }
>
> Do you want to fold this into 6/8 or submit separately?
> It looks like a bugfix on current tables that is independent of the
> changes in the Pci/Ahci sets.
>

It makes sense to keep it separate, although it currently requires the
preprocessor symbol DO_SATA1 to be defined, which we never do, so it
does not matter much in practice.

Patch

diff --git a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
index 4e80e4e59547..7edec3d1ec28 100644
--- a/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
+++ b/Platforms/AMD/Styx/AcpiTables/Dsdt.asl
@@ -130,7 +130,7 @@  DefinitionBlock ("DSDT.aml", "DSDT", 2, "AMDINC",
"SEATTLE ", 3)
                     0xE000007C,         // Address Base (SGPIO)
                     0x00000001,         // Address Length
                     )
-                Interrupt (ResourceConsumer, Level, ActiveHigh,
Exclusive, ,, ) {0x00000189, }
+                Interrupt (ResourceConsumer, Level, ActiveHigh,
Exclusive, ,, ) {0x00000182, }