Re: [PATCH edk2-platforms v1 08/14] Hisilicon/PCIe: Disable PCIe ASPM

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 18 January 2018 at 15:01, Ming Huang <heyi.guo@linaro.org> wrote:
> From: Yan Zhang <zhangyan81@huawei.com>
>
> In order to replace command line parameter pcie_aspm=off, BIOS needs to
> disable Pcie Aspm support during Pcie initilization.
> D03 and D05 do not support PCIe ASPM, so we disable it in BIOS.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <huangming23@huawei.com>
> Signed-off-by: Heyi Guo <heyi.guo@linaro.org>
> Signed-off-by: Yan Zhang <zhangyan81@huawei.com>
> ---
>  Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c | 71 ++++++++++++++++++++
>  Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.h |  2 +
>  Silicon/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h        |  2 +
>  3 files changed, 75 insertions(+)
>
> diff --git a/Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c b/Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
> index f420c91..ca3b2f8 100644
> --- a/Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
> +++ b/Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c
> @@ -1033,6 +1033,74 @@ DisableRcOptionRom (
>    return;
>  }
>

STATIC

> +VOID
> +PcieDbiCs2Enable(

Space before (

> +  IN UINT32 HostBridgeNum,
> +  IN UINT32 Port,
> +  IN BOOLEAN Val
> +  )
> +{
> +  UINT32 RegVal;
> +  RegRead (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_SUBCTRL_SC_PCIE_SYS_CTRL21, RegVal);

Line length

> +  if (Val) {
> +    RegVal = RegVal | BIT2;
> +    /*BIT2: DBI Chip Select indicator. 0 indicates CS, 1 indicates CS2.*/

Space after /*

> +  } else {
> +    RegVal = RegVal & (~BIT2);
> +  }
> +  RegWrite (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_SUBCTRL_SC_PCIE_SYS_CTRL21, RegVal);
> +}
> +

STATIC

> +BOOLEAN
> +PcieDBIReadOnlyWriteEnable(

Space before (

> +  IN UINT32 HostBridgeNum,
> +  IN UINT32 Port
> +  )
> +{
> +  UINT32  Val;
> +  RegRead (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_DBI_READ_ONLY_WRITE_ENABLE, Val);
> +  if (Val == 0x1) {
> +    return TRUE;
> +  } else {
> +    RegWrite (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_DBI_READ_ONLY_WRITE_ENABLE, 0x1);
> +    /*Delay 10us to make sure the PCIE device have enouph time to response. */

Space after /*

> +    MicroSecondDelay(10);
> +    RegRead (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_DBI_READ_ONLY_WRITE_ENABLE, Val);

Line length

> +    if (Val == 0x1) {
> +      return TRUE;
> +    }
> +  }
> +  DEBUG ((DEBUG_ERROR,"PcieDBIReadOnlyWriteEnable Fail!!!\n"));
> +  return FALSE;
> +}

STATIC

> +VOID
> +SwitchPcieASPMSupport (
> +  IN UINT32 HostBridgeNum,
> +  IN UINT32 Port,
> +  IN UINT8 Val
> +  )
> +{
> +  PCIE_EP_PCIE_CAP3_U pcie_cap3;

PcieCap3

> +
> +  if (Port >= PCIE_MAX_ROOTBRIDGE) {
> +    DEBUG ((DEBUG_ERROR, "Port is not valid\n"));
> +    return;
> +  }
> +  if (!PcieDBIReadOnlyWriteEnable (HostBridgeNum, Port)) {
> +    DEBUG ((DEBUG_INFO, "PcieDeEmphasisLevelSet ReadOnly Reg do not Enable!!!\n"));

PcieDeEmphasisLevelSet ?

> +    return;
> +  }
> +  PcieDbiCs2Enable (HostBridgeNum, Port, FALSE);
> +
> +  RegRead (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_EP_PCIE_CAP3_REG, pcie_cap3.UInt32);
> +  pcie_cap3.Bits.active_state_power_management = Val;
> +  RegWrite (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_EP_PCIE_CAP3_REG, pcie_cap3.UInt32);
> +  RegRead (PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_EP_PCIE_CAP3_REG, pcie_cap3.UInt32);
> +  DEBUG ((DEBUG_INFO, "ASPI active state power management: %d\n", pcie_cap3.Bits.active_state_power_management));
> +

Line length

> +  PcieDbiCs2Enable (HostBridgeNum, Port, TRUE);
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  PciePortInit (
> @@ -1090,6 +1158,9 @@ PciePortInit (
>       /* disable link up interrupt */
>       (VOID)PcieMaskLinkUpInit(soctype, HostBridgeNum, PortIndex);
>
> +     //disable ASPM

Please use comment style consistent with the other code

/* disable ASPM */

> +     SwitchPcieASPMSupport (HostBridgeNum, PortIndex, PCIE_ASPM_DISABLE);
> +
>       /* Pcie Equalization*/
>       (VOID)PcieEqualization(soctype ,HostBridgeNum, PortIndex);
>
> diff --git a/Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.h b/Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.h
> index 9a0f636..e96c53c 100644
> --- a/Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.h
> +++ b/Silicon/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.h
> @@ -77,6 +77,8 @@
>  #define RegWrite(addr,data)            MmioWrite32((addr), (data))
>  #define RegRead(addr,data)             ((data) = MmioRead32 (addr))
>
> +#define PCIE_ASPM_DISABLE 0x0
> +#define PCIE_ASPM_ENABLE 0x1
>
>  typedef struct tagPcieDebugInfo
>  {
> diff --git a/Silicon/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h b/Silicon/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h
> index bf57652..c8b9781 100644
> --- a/Silicon/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h
> +++ b/Silicon/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h
> @@ -135,6 +135,7 @@
>  #define PCIE_EEP_PORTLOGIC53_REG                  (0x888)
>  #define PCIE_EEP_GEN3_CONTRL_REG                  (0x890)
>  #define PCIE_EEP_PIPE_LOOPBACK_REG                (0x8B8)
> +#define PCIE_DBI_READ_ONLY_WRITE_ENABLE           (0x8BC)
>  #define PCIE_EEP_PORTLOGIC54_REG                  (0x900)
>  #define PCIE_EEP_PORTLOGIC55_REG                  (0x904)
>  #define PCIE_EEP_PORTLOGIC56_REG                  (0x908)
> @@ -12556,6 +12557,7 @@ typedef union tagPortlogic93
>  #define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY5_REG            (PCIE_SUBCTRL_BASE + 0x1018)
>  #define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY6_REG            (PCIE_SUBCTRL_BASE + 0x101C)
>  #define PCIE_SUBCTRL_SC_DISPATCH_DAW_ARRAY7_REG            (PCIE_SUBCTRL_BASE + 0x1020)
> +#define PCIE_SUBCTRL_SC_PCIE_SYS_CTRL21                    (PCIE_SUBCTRL_BASE + 0x1024)
>  #define PCIE_SUBCTRL_SC_DISPATCH_RETRY_CONTROL_REG         (PCIE_SUBCTRL_BASE + 0x1030)
>  #define PCIE_SUBCTRL_SC_DISPATCH_INTMASK_REG               (PCIE_SUBCTRL_BASE + 0x1100)
>  #define PCIE_SUBCTRL_SC_DISPATCH_RAWINT_REG                (PCIE_SUBCTRL_BASE + 0x1104)
> --
> 1.9.1
>