[v3] soc: qcom: aoss: Add debugfs interface for sending messages

Message ID 20240117-qcom-aoss-debugfs-v2-v3-1-1aa779124822@quicinc.com
State New
Headers
Series [v3] soc: qcom: aoss: Add debugfs interface for sending messages |

Commit Message

Bjorn Andersson Jan. 18, 2024, 2:31 a.m. UTC
  In addition to the normal runtime commands, the Always On Processor
(AOP) provides a number of debug commands which can be used during
system debugging for things such as preventing power collapse or placing
floor votes for certain resources. Some of these are documented in the
Robotics RB5 "Debug AOP ADB" linked below.

Provide a debugfs interface for the developer/tester to send some of
these commands to the AOP, which allow the user to override the DDR
frequency, preventing power collapse of cx and ddr, and prevent AOSS
from going to sleep.

Link: https://docs.qualcomm.com/bundle/publicresource/topics/80-88500-3/85_Debugging_AOP_ADB.html
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
Changes in v3:
- Stop passing command string straight from userspace to the firmware
- Format the commands in the driver instead
- Provide a few useful commands
- Link to v2: https://lore.kernel.org/linux-arm-msm/20230811205839.727373-3-quic_bjorande@quicinc.com/
---
 drivers/soc/qcom/qcom_aoss.c | 94 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)


---
base-commit: 943b9f0ab2cfbaea148dd6ac279957eb08b96904
change-id: 20240117-qcom-aoss-debugfs-v2-5e25b1934e19

Best regards,
  

Comments

