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

2018-03-07 Thread Greg KH
On Thu, Mar 08, 2018 at 01:14:39AM +, mario.limoncie...@dell.com wrote:
> 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.

Oops, for some reason I thought that was going to have a stable@ tag on
it.  Now queued up.

greg k-h


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

2018-03-07 Thread Greg KH
On Thu, Mar 08, 2018 at 01:14:39AM +, mario.limoncie...@dell.com wrote:
> 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.

Oops, for some reason I thought that was going to have a stable@ tag on
it.  Now queued up.

greg k-h


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

2018-03-07 Thread Mario.Limonciello
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 ; sta...@vger.kernel.org;
> Pali Rohar ; Limonciello, Mario
> ; Andy Shevchenko
> 
> 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 
> 
> 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 
> Signed-off-by: Mario Limonciello 
> Signed-off-by: Andy Shevchenko 
> Signed-off-by: Greg Kroah-Hartman 
> 
> 
> ---
>  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(, 0, 0, 0, 0);
> + ret = dell_send_request(, 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(, 0x2, 0, 0, 0);
> + ret = dell_send_request(, 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(, 1 | (radio<<8) | (disable << 16), 0, 0, 0);
> + ret = dell_send_request(, 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(,
> +1 | (radio << 8) | (block << 16), 0, 0, 0);
> + dell_send_request(, CLASS_INFO, SELECT_RFKILL);
>   } else {
>   /* No hw-switch, sync BIOS state to sw_state */
>   

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

2018-03-07 Thread Mario.Limonciello
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 ; sta...@vger.kernel.org;
> Pali Rohar ; Limonciello, Mario
> ; Andy Shevchenko
> 
> 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 
> 
> 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 
> Signed-off-by: Mario Limonciello 
> Signed-off-by: Andy Shevchenko 
> Signed-off-by: Greg Kroah-Hartman 
> 
> 
> ---
>  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(, 0, 0, 0, 0);
> + ret = dell_send_request(, 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(, 0x2, 0, 0, 0);
> + ret = dell_send_request(, 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(, 1 | (radio<<8) | (disable << 16), 0, 0, 0);
> + ret = dell_send_request(, 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(,
> +1 | (radio << 8) | (block << 16), 0, 0, 0);
> + dell_send_request(, 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