Two inline comments below:
> -----Original Message----- > From: xueshengfeng <xueshengf...@byosoft.com.cn> > Sent: Wednesday, September 22, 2021 11:18 AM > To: devel@edk2.groups.io; gaolim...@byosoft.com.cn; Wu, Hao A > <hao.a...@intel.com>; Ni, Ray <ray...@intel.com> > Cc: Xue, ShengfengX <shengfengx....@intel.com>; Liang, PanlingX > <panlingx.li...@intel.com> > Subject: [PATCH] On branch PCIBus dulePkg/PciBusDxe: > PciTestSupportedAttribute logic should be changed. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3635 > > In file Edk2\MdeModulePkg\Bus\Pci\PciBusDxe\PciEnumeratorSupport.c > Function PciTestSupportedAttribute, This function is called when doing the > PCI > enumerate, it tries to test whether the device can support given attributes > by > follow steps. > 1), read the original register value > 2), set to the input register value > 3), read back the register value, return this value as output 4), restore > the > original value This will cause the enabled bits in this > register be disabled during this sequence. > > Below are the new suggested flow: > 1) , read the original register value. > 2), set to input register value OR(|) the original register value. > 3), read back the register value, return the value AND(&) the input > command value as output. > 4), restore the original value > > Above sequence will not change the enabled bits in the register, > and can the new supported attributes by the device. > > Signed-off-by: shengfengx....@intel.com > Reviewed-by: gaolim...@byosoft.com.cn Please fix the inconsistent space indent within the commit log message. > --- > MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > index db1b35f8ef..2462f58833 100644 > --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c > @@ -933,6 +933,7 @@ PciTestSupportedAttribute ( > ) > { > EFI_TPL OldTpl; > + UINT16 CommandTemp = 0; Please separate the local variable definition and its initial value assignment. With the above 2 comments handled, Reviewed-by: Hao A Wu <hao.a...@intel.com> Best Regards, Hao Wu > > // > // Preserve the original value > @@ -944,9 +945,10 @@ PciTestSupportedAttribute ( > // > OldTpl = gBS->RaiseTPL (TPL_HIGH_LEVEL); > > - PCI_SET_COMMAND_REGISTER (PciIoDevice, *Command); > - PCI_READ_COMMAND_REGISTER (PciIoDevice, Command); > + PCI_SET_COMMAND_REGISTER (PciIoDevice, (*Command | *OldCommand)); > + PCI_READ_COMMAND_REGISTER (PciIoDevice, &CommandTemp); > > + *Command = (*Command) & CommandTemp; > // > // Write back the original value > // > -- > 2.31.1.windows.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80944): https://edk2.groups.io/g/devel/message/80944 Mute This Topic: https://groups.io/mt/85783989/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
