Re: [PATCH] OvmfPkg/SmmControl2Dxe: correct PCI_CONFIG_READ_WRITE in S3 boot script

[Laszlo Ersek <lersek@redhat.com>]

On 01/05/17 02:45, Yao, Jiewen wrote:
> Sorry, fix typo: I agree we do **not** use EFI_ prefix here.
> 
> But using _EFI as suffix is also odd. :)
> 
> Do we have better name?
> 
> Such as POWER_MGMT_REGISTER_Q35_ADDRESS ?

POWER_MGMT_REGISTER_Q35() already returns a flat address, just in PciLib
encoding. So an _ADDRESS suffix for the variant with the UEFI spec
encoding is not particularly telling.

The new macro name should reflect the PciLib encoding <-> UEFI spec
encoding difference.

Thanks!
Laszlo


> 
>  
> 
> Thank you
> 
> Yao Jiewen
> 
>  
> 
> *From:*edk2-devel [mailto:edk2-devel-bounces@lists.01.org] *On Behalf Of
> *Yao, Jiewen
> *Sent:* Thursday, January 5, 2017 9:03 AM
> *To:* Justen, Jordan L <jordan.l.justen@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; edk2-devel-01 <edk2-devel@lists.01.org>
> *Subject:* Re: [edk2] [PATCH] OvmfPkg/SmmControl2Dxe: correct
> PCI_CONFIG_READ_WRITE in S3 boot script
> 
>  
> 
> Hi
> I agree we do use EFI_ prefix here.
> 
> But using _EFI as suffix is also odd. :)
> 
> Do we have better name?
> 
> Thank you
> Yao Jiewen
> 
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jordan Justen
> Sent: Thursday, January 5, 2017 6:02 AM
> To: Laszlo Ersek <lersek@redhat.com
> <mailto:lersek@redhat.com>>; edk2-devel-01 <edk2-devel@lists.01.org
> <mailto:edk2-devel@lists.01.org>>
> Subject: Re: [edk2] [PATCH] OvmfPkg/SmmControl2Dxe: correct PCI_CONFIG_READ_WRITE in S3 boot script
> 
> On 2017-01-04 03:19:20, Laszlo Ersek wrote:
>> On 01/04/17 02:30, Jordan Justen wrote:
>> > On 2016-12-02 02:48:44, Laszlo Ersek wrote:
>> >> @@ -362,7 +390,9 @@ OnS3SaveStateInstalled (
>> >>                            S3SaveState,
>> >>                            EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE_OPCODE,
>> >>                            EfiBootScriptWidthUint16,
>> >> -                          (UINT64)POWER_MGMT_REGISTER_Q35 (ICH9_GEN_PMCON_1),
>> >> +                          ConvertPciLibToEfiPciAddress (
>> >> +                            POWER_MGMT_REGISTER_Q35 (ICH9_GEN_PMCON_1)
>> >
>> > I think we should just add a EFI_POWER_MGMT_REGISTER_Q35 macro.
>>
>> I thought of that, but I didn't want to use the EFI_ prefix for a macro
>> that has nothing to do with the UEFI / PI specs. Can you suggest an
>> alternative name? Perhaps POWER_MGMT_REGISTER_Q35_EFI?
> 
> Good point. Yeah, that name seems fine to me.
> 
> -Jordan
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org
> <mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>
> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
>