2018-11-12 08:03:34

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 0/3] Qualcomm AOSS QMP side channel binding and driver

Add binding and driver for the QMP based side-channel communication mechanism
to the AOSS, which is used to control resources not exposed through the RPMh
interface.

Currently implemented is a genpd provider, but pending some improvements in the
thermal framework a cooling device will be added at a later point.

Bjorn Andersson (3):
dt-bindings: soc: qcom: Add AOSS QMP binding
soc: qcom: Add AOSS QMP communication driver
soc: qcom: Add AOSS QMP genpd provider

.../bindings/soc/qcom/qcom,aoss-qmp.txt | 63 ++++
drivers/soc/qcom/Kconfig | 15 +
drivers/soc/qcom/Makefile | 2 +
drivers/soc/qcom/aoss-qmp-pd.c | 135 ++++++++
drivers/soc/qcom/aoss-qmp.c | 313 ++++++++++++++++++
include/dt-bindings/power/qcom-aoss-qmp.h | 15 +
include/linux/soc/qcom/aoss-qmp.h | 12 +
7 files changed, 555 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
create mode 100644 drivers/soc/qcom/aoss-qmp-pd.c
create mode 100644 drivers/soc/qcom/aoss-qmp.c
create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
create mode 100644 include/linux/soc/qcom/aoss-qmp.h

--
2.18.0



2018-11-12 08:03:09

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 2/3] soc: qcom: Add AOSS QMP communication driver

The AOSS QMP driver is used to communicate with the AOSS for certain
side-channel requests, that are not enabled through the RPMh interface.

The communication is a very simple synchronous mechanism of messages
being written in message RAM and a doorbell in the AOSS is rung. As the
AOSS has processed the message length is cleared and an interrupt is
fired by the AOSS as acknowledgment.

Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/soc/qcom/Kconfig | 7 +
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/aoss-qmp.c | 313 ++++++++++++++++++++++++++++++
include/linux/soc/qcom/aoss-qmp.h | 12 ++
4 files changed, 333 insertions(+)
create mode 100644 drivers/soc/qcom/aoss-qmp.c
create mode 100644 include/linux/soc/qcom/aoss-qmp.h

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index a51458022d21..ba08fc00d7f5 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -3,6 +3,13 @@
#
menu "Qualcomm SoC drivers"

