diff mbox

[v4,14/20] hw/acpi/aml-build: Add aml_or() term

Message ID 1428055432-12120-15-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao April 3, 2015, 10:03 a.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>

Add aml_or() term and make aml_and can take three args.
Expose build_append_int_noprefix as it wiil be used by
creating a buffer.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/acpi/aml-build.c         | 24 +++++++++++++++++++++---
 hw/i386/acpi-build.c        |  2 +-
 include/hw/acpi/aml-build.h |  4 +++-
 3 files changed, 25 insertions(+), 5 deletions(-)

Comments

Igor Mammedov April 9, 2015, 1:35 p.m. UTC | #1
On Fri, 3 Apr 2015 18:03:46 +0800
Shannon Zhao <zhaoshenglong@huawei.com> wrote:

> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Add aml_or() term and make aml_and can take three args.
> Expose build_append_int_noprefix as it wiil be used by
> creating a buffer.
> 
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/acpi/aml-build.c         | 24 +++++++++++++++++++++---
>  hw/i386/acpi-build.c        |  2 +-
>  include/hw/acpi/aml-build.h |  4 +++-
>  3 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 5a94fc9..312afb6 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -240,7 +240,7 @@ static void build_extop_package(GArray *package, uint8_t op)
>      build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
>  }
>  
> -static void build_append_int_noprefix(GArray *table, uint64_t value, int size)
> +void build_append_int_noprefix(GArray *table, uint64_t value, int size)
>  {
>      int i;
>  
> @@ -445,12 +445,30 @@ Aml *aml_store(Aml *val, Aml *target)
>  }
>  
>  /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAnd */
> -Aml *aml_and(Aml *arg1, Aml *arg2)
> +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3)
I know that it's possible to Store inside of And(a, b, save_here) ASL op,
but could you instead rewrite it to

 Store(And(a, b), save_here)

so it wouldn't clatter trivial  And(a,b) uses and drop this hunk.

>  {
>      Aml *var = aml_opcode(0x7B /* AndOp */);
>      aml_append(var, arg1);
>      aml_append(var, arg2);
> -    build_append_byte(var->buf, 0x00 /* NullNameOp */);
> +    if (arg3 == NULL) {
> +        build_append_byte(var->buf, 0x00 /* NullNameOp */);
> +    } else {
> +        aml_append(var, arg3);
> +    }
> +    return var;
> +}
> +
> +/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefOr */
> +Aml *aml_or(Aml *arg1, Aml *arg2, Aml *arg3)
same here for arg3

> +{
> +    Aml *var = aml_opcode(0x7D /* OrOp */);
> +    aml_append(var, arg1);
> +    aml_append(var, arg2);
> +    if (arg3 == NULL) {
> +        build_append_byte(var->buf, 0x00 /* NullNameOp */);
> +    } else {
> +        aml_append(var, arg3);
> +    }
>      return var;
>  }
>  
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 7b5210e..133685e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -452,7 +452,7 @@ static void build_append_pcihp_notify_entry(Aml *method, int slot)
>      Aml *if_ctx;
>      int32_t devfn = PCI_DEVFN(slot, 0);
>  
> -    if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot)));
> +    if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot), NULL));
>      aml_append(if_ctx, aml_notify(aml_name("S%.02X", devfn), aml_arg(1)));
>      aml_append(method, if_ctx);
>  }
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 942d986..3473d6e 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -156,7 +156,8 @@ Aml *aml_return(Aml *val);
>  Aml *aml_int(const uint64_t val);
>  Aml *aml_arg(int pos);
>  Aml *aml_store(Aml *val, Aml *target);
> -Aml *aml_and(Aml *arg1, Aml *arg2);
> +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3);
> +Aml *aml_or(Aml *arg1, Aml *arg2, Aml *arg3);
>  Aml *aml_notify(Aml *arg1, Aml *arg2);
>  Aml *aml_call1(const char *method, Aml *arg1);
>  Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
> @@ -211,6 +212,7 @@ Aml *aml_field(const char *name, AmlFieldFlags flags);
>  Aml *aml_varpackage(uint32_t num_elements);
>  Aml *aml_touuid(int32_t val1, int16_t val2, int16_t val3,
>                  int16_t val4, int64_t val5);
> +void build_append_int_noprefix(GArray *table, uint64_t value, int size);
>  
>  void
>  build_header(GArray *linker, GArray *table_data,
Shannon Zhao April 10, 2015, 6:15 a.m. UTC | #2
On 2015/4/9 21:35, Igor Mammedov wrote:
> On Fri, 3 Apr 2015 18:03:46 +0800
> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> 
>> > From: Shannon Zhao <shannon.zhao@linaro.org>
>> > 
>> > Add aml_or() term and make aml_and can take three args.
>> > Expose build_append_int_noprefix as it wiil be used by
>> > creating a buffer.
>> > 
>> > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> > ---
>> >  hw/acpi/aml-build.c         | 24 +++++++++++++++++++++---
>> >  hw/i386/acpi-build.c        |  2 +-
>> >  include/hw/acpi/aml-build.h |  4 +++-
>> >  3 files changed, 25 insertions(+), 5 deletions(-)
>> > 
>> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> > index 5a94fc9..312afb6 100644
>> > --- a/hw/acpi/aml-build.c
>> > +++ b/hw/acpi/aml-build.c
>> > @@ -240,7 +240,7 @@ static void build_extop_package(GArray *package, uint8_t op)
>> >      build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
>> >  }
>> >  
>> > -static void build_append_int_noprefix(GArray *table, uint64_t value, int size)
>> > +void build_append_int_noprefix(GArray *table, uint64_t value, int size)
>> >  {
>> >      int i;
>> >  
>> > @@ -445,12 +445,30 @@ Aml *aml_store(Aml *val, Aml *target)
>> >  }
>> >  
>> >  /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAnd */
>> > -Aml *aml_and(Aml *arg1, Aml *arg2)
>> > +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3)
> I know that it's possible to Store inside of And(a, b, save_here) ASL op,
> but could you instead rewrite it to
> 
>  Store(And(a, b), save_here)
> 
> so it wouldn't clatter trivial  And(a,b) uses and drop this hunk.
> 
Yes, we can use Store(And(a, b), save_here) but according to the SPEC
the And op can have 3 args. We don't support it?
Igor Mammedov April 10, 2015, 7:46 a.m. UTC | #3
On Fri, 10 Apr 2015 14:15:32 +0800
Shannon Zhao <zhaoshenglong@huawei.com> wrote:

