Message ID | 1490015485-53685-4-git-send-email-chenhui.sun@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | D02/D03 platforms bug fix | expand |
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 >
Hi Leif, Thank you for pointing this out. :) Regards 在 2017/3/21 0:05, Leif Lindholm 写道: > 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 >>
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)) {