RE: [PATCH 4.15 004/122] platform/x86: dell-laptop: Allocate buffer on heap rather than globally

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);