Re: [PATCH 3/9] platform/x86: hp-wmi: Standardize enum usage for constants

[Darren Hart]

On Wed, Apr 19, 2017 at 07:25:15PM -0700, Darren Hart wrote:
> From: "Darren Hart (VMware)" <dvhart@xxxxxxxxxxxxx>
> 
> Use enums consistently throughout the hp-wmi driver for groups of
> related constants. Use hex and align the assignment within groups. Move
> the *QUERY constants into an enum, create a new enum defining the READ,
> WRITE, and ODM constants and use them instead of 0 and 1 at the call
> sites.
> 
> Signed-off-by: Darren Hart (VMware) <dvhart@xxxxxxxxxxxxx>
> ---
>  drivers/platform/x86/hp-wmi.c | 119 +++++++++++++++++++++++-------------------
>  1 file changed, 64 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index aa9d99c..60d1e4c 100644
> --- a/drivers/platform/x86/hp-wmi.c
> +++ b/drivers/platform/x86/hp-wmi.c

...

> +enum hp_wmi_command {
> +	HPWMI_READ	= 0x00,
> +	HPWMI_WRITE	= 0x01,
> +	HPWMI_ODM	= 0x03,
> +};
> +
>  #define BIOS_ARGS_INIT(write, ctype, size)				\
>  	(struct bios_args)	{	.signature = 0x55434553,	\
>  					.command = (write) ? 0x2 : 0x1,	\
> @@ -177,8 +186,8 @@ static struct rfkill2_device rfkill2[HPWMI_MAX_RFKILL2_DEVICES];
>  /*
>   * hp_wmi_perform_query
>   *
> - * query:	The commandtype -> What should be queried
> - * write:	The command -> 0 read, 1 write, 3 ODM specific
> + * query:	The commandtype (enum hp_wmi_commandtype)
> + * write:	The command (enum hp_wmi_command)

Eeek, I failed to use the enum directly and was still using the ternary operator
in the BIOS_ARGS_INIT above. Luckily, with read as 0 and write as non-zero in
the new ENUMs, and no usage of ODM, things happened to just work. Updated patch
below.

I've now:

* dropped the BIOS_ARGS_INIT per Andy's recommendation.
* Renumbered the hp_wmi_command to match the ternary assignment:
  HPWMI_READ = 0x01,
  HPWMI_WRITE = 0x02,
* Assigned bios_args.command with one of the enums directly
* replaced the "int write" argument with "enum hp_wmi_command command"

As follows:

Queued to testing, please let me know if you have any concerns.