Re: [PATCH edk2-platforms v2 2/2] LS1046 : Enable support of SATA controller

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 9 January 2018 at 04:37, Meenakshi Aggarwal
<meenakshi.aggarwal@nxp.com> wrote:
>
>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Monday, January 08, 2018 8:42 PM
>> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Udit Kumar
>> <udit.kumar@nxp.com>; Varun Sethi <V.Sethi@nxp.com>
>> Subject: Re: [PATCH edk2-platforms v2 2/2] LS1046 : Enable support of SATA
>> controller
>>
>> On 8 January 2018 at 15:55, Meenakshi Aggarwal
>> <meenakshi.aggarwal@nxp.com> wrote:
>> > Enable support of SATA drives on ls1046 board.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
>> > ---
>> >  Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc                 |  8 ++++++++
>> >  Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf                 | 12
>> ++++++++++++
>> >  .../NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf |  2 ++
>> >  .../NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c    |  8
>> ++++++++
>> >  Silicon/NXP/LS1046A/LS1046A.dsc                              |  5 +++++
>> >  5 files changed, 35 insertions(+)
>> >
>> > diff --git a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> > index 9d2482b..93fc848 100644
>> > --- a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> > +++ b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> > @@ -63,6 +63,13 @@
>> >    #
>> >    gNxpQoriqLsTokenSpaceGuid.PcdI2cSlaveAddress|0x51
>> >
>> > +  #
>> > +  # Errata Pcds
>> > +  #
>> > +  gNxpQoriqLsTokenSpaceGuid.PcdSataErratumA009185|TRUE
>> > +  gNxpQoriqLsTokenSpaceGuid.PcdSataErratumA010554|TRUE
>> > +  gNxpQoriqLsTokenSpaceGuid.PcdSataErratumA010635|TRUE
>> > +
>> >
>> ##########################################################
>> ######################
>> >  #
>> >  # Components Section - list of all EDK II Modules needed by this Platform
>> > @@ -71,3 +78,4 @@
>> >  [Components.common]
>> >    edk2-platforms/Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf
>> >    edk2-platforms/Platform/NXP/Drivers/I2cDxe/I2cDxe.inf
>> > +  edk2-platforms/Platform/NXP/Drivers/SataInitDxe/SataInitDxe.inf
>>
>> This looks wrong to me. Your .dsc/.fdf files should not contain these
>> edk2-platforms prefixes. Instead, you should set your PACKAGES_PATH
>> correctly to include your edk2-platforms directory.
>>
> OK, We will remove this from .dsc/.fdf files.
> My concern is as there are already a lot of patches are under review so it will be
> Better if review gets completed once, then we will share the updated in next revision of patch
> As this needs to be change in multiple patches.
>
> There is one more comment from you on keeping shred Drivers and Library in Silicon/NXP directory.
> In this case also, this will need a rework in all patches sent till date.
>
> So once review comments been recieved we will made the changes in next revision of patch.
>

I have a better idea. Let's disregard all current submissions in
flight, and repost the next one as a single series so that I don't
have to keep track of all the different ones.