Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Robert Hoo]

On Fri, 2022-09-09 at 15:39 +0200, Igor Mammedov wrote:
...
> looks more or less fine except of excessive use of named variables
> which creates global scope variables.
> 
> I'd suggest to store temporary buffers/packages in LocalX variales,
> you should be able to do that for everything modulo
> aml_create_dword_field().
> 
> see an example below
> 
...
> >  
> > +        /*
> > +         * ACPI v6.4: Section 6.5.10 NVDIMM Label Methods
> > +         */
> > +        /* _LSI */
> > +        method = aml_method("_LSI", 0, AML_SERIALIZED);
> > +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> > +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> > +                            aml_int(1), aml_int(4), aml_int(0),
> > +                            aml_int(handle));
> > +        aml_append(method, aml_store(com_call, aml_local(0)));
> > +
> > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > +                                                  aml_int(0),
> > "STTS"));
> > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > aml_int(4),
> > +                                                  "SLSA"));
> > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > aml_int(8),
> > +                                                  "MAXT"));
> > +
> > +        pkg = aml_package(3);
> > +        aml_append(pkg, aml_name("STTS"));
> > +        aml_append(pkg, aml_name("SLSA"));
> > +        aml_append(pkg, aml_name("MAXT"));
> > +        aml_append(method, aml_name_decl("RET", pkg));
> 
> ex: put it in local instead of named variable and return that
> the same applies to other named temporary named variables.
> 
Fine, get your point now.
In ASL it will look like this:
                    Local1 = Package (0x3) {STTS, SLSA, MAXT}
                    Return (Local1)

But as for 
                    CreateDWordField (Local0, Zero, STTS)  // Status
                    CreateDWordField (Local0, 0x04, SLSA)  // SizeofLSA
                    CreateDWordField (Local0, 0x08, MAXT)  // Max Trans

I cannot figure out how to substitute with LocalX. Can you shed more
light?

CreateQWordFieldTerm :=
CreateQWordField (
SourceBuffer, // TermArg => Buffer
ByteIndex, // TermArg => Integer
QWordFieldName // NameString
)
NameString :=
<RootChar NamePath> | <ParentPrefixChar PrefixPath NamePath> |
NonEmptyNamePath

> > +        aml_append(method, aml_return(aml_name("RET")));
> > +
...
> > +        field = aml_create_dword_field(aml_local(3), aml_int(0),
> > "STTS");
> > +        aml_append(method, field);
> > +        aml_append(method,
> > aml_return(aml_to_integer(aml_name("STTS"))));
> 
> why do you need explicitly convert DWORD field to integer?
> it should be fine to return STTS directly (implicit conversion should
> take care of the rest)

Explicit convert eases my anxiety on uncertainty. ;)

> 
> > +        aml_append(nvdimm_dev, method);
> > +
...