Re: [PATCH v4 3/4] platform/x86: ideapad-laptop: move ACPI helpers from header to source file

[Hans de Goede <hdegoede@xxxxxxxxxx>]

Hi,

On 7/25/24 11:21 AM, Gergo Koteles wrote:
> Since moving ymc_trigger_ec from lenovo-ymc to ideapad-laptop, only the
> latter uses these definitions and functions.
> 
> Move them back to source file.
> 
> Signed-off-by: Gergo Koteles <soyer@xxxxxx>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans




> ---
>  drivers/platform/x86/ideapad-laptop.c | 136 +++++++++++++++++++++++++
>  drivers/platform/x86/ideapad-laptop.h | 139 --------------------------
>  2 files changed, 136 insertions(+), 139 deletions(-)
> 
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index 9fc1bb990e47..8398774cdfe2 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -22,6 +22,7 @@
>  #include <linux/init.h>
>  #include <linux/input.h>
>  #include <linux/input/sparse-keymap.h>
> +#include <linux/jiffies.h>
>  #include <linux/kernel.h>
>  #include <linux/leds.h>
>  #include <linux/module.h>
> @@ -87,6 +88,34 @@ enum {
>  	SALS_FNLOCK_OFF       = 0xf,
>  };
>  
> +enum {
> +	VPCCMD_R_VPC1 = 0x10,
> +	VPCCMD_R_BL_MAX,
> +	VPCCMD_R_BL,
> +	VPCCMD_W_BL,
> +	VPCCMD_R_WIFI,
> +	VPCCMD_W_WIFI,
> +	VPCCMD_R_BT,
> +	VPCCMD_W_BT,
> +	VPCCMD_R_BL_POWER,
> +	VPCCMD_R_NOVO,
> +	VPCCMD_R_VPC2,
> +	VPCCMD_R_TOUCHPAD,
> +	VPCCMD_W_TOUCHPAD,
> +	VPCCMD_R_CAMERA,
> +	VPCCMD_W_CAMERA,
> +	VPCCMD_R_3G,
> +	VPCCMD_W_3G,
> +	VPCCMD_R_ODD, /* 0x21 */
> +	VPCCMD_W_FAN,
> +	VPCCMD_R_RF,
> +	VPCCMD_W_RF,
> +	VPCCMD_W_YMC = 0x2A,
> +	VPCCMD_R_FAN = 0x2B,
> +	VPCCMD_R_SPECIAL_BUTTONS = 0x31,
> +	VPCCMD_W_BL_POWER = 0x33,
> +};
> +
>  /*
>   * These correspond to the number of supported states - 1
>   * Future keyboard types may need a new system, if there's a collision
> @@ -236,6 +265,7 @@ static void ideapad_shared_exit(struct ideapad_private *priv)
>  /*
>   * ACPI Helpers
>   */
> +#define IDEAPAD_EC_TIMEOUT 200 /* in ms */
>  
>  static int eval_int(acpi_handle handle, const char *name, unsigned long *res)
>  {
> @@ -251,6 +281,29 @@ static int eval_int(acpi_handle handle, const char *name, unsigned long *res)
>  	return 0;
>  }
>  
> +static int eval_int_with_arg(acpi_handle handle, const char *name, unsigned long arg,
> +			     unsigned long *res)
> +{
> +	struct acpi_object_list params;
> +	unsigned long long result;
> +	union acpi_object in_obj;
> +	acpi_status status;
> +
> +	params.count = 1;
> +	params.pointer = &in_obj;
> +	in_obj.type = ACPI_TYPE_INTEGER;
> +	in_obj.integer.value = arg;
> +
> +	status = acpi_evaluate_integer(handle, (char *)name, &params, &result);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	if (res)
> +		*res = result;
> +
> +	return 0;
> +}
> +
>  static int exec_simple_method(acpi_handle handle, const char *name, unsigned long arg)
>  {
>  	acpi_status status = acpi_execute_simple_method(handle, (char *)name, arg);
> @@ -293,6 +346,89 @@ static int eval_dytc(acpi_handle handle, unsigned long cmd, unsigned long *res)
>  	return eval_int_with_arg(handle, "DYTC", cmd, res);
>  }
>  
> +static int eval_vpcr(acpi_handle handle, unsigned long cmd, unsigned long *res)
> +{
> +	return eval_int_with_arg(handle, "VPCR", cmd, res);
> +}
> +
> +static int eval_vpcw(acpi_handle handle, unsigned long cmd, unsigned long data)
> +{
> +	struct acpi_object_list params;
> +	union acpi_object in_obj[2];
> +	acpi_status status;
> +
> +	params.count = 2;
> +	params.pointer = in_obj;
> +	in_obj[0].type = ACPI_TYPE_INTEGER;
> +	in_obj[0].integer.value = cmd;
> +	in_obj[1].type = ACPI_TYPE_INTEGER;
> +	in_obj[1].integer.value = data;
> +
> +	status = acpi_evaluate_object(handle, "VPCW", &params, NULL);
> +	if (ACPI_FAILURE(status))
> +		return -EIO;
> +
> +	return 0;
> +}
> +
> +static int read_ec_data(acpi_handle handle, unsigned long cmd, unsigned long *data)
> +{
> +	unsigned long end_jiffies, val;
> +	int err;
> +
> +	err = eval_vpcw(handle, 1, cmd);
> +	if (err)
> +		return err;
> +
> +	end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
> +
> +	while (time_before(jiffies, end_jiffies)) {
> +		schedule();
> +
> +		err = eval_vpcr(handle, 1, &val);
> +		if (err)
> +			return err;
> +
> +		if (val == 0)
> +			return eval_vpcr(handle, 0, data);
> +	}
> +
> +	acpi_handle_err(handle, "timeout in %s\n", __func__);
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int write_ec_cmd(acpi_handle handle, unsigned long cmd, unsigned long data)
> +{
> +	unsigned long end_jiffies, val;
> +	int err;
> +
> +	err = eval_vpcw(handle, 0, data);
> +	if (err)
> +		return err;
> +
> +	err = eval_vpcw(handle, 1, cmd);
> +	if (err)
> +		return err;
> +
> +	end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
> +
> +	while (time_before(jiffies, end_jiffies)) {
> +		schedule();
> +
> +		err = eval_vpcr(handle, 1, &val);
> +		if (err)
> +			return err;
> +
> +		if (val == 0)
> +			return 0;
> +	}
> +
> +	acpi_handle_err(handle, "timeout in %s\n", __func__);
> +
> +	return -ETIMEDOUT;
> +}
> +
>  /*
>   * debugfs
>   */
> diff --git a/drivers/platform/x86/ideapad-laptop.h b/drivers/platform/x86/ideapad-laptop.h
> index 948cc61800a9..1e52f2aa0aac 100644
> --- a/drivers/platform/x86/ideapad-laptop.h
> +++ b/drivers/platform/x86/ideapad-laptop.h
> @@ -9,9 +9,6 @@
>  #ifndef _IDEAPAD_LAPTOP_H_
>  #define _IDEAPAD_LAPTOP_H_
>  
> -#include <linux/acpi.h>
> -#include <linux/jiffies.h>
> -#include <linux/errno.h>
>  #include <linux/notifier.h>
>  
>  enum ideapad_laptop_notifier_actions {
> @@ -22,140 +19,4 @@ int ideapad_laptop_register_notifier(struct notifier_block *nb);
>  int ideapad_laptop_unregister_notifier(struct notifier_block *nb);
>  void ideapad_laptop_call_notifier(unsigned long action, void *data);
>  
> -enum {
> -	VPCCMD_R_VPC1 = 0x10,
> -	VPCCMD_R_BL_MAX,
> -	VPCCMD_R_BL,
> -	VPCCMD_W_BL,
> -	VPCCMD_R_WIFI,
> -	VPCCMD_W_WIFI,
> -	VPCCMD_R_BT,
> -	VPCCMD_W_BT,
> -	VPCCMD_R_BL_POWER,
> -	VPCCMD_R_NOVO,
> -	VPCCMD_R_VPC2,
> -	VPCCMD_R_TOUCHPAD,
> -	VPCCMD_W_TOUCHPAD,
> -	VPCCMD_R_CAMERA,
> -	VPCCMD_W_CAMERA,
> -	VPCCMD_R_3G,
> -	VPCCMD_W_3G,
> -	VPCCMD_R_ODD, /* 0x21 */
> -	VPCCMD_W_FAN,
> -	VPCCMD_R_RF,
> -	VPCCMD_W_RF,
> -	VPCCMD_W_YMC = 0x2A,
> -	VPCCMD_R_FAN = 0x2B,
> -	VPCCMD_R_SPECIAL_BUTTONS = 0x31,
> -	VPCCMD_W_BL_POWER = 0x33,
> -};
> -
> -static inline int eval_int_with_arg(acpi_handle handle, const char *name, unsigned long arg, unsigned long *res)
> -{
> -	struct acpi_object_list params;
> -	unsigned long long result;
> -	union acpi_object in_obj;
> -	acpi_status status;
> -
> -	params.count = 1;
> -	params.pointer = &in_obj;
> -	in_obj.type = ACPI_TYPE_INTEGER;
> -	in_obj.integer.value = arg;
> -
> -	status = acpi_evaluate_integer(handle, (char *)name, &params, &result);
> -	if (ACPI_FAILURE(status))
> -		return -EIO;
> -
> -	if (res)
> -		*res = result;
> -
> -	return 0;
> -}
> -
> -static inline int eval_vpcr(acpi_handle handle, unsigned long cmd, unsigned long *res)
> -{
> -	return eval_int_with_arg(handle, "VPCR", cmd, res);
> -}
> -
> -static inline int eval_vpcw(acpi_handle handle, unsigned long cmd, unsigned long data)
> -{
> -	struct acpi_object_list params;
> -	union acpi_object in_obj[2];
> -	acpi_status status;
> -
> -	params.count = 2;
> -	params.pointer = in_obj;
> -	in_obj[0].type = ACPI_TYPE_INTEGER;
> -	in_obj[0].integer.value = cmd;
> -	in_obj[1].type = ACPI_TYPE_INTEGER;
> -	in_obj[1].integer.value = data;
> -
> -	status = acpi_evaluate_object(handle, "VPCW", &params, NULL);
> -	if (ACPI_FAILURE(status))
> -		return -EIO;
> -
> -	return 0;
> -}
> -
> -#define IDEAPAD_EC_TIMEOUT 200 /* in ms */
> -
> -static inline int read_ec_data(acpi_handle handle, unsigned long cmd, unsigned long *data)
> -{
> -	unsigned long end_jiffies, val;
> -	int err;
> -
> -	err = eval_vpcw(handle, 1, cmd);
> -	if (err)
> -		return err;
> -
> -	end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
> -
> -	while (time_before(jiffies, end_jiffies)) {
> -		schedule();
> -
> -		err = eval_vpcr(handle, 1, &val);
> -		if (err)
> -			return err;
> -
> -		if (val == 0)
> -			return eval_vpcr(handle, 0, data);
> -	}
> -
> -	acpi_handle_err(handle, "timeout in %s\n", __func__);
> -
> -	return -ETIMEDOUT;
> -}
> -
> -static inline int write_ec_cmd(acpi_handle handle, unsigned long cmd, unsigned long data)
> -{
> -	unsigned long end_jiffies, val;
> -	int err;
> -
> -	err = eval_vpcw(handle, 0, data);
> -	if (err)
> -		return err;
> -
> -	err = eval_vpcw(handle, 1, cmd);
> -	if (err)
> -		return err;
> -
> -	end_jiffies = jiffies + msecs_to_jiffies(IDEAPAD_EC_TIMEOUT) + 1;
> -
> -	while (time_before(jiffies, end_jiffies)) {
> -		schedule();
> -
> -		err = eval_vpcr(handle, 1, &val);
> -		if (err)
> -			return err;
> -
> -		if (val == 0)
> -			return 0;
> -	}
> -
> -	acpi_handle_err(handle, "timeout in %s\n", __func__);
> -
> -	return -ETIMEDOUT;
> -}
> -
> -#undef IDEAPAD_EC_TIMEOUT
>  #endif /* !_IDEAPAD_LAPTOP_H_ */