qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement
@ 2019-03-05 22:03 Heyi Guo
  2019-03-05 22:03 ` [Qemu-devel] [PATCH v3 2/2] hw/arm/acpi: enable SHPC native hot plug Heyi Guo
  2019-03-06 10:33 ` [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement Igor Mammedov
  0 siblings, 2 replies; 7+ messages in thread
From: Heyi Guo @ 2019-03-05 22:03 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: wanghaibin.wang, Heyi Guo, Shannon Zhao, Peter Maydell,
	Michael S. Tsirkin, Igor Mammedov

The last argument of AML bit and/or statement is the target variable,
so we don't need to use a NULL target and then an additional store
operation; a single bit and/or statement is enough.

Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Heyi Guo <guoheyi@huawei.com>
---
 hw/arm/virt-acpi-build.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 04b62c7..1c84e87 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -265,16 +265,16 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
         aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
     aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
     aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
-    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL),
+    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
                                 aml_name("CTRL")));
 
     ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
-    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
+    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08),
                                  aml_name("CDW1")));
     aml_append(ifctx, ifctx1);
 
     ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
-    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
+    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10),
                                  aml_name("CDW1")));
     aml_append(ifctx, ifctx1);
 
@@ -283,7 +283,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
     aml_append(method, ifctx);
 
     elsectx = aml_else();
-    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
+    aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4),
                                   aml_name("CDW1")));
     aml_append(elsectx, aml_return(aml_arg(3)));
     aml_append(method, elsectx);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Qemu-devel] [PATCH v3 2/2] hw/arm/acpi: enable SHPC native hot plug
  2019-03-05 22:03 [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement Heyi Guo
@ 2019-03-05 22:03 ` Heyi Guo
  2019-03-06 10:34   ` Igor Mammedov
  2019-03-06 10:33 ` [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement Igor Mammedov
  1 sibling, 1 reply; 7+ messages in thread
From: Heyi Guo @ 2019-03-05 22:03 UTC (permalink / raw)
  To: qemu-arm, qemu-devel
  Cc: wanghaibin.wang, Heyi Guo, Shannon Zhao, Peter Maydell,
	Michael S. Tsirkin, Igor Mammedov

After the introduction of generic PCIe root port and PCIe-PCI bridge,
we will also have SHPC controller on ARM, so just enalbe SHPC native
hot plug.

Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Heyi Guo <guoheyi@huawei.com>
---
 hw/arm/virt-acpi-build.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 1c84e87..e8e00fe 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -265,7 +265,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
         aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
     aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
     aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
-    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
+
+    /*
+     * Allow OS control for all 5 features:
+     * PCIeHotplug SHPCHotplug PME AER PCIeCapability.
+     */
+    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1F),
                                 aml_name("CTRL")));
 
     ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement
  2019-03-05 22:03 [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement Heyi Guo
  2019-03-05 22:03 ` [Qemu-devel] [PATCH v3 2/2] hw/arm/acpi: enable SHPC native hot plug Heyi Guo
@ 2019-03-06 10:33 ` Igor Mammedov
  2019-03-06 13:09   ` Heyi Guo
  1 sibling, 1 reply; 7+ messages in thread
From: Igor Mammedov @ 2019-03-06 10:33 UTC (permalink / raw)
  To: Heyi Guo
  Cc: qemu-arm, qemu-devel, Peter Maydell, Michael S. Tsirkin,
	Shannon Zhao, wanghaibin.wang

On Wed, 6 Mar 2019 06:03:48 +0800
Heyi Guo <guoheyi@huawei.com> wrote:

> The last argument of AML bit and/or statement is the target variable,
> so we don't need to use a NULL target and then an additional store
> operation; a single bit and/or statement is enough.
> 
> Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
> ---
>  hw/arm/virt-acpi-build.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 04b62c7..1c84e87 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -265,16 +265,16 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>          aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
>      aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
>      aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> -    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL),
> +    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
>                                  aml_name("CTRL")));
pls fix indent in all such cases. I'd align it like this:
   aml_append(ifctx,
              aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL")));

or
   aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
                             aml_name("CTRL")));


PS:
When making multi-patch series use --cover-letter git option

>  
>      ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
> -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
> +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08),
>                                   aml_name("CDW1")));
>      aml_append(ifctx, ifctx1);
>  
>      ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
> -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
> +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10),
>                                   aml_name("CDW1")));
>      aml_append(ifctx, ifctx1);
>  
> @@ -283,7 +283,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>      aml_append(method, ifctx);
>  
>      elsectx = aml_else();
> -    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
> +    aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4),
>                                    aml_name("CDW1")));
>      aml_append(elsectx, aml_return(aml_arg(3)));
>      aml_append(method, elsectx);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v3 2/2] hw/arm/acpi: enable SHPC native hot plug
  2019-03-05 22:03 ` [Qemu-devel] [PATCH v3 2/2] hw/arm/acpi: enable SHPC native hot plug Heyi Guo
@ 2019-03-06 10:34   ` Igor Mammedov
  0 siblings, 0 replies; 7+ messages in thread
