[edk2-devel] [PATCH] On branch PCIBus dulePkg/PciBusDxe: PciTestSupportedAttribute logic should be changed.

[Wu, Hao A]

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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Wu, Hao A
> Sent: Wednesday, September 22, 2021 1:45 PM
> To: Xue, Shengfeng <xueshengfeng at byosoft.com.cn>; devel at edk2.groups.io;
> gaoliming at byosoft.com.cn; Ni, Ray <ray.ni at intel.com>
> Cc: Xue, ShengfengX <shengfengx.xue at intel.com>; Liang, PanlingX
> <panlingx.liang at 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 <xueshengfeng at byosoft.com.cn>
> > Sent: Wednesday, September 22, 2021 11:18 AM
> > To: devel at edk2.groups.io; gaoliming at byosoft.com.cn; Wu, Hao A
> > <hao.a.wu at intel.com>; Ni, Ray <ray.ni at intel.com>
> > Cc: Xue, ShengfengX <shengfengx.xue at intel.com>; Liang, PanlingX
> > <panlingx.liang at 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.xue at intel.com
> >      Reviewed-by: gaoliming at 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.wu at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-