From patchwork Tue Jan 31 13:09:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Woithe X-Patchwork-Id: 9547183 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0E03360425 for ; Tue, 31 Jan 2017 13:09:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F349927F2B for ; Tue, 31 Jan 2017 13:09:13 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E7B8428334; Tue, 31 Jan 2017 13:09:13 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EE19327F2B for ; Tue, 31 Jan 2017 13:09:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752029AbdAaNJM (ORCPT ); Tue, 31 Jan 2017 08:09:12 -0500 Received: from server.atrad.com.au ([150.101.241.2]:52682 "EHLO server.atrad.com.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751882AbdAaNJM (ORCPT ); Tue, 31 Jan 2017 08:09:12 -0500 Received: from marvin.atrad.com.au (IDENT:1008@marvin.atrad.com.au [192.168.0.2]) by server.atrad.com.au (8.14.9/8.14.9) with ESMTP id v0VD94cl029147 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 31 Jan 2017 23:39:05 +1030 Date: Tue, 31 Jan 2017 23:39:04 +1030 From: Jonathan Woithe To: kernel@kempniu.pl Cc: platform-driver-x86@vger.kernel.org Subject: [PATCH 2/4][RFC v2] fujitsu-laptop: fujitsu-laptop: restructure initialisation Message-ID: <20170131130904.GC6218@marvin.atrad.com.au> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) X-MIMEDefang-action: accept X-Scanned-By: MIMEDefang 2.78 on 192.168.0.1 Sender: platform-driver-x86-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Don't register backlight and platform devices on non-fujitsu hardware - bail out instead. Register the backlight driver _after_ the main acpi driver. The backlight device needs the main device in order to get and set its power state. This also allows us to move the keycodes and platform device from "struct fujitsu_bl" to the main "struct fujitsu". Similarly, the platform device interface is now exported to userspace _after_ the acpi devices have been initialized. This closes a gap during initialization where userspace would read undefined values, and where writes from userspace might provoke undefined behaviour. While we're at it, make sure the platform driver is registered before the platform device. Also replace kmalloc() + memset() with kzalloc(). Signed-off-by: Alan Jenkins Signed-off-by: Jonathan Woithe --- drivers/platform/x86/fujitsu-laptop.c | 222 +++++++++++++++------------------- 1 file changed, 98 insertions(+), 124 deletions(-) diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c index a3752bb..c410047 100644 --- a/drivers/platform/x86/fujitsu-laptop.c +++ b/drivers/platform/x86/fujitsu-laptop.c @@ -146,9 +146,6 @@ struct fujitsu_bl { struct input_dev *input; char phys[32]; struct backlight_device *bl_device; - struct platform_device *pf_device; - int keycode1, keycode2, keycode3, keycode4, keycode5; - unsigned int max_brightness; unsigned int brightness_changed; unsigned int brightness_level; @@ -167,7 +164,8 @@ struct fujitsu_laptop { struct platform_device *pf_device; struct kfifo fifo; spinlock_t fifo_lock; - int flags_supported; + int keycode1, keycode2, keycode3, keycode4, keycode5; + int flags_supported; int flags_state; int logolamp_registered; int kblamps_registered; @@ -634,25 +632,25 @@ static void __init dmi_check_cb_common(const struct dmi_system_id *id) static int __init dmi_check_cb_s6410(const struct dmi_system_id *id) { dmi_check_cb_common(id); - fujitsu_bl->keycode1 = KEY_SCREENLOCK; /* "Lock" */ - fujitsu_bl->keycode2 = KEY_HELP; /* "Mobility Center" */ + fujitsu_laptop->keycode1 = KEY_SCREENLOCK; /* "Lock" */ + fujitsu_laptop->keycode2 = KEY_HELP; /* "Mobility Center" */ return 1; } static int __init dmi_check_cb_s6420(const struct dmi_system_id *id) { dmi_check_cb_common(id); - fujitsu_bl->keycode1 = KEY_SCREENLOCK; /* "Lock" */ - fujitsu_bl->keycode2 = KEY_HELP; /* "Mobility Center" */ + fujitsu_laptop->keycode1 = KEY_SCREENLOCK; /* "Lock" */ + fujitsu_laptop->keycode2 = KEY_HELP; /* "Mobility Center" */ return 1; } static int __init dmi_check_cb_p8010(const struct dmi_system_id *id) { dmi_check_cb_common(id); - fujitsu_bl->keycode1 = KEY_HELP; /* "Support" */ - fujitsu_bl->keycode3 = KEY_SWITCHVIDEOMODE; /* "Presentation" */ - fujitsu_bl->keycode4 = KEY_WWW; /* "Internet" */ + fujitsu_laptop->keycode1 = KEY_HELP; /* "Support" */ + fujitsu_laptop->keycode3 = KEY_SWITCHVIDEOMODE; /* "Presentation" */ + fujitsu_laptop->keycode4 = KEY_WWW; /* "Internet" */ return 1; } @@ -685,6 +683,7 @@ static const struct dmi_system_id fujitsu_laptop_dmi_table[] __initconst = { static int acpi_fujitsu_bl_add(struct acpi_device *device) { + int result = 0; int state = 0; struct input_dev *input; int error; @@ -692,6 +691,9 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device) if (!device) return -EINVAL; + fujitsu_bl = kzalloc(sizeof(struct fujitsu_bl), GFP_KERNEL); + if (!fujitsu_bl) + return -ENOMEM; fujitsu_bl->acpi_handle = device->handle; sprintf(acpi_device_name(device), "%s", ACPI_FUJITSU_BL_DEVICE_NAME); sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS); @@ -700,7 +702,7 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device) fujitsu_bl->input = input = input_allocate_device(); if (!input) { error = -ENOMEM; - goto err_stop; + goto err_free; } snprintf(fujitsu_bl->phys, sizeof(fujitsu_bl->phys), @@ -751,14 +753,35 @@ static int acpi_fujitsu_bl_add(struct acpi_device *device) fujitsu_bl->max_brightness = FUJITSU_LCD_N_LEVELS; get_lcd_level(); - return 0; + /* Register backlight stuff */ + + if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { + struct backlight_properties props; + + memset(&props, 0, sizeof(struct backlight_properties)); + props.type = BACKLIGHT_PLATFORM; + props.max_brightness = fujitsu_bl->max_brightness - 1; + fujitsu_bl->bl_device = backlight_device_register("fujitsu-laptop", + NULL, NULL, + &fujitsubl_ops, + &props); + if (IS_ERR(fujitsu_bl->bl_device)) { + result = PTR_ERR(fujitsu_bl->bl_device); + fujitsu_bl->bl_device = NULL; + goto err_unregister_input_dev; + } + fujitsu_bl->bl_device->props.brightness = fujitsu_bl->brightness_level; + } + + return result; err_unregister_input_dev: input_unregister_device(input); input = NULL; err_free_input_dev: input_free_device(input); -err_stop: +err_free: + kfree(fujitsu_bl); return error; } @@ -767,9 +790,13 @@ static int acpi_fujitsu_bl_remove(struct acpi_device *device) struct fujitsu_bl *fujitsu_bl = acpi_driver_data(device); struct input_dev *input = fujitsu_bl->input; + if (fujitsu_bl->bl_device) + backlight_device_unregister(fujitsu_bl->bl_device); + input_unregister_device(input); - fujitsu_bl->acpi_handle = NULL; + kfree(fujitsu_bl); + fujitsu_bl = NULL; return 0; } @@ -841,19 +868,28 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device) if (!device) return -EINVAL; + fujitsu_laptop = kzalloc(sizeof(struct fujitsu_laptop), GFP_KERNEL); + if (!fujitsu_laptop) + return -ENOMEM; fujitsu_laptop->acpi_handle = device->handle; sprintf(acpi_device_name(device), "%s", ACPI_FUJITSU_LAPTOP_DEVICE_NAME); sprintf(acpi_device_class(device), "%s", ACPI_FUJITSU_CLASS); device->driver_data = fujitsu_laptop; + fujitsu_laptop->keycode1 = KEY_PROG1; + fujitsu_laptop->keycode2 = KEY_PROG2; + fujitsu_laptop->keycode3 = KEY_PROG3; + fujitsu_laptop->keycode4 = KEY_PROG4; + dmi_check_system(fujitsu_laptop_dmi_table); + /* kfifo */ spin_lock_init(&fujitsu_laptop->fifo_lock); error = kfifo_alloc(&fujitsu_laptop->fifo, RINGBUFFERSIZE * sizeof(int), GFP_KERNEL); if (error) { pr_err("kfifo_alloc failed\n"); - goto err_stop; + goto err_free; } fujitsu_laptop->input = input = input_allocate_device(); @@ -872,11 +908,11 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device) input->dev.parent = &device->dev; set_bit(EV_KEY, input->evbit); - set_bit(fujitsu_bl->keycode1, input->keybit); - set_bit(fujitsu_bl->keycode2, input->keybit); - set_bit(fujitsu_bl->keycode3, input->keybit); - set_bit(fujitsu_bl->keycode4, input->keybit); - set_bit(fujitsu_bl->keycode5, input->keybit); + set_bit(fujitsu_laptop->keycode1, input->keybit); + set_bit(fujitsu_laptop->keycode2, input->keybit); + set_bit(fujitsu_laptop->keycode3, input->keybit); + set_bit(fujitsu_laptop->keycode4, input->keybit); + set_bit(fujitsu_laptop->keycode5, input->keybit); set_bit(KEY_TOUCHPAD_TOGGLE, input->keybit); set_bit(KEY_UNKNOWN, input->keybit); @@ -927,7 +963,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device) #if IS_ENABLED(CONFIG_LEDS_CLASS) if (call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) { - result = led_classdev_register(&fujitsu_bl->pf_device->dev, + result = led_classdev_register(&fujitsu_laptop->pf_device->dev, &logolamp_led); if (result == 0) { fujitsu_laptop->logolamp_registered = 1; @@ -939,7 +975,7 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device) if ((call_fext_func(FUNC_LEDS, 0x0, 0x0, 0x0) & KEYBOARD_LAMPS) && (call_fext_func(FUNC_BUTTONS, 0x0, 0x0, 0x0) == 0x0)) { - result = led_classdev_register(&fujitsu_bl->pf_device->dev, + result = led_classdev_register(&fujitsu_laptop->pf_device->dev, &kblamps_led); if (result == 0) { fujitsu_laptop->kblamps_registered = 1; @@ -993,7 +1029,8 @@ static int acpi_fujitsu_laptop_add(struct acpi_device *device) input_free_device(input); err_free_fifo: kfifo_free(&fujitsu_laptop->fifo); -err_stop: +err_free: + kfree(fujitsu_laptop); return error; } @@ -1020,7 +1057,8 @@ static int acpi_fujitsu_laptop_remove(struct acpi_device *device) kfifo_free(&fujitsu_laptop->fifo); - fujitsu_laptop->acpi_handle = NULL; + kfree(fujitsu_laptop); + fujitsu_laptop = NULL; return 0; } @@ -1046,19 +1084,19 @@ static void acpi_fujitsu_laptop_notify(struct acpi_device *device, u32 event) && (i++) < MAX_HOTKEY_RINGBUFFER_SIZE) { switch (irb & 0x4ff) { case KEY1_CODE: - keycode = fujitsu_bl->keycode1; + keycode = fujitsu_laptop->keycode1; break; case KEY2_CODE: - keycode = fujitsu_bl->keycode2; + keycode = fujitsu_laptop->keycode2; break; case KEY3_CODE: - keycode = fujitsu_bl->keycode3; + keycode = fujitsu_laptop->keycode3; break; case KEY4_CODE: - keycode = fujitsu_bl->keycode4; + keycode = fujitsu_laptop->keycode4; break; case KEY5_CODE: - keycode = fujitsu_bl->keycode5; + keycode = fujitsu_laptop->keycode5; break; case 0: keycode = 0; @@ -1171,134 +1209,70 @@ MODULE_DEVICE_TABLE(acpi, fujitsu_ids); static int __init fujitsu_init(void) { - int ret, result; + int ret; if (acpi_disabled) return -ENODEV; - fujitsu_bl = kzalloc(sizeof(struct fujitsu_bl), GFP_KERNEL); - if (!fujitsu_bl) - return -ENOMEM; - fujitsu_bl->keycode1 = KEY_PROG1; - fujitsu_bl->keycode2 = KEY_PROG2; - fujitsu_bl->keycode3 = KEY_PROG3; - fujitsu_bl->keycode4 = KEY_PROG4; - fujitsu_bl->keycode5 = KEY_RFKILL; - dmi_check_system(fujitsu_laptop_dmi_table); - - result = acpi_bus_register_driver(&acpi_fujitsu_bl_driver); - if (result < 0) { + ret = acpi_bus_register_driver(&acpi_fujitsu_laptop_driver); + if (ret < 0) + goto err_stop; + if (!fujitsu_laptop) { ret = -ENODEV; - goto fail_acpi; + goto err_unregister_acpi1; } /* Register platform stuff */ - fujitsu_bl->pf_device = platform_device_alloc("fujitsu-laptop", -1); - if (!fujitsu_bl->pf_device) { - ret = -ENOMEM; - goto fail_platform_driver; - } - - ret = platform_device_add(fujitsu_bl->pf_device); - if (ret) - goto fail_platform_device1; - - ret = - sysfs_create_group(&fujitsu_bl->pf_device->dev.kobj, - &fujitsu_pf_attribute_group); - if (ret) - goto fail_platform_device2; - - /* Register backlight stuff */ - - if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { - struct backlight_properties props; - - memset(&props, 0, sizeof(struct backlight_properties)); - props.type = BACKLIGHT_PLATFORM; - props.max_brightness = fujitsu_bl->max_brightness - 1; - fujitsu_bl->bl_device = backlight_device_register("fujitsu-laptop", - NULL, NULL, - &fujitsubl_ops, - &props); - if (IS_ERR(fujitsu_bl->bl_device)) { - ret = PTR_ERR(fujitsu_bl->bl_device); - fujitsu_bl->bl_device = NULL; - goto fail_sysfs_group; - } - fujitsu_bl->bl_device->props.brightness = fujitsu_bl->brightness_level; - } - ret = platform_driver_register(&fujitsu_pf_driver); if (ret) - goto fail_backlight; + goto err_unregister_acpi2; - /* Register laptop driver */ - - fujitsu_laptop = kzalloc(sizeof(struct fujitsu_laptop), GFP_KERNEL); - if (!fujitsu_laptop) { + fujitsu_laptop->pf_device = platform_device_alloc("fujitsu-laptop", -1); + if (!fujitsu_laptop->pf_device) { ret = -ENOMEM; - goto fail_laptop; - } - - result = acpi_bus_register_driver(&acpi_fujitsu_laptop_driver); - if (result < 0) { - ret = -ENODEV; - goto fail_laptop1; + goto err_free_platform_driver; } + ret = platform_device_add(fujitsu_laptop->pf_device); + if (ret) + goto err_free_platform_device1; - /* Sync backlight power status (needs FUJ02E3 device, hence deferred) */ - if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { - if (call_fext_func(FUNC_BACKLIGHT, 0x2, 0x4, 0x0) == 3) - fujitsu_bl->bl_device->props.power = FB_BLANK_POWERDOWN; - else - fujitsu_bl->bl_device->props.power = FB_BLANK_UNBLANK; - } + ret = sysfs_create_group(&fujitsu_laptop->pf_device->dev.kobj, + &fujitsu_pf_attribute_group); + if (ret) + goto err_free_platform_device2; pr_info("driver " FUJITSU_DRIVER_VERSION " successfully loaded\n"); return 0; -fail_laptop1: - kfree(fujitsu_laptop); -fail_laptop: +err_free_platform_device2: + platform_device_del(fujitsu_laptop->pf_device); +err_free_platform_device1: + platform_device_put(fujitsu_laptop->pf_device); +err_free_platform_driver: platform_driver_unregister(&fujitsu_pf_driver); -fail_backlight: - backlight_device_unregister(fujitsu_bl->bl_device); -fail_sysfs_group: - sysfs_remove_group(&fujitsu_bl->pf_device->dev.kobj, - &fujitsu_pf_attribute_group); -fail_platform_device2: - platform_device_del(fujitsu_bl->pf_device); -fail_platform_device1: - platform_device_put(fujitsu_bl->pf_device); -fail_platform_driver: +err_unregister_acpi2: acpi_bus_unregister_driver(&acpi_fujitsu_bl_driver); -fail_acpi: - kfree(fujitsu_bl); +err_unregister_acpi1: + acpi_bus_unregister_driver(&acpi_fujitsu_laptop_driver); +err_stop: return ret; } static void __exit fujitsu_cleanup(void) { - acpi_bus_unregister_driver(&acpi_fujitsu_laptop_driver); - - kfree(fujitsu_laptop); + platform_device_unregister(fujitsu_laptop->pf_device); - platform_driver_unregister(&fujitsu_pf_driver); - - backlight_device_unregister(fujitsu_bl->bl_device); - - sysfs_remove_group(&fujitsu_bl->pf_device->dev.kobj, + sysfs_remove_group(&fujitsu_laptop->pf_device->dev.kobj, &fujitsu_pf_attribute_group); - platform_device_unregister(fujitsu_bl->pf_device); + platform_driver_unregister(&fujitsu_pf_driver); acpi_bus_unregister_driver(&acpi_fujitsu_bl_driver); - kfree(fujitsu_bl); + acpi_bus_unregister_driver(&acpi_fujitsu_laptop_driver); pr_info("driver unloaded\n"); }