Andrew Lunn Jan. 19, 2024, 1:43 p.m. UTC | #1
> +static void qmp_debugfs_create(struct qmp *qmp)
> +{
> +	const struct qmp_debugfs_entry *entry;
> +	int i;
> +
> +	qmp->debugfs_root = debugfs_create_dir("qcom_aoss", NULL);
> +
> +	for (i = 0; i < ARRAY_SIZE(qmp->debugfs_files); i++) {
> +		entry = &qmp_debugfs_entries[i];
> +
> +		qmp->debugfs_files[i] = debugfs_create_file(entry->name, 0220,

group write? Generally you only want root able to do writes.

However, thanks for reworking this to just support some well defined
commands, rather than binary blobs from user space.

      Andrew
  
Konrad Dybcio Jan. 22, 2024, 10:44 a.m. UTC | #2
On 18.01.2024 03:31, Bjorn Andersson wrote:
> In addition to the normal runtime commands, the Always On Processor
> (AOP) provides a number of debug commands which can be used during
> system debugging for things such as preventing power collapse or placing
> floor votes for certain resources. Some of these are documented in the
> Robotics RB5 "Debug AOP ADB" linked below.
> 
> Provide a debugfs interface for the developer/tester to send some of
> these commands to the AOP, which allow the user to override the DDR
> frequency, preventing power collapse of cx and ddr, and prevent AOSS
> from going to sleep.
> 
> Link: https://docs.qualcomm.com/bundle/publicresource/topics/80-88500-3/85_Debugging_AOP_ADB.html
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---

[...]

>  
> +struct qmp_debugfs_entry {
> +	const char *name;
> +	const char *fmt;
> +	bool is_bool;

This can also be const

> +	const char *true_val;
> +	const char *false_val;

All of these strings can be const ptrs to const data

Konrad
  
Bjorn Andersson Jan. 23, 2024, 12:29 a.m. UTC | #3
On Mon, Jan 22, 2024 at 11:44:30AM +0100, Konrad Dybcio wrote:
> On 18.01.2024 03:31, Bjorn Andersson wrote:
> > In addition to the normal runtime commands, the Always On Processor
> > (AOP) provides a number of debug commands which can be used during
> > system debugging for things such as preventing power collapse or placing
> > floor votes for certain resources. Some of these are documented in the
> > Robotics RB5 "Debug AOP ADB" linked below.
> > 
> > Provide a debugfs interface for the developer/tester to send some of
> > these commands to the AOP, which allow the user to override the DDR
> > frequency, preventing power collapse of cx and ddr, and prevent AOSS
> > from going to sleep.
> > 
> > Link: https://docs.qualcomm.com/bundle/publicresource/topics/80-88500-3/85_Debugging_AOP_ADB.html
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> 
> [...]
> 
> >  
> > +struct qmp_debugfs_entry {
> > +	const char *name;
> > +	const char *fmt;
> > +	bool is_bool;
> 
> This can also be const
> 
> > +	const char *true_val;
> > +	const char *false_val;
> 
> All of these strings can be const ptrs to const data
> 

These are all const data now, but what would the reason for enforcing it
in the struct definition be?

Regards,
Bjorn
  
Konrad Dybcio Jan. 23, 2024, 6:37 p.m. UTC | #4
On 1/23/24 01:29, Bjorn Andersson wrote:
> On Mon, Jan 22, 2024 at 11:44:30AM +0100, Konrad Dybcio wrote:
>> On 18.01.2024 03:31, Bjorn Andersson wrote:
>>> In addition to the normal runtime commands, the Always On Processor
>>> (AOP) provides a number of debug commands which can be used during
>>> system debugging for things such as preventing power collapse or placing
>>> floor votes for certain resources. Some of these are documented in the
>>> Robotics RB5 "Debug AOP ADB" linked below.
>>>
>>> Provide a debugfs interface for the developer/tester to send some of
>>> these commands to the AOP, which allow the user to override the DDR
>>> frequency, preventing power collapse of cx and ddr, and prevent AOSS
>>> from going to sleep.
>>>
>>> Link: https://docs.qualcomm.com/bundle/publicresource/topics/80-88500-3/85_Debugging_AOP_ADB.html
>>> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>>> ---
>>
>> [...]
>>
>>>   
>>> +struct qmp_debugfs_entry {
>>> +	const char *name;
>>> +	const char *fmt;
>>> +	bool is_bool;
>>
>> This can also be const
>>
>>> +	const char *true_val;
>>> +	const char *false_val;
>>
>> All of these strings can be const ptrs to const data
>>
> 
> These are all const data now, but what would the reason for enforcing it
> in the struct definition be?

I've just written too much rust lately and I'm simply paranoid
about mutability, it seems ;)

Konrad
  
Chris Lew Jan. 27, 2024, 1:35 a.m. UTC | #5
On 1/17/2024 6:31 PM, Bjorn Andersson wrote:
> In addition to the normal runtime commands, the Always On Processor
> (AOP) provides a number of debug commands which can be used during
> system debugging for things such as preventing power collapse or placing
> floor votes for certain resources. Some of these are documented in the
> Robotics RB5 "Debug AOP ADB" linked below.
> 
> Provide a debugfs interface for the developer/tester to send some of
> these commands to the AOP, which allow the user to override the DDR
> frequency, preventing power collapse of cx and ddr, and prevent AOSS
> from going to sleep.
> 
> Link: https://docs.qualcomm.com/bundle/publicresource/topics/80-88500-3/85_Debugging_AOP_ADB.html
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
> Changes in v3:
> - Stop passing command string straight from userspace to the firmware
> - Format the commands in the driver instead
> - Provide a few useful commands
> - Link to v2: https://lore.kernel.org/linux-arm-msm/20230811205839.727373-3-quic_bjorande@quicinc.com/
> ---
> +		qmp->debugfs_files[i] = debugfs_create_file(entry->name, 0220,
> +							    qmp->debugfs_root,
> +							    qmp,
> +							    &qmp_debugfs_fops);

With group permissions fixed:

Reviewed-by: Chris Lew <quic_clew@quicinc.com>
  
Bjorn Andersson Jan. 28, 2024, 5:45 p.m. UTC | #6
On Wed, 17 Jan 2024 18:31:52 -0800, Bjorn Andersson wrote:
> In addition to the normal runtime commands, the Always On Processor
> (AOP) provides a number of debug commands which can be used during
> system debugging for things such as preventing power collapse or placing
> floor votes for certain resources. Some of these are documented in the
> Robotics RB5 "Debug AOP ADB" linked below.
> 
> Provide a debugfs interface for the developer/tester to send some of
> these commands to the AOP, which allow the user to override the DDR
> frequency, preventing power collapse of cx and ddr, and prevent AOSS
> from going to sleep.
> 
> [...]

Applied, thanks!

[1/1] soc: qcom: aoss: Add debugfs interface for sending messages
      commit: d51d984c5525aebac0f90356ab1d923541b5cc60

Best regards,
  

Patch

diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
index aff0cfb71482..30c7d59d3424 100644
--- a/drivers/soc/qcom/qcom_aoss.c
+++ b/drivers/soc/qcom/qcom_aoss.c
@@ -3,6 +3,7 @@ 
  * Copyright (c) 2019, Linaro Ltd
  */
 #include <linux/clk-provider.h>
+#include <linux/debugfs.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/mailbox_client.h>
@@ -44,6 +45,8 @@ 
 
 #define QMP_NUM_COOLING_RESOURCES	2
 
+#define QMP_DEBUGFS_FILES		4
+
 static bool qmp_cdev_max_state = 1;
 
 struct qmp_cooling_device {
@@ -82,6 +85,8 @@  struct qmp {
 
 	struct clk_hw qdss_clk;
 	struct qmp_cooling_device *cooling_devs;
+	struct dentry *debugfs_root;
+	struct dentry *debugfs_files[QMP_DEBUGFS_FILES];
 };
 
 static void qmp_kick(struct qmp *qmp)
@@ -475,6 +480,91 @@  void qmp_put(struct qmp *qmp)
 }
 EXPORT_SYMBOL_GPL(qmp_put);
 
+struct qmp_debugfs_entry {
+	const char *name;
+	const char *fmt;
+	bool is_bool;
+	const char *true_val;
+	const char *false_val;
+};
+
+static const struct qmp_debugfs_entry qmp_debugfs_entries[QMP_DEBUGFS_FILES] = {
+	{ "ddr_frequency_mhz", "{class: ddr, res: fixed, val: %u}", false },
+	{ "prevent_aoss_sleep", "{class: aoss_slp, res: sleep: %s}", true, "enable", "disable" },
+	{ "prevent_cx_collapse", "{class: cx_mol, res: cx, val: %s}", true, "mol", "off" },
+	{ "prevent_ddr_collapse", "{class: ddr_mol, res: ddr, val: %s}", true, "mol", "off" },
+};
+
+static ssize_t qmp_debugfs_write(struct file *file, const char __user *user_buf,
+				 size_t count, loff_t *pos)
+{
+	const struct qmp_debugfs_entry *entry = NULL;
+	struct qmp *qmp = file->private_data;
+	char buf[QMP_MSG_LEN];
+	unsigned int uint_val;
+	const char *str_val;
+	bool bool_val;
+	int ret;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(qmp->debugfs_files); i++) {
+		if (qmp->debugfs_files[i] == file->f_path.dentry) {
+			entry = &qmp_debugfs_entries[i];
+			break;
+		}
+	}
+	if (WARN_ON(!entry))
+		return -EFAULT;
+
+	if (entry->is_bool) {
+		ret = kstrtobool_from_user(user_buf, count, &bool_val);
+		if (ret)
+			return ret;
+
+		str_val = bool_val ? entry->true_val : entry->false_val;
+
+		ret = snprintf(buf, sizeof(buf), entry->fmt, str_val);
+		if (ret >= sizeof(buf))
+			return -EINVAL;
+	} else {
+		ret = kstrtou32_from_user(user_buf, count, 0, &uint_val);
+		if (ret)
+			return ret;
+
+		ret = snprintf(buf, sizeof(buf), entry->fmt, uint_val);
+		if (ret >= sizeof(buf))
+			return -EINVAL;
+	}
+
+	ret = qmp_send(qmp, buf);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations qmp_debugfs_fops = {
+	.open = simple_open,
+	.write = qmp_debugfs_write,
+};
+
+static void qmp_debugfs_create(struct qmp *qmp)
+{
+	const struct qmp_debugfs_entry *entry;
+	int i;
+
+	qmp->debugfs_root = debugfs_create_dir("qcom_aoss", NULL);
+
+	for (i = 0; i < ARRAY_SIZE(qmp->debugfs_files); i++) {
+		entry = &qmp_debugfs_entries[i];
+
+		qmp->debugfs_files[i] = debugfs_create_file(entry->name, 0220,
+							    qmp->debugfs_root,
+							    qmp,
+							    &qmp_debugfs_fops);
+	}
+}
+
 static int qmp_probe(struct platform_device *pdev)
 {
 	struct qmp *qmp;
@@ -523,6 +613,8 @@  static int qmp_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, qmp);
 
+	qmp_debugfs_create(qmp);
+
 	return 0;
 
 err_close_qmp:
@@ -537,6 +629,8 @@  static void qmp_remove(struct platform_device *pdev)
 {
 	struct qmp *qmp = platform_get_drvdata(pdev);
 
+	debugfs_remove_recursive(qmp->debugfs_root);
+
 	qmp_qdss_clk_remove(qmp);
 	qmp_cooling_devices_remove(qmp);