* [PATCH v6 06/15] platform/x86/amd/pmf: Add support to get inputs from other subsystems
From: Shyam Sundar S K @ 2023-12-04 10:15 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
benjamin.tissoires
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
Shyam Sundar S K
In-Reply-To: <20231204101548.1458499-1-Shyam-sundar.S-k@amd.com>
PMF driver sends changing inputs from each subystem to TA for evaluating
the conditions in the policy binary.
Add initial support of plumbing in the PMF driver for Smart PC to get
information from other subsystems in the kernel.
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/Makefile | 2 +-
drivers/platform/x86/amd/pmf/pmf.h | 18 ++++
drivers/platform/x86/amd/pmf/spc.c | 122 ++++++++++++++++++++++++++
drivers/platform/x86/amd/pmf/tee-if.c | 3 +
4 files changed, 144 insertions(+), 1 deletion(-)
create mode 100644 drivers/platform/x86/amd/pmf/spc.c
diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
index d2746ee7369f..6b26e48ce8ad 100644
--- a/drivers/platform/x86/amd/pmf/Makefile
+++ b/drivers/platform/x86/amd/pmf/Makefile
@@ -7,4 +7,4 @@
obj-$(CONFIG_AMD_PMF) += amd-pmf.o
amd-pmf-objs := core.o acpi.o sps.o \
auto-mode.o cnqf.o \
- tee-if.o
+ tee-if.o spc.o
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 092be501d4d3..a4a73b845c09 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -150,6 +150,21 @@ struct smu_pmf_metrics {
u16 infra_gfx_maxfreq; /* in MHz */
u16 skin_temp; /* in centi-Celsius */
u16 device_state;
+ u16 curtemp; /* in centi-Celsius */
+ u16 filter_alpha_value;
+ u16 avg_gfx_clkfrequency;
+ u16 avg_fclk_frequency;
+ u16 avg_gfx_activity;
+ u16 avg_socclk_frequency;
+ u16 avg_vclk_frequency;
+ u16 avg_vcn_activity;
+ u16 avg_dram_reads;
+ u16 avg_dram_writes;
+ u16 avg_socket_power;
+ u16 avg_core_power[2];
+ u16 avg_core_c0residency[16];
+ u16 spare1;
+ u32 metrics_counter;
} __packed;
enum amd_stt_skin_temp {
@@ -601,4 +616,7 @@ extern const struct attribute_group cnqf_feature_attribute_group;
int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
+
+/* Smart PC - TA interfaces */
+void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in);
#endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
new file mode 100644
index 000000000000..351efcbe83c4
--- /dev/null
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Platform Management Framework Driver - Smart PC Capabilities
+ *
+ * Copyright (c) 2023, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ * Patil Rajesh Reddy <Patil.Reddy@amd.com>
+ */
+
+#include <acpi/button.h>
+#include <linux/power_supply.h>
+#include <linux/units.h>
+#include "pmf.h"
+
+static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ u16 max, avg = 0;
+ int i;
+
+ memset(dev->buf, 0, sizeof(dev->m_table));
+ amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
+ memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
+
+ in->ev_info.socket_power = dev->m_table.apu_power + dev->m_table.dgpu_power;
+ in->ev_info.skin_temperature = dev->m_table.skin_temp;
+
+ /* Get the avg and max C0 residency of all the cores */
+ max = dev->m_table.avg_core_c0residency[0];
+ for (i = 0; i < ARRAY_SIZE(dev->m_table.avg_core_c0residency); i++) {
+ avg += dev->m_table.avg_core_c0residency[i];
+ if (dev->m_table.avg_core_c0residency[i] > max)
+ max = dev->m_table.avg_core_c0residency[i];
+ }
+
+ avg = DIV_ROUND_CLOSEST(avg, ARRAY_SIZE(dev->m_table.avg_core_c0residency));
+ in->ev_info.avg_c0residency = avg;
+ in->ev_info.max_c0residency = max;
+ in->ev_info.gfx_busy = dev->m_table.avg_gfx_activity;
+}
+
+static const char * const pmf_battery_supply_name[] = {
+ "BATT",
+ "BAT0",
+};
+
+static int amd_pmf_get_battery_prop(enum power_supply_property prop)
+{
+ union power_supply_propval value;
+ struct power_supply *psy;
+ int i, ret;
+
+ for (i = 0; i < ARRAY_SIZE(pmf_battery_supply_name); i++) {
+ psy = power_supply_get_by_name(pmf_battery_supply_name[i]);
+ if (!psy)
+ continue;
+
+ ret = power_supply_get_property(psy, prop, &value);
+ if (ret) {
+ power_supply_put(psy);
+ return ret;
+ }
+ }
+
+ return value.intval;
+}
+
+static int amd_pmf_get_battery_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ int val;
+
+ val = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_PRESENT);
+ if (val < 0)
+ return val;
+ if (val != 1)
+ return -ENODEV;
+
+ in->ev_info.bat_percentage = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_CAPACITY);
+ /* all values in mWh metrics */
+ in->ev_info.bat_design = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN) /
+ MILLIWATT_PER_WATT;
+ in->ev_info.full_charge_capacity = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_ENERGY_FULL) /
+ MILLIWATT_PER_WATT;
+ in->ev_info.drain_rate = amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_POWER_NOW) /
+ MILLIWATT_PER_WATT;
+
+ return 0;
+}
+
+static int amd_pmf_get_slider_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ int val;
+
+ switch (dev->current_profile) {
+ case PLATFORM_PROFILE_PERFORMANCE:
+ val = TA_BEST_PERFORMANCE;
+ break;
+ case PLATFORM_PROFILE_BALANCED:
+ val = TA_BETTER_PERFORMANCE;
+ break;
+ case PLATFORM_PROFILE_LOW_POWER:
+ val = TA_BEST_BATTERY;
+ break;
+ default:
+ dev_err(dev->dev, "Unknown Platform Profile.\n");
+ return -EOPNOTSUPP;
+ }
+ in->ev_info.power_slider = val;
+
+ return 0;
+}
+
+void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ /* TA side lid open is 1 and close is 0, hence the ! here */
+ in->ev_info.lid_state = !acpi_lid_open();
+ in->ev_info.power_source = amd_pmf_get_power_source();
+ amd_pmf_get_smu_info(dev, in);
+ amd_pmf_get_battery_info(dev, in);
+ amd_pmf_get_slider_info(dev, in);
+}
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 468f3797a848..e3f17852d532 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -113,6 +113,7 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
{
struct ta_pmf_shared_memory *ta_sm = NULL;
struct ta_pmf_enact_result *out = NULL;
+ struct ta_pmf_enact_table *in = NULL;
struct tee_param param[MAX_TEE_PARAM];
struct tee_ioctl_invoke_arg arg;
int ret = 0;
@@ -123,11 +124,13 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
memset(dev->shbuf, 0, dev->policy_sz);
ta_sm = dev->shbuf;
out = &ta_sm->pmf_output.policy_apply_table;
+ in = &ta_sm->pmf_input.enact_table;
memset(ta_sm, 0, sizeof(*ta_sm));
ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES;
ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
+ amd_pmf_populate_ta_inputs(dev, in);
amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES, &arg, param);
ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
--
2.25.1
^ permalink raw reply related
* [PATCH v6 05/15] platform/x86/amd/pmf: change amd_pmf_init_features() call sequence
From: Shyam Sundar S K @ 2023-12-04 10:15 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
benjamin.tissoires
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
Shyam Sundar S K
In-Reply-To: <20231204101548.1458499-1-Shyam-sundar.S-k@amd.com>
To sideload pmf policy binaries, the Smart PC Solution Builder provides a
debugfs file called "update_policy"; that gets created under a new debugfs
directory called "pb" and this new directory has to be associated with
existing parent directory for PMF driver called "amd_pmf".
In the current code structure, amd_pmf_dbgfs_register() is called after
amd_pmf_init_features(). This will not help when the Smart PC builder
feature has to be assoicated to the parent directory.
Hence change the order of amd_pmf_dbgfs_register() and call it before
amd_pmf_init_features() so that when the Smart PC init happens, it has the
parent debugfs directory to get itself hooked.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index c8f6ec4c2e2c..4b8156033fa6 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -442,9 +442,9 @@ static int amd_pmf_probe(struct platform_device *pdev)
apmf_acpi_init(dev);
platform_set_drvdata(pdev, dev);
+ amd_pmf_dbgfs_register(dev);
amd_pmf_init_features(dev);
apmf_install_handler(dev);
- amd_pmf_dbgfs_register(dev);
dev_info(dev->dev, "registered PMF device successfully\n");
--
2.25.1
^ permalink raw reply related
* [PATCH v6 04/15] platform/x86/amd/pmf: Add support for PMF Policy Binary
From: Shyam Sundar S K @ 2023-12-04 10:15 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
benjamin.tissoires
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
Shyam Sundar S K
In-Reply-To: <20231204101548.1458499-1-Shyam-sundar.S-k@amd.com>
PMF Policy binary is a encrypted and signed binary that will be part
of the BIOS. PMF driver via the ACPI interface checks the existence
of Smart PC bit. If the advertised bit is found, PMF driver walks
the acpi namespace to find out the policy binary size and the address
which has to be passed to the TA during the TA init sequence.
The policy binary is comprised of inputs (or the events) and outputs
(or the actions). With the PMF ecosystem, OEMs generate the policy
binary (or could be multiple binaries) that contains a supported set
of inputs and outputs which could be specifically carved out for each
usage segment (or for each user also) that could influence the system
behavior either by enriching the user experience or/and boost/throttle
power limits.
Once the TA init command succeeds, the PMF driver sends the changing
events in the current environment to the TA for a constant sampling
frequency time (the event here could be a lid close or open) and
if the policy binary has corresponding action built within it, the
TA sends the action for it in the subsequent enact command.
If the inputs sent to the TA has no output defined in the policy
binary generated by OEMs, there will be no action to be performed
by the PMF driver.
Example policies:
1) if slider is performance ; set the SPL to 40W
Here PMF driver registers with the platform profile interface and
when the slider position is changed, PMF driver lets the TA know
about this. TA sends back an action to update the Sustained
Power Limit (SPL). PMF driver updates this limit via the PMFW mailbox.
2) if user_away ; then lock the system
Here PMF driver hooks to the AMD SFH driver to know the user presence
and send the inputs to TA and if the condition is met, the TA sends
the action of locking the system. PMF driver generates a uevent and
based on the udev rule in the userland the system gets locked with
systemctl.
The intent here is to provide the OEM's to make a policy to lock the
system when the user is away ; but the userland can make a choice to
ignore it.
and so on.
The OEMs will have an utility to create numerous such policies and
the policies shall be reviewed by AMD before signing and encrypting
them. Policies are shared between operating systems to have seemless user
experience.
Since all this action has to happen via the "amdtee" driver, currently
there is no caller for it in the kernel which can load the amdtee driver.
Without amdtee driver loading onto the system the "tee" calls shall fail
from the PMF driver. Hence an explicit MODULE_SOFTDEP has been added
to address this.
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/Kconfig | 2 +-
drivers/platform/x86/amd/pmf/acpi.c | 37 +++++++
drivers/platform/x86/amd/pmf/core.c | 9 ++
drivers/platform/x86/amd/pmf/pmf.h | 141 ++++++++++++++++++++++++
drivers/platform/x86/amd/pmf/tee-if.c | 147 +++++++++++++++++++++++++-
5 files changed, 333 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index 32a029e8db80..f246252bddd8 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -9,7 +9,7 @@ config AMD_PMF
depends on POWER_SUPPLY
depends on AMD_NB
select ACPI_PLATFORM_PROFILE
- depends on TEE
+ depends on TEE && AMDTEE
help
This driver provides support for the AMD Platform Management Framework.
The goal is to enhance end user experience by making AMD PCs smarter,
diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
index 3fc5e4547d9f..4ec7957eb707 100644
--- a/drivers/platform/x86/amd/pmf/acpi.c
+++ b/drivers/platform/x86/amd/pmf/acpi.c
@@ -286,6 +286,43 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev)
return 0;
}
+static acpi_status apmf_walk_resources(struct acpi_resource *res, void *data)
+{
+ struct amd_pmf_dev *dev = data;
+
+ switch (res->type) {
+ case ACPI_RESOURCE_TYPE_ADDRESS64:
+ dev->policy_addr = res->data.address64.address.minimum;
+ dev->policy_sz = res->data.address64.address.address_length;
+ break;
+ case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
+ dev->policy_addr = res->data.fixed_memory32.address;
+ dev->policy_sz = res->data.fixed_memory32.address_length;
+ break;
+ }
+
+ if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || dev->policy_sz == 0) {
+ pr_err("Incorrect Policy params, possibly a SBIOS bug\n");
+ return AE_ERROR;
+ }
+
+ return AE_OK;
+}
+
+int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev)
+{
+ acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
+ acpi_status status;
+
+ status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, apmf_walk_resources, pmf_dev);
+ if (ACPI_FAILURE(status)) {
+ dev_err(pmf_dev->dev, "acpi_walk_resources failed :%d\n", status);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev)
{
acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index f50b7ec9a625..c8f6ec4c2e2c 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -395,6 +395,14 @@ static int amd_pmf_probe(struct platform_device *pdev)
return -ENOMEM;
dev->dev = &pdev->dev;
+ err = apmf_check_smart_pc(dev);
+ if (err)
+ /*
+ * Lets not return from here if Smart PC bit is not advertised in
+ * the BIOS. This way, there will be some amount of power savings
+ * to the user with static slider (if enabled).
+ */
+ pr_err("PMF Smart PC not advertised in BIOS!:%d\n", err);
rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
@@ -474,3 +482,4 @@ module_platform_driver(amd_pmf_driver);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("AMD Platform Management Framework Driver");
+MODULE_SOFTDEP("pre: amdtee");
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 6a0e4c446dd3..092be501d4d3 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -14,6 +14,11 @@
#include <linux/acpi.h>
#include <linux/platform_profile.h>
+#define POLICY_BUF_MAX_SZ 0x4b000
+#define POLICY_SIGN_COOKIE 0x31535024
+#define POLICY_COOKIE_OFFSET 0x10
+#define POLICY_COOKIE_LEN 0x14
+
/* APMF Functions */
#define APMF_FUNC_VERIFY_INTERFACE 0
#define APMF_FUNC_GET_SYS_PARAMS 1
@@ -59,8 +64,21 @@
#define ARG_NONE 0
#define AVG_SAMPLE_SIZE 3
+/* Policy Actions */
+#define PMF_POLICY_SPL 2
+#define PMF_POLICY_SPPT 3
+#define PMF_POLICY_FPPT 4
+#define PMF_POLICY_SPPT_APU_ONLY 5
+#define PMF_POLICY_STT_MIN 6
+#define PMF_POLICY_STT_SKINTEMP_APU 7
+#define PMF_POLICY_STT_SKINTEMP_HS2 8
+
/* TA macros */
#define PMF_TA_IF_VERSION_MAJOR 1
+#define TA_PMF_ACTION_MAX 32
+#define TA_PMF_UNDO_MAX 8
+#define TA_OUTPUT_RESERVED_MEM 906
+#define MAX_OPERATION_PARAMS 4
/* AMD PMF BIOS interfaces */
struct apmf_verify_interface {
@@ -183,11 +201,16 @@ struct amd_pmf_dev {
bool cnqf_supported;
struct notifier_block pwr_src_notifier;
/* Smart PC solution builder */
+ unsigned char *policy_buf;
+ u32 policy_sz;
struct tee_context *tee_ctx;
struct tee_shm *fw_shm_pool;
u32 session_id;
void *shbuf;
struct delayed_work pb_work;
+ struct pmf_action_table *prev_data;
+ u64 policy_addr;
+ void *policy_base;
bool smart_pc_enabled;
};
@@ -399,17 +422,134 @@ struct apmf_dyn_slider_output {
struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
} __packed;
+enum smart_pc_status {
+ PMF_SMART_PC_ENABLED,
+ PMF_SMART_PC_DISABLED,
+};
+
+/* Smart PC - TA internals */
+enum ta_slider {
+ TA_BEST_BATTERY,
+ TA_BETTER_BATTERY,
+ TA_BETTER_PERFORMANCE,
+ TA_BEST_PERFORMANCE,
+ TA_MAX,
+};
+
/* Command ids for TA communication */
enum ta_pmf_command {
TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE,
TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES,
};
+enum ta_pmf_error_type {
+ TA_PMF_TYPE_SUCCESS,
+ TA_PMF_ERROR_TYPE_GENERIC,
+ TA_PMF_ERROR_TYPE_CRYPTO,
+ TA_PMF_ERROR_TYPE_CRYPTO_VALIDATE,
+ TA_PMF_ERROR_TYPE_CRYPTO_VERIFY_OEM,
+ TA_PMF_ERROR_TYPE_POLICY_BUILDER,
+ TA_PMF_ERROR_TYPE_PB_CONVERT,
+ TA_PMF_ERROR_TYPE_PB_SETUP,
+ TA_PMF_ERROR_TYPE_PB_ENACT,
+ TA_PMF_ERROR_TYPE_ASD_GET_DEVICE_INFO,
+ TA_PMF_ERROR_TYPE_ASD_GET_DEVICE_PCIE_INFO,
+ TA_PMF_ERROR_TYPE_SYS_DRV_FW_VALIDATION,
+ TA_PMF_ERROR_TYPE_MAX,
+};
+
+struct pmf_action_table {
+ u32 spl; /* in mW */
+ u32 sppt; /* in mW */
+ u32 sppt_apuonly; /* in mW */
+ u32 fppt; /* in mW */
+ u32 stt_minlimit; /* in mW */
+ u32 stt_skintemp_apu; /* in C */
+ u32 stt_skintemp_hs2; /* in C */
+};
+
+/* Input conditions */
+struct ta_pmf_condition_info {
+ u32 power_source;
+ u32 bat_percentage;
+ u32 power_slider;
+ u32 lid_state;
+ bool user_present;
+ u32 rsvd1[2];
+ u32 monitor_count;
+ u32 rsvd2[2];
+ u32 bat_design;
+ u32 full_charge_capacity;
+ int drain_rate;
+ bool user_engaged;
+ u32 device_state;
+ u32 socket_power;
+ u32 skin_temperature;
+ u32 rsvd3[5];
+ u32 ambient_light;
+ u32 length;
+ u32 avg_c0residency;
+ u32 max_c0residency;
+ u32 s0i3_entry;
+ u32 gfx_busy;
+ u32 rsvd4[7];
+ bool camera_state;
+ u32 workload_type;
+ u32 display_type;
+ u32 display_state;
+ u32 rsvd5[150];
+};
+
+struct ta_pmf_load_policy_table {
+ u32 table_size;
+ u8 table[POLICY_BUF_MAX_SZ];
+};
+
+/* TA initialization params */
+struct ta_pmf_init_table {
+ u32 frequency; /* SMU sampling frequency */
+ bool validate;
+ bool sku_check;
+ bool metadata_macrocheck;
+ struct ta_pmf_load_policy_table policies_table;
+};
+
+/* Everything the TA needs to Enact Policies */
+struct ta_pmf_enact_table {
+ struct ta_pmf_condition_info ev_info;
+ u32 name;
+};
+
+struct ta_pmf_action {
+ u32 action_index;
+ u32 value;
+};
+
+/* Output actions from TA */
+struct ta_pmf_enact_result {
+ u32 actions_count;
+ struct ta_pmf_action actions_list[TA_PMF_ACTION_MAX];
+ u32 undo_count;
+ struct ta_pmf_action undo_list[TA_PMF_UNDO_MAX];
+};
+
+union ta_pmf_input {
+ struct ta_pmf_enact_table enact_table;
+ struct ta_pmf_init_table init_table;
+};
+
+union ta_pmf_output {
+ struct ta_pmf_enact_result policy_apply_table;
+ u32 rsvd[TA_OUTPUT_RESERVED_MEM];
+};
+
struct ta_pmf_shared_memory {
int command_id;
int resp_id;
u32 pmf_result;
u32 if_version;
+ union ta_pmf_output pmf_output;
+ union ta_pmf_input pmf_input;
};
/* Core Layer */
@@ -460,4 +600,5 @@ extern const struct attribute_group cnqf_feature_attribute_group;
/* Smart PC builder Layer */
int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
+int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev);
#endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 4036f435f1e2..468f3797a848 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -42,9 +42,77 @@ static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
param[0].u.memref.shm_offs = 0;
}
+static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
+{
+ u32 val;
+ int idx;
+
+ for (idx = 0; idx < out->actions_count; idx++) {
+ val = out->actions_list[idx].value;
+ switch (out->actions_list[idx].action_index) {
+ case PMF_POLICY_SPL:
+ if (dev->prev_data->spl != val) {
+ amd_pmf_send_cmd(dev, SET_SPL, false, val, NULL);
+ dev_dbg(dev->dev, "update SPL: %u\n", val);
+ dev->prev_data->spl = val;
+ }
+ break;
+
+ case PMF_POLICY_SPPT:
+ if (dev->prev_data->sppt != val) {
+ amd_pmf_send_cmd(dev, SET_SPPT, false, val, NULL);
+ dev_dbg(dev->dev, "update SPPT: %u\n", val);
+ dev->prev_data->sppt = val;
+ }
+ break;
+
+ case PMF_POLICY_FPPT:
+ if (dev->prev_data->fppt != val) {
+ amd_pmf_send_cmd(dev, SET_FPPT, false, val, NULL);
+ dev_dbg(dev->dev, "update FPPT: %u\n", val);
+ dev->prev_data->fppt = val;
+ }
+ break;
+
+ case PMF_POLICY_SPPT_APU_ONLY:
+ if (dev->prev_data->sppt_apuonly != val) {
+ amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, val, NULL);
+ dev_dbg(dev->dev, "update SPPT_APU_ONLY: %u\n", val);
+ dev->prev_data->sppt_apuonly = val;
+ }
+ break;
+
+ case PMF_POLICY_STT_MIN:
+ if (dev->prev_data->stt_minlimit != val) {
+ amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, val, NULL);
+ dev_dbg(dev->dev, "update STT_MIN: %u\n", val);
+ dev->prev_data->stt_minlimit = val;
+ }
+ break;
+
+ case PMF_POLICY_STT_SKINTEMP_APU:
+ if (dev->prev_data->stt_skintemp_apu != val) {
+ amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false, val, NULL);
+ dev_dbg(dev->dev, "update STT_SKINTEMP_APU: %u\n", val);
+ dev->prev_data->stt_skintemp_apu = val;
+ }
+ break;
+
+ case PMF_POLICY_STT_SKINTEMP_HS2:
+ if (dev->prev_data->stt_skintemp_hs2 != val) {
+ amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false, val, NULL);
+ dev_dbg(dev->dev, "update STT_SKINTEMP_HS2: %u\n", val);
+ dev->prev_data->stt_skintemp_hs2 = val;
+ }
+ break;
+ }
+ }
+}
+
static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
{
struct ta_pmf_shared_memory *ta_sm = NULL;
+ struct ta_pmf_enact_result *out = NULL;
struct tee_param param[MAX_TEE_PARAM];
struct tee_ioctl_invoke_arg arg;
int ret = 0;
@@ -52,7 +120,10 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
if (!dev->tee_ctx)
return -ENODEV;
+ memset(dev->shbuf, 0, dev->policy_sz);
ta_sm = dev->shbuf;
+ out = &ta_sm->pmf_output.policy_apply_table;
+
memset(ta_sm, 0, sizeof(*ta_sm));
ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES;
ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
@@ -65,6 +136,12 @@ static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
return ret;
}
+ if (ta_sm->pmf_result == TA_PMF_TYPE_SUCCESS && out->actions_count) {
+ dev_dbg(dev->dev, "action count:%u result:%x\n", out->actions_count,
+ ta_sm->pmf_result);
+ amd_pmf_apply_policies(dev, out);
+ }
+
return 0;
}
@@ -72,6 +149,7 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
{
struct ta_pmf_shared_memory *ta_sm = NULL;
struct tee_param param[MAX_TEE_PARAM];
+ struct ta_pmf_init_table *in = NULL;
struct tee_ioctl_invoke_arg arg;
int ret = 0;
@@ -80,10 +158,21 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
return -ENODEV;
}
+ dev_dbg(dev->dev, "Policy Binary size: %u bytes\n", dev->policy_sz);
+ memset(dev->shbuf, 0, dev->policy_sz);
ta_sm = dev->shbuf;
+ in = &ta_sm->pmf_input.init_table;
+
ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE;
ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
+ in->metadata_macrocheck = false;
+ in->sku_check = false;
+ in->validate = true;
+ in->frequency = pb_actions_ms;
+ in->policies_table.table_size = dev->policy_sz;
+
+ memcpy(in->policies_table.table, dev->policy_buf, dev->policy_sz);
amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE, &arg, param);
ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
@@ -103,6 +192,52 @@ static void amd_pmf_invoke_cmd(struct work_struct *work)
schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms));
}
+static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
+{
+ u32 cookie, length;
+ int res;
+
+ cookie = readl(dev->policy_buf + POLICY_COOKIE_OFFSET);
+ length = readl(dev->policy_buf + POLICY_COOKIE_LEN);
+
+ if (cookie != POLICY_SIGN_COOKIE || !length)
+ return -EINVAL;
+
+ /* Update the actual length */
+ dev->policy_sz = length + 512;
+ res = amd_pmf_invoke_cmd_init(dev);
+ if (res == TA_PMF_TYPE_SUCCESS) {
+ /* Now its safe to announce that smart pc is enabled */
+ dev->smart_pc_enabled = PMF_SMART_PC_ENABLED;
+ /*
+ * Start collecting the data from TA FW after a small delay
+ * or else, we might end up getting stale values.
+ */
+ schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms * 3));
+ } else {
+ dev_err(dev->dev, "ta invoke cmd init failed err: %x\n", res);
+ dev->smart_pc_enabled = PMF_SMART_PC_DISABLED;
+ return res;
+ }
+
+ return 0;
+}
+
+static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
+{
+ dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
+ if (!dev->policy_buf)
+ return -ENOMEM;
+
+ dev->policy_base = devm_ioremap(dev->dev, dev->policy_addr, dev->policy_sz);
+ if (!dev->policy_base)
+ return -ENOMEM;
+
+ memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
+
+ return amd_pmf_start_policy_engine(dev);
+}
+
static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
{
return ver->impl_id == TEE_IMPL_ID_AMDTEE;
@@ -146,7 +281,7 @@ static int amd_pmf_tee_init(struct amd_pmf_dev *dev)
goto out_ctx;
}
- size = sizeof(struct ta_pmf_shared_memory);
+ size = sizeof(struct ta_pmf_shared_memory) + dev->policy_sz;
dev->fw_shm_pool = tee_shm_alloc_kernel_buf(dev->tee_ctx, size);
if (IS_ERR(dev->fw_shm_pool)) {
dev_err(dev->dev, "Failed to alloc TEE shared memory\n");
@@ -190,11 +325,19 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
return ret;
INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
- return 0;
+ amd_pmf_set_dram_addr(dev);
+ amd_pmf_get_bios_buffer(dev);
+ dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL);
+ if (!dev->prev_data)
+ return -ENOMEM;
+
+ return dev->smart_pc_enabled;
}
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
{
+ kfree(dev->prev_data);
+ kfree(dev->policy_buf);
cancel_delayed_work_sync(&dev->pb_work);
amd_pmf_tee_deinit(dev);
}
--
2.25.1
^ permalink raw reply related
* [PATCH v6 03/15] platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
From: Shyam Sundar S K @ 2023-12-04 10:15 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
benjamin.tissoires
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
Shyam Sundar S K
In-Reply-To: <20231204101548.1458499-1-Shyam-sundar.S-k@amd.com>
In the current code, the metrics table information was required only
for auto-mode or CnQF at a given time. Hence keeping the return type
of amd_pmf_set_dram_addr() as static made sense.
But with the addition of Smart PC builder feature, the metrics table
information has to be shared by the Smart PC also and this feature
resides outside of core.c.
To make amd_pmf_set_dram_addr() visible outside of core.c make it
as a non-static function and move the allocation of memory for
metrics table from amd_pmf_init_metrics_table() to amd_pmf_set_dram_addr()
as amd_pmf_set_dram_addr() is the common function to set the DRAM
address.
Add a suspend handler that can free up the allocated memory for getting
the metrics table information.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/core.c | 42 ++++++++++++++++++++++-------
drivers/platform/x86/amd/pmf/pmf.h | 1 +
2 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index ec92d1cc0dac..f50b7ec9a625 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -251,29 +251,35 @@ static const struct pci_device_id pmf_pci_ids[] = {
{ }
};
-static void amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
+int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
{
u64 phys_addr;
u32 hi, low;
+ /* Get Metrics Table Address */
+ dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
+ if (!dev->buf)
+ return -ENOMEM;
+
phys_addr = virt_to_phys(dev->buf);
hi = phys_addr >> 32;
low = phys_addr & GENMASK(31, 0);
amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
+
+ return 0;
}
int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
{
- /* Get Metrics Table Address */
- dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
- if (!dev->buf)
- return -ENOMEM;
+ int ret;
INIT_DELAYED_WORK(&dev->work_buffer, amd_pmf_get_metrics);
- amd_pmf_set_dram_addr(dev);
+ ret = amd_pmf_set_dram_addr(dev);
+ if (ret)
+ return ret;
/*
* Start collecting the metrics data after a small delay
@@ -284,17 +290,35 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
return 0;
}
+static int amd_pmf_suspend_handler(struct device *dev)
+{
+ struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
+
+ /*
+ * Free the buffer allocated for storing the metrics table
+ * information, as will have to allocate it freshly after
+ * resume.
+ */
+ kfree(pdev->buf);
+
+ return 0;
+}
+
static int amd_pmf_resume_handler(struct device *dev)
{
struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
+ int ret;
- if (pdev->buf)
- amd_pmf_set_dram_addr(pdev);
+ if (pdev->buf) {
+ ret = amd_pmf_set_dram_addr(pdev);
+ if (ret)
+ return ret;
+ }
return 0;
}
-static DEFINE_SIMPLE_DEV_PM_OPS(amd_pmf_pm, NULL, amd_pmf_resume_handler);
+static DEFINE_SIMPLE_DEV_PM_OPS(amd_pmf_pm, amd_pmf_suspend_handler, amd_pmf_resume_handler);
static void amd_pmf_init_features(struct amd_pmf_dev *dev)
{
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index a24e34e42032..6a0e4c446dd3 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -421,6 +421,7 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev);
int amd_pmf_get_power_source(void);
int apmf_install_handler(struct amd_pmf_dev *pmf_dev);
int apmf_os_power_slider_update(struct amd_pmf_dev *dev, u8 flag);
+int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev);
/* SPS Layer */
int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);
--
2.25.1
^ permalink raw reply related
* [PATCH v6 02/15] platform/x86/amd/pmf: Add support for PMF-TA interaction
From: Shyam Sundar S K @ 2023-12-04 10:15 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
benjamin.tissoires
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
Shyam Sundar S K
In-Reply-To: <20231204101548.1458499-1-Shyam-sundar.S-k@amd.com>
PMF TA (Trusted Application) loads via the TEE environment into the
AMD ASP.
PMF-TA supports two commands:
1) Init: Initialize the TA with the PMF Smart PC policy binary and
start the policy engine. A policy is a combination of inputs and
outputs, where;
- the inputs are the changing dynamics of the system like the user
behaviour, system heuristics etc.
- the outputs, which are the actions to be set on the system which
lead to better power management and enhanced user experience.
PMF driver acts as a central manager in this case to supply the
inputs required to the TA (either by getting the information from
the other kernel subsystems or from userland)
2) Enact: Enact the output actions from the TA. The action could be
applying a new thermal limit to boost/throttle the power limits or
change system behavior.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/pmf.h | 10 +++
drivers/platform/x86/amd/pmf/tee-if.c | 97 ++++++++++++++++++++++++++-
2 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index bd40458937ba..a24e34e42032 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -59,6 +59,9 @@
#define ARG_NONE 0
#define AVG_SAMPLE_SIZE 3
+/* TA macros */
+#define PMF_TA_IF_VERSION_MAJOR 1
+
/* AMD PMF BIOS interfaces */
struct apmf_verify_interface {
u16 size;
@@ -184,6 +187,7 @@ struct amd_pmf_dev {
struct tee_shm *fw_shm_pool;
u32 session_id;
void *shbuf;
+ struct delayed_work pb_work;
bool smart_pc_enabled;
};
@@ -395,6 +399,12 @@ struct apmf_dyn_slider_output {
struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
} __packed;
+/* Command ids for TA communication */
+enum ta_pmf_command {
+ TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE,
+ TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES,
+};
+
struct ta_pmf_shared_memory {
int command_id;
int resp_id;
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 6ec8c3726624..4036f435f1e2 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -13,9 +13,96 @@
#include "pmf.h"
#define MAX_TEE_PARAM 4
+
+/* Policy binary actions sampling frequency (in ms) */
+static int pb_actions_ms = MSEC_PER_SEC;
+#ifdef CONFIG_AMD_PMF_DEBUG
+module_param(pb_actions_ms, int, 0644);
+MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)");
+#endif
+
static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
+static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
+ struct tee_ioctl_invoke_arg *arg,
+ struct tee_param *param)
+{
+ memset(arg, 0, sizeof(*arg));
+ memset(param, 0, MAX_TEE_PARAM * sizeof(*param));
+
+ arg->func = cmd;
+ arg->session = dev->session_id;
+ arg->num_params = MAX_TEE_PARAM;
+
+ /* Fill invoke cmd params */
+ param[0].u.memref.size = sizeof(struct ta_pmf_shared_memory);
+ param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT;
+ param[0].u.memref.shm = dev->fw_shm_pool;
+ param[0].u.memref.shm_offs = 0;
+}
+
+static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
+{
+ struct ta_pmf_shared_memory *ta_sm = NULL;
+ struct tee_param param[MAX_TEE_PARAM];
+ struct tee_ioctl_invoke_arg arg;
+ int ret = 0;
+
+ if (!dev->tee_ctx)
+ return -ENODEV;
+
+ ta_sm = dev->shbuf;
+ memset(ta_sm, 0, sizeof(*ta_sm));
+ ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES;
+ ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
+
+ amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER_ENACT_POLICIES, &arg, param);
+
+ ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
+ if (ret < 0 || arg.ret != 0) {
+ dev_err(dev->dev, "TEE enact cmd failed. err: %x, ret:%d\n", arg.ret, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
+{
+ struct ta_pmf_shared_memory *ta_sm = NULL;
+ struct tee_param param[MAX_TEE_PARAM];
+ struct tee_ioctl_invoke_arg arg;
+ int ret = 0;
+
+ if (!dev->tee_ctx) {
+ dev_err(dev->dev, "Failed to get TEE context\n");
+ return -ENODEV;
+ }
+
+ ta_sm = dev->shbuf;
+ ta_sm->command_id = TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE;
+ ta_sm->if_version = PMF_TA_IF_VERSION_MAJOR;
+
+ amd_pmf_prepare_args(dev, TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE, &arg, param);
+
+ ret = tee_client_invoke_func(dev->tee_ctx, &arg, param);
+ if (ret < 0 || arg.ret != 0) {
+ dev_err(dev->dev, "Failed to invoke TEE init cmd. err: %x, ret:%d\n", arg.ret, ret);
+ return ret;
+ }
+
+ return ta_sm->pmf_result;
+}
+
+static void amd_pmf_invoke_cmd(struct work_struct *work)
+{
+ struct amd_pmf_dev *dev = container_of(work, struct amd_pmf_dev, pb_work.work);
+
+ amd_pmf_invoke_cmd_enact(dev);
+ schedule_delayed_work(&dev->pb_work, msecs_to_jiffies(pb_actions_ms));
+}
+
static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
{
return ver->impl_id == TEE_IMPL_ID_AMDTEE;
@@ -96,10 +183,18 @@ static void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
{
- return amd_pmf_tee_init(dev);
+ int ret;
+
+ ret = amd_pmf_tee_init(dev);
+ if (ret)
+ return ret;
+
+ INIT_DELAYED_WORK(&dev->pb_work, amd_pmf_invoke_cmd);
+ return 0;
}
void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
{
+ cancel_delayed_work_sync(&dev->pb_work);
amd_pmf_tee_deinit(dev);
}
--
2.25.1
^ permalink raw reply related
* [PATCH v6 01/15] platform/x86/amd/pmf: Add PMF TEE interface
From: Shyam Sundar S K @ 2023-12-04 10:15 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
benjamin.tissoires
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
Shyam Sundar S K
In-Reply-To: <20231204101548.1458499-1-Shyam-sundar.S-k@amd.com>
AMD PMF driver loads the PMF TA (Trusted Application) into the AMD
ASP's (AMD Security Processor) TEE (Trusted Execution Environment).
PMF Trusted Application is a secured firmware placed under
/lib/firmware/amdtee gets loaded only when the TEE environment is
initialized. Add the initial code path to build these pipes.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/Kconfig | 1 +
drivers/platform/x86/amd/pmf/Makefile | 3 +-
drivers/platform/x86/amd/pmf/core.c | 10 ++-
drivers/platform/x86/amd/pmf/pmf.h | 16 ++++
drivers/platform/x86/amd/pmf/tee-if.c | 105 ++++++++++++++++++++++++++
5 files changed, 130 insertions(+), 5 deletions(-)
create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index 3064bc8ea167..32a029e8db80 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -9,6 +9,7 @@ config AMD_PMF
depends on POWER_SUPPLY
depends on AMD_NB
select ACPI_PLATFORM_PROFILE
+ depends on TEE
help
This driver provides support for the AMD Platform Management Framework.
The goal is to enhance end user experience by making AMD PCs smarter,
diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
index fdededf54392..d2746ee7369f 100644
--- a/drivers/platform/x86/amd/pmf/Makefile
+++ b/drivers/platform/x86/amd/pmf/Makefile
@@ -6,4 +6,5 @@
obj-$(CONFIG_AMD_PMF) += amd-pmf.o
amd-pmf-objs := core.o acpi.o sps.o \
- auto-mode.o cnqf.o
+ auto-mode.o cnqf.o \
+ tee-if.o
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index 78ed3ee22555..ec92d1cc0dac 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -309,13 +309,13 @@ static void amd_pmf_init_features(struct amd_pmf_dev *dev)
dev_dbg(dev->dev, "SPS enabled and Platform Profiles registered\n");
}
- /* Enable Auto Mode */
- if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
+ if (!amd_pmf_init_smart_pc(dev)) {
+ dev_dbg(dev->dev, "Smart PC Solution Enabled\n");
+ } else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
amd_pmf_init_auto_mode(dev);
dev_dbg(dev->dev, "Auto Mode Init done\n");
} else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_DC)) {
- /* Enable Cool n Quiet Framework (CnQF) */
ret = amd_pmf_init_cnqf(dev);
if (ret)
dev_warn(dev->dev, "CnQF Init failed\n");
@@ -330,7 +330,9 @@ static void amd_pmf_deinit_features(struct amd_pmf_dev *dev)
amd_pmf_deinit_sps(dev);
}
- if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
+ if (!dev->smart_pc_enabled) {
+ amd_pmf_deinit_smart_pc(dev);
+ } else if (is_apmf_func_supported(dev, APMF_FUNC_AUTO_MODE)) {
amd_pmf_deinit_auto_mode(dev);
} else if (is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_AC) ||
is_apmf_func_supported(dev, APMF_FUNC_DYN_SLIDER_DC)) {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index deba88e6e4c8..bd40458937ba 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -179,6 +179,12 @@ struct amd_pmf_dev {
bool cnqf_enabled;
bool cnqf_supported;
struct notifier_block pwr_src_notifier;
+ /* Smart PC solution builder */
+ struct tee_context *tee_ctx;
+ struct tee_shm *fw_shm_pool;
+ u32 session_id;
+ void *shbuf;
+ bool smart_pc_enabled;
};
struct apmf_sps_prop_granular {
@@ -389,6 +395,13 @@ struct apmf_dyn_slider_output {
struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
} __packed;
+struct ta_pmf_shared_memory {
+ int command_id;
+ int resp_id;
+ u32 pmf_result;
+ u32 if_version;
+};
+
/* Core Layer */
int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev);
@@ -433,4 +446,7 @@ void amd_pmf_deinit_cnqf(struct amd_pmf_dev *dev);
int amd_pmf_trans_cnqf(struct amd_pmf_dev *dev, int socket_power, ktime_t time_lapsed_ms);
extern const struct attribute_group cnqf_feature_attribute_group;
+/* Smart PC builder Layer */
+int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev);
+void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev);
#endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
new file mode 100644
index 000000000000..6ec8c3726624
--- /dev/null
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Platform Management Framework Driver - TEE Interface
+ *
+ * Copyright (c) 2023, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ */
+
+#include <linux/tee_drv.h>
+#include <linux/uuid.h>
+#include "pmf.h"
+
+#define MAX_TEE_PARAM 4
+static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
+ 0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
+
+static int amd_pmf_amdtee_ta_match(struct tee_ioctl_version_data *ver, const void *data)
+{
+ return ver->impl_id == TEE_IMPL_ID_AMDTEE;
+}
+
+static int amd_pmf_ta_open_session(struct tee_context *ctx, u32 *id)
+{
+ struct tee_ioctl_open_session_arg sess_arg = {};
+ int rc;
+
+ export_uuid(sess_arg.uuid, &amd_pmf_ta_uuid);
+ sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
+ sess_arg.num_params = 0;
+
+ rc = tee_client_open_session(ctx, &sess_arg, NULL);
+ if (rc < 0 || sess_arg.ret != 0) {
+ pr_err("Failed to open TEE session err:%#x, rc:%d\n", sess_arg.ret, rc);
+ return rc;
+ }
+
+ *id = sess_arg.session;
+
+ return rc;
+}
+
+static int amd_pmf_tee_init(struct amd_pmf_dev *dev)
+{
+ u32 size;
+ int ret;
+
+ dev->tee_ctx = tee_client_open_context(NULL, amd_pmf_amdtee_ta_match, NULL, NULL);
+ if (IS_ERR(dev->tee_ctx)) {
+ dev_err(dev->dev, "Failed to open TEE context\n");
+ return PTR_ERR(dev->tee_ctx);
+ }
+
+ ret = amd_pmf_ta_open_session(dev->tee_ctx, &dev->session_id);
+ if (ret) {
+ dev_err(dev->dev, "Failed to open TA session (%d)\n", ret);
+ ret = -EINVAL;
+ goto out_ctx;
+ }
+
+ size = sizeof(struct ta_pmf_shared_memory);
+ dev->fw_shm_pool = tee_shm_alloc_kernel_buf(dev->tee_ctx, size);
+ if (IS_ERR(dev->fw_shm_pool)) {
+ dev_err(dev->dev, "Failed to alloc TEE shared memory\n");
+ ret = PTR_ERR(dev->fw_shm_pool);
+ goto out_sess;
+ }
+
+ dev->shbuf = tee_shm_get_va(dev->fw_shm_pool, 0);
+ if (IS_ERR(dev->shbuf)) {
+ dev_err(dev->dev, "Failed to get TEE virtual address\n");
+ ret = PTR_ERR(dev->shbuf);
+ goto out_shm;
+ }
+ dev_dbg(dev->dev, "TEE init done\n");
+
+ return 0;
+
+out_shm:
+ tee_shm_free(dev->fw_shm_pool);
+out_sess:
+ tee_client_close_session(dev->tee_ctx, dev->session_id);
+out_ctx:
+ tee_client_close_context(dev->tee_ctx);
+
+ return ret;
+}
+
+static void amd_pmf_tee_deinit(struct amd_pmf_dev *dev)
+{
+ tee_shm_free(dev->fw_shm_pool);
+ tee_client_close_session(dev->tee_ctx, dev->session_id);
+ tee_client_close_context(dev->tee_ctx);
+}
+
+int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
+{
+ return amd_pmf_tee_init(dev);
+}
+
+void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
+{
+ amd_pmf_tee_deinit(dev);
+}
--
2.25.1
^ permalink raw reply related
* [PATCH v6 00/15] Introduce PMF Smart PC Solution Builder Feature
From: Shyam Sundar S K @ 2023-12-04 10:15 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
benjamin.tissoires
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
Shyam Sundar S K
Smart PC Solutions Builder allows for OEM to define a large number of
custom system states to dynamically switch to. The system states are
referred to as policies, and multiple policies can be loaded onto the
system at any given time, however only one policy can be active at a
given time.
Policy is a combination of PMF input and output capabilities. The inputs
are the incoming information from the other kernel subsystems like LID
state, Sensor info, GPU info etc and the actions are the updating the
power limits of SMU etc.
The policy binary is signed and encrypted by a special key from AMD. This
policy binary shall have the inputs and outputs which the OEMs can build
for the platform customization that can enhance the user experience and
system behavior.
This series adds the initial support for Smart PC solution to PMF driver.
Note that, on platforms where CnQF and Smart PC is advertised, Smart PC
shall have higher precedence and same applies for Auto Mode.
v5->v6:
---------
- Add Ilpo's and Mario's Reviewed-by tags
- Drop 13/17 and 14/17 patches from this series which are GPU centric
- Drop separate checks for battery handling.
- Handle SFH failure cases
v4->v5:
---------
- Remove PMF-GPU interface from amdgpu driver and add DRM/backlight
changes within PMF
- Add module_softdep for AMDGPU
- remove error checks for debugfs_create_file()
- Add "Reviewed-by:" tags
- Add kerneldoc for kernel-wide headers
- Add checks for acpi_backlight_native
- Add early return for SFH path
- other cosmetic changes
v3->v4:
---------
- Split v3 9/16 into 2 patches, that addresses using generic fn names
- Add softdep [Ilpo] instead of request_module()
- return proper ACPI status [Mario]
- Update comments in code [Mario]
- Remove missed double _ remarks
- handle battery status branches [Ilpo]
- Address KASAN problems
v2->v3:
---------
- Remove pci_get_device() for getting gpu handle
- add .suspend handler for pmf driver
- remove unwanted type caste
- Align comments, spaces etc.
- add wrapper for print_hex_dump_debug()
- Remove lkp tags in commit-msg
- Add macros for magic numbers
- use right format specifiers for printing
- propagate error codes back to the caller
- remove unwanted comments
v1->v2:
---------
- Remove __func__ macros
- Remove manual function names inside prints
- Handle tee_shm_get_va() failure
- Remove double _
- Add meaningful prints
- pass amd_pmf_set_dram_addr() failure errors
- Add more information to commit messages
- use right format specifiers
- use devm_ioremap() instead of ioremap()
- address unsigned long vs u32 problems
- Fix lkp reported issues
- Add amd_pmf_remove_pb() to remove the debugfs files created(if any).
- Make amd_pmf_open_pb() as static.
- Add cooling device APIs for controlling amdgpu backlight
- handle amd_pmf_apply_policies() failures
- Split v1 14/15 into 2 patches further
- use linux/units.h for better handling
- add "depends on" AMD_SFH_HID for interaction with SFH
- other cosmetic remarks
Basavaraj Natikar (3):
HID: amd_sfh: rename float_to_int() to amd_sfh_float_to_int()
platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS
Shyam Sundar S K (12):
platform/x86/amd/pmf: Add PMF TEE interface
platform/x86/amd/pmf: Add support for PMF-TA interaction
platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
platform/x86/amd/pmf: Add support for PMF Policy Binary
platform/x86/amd/pmf: change amd_pmf_init_features() call sequence
platform/x86/amd/pmf: Add support to get inputs from other subsystems
platform/x86/amd/pmf: Add support update p3t limit
platform/x86/amd/pmf: Add support to update system state
platform/x86/amd/pmf: Make source_as_str() as non-static
platform/x86/amd/pmf: Add facility to dump TA inputs
platform/x86/amd/pmf: Add capability to sideload of policy binary
platform/x86/amd/pmf: dump policy binary data
Documentation/admin-guide/index.rst | 1 +
Documentation/admin-guide/pmf.rst | 24 +
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 6 +
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 28 +-
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 20 +
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 52 ++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 2 +
drivers/platform/x86/amd/pmf/Kconfig | 2 +
drivers/platform/x86/amd/pmf/Makefile | 3 +-
drivers/platform/x86/amd/pmf/acpi.c | 37 ++
drivers/platform/x86/amd/pmf/core.c | 63 ++-
drivers/platform/x86/amd/pmf/pmf.h | 203 ++++++++
drivers/platform/x86/amd/pmf/spc.c | 187 ++++++++
drivers/platform/x86/amd/pmf/sps.c | 5 +-
drivers/platform/x86/amd/pmf/tee-if.c | 452 ++++++++++++++++++
include/linux/amd-pmf-io.h | 50 ++
16 files changed, 1105 insertions(+), 30 deletions(-)
create mode 100644 Documentation/admin-guide/pmf.rst
create mode 100644 drivers/platform/x86/amd/pmf/spc.c
create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c
create mode 100644 include/linux/amd-pmf-io.h
--
2.25.1
^ permalink raw reply
* Re: [PATCH v2 1/6] x86/vmware: Move common macros to vmware.h
From: Borislav Petkov @ 2023-12-04 10:32 UTC (permalink / raw)
To: Alexey Makhalov
Cc: linux-kernel, virtualization, hpa, dave.hansen, bp, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <20231201232452.220355-2-amakhalov@vmware.com>
On Fri, Dec 01, 2023 at 03:24:47PM -0800, Alexey Makhalov wrote:
> Move VMware hypercall macros to vmware.h as a preparation step
> for the next commit. No functional changes besides exporting
"next commit" in git is ambiguous. Get rid of such formulations.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 6/6] x86/vmware: Add TDX hypercall support
From: Borislav Petkov @ 2023-12-04 10:31 UTC (permalink / raw)
To: Alexey Makhalov
Cc: linux-kernel, virtualization, hpa, dave.hansen, bp, mingo, tglx,
x86, netdev, richardcochran, linux-input, dmitry.torokhov, zackr,
linux-graphics-maintainer, pv-drivers, namit, timothym, akaher,
jsipek, dri-devel, daniel, airlied, tzimmermann, mripard,
maarten.lankhorst, horms
In-Reply-To: <20231201232452.220355-7-amakhalov@vmware.com>
On Fri, Dec 01, 2023 at 03:24:52PM -0800, Alexey Makhalov wrote:
> +#ifdef CONFIG_INTEL_TDX_GUEST
> +/* __tdx_hypercall() is not exported. So, export the wrapper */
> +void vmware_tdx_hypercall_args(struct tdx_module_args *args)
> +{
> + __tdx_hypercall(args);
> +}
> +EXPORT_SYMBOL_GPL(vmware_tdx_hypercall_args);
Uuuh, lovely. I'd like to see what the TDX folks think about this
export first.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: supporting binary (near-far) proximity sensors over gpio
From: Jonathan Cameron @ 2023-12-04 10:09 UTC (permalink / raw)
To: Philipp Jungkamp
Cc: Jeff LaBundy, David Lechner, Sicelo, linux-iio, maemo-leste,
Ivaylo Dimitrov, linux-input
In-Reply-To: <94eafe4dfe176d0ca01ddbd0ec599e3c5fb2e0a3.camel@gmx.net>
On Sun, 26 Nov 2023 11:59:05 +0100
Philipp Jungkamp <p.jungkamp@gmx.net> wrote:
> Hi,
>
> would it make sense to move the HID human presence driver at
> drivers/iio/light/hid-sensor-prox.c to the input system then? This
> driver only checks for the "Biometric" (0x2004b0) and "Biometric: Human
> Presence" (0x2004b1) HID usages. The former has a vendor defined value
> range, the latter is defined as a boolean switch. See the HID Usage
> Tables, section 21.1 and 21.6.
These boundaries always get messy. Given there is a value range, it
might get used for something else.
>
> I take from this discussion that the input subsystem would make more
> sense for the "Biometric: Human Presence" usage.
Potentially yes
>
> I just wanted to chime in as there seems to be some older precedent for
> a binary sensor in the iio subsystem. I tried to get that sensor
> working for the proximity sensor on my laptop last year.
I guess this one landed because it looked much like the hid light sensors
and no one raised the question at the time (that I recall).
Probably not worth moving it, but we may well have suggested putting
it in input if we'd noticed at the time!
Jonathan
>
> Regards,
> Philipp Jungkamp
>
> On Sat, 2023-11-25 at 22:33 -0600, Jeff LaBundy wrote:
> > Hi Sicelo and David,
> >
> > On Sat, Nov 18, 2023 at 06:09:18PM -0600, David Lechner wrote:
> > > On Fri, Nov 17, 2023 at 12:22 PM Sicelo <absicsz@gmail.com> wrote:
> > > >
> > > > Hi
> > > >
> > > > Some phones have 1-bit proximity sensors, which simply toggle a
> > > > GPIO
> > > > line to indicate that an object is near or far. Thresholds are
> > > > set at
> > > > hardware level. One such sensor is OSRAM SFH 7741 [1], which is
> > > > used on
> > > > the Nokia N900.
> > > >
> > > > It is currently exported over evdev, emitting the
> > > > SW_FRONT_PROXIMITY key
> > > > code [2].
> > > >
> > > > So the question is: should a new, general purpose iio-gpio driver
> > > > be
> > > > written, that would switch such a proximity sensor to the iio
> > > > framework?
> > > > Or evdev is really the best place to support it?
> > > >
> > > > There are a couple of people who are willing to write such an iio
> > > > driver, if iio is the way to go.
> > > >
> > > > Regards,
> > > > Sicelo
> > > >
> > > > [1]
> > > > https://media.digikey.com/pdf/Data%20Sheets/Osram%20PDFs/SFH_7741.pdf
> > > > [2]
> > > > https://elixir.bootlin.com/linux/v6.6.1/source/arch/arm/boot/dts/ti/omap/omap3-n900.dts#L111
> > > >
> > > Since this is really a proximity switch (it is either on or off)
> > > rather than measuring a proximity value over a continuous range, it
> > > doesn't seem like a good fit for the iio subsystem. If the sensor
> > > is
> > > on a phone, then it is likely to detect human presence so the input
> > > subsystem does seem like the right one for that application.
> > >
> > > More at
> > > https://www.kernel.org/doc/html/latest/driver-api/iio/intro.html
> > >
> >
> > I tend to agree; if there are only two discrete states as is the case
> > for a
> > GPIO, then this is technically a switch and not a sensor. Therefore,
> > input
> > seems like a better fit; that is just my $.02.
> >
> > FWIW, a similar discussion came up a few years ago in [1] and again
> > the key
> > differentiator was whether the output is discrete or continuous.
> >
> > Kind regards,
> > Jeff LaBundy
> >
> > [1]
> > https://lore.kernel.org/linux-iio/9f9b0ff6-3bf1-63c4-eb36-901cecd7c4d9@redhat.com/
> >
>
^ permalink raw reply
* Re: [RFC PATCH v3 2/5] i2c: of: Introduce component probe function
From: Chen-Yu Tsai @ 2023-12-04 9:52 UTC (permalink / raw)
To: Doug Anderson, Wolfram Sang
Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Benson Leung,
Tzung-Bi Shih, chrome-platform, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel, Johan Hovold, Hsin-Yi Wang,
Dmitry Torokhov, andriy.shevchenko, Jiri Kosina, linus.walleij,
broonie, gregkh, hdegoede, james.clark, james, keescook, rafael,
tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAD=FV=U_+iQJtV0Wii89DQT1V_fJCeS9wcqA8EJAs-hmmmLLLg@mail.gmail.com>
On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > @@ -217,4 +217,114 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
> > struct notifier_block i2c_of_notifier = {
> > .notifier_call = of_i2c_notify,
> > };
> > +
> > +/*
> > + * Some devices, such as Google Hana Chromebooks, are produced by multiple
> > + * vendors each using their preferred components. Such components are all
> > + * in the device tree. Instead of having all of them enabled and having each
> > + * driver separately try and probe its device while fighting over shared
> > + * resources, they can be marked as "fail-needs-probe" and have a prober
> > + * figure out which one is actually used beforehand.
> > + *
> > + * This prober assumes such drop-in parts are on the same I2C bus, have
> > + * non-conflicting addresses, and can be directly probed by seeing which
> > + * address responds.
> > + *
> > + * TODO:
> > + * - Support handling common regulators and GPIOs.
>
> IMO you should prototype how you're going to handle regulators and
> GPIOs before finalizing the design. I was going to write that you
> should just document that it was up to the caller to power things up
> before calling this function, but then I realized that the caller
> would have to duplicate much of this function in order to do so. In
> the very least they'd have to find the nodes of the relevant devices
> so that they could grab regulators and/or GPIOs. In order to avoid
> this duplication, would the design need to change? Perhaps this would
> be as simple as adding a callback function here that's called with all
> of the nodes before probing? If that's right, it would be nice to have
> that callback from the beginning so we don't need two variants of the
> function...
So I think I can prototype designs with one GPIO and multiple regulators,
assuming each node has the same number of both? At least they should if
they're on the same connector.
More than one GPIO probably means there are some ordering and timing
constraints, and won't be as generic.
> > + * - Support I2C muxes
> > + */
> > +
> > +/**
> > + * i2c_of_probe_component() - probe for devices of "type" on the same i2c bus
> > + * @dev: &struct device of the caller, only used for dev_* printk messages
> > + * @type: a string to match the device node name prefix to probe for
> > + *
> > + * Probe for possible I2C components of the same "type" on the same I2C bus
> > + * that have their status marked as "fail".
>
> Should document these current limitations with the code:
>
> * Assumes that across the entire device tree the only instances of
> nodes named "type" are ones we're trying to handle second sourcing
> for. In other words if we're searching for "touchscreen" then all
> nodes named "touchscreen" are ones that need to be probed.
>
> * Assumes that there is exactly one group of each "type". In other
> words, if we're searching for "touchscreen" then exactly one
> touchscreen will be enabled across the whole tree.
>
> Obviously those could be relaxed with more code, but that's the
> current limitation and it makes the code easier to understand with
> that context.
Done.
> > + */
> > +int i2c_of_probe_component(struct device *dev, const char *type)
> > +{
> > + struct device_node *node, *i2c_node;
> > + struct i2c_adapter *i2c;
> > + struct of_changeset *ocs = NULL;
> > + int ret;
> > +
> > + node = of_find_node_by_name(NULL, type);
> > + if (!node)
> > + return dev_err_probe(dev, -ENODEV, "Could not find %s device node\n", type);
> > +
> > + i2c_node = of_get_next_parent(node);
> > + if (!of_node_name_eq(i2c_node, "i2c")) {
> > + of_node_put(i2c_node);
> > + return dev_err_probe(dev, -EINVAL, "%s device isn't on I2C bus\n", type);
> > + }
>
> Personally I'd skip checking for the "i2c" node name. Just rely on
> of_get_i2c_adapter_by_node() returning an error.
>
> Oh, I guess you have this because you need to tell the difference
> between -EINVAL and -EPROBE_DEFER? It feels like instead you could use
> the firmware node to lookup a device more generically. If a device
> isn't associated with the node at all then you return -EPROBE_DEFER.
> Otherwise if it's associated but not an i2c device then you return
> -EINVAL. I guess maybe it doesn't make a huge difference, but it
> somehow feels less fragile than relying on the node name being "i2c".
> Maybe I just haven't had enough DT Kool-Aid.
The current way it's written is to do the device tree structure checks
first before doing any driver model access. Also it needs to tell
(as you mentioned later) if the i2c adapter is "disabled" or just not
probed yet.
> One thing this made me wonder is if of_get_i2c_adapter_by_node() is
> race free anyway. Can't that return you a device that hasn't finished
> probing yet? I see:
>
> - i2c_register_adapter()
> -- device_register()
> --- device_add()
> ---- bus_add_device()
> ---- bus_probe_device()
>
> As soon as bus_add_device() is called then it will be in
> "klist_devices" and I believe i2c_find_device_by_fwnode() will be able
> to find it. However, it hasn't necessarily been probed yet. I think
> that means calling i2c_smbus_xfer() on it might not work yet...
It does look like there's a small window between the device_register()
and when the i2c adapter is ready, i.e. can't bail out on an error.
I guess it needs either a flag or some reordering of the code.
It looks like Wolfram will be at OSS JP. I'll try and have a chat about
the whole thing.
> One last thing is that you should check to make sure that the i2c
> adapter is not marked "disabled". ...otherwise I think you'd end up
> constantly trying again and again...
Yeah. I added a check for that as well.
> > + for_each_child_of_node(i2c_node, node) {
> > + if (!of_node_name_prefix(node, type))
> > + continue;
> > + if (of_device_is_available(node)) {
> > + /*
> > + * Device tree has component already enabled. Either the
> > + * device tree isn't supported or we already probed once.
>
> I guess the "already probed once" is somewhat expected if more than
> one type of second source component is defined and we end up deferring
> the second one? We don't undo the resolution of the first one and
> probably don't keep track of the fact that it succeeded?
That's right.
> Probably should be added to the function comments that this is an
> expected/normal case?
Added to the Return: section of the kernel-doc.
> > + */
> > + of_node_put(node);
> > + of_node_put(i2c_node);
> > + return 0;
> > + }
> > + }
> > +
> > + i2c = of_get_i2c_adapter_by_node(i2c_node);
> > + if (!i2c) {
> > + of_node_put(i2c_node);
> > + return dev_err_probe(dev, -EPROBE_DEFER, "Couldn't get I2C adapter\n");
> > + }
> > +
> > + ret = 0;
>
> Why init ret to 0?
It was requested by Andy. I believe the reason was having the initial
value used for bailout closer was better.
> > + for_each_child_of_node(i2c_node, node) {
> > + union i2c_smbus_data data;
> > + u32 addr;
> > +
> > + if (!of_node_name_prefix(node, type))
> > + continue;
> > + if (of_property_read_u32(node, "reg", &addr))
> > + continue;
> > + if (i2c_smbus_xfer(i2c, addr, 0, I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &data) < 0)
>
> I'd be tempted to say that the caller should be able to pass in a
> function pointer here so they could use an alternative method to probe
> instead of i2c_smbus_xfer(), though you'd want to make it easy to
> default to i2c_smbus_xfer(). I could imagine someone might need a
> different way to probe. For instance if you had two touchscreens both
> at the same "reg" but that had different "hid-descr-addr" then this
> could be important.
I'd say the only specific probable type is hid-i2c. And that could be
generic enough that we could incorporate it here if we wanted. However
I think we want to keep the initial version a bit simpler.
> > + continue;
> > +
>
> Put the "break" right here. You've found the device and that was the
> point of the loop.
In its place we'd have an if (node) { <enable node> } block. I guess it
makes it easier to read still?
> Once you do that then the error handling makes a little more sense. It
> was weird that the error handling was jumped through from inside the
> loop...
>
>
> > + dev_info(dev, "Enabling %pOF\n", node);
> > +
> > + ocs = kzalloc(sizeof(*ocs), GFP_KERNEL);
> > + if (!ocs) {
> > + ret = -ENOMEM;
> > + goto err_put_node;
>
> I think this error path (and some others) miss "i2c_put_adapter()" and
> "of_node_put(i2c_node)"
Right. I'll check.
Thanks
ChenYu
^ permalink raw reply
* [PATCH] HID: nintendo: add support for nso controllers
From: Ryan McClelland @ 2023-12-04 8:17 UTC (permalink / raw)
To: linux-input; +Cc: djogorchock, benjamin.tissoires, jikos, Ryan McClelland
This adds support for the nintendo switch online controllers which
include the SNES, Genesis, and N64 Controllers.
As each nso controller only implements a subset of what a pro
controller can do. Each of these 'features' were broken up in to
seperate functions which include right stick, left stick, imu, and
dpad and depending on the controller type that it is, it will call
the supported functions appropriately.
Each controller now has a struct which maps the bit within the hid
in report to a button.
The name given to the device now comes directly from the hid
device name rather than looking up a predefined string.
Signed-off-by: Ryan McClelland <rymcclel@gmail.com>
---
drivers/hid/Kconfig | 13 +-
drivers/hid/hid-ids.h | 5 +-
drivers/hid/hid-nintendo.c | 897 ++++++++++++++++++++++++++-----------
3 files changed, 649 insertions(+), 266 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ce74af79657..347c284fb27e 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -761,14 +761,15 @@ config HID_MULTITOUCH
module will be called hid-multitouch.
config HID_NINTENDO
- tristate "Nintendo Joy-Con and Pro Controller support"
+ tristate "Nintendo Joy-Con, NSO, and Pro Controller support"
depends on NEW_LEDS
depends on LEDS_CLASS
select POWER_SUPPLY
help
- Adds support for the Nintendo Switch Joy-Cons and Pro Controller.
+ Adds support for the Nintendo Switch Joy-Cons, NSO, Pro Controller.
All controllers support bluetooth, and the Pro Controller also supports
- its USB mode.
+ its USB mode. This also includes support for the Nintendo Switch Online
+ Controllers which include the Genesis, SNES, and N64 controllers.
To compile this driver as a module, choose M here: the
module will be called hid-nintendo.
@@ -779,9 +780,9 @@ config NINTENDO_FF
select INPUT_FF_MEMLESS
help
Say Y here if you have a Nintendo Switch controller and want to enable
- force feedback support for it. This works for both joy-cons and the pro
- controller. For the pro controller, both rumble motors can be controlled
- individually.
+ force feedback support for it. This works for both joy-cons, the pro
+ controller, and the NSO N64 controller. For the pro controller, both
+ rumble motors can be controlled individually.
config HID_NTI
tristate "NTI keyboard adapters"
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c6e4e0d1f214..a90aa3c31dd0 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -986,7 +986,10 @@
#define USB_DEVICE_ID_NINTENDO_JOYCONL 0x2006
#define USB_DEVICE_ID_NINTENDO_JOYCONR 0x2007
#define USB_DEVICE_ID_NINTENDO_PROCON 0x2009
-#define USB_DEVICE_ID_NINTENDO_CHRGGRIP 0x200E
+#define USB_DEVICE_ID_NINTENDO_CHRGGRIP 0x200e
+#define USB_DEVICE_ID_NINTENDO_SNESCON 0x2017
+#define USB_DEVICE_ID_NINTENDO_GENCON 0x201e
+#define USB_DEVICE_ID_NINTENDO_N64CON 0x2019
#define USB_VENDOR_ID_NOVATEK 0x0603
#define USB_DEVICE_ID_NOVATEK_PCT 0x0600
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 138f154fecef..47af111ef3a2 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -3,6 +3,9 @@
* HID driver for Nintendo Switch Joy-Cons and Pro Controllers
*
* Copyright (c) 2019-2021 Daniel J. Ogorchock <djogorchock@gmail.com>
+ * Portions Copyright (c) 2020 Nadia Holmquist Pedersen <nadia@nhp.sh>
+ * Copyright (c) 2022 Emily Strickland <linux@emily.st>
+ * Copyright (c) 2023 Ryan McClelland <rymcclel@gmail.com>
*
* The following resources/projects were referenced for this driver:
* https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
@@ -17,6 +20,9 @@
* This driver supports the Nintendo Switch Joy-Cons and Pro Controllers. The
* Pro Controllers can either be used over USB or Bluetooth.
*
+ * This driver also incorporates support for Nintendo Switch Online controllers
+ * for the NES, SNES, Sega Genesis, and N64.
+ *
* The driver will retrieve the factory calibration info from the controllers,
* so little to no user calibration should be required.
*
@@ -305,9 +311,14 @@ enum joycon_ctlr_state {
/* Controller type received as part of device info */
enum joycon_ctlr_type {
- JOYCON_CTLR_TYPE_JCL = 0x01,
- JOYCON_CTLR_TYPE_JCR = 0x02,
- JOYCON_CTLR_TYPE_PRO = 0x03,
+ JOYCON_CTLR_TYPE_JCL = 0x01,
+ JOYCON_CTLR_TYPE_JCR = 0x02,
+ JOYCON_CTLR_TYPE_PRO = 0x03,
+ JOYCON_CTLR_TYPE_NESL = 0x09,
+ JOYCON_CTLR_TYPE_NESR = 0x0A,
+ JOYCON_CTLR_TYPE_SNES = 0x0B,
+ JOYCON_CTLR_TYPE_GEN = 0x0D,
+ JOYCON_CTLR_TYPE_N64 = 0x0C,
};
struct joycon_stick_cal {
@@ -348,6 +359,137 @@ static const u32 JC_BTN_SL_L = BIT(21);
static const u32 JC_BTN_L = BIT(22);
static const u32 JC_BTN_ZL = BIT(23);
+struct joycon_ctlr_button_mapping {
+ u32 code;
+ u32 bit;
+};
+
+/*
+ * D-pad is configured as buttons for the left Joy-Con only!
+ */
+static const struct joycon_ctlr_button_mapping left_joycon_button_mappings[] = {
+ { BTN_TL, JC_BTN_L, },
+ { BTN_TL2, JC_BTN_ZL, },
+ { BTN_SELECT, JC_BTN_MINUS, },
+ { BTN_THUMBL, JC_BTN_LSTICK, },
+ { BTN_DPAD_UP, JC_BTN_UP, },
+ { BTN_DPAD_DOWN, JC_BTN_DOWN, },
+ { BTN_DPAD_LEFT, JC_BTN_LEFT, },
+ { BTN_DPAD_RIGHT, JC_BTN_RIGHT, },
+ { BTN_Z, JC_BTN_CAP, },
+ { /* sentinel */ },
+};
+
+/*
+ * The unused *right*-side triggers become the SL/SR triggers for the *left*
+ * Joy-Con, if and only if we're not using a charging grip.
+ */
+static const struct joycon_ctlr_button_mapping left_joycon_s_button_mappings[] = {
+ { BTN_TR, JC_BTN_SL_L, },
+ { BTN_TR2, JC_BTN_SR_L, },
+ { /* sentinel */ },
+};
+
+static const struct joycon_ctlr_button_mapping right_joycon_button_mappings[] = {
+ { BTN_EAST, JC_BTN_A, },
+ { BTN_SOUTH, JC_BTN_B, },
+ { BTN_NORTH, JC_BTN_X, },
+ { BTN_WEST, JC_BTN_Y, },
+ { BTN_TR, JC_BTN_R, },
+ { BTN_TR2, JC_BTN_ZR, },
+ { BTN_START, JC_BTN_PLUS, },
+ { BTN_THUMBR, JC_BTN_RSTICK, },
+ { BTN_MODE, JC_BTN_HOME, },
+ { /* sentinel */ },
+};
+
+/*
+ * The unused *left*-side triggers become the SL/SR triggers for the *right*
+ * Joy-Con, if and only if we're not using a charging grip.
+ */
+static const struct joycon_ctlr_button_mapping right_joycon_s_button_mappings[] = {
+ { BTN_TL, JC_BTN_SL_R, },
+ { BTN_TL2, JC_BTN_SR_R, },
+ { /* sentinel */ },
+};
+
+static const struct joycon_ctlr_button_mapping procon_button_mappings[] = {
+ { BTN_EAST, JC_BTN_A, },
+ { BTN_SOUTH, JC_BTN_B, },
+ { BTN_NORTH, JC_BTN_X, },
+ { BTN_WEST, JC_BTN_Y, },
+ { BTN_TL, JC_BTN_L, },
+ { BTN_TR, JC_BTN_R, },
+ { BTN_TL2, JC_BTN_ZL, },
+ { BTN_TR2, JC_BTN_ZR, },
+ { BTN_SELECT, JC_BTN_MINUS, },
+ { BTN_START, JC_BTN_PLUS, },
+ { BTN_THUMBL, JC_BTN_LSTICK, },
+ { BTN_THUMBR, JC_BTN_RSTICK, },
+ { BTN_MODE, JC_BTN_HOME, },
+ { BTN_Z, JC_BTN_CAP, },
+ { /* sentinel */ },
+};
+
+static const struct joycon_ctlr_button_mapping nescon_button_mappings[] = {
+ { BTN_SOUTH, JC_BTN_A, },
+ { BTN_EAST, JC_BTN_B, },
+ { BTN_TL, JC_BTN_L, },
+ { BTN_TR, JC_BTN_R, },
+ { BTN_SELECT, JC_BTN_MINUS, },
+ { BTN_START, JC_BTN_PLUS, },
+ { /* sentinel */ },
+};
+
+static const struct joycon_ctlr_button_mapping snescon_button_mappings[] = {
+ { BTN_EAST, JC_BTN_A, },
+ { BTN_SOUTH, JC_BTN_B, },
+ { BTN_NORTH, JC_BTN_X, },
+ { BTN_WEST, JC_BTN_Y, },
+ { BTN_TL, JC_BTN_L, },
+ { BTN_TR, JC_BTN_R, },
+ { BTN_TL2, JC_BTN_ZL, },
+ { BTN_TR2, JC_BTN_ZR, },
+ { BTN_SELECT, JC_BTN_MINUS, },
+ { BTN_START, JC_BTN_PLUS, },
+ { /* sentinel */ },
+};
+
+/*
+ * "A", "B", and "C" are mapped positionally, rather than by label (e.g., "A"
+ * gets assigned to BTN_EAST instead of BTN_A).
+ */
+static const struct joycon_ctlr_button_mapping gencon_button_mappings[] = {
+ { BTN_SOUTH, JC_BTN_A, },
+ { BTN_EAST, JC_BTN_B, },
+ { BTN_WEST, JC_BTN_R, },
+ { BTN_SELECT, JC_BTN_ZR, },
+ { BTN_START, JC_BTN_PLUS, },
+ { BTN_MODE, JC_BTN_HOME, },
+ { BTN_Z, JC_BTN_CAP, },
+ { /* sentinel */ },
+};
+
+/*
+ * N64's C buttons get assigned to d-pad directions and registered as buttons.
+ */
+static const struct joycon_ctlr_button_mapping n64con_button_mappings[] = {
+ { BTN_A, JC_BTN_A, },
+ { BTN_B, JC_BTN_B, },
+ { BTN_TL2, JC_BTN_ZL, }, /* Z */
+ { BTN_TL, JC_BTN_L, },
+ { BTN_TR, JC_BTN_R, },
+ { BTN_TR2, JC_BTN_LSTICK, }, /* ZR */
+ { BTN_START, JC_BTN_PLUS, },
+ { BTN_FORWARD, JC_BTN_Y, }, /* C UP */
+ { BTN_BACK, JC_BTN_ZR, }, /* C DOWN */
+ { BTN_LEFT, JC_BTN_X, }, /* C LEFT */
+ { BTN_RIGHT, JC_BTN_MINUS, }, /* C RIGHT */
+ { BTN_MODE, JC_BTN_HOME, },
+ { BTN_Z, JC_BTN_CAP, },
+ { /* sentinel */ },
+};
+
enum joycon_msg_type {
JOYCON_MSG_TYPE_NONE,
JOYCON_MSG_TYPE_USB,
@@ -506,13 +648,182 @@ struct joycon_ctlr {
/* Does this controller have inputs associated with left joycon? */
#define jc_type_has_left(ctlr) \
(ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCL || \
- ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO)
+ ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO || \
+ ctlr->ctlr_type == JOYCON_CTLR_TYPE_N64)
/* Does this controller have inputs associated with right joycon? */
#define jc_type_has_right(ctlr) \
(ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCR || \
ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO)
+
+/*
+ * Controller device helpers
+ *
+ * These look at the device ID known to the HID subsystem to identify a device,
+ * but take caution: some NSO devices lie about themselves (NES Joy-Cons and
+ * Sega Genesis controller). See type helpers below.
+ *
+ * These helpers are most useful early during the HID probe or in conjunction
+ * with the capability helpers below.
+ */
+static inline bool joycon_device_is_left_joycon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONL;
+}
+
+static inline bool joycon_device_is_right_joycon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONR;
+}
+
+static inline bool joycon_device_is_procon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_PROCON;
+}
+
+static inline bool joycon_device_is_chrggrip(struct joycon_ctlr *ctlr)
+{
+ return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_CHRGGRIP;
+}
+
+static inline bool joycon_device_is_snescon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_SNESCON;
+}
+
+static inline bool joycon_device_is_gencon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_GENCON;
+}
+
+static inline bool joycon_device_is_n64con(struct joycon_ctlr *ctlr)
+{
+ return ctlr->hdev->product == USB_DEVICE_ID_NINTENDO_N64CON;
+}
+
+static inline bool joycon_device_has_usb(struct joycon_ctlr *ctlr)
+{
+ return joycon_device_is_procon(ctlr) ||
+ joycon_device_is_chrggrip(ctlr) ||
+ joycon_device_is_snescon(ctlr) ||
+ joycon_device_is_gencon(ctlr) ||
+ joycon_device_is_n64con(ctlr);
+}
+
+/*
+ * Controller type helpers
+ *
+ * These are slightly different than the device-ID-based helpers above. They are
+ * generally more reliable, since they can distinguish between, e.g., Genesis
+ * versus SNES, or NES Joy-Cons versus regular Switch Joy-Cons. They're most
+ * useful for reporting available inputs. For other kinds of distinctions, see
+ * the capability helpers below.
+ *
+ * They have two major drawbacks: (1) they're not available until after we set
+ * the reporting method and then request the device info; (2) they can't
+ * distinguish all controllers (like the Charging Grip from the Pro controller.)
+ */
+static inline bool joycon_type_is_left_joycon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCL;
+}
+
+static inline bool joycon_type_is_right_joycon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->ctlr_type == JOYCON_CTLR_TYPE_JCR;
+}
+
+static inline bool joycon_type_is_procon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO;
+}
+
+static inline bool joycon_type_is_snescon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->ctlr_type == JOYCON_CTLR_TYPE_SNES;
+}
+
+static inline bool joycon_type_is_gencon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->ctlr_type == JOYCON_CTLR_TYPE_GEN;
+}
+
+static inline bool joycon_type_is_n64con(struct joycon_ctlr *ctlr)
+{
+ return ctlr->ctlr_type == JOYCON_CTLR_TYPE_N64;
+}
+
+static inline bool joycon_type_is_left_nescon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->ctlr_type == JOYCON_CTLR_TYPE_NESL;
+}
+
+static inline bool joycon_type_is_right_nescon(struct joycon_ctlr *ctlr)
+{
+ return ctlr->ctlr_type == JOYCON_CTLR_TYPE_NESR;
+}
+
+static inline bool joycon_type_has_left_controls(struct joycon_ctlr *ctlr)
+{
+ return joycon_type_is_left_joycon(ctlr) ||
+ joycon_type_is_procon(ctlr);
+}
+
+static inline bool joycon_type_has_right_controls(struct joycon_ctlr *ctlr)
+{
+ return joycon_type_is_right_joycon(ctlr) ||
+ joycon_type_is_procon(ctlr);
+}
+
+static inline bool joycon_type_is_any_joycon(struct joycon_ctlr *ctlr)
+{
+ return joycon_type_is_left_joycon(ctlr) ||
+ joycon_type_is_right_joycon(ctlr) ||
+ joycon_device_is_chrggrip(ctlr);
+}
+
+static inline bool joycon_type_is_any_nescon(struct joycon_ctlr *ctlr)
+{
+ return joycon_type_is_left_nescon(ctlr) ||
+ joycon_type_is_right_nescon(ctlr);
+}
+
+/*
+ * Controller capability helpers
+ *
+ * These helpers combine the use of the helpers above to detect certain
+ * capabilities during initialization. They are always accurate but (since they
+ * use type helpers) cannot be used early in the HID probe.
+ */
+static inline bool joycon_has_imu(struct joycon_ctlr *ctlr)
+{
+ return joycon_device_is_chrggrip(ctlr) ||
+ joycon_type_is_any_joycon(ctlr) ||
+ joycon_type_is_procon(ctlr);
+}
+
+static inline bool joycon_has_joysticks(struct joycon_ctlr *ctlr)
+{
+ return joycon_device_is_chrggrip(ctlr) ||
+ joycon_type_is_any_joycon(ctlr) ||
+ joycon_type_is_procon(ctlr) ||
+ joycon_type_is_n64con(ctlr);
+}
+
+static inline bool joycon_has_rumble(struct joycon_ctlr *ctlr)
+{
+ return joycon_device_is_chrggrip(ctlr) ||
+ joycon_type_is_any_joycon(ctlr) ||
+ joycon_type_is_procon(ctlr) ||
+ joycon_type_is_n64con(ctlr);
+}
+
+static inline bool joycon_using_usb(struct joycon_ctlr *ctlr)
+{
+ return ctlr->hdev->bus == BUS_USB;
+}
+
static int __joycon_hid_send(struct hid_device *hdev, u8 *data, size_t len)
{
u8 *buf;
@@ -1283,15 +1594,10 @@ static void joycon_parse_imu_report(struct joycon_ctlr *ctlr,
}
}
-static void joycon_parse_report(struct joycon_ctlr *ctlr,
- struct joycon_input_report *rep)
+static void joycon_handle_rumble_report(struct joycon_ctlr *ctlr, struct joycon_input_report *rep)
{
- struct input_dev *dev = ctlr->input;
unsigned long flags;
- u8 tmp;
- u32 btns;
unsigned long msecs = jiffies_to_msecs(jiffies);
- unsigned long report_delta_ms = msecs - ctlr->last_input_report_msecs;
spin_lock_irqsave(&ctlr->lock, flags);
if (IS_ENABLED(CONFIG_NINTENDO_FF) && rep->vibrator_report &&
@@ -1310,11 +1616,21 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
queue_work(ctlr->rumble_queue, &ctlr->rumble_worker);
}
- /* Parse the battery status */
+ spin_unlock_irqrestore(&ctlr->lock, flags);
+}
+
+static void joycon_parse_battery_status(struct joycon_ctlr *ctlr, struct joycon_input_report *rep)
+{
+ u8 tmp;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ctlr->lock, flags);
+
tmp = rep->bat_con;
ctlr->host_powered = tmp & BIT(0);
ctlr->battery_charging = tmp & BIT(4);
tmp = tmp >> 5;
+
switch (tmp) {
case 0: /* empty */
ctlr->battery_capacity = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL;
@@ -1336,102 +1652,121 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
hid_warn(ctlr->hdev, "Invalid battery status\n");
break;
}
+
spin_unlock_irqrestore(&ctlr->lock, flags);
+}
- /* Parse the buttons and sticks */
- btns = hid_field_extract(ctlr->hdev, rep->button_status, 0, 24);
-
- if (jc_type_has_left(ctlr)) {
- u16 raw_x;
- u16 raw_y;
- s32 x;
- s32 y;
-
- /* get raw stick values */
- raw_x = hid_field_extract(ctlr->hdev, rep->left_stick, 0, 12);
- raw_y = hid_field_extract(ctlr->hdev,
- rep->left_stick + 1, 4, 12);
- /* map the stick values */
- x = joycon_map_stick_val(&ctlr->left_stick_cal_x, raw_x);
- y = -joycon_map_stick_val(&ctlr->left_stick_cal_y, raw_y);
- /* report sticks */
- input_report_abs(dev, ABS_X, x);
- input_report_abs(dev, ABS_Y, y);
-
- /* report buttons */
- input_report_key(dev, BTN_TL, btns & JC_BTN_L);
- input_report_key(dev, BTN_TL2, btns & JC_BTN_ZL);
- input_report_key(dev, BTN_SELECT, btns & JC_BTN_MINUS);
- input_report_key(dev, BTN_THUMBL, btns & JC_BTN_LSTICK);
- input_report_key(dev, BTN_Z, btns & JC_BTN_CAP);
-
- if (jc_type_is_joycon(ctlr)) {
- /* Report the S buttons as the non-existent triggers */
- input_report_key(dev, BTN_TR, btns & JC_BTN_SL_L);
- input_report_key(dev, BTN_TR2, btns & JC_BTN_SR_L);
-
- /* Report d-pad as digital buttons for the joy-cons */
- input_report_key(dev, BTN_DPAD_DOWN,
- btns & JC_BTN_DOWN);
- input_report_key(dev, BTN_DPAD_UP, btns & JC_BTN_UP);
- input_report_key(dev, BTN_DPAD_RIGHT,
- btns & JC_BTN_RIGHT);
- input_report_key(dev, BTN_DPAD_LEFT,
- btns & JC_BTN_LEFT);
- } else {
- int hatx = 0;
- int haty = 0;
-
- /* d-pad x */
- if (btns & JC_BTN_LEFT)
- hatx = -1;
- else if (btns & JC_BTN_RIGHT)
- hatx = 1;
- input_report_abs(dev, ABS_HAT0X, hatx);
-
- /* d-pad y */
- if (btns & JC_BTN_UP)
- haty = -1;
- else if (btns & JC_BTN_DOWN)
- haty = 1;
- input_report_abs(dev, ABS_HAT0Y, haty);
- }
- }
- if (jc_type_has_right(ctlr)) {
- u16 raw_x;
- u16 raw_y;
- s32 x;
- s32 y;
-
- /* get raw stick values */
- raw_x = hid_field_extract(ctlr->hdev, rep->right_stick, 0, 12);
- raw_y = hid_field_extract(ctlr->hdev,
- rep->right_stick + 1, 4, 12);
- /* map stick values */
- x = joycon_map_stick_val(&ctlr->right_stick_cal_x, raw_x);
- y = -joycon_map_stick_val(&ctlr->right_stick_cal_y, raw_y);
- /* report sticks */
- input_report_abs(dev, ABS_RX, x);
- input_report_abs(dev, ABS_RY, y);
-
- /* report buttons */
- input_report_key(dev, BTN_TR, btns & JC_BTN_R);
- input_report_key(dev, BTN_TR2, btns & JC_BTN_ZR);
- if (jc_type_is_joycon(ctlr)) {
- /* Report the S buttons as the non-existent triggers */
- input_report_key(dev, BTN_TL, btns & JC_BTN_SL_R);
- input_report_key(dev, BTN_TL2, btns & JC_BTN_SR_R);
- }
- input_report_key(dev, BTN_START, btns & JC_BTN_PLUS);
- input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK);
- input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME);
- input_report_key(dev, BTN_WEST, btns & JC_BTN_Y);
- input_report_key(dev, BTN_NORTH, btns & JC_BTN_X);
- input_report_key(dev, BTN_EAST, btns & JC_BTN_A);
- input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B);
+static void joycon_report_left_stick(struct joycon_ctlr *ctlr,
+ struct joycon_input_report *rep)
+{
+ u16 raw_x;
+ u16 raw_y;
+ s32 x;
+ s32 y;
+
+ raw_x = hid_field_extract(ctlr->hdev, rep->left_stick, 0, 12);
+ raw_y = hid_field_extract(ctlr->hdev, rep->left_stick + 1, 4, 12);
+
+ x = joycon_map_stick_val(&ctlr->left_stick_cal_x, raw_x);
+ y = -joycon_map_stick_val(&ctlr->left_stick_cal_y, raw_y);
+
+ input_report_abs(ctlr->input, ABS_X, x);
+ input_report_abs(ctlr->input, ABS_Y, y);
+}
+
+static void joycon_report_right_stick(struct joycon_ctlr *ctlr,
+ struct joycon_input_report *rep)
+{
+ u16 raw_x;
+ u16 raw_y;
+ s32 x;
+ s32 y;
+
+ raw_x = hid_field_extract(ctlr->hdev, rep->right_stick, 0, 12);
+ raw_y = hid_field_extract(ctlr->hdev, rep->right_stick + 1, 4, 12);
+
+ x = joycon_map_stick_val(&ctlr->right_stick_cal_x, raw_x);
+ y = -joycon_map_stick_val(&ctlr->right_stick_cal_y, raw_y);
+
+ input_report_abs(ctlr->input, ABS_RX, x);
+ input_report_abs(ctlr->input, ABS_RY, y);
+}
+
+static void joycon_report_dpad(struct joycon_ctlr *ctlr,
+ struct joycon_input_report *rep)
+{
+ int hatx = 0;
+ int haty = 0;
+ u32 btns = hid_field_extract(ctlr->hdev, rep->button_status, 0, 24);
+
+ if (btns & JC_BTN_LEFT)
+ hatx = -1;
+ else if (btns & JC_BTN_RIGHT)
+ hatx = 1;
+
+ if (btns & JC_BTN_UP)
+ haty = -1;
+ else if (btns & JC_BTN_DOWN)
+ haty = 1;
+
+ input_report_abs(ctlr->input, ABS_HAT0X, hatx);
+ input_report_abs(ctlr->input, ABS_HAT0Y, haty);
+}
+
+static void joycon_report_buttons(struct joycon_ctlr *ctlr,
+ struct joycon_input_report *rep,
+ const struct joycon_ctlr_button_mapping button_mappings[])
+{
+ const struct joycon_ctlr_button_mapping *button;
+ u32 status = hid_field_extract(ctlr->hdev, rep->button_status, 0, 24);
+
+ for (button = button_mappings; button->code; button++)
+ input_report_key(ctlr->input, button->code, status & button->bit);
+}
+
+static void joycon_parse_report(struct joycon_ctlr *ctlr,
+ struct joycon_input_report *rep)
+{
+ unsigned long flags;
+ unsigned long msecs = jiffies_to_msecs(jiffies);
+ unsigned long report_delta_ms = msecs - ctlr->last_input_report_msecs;
+
+ if (joycon_has_rumble(ctlr))
+ joycon_handle_rumble_report(ctlr, rep);
+
+ joycon_parse_battery_status(ctlr, rep);
+
+ if (joycon_type_is_left_joycon(ctlr)) {
+ joycon_report_left_stick(ctlr, rep);
+ joycon_report_buttons(ctlr, rep, left_joycon_button_mappings);
+ if (!joycon_device_is_chrggrip(ctlr))
+ joycon_report_buttons(ctlr, rep, left_joycon_s_button_mappings);
+ } else if (joycon_type_is_right_joycon(ctlr)) {
+ joycon_report_right_stick(ctlr, rep);
+ joycon_report_buttons(ctlr, rep, right_joycon_button_mappings);
+ if (!joycon_device_is_chrggrip(ctlr))
+ joycon_report_buttons(ctlr, rep, right_joycon_s_button_mappings);
+ } else if (joycon_type_is_procon(ctlr)) {
+ joycon_report_left_stick(ctlr, rep);
+ joycon_report_right_stick(ctlr, rep);
+ joycon_report_dpad(ctlr, rep);
+ joycon_report_buttons(ctlr, rep, procon_button_mappings);
+ } else if (joycon_type_is_any_nescon(ctlr)) {
+ joycon_report_dpad(ctlr, rep);
+ joycon_report_buttons(ctlr, rep, nescon_button_mappings);
+ } else if (joycon_type_is_snescon(ctlr)) {
+ joycon_report_dpad(ctlr, rep);
+ joycon_report_buttons(ctlr, rep, snescon_button_mappings);
+ } else if (joycon_type_is_gencon(ctlr)) {
+ joycon_report_dpad(ctlr, rep);
+ joycon_report_buttons(ctlr, rep, gencon_button_mappings);
+ } else if (joycon_type_is_n64con(ctlr)) {
+ joycon_report_left_stick(ctlr, rep);
+ joycon_report_dpad(ctlr, rep);
+ joycon_report_buttons(ctlr, rep, n64con_button_mappings);
}
- input_sync(dev);
+ input_sync(ctlr->input);
spin_lock_irqsave(&ctlr->lock, flags);
ctlr->last_input_report_msecs = msecs;
@@ -1471,7 +1806,7 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
}
/* parse IMU data if present */
- if (rep->id == JC_INPUT_IMU_DATA)
+ if ((rep->id == JC_INPUT_IMU_DATA) && joycon_has_imu(ctlr))
joycon_parse_imu_report(ctlr, rep);
}
@@ -1684,123 +2019,65 @@ static int joycon_play_effect(struct input_dev *dev, void *data,
}
#endif /* IS_ENABLED(CONFIG_NINTENDO_FF) */
-static const unsigned int joycon_button_inputs_l[] = {
- BTN_SELECT, BTN_Z, BTN_THUMBL,
- BTN_TL, BTN_TL2,
- 0 /* 0 signals end of array */
-};
-
-static const unsigned int joycon_button_inputs_r[] = {
- BTN_START, BTN_MODE, BTN_THUMBR,
- BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST,
- BTN_TR, BTN_TR2,
- 0 /* 0 signals end of array */
-};
-
-/* We report joy-con d-pad inputs as buttons and pro controller as a hat. */
-static const unsigned int joycon_dpad_inputs_jc[] = {
- BTN_DPAD_UP, BTN_DPAD_DOWN, BTN_DPAD_LEFT, BTN_DPAD_RIGHT,
- 0 /* 0 signals end of array */
-};
-
-static int joycon_input_create(struct joycon_ctlr *ctlr)
+static void joycon_config_left_stick(struct input_dev *idev)
{
- struct hid_device *hdev;
- const char *name;
- const char *imu_name;
- int ret;
- int i;
-
- hdev = ctlr->hdev;
+ input_set_abs_params(idev,
+ ABS_X,
+ -JC_MAX_STICK_MAG,
+ JC_MAX_STICK_MAG,
+ JC_STICK_FUZZ,
+ JC_STICK_FLAT);
+ input_set_abs_params(idev,
+ ABS_Y,
+ -JC_MAX_STICK_MAG,
+ JC_MAX_STICK_MAG,
+ JC_STICK_FUZZ,
+ JC_STICK_FLAT);
+}
- switch (hdev->product) {
- case USB_DEVICE_ID_NINTENDO_PROCON:
- name = "Nintendo Switch Pro Controller";
- imu_name = "Nintendo Switch Pro Controller IMU";
- break;
- case USB_DEVICE_ID_NINTENDO_CHRGGRIP:
- if (jc_type_has_left(ctlr)) {
- name = "Nintendo Switch Left Joy-Con (Grip)";
- imu_name = "Nintendo Switch Left Joy-Con IMU (Grip)";
- } else {
- name = "Nintendo Switch Right Joy-Con (Grip)";
- imu_name = "Nintendo Switch Right Joy-Con IMU (Grip)";
- }
- break;
- case USB_DEVICE_ID_NINTENDO_JOYCONL:
- name = "Nintendo Switch Left Joy-Con";
- imu_name = "Nintendo Switch Left Joy-Con IMU";
- break;
- case USB_DEVICE_ID_NINTENDO_JOYCONR:
- name = "Nintendo Switch Right Joy-Con";
- imu_name = "Nintendo Switch Right Joy-Con IMU";
- break;
- default: /* Should be impossible */
- hid_err(hdev, "Invalid hid product\n");
- return -EINVAL;
- }
+static void joycon_config_right_stick(struct input_dev *idev)
+{
+ input_set_abs_params(idev,
+ ABS_RX,
+ -JC_MAX_STICK_MAG,
+ JC_MAX_STICK_MAG,
+ JC_STICK_FUZZ,
+ JC_STICK_FLAT);
+ input_set_abs_params(idev,
+ ABS_RY,
+ -JC_MAX_STICK_MAG,
+ JC_MAX_STICK_MAG,
+ JC_STICK_FUZZ,
+ JC_STICK_FLAT);
+}
- ctlr->input = devm_input_allocate_device(&hdev->dev);
- if (!ctlr->input)
- return -ENOMEM;
- ctlr->input->id.bustype = hdev->bus;
- ctlr->input->id.vendor = hdev->vendor;
- ctlr->input->id.product = hdev->product;
- ctlr->input->id.version = hdev->version;
- ctlr->input->uniq = ctlr->mac_addr_str;
- ctlr->input->name = name;
- ctlr->input->phys = hdev->phys;
- input_set_drvdata(ctlr->input, ctlr);
+static void joycon_config_dpad(struct input_dev *idev)
+{
+ input_set_abs_params(idev,
+ ABS_HAT0X,
+ -JC_MAX_DPAD_MAG,
+ JC_MAX_DPAD_MAG,
+ JC_DPAD_FUZZ,
+ JC_DPAD_FLAT);
+ input_set_abs_params(idev,
+ ABS_HAT0Y,
+ -JC_MAX_DPAD_MAG,
+ JC_MAX_DPAD_MAG,
+ JC_DPAD_FUZZ,
+ JC_DPAD_FLAT);
+}
- /* set up sticks and buttons */
- if (jc_type_has_left(ctlr)) {
- input_set_abs_params(ctlr->input, ABS_X,
- -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
- JC_STICK_FUZZ, JC_STICK_FLAT);
- input_set_abs_params(ctlr->input, ABS_Y,
- -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
- JC_STICK_FUZZ, JC_STICK_FLAT);
-
- for (i = 0; joycon_button_inputs_l[i] > 0; i++)
- input_set_capability(ctlr->input, EV_KEY,
- joycon_button_inputs_l[i]);
-
- /* configure d-pad differently for joy-con vs pro controller */
- if (hdev->product != USB_DEVICE_ID_NINTENDO_PROCON) {
- for (i = 0; joycon_dpad_inputs_jc[i] > 0; i++)
- input_set_capability(ctlr->input, EV_KEY,
- joycon_dpad_inputs_jc[i]);
- } else {
- input_set_abs_params(ctlr->input, ABS_HAT0X,
- -JC_MAX_DPAD_MAG, JC_MAX_DPAD_MAG,
- JC_DPAD_FUZZ, JC_DPAD_FLAT);
- input_set_abs_params(ctlr->input, ABS_HAT0Y,
- -JC_MAX_DPAD_MAG, JC_MAX_DPAD_MAG,
- JC_DPAD_FUZZ, JC_DPAD_FLAT);
- }
- }
- if (jc_type_has_right(ctlr)) {
- input_set_abs_params(ctlr->input, ABS_RX,
- -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
- JC_STICK_FUZZ, JC_STICK_FLAT);
- input_set_abs_params(ctlr->input, ABS_RY,
- -JC_MAX_STICK_MAG, JC_MAX_STICK_MAG,
- JC_STICK_FUZZ, JC_STICK_FLAT);
-
- for (i = 0; joycon_button_inputs_r[i] > 0; i++)
- input_set_capability(ctlr->input, EV_KEY,
- joycon_button_inputs_r[i]);
- }
+static void joycon_config_buttons(struct input_dev *idev,
+ const struct joycon_ctlr_button_mapping button_mappings[])
+{
+ const struct joycon_ctlr_button_mapping *button;
- /* Let's report joy-con S triggers separately */
- if (hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONL) {
- input_set_capability(ctlr->input, EV_KEY, BTN_TR);
- input_set_capability(ctlr->input, EV_KEY, BTN_TR2);
- } else if (hdev->product == USB_DEVICE_ID_NINTENDO_JOYCONR) {
- input_set_capability(ctlr->input, EV_KEY, BTN_TL);
- input_set_capability(ctlr->input, EV_KEY, BTN_TL2);
- }
+ for (button = button_mappings; button->code; button++)
+ input_set_capability(idev, EV_KEY, button->code);
+}
+static void joycon_config_rumble(struct joycon_ctlr *ctlr)
+{
#if IS_ENABLED(CONFIG_NINTENDO_FF)
/* set up rumble */
input_set_capability(ctlr->input, EV_FF, FF_RUMBLE);
@@ -1813,10 +2090,15 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
joycon_set_rumble(ctlr, 0, 0, false);
ctlr->rumble_msecs = jiffies_to_msecs(jiffies);
#endif
+}
- ret = input_register_device(ctlr->input);
- if (ret)
- return ret;
+static int joycon_imu_input_create(struct joycon_ctlr *ctlr)
+{
+ struct hid_device *hdev;
+ const char *imu_name;
+ int ret;
+
+ hdev = ctlr->hdev;
/* configure the imu input device */
ctlr->imu_input = devm_input_allocate_device(&hdev->dev);
@@ -1828,8 +2110,14 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
ctlr->imu_input->id.product = hdev->product;
ctlr->imu_input->id.version = hdev->version;
ctlr->imu_input->uniq = ctlr->mac_addr_str;
- ctlr->imu_input->name = imu_name;
ctlr->imu_input->phys = hdev->phys;
+
+ imu_name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "%s (IMU)", ctlr->input->name);
+ if (!imu_name)
+ return -ENOMEM;
+
+ ctlr->imu_input->name = imu_name;
+
input_set_drvdata(ctlr->imu_input, ctlr);
/* configure imu axes */
@@ -1871,6 +2159,71 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
return 0;
}
+static int joycon_input_create(struct joycon_ctlr *ctlr)
+{
+ struct hid_device *hdev;
+ int ret;
+
+ hdev = ctlr->hdev;
+
+ ctlr->input = devm_input_allocate_device(&hdev->dev);
+ if (!ctlr->input)
+ return -ENOMEM;
+ ctlr->input->id.bustype = hdev->bus;
+ ctlr->input->id.vendor = hdev->vendor;
+ ctlr->input->id.product = hdev->product;
+ ctlr->input->id.version = hdev->version;
+ ctlr->input->uniq = ctlr->mac_addr_str;
+ ctlr->input->name = hdev->name;
+ ctlr->input->phys = hdev->phys;
+ input_set_drvdata(ctlr->input, ctlr);
+
+ ret = input_register_device(ctlr->input);
+ if (ret)
+ return ret;
+
+ if (joycon_type_is_right_joycon(ctlr)) {
+ joycon_config_right_stick(ctlr->input);
+ joycon_config_buttons(ctlr->input, right_joycon_button_mappings);
+ if (!joycon_device_is_chrggrip(ctlr))
+ joycon_config_buttons(ctlr->input, right_joycon_s_button_mappings);
+ } else if (joycon_type_is_left_joycon(ctlr)) {
+ joycon_config_left_stick(ctlr->input);
+ joycon_config_buttons(ctlr->input, left_joycon_button_mappings);
+ if (!joycon_device_is_chrggrip(ctlr))
+ joycon_config_buttons(ctlr->input, left_joycon_s_button_mappings);
+ } else if (joycon_type_is_procon(ctlr)) {
+ joycon_config_left_stick(ctlr->input);
+ joycon_config_right_stick(ctlr->input);
+ joycon_config_dpad(ctlr->input);
+ joycon_config_buttons(ctlr->input, procon_button_mappings);
+ } else if (joycon_type_is_any_nescon(ctlr)) {
+ joycon_config_dpad(ctlr->input);
+ joycon_config_buttons(ctlr->input, nescon_button_mappings);
+ } else if (joycon_type_is_snescon(ctlr)) {
+ joycon_config_dpad(ctlr->input);
+ joycon_config_buttons(ctlr->input, snescon_button_mappings);
+ } else if (joycon_type_is_gencon(ctlr)) {
+ joycon_config_dpad(ctlr->input);
+ joycon_config_buttons(ctlr->input, gencon_button_mappings);
+ } else if (joycon_type_is_n64con(ctlr)) {
+ joycon_config_dpad(ctlr->input);
+ joycon_config_left_stick(ctlr->input);
+ joycon_config_buttons(ctlr->input, n64con_button_mappings);
+ }
+
+ if (joycon_has_imu(ctlr)) {
+ ret = joycon_imu_input_create(ctlr);
+ if (ret)
+ return ret;
+ }
+
+ if (joycon_has_rumble(ctlr))
+ joycon_config_rumble(ctlr);
+
+ return 0;
+}
+
/* Because the subcommand sets all the leds at once, the brightness argument is ignored */
static int joycon_player_led_brightness_set(struct led_classdev *led,
enum led_brightness brightness)
@@ -2107,9 +2460,7 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
struct joycon_input_report *report;
req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
- mutex_lock(&ctlr->output_mutex);
ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
- mutex_unlock(&ctlr->output_mutex);
if (ret) {
hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
return ret;
@@ -2132,8 +2483,17 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
return -ENOMEM;
hid_info(ctlr->hdev, "controller MAC = %s\n", ctlr->mac_addr_str);
- /* Retrieve the type so we can distinguish for charging grip */
+ /*
+ * Retrieve the type so we can distinguish the controller type
+ * Unfortantly the hdev->product can't always be used due to a ?bug?
+ * with the NSO Genesis controller. Over USB, it will report the
+ * PID as 0x201E, but over bluetooth it will report the PID as 0x2017
+ * which is the same as the NSO SNES controller. This is different from
+ * the rest of the controllers which will report the same PID over USB
+ * and bluetooth.
+ */
ctlr->ctlr_type = report->subcmd_reply.data[2];
+ hid_dbg(ctlr->hdev, "controller type = 0x%02X\n", ctlr->ctlr_type);
return 0;
}
@@ -2145,8 +2505,7 @@ static int joycon_init(struct hid_device *hdev)
mutex_lock(&ctlr->output_mutex);
/* if handshake command fails, assume ble pro controller */
- if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
- !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
+ if (joycon_using_usb(ctlr) && !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
hid_dbg(hdev, "detected USB controller\n");
/* set baudrate for improved latency */
ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
@@ -2171,24 +2530,43 @@ static int joycon_init(struct hid_device *hdev)
goto out_unlock;
}
- /* get controller calibration data, and parse it */
- ret = joycon_request_calibration(ctlr);
+ /* needed to retrieve the controller type */
+ ret = joycon_read_info(ctlr);
if (ret) {
- /*
- * We can function with default calibration, but it may be
- * inaccurate. Provide a warning, and continue on.
- */
- hid_warn(hdev, "Analog stick positions may be inaccurate\n");
+ hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
+ ret);
+ goto out_unlock;
}
- /* get IMU calibration data, and parse it */
- ret = joycon_request_imu_calibration(ctlr);
- if (ret) {
- /*
- * We can function with default calibration, but it may be
- * inaccurate. Provide a warning, and continue on.
- */
- hid_warn(hdev, "Unable to read IMU calibration data\n");
+ if (joycon_has_joysticks(ctlr)) {
+ /* get controller calibration data, and parse it */
+ ret = joycon_request_calibration(ctlr);
+ if (ret) {
+ /*
+ * We can function with default calibration, but it may be
+ * inaccurate. Provide a warning, and continue on.
+ */
+ hid_warn(hdev, "Analog stick positions may be inaccurate\n");
+ }
+ }
+
+ if (joycon_has_imu(ctlr)) {
+ /* get IMU calibration data, and parse it */
+ ret = joycon_request_imu_calibration(ctlr);
+ if (ret) {
+ /*
+ * We can function with default calibration, but it may be
+ * inaccurate. Provide a warning, and continue on.
+ */
+ hid_warn(hdev, "Unable to read IMU calibration data\n");
+ }
+
+ /* Enable the IMU */
+ ret = joycon_enable_imu(ctlr);
+ if (ret) {
+ hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
+ goto out_unlock;
+ }
}
/* Set the reporting mode to 0x30, which is the full report mode */
@@ -2198,18 +2576,13 @@ static int joycon_init(struct hid_device *hdev)
goto out_unlock;
}
- /* Enable rumble */
- ret = joycon_enable_rumble(ctlr);
- if (ret) {
- hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
- goto out_unlock;
- }
-
- /* Enable the IMU */
- ret = joycon_enable_imu(ctlr);
- if (ret) {
- hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
- goto out_unlock;
+ if (joycon_has_rumble(ctlr)) {
+ /* Enable rumble */
+ ret = joycon_enable_rumble(ctlr);
+ if (ret) {
+ hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
+ goto out_unlock;
+ }
}
out_unlock:
@@ -2354,13 +2727,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
goto err_close;
}
- ret = joycon_read_info(ctlr);
- if (ret) {
- hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
- ret);
- goto err_close;
- }
-
/* Initialize the leds */
ret = joycon_leds_create(ctlr);
if (ret) {
@@ -2432,6 +2798,12 @@ static int nintendo_hid_resume(struct hid_device *hdev)
static const struct hid_device_id nintendo_hid_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
USB_DEVICE_ID_NINTENDO_PROCON) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
+ USB_DEVICE_ID_NINTENDO_SNESCON) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
+ USB_DEVICE_ID_NINTENDO_GENCON) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
+ USB_DEVICE_ID_NINTENDO_N64CON) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
USB_DEVICE_ID_NINTENDO_PROCON) },
{ HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
@@ -2440,6 +2812,12 @@ static const struct hid_device_id nintendo_hid_devices[] = {
USB_DEVICE_ID_NINTENDO_JOYCONL) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
USB_DEVICE_ID_NINTENDO_JOYCONR) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
+ USB_DEVICE_ID_NINTENDO_SNESCON) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
+ USB_DEVICE_ID_NINTENDO_GENCON) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
+ USB_DEVICE_ID_NINTENDO_N64CON) },
{ }
};
MODULE_DEVICE_TABLE(hid, nintendo_hid_devices);
@@ -2458,6 +2836,7 @@ static struct hid_driver nintendo_hid_driver = {
module_hid_driver(nintendo_hid_driver);
MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Ryan McClelland <rymcclel@gmail.com>");
+MODULE_AUTHOR("Emily Strickland <linux@emily.st>");
MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");
-
--
2.34.1
^ permalink raw reply related
* Re: [RFC PATCH v3 3/5] platform/chrome: Introduce device tree hardware prober
From: Chen-Yu Tsai @ 2023-12-04 7:53 UTC (permalink / raw)
To: Doug Anderson
Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Wolfram Sang,
Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAGXv+5Hz3wfjCRa2AiOQgOv7zo8bzAmtG=a=jWJhO2MZNrFtpw@mail.gmail.com>
On Mon, Dec 4, 2023 at 3:24 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > >
> > > @@ -61,6 +61,17 @@ config CHROMEOS_TBMC
> > > To compile this driver as a module, choose M here: the
> > > module will be called chromeos_tbmc.
> > >
> > > +config CHROMEOS_OF_HW_PROBER
> > > + bool "ChromeOS Device Tree Hardware Prober"
> >
> > Any reason that it can't be a module?
>
> No technical one. However if it's a module, the user has to manually load
> it. So I think it's more of a usability thing.
We could probably manually add module aliases. I thought about aliases
against the machine compatibles, but there doesn't seem to be a device
for it to trigger. Or target something common to ChromeOS devices like
the EC? It's really hacky though.
ChenYu
> OOTH I think this needs to be a module if I2C is built as a module.
> Somehow I had thought of it at one point but then it slipped my mind.
>
> > > + depends on OF
> > > + depends on I2C
> > > + select OF_DYNAMIC
> > > + default OF
> >
> > You probably don't want "default OF". This means that everyone will
> > automatically get this new driver enabled which is unlikely to be
> > right.
>
> I thought this whole section was guarded behind KCONFIG_CHROME_PLATFORMS.
> So if the user has CHROME_PLATFORMS enabled and has OF enabled, they
> likely need the prober.
>
> > > +static int chromeos_of_hw_prober_probe(struct platform_device *pdev)
> > > +{
> > > + for (size_t i = 0; i < ARRAY_SIZE(hw_prober_platforms); i++)
> > > + if (of_machine_is_compatible(hw_prober_platforms[i].compatible)) {
> > > + int ret;
> > > +
> > > + ret = hw_prober_platforms[i].prober(&pdev->dev,
> > > + hw_prober_platforms[i].data);
> > > + if (ret)
> >
> > Should it only check for -EPROBE_DEFER here? ...and then maybe warn
> > for other cases and go through the loop? If there's some error
> > enabling the touchscreen I'd still want the trackpad to probe...
>
> Makes sense. However there's no extra information to give in the
> warning though.
>
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> >
> > Random thought: once we get here, the driver is useless / just wasting
> > memory. Any way to have it freed? ;-)
>
> I don't think there is a good way to do that, except maybe marking all
> the functions as __init? But that likely doesn't work in combination
> with deferred probing (say the i2c driver is a module).
>
> ChenYu
^ permalink raw reply
* Re: [RFC PATCH v3 3/5] platform/chrome: Introduce device tree hardware prober
From: Chen-Yu Tsai @ 2023-12-04 7:24 UTC (permalink / raw)
To: Doug Anderson
Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Wolfram Sang,
Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAD=FV=XV0+G=uFBE_n6WFGVW2szGcKToZgCNTdSrNf3LVk9MOQ@mail.gmail.com>
On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > @@ -61,6 +61,17 @@ config CHROMEOS_TBMC
> > To compile this driver as a module, choose M here: the
> > module will be called chromeos_tbmc.
> >
> > +config CHROMEOS_OF_HW_PROBER
> > + bool "ChromeOS Device Tree Hardware Prober"
>
> Any reason that it can't be a module?
No technical one. However if it's a module, the user has to manually load
it. So I think it's more of a usability thing.
OOTH I think this needs to be a module if I2C is built as a module.
Somehow I had thought of it at one point but then it slipped my mind.
> > + depends on OF
> > + depends on I2C
> > + select OF_DYNAMIC
> > + default OF
>
> You probably don't want "default OF". This means that everyone will
> automatically get this new driver enabled which is unlikely to be
> right.
I thought this whole section was guarded behind KCONFIG_CHROME_PLATFORMS.
So if the user has CHROME_PLATFORMS enabled and has OF enabled, they
likely need the prober.
> > +static int chromeos_of_hw_prober_probe(struct platform_device *pdev)
> > +{
> > + for (size_t i = 0; i < ARRAY_SIZE(hw_prober_platforms); i++)
> > + if (of_machine_is_compatible(hw_prober_platforms[i].compatible)) {
> > + int ret;
> > +
> > + ret = hw_prober_platforms[i].prober(&pdev->dev,
> > + hw_prober_platforms[i].data);
> > + if (ret)
>
> Should it only check for -EPROBE_DEFER here? ...and then maybe warn
> for other cases and go through the loop? If there's some error
> enabling the touchscreen I'd still want the trackpad to probe...
Makes sense. However there's no extra information to give in the
warning though.
> > + return ret;
> > + }
> > +
> > + return 0;
>
> Random thought: once we get here, the driver is useless / just wasting
> memory. Any way to have it freed? ;-)
I don't think there is a good way to do that, except maybe marking all
the functions as __init? But that likely doesn't work in combination
with deferred probing (say the i2c driver is a module).
ChenYu
^ permalink raw reply
* Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail
From: Chen-Yu Tsai @ 2023-12-04 6:59 UTC (permalink / raw)
To: Doug Anderson
Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Wolfram Sang,
Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAD=FV=W01gfxV6RN2o6CVS7jjf8qgKP-jUy9Bp94d2hWzVC48A@mail.gmail.com>
On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
> > reg = <0x2c>;
> > hid-descr-addr = <0x0020>;
> > wakeup-source;
> > + status = "fail-needs-probe";
>
> While doing this, you could also remove the hack where the trackpad
> IRQ pinctrl is listed under i2c4.
Sure. I do think we can do away with it though. According to at least one
schematic, the interrupt line has pull-ups on both sides of the voltage
shifter.
BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
both sides of the voltage shifter as well.
ChenYu
^ permalink raw reply
* Re: [RFC PATCH v3 1/5] of: dynamic: Add of_changeset_update_prop_string
From: Chen-Yu Tsai @ 2023-12-04 6:28 UTC (permalink / raw)
To: Doug Anderson
Cc: Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Wolfram Sang,
Benson Leung, Tzung-Bi Shih, chrome-platform, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, Johan Hovold,
Hsin-Yi Wang, Dmitry Torokhov, andriy.shevchenko, Jiri Kosina,
linus.walleij, broonie, gregkh, hdegoede, james.clark, james,
keescook, rafael, tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <CAD=FV=V_v11eZ6+3gUwOvdWGNM9owG7zCK5EiezTY7RJ3eaEMw@mail.gmail.com>
On Sat, Dec 2, 2023 at 9:01 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > @@ -1039,3 +1039,50 @@ int of_changeset_add_prop_u32_array(struct of_changeset *ocs,
> > return ret;
> > }
> > EXPORT_SYMBOL_GPL(of_changeset_add_prop_u32_array);
> > +
> > +static int of_changeset_update_prop_helper(struct of_changeset *ocs,
> > + struct device_node *np,
> > + const struct property *pp)
> > +{
> > + struct property *new_pp;
> > + int ret;
> > +
> > + new_pp = __of_prop_dup(pp, GFP_KERNEL);
> > + if (!new_pp)
> > + return -ENOMEM;
> > +
> > + ret = of_changeset_update_property(ocs, np, new_pp);
> > + if (ret) {
> > + kfree(new_pp->name);
> > + kfree(new_pp->value);
> > + kfree(new_pp);
>
> Given that this is the 3rd copy of the freeing logic, does it make
> sense to make __of_prop_free() that's documented to free what was
> returned by __of_prop_dupe()?
Makes sense. There's also one in property_list_free(). I'll add a patch
for it.
ChenYu
^ permalink raw reply
* RE: [PATCH] input/vmmouse: Fix device name copies
From: David Laight @ 2023-12-03 21:14 UTC (permalink / raw)
To: 'Arnd Bergmann', Dmitry Torokhov, Zack Rusin
Cc: linux-kernel@vger.kernel.org, VMware Graphics Reviewers,
Robert Jarzmik, Raul Rangel, linux-input@vger.kernel.org,
stable@vger.kernel.org
In-Reply-To: <d180f06b-64b0-4885-9794-5127c297a0f0@app.fastmail.com>
From: Arnd Bergmann
> Sent: 03 December 2023 20:51
> On Sun, Dec 3, 2023, at 19:41, Dmitry Torokhov wrote:
> > On Mon, Nov 27, 2023 at 03:42:06PM -0500, Zack Rusin wrote:
> >> From: Zack Rusin <zackr@vmware.com>
> >>
> >> Make sure vmmouse_data::phys can hold serio::phys (which is 32 bytes)
> >> plus an extra string, extend it to 64.
> >>
> >> Fixes gcc13 warnings:
> >> drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
> >> drivers/input/mouse/vmmouse.c:455:53: warning: ‘/input1’ directive output may be truncated writing
> 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
> >> 455 | snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> >> | ^~~~~~~
> >> drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 bytes into a
> destination of size 32
> >> 455 | snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> 456 | psmouse->ps2dev.serio->phys);
> >> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > This simply wastes 32 bytes. It is perfectly fine to truncate phys
> > (which does not happen in real life).
> >
> > -Wformat-truncation is disabled in normal builds, folks should stop
> > using it with W=1 as well.
>
> It does find real bugs, and we are fairly close to being able
> to enable it by default once the remaining warnings are all
> fixed.
>
> It also doesn't waste any memory in this specific case since
> vmmouse_data is currently at 168 bytes, which gets rounded
> up to either 192 or 256 bytes anyway. I'd suggest using
> the minimum size that is large enough though, in this case
> 39 bytes for the string I guess.
That rather depends on whether any of the earlier char[] lengths
have been rounded up to a 'nice' value.
I'd also have thought that dangerous overflows would come from
unbounded %s formats, not fixed size strings or integers that are
always small.
There really ought to be a sane method of telling gcc not to bleat
about snprintf() potentially overflowing the target.
I've tried a few thing but none of them work.
IIRC using the result (in some ways) is enough, but neither
(void)snprintf(...); or if (snprintf(...)); is enough
(but they 'fix' 'warn unused result').
David
>
> Arnd
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH] input/vmmouse: Fix device name copies
From: Arnd Bergmann @ 2023-12-03 20:50 UTC (permalink / raw)
To: Dmitry Torokhov, Zack Rusin
Cc: linux-kernel, VMware Graphics Reviewers, Robert Jarzmik,
Raul Rangel, linux-input, stable
In-Reply-To: <ZWzLvctpo1nNTMOo@google.com>
On Sun, Dec 3, 2023, at 19:41, Dmitry Torokhov wrote:
> On Mon, Nov 27, 2023 at 03:42:06PM -0500, Zack Rusin wrote:
>> From: Zack Rusin <zackr@vmware.com>
>>
>> Make sure vmmouse_data::phys can hold serio::phys (which is 32 bytes)
>> plus an extra string, extend it to 64.
>>
>> Fixes gcc13 warnings:
>> drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
>> drivers/input/mouse/vmmouse.c:455:53: warning: ‘/input1’ directive output may be truncated writing 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
>> 455 | snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>> | ^~~~~~~
>> drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 bytes into a destination of size 32
>> 455 | snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 456 | psmouse->ps2dev.serio->phys);
>> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This simply wastes 32 bytes. It is perfectly fine to truncate phys
> (which does not happen in real life).
>
> -Wformat-truncation is disabled in normal builds, folks should stop
> using it with W=1 as well.
It does find real bugs, and we are fairly close to being able
to enable it by default once the remaining warnings are all
fixed.
It also doesn't waste any memory in this specific case since
vmmouse_data is currently at 168 bytes, which gets rounded
up to either 192 or 256 bytes anyway. I'd suggest using
the minimum size that is large enough though, in this case
39 bytes for the string I guess.
Arnd
^ permalink raw reply
* Re: [PATCH] ipaq-micro-keys: Add error handling for devm_kmemdup
From: Dmitry Torokhov @ 2023-12-03 19:06 UTC (permalink / raw)
To: Haoran Liu; +Cc: linux-input, linux-kernel
In-Reply-To: <20231203164653.38983-1-liuhaoran14@163.com>
On Sun, Dec 03, 2023 at 08:46:53AM -0800, Haoran Liu wrote:
> Check the return value of i2c_add_adapter. Static analysis revealed that
> the function did not properly handle potential failures of
> i2c_add_adapter, which could lead to partial initialization of the I2C
> adapter and unstable operation.
>
> Signed-off-by: Haoran Liu <liuhaoran14@163.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input/vmmouse: Fix device name copies
From: Dmitry Torokhov @ 2023-12-03 18:41 UTC (permalink / raw)
To: Zack Rusin
Cc: linux-kernel, VMware Graphics Reviewers, Arnd Bergmann,
Robert Jarzmik, Raul Rangel, linux-input, stable
In-Reply-To: <20231127204206.3593559-1-zack@kde.org>
Zack,
On Mon, Nov 27, 2023 at 03:42:06PM -0500, Zack Rusin wrote:
> From: Zack Rusin <zackr@vmware.com>
>
> Make sure vmmouse_data::phys can hold serio::phys (which is 32 bytes)
> plus an extra string, extend it to 64.
>
> Fixes gcc13 warnings:
> drivers/input/mouse/vmmouse.c: In function ‘vmmouse_init’:
> drivers/input/mouse/vmmouse.c:455:53: warning: ‘/input1’ directive output may be truncated writing 7 bytes into a region of size between 1 and 32 [-Wformat-truncation=]
> 455 | snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> | ^~~~~~~
> drivers/input/mouse/vmmouse.c:455:9: note: ‘snprintf’ output between 8 and 39 bytes into a destination of size 32
> 455 | snprintf(priv->phys, sizeof(priv->phys), "%s/input1",
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 456 | psmouse->ps2dev.serio->phys);
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Signed-off-by: Zack Rusin <zackr@vmware.com>
> Fixes: 8b8be51b4fd3 ("Input: add vmmouse driver")
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: VMware Graphics Reviewers <linux-graphics-maintainer@vmware.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Robert Jarzmik <robert.jarzmik@free.fr>
> Cc: Raul Rangel <rrangel@chromium.org>
> Cc: linux-input@vger.kernel.org
> Cc: <stable@vger.kernel.org> # v4.1+
> ---
> drivers/input/mouse/vmmouse.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/vmmouse.c b/drivers/input/mouse/vmmouse.c
> index ea9eff7c8099..7248cada4c8c 100644
> --- a/drivers/input/mouse/vmmouse.c
> +++ b/drivers/input/mouse/vmmouse.c
> @@ -72,7 +72,7 @@
> */
> struct vmmouse_data {
> struct input_dev *abs_dev;
> - char phys[32];
> + char phys[64];
This simply wastes 32 bytes. It is perfectly fine to truncate phys
(which does not happen in real life).
-Wformat-truncation is disabled in normal builds, folks should stop
using it with W=1 as well.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: How to disable the touchscreen so it does not draw power?
From: Paul Menzel @ 2023-12-03 17:41 UTC (permalink / raw)
To: Hans de Goede; +Cc: Dmitry Torokhov, linux-input
In-Reply-To: <99dcbb8d-06a8-4c32-ab03-94bc3bf5658f@redhat.com>
Dear Hans,
Thank you for taking the time to reply.
Am 13.11.23 um 13:08 schrieb Hans de Goede:
> On 11/13/23 09:39, Paul Menzel wrote:
>> On a Dell XPS 13 9360 with a touchscreen, when it’s connect to an
>> external monitor, that is set up as the only display, the
>> touchscreen – although the internal monitor is disabled (in GNOME)
>> – is still active and draws power according to PowerTOP: >>
>> $ uname -a
>> Linux abreu 6.5.0-4-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.5.10-1 (2023-11-03) x86_64 GNU/Linux
>> $ lsusb | grep Touchscreen
>> Bus 001 Device 003: ID 04f3:2234 Elan Microelectronics Corp. Touchscreen
>> $ sudo LANG= powertop
>> Power est. Usage Events/s Category Description
>> 9.39 W 100.0% Device USB device: DELL DA300 (Bizlink)
>> 1.39 W 100.0% Device USB device: Touchscreen (ELAN)
>>
>> Is there a way to disable the touchscreen, so it does not draw power?
>>
>> `sudo modprobe -r hid-multitouch` also affects the touchpad, which is not wanted.
>
> Ideally userspace would close the /dev/input/event node belonging to
> the touchscreen when the internal panel is off. Please file an issue
> for that against libinput (to add the plumbing for this to libinput,
> ultimately the wayland-compositor, e.g. mutter, then needs to use
> that plumbing).
I created issue *Disabling the panel of a touchscreen does not disable
the input/touch part wasting energy* [1].
[…]
Kind regards,
Paul
[1]: https://gitlab.freedesktop.org/libinput/libinput/-/issues/956
^ permalink raw reply
* [PATCH] ipaq-micro-keys: Add error handling for devm_kmemdup
From: Haoran Liu @ 2023-12-03 16:46 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Haoran Liu
Check the return value of i2c_add_adapter. Static analysis revealed that
the function did not properly handle potential failures of
i2c_add_adapter, which could lead to partial initialization of the I2C
adapter and unstable operation.
Signed-off-by: Haoran Liu <liuhaoran14@163.com>
---
Although the error addressed by this patch may not occur in the current
environment, I still suggest implementing these error handling routines
if the function is not highly time-sensitive. As the environment evolves
or the code gets reused in different contexts, there's a possibility that
these errors might occur. In case you find this addition unnecessary, I
completely understand and respect your perspective. My intention was to
enhance the robustness of the code, but I acknowledge that practical
considerations and current functionality might not warrant this change
at this point.
---
drivers/input/keyboard/ipaq-micro-keys.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
index 7b509bce2b33..1d71dd79ffd2 100644
--- a/drivers/input/keyboard/ipaq-micro-keys.c
+++ b/drivers/input/keyboard/ipaq-micro-keys.c
@@ -105,6 +105,9 @@ static int micro_key_probe(struct platform_device *pdev)
keys->codes = devm_kmemdup(&pdev->dev, micro_keycodes,
keys->input->keycodesize * keys->input->keycodemax,
GFP_KERNEL);
+ if (!keys->codes)
+ return -ENOMEM;
+
keys->input->keycode = keys->codes;
__set_bit(EV_KEY, keys->input->evbit);
--
2.17.1
^ permalink raw reply related
* [PATCH] Input: expand keycode space
From: Wei Shuai @ 2023-12-03 12:50 UTC (permalink / raw)
To: Dmitry Torokhov,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list
Dear Dmitry Torokhov,
Today I found out that in 2008, you have expanded the number of
potential key codes from 512 to 768.
I request to expand keycode space again, from 0x2ff to 0x4ff. I have
tested this patch on Ubuntu Kernel, it works well with a USB HID
joystick that has more than 80 keys.
Story back to I found Linux kernel has max USB HID joystick buttons up
to 80, no more. But Windows OS and Mac OS have no such limitations.
I have a USB HID joystick device, which has 104 buttons, larger than 80.
I did a lot of google search, but nothing I got. then I have to look
at Kernel source, to find out where this number max 80 comes from
Eventually, I found the final limitation
#define BTN_JOYSTICK 0x120
#define BTN_DEAD 0x12f
#define BTN_TRIGGER_HAPPY 0x2c0
#define KEY_MAX 0x2ff
include/uapi/linux/input-event-codes.h
according to function hidinput_configure_usage() in file drivers/hid/hid-input.c
the USB joystick button mapping is not a continuous space, generally speak
the mapping space is from
BTN_JOYSTICK~BTN_DEAD 0x120~0x12f=0x10
BTN_TRIGGER_HAPPY~KEY_MAX 0x2c0~0x2ff=40
0x10+0x40=0x60=80
and finally, I got the max limitation of 80 for joystick keys.
This patch is about expanding keycode space from 0x2ff to 0x4ff. I
expect this patch won't go with ease, because it affects user space
application definition as well. I believe sooner or later Linux kernel
will face keycode is not enough
Signed-off-by: Wei Shuai <cpuwolf@gmail.com>
---
include/linux/mod_devicetable.h | 2 +-
include/uapi/linux/input-event-codes.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 8d764aab29de..35eb59ae1f19 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -311,7 +311,7 @@ struct pcmcia_device_id {
/* Input */
#define INPUT_DEVICE_ID_EV_MAX 0x1f
#define INPUT_DEVICE_ID_KEY_MIN_INTERESTING 0x71
-#define INPUT_DEVICE_ID_KEY_MAX 0x2ff
+#define INPUT_DEVICE_ID_KEY_MAX 0x4ff
#define INPUT_DEVICE_ID_REL_MAX 0x0f
#define INPUT_DEVICE_ID_ABS_MAX 0x3f
#define INPUT_DEVICE_ID_MSC_MAX 0x07
diff --git a/include/uapi/linux/input-event-codes.h
b/include/uapi/linux/input-event-codes.h
index b6a835d37826..ad1b9bed3828 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -774,7 +774,7 @@
/* We avoid low common keys in module aliases so they don't get huge. */
#define KEY_MIN_INTERESTING KEY_MUTE
-#define KEY_MAX 0x2ff
+#define KEY_MAX 0x4ff
#define KEY_CNT (KEY_MAX+1)
/*
--
2.17.1
^ permalink raw reply related
* RE: [PATCH v3 00/11] Convert DA906{1,2} bindings to json-schema
From: Biju Das @ 2023-12-03 12:43 UTC (permalink / raw)
To: Conor Dooley
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Steve Twiss, linux-input@vger.kernel.org,
devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
Geert Uytterhoeven, Prabhakar Mahadev Lad, biju.das.au,
linux-renesas-soc@vger.kernel.org
In-Reply-To: <20231203-unthread-suffering-411df4cb0f4c@spud>
Hi Conor,
Thanks for the feedback.
> Subject: Re: [PATCH v3 00/11] Convert DA906{1,2} bindings to json-schema
>
> On Sun, Dec 03, 2023 at 11:31:48AM +0000, Biju Das wrote:
> > Convert the below bindings to json-schema
> > 1) DA906{1,2} mfd bindings
> > 2) DA906{1,2,3} onkey bindings
> > 3) DA906{1,2,3} thermal bindings
> >
> > Also add fallback for DA9061 watchdog device and document
> > DA9063 watchdog device.
>
> Horrible timing, you sent this after I started looking at the previous
> revision of the patchset :( I left some comments and tags on the previous
> version which I would imagine that some of them also apply here.
I missed some checks which is pointed out by bot. So I thought of sending v3
Before anyone started reviewing it.
Sure, I will take care you review comments in v2 when sending new version.
Cheers,
Biju
>
> >
> > v2->v3:
> > * Updated Maintainer entries for watchdog,onkey and thermal bindings
> > * Fixed bot errors related to MAINTAINERS entry, invalid doc
> > references and thermal examples by merging patch#4.
> >
> > v1->v2:
> > Ref:
> > https://lore.kernel.org/all/20231201110840.37408-5-biju.das.jz@bp.rene
> > sas.com/
> > * DA9062 and DA9061 merged with DA9063
> > * Sorted the child devices
> > * mfd,onkey and thermal are pointing to child bindings
> >
> >
> >
> > Biju Das (11):
> > MAINTAINERS: Update da9062-watchdog bindings
> > dt-bindings: watchdog: dlg,da9062-watchdog: Add fallback for DA9061
> > watchdog
> > dt-bindings: watchdog: dlg,da9062-watchdog: Document DA9063 watchdog
> > dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
> > dt-bindings: mfd: dlg,da9063: Update watchdog property
> > dt-bindings: mfd: dlg,da9063: Update onkey property
> > dt-bindings: mfd: dlg,da9063: Sort child devices
> > dt-bindings: mfd: da9062: Update watchdog description
> > dt-bindings: mfd: da9062: Update onkey description
> > dt-bindings: mfd: da9062: Update thermal description
> > dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
> >
> > .../bindings/input/da9062-onkey.txt | 47 ----
> > .../bindings/input/dlg,da9062-onkey.yaml | 60 +++++
> > .../devicetree/bindings/mfd/da9062.txt | 124 ----------
> > .../devicetree/bindings/mfd/dlg,da9063.yaml | 221 +++++++++++++++---
> > .../bindings/thermal/da9062-thermal.txt | 36 ---
> > .../bindings/thermal/dlg,da9062-thermal.yaml | 78 +++++++
> > .../watchdog/dlg,da9062-watchdog.yaml | 12 +-
> > MAINTAINERS | 6 +-
> > 8 files changed, 336 insertions(+), 248 deletions(-) delete mode
> > 100644 Documentation/devicetree/bindings/input/da9062-onkey.txt
> > create mode 100644
> > Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> > delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
> > delete mode 100644
> > Documentation/devicetree/bindings/thermal/da9062-thermal.txt
> > create mode 100644
> > Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
> >
> > --
> > 2.39.2
> >
^ permalink raw reply
* [PATCH] HID: apple: Add "hfd.cn" and "WKB603" to the list of non-apple keyboards
From: yjun @ 2023-12-03 11:50 UTC (permalink / raw)
To: jkosina; +Cc: jikos, benjamin.tissoires, linux-input, yjun
JingZao(京造) WKB603 keyboard is a rebranded product
of Jamesdonkey RS2 keyboard, identified as "hfd.cn WKB603"
in wired mode, "WKB603" in bluetooth mode. Adding them
to the list of non-apple keyboards fixes function key.
Signed-off-by: Yan Jun <jerrysteve1101@gmail.com>
---
drivers/hid/hid-apple.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index d9e9829b2200..b9c7c0ed7bcc 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -347,6 +347,8 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
{ "Hailuck" },
{ "Jamesdonkey" },
{ "A3R" },
+ { "hfd.cn" },
+ { "WKB603" },
};
static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox