[PATCH V7 0/2] qcom aoss qmp_get and debugfs support patches

[PATCH V7 0/2] qcom aoss qmp_get and debugfs support patches

[Deepak Kumar Singh]

[Change from V6]
soc: qcom: aoss: Expose send for generic usecase
  Changed comment for qmp_put
  Added comment after put_device in qmp_put

soc: qcom: aoss: Add debugfs entry
  Removed CONFIG_DEBUG_FS check

Deepak Kumar Singh (2):
  soc: qcom: aoss: Expose send for generic usecase
  soc: qcom: aoss: Add debugfs entry

 drivers/soc/qcom/qcom_aoss.c       | 86 +++++++++++++++++++++++++++++++++++++-
 include/linux/soc/qcom/qcom_aoss.h | 38 +++++++++++++++++
 2 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/soc/qcom/qcom_aoss.h

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH V7 1/2] soc: qcom: aoss: Expose send for generic usecase

[Deepak Kumar Singh]

Not all upcoming usecases will have an interface to allow the aoss
driver to hook onto. Expose the send api and create a get function to
enable drivers to send their own messages to aoss.

Signed-off-by: Chris Lew <clew@codeaurora.org>
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/soc/qcom/qcom_aoss.c       | 54 +++++++++++++++++++++++++++++++++++++-
 include/linux/soc/qcom/qcom_aoss.h | 38 +++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/soc/qcom/qcom_aoss.h

diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
index 934fcc4..bf0a6280 100644
--- a/drivers/soc/qcom/qcom_aoss.c
+++ b/drivers/soc/qcom/qcom_aoss.c
@@ -8,10 +8,12 @@
 #include <linux/io.h>
 #include <linux/mailbox_client.h>
 #include <linux/module.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/thermal.h>
 #include <linux/slab.h>
+#include <linux/soc/qcom/qcom_aoss.h>
 
 #define QMP_DESC_MAGIC			0x0
 #define QMP_DESC_VERSION		0x4
@@ -223,11 +225,14 @@ static bool qmp_message_empty(struct qmp *qmp)
  *
  * Return: 0 on success, negative errno on failure
  */
-static int qmp_send(struct qmp *qmp, const void *data, size_t len)
+int qmp_send(struct qmp *qmp, const void *data, size_t len)
 {
 	long time_left;
 	int ret;
 
+	if (WARN_ON(IS_ERR_OR_NULL(qmp) || !data))
+		return -EINVAL;
+
 	if (WARN_ON(len + sizeof(u32) > qmp->size))
 		return -EINVAL;
 
@@ -261,6 +266,7 @@ static int qmp_send(struct qmp *qmp, const void *data, size_t len)
 
 	return ret;
 }
+EXPORT_SYMBOL(qmp_send);
 
 static int qmp_qdss_clk_prepare(struct clk_hw *hw)
 {
@@ -515,6 +521,51 @@ static void qmp_cooling_devices_remove(struct qmp *qmp)
 		thermal_cooling_device_unregister(qmp->cooling_devs[i].cdev);
 }
 
