Re: [edk2] [PATCH edk2-platforms v1 09/16] Hisilicon/D06: Add PCI_OSC_SUPPORT
On Wed, Feb 13, 2019 at 10:59:17AM +0800, Ming Huang wrote: > On 2/12/2019 2:51 AM, Leif Lindholm wrote: > > On Fri, Feb 01, 2019 at 09:34:29PM +0800, Ming Huang wrote: > >> Add PCI_OSC_SUPPORT for remaining host bridges to remove fail > >> output in kernel: > >> [ 103.478893] acpi PNP0A08:01: _OSC failed (AE_NOT_FOUND); > >> > >> Contributed-under: TianoCore Contribution Agreement 1.1 > >> Signed-off-by: Ming Huang > >> --- > >> Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 64 > >> > >> 1 file changed, 64 insertions(+) > >> > >> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl > >> b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl > >> index 4d9d9d95be68..86d8728b82f2 100644 > >> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl > >> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl > >> @@ -17,6 +17,50 @@ > >> **/ > >> > >> //#include "ArmPlatform.h" > >> + > >> +/* > >> + See ACPI 6.1 Spec, 6.2.11, PCI Firmware Spec 3.0, 4.5 > >> +*/ > >> +#define PCI_OSC_SUPPORT() \ > > > > PCI0 and PCI6 already have _OSC entries. > > This macro ends up being used for 1-5 and 7-B. > > So calling it PCI_OSC_SUPPORT seems somewhat misleading. > > > > Then again, there is a lot of similarities between this macro and the > > existing entries. Could the same macro be used for 0 and 6? Or could > > the macro be split up into multiple parts and reused? > > When I make this patch, I try to rewrite PCI0/6 with the same macro, but > the macro don't support parameter. For spliting up multiple parts, if modify > something in future, the parts need to split up to smaller parts. So, if > need to rewrite PCI0/6 with macro, is it applicable to add another macro > PCI_OSC_SUPPORT_HOTPLUG? Yes, that sounds like a good solution to me. Regards, Leif ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH edk2-platforms v1 09/16] Hisilicon/D06: Add PCI_OSC_SUPPORT
On 2/12/2019 2:51 AM, Leif Lindholm wrote:
> On Fri, Feb 01, 2019 at 09:34:29PM +0800, Ming Huang wrote:
>> Add PCI_OSC_SUPPORT for remaining host bridges to remove fail
>> output in kernel:
>> [ 103.478893] acpi PNP0A08:01: _OSC failed (AE_NOT_FOUND);
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang
>> ---
>> Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 64
>>
>> 1 file changed, 64 insertions(+)
>>
>> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> index 4d9d9d95be68..86d8728b82f2 100644
>> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
>> @@ -17,6 +17,50 @@
>> **/
>>
>> //#include "ArmPlatform.h"
>> +
>> +/*
>> + See ACPI 6.1 Spec, 6.2.11, PCI Firmware Spec 3.0, 4.5
>> +*/
>> +#define PCI_OSC_SUPPORT() \
>
> PCI0 and PCI6 already have _OSC entries.
> This macro ends up being used for 1-5 and 7-B.
> So calling it PCI_OSC_SUPPORT seems somewhat misleading.
>
> Then again, there is a lot of similarities between this macro and the
> existing entries. Could the same macro be used for 0 and 6? Or could
> the macro be split up into multiple parts and reused?
When I make this patch, I try to rewrite PCI0/6 with the same macro, but
the macro don't support parameter. For spliting up multiple parts, if modify
something in future, the parts need to split up to smaller parts. So, if
need to rewrite PCI0/6 with macro, is it applicable to add another macro
PCI_OSC_SUPPORT_HOTPLUG?
Thanks
>
> /
> Leif
>
>> + Name(SUPP, Zero) /* PCI _OSC Support Field value */ \
>> + Name(CTRL, Zero) /* PCI _OSC Control Field value */ \
>> + Method(_OSC,4) { \
>> +If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) { \
>> + /* Create DWord-adressable fields from the Capabilities Buffer */ \
>> + CreateDWordField(Arg3,0,CDW1) \
>> + CreateDWordField(Arg3,4,CDW2) \
>> + CreateDWordField(Arg3,8,CDW3) \
>> + /* Save Capabilities DWord2 & 3 */ \
>> + Store(CDW2,SUPP) \
>> + Store(CDW3,CTRL) \
>> + /* Only allow native hot plug control if OS supports: */ \
>> + /* ASPM */ \
>> + /* Clock PM */ \
>> + /* MSI/MSI-X */ \
>> + If(LNotEqual(And(SUPP, 0x16), 0x16)) { \
>> +And(CTRL,0x1E,CTRL) \
>> + }\
>> + \
>> + /* Do not allow native PME, AER */ \
>> + /* Never allow SHPC (no SHPC controller in this system)*/ \
>> + And(CTRL,0x10,CTRL) \
>> + If(LNotEqual(Arg1,One)) { /* Unknown revision */ \
>> +Or(CDW1,0x08,CDW1) \
>> + } \
>> + \
>> + If(LNotEqual(CDW3,CTRL)) { /* Capabilities bits were masked */ \
>> +Or(CDW1,0x10,CDW1) \
>> + } \
>> + \
>> + /* Update DWORD3 in the buffer */ \
>> + Store(CTRL,CDW3) \
>> + Return(Arg3) \
>> +} Else { \
>> + Or(CDW1,4,CDW1) /* Unrecognized UUID */ \
>> + Return(Arg3) \
>> +} \
>> + } // End _OSC
>> +
>> Scope(_SB)
>> {
>>Device (PCI0)
>> @@ -270,6 +314,8 @@ Device (PCI1)
>> Return (RBUF)
>>} // Method(_CRS), this method
>> return RBUF!
>>
>> + PCI_OSC_SUPPORT ()
>> +
>>Method (_STA, 0x0, NotSerialized)
>>{
>> Return (0xf)
>> @@ -333,6 +379,8 @@ Device (PCI2)
>> Return (RBUF)
>>} // Method(_CRS), this method
>> return RBUF!
>>
>> + PCI_OSC_SUPPORT ()
>> +
>>Method (_STA, 0x0, NotSerialized)
>>{
>> Return (0xf)
>> @@ -382,6 +430,8 @@ Device (PCI3)
>> Return (RBUF)
>>} // Method(_CRS), this method
>> return RBUF!
>>
>> + PCI_OSC_SUPPORT ()
>> +
>>Method (_STA, 0x0, NotSerialized)
>>{
>> Return (0xf)
>> @@ -431,6 +481,8 @@ Device (PCI4)
>> Return (RBUF)
>>} // Method(_CRS), this method
>> return RBUF!
>>
>> + PCI_OSC_SUPPORT ()
>> +
>>Method (_STA, 0x0, NotSerialized)
>>{
>> Return (0x0F)
>> @@ -505,6 +557,8 @@ Device (PCI5)
>> Return (RBUF)
>>}// Method(_CRS), this method return
>> RBUF!
>>
>> + PCI_OSC_SUPPORT ()
>> +
>>Method (_STA, 0x0, NotSerialized)
>>{
>> Return (0xf)
>> @@ -1002,6 +1056,8 @@ Device (PCI7)
>> Return (RBUF)
>>} // Method(_CRS), this method
>> return RBUF!
>>
>> + PCI_OSC_SUPPORT ()
>> +
>>Method (_STA, 0x0, NotSerialized)
>>{
>> Return (0xf)
>> @@ -1066,6 +1122,8 @@ Device (PCI8)
>> Return (RBUF)
>>} // Method(_CRS), this method
>> return RBUF!
>>
>> + PCI_OSC_SUPPORT ()
>> +
>>Method (_STA, 0x0, NotSerialized)
>>{
>> Return (0xf)
>> @@ -1115,6 +1173,8 @@ Device
Re: [edk2] [PATCH edk2-platforms v1 09/16] Hisilicon/D06: Add PCI_OSC_SUPPORT
On Fri, Feb 01, 2019 at 09:34:29PM +0800, Ming Huang wrote:
> Add PCI_OSC_SUPPORT for remaining host bridges to remove fail
> output in kernel:
> [ 103.478893] acpi PNP0A08:01: _OSC failed (AE_NOT_FOUND);
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang
> ---
> Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl | 64
>
> 1 file changed, 64 insertions(+)
>
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> index 4d9d9d95be68..86d8728b82f2 100644
> --- a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl
> @@ -17,6 +17,50 @@
> **/
>
> //#include "ArmPlatform.h"
> +
> +/*
> + See ACPI 6.1 Spec, 6.2.11, PCI Firmware Spec 3.0, 4.5
> +*/
> +#define PCI_OSC_SUPPORT() \
PCI0 and PCI6 already have _OSC entries.
This macro ends up being used for 1-5 and 7-B.
So calling it PCI_OSC_SUPPORT seems somewhat misleading.
Then again, there is a lot of similarities between this macro and the
existing entries. Could the same macro be used for 0 and 6? Or could
the macro be split up into multiple parts and reused?
/
Leif
> + Name(SUPP, Zero) /* PCI _OSC Support Field value */ \
> + Name(CTRL, Zero) /* PCI _OSC Control Field value */ \
> + Method(_OSC,4) { \
> +If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) { \
> + /* Create DWord-adressable fields from the Capabilities Buffer */ \
> + CreateDWordField(Arg3,0,CDW1) \
> + CreateDWordField(Arg3,4,CDW2) \
> + CreateDWordField(Arg3,8,CDW3) \
> + /* Save Capabilities DWord2 & 3 */ \
> + Store(CDW2,SUPP) \
> + Store(CDW3,CTRL) \
> + /* Only allow native hot plug control if OS supports: */ \
> + /* ASPM */ \
> + /* Clock PM */ \
> + /* MSI/MSI-X */ \
> + If(LNotEqual(And(SUPP, 0x16), 0x16)) { \
> +And(CTRL,0x1E,CTRL) \
> + }\
> + \
> + /* Do not allow native PME, AER */ \
> + /* Never allow SHPC (no SHPC controller in this system)*/ \
> + And(CTRL,0x10,CTRL) \
> + If(LNotEqual(Arg1,One)) { /* Unknown revision */ \
> +Or(CDW1,0x08,CDW1) \
> + } \
> + \
> + If(LNotEqual(CDW3,CTRL)) { /* Capabilities bits were masked */ \
> +Or(CDW1,0x10,CDW1) \
> + } \
> + \
> + /* Update DWORD3 in the buffer */ \
> + Store(CTRL,CDW3) \
> + Return(Arg3) \
> +} Else { \
> + Or(CDW1,4,CDW1) /* Unrecognized UUID */ \
> + Return(Arg3) \
> +} \
> + } // End _OSC
> +
> Scope(_SB)
> {
>Device (PCI0)
> @@ -270,6 +314,8 @@ Device (PCI1)
> Return (RBUF)
>} // Method(_CRS), this method
> return RBUF!
>
> + PCI_OSC_SUPPORT ()
> +
>Method (_STA, 0x0, NotSerialized)
>{
> Return (0xf)
> @@ -333,6 +379,8 @@ Device (PCI2)
> Return (RBUF)
>} // Method(_CRS), this method
> return RBUF!
>
> + PCI_OSC_SUPPORT ()
> +
>Method (_STA, 0x0, NotSerialized)
>{
> Return (0xf)
> @@ -382,6 +430,8 @@ Device (PCI3)
> Return (RBUF)
>} // Method(_CRS), this method
> return RBUF!
>
> + PCI_OSC_SUPPORT ()
> +
>Method (_STA, 0x0, NotSerialized)
>{
> Return (0xf)
> @@ -431,6 +481,8 @@ Device (PCI4)
> Return (RBUF)
>} // Method(_CRS), this method
> return RBUF!
>
> + PCI_OSC_SUPPORT ()
> +
>Method (_STA, 0x0, NotSerialized)
>{
> Return (0x0F)
> @@ -505,6 +557,8 @@ Device (PCI5)
> Return (RBUF)
>}// Method(_CRS), this method return
> RBUF!
>
> + PCI_OSC_SUPPORT ()
> +
>Method (_STA, 0x0, NotSerialized)
>{
> Return (0xf)
> @@ -1002,6 +1056,8 @@ Device (PCI7)
> Return (RBUF)
>} // Method(_CRS), this method
> return RBUF!
>
> + PCI_OSC_SUPPORT ()
> +
>Method (_STA, 0x0, NotSerialized)
>{
> Return (0xf)
> @@ -1066,6 +1122,8 @@ Device (PCI8)
> Return (RBUF)
>} // Method(_CRS), this method
> return RBUF!
>
> + PCI_OSC_SUPPORT ()
> +
>Method (_STA, 0x0, NotSerialized)
>{
> Return (0xf)
> @@ -1115,6 +1173,8 @@ Device (PCI9)
> Return (RBUF)
>} // Method(_CRS), this method
> return RBUF!
>
> + PCI_OSC_SUPPORT ()
> +
>Method (_STA, 0x0, NotSerialized)
>{
> Return (0xf)
> @@ -1164,6 +1224,8 @@ Device (PCIA)
> Return (RBUF)
>} // Method(_CRS), this method
> return RBUF!
>
> + PCI_OSC_SUPPORT ()
> +
>Method (_STA, 0x0, NotSerialized)
>{
> Return (0x0F)
> @@ -1238,6 +1300,8 @@ Device (PCIB)
> Return (RBUF)
>}
