Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-11 Thread Ard Biesheuvel
On 11 January 2018 at 02:25, Ni, Ruiyu  wrote:
> On 1/10/2018 5:52 PM, Ard Biesheuvel wrote:
>>
>> On 10 January 2018 at 09:43, Udit Kumar  wrote:
>>>
>>> Hi Ruiyu,
>>>
 -Original Message-
>
>
> And this change will not impact any other hardware so no one is
> basically

 impacted by this change.

 Your buggy HW only need the value zero. But the addition of PCD exposes
 an interface that can use any size of PRD.
 I am not sure the AtaAtapiPassThru can work if some platform sets the
 PCD
 value to others than 0 or 3F_h.
>>>
>>>
>>> I don't see someone using this driver will set Pcd randomly, but I agree
>>> on this
>>> point other value should be handled.
>>> Error or Assert could be added, if Pcd value is not 0 or 3F_h.
>>>
 Can you please
just duplicate the AtaAtapiPassThru in your platform?
 Because the driver is very stable today, not much code sync effort will
 be
 needed if core version is changed.
>>>
>>>
>>> Duplicating is always a possibility :), but When we will push this
>>> duplicated driver
>>> (just for one line change) for upstreaming.
>>> will this be acceptable ??
>>
>>
>> My main concern with this (and with using a PCD) is that the setting
>> affects all SATA controllers in the system, including ones the you
>> stick into a PCIe slot.
>>
>> So I think forking the driver is the only possible solution, but it
>> will not be a one-line change: you need to ensure that you apply the
>> workaround only to the SATA controllers in the SoC.
>>
>
> Depending on the new driver's location. The package owner decides
> whether forking is acceptable :)
>

I think forking is reasonable in this case: the workaround does not
belong in generic code, and it seems only early silicon is affected
anyway.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-10 Thread Ni, Ruiyu

On 1/10/2018 5:52 PM, Ard Biesheuvel wrote:

On 10 January 2018 at 09:43, Udit Kumar  wrote:

Hi Ruiyu,


-Original Message-


And this change will not impact any other hardware so no one is basically

impacted by this change.

Your buggy HW only need the value zero. But the addition of PCD exposes
an interface that can use any size of PRD.
I am not sure the AtaAtapiPassThru can work if some platform sets the PCD
value to others than 0 or 3F_h.


I don't see someone using this driver will set Pcd randomly, but I agree on this
point other value should be handled.
Error or Assert could be added, if Pcd value is not 0 or 3F_h.


Can you please
   just duplicate the AtaAtapiPassThru in your platform?
Because the driver is very stable today, not much code sync effort will be
needed if core version is changed.


Duplicating is always a possibility :), but When we will push this duplicated 
driver
(just for one line change) for upstreaming.
will this be acceptable ??


My main concern with this (and with using a PCD) is that the setting
affects all SATA controllers in the system, including ones the you
stick into a PCIe slot.

So I think forking the driver is the only possible solution, but it
will not be a one-line change: you need to ensure that you apply the
workaround only to the SATA controllers in the SoC.



Depending on the new driver's location. The package owner decides
whether forking is acceptable :)


--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-10 Thread Udit Kumar


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Wednesday, January 10, 2018 3:23 PM
> To: Udit Kumar <udit.ku...@nxp.com>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Meenakshi Aggarwal
> <meenakshi.aggar...@nxp.com>; edk2-devel@lists.01.org; Zeng, Star
> <star.z...@intel.com>; leif.lindh...@linaro.org
> Subject: Re: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> On 10 January 2018 at 09:43, Udit Kumar <udit.ku...@nxp.com> wrote:
> > Hi Ruiyu,
> >
> >> -Original Message-
> >> >
> >> > And this change will not impact any other hardware so no one is
> >> > basically
> >> impacted by this change.
> >>
> >> Your buggy HW only need the value zero. But the addition of PCD
> >> exposes an interface that can use any size of PRD.
> >> I am not sure the AtaAtapiPassThru can work if some platform sets the
> >> PCD value to others than 0 or 3F_h.
> >
> > I don't see someone using this driver will set Pcd randomly, but I
> > agree on this point other value should be handled.
> > Error or Assert could be added, if Pcd value is not 0 or 3F_h.
> >
> >> Can you please
> >>   just duplicate the AtaAtapiPassThru in your platform?
> >> Because the driver is very stable today, not much code sync effort
> >> will be needed if core version is changed.
> >
> > Duplicating is always a possibility :), but When we will push this
> > duplicated driver (just for one line change) for upstreaming.
> > will this be acceptable ??
> 
> My main concern with this (and with using a PCD) is that the setting affects
> all SATA controllers in the system, including ones the you stick into a PCIe
> slot.

Agreed,  If two types of Sata controllers are used then this patch 
may break one controller. 
At present, we don’t have anyone asking SATA controller behind PCIe
but we shouldn’t ignore this. This is valid use case. 

> So I think forking the driver is the only possible solution, but it will not 
> be a
> one-line change: you need to ensure that you apply the workaround only to
> the SATA controllers in the SoC.

Yes, Considering unrelated SATA controllers this will be more than copy 

> --
> Ard.

Thanks 
Udit
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-10 Thread Ard Biesheuvel
On 10 January 2018 at 09:43, Udit Kumar  wrote:
> Hi Ruiyu,
>
>> -Original Message-
>> >
>> > And this change will not impact any other hardware so no one is basically
>> impacted by this change.
>>
>> Your buggy HW only need the value zero. But the addition of PCD exposes
>> an interface that can use any size of PRD.
>> I am not sure the AtaAtapiPassThru can work if some platform sets the PCD
>> value to others than 0 or 3F_h.
>
> I don't see someone using this driver will set Pcd randomly, but I agree on 
> this
> point other value should be handled.
> Error or Assert could be added, if Pcd value is not 0 or 3F_h.
>
>> Can you please
>>   just duplicate the AtaAtapiPassThru in your platform?
>> Because the driver is very stable today, not much code sync effort will be
>> needed if core version is changed.
>
> Duplicating is always a possibility :), but When we will push this duplicated 
> driver
> (just for one line change) for upstreaming.
> will this be acceptable ??

My main concern with this (and with using a PCD) is that the setting
affects all SATA controllers in the system, including ones the you
stick into a PCIe slot.

So I think forking the driver is the only possible solution, but it
will not be a one-line change: you need to ensure that you apply the
workaround only to the SATA controllers in the SoC.

-- 
Ard.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-10 Thread Udit Kumar
Hi Ruiyu, 

> -Original Message-
> >
> > And this change will not impact any other hardware so no one is basically
> impacted by this change.
> 
> Your buggy HW only need the value zero. But the addition of PCD exposes
> an interface that can use any size of PRD.
> I am not sure the AtaAtapiPassThru can work if some platform sets the PCD
> value to others than 0 or 3F_h.

I don't see someone using this driver will set Pcd randomly, but I agree on this
point other value should be handled. 
Error or Assert could be added, if Pcd value is not 0 or 3F_h.

> Can you please
>   just duplicate the AtaAtapiPassThru in your platform?
> Because the driver is very stable today, not much code sync effort will be
> needed if core version is changed.

Duplicating is always a possibility :), but When we will push this duplicated 
driver
(just for one line change) for upstreaming.
will this be acceptable ??

Thanks

 
> >
> >>>
> >>> 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 
> >>> ---
> >>>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
> >> 0e
> >>>
> >>> +  ## This PCD specifies the Maximum data length for a PRD Entry
> >>> +
> >>
> gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3F|UIN
> >> T32|0x400f
> >>> +
> >>>[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic,
> >> PcdsDynamicEx]
> >>>  ## UART clock frequency is for the baud rate configuration.
> >>>  # @Prompt Serial Port Clock Rate.
> >>>
> >>
> >>
> >> --
> >> Thanks,
> >> Ray
> >> ___
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fli
> >> st
> >> s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> >>
> devel=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C60cadf13ac95
> >>
> 48486c4908d55712d85c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %
> >>
> 7C636510660888390946=4oW2agf8z869IMz%2F7%2B2di9vC%2BB3js7K
> >> hJx1LR2b4Tc4%3D=0
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > ts.01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel=02%7C01%7Cudit.kumar%
> >
> 40nxp.com%7C0b697e2364f348745a7c08d5580cf5bf%7C686ea1d3bc2b4c6fa9
> 2cd99
> >
> c5c301635%7C0%7C0%7C636511735060291299=eWI7ueE5wzr2oSNv0t
> CP5hBE0
> > uEfB9S0PyABHWocBaA%3D=0
> >
> 
> 
> --
> Thanks,
> Ray
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> 01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel=02%7C01%7Cudit.kumar%40nxp.com%7C0b697e2364f348745a7c
> 

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-10 Thread Ni, Ruiyu

On 1/10/2018 4:56 PM, Meenakshi Aggarwal wrote:



-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni,
Ruiyu
Sent: Tuesday, January 09, 2018 9:11 AM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

