All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: drjones@redhat.com, marcel.a@redhat.com,
	claudio.fontana@huawei.com, qemu-devel@nongnu.org,
	zhaoshenglong@huawei.com
Subject: Re: [Qemu-devel] [PATCH v3 04/52] acpi: factor out ACPI const int packing out of build_append_value()
Date: Tue, 17 Feb 2015 20:53:52 +0100	[thread overview]
Message-ID: <20150217195352.GA8388@redhat.com> (raw)
In-Reply-To: <1423479254-15342-5-git-send-email-imammedo@redhat.com>

On Mon, Feb 09, 2015 at 10:53:26AM +0000, Igor Mammedov wrote:
> it will be reused for adding a plain integer value into AML.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/acpi/aml-build.c  | 19 +++----------------
>  hw/i386/acpi-build.c |  6 +++---
>  2 files changed, 6 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 096f347..67d1371 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -221,24 +221,8 @@ void build_extop_package(GArray *package, uint8_t op)
>  
>  void build_append_value(GArray *table, uint32_t value, int size)
>  {
> -    uint8_t prefix;
>      int i;
>  
> -    switch (size) {
> -    case 1:
> -        prefix = 0x0A; /* BytePrefix */
> -        break;
> -    case 2:
> -        prefix = 0x0B; /* WordPrefix */
> -        break;
> -    case 4:
> -        prefix = 0x0C; /* DWordPrefix */
> -        break;
> -    default:
> -        assert(0);
> -        return;
> -    }
> -    build_append_byte(table, prefix);
>      for (i = 0; i < size; ++i) {
>          build_append_byte(table, value & 0xFF);
>          value = value >> 8;

This really makes this an awkward API,
you must remember to add the prefix.
IMO you want a new name - build_append_value_noprefix maybe -
then call build_append_value_noprefix from build_append_value.

But really, it's best to squash such changes with
where they are used, it's just making review harder:
as you add APIs with no documentation, I can't
see what they do without reading follow up patch
to see how they are used.


> @@ -252,10 +236,13 @@ void build_append_int(GArray *table, uint32_t value)
>      } else if (value == 0x01) {
>          build_append_byte(table, 0x01); /* OneOp */
>      } else if (value <= 0xFF) {
> +        build_append_byte(table, 0x0A); /* BytePrefix */
>          build_append_value(table, value, 1);
>      } else if (value <= 0xFFFF) {
> +        build_append_byte(table, 0x0B); /* WordPrefix */
>          build_append_value(table, value, 2);
>      } else {
> +        build_append_byte(table, 0x0C); /* DWordPrefix */
>          build_append_value(table, value, 4);
>      }
>  }
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 788962e..a1bf450 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -302,14 +302,14 @@ static void build_append_and_cleanup_method(GArray *device, GArray *method)
>  
>  static void build_append_notify_target_ifequal(GArray *method,
>                                                 GArray *target_name,
> -                                               uint32_t value, int size)
> +                                               uint32_t value)
>  {
>      GArray *notify = build_alloc_array();
>      uint8_t op = 0xA0; /* IfOp */
>  
>      build_append_byte(notify, 0x93); /* LEqualOp */
>      build_append_byte(notify, 0x68); /* Arg0Op */
> -    build_append_value(notify, value, size);
> +    build_append_int(notify, value);
>      build_append_byte(notify, 0x86); /* NotifyOp */
>      build_append_array(notify, target_name);
>      build_append_byte(notify, 0x69); /* Arg1Op */
> @@ -578,7 +578,7 @@ build_append_notify_method(GArray *device, const char *name,
>          GArray *target = build_alloc_array();
>          build_append_namestring(target, format, i);
>          assert(i < 256); /* Fits in 1 byte */
> -        build_append_notify_target_ifequal(method, target, i, 1);
> +        build_append_notify_target_ifequal(method, target, i);
>          build_free_array(target);
>      }
>  
> -- 
> 1.8.3.1

  reply	other threads:[~2015-02-17 19:54 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-09 10:53 [Qemu-devel] [PATCH v3 00/52] ACPI refactoring: replace template patching with C AML API Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 01/52] acpi: introduce AML composer aml_append() Igor Mammedov
2015-02-17 16:26   ` Michael S. Tsirkin
2015-02-17 17:50     ` Igor Mammedov
2015-02-17 19:12       ` Michael S. Tsirkin
2015-02-17 19:46   ` Michael S. Tsirkin
2015-02-18  8:50     ` Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 02/52] pc: acpi: use local var for accessing ACPI tables blob in acpi_build() Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 03/52] pc: acpi: make top level ACPI tables blob Aml* Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 04/52] acpi: factor out ACPI const int packing out of build_append_value() Igor Mammedov
2015-02-17 19:53   ` Michael S. Tsirkin [this message]
2015-02-18  9:41     ` Igor Mammedov
2015-02-18 13:51       ` Michael S. Tsirkin
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 05/52] acpi: add aml_def_block() term Igor Mammedov
2015-02-17 16:35   ` Michael S. Tsirkin
2015-02-17 16:47     ` Igor Mammedov
2015-02-17 19:03       ` Michael S. Tsirkin
2015-02-18 10:47         ` Igor Mammedov
2015-02-18 11:36           ` Michael S. Tsirkin
2015-02-17 19:15       ` Michael S. Tsirkin
2015-02-18  9:50         ` Igor Mammedov
2015-02-18 11:35           ` Michael S. Tsirkin
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 06/52] pc: acpi-build: use aml_def_block() for declaring SSDT table Igor Mammedov
2015-02-17 16:42   ` Michael S. Tsirkin
2015-02-18  9:57     ` Igor Mammedov
2015-02-18 11:39       ` Michael S. Tsirkin
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 07/52] acpi: add aml_scope() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 08/52] pc: acpi-build: use aml_scope() for \_SB scope Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 09/52] acpi: add aml_device() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 10/52] acpi: add aml_method() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 11/52] acpi: add aml_if() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 12/52] acpi: add aml_name() & aml_name_decl() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 13/52] acpi: extend build_append_{value|int}() to support 64-bit values Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 14/52] acpi: add aml_int() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 15/52] acpi: add aml_return() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 16/52] acpi: add aml_arg() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 17/52] acpi: add aml_store() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 18/52] acpi: add aml_and() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 19/52] acpi: add aml_notify() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 20/52] acpi: add aml_call1(), aml_call2(), aml_call3(), aml_call4() helpers Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 21/52] pc: acpi-build: drop template patching and create PCI bus tree dynamically Igor Mammedov
2015-02-17 19:39   ` Michael S. Tsirkin
2015-02-18 10:00     ` Igor Mammedov
2015-02-18 10:42     ` Igor Mammedov
2015-02-18 11:38       ` Michael S. Tsirkin
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 22/52] acpi: add aml_package() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 23/52] pc: acpi-build: drop unsupported PM1b_CNT.SLP_TYP Igor Mammedov
2015-02-17 16:41   ` Michael S. Tsirkin
2015-02-18 10:03     ` Igor Mammedov
2015-02-18 12:41       ` Michael S. Tsirkin
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 24/52] pc: acpi-build: generate _S[345] packages dynamically Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 25/52] acpi: add aml_buffer() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 26/52] acpi: add aml_resource_template() helper Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 27/52] acpi: add aml_io() helper Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 28/52] acpi: include PkgLength size only when requested Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 29/52] acpi: add aml_operation_region() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 30/52] acpi: add aml_field() & aml_named_field() terms Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 31/52] acpi: add aml_local() term Igor Mammedov
2015-02-17 12:18   ` Michael S. Tsirkin
2015-02-17 14:06     ` Igor Mammedov
2015-02-17 14:24   ` [Qemu-devel] [PATCH v3 31/52] fixup! " Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 32/52] acpi: add aml_string() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 33/52] pc: acpi-build: generate pvpanic device description dynamically Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 34/52] acpi: add aml_varpackage() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 35/52] acpi: add aml_equal() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 36/52] acpi: add aml_processor() term Igor Mammedov
2015-02-09 10:53 ` [Qemu-devel] [PATCH v3 37/52] acpi: add aml_eisaid() term Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 38/52] pc: acpi-build: drop template patching and CPU hotplug objects dynamically Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 39/52] pc: acpi-build: create CPU hotplug IO region dynamically Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 40/52] acpi: add aml_reserved_field() term Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 41/52] pc: acpi-build: drop template patching and memory hotplug objects dynamically Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 42/52] pc: acpi-build: create memory hotplug IO region dynamically Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 43/52] acpi: add aml_word_bus_number(), aml_word_io(), aml_dword_memory(), aml_qword_memory() terms Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 44/52] pc: pcihp: expose MMIO base and len as properties Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 45/52] pc: acpi-build: reserve PCIHP MMIO resources Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 46/52] pc: acpi-build: create PCI0._CRS dynamically Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 47/52] pc: acpi-build: drop remaining ssdt_misc template and use acpi_def_block() Igor Mammedov
2015-02-17 16:44   ` Michael S. Tsirkin
2015-02-18 10:09     ` Igor Mammedov
2015-02-18 13:13       ` Michael S. Tsirkin
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 48/52] acpi: add acpi_irq_no_flags() term Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 49/52] pc: export applesmc IO port/len Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 50/52] pc: acpi-build: drop template patching and create Device(SMC) dynamically Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 51/52] pc: acpi-build: update [q35-]acpi-dsdt.hex.generated due to moved SMC Igor Mammedov
2015-02-09 10:54 ` [Qemu-devel] [PATCH v3 52/52] acpi: make build_*() routines static to aml-build.c Igor Mammedov
2015-02-17 16:47 ` [Qemu-devel] [PATCH v3 00/52] ACPI refactoring: replace template patching with C AML API Michael S. Tsirkin
2015-02-17 17:51   ` Igor Mammedov
2015-02-17 19:14     ` Michael S. Tsirkin
2015-02-18 10:35       ` Igor Mammedov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150217195352.GA8388@redhat.com \
    --to=mst@redhat.com \
    --cc=claudio.fontana@huawei.com \
    --cc=drjones@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=marcel.a@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=zhaoshenglong@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.