On 20 August 2017 at 19:09, Ard Biesheuvel <[email protected]> wrote: > On 20 August 2017 at 19:08, Alan Ott <[email protected]> wrote: >> On 08/20/2017 01:46 PM, Ard Biesheuvel wrote: >>> >>> On 19 August 2017 at 22:41, Alan Ott <[email protected]> wrote: >>>> >>>> Extra bits are needed to accomodate all 14 SATA ports >>>> >>>> Signed-off-by: Alan Ott <[email protected]> >>>> 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); >>>> } >>>> >>> >>> This doesn't look right to me. PortNum will always be in the set { 0, >>> 2, 4, 6 } due to SataPortCount being equal to either the port count of >>> sata 0 or of sata 1. So the top 16 bits of the new PCD are never >>> referenced, and the port mode for sata 0 ends up being applied to sata >>> 1. >>> >>> Better use >>> >>> for (PortNum = StartPort; PortNum < StartPort + SataPortCount; ...) >>> ..etc.. >>> >> >> I agree. This has apparently been broken all along. I'll fix and resubmit. >> >>> >>> Also, this code relies rather heavily on SataChPerSerdes == 2, so we >>> might want to add an ASSERT () for that. >>> >> >> I can do that too. >>
Thanks _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
