Re: [RFC] SATA : Implemented NXP errata A008402

["Zeng, Star" <star.zeng@intel.com>]

How will the code work based on your patch if the this PCD is configured to other value, for example 0x200000/0x300000?


Thanks,
Star
-----Original Message-----
From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com] 
Sent: Monday, January 8, 2018 5:54 PM
To: Zeng, Star <star.zeng@intel.com>; ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402


> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Monday, January 08, 2018 3:18 PM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; 
> ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2- 
> devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> So this PCD needs to be defined as 0x3FFFFF or may be 0x400000, then 
> it needs to be configured to 0 for your case, right?
> Could the PCD be configured to other values?
No, in my case i need to set it to 0 only. No other value is needed.
> 
> According to the statement you provided, is it possible to handle the 
> case in the device firmware?
> 
> " Due to a logic error, 3F_FFFFh is misinterpreted by the device as 
> zero length."
> 
No, it can't be handle in device firmware for existing SATA controller.
There are chances that in future releases of SATA controller it will get fixed in RTL, but this change will still be needed for LS2088 board. 


> 
> Thanks,
> Star
> -----Original Message-----
> From: Meenakshi Aggarwal [mailto:meenakshi.aggarwal@nxp.com]
> Sent: Monday, January 8, 2018 2:26 PM
> To: Zeng, Star <star.zeng@intel.com>; ard.biesheuvel@linaro.org; 
> leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric 
> <eric.dong@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> Hi Star,
> 
> Apologies and some correction in my last reply.
> 
> As per the errata, PRDT Maximum value needs to be set to 0 only  when 
> creating a PRD entry for a maximum data transfer size.
> 
> So there is no need to replace all occurrences of 
> EFI_AHCI_MAX_DATA_PER_PRDT in file.
> Just need to change it where we are setting the Data length.
> 
> I define it to 0x3FFFFF, as this is the actual value we are setting 
> and this PCD need to be used only once.
> 
> I know, its NXP specific patch only, and i try to made changes which 
> will not impact any other package.
> 
> 
> Thanks,
> Meenakshi
> 
> > -----Original Message-----
> > From: Meenakshi Aggarwal
> > Sent: Monday, January 08, 2018 11:25 AM
> > To: 'Zeng, Star' <star.zeng@intel.com>; ard.biesheuvel@linaro.org; 
> > leif.lindholm@linaro.org; edk2-devel@lists.01.org; Dong, Eric 
> > <eric.dong@intel.com>
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Hi,
> >
> > I didn't prepare the full patch but will send in next few minutes,
> >
> > i  made the very basic changes required to test Errata implementation.
> >
> > I will redefine  it to 0x400000.
> > PcdPrdtMaxDataLength was defined to 0x3FFFFF just for testing purpose.
> >
> > Thanks,
> > Meenakshi
> >
> > > -----Original Message-----
> > > From: Zeng, Star [mailto:star.zeng@intel.com]
> > > Sent: Monday, January 08, 2018 11:19 AM
> > > To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; 
> > > ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2- 
> > > devel@lists.01.org; Dong, Eric <eric.dong@intel.com>
> > > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star 
> > > <star.zeng@intel.com>
> > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Do you have a full patch already?
> > > Why the PcdPrdtMaxDataLength is defined to 0x3FFFFF, but not
> 0x400000?
> > >
> > >
> > > Thanks,
> > > Star
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On 
> > > Behalf Of Meenakshi Aggarwal
> > > Sent: Monday, January 8, 2018 7:17 PM
> > > To: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; edk2- 
> > > devel@lists.01.org; Zeng, Star <star.zeng@intel.com>; Dong, Eric 
> > > <eric.dong@intel.com>
> > > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Description:
> > > Commands with 4 MB PRD length entries fail if PRD[DBC] is set to 
> > > the value according to AHCI standard spec.
> > > Due to a logic error, 3F_FFFFh is misinterpreted by the device as 
> > > zero length.
> > >
> > > Workaround:
> > > Set PRD length to 0 when creating a PRD entry for a maximum data 
> > > transfer size of 4 MB to fix the erratum.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> > > ---
> > >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c           | 2 +-
> > >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf | 1 +
> > >  MdeModulePkg/MdeModulePkg.dec                              | 3 +++
> > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > index e6de5d6..fb6dc0b 100644
> > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > @@ -591,7 +591,7 @@ AhciBuildCommand (
> > >      if (RemainedData < EFI_AHCI_MAX_DATA_PER_PRDT) {
> > >        AhciRegisters->AhciCommandTable- 
> > >PrdtTable[PrdtIndex].AhciPrdtDbc  = (UINT32)RemainedData - 1;
> > >      } else {
> > > -      AhciRegisters->AhciCommandTable-
> > >PrdtTable[PrdtIndex].AhciPrdtDbc
> > > = EFI_AHCI_MAX_DATA_PER_PRDT - 1;
> > > +      AhciRegisters->AhciCommandTable-
> > >PrdtTable[PrdtIndex].AhciPrdtDbc
> > > = PcdGet32 (PcdPrdtMaxDataLength);
> > >      }
> > >
> > >      Data64.Uint64 = (UINT64)MemAddr; diff --git
> > a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> > > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> > > index 82d5f7a..8921dd5 100644
> > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> > > @@ -70,6 +70,7 @@
> > >
> > >  [Pcd]
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdAtaSmartEnable   ##
> > > SOMETIMES_CONSUMES
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength
> > >
> > >  # [Event]
> > >  # EVENT_TYPE_PERIODIC_TIMER ## SOMETIMES_CONSUMES diff --git 
> > > a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> > > index 8efad57..b2f9f2b 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dec
> > > +++ b/MdeModulePkg/MdeModulePkg.dec
> > > @@ -1434,6 +1434,9 @@
> > >    # @Prompt Console Output Row of Text Setup
> > >
> > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdSetupConOutRow|25|UINT32|0x40
> > > 00000e
> > >
> > > +  ## This PCD specifies the Maximum data length for a PRD Entry
> > > +
> > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3FFFFF|UIN
> > > T32|0x4000000f
> > > +
> > >  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic,
> > PcdsDynamicEx]
> > >    ## UART clock frequency is for the baud rate configuration.
> > >    # @Prompt Serial Port Clock Rate.
> > > --
> > > 1.9.1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > >
> >
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > t
> > > s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> > >
> >
> devel&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9b1e2bde5b
> > >
> >
> 4b47746d0e08d5565b8394%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> > >
> >
> 0%7C636509873433437533&sdata=wg3Sg5SiU2MrbZSkZMlboldcXnMBKKg3jG
> > > yA45YZo1Q%3D&reserved=0