lkml.org 
[lkml]   [2018]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH 4.15 004/122] platform/x86: dell-laptop: Allocate buffer on heap rather than globally
Date
Greg,

Can you please make sure that the one that came in right after this is also applied at the same time?
https://www.spinics.net/lists/stable/msg215268.html

As you recommended before they weren't squashed together, but they should both come in at the
same time.

Thanks,

> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org]
> Sent: Thursday, March 8, 2018 3:37 AM
> To: linux-kernel@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; stable@vger.kernel.org;
> Pali Rohar <pali.rohar@gmail.com>; Limonciello, Mario
> <Mario_Limonciello@Dell.com>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>
> Subject: [PATCH 4.15 004/122] platform/x86: dell-laptop: Allocate buffer on heap
> rather than globally
>
> 4.15-stable review patch. If anyone has any objections, please let me know.
>
> ------------------
>
> From: Mario Limonciello <mario.limonciello@dell.com>
>
> commit 9862b43624a5450a097cc4122732857b869dbbca upstream.
>
> There is no longer a need for the buffer to be defined in
> first 4GB physical address space.
>
> Furthermore there may be race conditions with multiple different functions
> working on a module wide buffer causing incorrect results.
>
> Fixes: 549b4930f057658dc50d8010e66219233119a4d8
> Suggested-by: Pali Rohar <pali.rohar@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
>
> ---
> drivers/platform/x86/dell-laptop.c | 188 ++++++++++++++++++++-----------------
> 1 file changed, 103 insertions(+), 85 deletions(-)
>
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -78,7 +78,6 @@ static struct platform_driver platform_d
> }
> };
>
> -static struct calling_interface_buffer *buffer;
> static struct platform_device *platform_device;
> static struct backlight_device *dell_backlight_device;
> static struct rfkill *wifi_rfkill;
> @@ -286,7 +285,8 @@ static const struct dmi_system_id dell_q
> { }
> };
>
> -void dell_set_arguments(u32 arg0, u32 arg1, u32 arg2, u32 arg3)
> +static void dell_fill_request(struct calling_interface_buffer *buffer,
> + u32 arg0, u32 arg1, u32 arg2, u32 arg3)
> {
> memset(buffer, 0, sizeof(struct calling_interface_buffer));
> buffer->input[0] = arg0;
> @@ -295,7 +295,8 @@ void dell_set_arguments(u32 arg0, u32 ar
> buffer->input[3] = arg3;
> }
>
> -int dell_send_request(u16 class, u16 select)
> +static int dell_send_request(struct calling_interface_buffer *buffer,
> + u16 class, u16 select)
> {
> int ret;
>
> @@ -432,21 +433,22 @@ static int dell_rfkill_set(void *data, b
> int disable = blocked ? 1 : 0;
> unsigned long radio = (unsigned long)data;
> int hwswitch_bit = (unsigned long)data - 1;
> + struct calling_interface_buffer buffer;
> int hwswitch;
> int status;
> int ret;
>
> - dell_set_arguments(0, 0, 0, 0);
> - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> + dell_fill_request(&buffer, 0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> if (ret)
> return ret;
> - status = buffer->output[1];
> + status = buffer.output[1];
>
> - dell_set_arguments(0x2, 0, 0, 0);
> - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> + dell_fill_request(&buffer, 0x2, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> if (ret)
> return ret;
> - hwswitch = buffer->output[1];
> + hwswitch = buffer.output[1];
>
> /* If the hardware switch controls this radio, and the hardware
> switch is disabled, always disable the radio */
> @@ -454,8 +456,8 @@ static int dell_rfkill_set(void *data, b
> (status & BIT(0)) && !(status & BIT(16)))
> disable = 1;
>
> - dell_set_arguments(1 | (radio<<8) | (disable << 16), 0, 0, 0);
> - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> + dell_fill_request(&buffer, 1 | (radio<<8) | (disable << 16), 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> return ret;
> }
>
> @@ -464,9 +466,11 @@ static void dell_rfkill_update_sw_state(
> {
> if (status & BIT(0)) {
> /* Has hw-switch, sync sw_state to BIOS */
> + struct calling_interface_buffer buffer;
> int block = rfkill_blocked(rfkill);
> - dell_set_arguments(1 | (radio << 8) | (block << 16), 0, 0, 0);
> - dell_send_request(CLASS_INFO, SELECT_RFKILL);
> + dell_fill_request(&buffer,
> + 1 | (radio << 8) | (block << 16), 0, 0, 0);
> + dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> } else {
> /* No hw-switch, sync BIOS state to sw_state */
> rfkill_set_sw_state(rfkill, !!(status & BIT(radio + 16)));
> @@ -483,21 +487,22 @@ static void dell_rfkill_update_hw_state(
> static void dell_rfkill_query(struct rfkill *rfkill, void *data)
> {
> int radio = ((unsigned long)data & 0xF);
> + struct calling_interface_buffer buffer;
> int hwswitch;
> int status;
> int ret;
>
> - dell_set_arguments(0, 0, 0, 0);
> - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> - status = buffer->output[1];
> + dell_fill_request(&buffer, 0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> + status = buffer.output[1];
>
> if (ret != 0 || !(status & BIT(0))) {
> return;
> }
>
> - dell_set_arguments(0, 0x2, 0, 0);
> - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> - hwswitch = buffer->output[1];
> + dell_fill_request(&buffer, 0, 0x2, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> + hwswitch = buffer.output[1];
>
> if (ret != 0)
> return;
> @@ -514,22 +519,23 @@ static struct dentry *dell_laptop_dir;
>
> static int dell_debugfs_show(struct seq_file *s, void *data)
> {
> + struct calling_interface_buffer buffer;
> int hwswitch_state;
> int hwswitch_ret;
> int status;
> int ret;
>
> - dell_set_arguments(0, 0, 0, 0);
> - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> + dell_fill_request(&buffer, 0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> if (ret)
> return ret;
> - status = buffer->output[1];
> + status = buffer.output[1];
>
> - dell_set_arguments(0, 0x2, 0, 0);
> - hwswitch_ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> + dell_fill_request(&buffer, 0, 0x2, 0, 0);
> + hwswitch_ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> if (hwswitch_ret)
> return hwswitch_ret;
> - hwswitch_state = buffer->output[1];
> + hwswitch_state = buffer.output[1];
>
> seq_printf(s, "return:\t%d\n", ret);
> seq_printf(s, "status:\t0x%X\n", status);
> @@ -610,22 +616,23 @@ static const struct file_operations dell
>
> static void dell_update_rfkill(struct work_struct *ignored)
> {
> + struct calling_interface_buffer buffer;
> int hwswitch = 0;
> int status;
> int ret;
>
> - dell_set_arguments(0, 0, 0, 0);
> - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> - status = buffer->output[1];
> + dell_fill_request(&buffer, 0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> + status = buffer.output[1];
>
> if (ret != 0)
> return;
>
> - dell_set_arguments(0, 0x2, 0, 0);
> - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> + dell_fill_request(&buffer, 0, 0x2, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
>
> if (ret == 0 && (status & BIT(0)))
> - hwswitch = buffer->output[1];
> + hwswitch = buffer.output[1];
>
> if (wifi_rfkill) {
> dell_rfkill_update_hw_state(wifi_rfkill, 1, status, hwswitch);
> @@ -683,6 +690,7 @@ static struct notifier_block dell_laptop
>
> static int __init dell_setup_rfkill(void)
> {
> + struct calling_interface_buffer buffer;
> int status, ret, whitelisted;
> const char *product;
>
> @@ -698,9 +706,9 @@ static int __init dell_setup_rfkill(void
> if (!force_rfkill && !whitelisted)
> return 0;
>
> - dell_set_arguments(0, 0, 0, 0);
> - ret = dell_send_request(CLASS_INFO, SELECT_RFKILL);
> - status = buffer->output[1];
> + dell_fill_request(&buffer, 0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_RFKILL);
> + status = buffer.output[1];
>
> /* dell wireless info smbios call is not supported */
> if (ret != 0)
> @@ -853,6 +861,7 @@ static void dell_cleanup_rfkill(void)
>
> static int dell_send_intensity(struct backlight_device *bd)
> {
> + struct calling_interface_buffer buffer;
> struct calling_interface_token *token;
> int ret;
>
> @@ -860,17 +869,21 @@ static int dell_send_intensity(struct ba
> if (!token)
> return -ENODEV;
>
> - dell_set_arguments(token->location, bd->props.brightness, 0, 0);
> + dell_fill_request(&buffer,
> + token->location, bd->props.brightness, 0, 0);
> if (power_supply_is_system_supplied() > 0)
> - ret = dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_AC);
> + ret = dell_send_request(&buffer,
> + CLASS_TOKEN_WRITE,
> SELECT_TOKEN_AC);
> else
> - ret = dell_send_request(CLASS_TOKEN_WRITE,
> SELECT_TOKEN_BAT);
> + ret = dell_send_request(&buffer,
> + CLASS_TOKEN_WRITE,
> SELECT_TOKEN_BAT);
>
> return ret;
> }
>
> static int dell_get_intensity(struct backlight_device *bd)
> {
> + struct calling_interface_buffer buffer;
> struct calling_interface_token *token;
> int ret;
>
> @@ -878,14 +891,17 @@ static int dell_get_intensity(struct bac
> if (!token)
> return -ENODEV;
>
> - dell_set_arguments(token->location, 0, 0, 0);
> + dell_fill_request(&buffer, token->location, 0, 0, 0);
> if (power_supply_is_system_supplied() > 0)
> - ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_AC);
> + ret = dell_send_request(&buffer,
> + CLASS_TOKEN_READ, SELECT_TOKEN_AC);
> else
> - ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_BAT);
> + ret = dell_send_request(&buffer,
> + CLASS_TOKEN_READ,
> SELECT_TOKEN_BAT);
>
> if (ret == 0)
> - ret = buffer->output[1];
> + ret = buffer.output[1];
> +
> return ret;
> }
>
> @@ -1149,31 +1165,33 @@ static DEFINE_MUTEX(kbd_led_mutex);
>
> static int kbd_get_info(struct kbd_info *info)
> {
> + struct calling_interface_buffer buffer;
> u8 units;
> int ret;
>
> - dell_set_arguments(0, 0, 0, 0);
> - ret = dell_send_request(CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT);
> + dell_fill_request(&buffer, 0, 0, 0, 0);
> + ret = dell_send_request(&buffer,
> + CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT);
> if (ret)
> return ret;
>
> - info->modes = buffer->output[1] & 0xFFFF;
> - info->type = (buffer->output[1] >> 24) & 0xFF;
> - info->triggers = buffer->output[2] & 0xFF;
> - units = (buffer->output[2] >> 8) & 0xFF;
> - info->levels = (buffer->output[2] >> 16) & 0xFF;
> + info->modes = buffer.output[1] & 0xFFFF;
> + info->type = (buffer.output[1] >> 24) & 0xFF;
> + info->triggers = buffer.output[2] & 0xFF;
> + units = (buffer.output[2] >> 8) & 0xFF;
> + info->levels = (buffer.output[2] >> 16) & 0xFF;
>
> if (quirks && quirks->kbd_led_levels_off_1 && info->levels)
> info->levels--;
>
> if (units & BIT(0))
> - info->seconds = (buffer->output[3] >> 0) & 0xFF;
> + info->seconds = (buffer.output[3] >> 0) & 0xFF;
> if (units & BIT(1))
> - info->minutes = (buffer->output[3] >> 8) & 0xFF;
> + info->minutes = (buffer.output[3] >> 8) & 0xFF;
> if (units & BIT(2))
> - info->hours = (buffer->output[3] >> 16) & 0xFF;
> + info->hours = (buffer.output[3] >> 16) & 0xFF;
> if (units & BIT(3))
> - info->days = (buffer->output[3] >> 24) & 0xFF;
> + info->days = (buffer.output[3] >> 24) & 0xFF;
>
> return ret;
> }
> @@ -1233,31 +1251,34 @@ static int kbd_set_level(struct kbd_stat
>
> static int kbd_get_state(struct kbd_state *state)
> {
> + struct calling_interface_buffer buffer;
> int ret;
>
> - dell_set_arguments(0x1, 0, 0, 0);
> - ret = dell_send_request(CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT);
> + dell_fill_request(&buffer, 0, 0, 0, 0);
> + ret = dell_send_request(&buffer,
> + CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT);
> if (ret)
> return ret;
>
> - state->mode_bit = ffs(buffer->output[1] & 0xFFFF);
> + state->mode_bit = ffs(buffer.output[1] & 0xFFFF);
> if (state->mode_bit != 0)
> state->mode_bit--;
>
> - state->triggers = (buffer->output[1] >> 16) & 0xFF;
> - state->timeout_value = (buffer->output[1] >> 24) & 0x3F;
> - state->timeout_unit = (buffer->output[1] >> 30) & 0x3;
> - state->als_setting = buffer->output[2] & 0xFF;
> - state->als_value = (buffer->output[2] >> 8) & 0xFF;
> - state->level = (buffer->output[2] >> 16) & 0xFF;
> - state->timeout_value_ac = (buffer->output[2] >> 24) & 0x3F;
> - state->timeout_unit_ac = (buffer->output[2] >> 30) & 0x3;
> + state->triggers = (buffer.output[1] >> 16) & 0xFF;
> + state->timeout_value = (buffer.output[1] >> 24) & 0x3F;
> + state->timeout_unit = (buffer.output[1] >> 30) & 0x3;
> + state->als_setting = buffer.output[2] & 0xFF;
> + state->als_value = (buffer.output[2] >> 8) & 0xFF;
> + state->level = (buffer.output[2] >> 16) & 0xFF;
> + state->timeout_value_ac = (buffer.output[2] >> 24) & 0x3F;
> + state->timeout_unit_ac = (buffer.output[2] >> 30) & 0x3;
>
> return ret;
> }
>
> static int kbd_set_state(struct kbd_state *state)
> {
> + struct calling_interface_buffer buffer;
> int ret;
> u32 input1;
> u32 input2;
> @@ -1270,8 +1291,9 @@ static int kbd_set_state(struct kbd_stat
> input2 |= (state->level & 0xFF) << 16;
> input2 |= (state->timeout_value_ac & 0x3F) << 24;
> input2 |= (state->timeout_unit_ac & 0x3) << 30;
> - dell_set_arguments(0x2, input1, input2, 0);
> - ret = dell_send_request(CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT);
> + dell_fill_request(&buffer, 0x2, input1, input2, 0);
> + ret = dell_send_request(&buffer,
> + CLASS_KBD_BACKLIGHT, SELECT_KBD_BACKLIGHT);
>
> return ret;
> }
> @@ -1298,6 +1320,7 @@ static int kbd_set_state_safe(struct kbd
>
> static int kbd_set_token_bit(u8 bit)
> {
> + struct calling_interface_buffer buffer;
> struct calling_interface_token *token;
> int ret;
>
> @@ -1308,14 +1331,15 @@ static int kbd_set_token_bit(u8 bit)
> if (!token)
> return -EINVAL;
>
> - dell_set_arguments(token->location, token->value, 0, 0);
> - ret = dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_STD);
> + dell_fill_request(&buffer, token->location, token->value, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_TOKEN_WRITE,
> SELECT_TOKEN_STD);
>
> return ret;
> }
>
> static int kbd_get_token_bit(u8 bit)
> {
> + struct calling_interface_buffer buffer;
> struct calling_interface_token *token;
> int ret;
> int val;
> @@ -1327,9 +1351,9 @@ static int kbd_get_token_bit(u8 bit)
> if (!token)
> return -EINVAL;
>
> - dell_set_arguments(token->location, 0, 0, 0);
> - ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_STD);
> - val = buffer->output[1];
> + dell_fill_request(&buffer, token->location, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_TOKEN_READ,
> SELECT_TOKEN_STD);
> + val = buffer.output[1];
>
> if (ret)
> return ret;
> @@ -2046,6 +2070,7 @@ static struct notifier_block dell_laptop
>
> int dell_micmute_led_set(int state)
> {
> + struct calling_interface_buffer buffer;
> struct calling_interface_token *token;
>
> if (state == 0)
> @@ -2058,8 +2083,8 @@ int dell_micmute_led_set(int state)
> if (!token)
> return -ENODEV;
>
> - dell_set_arguments(token->location, token->value, 0, 0);
> - dell_send_request(CLASS_TOKEN_WRITE, SELECT_TOKEN_STD);
> + dell_fill_request(&buffer, token->location, token->value, 0, 0);
> + dell_send_request(&buffer, CLASS_TOKEN_WRITE, SELECT_TOKEN_STD);
>
> return state;
> }
> @@ -2090,13 +2115,6 @@ static int __init dell_init(void)
> if (ret)
> goto fail_platform_device2;
>
> - buffer = kzalloc(sizeof(struct calling_interface_buffer), GFP_KERNEL);
> - if (!buffer) {
> - ret = -ENOMEM;
> - goto fail_buffer;
> - }
> -
> -
> ret = dell_setup_rfkill();
>
> if (ret) {
> @@ -2121,10 +2139,13 @@ static int __init dell_init(void)
>
> token = dell_smbios_find_token(BRIGHTNESS_TOKEN);
> if (token) {
> - dell_set_arguments(token->location, 0, 0, 0);
> - ret = dell_send_request(CLASS_TOKEN_READ, SELECT_TOKEN_AC);
> + struct calling_interface_buffer buffer;
> +
> + dell_fill_request(&buffer, token->location, 0, 0, 0);
> + ret = dell_send_request(&buffer,
> + CLASS_TOKEN_READ, SELECT_TOKEN_AC);
> if (ret)
> - max_intensity = buffer->output[3];
> + max_intensity = buffer.output[3];
> }
>
> if (max_intensity) {
> @@ -2158,8 +2179,6 @@ static int __init dell_init(void)
> fail_get_brightness:
> backlight_device_unregister(dell_backlight_device);
> fail_backlight:
> - kfree(buffer);
> -fail_buffer:
> dell_cleanup_rfkill();
> fail_rfkill:
> platform_device_del(platform_device);
> @@ -2179,7 +2198,6 @@ static void __exit dell_exit(void)
> touchpad_led_exit();
> kbd_led_exit();
> backlight_device_unregister(dell_backlight_device);
> - kfree(buffer);
> dell_cleanup_rfkill();
> if (platform_device) {
> platform_device_unregister(platform_device);
>

\
 
 \ /
  Last update: 2018-03-08 02:15    [W:0.285 / U:25.556 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site