From patchwork Mon Jan 30 12:44:42 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Woithe X-Patchwork-Id: 9545255 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 0DE3260417 for ; Mon, 30 Jan 2017 13:36:06 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 06F3A271FD for ; Mon, 30 Jan 2017 13:36:06 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EFD5327BA5; Mon, 30 Jan 2017 13:36:05 +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 0C21527813 for ; Mon, 30 Jan 2017 13:36:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752783AbdA3NgE (ORCPT ); Mon, 30 Jan 2017 08:36:04 -0500 Received: from server.atrad.com.au ([150.101.241.2]:51888 "EHLO server.atrad.com.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753408AbdA3NgB (ORCPT ); Mon, 30 Jan 2017 08:36:01 -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 v0UCigR6015465 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 30 Jan 2017 23:14:44 +1030 Date: Mon, 30 Jan 2017 23:14:42 +1030 From: Jonathan Woithe To: kernel@kempniu.pl Cc: platform-driver-x86@vger.kernel.org Subject: [PATCH 2/4][RFC] fujitsu-laptop: restructure initialisation Message-ID: <20170130124442.GE10561@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 --- a/drivers/platform/x86/fujitsu-laptop.c 2017-01-30 22:41:42.128106483 +1030 +++ b/drivers/platform/x86/fujitsu-laptop.c 2017-01-30 22:41:22.976106164 +1030 @@ -146,9 +146,6 @@ 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 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 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 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ #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 @@ 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 @@ 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 @@ kfifo_free(&fujitsu_laptop->fifo); - fujitsu_laptop->acpi_handle = NULL; + kfree(fujitsu_laptop); + fujitsu_laptop = NULL; return 0; } @@ -1046,19 +1084,19 @@ && (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 @@ 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; - - /* Register laptop driver */ + goto err_unregister_acpi2; - 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_latop); -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"); }