> On 2015/4/9 21:35, Igor Mammedov wrote:
> > On Fri, 3 Apr 2015 18:03:46 +0800
> > Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> > 
> >> > From: Shannon Zhao <shannon.zhao@linaro.org>
> >> > 
> >> > Add aml_or() term and make aml_and can take three args.
> >> > Expose build_append_int_noprefix as it wiil be used by
> >> > creating a buffer.
> >> > 
> >> > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >> > ---
> >> >  hw/acpi/aml-build.c         | 24 +++++++++++++++++++++---
> >> >  hw/i386/acpi-build.c        |  2 +-
> >> >  include/hw/acpi/aml-build.h |  4 +++-
> >> >  3 files changed, 25 insertions(+), 5 deletions(-)
> >> > 
> >> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> >> > index 5a94fc9..312afb6 100644
> >> > --- a/hw/acpi/aml-build.c
> >> > +++ b/hw/acpi/aml-build.c
> >> > @@ -240,7 +240,7 @@ static void build_extop_package(GArray
> >> > *package, uint8_t op) build_prepend_byte(package, 0x5B); /*
> >> > ExtOpPrefix */ }
> >> >  
> >> > -static void build_append_int_noprefix(GArray *table, uint64_t
> >> > value, int size) +void build_append_int_noprefix(GArray *table,
> >> > uint64_t value, int size) {
> >> >      int i;
> >> >  
> >> > @@ -445,12 +445,30 @@ Aml *aml_store(Aml *val, Aml *target)
> >> >  }
> >> >  
> >> >  /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAnd */
> >> > -Aml *aml_and(Aml *arg1, Aml *arg2)
> >> > +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3)
> > I know that it's possible to Store inside of And(a, b, save_here)
> > ASL op, but could you instead rewrite it to
> > 
> >  Store(And(a, b), save_here)
> > 
> > so it wouldn't clatter trivial  And(a,b) uses and drop this hunk.
> > 
> Yes, we can use Store(And(a, b), save_here) but according to the SPEC
> the And op can have 3 args. We don't support it?
I don't think that we should do it if it could be implemented
using 2 already existing API calls to keep it simple and not to
pollute code with extra ", NULL" argument in most cases.
Shannon Zhao April 10, 2015, 8:04 a.m. UTC | #4
On 2015/4/10 15:46, Igor Mammedov wrote:
> On Fri, 10 Apr 2015 14:15:32 +0800
> Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> 
>> > On 2015/4/9 21:35, Igor Mammedov wrote:
>>> > > On Fri, 3 Apr 2015 18:03:46 +0800
>>> > > Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>>> > > 
>>>>> > >> > From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>> > >> > 
>>>>> > >> > Add aml_or() term and make aml_and can take three args.
>>>>> > >> > Expose build_append_int_noprefix as it wiil be used by
>>>>> > >> > creating a buffer.
>>>>> > >> > 
>>>>> > >> > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>>>> > >> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>> > >> > ---
>>>>> > >> >  hw/acpi/aml-build.c         | 24 +++++++++++++++++++++---
>>>>> > >> >  hw/i386/acpi-build.c        |  2 +-
>>>>> > >> >  include/hw/acpi/aml-build.h |  4 +++-
>>>>> > >> >  3 files changed, 25 insertions(+), 5 deletions(-)
>>>>> > >> > 
>>>>> > >> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>>>>> > >> > index 5a94fc9..312afb6 100644
>>>>> > >> > --- a/hw/acpi/aml-build.c
>>>>> > >> > +++ b/hw/acpi/aml-build.c
>>>>> > >> > @@ -240,7 +240,7 @@ static void build_extop_package(GArray
>>>>> > >> > *package, uint8_t op) build_prepend_byte(package, 0x5B); /*
>>>>> > >> > ExtOpPrefix */ }
>>>>> > >> >  
>>>>> > >> > -static void build_append_int_noprefix(GArray *table, uint64_t
>>>>> > >> > value, int size) +void build_append_int_noprefix(GArray *table,
>>>>> > >> > uint64_t value, int size) {
>>>>> > >> >      int i;
>>>>> > >> >  
>>>>> > >> > @@ -445,12 +445,30 @@ Aml *aml_store(Aml *val, Aml *target)
>>>>> > >> >  }
>>>>> > >> >  
>>>>> > >> >  /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAnd */
>>>>> > >> > -Aml *aml_and(Aml *arg1, Aml *arg2)
>>>>> > >> > +Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3)
>>> > > I know that it's possible to Store inside of And(a, b, save_here)
>>> > > ASL op, but could you instead rewrite it to
>>> > > 
>>> > >  Store(And(a, b), save_here)
>>> > > 
>>> > > so it wouldn't clatter trivial  And(a,b) uses and drop this hunk.
>>> > > 
>> > Yes, we can use Store(And(a, b), save_here) but according to the SPEC
>> > the And op can have 3 args. We don't support it?
> I don't think that we should do it if it could be implemented
> using 2 already existing API calls to keep it simple and not to
> pollute code with extra ", NULL" argument in most cases.

