On Thu, 10 Dec 2015 09:59:16 +0800
Shannon Zhao <zhaoshengl...@huawei.com> wrote:
>
>
> On 2015/12/10 7:41, Igor Mammedov wrote:
> > Currently AML API doesn't compose terms in form of
> > following pattern:
> >
> > Opcode Arg2 Arg2 [Dst]
> >
> > but ASL used in piix4/q35 DSDT ACPI tables uses that
> > form, so for clean conversion of it, AML API should
> > be able to handle an optional 'Dst' argumet used there.
> >
> > Since above pattern is used by arithmetic/bit ops,
> > introduce helper that they could reuse.
> > It reduces code duplication in existing 5 aml_foo()
> > functions and also will prevent more duplication
> > when exiting functions are extended to support
> > optional 'Dst' argument.
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > ---
> > hw/acpi/aml-build.c | 61
> > ++++++++++++++++++++++++++++------------------------- 1 file
> > changed, 32 insertions(+), 29 deletions(-)
> >
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index a6e4c54..22015d2 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -436,44 +436,55 @@ Aml *aml_store(Aml *val, Aml *target)
> > return var;
> > }
> >
> > -/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefAnd */
> > -Aml *aml_and(Aml *arg1, Aml *arg2)
> > +/**
> > + * build_opcode_2arg_dst:
> > + * @op: 1-byte opcode
> > + * @arg1: 1st operand
> > + * @arg2: 2nd operand
> > + * @dst: optional target to store to, set to NULL if it's not
> > required
> > + *
> > + * An internal helper to compose AML terms that have
> > + * "Op Operand Operand Target"
> > + * pattern.
> > + *
> > + * Returns: The newly allocated and composed according to patter
> > Aml object.
> > + */
> > +static Aml *
> > +build_opcode_2arg_dst(uint8_t op, Aml *arg1, Aml *arg2, Aml *dst)
> > {
> > - Aml *var = aml_opcode(0x7B /* AndOp */);
> > + Aml *var = aml_opcode(op);
> > aml_append(var, arg1);
> > aml_append(var, arg2);
> > - build_append_byte(var->buf, 0x00 /* NullNameOp */);
> > + if (dst) {
> > + aml_append(var, dst);
> > + } else {
> > + build_append_byte(var->buf, 0x00 /* NullNameOp */);
> > + }
> > return var;
> > }
> >
> This looks good. Maybe you could add a helper like
> build_opcode_2arg(), then aml_lor() and aml_lgreater() could use this.
If it reduces number code size, it could be made on top of this
series.
If AML API patches ('acpi:' prefixed) would have 'Reviewed-by:' we
could merge them first, before the PC:DSTD conversion patches are
reviewed so that AML API could be improved/extendend further without
conflicts.
That would also make series smaller on respin.
