| Message ID | 20241216103855.18457-1-josh@joshuagrisham.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v3] platform/x86: samsung-galaxybook: Add samsung-galaxybook driver | expand |
Hi Joshua! (disclaimer: I didn't read all the previous reviews) On 2024-12-16 11:38:54+0100, Joshua Grisham wrote: > Adds a new driver for Samsung Galaxy Book series notebook devices. This > includes the driver itself, a tracepoint header file, a new page in the > documentation, and relevant updates to Kconfig, Makefile, and MAINTAINERS > files related to this new driver. IMO it would be better to describe the features and handled devices a bit instead of listing the contents of the patch, which can be read just below, too. > > Signed-off-by: Joshua Grisham <josh@joshuagrisham.com> > --- > v1->v2: > - Attempt to resolve all review comments from v1 as written here: > https://lore.kernel.org/platform-driver-x86/53c5075b-1967-45d0-937f-463912dd966d@gmx.de/T/#mbcbd8d5d9bc4496bac5486636c7d3b32bc3e5cd0 > > v2->v3: > - Tweak to battery attribute to closer match pattern in dell-wmi-ddv > - implement platform_profile_remove() change from > 9b3bb37b44a317626464e79da8b39989b421963f > - Small tweak to Documentation page > --- > .../laptops/samsung-galaxybook.rst | 280 ++++ > MAINTAINERS | 8 + > drivers/platform/x86/Kconfig | 18 + > drivers/platform/x86/Makefile | 5 +- > .../platform/x86/samsung-galaxybook-trace.h | 51 + > drivers/platform/x86/samsung-galaxybook.c | 1380 +++++++++++++++++ > 6 files changed, 1740 insertions(+), 2 deletions(-) > create mode 100644 Documentation/admin-guide/laptops/samsung-galaxybook.rst > create mode 100644 drivers/platform/x86/samsung-galaxybook-trace.h > create mode 100644 drivers/platform/x86/samsung-galaxybook.c > > diff --git a/Documentation/admin-guide/laptops/samsung-galaxybook.rst b/Documentation/admin-guide/laptops/samsung-galaxybook.rst > new file mode 100644 > index 000000000000..947810c5f998 > --- /dev/null > +++ b/Documentation/admin-guide/laptops/samsung-galaxybook.rst Needs to be added to index.rst to be rendered. > @@ -0,0 +1,280 @@ > +.. SPDX-License-Identifier: GPL-2.0-or-later > + > +========================== > +Samsung Galaxy Book Extras > +========================== > + > +December 15, 2024 No need for the date. It will just go stale instantly. > +Joshua Grisham <josh@joshuagrisham.com> > + > +This is a Linux x86 platform driver for Samsung Galaxy Book series notebook > +devices which utilizes Samsung's ``SCAI`` ACPI device in order to control > +extra features and receive various notifications. > + > + > +Supported devices > +================= > + > +Any device with one of the supported ACPI device IDs should be supported. This > +covers most of the "Samsung Galaxy Book" series notebooks that are currently > +available as of this writing, and could include other Samsung notebook devices > +as well. > + > + > +Status > +====== > + > +The following features are currently supported: > + > +- :ref:`Keyboard backlight <keyboard-backlight>` control > +- :ref:`Performance mode <performance-mode>` control implemented using the > + platform profile interface > +- :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:`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 > +for a valid response before enabling its support (registering additional devices > +or extensions, adding sysfs attributes, etc). Therefore, it can be important to > +note that not all features may be supported for your particular device. > + > +The following features might be possible to implement but will require > +additional investigation and are therefore not supported at this time: > + > +- "Dolby Atmos" mode for the speakers > +- "Outdoor Mode" for increasing screen brightness on models with ``SAM0427`` > +- "Silent Mode" on models with ``SAM0427`` > + > + > +Parameters > +========== > + > +The driver includes a list of boolean parameters that allow for manually > +enabling or disabling various features: Can you explain *why* these are configurable? In general the addition of kernel parameters is frowned upon. > + > +- ``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) > +- ``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 > + (default on) > + > +.. note:: > + Even if you explicitly try to enable a feature using its parameter, support > + for it will still be evaluated by the driver, and the feature will be > + disabled if it does not appear to be supported on your device. > + > +The availability of various sysfs file-based "settings" attributes > +(``usb_charge``, ``start_on_lid_open``, etc) will be determined automatically > +and cannot be manually disabled at this time. > + > + > +.. _keyboard-backlight: > + > +Keyboard backlight > +================== > + > +Controlled by parameter: ``kbd_backlight`` > + > +A new LED class named ``samsung-galaxybook::kbd_backlight`` is created which > +will then expose the device using the standard sysfs-based LED interface at > +``/sys/class/leds/samsung-galaxybook::kbd_backlight``. Brightness can be > +controlled by writing values 0 to 3 to the ``brightness`` sysfs attribute or > +with any other desired userspace utility. Don't document the actual values. Userspace can discover them from sysfs. > + > +.. note:: > + Most of these devices have an ambient light sensor which also turns > + off the keyboard backlight under well-lit conditions. This behavior does not > + seem possible to control at this time, but can be good to be aware of. > + > + > +.. _performance-mode: > + > +Performance mode > +================ > + > +Controlled by parameter: ``performance_mode`` > + > +This driver implements the > +Documentation/userspace-api/sysfs-platform_profile.rst interface for working > +with the "performance mode" function of the Samsung ACPI device. > + > +Mapping of each Samsung "performance mode" to its respective platform profile is > +done dynamically based on a list of the supported modes reported by the device > +itself. Preference is given to always try and map ``low-power``, ``balanced``, > +and ``performance`` profiles, as these seem to be the most common profiles > +utilized (and sometimes even required) by various userspace tools. > + > +The result of the mapping will be printed in the kernel log when the module is > +loaded. Supported profiles can also be retrieved from > +``/sys/firmware/acpi/platform_profile_choices``, while > +``/sys/firmware/acpi/platform_profile`` can be used to read or write the > +currently selected profile. > + > +The ``balanced`` platform profile will be set during module load if no profile > +has been previously set. > + > + > +.. _battery-charge-control-end-threshold: > + > +Battery charge control end threshold > +==================================== > + > +Controlled by parameter: ``battery_threshold`` > + > +This platform driver will add the ability to set the battery's charge control > +end threshold, but does not have the ability to set a start threshold. > + > +This feature is typically called "Battery Saver" by the various Samsung > +applications in Windows, but in Linux we have implemented the standardized > +"charge control threshold" sysfs interface on the battery device to allow for > +controlling this functionality from the userspace. > + > +The sysfs attribute > +``/sys/class/power_supply/BAT1/charge_control_end_threshold`` can be used to > +read or set the desired charge end threshold. > + > +If you wish to maintain interoperability with Windows, then you should set the > +value to 80 to represent "on", or 0 to represent "off", as these are the values > +currently recognized by the various Windows-based Samsung applications and > +services as "on" or "off". Otherwise, the device will accept any value between 0 > +(off) and 99 as the percentage that you wish the battery to stop charging at. > + > +.. note:: > + If you try to set a value of 100, the driver will also accept this input, but > + will set the attribute value to 0 (i.e. 100% will "remove" the end threshold). > + > + > +.. _settings-attributes: > + > +Settings Attributes > +=================== > + > +Various hardware settings can be controlled by the following sysfs attributes: > + > +- ``allow_recording`` (allows or blocks usage of built-in camera and microphone) > +- ``start_on_lid_open`` (power on automatically when opening the lid) > +- ``usb_charge`` (allows USB ports to provide power even when device is off) Non-standard sysfs attributes should be avoided where possible. Userspace will need bespoke code to handle them. This looks like it could be handled by the standard firmware_attributes interface. This would standardize discovery and usage. There would be no need for the explanations below. > +These attributes will be available under the path for your supported ACPI Device > +ID's platform device (``SAM0428``, ``SAM0429``, etc), and can most reliably > +be found by seeing which device has been bound to the ``samsung-galaxybook`` > +driver. Here are some examples: :: > + > + # find which device ID has been bound to the driver > + ls /sys/bus/platform/drivers/samsung-galaxybook/ | grep SAM > + > + # see SAM0429 attributes > + ls /sys/bus/platform/drivers/samsung-galaxybook/SAM0429\:00 > + > + # see attributes no matter the device ID (using wildcard expansion) > + ls /sys/bus/platform/drivers/samsung-galaxybook/SAM* > + > +Most shells should support using wildcard expansion to directly read and write > +these attributes using the above pattern. Example: :: > + > + # read value of start_on_lid_open > + cat /sys/bus/platform/drivers/samsung-galaxybook/SAM*/start_on_lid_open > + > + # turn on start_on_lid_open > + echo true | sudo tee /sys/bus/platform/drivers/samsung-galaxybook/SAM*/start_on_lid_open > + > +It is also possible to use a udev rule to create a fixed-path symlink to your > +device under ``/dev`` (e.g. ``/dev/samsung-galaxybook``), no matter the device > +ID, to further simplify reading and writing these attributes in the userspace. > + > +Allow recording (allow_recording) > +--------------------------------- > + > +``/sys/bus/platform/drivers/samsung-galaxybook/SAM*/allow_recording`` > + > +Controlled by parameter: ``allow_recording`` > + > +Controls the "Allow recording" setting, which allows or blocks usage of the > +built-in camera and microphone (boolean). > + > +Start on lid open (start_on_lid_open) > +------------------------------------- > + > +``/sys/bus/platform/drivers/samsung-galaxybook/SAM*/start_on_lid_open`` > + > +Controls the "Start on lid open" setting, which sets the device to power on > +automatically when the lid is opened (boolean). > + > +USB charge (usb_charge) > +----------------------- > + > +``/sys/bus/platform/drivers/samsung-galaxybook/SAM*/usb_charge`` > + > +Controls the "USB charge" setting, which allows USB ports to provide power even > +when the device is turned off (boolean). > + > +.. note:: > + For most devices, this setting seems to only apply to the USB-C ports. > + > + > +.. _keyboard-hotkey-actions: > + > +Keyboard hotkey actions (i8042 filter) > +====================================== > + > +Controlled by parameter: ``i8042_filter`` > + > +The i8042 filter will swallow the keyboard events for the Fn+F9 hotkey (Multi- > +level keyboard backlight toggle) and Fn+F10 hotkey (Allow/block recording > +toggle) and instead execute their actions within the driver itself. > + > +Fn+F9 will cycle through the brightness levels of the keyboard backlight. A > +notification will be sent using ``led_classdev_notify_brightness_hw_changed`` > +so that the userspace can be aware of the change. This mimics the behavior of > +other existing devices where the brightness level is cycled internally by the > +embedded controller and then reported via a notification. > + > +Fn+F10 will toggle the value of the "Allow recording" setting. Personally I'm not a big fan to implement policy this way in the kernel. But others may disagree. > +ACPI notifications and ACPI hotkey actions > +========================================== > + > +There is a new "Samsung Galaxy Book extra buttons" input device created which > +will send input events for the following notifications from the ACPI device: > + > +- Notification when the battery charge control end threshold has been reached > + and the "battery saver" feature has stopped the battery from charging Does the ACPI battery not emit an uevent when charging stops in any case? If so, this seems unnecesarry. If not, use power_supply_changed() instead. > +- Notification when the device has been placed on a table (not available on all > + models) > +- Notification when the device has been lifted from a table (not available on > + all models) > + > +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. It feels wrong to do this in a driver. Tracing at this abstraction level should be provided by the ACPI core, if it is not already. > + > +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 3809931b9240..9e3b45cf799f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -20733,6 +20733,14 @@ L: linux-fbdev@vger.kernel.org > S: Maintained > F: drivers/video/fbdev/s3c-fb.c > > +SAMSUNG GALAXY BOOK EXTRAS DRIVER > +M: Joshua Grisham <josh@joshuagrisham.com> > +L: platform-driver-x86@vger.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 > M: Sylwester Nawrocki <s.nawrocki@samsung.com> > M: Artur Świgoń <a.swigon@samsung.com> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 0258dd879d64..03f4fb0e93e7 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -778,6 +778,24 @@ config BARCO_P50_GPIO > To compile this driver as a module, choose M here: the module > will be called barco-p50-gpio. > > +config SAMSUNG_GALAXYBOOK > + tristate "Samsung Galaxy Book extras driver" > + depends on ACPI > + depends on ACPI_BATTERY > + depends on INPUT > + depends on SERIO_I8042 > + select ACPI_PLATFORM_PROFILE > + select INPUT_SPARSEKMAP > + select NEW_LEDS > + select LEDS_CLASS LEDS_CLASS implies NEW_LEDS. Also LEDS_CLASS should be a "depends on". > + help > + This is a driver for Samsung Galaxy Book series notebooks. It adds > + support for the keyboard backlight control, performance mode control, fan > + speed reporting, function keys, and various other device controls. > + > + For more information about this driver, see > + <file:Documentation/admin-guide/laptops/samsung-galaxybook.rst>. > + > config SAMSUNG_LAPTOP > tristate "Samsung Laptop driver" > depends on RFKILL || RFKILL = n > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index e1b142947067..32ec4cb9d902 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -95,8 +95,9 @@ obj-$(CONFIG_PCENGINES_APU2) += pcengines-apuv2.o > obj-$(CONFIG_BARCO_P50_GPIO) += barco-p50-gpio.o > > # Samsung > -obj-$(CONFIG_SAMSUNG_LAPTOP) += samsung-laptop.o > -obj-$(CONFIG_SAMSUNG_Q10) += samsung-q10.o > +obj-$(CONFIG_SAMSUNG_GALAXYBOOK) += samsung-galaxybook.o > +obj-$(CONFIG_SAMSUNG_LAPTOP) += samsung-laptop.o > +obj-$(CONFIG_SAMSUNG_Q10) += samsung-q10.o > > # Toshiba > obj-$(CONFIG_TOSHIBA_BT_RFKILL) += toshiba_bluetooth.o > diff --git a/drivers/platform/x86/samsung-galaxybook-trace.h b/drivers/platform/x86/samsung-galaxybook-trace.h > new file mode 100644 > index 000000000000..09ab6dbe6586 > --- /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@joshuagrisham.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 > new file mode 100644 > index 000000000000..7baa3441fbfa > --- /dev/null > +++ b/drivers/platform/x86/samsung-galaxybook.c > @@ -0,0 +1,1380 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Samsung Galaxy Book series extras driver > + * > + * Copyright (c) 2024 Joshua Grisham <josh@joshuagrisham.com> > + * > + * With contributions to the SCAI ACPI device interface: > + * Copyright (c) 2024 Giulio Girardi <giulio.girardi@protechgroup.it> > + * > + * Implementation inspired by existing x86 platform drivers. > + * Thank you to the authors! > + */ > + > +#include <linux/acpi.h> > +#include <linux/err.h> > +#include <linux/i8042.h> > +#include <linux/init.h> > +#include <linux/input.h> > +#include <linux/input/sparse-keymap.h> > +#include <linux/kernel.h> > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/moduleparam.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 CREATE_TRACE_POINTS > +#include "samsung-galaxybook-trace.h" > + > +#define DRIVER_NAME "samsung-galaxybook" > + > +/* > + * Module parameters > + */ > + > +static bool kbd_backlight = true; > +static bool battery_threshold = true; > +static bool performance_mode = true; > +static bool allow_recording = true; > +static bool i8042_filter = true; > + > +module_param(kbd_backlight, bool, 0); > +MODULE_PARM_DESC(kbd_backlight, "Enable Keyboard Backlight control (default on)"); > +module_param(battery_threshold, bool, 0); > +MODULE_PARM_DESC(battery_threshold, "Enable battery charge threshold control (default on)"); > +module_param(performance_mode, bool, 0); > +MODULE_PARM_DESC(performance_mode, "Enable Performance Mode control (default on)"); > +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(i8042_filter, bool, 0); > +MODULE_PARM_DESC(i8042_filter, "Enable capturing keyboard hotkey events (default on)"); > + > +/* > + * Device definitions and matching > + */ > + > +static const struct acpi_device_id galaxybook_device_ids[] = { > + { "SAM0427" }, > + { "SAM0428" }, > + { "SAM0429" }, > + { "SAM0430" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(acpi, galaxybook_device_ids); > + > +struct samsung_galaxybook { > + struct platform_device *platform; > + struct acpi_device *acpi; > + struct mutex acpi_lock; > + > + struct led_classdev kbd_backlight; > + > + struct input_dev *input; > + struct mutex input_lock; > + > + void *i8042_filter_ptr; > + struct work_struct kbd_backlight_hotkey_work; > + struct work_struct allow_recording_hotkey_work; > + > + struct acpi_battery_hook battery_hook; > + struct device_attribute charge_control_end_threshold_attr; > + > + u8 *profile_performance_modes; > + struct platform_profile_handler profile_handler; > +}; > +static struct samsung_galaxybook *galaxybook_ptr; > + > +struct sawb { > + u16 safn; > + u16 sasb; > + u8 rflg; > + union { > + struct { > + u8 gunm; > + u8 guds[250]; > + }; > + struct { > + u8 caid[16]; > + u8 fncn; > + u8 subn; > + u8 iob0; > + u8 iob1; > + u8 iob2; > + u8 iob3; > + u8 iob4; > + u8 iob5; > + u8 iob6; > + u8 iob7; > + u8 iob8; > + u8 iob9; > + }; > + struct { > + u8 iob_prefix[18]; > + u8 iob_values[10]; > + }; > + }; > +}; __packed for device ABI structs. > + > +#define SAWB_LEN_SETTINGS 0x15 > +#define SAWB_LEN_PERFORMANCE_MODE 0x100 > + > +#define SAFN 0x5843 > + > +#define SASB_KBD_BACKLIGHT 0x78 > +#define SASB_POWER_MANAGEMENT 0x7a > +#define SASB_USB_CHARGE_GET 0x67 > +#define SASB_USB_CHARGE_SET 0x68 > +#define SASB_NOTIFICATIONS 0x86 > +#define SASB_ALLOW_RECORDING 0x8a > +#define SASB_PERFORMANCE_MODE 0x91 > + > +#define SAWB_RFLG_POS 4 > +#define SAWB_GUNM_POS 5 > + > +#define RFLG_SUCCESS 0xaa > +#define GUNM_FAIL 0xff > + > +#define GUNM_FEATURE_ENABLE 0xbb > +#define GUNM_FEATURE_ENABLE_SUCCESS 0xdd > +#define GUDS_FEATURE_ENABLE 0xaa > +#define GUDS_FEATURE_ENABLE_SUCCESS 0xcc > + > +#define GUNM_GET 0x81 > +#define GUNM_SET 0x82 > + > +#define GUNM_POWER_MANAGEMENT 0x82 > + > +#define GUNM_USB_CHARGE_GET 0x80 > +#define GUNM_USB_CHARGE_ON 0x81 > +#define GUNM_USB_CHARGE_OFF 0x80 > +#define GUDS_START_ON_LID_OPEN 0xa3 > +#define GUDS_START_ON_LID_OPEN_GET 0x81 > +#define GUDS_START_ON_LID_OPEN_SET 0x80 > +#define GUDS_BATTERY_CHARGE_CONTROL 0xe9 > +#define GUDS_BATTERY_CHARGE_CONTROL_GET 0x91 > +#define GUDS_BATTERY_CHARGE_CONTROL_SET 0x90 > +#define GUNM_ACPI_NOTIFY_ENABLE 0x80 > +#define GUDS_ACPI_NOTIFY_ENABLE 0x02 > + > +#define FNCN_PERFORMANCE_MODE 0x51 > +#define SUBN_PERFORMANCE_MODE_LIST 0x01 > +#define SUBN_PERFORMANCE_MODE_GET 0x02 > +#define SUBN_PERFORMANCE_MODE_SET 0x03 > + > +/* guid 8246028d-8bca-4a55-ba0f-6f1e6b921b8f */ > +static const guid_t performance_mode_guid_value = > + GUID_INIT(0x8246028d, 0x8bca, 0x4a55, 0xba, 0x0f, 0x6f, 0x1e, 0x6b, 0x92, 0x1b, 0x8f); > +#define PERFORMANCE_MODE_GUID performance_mode_guid_value > + > +#define PERFORMANCE_MODE_ULTRA 0x16 > +#define PERFORMANCE_MODE_PERFORMANCE 0x15 > +#define PERFORMANCE_MODE_SILENT 0xb > +#define PERFORMANCE_MODE_QUIET 0xa > +#define PERFORMANCE_MODE_OPTIMIZED 0x2 > +#define PERFORMANCE_MODE_PERFORMANCE_LEGACY 0x1 > +#define PERFORMANCE_MODE_OPTIMIZED_LEGACY 0x0 > +#define PERFORMANCE_MODE_UNKNOWN 0xff > + > +#define DEFAULT_PLATFORM_PROFILE PLATFORM_PROFILE_BALANCED > + > +#define ACPI_METHOD_ENABLE "SDLS" > +#define ACPI_METHOD_ENABLE_ON 1 > +#define ACPI_METHOD_ENABLE_OFF 0 > +#define ACPI_METHOD_SETTINGS "CSFI" > +#define ACPI_METHOD_PERFORMANCE_MODE "CSXI" > + > +#define KBD_BACKLIGHT_MAX_BRIGHTNESS 3 > + > +#define ACPI_NOTIFY_BATTERY_STATE_CHANGED 0x61 > +#define ACPI_NOTIFY_DEVICE_ON_TABLE 0x6c > +#define ACPI_NOTIFY_DEVICE_OFF_TABLE 0x6d > +#define ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE 0x70 > + > +#define GB_KEY_KBD_BACKLIGHT_KEYDOWN 0x2c > +#define GB_KEY_KBD_BACKLIGHT_KEYUP 0xac > +#define GB_KEY_ALLOW_RECORDING_KEYDOWN 0x1f > +#define GB_KEY_ALLOW_RECORDING_KEYUP 0x9f > + > +static const struct key_entry galaxybook_acpi_keymap[] = { > + { KE_KEY, ACPI_NOTIFY_BATTERY_STATE_CHANGED, { KEY_BATTERY } }, > + { KE_KEY, ACPI_NOTIFY_DEVICE_ON_TABLE, { KEY_F14 } }, > + { KE_KEY, ACPI_NOTIFY_DEVICE_OFF_TABLE, { KEY_F15 } }, > + { KE_KEY, ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE, { KEY_UNKNOWN } }, > + { KE_END, 0 }, > +}; > + > +/* > + * ACPI method handling > + */ > + > +static int galaxybook_acpi_method(struct samsung_galaxybook *galaxybook, acpi_string method, > + 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; > + 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 *)in_buf; > + > + input.count = 1; > + input.pointer = &in_obj; > + > + trace_samsung_galaxybook_acpi(dev_name(&galaxybook->acpi->dev), method, purpose_str, > + in_obj.buffer.pointer, in_obj.buffer.length); > + > + status = acpi_evaluate_object_typed(galaxybook->acpi->handle, method, &input, &output, > + ACPI_TYPE_BUFFER); > + > + if (ACPI_FAILURE(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; > + > + 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 || 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); > + err = -ETOOSMALL; > + goto out_free; > + } > + if (out_obj->buffer.pointer[SAWB_RFLG_POS] != RFLG_SUCCESS) { > + 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); > + err = -EIO; > + goto out_free; > + } > + if (out_obj->buffer.pointer[SAWB_GUNM_POS] == GUNM_FAIL) { > + dev_err(&galaxybook->acpi->dev, > + "failed %s with ACPI method %s; device responded with failure code 0x%x\n", > + purpose_str, method, GUNM_FAIL); > + err = -EIO; > + goto out_free; > + } > + > + memcpy(out_buf, out_obj->buffer.pointer, len); > + err = 0; > + > +out_free: > + kfree(out_obj); > + mutex_unlock(&galaxybook->acpi_lock); > + return err; > +} > + > +static int galaxybook_enable_acpi_feature(struct samsung_galaxybook *galaxybook, const u16 sasb) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = sasb; > + buf.gunm = GUNM_FEATURE_ENABLE; > + buf.guds[0] = GUDS_FEATURE_ENABLE; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "enabling ACPI feature", &buf); > + if (err) > + return err; > + > + if (buf.gunm != GUNM_FEATURE_ENABLE_SUCCESS && buf.guds[0] != GUDS_FEATURE_ENABLE_SUCCESS) > + return -ENODEV; > + > + return 0; > +} > + > +/* > + * Keyboard Backlight > + */ > + > +static int kbd_backlight_acpi_set(struct samsung_galaxybook *galaxybook, > + const enum led_brightness brightness) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_KBD_BACKLIGHT; > + buf.gunm = GUNM_SET; > + > + buf.guds[0] = brightness; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "setting kbd_backlight brightness", &buf); > + if (err) > + return err; > + > + galaxybook->kbd_backlight.brightness = brightness; It should not be necessary to touch the internals of the backlight device here. > + > + return 0; > +} > + > +static int kbd_backlight_acpi_get(struct samsung_galaxybook *galaxybook, > + enum led_brightness *brightness) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_KBD_BACKLIGHT; > + buf.gunm = GUNM_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting kbd_backlight brightness", &buf); > + if (err) > + return err; > + > + *brightness = buf.gunm; > + galaxybook->kbd_backlight.brightness = buf.gunm; Same as above. > + > + return 0; > +} > + > +static int kbd_backlight_store(struct led_classdev *led, > + const enum led_brightness brightness) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(led, struct samsung_galaxybook, kbd_backlight); container_of_const() is slightly safer. (Doesn't matter here, but for new code, let's do it correctly) > + > + return kbd_backlight_acpi_set(galaxybook, brightness); > +} > + > +static enum led_brightness kbd_backlight_show(struct led_classdev *led) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(led, struct samsung_galaxybook, kbd_backlight); > + enum led_brightness brightness; > + int err; > + > + err = kbd_backlight_acpi_get(galaxybook, &brightness); > + if (err) > + return err; > + > + return brightness; > +} > + > +static int galaxybook_kbd_backlight_init(struct samsung_galaxybook *galaxybook) > +{ > + struct led_init_data init_data = {}; > + enum led_brightness brightness; > + int err; > + > + err = galaxybook_enable_acpi_feature(galaxybook, SASB_KBD_BACKLIGHT); > + if (err) > + return err; > + > + /* verify we can read the value, otherwise init should stop and fail */ > + err = kbd_backlight_acpi_get(galaxybook, &brightness); > + if (err) > + return err; > + > + init_data.devicename = DRIVER_NAME; > + init_data.default_label = ":" LED_FUNCTION_KBD_BACKLIGHT; > + init_data.devname_mandatory = true; > + > + 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; > + > + return devm_led_classdev_register_ext(&galaxybook->platform->dev, > + &galaxybook->kbd_backlight, &init_data); > +} > + > +/* > + * Platform device attributes (configuration properties which can be controlled via userspace) > + */ > + > +/* Start on lid open (device should power on when lid is opened) */ > + > +static int start_on_lid_open_acpi_set(struct samsung_galaxybook *galaxybook, const bool value) > +{ > + struct sawb buf = { 0 }; > + > + 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 ? 1 : 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) > +{ > + 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_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting start_on_lid_open", &buf); > + if (err) > + return err; > + > + *value = buf.guds[1]; > + > + return 0; > +} > + > +static ssize_t start_on_lid_open_store(struct device *dev, struct device_attribute *attr, > + const char *buffer, size_t count) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + 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; > + > + return count; > +} > + > +static ssize_t start_on_lid_open_show(struct device *dev, struct device_attribute *attr, > + char *buffer) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + err = start_on_lid_open_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + return sysfs_emit(buffer, "%u\n", value); > +} > + > +static DEVICE_ATTR_RW(start_on_lid_open); > + > +/* USB Charge (USB ports can charge other devices even when device is powered off) */ > + > +static int usb_charge_acpi_set(struct samsung_galaxybook *galaxybook, const bool value) > +{ > + struct sawb buf = { 0 }; > + > + buf.safn = SAFN; > + buf.sasb = SASB_USB_CHARGE_SET; > + buf.gunm = value ? GUNM_USB_CHARGE_ON : GUNM_USB_CHARGE_OFF; > + > + 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_USB_CHARGE_GET; > + buf.gunm = GUNM_USB_CHARGE_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting usb_charge", &buf); > + if (err) > + return err; > + > + *value = buf.gunm; > + > + return 0; > +} > + > +static ssize_t usb_charge_store(struct device *dev, struct device_attribute *attr, > + const char *buffer, size_t count) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + if (!count) > + return -EINVAL; > + > + err = kstrtobool(buffer, &value); > + if (err) > + return err; > + > + err = usb_charge_acpi_set(galaxybook, value); > + if (err) > + return err; > + > + return count; > +} > + > +static ssize_t usb_charge_show(struct device *dev, struct device_attribute *attr, char *buffer) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + err = usb_charge_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + return sysfs_emit(buffer, "%u\n", value); > +} > + > +static DEVICE_ATTR_RW(usb_charge); > + > +/* Allow recording (allows or blocks access to camera and microphone) */ > + > +static int allow_recording_acpi_set(struct samsung_galaxybook *galaxybook, const bool value) > +{ > + struct sawb buf = { 0 }; > + > + buf.safn = SAFN; > + buf.sasb = SASB_ALLOW_RECORDING; > + buf.gunm = GUNM_SET; > + buf.guds[0] = value ? 1 : 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_ALLOW_RECORDING; > + buf.gunm = GUNM_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting allow_recording", &buf); > + if (err) > + return err; > + > + *value = buf.gunm == 1; > + > + return 0; > +} > + > +static ssize_t allow_recording_store(struct device *dev, struct device_attribute *attr, > + const char *buffer, size_t count) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + if (!count) > + return -EINVAL; > + > + err = kstrtobool(buffer, &value); > + if (err) > + return err; > + > + err = allow_recording_acpi_set(galaxybook, value); > + if (err) > + return err; > + > + return count; > +} > + > +static ssize_t allow_recording_show(struct device *dev, struct device_attribute *attr, char *buffer) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + err = allow_recording_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + return sysfs_emit(buffer, "%u\n", value); > +} > + > +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, u8 value) > +{ > + struct sawb buf = { 0 }; > + > + 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; > + > + 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + 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_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting battery charge_control_end_threshold", &buf); > + if (err) > + return err; > + > + *value = 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 samsung_galaxybook *galaxybook = > + container_of(attr, struct samsung_galaxybook, charge_control_end_threshold_attr); > + u8 value; > + int err; > + > + if (!count) > + return -EINVAL; > + > + err = kstrtou8(buffer, 0, &value); > + if (err) > + return err; > + > + err = charge_control_end_threshold_acpi_set(galaxybook, value); > + if (err) > + return err; > + > + return count; > +} > + > +static ssize_t charge_control_end_threshold_show(struct device *dev, struct device_attribute *attr, > + char *buffer) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(attr, struct samsung_galaxybook, charge_control_end_threshold_attr); > + u8 value; > + int err; > + > + err = charge_control_end_threshold_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + return sysfs_emit(buffer, "%d\n", value); > +} > + > +static int galaxybook_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(hook, struct samsung_galaxybook, battery_hook); > + > + return device_create_file(&battery->dev, &galaxybook->charge_control_end_threshold_attr); > +} > + > +static int galaxybook_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(hook, struct samsung_galaxybook, battery_hook); > + > + device_remove_file(&battery->dev, &galaxybook->charge_control_end_threshold_attr); > + return 0; > +} > + > +static int galaxybook_battery_threshold_init(struct samsung_galaxybook *galaxybook) > +{ > + struct acpi_battery_hook *hook; > + struct device_attribute *attr; > + u8 value; > + int err; > + > + err = charge_control_end_threshold_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + hook = &galaxybook->battery_hook; > + hook->add_battery = galaxybook_battery_add; > + hook->remove_battery = galaxybook_battery_remove; > + hook->name = "Samsung Galaxy Book Battery Extension"; > + > + attr = &galaxybook->charge_control_end_threshold_attr; > + sysfs_attr_init(attr->attr); sysfs_attr_init(&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; > + > + return devm_battery_hook_register(&galaxybook->platform->dev, &galaxybook->battery_hook); > +} > + > +/* > + * Platform Profile / Performance mode > + */ > + > +static int performance_mode_acpi_set(struct samsung_galaxybook *galaxybook, > + const u8 performance_mode) > +{ > + struct sawb buf = { 0 }; > + > + buf.safn = SAFN; > + buf.sasb = SASB_PERFORMANCE_MODE; > + export_guid(buf.caid, &PERFORMANCE_MODE_GUID); > + buf.fncn = FNCN_PERFORMANCE_MODE; > + buf.subn = SUBN_PERFORMANCE_MODE_SET; > + buf.iob0 = performance_mode; > + > + 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_PERFORMANCE_MODE; > + export_guid(buf.caid, &PERFORMANCE_MODE_GUID); > + buf.fncn = FNCN_PERFORMANCE_MODE; > + buf.subn = SUBN_PERFORMANCE_MODE_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_PERFORMANCE_MODE, &buf, > + SAWB_LEN_PERFORMANCE_MODE, "getting performance_mode", &buf); > + if (err) > + return err; > + > + *performance_mode = buf.iob0; You could fit this into the return value, removing the need for an output parameter. > + > + return 0; > +} > + > +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) { > + if (profile) > + *profile = i; > + return 0; > + } > + } > + > + 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); > + > + return performance_mode_acpi_set(galaxybook, > + galaxybook->profile_performance_modes[profile]); > +} > + > +static int galaxybook_platform_profile_get(struct platform_profile_handler *pprof, > + enum platform_profile_option *profile) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(pprof, struct samsung_galaxybook, profile_handler); > + u8 performance_mode; > + int err; > + > + err = performance_mode_acpi_get(galaxybook, &performance_mode); > + if (err) > + return err; > + > + return get_performance_mode_profile(galaxybook, performance_mode, profile); > +} > + > +static void galaxybook_profile_exit(void *data) > +{ > + struct samsung_galaxybook *galaxybook = data; > + > + platform_profile_remove(&galaxybook->profile_handler); > +} > + > +#define IGNORE_PERFORMANCE_MODE_MAPPING -1 > + > +static int galaxybook_profile_init(struct samsung_galaxybook *galaxybook) > +{ > + 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; > + > + /* fetch supported performance mode values from ACPI method */ > + buf.safn = SAFN; > + buf.sasb = SASB_PERFORMANCE_MODE; > + export_guid(buf.caid, &PERFORMANCE_MODE_GUID); > + buf.fncn = FNCN_PERFORMANCE_MODE; > + buf.subn = SUBN_PERFORMANCE_MODE_LIST; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_PERFORMANCE_MODE, > + &buf, SAWB_LEN_PERFORMANCE_MODE, > + "get supported performance modes", &buf); > + if (err) > + return err; > + > + /* set up profile_performance_modes with "unknown" as init value */ > + galaxybook->profile_performance_modes = > + kcalloc(PLATFORM_PROFILE_LAST, sizeof(u8), GFP_KERNEL); These are 7 bytes; just inline the array into 'struct samsung_galaxybook'. > + if (!galaxybook->profile_performance_modes) > + return -ENOMEM; > + for (i = 0; i < PLATFORM_PROFILE_LAST; i++) > + galaxybook->profile_performance_modes[i] = PERFORMANCE_MODE_UNKNOWN; > + > + /* > + * Value returned in iob0 will have the number of supported performance modes. > + * The performance mode values will then be given as a list after this (iob1-iobX). > + * 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. > + */ > + 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 > + * (power-profiles-daemon, etc). > + * - performance = "ultra", otherwise "performance" > + * - balanced = "optimized", otherwise "performance" when "ultra" is supported > + * - low-power = "silent", otherwise "quiet" > + * Different models support different modes. Additional supported modes will be > + * mapped to profiles that fall in between these 3. > + */ > + switch (buf.iob_values[i]) { > + > + case PERFORMANCE_MODE_ULTRA: > + /* ultra always maps to performance */ > + mode_profile = PLATFORM_PROFILE_PERFORMANCE; > + break; > + > + case PERFORMANCE_MODE_PERFORMANCE: > + /* if ultra exists, map performance to balanced-performance */ > + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_PERFORMANCE] != > + PERFORMANCE_MODE_UNKNOWN) > + mode_profile = PLATFORM_PROFILE_BALANCED_PERFORMANCE; > + else /* otherwise map it to performance instead */ > + mode_profile = PLATFORM_PROFILE_PERFORMANCE; > + break; > + > + case PERFORMANCE_MODE_SILENT: > + /* silent always maps to low-power */ > + mode_profile = PLATFORM_PROFILE_LOW_POWER; > + break; > + > + case PERFORMANCE_MODE_QUIET: > + /* if silent exists, map quiet to quiet */ > + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_LOW_POWER] != > + PERFORMANCE_MODE_UNKNOWN) > + mode_profile = PLATFORM_PROFILE_QUIET; > + else /* otherwise map it to low-power for better userspace tool support */ > + mode_profile = PLATFORM_PROFILE_LOW_POWER; > + break; > + > + case PERFORMANCE_MODE_OPTIMIZED: > + /* optimized always maps to balanced */ > + mode_profile = PLATFORM_PROFILE_BALANCED; > + break; > + > + case PERFORMANCE_MODE_PERFORMANCE_LEGACY: > + /* map to performance if performance is not already supported */ > + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_PERFORMANCE] == > + PERFORMANCE_MODE_UNKNOWN) > + mode_profile = PLATFORM_PROFILE_PERFORMANCE; > + else /* otherwise, ignore */ > + mode_profile = IGNORE_PERFORMANCE_MODE_MAPPING; > + break; > + > + case PERFORMANCE_MODE_OPTIMIZED_LEGACY: > + /* map to balanced if balanced is not already supported */ > + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_BALANCED] == > + PERFORMANCE_MODE_UNKNOWN) > + mode_profile = PLATFORM_PROFILE_BALANCED; > + else /* otherwise, ignore */ > + mode_profile = IGNORE_PERFORMANCE_MODE_MAPPING; > + break; > + > + default: /* any other value is not supported */ > + mode_profile = IGNORE_PERFORMANCE_MODE_MAPPING; > + break; > + } > + > + /* if current mode value mapped to a supported platform_profile_option, set it up */ > + 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); > + dev_dbg(&galaxybook->platform->dev, > + "will support platform profile %d (performance mode 0x%x)\n", > + mode_profile, buf.iob_values[i]); > + } else { > + dev_dbg(&galaxybook->platform->dev, > + "unmapped performance mode 0x%x will be ignored\n", > + buf.iob_values[i]); > + } > + } > + > + if (mapped_profiles == 0) > + return -ENODEV; > + > + err = platform_profile_register(&galaxybook->profile_handler); > + if (err) > + return err; > + > + /* now check currently set performance mode; if not supported then set default profile */ > + err = performance_mode_acpi_get(galaxybook, ¤t_performance_mode); > + if (err) > + 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) > + 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, galaxybook); > + if (err) > + goto err_remove_exit; > + > + return 0; > + > +err_remove_exit: > + galaxybook_profile_exit(galaxybook); > + return err; > +} > + > +/* > + * Hotkey work and filters > + */ > + > +static void galaxybook_kbd_backlight_hotkey_work(struct work_struct *work) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(work, struct samsung_galaxybook, kbd_backlight_hotkey_work); > + > + if (galaxybook->kbd_backlight.brightness < galaxybook->kbd_backlight.max_brightness) > + kbd_backlight_acpi_set(galaxybook, galaxybook->kbd_backlight.brightness + 1); > + else > + kbd_backlight_acpi_set(galaxybook, 0); > + > + led_classdev_notify_brightness_hw_changed( > + &galaxybook->kbd_backlight, > + galaxybook->kbd_backlight.brightness); > +} > + > +static void galaxybook_allow_recording_hotkey_work(struct work_struct *work) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(work, struct samsung_galaxybook, allow_recording_hotkey_work); > + bool value; > + > + allow_recording_acpi_get(galaxybook, &value); > + allow_recording_acpi_set(galaxybook, !value); > +} > + > +static bool galaxybook_i8042_filter(unsigned char data, unsigned char str, struct serio *port) > +{ > + static bool extended; > + > + if (str & I8042_STR_AUXDATA) > + return false; > + > + if (unlikely(data == 0xe0)) { No need for unlikely() in normal driver code. > + extended = true; > + return true; > + } else if (unlikely(extended)) { > + extended = false; > + switch (data) { > + > + case GB_KEY_KBD_BACKLIGHT_KEYDOWN: > + return true; > + case GB_KEY_KBD_BACKLIGHT_KEYUP: > + if (kbd_backlight) > + schedule_work(&galaxybook_ptr->kbd_backlight_hotkey_work); > + return true; > + > + case GB_KEY_ALLOW_RECORDING_KEYDOWN: > + return true; > + case GB_KEY_ALLOW_RECORDING_KEYUP: > + if (allow_recording) > + schedule_work(&galaxybook_ptr->allow_recording_hotkey_work); > + return true; > + > + default: > + /* > + * Report the previously filtered e0 before continuing > + * with the next non-filtered byte. > + */ > + serio_interrupt(port, 0xe0, 0); > + return false; > + } > + } > + > + return false; > +} > + > +static void galaxybook_i8042_filter_remove(void *data) > +{ > + 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_i8042_filter_install(struct samsung_galaxybook *galaxybook) > +{ > + int err; > + > + /* 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) { > + cancel_work_sync(&galaxybook->kbd_backlight_hotkey_work); The work queue may not haven been initialized if !kbd_backlight. > + cancel_work_sync(&galaxybook->allow_recording_hotkey_work); > + return err; > + } > + > + /* 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); The _or_reset() suffix indicates that the cleanup action will automatically be executed on failure. So you don't have to call it yourself below. > + if (err) { > + galaxybook_i8042_filter_remove(galaxybook); > + return err; > + } > + > + return 0; > +} > + > +/* > + * Input device (hotkeys and notifications) > + */ > + > +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_input_init(struct samsung_galaxybook *galaxybook) > +{ > + int err; > + > + galaxybook->input = devm_input_allocate_device(&galaxybook->platform->dev); > + if (!galaxybook->input) > + return -ENOMEM; > + > + 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 = sparse_keymap_setup(galaxybook->input, galaxybook_acpi_keymap, NULL); > + if (err) > + return err; > + > + return input_register_device(galaxybook->input); > +} > + > +/* > + * ACPI device setup > + */ > + > +static void galaxybook_acpi_notify(acpi_handle handle, u32 event, void *data) > +{ > + struct samsung_galaxybook *galaxybook = data; > + > + if (event == ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE) { > + if (performance_mode) { > + 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + err = galaxybook_enable_acpi_feature(galaxybook, SASB_NOTIFICATIONS); > + if (err) > + return err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_NOTIFICATIONS; > + buf.gunm = GUNM_ACPI_NOTIFY_ENABLE; > + buf.guds[0] = GUDS_ACPI_NOTIFY_ENABLE; > + > + return galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "activate ACPI notifications", &buf); > +} > + > +static void galaxybook_acpi_remove_notify_handler(void *data) > +{ > + struct samsung_galaxybook *galaxybook = data; > + > + acpi_remove_notify_handler(galaxybook->acpi->handle, ACPI_ALL_NOTIFY, > + galaxybook_acpi_notify); > +} > + > +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) \ Use uppercase name for macros. > +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; > + int err; > + > + if (!adev) > + return -ENODEV; > + > + dev_dbg(&pdev->dev, "loading driver\n"); Remove generic lifecycle logging for submission. The driver core can already provide this. > + > + galaxybook = devm_kzalloc(&pdev->dev, sizeof(*galaxybook), GFP_KERNEL); > + if (!galaxybook) > + return -ENOMEM; > + /* 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); Check return values. > + > + err = galaxybook_acpi_init(galaxybook); > + if (err) { > + dev_err(&galaxybook->acpi->dev, "failed to initialize the ACPI device\n"); return dev_err_probe() > + return err; > + } > + > + err = galaxybook_enable_acpi_feature(galaxybook, SASB_POWER_MANAGEMENT); > + if (err) { > + 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; > + } > + > + galaxybook_init_feature(performance_mode, galaxybook_profile_init); > + galaxybook_init_feature(battery_threshold, galaxybook_battery_threshold_init); > + > + /* add attrs group here as the is_visible requires above initializations */ > + err = devm_device_add_group(&galaxybook->platform->dev, &galaxybook_attrs_group); > + if (err) > + dev_warn(&galaxybook->platform->dev, "failed to add platform device attributes\n"); Why not use driver.dev_groups? They will only be added after the driver has probed successfully. > + > + 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; > + > + galaxybook_init_feature(i8042_filter, galaxybook_i8042_filter_install); > + > + dev_dbg(&galaxybook->platform->dev, "driver successfully loaded\n"); > + > + return 0; > +} > + > +static void galaxybook_remove(struct platform_device *pdev) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(&pdev->dev); > + > + if (galaxybook_ptr) > + galaxybook_ptr = NULL; > + > + dev_dbg(&galaxybook->platform->dev, "driver removed\n"); > +} > + > +static struct platform_driver galaxybook_platform_driver = { Could this be a 'struct acpi_driver'? > + .driver = { > + .name = DRIVER_NAME, > + .acpi_match_table = galaxybook_device_ids, > + }, > + .probe = galaxybook_probe, > + .remove = galaxybook_remove, > +}; > +module_platform_driver(galaxybook_platform_driver); > + > +MODULE_AUTHOR("Joshua Grisham <josh@joshuagrisham.com>"); > +MODULE_DESCRIPTION("Samsung Galaxy Book Extras"); > +MODULE_LICENSE("GPL"); > -- > 2.45.2 > >
Hi, On 16-Dec-24 5:46 PM, Thomas Weißschuh wrote: > Hi Joshua! > > (disclaimer: I didn't read all the previous reviews) > Disclaimer: I also did not read all of the previous reviews. <snip> >> +Parameters >> +========== >> + >> +The driver includes a list of boolean parameters that allow for manually >> +enabling or disabling various features: > > Can you explain *why* these are configurable? In general the addition of > kernel parameters is frowned upon. Ack, Joshua if posssible just remove the kernel module parameters. We can always add them back later if there is a good reason for them. removing them later OTOH can be more tricky. <snip> >> +.. _settings-attributes: >> + >> +Settings Attributes >> +=================== >> + >> +Various hardware settings can be controlled by the following sysfs attributes: >> + >> +- ``allow_recording`` (allows or blocks usage of built-in camera and microphone) >> +- ``start_on_lid_open`` (power on automatically when opening the lid) >> +- ``usb_charge`` (allows USB ports to provide power even when device is off) > > Non-standard sysfs attributes should be avoided where possible. > Userspace will need bespoke code to handle them. > This looks like it could be handled by the standard firmware_attributes > interface. > This would standardize discovery and usage. Ack this really feels like firmware-attributes. I would not be surprised if there are matching BIOS settings and if changing those also changes the sysfs files and likewise if the sysfs settings persist over reboot. <snip> >> +Keyboard hotkey actions (i8042 filter) >> +====================================== >> + >> +Controlled by parameter: ``i8042_filter`` >> + >> +The i8042 filter will swallow the keyboard events for the Fn+F9 hotkey (Multi- >> +level keyboard backlight toggle) and Fn+F10 hotkey (Allow/block recording >> +toggle) and instead execute their actions within the driver itself. >> + >> +Fn+F9 will cycle through the brightness levels of the keyboard backlight. A >> +notification will be sent using ``led_classdev_notify_brightness_hw_changed`` >> +so that the userspace can be aware of the change. This mimics the behavior of >> +other existing devices where the brightness level is cycled internally by the >> +embedded controller and then reported via a notification. >> + >> +Fn+F10 will toggle the value of the "Allow recording" setting. > > Personally I'm not a big fan to implement policy this way in the kernel. > But others may disagree. The keyboard backlight cycling ws already discussed and handling this in the driver is ok. The allow-recording setting toggle is new to me. So this is changeable at runtime, interesting. Joshua above you write this toggle both the microphone mute and disables the camera? It would be good to report the camera state to userspace using a separate input/evdev device which reports SW_CAMERA_LENS_COVER as discussed here: https://lore.kernel.org/linux-media/CANiDSCtjpPG3XzaEOEeczZWO5gL-V_sj_Fv5=w82D6zKC9hnpw@mail.gmail.com/ the plan is to make that the canonical API to reported "muted" cameras. What happens with the camera when recording is disallowed, dus it drop of the USB bus or does it only produce black frames ? It is a bit unexpected that this one button controls both microphone and camera mute. But given that unique behavior I guess that handling this in the kernel is probably best. The alsamixer should send some events for the mic mute/unmute I hope and we can use SW_CAMERA_LENS_COVER to report the camera state. <snip> >> +static struct platform_driver galaxybook_platform_driver = { > > Could this be a 'struct acpi_driver'? The use of acpi_driver is deprecated. AFAIK the plan it to move the remaining ones to platform-drivers and eventually remove the whole ACPI bus concept turning ACPI companion nodes into "normal" fwnodes like the current software and openfirmware fwnodes. Regards, Hans
On 2024-12-17 15:23:22+0100, Hans de Goede wrote: > On 16-Dec-24 5:46 PM, Thomas Weißschuh wrote: [..] > >> +Keyboard hotkey actions (i8042 filter) > >> +====================================== > >> + > >> +Controlled by parameter: ``i8042_filter`` > >> + > >> +The i8042 filter will swallow the keyboard events for the Fn+F9 hotkey (Multi- > >> +level keyboard backlight toggle) and Fn+F10 hotkey (Allow/block recording > >> +toggle) and instead execute their actions within the driver itself. > >> + > >> +Fn+F9 will cycle through the brightness levels of the keyboard backlight. A > >> +notification will be sent using ``led_classdev_notify_brightness_hw_changed`` > >> +so that the userspace can be aware of the change. This mimics the behavior of > >> +other existing devices where the brightness level is cycled internally by the > >> +embedded controller and then reported via a notification. > >> + > >> +Fn+F10 will toggle the value of the "Allow recording" setting. > > > > Personally I'm not a big fan to implement policy this way in the kernel. > > But others may disagree. > > The keyboard backlight cycling ws already discussed and handling this in > the driver is ok. Ack. [..] > >> +static struct platform_driver galaxybook_platform_driver = { > > > > Could this be a 'struct acpi_driver'? > > The use of acpi_driver is deprecated. AFAIK the plan it to > move the remaining ones to platform-drivers and eventually > remove the whole ACPI bus concept turning ACPI companion > nodes into "normal" fwnodes like the current software and > openfirmware fwnodes. Thanks for this explanation, I was not aware. It explains the comment from Rafael in [0]. [0] https://lore.kernel.org/lkml/CAJZ5v0g0B8UFqYza88ahMfC-1_FzzizeS6QN=Qtt7vGv9+TK1w@mail.gmail.com/
Am 16.12.24 um 11:38 schrieb Joshua Grisham: > Adds a new driver for Samsung Galaxy Book series notebook devices. This > includes the driver itself, a tracepoint header file, a new page in the > documentation, and relevant updates to Kconfig, Makefile, and MAINTAINERS > files related to this new driver. Can you make sure that scripts/checkpatch --strict find no more complaints in the next patch revision? Currently it finds 1 warning and 16 checks. > Signed-off-by: Joshua Grisham <josh@joshuagrisham.com> > --- > v1->v2: > - Attempt to resolve all review comments from v1 as written here: > https://lore.kernel.org/platform-driver-x86/53c5075b-1967-45d0-937f-463912dd966d@gmx.de/T/#mbcbd8d5d9bc4496bac5486636c7d3b32bc3e5cd0 > > v2->v3: > - Tweak to battery attribute to closer match pattern in dell-wmi-ddv > - implement platform_profile_remove() change from > 9b3bb37b44a317626464e79da8b39989b421963f > - Small tweak to Documentation page > --- > .../laptops/samsung-galaxybook.rst | 280 ++++ > MAINTAINERS | 8 + > drivers/platform/x86/Kconfig | 18 + > drivers/platform/x86/Makefile | 5 +- > .../platform/x86/samsung-galaxybook-trace.h | 51 + > drivers/platform/x86/samsung-galaxybook.c | 1380 +++++++++++++++++ > 6 files changed, 1740 insertions(+), 2 deletions(-) > create mode 100644 Documentation/admin-guide/laptops/samsung-galaxybook.rst > create mode 100644 drivers/platform/x86/samsung-galaxybook-trace.h > create mode 100644 drivers/platform/x86/samsung-galaxybook.c > > diff --git a/Documentation/admin-guide/laptops/samsung-galaxybook.rst b/Documentation/admin-guide/laptops/samsung-galaxybook.rst > new file mode 100644 > index 000000000000..947810c5f998 > --- /dev/null > +++ b/Documentation/admin-guide/laptops/samsung-galaxybook.rst > @@ -0,0 +1,280 @@ > +.. SPDX-License-Identifier: GPL-2.0-or-later > + > +========================== > +Samsung Galaxy Book Extras > +========================== > + > +December 15, 2024 > + > +Joshua Grisham <josh@joshuagrisham.com> > + > +This is a Linux x86 platform driver for Samsung Galaxy Book series notebook > +devices which utilizes Samsung's ``SCAI`` ACPI device in order to control > +extra features and receive various notifications. > + Please only use a single empty lines in this doc. > + > +Supported devices > +================= > + > +Any device with one of the supported ACPI device IDs should be supported. This > +covers most of the "Samsung Galaxy Book" series notebooks that are currently > +available as of this writing, and could include other Samsung notebook devices > +as well. > + > + > +Status > +====== > + > +The following features are currently supported: > + > +- :ref:`Keyboard backlight <keyboard-backlight>` control > +- :ref:`Performance mode <performance-mode>` control implemented using the > + platform profile interface > +- :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:`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 > +for a valid response before enabling its support (registering additional devices > +or extensions, adding sysfs attributes, etc). Therefore, it can be important to > +note that not all features may be supported for your particular device. > + > +The following features might be possible to implement but will require > +additional investigation and are therefore not supported at this time: > + > +- "Dolby Atmos" mode for the speakers > +- "Outdoor Mode" for increasing screen brightness on models with ``SAM0427`` > +- "Silent Mode" on models with ``SAM0427`` > + > + > +Parameters > +========== > + > +The driver includes a list of boolean parameters that allow for manually > +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) > +- ``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 > + (default on) > + > +.. note:: > + Even if you explicitly try to enable a feature using its parameter, support > + for it will still be evaluated by the driver, and the feature will be > + disabled if it does not appear to be supported on your device. > + > +The availability of various sysfs file-based "settings" attributes > +(``usb_charge``, ``start_on_lid_open``, etc) will be determined automatically > +and cannot be manually disabled at this time. > + > + > +.. _keyboard-backlight: > + > +Keyboard backlight > +================== > + > +Controlled by parameter: ``kbd_backlight`` > + > +A new LED class named ``samsung-galaxybook::kbd_backlight`` is created which > +will then expose the device using the standard sysfs-based LED interface at > +``/sys/class/leds/samsung-galaxybook::kbd_backlight``. Brightness can be > +controlled by writing values 0 to 3 to the ``brightness`` sysfs attribute or > +with any other desired userspace utility. > + > +.. note:: > + Most of these devices have an ambient light sensor which also turns > + off the keyboard backlight under well-lit conditions. This behavior does not > + seem possible to control at this time, but can be good to be aware of. > + > + > +.. _performance-mode: > + > +Performance mode > +================ > + > +Controlled by parameter: ``performance_mode`` > + > +This driver implements the > +Documentation/userspace-api/sysfs-platform_profile.rst interface for working > +with the "performance mode" function of the Samsung ACPI device. > + > +Mapping of each Samsung "performance mode" to its respective platform profile is > +done dynamically based on a list of the supported modes reported by the device > +itself. Preference is given to always try and map ``low-power``, ``balanced``, > +and ``performance`` profiles, as these seem to be the most common profiles > +utilized (and sometimes even required) by various userspace tools. > + > +The result of the mapping will be printed in the kernel log when the module is > +loaded. Supported profiles can also be retrieved from > +``/sys/firmware/acpi/platform_profile_choices``, while > +``/sys/firmware/acpi/platform_profile`` can be used to read or write the > +currently selected profile. > + > +The ``balanced`` platform profile will be set during module load if no profile > +has been previously set. > + > + > +.. _battery-charge-control-end-threshold: > + > +Battery charge control end threshold > +==================================== > + > +Controlled by parameter: ``battery_threshold`` > + > +This platform driver will add the ability to set the battery's charge control > +end threshold, but does not have the ability to set a start threshold. > + > +This feature is typically called "Battery Saver" by the various Samsung > +applications in Windows, but in Linux we have implemented the standardized > +"charge control threshold" sysfs interface on the battery device to allow for > +controlling this functionality from the userspace. > + > +The sysfs attribute > +``/sys/class/power_supply/BAT1/charge_control_end_threshold`` can be used to > +read or set the desired charge end threshold. > + > +If you wish to maintain interoperability with Windows, then you should set the > +value to 80 to represent "on", or 0 to represent "off", as these are the values > +currently recognized by the various Windows-based Samsung applications and > +services as "on" or "off". Otherwise, the device will accept any value between 0 > +(off) and 99 as the percentage that you wish the battery to stop charging at. > + > +.. note:: > + If you try to set a value of 100, the driver will also accept this input, but > + will set the attribute value to 0 (i.e. 100% will "remove" the end threshold). > + > + > +.. _settings-attributes: > + > +Settings Attributes > +=================== > + > +Various hardware settings can be controlled by the following sysfs attributes: > + > +- ``allow_recording`` (allows or blocks usage of built-in camera and microphone) > +- ``start_on_lid_open`` (power on automatically when opening the lid) > +- ``usb_charge`` (allows USB ports to provide power even when device is off) > + > +These attributes will be available under the path for your supported ACPI Device > +ID's platform device (``SAM0428``, ``SAM0429``, etc), and can most reliably > +be found by seeing which device has been bound to the ``samsung-galaxybook`` > +driver. Here are some examples: :: > + > + # find which device ID has been bound to the driver > + ls /sys/bus/platform/drivers/samsung-galaxybook/ | grep SAM > + > + # see SAM0429 attributes > + ls /sys/bus/platform/drivers/samsung-galaxybook/SAM0429\:00 > + > + # see attributes no matter the device ID (using wildcard expansion) > + ls /sys/bus/platform/drivers/samsung-galaxybook/SAM* > + > +Most shells should support using wildcard expansion to directly read and write > +these attributes using the above pattern. Example: :: > + > + # read value of start_on_lid_open > + cat /sys/bus/platform/drivers/samsung-galaxybook/SAM*/start_on_lid_open > + > + # turn on start_on_lid_open > + echo true | sudo tee /sys/bus/platform/drivers/samsung-galaxybook/SAM*/start_on_lid_open > + > +It is also possible to use a udev rule to create a fixed-path symlink to your > +device under ``/dev`` (e.g. ``/dev/samsung-galaxybook``), no matter the device > +ID, to further simplify reading and writing these attributes in the userspace. > + > +Allow recording (allow_recording) > +--------------------------------- > + > +``/sys/bus/platform/drivers/samsung-galaxybook/SAM*/allow_recording`` > + > +Controlled by parameter: ``allow_recording`` > + > +Controls the "Allow recording" setting, which allows or blocks usage of the > +built-in camera and microphone (boolean). > + > +Start on lid open (start_on_lid_open) > +------------------------------------- > + > +``/sys/bus/platform/drivers/samsung-galaxybook/SAM*/start_on_lid_open`` > + > +Controls the "Start on lid open" setting, which sets the device to power on > +automatically when the lid is opened (boolean). > + > +USB charge (usb_charge) > +----------------------- > + > +``/sys/bus/platform/drivers/samsung-galaxybook/SAM*/usb_charge`` > + > +Controls the "USB charge" setting, which allows USB ports to provide power even > +when the device is turned off (boolean). > + > +.. note:: > + For most devices, this setting seems to only apply to the USB-C ports. > + > + > +.. _keyboard-hotkey-actions: > + > +Keyboard hotkey actions (i8042 filter) > +====================================== > + > +Controlled by parameter: ``i8042_filter`` > + > +The i8042 filter will swallow the keyboard events for the Fn+F9 hotkey (Multi- > +level keyboard backlight toggle) and Fn+F10 hotkey (Allow/block recording > +toggle) and instead execute their actions within the driver itself. > + > +Fn+F9 will cycle through the brightness levels of the keyboard backlight. A > +notification will be sent using ``led_classdev_notify_brightness_hw_changed`` > +so that the userspace can be aware of the change. This mimics the behavior of > +other existing devices where the brightness level is cycled internally by the > +embedded controller and then reported via a notification. > + > +Fn+F10 will toggle the value of the "Allow recording" setting. > + > + > +ACPI notifications and ACPI hotkey actions > +========================================== > + > +There is a new "Samsung Galaxy Book extra buttons" input device created which > +will send input events for the following notifications from the ACPI device: > + > +- Notification when the battery charge control end threshold has been reached > + and the "battery saver" feature has stopped the battery from charging > +- Notification when the device has been placed on a table (not available on all > + models) > +- Notification when the device has been lifted from a table (not available on > + all models) > + > +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 3809931b9240..9e3b45cf799f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -20733,6 +20733,14 @@ L: linux-fbdev@vger.kernel.org > S: Maintained > F: drivers/video/fbdev/s3c-fb.c > > +SAMSUNG GALAXY BOOK EXTRAS DRIVER > +M: Joshua Grisham <josh@joshuagrisham.com> > +L: platform-driver-x86@vger.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 > M: Sylwester Nawrocki <s.nawrocki@samsung.com> > M: Artur Świgoń <a.swigon@samsung.com> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 0258dd879d64..03f4fb0e93e7 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -778,6 +778,24 @@ config BARCO_P50_GPIO > To compile this driver as a module, choose M here: the module > will be called barco-p50-gpio. > > +config SAMSUNG_GALAXYBOOK > + tristate "Samsung Galaxy Book extras driver" > + depends on ACPI > + depends on ACPI_BATTERY > + depends on INPUT > + depends on SERIO_I8042 > + select ACPI_PLATFORM_PROFILE > + select INPUT_SPARSEKMAP > + select NEW_LEDS > + select LEDS_CLASS > + help > + This is a driver for Samsung Galaxy Book series notebooks. It adds > + support for the keyboard backlight control, performance mode control, fan > + speed reporting, function keys, and various other device controls. > + > + For more information about this driver, see > + <file:Documentation/admin-guide/laptops/samsung-galaxybook.rst>. > + > config SAMSUNG_LAPTOP > tristate "Samsung Laptop driver" > depends on RFKILL || RFKILL = n > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index e1b142947067..32ec4cb9d902 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -95,8 +95,9 @@ obj-$(CONFIG_PCENGINES_APU2) += pcengines-apuv2.o > obj-$(CONFIG_BARCO_P50_GPIO) += barco-p50-gpio.o > > # Samsung > -obj-$(CONFIG_SAMSUNG_LAPTOP) += samsung-laptop.o > -obj-$(CONFIG_SAMSUNG_Q10) += samsung-q10.o > +obj-$(CONFIG_SAMSUNG_GALAXYBOOK) += samsung-galaxybook.o > +obj-$(CONFIG_SAMSUNG_LAPTOP) += samsung-laptop.o > +obj-$(CONFIG_SAMSUNG_Q10) += samsung-q10.o > > # Toshiba > obj-$(CONFIG_TOSHIBA_BT_RFKILL) += toshiba_bluetooth.o > diff --git a/drivers/platform/x86/samsung-galaxybook-trace.h b/drivers/platform/x86/samsung-galaxybook-trace.h > new file mode 100644 > index 000000000000..09ab6dbe6586 > --- /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@joshuagrisham.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)) > +); I CCed the tracing guys so they can share their opinion on this. > + > +#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 > new file mode 100644 > index 000000000000..7baa3441fbfa > --- /dev/null > +++ b/drivers/platform/x86/samsung-galaxybook.c > @@ -0,0 +1,1380 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Samsung Galaxy Book series extras driver > + * > + * Copyright (c) 2024 Joshua Grisham <josh@joshuagrisham.com> > + * > + * With contributions to the SCAI ACPI device interface: > + * Copyright (c) 2024 Giulio Girardi <giulio.girardi@protechgroup.it> > + * > + * Implementation inspired by existing x86 platform drivers. > + * Thank you to the authors! > + */ > + > +#include <linux/acpi.h> > +#include <linux/err.h> > +#include <linux/i8042.h> > +#include <linux/init.h> > +#include <linux/input.h> > +#include <linux/input/sparse-keymap.h> > +#include <linux/kernel.h> > +#include <linux/leds.h> > +#include <linux/module.h> > +#include <linux/moduleparam.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 CREATE_TRACE_POINTS > +#include "samsung-galaxybook-trace.h" > + > +#define DRIVER_NAME "samsung-galaxybook" > + > +/* > + * Module parameters > + */ > + > +static bool kbd_backlight = true; > +static bool battery_threshold = true; > +static bool performance_mode = true; > +static bool allow_recording = true; > +static bool i8042_filter = true; > + > +module_param(kbd_backlight, bool, 0); > +MODULE_PARM_DESC(kbd_backlight, "Enable Keyboard Backlight control (default on)"); > +module_param(battery_threshold, bool, 0); > +MODULE_PARM_DESC(battery_threshold, "Enable battery charge threshold control (default on)"); > +module_param(performance_mode, bool, 0); > +MODULE_PARM_DESC(performance_mode, "Enable Performance Mode control (default on)"); > +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(i8042_filter, bool, 0); > +MODULE_PARM_DESC(i8042_filter, "Enable capturing keyboard hotkey events (default on)"); > + > +/* > + * Device definitions and matching > + */ > + > +static const struct acpi_device_id galaxybook_device_ids[] = { > + { "SAM0427" }, > + { "SAM0428" }, > + { "SAM0429" }, > + { "SAM0430" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(acpi, galaxybook_device_ids); > + > +struct samsung_galaxybook { > + struct platform_device *platform; > + struct acpi_device *acpi; > + struct mutex acpi_lock; > + > + struct led_classdev kbd_backlight; > + > + struct input_dev *input; > + struct mutex input_lock; > + > + void *i8042_filter_ptr; > + struct work_struct kbd_backlight_hotkey_work; > + struct work_struct allow_recording_hotkey_work; > + > + struct acpi_battery_hook battery_hook; > + struct device_attribute charge_control_end_threshold_attr; > + > + u8 *profile_performance_modes; > + struct platform_profile_handler profile_handler; > +}; > +static struct samsung_galaxybook *galaxybook_ptr; > + > +struct sawb { > + u16 safn; > + u16 sasb; > + u8 rflg; > + union { > + struct { > + u8 gunm; > + u8 guds[250]; > + }; > + struct { > + u8 caid[16]; > + u8 fncn; > + u8 subn; > + u8 iob0; > + u8 iob1; > + u8 iob2; > + u8 iob3; > + u8 iob4; > + u8 iob5; > + u8 iob6; > + u8 iob7; > + u8 iob8; > + u8 iob9; > + }; > + struct { > + u8 iob_prefix[18]; > + u8 iob_values[10]; > + }; > + }; > +}; > + > +#define SAWB_LEN_SETTINGS 0x15 > +#define SAWB_LEN_PERFORMANCE_MODE 0x100 > + > +#define SAFN 0x5843 > + > +#define SASB_KBD_BACKLIGHT 0x78 > +#define SASB_POWER_MANAGEMENT 0x7a > +#define SASB_USB_CHARGE_GET 0x67 > +#define SASB_USB_CHARGE_SET 0x68 > +#define SASB_NOTIFICATIONS 0x86 > +#define SASB_ALLOW_RECORDING 0x8a > +#define SASB_PERFORMANCE_MODE 0x91 > + > +#define SAWB_RFLG_POS 4 > +#define SAWB_GUNM_POS 5 > + > +#define RFLG_SUCCESS 0xaa > +#define GUNM_FAIL 0xff > + > +#define GUNM_FEATURE_ENABLE 0xbb > +#define GUNM_FEATURE_ENABLE_SUCCESS 0xdd > +#define GUDS_FEATURE_ENABLE 0xaa > +#define GUDS_FEATURE_ENABLE_SUCCESS 0xcc > + > +#define GUNM_GET 0x81 > +#define GUNM_SET 0x82 > + > +#define GUNM_POWER_MANAGEMENT 0x82 > + > +#define GUNM_USB_CHARGE_GET 0x80 > +#define GUNM_USB_CHARGE_ON 0x81 > +#define GUNM_USB_CHARGE_OFF 0x80 > +#define GUDS_START_ON_LID_OPEN 0xa3 > +#define GUDS_START_ON_LID_OPEN_GET 0x81 > +#define GUDS_START_ON_LID_OPEN_SET 0x80 > +#define GUDS_BATTERY_CHARGE_CONTROL 0xe9 > +#define GUDS_BATTERY_CHARGE_CONTROL_GET 0x91 > +#define GUDS_BATTERY_CHARGE_CONTROL_SET 0x90 > +#define GUNM_ACPI_NOTIFY_ENABLE 0x80 > +#define GUDS_ACPI_NOTIFY_ENABLE 0x02 > + > +#define FNCN_PERFORMANCE_MODE 0x51 > +#define SUBN_PERFORMANCE_MODE_LIST 0x01 > +#define SUBN_PERFORMANCE_MODE_GET 0x02 > +#define SUBN_PERFORMANCE_MODE_SET 0x03 > + > +/* guid 8246028d-8bca-4a55-ba0f-6f1e6b921b8f */ > +static const guid_t performance_mode_guid_value = > + GUID_INIT(0x8246028d, 0x8bca, 0x4a55, 0xba, 0x0f, 0x6f, 0x1e, 0x6b, 0x92, 0x1b, 0x8f); > +#define PERFORMANCE_MODE_GUID performance_mode_guid_value > + > +#define PERFORMANCE_MODE_ULTRA 0x16 > +#define PERFORMANCE_MODE_PERFORMANCE 0x15 > +#define PERFORMANCE_MODE_SILENT 0xb > +#define PERFORMANCE_MODE_QUIET 0xa > +#define PERFORMANCE_MODE_OPTIMIZED 0x2 > +#define PERFORMANCE_MODE_PERFORMANCE_LEGACY 0x1 > +#define PERFORMANCE_MODE_OPTIMIZED_LEGACY 0x0 > +#define PERFORMANCE_MODE_UNKNOWN 0xff > + > +#define DEFAULT_PLATFORM_PROFILE PLATFORM_PROFILE_BALANCED > + > +#define ACPI_METHOD_ENABLE "SDLS" > +#define ACPI_METHOD_ENABLE_ON 1 > +#define ACPI_METHOD_ENABLE_OFF 0 > +#define ACPI_METHOD_SETTINGS "CSFI" > +#define ACPI_METHOD_PERFORMANCE_MODE "CSXI" > + > +#define KBD_BACKLIGHT_MAX_BRIGHTNESS 3 > + > +#define ACPI_NOTIFY_BATTERY_STATE_CHANGED 0x61 > +#define ACPI_NOTIFY_DEVICE_ON_TABLE 0x6c > +#define ACPI_NOTIFY_DEVICE_OFF_TABLE 0x6d > +#define ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE 0x70 > + > +#define GB_KEY_KBD_BACKLIGHT_KEYDOWN 0x2c > +#define GB_KEY_KBD_BACKLIGHT_KEYUP 0xac > +#define GB_KEY_ALLOW_RECORDING_KEYDOWN 0x1f > +#define GB_KEY_ALLOW_RECORDING_KEYUP 0x9f > + > +static const struct key_entry galaxybook_acpi_keymap[] = { > + { KE_KEY, ACPI_NOTIFY_BATTERY_STATE_CHANGED, { KEY_BATTERY } }, > + { KE_KEY, ACPI_NOTIFY_DEVICE_ON_TABLE, { KEY_F14 } }, > + { KE_KEY, ACPI_NOTIFY_DEVICE_OFF_TABLE, { KEY_F15 } }, > + { KE_KEY, ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE, { KEY_UNKNOWN } }, > + { KE_END, 0 }, > +}; > + > +/* > + * ACPI method handling > + */ > + > +static int galaxybook_acpi_method(struct samsung_galaxybook *galaxybook, acpi_string method, > + 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; > + acpi_status status; > + int err; > + > + mutex_lock(&galaxybook->acpi_lock); I do not think that providing locking around each ACPI method call is necessary, the AML code will do this itself if necessary. > + > + in_obj.type = ACPI_TYPE_BUFFER; > + in_obj.buffer.length = len; > + in_obj.buffer.pointer = (u8 *)in_buf; > + > + input.count = 1; > + input.pointer = &in_obj; > + > + trace_samsung_galaxybook_acpi(dev_name(&galaxybook->acpi->dev), method, purpose_str, > + in_obj.buffer.pointer, in_obj.buffer.length); > + > + status = acpi_evaluate_object_typed(galaxybook->acpi->handle, method, &input, &output, > + ACPI_TYPE_BUFFER); > + > + if (ACPI_FAILURE(status)) { > + dev_err(&galaxybook->acpi->dev, "failed %s with ACPI method %s; got %s\n", > + purpose_str, method, acpi_format_exception(status)); > + err = -ENXIO; -EIO please. > + goto out_free; out_obj is not assigned here so this would cause problems. Please just return directly. > + } > + > + out_obj = output.pointer; > + > + 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 || 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); > + err = -ETOOSMALL; -EPROTO please. > + goto out_free; > + } > + if (out_obj->buffer.pointer[SAWB_RFLG_POS] != RFLG_SUCCESS) { > + 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); > + err = -EIO; -ENXIO please. > + goto out_free; > + } > + if (out_obj->buffer.pointer[SAWB_GUNM_POS] == GUNM_FAIL) { > + dev_err(&galaxybook->acpi->dev, > + "failed %s with ACPI method %s; device responded with failure code 0x%x\n", > + purpose_str, method, GUNM_FAIL); > + err = -EIO; -ENXIO please. > + goto out_free; > + } > + > + memcpy(out_buf, out_obj->buffer.pointer, len); > + err = 0; > + > +out_free: > + kfree(out_obj); > + mutex_unlock(&galaxybook->acpi_lock); > + return err; > +} > + > +static int galaxybook_enable_acpi_feature(struct samsung_galaxybook *galaxybook, const u16 sasb) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = sasb; > + buf.gunm = GUNM_FEATURE_ENABLE; > + buf.guds[0] = GUDS_FEATURE_ENABLE; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "enabling ACPI feature", &buf); > + if (err) > + return err; > + > + if (buf.gunm != GUNM_FEATURE_ENABLE_SUCCESS && buf.guds[0] != GUDS_FEATURE_ENABLE_SUCCESS) > + return -ENODEV; > + > + return 0; > +} > + > +/* > + * Keyboard Backlight > + */ > + > +static int kbd_backlight_acpi_set(struct samsung_galaxybook *galaxybook, > + const enum led_brightness brightness) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_KBD_BACKLIGHT; > + buf.gunm = GUNM_SET; > + > + buf.guds[0] = brightness; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "setting kbd_backlight brightness", &buf); > + if (err) > + return err; > + > + galaxybook->kbd_backlight.brightness = brightness; This variable is handled by the LED class code, please do not access it. > + > + return 0; > +} > + > +static int kbd_backlight_acpi_get(struct samsung_galaxybook *galaxybook, > + enum led_brightness *brightness) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_KBD_BACKLIGHT; > + buf.gunm = GUNM_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting kbd_backlight brightness", &buf); > + if (err) > + return err; > + > + *brightness = buf.gunm; > + galaxybook->kbd_backlight.brightness = buf.gunm; Same as above. > + > + return 0; > +} > + > +static int kbd_backlight_store(struct led_classdev *led, > + const enum led_brightness brightness) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(led, struct samsung_galaxybook, kbd_backlight); > + > + return kbd_backlight_acpi_set(galaxybook, brightness); > +} > + > +static enum led_brightness kbd_backlight_show(struct led_classdev *led) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(led, struct samsung_galaxybook, kbd_backlight); > + enum led_brightness brightness; > + int err; > + > + err = kbd_backlight_acpi_get(galaxybook, &brightness); > + if (err) > + return err; > + > + return brightness; > +} > + > +static int galaxybook_kbd_backlight_init(struct samsung_galaxybook *galaxybook) > +{ > + struct led_init_data init_data = {}; > + enum led_brightness brightness; > + int err; > + > + err = galaxybook_enable_acpi_feature(galaxybook, SASB_KBD_BACKLIGHT); > + if (err) > + return err; > + > + /* verify we can read the value, otherwise init should stop and fail */ > + err = kbd_backlight_acpi_get(galaxybook, &brightness); > + if (err) > + return err; > + > + init_data.devicename = DRIVER_NAME; > + init_data.default_label = ":" LED_FUNCTION_KBD_BACKLIGHT; > + init_data.devname_mandatory = true; > + > + 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; > + > + return devm_led_classdev_register_ext(&galaxybook->platform->dev, > + &galaxybook->kbd_backlight, &init_data); > +} > + > +/* > + * Platform device attributes (configuration properties which can be controlled via userspace) > + */ > + > +/* Start on lid open (device should power on when lid is opened) */ > + > +static int start_on_lid_open_acpi_set(struct samsung_galaxybook *galaxybook, const bool value) > +{ > + struct sawb buf = { 0 }; > + > + 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 ? 1 : 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) > +{ > + 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_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting start_on_lid_open", &buf); > + if (err) > + return err; > + > + *value = buf.guds[1]; > + > + return 0; > +} > + > +static ssize_t start_on_lid_open_store(struct device *dev, struct device_attribute *attr, > + const char *buffer, size_t count) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + 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; > + > + return count; > +} > + > +static ssize_t start_on_lid_open_show(struct device *dev, struct device_attribute *attr, > + char *buffer) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + err = start_on_lid_open_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + return sysfs_emit(buffer, "%u\n", value); > +} > + > +static DEVICE_ATTR_RW(start_on_lid_open); > + > +/* USB Charge (USB ports can charge other devices even when device is powered off) */ > + > +static int usb_charge_acpi_set(struct samsung_galaxybook *galaxybook, const bool value) > +{ > + struct sawb buf = { 0 }; > + > + buf.safn = SAFN; > + buf.sasb = SASB_USB_CHARGE_SET; > + buf.gunm = value ? GUNM_USB_CHARGE_ON : GUNM_USB_CHARGE_OFF; > + > + 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_USB_CHARGE_GET; > + buf.gunm = GUNM_USB_CHARGE_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting usb_charge", &buf); > + if (err) > + return err; > + > + *value = buf.gunm; > + > + return 0; > +} > + > +static ssize_t usb_charge_store(struct device *dev, struct device_attribute *attr, > + const char *buffer, size_t count) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + if (!count) > + return -EINVAL; > + > + err = kstrtobool(buffer, &value); > + if (err) > + return err; > + > + err = usb_charge_acpi_set(galaxybook, value); > + if (err) > + return err; > + > + return count; > +} > + > +static ssize_t usb_charge_show(struct device *dev, struct device_attribute *attr, char *buffer) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + err = usb_charge_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + return sysfs_emit(buffer, "%u\n", value); > +} > + > +static DEVICE_ATTR_RW(usb_charge); > + > +/* Allow recording (allows or blocks access to camera and microphone) */ > + > +static int allow_recording_acpi_set(struct samsung_galaxybook *galaxybook, const bool value) > +{ > + struct sawb buf = { 0 }; > + > + buf.safn = SAFN; > + buf.sasb = SASB_ALLOW_RECORDING; > + buf.gunm = GUNM_SET; > + buf.guds[0] = value ? 1 : 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_ALLOW_RECORDING; > + buf.gunm = GUNM_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting allow_recording", &buf); > + if (err) > + return err; > + > + *value = buf.gunm == 1; > + > + return 0; > +} > + > +static ssize_t allow_recording_store(struct device *dev, struct device_attribute *attr, > + const char *buffer, size_t count) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + if (!count) > + return -EINVAL; > + > + err = kstrtobool(buffer, &value); > + if (err) > + return err; > + > + err = allow_recording_acpi_set(galaxybook, value); > + if (err) > + return err; > + > + return count; > +} > + > +static ssize_t allow_recording_show(struct device *dev, struct device_attribute *attr, char *buffer) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); > + bool value; > + int err; > + > + err = allow_recording_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + return sysfs_emit(buffer, "%u\n", value); > +} > + > +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, > +}; Needs to be NULL-terminated. > + > +static const struct attribute_group galaxybook_attrs_group = { > + .attrs = galaxybook_attrs, > + .is_visible = galaxybook_attr_is_visible, > +}; AFAIK this can safely be added to the drivers .dev_groups. The driver core will ensure that those sysfs attrs are registers after the drivers .probe callback returned successfully. > + > +/* > + * Battery Extension (adds charge_control_end_threshold to the battery device) > + */ > + > +static int charge_control_end_threshold_acpi_set(struct samsung_galaxybook *galaxybook, u8 value) > +{ > + struct sawb buf = { 0 }; > + > + if (value > 100) > + return -EINVAL; > + /* if setting to 100, should be set to 0 (no threshold) */ > + if (value == 100) > + value = 0; Please also handle the case here value is zero. You can for example return -EINVAL in this case. > + > + 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; > + > + 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + 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_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "getting battery charge_control_end_threshold", &buf); > + if (err) > + return err; > + > + *value = 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 samsung_galaxybook *galaxybook = > + container_of(attr, struct samsung_galaxybook, charge_control_end_threshold_attr); > + u8 value; > + int err; > + > + if (!count) > + return -EINVAL; > + > + err = kstrtou8(buffer, 0, &value); > + if (err) > + return err; > + > + err = charge_control_end_threshold_acpi_set(galaxybook, value); > + if (err) > + return err; > + > + return count; > +} > + > +static ssize_t charge_control_end_threshold_show(struct device *dev, struct device_attribute *attr, > + char *buffer) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(attr, struct samsung_galaxybook, charge_control_end_threshold_attr); > + u8 value; > + int err; > + > + err = charge_control_end_threshold_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + return sysfs_emit(buffer, "%d\n", value); > +} > + > +static int galaxybook_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(hook, struct samsung_galaxybook, battery_hook); > + > + return device_create_file(&battery->dev, &galaxybook->charge_control_end_threshold_attr); > +} > + > +static int galaxybook_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(hook, struct samsung_galaxybook, battery_hook); > + > + device_remove_file(&battery->dev, &galaxybook->charge_control_end_threshold_attr); > + return 0; > +} > + > +static int galaxybook_battery_threshold_init(struct samsung_galaxybook *galaxybook) > +{ > + struct acpi_battery_hook *hook; > + struct device_attribute *attr; > + u8 value; > + int err; > + > + err = charge_control_end_threshold_acpi_get(galaxybook, &value); > + if (err) > + return err; > + > + hook = &galaxybook->battery_hook; > + hook->add_battery = galaxybook_battery_add; > + hook->remove_battery = galaxybook_battery_remove; > + hook->name = "Samsung Galaxy Book Battery Extension"; > + > + attr = &galaxybook->charge_control_end_threshold_attr; > + sysfs_attr_init(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; > + > + return devm_battery_hook_register(&galaxybook->platform->dev, &galaxybook->battery_hook); > +} > + > +/* > + * Platform Profile / Performance mode > + */ > + > +static int performance_mode_acpi_set(struct samsung_galaxybook *galaxybook, > + const u8 performance_mode) > +{ > + struct sawb buf = { 0 }; > + > + buf.safn = SAFN; > + buf.sasb = SASB_PERFORMANCE_MODE; > + export_guid(buf.caid, &PERFORMANCE_MODE_GUID); > + buf.fncn = FNCN_PERFORMANCE_MODE; > + buf.subn = SUBN_PERFORMANCE_MODE_SET; > + buf.iob0 = performance_mode; > + > + 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_PERFORMANCE_MODE; > + export_guid(buf.caid, &PERFORMANCE_MODE_GUID); > + buf.fncn = FNCN_PERFORMANCE_MODE; > + buf.subn = SUBN_PERFORMANCE_MODE_GET; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_PERFORMANCE_MODE, &buf, > + SAWB_LEN_PERFORMANCE_MODE, "getting performance_mode", &buf); > + if (err) > + return err; > + > + *performance_mode = buf.iob0; > + > + return 0; > +} > + > +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) { > + if (profile) > + *profile = i; > + return 0; > + } > + } > + > + 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); > + > + return performance_mode_acpi_set(galaxybook, > + galaxybook->profile_performance_modes[profile]); > +} > + > +static int galaxybook_platform_profile_get(struct platform_profile_handler *pprof, > + enum platform_profile_option *profile) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(pprof, struct samsung_galaxybook, profile_handler); > + u8 performance_mode; > + int err; > + > + err = performance_mode_acpi_get(galaxybook, &performance_mode); > + if (err) > + return err; > + > + return get_performance_mode_profile(galaxybook, performance_mode, profile); > +} > + > +static void galaxybook_profile_exit(void *data) > +{ > + struct samsung_galaxybook *galaxybook = data; > + > + platform_profile_remove(&galaxybook->profile_handler); > +} > + > +#define IGNORE_PERFORMANCE_MODE_MAPPING -1 > + > +static int galaxybook_profile_init(struct samsung_galaxybook *galaxybook) > +{ > + 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; > + > + /* fetch supported performance mode values from ACPI method */ > + buf.safn = SAFN; > + buf.sasb = SASB_PERFORMANCE_MODE; > + export_guid(buf.caid, &PERFORMANCE_MODE_GUID); > + buf.fncn = FNCN_PERFORMANCE_MODE; > + buf.subn = SUBN_PERFORMANCE_MODE_LIST; > + > + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_PERFORMANCE_MODE, > + &buf, SAWB_LEN_PERFORMANCE_MODE, > + "get supported performance modes", &buf); > + if (err) > + return err; > + > + /* set up profile_performance_modes with "unknown" as init value */ > + galaxybook->profile_performance_modes = > + kcalloc(PLATFORM_PROFILE_LAST, sizeof(u8), GFP_KERNEL); > + if (!galaxybook->profile_performance_modes) > + return -ENOMEM; > + for (i = 0; i < PLATFORM_PROFILE_LAST; i++) > + galaxybook->profile_performance_modes[i] = PERFORMANCE_MODE_UNKNOWN; > + > + /* > + * Value returned in iob0 will have the number of supported performance modes. > + * The performance mode values will then be given as a list after this (iob1-iobX). > + * 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. > + */ > + 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 > + * (power-profiles-daemon, etc). > + * - performance = "ultra", otherwise "performance" > + * - balanced = "optimized", otherwise "performance" when "ultra" is supported > + * - low-power = "silent", otherwise "quiet" > + * Different models support different modes. Additional supported modes will be > + * mapped to profiles that fall in between these 3. > + */ > + switch (buf.iob_values[i]) { > + > + case PERFORMANCE_MODE_ULTRA: > + /* ultra always maps to performance */ > + mode_profile = PLATFORM_PROFILE_PERFORMANCE; > + break; > + > + case PERFORMANCE_MODE_PERFORMANCE: > + /* if ultra exists, map performance to balanced-performance */ > + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_PERFORMANCE] != > + PERFORMANCE_MODE_UNKNOWN) > + mode_profile = PLATFORM_PROFILE_BALANCED_PERFORMANCE; > + else /* otherwise map it to performance instead */ > + mode_profile = PLATFORM_PROFILE_PERFORMANCE; > + break; > + > + case PERFORMANCE_MODE_SILENT: > + /* silent always maps to low-power */ > + mode_profile = PLATFORM_PROFILE_LOW_POWER; > + break; > + > + case PERFORMANCE_MODE_QUIET: > + /* if silent exists, map quiet to quiet */ > + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_LOW_POWER] != > + PERFORMANCE_MODE_UNKNOWN) > + mode_profile = PLATFORM_PROFILE_QUIET; > + else /* otherwise map it to low-power for better userspace tool support */ > + mode_profile = PLATFORM_PROFILE_LOW_POWER; > + break; > + > + case PERFORMANCE_MODE_OPTIMIZED: > + /* optimized always maps to balanced */ > + mode_profile = PLATFORM_PROFILE_BALANCED; > + break; > + > + case PERFORMANCE_MODE_PERFORMANCE_LEGACY: > + /* map to performance if performance is not already supported */ > + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_PERFORMANCE] == > + PERFORMANCE_MODE_UNKNOWN) > + mode_profile = PLATFORM_PROFILE_PERFORMANCE; > + else /* otherwise, ignore */ > + mode_profile = IGNORE_PERFORMANCE_MODE_MAPPING; > + break; > + > + case PERFORMANCE_MODE_OPTIMIZED_LEGACY: > + /* map to balanced if balanced is not already supported */ > + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_BALANCED] == > + PERFORMANCE_MODE_UNKNOWN) > + mode_profile = PLATFORM_PROFILE_BALANCED; > + else /* otherwise, ignore */ > + mode_profile = IGNORE_PERFORMANCE_MODE_MAPPING; > + break; > + > + default: /* any other value is not supported */ > + mode_profile = IGNORE_PERFORMANCE_MODE_MAPPING; > + break; > + } > + > + /* if current mode value mapped to a supported platform_profile_option, set it up */ > + 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); > + dev_dbg(&galaxybook->platform->dev, > + "will support platform profile %d (performance mode 0x%x)\n", > + mode_profile, buf.iob_values[i]); > + } else { > + dev_dbg(&galaxybook->platform->dev, > + "unmapped performance mode 0x%x will be ignored\n", > + buf.iob_values[i]); > + } > + } > + > + if (mapped_profiles == 0) > + return -ENODEV; > + > + err = platform_profile_register(&galaxybook->profile_handler); Please update the code to conform to the updated platform profile API. > + if (err) > + return err; > + > + /* now check currently set performance mode; if not supported then set default profile */ > + err = performance_mode_acpi_get(galaxybook, ¤t_performance_mode); > + if (err) > + 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) > + 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, galaxybook); > + if (err) > + goto err_remove_exit; > + > + return 0; > + > +err_remove_exit: > + galaxybook_profile_exit(galaxybook); > + return err; The error handling is a bit strange. Please try to set the default platform profile first before registering the platform profile interface, then register the platform profile interface and use devm_add_action_or_reset(). > +} > + > +/* > + * Hotkey work and filters > + */ > + > +static void galaxybook_kbd_backlight_hotkey_work(struct work_struct *work) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(work, struct samsung_galaxybook, kbd_backlight_hotkey_work); > + > + if (galaxybook->kbd_backlight.brightness < galaxybook->kbd_backlight.max_brightness) > + kbd_backlight_acpi_set(galaxybook, galaxybook->kbd_backlight.brightness + 1); > + else > + kbd_backlight_acpi_set(galaxybook, 0); > + > + led_classdev_notify_brightness_hw_changed( > + &galaxybook->kbd_backlight, > + galaxybook->kbd_backlight.brightness); This is the exact reason why i think this should be done in userspace. You can replace this code with a simple input event submission using the KBDILLUM* key codes. This would also allow you to avoid any special locking in this case. This would also allow userspace to configure the step with of the brightness changes. > +} > + > +static void galaxybook_allow_recording_hotkey_work(struct work_struct *work) > +{ > + struct samsung_galaxybook *galaxybook = > + container_of(work, struct samsung_galaxybook, allow_recording_hotkey_work); > + bool value; > + > + allow_recording_acpi_get(galaxybook, &value); > + allow_recording_acpi_set(galaxybook, !value); Please add some locking here, the value of "allow recording" might have been changed by another thread in the mid of those two function calls. > +} > + > +static bool galaxybook_i8042_filter(unsigned char data, unsigned char str, struct serio *port) > +{ > + static bool extended; > + > + if (str & I8042_STR_AUXDATA) > + return false; > + > + if (unlikely(data == 0xe0)) { > + extended = true; > + return true; > + } else if (unlikely(extended)) { > + extended = false; > + switch (data) { > + > + case GB_KEY_KBD_BACKLIGHT_KEYDOWN: > + return true; > + case GB_KEY_KBD_BACKLIGHT_KEYUP: > + if (kbd_backlight) > + schedule_work(&galaxybook_ptr->kbd_backlight_hotkey_work); > + return true; > + > + case GB_KEY_ALLOW_RECORDING_KEYDOWN: > + return true; > + case GB_KEY_ALLOW_RECORDING_KEYUP: > + if (allow_recording) > + schedule_work(&galaxybook_ptr->allow_recording_hotkey_work); > + return true; > + > + default: > + /* > + * Report the previously filtered e0 before continuing > + * with the next non-filtered byte. > + */ > + serio_interrupt(port, 0xe0, 0); > + return false; > + } > + } > + > + return false; > +} > + > +static void galaxybook_i8042_filter_remove(void *data) > +{ > + 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_i8042_filter_install(struct samsung_galaxybook *galaxybook) > +{ > + int err; > + > + /* 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) { > + cancel_work_sync(&galaxybook->kbd_backlight_hotkey_work); > + cancel_work_sync(&galaxybook->allow_recording_hotkey_work); > + return err; > + } No work should be active should i8042_install_filter fail. > + > + /* 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); This will be called automatically be devm_add_action_or_reset(). > + return err; > + } > + > + return 0; > +} > + > +/* > + * Input device (hotkeys and notifications) > + */ > + > +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_input_init(struct samsung_galaxybook *galaxybook) > +{ > + int err; > + > + galaxybook->input = devm_input_allocate_device(&galaxybook->platform->dev); > + if (!galaxybook->input) > + return -ENOMEM; > + > + 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 = sparse_keymap_setup(galaxybook->input, galaxybook_acpi_keymap, NULL); > + if (err) > + return err; > + > + return input_register_device(galaxybook->input); > +} > + > +/* > + * ACPI device setup > + */ > + > +static void galaxybook_acpi_notify(acpi_handle handle, u32 event, void *data) > +{ > + struct samsung_galaxybook *galaxybook = data; > + > + if (event == ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE) { > + if (performance_mode) { > + 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) > +{ > + struct sawb buf = { 0 }; > + int err; > + > + err = galaxybook_enable_acpi_feature(galaxybook, SASB_NOTIFICATIONS); > + if (err) > + return err; > + > + buf.safn = SAFN; > + buf.sasb = SASB_NOTIFICATIONS; > + buf.gunm = GUNM_ACPI_NOTIFY_ENABLE; > + buf.guds[0] = GUDS_ACPI_NOTIFY_ENABLE; > + > + return galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, > + "activate ACPI notifications", &buf); > +} > + > +static void galaxybook_acpi_remove_notify_handler(void *data) > +{ > + struct samsung_galaxybook *galaxybook = data; > + > + acpi_remove_notify_handler(galaxybook->acpi->handle, ACPI_ALL_NOTIFY, > + galaxybook_acpi_notify); > +} > + > +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; -EIO please. > + 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; Same as above. > + 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"); You should initialize the input device before registering the i8042 filter and the ACPI notify handler, or else they might try to submit input events on a allocated but not-yet-registered input device. Also please call galaxybook_enable_acpi_notify() after registering the input device. > + } > + > + 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) See checkpatch for complains about this. Also i would prefer if you drop this macro. > + > +static int galaxybook_probe(struct platform_device *pdev) > +{ > + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev); > + struct samsung_galaxybook *galaxybook; > + int err; > + > + if (!adev) > + return -ENODEV; > + > + dev_dbg(&pdev->dev, "loading driver\n"); > + > + galaxybook = devm_kzalloc(&pdev->dev, sizeof(*galaxybook), GFP_KERNEL); > + if (!galaxybook) > + return -ENOMEM; > + /* set static pointer here so it can be used in i8042 filter */ > + galaxybook_ptr = galaxybook; Please check if this pointer is already set and return -EBUSY in such a case. > + > + galaxybook->platform = pdev; > + galaxybook->acpi = adev; You can also just store pdev->dev and the ACPI handle here. > + > + 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); Missing error handling here. > + > + err = galaxybook_acpi_init(galaxybook); > + if (err) { > + dev_err(&galaxybook->acpi->dev, "failed to initialize the ACPI device\n"); > + return err; > + } > + > + err = galaxybook_enable_acpi_feature(galaxybook, SASB_POWER_MANAGEMENT); > + if (err) { > + 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; > + } > + > + galaxybook_init_feature(performance_mode, galaxybook_profile_init); > + galaxybook_init_feature(battery_threshold, galaxybook_battery_threshold_init); > + > + /* add attrs group here as the is_visible requires above initializations */ > + err = devm_device_add_group(&galaxybook->platform->dev, &galaxybook_attrs_group); > + if (err) > + dev_warn(&galaxybook->platform->dev, "failed to add platform device attributes\n"); When using .dev_groups the driver core will registers those after the probe callback returns, so please use .dev_groups. > + > + 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; > + > + galaxybook_init_feature(i8042_filter, galaxybook_i8042_filter_install); Does installing the i8042 filter involve any IO? If no then please fail probing if an error occurs here. I think with would make sense to do this printing inside the initialization functions. They can also return 0 instead of an error when they determine that the feature is simply not available on a given machine so that all real errors can be treated as such and result in an probe failure. I think the dell-wmi-ddv driver does something similar. > + > + dev_dbg(&galaxybook->platform->dev, "driver successfully loaded\n"); > + > + return 0; > +} > + > +static void galaxybook_remove(struct platform_device *pdev) > +{ > + struct samsung_galaxybook *galaxybook = dev_get_drvdata(&pdev->dev); > + > + if (galaxybook_ptr) > + galaxybook_ptr = NULL; > + > + dev_dbg(&galaxybook->platform->dev, "driver removed\n"); Maybe you should also use devres for this, since this will only get called if the previous probe callback was successful. In general i am very positive about this driver. Nearly all issues are rather small, so i am looking forward to the v4 patch series :). Thanks, Armin Wolf > +} > + > +static struct platform_driver galaxybook_platform_driver = { > + .driver = { > + .name = DRIVER_NAME, > + .acpi_match_table = galaxybook_device_ids, > + }, > + .probe = galaxybook_probe, > + .remove = galaxybook_remove, > +}; > +module_platform_driver(galaxybook_platform_driver); > + > +MODULE_AUTHOR("Joshua Grisham <josh@joshuagrisham.com>"); > +MODULE_DESCRIPTION("Samsung Galaxy Book Extras"); > +MODULE_LICENSE("GPL");
Hi Armin, Thank you for reviewing this new driver. On 17-Dec-24 10:31 PM, Armin Wolf wrote: > Am 16.12.24 um 11:38 schrieb Joshua Grisham: <snip> >> +/* >> + * Hotkey work and filters >> + */ >> + >> +static void galaxybook_kbd_backlight_hotkey_work(struct work_struct *work) >> +{ >> + struct samsung_galaxybook *galaxybook = >> + container_of(work, struct samsung_galaxybook, kbd_backlight_hotkey_work); >> + >> + if (galaxybook->kbd_backlight.brightness < galaxybook->kbd_backlight.max_brightness) >> + kbd_backlight_acpi_set(galaxybook, galaxybook->kbd_backlight.brightness + 1); >> + else >> + kbd_backlight_acpi_set(galaxybook, 0); >> + >> + led_classdev_notify_brightness_hw_changed( >> + &galaxybook->kbd_backlight, >> + galaxybook->kbd_backlight.brightness); > > This is the exact reason why i think this should be done in userspace. You can replace this code > with a simple input event submission using the KBDILLUM* key codes. This would also allow you to > avoid any special locking in this case. > > This would also allow userspace to configure the step with of the brightness changes. As discussed in an earlier thread, there is really no good way to let userspace handle this atm. KEY_KBDILLUMTOGGLE gets mapped to XF86KbdLightOnOff while we really want Cycle, as we have in e.g. XF86MonBrightnessCycle. I just checked gnome-settings-daemon and it does handle XF86KbdLightOnOff but it only toggles on/off. In most laptops the cycle key is handled by the embedded controller and this simply emulates that common setup to match what userspace currently expects. Handling this in userspace would require adding a new KEY_KBDILLUMCYCLE and then adding support for that to xkb and maybe also upower and gnome-settings-daemon and KDE and then wait for that all to land before things start to work. As for your argument about allowing to set the percentage to step, note that this in kernel handling only increaeses the level by 1 on the hotkey press that make sense because typically these kbds backlights only have 2 - 4 levels. So IMHO handling this in the kernel is an acceptable compromise, (yes I do realize that this is a compromise). If some need arises later to do move this to userspace we can always add a module parameter to completely disable the in kernel handling and instead send out a new KEY_KBDILLUMCYCLE key-code when this now module parameter is set. Regards, Hans
Am 18.12.24 um 15:24 schrieb Hans de Goede: > Hi Armin, > > Thank you for reviewing this new driver. > > On 17-Dec-24 10:31 PM, Armin Wolf wrote: >> Am 16.12.24 um 11:38 schrieb Joshua Grisham: > <snip> > >>> +/* >>> + * Hotkey work and filters >>> + */ >>> + >>> +static void galaxybook_kbd_backlight_hotkey_work(struct work_struct *work) >>> +{ >>> + struct samsung_galaxybook *galaxybook = >>> + container_of(work, struct samsung_galaxybook, kbd_backlight_hotkey_work); >>> + >>> + if (galaxybook->kbd_backlight.brightness < galaxybook->kbd_backlight.max_brightness) >>> + kbd_backlight_acpi_set(galaxybook, galaxybook->kbd_backlight.brightness + 1); >>> + else >>> + kbd_backlight_acpi_set(galaxybook, 0); >>> + >>> + led_classdev_notify_brightness_hw_changed( >>> + &galaxybook->kbd_backlight, >>> + galaxybook->kbd_backlight.brightness); >> This is the exact reason why i think this should be done in userspace. You can replace this code >> with a simple input event submission using the KBDILLUM* key codes. This would also allow you to >> avoid any special locking in this case. >> >> This would also allow userspace to configure the step with of the brightness changes. > As discussed in an earlier thread, there is really no good way to let > userspace handle this atm. KEY_KBDILLUMTOGGLE gets mapped to XF86KbdLightOnOff > while we really want Cycle, as we have in e.g. XF86MonBrightnessCycle. > > I just checked gnome-settings-daemon and it does handle XF86KbdLightOnOff > but it only toggles on/off. In most laptops the cycle key is handled by > the embedded controller and this simply emulates that common setup to > match what userspace currently expects. > > Handling this in userspace would require adding a new KEY_KBDILLUMCYCLE > and then adding support for that to xkb and maybe also upower and > gnome-settings-daemon and KDE and then wait for that all to land before > things start to work. > > As for your argument about allowing to set the percentage to step, > note that this in kernel handling only increaeses the level by 1 on > the hotkey press that make sense because typically these kbds backlights > only have 2 - 4 levels. > > So IMHO handling this in the kernel is an acceptable compromise, > (yes I do realize that this is a compromise). > > If some need arises later to do move this to userspace we can always > add a module parameter to completely disable the in kernel handling and > instead send out a new KEY_KBDILLUMCYCLE key-code when this now > module parameter is set. > > Regards, > > Hans I see, in this case we can indeed handle this inside the kernel. However we should really use led_set_brightness_sync() here instead of manually updating the brightness. Also we need a mutex here so that the whole read-modify-write brightness update is atomic. Thanks, Armin Wolf
Thank you both Thomas and Hans for your review and comments! I am working on a v4 of the patch but had a few questions which I wanted to clarify (they can also come after in a v5 etc in case I managed to get this ready to go before anyone has the time to confirm and/or clarify some things!). Den tis 17 dec. 2024 kl 15:23 skrev Hans de Goede <hdegoede@redhat.com>: > > On 16-Dec-24 5:46 PM, Thomas Weißschuh wrote: > >> +Various hardware settings can be controlled by the following sysfs attributes: > >> + > >> +- ``allow_recording`` (allows or blocks usage of built-in camera and microphone) > >> +- ``start_on_lid_open`` (power on automatically when opening the lid) > >> +- ``usb_charge`` (allows USB ports to provide power even when device is off) > > > > Non-standard sysfs attributes should be avoided where possible. > > Userspace will need bespoke code to handle them. > > This looks like it could be handled by the standard firmware_attributes > > interface. > > This would standardize discovery and usage. > > Ack this really feels like firmware-attributes. I would not be surprised > if there are matching BIOS settings and if changing those also changes > the sysfs files and likewise if the sysfs settings persist over reboot. > Yes 2 of these (not this "allow_recording" I think) are available via BIOS and all 3 of them persist over restarts. Just so I am 100% clear what you mean here -- these type of attributes should be created using the utilities available in drivers/platform/x86/firmware_attributes_class.h so that they are created under the path /sys/class/firmware-attributes/*/attributes/*/ ? What exactly should they be named (any preference?) and should I also add some documentation for them in Documentation/ABI/testing/sysfs-class-firmware-attributes ? I am fairly sure I understand the concept and can agree that it kind of makes a lot of sense to be able to standardize the userspace interface, especially for attributes which do the exact same thing across different vendors/devices (unless it just as easily possible to go based on some pattern matching e.g. like is done in udev and upower with "*kbd_backlight*" etc) but as of now it looks like the only examples implemented are for thinklmi, dell-wmi, and hp-bioscfg that I can see so far? Before, I had tried to look through all of the various platform/x86 drivers and harmonize which names I picked for these sysfs attributes (that is how I landed on "usb_charge" and "start_on_lid_open" as I recall correctly) but I am not aware of any existing userspace tools which are looking for anything like these (apart for driver/vendor-specific utilities). Any recommendation from the very wise people here would certainly be appreciated for these :) > > The allow-recording setting toggle is new to me. So this is changeable > at runtime, interesting. > > Joshua above you write this toggle both the microphone mute and > disables the camera? > > It would be good to report the camera state to userspace using > a separate input/evdev device which reports SW_CAMERA_LENS_COVER > as discussed here: > > https://lore.kernel.org/linux-media/CANiDSCtjpPG3XzaEOEeczZWO5gL-V_sj_Fv5=w82D6zKC9hnpw@mail.gmail.com/ > > the plan is to make that the canonical API to reported "muted" > cameras. > > What happens with the camera when recording is disallowed, > dus it drop of the USB bus or does it only produce black > frames ? > > It is a bit unexpected that this one button controls both > microphone and camera mute. But given that unique behavior > I guess that handling this in the kernel is probably best. > > The alsamixer should send some events for the mic mute/unmute > I hope and we can use SW_CAMERA_LENS_COVER to report the camera > state. > Yes this is kind of an interesting one, also... In the user manuals for these Samsung devices, they actually call this feature "Block Recording." See the second link here with the same title: https://www.samsung.com/ca/support/computing/samsung-laptop-disable-the-webcam/ There is this software control in their "Samsung Settings" and/or "Samsung Security" application plus the hotkey, but they both function exactly the same (executing this ACPI method with the right payload). The reason I called it "allow recording" is because I was trying to take a simple approach in the beginning, and let the device value and userspace value have a 1:1 mapping (you send 0x1 if you want the webcam and mic to NOT block recording, i.e. be "allowed" and 0x0 if you them to be blocked). I thought that echo 1 > block_recording to turn OFF "blocking" felt backwards so I just reversed the name instead ;) But in theory it could as easily be called "block_recording" and the kernel driver could handle the flip (0x1 from userspace becomes 0x0 to/from the device and vice-versa). When you press the hotkey in Windows then there is an OSD popup from their own background software, but nothing actually happens to the devices themselves. Even in Linux via this driver or if you just directly execute the ACPI method with the right values in the buffer, what happens is that the image feed from the camera just becomes solid black and the mic input is just completely silent. The USB camera device is not removed or seemingly touched in anyway, and there does not seem to be any kind of sound device event at all from what I can tell (I tried to check using "amixer events" and a few other methods but never saw anything, and the mixer control in alsa is always un-muted like normal when toggling the feature on and off even though it stops the sound from being able to be recorded). It is as this switch name SW_CAMERA_LENS_COVER indicates, almost like a physical (virtual?) cover has been drawn over the camera and the microphone but they are still seen as operational and completely unchanged from a device perspective. What I have started with for now based on this thread, is that as it seems like KE_SW key entries should have a "sw" struct with code and value, that I am assuming I should send .code = SW_CAMERA_LENS_COVER, .value = 0 for "cover is removed/off/not blocking", and .code = SW_CAMERA_LENS_COVER, .value = 1 for "cover is placed/on/blocking", also it looks like I should send this event as part of init for the current state at init time (the input device seems to have a "state" associated to this switch) -- is this correct? And then is there any events or functions that should be called to notify of a mic mute switch (not actually to send the event to toggle mic mute to userspace, but to just report to userspace somehow that the mic has been muted by hardware, similar to this SW_CAMERA_LENS_COVER or led_classdev_notify_brightness_hw_changed() etc ? At the same time I am still not convinced what exactly the name of this attribute should be ("allow_recording" vs "block_recording" vs "camera_mic_privacy" etc ??) Other notifications that I am wondering what the "right" way to handle / using the right interface: - Are there better events to use for these which these devices are reporting for "ACPI_NOTIFY_DEVICE_ON_TABLE" and "ACPI_NOTIFY_DEVICE_OFF_TABLE" , i.e. some kind of standard "switch"-like notification that the motion sensor in the device has detected that it has been placed or lifted from a flat surface? - When the battery charge control end threshold is reached, there is an ACPI notification on this device as well that is the one I have marked "ACPI_NOTIFY_BATTERY_STATE_CHANGED" -- the Samsung background apps pop up a custom OSD that basically says something to the effect that their "Battery saver is protecting the battery by stopping charging" (can't remember the exact verbiage) and they change the battery icon, but without doing anything else in my driver currently the battery still reports state of "charging" even though it just sits constantly at the percentage (and has the charging icon in GNOME etc). I have seen the event come and go occasionally when I did not expect it, but my working theory is that maybe it is if/when the battery starts charging again if it dips too far below the target "end threshold" and then notifies again when the threshold has been reached. Armin also mentioned this before in a different mail; I guess I would hope/expect there is an event or a function I could call to have the state reflected correctly but I would not want that it negatively impacts the normal behavior of charging the battery itself (just that the state/icon would change would be ideal! as it functions perfectly, it is just that the state and icon are not accurate). > <snip> > > Regards, > > Hans > Thanks again! Best regards, Joshua
diff --git a/Documentation/admin-guide/laptops/samsung-galaxybook.rst b/Documentation/admin-guide/laptops/samsung-galaxybook.rst new file mode 100644 index 000000000000..947810c5f998 --- /dev/null +++ b/Documentation/admin-guide/laptops/samsung-galaxybook.rst @@ -0,0 +1,280 @@ +.. SPDX-License-Identifier: GPL-2.0-or-later + +========================== +Samsung Galaxy Book Extras +========================== + +December 15, 2024 + +Joshua Grisham <josh@joshuagrisham.com> + +This is a Linux x86 platform driver for Samsung Galaxy Book series notebook +devices which utilizes Samsung's ``SCAI`` ACPI device in order to control +extra features and receive various notifications. + + +Supported devices +================= + +Any device with one of the supported ACPI device IDs should be supported. This +covers most of the "Samsung Galaxy Book" series notebooks that are currently +available as of this writing, and could include other Samsung notebook devices +as well. + + +Status +====== + +The following features are currently supported: + +- :ref:`Keyboard backlight <keyboard-backlight>` control +- :ref:`Performance mode <performance-mode>` control implemented using the + platform profile interface +- :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:`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 +for a valid response before enabling its support (registering additional devices +or extensions, adding sysfs attributes, etc). Therefore, it can be important to +note that not all features may be supported for your particular device. + +The following features might be possible to implement but will require +additional investigation and are therefore not supported at this time: + +- "Dolby Atmos" mode for the speakers +- "Outdoor Mode" for increasing screen brightness on models with ``SAM0427`` +- "Silent Mode" on models with ``SAM0427`` + + +Parameters +========== + +The driver includes a list of boolean parameters that allow for manually +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) +- ``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 + (default on) + +.. note:: + Even if you explicitly try to enable a feature using its parameter, support + for it will still be evaluated by the driver, and the feature will be + disabled if it does not appear to be supported on your device. + +The availability of various sysfs file-based "settings" attributes +(``usb_charge``, ``start_on_lid_open``, etc) will be determined automatically +and cannot be manually disabled at this time. + + +.. _keyboard-backlight: + +Keyboard backlight +================== + +Controlled by parameter: ``kbd_backlight`` + +A new LED class named ``samsung-galaxybook::kbd_backlight`` is created which +will then expose the device using the standard sysfs-based LED interface at +``/sys/class/leds/samsung-galaxybook::kbd_backlight``. Brightness can be +controlled by writing values 0 to 3 to the ``brightness`` sysfs attribute or +with any other desired userspace utility. + +.. note:: + Most of these devices have an ambient light sensor which also turns + off the keyboard backlight under well-lit conditions. This behavior does not + seem possible to control at this time, but can be good to be aware of. + + +.. _performance-mode: + +Performance mode +================ + +Controlled by parameter: ``performance_mode`` + +This driver implements the +Documentation/userspace-api/sysfs-platform_profile.rst interface for working +with the "performance mode" function of the Samsung ACPI device. + +Mapping of each Samsung "performance mode" to its respective platform profile is +done dynamically based on a list of the supported modes reported by the device +itself. Preference is given to always try and map ``low-power``, ``balanced``, +and ``performance`` profiles, as these seem to be the most common profiles +utilized (and sometimes even required) by various userspace tools. + +The result of the mapping will be printed in the kernel log when the module is +loaded. Supported profiles can also be retrieved from +``/sys/firmware/acpi/platform_profile_choices``, while +``/sys/firmware/acpi/platform_profile`` can be used to read or write the +currently selected profile. + +The ``balanced`` platform profile will be set during module load if no profile +has been previously set. + + +.. _battery-charge-control-end-threshold: + +Battery charge control end threshold +==================================== + +Controlled by parameter: ``battery_threshold`` + +This platform driver will add the ability to set the battery's charge control +end threshold, but does not have the ability to set a start threshold. + +This feature is typically called "Battery Saver" by the various Samsung +applications in Windows, but in Linux we have implemented the standardized +"charge control threshold" sysfs interface on the battery device to allow for +controlling this functionality from the userspace. + +The sysfs attribute +``/sys/class/power_supply/BAT1/charge_control_end_threshold`` can be used to +read or set the desired charge end threshold. + +If you wish to maintain interoperability with Windows, then you should set the +value to 80 to represent "on", or 0 to represent "off", as these are the values +currently recognized by the various Windows-based Samsung applications and +services as "on" or "off". Otherwise, the device will accept any value between 0 +(off) and 99 as the percentage that you wish the battery to stop charging at. + +.. note:: + If you try to set a value of 100, the driver will also accept this input, but + will set the attribute value to 0 (i.e. 100% will "remove" the end threshold). + + +.. _settings-attributes: + +Settings Attributes +=================== + +Various hardware settings can be controlled by the following sysfs attributes: + +- ``allow_recording`` (allows or blocks usage of built-in camera and microphone) +- ``start_on_lid_open`` (power on automatically when opening the lid) +- ``usb_charge`` (allows USB ports to provide power even when device is off) + +These attributes will be available under the path for your supported ACPI Device +ID's platform device (``SAM0428``, ``SAM0429``, etc), and can most reliably +be found by seeing which device has been bound to the ``samsung-galaxybook`` +driver. Here are some examples: :: + + # find which device ID has been bound to the driver + ls /sys/bus/platform/drivers/samsung-galaxybook/ | grep SAM + + # see SAM0429 attributes + ls /sys/bus/platform/drivers/samsung-galaxybook/SAM0429\:00 + + # see attributes no matter the device ID (using wildcard expansion) + ls /sys/bus/platform/drivers/samsung-galaxybook/SAM* + +Most shells should support using wildcard expansion to directly read and write +these attributes using the above pattern. Example: :: + + # read value of start_on_lid_open + cat /sys/bus/platform/drivers/samsung-galaxybook/SAM*/start_on_lid_open + + # turn on start_on_lid_open + echo true | sudo tee /sys/bus/platform/drivers/samsung-galaxybook/SAM*/start_on_lid_open + +It is also possible to use a udev rule to create a fixed-path symlink to your +device under ``/dev`` (e.g. ``/dev/samsung-galaxybook``), no matter the device +ID, to further simplify reading and writing these attributes in the userspace. + +Allow recording (allow_recording) +--------------------------------- + +``/sys/bus/platform/drivers/samsung-galaxybook/SAM*/allow_recording`` + +Controlled by parameter: ``allow_recording`` + +Controls the "Allow recording" setting, which allows or blocks usage of the +built-in camera and microphone (boolean). + +Start on lid open (start_on_lid_open) +------------------------------------- + +``/sys/bus/platform/drivers/samsung-galaxybook/SAM*/start_on_lid_open`` + +Controls the "Start on lid open" setting, which sets the device to power on +automatically when the lid is opened (boolean). + +USB charge (usb_charge) +----------------------- + +``/sys/bus/platform/drivers/samsung-galaxybook/SAM*/usb_charge`` + +Controls the "USB charge" setting, which allows USB ports to provide power even +when the device is turned off (boolean). + +.. note:: + For most devices, this setting seems to only apply to the USB-C ports. + + +.. _keyboard-hotkey-actions: + +Keyboard hotkey actions (i8042 filter) +====================================== + +Controlled by parameter: ``i8042_filter`` + +The i8042 filter will swallow the keyboard events for the Fn+F9 hotkey (Multi- +level keyboard backlight toggle) and Fn+F10 hotkey (Allow/block recording +toggle) and instead execute their actions within the driver itself. + +Fn+F9 will cycle through the brightness levels of the keyboard backlight. A +notification will be sent using ``led_classdev_notify_brightness_hw_changed`` +so that the userspace can be aware of the change. This mimics the behavior of +other existing devices where the brightness level is cycled internally by the +embedded controller and then reported via a notification. + +Fn+F10 will toggle the value of the "Allow recording" setting. + + +ACPI notifications and ACPI hotkey actions +========================================== + +There is a new "Samsung Galaxy Book extra buttons" input device created which +will send input events for the following notifications from the ACPI device: + +- Notification when the battery charge control end threshold has been reached + and the "battery saver" feature has stopped the battery from charging +- Notification when the device has been placed on a table (not available on all + models) +- Notification when the device has been lifted from a table (not available on + all models) + +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 3809931b9240..9e3b45cf799f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20733,6 +20733,14 @@ L: linux-fbdev@vger.kernel.org S: Maintained F: drivers/video/fbdev/s3c-fb.c +SAMSUNG GALAXY BOOK EXTRAS DRIVER +M: Joshua Grisham <josh@joshuagrisham.com> +L: platform-driver-x86@vger.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 M: Sylwester Nawrocki <s.nawrocki@samsung.com> M: Artur Świgoń <a.swigon@samsung.com> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 0258dd879d64..03f4fb0e93e7 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -778,6 +778,24 @@ config BARCO_P50_GPIO To compile this driver as a module, choose M here: the module will be called barco-p50-gpio. +config SAMSUNG_GALAXYBOOK + tristate "Samsung Galaxy Book extras driver" + depends on ACPI + depends on ACPI_BATTERY + depends on INPUT + depends on SERIO_I8042 + select ACPI_PLATFORM_PROFILE + select INPUT_SPARSEKMAP + select NEW_LEDS + select LEDS_CLASS + help + This is a driver for Samsung Galaxy Book series notebooks. It adds + support for the keyboard backlight control, performance mode control, fan + speed reporting, function keys, and various other device controls. + + For more information about this driver, see + <file:Documentation/admin-guide/laptops/samsung-galaxybook.rst>. + config SAMSUNG_LAPTOP tristate "Samsung Laptop driver" depends on RFKILL || RFKILL = n diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index e1b142947067..32ec4cb9d902 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -95,8 +95,9 @@ obj-$(CONFIG_PCENGINES_APU2) += pcengines-apuv2.o obj-$(CONFIG_BARCO_P50_GPIO) += barco-p50-gpio.o # Samsung -obj-$(CONFIG_SAMSUNG_LAPTOP) += samsung-laptop.o -obj-$(CONFIG_SAMSUNG_Q10) += samsung-q10.o +obj-$(CONFIG_SAMSUNG_GALAXYBOOK) += samsung-galaxybook.o +obj-$(CONFIG_SAMSUNG_LAPTOP) += samsung-laptop.o +obj-$(CONFIG_SAMSUNG_Q10) += samsung-q10.o # Toshiba obj-$(CONFIG_TOSHIBA_BT_RFKILL) += toshiba_bluetooth.o diff --git a/drivers/platform/x86/samsung-galaxybook-trace.h b/drivers/platform/x86/samsung-galaxybook-trace.h new file mode 100644 index 000000000000..09ab6dbe6586 --- /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@joshuagrisham.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 new file mode 100644 index 000000000000..7baa3441fbfa --- /dev/null +++ b/drivers/platform/x86/samsung-galaxybook.c @@ -0,0 +1,1380 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Samsung Galaxy Book series extras driver + * + * Copyright (c) 2024 Joshua Grisham <josh@joshuagrisham.com> + * + * With contributions to the SCAI ACPI device interface: + * Copyright (c) 2024 Giulio Girardi <giulio.girardi@protechgroup.it> + * + * Implementation inspired by existing x86 platform drivers. + * Thank you to the authors! + */ + +#include <linux/acpi.h> +#include <linux/err.h> +#include <linux/i8042.h> +#include <linux/init.h> +#include <linux/input.h> +#include <linux/input/sparse-keymap.h> +#include <linux/kernel.h> +#include <linux/leds.h> +#include <linux/module.h> +#include <linux/moduleparam.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 CREATE_TRACE_POINTS +#include "samsung-galaxybook-trace.h" + +#define DRIVER_NAME "samsung-galaxybook" + +/* + * Module parameters + */ + +static bool kbd_backlight = true; +static bool battery_threshold = true; +static bool performance_mode = true; +static bool allow_recording = true; +static bool i8042_filter = true; + +module_param(kbd_backlight, bool, 0); +MODULE_PARM_DESC(kbd_backlight, "Enable Keyboard Backlight control (default on)"); +module_param(battery_threshold, bool, 0); +MODULE_PARM_DESC(battery_threshold, "Enable battery charge threshold control (default on)"); +module_param(performance_mode, bool, 0); +MODULE_PARM_DESC(performance_mode, "Enable Performance Mode control (default on)"); +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(i8042_filter, bool, 0); +MODULE_PARM_DESC(i8042_filter, "Enable capturing keyboard hotkey events (default on)"); + +/* + * Device definitions and matching + */ + +static const struct acpi_device_id galaxybook_device_ids[] = { + { "SAM0427" }, + { "SAM0428" }, + { "SAM0429" }, + { "SAM0430" }, + {}, +}; +MODULE_DEVICE_TABLE(acpi, galaxybook_device_ids); + +struct samsung_galaxybook { + struct platform_device *platform; + struct acpi_device *acpi; + struct mutex acpi_lock; + + struct led_classdev kbd_backlight; + + struct input_dev *input; + struct mutex input_lock; + + void *i8042_filter_ptr; + struct work_struct kbd_backlight_hotkey_work; + struct work_struct allow_recording_hotkey_work; + + struct acpi_battery_hook battery_hook; + struct device_attribute charge_control_end_threshold_attr; + + u8 *profile_performance_modes; + struct platform_profile_handler profile_handler; +}; +static struct samsung_galaxybook *galaxybook_ptr; + +struct sawb { + u16 safn; + u16 sasb; + u8 rflg; + union { + struct { + u8 gunm; + u8 guds[250]; + }; + struct { + u8 caid[16]; + u8 fncn; + u8 subn; + u8 iob0; + u8 iob1; + u8 iob2; + u8 iob3; + u8 iob4; + u8 iob5; + u8 iob6; + u8 iob7; + u8 iob8; + u8 iob9; + }; + struct { + u8 iob_prefix[18]; + u8 iob_values[10]; + }; + }; +}; + +#define SAWB_LEN_SETTINGS 0x15 +#define SAWB_LEN_PERFORMANCE_MODE 0x100 + +#define SAFN 0x5843 + +#define SASB_KBD_BACKLIGHT 0x78 +#define SASB_POWER_MANAGEMENT 0x7a +#define SASB_USB_CHARGE_GET 0x67 +#define SASB_USB_CHARGE_SET 0x68 +#define SASB_NOTIFICATIONS 0x86 +#define SASB_ALLOW_RECORDING 0x8a +#define SASB_PERFORMANCE_MODE 0x91 + +#define SAWB_RFLG_POS 4 +#define SAWB_GUNM_POS 5 + +#define RFLG_SUCCESS 0xaa +#define GUNM_FAIL 0xff + +#define GUNM_FEATURE_ENABLE 0xbb +#define GUNM_FEATURE_ENABLE_SUCCESS 0xdd +#define GUDS_FEATURE_ENABLE 0xaa +#define GUDS_FEATURE_ENABLE_SUCCESS 0xcc + +#define GUNM_GET 0x81 +#define GUNM_SET 0x82 + +#define GUNM_POWER_MANAGEMENT 0x82 + +#define GUNM_USB_CHARGE_GET 0x80 +#define GUNM_USB_CHARGE_ON 0x81 +#define GUNM_USB_CHARGE_OFF 0x80 +#define GUDS_START_ON_LID_OPEN 0xa3 +#define GUDS_START_ON_LID_OPEN_GET 0x81 +#define GUDS_START_ON_LID_OPEN_SET 0x80 +#define GUDS_BATTERY_CHARGE_CONTROL 0xe9 +#define GUDS_BATTERY_CHARGE_CONTROL_GET 0x91 +#define GUDS_BATTERY_CHARGE_CONTROL_SET 0x90 +#define GUNM_ACPI_NOTIFY_ENABLE 0x80 +#define GUDS_ACPI_NOTIFY_ENABLE 0x02 + +#define FNCN_PERFORMANCE_MODE 0x51 +#define SUBN_PERFORMANCE_MODE_LIST 0x01 +#define SUBN_PERFORMANCE_MODE_GET 0x02 +#define SUBN_PERFORMANCE_MODE_SET 0x03 + +/* guid 8246028d-8bca-4a55-ba0f-6f1e6b921b8f */ +static const guid_t performance_mode_guid_value = + GUID_INIT(0x8246028d, 0x8bca, 0x4a55, 0xba, 0x0f, 0x6f, 0x1e, 0x6b, 0x92, 0x1b, 0x8f); +#define PERFORMANCE_MODE_GUID performance_mode_guid_value + +#define PERFORMANCE_MODE_ULTRA 0x16 +#define PERFORMANCE_MODE_PERFORMANCE 0x15 +#define PERFORMANCE_MODE_SILENT 0xb +#define PERFORMANCE_MODE_QUIET 0xa +#define PERFORMANCE_MODE_OPTIMIZED 0x2 +#define PERFORMANCE_MODE_PERFORMANCE_LEGACY 0x1 +#define PERFORMANCE_MODE_OPTIMIZED_LEGACY 0x0 +#define PERFORMANCE_MODE_UNKNOWN 0xff + +#define DEFAULT_PLATFORM_PROFILE PLATFORM_PROFILE_BALANCED + +#define ACPI_METHOD_ENABLE "SDLS" +#define ACPI_METHOD_ENABLE_ON 1 +#define ACPI_METHOD_ENABLE_OFF 0 +#define ACPI_METHOD_SETTINGS "CSFI" +#define ACPI_METHOD_PERFORMANCE_MODE "CSXI" + +#define KBD_BACKLIGHT_MAX_BRIGHTNESS 3 + +#define ACPI_NOTIFY_BATTERY_STATE_CHANGED 0x61 +#define ACPI_NOTIFY_DEVICE_ON_TABLE 0x6c +#define ACPI_NOTIFY_DEVICE_OFF_TABLE 0x6d +#define ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE 0x70 + +#define GB_KEY_KBD_BACKLIGHT_KEYDOWN 0x2c +#define GB_KEY_KBD_BACKLIGHT_KEYUP 0xac +#define GB_KEY_ALLOW_RECORDING_KEYDOWN 0x1f +#define GB_KEY_ALLOW_RECORDING_KEYUP 0x9f + +static const struct key_entry galaxybook_acpi_keymap[] = { + { KE_KEY, ACPI_NOTIFY_BATTERY_STATE_CHANGED, { KEY_BATTERY } }, + { KE_KEY, ACPI_NOTIFY_DEVICE_ON_TABLE, { KEY_F14 } }, + { KE_KEY, ACPI_NOTIFY_DEVICE_OFF_TABLE, { KEY_F15 } }, + { KE_KEY, ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE, { KEY_UNKNOWN } }, + { KE_END, 0 }, +}; + +/* + * ACPI method handling + */ + +static int galaxybook_acpi_method(struct samsung_galaxybook *galaxybook, acpi_string method, + 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; + 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 *)in_buf; + + input.count = 1; + input.pointer = &in_obj; + + trace_samsung_galaxybook_acpi(dev_name(&galaxybook->acpi->dev), method, purpose_str, + in_obj.buffer.pointer, in_obj.buffer.length); + + status = acpi_evaluate_object_typed(galaxybook->acpi->handle, method, &input, &output, + ACPI_TYPE_BUFFER); + + if (ACPI_FAILURE(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; + + 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 || 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); + err = -ETOOSMALL; + goto out_free; + } + if (out_obj->buffer.pointer[SAWB_RFLG_POS] != RFLG_SUCCESS) { + 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); + err = -EIO; + goto out_free; + } + if (out_obj->buffer.pointer[SAWB_GUNM_POS] == GUNM_FAIL) { + dev_err(&galaxybook->acpi->dev, + "failed %s with ACPI method %s; device responded with failure code 0x%x\n", + purpose_str, method, GUNM_FAIL); + err = -EIO; + goto out_free; + } + + memcpy(out_buf, out_obj->buffer.pointer, len); + err = 0; + +out_free: + kfree(out_obj); + mutex_unlock(&galaxybook->acpi_lock); + return err; +} + +static int galaxybook_enable_acpi_feature(struct samsung_galaxybook *galaxybook, const u16 sasb) +{ + struct sawb buf = { 0 }; + int err; + + buf.safn = SAFN; + buf.sasb = sasb; + buf.gunm = GUNM_FEATURE_ENABLE; + buf.guds[0] = GUDS_FEATURE_ENABLE; + + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, + "enabling ACPI feature", &buf); + if (err) + return err; + + if (buf.gunm != GUNM_FEATURE_ENABLE_SUCCESS && buf.guds[0] != GUDS_FEATURE_ENABLE_SUCCESS) + return -ENODEV; + + return 0; +} + +/* + * Keyboard Backlight + */ + +static int kbd_backlight_acpi_set(struct samsung_galaxybook *galaxybook, + const enum led_brightness brightness) +{ + struct sawb buf = { 0 }; + int err; + + buf.safn = SAFN; + buf.sasb = SASB_KBD_BACKLIGHT; + buf.gunm = GUNM_SET; + + buf.guds[0] = brightness; + + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, + "setting kbd_backlight brightness", &buf); + if (err) + return err; + + galaxybook->kbd_backlight.brightness = brightness; + + return 0; +} + +static int kbd_backlight_acpi_get(struct samsung_galaxybook *galaxybook, + enum led_brightness *brightness) +{ + struct sawb buf = { 0 }; + int err; + + buf.safn = SAFN; + buf.sasb = SASB_KBD_BACKLIGHT; + buf.gunm = GUNM_GET; + + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, + "getting kbd_backlight brightness", &buf); + if (err) + return err; + + *brightness = buf.gunm; + galaxybook->kbd_backlight.brightness = buf.gunm; + + return 0; +} + +static int kbd_backlight_store(struct led_classdev *led, + const enum led_brightness brightness) +{ + struct samsung_galaxybook *galaxybook = + container_of(led, struct samsung_galaxybook, kbd_backlight); + + return kbd_backlight_acpi_set(galaxybook, brightness); +} + +static enum led_brightness kbd_backlight_show(struct led_classdev *led) +{ + struct samsung_galaxybook *galaxybook = + container_of(led, struct samsung_galaxybook, kbd_backlight); + enum led_brightness brightness; + int err; + + err = kbd_backlight_acpi_get(galaxybook, &brightness); + if (err) + return err; + + return brightness; +} + +static int galaxybook_kbd_backlight_init(struct samsung_galaxybook *galaxybook) +{ + struct led_init_data init_data = {}; + enum led_brightness brightness; + int err; + + err = galaxybook_enable_acpi_feature(galaxybook, SASB_KBD_BACKLIGHT); + if (err) + return err; + + /* verify we can read the value, otherwise init should stop and fail */ + err = kbd_backlight_acpi_get(galaxybook, &brightness); + if (err) + return err; + + init_data.devicename = DRIVER_NAME; + init_data.default_label = ":" LED_FUNCTION_KBD_BACKLIGHT; + init_data.devname_mandatory = true; + + 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; + + return devm_led_classdev_register_ext(&galaxybook->platform->dev, + &galaxybook->kbd_backlight, &init_data); +} + +/* + * Platform device attributes (configuration properties which can be controlled via userspace) + */ + +/* Start on lid open (device should power on when lid is opened) */ + +static int start_on_lid_open_acpi_set(struct samsung_galaxybook *galaxybook, const bool value) +{ + struct sawb buf = { 0 }; + + 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 ? 1 : 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) +{ + 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_GET; + + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, + "getting start_on_lid_open", &buf); + if (err) + return err; + + *value = buf.guds[1]; + + return 0; +} + +static ssize_t start_on_lid_open_store(struct device *dev, struct device_attribute *attr, + const char *buffer, size_t count) +{ + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); + bool value; + int err; + + 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; + + return count; +} + +static ssize_t start_on_lid_open_show(struct device *dev, struct device_attribute *attr, + char *buffer) +{ + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); + bool value; + int err; + + err = start_on_lid_open_acpi_get(galaxybook, &value); + if (err) + return err; + + return sysfs_emit(buffer, "%u\n", value); +} + +static DEVICE_ATTR_RW(start_on_lid_open); + +/* USB Charge (USB ports can charge other devices even when device is powered off) */ + +static int usb_charge_acpi_set(struct samsung_galaxybook *galaxybook, const bool value) +{ + struct sawb buf = { 0 }; + + buf.safn = SAFN; + buf.sasb = SASB_USB_CHARGE_SET; + buf.gunm = value ? GUNM_USB_CHARGE_ON : GUNM_USB_CHARGE_OFF; + + 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) +{ + struct sawb buf = { 0 }; + int err; + + buf.safn = SAFN; + buf.sasb = SASB_USB_CHARGE_GET; + buf.gunm = GUNM_USB_CHARGE_GET; + + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, + "getting usb_charge", &buf); + if (err) + return err; + + *value = buf.gunm; + + return 0; +} + +static ssize_t usb_charge_store(struct device *dev, struct device_attribute *attr, + const char *buffer, size_t count) +{ + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); + bool value; + int err; + + if (!count) + return -EINVAL; + + err = kstrtobool(buffer, &value); + if (err) + return err; + + err = usb_charge_acpi_set(galaxybook, value); + if (err) + return err; + + return count; +} + +static ssize_t usb_charge_show(struct device *dev, struct device_attribute *attr, char *buffer) +{ + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); + bool value; + int err; + + err = usb_charge_acpi_get(galaxybook, &value); + if (err) + return err; + + return sysfs_emit(buffer, "%u\n", value); +} + +static DEVICE_ATTR_RW(usb_charge); + +/* Allow recording (allows or blocks access to camera and microphone) */ + +static int allow_recording_acpi_set(struct samsung_galaxybook *galaxybook, const bool value) +{ + struct sawb buf = { 0 }; + + buf.safn = SAFN; + buf.sasb = SASB_ALLOW_RECORDING; + buf.gunm = GUNM_SET; + buf.guds[0] = value ? 1 : 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) +{ + struct sawb buf = { 0 }; + int err; + + buf.safn = SAFN; + buf.sasb = SASB_ALLOW_RECORDING; + buf.gunm = GUNM_GET; + + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, + "getting allow_recording", &buf); + if (err) + return err; + + *value = buf.gunm == 1; + + return 0; +} + +static ssize_t allow_recording_store(struct device *dev, struct device_attribute *attr, + const char *buffer, size_t count) +{ + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); + bool value; + int err; + + if (!count) + return -EINVAL; + + err = kstrtobool(buffer, &value); + if (err) + return err; + + err = allow_recording_acpi_set(galaxybook, value); + if (err) + return err; + + return count; +} + +static ssize_t allow_recording_show(struct device *dev, struct device_attribute *attr, char *buffer) +{ + struct samsung_galaxybook *galaxybook = dev_get_drvdata(dev); + bool value; + int err; + + err = allow_recording_acpi_get(galaxybook, &value); + if (err) + return err; + + return sysfs_emit(buffer, "%u\n", value); +} + +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, u8 value) +{ + struct sawb buf = { 0 }; + + 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; + + 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) +{ + struct sawb buf = { 0 }; + int err; + + 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_GET; + + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, + "getting battery charge_control_end_threshold", &buf); + if (err) + return err; + + *value = 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 samsung_galaxybook *galaxybook = + container_of(attr, struct samsung_galaxybook, charge_control_end_threshold_attr); + u8 value; + int err; + + if (!count) + return -EINVAL; + + err = kstrtou8(buffer, 0, &value); + if (err) + return err; + + err = charge_control_end_threshold_acpi_set(galaxybook, value); + if (err) + return err; + + return count; +} + +static ssize_t charge_control_end_threshold_show(struct device *dev, struct device_attribute *attr, + char *buffer) +{ + struct samsung_galaxybook *galaxybook = + container_of(attr, struct samsung_galaxybook, charge_control_end_threshold_attr); + u8 value; + int err; + + err = charge_control_end_threshold_acpi_get(galaxybook, &value); + if (err) + return err; + + return sysfs_emit(buffer, "%d\n", value); +} + +static int galaxybook_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook) +{ + struct samsung_galaxybook *galaxybook = + container_of(hook, struct samsung_galaxybook, battery_hook); + + return device_create_file(&battery->dev, &galaxybook->charge_control_end_threshold_attr); +} + +static int galaxybook_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook) +{ + struct samsung_galaxybook *galaxybook = + container_of(hook, struct samsung_galaxybook, battery_hook); + + device_remove_file(&battery->dev, &galaxybook->charge_control_end_threshold_attr); + return 0; +} + +static int galaxybook_battery_threshold_init(struct samsung_galaxybook *galaxybook) +{ + struct acpi_battery_hook *hook; + struct device_attribute *attr; + u8 value; + int err; + + err = charge_control_end_threshold_acpi_get(galaxybook, &value); + if (err) + return err; + + hook = &galaxybook->battery_hook; + hook->add_battery = galaxybook_battery_add; + hook->remove_battery = galaxybook_battery_remove; + hook->name = "Samsung Galaxy Book Battery Extension"; + + attr = &galaxybook->charge_control_end_threshold_attr; + sysfs_attr_init(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; + + return devm_battery_hook_register(&galaxybook->platform->dev, &galaxybook->battery_hook); +} + +/* + * Platform Profile / Performance mode + */ + +static int performance_mode_acpi_set(struct samsung_galaxybook *galaxybook, + const u8 performance_mode) +{ + struct sawb buf = { 0 }; + + buf.safn = SAFN; + buf.sasb = SASB_PERFORMANCE_MODE; + export_guid(buf.caid, &PERFORMANCE_MODE_GUID); + buf.fncn = FNCN_PERFORMANCE_MODE; + buf.subn = SUBN_PERFORMANCE_MODE_SET; + buf.iob0 = performance_mode; + + 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) +{ + struct sawb buf = { 0 }; + int err; + + buf.safn = SAFN; + buf.sasb = SASB_PERFORMANCE_MODE; + export_guid(buf.caid, &PERFORMANCE_MODE_GUID); + buf.fncn = FNCN_PERFORMANCE_MODE; + buf.subn = SUBN_PERFORMANCE_MODE_GET; + + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_PERFORMANCE_MODE, &buf, + SAWB_LEN_PERFORMANCE_MODE, "getting performance_mode", &buf); + if (err) + return err; + + *performance_mode = buf.iob0; + + return 0; +} + +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) { + if (profile) + *profile = i; + return 0; + } + } + + 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); + + return performance_mode_acpi_set(galaxybook, + galaxybook->profile_performance_modes[profile]); +} + +static int galaxybook_platform_profile_get(struct platform_profile_handler *pprof, + enum platform_profile_option *profile) +{ + struct samsung_galaxybook *galaxybook = + container_of(pprof, struct samsung_galaxybook, profile_handler); + u8 performance_mode; + int err; + + err = performance_mode_acpi_get(galaxybook, &performance_mode); + if (err) + return err; + + return get_performance_mode_profile(galaxybook, performance_mode, profile); +} + +static void galaxybook_profile_exit(void *data) +{ + struct samsung_galaxybook *galaxybook = data; + + platform_profile_remove(&galaxybook->profile_handler); +} + +#define IGNORE_PERFORMANCE_MODE_MAPPING -1 + +static int galaxybook_profile_init(struct samsung_galaxybook *galaxybook) +{ + 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; + + /* fetch supported performance mode values from ACPI method */ + buf.safn = SAFN; + buf.sasb = SASB_PERFORMANCE_MODE; + export_guid(buf.caid, &PERFORMANCE_MODE_GUID); + buf.fncn = FNCN_PERFORMANCE_MODE; + buf.subn = SUBN_PERFORMANCE_MODE_LIST; + + err = galaxybook_acpi_method(galaxybook, ACPI_METHOD_PERFORMANCE_MODE, + &buf, SAWB_LEN_PERFORMANCE_MODE, + "get supported performance modes", &buf); + if (err) + return err; + + /* set up profile_performance_modes with "unknown" as init value */ + galaxybook->profile_performance_modes = + kcalloc(PLATFORM_PROFILE_LAST, sizeof(u8), GFP_KERNEL); + if (!galaxybook->profile_performance_modes) + return -ENOMEM; + for (i = 0; i < PLATFORM_PROFILE_LAST; i++) + galaxybook->profile_performance_modes[i] = PERFORMANCE_MODE_UNKNOWN; + + /* + * Value returned in iob0 will have the number of supported performance modes. + * The performance mode values will then be given as a list after this (iob1-iobX). + * 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. + */ + 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 + * (power-profiles-daemon, etc). + * - performance = "ultra", otherwise "performance" + * - balanced = "optimized", otherwise "performance" when "ultra" is supported + * - low-power = "silent", otherwise "quiet" + * Different models support different modes. Additional supported modes will be + * mapped to profiles that fall in between these 3. + */ + switch (buf.iob_values[i]) { + + case PERFORMANCE_MODE_ULTRA: + /* ultra always maps to performance */ + mode_profile = PLATFORM_PROFILE_PERFORMANCE; + break; + + case PERFORMANCE_MODE_PERFORMANCE: + /* if ultra exists, map performance to balanced-performance */ + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_PERFORMANCE] != + PERFORMANCE_MODE_UNKNOWN) + mode_profile = PLATFORM_PROFILE_BALANCED_PERFORMANCE; + else /* otherwise map it to performance instead */ + mode_profile = PLATFORM_PROFILE_PERFORMANCE; + break; + + case PERFORMANCE_MODE_SILENT: + /* silent always maps to low-power */ + mode_profile = PLATFORM_PROFILE_LOW_POWER; + break; + + case PERFORMANCE_MODE_QUIET: + /* if silent exists, map quiet to quiet */ + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_LOW_POWER] != + PERFORMANCE_MODE_UNKNOWN) + mode_profile = PLATFORM_PROFILE_QUIET; + else /* otherwise map it to low-power for better userspace tool support */ + mode_profile = PLATFORM_PROFILE_LOW_POWER; + break; + + case PERFORMANCE_MODE_OPTIMIZED: + /* optimized always maps to balanced */ + mode_profile = PLATFORM_PROFILE_BALANCED; + break; + + case PERFORMANCE_MODE_PERFORMANCE_LEGACY: + /* map to performance if performance is not already supported */ + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_PERFORMANCE] == + PERFORMANCE_MODE_UNKNOWN) + mode_profile = PLATFORM_PROFILE_PERFORMANCE; + else /* otherwise, ignore */ + mode_profile = IGNORE_PERFORMANCE_MODE_MAPPING; + break; + + case PERFORMANCE_MODE_OPTIMIZED_LEGACY: + /* map to balanced if balanced is not already supported */ + if (galaxybook->profile_performance_modes[PLATFORM_PROFILE_BALANCED] == + PERFORMANCE_MODE_UNKNOWN) + mode_profile = PLATFORM_PROFILE_BALANCED; + else /* otherwise, ignore */ + mode_profile = IGNORE_PERFORMANCE_MODE_MAPPING; + break; + + default: /* any other value is not supported */ + mode_profile = IGNORE_PERFORMANCE_MODE_MAPPING; + break; + } + + /* if current mode value mapped to a supported platform_profile_option, set it up */ + 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); + dev_dbg(&galaxybook->platform->dev, + "will support platform profile %d (performance mode 0x%x)\n", + mode_profile, buf.iob_values[i]); + } else { + dev_dbg(&galaxybook->platform->dev, + "unmapped performance mode 0x%x will be ignored\n", + buf.iob_values[i]); + } + } + + if (mapped_profiles == 0) + return -ENODEV; + + err = platform_profile_register(&galaxybook->profile_handler); + if (err) + return err; + + /* now check currently set performance mode; if not supported then set default profile */ + err = performance_mode_acpi_get(galaxybook, ¤t_performance_mode); + if (err) + 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) + 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, galaxybook); + if (err) + goto err_remove_exit; + + return 0; + +err_remove_exit: + galaxybook_profile_exit(galaxybook); + return err; +} + +/* + * Hotkey work and filters + */ + +static void galaxybook_kbd_backlight_hotkey_work(struct work_struct *work) +{ + struct samsung_galaxybook *galaxybook = + container_of(work, struct samsung_galaxybook, kbd_backlight_hotkey_work); + + if (galaxybook->kbd_backlight.brightness < galaxybook->kbd_backlight.max_brightness) + kbd_backlight_acpi_set(galaxybook, galaxybook->kbd_backlight.brightness + 1); + else + kbd_backlight_acpi_set(galaxybook, 0); + + led_classdev_notify_brightness_hw_changed( + &galaxybook->kbd_backlight, + galaxybook->kbd_backlight.brightness); +} + +static void galaxybook_allow_recording_hotkey_work(struct work_struct *work) +{ + struct samsung_galaxybook *galaxybook = + container_of(work, struct samsung_galaxybook, allow_recording_hotkey_work); + bool value; + + allow_recording_acpi_get(galaxybook, &value); + allow_recording_acpi_set(galaxybook, !value); +} + +static bool galaxybook_i8042_filter(unsigned char data, unsigned char str, struct serio *port) +{ + static bool extended; + + if (str & I8042_STR_AUXDATA) + return false; + + if (unlikely(data == 0xe0)) { + extended = true; + return true; + } else if (unlikely(extended)) { + extended = false; + switch (data) { + + case GB_KEY_KBD_BACKLIGHT_KEYDOWN: + return true; + case GB_KEY_KBD_BACKLIGHT_KEYUP: + if (kbd_backlight) + schedule_work(&galaxybook_ptr->kbd_backlight_hotkey_work); + return true; + + case GB_KEY_ALLOW_RECORDING_KEYDOWN: + return true; + case GB_KEY_ALLOW_RECORDING_KEYUP: + if (allow_recording) + schedule_work(&galaxybook_ptr->allow_recording_hotkey_work); + return true; + + default: + /* + * Report the previously filtered e0 before continuing + * with the next non-filtered byte. + */ + serio_interrupt(port, 0xe0, 0); + return false; + } + } + + return false; +} + +static void galaxybook_i8042_filter_remove(void *data) +{ + 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_i8042_filter_install(struct samsung_galaxybook *galaxybook) +{ + int err; + + /* 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) { + cancel_work_sync(&galaxybook->kbd_backlight_hotkey_work); + cancel_work_sync(&galaxybook->allow_recording_hotkey_work); + return err; + } + + /* 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; + } + + return 0; +} + +/* + * Input device (hotkeys and notifications) + */ + +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_input_init(struct samsung_galaxybook *galaxybook) +{ + int err; + + galaxybook->input = devm_input_allocate_device(&galaxybook->platform->dev); + if (!galaxybook->input) + return -ENOMEM; + + 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 = sparse_keymap_setup(galaxybook->input, galaxybook_acpi_keymap, NULL); + if (err) + return err; + + return input_register_device(galaxybook->input); +} + +/* + * ACPI device setup + */ + +static void galaxybook_acpi_notify(acpi_handle handle, u32 event, void *data) +{ + struct samsung_galaxybook *galaxybook = data; + + if (event == ACPI_NOTIFY_HOTKEY_PERFORMANCE_MODE) { + if (performance_mode) { + 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) +{ + struct sawb buf = { 0 }; + int err; + + err = galaxybook_enable_acpi_feature(galaxybook, SASB_NOTIFICATIONS); + if (err) + return err; + + buf.safn = SAFN; + buf.sasb = SASB_NOTIFICATIONS; + buf.gunm = GUNM_ACPI_NOTIFY_ENABLE; + buf.guds[0] = GUDS_ACPI_NOTIFY_ENABLE; + + return galaxybook_acpi_method(galaxybook, ACPI_METHOD_SETTINGS, &buf, SAWB_LEN_SETTINGS, + "activate ACPI notifications", &buf); +} + +static void galaxybook_acpi_remove_notify_handler(void *data) +{ + struct samsung_galaxybook *galaxybook = data; + + acpi_remove_notify_handler(galaxybook->acpi->handle, ACPI_ALL_NOTIFY, + galaxybook_acpi_notify); +} + +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; + int err; + + if (!adev) + return -ENODEV; + + dev_dbg(&pdev->dev, "loading driver\n"); + + galaxybook = devm_kzalloc(&pdev->dev, sizeof(*galaxybook), GFP_KERNEL); + if (!galaxybook) + return -ENOMEM; + /* 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); + + err = galaxybook_acpi_init(galaxybook); + if (err) { + dev_err(&galaxybook->acpi->dev, "failed to initialize the ACPI device\n"); + return err; + } + + err = galaxybook_enable_acpi_feature(galaxybook, SASB_POWER_MANAGEMENT); + if (err) { + 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; + } + + galaxybook_init_feature(performance_mode, galaxybook_profile_init); + galaxybook_init_feature(battery_threshold, galaxybook_battery_threshold_init); + + /* add attrs group here as the is_visible requires above initializations */ + err = devm_device_add_group(&galaxybook->platform->dev, &galaxybook_attrs_group); + if (err) + dev_warn(&galaxybook->platform->dev, "failed to add platform device attributes\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; + + galaxybook_init_feature(i8042_filter, galaxybook_i8042_filter_install); + + dev_dbg(&galaxybook->platform->dev, "driver successfully loaded\n"); + + return 0; +} + +static void galaxybook_remove(struct platform_device *pdev) +{ + struct samsung_galaxybook *galaxybook = dev_get_drvdata(&pdev->dev); + + if (galaxybook_ptr) + galaxybook_ptr = NULL; + + dev_dbg(&galaxybook->platform->dev, "driver removed\n"); +} + +static struct platform_driver galaxybook_platform_driver = { + .driver = { + .name = DRIVER_NAME, + .acpi_match_table = galaxybook_device_ids, + }, + .probe = galaxybook_probe, + .remove = galaxybook_remove, +}; +module_platform_driver(galaxybook_platform_driver); + +MODULE_AUTHOR("Joshua Grisham <josh@joshuagrisham.com>"); +MODULE_DESCRIPTION("Samsung Galaxy Book Extras"); +MODULE_LICENSE("GPL");
Adds a new driver for Samsung Galaxy Book series notebook devices. This includes the driver itself, a tracepoint header file, a new page in the documentation, and relevant updates to Kconfig, Makefile, and MAINTAINERS files related to this new driver. Signed-off-by: Joshua Grisham <josh@joshuagrisham.com> --- v1->v2: - Attempt to resolve all review comments from v1 as written here: https://lore.kernel.org/platform-driver-x86/53c5075b-1967-45d0-937f-463912dd966d@gmx.de/T/#mbcbd8d5d9bc4496bac5486636c7d3b32bc3e5cd0 v2->v3: - Tweak to battery attribute to closer match pattern in dell-wmi-ddv - implement platform_profile_remove() change from 9b3bb37b44a317626464e79da8b39989b421963f - Small tweak to Documentation page --- .../laptops/samsung-galaxybook.rst | 280 ++++ MAINTAINERS | 8 + drivers/platform/x86/Kconfig | 18 + drivers/platform/x86/Makefile | 5 +- .../platform/x86/samsung-galaxybook-trace.h | 51 + drivers/platform/x86/samsung-galaxybook.c | 1380 +++++++++++++++++ 6 files changed, 1740 insertions(+), 2 deletions(-) create mode 100644 Documentation/admin-guide/laptops/samsung-galaxybook.rst create mode 100644 drivers/platform/x86/samsung-galaxybook-trace.h create mode 100644 drivers/platform/x86/samsung-galaxybook.c