From: Igor Mammedov @ 2019-03-06 10:34 UTC (permalink / raw)
  To: Heyi Guo
  Cc: qemu-arm, qemu-devel, Peter Maydell, Michael S. Tsirkin,
	Shannon Zhao, wanghaibin.wang

On Wed, 6 Mar 2019 06:03:49 +0800
Heyi Guo <guoheyi@huawei.com> wrote:

> After the introduction of generic PCIe root port and PCIe-PCI bridge,
> we will also have SHPC controller on ARM, so just enalbe SHPC native
> hot plug.
> 
> Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
> ---
>  hw/arm/virt-acpi-build.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 1c84e87..e8e00fe 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -265,7 +265,12 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>          aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
>      aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
>      aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> -    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
> +
> +    /*
> +     * Allow OS control for all 5 features:
> +     * PCIeHotplug SHPCHotplug PME AER PCIeCapability.
> +     */
> +    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1F),
>                                  aml_name("CTRL")));
ditto: fix indent

>  
>      ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement
  2019-03-06 10:33 ` [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement Igor Mammedov
@ 2019-03-06 13:09   ` Heyi Guo
  2019-03-06 16:20     ` Igor Mammedov
  0 siblings, 1 reply; 7+ messages in thread
From: Heyi Guo @ 2019-03-06 13:09 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: qemu-arm, qemu-devel, Peter Maydell, Michael S. Tsirkin,
	Shannon Zhao, wanghaibin.wang

Sorry, I didn't know the indention policy of qemu code. So we need to indent the 2nd line just after the parentheses it belongs to?

Will change that and send next version.

Thanks,

Heyi


On 2019/3/6 18:33, Igor Mammedov wrote:
> On Wed, 6 Mar 2019 06:03:48 +0800
> Heyi Guo <guoheyi@huawei.com> wrote:
>
>> The last argument of AML bit and/or statement is the target variable,
>> so we don't need to use a NULL target and then an additional store
>> operation; a single bit and/or statement is enough.
>>
>> Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
>> Cc: Peter Maydell <peter.maydell@linaro.org>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
>> ---
>>   hw/arm/virt-acpi-build.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 04b62c7..1c84e87 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -265,16 +265,16 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>>           aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
>>       aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
>>       aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
>> -    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL),
>> +    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
>>                                   aml_name("CTRL")));
> pls fix indent in all such cases. I'd align it like this:
>     aml_append(ifctx,
>                aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL")));
>
> or
>     aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
>                               aml_name("CTRL")));
>
>
> PS:
> When making multi-patch series use --cover-letter git option
>
>>   
>>       ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
>> -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
>> +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08),
>>                                    aml_name("CDW1")));
>>       aml_append(ifctx, ifctx1);
>>   
>>       ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
>> -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
>> +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10),
>>                                    aml_name("CDW1")));
>>       aml_append(ifctx, ifctx1);
>>   
>> @@ -283,7 +283,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>>       aml_append(method, ifctx);
>>   
>>       elsectx = aml_else();
>> -    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
>> +    aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4),
>>                                     aml_name("CDW1")));
>>       aml_append(elsectx, aml_return(aml_arg(3)));
>>       aml_append(method, elsectx);
>
> .
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement
  2019-03-06 13:09   ` Heyi Guo
@ 2019-03-06 16:20     ` Igor Mammedov
  2019-03-07  1:07       ` Heyi Guo
  0 siblings, 1 reply; 7+ messages in thread
From: Igor Mammedov @ 2019-03-06 16:20 UTC (permalink / raw)
  To: Heyi Guo
  Cc: Peter Maydell, Michael S. Tsirkin, qemu-devel, Shannon Zhao,
	qemu-arm, wanghaibin.wang

On Wed, 6 Mar 2019 21:09:11 +0800
Heyi Guo <guoheyi@huawei.com> wrote:

> Sorry, I didn't know the indention policy of qemu code. So we need to indent the 2nd line just after the parentheses it belongs to?

See "[PATCH v6 0/2] CODING_STYLE: trivial update" for clarification

