[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ad96404e-60c4-40cf-b287-f81fa85d85b7@gmx.de>
Date: Sun, 15 Dec 2024 21:02:18 +0100
From: Armin Wolf <W_Armin@....de>
To: Joshua Grisham <josh@...huagrisham.com>, corbet@....net,
hdegoede@...hat.com, ilpo.jarvinen@...ux.intel.com, jdelvare@...e.com,
linux@...ck-us.net, platform-driver-x86@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v2] platform/x86: samsung-galaxybook: Add
samsung-galaxybook driver
Am 15.12.24 um 15:13 schrieb Joshua Grisham:
> Thank you all very much for the wealth of information provided in the
> response to the first version of this patch. As there was quite a lot of
> information to look into, I took a few days as I had the time and tried to
> resolve everything that I could; the result is the attached updates to the
> first patch.
>
> I will aim to reply a bit more in detail to the thread and try to address
> each review comment one-by-one, but within this v2 of the patch itself I
> just wanted to mention some highlights of what has been changed with this
> attached update:
>
> 1. Completely removed support for fan speed monitoring (including the
> associated hwmon device) -- I agree, it does make a lot more sense if
> support for fan devices with only _FST method is built in to the ACPI core
> (as my guess is there are MANY devices in the wild like this). This
> actually removes many of the review comments/issues as well which is a bit
> of a win-win!
>
> 2. Took a step back to regroup regarding what the logging within the driver
> should be like if it is "production ready". In pursuit of this, I did the
> following:
> - Removed almost all of the existing print messages
> - Those which remained I have changed to use a relevant dev_* function
> (instead of pr_*) and have moved everything to either warn, error, or
> debug (for messages which myself as a likely initial maintainer of this
> driver would get a lot of help from), as seemed appropriate to me.
> - I removed the built-in hex printing of the ACPI buffers and as
> suggested by @Armin I have created a new tracepoint which can be enabled
> and will provide full debug of all buffers that are being sent and
> received from the ACPI device itself via this driver. Note: As the
> payloads for these devices can be quite large (up to 256 bytes), plus
> that different models behave slightly differently and have different
> features, I have found it very necessary to have the availabily for users
> to send this information to me, whether it is in the form of a tracepoint
> as I have added now, or otherwise.
>
> 3. Everywhere possible, I have tried to implement the devres version of
> various registration functions, and when there was not a devres function
> available then I have tried to make use of devm_add_action_or_reset() to
> add a hook to handle the exits to try and achieve a similar result.
>
> 4. Tried to remove usage of the global pointer galaxybook_ptr, but with
> mixed success.
> - Again, as the hwmon device has been removed, its usage of
> galaxybook_ptr was also removed!
> - From what I can see, I don't think there are actually any existing x86
> platform drivers that do NOT currently use a global pointer variable for
> the battery extension sysfs attributes (even the Dell driver is
> eventually using a static struct of "interface tokens" when you drill
The dell-wmi-ddv driver does manage his battery extension without any global variables.
Could it be that you confused it with another Dell driver?
> all the way down to the bottom) BUT I was able to take some inspiration
> from the Chrome OS battery driver and do think I have managed it quite
> well now in this driver by using a dev_ext_attribute where I have added a
> reference to the needed private data on the attribute itself.
> - For the i8042 filter, again I could not find any instance of any driver
> that seems to NOT be using a global pointer variable to execute any
> actions, and on top of that I was not really able to easily figure out a
> good way to rig up a struct member that would work using container_of. I
> tried several things including trying to use a double pointer to the
> function itself to then fetch the private data struct, but it did not
> really work super well and the logic felt very "heavy" for an IO filter
> in the end. I think a better design for this would be if i8042 filters
> could be installed with extended data so that we can actually provide a
> pointer and then just use it directly in the filter hook function itself.
> So, for now, I have left the galaxybook_ptr in, but it is only used in
> the i8042 filter.
Good point, i think this is an issue within the i8042 driver itself, so using
galaxybook_ptr here is OK for now.
Please submit this v2 patch as its own separate patch as described in
https://docs.kernel.org/process/submitting-patches.html ("The canonical patch format").
If you use "git send-email", you can use the -v2 option to automatically adjust the patch
header.
You can describe the changes you made like this:
<commit message>
...
Signed-off-by: Author <author@...l>
---
V1 -> V2:
- <description of changes>
- ...
path/to/file | 5+++--
...
Thanks,
Armin Wolf
>
> Signed-off-by: Joshua Grisham <josh@...huagrisham.com>
> ---
> .../laptops/samsung-galaxybook.rst | 65 +-
> MAINTAINERS | 2 +
> drivers/platform/x86/Kconfig | 1 -
> .../platform/x86/samsung-galaxybook-trace.h | 51 +
> drivers/platform/x86/samsung-galaxybook.c | 1329 +++++------------
> 5 files changed, 446 insertions(+), 1002 deletions(-)
> create mode 100644 drivers/platform/x86/samsung-galaxybook-trace.h
>
> diff --git a/Documentation/admin-guide/laptops/samsung-galaxybook.rst b/Documentation/admin-guide/laptops/samsung-galaxybook.rst
> index ab12f0001..c174cb255 100644
> --- a/Documentation/admin-guide/laptops/samsung-galaxybook.rst
> +++ b/Documentation/admin-guide/laptops/samsung-galaxybook.rst
> @@ -33,11 +33,10 @@ The following features are currently supported:
> - :ref:`Battery charge control end threshold
> <battery-charge-control-end-threshold>` (stop charging battery at given
> percentage value) implemented as a battery device extension
> -- :ref:`Fan speed <fan-speed>` monitoring via ``fan_speed_rpm`` sysfs attribute
> - plus a new hwmon device
> - :ref:`Settings Attributes <settings-attributes>` to allow control of various
> device settings
> - :ref:`Handling of Fn hotkeys <keyboard-hotkey-actions>` for various actions
> +- :ref:`Tracepoint <tracepoint>` event for debugging ACPI device communication
>
> Because different models of these devices can vary in their features, there is
> logic built within the driver which attempts to test each implemented feature
> @@ -62,7 +61,6 @@ enabling or disabling various features:
> - ``kbd_backlight``: Enable Keyboard Backlight control (default on)
> - ``performance_mode``: Enable Performance Mode control (default on)
> - ``battery_threshold``: Enable battery charge threshold control (default on)
> -- ``fan_speed``: Enable fan speed (default on)
> - ``allow_recording``: Enable control to allow or block access to camera and
> microphone (default on)
> - ``i8042_filter``: Enable capture and execution of keyboard-based hotkey events
> @@ -154,46 +152,6 @@ services as "on" or "off". Otherwise, the device will accept any value between 0
> will set the attribute value to 0 (i.e. 100% will "remove" the end threshold).
>
>
> -.. _fan-speed:
> -
> -Fan speed
> -=========
> -
> -Controlled by parameter: ``fan_speed``
> -
> -The number and type of fans on these devices can vary, and different methods
> -must be used in order to be able to successfully read their status.
> -
> -In cases where Samsung has implemented the standard ACPI method ``_FST`` for a
> -fan device, the other methods in the ACPI specification which would cause
> -the kernel to automatically add the ``fan_speed_rpm`` attribute are not always
> -present. On top of this, it seems that there are some bugs in the firmware that
> -throw an exception when the ``_FST`` method is executed.
> -
> -This platform driver attempts to resolve all PNP fans that are present in the
> -ACPI of supported devices, and add support for reading their speed using the
> -following decision tree:
> -
> -1. Do all 4 required methods exist so that the fan speed should be reported
> - out-of-the-box by ACPI? If yes, then assume this fan is already set up and
> - available.
> -
> -2. Does the method ``_FST`` exist and appears to be working (returns a speed
> - value greater than 0)? If yes, add an attribute ``fan_speed_rpm`` to this fan
> - device and add a fan input channel for it to the hwmon device. The returned
> - value will be directly read from the ``_FST`` method.
> -
> -3. Does the field ``FANS`` (fan speed level) exist on the embedded controller,
> - and the table ``FANT`` (fan speed level table) exist on the fan device? If
> - yes, add the ``fan_speed_rpm`` attribute to this fan device and add a fan
> - input channel for it to the hwmon device. The returned value will be based
> - on a match of the current value of ``FANS`` compared to a list of level
> - speeds from the ``FANT`` table.
> -
> -The fan speed for all supported fans can be monitored using hwmon sensors or by
> -reading the ``fan_speed_rpm`` sysfs attribute of each fan device.
> -
> -
> .. _settings-attributes:
>
> Settings Attributes
> @@ -299,3 +257,24 @@ The Fn+F11 Performance mode hotkey is received as an ACPI notification. It will
> be handled in a similar way as the Fn+F9 and Fn+F10 hotkeys; namely, that the
> keypress will be swallowed by the driver and each press will cycle to the next
> available platform profile.
> +
> +
> +.. _tracepoint:
> +
> +Tracepoint for debugging ACPI communication
> +===========================================
> +
> +A new tracepoint event ``samsung_galaxybook:samsung_galaxybook_acpi`` will
> +provide a trace of the buffers sent to, and received from, the ACPI device
> +methods.
> +
> +Here is an example of how to use it: ::
> +
> + # Enable tracepoint events
> + echo 1 | sudo tee /sys/kernel/tracing/events/samsung_galaxybook/enable
> +
> + # Perform some actions using the driver and then read the result
> + sudo cat /sys/kernel/tracing/trace
> +
> + # Disable tracepoint events when you are finished
> + echo 0 | sudo tee /sys/kernel/tracing/events/samsung_galaxybook/enable
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 21b4fc504..9e3b45cf7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20737,6 +20737,8 @@ SAMSUNG GALAXY BOOK EXTRAS DRIVER
> M: Joshua Grisham <josh@...huagrisham.com>
> L: platform-driver-x86@...r.kernel.org
> S: Maintained
> +F: Documentation/admin-guide/laptops/samsung-galaxybook.rst
> +F: drivers/platform/x86/samsung-galaxybook-trace.h
> F: drivers/platform/x86/samsung-galaxybook.c
>
> SAMSUNG INTERCONNECT DRIVERS
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index b6d28b6a4..03f4fb0e9 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -784,7 +784,6 @@ config SAMSUNG_GALAXYBOOK
> depends on ACPI_BATTERY
> depends on INPUT
> depends on SERIO_I8042
> - depends on HWMON || HWMON = n
> select ACPI_PLATFORM_PROFILE
> select INPUT_SPARSEKMAP
> select NEW_LEDS
> diff --git a/drivers/platform/x86/samsung-galaxybook-trace.h b/drivers/platform/x86/samsung-galaxybook-trace.h
> new file mode 100644
> index 000000000..09ab6dbe6
> --- /dev/null
> +++ b/drivers/platform/x86/samsung-galaxybook-trace.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Samsung Galaxy Book series extras driver tracepoint events
> + *
> + * Copyright (c) 2024 Joshua Grisham <josh@...huagrisham.com>
> + */
> +
> +#if !defined(_TRACE_SAMSUNG_GALAXYBOOK_H_) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_SAMSUNG_GALAXYBOOK_H_
> +
> +#include <linux/types.h>
> +#include <linux/tracepoint.h>
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM samsung_galaxybook
> +
> +#define GALAXYBOOK_TRACE_MAX_ACPI_BUF_LENGTH 0x100
> +
> +TRACE_EVENT(samsung_galaxybook_acpi,
> + TP_PROTO(const char *devname, const char *method, const char *label, u8 *buf, size_t len),
> + TP_ARGS(devname, method, label, buf, len),
> + TP_STRUCT__entry(
> + __string(devname, devname)
> + __string(method, method)
> + __string(label, label)
> + __array(u8, buf, GALAXYBOOK_TRACE_MAX_ACPI_BUF_LENGTH)
> + __field(size_t, len)
> + ),
> + TP_fast_assign(
> + __assign_str(devname);
> + __assign_str(method);
> + __assign_str(label);
> + memcpy(__entry->buf, buf, len);
> + __entry->len = len;
> + ),
> + TP_printk("device: %s, method: %s, %s: %s",
> + __get_str(devname),
> + __get_str(method),
> + __get_str(label),
> + __print_hex(__entry->buf, __entry->len))
> +);
> +
> +#endif
> +
> +#undef TRACE_INCLUDE_PATH
> +#undef TRACE_INCLUDE_FILE
> +
> +#define TRACE_INCLUDE_PATH ../../drivers/platform/x86
> +#define TRACE_INCLUDE_FILE samsung-galaxybook-trace
> +
> +#include <trace/define_trace.h>
> diff --git a/drivers/platform/x86/samsung-galaxybook.c b/drivers/platform/x86/samsung-galaxybook.c
> index ce8b76d91..4bbd33e83 100644
> --- a/drivers/platform/x86/samsung-galaxybook.c
> +++ b/drivers/platform/x86/samsung-galaxybook.c
> @@ -11,11 +11,8 @@
> * Thank you to the authors!
> */
>
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> #include <linux/acpi.h>
> -#include <linux/dmi.h>
> -#include <linux/hwmon.h>
> +#include <linux/err.h>
> #include <linux/i8042.h>
> #include <linux/init.h>
> #include <linux/input.h>
> @@ -24,15 +21,19 @@
> #include <linux/leds.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> -#include <linux/nls.h>
> +#include <linux/mutex.h>
> #include <linux/platform_device.h>
> #include <linux/platform_profile.h>
> #include <linux/serio.h>
> +#include <linux/sysfs.h>
> +#include <linux/uuid.h>
> #include <linux/workqueue.h>
> #include <acpi/battery.h>
>
> -#define SAMSUNG_GALAXYBOOK_CLASS "samsung-galaxybook"
> -#define SAMSUNG_GALAXYBOOK_NAME "Samsung Galaxy Book Extras"
> +#define CREATE_TRACE_POINTS
> +#include "samsung-galaxybook-trace.h"
> +
> +#define DRIVER_NAME "samsung-galaxybook"
>
> /*
> * Module parameters
> @@ -42,21 +43,18 @@ static bool kbd_backlight = true;
> static bool battery_threshold = true;
> static bool performance_mode = true;
> static bool allow_recording = true;
> -static bool fan_speed = true;
> static bool i8042_filter = true;
>
> -module_param(kbd_backlight, bool, 0644);
> +module_param(kbd_backlight, bool, 0);
> MODULE_PARM_DESC(kbd_backlight, "Enable Keyboard Backlight control (default on)");
> -module_param(battery_threshold, bool, 0644);
> +module_param(battery_threshold, bool, 0);
> MODULE_PARM_DESC(battery_threshold, "Enable battery charge threshold control (default on)");
> -module_param(performance_mode, bool, 0644);
> +module_param(performance_mode, bool, 0);
> MODULE_PARM_DESC(performance_mode, "Enable Performance Mode control (default on)");
> -module_param(allow_recording, bool, 0644);
> +module_param(allow_recording, bool, 0);
> MODULE_PARM_DESC(allow_recording,
> "Enable control to allow or block access to camera and microphone (default on)");
> -module_param(fan_speed, bool, 0644);
> -MODULE_PARM_DESC(fan_speed, "Enable fan speed (default on)");
> -module_param(i8042_filter, bool, 0644);
> +module_param(i8042_filter, bool, 0);
> MODULE_PARM_DESC(i8042_filter, "Enable capturing keyboard hotkey events (default on)");
>
> /*
> @@ -72,49 +70,25 @@ static const struct acpi_device_id galaxybook_device_ids[] = {
> };
> MODULE_DEVICE_TABLE(acpi, galaxybook_device_ids);
>
> -static const struct dmi_system_id galaxybook_dmi_ids[] = {
> - {
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD."),
> - DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /* Notebook */
> - },
> - },
> - {}
> -};
> -
> -struct galaxybook_fan {
> - struct acpi_device fan;
> - char *description;
> - bool supports_fst;
> - unsigned int *fan_speeds;
> - int fan_speeds_count;
> - struct dev_ext_attribute fan_speed_rpm_ext_attr;
> -};
> -
> -#define MAX_FAN_COUNT 5
> -
> struct samsung_galaxybook {
> struct platform_device *platform;
> struct acpi_device *acpi;
> + struct mutex acpi_lock;
>
> struct led_classdev kbd_backlight;
> - struct work_struct kbd_backlight_hotkey_work;
>
> struct input_dev *input;
> - struct key_entry *keymap;
> -
> - u8 *profile_performance_modes;
> - struct platform_profile_handler profile_handler;
> - struct work_struct performance_mode_hotkey_work;
> + struct mutex input_lock;
>
> + void *i8042_filter_ptr;
> + struct work_struct kbd_backlight_hotkey_work;
> struct work_struct allow_recording_hotkey_work;
>
> - struct galaxybook_fan fans[MAX_FAN_COUNT];
> - int fans_count;
> + struct acpi_battery_hook battery_hook;
> + struct dev_ext_attribute battery_charge_control_attr;
>
> -#if IS_ENABLED(CONFIG_HWMON)
> - struct device *hwmon;
> -#endif
> + u8 *profile_performance_modes;
> + struct platform_profile_handler profile_handler;
> };
> static struct samsung_galaxybook *galaxybook_ptr;
>
> @@ -217,10 +191,6 @@ static const guid_t performance_mode_guid_value =
> #define ACPI_METHOD_SETTINGS "CSFI"
> #define ACPI_METHOD_PERFORMANCE_MODE "CSXI"
>
> -#define ACPI_FAN_DEVICE_ID "PNP0C0B"
> -#define ACPI_FAN_SPEED_LIST "FANT"
> -#define ACPI_FAN_SPEED_VALUE "\\_SB.PC00.LPCB.H_EC.FANS"
> -
> #define KBD_BACKLIGHT_MAX_BRIGHTNESS 3
>
> #define ACPI_NOTIFY_BATTERY_STATE_CHANGED 0x61
> @@ -245,126 +215,72 @@ static const struct key_entry galaxybook_acpi_keymap[] = {
> * ACPI method handling
> */
>
> -#define pr_debug_prefixed(...) pr_debug("[DEBUG] " __VA_ARGS__)
> -
> -#define print_acpi_object_buffer_debug(header_str, buf_ptr, buf_len) \
> - do { \
> - pr_debug_prefixed("%s\n", header_str); \
> - print_hex_dump_debug("samsung_galaxybook: [DEBUG] ", \
> - DUMP_PREFIX_NONE, 16, 1, buf_ptr, \
> - buf_len, false); \
> - } while (0)
> -
> -static char *get_acpi_device_description(struct acpi_device *acpi_dev)
> -{
> - struct acpi_buffer str_buf = { ACPI_ALLOCATE_BUFFER, NULL };
> - union acpi_object *str_obj;
> - struct acpi_buffer name_buf = { ACPI_ALLOCATE_BUFFER, NULL };
> - acpi_status status;
> - int result;
> -
> - /* first try to get value of _STR (and also convert it to utf8) */
> - if (!acpi_has_method(acpi_dev->handle, "_STR"))
> - goto use_name;
> - status = acpi_evaluate_object_typed(acpi_dev->handle, "_STR", NULL,
> - &str_buf, ACPI_TYPE_BUFFER);
> - if (ACPI_SUCCESS(status) && str_buf.length > 0) {
> - str_obj = str_buf.pointer;
> - char *buf = kzalloc(sizeof(*buf) * str_obj->buffer.length, GFP_KERNEL);
> -
> - result = utf16s_to_utf8s((wchar_t *)str_obj->buffer.pointer,
> - str_obj->buffer.length,
> - UTF16_LITTLE_ENDIAN, buf,
> - PAGE_SIZE - 1);
> - kfree(str_obj);
> - if (result > 0)
> - return buf;
> - /* if no result then free buf */
> - kfree(buf);
> - }
> -
> - kfree(str_buf.pointer);
> -
> -use_name:
> - /* if _STR is missing then just use the device name */
> - status = acpi_get_name(acpi_dev->handle, ACPI_SINGLE_NAME, &name_buf);
> - if (ACPI_SUCCESS(status) && name_buf.length > 0)
> - return name_buf.pointer;
> -
> - kfree(name_buf.pointer);
> -
> - return NULL;
> -}
> -
> static int galaxybook_acpi_method(struct samsung_galaxybook *galaxybook, acpi_string method,
> - struct sawb *buf, u32 len, const char *purpose_str,
> - struct sawb *ret)
> + struct sawb *in_buf, size_t len, const char *purpose_str,
> + struct sawb *out_buf)
> {
> + struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
> union acpi_object in_obj, *out_obj;
> struct acpi_object_list input;
> - struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
> acpi_status status;
> + int err;
> +
> + mutex_lock(&galaxybook->acpi_lock);
>
> in_obj.type = ACPI_TYPE_BUFFER;
> in_obj.buffer.length = len;
> - in_obj.buffer.pointer = (u8 *)buf;
> + in_obj.buffer.pointer = (u8 *)in_buf;
>
> input.count = 1;
> input.pointer = &in_obj;
>
> - print_acpi_object_buffer_debug(purpose_str, in_obj.buffer.pointer, in_obj.buffer.length);
> + trace_samsung_galaxybook_acpi(dev_name(&galaxybook->acpi->dev), method, purpose_str,
> + in_obj.buffer.pointer, in_obj.buffer.length);
>
> - status = acpi_evaluate_object(galaxybook->acpi->handle, method, &input, &output);
> + status = acpi_evaluate_object_typed(galaxybook->acpi->handle, method, &input, &output,
> + ACPI_TYPE_BUFFER);
>
> if (ACPI_FAILURE(status)) {
> - pr_err("failed %s with ACPI method %s; got %s\n", purpose_str, method,
> - acpi_format_exception(status));
> - return status;
> + dev_err(&galaxybook->acpi->dev, "failed %s with ACPI method %s; got %s\n",
> + purpose_str, method, acpi_format_exception(status));
> + err = -ENXIO;
> + goto out_free;
> }
>
> out_obj = output.pointer;
>
> - if (out_obj->type != ACPI_TYPE_BUFFER) {
> - pr_err("failed %s with ACPI method %s; response was not a buffer\n",
> - purpose_str, method);
> - status = -EIO;
> - goto out_free;
> - }
> -
> - print_acpi_object_buffer_debug("response was: ", out_obj->buffer.pointer,
> - out_obj->buffer.length);
> + trace_samsung_galaxybook_acpi(dev_name(&galaxybook->acpi->dev), method, "response",
> + out_obj->buffer.pointer, out_obj->buffer.length);
>
> - if (out_obj->buffer.length != len) {
> - pr_err("failed %s with ACPI method %s; response length mismatch\n",
> + if (out_obj->buffer.length != len || out_obj->buffer.length < SAWB_GUNM_POS + 1) {
> + dev_err(&galaxybook->acpi->dev, "failed %s with ACPI method %s; "
> + "response length mismatch\n",
> purpose_str, method);
> - status = -EIO;
> - goto out_free;
> - }
> - if (out_obj->buffer.length < SAWB_GUNM_POS + 1) {
> - pr_err("failed %s with ACPI method %s; response from device was too short\n",
> - purpose_str, method);
> - status = -EIO;
> + err = -ETOOSMALL;
> goto out_free;
> }
> if (out_obj->buffer.pointer[SAWB_RFLG_POS] != RFLG_SUCCESS) {
> - pr_err("failed %s with ACPI method %s; "
> + dev_err(&galaxybook->acpi->dev, "failed %s with ACPI method %s; "
> "device did not respond with success code 0x%x\n",
> purpose_str, method, RFLG_SUCCESS);
> - status = -EIO;
> + err = -EIO;
> goto out_free;
> }
> if (out_obj->buffer.pointer[SAWB_GUNM_POS] == GUNM_FAIL) {
> - pr_err("failed %s with ACPI method %s; device responded with failure code 0x%x\n",
> + dev_err(&galaxybook->acpi->dev,
> + "failed %s with ACPI method %s; device responded with failure code 0x%x\n",
> purpose_str, method, GUNM_FAIL);
> - status = -EIO;
> + err = -EIO;
> goto out_free;
> }
>
> - memcpy(ret, out_obj->buffer.pointer, len);
> + memcpy(out_buf, out_obj->buffer.pointer, len);
> + err = 0;
>
> out_free:
> - kfree(output.pointer);
> - return status;
> + kfree(out_obj);
> + mutex_unlock(&galaxybook->acpi_lock);
> + return err;
> }
>
> static int galaxybook_enable_acpi_feature(struct samsung_galaxybook *galaxybook, const u16 sasb)
> @@ -411,8 +327,6 @@ static int kbd_backlight_acpi_set(struct samsung_galaxybook *galaxybook,
>
> galaxybook->kbd_backlight.brightness = brightness;
>
> - pr_debug_prefixed("set kbd_backlight brightness to %d\n", brightness);
> -
> return 0;
> }
>
> @@ -434,8 +348,6 @@ static int kbd_backlight_acpi_get(struct samsung_galaxybook *galaxybook,
> *brightness = buf.gunm;
> galaxybook->kbd_backlight.brightness = buf.gunm;
>
> - pr_debug_prefixed("current kbd_backlight brightness is %d\n", buf.gunm);
> -
> return 0;
> }
>
> @@ -444,13 +356,8 @@ static int kbd_backlight_store(struct led_classdev *led,
> {
> struct samsung_galaxybook *galaxybook =
> container_of(led, struct samsung_galaxybook, kbd_backlight);
> - int err;
> -
> - err = kbd_backlight_acpi_set(galaxybook, brightness);
> - if (err)
> - return err;
>
> - return 0;
> + return kbd_backlight_acpi_set(galaxybook, brightness);
> }
>
> static enum led_brightness kbd_backlight_show(struct led_classdev *led)
> @@ -469,8 +376,8 @@ static enum led_brightness kbd_backlight_show(struct led_classdev *led)
>
> static int galaxybook_kbd_backlight_init(struct samsung_galaxybook *galaxybook)
> {
> - enum led_brightness brightness;
> struct led_init_data init_data = {};
> + enum led_brightness brightness;
> int err;
>
> err = galaxybook_enable_acpi_feature(galaxybook, SASB_KBD_BACKLIGHT);
> @@ -482,27 +389,17 @@ static int galaxybook_kbd_backlight_init(struct samsung_galaxybook *galaxybook)
> if (err)
> return err;
>
> - init_data.devicename = SAMSUNG_GALAXYBOOK_CLASS;
> - init_data.default_label = ":kbd_backlight";
> + init_data.devicename = DRIVER_NAME;
> + init_data.default_label = ":" LED_FUNCTION_KBD_BACKLIGHT;
> init_data.devname_mandatory = true;
>
> - galaxybook->kbd_backlight = (struct led_classdev){
> - .brightness_get = kbd_backlight_show,
> - .brightness_set_blocking = kbd_backlight_store,
> - .flags = LED_BRIGHT_HW_CHANGED,
> - .max_brightness = KBD_BACKLIGHT_MAX_BRIGHTNESS,
> - };
> -
> - pr_info("registering LED class using default name of %s:%s\n",
> - init_data.devicename, init_data.default_label);
> -
> - return led_classdev_register_ext(&galaxybook->platform->dev, &galaxybook->kbd_backlight,
> - &init_data);
> -}
> + galaxybook->kbd_backlight.brightness_get = kbd_backlight_show;
> + galaxybook->kbd_backlight.brightness_set_blocking = kbd_backlight_store;
> + galaxybook->kbd_backlight.flags = LED_BRIGHT_HW_CHANGED;
> + galaxybook->kbd_backlight.max_brightness = KBD_BACKLIGHT_MAX_BRIGHTNESS;
>
> -static void galaxybook_kbd_backlight_exit(struct samsung_galaxybook *galaxybook)
> -{
> - led_classdev_unregister(&galaxybook->kbd_backlight);
> + return devm_led_classdev_register_ext(&galaxybook->platform->dev,
> + &galaxybook->kbd_backlight, &init_data);
> }
>
> /*
> @@ -514,23 +411,16 @@ static void galaxybook_kbd_backlight_exit(struct samsung_galaxybook *galaxybook)
> static int start_on_lid_open_acpi_set(struct samsung_galaxybook *galaxybook, const bool value)
> {
> struct sawb buf = { 0 };
> - int err;
>
> buf.safn = SAFN;
> buf.sasb = SASB_POWER_MANAGEMENT;
> buf.gunm = GUNM_POWER_MANAGEMENT;
> buf.guds[0] = GUDS_START_ON_LID_OPEN;
> buf.guds[1] = GUDS_START_ON_LID_OPEN_SET;
> - buf.guds[2] = value;
> + buf.guds[2] = value ? 1 : 0;
>
> - err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS,
> - "setting start_on_lid_open", &buf);
> - if (err)
> - return err;
> -
> - pr_debug_prefixed("turned start_on_lid_open %s\n", value ? "on (1)" : "off (0)");
> -
> - return 0;
> + return galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS,
> + "setting start_on_lid_open", &buf);
> }
>
> static int start_on_lid_open_acpi_get(struct samsung_galaxybook *galaxybook, bool *value)
> @@ -551,9 +441,6 @@ static int start_on_lid_open_acpi_get(struct samsung_galaxybook *galaxybook, boo
>
> *value = buf.guds[1];
>
> - pr_debug_prefixed("start_on_lid_open is currently %s\n",
> - (buf.guds[1] ? "on (1)" : "off (0)"));
> -
> return 0;
> }
>
> @@ -564,9 +451,13 @@ static ssize_t start_on_lid_open_store(struct device *dev, struct device_attribu
> bool value;
> int err;
>
> - if (!count || kstrtobool(buffer, &value))
> + if (!count)
> return -EINVAL;
>
> + err = kstrtobool(buffer, &value);
> + if (err)
> + return err;
> +
> err = start_on_lid_open_acpi_set(galaxybook, value);
> if (err)
> return err;
> @@ -595,20 +486,13 @@ static DEVICE_ATTR_RW(start_on_lid_open);
> static int usb_charge_acpi_set(struct samsung_galaxybook *galaxybook, const bool value)
> {
> struct sawb buf = { 0 };
> - int err;
>
> buf.safn = SAFN;
> buf.sasb = SASB_USB_CHARGE_SET;
> buf.gunm = value ? GUNM_USB_CHARGE_ON : GUNM_USB_CHARGE_OFF;
>
> - err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS,
> - "setting usb_charge", &buf);
> - if (err)
> - return err;
> -
> - pr_debug_prefixed("turned usb_charge %s\n", value ? "on (1)" : "off (0)");
> -
> - return 0;
> + return galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS,
> + "setting usb_charge", &buf);
> }
>
> static int usb_charge_acpi_get(struct samsung_galaxybook *galaxybook, bool *value)
> @@ -627,8 +511,6 @@ static int usb_charge_acpi_get(struct samsung_galaxybook *galaxybook, bool *valu
>
> *value = buf.gunm;
>
> - pr_debug_prefixed("usb_charge is currently %s\n", (buf.gunm ? "on (1)" : "off (0)"));
> -
> return 0;
> }
>
> @@ -639,9 +521,13 @@ static ssize_t usb_charge_store(struct device *dev, struct device_attribute *att
> bool value;
> int err;
>
> - if (!count || kstrtobool(buffer, &value))
> + if (!count)
> return -EINVAL;
>
> + err = kstrtobool(buffer, &value);
> + if (err)
> + return err;
> +
> err = usb_charge_acpi_set(galaxybook, value);
> if (err)
> return err;
> @@ -669,21 +555,14 @@ static DEVICE_ATTR_RW(usb_charge);
> static int allow_recording_acpi_set(struct samsung_galaxybook *galaxybook, const bool value)
> {
> struct sawb buf = { 0 };
> - int err;
>
> buf.safn = SAFN;
> buf.sasb = SASB_ALLOW_RECORDING;
> buf.gunm = GUNM_SET;
> - buf.guds[0] = value;
> -
> - err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS,
> - "setting allow_recording", &buf);
> - if (err)
> - return err;
> -
> - pr_debug_prefixed("turned allow_recording %s\n", value ? "on (1)" : "off (0)");
> + buf.guds[0] = value ? 1 : 0;
>
> - return 0;
> + return galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS,
> + "setting allow_recording", &buf);
> }
>
> static int allow_recording_acpi_get(struct samsung_galaxybook *galaxybook, bool *value)
> @@ -700,9 +579,7 @@ static int allow_recording_acpi_get(struct samsung_galaxybook *galaxybook, bool
> if (err)
> return err;
>
> - *value = buf.gunm;
> -
> - pr_debug_prefixed("allow_recording is currently %s\n", (buf.gunm ? "on (1)" : "off (0)"));
> + *value = buf.gunm == 1;
>
> return 0;
> }
> @@ -714,9 +591,13 @@ static ssize_t allow_recording_store(struct device *dev, struct device_attribute
> bool value;
> int err;
>
> - if (!count || kstrtobool(buffer, &value))
> + if (!count)
> return -EINVAL;
>
> + err = kstrtobool(buffer, &value);
> + if (err)
> + return err;
> +
> err = allow_recording_acpi_set(galaxybook, value);
> if (err)
> return err;
> @@ -739,36 +620,78 @@ static ssize_t allow_recording_show(struct device *dev, struct device_attribute
>
> static DEVICE_ATTR_RW(allow_recording);
>
> +static umode_t galaxybook_attr_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev);
> + bool value;
> + int err;
> +
> + if (attr == &dev_attr_start_on_lid_open.attr) {
> + err = start_on_lid_open_acpi_get(galaxybook, &value);
> + return err ? 0 : attr->mode;
> + }
> +
> + if (attr == &dev_attr_usb_charge.attr) {
> + err = usb_charge_acpi_get(galaxybook, &value);
> + return err ? 0 : attr->mode;
> + }
> +
> + if (attr == &dev_attr_allow_recording.attr) {
> + if (!allow_recording)
> + return 0;
> + err = galaxybook_enable_acpi_feature(galaxybook, SASB_ALLOW_RECORDING);
> + if (err) {
> + dev_dbg(&galaxybook->platform->dev,
> + "failed to initialize ACPI allow_recording feature\n");
> + allow_recording = false;
> + return 0;
> + }
> + err = allow_recording_acpi_get(galaxybook, &value);
> + if (err) {
> + allow_recording = false;
> + return 0;
> + }
> + return attr->mode;
> + }
> +
> + return attr->mode;
> +}
> +
> +static struct attribute *galaxybook_attrs[] = {
> + &dev_attr_start_on_lid_open.attr,
> + &dev_attr_usb_charge.attr,
> + &dev_attr_allow_recording.attr,
> +};
> +
> +static const struct attribute_group galaxybook_attrs_group = {
> + .attrs = galaxybook_attrs,
> + .is_visible = galaxybook_attr_is_visible,
> +};
> +
> /*
> * Battery Extension (adds charge_control_end_threshold to the battery device)
> */
>
> -static int charge_control_end_threshold_acpi_set(struct samsung_galaxybook *galaxybook,
> - const u8 value)
> +static int charge_control_end_threshold_acpi_set(struct samsung_galaxybook *galaxybook, u8 value)
> {
> struct sawb buf = { 0 };
> - int err;
>
> if (value > 100)
> return -EINVAL;
> + /* if setting to 100, should be set to 0 (no threshold) */
> + if (value == 100)
> + value = 0;
>
> buf.safn = SAFN;
> buf.sasb = SASB_POWER_MANAGEMENT;
> buf.gunm = GUNM_POWER_MANAGEMENT;
> buf.guds[0] = GUDS_BATTERY_CHARGE_CONTROL;
> buf.guds[1] = GUDS_BATTERY_CHARGE_CONTROL_SET;
> + buf.guds[2] = value;
>
> - buf.guds[2] = (value == 100 ? 0 : value); /* if setting to 100, should be set to 0 (off) */
> -
> - err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS,
> - "setting battery charge_control_end_threshold", &buf);
> - if (err)
> - return err;
> -
> - pr_debug_prefixed("set battery charge_control_end_threshold to %d\n",
> - (value == 100 ? 0 : value));
> -
> - return 0;
> + return galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS,
> + "setting battery charge_control_end_threshold", &buf);
> }
>
> static int charge_control_end_threshold_acpi_get(struct samsung_galaxybook *galaxybook, u8 *value)
> @@ -789,23 +712,28 @@ static int charge_control_end_threshold_acpi_get(struct samsung_galaxybook *gala
>
> *value = buf.guds[1];
>
> - pr_debug_prefixed("battery charge control is currently %s; "
> - "battery charge_control_end_threshold is %d\n",
> - (buf.guds[1] > 0 ? "on" : "off"), buf.guds[1]);
> -
> return 0;
> }
>
> static ssize_t charge_control_end_threshold_store(struct device *dev, struct device_attribute *attr,
> const char *buffer, size_t count)
> {
> + struct dev_ext_attribute *ea = container_of(attr, struct dev_ext_attribute, attr);
> + struct samsung_galaxybook *galaxybook = ea->var;
> u8 value;
> int err;
>
> - if (!count || kstrtou8(buffer, 0, &value))
> + if (!galaxybook)
> + return -ENODEV;
> +
> + if (!count)
> return -EINVAL;
>
> - err = charge_control_end_threshold_acpi_set(galaxybook_ptr, value);
> + err = kstrtou8(buffer, 0, &value);
> + if (err)
> + return err;
> +
> + err = charge_control_end_threshold_acpi_set(galaxybook, value);
> if (err)
> return err;
>
> @@ -815,39 +743,42 @@ static ssize_t charge_control_end_threshold_store(struct device *dev, struct dev
> static ssize_t charge_control_end_threshold_show(struct device *dev, struct device_attribute *attr,
> char *buffer)
> {
> + struct dev_ext_attribute *ea = container_of(attr, struct dev_ext_attribute, attr);
> + struct samsung_galaxybook *galaxybook = ea->var;
> u8 value;
> int err;
>
> - err = charge_control_end_threshold_acpi_get(galaxybook_ptr, &value);
> + if (!galaxybook)
> + return -ENODEV;
> +
> + err = charge_control_end_threshold_acpi_get(galaxybook, &value);
> if (err)
> return err;
>
> return sysfs_emit(buffer, "%d\n", value);
> }
>
> -static DEVICE_ATTR_RW(charge_control_end_threshold);
> -
> static int galaxybook_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook)
> {
> - if (device_create_file(&battery->dev, &dev_attr_charge_control_end_threshold))
> - return -ENODEV;
> - return 0;
> + struct samsung_galaxybook *galaxybook =
> + container_of(hook, struct samsung_galaxybook, battery_hook);
> +
> + return device_create_file(&battery->dev, &galaxybook->battery_charge_control_attr.attr);
> }
>
> static int galaxybook_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
> {
> - device_remove_file(&battery->dev, &dev_attr_charge_control_end_threshold);
> + struct samsung_galaxybook *galaxybook =
> + container_of(hook, struct samsung_galaxybook, battery_hook);
> +
> + device_remove_file(&battery->dev, &galaxybook->battery_charge_control_attr.attr);
> return 0;
> }
>
> -static struct acpi_battery_hook galaxybook_battery_hook = {
> - .add_battery = galaxybook_battery_add,
> - .remove_battery = galaxybook_battery_remove,
> - .name = "Samsung Galaxy Book Battery Extension",
> -};
> -
> static int galaxybook_battery_threshold_init(struct samsung_galaxybook *galaxybook)
> {
> + struct acpi_battery_hook *hook;
> + struct device_attribute *attr;
> u8 value;
> int err;
>
> @@ -855,397 +786,22 @@ static int galaxybook_battery_threshold_init(struct samsung_galaxybook *galaxybo
> if (err)
> return err;
>
> - battery_hook_register(&galaxybook_battery_hook);
> - return 0;
> -}
> -
> -static void galaxybook_battery_threshold_exit(struct samsung_galaxybook *galaxybook)
> -{
> - battery_hook_unregister(&galaxybook_battery_hook);
> -}
> -
> -/*
> - * Fan speed
> - */
> -
> -static int fan_speed_get_fst(struct galaxybook_fan *fan, unsigned int *speed)
> -{
> - struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
> - union acpi_object *response_obj = NULL;
> - acpi_status status;
> - int ret = 0;
> -
> - status = acpi_evaluate_object(fan->fan.handle, "_FST", NULL, &response);
> - if (ACPI_FAILURE(status)) {
> - pr_err("Get fan state failed\n");
> - return -ENODEV;
> - }
> -
> - response_obj = response.pointer;
> - if (!response_obj || response_obj->type != ACPI_TYPE_PACKAGE ||
> - response_obj->package.count != 3 ||
> - response_obj->package.elements[2].type != ACPI_TYPE_INTEGER) {
> - pr_err("Invalid _FST data\n");
> - ret = -EINVAL;
> - goto out_free;
> - }
> -
> - *speed = response_obj->package.elements[2].integer.value;
> -
> - pr_debug_prefixed("fan device %s (%s) reporting fan speed of %d\n",
> - dev_name(&fan->fan.dev), fan->description, *speed);
> -
> -out_free:
> - ACPI_FREE(response.pointer);
> - return ret;
> -}
> -
> -static int fan_speed_get_fans(struct galaxybook_fan *fan, unsigned int *speed)
> -{
> - struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
> - union acpi_object *response_obj = NULL;
> - acpi_status status;
> - int ret = 0;
> - int speed_level = -1;
> -
> - status = acpi_evaluate_object(NULL, ACPI_FAN_SPEED_VALUE, NULL, &response);
> - if (ACPI_FAILURE(status)) {
> - pr_err("Get fan state failed\n");
> - return -ENODEV;
> - }
> -
> - response_obj = response.pointer;
> - if (!response_obj || response_obj->type != ACPI_TYPE_INTEGER ||
> - response_obj->integer.value > INT_MAX ||
> - (int)response_obj->integer.value > fan->fan_speeds_count) {
> - pr_err("invalid fan speed data\n");
> - ret = -EINVAL;
> - goto out_free;
> - }
> -
> - speed_level = (int)response_obj->integer.value;
> - *speed = fan->fan_speeds[speed_level];
> -
> - pr_debug_prefixed("fan device %s (%s) reporting fan speed of %d (level %d)\n",
> - dev_name(&fan->fan.dev), fan->description, *speed, speed_level);
> -
> -out_free:
> - ACPI_FREE(response.pointer);
> - return ret;
> -}
> -
> -static int fan_speed_get(struct galaxybook_fan *fan, unsigned int *speed)
> -{
> - if (!fan)
> - return -ENODEV;
> - if (fan->supports_fst)
> - return fan_speed_get_fst(fan, speed);
> - else
> - return fan_speed_get_fans(fan, speed);
> -}
> -
> -static ssize_t fan_speed_rpm_show(struct device *dev, struct device_attribute *attr, char *buffer)
> -{
> - struct dev_ext_attribute *ea = container_of(attr, struct dev_ext_attribute, attr);
> - struct galaxybook_fan *fan = ea->var;
> - unsigned int speed;
> - int ret = 0;
> -
> - if (!fan)
> - return -ENODEV;
> -
> - ret = fan_speed_get(fan, &speed);
> - if (ret)
> - return ret;
> -
> - return sysfs_emit(buffer, "%u\n", speed);
> -}
> -
> -static int __init fan_speed_list_init(acpi_handle handle, struct galaxybook_fan *fan)
> -{
> - struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
> - union acpi_object *response_obj = NULL;
> - acpi_status status;
> - unsigned int speed;
> - int i;
> -
> - status = acpi_evaluate_object(handle, ACPI_FAN_SPEED_LIST, NULL, &response);
> - if (ACPI_FAILURE(status)) {
> - pr_err("failed to read fan speed list\n");
> - return -ENODEV;
> - }
> -
> - response_obj = response.pointer;
> - if (!response_obj || response_obj->type != ACPI_TYPE_PACKAGE ||
> - response_obj->package.count == 0) {
> - pr_err("invalid fan speed list data\n");
> - status = -EINVAL;
> - goto out_free;
> - }
> -
> - /*
> - * fan_speeds[] starts with a hard-coded 0 (fan is off), then has some "funny" logic:
> - * - fetch the speed level values read in from FANT and add 0x0a to each value
> - * - _FST method in the DSDT seems to indicate that level 3 and 4 should have same value,
> - * however real-life observation suggests that the speed actually does change
> - * - _FST says that level 5 should give the 4th value from FANT but it seems significantly
> - * louder -- we will just "guess" it is 1000 RPM faster than the highest value from FANT?
> - */
> -
> - fan->fan_speeds = kzalloc(sizeof(unsigned int) * (response_obj->package.count + 2),
> - GFP_KERNEL);
> - if (!fan->fan_speeds)
> - return -ENOMEM;
> -
> - /* hard-coded "off" value (0) */
> - fan->fan_speeds[0] = 0;
> - fan->fan_speeds_count = 1;
> -
> - /* fetch and assign the next values from FANT response */
> - i = 0;
> - for (i = 1; i <= response_obj->package.count; i++) {
> - if (response_obj->package.elements[i - 1].type != ACPI_TYPE_INTEGER) {
> - pr_err("invalid fan speed list value at position %d; "
> - "expected type %d, got type %d\n",
> - i - 1, ACPI_TYPE_INTEGER,
> - response_obj->package.elements[i - 1].type);
> - status = -EINVAL;
> - goto err_fan_speeds_free;
> - }
> - fan->fan_speeds[i] = response_obj->package.elements[i - 1].integer.value + 0x0a;
> - fan->fan_speeds_count++;
> - }
> -
> - /* add the missing final level where we "guess" 1000 RPM faster than highest from FANT */
> - if (fan->fan_speeds_count > 1) {
> - fan->fan_speeds[i] = fan->fan_speeds[i - 1] + 1000;
> - fan->fan_speeds_count++;
> - }
> -
> - /* test that it actually works to read the speed, otherwise the init should fail */
> - status = fan_speed_get_fans(fan, &speed);
> - if (ACPI_FAILURE(status)) {
> - pr_err("failed to read fan speed level from FANS\n");
> - goto err_fan_speeds_free;
> - }
> -
> - pr_info("initialized fan speed reporting for device %s (%s) with the following levels:\n",
> - dev_name(&fan->fan.dev), fan->description);
> - for (i = 0; i < fan->fan_speeds_count; i++)
> - pr_info(" %s (%s) fan speed level %d = %d\n",
> - dev_name(&fan->fan.dev), fan->description, i, fan->fan_speeds[i]);
> + hook = &galaxybook->battery_hook;
> + hook->add_battery = galaxybook_battery_add;
> + hook->remove_battery = galaxybook_battery_remove;
> + hook->name = "Samsung Galaxy Book Battery Extension";
>
> -out_free:
> - ACPI_FREE(response.pointer);
> - return status;
> + attr = &galaxybook->battery_charge_control_attr.attr;
> + attr->attr.name = "charge_control_end_threshold";
> + attr->attr.mode = 0644;
> + attr->show = charge_control_end_threshold_show;
> + attr->store = charge_control_end_threshold_store;
> + /* ext attr var points to this galaxybook so it can used in show and store */
> + galaxybook->battery_charge_control_attr.var = galaxybook;
>
> -err_fan_speeds_free:
> - kfree(fan->fan_speeds);
> - goto out_free;
> + return devm_battery_hook_register(&galaxybook->platform->dev, &galaxybook->battery_hook);
> }
>
> -static acpi_status galaxybook_add_fan(acpi_handle handle, u32 level, void *context,
> - void **return_value)
> -{
> - struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
> - struct samsung_galaxybook *galaxybook = context;
> - struct galaxybook_fan *fan;
> - int speed = -1;
> -
> - pr_info("found fan device %s\n", dev_name(&adev->dev));
> -
> - /* if fan meets acpi4 fan device requirements, assume it is added already under ACPI */
> - if (acpi_has_method(handle, "_FIF") &&
> - acpi_has_method(handle, "_FPS") &&
> - acpi_has_method(handle, "_FSL") &&
> - acpi_has_method(handle, "_FST")) {
> - pr_info("fan device %s should already be available as an ACPI fan; skipping\n",
> - dev_name(&adev->dev));
> - return 0;
> - }
> -
> - if (galaxybook->fans_count >= MAX_FAN_COUNT) {
> - pr_err("maximum number of %d fans has already been reached\n", MAX_FAN_COUNT);
> - return 0;
> - }
> -
> - fan = &galaxybook->fans[galaxybook->fans_count];
> - fan->fan = *adev;
> - fan->description = get_acpi_device_description(&fan->fan);
> -
> - /* try to get speed from _FST */
> - if (ACPI_FAILURE(fan_speed_get_fst(fan, &speed))) {
> - pr_debug_prefixed("_FST is present but failed on fan device %s (%s); "
> - "will attempt to add fan speed support using FANT and FANS\n",
> - dev_name(&fan->fan.dev), fan->description);
> - fan->supports_fst = false;
> - }
> - /* if speed was 0 and FANT and FANS exist, they should be used anyway due to bugs in ACPI */
> - else if (speed <= 0 &&
> - acpi_has_method(handle, ACPI_FAN_SPEED_LIST) &&
> - acpi_has_method(NULL, ACPI_FAN_SPEED_VALUE)) {
> - pr_debug_prefixed("_FST is present on fan device %s (%s) but returned value of 0; "
> - "will attempt to add fan speed support using FANT and FANS\n",
> - dev_name(&fan->fan.dev), fan->description);
> - fan->supports_fst = false;
> - } else {
> - fan->supports_fst = true;
> - }
> -
> - if (!fan->supports_fst) {
> - /* since FANS is a field on the EC, it does not make sense to use more than once */
> - for (int i = 0; i < galaxybook->fans_count; i++) {
> - if (!galaxybook->fans[i].supports_fst) {
> - pr_err("more than one fan using FANS is not supported\n");
> - return 0;
> - }
> - }
> - if (ACPI_FAILURE(fan_speed_list_init(handle, fan))) {
> - pr_err("unable to initialize fan speeds for fan device %s (%s)\n",
> - dev_name(&fan->fan.dev), fan->description);
> - return 0;
> - }
> - } else {
> - pr_info("initialized fan speed reporting for device %s (%s) using method _FST\n",
> - dev_name(&fan->fan.dev), fan->description);
> - }
> -
> - /* set up RO dev_ext_attribute */
> - fan->fan_speed_rpm_ext_attr.attr.attr.name = "fan_speed_rpm";
> - fan->fan_speed_rpm_ext_attr.attr.attr.mode = 0444;
> - fan->fan_speed_rpm_ext_attr.attr.show = fan_speed_rpm_show;
> - /* extended attribute var points to this galaxybook_fan so it can used in the show method */
> - fan->fan_speed_rpm_ext_attr.var = fan;
> -
> - if (sysfs_create_file(&adev->dev.kobj, &fan->fan_speed_rpm_ext_attr.attr.attr))
> - pr_err("unable to create fan_speed_rpm attribute for fan device %s (%s)\n",
> - dev_name(&fan->fan.dev), fan->description);
> -
> - galaxybook->fans_count++;
> -
> - return 0;
> -}
> -
> -static int __init galaxybook_fan_speed_init(struct samsung_galaxybook *galaxybook)
> -{
> - acpi_status status;
> -
> - /* get and set up all fans matching ACPI_FAN_DEVICE_ID */
> - status = acpi_get_devices(ACPI_FAN_DEVICE_ID, galaxybook_add_fan, galaxybook, NULL);
> -
> - if (galaxybook->fans_count == 0)
> - return -ENODEV;
> -
> - return status;
> -}
> -
> -static void galaxybook_fan_speed_exit(struct samsung_galaxybook *galaxybook)
> -{
> - for (int i = 0; i < galaxybook->fans_count; i++)
> - sysfs_remove_file(&galaxybook->fans[i].fan.dev.kobj,
> - &galaxybook->fans[i].fan_speed_rpm_ext_attr.attr.attr);
> -}
> -
> -/*
> - * Hwmon device
> - */
> -
> -#if IS_ENABLED(CONFIG_HWMON)
> -static umode_t galaxybook_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> - u32 attr, int channel)
> -{
> - switch (type) {
> - case hwmon_fan:
> - if (channel < galaxybook_ptr->fans_count &&
> - (attr == hwmon_fan_input || attr == hwmon_fan_label))
> - return 0444;
> - return 0;
> - default:
> - return 0;
> - }
> -}
> -
> -static int galaxybook_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> - u32 attr, int channel, long *val)
> -{
> - unsigned int speed;
> -
> - switch (type) {
> - case hwmon_fan:
> - if (channel < galaxybook_ptr->fans_count && attr == hwmon_fan_input) {
> - if (fan_speed_get(&galaxybook_ptr->fans[channel], &speed))
> - return -EIO;
> - *val = speed;
> - return 0;
> - }
> - return -EOPNOTSUPP;
> - default:
> - return -EOPNOTSUPP;
> - }
> -}
> -
> -static int galaxybook_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
> - u32 attr, int channel, const char **str)
> -{
> - switch (type) {
> - case hwmon_fan:
> - if (channel < galaxybook_ptr->fans_count && attr == hwmon_fan_label) {
> - *str = galaxybook_ptr->fans[channel].description;
> - return 0;
> - }
> - return -EOPNOTSUPP;
> - default:
> - return -EOPNOTSUPP;
> - }
> -}
> -
> -static const struct hwmon_ops galaxybook_hwmon_ops = {
> - .is_visible = galaxybook_hwmon_is_visible,
> - .read = galaxybook_hwmon_read,
> - .read_string = galaxybook_hwmon_read_string,
> -};
> -
> -static const struct hwmon_channel_info *const galaxybook_hwmon_info[] = {
> - /* note: number of max possible fan channel entries here should match MAX_FAN_COUNT */
> - HWMON_CHANNEL_INFO(fan,
> - HWMON_F_INPUT | HWMON_F_LABEL,
> - HWMON_F_INPUT | HWMON_F_LABEL,
> - HWMON_F_INPUT | HWMON_F_LABEL,
> - HWMON_F_INPUT | HWMON_F_LABEL,
> - HWMON_F_INPUT | HWMON_F_LABEL),
> - NULL
> -};
> -
> -static const struct hwmon_chip_info galaxybook_hwmon_chip_info = {
> - .ops = &galaxybook_hwmon_ops,
> - .info = galaxybook_hwmon_info,
> -};
> -
> -static int galaxybook_hwmon_init(struct samsung_galaxybook *galaxybook)
> -{
> - int ret = 0;
> -
> - char *hwmon_device_name = devm_hwmon_sanitize_name(&galaxybook->platform->dev,
> - SAMSUNG_GALAXYBOOK_CLASS);
> -
> - galaxybook->hwmon = devm_hwmon_device_register_with_info(
> - &galaxybook->platform->dev, hwmon_device_name, NULL,
> - &galaxybook_hwmon_chip_info, NULL);
> - if (PTR_ERR_OR_ZERO(galaxybook->hwmon)) {
> - ret = PTR_ERR(galaxybook->hwmon);
> - galaxybook->hwmon = NULL;
> - }
> -
> - return ret;
> -}
> -
> -static void galaxybook_hwmon_exit(struct samsung_galaxybook *galaxybook)
> -{
> - if (galaxybook->hwmon)
> - hwmon_device_unregister(galaxybook->hwmon);
> -}
> -#endif
> -
> /*
> * Platform Profile / Performance mode
> */
> @@ -1254,7 +810,6 @@ static int performance_mode_acpi_set(struct samsung_galaxybook *galaxybook,
> const u8 performance_mode)
> {
> struct sawb buf = { 0 };
> - int err;
>
> buf.safn = SAFN;
> buf.sasb = SASB_PERFORMANCE_MODE;
> @@ -1263,12 +818,8 @@ static int performance_mode_acpi_set(struct samsung_galaxybook *galaxybook,
> buf.subn = SUBN_PERFORMANCE_MODE_SET;
> buf.iob0 = performance_mode;
>
> - err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_PERFORMANCE_MODE, &buf,
> - SAWB_LEN_PERFORMANCE_MODE, "setting performance_mode", &buf);
> - if (err)
> - return err;
> -
> - return 0;
> + return galaxybook_acpi_method(galaxybook, ACPI_METHOD_PERFORMANCE_MODE, &buf,
> + SAWB_LEN_PERFORMANCE_MODE, "setting performance_mode", &buf);
> }
>
> static int performance_mode_acpi_get(struct samsung_galaxybook *galaxybook, u8 *performance_mode)
> @@ -1292,40 +843,29 @@ static int performance_mode_acpi_get(struct samsung_galaxybook *galaxybook, u8 *
> return 0;
> }
>
> -static enum platform_profile_option
> -profile_performance_mode(struct samsung_galaxybook *galaxybook, const u8 performance_mode)
> +static int get_performance_mode_profile(struct samsung_galaxybook *galaxybook,
> + const u8 performance_mode,
> + enum platform_profile_option *profile)
> {
> - for (int i = 0; i < PLATFORM_PROFILE_LAST; i++)
> - if (galaxybook->profile_performance_modes[i] == performance_mode)
> - return i;
> - return -1;
> -}
> + for (int i = 0; i < PLATFORM_PROFILE_LAST; i++) {
> + if (galaxybook->profile_performance_modes[i] == performance_mode) {
> + if (profile)
> + *profile = i;
> + return 0;
> + }
> + }
>
> -/* copied from platform_profile.c; better if this could be fetched from a public function, maybe? */
> -static const char *const profile_names[] = {
> - [PLATFORM_PROFILE_LOW_POWER] = "low-power",
> - [PLATFORM_PROFILE_COOL] = "cool",
> - [PLATFORM_PROFILE_QUIET] = "quiet",
> - [PLATFORM_PROFILE_BALANCED] = "balanced",
> - [PLATFORM_PROFILE_BALANCED_PERFORMANCE] = "balanced-performance",
> - [PLATFORM_PROFILE_PERFORMANCE] = "performance",
> -};
> -static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST);
> + return -ENODATA;
> +}
>
> static int galaxybook_platform_profile_set(struct platform_profile_handler *pprof,
> enum platform_profile_option profile)
> {
> struct samsung_galaxybook *galaxybook =
> container_of(pprof, struct samsung_galaxybook, profile_handler);
> - int err;
> -
> - err = performance_mode_acpi_set(galaxybook, galaxybook->profile_performance_modes[profile]);
> - if (err)
> - return err;
>
> - pr_debug_prefixed("set platform profile to '%s' (performance mode 0x%x)\n",
> - profile_names[profile], galaxybook->profile_performance_modes[profile]);
> - return 0;
> + return performance_mode_acpi_set(galaxybook,
> + galaxybook->profile_performance_modes[profile]);
> }
>
> static int galaxybook_platform_profile_get(struct platform_profile_handler *pprof,
> @@ -1340,23 +880,24 @@ static int galaxybook_platform_profile_get(struct platform_profile_handler *ppro
> if (err)
> return err;
>
> - *profile = profile_performance_mode(galaxybook, performance_mode);
> - if (*profile == -1)
> - return -EINVAL;
> -
> - pr_debug_prefixed("platform profile is currently '%s' (performance mode 0x%x)\n",
> - profile_names[*profile], performance_mode);
> + return get_performance_mode_profile(galaxybook, performance_mode, profile);
> +}
>
> - return 0;
> +static void galaxybook_profile_exit(void *data)
> +{
> + platform_profile_remove();
> }
>
> #define IGNORE_PERFORMANCE_MODE_MAPPING -1
>
> static int galaxybook_profile_init(struct samsung_galaxybook *galaxybook)
> {
> - struct sawb buf = { 0 };
> - int mode_profile, err;
> u8 current_performance_mode;
> + struct sawb buf = { 0 };
> + int mapped_profiles;
> + int mode_profile;
> + int err;
> + int i;
>
> galaxybook->profile_handler.profile_get = galaxybook_platform_profile_get;
> galaxybook->profile_handler.profile_set = galaxybook_platform_profile_set;
> @@ -1376,10 +917,10 @@ static int galaxybook_profile_init(struct samsung_galaxybook *galaxybook)
>
> /* set up profile_performance_modes with "unknown" as init value */
> galaxybook->profile_performance_modes =
> - kzalloc(sizeof(u8) * PLATFORM_PROFILE_LAST, GFP_KERNEL);
> + kcalloc(PLATFORM_PROFILE_LAST, sizeof(u8), GFP_KERNEL);
> if (!galaxybook->profile_performance_modes)
> return -ENOMEM;
> - for (int i = 0; i < PLATFORM_PROFILE_LAST; i++)
> + for (i = 0; i < PLATFORM_PROFILE_LAST; i++)
> galaxybook->profile_performance_modes[i] = PERFORMANCE_MODE_UNKNOWN;
>
> /*
> @@ -1388,8 +929,7 @@ static int galaxybook_profile_init(struct samsung_galaxybook *galaxybook)
> * Loop backwards from last value to first value (to handle fallback cases which come with
> * smaller values) and map each supported value to its correct platform_profile_option.
> */
> - err = -ENODEV; /* set err to "no device" to signal that we have not yet mapped profiles */
> - for (int i = buf.iob0; i > 0; i--) {
> + for (i = buf.iob0; i > 0; i--) {
> /*
> * Prefer mapping to at least performance, balanced, and low-power profiles, as they
> * are the profiles which are typically supported by userspace tools
> @@ -1459,21 +999,22 @@ static int galaxybook_profile_init(struct samsung_galaxybook *galaxybook)
> }
>
> /* if current mode value mapped to a supported platform_profile_option, set it up */
> - if (mode_profile > IGNORE_PERFORMANCE_MODE_MAPPING) {
> - err = 0; /* clear err to signal that at least one profile is now mapped */
> + if (mode_profile != IGNORE_PERFORMANCE_MODE_MAPPING) {
> + mapped_profiles++;
> galaxybook->profile_performance_modes[mode_profile] = buf.iob_values[i];
> set_bit(mode_profile, galaxybook->profile_handler.choices);
> - pr_info("will support platform profile '%s' (performance mode 0x%x)\n",
> - profile_names[mode_profile], buf.iob_values[i]);
> + dev_dbg(&galaxybook->platform->dev,
> + "will support platform profile %d (performance mode 0x%x)\n",
> + mode_profile, buf.iob_values[i]);
> } else {
> - pr_debug_prefixed("unmapped performance mode 0x%x will be ignored\n",
> - buf.iob_values[i]);
> + dev_dbg(&galaxybook->platform->dev,
> + "unmapped performance mode 0x%x will be ignored\n",
> + buf.iob_values[i]);
> }
> }
>
> - /* if no performance modes were mapped (err is still -ENODEV) then stop and fail here */
> - if (err)
> - return err;
> + if (mapped_profiles == 0)
> + return -ENODEV;
>
> err = platform_profile_register(&galaxybook->profile_handler);
> if (err)
> @@ -1482,33 +1023,35 @@ static int galaxybook_profile_init(struct samsung_galaxybook *galaxybook)
> /* now check currently set performance mode; if not supported then set default profile */
> err = performance_mode_acpi_get(galaxybook, ¤t_performance_mode);
> if (err)
> - pr_warn("failed with code %d when fetching initial performance mode\n", err);
> - if (profile_performance_mode(galaxybook, current_performance_mode) == -1) {
> - pr_debug_prefixed("initial performance mode value is not supported by device; "
> - "setting to default\n");
> + goto err_remove_exit;
> + err = get_performance_mode_profile(galaxybook, current_performance_mode, NULL);
> + if (err) {
> + dev_dbg(&galaxybook->platform->dev,
> + "initial performance mode value is not supported by device; "
> + "setting to default\n");
> err = galaxybook_platform_profile_set(&galaxybook->profile_handler,
> DEFAULT_PLATFORM_PROFILE);
> if (err)
> - return err;
> + goto err_remove_exit;
> }
>
> + /* if adding dev remove action fails, remove now and return failure */
> + err = devm_add_action_or_reset(&galaxybook->platform->dev,
> + galaxybook_profile_exit, NULL);
> + if (err)
> + goto err_remove_exit;
> +
> return 0;
> -}
>
> -static void galaxybook_profile_exit(struct samsung_galaxybook *galaxybook)
> -{
> - platform_profile_remove();
> +err_remove_exit:
> + galaxybook_profile_exit(NULL);
> + return err;
> }
>
> /*
> * Hotkey work and filters
> */
>
> -static void galaxybook_performance_mode_hotkey_work(struct work_struct *work)
> -{
> - platform_profile_cycle();
> -}
> -
> static void galaxybook_kbd_backlight_hotkey_work(struct work_struct *work)
> {
> struct samsung_galaxybook *galaxybook =
> @@ -1575,118 +1118,77 @@ static bool galaxybook_i8042_filter(unsigned char data, unsigned char str, struc
> return false;
> }
>
> -/*
> - * Input device (hotkeys and notifications)
> - */
> -
> -static void galaxybook_input_notify(struct samsung_galaxybook *galaxybook, int event)
> +static void galaxybook_i8042_filter_remove(void *data)
> {
> - if (!galaxybook->input)
> - return;
> - pr_debug_prefixed("input notification event: 0x%x\n", event);
> - if (!sparse_keymap_report_event(galaxybook->input, event, 1, true))
> - pr_warn("unknown input notification event: 0x%x\n", event);
> + struct samsung_galaxybook *galaxybook = data;
> +
> + i8042_remove_filter(galaxybook_i8042_filter);
> + cancel_work_sync(&galaxybook->kbd_backlight_hotkey_work);
> + cancel_work_sync(&galaxybook->allow_recording_hotkey_work);
> }
>
> -static int galaxybook_input_init(struct samsung_galaxybook *galaxybook)
> +static int galaxybook_i8042_filter_install(struct samsung_galaxybook *galaxybook)
> {
> - struct input_dev *input;
> - int error;
> -
> - input = input_allocate_device();
> - if (!input)
> - return -ENOMEM;
> + int err;
>
> - input->name = "Samsung Galaxy Book Extra Buttons";
> - input->phys = SAMSUNG_GALAXYBOOK_CLASS "/input0";
> - input->id.bustype = BUS_HOST;
> - input->dev.parent = &galaxybook->platform->dev;
> + /* initialize hotkey work queues */
> + if (kbd_backlight)
> + INIT_WORK(&galaxybook->kbd_backlight_hotkey_work,
> + galaxybook_kbd_backlight_hotkey_work);
> + if (allow_recording)
> + INIT_WORK(&galaxybook->allow_recording_hotkey_work,
> + galaxybook_allow_recording_hotkey_work);
>
> - error = sparse_keymap_setup(input, galaxybook_acpi_keymap, NULL);
> - if (error) {
> - pr_err("Unable to setup input device keymap\n");
> - goto err_free_dev;
> + err = i8042_install_filter(galaxybook_i8042_filter);
> + if (err) {
> + cancel_work_sync(&galaxybook->kbd_backlight_hotkey_work);
> + cancel_work_sync(&galaxybook->allow_recording_hotkey_work);
> + return err;
> }
> - error = input_register_device(input);
> - if (error) {
> - pr_err("Unable to register input device\n");
> - goto err_free_dev;
> +
> + /* if adding dev remove action fails, remove now and return failure */
> + err = devm_add_action_or_reset(&galaxybook->platform->dev,
> + galaxybook_i8042_filter_remove, galaxybook);
> + if (err) {
> + galaxybook_i8042_filter_remove(galaxybook);
> + return err;
> }
>
> - galaxybook->input = input;
> return 0;
> -
> -err_free_dev:
> - input_free_device(input);
> - return error;
> -}
> -
> -static void galaxybook_input_exit(struct samsung_galaxybook *galaxybook)
> -{
> - if (galaxybook->input)
> - input_unregister_device(galaxybook->input);
> - galaxybook->input = NULL;
> }
>
> /*
> - * Platform device attributes
> + * Input device (hotkeys and notifications)
> */
>
> -/* galaxybook_attrs can include start_on_lid_open, usb_charge, and/or allow_recording */
> -#define MAX_NUM_DEVICE_ATTRIBUTES 3
> -
> -static struct attribute *galaxybook_attrs[MAX_NUM_DEVICE_ATTRIBUTES + 1] = { NULL };
> -static const struct attribute_group galaxybook_attrs_group = {
> - .attrs = galaxybook_attrs,
> -};
> +static void galaxybook_input_notify(struct samsung_galaxybook *galaxybook, int event)
> +{
> + if (!galaxybook->input)
> + return;
> + mutex_lock(&galaxybook->input_lock);
> + if (!sparse_keymap_report_event(galaxybook->input, event, 1, true))
> + dev_warn(&galaxybook->acpi->dev, "unknown input notification event: 0x%x\n", event);
> + mutex_unlock(&galaxybook->input_lock);
> +}
>
> -static int galaxybook_device_attrs_init(struct samsung_galaxybook *galaxybook)
> +static int galaxybook_input_init(struct samsung_galaxybook *galaxybook)
> {
> - bool value;
> int err;
> - int i = 0;
>
> - /* attempt to get each attribute's value and add them if the get does not fail */
> + galaxybook->input = devm_input_allocate_device(&galaxybook->platform->dev);
> + if (!galaxybook->input)
> + return -ENOMEM;
>
> - err = start_on_lid_open_acpi_get(galaxybook, &value);
> - if (err)
> - pr_debug_prefixed("failed to get start_on_lid_open value; "
> - "this feature will not be enabled\n");
> - else
> - galaxybook_attrs[i++] = &dev_attr_start_on_lid_open.attr;
> + galaxybook->input->name = "Samsung Galaxy Book Extra Buttons";
> + galaxybook->input->phys = DRIVER_NAME "/input0";
> + galaxybook->input->id.bustype = BUS_HOST;
> + galaxybook->input->dev.parent = &galaxybook->platform->dev;
>
> - err = usb_charge_acpi_get(galaxybook, &value);
> + err = sparse_keymap_setup(galaxybook->input, galaxybook_acpi_keymap, NULL);
> if (err)
> - pr_debug_prefixed("failed to get usb_charge value; "
> - "this feature will not be enabled\n");
> - else
> - galaxybook_attrs[i++] = &dev_attr_usb_charge.attr;
> -
> - if (allow_recording) {
> - pr_debug_prefixed("initializing ACPI allow_recording feature\n");
> - err = galaxybook_enable_acpi_feature(galaxybook, SASB_ALLOW_RECORDING);
> - if (err) {
> - pr_debug_prefixed("failed to initialize ACPI allow_recording feature\n");
> - allow_recording = false;
> - return 0;
> - }
> -
> - err = allow_recording_acpi_get(galaxybook, &value);
> - if (err) {
> - pr_debug_prefixed("failed to get allow_recording value; "
> - "this feature will not be enabled\n");
> - allow_recording = false;
> - } else {
> - galaxybook_attrs[i++] = &dev_attr_allow_recording.attr;
> - }
> - }
> -
> - return device_add_group(&galaxybook->platform->dev, &galaxybook_attrs_group);
> -};
> + return err;
>
> -static void galaxybook_device_attrs_exit(struct samsung_galaxybook *galaxybook)
> -{
> - device_remove_group(&galaxybook->platform->dev, &galaxybook_attrs_group);
> + return input_register_device(galaxybook->input);
> }
>
> /*
> @@ -1698,14 +1200,17 @@ static void galaxybook_acpi_notify(acpi_handle handle, u32 event, void *data)
> struct samsung_galaxybook *galaxybook = data;
>
> if (event == ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE) {
> - pr_debug_prefixed("hotkey: performance_mode keydown\n");
> if (performance_mode) {
> - schedule_work(&galaxybook->performance_mode_hotkey_work);
> - return;
> + platform_profile_cycle();
> + goto exit;
> }
> }
>
> galaxybook_input_notify(galaxybook, event);
> +
> +exit:
> + acpi_bus_generate_netlink_event(DRIVER_NAME, dev_name(&galaxybook->platform->dev),
> + event, 0);
> }
>
> static int galaxybook_enable_acpi_notify(struct samsung_galaxybook *galaxybook)
> @@ -1726,247 +1231,155 @@ static int galaxybook_enable_acpi_notify(struct samsung_galaxybook *galaxybook)
> "activate ACPI notifications", &buf);
> }
>
> -static int galaxybook_acpi_init(struct samsung_galaxybook *galaxybook)
> +static void galaxybook_acpi_remove_notify_handler(void *data)
> {
> - return acpi_execute_simple_method(galaxybook->acpi->handle,
> - ACPI_METHOD_ENABLE, ACPI_METHOD_ENABLE_ON);
> + struct samsung_galaxybook *galaxybook = data;
> +
> + acpi_remove_notify_handler(galaxybook->acpi->handle, ACPI_ALL_NOTIFY,
> + galaxybook_acpi_notify);
> }
>
> -static void galaxybook_acpi_exit(struct samsung_galaxybook *galaxybook)
> +static void galaxybook_acpi_disable(void *data)
> {
> + struct samsung_galaxybook *galaxybook = data;
> +
> acpi_execute_simple_method(galaxybook->acpi->handle,
> ACPI_METHOD_ENABLE, ACPI_METHOD_ENABLE_OFF);
> }
>
> +static int galaxybook_acpi_init(struct samsung_galaxybook *galaxybook)
> +{
> + acpi_status status;
> + int err;
> +
> + status = acpi_execute_simple_method(galaxybook->acpi->handle, ACPI_METHOD_ENABLE,
> + ACPI_METHOD_ENABLE_ON);
> + if (ACPI_FAILURE(status))
> + return -ENXIO;
> + err = devm_add_action_or_reset(&galaxybook->platform->dev,
> + galaxybook_acpi_disable, galaxybook);
> + if (err)
> + return err;
> +
> + status = acpi_install_notify_handler(galaxybook->acpi->handle, ACPI_ALL_NOTIFY,
> + galaxybook_acpi_notify, galaxybook);
> + if (ACPI_FAILURE(status))
> + return -ENXIO;
> + err = devm_add_action_or_reset(&galaxybook->platform->dev,
> + galaxybook_acpi_remove_notify_handler, galaxybook);
> + if (err)
> + return err;
> +
> + err = galaxybook_enable_acpi_notify(galaxybook);
> + if (err) {
> + dev_warn(&galaxybook->platform->dev, "failed to enable ACPI notifications; "
> + "some hotkeys will not be supported\n");
> + } else {
> + err = galaxybook_input_init(galaxybook);
> + if (err)
> + dev_warn(&galaxybook->platform->dev,
> + "failed to initialize input device\n");
> + }
> +
> + return 0;
> +}
> +
> /*
> * Platform driver
> */
>
> +#define galaxybook_init_feature(module_param, init_func) \
> +do { \
> + if (module_param) { \
> + err = init_func(galaxybook); \
> + if (err) { \
> + dev_dbg(&galaxybook->platform->dev, \
> + "failed to initialize " #module_param "\n"); \
> + module_param = false; \
> + } \
> + } \
> +} while (0)
> +
> static int galaxybook_probe(struct platform_device *pdev)
> {
> struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
> struct samsung_galaxybook *galaxybook;
> - acpi_status status;
> int err;
>
> - dmi_check_system(galaxybook_dmi_ids);
> + if (!adev)
> + return -ENODEV;
>
> - pr_info("found matched device %s; loading driver\n", dev_name(&adev->dev));
> + dev_dbg(&pdev->dev, "loading driver\n");
>
> - galaxybook = kzalloc(sizeof(struct samsung_galaxybook), GFP_KERNEL);
> + galaxybook = devm_kzalloc(&pdev->dev, sizeof(*galaxybook), GFP_KERNEL);
> if (!galaxybook)
> return -ENOMEM;
> - /* set static pointer here so it can be used in various methods for hotkeys, hwmon, etc */
> + /* set static pointer here so it can be used in i8042 filter */
> galaxybook_ptr = galaxybook;
>
> galaxybook->platform = pdev;
> galaxybook->acpi = adev;
>
> dev_set_drvdata(&galaxybook->platform->dev, galaxybook);
> + devm_mutex_init(&galaxybook->platform->dev, &galaxybook->acpi_lock);
> + devm_mutex_init(&galaxybook->platform->dev, &galaxybook->input_lock);
>
> - pr_debug_prefixed("initializing ACPI device\n");
> err = galaxybook_acpi_init(galaxybook);
> if (err) {
> - pr_err("failed to initialize the ACPI device\n");
> - goto err_free;
> + dev_err(&galaxybook->acpi->dev, "failed to initialize the ACPI device\n");
> + return err;
> }
>
> - pr_debug_prefixed("initializing ACPI power management features\n");
> err = galaxybook_enable_acpi_feature(galaxybook, SASB_POWER_MANAGEMENT);
> if (err) {
> - pr_warn("failed to initialize ACPI power management features; "
> - "many features of this driver will not be available\n");
> + dev_warn(&galaxybook->acpi->dev,
> + "failed to initialize ACPI power management features; "
> + "many features of this driver will not be available\n");
> performance_mode = false;
> battery_threshold = false;
> }
>
> - if (performance_mode) {
> - pr_debug_prefixed("initializing performance mode and platform profile\n");
> - err = galaxybook_profile_init(galaxybook);
> - if (err) {
> - pr_debug_prefixed(
> - "failed to initialize performance mode and platform profile\n");
> - performance_mode = false;
> - }
> - } else {
> - pr_debug_prefixed("performance_mode is disabled\n");
> - }
> -
> - if (battery_threshold) {
> - pr_debug_prefixed("initializing battery charge threshold control\n");
> - err = galaxybook_battery_threshold_init(galaxybook);
> - if (err) {
> - pr_debug_prefixed(
> - "failed to initialize battery charge threshold control\n");
> - battery_threshold = false;
> - }
> - } else {
> - pr_debug_prefixed("battery_threshold is disabled\n");
> - }
> + galaxybook_init_feature(performance_mode, galaxybook_profile_init);
> + galaxybook_init_feature(battery_threshold, galaxybook_battery_threshold_init);
>
> - pr_debug_prefixed("adding platform device attributes\n");
> - err = galaxybook_device_attrs_init(galaxybook);
> + /* add attrs group here as the is_visible requires above initializations */
> + err = devm_device_add_group(&galaxybook->platform->dev, &galaxybook_attrs_group);
> if (err)
> - pr_err("failed to add platform device attributes\n");
> + dev_warn(&galaxybook->platform->dev, "failed to add platform device attributes\n");
>
> - if (kbd_backlight) {
> - pr_debug_prefixed("initializing kbd_backlight\n");
> - err = galaxybook_kbd_backlight_init(galaxybook);
> - if (err) {
> - pr_debug_prefixed("failed to initialize kbd_backlight\n");
> - kbd_backlight = false;
> - }
> - } else {
> - pr_debug_prefixed("kbd_backlight is disabled\n");
> - }
> -
> - if (fan_speed) {
> - pr_debug_prefixed("initializing fan speed\n");
> - err = galaxybook_fan_speed_init(galaxybook);
> - if (err) {
> - pr_debug_prefixed("failed to initialize fan speed\n");
> - fan_speed = false;
> - } else {
> -#if IS_ENABLED(CONFIG_HWMON)
> - pr_debug_prefixed("initializing hwmon device\n");
> - err = galaxybook_hwmon_init(galaxybook);
> - if (err)
> - pr_warn("failed to initialize hwmon device\n");
> -#endif
> - }
> - } else {
> - pr_debug_prefixed("fan_speed is disabled\n");
> - }
> + galaxybook_init_feature(kbd_backlight, galaxybook_kbd_backlight_init);
>
> /* i8042_filter should be disabled if kbd_backlight and allow_recording are disabled */
> if (!kbd_backlight && !allow_recording)
> i8042_filter = false;
>
> - if (i8042_filter) {
> - pr_debug_prefixed("installing i8402 key filter to capture hotkey input\n");
> + galaxybook_init_feature(i8042_filter, galaxybook_i8042_filter_install);
>
> - /* initialize hotkey work queues */
> - if (kbd_backlight)
> - INIT_WORK(&galaxybook->kbd_backlight_hotkey_work,
> - galaxybook_kbd_backlight_hotkey_work);
> - if (allow_recording)
> - INIT_WORK(&galaxybook->allow_recording_hotkey_work,
> - galaxybook_allow_recording_hotkey_work);
> -
> - err = i8042_install_filter(galaxybook_i8042_filter);
> - if (err) {
> - pr_err("failed to install i8402 key filter\n");
> - cancel_work_sync(&galaxybook->kbd_backlight_hotkey_work);
> - cancel_work_sync(&galaxybook->allow_recording_hotkey_work);
> - i8042_filter = false;
> - }
> - } else {
> - pr_debug_prefixed("i8042_filter is disabled\n");
> - }
> -
> - pr_debug_prefixed("installing ACPI notify handler\n");
> - status = acpi_install_notify_handler(galaxybook->acpi->handle, ACPI_ALL_NOTIFY,
> - galaxybook_acpi_notify, galaxybook);
> - if (ACPI_SUCCESS(status)) {
> - pr_debug_prefixed("enabling ACPI notifications\n");
> - err = galaxybook_enable_acpi_notify(galaxybook);
> - if (err) {
> - pr_warn("failed to enable ACPI notifications; "
> - "some hotkeys will not be supported\n");
> - } else {
> - /* initialize ACPI hotkey work queues */
> - INIT_WORK(&galaxybook->performance_mode_hotkey_work,
> - galaxybook_performance_mode_hotkey_work);
> -
> - pr_debug_prefixed("initializing input device\n");
> - err = galaxybook_input_init(galaxybook);
> - if (err) {
> - pr_err("failed to initialize input device\n");
> - cancel_work_sync(&galaxybook->performance_mode_hotkey_work);
> - galaxybook_input_exit(galaxybook);
> - }
> - }
> - } else {
> - pr_debug_prefixed("failed to install ACPI notify handler\n");
> - }
> -
> - pr_info("driver successfully loaded\n");
> + dev_dbg(&galaxybook->platform->dev, "driver successfully loaded\n");
>
> return 0;
> -
> -err_free:
> - kfree(galaxybook);
> - return err;
> }
>
> static void galaxybook_remove(struct platform_device *pdev)
> {
> struct samsung_galaxybook *galaxybook = dev_get_drvdata(&pdev->dev);
>
> - pr_info("removing driver\n");
> -
> - galaxybook_device_attrs_exit(galaxybook);
> -
> - galaxybook_input_exit(galaxybook);
> - cancel_work_sync(&galaxybook->performance_mode_hotkey_work);
> -
> - if (i8042_filter) {
> - i8042_remove_filter(galaxybook_i8042_filter);
> - cancel_work_sync(&galaxybook->kbd_backlight_hotkey_work);
> - cancel_work_sync(&galaxybook->allow_recording_hotkey_work);
> - }
> -
> - acpi_remove_notify_handler(galaxybook->acpi->handle, ACPI_ALL_NOTIFY,
> - galaxybook_acpi_notify);
> -
> - if (fan_speed) {
> - galaxybook_fan_speed_exit(galaxybook);
> -#if IS_ENABLED(CONFIG_HWMON)
> - galaxybook_hwmon_exit(galaxybook);
> -#endif
> - }
> -
> - if (kbd_backlight)
> - galaxybook_kbd_backlight_exit(galaxybook);
> -
> - if (battery_threshold)
> - galaxybook_battery_threshold_exit(galaxybook);
> -
> - if (performance_mode)
> - galaxybook_profile_exit(galaxybook);
> -
> - galaxybook_acpi_exit(galaxybook);
> -
> if (galaxybook_ptr)
> galaxybook_ptr = NULL;
>
> - kfree(galaxybook);
> -
> - pr_info("driver successfully removed\n");
> + dev_dbg(&galaxybook->platform->dev, "driver removed\n");
> }
>
> static struct platform_driver galaxybook_platform_driver = {
> .driver = {
> - .name = SAMSUNG_GALAXYBOOK_CLASS,
> + .name = DRIVER_NAME,
> .acpi_match_table = galaxybook_device_ids,
> },
> .probe = galaxybook_probe,
> .remove = galaxybook_remove,
> };
> -
> -static int __init samsung_galaxybook_init(void)
> -{
> - return platform_driver_register(&galaxybook_platform_driver);
> -}
> -
> -static void __exit samsung_galaxybook_exit(void)
> -{
> - platform_driver_unregister(&galaxybook_platform_driver);
> -}
> -
> -module_init(samsung_galaxybook_init);
> -module_exit(samsung_galaxybook_exit);
> +module_platform_driver(galaxybook_platform_driver);
>
> MODULE_AUTHOR("Joshua Grisham <josh@...huagrisham.com>");
> -MODULE_DESCRIPTION(SAMSUNG_GALAXYBOOK_NAME);
> +MODULE_DESCRIPTION("Samsung Galaxy Book Extras");
> MODULE_LICENSE("GPL");
Powered by blists - more mailing lists