On 1/8/2018 7:16 PM, Meenakshi Aggarwal wrote:

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_h is misinterpreted by the
device as zero length.


Is the logic error mentioned here is the error in HW?
Then I do not prefer to add such PCD for a HW workaround.


Yes, the error is in hardware.
There is no other option visible to implement this errata if we want to use the 
existing AHCI framework.

I completely agree with you, but no other solution seems to exist in this case.

And this change will not impact any other hardware so no one is basically 
impacted by this change.


Your buggy HW only need the value zero. But the addition of PCD
exposes an interface that can use any size of PRD.
I am not sure the AtaAtapiPassThru can work if some platform
sets the PCD value to others than 0 or 3F_h.

Can you please
 just duplicate the AtaAtapiPassThru in your platform?
Because the driver is very stable today, not much code sync effort
will be needed if core version is changed.





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.aggar...@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
0e


+  ## This PCD specifies the Maximum data length for a PRD Entry
+

gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3F|UIN
T32|0x400f

+
   [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic,

PcdsDynamicEx]

 ## UART clock frequency is for the baud rate configuration.
 # @Prompt Serial Port Clock Rate.




--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
s.01.org%2Fmailman%2Flistinfo%2Fedk2-
devel=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C60cadf13ac95
48486c4908d55712d85c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
7C636510660888390946=4oW2agf8z869IMz%2F7%2B2di9vC%2BB3js7K
hJx1LR2b4Tc4%3D=0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel




--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-10 Thread Meenakshi Aggarwal

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni,
> Ruiyu
> Sent: Tuesday, January 09, 2018 9:11 AM
> To: edk2-devel@lists.01.org
> Subject: Re: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> On 1/8/2018 7:16 PM, Meenakshi Aggarwal wrote:
> > 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_h is misinterpreted by the
> > device as zero length.
> 
> Is the logic error mentioned here is the error in HW?
> Then I do not prefer to add such PCD for a HW workaround.
> 
Yes, the error is in hardware.
There is no other option visible to implement this errata if we want to use the 
existing AHCI framework.

I completely agree with you, but no other solution seems to exist in this case.

And this change will not impact any other hardware so no one is basically 
impacted by this change.

> >
> > 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.aggar...@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
> 0e
> >
> > +  ## This PCD specifies the Maximum data length for a PRD Entry
> > +
> gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3F|UIN
> T32|0x400f
> > +
> >   [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic,
> PcdsDynamicEx]
> > ## UART clock frequency is for the baud rate configuration.
> > # @Prompt Serial Port Clock Rate.
> >
> 
> 
> --
> Thanks,
> Ray
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C60cadf13ac95
> 48486c4908d55712d85c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C636510660888390946=4oW2agf8z869IMz%2F7%2B2di9vC%2BB3js7K
> hJx1LR2b4Tc4%3D=0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-08 Thread Ni, Ruiyu

On 1/8/2018 7:16 PM, Meenakshi Aggarwal wrote:

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_h is misinterpreted by the
device as zero length.


Is the logic error mentioned here is the error in HW?
Then I do not prefer to add such PCD for a HW workaround.



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 
---
  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|0x400e
  
+  ## This PCD specifies the Maximum data length for a PRD Entry

+  
gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3F|UINT32|0x400f
+
  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## UART clock frequency is for the baud rate configuration.
# @Prompt Serial Port Clock Rate.




--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-08 Thread Meenakshi Aggarwal
Hi,

We will set PCD value to 0 to support our board and its default value is set to 
maximum data length.

If someone change its value to any other value in their package, then he/she 
must be aware of the consequences.

I cannot say how controller will react if value is other than 0x3f_ and 0 
(in our case).

Thanks,
Meenakshi

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 3:54 PM
> To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>;
> ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> de...@lists.01.org; Dong, Eric <eric.d...@intel.com>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Zeng, Star <star.z...@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> How will the code work based on your patch if the this PCD is configured to
> other value, for example 0x20/0x30?
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> Sent: Monday, January 8, 2018 5:54 PM
> To: Zeng, Star <star.z...@intel.com>; ard.biesheu...@linaro.org;
> leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> <eric.d...@intel.com>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> 
> > -Original Message-
> > From: Zeng, Star [mailto:star.z...@intel.com]
> > Sent: Monday, January 08, 2018 3:18 PM
> > To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>;
> > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> > de...@lists.01.org; Dong, Eric <eric.d...@intel.com>
> > Cc: Ni, Ruiyu <ruiyu...@intel.com>; Zeng, Star <star.z...@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > So this PCD needs to be defined as 0x3F or may be 0x40, 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_h 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.aggar...@nxp.com]
> > Sent: Monday, January 8, 2018 2:26 PM
> > To: Zeng, Star <star.z...@intel.com>; ard.biesheu...@linaro.org;
> > leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > <eric.d...@intel.com>
> > Cc: Ni, Ruiyu <ruiyu...@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 0x3F, 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.z...@intel.com>; ard.biesheu...@linaro.org;
> > > leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > > <eric.d...@intel.com>
> > > Cc: Ni, Ruiyu <ruiyu...@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 0x40.
> > > PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.
> > >
> > > Thanks,
> > > Meenakshi
> > >
> > > > -Original M

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-08 Thread Zeng, Star
How will the code work based on your patch if the this PCD is configured to 
other value, for example 0x20/0x30?


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


> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 3:18 PM
> To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; 
> ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> de...@lists.01.org; Dong, Eric <eric.d...@intel.com>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Zeng, Star <star.z...@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> So this PCD needs to be defined as 0x3F or may be 0x40, 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_h 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.aggar...@nxp.com]
> Sent: Monday, January 8, 2018 2:26 PM
> To: Zeng, Star <star.z...@intel.com>; ard.biesheu...@linaro.org; 
> leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric 
> <eric.d...@intel.com>
> Cc: Ni, Ruiyu <ruiyu...@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 0x3F, 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.z...@intel.com>; ard.biesheu...@linaro.org; 
> > leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric 
> > <eric.d...@intel.com>
> > Cc: Ni, Ruiyu <ruiyu...@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 0x40.
> > PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.
> >
> > Thanks,
> > Meenakshi
> >
> > > -Original Message-
> > > From: Zeng, Star [mailto:star.z...@intel.com]
> > > Sent: Monday, January 08, 2018 11:19 AM
> > > To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; 
> > > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> > > de...@lists.01.org; Dong, Eric <eric.d...@intel.com>
> > > Cc: Ni, Ruiyu <ruiyu...@intel.com>; Zeng, Star 
> > > <star.z...@intel.com>
> > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Do you have a full patch already?
> > > Why the PcdPrdtMaxDataLength is defined to 0x3F, but not
> 0x40?
> > >
> > >
> > > Thanks,
> > > Star
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On 
> > > Behalf Of Meenakshi Aggarwal
> > > Sent: Monday, January 8, 2018 7:17 PM
> > > To: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> > > de...@lists.01.org; Zeng, Star <star.z...@intel.com>; Dong, Eric 
> > > <eric.d...@intel.com>
> > > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-08 Thread Meenakshi Aggarwal

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 3:18 PM
> To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>;
> ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> de...@lists.01.org; Dong, Eric <eric.d...@intel.com>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Zeng, Star <star.z...@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> So this PCD needs to be defined as 0x3F or may be 0x40, 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_h 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.aggar...@nxp.com]
> Sent: Monday, January 8, 2018 2:26 PM
> To: Zeng, Star <star.z...@intel.com>; ard.biesheu...@linaro.org;
> leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> <eric.d...@intel.com>
> Cc: Ni, Ruiyu <ruiyu...@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 0x3F, 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.z...@intel.com>; ard.biesheu...@linaro.org;
> > leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > <eric.d...@intel.com>
> > Cc: Ni, Ruiyu <ruiyu...@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 0x40.
> > PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.
> >
> > Thanks,
> > Meenakshi
> >
> > > -Original Message-
> > > From: Zeng, Star [mailto:star.z...@intel.com]
> > > Sent: Monday, January 08, 2018 11:19 AM
> > > To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>;
> > > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> > > de...@lists.01.org; Dong, Eric <eric.d...@intel.com>
> > > Cc: Ni, Ruiyu <ruiyu...@intel.com>; Zeng, Star <star.z...@intel.com>
> > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Do you have a full patch already?
> > > Why the PcdPrdtMaxDataLength is defined to 0x3F, but not
> 0x40?
> > >
> > >
> > > Thanks,
> > > Star
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > > Of Meenakshi Aggarwal
> > > Sent: Monday, January 8, 2018 7:17 PM
> > > To: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> > > de...@lists.01.org; Zeng, Star <star.z...@intel.com>; Dong, Eric
> > > <eric.d...@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_h 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
> 

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-08 Thread Zeng, Star
So this PCD needs to be defined as 0x3F or may be 0x40, then it needs 
to be configured to 0 for your case, right?
Could the PCD be configured to other values?

According to the statement you provided, is it possible to handle the case in 
the device firmware?

