On 10/19/2015 03:47 PM, Michael S. Tsirkin wrote:
aml_arg(3) is used many times below.
Pls give it a name that makes sense (not arg3! what is it for?)
Er. aml_arg(3) is just the fourth parameter of _DSM method. Will add some
comments:
/*
* The fourth parameter (Arg3) of _DMS is a package which contains a buffer,
the
* layout of the buffer is specified by UUID (Arg0), Revision ID (Arg1) and
* Function Index (Arg2) which are documented in the DSM specification.
*/
+ aml_int(1))));
Pls document AML constants used.
Like this:
ifctx = aml_if(aml_and(aml_equal(aml_object_type(aml_arg(3)),
aml_int(4 /* 4 - Package */) ),
aml_equal(aml_sizeof(aml_arg(3)),
aml_int(1))));
+ {
+ /* Local0 = Index(Arg3, 0) */
+ aml_append(ifctx, aml_store(aml_index(aml_arg(3), aml_int(0)),
+ aml_local(0)));
+ /* Local3 = DeRefOf(Local0) */
+ aml_append(ifctx, aml_store(aml_derefof(aml_local(0)),
+ aml_local(3)));
+ /* ARG3 = Local3 */
+ aml_append(ifctx, aml_store(aml_local(3), aml_name("ARG3")));
This isn't a good way to comment things: you are
just adding ASL before the equivalent C.
Pls document what's going on.
Okay... i just thought C is little readable than AML. Will change the comment
to:
/* fetch buffer from the package (Arg3) and store it to DSM memory. */
Thanks.
You can use variables to make the logic clear. E.g.:
Aml *pckg = aml_arg(3);
Aml *pckg_idx = aml_local(0);
Aml *pckg_buf = aml_local(3);
aml_append(ifctx, aml_store(aml_index(pckg, aml_int(0)), pckg_idx);
aml_append(ifctx, aml_store(aml_derefof(pckg_idx), pckg_buf));
This is also better than repeating aml_arg(3) many times.
Indeed, it's more clearer now.
Thanks for your review and really appreciate for your patience, Michael!
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