+config QCOM_AOSS_QMP
+ tristate "Qualcomm AOSS Messaging Driver"
+ help
+ This driver provides the means for communicating with the
+ micro-controller in the AOSS, using QMP, to control certain resource
+ that are not exposed through RPMh.
+
config QCOM_COMMAND_DB
bool "Qualcomm Command DB"
depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 67cb85d0373c..d0d7fdc94d9a 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS_rpmh-rsc.o := -I$(src)
+obj-$(CONFIG_QCOM_AOSS_QMP) += aoss-qmp.o
obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
obj-$(CONFIG_QCOM_GLINK_SSR) += glink_ssr.o
diff --git a/drivers/soc/qcom/aoss-qmp.c b/drivers/soc/qcom/aoss-qmp.c
new file mode 100644
index 000000000000..acc5677a06ed
--- /dev/null
+++ b/drivers/soc/qcom/aoss-qmp.c
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, Linaro Ltd
+ */
+#include <linux/interrupt.h>
+#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/soc/qcom/aoss-qmp.h>
+#include <linux/wait.h>
+
+#define QMP_DESC_MAGIC 0x0
+#define QMP_DESC_VERSION 0x4
+#define QMP_DESC_FEATURES 0x8
+
+#define QMP_DESC_UCORE_LINK_STATE 0xc
+#define QMP_DESC_UCORE_LINK_STATE_ACK 0x10
+#define QMP_DESC_UCORE_CH_STATE 0x14
+#define QMP_DESC_UCORE_CH_STATE_ACK 0x18
+#define QMP_DESC_UCORE_MBOX_SIZE 0x1c
+#define QMP_DESC_UCORE_MBOX_OFFSET 0x20
+
+#define QMP_DESC_MCORE_LINK_STATE 0x24
+#define QMP_DESC_MCORE_LINK_STATE_ACK 0x28
+#define QMP_DESC_MCORE_CH_STATE 0x2c
+#define QMP_DESC_MCORE_CH_STATE_ACK 0x30
+#define QMP_DESC_MCORE_MBOX_SIZE 0x34
+#define QMP_DESC_MCORE_MBOX_OFFSET 0x38
+
+#define QMP_STATE_UP 0x0000ffff
+#define QMP_STATE_DOWN 0xffff0000
+
+#define QMP_MAGIC 0x4d41494c
+#define QMP_VERSION 1
+
+/**
+ * struct qmp - driver state for QMP implementation
+ * @msgram: iomem referencing the message RAM used for communication
+ * @dev: reference to QMP device
+ * @mbox_client: mailbox client used to ring the doorbell on transmit
+ * @mbox_chan: mailbox channel used to ring the doorbell on transmit
+ * @offset: offset within @msgram where messages should be written
+ * @size: maximum size of the messages to be transmitted
+ * @event: wait_queue for synchronization with the IRQ
+ * @tx_lock: provides syncrhonization between multiple callers of qmp_send()
+ */
+struct qmp {
+ void __iomem *msgram;
+ struct device *dev;
+
+ struct mbox_client mbox_client;
+ struct mbox_chan *mbox_chan;
+
+ size_t offset;
+ size_t size;
+
+ wait_queue_head_t event;
+
+ struct mutex tx_lock;
+};
+
+static void qmp_kick(struct qmp *qmp)
+{
+ mbox_send_message(qmp->mbox_chan, NULL);
+ mbox_client_txdone(qmp->mbox_chan, 0);
+}
+
+static bool qmp_magic_valid(struct qmp *qmp)
+{
+ return readl(qmp->msgram + QMP_DESC_MAGIC) == QMP_MAGIC;
+}
+
+static bool qmp_link_acked(struct qmp *qmp)
+{
+ return readl(qmp->msgram + QMP_DESC_MCORE_LINK_STATE_ACK) == QMP_STATE_UP;
+}
+
+static bool qmp_mcore_channel_acked(struct qmp *qmp)
+{
+ return readl(qmp->msgram + QMP_DESC_MCORE_CH_STATE_ACK) == QMP_STATE_UP;
+}
+
+static bool qmp_ucore_channel_up(struct qmp *qmp)
+{
+ return readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE) == QMP_STATE_UP;
+}
+
+static int qmp_open(struct qmp *qmp)
+{
+ int ret;
+ u32 val;
+
+ ret = wait_event_timeout(qmp->event, qmp_magic_valid(qmp), HZ);
+ if (!ret) {
+ dev_err(qmp->dev, "QMP magic doesn't match\n");
+ return -ETIMEDOUT;
+ }
+
+ val = readl(qmp->msgram + QMP_DESC_VERSION);
+ if (val != QMP_VERSION) {
+ dev_err(qmp->dev, "unsupported QMP version %d\n", val);
+ return -EINVAL;
+ }
+
+ qmp->offset = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_OFFSET);
+ qmp->size = readl(qmp->msgram + QMP_DESC_MCORE_MBOX_SIZE);
+ if (!qmp->size) {
+ dev_err(qmp->dev, "invalid mailbox size 0x%zx\n", qmp->size);
+ return -EINVAL;
+ }
+
+ /* Ack remote core's link state */
+ val = readl(qmp->msgram + QMP_DESC_UCORE_LINK_STATE);
+ writel(val, qmp->msgram + QMP_DESC_UCORE_LINK_STATE_ACK);
+
+ /* Set local core's link state to up */
+ writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_LINK_STATE);
+
+ qmp_kick(qmp);
+
+ ret = wait_event_timeout(qmp->event, qmp_link_acked(qmp), HZ);
+ if (!ret) {
+ dev_err(qmp->dev, "ucore didn't ack link\n");
+ goto timeout_close_link;
+ }
+
+ writel(QMP_STATE_UP, qmp->msgram + QMP_DESC_MCORE_CH_STATE);
+
+ ret = wait_event_timeout(qmp->event, qmp_ucore_channel_up(qmp), HZ);
+ if (!ret) {
+ dev_err(qmp->dev, "ucore didn't open channel\n");
+ goto timeout_close_channel;
+ }
+
+ /* Ack remote core's channel state */
+ val = readl(qmp->msgram + QMP_DESC_UCORE_CH_STATE);
+ writel(val, qmp->msgram + QMP_DESC_UCORE_CH_STATE_ACK);
+
+ qmp_kick(qmp);
+
+ ret = wait_event_timeout(qmp->event, qmp_mcore_channel_acked(qmp), HZ);
+ if (!ret) {
+ dev_err(qmp->dev, "ucore didn't ack channel\n");
+ goto timeout_close_channel;
+ }
+
+ return 0;
+
+timeout_close_channel:
+ writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_CH_STATE);
+
+timeout_close_link:
+ writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_LINK_STATE);
+ qmp_kick(qmp);
+
+ return -ETIMEDOUT;
+}
+
+static void qmp_close(struct qmp *qmp)
+{
+ writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_CH_STATE);
+ writel(QMP_STATE_DOWN, qmp->msgram + QMP_DESC_MCORE_LINK_STATE);
+ qmp_kick(qmp);
+}
+
+static irqreturn_t qmp_intr(int irq, void *data)
+{
+ struct qmp *qmp = data;
+
+ wake_up_interruptible_all(&qmp->event);
+
+ return IRQ_HANDLED;
+}
+
+static bool qmp_message_empty(struct qmp *qmp)
+{
+ return readl(qmp->msgram + qmp->offset) == 0;
+}
+
+/**
+ * qmp_send() - send a message to the AOSS
+ * @qmp: qmp context
+ * @data: message to be sent
+ * @len: length of the message
+ *
+ * Transmit @data to AOSS and wait for the AOSS to acknowledge the message.
+ * @len must be a multiple of 4 and not longer than the mailbox size. Access is
+ * synchronized by this implementation.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int qmp_send(struct qmp *qmp, const void *data, size_t len)
+{
+ int ret;
+
+ if (WARN_ON(len + sizeof(u32) > qmp->size)) {
+ dev_err(qmp->dev, "message too long\n");
+ return -EINVAL;
+ }
+
+ if (WARN_ON(len % sizeof(u32))) {
+ dev_err(qmp->dev, "message not 32-bit aligned\n");
+ return -EINVAL;
+ }
+
+ mutex_lock(&qmp->tx_lock);
+
+ if (!qmp_message_empty(qmp)) {
+ dev_err(qmp->dev, "mailbox left busy\n");
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ /* The message RAM only implements 32-bit accesses */
+ __iowrite32_copy(qmp->msgram + qmp->offset + sizeof(u32),
+ data, len / sizeof(u32));
+ writel(len, qmp->msgram + qmp->offset);
+ qmp_kick(qmp);
+
+ ret = wait_event_interruptible_timeout(qmp->event,
+ qmp_message_empty(qmp), HZ);
+ if (!ret) {
+ dev_err(qmp->dev, "ucore did not ack channel\n");
+ ret = -ETIMEDOUT;
+
+ writel(0, qmp->msgram + qmp->offset);
+ } else {
+ ret = 0;
+ }
+
+out_unlock:
+ mutex_unlock(&qmp->tx_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL(qmp_send);
+
+static int qmp_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct qmp *qmp;
+ int irq;
+ int ret;
+
+ qmp = devm_kzalloc(&pdev->dev, sizeof(*qmp), GFP_KERNEL);
+ if (!qmp)
+ return -ENOMEM;
+
+ qmp->dev = &pdev->dev;
+ init_waitqueue_head(&qmp->event);
+ mutex_init(&qmp->tx_lock);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ qmp->msgram = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(qmp->msgram))
+ return PTR_ERR(qmp->msgram);
+
+ qmp->mbox_client.dev = &pdev->dev;
+ qmp->mbox_client.knows_txdone = true;
+ qmp->mbox_chan = mbox_request_channel(&qmp->mbox_client, 0);
+ if (IS_ERR(qmp->mbox_chan)) {
+ dev_err(&pdev->dev, "failed to acquire ipc mailbox\n");
+ return PTR_ERR(qmp->mbox_chan);
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ ret = devm_request_irq(&pdev->dev, irq, qmp_intr, IRQF_ONESHOT,
+ "aoss-qmp", qmp);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to request interrupt\n");
+ return ret;
+ }
+
+ ret = qmp_open(qmp);
+ if (ret < 0)
+ return ret;
+
+ platform_set_drvdata(pdev, qmp);
+
+ return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+}
+
+static int qmp_remove(struct platform_device *pdev)
+{
+ struct qmp *qmp = platform_get_drvdata(pdev);
+
+ of_platform_depopulate(&pdev->dev);
+
+ qmp_close(qmp);
+
+ return 0;
+}
+
+static const struct of_device_id qmp_dt_match[] = {
+ { .compatible = "qcom,sdm845-aoss-qmp", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, qmp_dt_match);
+
+static struct platform_driver qmp_driver = {
+ .driver = {
+ .name = "aoss_qmp",
+ .of_match_table = qmp_dt_match,
+ },
+ .probe = qmp_probe,
+ .remove = qmp_remove,
+};
+module_platform_driver(qmp_driver);
+
+MODULE_DESCRIPTION("Qualcomm AOSS QMP driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/soc/qcom/aoss-qmp.h b/include/linux/soc/qcom/aoss-qmp.h
new file mode 100644
index 000000000000..32ccaa091a9f
--- /dev/null
+++ b/include/linux/soc/qcom/aoss-qmp.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018, Linaro Ltd
+ */
+#ifndef __AOP_QMP_H__
+#define __AOP_QMP_H__
+
+struct qmp;
+
+int qmp_send(struct qmp *qmp, const void *data, size_t len);
+
+#endif
--
2.18.0


2018-11-12 08:04:09

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 3/3] soc: qcom: Add AOSS QMP genpd provider

The AOSS QMP genpd provider implements control over power-related
resources related to low-power state associated with the remoteprocs in
the system as well as control over a set of clocks related to debug
hardware in the SoC.

Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/soc/qcom/Kconfig | 8 ++
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/aoss-qmp-pd.c | 135 +++++++++++++++++++++++++++++++++
3 files changed, 144 insertions(+)
create mode 100644 drivers/soc/qcom/aoss-qmp-pd.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index ba08fc00d7f5..e1eda3d59748 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -10,6 +10,14 @@ config QCOM_AOSS_QMP
micro-controller in the AOSS, using QMP, to control certain resource
that are not exposed through RPMh.

+config QCOM_AOSS_QMP_PD
+ tristate "Qualcomm AOSS Messaging Power Domain driver"
+ depends on QCOM_AOSS_QMP
+ help
+ This driver provides the means of controlling the AOSSs handling of
+ low-power state for resources related to the remoteproc subsystems as
+ well as controlling the debug clocks.
+
config QCOM_COMMAND_DB
bool "Qualcomm Command DB"
depends on ARCH_QCOM || COMPILE_TEST
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index d0d7fdc94d9a..ebfa414a5b77 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS_rpmh-rsc.o := -I$(src)
obj-$(CONFIG_QCOM_AOSS_QMP) += aoss-qmp.o
+obj-$(CONFIG_QCOM_AOSS_QMP_PD) += aoss-qmp-pd.o
obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
obj-$(CONFIG_QCOM_GLINK_SSR) += glink_ssr.o
diff --git a/drivers/soc/qcom/aoss-qmp-pd.c b/drivers/soc/qcom/aoss-qmp-pd.c
new file mode 100644
index 000000000000..467d0db4abfa
--- /dev/null
+++ b/drivers/soc/qcom/aoss-qmp-pd.c
@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, Linaro Ltd
+ */
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/soc/qcom/aoss-qmp.h>
+
+#include <dt-bindings/power/qcom-aoss-qmp.h>
+
+struct qmp_pd {
+ struct qmp *qmp;
+
+ struct generic_pm_domain pd;
+
+ const char *name;
+};
+
+#define to_qmp_pd_resource(res) container_of(res, struct qmp_pd, pd)
+
+struct qmp_pd_resource {
+ const char *name;
+ int (*on)(struct generic_pm_domain *domain);
+ int (*off)(struct generic_pm_domain *domain);
+};
+
+static int qmp_pd_clock_toggle(struct qmp_pd *res, bool enable)
+{
+ char buf[96];
+ size_t n;
+
+ n = snprintf(buf, sizeof(buf), "{class: clock, res: %s, val: %d}",
+ res->name, !!enable);
+ return qmp_send(res->qmp, buf, n);
+}
+
+static int qmp_pd_clock_on(struct generic_pm_domain *domain)
+{
+ return qmp_pd_clock_toggle(to_qmp_pd_resource(domain), true);
+}
+
+static int qmp_pd_clock_off(struct generic_pm_domain *domain)
+{
+ return qmp_pd_clock_toggle(to_qmp_pd_resource(domain), false);
+}
+
+static int qmp_pd_image_toggle(struct qmp_pd *res, bool enable)
+{
+ char buf[96];
+ size_t n;
+
+ n = snprintf(buf, sizeof(buf),
+ "{class: image, res: load_state, name: %s, val: %s}",
+ res->name, enable ? "on" : "off");
+ return qmp_send(res->qmp, buf, sizeof(buf));
+}
+
+static int qmp_pd_image_on(struct generic_pm_domain *domain)
+{
+ return qmp_pd_image_toggle(to_qmp_pd_resource(domain), true);
+}
+
+static int qmp_pd_image_off(struct generic_pm_domain *domain)
+{
+ return qmp_pd_image_toggle(to_qmp_pd_resource(domain), false);
+}
+
+static const struct qmp_pd_resource sdm845_resources[] = {
+ [AOSS_QMP_QDSS_CLK] = { "qdss", qmp_pd_clock_on, qmp_pd_clock_off },
+ [AOSS_QMP_LS_CDSP] = { "cdsp", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_LPASS] = { "adsp", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_MODEM] = { "modem", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_SLPI] = { "slpi", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_SPSS] = { "spss", qmp_pd_image_on, qmp_pd_image_off },
+ [AOSS_QMP_LS_VENUS] = { "venus", qmp_pd_image_on, qmp_pd_image_off },
+};
+
+static int qmp_pd_probe(struct platform_device *pdev)
+{
+ struct genpd_onecell_data *data;
+ struct qmp_pd *res;
+ struct qmp *qmp;
+ size_t num = ARRAY_SIZE(sdm845_resources);
+ int i;
+
+ qmp = dev_get_drvdata(pdev->dev.parent);
+ if (!qmp)
+ return -EINVAL;
+
+ res = devm_kcalloc(&pdev->dev, num, sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return -ENOMEM;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->domains = devm_kcalloc(&pdev->dev, num, sizeof(*data->domains),
+ GFP_KERNEL);
+
+ for (i = 0; i < num; i++) {
+ pm_genpd_init(&res[i].pd, NULL, true);
+ res[i].qmp = qmp;
+ res[i].name = sdm845_resources[i].name;
+
+ res[i].pd.name = sdm845_resources[i].name;
+ res[i].pd.power_on = sdm845_resources[i].on;
+ res[i].pd.power_off = sdm845_resources[i].off;
+
+ data->domains[data->num_domains++] = &res[i].pd;
+ }
+
+ return of_genpd_add_provider_onecell(pdev->dev.of_node, data);
+}
+
+static const struct of_device_id qmp_pd_dt_match[] = {
+ { .compatible = "qcom,sdm845-aoss-qmp-pd", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, qmp_pd_dt_match);
+
+static struct platform_driver qmp_pd_driver = {
+ .driver = {
+ .name = "aoss_qmp_pd",
+ .of_match_table = qmp_pd_dt_match,
+ },
+ .probe = qmp_pd_probe,
+};
+module_platform_driver(qmp_pd_driver);
+
+MODULE_DESCRIPTION("Qualcomm AOSS QMP load-state driver");
+MODULE_LICENSE("GPL v2");
--
2.18.0


2018-11-12 08:04:32

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH 1/3] dt-bindings: soc: qcom: Add AOSS QMP binding

Add binding for the QMP based side-channel communication mechanism to
the AOSS, which is used to control resources not exposed through the
RPMh interface.

Signed-off-by: Bjorn Andersson <[email protected]>
---
.../bindings/soc/qcom/qcom,aoss-qmp.txt | 63 +++++++++++++++++++
include/dt-bindings/power/qcom-aoss-qmp.h | 15 +++++
2 files changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
new file mode 100644
index 000000000000..e3c8cb4372f2
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
@@ -0,0 +1,63 @@
+Qualcomm Always-On Subsystem side channel binding
+
+This binding describes the hardware component responsible for side channel
+requests to the always-on subsystem (AOSS), used for certain power management
+requests that is not handled by the standard RPMh interface. Each client in the
+SoC has it's own block of message RAM and IRQ for communication with the AOSS.
+The protocol used to communicate in the message RAM is known as QMP.
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: must be "qcom,sdm845-aoss-qmp"
+
+- reg:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: the base address and size of the message RAM for this
+ client's communication with the AOSS
+
+- interrupts:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: should specify the AOSS message IRQ for this client
+
+- mboxes:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: reference to the mailbox representing the outgoing doorbell
+ in APCS for this client, as described in mailbox/mailbox.txt
+
+= AOSS Power-domains
+The AOSS side channel exposes control over a set of resources, used to control
+a set of debug related clocks and to affect the low power state of resources
+related to the secondary subsystems. These resources are described as a set of
+power-domains in a subnode of hte AOSS side channel node.
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: must be "qcom,sdm845-aoss-qmp-pd"
+
+- #power-domain-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: must be 1
+
+= EXAMPLE
+
+The following example represents the AOSS side-channel message RAM and the
+mechanism exposing the power-domains, as found in SDM845.
+
+ aoss_qmp: qmp@c300000 {
+ compatible = "qcom,sdm845-aoss-qmp";
+ reg = <0x0c300000 0x100000>;
+ interrupts = <GIC_SPI 389 IRQ_TYPE_EDGE_RISING>;
+
+ mboxes = <&apss_shared 0>;
+
+ aoss_qmp_pd: power-controller {
+ compatible = "qcom,sdm845-aoss-qmp-pd";
+ #power-domain-cells = <1>;
+ };
+ };
diff --git a/include/dt-bindings/power/qcom-aoss-qmp.h b/include/dt-bindings/power/qcom-aoss-qmp.h
new file mode 100644
index 000000000000..7d8ac1a4f90c
--- /dev/null
+++ b/include/dt-bindings/power/qcom-aoss-qmp.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2018, Linaro Ltd. */
+
+#ifndef __DT_BINDINGS_POWER_QCOM_AOSS_QMP_H
+#define __DT_BINDINGS_POWER_QCOM_AOSS_QMP_H
+
+#define AOSS_QMP_QDSS_CLK 0
+#define AOSS_QMP_LS_CDSP 1
+#define AOSS_QMP_LS_LPASS 2
+#define AOSS_QMP_LS_MODEM 3
+#define AOSS_QMP_LS_SLPI 4
+#define AOSS_QMP_LS_SPSS 5
+#define AOSS_QMP_LS_VENUS 6
+
+#endif
--
2.18.0


2018-11-27 03:44:25

by Sai Prakash Ranjan

[permalink] [raw]
Subject: Re: [PATCH 3/3] soc: qcom: Add AOSS QMP genpd provider

Hi Bjorn,

On 11/12/2018 1:35 PM, Bjorn Andersson wrote:
> The AOSS QMP genpd provider implements control over power-related
> resources related to low-power state associated with the remoteprocs in
> the system as well as control over a set of clocks related to debug
> hardware in the SoC.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/soc/qcom/Kconfig | 8 ++
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/aoss-qmp-pd.c | 135 +++++++++++++++++++++++++++++++++
> 3 files changed, 144 insertions(+)
> create mode 100644 drivers/soc/qcom/aoss-qmp-pd.c
>
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index ba08fc00d7f5..e1eda3d59748 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -10,6 +10,14 @@ config QCOM_AOSS_QMP
> micro-controller in the AOSS, using QMP, to control certain resource
> that are not exposed through RPMh.
>
> +config QCOM_AOSS_QMP_PD
> + tristate "Qualcomm AOSS Messaging Power Domain driver"
> + depends on QCOM_AOSS_QMP
> + help
> + This driver provides the means of controlling the AOSSs handling of
> + low-power state for resources related to the remoteproc subsystems as
> + well as controlling the debug clocks.
> +
> config QCOM_COMMAND_DB
> bool "Qualcomm Command DB"
> depends on ARCH_QCOM || COMPILE_TEST
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index d0d7fdc94d9a..ebfa414a5b77 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> CFLAGS_rpmh-rsc.o := -I$(src)
> obj-$(CONFIG_QCOM_AOSS_QMP) += aoss-qmp.o
> +obj-$(CONFIG_QCOM_AOSS_QMP_PD) += aoss-qmp-pd.o
> obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
> obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
> obj-$(CONFIG_QCOM_GLINK_SSR) += glink_ssr.o
> diff --git a/drivers/soc/qcom/aoss-qmp-pd.c b/drivers/soc/qcom/aoss-qmp-pd.c
> new file mode 100644
> index 000000000000..467d0db4abfa
> --- /dev/null
> +++ b/drivers/soc/qcom/aoss-qmp-pd.c
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, Linaro Ltd
> + */
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> +#include <linux/soc/qcom/aoss-qmp.h>
> +
> +#include <dt-bindings/power/qcom-aoss-qmp.h>
> +
> +struct qmp_pd {
> + struct qmp *qmp;
> +
> + struct generic_pm_domain pd;
> +
> + const char *name;
> +};
> +
> +#define to_qmp_pd_resource(res) container_of(res, struct qmp_pd, pd)
> +
> +struct qmp_pd_resource {
> + const char *name;
> + int (*on)(struct generic_pm_domain *domain);
> + int (*off)(struct generic_pm_domain *domain);
> +};
> +
> +static int qmp_pd_clock_toggle(struct qmp_pd *res, bool enable)
> +{
> + char buf[96];
> + size_t n;
> +
> + n = snprintf(buf, sizeof(buf), "{class: clock, res: %s, val: %d}",
> + res->name, !!enable);
> + return qmp_send(res->qmp, buf, n);
> +}
> +

I was trying to get QDSS working with these patches and found one issue
in qmp_send of qmp_pd_clock_toggle.

The third return value should be sizeof(buf) instead of n because n just
returns len as 33 and the below check in qmp send will always fail and
trigger WARN_ON's.

if (WARN_ON(len % sizeof(u32))) {
dev_err(qmp->dev, "message not 32-bit aligned\n");
return -EINVAL;
}

Also I observed that multiple "ucore will not ack channel" messages with
len being returned n instead of buf size.

One more thing is do we really require *WARN_ON and dev_err* both
because it just spams the kernel logs, I think dev_err message is clear
enough to be able to understand the error condition.

Thanks,
Sai

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2018-11-27 13:28:45

by Sai Prakash Ranjan

[permalink] [raw]
Subject: Re: [PATCH 3/3] soc: qcom: Add AOSS QMP genpd provider

Hi Bjorn,

On 11/12/2018 1:35 PM, Bjorn Andersson wrote:
> The AOSS QMP genpd provider implements control over power-related
> resources related to low-power state associated with the remoteprocs in
> the system as well as control over a set of clocks related to debug
> hardware in the SoC.
>
> Signed-off-by: Bjorn Andersson <[email protected]>

One more issue is that amba bus probe fails and coresight(qdss) does not
work with these because of clocks being modeled as power-domain.

Below is the log snippet:

[ 4.580715] coresight-etm4x: probe of 7040000.etm failed with error -2
[ 4.588087] coresight-etm4x: probe of 7140000.etm failed with error -2
[ 4.595407] coresight-etm4x: probe of 7240000.etm failed with error -2
[ 4.602796] coresight-etm4x: probe of 7340000.etm failed with error -2
[ 4.610108] coresight-etm4x: probe of 7440000.etm failed with error -2
[ 4.617453] coresight-etm4x: probe of 7540000.etm failed with error -2
[ 4.624831] coresight-etm4x: probe of 7640000.etm failed with error -2
[ 4.632190] coresight-etm4x: probe of 7740000.etm failed with error -2

This is because Amba bus probe has amba_get_enable_pclk() which gets
apb_pclk and returns error if it can't get that clk.

Just for testing, I ignored amba_get_enable_pclk() in probe and
coresight seems to work fine.

Thanks,
Sai

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

2018-12-03 23:53:03

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: soc: qcom: Add AOSS QMP binding

On Mon, Nov 12, 2018 at 12:05:55AM -0800, Bjorn Andersson wrote:
> Add binding for the QMP based side-channel communication mechanism to
> the AOSS, which is used to control resources not exposed through the
> RPMh interface.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> .../bindings/soc/qcom/qcom,aoss-qmp.txt | 63 +++++++++++++++++++
> include/dt-bindings/power/qcom-aoss-qmp.h | 15 +++++
> 2 files changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> create mode 100644 include/dt-bindings/power/qcom-aoss-qmp.h
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> new file mode 100644
> index 000000000000..e3c8cb4372f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,aoss-qmp.txt
> @@ -0,0 +1,63 @@
> +Qualcomm Always-On Subsystem side channel binding
> +
> +This binding describes the hardware component responsible for side channel
> +requests to the always-on subsystem (AOSS), used for certain power management
> +requests that is not handled by the standard RPMh interface. Each client in the
> +SoC has it's own block of message RAM and IRQ for communication with the AOSS.
> +The protocol used to communicate in the message RAM is known as QMP.
> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must be "qcom,sdm845-aoss-qmp"
> +
> +- reg:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: the base address and size of the message RAM for this
> + client's communication with the AOSS
> +
> +- interrupts:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: should specify the AOSS message IRQ for this client
> +
> +- mboxes:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: reference to the mailbox representing the outgoing doorbell
> + in APCS for this client, as described in mailbox/mailbox.txt
> +
> += AOSS Power-domains
> +The AOSS side channel exposes control over a set of resources, used to control
> +a set of debug related clocks and to affect the low power state of resources
> +related to the secondary subsystems. These resources are described as a set of
> +power-domains in a subnode of hte AOSS side channel node.

Why does this need to be a sub-node? Are there other sub-nodes needed?

> +
> +- compatible:
> + Usage: required
> + Value type: <string>
> + Definition: must be "qcom,sdm845-aoss-qmp-pd"
> +
> +- #power-domain-cells:
> + Usage: required
> + Value type: <u32>
> + Definition: must be 1

2018-12-11 00:46:34

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH 1/3] dt-bindings: soc: qcom: Add AOSS QMP binding

Hi,

On Mon, Nov 12, 2018 at 12:02 AM Bjorn Andersson
<[email protected]> wrote:
> + aoss_qmp: qmp@c300000 {
> + compatible = "qcom,sdm845-aoss-qmp";
> + reg = <0x0c300000 0x100000>;

drive-by nit: s/0x0c300000/0xc300000

...another random suggestion is that I think Rob H. has been
requesting that examples have the #include in them...

-Doug

2018-12-26 21:05:48

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 2/3] soc: qcom: Add AOSS QMP communication driver

On Tue 20 Nov 04:22 PST 2018, Arun Kumar Neelakantam wrote:

Thanks for the review Arun.

> On 11/12/2018 1:35 PM, Bjorn Andersson wrote:
[..]
> > +int qmp_send(struct qmp *qmp, const void *data, size_t len)
> > +{
> > + int ret;
> > +
> > + if (WARN_ON(len + sizeof(u32) > qmp->size)) {
> > + dev_err(qmp->dev, "message too long\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (WARN_ON(len % sizeof(u32))) {
> > + dev_err(qmp->dev, "message not 32-bit aligned\n");
> > + return -EINVAL;
> > + }
> > +
> > + mutex_lock(&qmp->tx_lock);
> > +
> > + if (!qmp_message_empty(qmp)) {
> > + dev_err(qmp->dev, "mailbox left busy\n");
> > + ret = -EINVAL;
> should it be -EBUSY ?

That makes more sense.

> And qmp_messge_empty will be done either by remote if it process the data
> else by this driver in TIMEOUT case, so does we need this check for every TX
> ? I think we can just reset to Zero once in open time.

Didn't think about that, should we really make the QMP link ready again
when we get a timeout? Can we expect that the firmware of the remote
side is ready to serve future messages?


Should we keep this check and remove the writel() below?

> > + goto out_unlock;
> > + }
> > +
> > + /* The message RAM only implements 32-bit accesses */
> > + __iowrite32_copy(qmp->msgram + qmp->offset + sizeof(u32),
> > + data, len / sizeof(u32));
> > + writel(len, qmp->msgram + qmp->offset);
> > + qmp_kick(qmp);
> > +
> > + ret = wait_event_interruptible_timeout(qmp->event,
> > + qmp_message_empty(qmp), HZ);
> > + if (!ret) {
> > + dev_err(qmp->dev, "ucore did not ack channel\n");
> > + ret = -ETIMEDOUT;
> > +
> > + writel(0, qmp->msgram + qmp->offset);
> > + } else {
> > + ret = 0;
> > + }
> > +
> > +out_unlock:
> > + mutex_unlock(&qmp->tx_lock);
> > +
> > + return ret;
> > +}

Regards,
Bjorn

2018-12-26 22:25:14

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 3/3] soc: qcom: Add AOSS QMP genpd provider

On Mon 26 Nov 19:31 PST 2018, Sai Prakash Ranjan wrote:

> Hi Bjorn,
>

Thanks for your review Sai!

> On 11/12/2018 1:35 PM, Bjorn Andersson wrote:
[..]
> > +static int qmp_pd_clock_toggle(struct qmp_pd *res, bool enable)
> > +{
> > + char buf[96];
> > + size_t n;
> > +
> > + n = snprintf(buf, sizeof(buf), "{class: clock, res: %s, val: %d}",
> > + res->name, !!enable);
> > + return qmp_send(res->qmp, buf, n);
> > +}
> > +
>
> I was trying to get QDSS working with these patches and found one issue
> in qmp_send of qmp_pd_clock_toggle.
>
> The third return value should be sizeof(buf) instead of n because n just
> returns len as 33 and the below check in qmp send will always fail and
> trigger WARN_ON's.
>
> if (WARN_ON(len % sizeof(u32))) {
> dev_err(qmp->dev, "message not 32-bit aligned\n");
> return -EINVAL;
> }
>
> Also I observed that multiple "ucore will not ack channel" messages with
> len being returned n instead of buf size.
>

I must have been "lucky" when I did my final pass of cleanups and
retests, thanks for spotting this!

> One more thing is do we really require *WARN_ON and dev_err* both because it
> just spams the kernel logs, I think dev_err message is clear
> enough to be able to understand the error condition.
>

No, that's just unnecessary.

Regards,
Bjorn

2019-01-03 23:41:09

by Arun Kumar Neelakantam

[permalink] [raw]
Subject: Re: [PATCH 2/3] soc: qcom: Add AOSS QMP communication driver


On 12/27/2018 1:58 AM, Bjorn Andersson wrote:
> On Tue 20 Nov 04:22 PST 2018, Arun Kumar Neelakantam wrote:
>
> Thanks for the review Arun.
>
>> On 11/12/2018 1:35 PM, Bjorn Andersson wrote:
> [..]
>>> +int qmp_send(struct qmp *qmp, const void *data, size_t len)
>>> +{
>>> + int ret;
>>> +
>>> + if (WARN_ON(len + sizeof(u32) > qmp->size)) {
>>> + dev_err(qmp->dev, "message too long\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + if (WARN_ON(len % sizeof(u32))) {
>>> + dev_err(qmp->dev, "message not 32-bit aligned\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + mutex_lock(&qmp->tx_lock);
>>> +
>>> + if (!qmp_message_empty(qmp)) {
>>> + dev_err(qmp->dev, "mailbox left busy\n");
>>> + ret = -EINVAL;
>> should it be -EBUSY ?
> That makes more sense.
>
>> And qmp_messge_empty will be done either by remote if it process the data
>> else by this driver in TIMEOUT case, so does we need this check for every TX
>> ? I think we can just reset to Zero once in open time.
> Didn't think about that, should we really make the QMP link ready again
> when we get a timeout? Can we expect that the firmware of the remote
> side is ready to serve future messages?
>
>
> Should we keep this check and remove the writel() below?
I prefer we can just remove this check and keep writel() below same as
down stream.
>
>>> + goto out_unlock;
>>> + }
>>> +
>>> + /* The message RAM only implements 32-bit accesses */
>>> + __iowrite32_copy(qmp->msgram + qmp->offset + sizeof(u32),
>>> + data, len / sizeof(u32));
>>> + writel(len, qmp->msgram + qmp->offset);
>>> + qmp_kick(qmp);
>>> +
>>> + ret = wait_event_interruptible_timeout(qmp->event,
>>> + qmp_message_empty(qmp), HZ);
>>> + if (!ret) {
>>> + dev_err(qmp->dev, "ucore did not ack channel\n");
>>> + ret = -ETIMEDOUT;
>>> +
>>> + writel(0, qmp->msgram + qmp->offset);
>>> + } else {
>>> + ret = 0;
>>> + }
>>> +
>>> +out_unlock:
>>> + mutex_unlock(&qmp->tx_lock);
>>> +
>>> + return ret;
>>> +}
> Regards,
> Bjorn