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

[Heyi Guo]

Got it; thanks.

Heyi

On 2019/3/7 0:20, Igor Mammedov wrote:
> On Wed, 6 Mar 2019 21:09:11 +0800
> Heyi Guo <address@hidden> wrote:
>
> > 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?
> See "[PATCH v6 0/2] CODING_STYLE: trivial update" for clarification
>
> > 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);
> > > .
>
> .