Re: [Qemu-devel] [PATCH v2 12/47] acpi: add acpi_store() term

[Marcel Apfelbaum]

On 01/22/2015 04:49 PM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
>   hw/acpi/acpi-build-utils.c         | 10 ++++++++++
>   include/hw/acpi/acpi-build-utils.h |  1 +
>   2 files changed, 11 insertions(+)
>
> diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c
> index 6e10712..d78d34f 100644
> --- a/hw/acpi/acpi-build-utils.c
> +++ b/hw/acpi/acpi-build-utils.c
> @@ -380,6 +380,16 @@ AcpiAml acpi_arg3(void)
>       return var;
>   }
>
> +/* ACPI 5.0: 20.2.5.4 Type 2 Opcodes Encoding: DefStore */
> +AcpiAml acpi_store(AcpiAml val, AcpiAml target)
> +{
> +    AcpiAml var = aml_allocate_internal(0, NON_BLOCK);
> +    build_append_byte(var.buf, 0x70); /* StoreOp */
> +    aml_append(&var, val);
> +    aml_append(&var, target);
> +    return var;
> +}

The same comment here:
There are other ACPI constructs exactly like the above, except
the operation number (0x70).
Maybe we can have a generic internal function and the API
will call it.
For example OrOp and AddOp.

Instead of rewriting the same code, I propose (for all cases)
to create an internal function, for this example:
 - AcpiAml acpi_2_args_op(AcpiAml arg1, AcpiAml arg2, int op)
And have a wrapper for each op.

A better name will be welcomed, maybe it can be extracted from the APCI spec.

Thanks,
Marcel


> +
>   /* ACPI 5.0: 20.2.5.3 Type 1 Opcodes Encoding: DefIfElse */
>   AcpiAml acpi_if(AcpiAml predicate)
>   {
> diff --git a/include/hw/acpi/acpi-build-utils.h 
> b/include/hw/acpi/acpi-build-utils.h
> index 18d9efa..5d3651d 100644
> --- a/include/hw/acpi/acpi-build-utils.h
> +++ b/include/hw/acpi/acpi-build-utils.h
> @@ -30,6 +30,7 @@ AcpiAml acpi_arg0(void);
>   AcpiAml acpi_arg1(void);
>   AcpiAml acpi_arg2(void);
>   AcpiAml acpi_arg3(void);
> +AcpiAml acpi_store(AcpiAml val, AcpiAml target);
>
>   /* Block ASL object primitives */
>   AcpiAml acpi_if(AcpiAml predicate);