Sorry for missing one comment. Please help to update the subject of the patch to: MdeModulePkg/PciBusDxe: PciTestSupportedAttribute logic should be changed
> -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao A > Sent: Wednesday, September 22, 2021 1:45 PM > To: Xue, Shengfeng <xueshengf...@byosoft.com.cn>; devel@edk2.groups.io; > gaolim...@byosoft.com.cn; Ni, Ray <ray...@intel.com> > Cc: Xue, ShengfengX <shengfengx....@intel.com>; Liang, PanlingX > <panlingx.li...@intel.com> > Subject: Re: [edk2-devel] [PATCH] On branch PCIBus dulePkg/PciBusDxe: > PciTestSupportedAttribute logic should be changed. > > 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 (#80945): https://edk2.groups.io/g/devel/message/80945 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] -=-=-=-=-=-=-=-=-=-=-=-
