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:gre...@linuxfoundation.org]
> Sent: Thursday, March 8, 2018 3:37 AM
> To: linux-kernel@vger.kernel.org
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>; sta...@vger.kernel.org;
> Pali Rohar <pali.ro...@gmail.com>; Limonciello, Mario
> <mario_limoncie...@dell.com>; Andy Shevchenko
> <andriy.shevche...@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.limoncie...@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.ro...@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limoncie...@dell.com>
> Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gre...@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);
> 

Reply via email to