On Sun, Jul 30, 2017 at 6:19 AM, Paulo Alcantara <pcacjr@xxxxxxxxx> wrote:
> The WMI queries are performed by evaluating the WMPV() method from ACPI
> DSDT tables, and it takes three arguments: instance index, method id and
> input data (buffer).
>
> Currently the method id is hard-coded to 0x3 in hp_wmi_perform_query()
> which means that it will perform WMI calls that expect an output data of
> size 0x80 (128). The output size is usually OK for the WMI queries we
> perform, however it would be better to pick the correct one before
> evaluating the WMI method.
>
> Which correct method id to choose can be figured out by looking at the
> following ASL code from WVPI() method:
>
> ...
> Name (PVSZ, Package (0x05)
> {
> Zero,
> 0x04,
> 0x80,
> 0x0400,
> 0x1000
> })
> Store (Zero, Local0)
> If (LAnd (LGreaterEqual (Arg1, One), LLessEqual (Arg1, 0x05)))
> {
> Store (DerefOf (Index (PVSZ, Subtract (Arg1, One))), Local0)
> }
> ...
>
> Arg1 is the method id and PVSZ is the package used to index the
> corresponding output size; 1 -> 0, 2 -> 4, 3 -> 128, 4 -> 1024, 5 ->
> 4096.
>
> This patch maps the output size passed in hp_wmi_perform_query() to the
> correct method id before evaluating the WMI method.
Thanks for the patch, my comments below.
> +/* map output size to the corresponding WMI method id */
> +static inline u32 outsize_to_mid(int outsize)
I would name it like
encode_outsize_for_pvsz
> +{
> + if (!outsize)
> + return 1;
> + if (outsize <= 0x4)
> + return 2;
> + if (outsize <= 0x80)
> + return 3;
> + if (outsize <= 0x400)
> + return 4;
> + if (outsize <= 0x1000)
> + return 5;
> + return (u32)-1;
I would do other way around, i.e.
if (outsize > 4096)
return U32_MAX;
if (outsize > ...)
return ...;
if (outsize > 0)
return 2;
return 1;
Note two things:
1) use decimal integers for size (slightly better to read);
2) don't trick negative numbers.
> +}
> + mid = outsize_to_mid(outsize);
> + if (WARN_ON(mid == (u32)-1))
And looking here, why you don't return int from that function directly
and use it's negative value as error code?
> + return -EINVAL;
> +
--
With Best Regards,
Andy Shevchenko
