On Mon, Mar 09, 2015 at 01:26:40PM +0100, Igor Mammedov wrote:
> On Mon, 9 Mar 2015 12:04:51 +0100
> "Michael S. Tsirkin" <m...@redhat.com> wrote:
>
> > On Mon, Mar 09, 2015 at 11:28:22AM +0100, Igor Mammedov wrote:
> > > On Sun, 8 Mar 2015 13:16:03 +0200
> > > Marcel Apfelbaum <mar...@redhat.com> wrote:
> > >
> > > > The DefLEqual op does not have a target operand. Remove it.
> > > >
> > > > Signed-off-by: Marcel Apfelbaum <mar...@redhat.com>
> > > Reviewed-by: Igor Mammedov <imamm...@redhat.com>
> > >
> > > > ---
> > > > hw/acpi/aml-build.c | 1 -
> > > > 1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > > index 876cada..0d14561 100644
> > > > --- a/hw/acpi/aml-build.c
> > > > +++ b/hw/acpi/aml-build.c
> > > > @@ -542,7 +542,6 @@ Aml *aml_equal(Aml *arg1, Aml *arg2)
> > > > Aml *var = aml_opcode(0x93 /* LequalOp */);
> > > > aml_append(var, arg1);
> > > > aml_append(var, arg2);
> > > > - build_append_int(var->buf, 0x00); /* NullNameOp */
> > > It's just happens to work in case CPU and PCI hotplug because
> > > it LEqual was the only predicate in if block and NullNameOp
> > > was considered as part of inner code-block, like:
> > > if (LEqual(a, b)) {
> > > NullName; // nop
> > > ...
> > > }
> >
> > So - maybe aml_if should get 3rd parameter - the command?
> it's not only one command it's block of AML code inside of 'if' scope.
> Adding 3rd argument would mean inventing another not defined by spec element
> like aml_block() where you could put AML items that are in block,
> I'd like to keep non spec items to minimum and not add them unless we have to.
In fact, it's TermList in spec, isn't it?
But I don't insist.
>
> Then for consistence purposes we would add this 'aml_block' argument
> to other block constructs like 'device, scope, package, ...'
> So I think current way of defining context and then putting items in it
> is pretty clean way as opposed to doing it backwards, first defining
> elements somewhere and then passing that somewhere as argument to
> a AML block construct.
I think it's just a question of adding a convenience wrapper
for the most common case.
/* Convenience wrapper for when there's a single
* term in TermList
*/
Aml *aml_if_then_1term(Aml *predicate, Aml *term)
{
Aml *if_ctx = aml_if(predicate);
aml_append(if_ctx, term);
return if_ctx;
}
