Hi,
Thank you for your patch.
On 11/24/23 10:41, SungHwan Jung wrote:
> The Acer Predator PHN16-71 has the mode key that is used to rotate
> thermal modes or toggle turbo mode with predator sense app (ver. 4) on
> windows.
> This patch includes platform profile and the mode key support for the
> device and also includes a small fix for "WMI_gaming_execute_u64"
> function.
>
> Signed-off-by: SungHwan Jung <onenowy@xxxxxxxxx>
> ---
> drivers/platform/x86/acer-wmi.c | 268 +++++++++++++++++++++++++++++++-
> 1 file changed, 267 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 0e472aa9b..e3650dce0 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -20,6 +20,7 @@
> #include <linux/backlight.h>
> #include <linux/leds.h>
> #include <linux/platform_device.h>
> +#include <linux/platform_profile.h>
> #include <linux/acpi.h>
> #include <linux/i8042.h>
> #include <linux/rfkill.h>
> @@ -62,9 +63,12 @@ MODULE_LICENSE("GPL");
>
> #define ACER_WMID_SET_GAMING_LED_METHODID 2
> #define ACER_WMID_GET_GAMING_LED_METHODID 4
> +#define ACER_WMID_GET_GAMING_SYS_INFO_METHODID 5
> #define ACER_WMID_SET_GAMING_FAN_BEHAVIOR 14
> #define ACER_WMID_SET_GAMING_MISC_SETTING_METHODID 22
>
> +#define ACER_PREDATOR_V4_THERMAL_PROFILE_EC_OFFSET 0x54
> +
> /*
> * Acer ACPI method GUIDs
> */
> @@ -90,6 +94,10 @@ enum acer_wmi_event_ids {
> WMID_GAMING_TURBO_KEY_EVENT = 0x7,
> };
>
> +enum acer_wmi_predator_v4_sys_info_command {
> + ACER_WMID_CMD_GET_PREDATOR_V4_BAT_STATUS = 0x02,
> +};
> +
> static const struct key_entry acer_wmi_keymap[] __initconst = {
> {KE_KEY, 0x01, {KEY_WLAN} }, /* WiFi */
> {KE_KEY, 0x03, {KEY_WLAN} }, /* WiFi */
> @@ -232,6 +240,7 @@ struct hotkey_function_type_aa {
> #define ACER_CAP_TURBO_OC BIT(7)
> #define ACER_CAP_TURBO_LED BIT(8)
> #define ACER_CAP_TURBO_FAN BIT(9)
> +#define ACER_CAP_PLATFORM_PROFILE BIT(10)
When Ilpo set to use tabs for indent here, he meant to do
this in this patch too, I have fixed this up while merging the patch.
I have also found several places where you separate blocks
by multiple empty lines instead of just 1 empty line,
I have also fixed this (reducing things to 1 empty line)
while applying this.
<snip>
> @@ -2245,6 +2499,12 @@ static int acer_platform_probe(struct platform_device *device)
> if (err)
> goto error_rfkill;
>
> + if (has_cap(ACER_CAP_PLATFORM_PROFILE)) {
> + err = acer_platform_profile_setup();
> + if (err)
> + goto error_platform_profile;
> + }
> +
> return err;
>
> error_rfkill:
The error handling here is wrong, on error you need to undo all previous steps,
so your error handling needs to add a label above the first (error_rfkill)
label and then undo the last step before the platform-profile-setup,
specifically call acer_rfkill_exit(); from the new label. IOW the code here
should look like this:
if (has_cap(ACER_CAP_PLATFORM_PROFILE)) {
err = acer_platform_profile_setup();
if (err)
goto error_platform_profile;
}
return err;
error_platform_profile:
acer_rfkill_exit();
error_rfkill:
if (has_cap(ACER_CAP_BRIGHTNESS))
acer_backlight_exit();
...
I have also fixed this up while merging this patch:
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
