Re: [PATCH] Input: i8042 - Add support for platform filter contexts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 16 Dec 2024, Armin Wolf wrote:

> Currently the platform filter cannot access any driver-specific state
> which forces drivers installing a i8042 filter to have at least some
> kind of global pointer for their filter.
> 
> This however might cause issues should such a driver probe multiple
> devices. Fix this by allowing callers of i8042_install_filter() to
> submit a context pointer which is then passed to the i8042 filter.
> 
> Also introduce a separate type for the i8042 filter (i8042_filter_t)
> so that the function definitions can stay compact.
> 
> Tested on a Dell Inspiron 3505.
> 
> Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
> ---
> Since most of the affected drivers are x86 platform drivers, i would
> like to have this changes merged through the platform-drivers-x86
> tree. This would also prevent a possible merge conflict should
> the samsung-galaxybook driver get merged.
> ---
>  drivers/input/misc/ideapad_slidebar.c   |  4 ++--
>  drivers/input/serio/i8042.c             | 17 ++++++++++-------
>  drivers/platform/x86/asus-nb-wmi.c      |  3 ++-
>  drivers/platform/x86/asus-wmi.c         |  2 +-
>  drivers/platform/x86/asus-wmi.h         |  3 +--
>  drivers/platform/x86/dell/dell-laptop.c |  6 +++---
>  drivers/platform/x86/hp/hp_accel.c      |  4 ++--
>  drivers/platform/x86/msi-laptop.c       |  6 +++---
>  drivers/platform/x86/panasonic-laptop.c |  4 ++--
>  drivers/platform/x86/toshiba_acpi.c     |  4 ++--
>  include/linux/i8042.h                   | 15 +++++++--------
>  11 files changed, 35 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/input/misc/ideapad_slidebar.c b/drivers/input/misc/ideapad_slidebar.c
> index f6e5fc807b4d..ab2e0a401904 100644
> --- a/drivers/input/misc/ideapad_slidebar.c
> +++ b/drivers/input/misc/ideapad_slidebar.c
> @@ -121,7 +121,7 @@ static void slidebar_mode_set(u8 mode)
>  }
> 
>  static bool slidebar_i8042_filter(unsigned char data, unsigned char str,
> -				  struct serio *port)
> +				  struct serio *port, void *context)
>  {
>  	static bool extended = false;
> 
> @@ -219,7 +219,7 @@ static int __init ideapad_probe(struct platform_device* pdev)
>  	input_set_capability(slidebar_input_dev, EV_ABS, ABS_X);
>  	input_set_abs_params(slidebar_input_dev, ABS_X, 0, 0xff, 0, 0);
> 
> -	err = i8042_install_filter(slidebar_i8042_filter);
> +	err = i8042_install_filter(slidebar_i8042_filter, NULL);
>  	if (err) {
>  		dev_err(&pdev->dev,
>  			"Failed to install i8042 filter: %d\n", err);
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index 509330a27880..cab5a4c5baf5 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -179,8 +179,8 @@ static struct platform_device *i8042_platform_device;
>  static struct notifier_block i8042_kbd_bind_notifier_block;
> 
>  static bool i8042_handle_data(int irq);
> -static bool (*i8042_platform_filter)(unsigned char data, unsigned char str,
> -				     struct serio *serio);
> +static i8042_filter_t i8042_platform_filter;
> +static void *i8042_platform_filter_context;
> 
>  void i8042_lock_chip(void)
>  {
> @@ -194,8 +194,7 @@ void i8042_unlock_chip(void)
>  }
>  EXPORT_SYMBOL(i8042_unlock_chip);
> 
> -int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
> -					struct serio *serio))
> +int i8042_install_filter(i8042_filter_t filter, void *context)
>  {
>  	guard(spinlock_irqsave)(&i8042_lock);
> 
> @@ -203,12 +202,12 @@ int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
>  		return -EBUSY;
> 
>  	i8042_platform_filter = filter;
> +	i8042_platform_filter_context = context;
>  	return 0;
>  }
>  EXPORT_SYMBOL(i8042_install_filter);
> 
> -int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str,
> -				       struct serio *port))
> +int i8042_remove_filter(i8042_filter_t filter)
>  {
>  	guard(spinlock_irqsave)(&i8042_lock);
> 
> @@ -216,6 +215,7 @@ int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str,
>  		return -EINVAL;
> 
>  	i8042_platform_filter = NULL;
> +	i8042_platform_filter_context = NULL;
>  	return 0;
>  }
>  EXPORT_SYMBOL(i8042_remove_filter);
> @@ -480,7 +480,10 @@ static bool i8042_filter(unsigned char data, unsigned char str,
>  		}
>  	}
> 
> -	if (i8042_platform_filter && i8042_platform_filter(data, str, serio)) {
> +	if (!i8042_platform_filter)
> +		return false;
> +
> +	if (i8042_platform_filter(data, str, serio, i8042_platform_filter_context)) {
>  		dbg("Filtered out by platform filter\n");
>  		return true;
>  	}
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index ef04d396f61c..a3d4b98045f8 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -50,7 +50,8 @@ MODULE_PARM_DESC(tablet_mode_sw, "Tablet mode detect: -1:auto 0:disable 1:kbd-do
>  static struct quirk_entry *quirks;
>  static bool atkbd_reports_vol_keys;
> 
> -static bool asus_i8042_filter(unsigned char data, unsigned char str, struct serio *port)
> +static bool asus_i8042_filter(unsigned char data, unsigned char str, struct serio *port,
> +			      void *context)
>  {
>  	static bool extended_e0;
>  	static bool extended_e1;
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index fdeebab96fc0..6c674de60ec0 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -4824,7 +4824,7 @@ static int asus_wmi_add(struct platform_device *pdev)
>  	}
> 
>  	if (asus->driver->i8042_filter) {
> -		err = i8042_install_filter(asus->driver->i8042_filter);
> +		err = i8042_install_filter(asus->driver->i8042_filter, NULL);
>  		if (err)
>  			pr_warn("Unable to install key filter - %d\n", err);
>  	}
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index d02f15fd3482..018dfde4025e 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -73,8 +73,7 @@ struct asus_wmi_driver {
>  	void (*key_filter) (struct asus_wmi_driver *driver, int *code,
>  			    unsigned int *value, bool *autorelease);
>  	/* Optional standard i8042 filter */
> -	bool (*i8042_filter)(unsigned char data, unsigned char str,
> -			     struct serio *serio);
> +	i8042_filter_t		i8042_filter;
> 
>  	int (*probe) (struct platform_device *device);
>  	void (*detect_quirks) (struct asus_wmi_driver *driver);
> diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
> index 5671bd0deee7..58b860b88fff 100644
> --- a/drivers/platform/x86/dell/dell-laptop.c
> +++ b/drivers/platform/x86/dell/dell-laptop.c
> @@ -725,8 +725,8 @@ static void dell_update_rfkill(struct work_struct *ignored)
>  }
>  static DECLARE_DELAYED_WORK(dell_rfkill_work, dell_update_rfkill);
> 
> -static bool dell_laptop_i8042_filter(unsigned char data, unsigned char str,
> -			      struct serio *port)
> +static bool dell_laptop_i8042_filter(unsigned char data, unsigned char str, struct serio *port,
> +				     void *context)
>  {
>  	static bool extended;
> 
> @@ -884,7 +884,7 @@ static int __init dell_setup_rfkill(void)
>  		pr_warn("Unable to register dell rbtn notifier\n");
>  		goto err_filter;
>  	} else {
> -		ret = i8042_install_filter(dell_laptop_i8042_filter);
> +		ret = i8042_install_filter(dell_laptop_i8042_filter, NULL);
>  		if (ret) {
>  			pr_warn("Unable to install key filter\n");
>  			goto err_filter;
> diff --git a/drivers/platform/x86/hp/hp_accel.c b/drivers/platform/x86/hp/hp_accel.c
> index 39a6530f5072..10d5af18d639 100644
> --- a/drivers/platform/x86/hp/hp_accel.c
> +++ b/drivers/platform/x86/hp/hp_accel.c
> @@ -267,7 +267,7 @@ static struct delayed_led_classdev hpled_led = {
>  };
> 
>  static bool hp_accel_i8042_filter(unsigned char data, unsigned char str,
> -				  struct serio *port)
> +				  struct serio *port, void *context)
>  {
>  	static bool extended;
> 
> @@ -326,7 +326,7 @@ static int lis3lv02d_probe(struct platform_device *device)
>  	/* filter to remove HPQ6000 accelerometer data
>  	 * from keyboard bus stream */
>  	if (strstr(dev_name(&device->dev), "HPQ6000"))
> -		i8042_install_filter(hp_accel_i8042_filter);
> +		i8042_install_filter(hp_accel_i8042_filter, NULL);
> 
>  	INIT_WORK(&hpled_led.work, delayed_set_status_worker);
>  	ret = led_classdev_register(NULL, &hpled_led.led_classdev);
> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> index e5391a37014d..c4b150fa093f 100644
> --- a/drivers/platform/x86/msi-laptop.c
> +++ b/drivers/platform/x86/msi-laptop.c
> @@ -806,8 +806,8 @@ static void msi_send_touchpad_key(struct work_struct *ignored)
>  }
>  static DECLARE_DELAYED_WORK(msi_touchpad_dwork, msi_send_touchpad_key);
> 
> -static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str,
> -				struct serio *port)
> +static bool msi_laptop_i8042_filter(unsigned char data, unsigned char str, struct serio *port,
> +				    void *context)
>  {
>  	static bool extended;
> 
> @@ -996,7 +996,7 @@ static int __init load_scm_model_init(struct platform_device *sdev)
>  	if (result)
>  		goto fail_input;
> 
> -	result = i8042_install_filter(msi_laptop_i8042_filter);
> +	result = i8042_install_filter(msi_laptop_i8042_filter, NULL);
>  	if (result) {
>  		pr_err("Unable to install key filter\n");
>  		goto fail_filter;
> diff --git a/drivers/platform/x86/panasonic-laptop.c b/drivers/platform/x86/panasonic-laptop.c
> index 22ca70eb8227..2987b4db6009 100644
> --- a/drivers/platform/x86/panasonic-laptop.c
> +++ b/drivers/platform/x86/panasonic-laptop.c
> @@ -260,7 +260,7 @@ struct pcc_acpi {
>   * keypress events over the PS/2 kbd interface, filter these out.
>   */
>  static bool panasonic_i8042_filter(unsigned char data, unsigned char str,
> -				   struct serio *port)
> +				   struct serio *port, void *context)
>  {
>  	static bool extended;
> 
> @@ -1100,7 +1100,7 @@ static int acpi_pcc_hotkey_add(struct acpi_device *device)
>  		pcc->platform = NULL;
>  	}
> 
> -	i8042_install_filter(panasonic_i8042_filter);
> +	i8042_install_filter(panasonic_i8042_filter, NULL);
>  	return 0;
> 
>  out_platform:
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 78a5aac2dcfd..5ad3a7183d33 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -2755,7 +2755,7 @@ static int toshiba_acpi_enable_hotkeys(struct toshiba_acpi_dev *dev)
>  }
> 
>  static bool toshiba_acpi_i8042_filter(unsigned char data, unsigned char str,
> -				      struct serio *port)
> +				      struct serio *port, void *context)
>  {
>  	if (str & I8042_STR_AUXDATA)
>  		return false;
> @@ -2915,7 +2915,7 @@ static int toshiba_acpi_setup_keyboard(struct toshiba_acpi_dev *dev)
>  	if (ec_handle && acpi_has_method(ec_handle, "NTFY")) {
>  		INIT_WORK(&dev->hotkey_work, toshiba_acpi_hotkey_work);
> 
> -		error = i8042_install_filter(toshiba_acpi_i8042_filter);
> +		error = i8042_install_filter(toshiba_acpi_i8042_filter, NULL);
>  		if (error) {
>  			pr_err("Error installing key filter\n");
>  			goto err_free_dev;
> diff --git a/include/linux/i8042.h b/include/linux/i8042.h
> index 95b07f8b77fe..be9460c874d6 100644
> --- a/include/linux/i8042.h
> +++ b/include/linux/i8042.h
> @@ -54,15 +54,16 @@
> 
>  struct serio;
> 
> +typedef bool (*i8042_filter_t)(unsigned char data, unsigned char str, struct serio *serio,
> +			       void *context);
> +

Could you please add the kerneldoc for the typedef.

Other than that, the change seems good.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>

-- 
 i.


>  #if defined(CONFIG_SERIO_I8042) || defined(CONFIG_SERIO_I8042_MODULE)
> 
>  void i8042_lock_chip(void);
>  void i8042_unlock_chip(void);
>  int i8042_command(unsigned char *param, int command);
> -int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
> -					struct serio *serio));
> -int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str,
> -				       struct serio *serio));
> +int i8042_install_filter(i8042_filter_t filter, void *context);
> +int i8042_remove_filter(i8042_filter_t filter);
> 
>  #else
> 
> @@ -79,14 +80,12 @@ static inline int i8042_command(unsigned char *param, int command)
>  	return -ENODEV;
>  }
> 
> -static inline int i8042_install_filter(bool (*filter)(unsigned char data, unsigned char str,
> -					struct serio *serio))
> +static inline int i8042_install_filter(i8042_filter_t filter, void *context)
>  {
>  	return -ENODEV;
>  }
> 
> -static inline int i8042_remove_filter(bool (*filter)(unsigned char data, unsigned char str,
> -				       struct serio *serio))
> +static inline int i8042_remove_filter(i8042_filter_t filter)
>  {
>  	return -ENODEV;
>  }
> --
> 2.39.5
> 

[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux