[Linaro-uefi,Linaro-uefi,v1,03/21] Hisilicon: disable RC Option Rom

Message ID 1490015485-53685-4-git-send-email-chenhui.sun@linaro.org
State New
Headers show
Series
  • D02/D03 platforms bug fix
Related show

Commit Message

Chenhui Sun March 20, 2017, 1:11 p.m.
The M3(the corprocessor)PCIe driver will read Option Rom header
durning enumeration, this opration 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: Chenhui Sun <sunchenhui@huawei.com>
Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
---
 .../Hi1610/Drivers/PcieInit1610/PcieInitLib.c      | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Leif Lindholm March 20, 2017, 4:05 p.m. | #1
On Mon, Mar 20, 2017 at 09:11:07PM +0800, Chenhui Sun wrote:
> The M3(the corprocessor)PCIe driver will read Option Rom header

"corprocessor" -> "coprocessor".

> durning enumeration, this opration will cause a completion error

"opration" -> "operation".

> 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: Chenhui Sun <sunchenhui@huawei.com>
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> Signed-off-by: Yi Li <phoenix.liyi@huawei.com>
> ---
>  .../Hi1610/Drivers/PcieInit1610/PcieInitLib.c      | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
> index 3bad240..57699e0 100644
> --- a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
> +++ b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
> @@ -904,6 +904,44 @@ void PcieConfigContextHi1610(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port)
>      return;
>  }
>  
> +UINT32 SysRegRead(UINT32 SocType, UINT32 HostBridgeNum, UINT32 Port, UINTN Reg)

Coding style says this should be

UINT32
SysRegRead (
  UINT32 SocType,
  UINT32 HostBridgeNum,
  UINT32 Port,
  UINTN  Reg
  )

Also, whether the arguments are IN, OUT or IN OUT.
5.7.1.11 of https://github.com/tianocore-docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf

> +{
> +  UINT32 Value;
> +  if (SocType == 0x1610) {
> +    RegRead(PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + Reg, Value);

Space before "(".

> +  } 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);

Space before "(".

> +  }
> +  return Value;
> +}
> +
> +VOID
> +DisableRcOptionRom (
> +  UINT32 soctype,

Since this is a new function, could you rename this variable
"SocType", like in the previous function? For coding style compliance.

> +  UINT32 HostBridgeNum,
> +  UINT32 Port,
> +  PCIE_PORT_TYPE PcieType
> +)

Please add IN/OUT indicators.

> +{
> +  UINT32 Value = 0;
> +  if (PcieType == PCIE_ROOT_COMPLEX) {
> +    Value = SysRegRead(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);

Space before "(".

> +    Value |= BIT2; //cs2 enable
> +    SysRegWrite(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);

Space before "(".

> +
> +    Value = SysRegRead(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG);

Space before "(".

> +    Value &= ~BIT0; //disable option rom
> +    SysRegWrite(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG, Value);

Space before "(".

> +
> +    Value = SysRegRead(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);

Space before "(".

> +    Value &= ~BIT2; //cs2 disable
> +    SysRegWrite(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);

Space before "(".

> +  }
> +  return;
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  PciePortInit (
> @@ -964,6 +1002,8 @@ PciePortInit (
>       /* Pcie Equalization*/
>       (VOID)PcieEqualization(soctype ,HostBridgeNum, PortIndex);
>  
> +     /* Disable RC Option Rom */
> +     DisableRcOptionRom(soctype, HostBridgeNum, PortIndex, PcieCfg->PortInfo.PortType);

Space before "(".

>       /* assert LTSSM enable */
>       (VOID)PcieEnableItssm(soctype, HostBridgeNum, PortIndex);
>       if (FeaturePcdGet(PcdIsPciPerfTuningEnable)) {
> -- 
> 1.9.1
>

Patch hide | download patch | download mbox

diff --git a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
index 3bad240..57699e0 100644
--- a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
+++ b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
@@ -904,6 +904,44 @@  void PcieConfigContextHi1610(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port)
     return;
 }
 
+UINT32 SysRegRead(UINT32 SocType, UINT32 HostBridgeNum, UINT32 Port, 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 (
+  UINT32 soctype,
+  UINT32 HostBridgeNum,
+  UINT32 Port,
+  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 (
@@ -964,6 +1002,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)) {