Re: [PATCH edk2-platforms 1/3] Silicon/SynQuacerPciHostBridgeLib: add workaround for PCIe MMIO64

[Leif Lindholm <leif.lindholm@linaro.org>]

On Wed, May 30, 2018 at 08:19:27PM +0200, Ard Biesheuvel wrote:
> From: Masahisa KOJIMA <masahisa.kojima@linaro.org>
> 
> The current revision of SC2A11 contains PCIe bus issue.
> In MRd transaction, 1st/Last DW BE fields are not correctly set
> by hardware.
> 
> As a workaround, set TH bit and specify MSG_CODE in iATU.
> With this setup, the value specified as MSG_CODE is set to the
> 1st/Last DW BE fields and PCIe controller can emit the correct
> MRd TLP header.
> Same workaround was already included for MMIO32 region,
> MMIO64 region also requires this workaround.
> Some deivices, such as Samsong SSD 970 EVO, do not work
> without this modification.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Masahisa KOJIMA <masahisa.kojima@linaro.org>

Please add own S-o-b.

> ---
>  Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> index e4679543cc66..227f9a725ce8 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerPciHostBridgeLib/SynQuacerPciHostBridgeLibConstructor.c
> @@ -359,8 +359,9 @@ PciInitControllerPost (
>        RootBridge->MemAbove4G.Base,
>        RootBridge->MemAbove4G.Base,
>        RootBridge->MemAbove4G.Limit - RootBridge->MemAbove4G.Base + 1,
> -      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM,
> -      0);
> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TYPE_MEM |
> +      IATU_REGION_CTRL_1_OFF_OUTBOUND_0_TH,
> +      IATU_REGION_CTRL_2_OFF_OUTBOUND_0_MSG_CODE_32BIT);

Hmm ...
This fix clearly needs to go in. But since this is working around a
bug in first-revision silicon, should we not have something
conditional here?

/
    Leif

>    }
>  
>    // enable link
> -- 
> 2.17.0
>