Re: [Qemu-arm] [Qemu-devel] [PATCH v3 1/2] hw/arm/acpi: simplify AML bit and/or statement

[Heyi Guo]

Sorry, I didn't know the indention policy of qemu code. So we need to indent 
the 2nd line just after the parentheses it belongs to?

Will change that and send next version.

Thanks,

Heyi


On 2019/3/6 18:33, Igor Mammedov wrote:
> On Wed, 6 Mar 2019 06:03:48 +0800
> Heyi Guo <address@hidden> wrote:
>
> > The last argument of AML bit and/or statement is the target variable,
> > so we don't need to use a NULL target and then an additional store
> > operation; a single bit and/or statement is enough.
> >
> > Cc: Shannon Zhao <address@hidden>
> > Cc: Peter Maydell <address@hidden>
> > Cc: "Michael S. Tsirkin" <address@hidden>
> > Cc: Igor Mammedov <address@hidden>
> > Suggested-by: Igor Mammedov <address@hidden>
> > Signed-off-by: Heyi Guo <address@hidden>
> > ---
> >   hw/arm/virt-acpi-build.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 04b62c7..1c84e87 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -265,16 +265,16 @@ static void acpi_dsdt_add_pci(Aml *scope, const 
> > MemMapEntry *memmap,
> >           aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3"));
> >       aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP")));
> >       aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL")));
> > -    aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL),
> > +    aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
> >                                   aml_name("CTRL")));
> pls fix indent in all such cases. I'd align it like this:
>     aml_append(ifctx,
>                aml_and(aml_name("CTRL"), aml_int(0x1D), aml_name("CTRL")));
>
> or
>     aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D),
>                               aml_name("CTRL")));
>
>
> PS:
> When making multi-patch series use --cover-letter git option
>
> >   
> >       ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1))));
> > -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL),
> > +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08),
> >                                    aml_name("CDW1")));
> >       aml_append(ifctx, ifctx1);
> >   
> >       ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL"))));
> > -    aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL),
> > +    aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10),
> >                                    aml_name("CDW1")));
> >       aml_append(ifctx, ifctx1);
> >   
> > @@ -283,7 +283,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> >       aml_append(method, ifctx);
> >   
> >       elsectx = aml_else();
> > -    aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL),
> > +    aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4),
> >                                     aml_name("CDW1")));
> >       aml_append(elsectx, aml_return(aml_arg(3)));
> >       aml_append(method, elsectx);
>
> .