> 
> Will change that and send next version.
> 
> Thanks,
> 
> Heyi
> 
> 
> On 2019/3/6 18:33, Igor Mammedov wrote:
> > On Wed, 6 Mar 2019 06:03:48 +0800
> > Heyi Guo <guoheyi@huawei.com> wrote:
> >
> >> The last argument of AML bit and/or statement is the target variable,
> >> so we don't need to use a NULL target and then an additional store
> >> operation; a single bit and/or statement is enough.
> >>
> >> Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
> >> Cc: Peter Maydell <peter.maydell@linaro.org>
> >> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> >> Cc: Igor Mammedov <imammedo@redhat.com>
> >> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> >> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
> >> ---
> >>   hw/arm/virt-acpi-build.c | 8 ++++----
> >>   1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> >> index 04b62c7..1c84e87 100644
> >> --- a/hw/arm/virt-acpi-build.c
> >> +++ b/hw/arm/virt-acpi-build.c
> >> @@ -265,16 +265,16 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> >>           aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
> >>       aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
> >>       aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> >> -    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL),
> >> +    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
> >>                                   aml_name("CTRL")));
> > pls fix indent in all such cases. I'd align it like this:
> >     aml_append(ifctx,
> >                aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL")));
> >
> > or
> >     aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
> >                               aml_name("CTRL")));
> >
> >
> > PS:
> > When making multi-patch series use --cover-letter git option
> >
> >>   
> >>       ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
> >> -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
> >> +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08),
> >>                                    aml_name("CDW1")));
> >>       aml_append(ifctx, ifctx1);
> >>   
> >>       ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
> >> -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
> >> +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10),
> >>                                    aml_name("CDW1")));
> >>       aml_append(ifctx, ifctx1);
> >>   
> >> @@ -283,7 +283,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> >>       aml_append(method, ifctx);
> >>   
> >>       elsectx = aml_else();
> >> -    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
> >> +    aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4),
> >>                                     aml_name("CDW1")));
> >>       aml_append(elsectx, aml_return(aml_arg(3)));
> >>       aml_append(method, elsectx);
> >
> > .
> >
> 
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement
  2019-03-06 16:20     ` Igor Mammedov
@ 2019-03-07  1:07       ` Heyi Guo
  0 siblings, 0 replies; 7+ messages in thread
From: Heyi Guo @ 2019-03-07  1:07 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Peter Maydell, Michael S. Tsirkin, qemu-devel, Shannon Zhao,
	qemu-arm, wanghaibin.wang

Got it; thanks.

Heyi

On 2019/3/7 0:20, Igor Mammedov wrote:
> On Wed, 6 Mar 2019 21:09:11 +0800
> Heyi Guo <guoheyi@huawei.com> wrote:
>
>> Sorry, I didn't know the indention policy of qemu code. So we need to indent the 2nd line just after the parentheses it belongs to?
> See "[PATCH v6 0/2] CODING_STYLE: trivial update" for clarification
>
>> Will change that and send next version.
>>
>> Thanks,
>>
>> Heyi
>>
>>
>> On 2019/3/6 18:33, Igor Mammedov wrote:
>>> On Wed, 6 Mar 2019 06:03:48 +0800
>>> Heyi Guo <guoheyi@huawei.com> wrote:
>>>
>>>> The last argument of AML bit and/or statement is the target variable,
>>>> so we don't need to use a NULL target and then an additional store
>>>> operation; a single bit and/or statement is enough.
>>>>
>>>> Cc: Shannon Zhao <shannon.zhaosl@gmail.com>
>>>> Cc: Peter Maydell <peter.maydell@linaro.org>
>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>>> Cc: Igor Mammedov <imammedo@redhat.com>
>>>> Suggested-by: Igor Mammedov <imammedo@redhat.com>
>>>> Signed-off-by: Heyi Guo <guoheyi@huawei.com>
>>>> ---
>>>>    hw/arm/virt-acpi-build.c | 8 ++++----
>>>>    1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>>>> index 04b62c7..1c84e87 100644
>>>> --- a/hw/arm/virt-acpi-build.c
>>>> +++ b/hw/arm/virt-acpi-build.c
>>>> @@ -265,16 +265,16 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>>>>            aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
>>>>        aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
>>>>        aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
>>>> -    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL),
>>>> +    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
>>>>                                    aml_name("CTRL")));
>>> pls fix indent in all such cases. I'd align it like this:
>>>      aml_append(ifctx,
>>>                 aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL")));
>>>
>>> or
>>>      aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
>>>                                aml_name("CTRL")));
>>>
>>>
>>> PS:
>>> When making multi-patch series use --cover-letter git option
>>>
>>>>    
>>>>        ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
>>>> -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
>>>> +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08),
>>>>                                     aml_name("CDW1")));
>>>>        aml_append(ifctx, ifctx1);
>>>>    
>>>>        ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
>>>> -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
>>>> +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10),
>>>>                                     aml_name("CDW1")));
>>>>        aml_append(ifctx, ifctx1);
>>>>    
>>>> @@ -283,7 +283,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>>>>        aml_append(method, ifctx);
>>>>    
>>>>        elsectx = aml_else();
>>>> -    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
>>>> +    aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4),
>>>>                                      aml_name("CDW1")));
>>>>        aml_append(elsectx, aml_return(aml_arg(3)));
>>>>        aml_append(method, elsectx);
>>> .
>>>
>>
>>
>
> .
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-03-07  1:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-05 22:03 [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement Heyi Guo
2019-03-05 22:03 ` [Qemu-devel] [PATCH v3 2/2] hw/arm/acpi: enable SHPC native hot plug Heyi Guo
2019-03-06 10:34   ` Igor Mammedov
2019-03-06 10:33 ` [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement Igor Mammedov
2019-03-06 13:09   ` Heyi Guo
2019-03-06 16:20     ` Igor Mammedov
2019-03-07  1:07       ` Heyi Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).