[v6,06/33] acpi: add aml_method_serialized

It avoid explicit Mutex and will be used by NVDIMM ACPI

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 hw/acpi/aml-build.c         | 26 ++++++++++++++++++++++++--
 include/hw/acpi/aml-build.h |  1 +
 2 files changed, 25 insertions(+), 2 deletions(-)

On Fri, Oct 30, 2015 at 01:56:00PM +0800, Xiao Guangrong wrote:
> It avoid explicit Mutex and will be used by NVDIMM ACPI
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>

I'd rather you squashed these utility patches in with where
the code is used. This is just making it harder to review
as I have to jump back and forth.

> ---
>  hw/acpi/aml-build.c         | 26 ++++++++++++++++++++++++--
>  include/hw/acpi/aml-build.h |  1 +
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 9f792ab..8bee8b2 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -696,14 +696,36 @@ Aml *aml_while(Aml *predicate)
>  }
>  
>  /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefMethod */
> -Aml *aml_method(const char *name, int arg_count)
> +static Aml *__aml_method(const char *name, int arg_count, bool serialized)

Please don't prefix names with __.
what should you call this?
For example, you can call it aml_method_serialized.

>  {
>      Aml *var = aml_bundle(0x14 /* MethodOp */, AML_PACKAGE);
> +    int methodflags;
> +
> +    /*
> +     * MethodFlags:
> +     *   bit 0-2: ArgCount (0-7)
> +     *   bit 3: SerializeFlag
> +     *     0: NotSerialized
> +     *     1: Serialized
> +     *   bit 4-7: reserved (must be 0)
> +     */
> +    assert(!(arg_count & ~7));

Or shorter assert(arg_count < 8);

> +    methodflags = arg_count | (serialized << 3);
>      build_append_namestring(var->buf, "%s", name);
> -    build_append_byte(var->buf, arg_count); /* MethodFlags: ArgCount */
> +    build_append_byte(var->buf, methodflags);
>      return var;
>  }
>  
> +Aml *aml_method(const char *name, int arg_count)
> +{
> +    return __aml_method(name, arg_count, false);
> +}
> +
> +Aml *aml_method_serialized(const char *name, int arg_count)
> +{
> +    return __aml_method(name, arg_count, true);
> +}
> +
>  /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefDevice */
>  Aml *aml_device(const char *name_format, ...)
>  {
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 5b8a118..00cf40e 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -263,6 +263,7 @@ Aml *aml_qword_memory(AmlDecode dec, AmlMinFixed min_fixed,
>  Aml *aml_scope(const char *name_format, ...) GCC_FMT_ATTR(1, 2);
>  Aml *aml_device(const char *name_format, ...) GCC_FMT_ATTR(1, 2);
>  Aml *aml_method(const char *name, int arg_count);
> +Aml *aml_method_serialized(const char *name, int arg_count);
>  Aml *aml_if(Aml *predicate);
>  Aml *aml_else(void);
>  Aml *aml_while(Aml *predicate);
> -- 
> 1.8.3.1

On 11/09/2015 07:14 PM, Michael S. Tsirkin wrote:
> On Fri, Oct 30, 2015 at 01:56:00PM +0800, Xiao Guangrong wrote:
>> It avoid explicit Mutex and will be used by NVDIMM ACPI
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>
> I'd rather you squashed these utility patches in with where
> the code is used. This is just making it harder to review
> as I have to jump back and forth.

Okay to me.

>
>> ---
>>   hw/acpi/aml-build.c         | 26 ++++++++++++++++++++++++--
>>   include/hw/acpi/aml-build.h |  1 +
>>   2 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index 9f792ab..8bee8b2 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -696,14 +696,36 @@ Aml *aml_while(Aml *predicate)
>>   }
>>
>>   /* ACPI 1.0b: 16.2.5.2 Named Objects Encoding: DefMethod */
>> -Aml *aml_method(const char *name, int arg_count)
>> +static Aml *__aml_method(const char *name, int arg_count, bool serialized)
>
> Please don't prefix names with __.
> what should you call this?
> For example, you can call it aml_method_serialized.

Igor disliked that aml_method_serialized() is spepated from aml_method(), so i
will unify them as a single function instead. :)

>
>>   {
>>       Aml *var = aml_bundle(0x14 /* MethodOp */, AML_PACKAGE);
>> +    int methodflags;
>> +
>> +    /*
>> +     * MethodFlags:
>> +     *   bit 0-2: ArgCount (0-7)
>> +     *   bit 3: SerializeFlag
>> +     *     0: NotSerialized
>> +     *     1: Serialized
>> +     *   bit 4-7: reserved (must be 0)
>> +     */
>> +    assert(!(arg_count & ~7));
>
> Or shorter assert(arg_count < 8);

Good to me.