Ok, will replace that.
diff mbox

Patch

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 5a94fc9..312afb6 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -240,7 +240,7 @@  static void build_extop_package(GArray *package, uint8_t op)
     build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
 }
 
-static void build_append_int_noprefix(GArray *table, uint64_t value, int size)
+void build_append_int_noprefix(GArray *table, uint64_t value, int size)
 {
     int i;
 
@@ -445,12 +445,30 @@  Aml *aml_store(Aml *val, Aml *target)
 }
 
 /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAnd */
-Aml *aml_and(Aml *arg1, Aml *arg2)
+Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3)
 {
     Aml *var = aml_opcode(0x7B /* AndOp */);
     aml_append(var, arg1);
     aml_append(var, arg2);
-    build_append_byte(var->buf, 0x00 /* NullNameOp */);
+    if (arg3 == NULL) {
+        build_append_byte(var->buf, 0x00 /* NullNameOp */);
+    } else {
+        aml_append(var, arg3);
+    }
+    return var;
+}
+
+/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefOr */
+Aml *aml_or(Aml *arg1, Aml *arg2, Aml *arg3)
+{
+    Aml *var = aml_opcode(0x7D /* OrOp */);
+    aml_append(var, arg1);
+    aml_append(var, arg2);
+    if (arg3 == NULL) {
+        build_append_byte(var->buf, 0x00 /* NullNameOp */);
+    } else {
+        aml_append(var, arg3);
+    }
     return var;
 }
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 7b5210e..133685e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -452,7 +452,7 @@  static void build_append_pcihp_notify_entry(Aml *method, int slot)
     Aml *if_ctx;
     int32_t devfn = PCI_DEVFN(slot, 0);
 
-    if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot)));
+    if_ctx = aml_if(aml_and(aml_arg(0), aml_int(0x1U << slot), NULL));
     aml_append(if_ctx, aml_notify(aml_name("S%.02X", devfn), aml_arg(1)));
     aml_append(method, if_ctx);
 }
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 942d986..3473d6e 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -156,7 +156,8 @@  Aml *aml_return(Aml *val);
 Aml *aml_int(const uint64_t val);
 Aml *aml_arg(int pos);
 Aml *aml_store(Aml *val, Aml *target);
-Aml *aml_and(Aml *arg1, Aml *arg2);
+Aml *aml_and(Aml *arg1, Aml *arg2, Aml *arg3);
+Aml *aml_or(Aml *arg1, Aml *arg2, Aml *arg3);
 Aml *aml_notify(Aml *arg1, Aml *arg2);
 Aml *aml_call1(const char *method, Aml *arg1);
 Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
@@ -211,6 +212,7 @@  Aml *aml_field(const char *name, AmlFieldFlags flags);
 Aml *aml_varpackage(uint32_t num_elements);
 Aml *aml_touuid(int32_t val1, int16_t val2, int16_t val3,
                 int16_t val4, int64_t val5);
+void build_append_int_noprefix(GArray *table, uint64_t value, int size);
 
 void
 build_header(GArray *linker, GArray *table_data,