| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 27 May 2024 12:39:51 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Lyndon Sanche <lsanche@...deno.ca>
cc: mario.limonciello@....com, pali@...nel.org, W_Armin@....de,
srinivas.pandruvada@...ux.intel.com, lkp@...el.com,
Hans de Goede <hdegoede@...hat.com>, Yijun.Shen@...l.com,
Matthew Garrett <mjg59@...f.ucam.org>,
Vegard Nossum <vegard.nossum@...cle.com>, Jonathan Corbet <corbet@....net>,
Heiner Kallweit <hkallweit1@...il.com>,
LKML <linux-kernel@...r.kernel.org>, platform-driver-x86@...r.kernel.org,
Dell.Client.Kernel@...l.com
Subject: Re: [PATCH v7 3/3] platform/x86: dell-pc: Implement
platform_profile
On Fri, 17 May 2024, Lyndon Sanche wrote:
> Some Dell laptops support configuration of preset fan modes through
> smbios tables.
>
> If the platform supports these fan modes, set up platform_profile to
> change these modes. If not supported, skip enabling platform_profile.
>
> Signed-off-by: Lyndon Sanche <lsanche@...deno.ca>
> ---
> MAINTAINERS | 6 +
> drivers/platform/x86/dell/Kconfig | 13 +
> drivers/platform/x86/dell/Makefile | 1 +
> drivers/platform/x86/dell/dell-pc.c | 310 +++++++++++++++++++
> drivers/platform/x86/dell/dell-smbios-base.c | 1 +
> drivers/platform/x86/dell/dell-smbios.h | 1 +
> 6 files changed, 332 insertions(+)
> create mode 100644 drivers/platform/x86/dell/dell-pc.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ebf03f5f0619..69c582b72a08 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5961,6 +5961,12 @@ F: Documentation/ABI/obsolete/procfs-i8k
> F: drivers/hwmon/dell-smm-hwmon.c
> F: include/uapi/linux/i8k.h
>
> +DELL PC DRIVER
> +M: Lyndon Sanche <lsanche@...deno.ca>
> +L: platform-driver-x86@...r.kernel.org
> +S: Maintained
> +F: drivers/platform/x86/dell/dell-pc.c
> +
> DELL REMOTE BIOS UPDATE DRIVER
> M: Stuart Hayes <stuart.w.hayes@...il.com>
> L: platform-driver-x86@...r.kernel.org
> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> index bd9f445974cc..0732850a3dd6 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -91,6 +91,19 @@ config DELL_RBTN
> To compile this driver as a module, choose M here: the module will
> be called dell-rbtn.
>
> +config DELL_PC
> + tristate "Dell PC Extras"
> + default m
> + depends on ACPI
> + depends on DMI
> + depends on DELL_SMBIOS
> + select ACPI_PLATFORM_PROFILE
> + help
> + This driver adds support for controlling the fan modes via platform_profile
> + on supported Dell systems regardless of formfactor.
> + Module will simply do nothing if thermal management commands are not
> + supported.
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/dmi.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/platform_profile.h>
> +#include <linux/slab.h>
> +#include "dell-smbios.h"
Add empty line between <> and "" includes.
> +static const struct dmi_system_id dell_device_table[] __initconst = {
> + {
> + .ident = "Dell Inc.",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + },
> + },
> + {
> + .ident = "Dell Computer Corporation",
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
> + },
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(dmi, dell_device_table);
> +enum thermal_mode_bits {
> + DELL_BALANCED = BIT(0),
> + DELL_COOL_BOTTOM = BIT(1),
> + DELL_QUIET = BIT(2),
> + DELL_PERFORMANCE = BIT(3),
A few nits still to address.
Can you please align these so that the values align (IIRC, I asked this
earlier but perhaps my request was too unclear):
DELL_XX = BIT(X),
DELL_YYYYYYYYY = BIT(Y),
> +static int thermal_get_supported_modes(int *supported_bits)
> +{
> + struct calling_interface_buffer buffer;
> + int ret;
> +
> + dell_fill_request(&buffer, 0x0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT);
> + if (ret) {
> + /* Thermal function not supported */
> + if (ret == -ENXIO) {
> + *supported_bits = 0;
> + return 0;
> + } else {
Drop else because the previous block ends into return.
> + return ret;
> + }
> + }
Remove the outer if (ret) block and put the inner ones directly on the
main level as two if () conditions.
> + *supported_bits = FIELD_GET(DELL_THERMAL_SUPPORTED, buffer.output[1]);
> + return 0;
> +}
> +
> +static int thermal_get_acc_mode(int *acc_mode)
> +{
> + struct calling_interface_buffer buffer;
> + int ret;
> +
> + dell_fill_request(&buffer, 0x0, 0, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT);
> + if (ret)
> + return ret;
> + *acc_mode = FIELD_GET(DELL_ACC_GET_FIELD, buffer.output[3]);
> + return 0;
> +}
> +
> +static int thermal_set_mode(enum thermal_mode_bits state)
> +{
> + struct calling_interface_buffer buffer;
> + int ret;
> + int acc_mode;
> +
> + ret = thermal_get_acc_mode(&acc_mode);
> + if (ret)
> + return ret;
> +
> + dell_fill_request(&buffer, 0x1, FIELD_PREP(DELL_ACC_SET_FIELD, acc_mode) | state, 0, 0);
> + ret = dell_send_request(&buffer, CLASS_INFO, SELECT_THERMAL_MANAGEMENT);
> + return ret;
Return directly on the previous line.
> +static int thermal_platform_profile_get(struct platform_profile_handler *pprof,
> + enum platform_profile_option *profile)
> +{
> + int ret;
> +
> + ret = thermal_get_mode();
> + if (ret < 0)
> + return ret;
> +
> + switch (ret) {
> + case DELL_BALANCED:
> + *profile = PLATFORM_PROFILE_BALANCED;
> + break;
> + case DELL_PERFORMANCE:
> + *profile = PLATFORM_PROFILE_PERFORMANCE;
> + break;
> + case DELL_COOL_BOTTOM:
> + *profile = PLATFORM_PROFILE_COOL;
> + break;
> + case DELL_QUIET:
> + *profile = PLATFORM_PROFILE_QUIET;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int thermal_init(void)
> +{
> + int ret;
> + int supported_modes;
> +
> + /* If thermal commands not supported, exit without error */
Fix grammar, you're perhaps missing "are".
> + if (!dell_smbios_class_is_supported(CLASS_INFO))
> + return 0;
> +
> + /* If thermal modes not supported, exit without error */
Ditto.
--
i.
Powered by blists - more mailing lists