+/**
+ * qmp_get() - get a qmp handle from a device
+ * @dev: client device pointer
+ *
+ * Return: handle to qmp device on success, ERR_PTR() on failure
+ */
+struct qmp *qmp_get(struct device *dev)
+{
+	struct platform_device *pdev;
+	struct device_node *np;
+	struct qmp *qmp;
+
+	if (!dev || !dev->of_node)
+		return ERR_PTR(-EINVAL);
+
+	np = of_parse_phandle(dev->of_node, "qcom,qmp", 0);
+	if (!np)
+		return ERR_PTR(-ENODEV);
+
+	pdev = of_find_device_by_node(np);
+	of_node_put(np);
+	if (!pdev)
+		return ERR_PTR(-EINVAL);
+
+	qmp = platform_get_drvdata(pdev);
+
+	return qmp ? qmp : ERR_PTR(-EPROBE_DEFER);
+}
+EXPORT_SYMBOL(qmp_get);
+
+/**
+ * qmp_put() - release a qmp handle
+ * @qmp: qmp handle obtained from qmp_get()
+ */
+void qmp_put(struct qmp *qmp)
+{
+	if (!IS_ERR_OR_NULL(qmp))
+		put_device(qmp->dev);
+	/*
+	 * Match get_device() inside of_find_device_by_node() in
+	 * qmp_get()
+	 */
+}
+EXPORT_SYMBOL(qmp_put);
+
 static int qmp_probe(struct platform_device *pdev)
 {
 	struct resource *res;
@@ -610,6 +661,7 @@ static struct platform_driver qmp_driver = {
 	.driver = {
 		.name		= "qcom_aoss_qmp",
 		.of_match_table	= qmp_dt_match,
+		.suppress_bind_attrs = true,
 	},
 	.probe = qmp_probe,
 	.remove	= qmp_remove,
diff --git a/include/linux/soc/qcom/qcom_aoss.h b/include/linux/soc/qcom/qcom_aoss.h
new file mode 100644
index 0000000..3c2a82e
--- /dev/null
+++ b/include/linux/soc/qcom/qcom_aoss.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __QCOM_AOSS_H__
+#define __QCOM_AOSS_H__
+
+#include <linux/err.h>
+#include <linux/device.h>
+
+struct qmp;
+
+#if IS_ENABLED(CONFIG_QCOM_AOSS_QMP)
+
+int qmp_send(struct qmp *qmp, const void *data, size_t len);
+struct qmp *qmp_get(struct device *dev);
+void qmp_put(struct qmp *qmp);
+
+#else
+
+static inline int qmp_send(struct qmp *qmp, const void *data, size_t len)
+{
+	return -ENODEV;
+}
+
+static inline struct qmp *qmp_get(struct device *dev)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline void qmp_put(struct qmp *qmp)
+{
+}
+
+#endif
+
+#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH V7 2/2] soc: qcom: aoss: Add debugfs entry

[Deepak Kumar Singh]

Some user space clients may require to change power states of various
parts of hardware. Add a debugfs node for qmp so messages can be sent
to aoss from user space.

Signed-off-by: Chris Lew <clew@codeaurora.org>
Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
---
 drivers/soc/qcom/qcom_aoss.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
index bf0a6280..4cd8670 100644
--- a/drivers/soc/qcom/qcom_aoss.c
+++ b/drivers/soc/qcom/qcom_aoss.c
@@ -4,6 +4,7 @@
  */
 #include <dt-bindings/power/qcom-aoss-qmp.h>
 #include <linux/clk-provider.h>
+#include <linux/debugfs.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/mailbox_client.h>
@@ -86,6 +87,7 @@ struct qmp {
 	struct clk_hw qdss_clk;
 	struct genpd_onecell_data pd_data;
 	struct qmp_cooling_device *cooling_devs;
+	struct dentry *debugfs_file;
 };
 
 struct qmp_pd {
@@ -566,6 +568,31 @@ void qmp_put(struct qmp *qmp)
 }
 EXPORT_SYMBOL(qmp_put);
 
+static ssize_t aoss_dbg_write(struct file *file, const char __user *userstr,
+			      size_t len, loff_t *pos)
+{
+	struct qmp *qmp = file->private_data;
+	char buf[QMP_MSG_LEN] = {};
+	int ret;
+
+	if (!len || len >= QMP_MSG_LEN)
+		return -EINVAL;
+
+	ret  = copy_from_user(buf, userstr, len);
+	if (ret) {
+		return -EFAULT;
+	}
+
+	ret = qmp_send(qmp, buf, QMP_MSG_LEN);
+
+	return ret ? ret : len;
+}
+
+static const struct file_operations aoss_dbg_fops = {
+	.open = simple_open,
+	.write = aoss_dbg_write,
+};
+
 static int qmp_probe(struct platform_device *pdev)
 {
 	struct resource *res;
@@ -620,6 +647,9 @@ static int qmp_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, qmp);
 
+	qmp->debugfs_file = debugfs_create_file("aoss_send_message", 0220, NULL,
+						qmp, &aoss_dbg_fops);
+
 	return 0;
 
 err_remove_qdss_clk:
@@ -636,6 +666,8 @@ static int qmp_remove(struct platform_device *pdev)
 {
 	struct qmp *qmp = platform_get_drvdata(pdev);
 
+	debugfs_remove(qmp->debugfs_file);
+
 	qmp_qdss_clk_remove(qmp);
 	qmp_pd_remove(qmp);
 	qmp_cooling_devices_remove(qmp);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V7 2/2] soc: qcom: aoss: Add debugfs entry

[Stephen Boyd]

Quoting Deepak Kumar Singh (2021-08-30 04:37:31)
> Some user space clients may require to change power states of various
> parts of hardware. Add a debugfs node for qmp so messages can be sent
> to aoss from user space.
>
> Signed-off-by: Chris Lew <clew@codeaurora.org>
> Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
> ---

NAK. We don't want userspace to treat debugfs as an ABI. See
https://lore.kernel.org/r/CAE-0n52YXvTzvK4B3Aggg1fcRyjy=+HzNADP315-J0iJ8bMWUQ@mail.gmail.com

Re: [PATCH V7 2/2] soc: qcom: aoss: Add debugfs entry

[Bjorn Andersson]

On Mon 30 Aug 06:37 CDT 2021, Deepak Kumar Singh wrote:

> Some user space clients may require to change power states of various
> parts of hardware. Add a debugfs node for qmp so messages can be sent
> to aoss from user space.
> 

I think this could be a useful tool while testing and developing client
drivers or perhaps during bringup of new platforms.

But your new commit message doesn't sound right, given that debugfs
isn't mounted in your production builds.

Regards,
Bjorn

> Signed-off-by: Chris Lew <clew@codeaurora.org>
> Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
> ---
>  drivers/soc/qcom/qcom_aoss.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
> index bf0a6280..4cd8670 100644
> --- a/drivers/soc/qcom/qcom_aoss.c
> +++ b/drivers/soc/qcom/qcom_aoss.c
> @@ -4,6 +4,7 @@
>   */
>  #include <dt-bindings/power/qcom-aoss-qmp.h>
>  #include <linux/clk-provider.h>
> +#include <linux/debugfs.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/mailbox_client.h>
> @@ -86,6 +87,7 @@ struct qmp {
>  	struct clk_hw qdss_clk;
>  	struct genpd_onecell_data pd_data;
>  	struct qmp_cooling_device *cooling_devs;
> +	struct dentry *debugfs_file;
>  };
>  
>  struct qmp_pd {
> @@ -566,6 +568,31 @@ void qmp_put(struct qmp *qmp)
>  }
>  EXPORT_SYMBOL(qmp_put);
>  
> +static ssize_t aoss_dbg_write(struct file *file, const char __user *userstr,
> +			      size_t len, loff_t *pos)
> +{
> +	struct qmp *qmp = file->private_data;
> +	char buf[QMP_MSG_LEN] = {};
> +	int ret;
> +
> +	if (!len || len >= QMP_MSG_LEN)
> +		return -EINVAL;
> +
> +	ret  = copy_from_user(buf, userstr, len);
> +	if (ret) {
> +		return -EFAULT;
> +	}
> +
> +	ret = qmp_send(qmp, buf, QMP_MSG_LEN);
> +
> +	return ret ? ret : len;
> +}
> +
> +static const struct file_operations aoss_dbg_fops = {
> +	.open = simple_open,
> +	.write = aoss_dbg_write,
> +};
> +
>  static int qmp_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
> @@ -620,6 +647,9 @@ static int qmp_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, qmp);
>  
> +	qmp->debugfs_file = debugfs_create_file("aoss_send_message", 0220, NULL,
> +						qmp, &aoss_dbg_fops);
> +
>  	return 0;
>  
>  err_remove_qdss_clk:
> @@ -636,6 +666,8 @@ static int qmp_remove(struct platform_device *pdev)
>  {
>  	struct qmp *qmp = platform_get_drvdata(pdev);
>  
> +	debugfs_remove(qmp->debugfs_file);
> +
>  	qmp_qdss_clk_remove(qmp);
>  	qmp_pd_remove(qmp);
>  	qmp_cooling_devices_remove(qmp);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH V7 1/2] soc: qcom: aoss: Expose send for generic usecase

[Bjorn Andersson]

On Mon 30 Aug 06:37 CDT 2021, Deepak Kumar Singh wrote:

> Not all upcoming usecases will have an interface to allow the aoss
> driver to hook onto. Expose the send api and create a get function to
> enable drivers to send their own messages to aoss.
> 
> Signed-off-by: Chris Lew <clew@codeaurora.org>
> Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
>  drivers/soc/qcom/qcom_aoss.c       | 54 +++++++++++++++++++++++++++++++++++++-
>  include/linux/soc/qcom/qcom_aoss.h | 38 +++++++++++++++++++++++++++
>  2 files changed, 91 insertions(+), 1 deletion(-)
>  create mode 100644 include/linux/soc/qcom/qcom_aoss.h
> 
> diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
> index 934fcc4..bf0a6280 100644
> --- a/drivers/soc/qcom/qcom_aoss.c
> +++ b/drivers/soc/qcom/qcom_aoss.c
> @@ -8,10 +8,12 @@
>  #include <linux/io.h>
>  #include <linux/mailbox_client.h>
>  #include <linux/module.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_domain.h>
>  #include <linux/thermal.h>
>  #include <linux/slab.h>
> +#include <linux/soc/qcom/qcom_aoss.h>
>  
>  #define QMP_DESC_MAGIC			0x0
>  #define QMP_DESC_VERSION		0x4
> @@ -223,11 +225,14 @@ static bool qmp_message_empty(struct qmp *qmp)
>   *
>   * Return: 0 on success, negative errno on failure
>   */
> -static int qmp_send(struct qmp *qmp, const void *data, size_t len)
> +int qmp_send(struct qmp *qmp, const void *data, size_t len)
>  {
>  	long time_left;
>  	int ret;
>  
> +	if (WARN_ON(IS_ERR_OR_NULL(qmp) || !data))
> +		return -EINVAL;
> +
>  	if (WARN_ON(len + sizeof(u32) > qmp->size))
>  		return -EINVAL;
>  
> @@ -261,6 +266,7 @@ static int qmp_send(struct qmp *qmp, const void *data, size_t len)
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL(qmp_send);
>  
>  static int qmp_qdss_clk_prepare(struct clk_hw *hw)
>  {
> @@ -515,6 +521,51 @@ static void qmp_cooling_devices_remove(struct qmp *qmp)
>  		thermal_cooling_device_unregister(qmp->cooling_devs[i].cdev);
>  }
>  
> +/**
> + * qmp_get() - get a qmp handle from a device
> + * @dev: client device pointer
> + *
> + * Return: handle to qmp device on success, ERR_PTR() on failure
> + */
> +struct qmp *qmp_get(struct device *dev)
> +{
> +	struct platform_device *pdev;
> +	struct device_node *np;
> +	struct qmp *qmp;
> +
> +	if (!dev || !dev->of_node)
> +		return ERR_PTR(-EINVAL);
> +
> +	np = of_parse_phandle(dev->of_node, "qcom,qmp", 0);
> +	if (!np)
> +		return ERR_PTR(-ENODEV);
> +
> +	pdev = of_find_device_by_node(np);
> +	of_node_put(np);
> +	if (!pdev)
> +		return ERR_PTR(-EINVAL);
> +
> +	qmp = platform_get_drvdata(pdev);
> +
> +	return qmp ? qmp : ERR_PTR(-EPROBE_DEFER);
> +}
> +EXPORT_SYMBOL(qmp_get);
> +
> +/**
> + * qmp_put() - release a qmp handle
> + * @qmp: qmp handle obtained from qmp_get()
> + */
> +void qmp_put(struct qmp *qmp)
> +{
> +	if (!IS_ERR_OR_NULL(qmp))
> +		put_device(qmp->dev);
> +	/*
> +	 * Match get_device() inside of_find_device_by_node() in
> +	 * qmp_get()
> +	 */

Afaict this comment relates to the put_device() above, which typically
would imply that the comment should be above or inside the if
statement?

> +}
> +EXPORT_SYMBOL(qmp_put);
> +

Regards,
Bjorn

>  static int qmp_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
> @@ -610,6 +661,7 @@ static struct platform_driver qmp_driver = {
>  	.driver = {
>  		.name		= "qcom_aoss_qmp",
>  		.of_match_table	= qmp_dt_match,
> +		.suppress_bind_attrs = true,
>  	},
>  	.probe = qmp_probe,
>  	.remove	= qmp_remove,
> diff --git a/include/linux/soc/qcom/qcom_aoss.h b/include/linux/soc/qcom/qcom_aoss.h
> new file mode 100644
> index 0000000..3c2a82e
> --- /dev/null
> +++ b/include/linux/soc/qcom/qcom_aoss.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef __QCOM_AOSS_H__
> +#define __QCOM_AOSS_H__
> +
> +#include <linux/err.h>
> +#include <linux/device.h>
> +
> +struct qmp;
> +
> +#if IS_ENABLED(CONFIG_QCOM_AOSS_QMP)
> +
> +int qmp_send(struct qmp *qmp, const void *data, size_t len);
> +struct qmp *qmp_get(struct device *dev);
> +void qmp_put(struct qmp *qmp);
> +
> +#else
> +
> +static inline int qmp_send(struct qmp *qmp, const void *data, size_t len)
> +{
> +	return -ENODEV;
> +}
> +
> +static inline struct qmp *qmp_get(struct device *dev)
> +{
> +	return ERR_PTR(-ENODEV);
> +}
> +
> +static inline void qmp_put(struct qmp *qmp)
> +{
> +}
> +
> +#endif
> +
> +#endif
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH V7 2/2] soc: qcom: aoss: Add debugfs entry

[Deepak Kumar Singh]

On 8/31/2021 4:48 AM, Bjorn Andersson wrote:
> On Mon 30 Aug 06:37 CDT 2021, Deepak Kumar Singh wrote:
>
>> Some user space clients may require to change power states of various
>> parts of hardware. Add a debugfs node for qmp so messages can be sent
>> to aoss from user space.
>>
> I think this could be a useful tool while testing and developing client
> drivers or perhaps during bringup of new platforms.
>
> But your new commit message doesn't sound right, given that debugfs
> isn't mounted in your production builds.
>
> Regards,
> Bjorn
Updated commit message in V8.
>
>> Signed-off-by: Chris Lew <clew@codeaurora.org>
>> Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
>> ---
>>   drivers/soc/qcom/qcom_aoss.c | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
>> index bf0a6280..4cd8670 100644
>> --- a/drivers/soc/qcom/qcom_aoss.c
>> +++ b/drivers/soc/qcom/qcom_aoss.c
>> @@ -4,6 +4,7 @@
>>    */
>>   #include <dt-bindings/power/qcom-aoss-qmp.h>
>>   #include <linux/clk-provider.h>
>> +#include <linux/debugfs.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/io.h>
>>   #include <linux/mailbox_client.h>
>> @@ -86,6 +87,7 @@ struct qmp {
>>   	struct clk_hw qdss_clk;
>>   	struct genpd_onecell_data pd_data;
>>   	struct qmp_cooling_device *cooling_devs;
>> +	struct dentry *debugfs_file;
>>   };
>>   
>>   struct qmp_pd {
>> @@ -566,6 +568,31 @@ void qmp_put(struct qmp *qmp)
>>   }
>>   EXPORT_SYMBOL(qmp_put);
>>   
>> +static ssize_t aoss_dbg_write(struct file *file, const char __user *userstr,
>> +			      size_t len, loff_t *pos)
>> +{
>> +	struct qmp *qmp = file->private_data;
>> +	char buf[QMP_MSG_LEN] = {};
>> +	int ret;
>> +
>> +	if (!len || len >= QMP_MSG_LEN)
>> +		return -EINVAL;
>> +
>> +	ret  = copy_from_user(buf, userstr, len);
>> +	if (ret) {
>> +		return -EFAULT;
>> +	}
>> +
>> +	ret = qmp_send(qmp, buf, QMP_MSG_LEN);
>> +
>> +	return ret ? ret : len;
>> +}
>> +
>> +static const struct file_operations aoss_dbg_fops = {
>> +	.open = simple_open,
>> +	.write = aoss_dbg_write,
>> +};
>> +
>>   static int qmp_probe(struct platform_device *pdev)
>>   {
>>   	struct resource *res;
>> @@ -620,6 +647,9 @@ static int qmp_probe(struct platform_device *pdev)
>>   
>>   	platform_set_drvdata(pdev, qmp);
>>   
>> +	qmp->debugfs_file = debugfs_create_file("aoss_send_message", 0220, NULL,
>> +						qmp, &aoss_dbg_fops);
>> +
>>   	return 0;
>>   
>>   err_remove_qdss_clk:
>> @@ -636,6 +666,8 @@ static int qmp_remove(struct platform_device *pdev)
>>   {
>>   	struct qmp *qmp = platform_get_drvdata(pdev);
>>   
>> +	debugfs_remove(qmp->debugfs_file);
>> +
>>   	qmp_qdss_clk_remove(qmp);
>>   	qmp_pd_remove(qmp);
>>   	qmp_cooling_devices_remove(qmp);
>> -- 
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>

Re: [PATCH V7 1/2] soc: qcom: aoss: Expose send for generic usecase

[Deepak Kumar Singh]

On 8/31/2021 4:55 AM, Bjorn Andersson wrote:
> On Mon 30 Aug 06:37 CDT 2021, Deepak Kumar Singh wrote:
>
>> Not all upcoming usecases will have an interface to allow the aoss
>> driver to hook onto. Expose the send api and create a get function to
>> enable drivers to send their own messages to aoss.
>>
>> Signed-off-by: Chris Lew <clew@codeaurora.org>
>> Signed-off-by: Deepak Kumar Singh <deesin@codeaurora.org>
>> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>> ---
>>   drivers/soc/qcom/qcom_aoss.c       | 54 +++++++++++++++++++++++++++++++++++++-
>>   include/linux/soc/qcom/qcom_aoss.h | 38 +++++++++++++++++++++++++++
>>   2 files changed, 91 insertions(+), 1 deletion(-)
>>   create mode 100644 include/linux/soc/qcom/qcom_aoss.h
>>
>> diff --git a/drivers/soc/qcom/qcom_aoss.c b/drivers/soc/qcom/qcom_aoss.c
>> index 934fcc4..bf0a6280 100644
>> --- a/drivers/soc/qcom/qcom_aoss.c
>> +++ b/drivers/soc/qcom/qcom_aoss.c
>> @@ -8,10 +8,12 @@
>>   #include <linux/io.h>
>>   #include <linux/mailbox_client.h>
>>   #include <linux/module.h>
>> +#include <linux/of_platform.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/pm_domain.h>
>>   #include <linux/thermal.h>
>>   #include <linux/slab.h>
>> +#include <linux/soc/qcom/qcom_aoss.h>
>>   
>>   #define QMP_DESC_MAGIC			0x0
>>   #define QMP_DESC_VERSION		0x4
>> @@ -223,11 +225,14 @@ static bool qmp_message_empty(struct qmp *qmp)
>>    *
>>    * Return: 0 on success, negative errno on failure
>>    */
>> -static int qmp_send(struct qmp *qmp, const void *data, size_t len)
>> +int qmp_send(struct qmp *qmp, const void *data, size_t len)
>>   {
>>   	long time_left;
>>   	int ret;
>>   
>> +	if (WARN_ON(IS_ERR_OR_NULL(qmp) || !data))
>> +		return -EINVAL;
>> +
>>   	if (WARN_ON(len + sizeof(u32) > qmp->size))
>>   		return -EINVAL;
>>   
>> @@ -261,6 +266,7 @@ static int qmp_send(struct qmp *qmp, const void *data, size_t len)
>>   
>>   	return ret;
>>   }
>> +EXPORT_SYMBOL(qmp_send);
>>   
>>   static int qmp_qdss_clk_prepare(struct clk_hw *hw)
>>   {
>> @@ -515,6 +521,51 @@ static void qmp_cooling_devices_remove(struct qmp *qmp)
>>   		thermal_cooling_device_unregister(qmp->cooling_devs[i].cdev);
>>   }
>>   
>> +/**
>> + * qmp_get() - get a qmp handle from a device
>> + * @dev: client device pointer
>> + *
>> + * Return: handle to qmp device on success, ERR_PTR() on failure
>> + */
>> +struct qmp *qmp_get(struct device *dev)
>> +{
>> +	struct platform_device *pdev;
>> +	struct device_node *np;
>> +	struct qmp *qmp;
>> +
>> +	if (!dev || !dev->of_node)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	np = of_parse_phandle(dev->of_node, "qcom,qmp", 0);
>> +	if (!np)
>> +		return ERR_PTR(-ENODEV);
>> +
>> +	pdev = of_find_device_by_node(np);
>> +	of_node_put(np);
>> +	if (!pdev)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	qmp = platform_get_drvdata(pdev);
>> +
>> +	return qmp ? qmp : ERR_PTR(-EPROBE_DEFER);
>> +}
>> +EXPORT_SYMBOL(qmp_get);
>> +
>> +/**
>> + * qmp_put() - release a qmp handle
>> + * @qmp: qmp handle obtained from qmp_get()
>> + */
>> +void qmp_put(struct qmp *qmp)
>> +{
>> +	if (!IS_ERR_OR_NULL(qmp))
>> +		put_device(qmp->dev);
>> +	/*
>> +	 * Match get_device() inside of_find_device_by_node() in
>> +	 * qmp_get()
>> +	 */
> Afaict this comment relates to the put_device() above, which typically
> would imply that the comment should be above or inside the if
> statement?
I think it will be good to put before if block. Moved comment before if 
block in V8.
>> +}
>> +EXPORT_SYMBOL(qmp_put);
>> +
> Regards,
> Bjorn
>
>>   static int qmp_probe(struct platform_device *pdev)
>>   {
>>   	struct resource *res;
>> @@ -610,6 +661,7 @@ static struct platform_driver qmp_driver = {
>>   	.driver = {
>>   		.name		= "qcom_aoss_qmp",
>>   		.of_match_table	= qmp_dt_match,
>> +		.suppress_bind_attrs = true,
>>   	},
>>   	.probe = qmp_probe,
>>   	.remove	= qmp_remove,
>> diff --git a/include/linux/soc/qcom/qcom_aoss.h b/include/linux/soc/qcom/qcom_aoss.h
>> new file mode 100644
>> index 0000000..3c2a82e
>> --- /dev/null
>> +++ b/include/linux/soc/qcom/qcom_aoss.h
>> @@ -0,0 +1,38 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#ifndef __QCOM_AOSS_H__
>> +#define __QCOM_AOSS_H__
>> +
>> +#include <linux/err.h>
>> +#include <linux/device.h>
>> +
>> +struct qmp;
>> +
>> +#if IS_ENABLED(CONFIG_QCOM_AOSS_QMP)
>> +
>> +int qmp_send(struct qmp *qmp, const void *data, size_t len);
>> +struct qmp *qmp_get(struct device *dev);
>> +void qmp_put(struct qmp *qmp);
>> +
>> +#else
>> +
>> +static inline int qmp_send(struct qmp *qmp, const void *data, size_t len)
>> +{
>> +	return -ENODEV;
>> +}
>> +
>> +static inline struct qmp *qmp_get(struct device *dev)
>> +{
>> +	return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static inline void qmp_put(struct qmp *qmp)
>> +{
>> +}
>> +
>> +#endif
>> +
>> +#endif
>> -- 
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>