diff mbox series

[Linaro-uefi,Linaro-uefi,v3,2/3] Hisilicon: disable RC Option Rom

Message ID 1491827595-84884-3-git-send-email-chenhui.sun@linaro.org
State New
Headers show
Series D03/D05 platforms bug fix | expand

Commit Message

Chenhui Sun April 10, 2017, 12:33 p.m. UTC 
The M3(the coprocessor)PCIe driver will read Option Rom header
durning enumeration, this operation will cause a completion error
when there is no device inserted to the RC port, and the Option rom
is uesless now. So we need to disable the RC Option Rom.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
Signed-off-by: Chenhui Sun <chenhui.sun@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 .../Hi1610/Drivers/PcieInit1610/PcieInitLib.c      | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)

Comments

Leif Lindholm April 10, 2017, 1:53 p.m. UTC | #1 
On Mon, Apr 10, 2017 at 08:33:14PM +0800, Chenhui Sun wrote:
> The M3(the coprocessor)PCIe driver will read Option Rom header
> durning enumeration, this operation will cause a completion error
> when there is no device inserted to the RC port, and the Option rom
> is uesless now. So we need to disable the RC Option Rom.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
> Signed-off-by: Chenhui Sun <chenhui.sun@linaro.org>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  .../Hi1610/Drivers/PcieInit1610/PcieInitLib.c      | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
> index a9b3d74..1df7a90 100644
> --- a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
> +++ b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
> @@ -901,6 +901,50 @@ void PcieConfigContextHi1610(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port)
>      return;
>  }
>  
> +UINT32
> +SysRegRead (
> +  IN UINT32 SocType,
> +  IN UINT32 HostBridgeNum,
> +  IN UINT32 Port,
> +  IN UINTN Reg
> +  )
> +{
> +  UINT32 Value;
> +  if (SocType == 0x1610) {
> +    RegRead (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + Reg, Value);
> +  } else {
> +    //PCIE_APB_SLVAE_BASE is for 660,and each PCIe Ccontroller has the same APB_SLVAE_BASE
> +    //in the same hostbridge.
> +    RegRead (PCIE_APB_SLVAE_BASE[HostBridgeNum] + Reg, Value);

This error was not introduced by this patch, so we don't need to fix
it for 2017.04, but it would be nice if after the release you could
submit a patch fixing the above spelling error (PCIE_APB_SLVAE_BASE ->
PCIE_APB_SLAVE_BASE). Extra visible here because
PCIE_APB_SLAVE_BASE_1610 gets it right.

Actually, even better would be if those could be modified to conform
to coding style: gPcieApbSlaveBase and gPcieApbSlaveBase1610.

(I already signed off on this patch, I just didn't push it since I
think it conflicted with other patches I had comments on.)

Regards,

Leif

> +  }
> +  return Value;
> +}
> +
> +VOID
> +DisableRcOptionRom (
> +  IN UINT32 Soctype,
> +  IN UINT32 HostBridgeNum,
> +  IN UINT32 Port,
> +  IN PCIE_PORT_TYPE PcieType
> +)
> +{
> +  UINT32 Value = 0;
> +  if (PcieType == PCIE_ROOT_COMPLEX) {
> +    Value = SysRegRead (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);
> +    Value |= BIT2; //cs2 enable
> +    SysRegWrite (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);
> +
> +    Value = SysRegRead (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG);
> +    Value &= ~BIT0; //disable option rom
> +    SysRegWrite (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG, Value);
> +
> +    Value = SysRegRead (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);
> +    Value &= ~BIT2; //cs2 disable
> +    SysRegWrite (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);
> +  }
> +  return;
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  PciePortInit (
> @@ -961,6 +1005,8 @@ PciePortInit (
>       /* Pcie Equalization*/
>       (VOID)PcieEqualization(soctype ,HostBridgeNum, PortIndex);
>  
> +     /* Disable RC Option Rom */
> +     DisableRcOptionRom (soctype, HostBridgeNum, PortIndex, PcieCfg->PortInfo.PortType);
>       /* assert LTSSM enable */
>       (VOID)PcieEnableItssm(soctype, HostBridgeNum, PortIndex);
>       if (FeaturePcdGet(PcdIsPciPerfTuningEnable)) {
> -- 
> 1.9.1
>
Chenhui Sun April 11, 2017, 12:43 a.m. UTC | #2 
Hi Leif,


在 2017/4/10 21:53, Leif Lindholm 写道:
> On Mon, Apr 10, 2017 at 08:33:14PM +0800, Chenhui Sun wrote:
>> The M3(the coprocessor)PCIe driver will read Option Rom header
>> durning enumeration, this operation will cause a completion error
>> when there is no device inserted to the RC port, and the Option rom
>> is uesless now. So we need to disable the RC Option Rom.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
>> Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
>> Signed-off-by: Chenhui Sun <chenhui.sun@linaro.org>
>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>> ---
>>   .../Hi1610/Drivers/PcieInit1610/PcieInitLib.c      | 46 ++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
>> index a9b3d74..1df7a90 100644
>> --- a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
>> +++ b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
>> @@ -901,6 +901,50 @@ void PcieConfigContextHi1610(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port)
>>       return;
>>   }
>>   
>> +UINT32
>> +SysRegRead (
>> +  IN UINT32 SocType,
>> +  IN UINT32 HostBridgeNum,
>> +  IN UINT32 Port,
>> +  IN UINTN Reg
>> +  )
>> +{
>> +  UINT32 Value;
>> +  if (SocType == 0x1610) {
>> +    RegRead (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + Reg, Value);
>> +  } else {
>> +    //PCIE_APB_SLVAE_BASE is for 660,and each PCIe Ccontroller has the same APB_SLVAE_BASE
>> +    //in the same hostbridge.
>> +    RegRead (PCIE_APB_SLVAE_BASE[HostBridgeNum] + Reg, Value);
> This error was not introduced by this patch, so we don't need to fix
> it for 2017.04, but it would be nice if after the release you could
> submit a patch fixing the above spelling error (PCIE_APB_SLVAE_BASE ->
> PCIE_APB_SLAVE_BASE). Extra visible here because
> PCIE_APB_SLAVE_BASE_1610 gets it right.
Thanks for pointing the spelling error.
> Actually, even better would be if those could be modified to conform
> to coding style: gPcieApbSlaveBase and gPcieApbSlaveBase1610.
ok, will do that.
> (I already signed off on this patch, I just didn't push it since I
> think it conflicted with other patches I had comments on.)
ok,

Thanks an Regards,
Chenhui
> Regards,
>
> Leif
>
>> +  }
>> +  return Value;
>> +}
>> +
>> +VOID
>> +DisableRcOptionRom (
>> +  IN UINT32 Soctype,
>> +  IN UINT32 HostBridgeNum,
>> +  IN UINT32 Port,
>> +  IN PCIE_PORT_TYPE PcieType
>> +)
>> +{
>> +  UINT32 Value = 0;
>> +  if (PcieType == PCIE_ROOT_COMPLEX) {
>> +    Value = SysRegRead (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);
>> +    Value |= BIT2; //cs2 enable
>> +    SysRegWrite (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);
>> +
>> +    Value = SysRegRead (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG);
>> +    Value &= ~BIT0; //disable option rom
>> +    SysRegWrite (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG, Value);
>> +
>> +    Value = SysRegRead (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);
>> +    Value &= ~BIT2; //cs2 disable
>> +    SysRegWrite (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);
>> +  }
>> +  return;
>> +}
>> +
>>   EFI_STATUS
>>   EFIAPI
>>   PciePortInit (
>> @@ -961,6 +1005,8 @@ PciePortInit (
>>        /* Pcie Equalization*/
>>        (VOID)PcieEqualization(soctype ,HostBridgeNum, PortIndex);
>>   
>> +     /* Disable RC Option Rom */
>> +     DisableRcOptionRom (soctype, HostBridgeNum, PortIndex, PcieCfg->PortInfo.PortType);
>>        /* assert LTSSM enable */
>>        (VOID)PcieEnableItssm(soctype, HostBridgeNum, PortIndex);
>>        if (FeaturePcdGet(PcdIsPciPerfTuningEnable)) {
>> -- 
>> 1.9.1
>>
diff mbox series

Patch

diff --git a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
index a9b3d74..1df7a90 100644
--- a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
+++ b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
@@ -901,6 +901,50 @@  void PcieConfigContextHi1610(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port)
     return;
 }
 
+UINT32
+SysRegRead (
+  IN UINT32 SocType,
+  IN UINT32 HostBridgeNum,
+  IN UINT32 Port,
+  IN UINTN Reg
+  )
+{
+  UINT32 Value;
+  if (SocType == 0x1610) {
+    RegRead (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + Reg, Value);
+  } else {
+    //PCIE_APB_SLVAE_BASE is for 660,and each PCIe Ccontroller has the same APB_SLVAE_BASE
+    //in the same hostbridge.
+    RegRead (PCIE_APB_SLVAE_BASE[HostBridgeNum] + Reg, Value);
+  }
+  return Value;
+}
+
+VOID
+DisableRcOptionRom (
+  IN UINT32 Soctype,
+  IN UINT32 HostBridgeNum,
+  IN UINT32 Port,
+  IN PCIE_PORT_TYPE PcieType
+)
+{
+  UINT32 Value = 0;
+  if (PcieType == PCIE_ROOT_COMPLEX) {
+    Value = SysRegRead (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);
+    Value |= BIT2; //cs2 enable
+    SysRegWrite (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);
+
+    Value = SysRegRead (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG);
+    Value &= ~BIT0; //disable option rom
+    SysRegWrite (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG, Value);
+
+    Value = SysRegRead (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);
+    Value &= ~BIT2; //cs2 disable
+    SysRegWrite (Soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);
+  }
+  return;
+}
+
 EFI_STATUS
 EFIAPI
 PciePortInit (
@@ -961,6 +1005,8 @@  PciePortInit (
      /* Pcie Equalization*/
      (VOID)PcieEqualization(soctype ,HostBridgeNum, PortIndex);
 
+     /* Disable RC Option Rom */
+     DisableRcOptionRom (soctype, HostBridgeNum, PortIndex, PcieCfg->PortInfo.PortType);
      /* assert LTSSM enable */
      (VOID)PcieEnableItssm(soctype, HostBridgeNum, PortIndex);
      if (FeaturePcdGet(PcdIsPciPerfTuningEnable)) {