| Message ID | 1446184587-142784-7-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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) { 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)); + 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);
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(-)