* [PATCH v3 15/16] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD
From: Shyam Sundar S K @ 2023-10-10 12:59 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Basavaraj Natikar, Shyam Sundar S K
In-Reply-To: <20231010125917.138225-1-Shyam-sundar.S-k@amd.com>
From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
AMDSFH has information about the User presence information via the Human
Presence Detection (HPD) sensor which is part of the AMD sensor fusion hub.
Add PMF and AMDSFH interface to get this information.
Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 5 ++++
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 11 ++++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 28 +++++++++++++++++++
drivers/platform/x86/amd/pmf/Kconfig | 1 +
drivers/platform/x86/amd/pmf/spc.c | 21 ++++++++++++++
include/linux/amd-pmf-io.h | 20 ++++++++++++-
6 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index 2643bb14fee2..cd57037bf217 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -37,6 +37,10 @@ struct amd_mp2_sensor_info {
dma_addr_t dma_address;
};
+struct sfh_dev_status {
+ bool is_hpd_present;
+};
+
struct amd_mp2_dev {
struct pci_dev *pdev;
struct amdtp_cl_data *cl_data;
@@ -47,6 +51,7 @@ struct amd_mp2_dev {
struct amd_input_data in_data;
/* mp2 active control status */
u32 mp2_acs;
+ struct sfh_dev_status dev_en;
};
struct amd_mp2_ops {
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
index e9c6413af24a..3dc652d41d7d 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
@@ -73,6 +73,12 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
int i, status;
for (i = 0; i < cl_data->num_hid_devices; i++) {
+ switch (cl_data->sensor_idx[i]) {
+ case HPD_IDX:
+ privdata->dev_en.is_hpd_present = false;
+ break;
+ }
+
if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
privdata->mp2_ops->stop(privdata, cl_data->sensor_idx[i]);
status = amd_sfh_wait_for_response
@@ -178,6 +184,11 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
rc = amdtp_hid_probe(i, cl_data);
if (rc)
goto cleanup;
+ switch (cl_data->sensor_idx[i]) {
+ case HPD_IDX:
+ privdata->dev_en.is_hpd_present = true;
+ break;
+ }
}
dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 4f81ef2d4f56..7637da0dec6f 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -7,11 +7,14 @@
*
* Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
*/
+#include <linux/amd-pmf-io.h>
#include <linux/io-64-nonatomic-lo-hi.h>
#include <linux/iopoll.h>
#include "amd_sfh_interface.h"
+static struct amd_mp2_dev *emp2;
+
static int amd_sfh_wait_response(struct amd_mp2_dev *mp2, u8 sid, u32 cmd_id)
{
struct sfh_cmd_response cmd_resp;
@@ -76,4 +79,29 @@ static struct amd_mp2_ops amd_sfh_ops = {
void sfh_interface_init(struct amd_mp2_dev *mp2)
{
mp2->mp2_ops = &amd_sfh_ops;
+ emp2 = mp2;
+}
+
+static int amd_sfh_hpd_info(u8 *user_present)
+{
+ if (emp2 && emp2->dev_en.is_hpd_present) {
+ struct hpd_status hpdstatus;
+
+ hpdstatus.val = readl(emp2->mmio + AMD_C2P_MSG(4));
+ *user_present = hpdstatus.shpd.presence;
+ return 0;
+ }
+ return -ENODEV;
+}
+
+int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
+{
+ if (sfh_info) {
+ switch (op) {
+ case MT_HPD:
+ return amd_sfh_hpd_info(&sfh_info->user_present);
+ }
+ }
+ return -EINVAL;
}
+EXPORT_SYMBOL_GPL(amd_get_sfh_info);
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index 7f430de7af44..d368d35a49ac 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -11,6 +11,7 @@ config AMD_PMF
select ACPI_PLATFORM_PROFILE
depends on TEE && AMDTEE
depends on DRM_AMDGPU
+ depends on AMD_SFH_HID
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/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index 58b51e11e424..e33bbf8a3de4 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -49,6 +49,7 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
dev_dbg(dev->dev, "Primary Display State : %s\n", in->ev_info.display_state ?
"Connected" : "disconnected/unknown");
dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? "Close" : "Open");
+ dev_dbg(dev->dev, "User Presence : %s\n", in->ev_info.user_present ? "Present" : "Away");
dev_dbg(dev->dev, "==== TA inputs END ====\n");
}
#else
@@ -157,6 +158,25 @@ static void amd_pmf_get_gpu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
in->ev_info.display_state = dev->gfx_data.con_status[0];
}
+static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+ struct amd_sfh_info sfh_info;
+
+ /* get HPD data */
+ amd_get_sfh_info(&sfh_info, MT_HPD);
+ switch (sfh_info.user_present) {
+ case SFH_NOT_DETECTED:
+ in->ev_info.user_present = 0xff; /* assume no sensors connected */
+ break;
+ case SFH_USER_PRESENT:
+ in->ev_info.user_present = 1;
+ break;
+ case SFH_USER_AWAY:
+ in->ev_info.user_present = 0;
+ break;
+ }
+}
+
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 */
@@ -166,4 +186,5 @@ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_tab
amd_pmf_get_battery_info(dev, in);
amd_pmf_get_slider_info(dev, in);
amd_pmf_get_gpu_info(dev, in);
+ amd_pmf_get_sensor_info(dev, in);
}
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index 5f79e66a41b3..76e42552b62c 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -5,7 +5,8 @@
* Copyright (c) 2023, Advanced Micro Devices, Inc.
* All Rights Reserved.
*
- * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ * Basavaraj Natikar <Basavaraj.Natikar@amd.com>
*/
#ifndef AMD_PMF_IO_H
@@ -32,4 +33,21 @@ struct amd_gpu_pmf_data {
int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
int amd_pmf_gpu_init(struct amd_gpu_pmf_data *pmf);
void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
+
+/* amd-sfh */
+enum sfh_message_type {
+ MT_HPD,
+};
+
+enum hpd_info {
+ SFH_NOT_DETECTED,
+ SFH_USER_PRESENT,
+ SFH_USER_AWAY,
+};
+
+struct amd_sfh_info {
+ u8 user_present;
+};
+
+int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);
#endif
--
2.25.1
^ permalink raw reply related
* [PATCH v3 16/16] platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS
From: Shyam Sundar S K @ 2023-10-10 12:59 UTC (permalink / raw)
To: hdegoede, markgross, ilpo.jarvinen, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input,
amd-gfx, dri-devel, Basavaraj Natikar, Shyam Sundar S K
In-Reply-To: <20231010125917.138225-1-Shyam-sundar.S-k@amd.com>
From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
AMDSFH has information about the Ambient light via the Ambient
Light Sensor (ALS) which is part of the AMD sensor fusion hub.
Add PMF and AMDSFH interface to get this information.
make amd_sfh_float_to_int() as non-static function so that this can
be called outside of the current file.
Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
drivers/hid/amd-sfh-hid/amd_sfh_common.h | 1 +
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 2 +-
drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 6 ++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 20 +++++++++++++++++++
.../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 +
drivers/platform/x86/amd/pmf/spc.c | 7 +++++++
include/linux/amd-pmf-io.h | 2 ++
7 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index cd57037bf217..a1950bc6e6ce 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -39,6 +39,7 @@ struct amd_mp2_sensor_info {
struct sfh_dev_status {
bool is_hpd_present;
+ bool is_als_present;
};
struct amd_mp2_dev {
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index 47a87b28e00e..dbc8c6943ca1 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -132,7 +132,7 @@ static void get_common_inputs(struct common_input_property *common, int report_i
common->event_type = HID_USAGE_SENSOR_EVENT_DATA_UPDATED_ENUM;
}
-static int amd_sfh_float_to_int(u32 flt32_val)
+int amd_sfh_float_to_int(u32 flt32_val)
{
int fraction, shift, mantissa, sign, exp, zeropre;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
index 3dc652d41d7d..f2890d329459 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
@@ -77,6 +77,9 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
case HPD_IDX:
privdata->dev_en.is_hpd_present = false;
break;
+ case ALS_IDX:
+ privdata->dev_en.is_als_present = false;
+ break;
}
if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
@@ -188,6 +191,9 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
case HPD_IDX:
privdata->dev_en.is_hpd_present = true;
break;
+ case ALS_IDX:
+ privdata->dev_en.is_als_present = true;
+ break;
}
}
dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 7637da0dec6f..48a7a450e029 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -94,12 +94,32 @@ static int amd_sfh_hpd_info(u8 *user_present)
return -ENODEV;
}
+static int amd_sfh_als_info(u32 *ambient_light)
+{
+ if (emp2 && emp2->dev_en.is_als_present) {
+ struct sfh_als_data als_data;
+ void __iomem *sensoraddr;
+
+ sensoraddr = emp2->vsbase +
+ (ALS_IDX * SENSOR_DATA_MEM_SIZE_DEFAULT) +
+ OFFSET_SENSOR_DATA_DEFAULT;
+ memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
+ *ambient_light = amd_sfh_float_to_int(als_data.lux);
+
+ return 0;
+ }
+
+ return -ENODEV;
+}
+
int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
{
if (sfh_info) {
switch (op) {
case MT_HPD:
return amd_sfh_hpd_info(&sfh_info->user_present);
+ case MT_ALS:
+ return amd_sfh_als_info(&sfh_info->ambient_light);
}
}
return -EINVAL;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 9d31d5b510eb..7ecddad430e4 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -149,6 +149,7 @@ struct hpd_status {
};
};
+int amd_sfh_float_to_int(u32 flt32_val);
void sfh_interface_init(struct amd_mp2_dev *mp2);
void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
#endif
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index e33bbf8a3de4..b6cf6d7e6ef5 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -50,6 +50,7 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
"Connected" : "disconnected/unknown");
dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? "Close" : "Open");
dev_dbg(dev->dev, "User Presence : %s\n", in->ev_info.user_present ? "Present" : "Away");
+ dev_dbg(dev->dev, "Ambient Light : %d\n", in->ev_info.ambient_light);
dev_dbg(dev->dev, "==== TA inputs END ====\n");
}
#else
@@ -161,6 +162,12 @@ static void amd_pmf_get_gpu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
{
struct amd_sfh_info sfh_info;
+ int ret;
+
+ /* get ALS data */
+ ret = amd_get_sfh_info(&sfh_info, MT_ALS);
+ if (!ret)
+ in->ev_info.ambient_light = sfh_info.ambient_light;
/* get HPD data */
amd_get_sfh_info(&sfh_info, MT_HPD);
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index 76e42552b62c..912d341d5fbe 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -37,6 +37,7 @@ void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
/* amd-sfh */
enum sfh_message_type {
MT_HPD,
+ MT_ALS,
};
enum hpd_info {
@@ -46,6 +47,7 @@ enum hpd_info {
};
struct amd_sfh_info {
+ u32 ambient_light;
u8 user_present;
};
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 11/16] platform/x86/amd/pmf: dump policy binary data
From: Ilpo Järvinen @ 2023-10-10 13:08 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231010125917.138225-12-Shyam-sundar.S-k@amd.com>
On Tue, 10 Oct 2023, Shyam Sundar S K wrote:
> Sometimes policy binary retrieved from the BIOS maybe incorrect that can
> end up in failing to enable the Smart PC solution feature.
>
> Use print_hex_dump_debug() to dump the policy binary in hex, so that we
> debug the issues related to the binary even before sending that to TA.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/platform/x86/amd/pmf/tee-if.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 994daf945795..e4386f503ad0 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -275,6 +275,12 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
> }
>
> #ifdef CONFIG_AMD_PMF_DEBUG
> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev)
> +{
> + print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
> + dev->policy_sz, false);
> +}
> +
You forgot to add the empty version of amd_pmf_hex_dump_pb function into
#else part (so the compile fails if CONFIG_AMD_PMF_DEBUG is not set).
> static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> size_t length, loff_t *pos)
> {
> @@ -289,6 +295,7 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
> return -EFAULT;
>
> + amd_pmf_hex_dump_pb(dev);
> ret = amd_pmf_start_policy_engine(dev);
> if (ret)
> return -EINVAL;
> @@ -327,6 +334,7 @@ static int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
> }
>
> static void amd_pmf_remove_pb(struct amd_pmf_dev *dev) {}
> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev) {}
> #endif
>
> static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
> @@ -341,6 +349,7 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
>
> memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
>
> + amd_pmf_hex_dump_pb(dev);
> if (pb_side_load)
> amd_pmf_open_pb(dev, dev->dbgfs_dir);
>
>
--
i.
^ permalink raw reply
* Re: [PATCH v3 11/16] platform/x86/amd/pmf: dump policy binary data
From: Shyam Sundar S K @ 2023-10-10 13:38 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <1f3e39fc-2c2f-72d5-ce61-16e7e5d257be@linux.intel.com>
On 10/10/2023 6:38 PM, Ilpo Järvinen wrote:
> On Tue, 10 Oct 2023, Shyam Sundar S K wrote:
>
>> Sometimes policy binary retrieved from the BIOS maybe incorrect that can
>> end up in failing to enable the Smart PC solution feature.
>>
>> Use print_hex_dump_debug() to dump the policy binary in hex, so that we
>> debug the issues related to the binary even before sending that to TA.
>>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> drivers/platform/x86/amd/pmf/tee-if.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 994daf945795..e4386f503ad0 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -275,6 +275,12 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
>> }
>>
>> #ifdef CONFIG_AMD_PMF_DEBUG
>> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev)
>> +{
>> + print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
>> + dev->policy_sz, false);
>> +}
>> +
>
> You forgot to add the empty version of amd_pmf_hex_dump_pb function into
> #else part (so the compile fails if CONFIG_AMD_PMF_DEBUG is not set).
>
It's there (see below). I have just grouped the functions that come
under #ifdef CONFIG_AMD_PMF_DEBUG and #else so that's more readable.
>> static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
>> size_t length, loff_t *pos)
>> {
>> @@ -289,6 +295,7 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
>> if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
>> return -EFAULT;
>>
>> + amd_pmf_hex_dump_pb(dev);
>> ret = amd_pmf_start_policy_engine(dev);
>> if (ret)
>> return -EINVAL;
>> @@ -327,6 +334,7 @@ static int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
>> }
>>
>> static void amd_pmf_remove_pb(struct amd_pmf_dev *dev) {}
>> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev) {}
Here is the empty amd_pmf_hex_dump_pb().
Thanks,
Shyam
>> #endif
>>
>> static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
>> @@ -341,6 +349,7 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
>>
>> memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
>>
>> + amd_pmf_hex_dump_pb(dev);
>> if (pb_side_load)
>> amd_pmf_open_pb(dev, dev->dbgfs_dir);
>>
>>
>
^ permalink raw reply
* Re: [PATCH v3 11/16] platform/x86/amd/pmf: dump policy binary data
From: Ilpo Järvinen @ 2023-10-10 14:03 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, markgross, basavaraj.natikar, jikos,
benjamin.tissoires, alexander.deucher, christian.koenig,
Xinhui.Pan, airlied, daniel, Patil.Reddy, mario.limonciello,
platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <c9d53b05-68d0-4244-86cb-cb110c2c4af7@amd.com>
[-- Attachment #1: Type: text/plain, Size: 2332 bytes --]
On Tue, 10 Oct 2023, Shyam Sundar S K wrote:
> On 10/10/2023 6:38 PM, Ilpo Järvinen wrote:
> > On Tue, 10 Oct 2023, Shyam Sundar S K wrote:
> >
> >> Sometimes policy binary retrieved from the BIOS maybe incorrect that can
> >> end up in failing to enable the Smart PC solution feature.
> >>
> >> Use print_hex_dump_debug() to dump the policy binary in hex, so that we
> >> debug the issues related to the binary even before sending that to TA.
> >>
> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> >> ---
> >> drivers/platform/x86/amd/pmf/tee-if.c | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> >> index 994daf945795..e4386f503ad0 100644
> >> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> >> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> >> @@ -275,6 +275,12 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
> >> }
> >>
> >> #ifdef CONFIG_AMD_PMF_DEBUG
> >> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev)
> >> +{
> >> + print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
> >> + dev->policy_sz, false);
> >> +}
> >> +
> >
> > You forgot to add the empty version of amd_pmf_hex_dump_pb function into
> > #else part (so the compile fails if CONFIG_AMD_PMF_DEBUG is not set).
> >
>
> It's there (see below). I have just grouped the functions that come
> under #ifdef CONFIG_AMD_PMF_DEBUG and #else so that's more readable.
>
> >> static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> >> size_t length, loff_t *pos)
> >> {
> >> @@ -289,6 +295,7 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> >> if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
> >> return -EFAULT;
> >>
> >> + amd_pmf_hex_dump_pb(dev);
> >> ret = amd_pmf_start_policy_engine(dev);
> >> if (ret)
> >> return -EINVAL;
> >> @@ -327,6 +334,7 @@ static int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
> >> }
> >>
> >> static void amd_pmf_remove_pb(struct amd_pmf_dev *dev) {}
> >> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev) {}
>
> Here is the empty amd_pmf_hex_dump_pb().
Ah, sorry. I was confused by the intermediate change.
--
i.
^ permalink raw reply
* Re: [PATCH v3 16/16] platform/x86/amd/pmf: Add PMF-AMDSFH interface for ALS
From: Mario Limonciello @ 2023-10-10 15:59 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231010125917.138225-17-Shyam-sundar.S-k@amd.com>
On 10/10/2023 07:59, Shyam Sundar S K wrote:
> From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
>
> AMDSFH has information about the Ambient light via the Ambient
> Light Sensor (ALS) which is part of the AMD sensor fusion hub.
> Add PMF and AMDSFH interface to get this information.
>
> make amd_sfh_float_to_int() as non-static function so that this can
> be called outside of the current file.
>
> Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/hid/amd-sfh-hid/amd_sfh_common.h | 1 +
> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 2 +-
> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 6 ++++++
> .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 20 +++++++++++++++++++
> .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 1 +
> drivers/platform/x86/amd/pmf/spc.c | 7 +++++++
> include/linux/amd-pmf-io.h | 2 ++
> 7 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> index cd57037bf217..a1950bc6e6ce 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
> @@ -39,6 +39,7 @@ struct amd_mp2_sensor_info {
>
> struct sfh_dev_status {
> bool is_hpd_present;
> + bool is_als_present;
> };
>
> struct amd_mp2_dev {
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> index 47a87b28e00e..dbc8c6943ca1 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
> @@ -132,7 +132,7 @@ static void get_common_inputs(struct common_input_property *common, int report_i
> common->event_type = HID_USAGE_SENSOR_EVENT_DATA_UPDATED_ENUM;
> }
>
> -static int amd_sfh_float_to_int(u32 flt32_val)
> +int amd_sfh_float_to_int(u32 flt32_val)
This change might roll into patch 14, but I don't think it's that big of
a deal.
> {
> int fraction, shift, mantissa, sign, exp, zeropre;
>
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> index 3dc652d41d7d..f2890d329459 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
> @@ -77,6 +77,9 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
> case HPD_IDX:
> privdata->dev_en.is_hpd_present = false;
> break;
> + case ALS_IDX:
> + privdata->dev_en.is_als_present = false;
> + break;
> }
>
> if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
> @@ -188,6 +191,9 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
> case HPD_IDX:
> privdata->dev_en.is_hpd_present = true;
> break;
> + case ALS_IDX:
> + privdata->dev_en.is_als_present = true;
> + break;
> }
> }
> dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> index 7637da0dec6f..48a7a450e029 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
> @@ -94,12 +94,32 @@ static int amd_sfh_hpd_info(u8 *user_present)
> return -ENODEV;
> }
>
> +static int amd_sfh_als_info(u32 *ambient_light)
> +{
> + if (emp2 && emp2->dev_en.is_als_present) {
> + struct sfh_als_data als_data;
> + void __iomem *sensoraddr;
> +
> + sensoraddr = emp2->vsbase +
> + (ALS_IDX * SENSOR_DATA_MEM_SIZE_DEFAULT) +
> + OFFSET_SENSOR_DATA_DEFAULT;
> + memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
> + *ambient_light = amd_sfh_float_to_int(als_data.lux);
> +
> + return 0;
> + }
> +
> + return -ENODEV;
> +}
> +
> int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
> {
> if (sfh_info) {
> switch (op) {
> case MT_HPD:
> return amd_sfh_hpd_info(&sfh_info->user_present);
> + case MT_ALS:
> + return amd_sfh_als_info(&sfh_info->ambient_light);
> }
> }
> return -EINVAL;
> diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> index 9d31d5b510eb..7ecddad430e4 100644
> --- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> +++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
> @@ -149,6 +149,7 @@ struct hpd_status {
> };
> };
>
> +int amd_sfh_float_to_int(u32 flt32_val);
> void sfh_interface_init(struct amd_mp2_dev *mp2);
> void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
> #endif
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index e33bbf8a3de4..b6cf6d7e6ef5 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -50,6 +50,7 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
> "Connected" : "disconnected/unknown");
> dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? "Close" : "Open");
> dev_dbg(dev->dev, "User Presence : %s\n", in->ev_info.user_present ? "Present" : "Away");
> + dev_dbg(dev->dev, "Ambient Light : %d\n", in->ev_info.ambient_light);
> dev_dbg(dev->dev, "==== TA inputs END ====\n");
> }
> #else
> @@ -161,6 +162,12 @@ static void amd_pmf_get_gpu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
> static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
> {
> struct amd_sfh_info sfh_info;
> + int ret;
> +
> + /* get ALS data */
> + ret = amd_get_sfh_info(&sfh_info, MT_ALS);
> + if (!ret)
> + in->ev_info.ambient_light = sfh_info.ambient_light;
>
> /* get HPD data */
> amd_get_sfh_info(&sfh_info, MT_HPD);
> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
> index 76e42552b62c..912d341d5fbe 100644
> --- a/include/linux/amd-pmf-io.h
> +++ b/include/linux/amd-pmf-io.h
> @@ -37,6 +37,7 @@ void amd_pmf_gpu_deinit(struct amd_gpu_pmf_data *pmf);
> /* amd-sfh */
> enum sfh_message_type {
> MT_HPD,
> + MT_ALS,
> };
>
> enum hpd_info {
> @@ -46,6 +47,7 @@ enum hpd_info {
> };
>
> struct amd_sfh_info {
> + u32 ambient_light;
> u8 user_present;
> };
>
^ permalink raw reply
* Re: [PATCH v3 11/16] platform/x86/amd/pmf: dump policy binary data
From: Mario Limonciello @ 2023-10-10 16:00 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231010125917.138225-12-Shyam-sundar.S-k@amd.com>
On 10/10/2023 07:59, Shyam Sundar S K wrote:
> Sometimes policy binary retrieved from the BIOS maybe incorrect that can
> end up in failing to enable the Smart PC solution feature.
>
> Use print_hex_dump_debug() to dump the policy binary in hex, so that we
> debug the issues related to the binary even before sending that to TA.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/platform/x86/amd/pmf/tee-if.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 994daf945795..e4386f503ad0 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -275,6 +275,12 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
> }
>
> #ifdef CONFIG_AMD_PMF_DEBUG
> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev)
> +{
> + print_hex_dump_debug("(pb): ", DUMP_PREFIX_OFFSET, 16, 1, dev->policy_buf,
> + dev->policy_sz, false);
> +}
> +
> static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> size_t length, loff_t *pos)
> {
> @@ -289,6 +295,7 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
> return -EFAULT;
>
> + amd_pmf_hex_dump_pb(dev);
> ret = amd_pmf_start_policy_engine(dev);
> if (ret)
> return -EINVAL;
> @@ -327,6 +334,7 @@ static int amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
> }
>
> static void amd_pmf_remove_pb(struct amd_pmf_dev *dev) {}
> +static void amd_pmf_hex_dump_pb(struct amd_pmf_dev *dev) {}
> #endif
>
> static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
> @@ -341,6 +349,7 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
>
> memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
>
> + amd_pmf_hex_dump_pb(dev);
> if (pb_side_load)
> amd_pmf_open_pb(dev, dev->dbgfs_dir);
>
^ permalink raw reply
* Re: [PATCH v3 08/16] platform/x86/amd/pmf: Add support to update system state
From: Mario Limonciello @ 2023-10-10 16:03 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231010125917.138225-9-Shyam-sundar.S-k@amd.com>
On 10/10/2023 07:59, Shyam Sundar S K wrote:
> PMF driver based on the output actions from the TA can request to update
> the system states like entering s0i3, lock screen etc. by generating
> an uevent. Based on the udev rules set in the userspace the event id
> matching the uevent shall get updated accordingly using the systemctl.
>
> Sample udev rules under Documentation/admin-guide/pmf.rst.
>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
One minor nit below.
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> Documentation/admin-guide/index.rst | 1 +
> Documentation/admin-guide/pmf.rst | 25 +++++++++++++++++++
> drivers/platform/x86/amd/pmf/pmf.h | 9 +++++++
> drivers/platform/x86/amd/pmf/tee-if.c | 36 ++++++++++++++++++++++++++-
> 4 files changed, 70 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/admin-guide/pmf.rst
>
> diff --git a/Documentation/admin-guide/index.rst b/Documentation/admin-guide/index.rst
> index 43ea35613dfc..fb40a1f6f79e 100644
> --- a/Documentation/admin-guide/index.rst
> +++ b/Documentation/admin-guide/index.rst
> @@ -119,6 +119,7 @@ configure specific aspects of kernel behavior to your liking.
> parport
> perf-security
> pm/index
> + pmf
> pnp
> rapidio
> ras
> diff --git a/Documentation/admin-guide/pmf.rst b/Documentation/admin-guide/pmf.rst
> new file mode 100644
> index 000000000000..6985bb5b9452
> --- /dev/null
> +++ b/Documentation/admin-guide/pmf.rst
> @@ -0,0 +1,25 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Set udev rules for PMF Smart PC Builder
> +---------------------------------------
> +
> +AMD PMF(Platform Management Framework) Smart PC Solution builder has to set the system states
> +like S0i3, Screen lock, hibernate etc, based on the output actions provided by the PMF
> +TA (Trusted Application).
> +
> +In order for this to work the PMF driver generates a uevent for userspace to react to. Below are
> +sample udev rules that can facilitate this experience when a machine has PMF Smart PC solution builder
> +enabled.
> +
> +Please add the following line(s) to
> +``/etc/udev/rules.d/99-local.rules``::
> +
> + DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="0", RUN+="/usr/bin/systemctl suspend"
> + DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="1", RUN+="/usr/bin/systemctl hibernate"
> + DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="2", RUN+="/bin/loginctl lock-sessions"
> +
> +EVENT_ID values:
> +0= Put the system to S0i3/S2Idle
> +1= Put the system to hibernate
> +2= Lock the screen
> +
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 20f3e16b0a32..67f11113d5a7 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -73,6 +73,7 @@
> #define PMF_POLICY_STT_MIN 6
> #define PMF_POLICY_STT_SKINTEMP_APU 7
> #define PMF_POLICY_STT_SKINTEMP_HS2 8
> +#define PMF_POLICY_SYSTEM_STATE 9
> #define PMF_POLICY_P3T 38
>
> /* TA macros */
> @@ -440,6 +441,13 @@ struct apmf_dyn_slider_output {
> } __packed;
>
> /* Smart PC - TA internals */
I know that Ilpo had a comment about this in an earlier version that
there is a "__" instead of "_". I know this is intended behavior for
consistency with internal usage, but maybe it's worth having a comment
somewhere mentioning it's intended behavior? I'm not sure where.
> +enum system_state {
> + SYSTEM_STATE__S0i3,
> + SYSTEM_STATE__S4,
> + SYSTEM_STATE__SCREEN_LOCK,
> + SYSTEM_STATE__MAX
> +};
> +
> enum ta_slider {
> TA_BEST_BATTERY, /* Best Battery */
> TA_BETTER_BATTERY, /* Better Battery */
> @@ -471,6 +479,7 @@ enum ta_pmf_error_type {
> };
>
> struct pmf_action_table {
> + enum system_state system_state;
> u32 spl; /* in mW */
> u32 sppt; /* in mW */
> u32 sppt_apuonly; /* in mW */
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 92879ae4f8f0..c08ef13a1494 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -24,6 +24,20 @@ MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (defau
> static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
> 0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85, 0x43);
>
> +static const char *amd_pmf_uevent_as_str(unsigned int state)
> +{
> + switch (state) {
> + case SYSTEM_STATE__S0i3:
> + return "S0i3";
> + case SYSTEM_STATE__S4:
> + return "S4";
> + case SYSTEM_STATE__SCREEN_LOCK:
> + return "SCREEN_LOCK";
> + default:
> + return "Unknown Smart PC event";
> + }
> +}
> +
> static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
> struct tee_ioctl_invoke_arg *arg,
> struct tee_param *param)
> @@ -42,9 +56,23 @@ static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
> param[0].u.memref.shm_offs = 0;
> }
>
> +static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
> +{
> + char *envp[2] = {};
> +
> + envp[0] = kasprintf(GFP_KERNEL, "EVENT_ID=%d", event);
> + if (!envp[0])
> + return -EINVAL;
> +
> + kobject_uevent_env(&dev->dev->kobj, KOBJ_CHANGE, envp);
> +
> + kfree(envp[0]);
> + return 0;
> +}
> +
> static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
> {
> - u32 val;
> + u32 val, event = 0;
> int idx;
>
> for (idx = 0; idx < out->actions_count; idx++) {
> @@ -113,6 +141,12 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
> dev->prev_data->p3t_limit = val;
> }
> break;
> +
> + case PMF_POLICY_SYSTEM_STATE:
> + amd_pmf_update_uevents(dev, event);
> + dev_dbg(dev->dev, "update SYSTEM_STATE : %s\n",
> + amd_pmf_uevent_as_str(event));
> + break;
> }
> }
> }
^ permalink raw reply
* Re: [PATCH v3 04/16] platform/x86/amd/pmf: Add support for PMF Policy Binary
From: Mario Limonciello @ 2023-10-10 16:26 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <20231010125917.138225-5-Shyam-sundar.S-k@amd.com>
On 10/10/2023 07:59, Shyam Sundar S K wrote:
> 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 "request_module" 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 | 13 +++
> drivers/platform/x86/amd/pmf/pmf.h | 136 ++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/tee-if.c | 146 +++++++++++++++++++++++++-
> 5 files changed, 331 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..d0512af2cd42 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\n");
> + return status;
You're returning acpi_status here, but the return for the function is
int. It "happens to work" but I think it would be better to do
something like:
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 ffb78e9709d9..96a41e7d4e7d 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -395,6 +395,19 @@ static int amd_pmf_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> dev->dev = &pdev->dev;
> + err = apmf_check_smart_pc(dev);
> + if (!err) {
Rather than just failing to init smart PC solution builder, shouldn't
you fail probe entirely if an err is set from probing the BIOS
resources? This seems fairly fatal.
For example I'd think that setting up static slider is relatively
pointless on a system intending to use smart PC solution builder if
smart PC solution builder isn't working.
> + /*
> + * In order for Smart PC solution to work it has a hard dependency
> + * on the amdtee driver to be loaded first even before the PMF driver
> + * loads. PMF ASL has a _CRS method that advertises the existence
> + * of Smart PC bit. If this information is present, use this to
> + * explicitly probe the amdtee driver, so that "tee" plumbing is done
> + * before the PMF Smart PC init happens.
> + */
> + if (request_module("amdtee"))
> + pr_err("Failed to load amdtee. PMF Smart PC not enabled!\n");
Did that softdep thing Ilpo mentioned not work for modprobe? Hopefully
that generally works for everything except the insmod case so this code
path is unlikely to be hit in the wild.
> + }
>
> rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
> if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index a91c22d9b532..51c0e17f7720 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,129 @@ struct apmf_dyn_slider_output {
> struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
> } __packed;
>
> +/* Smart PC - TA internals */
> +enum ta_slider {
> + TA_BEST_BATTERY, /* Best Battery */
> + TA_BETTER_BATTERY, /* Better Battery */
> + TA_BETTER_PERFORMANCE, /* Better Performance */
> + TA_BEST_PERFORMANCE, /* Best Performance */
> + TA_MAX,
> +};
The comments above at end of the line don't add any value.
> +
> /* cmd 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 +595,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..38f02676261d 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,51 @@ 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 = 1;
> + /*
> + * Start collecting the data from PMFW 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);
> + 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 +280,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 +324,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);
> }
^ permalink raw reply
* Re: [PATCH RFT v7 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Duje Mihanović @ 2023-10-10 16:33 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <CAMRc=Mc7=E9bMQgiUM8qqk7UD4+exhJZqw2DucTcsnqHcttR3Q@mail.gmail.com>
On Tuesday, October 10, 2023 1:12:05 PM CEST Bartosz Golaszewski wrote:
> Gah! I should have noticed this earlier but this is a perfect
> candidate for using hogs. Can you use gpiod_add_hogs() from
> linux/gpio/machine.h instead? That would save you having the lookup
> and the static leds descriptor array.
From what I can tell, the hogs keep a certain pin at a certain state as long
as the machine is powered on. Is this really what we want to do with LEDs or
am I missing something?
Regards,
Duje
^ permalink raw reply
* Re: [PATCH RFT v7 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-10 17:39 UTC (permalink / raw)
To: Duje Mihanović
Cc: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <12313687.O9o76ZdvQC@radijator>
On Tue, Oct 10, 2023 at 6:33 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> On Tuesday, October 10, 2023 1:12:05 PM CEST Bartosz Golaszewski wrote:
> > Gah! I should have noticed this earlier but this is a perfect
> > candidate for using hogs. Can you use gpiod_add_hogs() from
> > linux/gpio/machine.h instead? That would save you having the lookup
> > and the static leds descriptor array.
>
> From what I can tell, the hogs keep a certain pin at a certain state as long
> as the machine is powered on. Is this really what we want to do with LEDs or
> am I missing something?
>
It doesn't seem like anyone is using these GPIOs once they're
requested? Wouldn't the above definitios be analogous to:
GPIO_HOG("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, GPIO_ACTIVE_HIGH, GPIOD_ASIS)
GPIO_HOG("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, GPIO_ACTIVE_HIGH, GPIOD_ASIS)
?
Bart
^ permalink raw reply
* [PATCH v4 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-10-10 18:48 UTC (permalink / raw)
To: linux-input, devicetree
Cc: Anshul Dalal, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shuah Khan, linux-kernel-mentees, linux-kernel
Adds bindings for the Adafruit Seesaw Gamepad.
The gamepad functions as an i2c device with the default address of 0x50
and has an IRQ pin that can be enabled in the driver to allow for a rising
edge trigger on each button press or joystick movement.
Product page:
https://www.adafruit.com/product/5743
Arduino driver:
https://github.com/adafruit/Adafruit_Seesaw
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---
Changes for v4:
- Fixed the URI for the id field
- Added `interrupts` property
Changes for v3:
- Updated id field to reflect updated file name from previous version
- Added `reg` property
Changes for v2:
- Renamed file to `adafruit,seesaw-gamepad.yaml`
- Removed quotes for `$id` and `$schema`
- Removed "Bindings for" from the description
- Changed node name to the generic name "joystick"
- Changed compatible to 'adafruit,seesaw-gamepad' instead of
'adafruit,seesaw_gamepad'
.../input/adafruit,seesaw-gamepad.yaml | 59 +++++++++++++++++++
1 file changed, 59 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
diff --git a/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
new file mode 100644
index 000000000000..e8e676006d2f
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/adafruit,seesaw-gamepad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Adafruit Mini I2C Gamepad with seesaw
+
+maintainers:
+ - Anshul Dalal <anshulusr@gmail.com>
+
+description: |
+ Adafruit Mini I2C Gamepad
+
+ +-----------------------------+
+ | ___ |
+ | / \ (X) |
+ | | S | __ __ (Y) (A) |
+ | \___/ |ST| |SE| (B) |
+ | |
+ +-----------------------------+
+
+ S -> 10-bit percision bidirectional analog joystick
+ ST -> Start
+ SE -> Select
+ X, A, B, Y -> Digital action buttons
+
+ Product page: https://www.adafruit.com/product/5743
+ Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
+
+properties:
+ compatible:
+ const: adafruit,seesaw-gamepad
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+ description:
+ The gamepad's IRQ pin triggers a rising edge if interrupts are enabled.
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ joystick@50 {
+ compatible = "adafruit,seesaw-gamepad";
+ reg = <0x50>;
+ };
+ };
--
2.42.0
^ permalink raw reply related
* [PATCH v4 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-10-10 18:48 UTC (permalink / raw)
To: linux-input, devicetree
Cc: Anshul Dalal, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shuah Khan, linux-kernel-mentees, linux-kernel
In-Reply-To: <20231010184827.1213507-1-anshulusr@gmail.com>
Adds a driver for a mini gamepad that communicates over i2c, the gamepad
has bidirectional thumb stick input and six buttons.
The gamepad chip utilizes the open framework from Adafruit called 'Seesaw'
to transmit the ADC data for the joystick and digital pin state for the
buttons. I have only implemented the functionality required to receive the
thumb stick and button state.
Steps in reading the gamepad state over i2c:
1. Reset the registers
2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input
`BUTTON_MASK`: A bit-map for the six digital pins internally
connected to the joystick buttons.
3. Enable internal pullup resistors for the `BUTTON_MASK`
4. Bulk set the pin state HIGH for `BUTTON_MASK`
5. Poll the device for button and joystick state done by:
`seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)`
Product page:
https://www.adafruit.com/product/5743
Arduino driver:
https://github.com/adafruit/Adafruit_Seesaw
Driver tested on RPi Zero 2W
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---
Changes for v4:
- Changed `1UL << BUTTON_` to BIT(BUTTON_)
- Removed `hardware_id` field from `struct seesaw_gamepad`
- Removed redundant checks for the number of bytes written and received by
`i2c_master_send` and `i2c_master_recv`
- Used `get_unaligned_be32` to instantiate `u32 result` from `read_buf`
- Changed `result & (1UL << BUTTON_)` to
`test_bit(BUTTON_, (long *)&result)`
- Changed `KBUILD_MODNAME` in id-tables to `SEESAW_DEVICE_NAME`
- Fixed formatting issues
- Changed button reporting:
Since the gamepad had the action buttons in a non-standard layout:
(X)
(Y) (A)
(B)
Therefore moved to using generic directional action button event codes
instead of BTN_[ABXY].
Changes for v3:
- no updates
Changes for v2:
adafruit-seesaw.c:
- Renamed file from 'adafruit_seesaw.c'
- Changed device name from 'seesaw_gamepad' to 'seesaw-gamepad'
- Changed count parameter for receiving joystick x on line 118:
`2` to `sizeof(write_buf)`
- Fixed invalid buffer size on line 123 and 126:
`data->y` to `sizeof(data->y)`
- Added comment for the `mdelay(10)` on line 169
- Changed inconsistent indentation on line 271
Kconfig:
- Fixed indentation for the help text
- Updated module name
Makefile:
- Updated module object file name
MAINTAINERS:
- Updated file name for the driver and bindings
MAINTAINERS | 7 +
drivers/input/joystick/Kconfig | 9 +
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/adafruit-seesaw.c | 269 +++++++++++++++++++++++
4 files changed, 286 insertions(+)
create mode 100644 drivers/input/joystick/adafruit-seesaw.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6c4cce45a09d..a314f9b48e21 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -441,6 +441,13 @@ W: http://wiki.analog.com/AD7879
W: https://ez.analog.com/linux-software-drivers
F: drivers/input/touchscreen/ad7879.c
+ADAFRUIT MINI I2C GAMEPAD
+M: Anshul Dalal <anshulusr@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
+F: drivers/input/joystick/adafruit-seesaw.c
+
ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
M: Jiri Kosina <jikos@kernel.org>
S: Maintained
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index ac6925ce8366..df9cd1830b29 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -412,4 +412,13 @@ config JOYSTICK_SENSEHAT
To compile this driver as a module, choose M here: the
module will be called sensehat_joystick.
+config JOYSTICK_SEESAW
+ tristate "Adafruit Mini I2C Gamepad with Seesaw"
+ depends on I2C
+ help
+ Say Y here if you want to use the Adafruit Mini I2C Gamepad.
+
+ To compile this driver as a module, choose M here: the module will be
+ called adafruit-seesaw.
+
endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 3937535f0098..9976f596a920 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_JOYSTICK_N64) += n64joy.o
obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
obj-$(CONFIG_JOYSTICK_PXRC) += pxrc.o
obj-$(CONFIG_JOYSTICK_QWIIC) += qwiic-joystick.o
+obj-$(CONFIG_JOYSTICK_SEESAW) += adafruit-seesaw.o
obj-$(CONFIG_JOYSTICK_SENSEHAT) += sensehat-joystick.o
obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
diff --git a/drivers/input/joystick/adafruit-seesaw.c b/drivers/input/joystick/adafruit-seesaw.c
new file mode 100644
index 000000000000..5cf10deb2372
--- /dev/null
+++ b/drivers/input/joystick/adafruit-seesaw.c
@@ -0,0 +1,269 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 Anshul Dalal <anshulusr@gmail.com>
+ *
+ * Driver for Adafruit Mini I2C Gamepad
+ *
+ * Based on the work of:
+ * Oleh Kravchenko (Sparkfun Qwiic Joystick driver)
+ *
+ * Product page: https://www.adafruit.com/product/5743
+ * Firmware and hardware sources: https://github.com/adafruit/Adafruit_Seesaw
+ */
+
+#include <asm-generic/unaligned.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+/* clang-format off */
+#define SEESAW_DEVICE_NAME "seesaw-gamepad"
+
+#define SEESAW_STATUS_BASE 0
+#define SEESAW_GPIO_BASE 1
+#define SEESAW_ADC_BASE 9
+
+#define SEESAW_GPIO_DIRCLR_BULK 3
+#define SEESAW_GPIO_BULK 4
+#define SEESAW_GPIO_BULK_SET 5
+#define SEESAW_GPIO_PULLENSET 11
+
+#define SEESAW_STATUS_HW_ID 1
+#define SEESAW_STATUS_SWRST 127
+
+#define SEESAW_ADC_OFFSET 7
+
+#define BUTTON_A 5
+#define BUTTON_B 1
+#define BUTTON_X 6
+#define BUTTON_Y 2
+#define BUTTON_START 16
+#define BUTTON_SELECT 0
+
+#define ANALOG_X 14
+#define ANALOG_Y 15
+
+#define SEESAW_JOYSTICK_MAX_AXIS 1023
+#define SEESAW_JOYSTICK_FUZZ 2
+#define SEESAW_JOYSTICK_FLAT 4
+
+#define SEESAW_GAMEPAD_POLL_INTERVAL 16
+#define SEESAW_GAMEPAD_POLL_MIN 8
+#define SEESAW_GAMEPAD_POLL_MAX 32
+/* clang-format on */
+
+u32 BUTTON_MASK = BIT(BUTTON_A) | BIT(BUTTON_B) | BIT(BUTTON_X) |
+ BIT(BUTTON_Y) | BIT(BUTTON_START) | BIT(BUTTON_SELECT);
+
+struct seesaw_gamepad {
+ char physical_path[32];
+ struct input_dev *input_dev;
+ struct i2c_client *i2c_client;
+};
+
+struct seesaw_data {
+ __be16 x;
+ __be16 y;
+ u8 button_a, button_b, button_x, button_y, button_start, button_select;
+};
+
+static int seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)
+{
+ int err;
+ unsigned char write_buf[2] = { SEESAW_GPIO_BASE, SEESAW_GPIO_BULK };
+ unsigned char read_buf[4];
+
+ err = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (err < 0)
+ return err;
+ err = i2c_master_recv(client, read_buf, sizeof(read_buf));
+ if (err < 0)
+ return err;
+
+ u32 result = get_unaligned_be32(&read_buf);
+
+ data->button_a = !test_bit(BUTTON_A, (long *)&result);
+ data->button_b = !test_bit(BUTTON_B, (long *)&result);
+ data->button_x = !test_bit(BUTTON_X, (long *)&result);
+ data->button_y = !test_bit(BUTTON_Y, (long *)&result);
+ data->button_start = !test_bit(BUTTON_START, (long *)&result);
+ data->button_select = !test_bit(BUTTON_SELECT, (long *)&result);
+
+ write_buf[0] = SEESAW_ADC_BASE;
+ write_buf[1] = SEESAW_ADC_OFFSET + ANALOG_X;
+ err = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (err < 0)
+ return err;
+ err = i2c_master_recv(client, (char *)&data->x, sizeof(data->x));
+ if (err < 0)
+ return err;
+ /*
+ * ADC reads left as max and right as 0, must be reversed since kernel
+ * expects reports in opposite order.
+ */
+ data->x = SEESAW_JOYSTICK_MAX_AXIS - be16_to_cpu(data->x);
+
+ write_buf[1] = SEESAW_ADC_OFFSET + ANALOG_Y;
+ err = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (err < 0)
+ return err;
+ err = i2c_master_recv(client, (char *)&data->y, sizeof(data->y));
+ if (err < 0)
+ return err;
+ data->y = be16_to_cpu(data->y);
+
+ return 0;
+}
+
+static void seesaw_poll(struct input_dev *input)
+{
+ struct seesaw_gamepad *private = input_get_drvdata(input);
+ struct seesaw_data data;
+ int err;
+
+ err = seesaw_read_data(private->i2c_client, &data);
+ if (err != 0)
+ return;
+
+ input_report_abs(input, ABS_X, data.x);
+ input_report_abs(input, ABS_Y, data.y);
+ input_report_key(input, BTN_EAST, data.button_a);
+ input_report_key(input, BTN_SOUTH, data.button_b);
+ input_report_key(input, BTN_NORTH, data.button_x);
+ input_report_key(input, BTN_WEST, data.button_y);
+ input_report_key(input, BTN_START, data.button_start);
+ input_report_key(input, BTN_SELECT, data.button_select);
+ input_sync(input);
+}
+
+static int seesaw_probe(struct i2c_client *client)
+{
+ int err;
+ struct seesaw_gamepad *private;
+ unsigned char register_reset[] = { SEESAW_STATUS_BASE,
+ SEESAW_STATUS_SWRST, 0xFF };
+ unsigned char get_hw_id[] = { SEESAW_STATUS_BASE, SEESAW_STATUS_HW_ID };
+
+ err = i2c_master_send(client, register_reset, sizeof(register_reset));
+ if (err < 0)
+ return err;
+
+ /* Wait for the registers to reset before proceeding */
+ mdelay(10);
+
+ private = devm_kzalloc(&client->dev, sizeof(*private), GFP_KERNEL);
+ if (!private)
+ return -ENOMEM;
+
+ err = i2c_master_send(client, get_hw_id, sizeof(get_hw_id));
+ if (err < 0)
+ return err;
+
+ unsigned char hardware_id;
+
+ err = i2c_master_recv(client, &hardware_id, 1);
+ if (err < 0)
+ return err;
+
+ dev_dbg(&client->dev, "Adafruit Seesaw Gamepad, Hardware ID: %02x\n",
+ hardware_id);
+
+ private->i2c_client = client;
+ scnprintf(private->physical_path, sizeof(private->physical_path),
+ "i2c/%s", dev_name(&client->dev));
+ i2c_set_clientdata(client, private);
+
+ private->input_dev = devm_input_allocate_device(&client->dev);
+ if (!private->input_dev)
+ return -ENOMEM;
+
+ private->input_dev->id.bustype = BUS_I2C;
+ private->input_dev->name = "Adafruit Seesaw Gamepad";
+ private->input_dev->phys = private->physical_path;
+ input_set_drvdata(private->input_dev, private);
+ input_set_abs_params(private->input_dev, ABS_X, 0,
+ SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+ SEESAW_JOYSTICK_FLAT);
+ input_set_abs_params(private->input_dev, ABS_Y, 0,
+ SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+ SEESAW_JOYSTICK_FLAT);
+ input_set_capability(private->input_dev, EV_KEY, BTN_EAST);
+ input_set_capability(private->input_dev, EV_KEY, BTN_SOUTH);
+ input_set_capability(private->input_dev, EV_KEY, BTN_NORTH);
+ input_set_capability(private->input_dev, EV_KEY, BTN_WEST);
+ input_set_capability(private->input_dev, EV_KEY, BTN_START);
+ input_set_capability(private->input_dev, EV_KEY, BTN_SELECT);
+
+ err = input_setup_polling(private->input_dev, seesaw_poll);
+ if (err) {
+ dev_err(&client->dev, "failed to set up polling: %d\n", err);
+ return err;
+ }
+
+ input_set_poll_interval(private->input_dev,
+ SEESAW_GAMEPAD_POLL_INTERVAL);
+ input_set_max_poll_interval(private->input_dev,
+ SEESAW_GAMEPAD_POLL_MAX);
+ input_set_min_poll_interval(private->input_dev,
+ SEESAW_GAMEPAD_POLL_MIN);
+
+ err = input_register_device(private->input_dev);
+ if (err) {
+ dev_err(&client->dev, "failed to register joystick: %d\n", err);
+ return err;
+ }
+
+ /* Set Pin Mode to input and enable pull-up resistors */
+ unsigned char pin_mode[] = { SEESAW_GPIO_BASE, SEESAW_GPIO_DIRCLR_BULK,
+ BUTTON_MASK >> 24, BUTTON_MASK >> 16,
+ BUTTON_MASK >> 8, BUTTON_MASK };
+ err = i2c_master_send(client, pin_mode, sizeof(pin_mode));
+ if (err < 0)
+ return err;
+ pin_mode[1] = SEESAW_GPIO_PULLENSET;
+ err = i2c_master_send(client, pin_mode, sizeof(pin_mode));
+ if (err < 0)
+ return err;
+ pin_mode[1] = SEESAW_GPIO_BULK_SET;
+ err = i2c_master_send(client, pin_mode, sizeof(pin_mode));
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id of_seesaw_match[] = {
+ {
+ .compatible = "adafruit,seesaw-gamepad",
+ },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_seesaw_match);
+#endif /* CONFIG_OF */
+
+/* clang-format off */
+static const struct i2c_device_id seesaw_id_table[] = {
+ { SEESAW_DEVICE_NAME, 0 },
+ { /* Sentinel */ }
+};
+/* clang-format on */
+
+MODULE_DEVICE_TABLE(i2c, seesaw_id_table);
+
+static struct i2c_driver seesaw_driver = {
+ .driver = {
+ .name = SEESAW_DEVICE_NAME,
+ .of_match_table = of_match_ptr(of_seesaw_match),
+ },
+ .id_table = seesaw_id_table,
+ .probe = seesaw_probe,
+};
+module_i2c_driver(seesaw_driver);
+
+MODULE_AUTHOR("Anshul Dalal <anshulusr@gmail.com>");
+MODULE_DESCRIPTION("Adafruit Mini I2C Gamepad driver");
+MODULE_LICENSE("GPL");
--
2.42.0
^ permalink raw reply related
* Re: [PATCH v4 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
From: Thomas Weißschuh @ 2023-10-10 19:04 UTC (permalink / raw)
To: Anshul Dalal
Cc: linux-input, devicetree, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shuah Khan,
linux-kernel-mentees, linux-kernel
In-Reply-To: <20231010184827.1213507-2-anshulusr@gmail.com>
On 2023-10-11 00:18:24+0530, Anshul Dalal wrote:
> Adds a driver for a mini gamepad that communicates over i2c, the gamepad
> has bidirectional thumb stick input and six buttons.
>
> The gamepad chip utilizes the open framework from Adafruit called 'Seesaw'
> to transmit the ADC data for the joystick and digital pin state for the
> buttons. I have only implemented the functionality required to receive the
> thumb stick and button state.
>
> Steps in reading the gamepad state over i2c:
> 1. Reset the registers
> 2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input
> `BUTTON_MASK`: A bit-map for the six digital pins internally
> connected to the joystick buttons.
> 3. Enable internal pullup resistors for the `BUTTON_MASK`
> 4. Bulk set the pin state HIGH for `BUTTON_MASK`
> 5. Poll the device for button and joystick state done by:
> `seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)`
>
> Product page:
> https://www.adafruit.com/product/5743
> Arduino driver:
> https://github.com/adafruit/Adafruit_Seesaw
>
> Driver tested on RPi Zero 2W
>
> Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
In general:
Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>
But one more nitpick below:
> [..]
> +static void seesaw_poll(struct input_dev *input)
> +{
> + struct seesaw_gamepad *private = input_get_drvdata(input);
> + struct seesaw_data data;
> + int err;
> +
> + err = seesaw_read_data(private->i2c_client, &data);
> + if (err != 0)
> + return;
This should be logged at debug level, so the user has some sort of
chance to figure out if something is broken.
> +
> + input_report_abs(input, ABS_X, data.x);
> + input_report_abs(input, ABS_Y, data.y);
> + input_report_key(input, BTN_EAST, data.button_a);
> + input_report_key(input, BTN_SOUTH, data.button_b);
> + input_report_key(input, BTN_NORTH, data.button_x);
> + input_report_key(input, BTN_WEST, data.button_y);
> + input_report_key(input, BTN_START, data.button_start);
> + input_report_key(input, BTN_SELECT, data.button_select);
> + input_sync(input);
> +}
> [..]
^ permalink raw reply
* Re: [PATCH RFT v7 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Linus Walleij @ 2023-10-10 20:04 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Duje Mihanović, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Russell King, Alan Stern, Greg Kroah-Hartman, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <CAMRc=MdWYNmBkJ6Nw6V_FzJKQw--g02tjLSztMYW_atNhisVpw@mail.gmail.com>
On Tue, Oct 10, 2023 at 7:39 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> It doesn't seem like anyone is using these GPIOs once they're
> requested? Wouldn't the above definitios be analogous to:
>
> GPIO_HOG("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, GPIO_ACTIVE_HIGH, GPIOD_ASIS)
> GPIO_HOG("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, GPIO_ACTIVE_HIGH, GPIOD_ASIS)
They are used:
+ spitz_gpio_leds[0].gpiod = leds->desc[0];
+ spitz_gpio_leds[1].gpiod = leds->desc[1];
The descriptors are passed to the leds-gpio driver.
But wait: no.
This whole thing:
+static struct gpio_descs *leds;
+
(...)
+ leds = gpiod_get_array_optional(&spitz_led_device.dev,
+ NULL, GPIOD_ASIS);
+ spitz_gpio_leds[0].gpiod = leds->desc[0];
+ spitz_gpio_leds[1].gpiod = leds->desc[1];
Just delete all that.
The leds-gpio driver will request and use the lines.
It was just so unorthodox that I missed it. Adding the descriptor
table is enough.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v3 08/16] platform/x86/amd/pmf: Add support to update system state
From: Shyam Sundar S K @ 2023-10-11 3:05 UTC (permalink / raw)
To: Mario Limonciello, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <0345caec-ccbc-4c54-a095-4fcef24fee25@amd.com>
On 10/10/2023 9:33 PM, Mario Limonciello wrote:
> On 10/10/2023 07:59, Shyam Sundar S K wrote:
>> PMF driver based on the output actions from the TA can request to
>> update
>> the system states like entering s0i3, lock screen etc. by generating
>> an uevent. Based on the udev rules set in the userspace the event id
>> matching the uevent shall get updated accordingly using the systemctl.
>>
>> Sample udev rules under Documentation/admin-guide/pmf.rst.
>>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>
> One minor nit below.
>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>
>> ---
>> Documentation/admin-guide/index.rst | 1 +
>> Documentation/admin-guide/pmf.rst | 25 +++++++++++++++++++
>> drivers/platform/x86/amd/pmf/pmf.h | 9 +++++++
>> drivers/platform/x86/amd/pmf/tee-if.c | 36
>> ++++++++++++++++++++++++++-
>> 4 files changed, 70 insertions(+), 1 deletion(-)
>> create mode 100644 Documentation/admin-guide/pmf.rst
>>
>> diff --git a/Documentation/admin-guide/index.rst
>> b/Documentation/admin-guide/index.rst
>> index 43ea35613dfc..fb40a1f6f79e 100644
>> --- a/Documentation/admin-guide/index.rst
>> +++ b/Documentation/admin-guide/index.rst
>> @@ -119,6 +119,7 @@ configure specific aspects of kernel behavior to
>> your liking.
>> parport
>> perf-security
>> pm/index
>> + pmf
>> pnp
>> rapidio
>> ras
>> diff --git a/Documentation/admin-guide/pmf.rst
>> b/Documentation/admin-guide/pmf.rst
>> new file mode 100644
>> index 000000000000..6985bb5b9452
>> --- /dev/null
>> +++ b/Documentation/admin-guide/pmf.rst
>> @@ -0,0 +1,25 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +Set udev rules for PMF Smart PC Builder
>> +---------------------------------------
>> +
>> +AMD PMF(Platform Management Framework) Smart PC Solution builder
>> has to set the system states
>> +like S0i3, Screen lock, hibernate etc, based on the output actions
>> provided by the PMF
>> +TA (Trusted Application).
>> +
>> +In order for this to work the PMF driver generates a uevent for
>> userspace to react to. Below are
>> +sample udev rules that can facilitate this experience when a
>> machine has PMF Smart PC solution builder
>> +enabled.
>> +
>> +Please add the following line(s) to
>> +``/etc/udev/rules.d/99-local.rules``::
>> +
>> + DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="0",
>> RUN+="/usr/bin/systemctl suspend"
>> + DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="1",
>> RUN+="/usr/bin/systemctl hibernate"
>> + DRIVERS=="amd-pmf", ACTION=="change", ENV{EVENT_ID}=="2",
>> RUN+="/bin/loginctl lock-sessions"
>> +
>> +EVENT_ID values:
>> +0= Put the system to S0i3/S2Idle
>> +1= Put the system to hibernate
>> +2= Lock the screen
>> +
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>> b/drivers/platform/x86/amd/pmf/pmf.h
>> index 20f3e16b0a32..67f11113d5a7 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -73,6 +73,7 @@
>> #define PMF_POLICY_STT_MIN 6
>> #define PMF_POLICY_STT_SKINTEMP_APU 7
>> #define PMF_POLICY_STT_SKINTEMP_HS2 8
>> +#define PMF_POLICY_SYSTEM_STATE 9
>> #define PMF_POLICY_P3T 38
>> /* TA macros */
>> @@ -440,6 +441,13 @@ struct apmf_dyn_slider_output {
>> } __packed;
>> /* Smart PC - TA internals */
>
> I know that Ilpo had a comment about this in an earlier version that
> there is a "__" instead of "_". I know this is intended behavior for
> consistency with internal usage, but maybe it's worth having a comment
> somewhere mentioning it's intended behavior? I'm not sure where.
>
I missed to change here. I have changed at other places too.
>> +enum system_state {
>> + SYSTEM_STATE__S0i3,
>> + SYSTEM_STATE__S4,
>> + SYSTEM_STATE__SCREEN_LOCK,
>> + SYSTEM_STATE__MAX
>> +};
>> +
>> enum ta_slider {
>> TA_BEST_BATTERY, /* Best Battery */
>> TA_BETTER_BATTERY, /* Better Battery */
>> @@ -471,6 +479,7 @@ enum ta_pmf_error_type {
>> };
>> struct pmf_action_table {
>> + enum system_state system_state;
>> u32 spl; /* in mW */
>> u32 sppt; /* in mW */
>> u32 sppt_apuonly; /* in mW */
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
>> b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 92879ae4f8f0..c08ef13a1494 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -24,6 +24,20 @@ MODULE_PARM_DESC(pb_actions_ms, "Policy binary
>> actions sampling frequency (defau
>> static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77,
>> 0x3fb8, 0x524d,
>> 0xb1, 0x2d, 0xc5, 0x29, 0xb1, 0x3d, 0x85,
>> 0x43);
>> +static const char *amd_pmf_uevent_as_str(unsigned int state)
>> +{
>> + switch (state) {
>> + case SYSTEM_STATE__S0i3:
>> + return "S0i3";
>> + case SYSTEM_STATE__S4:
>> + return "S4";
>> + case SYSTEM_STATE__SCREEN_LOCK:
>> + return "SCREEN_LOCK";
>> + default:
>> + return "Unknown Smart PC event";
>> + }
>> +}
>> +
>> static void amd_pmf_prepare_args(struct amd_pmf_dev *dev, int cmd,
>> struct tee_ioctl_invoke_arg *arg,
>> struct tee_param *param)
>> @@ -42,9 +56,23 @@ static void amd_pmf_prepare_args(struct
>> amd_pmf_dev *dev, int cmd,
>> param[0].u.memref.shm_offs = 0;
>> }
>> +static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16
>> event)
>> +{
>> + char *envp[2] = {};
>> +
>> + envp[0] = kasprintf(GFP_KERNEL, "EVENT_ID=%d", event);
>> + if (!envp[0])
>> + return -EINVAL;
>> +
>> + kobject_uevent_env(&dev->dev->kobj, KOBJ_CHANGE, envp);
>> +
>> + kfree(envp[0]);
>> + return 0;
>> +}
>> +
>> static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct
>> ta_pmf_enact_result *out)
>> {
>> - u32 val;
>> + u32 val, event = 0;
>> int idx;
>> for (idx = 0; idx < out->actions_count; idx++) {
>> @@ -113,6 +141,12 @@ static void amd_pmf_apply_policies(struct
>> amd_pmf_dev *dev, struct ta_pmf_enact_
>> dev->prev_data->p3t_limit = val;
>> }
>> break;
>> +
>> + case PMF_POLICY_SYSTEM_STATE:
>> + amd_pmf_update_uevents(dev, event);
>> + dev_dbg(dev->dev, "update SYSTEM_STATE : %s\n",
>> + amd_pmf_uevent_as_str(event));
>> + break;
>> }
>> }
>> }
>
^ permalink raw reply
* Re: [PATCH v3 04/16] platform/x86/amd/pmf: Add support for PMF Policy Binary
From: Shyam Sundar S K @ 2023-10-11 3:17 UTC (permalink / raw)
To: Mario Limonciello, hdegoede, markgross, ilpo.jarvinen,
basavaraj.natikar, jikos, benjamin.tissoires, alexander.deucher,
christian.koenig, Xinhui.Pan, airlied, daniel
Cc: Patil.Reddy, platform-driver-x86, linux-input, amd-gfx, dri-devel
In-Reply-To: <db29fe6f-1ac1-4e8f-9f77-f3569c503a76@amd.com>
On 10/10/2023 9:56 PM, Mario Limonciello wrote:
> On 10/10/2023 07:59, Shyam Sundar S K wrote:
>> 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 "request_module" 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 | 13 +++
>> drivers/platform/x86/amd/pmf/pmf.h | 136 ++++++++++++++++++++++++
>> drivers/platform/x86/amd/pmf/tee-if.c | 146
>> +++++++++++++++++++++++++-
>> 5 files changed, 331 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..d0512af2cd42 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\n");
>> + return status;
>
> You're returning acpi_status here, but the return for the function is
> int. It "happens to work" but I think it would be better to do
> something like:
>
> dev_err(pmf_dev->dev, "acpi_walk_resources failed: %d\n, status);
> return -EINVAL;
OK, I will change this.
>
>> + }
>> +
>> + 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 ffb78e9709d9..96a41e7d4e7d 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -395,6 +395,19 @@ static int amd_pmf_probe(struct platform_device
>> *pdev)
>> return -ENOMEM;
>> dev->dev = &pdev->dev;
>> + err = apmf_check_smart_pc(dev);
>> + if (!err) {
>
> Rather than just failing to init smart PC solution builder, shouldn't
> you fail probe entirely if an err is set from probing the BIOS
> resources? This seems fairly fatal.
>
> For example I'd think that setting up static slider is relatively
> pointless on a system intending to use smart PC solution builder if
> smart PC solution builder isn't working.
>
What if - the BIOS advertises the smart PC bit and forgets to add
resources (or a buggy ACPI thing)?
Atleast that way if the static slider is enabled, atleast some amount
of power saving can happen.
Note that, I have tried the fallback to static slider if the smart pc
fails and that's working with the current we have in place.
>> + /*
>> + * In order for Smart PC solution to work it has a hard
>> dependency
>> + * on the amdtee driver to be loaded first even before the
>> PMF driver
>> + * loads. PMF ASL has a _CRS method that advertises the
>> existence
>> + * of Smart PC bit. If this information is present, use
>> this to
>> + * explicitly probe the amdtee driver, so that "tee"
>> plumbing is done
>> + * before the PMF Smart PC init happens.
>> + */
>> + if (request_module("amdtee"))
>> + pr_err("Failed to load amdtee. PMF Smart PC not
>> enabled!\n");
>
> Did that softdep thing Ilpo mentioned not work for modprobe?
> Hopefully that generally works for everything except the insmod case
> so this code path is unlikely to be hit in the wild.
>
unfortunately no. Hence that's the reason I had to retain back the
request_module() here.
>> + }
>> rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>> if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
>> b/drivers/platform/x86/amd/pmf/pmf.h
>> index a91c22d9b532..51c0e17f7720 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,129 @@ struct apmf_dyn_slider_output {
>> struct apmf_cnqf_power_set ps[APMF_CNQF_MAX];
>> } __packed;
>> +/* Smart PC - TA internals */
>> +enum ta_slider {
>> + TA_BEST_BATTERY, /* Best Battery */
>> + TA_BETTER_BATTERY, /* Better Battery */
>> + TA_BETTER_PERFORMANCE, /* Better Performance */
>> + TA_BEST_PERFORMANCE, /* Best Performance */
>> + TA_MAX,
>> +};
>
> The comments above at end of the line don't add any value.
OK.
Thanks,
Shyam
>
>> +
>> /* cmd 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 +595,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..38f02676261d 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,51 @@ 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 = 1;
>> + /*
>> + * Start collecting the data from PMFW 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);
>> + 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 +280,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 +324,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);
>> }
>
^ permalink raw reply
* Re: uinput: waiting for UI_FF_UPLOAD events will not inform user when allocation is required
From: John Salamon @ 2023-10-11 6:40 UTC (permalink / raw)
To: dmitry.torokhov, rydberg; +Cc: linux-input, linux-kernel
In-Reply-To: <CA+fyA4RABYNPZZSk9+9U51u53kbSzqgwdi1KDDGRxXi8q5TtxQ@mail.gmail.com>
Correction, "old" is a pointer to struct ff_effect, which after being
set to NULL looks like it gets pushed out by uinput on a struct
uinput_request.
On Tue, Oct 10, 2023 at 5:38 PM John Salamon <salamonj9@gmail.com> wrote:
>
> Currently the "fake" input events generated by uinput in response to
> effect uploads will return an effect with an id that has already been
> handled by input_ff_upload in ff-core.c, which can modify the effect
> id. This causes a problem specifically when the effect originally
> uploaded via the EVIOCSFF ioctl contained an effect with -1, as the
> userspace code handling UI_FF_UPLOAD receives an effect with an id
> other than -1, and therefore will not know an allocation was
> requested.
>
> I notice that the "old" field on the ff_effect struct is set to NULL
> when the -1 id is changed (in input_ff_upload), which can serve as a
> flag that an allocation was requested. If it is the intention is that
> uinput users check if old == NULL to know when allocations are needed
> I think uinput documentation should describe this.
>
> I first noticed this using python-evdev, see my issue report here:
> https://github.com/gvalkov/python-evdev/issues/199
^ permalink raw reply
* Re: [PATCH RFT v7 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Bartosz Golaszewski @ 2023-10-11 7:59 UTC (permalink / raw)
To: Linus Walleij
Cc: Duje Mihanović, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
Russell King, Alan Stern, Greg Kroah-Hartman, Andy Shevchenko,
Dmitry Torokhov, Mark Brown, linux-arm-kernel, linux-kernel,
linux-usb, linux-gpio, linux-input, linux-spi
In-Reply-To: <CACRpkda4ZeQ8eYKqXBR7XmWj9jJF58C+PLeRWqx2m7aSf2FWQw@mail.gmail.com>
On Tue, Oct 10, 2023 at 10:04 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Oct 10, 2023 at 7:39 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > It doesn't seem like anyone is using these GPIOs once they're
> > requested? Wouldn't the above definitios be analogous to:
> >
> > GPIO_HOG("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, GPIO_ACTIVE_HIGH, GPIOD_ASIS)
> > GPIO_HOG("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, GPIO_ACTIVE_HIGH, GPIOD_ASIS)
>
> They are used:
> + spitz_gpio_leds[0].gpiod = leds->desc[0];
> + spitz_gpio_leds[1].gpiod = leds->desc[1];
>
> The descriptors are passed to the leds-gpio driver.
>
> But wait: no.
>
> This whole thing:
>
> +static struct gpio_descs *leds;
> +
> (...)
> + leds = gpiod_get_array_optional(&spitz_led_device.dev,
> + NULL, GPIOD_ASIS);
> + spitz_gpio_leds[0].gpiod = leds->desc[0];
> + spitz_gpio_leds[1].gpiod = leds->desc[1];
>
> Just delete all that.
>
> The leds-gpio driver will request and use the lines.
>
> It was just so unorthodox that I missed it. Adding the descriptor
> table is enough.
Ah, good catch. Your suggestion is of course the correct one.
Bart
>
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [RFC PATCH] Input: omap4-keypad: react on keypresses if device is runtime-suspended
From: Tony Lindgren @ 2023-10-11 10:07 UTC (permalink / raw)
To: Andreas Kemnade
Cc: dmitry.torokhov, Jonathan.Cameron, robh, frank.li, linux-input,
linux-kernel, linux-omap
In-Reply-To: <20230929200046.2300333-1-andreas@kemnade.info>
Hi,
* Andreas Kemnade <andreas@kemnade.info> [230929 23:01]:
> According to SWPU235AB, table 26-6, fclk is required to generate events
> at least on OMAP4460, so keep fclk enabled all the time the device
> is opened.
Sorry for the delay, the patch looks good to me:
Reviewed-by: Tony Lindgren <tony@atomide.com>
> Open questions:
> - Should we rather (or also use) padconf irqs?
The keypad is in the wakeup domain and has SYSC_OMAP2_ENAWAKEUP set in
the dts, so padconf irqs should not be needed as long as the device is
clocked. As the source clock is sys_32k_ck, not sure if gating it would
save any power, so padconf interrupts are probably not needed.
> - It seems not to be required everywhere. Does it harm somewhere?
Sounds like on 4430 the gate bit does not do anything while on 4460 it
does. The source clock seems to be sys_32k_ck so it's enabled anyways
and should not affect power management.
Regards,
Tony
^ permalink raw reply
* [PATCH RFT v8 2/6] ARM: pxa: Convert Spitz LEDs to GPIO descriptors
From: Duje Mihanović @ 2023-10-11 13:23 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović
In-Reply-To: <20231011-pxa-gpio-v8-0-eed08a0fcac8@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for configuring
its two onboard LEDs.
Convert them to use the GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 535e2b2e997b..9efd603c715a 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -452,16 +452,25 @@ static inline void spitz_keys_init(void) {}
* LEDs
******************************************************************************/
#if defined(CONFIG_LEDS_GPIO) || defined(CONFIG_LEDS_GPIO_MODULE)
+static struct gpiod_lookup_table spitz_led_gpio_table = {
+ .dev_id = "leds-gpio",
+ .table = {
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_ORANGE, NULL, 0,
+ GPIO_ACTIVE_HIGH),
+ GPIO_LOOKUP_IDX("pxa-gpio", SPITZ_GPIO_LED_GREEN, NULL, 1,
+ GPIO_ACTIVE_HIGH),
+ { }
+ }
+};
+
static struct gpio_led spitz_gpio_leds[] = {
{
.name = "spitz:amber:charge",
.default_trigger = "sharpsl-charge",
- .gpio = SPITZ_GPIO_LED_ORANGE,
},
{
.name = "spitz:green:hddactivity",
.default_trigger = "disk-activity",
- .gpio = SPITZ_GPIO_LED_GREEN,
},
};
@@ -480,6 +489,7 @@ static struct platform_device spitz_led_device = {
static void __init spitz_leds_init(void)
{
+ gpiod_add_lookup_table(&spitz_led_gpio_table);
platform_device_register(&spitz_led_device);
}
#else
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v8 3/6] ARM: pxa: Convert Spitz CF power control to GPIO descriptors
From: Duje Mihanović @ 2023-10-11 13:23 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
In-Reply-To: <20231011-pxa-gpio-v8-0-eed08a0fcac8@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for controlling
the power supply to its CF and SD card slots.
Convert it to use the GPIO descriptor interface.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 9efd603c715a..b72c2916426a 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -133,6 +133,10 @@ static unsigned long spitz_pin_config[] __initdata = {
* Scoop GPIO expander
******************************************************************************/
#if defined(CONFIG_SHARP_SCOOP) || defined(CONFIG_SHARP_SCOOP_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_card_pwr_ctrl_gpio_table, "pxa2xx-mci.0",
+ "sharp-scoop", SPITZ_GPIO_CF_POWER, "cf_power",
+ GPIO_ACTIVE_HIGH);
+
/* SCOOP Device #1 */
static struct resource spitz_scoop_1_resources[] = {
[0] = {
@@ -190,6 +194,7 @@ struct platform_device spitz_scoop_2_device = {
static void __init spitz_scoop_init(void)
{
platform_device_register(&spitz_scoop_1_device);
+ gpiod_add_lookup_table(&spitz_card_pwr_ctrl_gpio_table);
/* Akita doesn't have the second SCOOP chip */
if (!machine_is_akita())
@@ -201,9 +206,18 @@ static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
{
unsigned short cpr;
unsigned long flags;
+ struct gpio_desc *cf_power;
+
+ cf_power = gpiod_get(&pxa_device_mci.dev, "cf_power", GPIOD_ASIS);
+ if (IS_ERR(cf_power)) {
+ dev_err(&pxa_device_mci.dev,
+ "failed to get power control GPIO with %ld\n",
+ PTR_ERR(cf_power));
+ return;
+ }
if (new_cpr & 0x7) {
- gpio_set_value(SPITZ_GPIO_CF_POWER, 1);
+ gpiod_direction_output(cf_power, 1);
mdelay(5);
}
@@ -222,8 +236,10 @@ static void __maybe_unused spitz_card_pwr_ctrl(uint8_t enable, uint8_t new_cpr)
if (!(cpr & 0x7)) {
mdelay(1);
- gpio_set_value(SPITZ_GPIO_CF_POWER, 0);
+ gpiod_direction_output(cf_power, 0);
}
+
+ gpiod_put(cf_power);
}
#else
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v8 1/6] ARM: pxa: Convert Spitz OHCI to GPIO descriptors
From: Duje Mihanović @ 2023-10-11 13:23 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
In-Reply-To: <20231011-pxa-gpio-v8-0-eed08a0fcac8@skole.hr>
Sharp's Spitz board still uses the legacy GPIO interface for controlling
a GPIO pin related to the USB host controller.
Convert this function to use the new GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/spitz.c | 13 ++++++-------
drivers/usb/host/ohci-pxa27x.c | 5 +++++
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index cc691b199429..535e2b2e997b 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -649,23 +649,22 @@ static inline void spitz_mmc_init(void) {}
* USB Host
******************************************************************************/
#if defined(CONFIG_USB_OHCI_HCD) || defined(CONFIG_USB_OHCI_HCD_MODULE)
+GPIO_LOOKUP_SINGLE(spitz_usb_host_gpio_table, "pxa27x-ohci", "gpio-pxa",
+ SPITZ_GPIO_USB_HOST, "usb-host", GPIO_ACTIVE_LOW);
+
static int spitz_ohci_init(struct device *dev)
{
- int err;
-
- err = gpio_request(SPITZ_GPIO_USB_HOST, "USB_HOST");
- if (err)
- return err;
+ gpiod_add_lookup_table(&spitz_usb_host_gpio_table);
/* Only Port 2 is connected, setup USB Port 2 Output Control Register */
UP2OCR = UP2OCR_HXS | UP2OCR_HXOE | UP2OCR_DPPDE | UP2OCR_DMPDE;
- return gpio_direction_output(SPITZ_GPIO_USB_HOST, 1);
+ return 0;
}
static void spitz_ohci_exit(struct device *dev)
{
- gpio_free(SPITZ_GPIO_USB_HOST);
+ gpiod_remove_lookup_table(&spitz_usb_host_gpio_table);
}
static struct pxaohci_platform_data spitz_ohci_platform_data = {
diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
index 357d9aee38a3..a809ba0bb25e 100644
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -121,6 +121,7 @@ struct pxa27x_ohci {
void __iomem *mmio_base;
struct regulator *vbus[3];
bool vbus_enabled[3];
+ struct gpio_desc *usb_host;
};
#define to_pxa27x_ohci(hcd) (struct pxa27x_ohci *)(hcd_to_ohci(hcd)->priv)
@@ -447,6 +448,10 @@ static int ohci_hcd_pxa27x_probe(struct platform_device *pdev)
pxa_ohci = to_pxa27x_ohci(hcd);
pxa_ohci->clk = usb_clk;
pxa_ohci->mmio_base = (void __iomem *)hcd->regs;
+ pxa_ohci->usb_host = devm_gpiod_get_optional(&pdev->dev, "usb-host", GPIOD_OUT_LOW);
+ if (IS_ERR(pxa_ohci->usb_host))
+ return dev_err_probe(&pdev->dev, PTR_ERR(pxa_ohci->usb_host),
+ "failed to get USB host GPIO\n");
for (i = 0; i < 3; ++i) {
char name[6];
--
2.42.0
^ permalink raw reply related
* [PATCH RFT v8 0/6] ARM: pxa: GPIO descriptor conversions
From: Duje Mihanović @ 2023-10-11 13:23 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
Hello,
Small series to convert some of the board files in the mach-pxa directory
to use the new GPIO descriptor interface.
Most notably, the am200epd, am300epd and Spitz matrix keypad among
others are not converted in this series.
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
Changes in v8:
- Address maintainer comments:
- Do not pointlessly gpiod_get() LED gpios
- Update trailers
- Rebase on v6.6-rc5
- Link to v7: https://lore.kernel.org/r/20231009-pxa-gpio-v7-0-c8f5f403e856@skole.hr
Changes in v7:
- Address maintainer comments:
- Drop gpiod_put in OHCI
- Make "struct gpio_descs *leds" in Spitz LEDs global
- Link to v6: https://lore.kernel.org/r/20231006-pxa-gpio-v6-0-981b4910d599@skole.hr
Changes in v6:
- Address maintainer comments:
- Use devm_gpiod_get_optional() in OHCI
- Use gpiod_get_array() in Spitz LEDs
- Update trailers
- Link to v5: https://lore.kernel.org/r/20231004-pxa-gpio-v5-0-d99ae6fceea8@skole.hr
Changes in v5:
- Address maintainer comments:
- Rename "reset generator" GPIO to "reset"
- Rename ads7846_wait_for_sync() to ads7846_wait_for_sync_gpio()
- Properly bail out when requesting USB host GPIO fails
- Use dev_err_probe() when requesting touchscreen sync GPIO fails
- Use static gpio_desc for gumstix bluetooth reset
- Pulse gumstix bluetooth reset line correctly (assert, then deassert)
- Fix style issue in ads7846_wait_for_sync_gpio()
- Update trailers
- Link to v4: https://lore.kernel.org/r/20231001-pxa-gpio-v4-0-0f3b975e6ed5@skole.hr
Changes in v4:
- Address maintainer comments:
- Move wait_for_sync() from spitz.c to driver
- Register LED platform device before getting its gpiod-s
- Add Linus' Reviewed-by
- Link to v3: https://lore.kernel.org/r/20230929-pxa-gpio-v3-0-af8d5e5d1f34@skole.hr
Changes in v3:
- Address maintainer comments:
- Use GPIO_LOOKUP_IDX for LEDs
- Drop unnecessary NULL assignments
- Don't give up on *all* SPI devices if hsync cannot be set up
- Add Linus' Acked-by
- Link to v2: https://lore.kernel.org/r/20230926-pxa-gpio-v2-0-984464d165dd@skole.hr
Changes in v2:
- Address maintainer comments:
- Change mentions of function to function()
- Drop cast in OHCI driver dev_warn() call
- Use %pe in OHCI and reset drivers
- Use GPIO _optional() API in OHCI driver
- Drop unnecessary not-null check in OHCI driver
- Use pr_err() instead of printk() in reset driver
- Rebase on v6.6-rc3
- Link to v1: https://lore.kernel.org/r/20230924-pxa-gpio-v1-0-2805b87d8894@skole.hr
---
Duje Mihanović (6):
ARM: pxa: Convert Spitz OHCI to GPIO descriptors
ARM: pxa: Convert Spitz LEDs to GPIO descriptors
ARM: pxa: Convert Spitz CF power control to GPIO descriptors
ARM: pxa: Convert reset driver to GPIO descriptors
ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors
input: ads7846: Move wait_for_sync() logic to driver
arch/arm/mach-pxa/gumstix.c | 22 ++++++-------
arch/arm/mach-pxa/reset.c | 39 ++++++++--------------
arch/arm/mach-pxa/reset.h | 3 +-
arch/arm/mach-pxa/spitz.c | 65 ++++++++++++++++++++++++-------------
drivers/input/touchscreen/ads7846.c | 22 +++++++++----
drivers/usb/host/ohci-pxa27x.c | 5 +++
include/linux/spi/ads7846.h | 1 -
7 files changed, 88 insertions(+), 69 deletions(-)
---
base-commit: 94f6f0550c625fab1f373bb86a6669b45e9748b3
change-id: 20230807-pxa-gpio-3ce25d574814
Best regards,
--
Duje Mihanović <duje.mihanovic@skole.hr>
^ permalink raw reply
* [PATCH RFT v8 5/6] ARM: pxa: Convert gumstix Bluetooth to GPIO descriptors
From: Duje Mihanović @ 2023-10-11 13:23 UTC (permalink / raw)
To: Daniel Mack, Haojian Zhuang, Robert Jarzmik, Russell King,
Alan Stern, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Andy Shevchenko, Dmitry Torokhov, Mark Brown
Cc: linux-arm-kernel, linux-kernel, linux-usb, linux-gpio,
linux-input, linux-spi, Duje Mihanović, Bartosz Golaszewski
In-Reply-To: <20231011-pxa-gpio-v8-0-eed08a0fcac8@skole.hr>
Gumstix still uses the legacy GPIO interface for resetting the Bluetooth
device.
Convert it to use the GPIO descriptor interface.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
arch/arm/mach-pxa/gumstix.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/arm/mach-pxa/gumstix.c b/arch/arm/mach-pxa/gumstix.c
index c9f0f62187bd..0bca6e2c80a9 100644
--- a/arch/arm/mach-pxa/gumstix.c
+++ b/arch/arm/mach-pxa/gumstix.c
@@ -20,8 +20,8 @@
#include <linux/delay.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
+#include <linux/gpio/consumer.h>
#include <linux/gpio/machine.h>
-#include <linux/gpio.h>
#include <linux/err.h>
#include <linux/clk.h>
@@ -129,6 +129,11 @@ static void gumstix_udc_init(void)
#endif
#ifdef CONFIG_BT
+GPIO_LOOKUP_SINGLE(gumstix_bt_gpio_table, "pxa2xx-uart.1", "pxa-gpio",
+ GPIO_GUMSTIX_BTRESET, "BTRST", GPIO_ACTIVE_LOW);
+
+static struct gpio_desc *bt_reset;
+
/* Normally, the bootloader would have enabled this 32kHz clock but many
** boards still have u-boot 1.1.4 so we check if it has been turned on and
** if not, we turn it on with a warning message. */
@@ -153,24 +158,19 @@ static void gumstix_setup_bt_clock(void)
static void __init gumstix_bluetooth_init(void)
{
- int err;
+ gpiod_add_lookup_table(&gumstix_bt_gpio_table);
gumstix_setup_bt_clock();
- err = gpio_request(GPIO_GUMSTIX_BTRESET, "BTRST");
- if (err) {
+ bt_reset = gpiod_get(&pxa_device_btuart.dev, "BTRST", GPIOD_OUT_HIGH);
+ if (IS_ERR(bt_reset)) {
pr_err("gumstix: failed request gpio for bluetooth reset\n");
return;
}
- err = gpio_direction_output(GPIO_GUMSTIX_BTRESET, 1);
- if (err) {
- pr_err("gumstix: can't reset bluetooth\n");
- return;
- }
- gpio_set_value(GPIO_GUMSTIX_BTRESET, 0);
+ gpiod_set_value(bt_reset, 1);
udelay(100);
- gpio_set_value(GPIO_GUMSTIX_BTRESET, 1);
+ gpiod_set_value(bt_reset, 0);
}
#else
static void gumstix_bluetooth_init(void)
--
2.42.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