" Due to a logic error, 3F_h is misinterpreted by the device as zero 
length."


Thanks,
Star
-Original Message-
From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com] 
Sent: Monday, January 8, 2018 2:26 PM
To: Zeng, Star <star.z...@intel.com>; ard.biesheu...@linaro.org; 
leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric 
<eric.d...@intel.com>
Cc: Ni, Ruiyu <ruiyu...@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 0x3F, 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.z...@intel.com>; ard.biesheu...@linaro.org; 
> leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric 
> <eric.d...@intel.com>
> Cc: Ni, Ruiyu <ruiyu...@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 0x40.
> PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.
> 
> Thanks,
> Meenakshi
> 
> > -Original Message-
> > From: Zeng, Star [mailto:star.z...@intel.com]
> > Sent: Monday, January 08, 2018 11:19 AM
> > To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; 
> > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> > de...@lists.01.org; Dong, Eric <eric.d...@intel.com>
> > Cc: Ni, Ruiyu <ruiyu...@intel.com>; Zeng, Star <star.z...@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Do you have a full patch already?
> > Why the PcdPrdtMaxDataLength is defined to 0x3F, but not 0x40?
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Meenakshi Aggarwal
> > Sent: Monday, January 8, 2018 7:17 PM
> > To: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> > de...@lists.01.org; Zeng, Star <star.z...@intel.com>; Dong, Eric 
> > <eric.d...@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_h 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.aggar...@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 (

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-07 Thread Meenakshi Aggarwal
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 0x3F, 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.z...@intel.com>; ard.biesheu...@linaro.org;
> leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> <eric.d...@intel.com>
> Cc: Ni, Ruiyu <ruiyu...@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 0x40.
> PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.
> 
> Thanks,
> Meenakshi
> 
> > -Original Message-
> > From: Zeng, Star [mailto:star.z...@intel.com]
> > Sent: Monday, January 08, 2018 11:19 AM
> > To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>;
> > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> > de...@lists.01.org; Dong, Eric <eric.d...@intel.com>
> > Cc: Ni, Ruiyu <ruiyu...@intel.com>; Zeng, Star <star.z...@intel.com>
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Do you have a full patch already?
> > Why the PcdPrdtMaxDataLength is defined to 0x3F, but not 0x40?
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Meenakshi Aggarwal
> > Sent: Monday, January 8, 2018 7:17 PM
> > To: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> > de...@lists.01.org; Zeng, Star <star.z...@intel.com>; Dong, Eric
> > <eric.d...@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_h 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.aggar...@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/MdeModuleP

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-07 Thread Meenakshi Aggarwal
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 0x40.
PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.

Thanks,
Meenakshi

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 11:19 AM
> To: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>;
> ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> de...@lists.01.org; Dong, Eric <eric.d...@intel.com>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Zeng, Star <star.z...@intel.com>
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> Do you have a full patch already?
> Why the PcdPrdtMaxDataLength is defined to 0x3F, but not 0x40?
> 
> 
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Meenakshi Aggarwal
> Sent: Monday, January 8, 2018 7:17 PM
> To: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> de...@lists.01.org; Zeng, Star <star.z...@intel.com>; Dong, Eric
> <eric.d...@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_h 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.aggar...@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
> 0e
> 
> +  ## This PCD specifies the Maximum data length for a PRD Entry
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3F|UIN
> T32|0x400f
> +
>  [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%2Flist
> s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C9b1e2bde5b
> 4b47746d0e08d5565b8394%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> 0%7C636509873433437533=wg3Sg5SiU2MrbZSkZMlboldcXnMBKKg3jG
> yA45YZo1Q%3D=0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-07 Thread Zeng, Star
Do you have a full patch already?
Why the PcdPrdtMaxDataLength is defined to 0x3F, but not 0x40?


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Meenakshi Aggarwal
Sent: Monday, January 8, 2018 7:17 PM
To: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; 
edk2-devel@lists.01.org; Zeng, Star <star.z...@intel.com>; Dong, Eric 
<eric.d...@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_h 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.aggar...@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|0x400e
 
+  ## This PCD specifies the Maximum data length for a PRD Entry
+  
gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3F|UINT32|0x400f
+
 [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://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [RFC] SATA : Implemented NXP errata A008402

2018-01-07 Thread Meenakshi Aggarwal
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_h 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 
---
 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|0x400e
 
+  ## This PCD specifies the Maximum data length for a PRD Entry
+  
gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3F|UINT32|0x400f
+
 [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://lists.01.org/mailman/listinfo/edk2-devel