Re: [PATCH v4 1/2] platform/x86: dell-privacy: Add support for Dell hardware privacy

diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 581475f59819..18c430456de7 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -51,7 +51,9 @@ obj-$(CONFIG_DELL_WMI_DESCRIPTOR) += dell-wmi-descriptor.o obj-$(CONFIG_DELL_WMI_AIO) += dell-wmi-aio.o obj-$(CONFIG_DELL_WMI_LED) += dell-wmi-led.o obj-$(CONFIG_DELL_WMI_SYSMAN) += dell-wmi-sysman/

+obj-$(CONFIG_DELL_PRIVACY) += dell-privacy.o +dell-privacy-objs := dell-privacy-wmi.o \

•                                   dell-privacy-acpi.o
  

  # Fujitsu obj-$(CONFIG_AMILO_RFKILL) += amilo-rfkill.o obj-$(CONFIG_FUJITSU_LAPTOP) += fujitsu-laptop.o

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c index 70edc5bb3a14..ec0dcc7fc17c 100644 --- a/drivers/platform/x86/dell-laptop.c +++ b/drivers/platform/x86/dell-laptop.c @@ -31,6 +31,8 @@ #include "dell-rbtn.h" #include "dell-smbios.h"

+#include "dell-privacy-wmi.h"

• struct quirk_entry { bool touchpad_led; bool kbd_led_not_present;

@@ -90,10 +92,12 @@ static struct rfkill *wifi_rfkill; static struct rfkill *bluetooth_rfkill; static struct rfkill *wwan_rfkill; static bool force_rfkill; +static bool has_privacy;

module_param(force_rfkill, bool, 0444); MODULE_PARM_DESC(force_rfkill, "enable rfkill on non whitelisted models");

spurious line change

static const struct dmi_system_id dell_device_table[] __initconst = { { .ident = "Dell laptop", @@ -2205,11 +2209,17 @@ static int __init dell_init(void) dell_laptop_register_notifier(&dell_laptop_notifier);

if (dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE) &&

•    dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE)) {
  

• micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
  

• ret = led_classdev_register(&platform_device->dev, &micmute_led_cdev);
  

• if (ret < 0)
  

• 	goto fail_led;
  

• 	dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE)) {
  

not sure why you changed the alignment?

• if (!privacy_valid)
  

• 	has_privacy = true;
  

• else
  

• 	has_privacy = false;
  

• if (!has_privacy) {
  

• 	micmute_led_cdev.brightness = ledtrig_audio_get(LED_AUDIO_MICMUTE);
  

• 	ret = led_classdev_register(&platform_device->dev, &micmute_led_cdev);
  

• 	if (ret < 0)
  

• 		goto fail_led;
  

• }
  

  }

  if (acpi_video_get_backlight_type() != acpi_backlight_vendor)

+static struct platform_driver dell_privacy_platform_drv = {

• .driver = {

• .name = PRIVACY_PLATFORM_NAME,
  

• },
• .probe = dell_privacy_acpi_probe,
• .remove = dell_privacy_acpi_remove,

+};

+int __init dell_privacy_acpi_init(void) +{

• int err;
• struct platform_device *pdev;
• if (!wmi_has_guid(DELL_PRIVACY_GUID))

• return -ENODEV;
  

• privacy_acpi = kzalloc(sizeof(*privacy_acpi), GFP_KERNEL);
• if (!privacy_acpi)

• return -ENOMEM;
  

• err = platform_driver_register(&dell_privacy_platform_drv);
• if (err)

• goto pdrv_err;
  

• pdev = platform_device_register_simple(

• 	PRIVACY_PLATFORM_NAME, PLATFORM_DEVID_NONE, NULL, 0);
  

• if (IS_ERR(pdev)) {

• err = PTR_ERR(pdev);
  

• goto pdev_err;
  

• }
• return 0;

+pdev_err:

• platform_device_unregister(pdev);

+pdrv_err:

• kfree(privacy_acpi);
• return err;

+}

don't you need some sort of device_initcall() to load this module on startup?

+void dell_privacy_process_event(int type, int code, int status) +{

• struct privacy_wmi_data *priv;
• const struct key_entry *key;
• mutex_lock(&list_mutex);
• priv = list_first_entry_or_null(&wmi_list,

• 	struct privacy_wmi_data,
  

• 	list);
  

• if (!priv) {

• pr_err("dell privacy priv is NULL\n");
  

• goto error;
  

• }
• key = sparse_keymap_entry_from_scancode(priv->input_dev, (type << 16) | code);
• if (!key) {

• dev_dbg(&priv->wdev->dev, "Unknown key with type 0x%04x and code 0x%04x pressed\n",
  

• 		type, code);
  

• goto error;
  

• }
• switch (code) {
• case DELL_PRIVACY_TYPE_AUDIO: /* Mic mute */

• priv->last_status = status;
  

• sparse_keymap_report_entry(priv->input_dev, key, 1, true);
  

• break;
  

• case DELL_PRIVACY_TYPE_CAMERA: /* Camera mute */

• priv->last_status = status;
  

• sparse_keymap_report_entry(priv->input_dev, key, 1, true);
  

• break;
  

You are doing the same things twice, so group the two cases:

case DELL_PRIVACY_TYPE_AUDIO: /* Mic mute */ case DELL_PRIVACY_TYPE_CAMERA: /* Camera mute */ priv->last_status = status; sparse_keymap_report_entry(priv->input_dev, key, 1, true); break;

• default:

• 	dev_dbg(&priv->wdev->dev, "unknown event type 0x%04x 0x%04x",
  

• 			type, code);
  

alignment?

• }

+error:

• mutex_unlock(&list_mutex);

+} +EXPORT_SYMBOL_GPL(dell_privacy_process_event);

+static int dell_privacy_wmi_probe(struct wmi_device *wdev, const void *context) +{

• struct privacy_wmi_data *priv;
• struct key_entry *keymap;
• int ret, i;
• priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
• if (!priv)

• return -ENOMEM;
  

• dev_set_drvdata(&wdev->dev, priv);
• priv->wdev = wdev;
• /* create evdev passing interface */
• priv->input_dev = devm_input_allocate_device(&wdev->dev);
• if (!priv->input_dev)

• return -ENOMEM;
  

• /* remap the wmi keymap event to new keymap */
• keymap = kcalloc(ARRAY_SIZE(dell_wmi_keymap_type_0012),

• 	sizeof(struct key_entry), GFP_KERNEL);
  

• if (!keymap) {

• ret = -ENOMEM;
  

• goto err_free_dev;
  

• }
• /* remap the keymap code with Dell privacy key type 0x12 as prefix

• * KEY_MICMUTE scancode will be reported as 0x120001
  

• */
  

• for (i = 0; i < ARRAY_SIZE(dell_wmi_keymap_type_0012); i++) {

• keymap[i] = dell_wmi_keymap_type_0012[i];
  

• keymap[i].code |= (0x0012 << 16);
  

• }
• ret = sparse_keymap_setup(priv->input_dev, keymap, NULL);
• if (ret)

• return ret;
  

• priv->input_dev->dev.parent = &wdev->dev;
• priv->input_dev->name = "Dell Privacy Driver";
• priv->input_dev->id.bustype = BUS_HOST;
• if (input_register_device(priv->input_dev)) {

• pr_debug("input_register_device failed to register!\n");
  

• goto err_free_keymap;
  

• }
• mutex_lock(&list_mutex);
• list_add_tail(&priv->list, &wmi_list);
• mutex_unlock(&list_mutex);
• if (get_current_status(priv->wdev))

• goto err_free_input;
  

• ret = devm_device_add_group(&wdev->dev, &privacy_attribute_group);
• if (ret)

• goto err_free_input;
  

• kfree(keymap);
• return 0;

+err_free_input:

• input_unregister_device(priv->input_dev);

+err_free_keymap:

• privacy_valid = -ENODEV;
• kfree(keymap);

+err_free_dev:

• input_free_device(priv->input_dev);

priv->input_dev is allocated with devm_, so why do you need to do anything with it? that seems like a miss.

• return ret;

+}

MODULE_AUTHOR("Matthew Garrett mjg@redhat.com"); MODULE_AUTHOR("Pali Rohár pali@kernel.org"); @@ -381,6 +383,7 @@ static void dell_wmi_notify(struct wmi_device *wdev, u16 *buffer_entry, *buffer_end; acpi_size buffer_size; int len, i;

• int err;

  if (obj->type != ACPI_TYPE_BUFFER) { pr_warn("bad response type %x\n", obj->type);

@@ -427,10 +430,9 @@ static void dell_wmi_notify(struct wmi_device *wdev,

switch (buffer_entry[1]) {
case 0x0000: /* One key pressed or event occurred */

• case 0x0012: /* Event with extended data occurred */
  if (len > 2)
  	dell_wmi_process_key(wdev, buffer_entry[1],
  

• 				     buffer_entry[2]);
  

• 				buffer_entry[2]);
  

keep the alignment?

	/* Extended data is currently ignored */
	break;
case 0x0010: /* Sequence of keys pressed */

@@ -439,6 +441,17 @@ static void dell_wmi_notify(struct wmi_device *wdev, dell_wmi_process_key(wdev, buffer_entry[1], buffer_entry[i]); break;

• case 0x0012:
  

• 	err = dell_privacy_state();
  

• 	if (err == 0) {
  

• 		dell_privacy_process_event(buffer_entry[1],
  

• 				buffer_entry[3], buffer_entry[4]);
  

• 	} else {
  

• 		if (len > 2)
  

• 			dell_wmi_process_key(wdev, buffer_entry[1],
  

• 				buffer_entry[2]);
  

• 	}
  

• 	break;
  

  default: /* Unknown event */ pr_info("Unknown WMI event type 0x%x\n", (int)buffer_entry[1]);