* [RFC PATCH 1/5] remoteproc: qcom: mdt_loader: Make the firmware authentication optional
2018-05-23 5:20 [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Bjorn Andersson
@ 2018-05-23 5:20 ` Bjorn Andersson
2018-05-23 5:20 ` [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling Bjorn Andersson
` (4 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Bjorn Andersson @ 2018-05-23 5:20 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson, Sricharan R, Sibi Sankar,
Rohit kumar, Andy Gross
Cc: linux-kernel, linux-remoteproc, linux-arm-msm
From: Sricharan R <sricharan@codeaurora.org>
qcom_mdt_load function loads the mdt type firmware and
initialises the secure memory as well. Make the initialisation only
when requested by the caller, so that the function can be used
by self-authenticating remoteproc as well.
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
No changes from Sricharan's V6.
drivers/soc/qcom/mdt_loader.c | 87 ++++++++++++++++++++---------
include/linux/soc/qcom/mdt_loader.h | 4 ++
2 files changed, 66 insertions(+), 25 deletions(-)
diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c
index dc09d7ac905f..1c488024c698 100644
--- a/drivers/soc/qcom/mdt_loader.c
+++ b/drivers/soc/qcom/mdt_loader.c
@@ -74,23 +74,10 @@ ssize_t qcom_mdt_get_size(const struct firmware *fw)
}
EXPORT_SYMBOL_GPL(qcom_mdt_get_size);
-/**
- * qcom_mdt_load() - load the firmware which header is loaded as fw
- * @dev: device handle to associate resources with
- * @fw: firmware object for the mdt file
- * @firmware: name of the firmware, for construction of segment file names
- * @pas_id: PAS identifier
- * @mem_region: allocated memory region to load firmware into
- * @mem_phys: physical address of allocated memory region
- * @mem_size: size of the allocated memory region
- * @reloc_base: adjusted physical address after relocation
- *
- * Returns 0 on success, negative errno otherwise.
- */
-int qcom_mdt_load(struct device *dev, const struct firmware *fw,
- const char *firmware, int pas_id, void *mem_region,
- phys_addr_t mem_phys, size_t mem_size,
- phys_addr_t *reloc_base)
+static int __qcom_mdt_load(struct device *dev, const struct firmware *fw,
+ const char *firmware, int pas_id, void *mem_region,
+ phys_addr_t mem_phys, size_t mem_size,
+ phys_addr_t *reloc_base, bool pas_init)
{
const struct elf32_phdr *phdrs;
const struct elf32_phdr *phdr;
@@ -121,10 +108,12 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
if (!fw_name)
return -ENOMEM;
- ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size);
- if (ret) {
- dev_err(dev, "invalid firmware metadata\n");
- goto out;
+ if (pas_init) {
+ ret = qcom_scm_pas_init_image(pas_id, fw->data, fw->size);
+ if (ret) {
+ dev_err(dev, "invalid firmware metadata\n");
+ goto out;
+ }
}
for (i = 0; i < ehdr->e_phnum; i++) {
@@ -144,10 +133,13 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
}
if (relocate) {
- ret = qcom_scm_pas_mem_setup(pas_id, mem_phys, max_addr - min_addr);
- if (ret) {
- dev_err(dev, "unable to setup relocation\n");
- goto out;
+ if (pas_init) {
+ ret = qcom_scm_pas_mem_setup(pas_id, mem_phys,
+ max_addr - min_addr);
+ if (ret) {
+ dev_err(dev, "unable to setup relocation\n");
+ goto out;
+ }
}
/*
@@ -202,7 +194,52 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
return ret;
}
+
+/**
+ * qcom_mdt_load() - load the firmware which header is loaded as fw
+ * @dev: device handle to associate resources with
+ * @fw: firmware object for the mdt file
+ * @firmware: name of the firmware, for construction of segment file names
+ * @pas_id: PAS identifier
+ * @mem_region: allocated memory region to load firmware into
+ * @mem_phys: physical address of allocated memory region
+ * @mem_size: size of the allocated memory region
+ * @reloc_base: adjusted physical address after relocation
+ *
+ * Returns 0 on success, negative errno otherwise.
+ */
+int qcom_mdt_load(struct device *dev, const struct firmware *fw,
+ const char *firmware, int pas_id, void *mem_region,
+ phys_addr_t mem_phys, size_t mem_size,
+ phys_addr_t *reloc_base)
+{
+ return __qcom_mdt_load(dev, fw, firmware, pas_id, mem_region, mem_phys,
+ mem_size, reloc_base, true);
+}
EXPORT_SYMBOL_GPL(qcom_mdt_load);
+/**
+ * qcom_mdt_load_no_init() - load the firmware which header is loaded as fw
+ * @dev: device handle to associate resources with
+ * @fw: firmware object for the mdt file
+ * @firmware: name of the firmware, for construction of segment file names
+ * @pas_id: PAS identifier
+ * @mem_region: allocated memory region to load firmware into
+ * @mem_phys: physical address of allocated memory region
+ * @mem_size: size of the allocated memory region
+ * @reloc_base: adjusted physical address after relocation
+ *
+ * Returns 0 on success, negative errno otherwise.
+ */
+int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
+ const char *firmware, int pas_id,
+ void *mem_region, phys_addr_t mem_phys,
+ size_t mem_size, phys_addr_t *reloc_base)
+{
+ return __qcom_mdt_load(dev, fw, firmware, pas_id, mem_region, mem_phys,
+ mem_size, reloc_base, false);
+}
+EXPORT_SYMBOL_GPL(qcom_mdt_load_no_init);
+
MODULE_DESCRIPTION("Firmware parser for Qualcomm MDT format");
MODULE_LICENSE("GPL v2");
diff --git a/include/linux/soc/qcom/mdt_loader.h b/include/linux/soc/qcom/mdt_loader.h
index 5b98bbdabc25..944b06aefb0f 100644
--- a/include/linux/soc/qcom/mdt_loader.h
+++ b/include/linux/soc/qcom/mdt_loader.h
@@ -17,4 +17,8 @@ int qcom_mdt_load(struct device *dev, const struct firmware *fw,
phys_addr_t mem_phys, size_t mem_size,
phys_addr_t *reloc_base);
+int qcom_mdt_load_no_init(struct device *dev, const struct firmware *fw,
+ const char *fw_name, int pas_id, void *mem_region,
+ phys_addr_t mem_phys, size_t mem_size,
+ phys_addr_t *reloc_base);
#endif
--
2.17.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling
2018-05-23 5:20 [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Bjorn Andersson
2018-05-23 5:20 ` [RFC PATCH 1/5] remoteproc: qcom: mdt_loader: Make the firmware authentication optional Bjorn Andersson
@ 2018-05-23 5:20 ` Bjorn Andersson
2018-06-01 6:16 ` Sricharan R
2018-05-23 5:20 ` [RFC PATCH 3/5] remoteproc: qcom: adsp: Use common q6v5 helpers Bjorn Andersson
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Bjorn Andersson @ 2018-05-23 5:20 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson, Sricharan R, Sibi Sankar, Rohit kumar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Shared between all Hexagon V5 based remoteprocs is the handling of the 5
interrupts and the SMP2P stop request, so break this out into a separate
function in order to allow these drivers to be cleaned up.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/remoteproc/Kconfig | 5 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_q6v5.c | 243 +++++++++++++++++++++++++++++++++
drivers/remoteproc/qcom_q6v5.h | 46 +++++++
4 files changed, 295 insertions(+)
create mode 100644 drivers/remoteproc/qcom_q6v5.c
create mode 100644 drivers/remoteproc/qcom_q6v5.h
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index cd1c168fd188..63b79ea91a21 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -102,6 +102,11 @@ config QCOM_ADSP_PIL
config QCOM_RPROC_COMMON
tristate
+config QCOM_Q6V5_COMMON
+ tristate
+ depends on ARCH_QCOM
+ depends on QCOM_SMEM
+
config QCOM_Q6V5_PIL
tristate "Qualcomm Hexagon V5 Peripherial Image Loader"
depends on OF && ARCH_QCOM
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 02627ede8d4a..5dd0249cf76a 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
obj-$(CONFIG_KEYSTONE_REMOTEPROC) += keystone_remoteproc.o
obj-$(CONFIG_QCOM_ADSP_PIL) += qcom_adsp_pil.o
obj-$(CONFIG_QCOM_RPROC_COMMON) += qcom_common.o
+obj-$(CONFIG_QCOM_Q6V5_COMMON) += qcom_q6v5.o
obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o
obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
new file mode 100644
index 000000000000..9076537a1671
--- /dev/null
+++ b/drivers/remoteproc/qcom_q6v5.c
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Qualcomm Peripheral Image Loader for Q6V5 WCSS
+ *
+ * Copyright (C) 2016-2018 Linaro Ltd.
+ * Copyright (C) 2014 Sony Mobile Communications AB
+ * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
+ */
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/soc/qcom/smem.h>
+#include <linux/soc/qcom/smem_state.h>
+#include <linux/remoteproc.h>
+#include "qcom_q6v5.h"
+
+/**
+ * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before start
+ * @q6v5: reference to qcom_q6v5 context to be reinitialized
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5)
+{
+ reinit_completion(&q6v5->start_done);
+ reinit_completion(&q6v5->stop_done);
+
+ q6v5->running = true;
+ q6v5->handover_issued = false;
+
+ enable_irq(q6v5->handover_irq);
+
+ return 0;
+}
+
+/**
+ * qcom_q6v5_unprepare() - unprepare the qcom_q6v5 context after stop
+ * @q6v5: reference to qcom_q6v5 context to be unprepared
+ *
+ * Return: 0 on success, 1 if handover hasn't yet been called
+ */
+int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5)
+{
+ disable_irq(q6v5->handover_irq);
+
+ return !q6v5->handover_issued;
+}
+
+static irqreturn_t q6v5_wdog_interrupt(int irq, void *data)
+{
+ struct qcom_q6v5 *q6v5 = data;
+ size_t len;
+ char *msg;
+
+ /* Sometimes the stop triggers a watchdog rather than a stop-ack */
+ if (!q6v5->running) {
+ complete(&q6v5->stop_done);
+ return IRQ_HANDLED;
+ }
+
+ msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
+ if (!IS_ERR(msg) && len > 0 && msg[0])
+ dev_err(q6v5->dev, "watchdog received: %s\n", msg);
+ else
+ dev_err(q6v5->dev, "watchdog without message\n");
+
+ rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
+{
+ struct qcom_q6v5 *q6v5 = data;
+ size_t len;
+ char *msg;
+
+ msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
+ if (!IS_ERR(msg) && len > 0 && msg[0])
+ dev_err(q6v5->dev, "fatal error received: %s\n", msg);
+ else
+ dev_err(q6v5->dev, "fatal error without message\n");
+
+ rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t q6v5_ready_interrupt(int irq, void *data)
+{
+ struct qcom_q6v5 *q6v5 = data;
+
+ complete(&q6v5->start_done);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * qcom_q6v5_wait_for_start() - wait for remote processor start signal
+ * @q6v5: reference to qcom_q6v5 context
+ * @timeout: timeout to wait for the event, in jiffies
+ *
+ * qcom_q6v5_unprepare() should not be called when this function fails.
+ *
+ * Return: 0 on success, -ETIMEDOUT on timeout
+ */
+int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout)
+{
+ int ret;
+
+ ret = wait_for_completion_timeout(&q6v5->start_done, timeout);
+ if (!ret)
+ disable_irq(q6v5->handover_irq);
+
+ return !ret ? -ETIMEDOUT : 0;
+}
+
+static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
+{
+ struct qcom_q6v5 *q6v5 = data;
+
+ if (q6v5->handover)
+ q6v5->handover(q6v5);
+
+ q6v5->handover_issued = true;
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t q6v5_stop_interrupt(int irq, void *data)
+{
+ struct qcom_q6v5 *q6v5 = data;
+
+ complete(&q6v5->stop_done);
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * qcom_q6v5_request_stop() - request the remote processor to stop
+ * @q6v5: reference to qcom_q6v5 context
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5)
+{
+ int ret;
+
+ q6v5->running = false;
+
+ qcom_smem_state_update_bits(q6v5->state,
+ BIT(q6v5->stop_bit), BIT(q6v5->stop_bit));
+
+ ret = wait_for_completion_timeout(&q6v5->stop_done, 5 * HZ);
+
+ qcom_smem_state_update_bits(q6v5->state, BIT(q6v5->stop_bit), 0);
+
+ return ret == 0 ? -ETIMEDOUT : 0;
+}
+
+/**
+ * qcom_q6v5_init() - initializer of the q6v5 common struct
+ * @q6v5: handle to be initialized
+ * @pdev: platform_device reference for acquiring resources
+ * @rproc: associated remoteproc instance
+ * @crash_reason: SMEM id for crash reason string, or 0 if none
+ * @handover: function to be called when proxy resources should be released
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
+ struct rproc *rproc, int crash_reason,
+ void (*handover)(struct qcom_q6v5 *q6v5))
+{
+ int ret;
+
+ q6v5->rproc = rproc;
+ q6v5->dev = &pdev->dev;
+ q6v5->crash_reason = crash_reason;
+ q6v5->handover = handover;
+
+ init_completion(&q6v5->start_done);
+ init_completion(&q6v5->stop_done);
+
+ q6v5->wdog_irq = platform_get_irq_byname(pdev, "wdog");
+ ret = devm_request_threaded_irq(&pdev->dev, q6v5->wdog_irq,
+ NULL, q6v5_wdog_interrupt,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ "q6v5 wdog", q6v5);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to acquire wdog IRQ\n");
+ return ret;
+ }
+
+ q6v5->fatal_irq = platform_get_irq_byname(pdev, "fatal");
+ ret = devm_request_threaded_irq(&pdev->dev, q6v5->fatal_irq,
+ NULL, q6v5_fatal_interrupt,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ "q6v5 fatal", q6v5);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to acquire fatal IRQ\n");
+ return ret;
+ }
+
+ q6v5->ready_irq = platform_get_irq_byname(pdev, "ready");
+ ret = devm_request_threaded_irq(&pdev->dev, q6v5->ready_irq,
+ NULL, q6v5_ready_interrupt,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ "q6v5 ready", q6v5);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to acquire ready IRQ\n");
+ return ret;
+ }
+
+ q6v5->handover_irq = platform_get_irq_byname(pdev, "handover");
+ ret = devm_request_threaded_irq(&pdev->dev, q6v5->handover_irq,
+ NULL, q6v5_handover_interrupt,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ "q6v5 handover", q6v5);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to acquire handover IRQ\n");
+ return ret;
+ }
+ disable_irq(q6v5->handover_irq);
+
+ q6v5->stop_irq = platform_get_irq_byname(pdev, "stop-ack");
+ ret = devm_request_threaded_irq(&pdev->dev, q6v5->stop_irq,
+ NULL, q6v5_stop_interrupt,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ "q6v5 stop", q6v5);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to acquire stop-ack IRQ\n");
+ return ret;
+ }
+
+ q6v5->state = qcom_smem_state_get(&pdev->dev, "stop", &q6v5->stop_bit);
+ if (IS_ERR(q6v5->state)) {
+ dev_err(&pdev->dev, "failed to acquire stop state\n");
+ return PTR_ERR(q6v5->state);
+ }
+
+ return 0;
+}
diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h
new file mode 100644
index 000000000000..7ac92c1e0f49
--- /dev/null
+++ b/drivers/remoteproc/qcom_q6v5.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __QCOM_Q6V5_H__
+#define __QCOM_Q6V5_H__
+
+#include <linux/kernel.h>
+#include <linux/completion.h>
+
+struct rproc;
+struct qcom_smem_state;
+
+struct qcom_q6v5 {
+ struct device *dev;
+ struct rproc *rproc;
+
+ struct qcom_smem_state *state;
+ unsigned stop_bit;
+
+ int wdog_irq;
+ int fatal_irq;
+ int ready_irq;
+ int handover_irq;
+ int stop_irq;
+
+ bool handover_issued;
+
+ struct completion start_done;
+ struct completion stop_done;
+
+ int crash_reason;
+
+ bool running;
+
+ void (*handover)(struct qcom_q6v5 *q6v5);
+};
+
+int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
+ struct rproc *rproc, int crash_reason,
+ void (*handover)(struct qcom_q6v5 *q6v5));
+
+int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5);
+int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5);
+int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5);
+int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout);
+
+#endif
--
2.17.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling
2018-05-23 5:20 ` [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling Bjorn Andersson
@ 2018-06-01 6:16 ` Sricharan R
2018-06-01 15:18 ` Sibi S
0 siblings, 1 reply; 19+ messages in thread
From: Sricharan R @ 2018-06-01 6:16 UTC (permalink / raw)
To: Bjorn Andersson, Ohad Ben-Cohen, Sibi Sankar, Rohit kumar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Hi Bjorn,
Thanks for this much needed consolidation.
On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
> Shared between all Hexagon V5 based remoteprocs is the handling of the 5
> interrupts and the SMP2P stop request, so break this out into a separate
> function in order to allow these drivers to be cleaned up.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> drivers/remoteproc/Kconfig | 5 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/qcom_q6v5.c | 243 +++++++++++++++++++++++++++++++++
> drivers/remoteproc/qcom_q6v5.h | 46 +++++++
> 4 files changed, 295 insertions(+)
> create mode 100644 drivers/remoteproc/qcom_q6v5.c
> create mode 100644 drivers/remoteproc/qcom_q6v5.h
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index cd1c168fd188..63b79ea91a21 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -102,6 +102,11 @@ config QCOM_ADSP_PIL
> config QCOM_RPROC_COMMON
> tristate
>
> +config QCOM_Q6V5_COMMON
> + tristate
> + depends on ARCH_QCOM
> + depends on QCOM_SMEM
> +
> config QCOM_Q6V5_PIL
> tristate "Qualcomm Hexagon V5 Peripherial Image Loader"
> depends on OF && ARCH_QCOM
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 02627ede8d4a..5dd0249cf76a 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
> obj-$(CONFIG_KEYSTONE_REMOTEPROC) += keystone_remoteproc.o
> obj-$(CONFIG_QCOM_ADSP_PIL) += qcom_adsp_pil.o
> obj-$(CONFIG_QCOM_RPROC_COMMON) += qcom_common.o
> +obj-$(CONFIG_QCOM_Q6V5_COMMON) += qcom_q6v5.o
> obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o
> obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
> obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
> new file mode 100644
> index 000000000000..9076537a1671
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_q6v5.c
> @@ -0,0 +1,243 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Qualcomm Peripheral Image Loader for Q6V5 WCSS
> + *
Probably just Q6V5, QCSS not needed.
> + * Copyright (C) 2016-2018 Linaro Ltd.
> + * Copyright (C) 2014 Sony Mobile Communications AB
> + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
> + */
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/soc/qcom/smem.h>
> +#include <linux/soc/qcom/smem_state.h>
> +#include <linux/remoteproc.h>
> +#include "qcom_q6v5.h"
> +
> +/**
> + * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before start
> + * @q6v5: reference to qcom_q6v5 context to be reinitialized
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5)
> +{
> + reinit_completion(&q6v5->start_done);
> + reinit_completion(&q6v5->stop_done);
> +
> + q6v5->running = true;
> + q6v5->handover_issued = false;
> +
> + enable_irq(q6v5->handover_irq);
> +
> + return 0;
> +}
> +
> +/**
> + * qcom_q6v5_unprepare() - unprepare the qcom_q6v5 context after stop
> + * @q6v5: reference to qcom_q6v5 context to be unprepared
> + *
> + * Return: 0 on success, 1 if handover hasn't yet been called
> + */
> +int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5)
> +{
> + disable_irq(q6v5->handover_irq);
> +
> + return !q6v5->handover_issued;
> +}
> +
> +static irqreturn_t q6v5_wdog_interrupt(int irq, void *data)
> +{
> + struct qcom_q6v5 *q6v5 = data;
> + size_t len;
> + char *msg;
> +
> + /* Sometimes the stop triggers a watchdog rather than a stop-ack */
> + if (!q6v5->running) {
> + complete(&q6v5->stop_done);
> + return IRQ_HANDLED;
> + }
> +
Does this change the behavior for adsp pil, which was unconditionally
doing a rproc_report_crash before, but now checks for the running flag
or probably this is the correct sequence ?
> + msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
> + if (!IS_ERR(msg) && len > 0 && msg[0])
> + dev_err(q6v5->dev, "watchdog received: %s\n", msg);
> + else
> + dev_err(q6v5->dev, "watchdog without message\n");
> +
> + rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
> +
Should be rproc_report_crash(q6v5->rproc, RPROC_WATCHDOG);
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
> +{
> + struct qcom_q6v5 *q6v5 = data;
> + size_t len;
> + char *msg;
> +
> + msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
> + if (!IS_ERR(msg) && len > 0 && msg[0])
> + dev_err(q6v5->dev, "fatal error received: %s\n", msg);
> + else
> + dev_err(q6v5->dev, "fatal error without message\n");
> +
> + rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t q6v5_ready_interrupt(int irq, void *data)
> +{
> + struct qcom_q6v5 *q6v5 = data;
> +
> + complete(&q6v5->start_done);
> +
> + return IRQ_HANDLED;
> +}
For adsp, previously start_done completion was done as a part of
handover interrupt, now its done in ready. Does it mean that the
entries in DT should be changed etc ?
> +
> +/**
> + * qcom_q6v5_wait_for_start() - wait for remote processor start signal
> + * @q6v5: reference to qcom_q6v5 context
> + * @timeout: timeout to wait for the event, in jiffies
> + *
> + * qcom_q6v5_unprepare() should not be called when this function fails.
> + *
> + * Return: 0 on success, -ETIMEDOUT on timeout
> + */
> +int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout)
> +{
> + int ret;
> +
> + ret = wait_for_completion_timeout(&q6v5->start_done, timeout);
> + if (!ret)
> + disable_irq(q6v5->handover_irq);
> +
> + return !ret ? -ETIMEDOUT : 0;
> +}
> +
> +static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
> +{
> + struct qcom_q6v5 *q6v5 = data;
> +
> + if (q6v5->handover)
> + q6v5->handover(q6v5);
> +
> + q6v5->handover_issued = true;
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t q6v5_stop_interrupt(int irq, void *data)
> +{
> + struct qcom_q6v5 *q6v5 = data;
> +
> + complete(&q6v5->stop_done);
> +
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * qcom_q6v5_request_stop() - request the remote processor to stop
> + * @q6v5: reference to qcom_q6v5 context
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5)
> +{
> + int ret;
> +
> + q6v5->running = false;
> +
> + qcom_smem_state_update_bits(q6v5->state,
> + BIT(q6v5->stop_bit), BIT(q6v5->stop_bit));
> +
> + ret = wait_for_completion_timeout(&q6v5->stop_done, 5 * HZ);
> +
> + qcom_smem_state_update_bits(q6v5->state, BIT(q6v5->stop_bit), 0);
> +
> + return ret == 0 ? -ETIMEDOUT : 0;
> +}
> +
> +/**
> + * qcom_q6v5_init() - initializer of the q6v5 common struct
> + * @q6v5: handle to be initialized
> + * @pdev: platform_device reference for acquiring resources
> + * @rproc: associated remoteproc instance
> + * @crash_reason: SMEM id for crash reason string, or 0 if none
> + * @handover: function to be called when proxy resources should be released
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
> + struct rproc *rproc, int crash_reason,
> + void (*handover)(struct qcom_q6v5 *q6v5))
> +{
> + int ret;
> +
> + q6v5->rproc = rproc;
> + q6v5->dev = &pdev->dev;
> + q6v5->crash_reason = crash_reason;
> + q6v5->handover = handover;
> +
> + init_completion(&q6v5->start_done);
> + init_completion(&q6v5->stop_done);
> +
> + q6v5->wdog_irq = platform_get_irq_byname(pdev, "wdog");
> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->wdog_irq,
> + NULL, q6v5_wdog_interrupt,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + "q6v5 wdog", q6v5);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to acquire wdog IRQ\n");
> + return ret;
> + }
> +
> + q6v5->fatal_irq = platform_get_irq_byname(pdev, "fatal");
> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->fatal_irq,
> + NULL, q6v5_fatal_interrupt,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + "q6v5 fatal", q6v5);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to acquire fatal IRQ\n");
> + return ret;
> + }
> +
> + q6v5->ready_irq = platform_get_irq_byname(pdev, "ready");
> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->ready_irq,
> + NULL, q6v5_ready_interrupt,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + "q6v5 ready", q6v5);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to acquire ready IRQ\n");
> + return ret;
> + }
> +
> + q6v5->handover_irq = platform_get_irq_byname(pdev, "handover");
> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->handover_irq,
> + NULL, q6v5_handover_interrupt,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + "q6v5 handover", q6v5);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to acquire handover IRQ\n");
> + return ret;
> + }
> + disable_irq(q6v5->handover_irq);
> +
> + q6v5->stop_irq = platform_get_irq_byname(pdev, "stop-ack");
> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->stop_irq,
> + NULL, q6v5_stop_interrupt,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + "q6v5 stop", q6v5);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to acquire stop-ack IRQ\n");
> + return ret;
> + }
> +
> + q6v5->state = qcom_smem_state_get(&pdev->dev, "stop", &q6v5->stop_bit);
> + if (IS_ERR(q6v5->state)) {
> + dev_err(&pdev->dev, "failed to acquire stop state\n");
> + return PTR_ERR(q6v5->state);
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h
> new file mode 100644
> index 000000000000..7ac92c1e0f49
> --- /dev/null
> +++ b/drivers/remoteproc/qcom_q6v5.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __QCOM_Q6V5_H__
> +#define __QCOM_Q6V5_H__
> +
> +#include <linux/kernel.h>
> +#include <linux/completion.h>
> +
> +struct rproc;
> +struct qcom_smem_state;
> +
> +struct qcom_q6v5 {
> + struct device *dev;
> + struct rproc *rproc;
> +
> + struct qcom_smem_state *state;
> + unsigned stop_bit;
> +
> + int wdog_irq;
> + int fatal_irq;
> + int ready_irq;
> + int handover_irq;
> + int stop_irq;
> +
> + bool handover_issued;
> +
> + struct completion start_done;
> + struct completion stop_done;
> +
> + int crash_reason;
> +
> + bool running;
> +
> + void (*handover)(struct qcom_q6v5 *q6v5);
> +};
> +
> +int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
> + struct rproc *rproc, int crash_reason,
> + void (*handover)(struct qcom_q6v5 *q6v5));
> +
> +int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5);
> +int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5);
> +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5);
> +int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout);
> +
> +#endif
>
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling
2018-06-01 6:16 ` Sricharan R
@ 2018-06-01 15:18 ` Sibi S
2018-06-01 16:39 ` Sricharan R
0 siblings, 1 reply; 19+ messages in thread
From: Sibi S @ 2018-06-01 15:18 UTC (permalink / raw)
To: Sricharan R, Bjorn Andersson, Ohad Ben-Cohen, Rohit kumar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Hi Sricharan,
On 06/01/2018 11:46 AM, Sricharan R wrote:
> Hi Bjorn,
> Thanks for this much needed consolidation.
>
> On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
>> Shared between all Hexagon V5 based remoteprocs is the handling of the 5
>> interrupts and the SMP2P stop request, so break this out into a separate
>> function in order to allow these drivers to be cleaned up.
>>
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> ---
>> drivers/remoteproc/Kconfig | 5 +
>> drivers/remoteproc/Makefile | 1 +
>> drivers/remoteproc/qcom_q6v5.c | 243 +++++++++++++++++++++++++++++++++
>> drivers/remoteproc/qcom_q6v5.h | 46 +++++++
>> 4 files changed, 295 insertions(+)
>> create mode 100644 drivers/remoteproc/qcom_q6v5.c
>> create mode 100644 drivers/remoteproc/qcom_q6v5.h
>>
>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>> index cd1c168fd188..63b79ea91a21 100644
>> --- a/drivers/remoteproc/Kconfig
>> +++ b/drivers/remoteproc/Kconfig
>> @@ -102,6 +102,11 @@ config QCOM_ADSP_PIL
>> config QCOM_RPROC_COMMON
>> tristate
>>
>> +config QCOM_Q6V5_COMMON
>> + tristate
>> + depends on ARCH_QCOM
>> + depends on QCOM_SMEM
>> +
>> config QCOM_Q6V5_PIL
>> tristate "Qualcomm Hexagon V5 Peripherial Image Loader"
>> depends on OF && ARCH_QCOM
>> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
>> index 02627ede8d4a..5dd0249cf76a 100644
>> --- a/drivers/remoteproc/Makefile
>> +++ b/drivers/remoteproc/Makefile
>> @@ -16,6 +16,7 @@ obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
>> obj-$(CONFIG_KEYSTONE_REMOTEPROC) += keystone_remoteproc.o
>> obj-$(CONFIG_QCOM_ADSP_PIL) += qcom_adsp_pil.o
>> obj-$(CONFIG_QCOM_RPROC_COMMON) += qcom_common.o
>> +obj-$(CONFIG_QCOM_Q6V5_COMMON) += qcom_q6v5.o
>> obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o
>> obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
>> obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
>> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
>> new file mode 100644
>> index 000000000000..9076537a1671
>> --- /dev/null
>> +++ b/drivers/remoteproc/qcom_q6v5.c
>> @@ -0,0 +1,243 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Qualcomm Peripheral Image Loader for Q6V5 WCSS
>> + *
>
> Probably just Q6V5, QCSS not needed.
>
>> + * Copyright (C) 2016-2018 Linaro Ltd.
>> + * Copyright (C) 2014 Sony Mobile Communications AB
>> + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
>> + */
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/soc/qcom/smem.h>
>> +#include <linux/soc/qcom/smem_state.h>
>> +#include <linux/remoteproc.h>
>> +#include "qcom_q6v5.h"
>> +
>> +/**
>> + * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before start
>> + * @q6v5: reference to qcom_q6v5 context to be reinitialized
>> + *
>> + * Return: 0 on success, negative errno on failure
>> + */
>> +int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5)
>> +{
>> + reinit_completion(&q6v5->start_done);
>> + reinit_completion(&q6v5->stop_done);
>> +
>> + q6v5->running = true;
>> + q6v5->handover_issued = false;
>> +
>> + enable_irq(q6v5->handover_irq);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * qcom_q6v5_unprepare() - unprepare the qcom_q6v5 context after stop
>> + * @q6v5: reference to qcom_q6v5 context to be unprepared
>> + *
>> + * Return: 0 on success, 1 if handover hasn't yet been called
>> + */
>> +int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5)
>> +{
>> + disable_irq(q6v5->handover_irq);
>> +
>> + return !q6v5->handover_issued;
>> +}
>> +
>> +static irqreturn_t q6v5_wdog_interrupt(int irq, void *data)
>> +{
>> + struct qcom_q6v5 *q6v5 = data;
>> + size_t len;
>> + char *msg;
>> +
>> + /* Sometimes the stop triggers a watchdog rather than a stop-ack */
>> + if (!q6v5->running) {
>> + complete(&q6v5->stop_done);
>> + return IRQ_HANDLED;
>> + }
>> +
>
> Does this change the behavior for adsp pil, which was unconditionally
> doing a rproc_report_crash before, but now checks for the running flag
> or probably this is the correct sequence ?
>
>> + msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
>> + if (!IS_ERR(msg) && len > 0 && msg[0])
>> + dev_err(q6v5->dev, "watchdog received: %s\n", msg);
>> + else
>> + dev_err(q6v5->dev, "watchdog without message\n");
>> +
>> + rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
>> +
>
> Should be rproc_report_crash(q6v5->rproc, RPROC_WATCHDOG);
>
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
>> +{
>> + struct qcom_q6v5 *q6v5 = data;
>> + size_t len;
>> + char *msg;
>> +
>> + msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
>> + if (!IS_ERR(msg) && len > 0 && msg[0])
>> + dev_err(q6v5->dev, "fatal error received: %s\n", msg);
>> + else
>> + dev_err(q6v5->dev, "fatal error without message\n");
>> +
>> + rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t q6v5_ready_interrupt(int irq, void *data)
>> +{
>> + struct qcom_q6v5 *q6v5 = data;
>> +
>> + complete(&q6v5->start_done);
>> +
>> + return IRQ_HANDLED;
>> +}
>
> For adsp, previously start_done completion was done as a part of
> handover interrupt, now its done in ready. Does it mean that the
> entries in DT should be changed etc ?
ready interrupt has always been a part dt entry however a corresponding
interrupt handler was never registered. The trigger condition for
the handover interrupt in the remote processor seem to vary across SoCs
but we can always rely on the ready interrupt to declare the rproc
device is running.
>> +
>> +/**
>> + * qcom_q6v5_wait_for_start() - wait for remote processor start signal
>> + * @q6v5: reference to qcom_q6v5 context
>> + * @timeout: timeout to wait for the event, in jiffies
>> + *
>> + * qcom_q6v5_unprepare() should not be called when this function fails.
>> + *
>> + * Return: 0 on success, -ETIMEDOUT on timeout
>> + */
>> +int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout)
>> +{
>> + int ret;
>> +
>> + ret = wait_for_completion_timeout(&q6v5->start_done, timeout);
>> + if (!ret)
>> + disable_irq(q6v5->handover_irq);
>> +
>> + return !ret ? -ETIMEDOUT : 0;
>> +}
>> +
>> +static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
>> +{
>> + struct qcom_q6v5 *q6v5 = data;
>> +
>> + if (q6v5->handover)
>> + q6v5->handover(q6v5);
>> +
>> + q6v5->handover_issued = true;
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t q6v5_stop_interrupt(int irq, void *data)
>> +{
>> + struct qcom_q6v5 *q6v5 = data;
>> +
>> + complete(&q6v5->stop_done);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +/**
>> + * qcom_q6v5_request_stop() - request the remote processor to stop
>> + * @q6v5: reference to qcom_q6v5 context
>> + *
>> + * Return: 0 on success, negative errno on failure
>> + */
>> +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5)
>> +{
>> + int ret;
>> +
>> + q6v5->running = false;
>> +
>> + qcom_smem_state_update_bits(q6v5->state,
>> + BIT(q6v5->stop_bit), BIT(q6v5->stop_bit));
>> +
>> + ret = wait_for_completion_timeout(&q6v5->stop_done, 5 * HZ);
>> +
>> + qcom_smem_state_update_bits(q6v5->state, BIT(q6v5->stop_bit), 0);
>> +
>> + return ret == 0 ? -ETIMEDOUT : 0;
>> +}
>> +
>> +/**
>> + * qcom_q6v5_init() - initializer of the q6v5 common struct
>> + * @q6v5: handle to be initialized
>> + * @pdev: platform_device reference for acquiring resources
>> + * @rproc: associated remoteproc instance
>> + * @crash_reason: SMEM id for crash reason string, or 0 if none
>> + * @handover: function to be called when proxy resources should be released
>> + *
>> + * Return: 0 on success, negative errno on failure
>> + */
>> +int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
>> + struct rproc *rproc, int crash_reason,
>> + void (*handover)(struct qcom_q6v5 *q6v5))
>> +{
>> + int ret;
>> +
>> + q6v5->rproc = rproc;
>> + q6v5->dev = &pdev->dev;
>> + q6v5->crash_reason = crash_reason;
>> + q6v5->handover = handover;
>> +
>> + init_completion(&q6v5->start_done);
>> + init_completion(&q6v5->stop_done);
>> +
>> + q6v5->wdog_irq = platform_get_irq_byname(pdev, "wdog");
>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->wdog_irq,
>> + NULL, q6v5_wdog_interrupt,
>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>> + "q6v5 wdog", q6v5);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to acquire wdog IRQ\n");
>> + return ret;
>> + }
>> +
>> + q6v5->fatal_irq = platform_get_irq_byname(pdev, "fatal");
>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->fatal_irq,
>> + NULL, q6v5_fatal_interrupt,
>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>> + "q6v5 fatal", q6v5);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to acquire fatal IRQ\n");
>> + return ret;
>> + }
>> +
>> + q6v5->ready_irq = platform_get_irq_byname(pdev, "ready");
>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->ready_irq,
>> + NULL, q6v5_ready_interrupt,
>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>> + "q6v5 ready", q6v5);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to acquire ready IRQ\n");
>> + return ret;
>> + }
>> +
>> + q6v5->handover_irq = platform_get_irq_byname(pdev, "handover");
>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->handover_irq,
>> + NULL, q6v5_handover_interrupt,
>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>> + "q6v5 handover", q6v5);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to acquire handover IRQ\n");
>> + return ret;
>> + }
>> + disable_irq(q6v5->handover_irq);
>> +
>> + q6v5->stop_irq = platform_get_irq_byname(pdev, "stop-ack");
>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->stop_irq,
>> + NULL, q6v5_stop_interrupt,
>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>> + "q6v5 stop", q6v5);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to acquire stop-ack IRQ\n");
>> + return ret;
>> + }
>> +
>> + q6v5->state = qcom_smem_state_get(&pdev->dev, "stop", &q6v5->stop_bit);
>> + if (IS_ERR(q6v5->state)) {
>> + dev_err(&pdev->dev, "failed to acquire stop state\n");
>> + return PTR_ERR(q6v5->state);
>> + }
>> +
>> + return 0;
>> +}
>> diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h
>> new file mode 100644
>> index 000000000000..7ac92c1e0f49
>> --- /dev/null
>> +++ b/drivers/remoteproc/qcom_q6v5.h
>> @@ -0,0 +1,46 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef __QCOM_Q6V5_H__
>> +#define __QCOM_Q6V5_H__
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/completion.h>
>> +
>> +struct rproc;
>> +struct qcom_smem_state;
>> +
>> +struct qcom_q6v5 {
>> + struct device *dev;
>> + struct rproc *rproc;
>> +
>> + struct qcom_smem_state *state;
>> + unsigned stop_bit;
>> +
>> + int wdog_irq;
>> + int fatal_irq;
>> + int ready_irq;
>> + int handover_irq;
>> + int stop_irq;
>> +
>> + bool handover_issued;
>> +
>> + struct completion start_done;
>> + struct completion stop_done;
>> +
>> + int crash_reason;
>> +
>> + bool running;
>> +
>> + void (*handover)(struct qcom_q6v5 *q6v5);
>> +};
>> +
>> +int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
>> + struct rproc *rproc, int crash_reason,
>> + void (*handover)(struct qcom_q6v5 *q6v5));
>> +
>> +int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5);
>> +int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5);
>> +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5);
>> +int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout);
>> +
>> +#endif
>>
>
> Regards,
> Sricharan
>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling
2018-06-01 15:18 ` Sibi S
@ 2018-06-01 16:39 ` Sricharan R
0 siblings, 0 replies; 19+ messages in thread
From: Sricharan R @ 2018-06-01 16:39 UTC (permalink / raw)
To: Sibi S, Bjorn Andersson, Ohad Ben-Cohen, Rohit kumar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Hi Sibi,
On 6/1/2018 8:48 PM, Sibi S wrote:
> Hi Sricharan,
>
> On 06/01/2018 11:46 AM, Sricharan R wrote:
>> Hi Bjorn,
>> Thanks for this much needed consolidation.
>>
>> On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
>>> Shared between all Hexagon V5 based remoteprocs is the handling of the 5
>>> interrupts and the SMP2P stop request, so break this out into a separate
>>> function in order to allow these drivers to be cleaned up.
>>>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>> ---
>>> drivers/remoteproc/Kconfig | 5 +
>>> drivers/remoteproc/Makefile | 1 +
>>> drivers/remoteproc/qcom_q6v5.c | 243 +++++++++++++++++++++++++++++++++
>>> drivers/remoteproc/qcom_q6v5.h | 46 +++++++
>>> 4 files changed, 295 insertions(+)
>>> create mode 100644 drivers/remoteproc/qcom_q6v5.c
>>> create mode 100644 drivers/remoteproc/qcom_q6v5.h
>>>
>>> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
>>> index cd1c168fd188..63b79ea91a21 100644
>>> --- a/drivers/remoteproc/Kconfig
>>> +++ b/drivers/remoteproc/Kconfig
>>> @@ -102,6 +102,11 @@ config QCOM_ADSP_PIL
>>> config QCOM_RPROC_COMMON
>>> tristate
>>> +config QCOM_Q6V5_COMMON
>>> + tristate
>>> + depends on ARCH_QCOM
>>> + depends on QCOM_SMEM
>>> +
>>> config QCOM_Q6V5_PIL
>>> tristate "Qualcomm Hexagon V5 Peripherial Image Loader"
>>> depends on OF && ARCH_QCOM
>>> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
>>> index 02627ede8d4a..5dd0249cf76a 100644
>>> --- a/drivers/remoteproc/Makefile
>>> +++ b/drivers/remoteproc/Makefile
>>> @@ -16,6 +16,7 @@ obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
>>> obj-$(CONFIG_KEYSTONE_REMOTEPROC) += keystone_remoteproc.o
>>> obj-$(CONFIG_QCOM_ADSP_PIL) += qcom_adsp_pil.o
>>> obj-$(CONFIG_QCOM_RPROC_COMMON) += qcom_common.o
>>> +obj-$(CONFIG_QCOM_Q6V5_COMMON) += qcom_q6v5.o
>>> obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o
>>> obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
>>> obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
>>> diff --git a/drivers/remoteproc/qcom_q6v5.c b/drivers/remoteproc/qcom_q6v5.c
>>> new file mode 100644
>>> index 000000000000..9076537a1671
>>> --- /dev/null
>>> +++ b/drivers/remoteproc/qcom_q6v5.c
>>> @@ -0,0 +1,243 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Qualcomm Peripheral Image Loader for Q6V5 WCSS
>>> + *
>>
>> Probably just Q6V5, QCSS not needed.
>>
>>> + * Copyright (C) 2016-2018 Linaro Ltd.
>>> + * Copyright (C) 2014 Sony Mobile Communications AB
>>> + * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
>>> + */
>>> +#include <linux/kernel.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/soc/qcom/smem.h>
>>> +#include <linux/soc/qcom/smem_state.h>
>>> +#include <linux/remoteproc.h>
>>> +#include "qcom_q6v5.h"
>>> +
>>> +/**
>>> + * qcom_q6v5_prepare() - reinitialize the qcom_q6v5 context before start
>>> + * @q6v5: reference to qcom_q6v5 context to be reinitialized
>>> + *
>>> + * Return: 0 on success, negative errno on failure
>>> + */
>>> +int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5)
>>> +{
>>> + reinit_completion(&q6v5->start_done);
>>> + reinit_completion(&q6v5->stop_done);
>>> +
>>> + q6v5->running = true;
>>> + q6v5->handover_issued = false;
>>> +
>>> + enable_irq(q6v5->handover_irq);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * qcom_q6v5_unprepare() - unprepare the qcom_q6v5 context after stop
>>> + * @q6v5: reference to qcom_q6v5 context to be unprepared
>>> + *
>>> + * Return: 0 on success, 1 if handover hasn't yet been called
>>> + */
>>> +int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5)
>>> +{
>>> + disable_irq(q6v5->handover_irq);
>>> +
>>> + return !q6v5->handover_issued;
>>> +}
>>> +
>>> +static irqreturn_t q6v5_wdog_interrupt(int irq, void *data)
>>> +{
>>> + struct qcom_q6v5 *q6v5 = data;
>>> + size_t len;
>>> + char *msg;
>>> +
>>> + /* Sometimes the stop triggers a watchdog rather than a stop-ack */
>>> + if (!q6v5->running) {
>>> + complete(&q6v5->stop_done);
>>> + return IRQ_HANDLED;
>>> + }
>>> +
>>
>> Does this change the behavior for adsp pil, which was unconditionally
>> doing a rproc_report_crash before, but now checks for the running flag
>> or probably this is the correct sequence ?
>>
>>> + msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
>>> + if (!IS_ERR(msg) && len > 0 && msg[0])
>>> + dev_err(q6v5->dev, "watchdog received: %s\n", msg);
>>> + else
>>> + dev_err(q6v5->dev, "watchdog without message\n");
>>> +
>>> + rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
>>> +
>>
>> Should be rproc_report_crash(q6v5->rproc, RPROC_WATCHDOG);
>>
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static irqreturn_t q6v5_fatal_interrupt(int irq, void *data)
>>> +{
>>> + struct qcom_q6v5 *q6v5 = data;
>>> + size_t len;
>>> + char *msg;
>>> +
>>> + msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, q6v5->crash_reason, &len);
>>> + if (!IS_ERR(msg) && len > 0 && msg[0])
>>> + dev_err(q6v5->dev, "fatal error received: %s\n", msg);
>>> + else
>>> + dev_err(q6v5->dev, "fatal error without message\n");
>>> +
>>> + rproc_report_crash(q6v5->rproc, RPROC_FATAL_ERROR);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static irqreturn_t q6v5_ready_interrupt(int irq, void *data)
>>> +{
>>> + struct qcom_q6v5 *q6v5 = data;
>>> +
>>> + complete(&q6v5->start_done);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>
>> For adsp, previously start_done completion was done as a part of
>> handover interrupt, now its done in ready. Does it mean that the
>> entries in DT should be changed etc ?
>
> ready interrupt has always been a part dt entry however a corresponding
> interrupt handler was never registered. The trigger condition for
> the handover interrupt in the remote processor seem to vary across SoCs
> but we can always rely on the ready interrupt to declare the rproc
> device is running.
>
All right. My doubt was more because start_done completion being
moved to ready. I guess Rohit has already verified the functionality
with the patch. So should be fine.
Regards,
Sricharan
>>> +
>>> +/**
>>> + * qcom_q6v5_wait_for_start() - wait for remote processor start signal
>>> + * @q6v5: reference to qcom_q6v5 context
>>> + * @timeout: timeout to wait for the event, in jiffies
>>> + *
>>> + * qcom_q6v5_unprepare() should not be called when this function fails.
>>> + *
>>> + * Return: 0 on success, -ETIMEDOUT on timeout
>>> + */
>>> +int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout)
>>> +{
>>> + int ret;
>>> +
>>> + ret = wait_for_completion_timeout(&q6v5->start_done, timeout);
>>> + if (!ret)
>>> + disable_irq(q6v5->handover_irq);
>>> +
>>> + return !ret ? -ETIMEDOUT : 0;
>>> +}
>>> +
>>> +static irqreturn_t q6v5_handover_interrupt(int irq, void *data)
>>> +{
>>> + struct qcom_q6v5 *q6v5 = data;
>>> +
>>> + if (q6v5->handover)
>>> + q6v5->handover(q6v5);
>>> +
>>> + q6v5->handover_issued = true;
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static irqreturn_t q6v5_stop_interrupt(int irq, void *data)
>>> +{
>>> + struct qcom_q6v5 *q6v5 = data;
>>> +
>>> + complete(&q6v5->stop_done);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +/**
>>> + * qcom_q6v5_request_stop() - request the remote processor to stop
>>> + * @q6v5: reference to qcom_q6v5 context
>>> + *
>>> + * Return: 0 on success, negative errno on failure
>>> + */
>>> +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5)
>>> +{
>>> + int ret;
>>> +
>>> + q6v5->running = false;
>>> +
>>> + qcom_smem_state_update_bits(q6v5->state,
>>> + BIT(q6v5->stop_bit), BIT(q6v5->stop_bit));
>>> +
>>> + ret = wait_for_completion_timeout(&q6v5->stop_done, 5 * HZ);
>>> +
>>> + qcom_smem_state_update_bits(q6v5->state, BIT(q6v5->stop_bit), 0);
>>> +
>>> + return ret == 0 ? -ETIMEDOUT : 0;
>>> +}
>>> +
>>> +/**
>>> + * qcom_q6v5_init() - initializer of the q6v5 common struct
>>> + * @q6v5: handle to be initialized
>>> + * @pdev: platform_device reference for acquiring resources
>>> + * @rproc: associated remoteproc instance
>>> + * @crash_reason: SMEM id for crash reason string, or 0 if none
>>> + * @handover: function to be called when proxy resources should be released
>>> + *
>>> + * Return: 0 on success, negative errno on failure
>>> + */
>>> +int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
>>> + struct rproc *rproc, int crash_reason,
>>> + void (*handover)(struct qcom_q6v5 *q6v5))
>>> +{
>>> + int ret;
>>> +
>>> + q6v5->rproc = rproc;
>>> + q6v5->dev = &pdev->dev;
>>> + q6v5->crash_reason = crash_reason;
>>> + q6v5->handover = handover;
>>> +
>>> + init_completion(&q6v5->start_done);
>>> + init_completion(&q6v5->stop_done);
>>> +
>>> + q6v5->wdog_irq = platform_get_irq_byname(pdev, "wdog");
>>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->wdog_irq,
>>> + NULL, q6v5_wdog_interrupt,
>>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>>> + "q6v5 wdog", q6v5);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to acquire wdog IRQ\n");
>>> + return ret;
>>> + }
>>> +
>>> + q6v5->fatal_irq = platform_get_irq_byname(pdev, "fatal");
>>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->fatal_irq,
>>> + NULL, q6v5_fatal_interrupt,
>>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>>> + "q6v5 fatal", q6v5);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to acquire fatal IRQ\n");
>>> + return ret;
>>> + }
>>> +
>>> + q6v5->ready_irq = platform_get_irq_byname(pdev, "ready");
>>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->ready_irq,
>>> + NULL, q6v5_ready_interrupt,
>>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>>> + "q6v5 ready", q6v5);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to acquire ready IRQ\n");
>>> + return ret;
>>> + }
>>> +
>>> + q6v5->handover_irq = platform_get_irq_byname(pdev, "handover");
>>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->handover_irq,
>>> + NULL, q6v5_handover_interrupt,
>>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>>> + "q6v5 handover", q6v5);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to acquire handover IRQ\n");
>>> + return ret;
>>> + }
>>> + disable_irq(q6v5->handover_irq);
>>> +
>>> + q6v5->stop_irq = platform_get_irq_byname(pdev, "stop-ack");
>>> + ret = devm_request_threaded_irq(&pdev->dev, q6v5->stop_irq,
>>> + NULL, q6v5_stop_interrupt,
>>> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>>> + "q6v5 stop", q6v5);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to acquire stop-ack IRQ\n");
>>> + return ret;
>>> + }
>>> +
>>> + q6v5->state = qcom_smem_state_get(&pdev->dev, "stop", &q6v5->stop_bit);
>>> + if (IS_ERR(q6v5->state)) {
>>> + dev_err(&pdev->dev, "failed to acquire stop state\n");
>>> + return PTR_ERR(q6v5->state);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> diff --git a/drivers/remoteproc/qcom_q6v5.h b/drivers/remoteproc/qcom_q6v5.h
>>> new file mode 100644
>>> index 000000000000..7ac92c1e0f49
>>> --- /dev/null
>>> +++ b/drivers/remoteproc/qcom_q6v5.h
>>> @@ -0,0 +1,46 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +
>>> +#ifndef __QCOM_Q6V5_H__
>>> +#define __QCOM_Q6V5_H__
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/completion.h>
>>> +
>>> +struct rproc;
>>> +struct qcom_smem_state;
>>> +
>>> +struct qcom_q6v5 {
>>> + struct device *dev;
>>> + struct rproc *rproc;
>>> +
>>> + struct qcom_smem_state *state;
>>> + unsigned stop_bit;
>>> +
>>> + int wdog_irq;
>>> + int fatal_irq;
>>> + int ready_irq;
>>> + int handover_irq;
>>> + int stop_irq;
>>> +
>>> + bool handover_issued;
>>> +
>>> + struct completion start_done;
>>> + struct completion stop_done;
>>> +
>>> + int crash_reason;
>>> +
>>> + bool running;
>>> +
>>> + void (*handover)(struct qcom_q6v5 *q6v5);
>>> +};
>>> +
>>> +int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev,
>>> + struct rproc *rproc, int crash_reason,
>>> + void (*handover)(struct qcom_q6v5 *q6v5));
>>> +
>>> +int qcom_q6v5_prepare(struct qcom_q6v5 *q6v5);
>>> +int qcom_q6v5_unprepare(struct qcom_q6v5 *q6v5);
>>> +int qcom_q6v5_request_stop(struct qcom_q6v5 *q6v5);
>>> +int qcom_q6v5_wait_for_start(struct qcom_q6v5 *q6v5, int timeout);
>>> +
>>> +#endif
>>>
>>
>> Regards,
>> Sricharan
>>
>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 3/5] remoteproc: qcom: adsp: Use common q6v5 helpers
2018-05-23 5:20 [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Bjorn Andersson
2018-05-23 5:20 ` [RFC PATCH 1/5] remoteproc: qcom: mdt_loader: Make the firmware authentication optional Bjorn Andersson
2018-05-23 5:20 ` [RFC PATCH 2/5] remoteproc: q6v5: Extract common resource handling Bjorn Andersson
@ 2018-05-23 5:20 ` Bjorn Andersson
2018-06-01 6:25 ` Sricharan R
2018-06-01 13:35 ` Rohit Kumar
2018-05-23 5:20 ` [RFC PATCH 4/5] remoteproc: qcom: q6v5-pil: " Bjorn Andersson
` (2 subsequent siblings)
5 siblings, 2 replies; 19+ messages in thread
From: Bjorn Andersson @ 2018-05-23 5:20 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson, Sricharan R, Sibi Sankar, Rohit kumar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Migrate the Hexagon V5 PAS (ADSP) driver to using the newly extracted
helper functions. The use of the handover callback does introduce latent
disabling of proxy resources. But apart from this there should be no
change in functionality.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/remoteproc/Kconfig | 1 +
drivers/remoteproc/qcom_adsp_pil.c | 156 +++++------------------------
2 files changed, 28 insertions(+), 129 deletions(-)
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 63b79ea91a21..d51d155cf8bd 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -93,6 +93,7 @@ config QCOM_ADSP_PIL
depends on QCOM_SYSMON || QCOM_SYSMON=n
select MFD_SYSCON
select QCOM_MDT_LOADER
+ select QCOM_Q6V5_COMMON
select QCOM_RPROC_COMMON
select QCOM_SCM
help
diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
index 89a86ce07f99..d4339a6da616 100644
--- a/drivers/remoteproc/qcom_adsp_pil.c
+++ b/drivers/remoteproc/qcom_adsp_pil.c
@@ -31,6 +31,7 @@
#include <linux/soc/qcom/smem_state.h>
#include "qcom_common.h"
+#include "qcom_q6v5.h"
#include "remoteproc_internal.h"
struct adsp_data {
@@ -48,14 +49,7 @@ struct qcom_adsp {
struct device *dev;
struct rproc *rproc;
- int wdog_irq;
- int fatal_irq;
- int ready_irq;
- int handover_irq;
- int stop_ack_irq;
-
- struct qcom_smem_state *state;
- unsigned stop_bit;
+ struct qcom_q6v5 q6v5;
struct clk *xo;
struct clk *aggre2_clk;
@@ -96,6 +90,8 @@ static int adsp_start(struct rproc *rproc)
struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
int ret;
+ qcom_q6v5_prepare(&adsp->q6v5);
+
ret = clk_prepare_enable(adsp->xo);
if (ret)
return ret;
@@ -119,16 +115,14 @@ static int adsp_start(struct rproc *rproc)
goto disable_px_supply;
}
- ret = wait_for_completion_timeout(&adsp->start_done,
- msecs_to_jiffies(5000));
- if (!ret) {
+ ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000));
+ if (ret == -ETIMEDOUT) {
dev_err(adsp->dev, "start timed out\n");
qcom_scm_pas_shutdown(adsp->pas_id);
- ret = -ETIMEDOUT;
goto disable_px_supply;
}
- ret = 0;
+ return 0;
disable_px_supply:
regulator_disable(adsp->px_supply);
@@ -142,28 +136,34 @@ static int adsp_start(struct rproc *rproc)
return ret;
}
+static void qcom_pas_handover(struct qcom_q6v5 *q6v5)
+{
+ struct qcom_adsp *adsp = container_of(q6v5, struct qcom_adsp, q6v5);
+
+ regulator_disable(adsp->px_supply);
+ regulator_disable(adsp->cx_supply);
+ clk_disable_unprepare(adsp->aggre2_clk);
+ clk_disable_unprepare(adsp->xo);
+}
+
static int adsp_stop(struct rproc *rproc)
{
struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
+ int handover;
int ret;
- qcom_smem_state_update_bits(adsp->state,
- BIT(adsp->stop_bit),
- BIT(adsp->stop_bit));
-
- ret = wait_for_completion_timeout(&adsp->stop_done,
- msecs_to_jiffies(5000));
- if (ret == 0)
+ ret = qcom_q6v5_request_stop(&adsp->q6v5);
+ if (ret == -ETIMEDOUT)
dev_err(adsp->dev, "timed out on wait\n");
- qcom_smem_state_update_bits(adsp->state,
- BIT(adsp->stop_bit),
- 0);
-
ret = qcom_scm_pas_shutdown(adsp->pas_id);
if (ret)
dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
+ handover = qcom_q6v5_unprepare(&adsp->q6v5);
+ if (handover)
+ qcom_pas_handover(&adsp->q6v5);
+
return ret;
}
@@ -187,53 +187,6 @@ static const struct rproc_ops adsp_ops = {
.load = adsp_load,
};
-static irqreturn_t adsp_wdog_interrupt(int irq, void *dev)
-{
- struct qcom_adsp *adsp = dev;
-
- rproc_report_crash(adsp->rproc, RPROC_WATCHDOG);
-
- return IRQ_HANDLED;
-}
-
-static irqreturn_t adsp_fatal_interrupt(int irq, void *dev)
-{
- struct qcom_adsp *adsp = dev;
- size_t len;
- char *msg;
-
- msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, adsp->crash_reason_smem, &len);
- if (!IS_ERR(msg) && len > 0 && msg[0])
- dev_err(adsp->dev, "fatal error received: %s\n", msg);
-
- rproc_report_crash(adsp->rproc, RPROC_FATAL_ERROR);
-
- return IRQ_HANDLED;
-}
-
-static irqreturn_t adsp_ready_interrupt(int irq, void *dev)
-{
- return IRQ_HANDLED;
-}
-
-static irqreturn_t adsp_handover_interrupt(int irq, void *dev)
-{
- struct qcom_adsp *adsp = dev;
-
- complete(&adsp->start_done);
-
- return IRQ_HANDLED;
-}
-
-static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
-{
- struct qcom_adsp *adsp = dev;
-
- complete(&adsp->stop_done);
-
- return IRQ_HANDLED;
-}
-
static int adsp_init_clock(struct qcom_adsp *adsp)
{
int ret;
@@ -272,29 +225,6 @@ static int adsp_init_regulator(struct qcom_adsp *adsp)
return PTR_ERR_OR_ZERO(adsp->px_supply);
}
-static int adsp_request_irq(struct qcom_adsp *adsp,
- struct platform_device *pdev,
- const char *name,
- irq_handler_t thread_fn)
-{
- int ret;
-
- ret = platform_get_irq_byname(pdev, name);
- if (ret < 0) {
- dev_err(&pdev->dev, "no %s IRQ defined\n", name);
- return ret;
- }
-
- ret = devm_request_threaded_irq(&pdev->dev, ret,
- NULL, thread_fn,
- IRQF_ONESHOT,
- "adsp", adsp);
- if (ret)
- dev_err(&pdev->dev, "request %s IRQ failed\n", name);
-
- return ret;
-}
-
static int adsp_alloc_memory_region(struct qcom_adsp *adsp)
{
struct device_node *node;
@@ -348,13 +278,9 @@ static int adsp_probe(struct platform_device *pdev)
adsp->dev = &pdev->dev;
adsp->rproc = rproc;
adsp->pas_id = desc->pas_id;
- adsp->crash_reason_smem = desc->crash_reason_smem;
adsp->has_aggre2_clk = desc->has_aggre2_clk;
platform_set_drvdata(pdev, adsp);
- init_completion(&adsp->start_done);
- init_completion(&adsp->stop_done);
-
ret = adsp_alloc_memory_region(adsp);
if (ret)
goto free_rproc;
@@ -367,37 +293,10 @@ static int adsp_probe(struct platform_device *pdev)
if (ret)
goto free_rproc;
- ret = adsp_request_irq(adsp, pdev, "wdog", adsp_wdog_interrupt);
- if (ret < 0)
- goto free_rproc;
- adsp->wdog_irq = ret;
-
- ret = adsp_request_irq(adsp, pdev, "fatal", adsp_fatal_interrupt);
- if (ret < 0)
- goto free_rproc;
- adsp->fatal_irq = ret;
-
- ret = adsp_request_irq(adsp, pdev, "ready", adsp_ready_interrupt);
- if (ret < 0)
- goto free_rproc;
- adsp->ready_irq = ret;
-
- ret = adsp_request_irq(adsp, pdev, "handover", adsp_handover_interrupt);
- if (ret < 0)
- goto free_rproc;
- adsp->handover_irq = ret;
-
- ret = adsp_request_irq(adsp, pdev, "stop-ack", adsp_stop_ack_interrupt);
- if (ret < 0)
- goto free_rproc;
- adsp->stop_ack_irq = ret;
-
- adsp->state = qcom_smem_state_get(&pdev->dev, "stop",
- &adsp->stop_bit);
- if (IS_ERR(adsp->state)) {
- ret = PTR_ERR(adsp->state);
+ ret = qcom_q6v5_init(&adsp->q6v5, pdev, rproc, desc->crash_reason_smem,
+ qcom_pas_handover);
+ if (ret)
goto free_rproc;
- }
qcom_add_glink_subdev(rproc, &adsp->glink_subdev);
qcom_add_smd_subdev(rproc, &adsp->smd_subdev);
@@ -422,7 +321,6 @@ static int adsp_remove(struct platform_device *pdev)
{
struct qcom_adsp *adsp = platform_get_drvdata(pdev);
- qcom_smem_state_put(adsp->state);
rproc_del(adsp->rproc);
qcom_remove_glink_subdev(adsp->rproc, &adsp->glink_subdev);
--
2.17.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCH 3/5] remoteproc: qcom: adsp: Use common q6v5 helpers
2018-05-23 5:20 ` [RFC PATCH 3/5] remoteproc: qcom: adsp: Use common q6v5 helpers Bjorn Andersson
@ 2018-06-01 6:25 ` Sricharan R
2018-06-01 13:35 ` Rohit Kumar
1 sibling, 0 replies; 19+ messages in thread
From: Sricharan R @ 2018-06-01 6:25 UTC (permalink / raw)
To: Bjorn Andersson, Ohad Ben-Cohen, Sibi Sankar, Rohit kumar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Hi Bjorn,
On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
> Migrate the Hexagon V5 PAS (ADSP) driver to using the newly extracted
> helper functions. The use of the handover callback does introduce latent
> disabling of proxy resources. But apart from this there should be no
> change in functionality.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> drivers/remoteproc/Kconfig | 1 +
> drivers/remoteproc/qcom_adsp_pil.c | 156 +++++------------------------
> 2 files changed, 28 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 63b79ea91a21..d51d155cf8bd 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -93,6 +93,7 @@ config QCOM_ADSP_PIL
> depends on QCOM_SYSMON || QCOM_SYSMON=n
> select MFD_SYSCON
> select QCOM_MDT_LOADER
> + select QCOM_Q6V5_COMMON
> select QCOM_RPROC_COMMON
> select QCOM_SCM
> help
> diff --git a/drivers/remoteproc/qcom_adsp_pil.c b/drivers/remoteproc/qcom_adsp_pil.c
> index 89a86ce07f99..d4339a6da616 100644
> --- a/drivers/remoteproc/qcom_adsp_pil.c
> +++ b/drivers/remoteproc/qcom_adsp_pil.c
> @@ -31,6 +31,7 @@
> #include <linux/soc/qcom/smem_state.h>
>
> #include "qcom_common.h"
> +#include "qcom_q6v5.h"
> #include "remoteproc_internal.h"
>
> struct adsp_data {
> @@ -48,14 +49,7 @@ struct qcom_adsp {
> struct device *dev;
> struct rproc *rproc;
>
> - int wdog_irq;
> - int fatal_irq;
> - int ready_irq;
> - int handover_irq;
> - int stop_ack_irq;
> -
> - struct qcom_smem_state *state;
> - unsigned stop_bit;
> + struct qcom_q6v5 q6v5;
>
> struct clk *xo;
> struct clk *aggre2_clk;
> @@ -96,6 +90,8 @@ static int adsp_start(struct rproc *rproc)
> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> int ret;
>
> + qcom_q6v5_prepare(&adsp->q6v5);
> +
> ret = clk_prepare_enable(adsp->xo);
> if (ret)
> return ret;
> @@ -119,16 +115,14 @@ static int adsp_start(struct rproc *rproc)
> goto disable_px_supply;
> }
>
> - ret = wait_for_completion_timeout(&adsp->start_done,
> - msecs_to_jiffies(5000));
> - if (!ret) {
> + ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000));
> + if (ret == -ETIMEDOUT) {
> dev_err(adsp->dev, "start timed out\n");
> qcom_scm_pas_shutdown(adsp->pas_id);
> - ret = -ETIMEDOUT;
> goto disable_px_supply;
> }
>
> - ret = 0;
> + return 0;
>
> disable_px_supply:
> regulator_disable(adsp->px_supply);
> @@ -142,28 +136,34 @@ static int adsp_start(struct rproc *rproc)
> return ret;
> }
>
> +static void qcom_pas_handover(struct qcom_q6v5 *q6v5)
> +{
> + struct qcom_adsp *adsp = container_of(q6v5, struct qcom_adsp, q6v5);
> +
> + regulator_disable(adsp->px_supply);
> + regulator_disable(adsp->cx_supply);
> + clk_disable_unprepare(adsp->aggre2_clk);
> + clk_disable_unprepare(adsp->xo);
> +}
> +
> static int adsp_stop(struct rproc *rproc)
> {
> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> + int handover;
> int ret;
>
> - qcom_smem_state_update_bits(adsp->state,
> - BIT(adsp->stop_bit),
> - BIT(adsp->stop_bit));
> -
> - ret = wait_for_completion_timeout(&adsp->stop_done,
> - msecs_to_jiffies(5000));
> - if (ret == 0)
> + ret = qcom_q6v5_request_stop(&adsp->q6v5);
> + if (ret == -ETIMEDOUT)
> dev_err(adsp->dev, "timed out on wait\n");
>
> - qcom_smem_state_update_bits(adsp->state,
> - BIT(adsp->stop_bit),
> - 0);
> -
> ret = qcom_scm_pas_shutdown(adsp->pas_id);
> if (ret)
> dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
>
> + handover = qcom_q6v5_unprepare(&adsp->q6v5);
> + if (handover)
> + qcom_pas_handover(&adsp->q6v5);
> +
> return ret;
> }
>
> @@ -187,53 +187,6 @@ static const struct rproc_ops adsp_ops = {
> .load = adsp_load,
> };
>
> -static irqreturn_t adsp_wdog_interrupt(int irq, void *dev)
> -{
> - struct qcom_adsp *adsp = dev;
> -
> - rproc_report_crash(adsp->rproc, RPROC_WATCHDOG);
> -
> - return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t adsp_fatal_interrupt(int irq, void *dev)
> -{
> - struct qcom_adsp *adsp = dev;
> - size_t len;
> - char *msg;
> -
> - msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, adsp->crash_reason_smem, &len);
> - if (!IS_ERR(msg) && len > 0 && msg[0])
> - dev_err(adsp->dev, "fatal error received: %s\n", msg);
> -
> - rproc_report_crash(adsp->rproc, RPROC_FATAL_ERROR);
> -
> - return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t adsp_ready_interrupt(int irq, void *dev)
> -{
> - return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t adsp_handover_interrupt(int irq, void *dev)
> -{
> - struct qcom_adsp *adsp = dev;
> -
> - complete(&adsp->start_done);
> -
> - return IRQ_HANDLED;
> -}
> -
> -static irqreturn_t adsp_stop_ack_interrupt(int irq, void *dev)
> -{
> - struct qcom_adsp *adsp = dev;
> -
> - complete(&adsp->stop_done);
> -
> - return IRQ_HANDLED;
> -}
> -
> static int adsp_init_clock(struct qcom_adsp *adsp)
> {
> int ret;
> @@ -272,29 +225,6 @@ static int adsp_init_regulator(struct qcom_adsp *adsp)
> return PTR_ERR_OR_ZERO(adsp->px_supply);
> }
>
> -static int adsp_request_irq(struct qcom_adsp *adsp,
> - struct platform_device *pdev,
> - const char *name,
> - irq_handler_t thread_fn)
> -{
> - int ret;
> -
> - ret = platform_get_irq_byname(pdev, name);
> - if (ret < 0) {
> - dev_err(&pdev->dev, "no %s IRQ defined\n", name);
> - return ret;
> - }
> -
> - ret = devm_request_threaded_irq(&pdev->dev, ret,
> - NULL, thread_fn,
> - IRQF_ONESHOT,
> - "adsp", adsp);
> - if (ret)
> - dev_err(&pdev->dev, "request %s IRQ failed\n", name);
> -
> - return ret;
> -}
> -
> static int adsp_alloc_memory_region(struct qcom_adsp *adsp)
> {
> struct device_node *node;
> @@ -348,13 +278,9 @@ static int adsp_probe(struct platform_device *pdev)
> adsp->dev = &pdev->dev;
> adsp->rproc = rproc;
> adsp->pas_id = desc->pas_id;
> - adsp->crash_reason_smem = desc->crash_reason_smem;
> adsp->has_aggre2_clk = desc->has_aggre2_clk;
> platform_set_drvdata(pdev, adsp);
>
> - init_completion(&adsp->start_done);
> - init_completion(&adsp->stop_done);
> -
> ret = adsp_alloc_memory_region(adsp);
> if (ret)
> goto free_rproc;
> @@ -367,37 +293,10 @@ static int adsp_probe(struct platform_device *pdev)
> if (ret)
> goto free_rproc;
>
> - ret = adsp_request_irq(adsp, pdev, "wdog", adsp_wdog_interrupt);
> - if (ret < 0)
> - goto free_rproc;
> - adsp->wdog_irq = ret;
> -
> - ret = adsp_request_irq(adsp, pdev, "fatal", adsp_fatal_interrupt);
> - if (ret < 0)
> - goto free_rproc;
> - adsp->fatal_irq = ret;
> -
> - ret = adsp_request_irq(adsp, pdev, "ready", adsp_ready_interrupt);
> - if (ret < 0)
> - goto free_rproc;
> - adsp->ready_irq = ret;
> -
> - ret = adsp_request_irq(adsp, pdev, "handover", adsp_handover_interrupt);
> - if (ret < 0)
> - goto free_rproc;
> - adsp->handover_irq = ret;
> -
> - ret = adsp_request_irq(adsp, pdev, "stop-ack", adsp_stop_ack_interrupt);
> - if (ret < 0)
> - goto free_rproc;
> - adsp->stop_ack_irq = ret;
> -
> - adsp->state = qcom_smem_state_get(&pdev->dev, "stop",
> - &adsp->stop_bit);
> - if (IS_ERR(adsp->state)) {
> - ret = PTR_ERR(adsp->state);
> + ret = qcom_q6v5_init(&adsp->q6v5, pdev, rproc, desc->crash_reason_smem,
> + qcom_pas_handover);
> + if (ret)
> goto free_rproc;
> - }
>
> qcom_add_glink_subdev(rproc, &adsp->glink_subdev);
> qcom_add_smd_subdev(rproc, &adsp->smd_subdev);
> @@ -422,7 +321,6 @@ static int adsp_remove(struct platform_device *pdev)
> {
> struct qcom_adsp *adsp = platform_get_drvdata(pdev);
>
> - qcom_smem_state_put(adsp->state);
Is this change required ?
Otherwise,
reviewed-by: Sricharan R <sricharan@codeaurora.org>
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 3/5] remoteproc: qcom: adsp: Use common q6v5 helpers
2018-05-23 5:20 ` [RFC PATCH 3/5] remoteproc: qcom: adsp: Use common q6v5 helpers Bjorn Andersson
2018-06-01 6:25 ` Sricharan R
@ 2018-06-01 13:35 ` Rohit Kumar
1 sibling, 0 replies; 19+ messages in thread
From: Rohit Kumar @ 2018-06-01 13:35 UTC (permalink / raw)
To: Bjorn Andersson, Ohad Ben-Cohen, Sricharan R, Sibi Sankar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Hi Bjorn,
Reviewed and validated PAS and non-PAS ADSP PIL on sdm845 with the changes.
Adding tag.
On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
> Migrate the Hexagon V5 PAS (ADSP) driver to using the newly extracted
> helper functions. The use of the handover callback does introduce latent
> disabling of proxy resources. But apart from this there should be no
> change in functionality.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-and-tested-by: Rohit kumar <rohitkr@codeaurora.org>
> ---
> drivers/remoteproc/Kconfig | 1 +
> drivers/remoteproc/qcom_adsp_pil.c | 156 +++++------------------------
> 2 files changed, 28 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 63b79ea91a21..d51d155cf8bd 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -93,6 +93,7 @@ config QCOM_ADSP_PIL
> depends on QCOM_SYSMON || QCOM_SYSMON=n
> select MFD_SYSCON
> select QCOM_MDT_LOADER
>
Thanks,
Rohit
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 4/5] remoteproc: qcom: q6v5-pil: Use common q6v5 helpers
2018-05-23 5:20 [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Bjorn Andersson
` (2 preceding siblings ...)
2018-05-23 5:20 ` [RFC PATCH 3/5] remoteproc: qcom: adsp: Use common q6v5 helpers Bjorn Andersson
@ 2018-05-23 5:20 ` Bjorn Andersson
2018-06-01 6:42 ` Sricharan R
2018-05-23 5:20 ` [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver Bjorn Andersson
2018-06-01 13:33 ` [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Rohit Kumar
5 siblings, 1 reply; 19+ messages in thread
From: Bjorn Andersson @ 2018-05-23 5:20 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson, Sricharan R, Sibi Sankar, Rohit kumar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Migrate the MSS remoteproc driver to use the newly extracted helper
functions.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/remoteproc/Kconfig | 4 +
drivers/remoteproc/qcom_q6v5_pil.c | 157 +++--------------------------
2 files changed, 19 insertions(+), 142 deletions(-)
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index d51d155cf8bd..2316908e9788 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -116,6 +116,10 @@ config QCOM_Q6V5_PIL
depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
depends on QCOM_SYSMON || QCOM_SYSMON=n
select MFD_SYSCON
+ select QCOM_Q6V5_COMMON
+ select QCOM_RPROC_COMMON
+ select QCOM_SCM
+ select QCOM_Q6V5_COMMON
select QCOM_RPROC_COMMON
select QCOM_SCM
help
diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index 2bf8e7c49f2a..e04319573c91 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -30,12 +30,11 @@
#include <linux/remoteproc.h>
#include <linux/reset.h>
#include <linux/soc/qcom/mdt_loader.h>
-#include <linux/soc/qcom/smem.h>
-#include <linux/soc/qcom/smem_state.h>
#include <linux/iopoll.h>
#include "remoteproc_internal.h"
#include "qcom_common.h"
+#include "qcom_q6v5.h"
#include <linux/qcom_scm.h>
@@ -151,12 +150,7 @@ struct q6v5 {
struct reset_control *mss_restart;
- struct qcom_smem_state *state;
- unsigned stop_bit;
-
- int handover_irq;
-
- bool proxy_unvoted;
+ struct qcom_q6v5 q6v5;
struct clk *active_clks[8];
struct clk *reset_clks[4];
@@ -170,8 +164,6 @@ struct q6v5 {
int active_reg_count;
int proxy_reg_count;
- struct completion start_done;
- struct completion stop_done;
bool running;
phys_addr_t mba_phys;
@@ -798,9 +790,7 @@ static int q6v5_start(struct rproc *rproc)
int xfermemop_ret;
int ret;
- qproc->proxy_unvoted = false;
-
- enable_irq(qproc->handover_irq);
+ qcom_q6v5_prepare(&qproc->q6v5);
ret = q6v5_regulator_enable(qproc, qproc->proxy_regs,
qproc->proxy_reg_count);
@@ -875,11 +865,9 @@ static int q6v5_start(struct rproc *rproc)
if (ret)
goto reclaim_mpss;
- ret = wait_for_completion_timeout(&qproc->start_done,
- msecs_to_jiffies(5000));
- if (ret == 0) {
+ ret = qcom_q6v5_wait_for_start(&qproc->q6v5, msecs_to_jiffies(5000));
+ if (ret == -ETIMEDOUT) {
dev_err(qproc->dev, "start timed out\n");
- ret = -ETIMEDOUT;
goto reclaim_mpss;
}
@@ -933,7 +921,7 @@ static int q6v5_start(struct rproc *rproc)
qproc->proxy_reg_count);
disable_irqs:
- disable_irq(qproc->handover_irq);
+ qcom_q6v5_unprepare(&qproc->q6v5);
return ret;
}
@@ -946,16 +934,10 @@ static int q6v5_stop(struct rproc *rproc)
qproc->running = false;
- qcom_smem_state_update_bits(qproc->state,
- BIT(qproc->stop_bit), BIT(qproc->stop_bit));
-
- ret = wait_for_completion_timeout(&qproc->stop_done,
- msecs_to_jiffies(5000));
- if (ret == 0)
+ ret = qcom_q6v5_request_stop(&qproc->q6v5);
+ if (ret == -ETIMEDOUT)
dev_err(qproc->dev, "timed out on wait\n");
- qcom_smem_state_update_bits(qproc->state, BIT(qproc->stop_bit), 0);
-
q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_q6);
q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem);
q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc);
@@ -976,9 +958,8 @@ static int q6v5_stop(struct rproc *rproc)
q6v5_reset_assert(qproc);
- disable_irq(qproc->handover_irq);
-
- if (!qproc->proxy_unvoted) {
+ ret = qcom_q6v5_unprepare(&qproc->q6v5);
+ if (ret) {
q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
qproc->proxy_clk_count);
q6v5_regulator_disable(qproc, qproc->proxy_regs,
@@ -1014,74 +995,14 @@ static const struct rproc_ops q6v5_ops = {
.load = q6v5_load,
};
-static irqreturn_t q6v5_wdog_interrupt(int irq, void *dev)
-{
- struct q6v5 *qproc = dev;
- size_t len;
- char *msg;
-
- /* Sometimes the stop triggers a watchdog rather than a stop-ack */
- if (!qproc->running) {
- complete(&qproc->stop_done);
- return IRQ_HANDLED;
- }
-
- msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, MPSS_CRASH_REASON_SMEM, &len);
- if (!IS_ERR(msg) && len > 0 && msg[0])
- dev_err(qproc->dev, "watchdog received: %s\n", msg);
- else
- dev_err(qproc->dev, "watchdog without message\n");
-
- rproc_report_crash(qproc->rproc, RPROC_WATCHDOG);
-
- return IRQ_HANDLED;
-}
-
-static irqreturn_t q6v5_fatal_interrupt(int irq, void *dev)
-{
- struct q6v5 *qproc = dev;
- size_t len;
- char *msg;
-
- msg = qcom_smem_get(QCOM_SMEM_HOST_ANY, MPSS_CRASH_REASON_SMEM, &len);
- if (!IS_ERR(msg) && len > 0 && msg[0])
- dev_err(qproc->dev, "fatal error received: %s\n", msg);
- else
- dev_err(qproc->dev, "fatal error without message\n");
-
- rproc_report_crash(qproc->rproc, RPROC_FATAL_ERROR);
-
- return IRQ_HANDLED;
-}
-
-static irqreturn_t q6v5_ready_interrupt(int irq, void *dev)
-{
- struct q6v5 *qproc = dev;
-
- complete(&qproc->start_done);
- return IRQ_HANDLED;
-}
-
-static irqreturn_t q6v5_handover_interrupt(int irq, void *dev)
+static void qcom_msa_handover(struct qcom_q6v5 *q6v5)
{
- struct q6v5 *qproc = dev;
+ struct q6v5 *qproc = container_of(q6v5, struct q6v5, q6v5);
q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
qproc->proxy_clk_count);
q6v5_regulator_disable(qproc, qproc->proxy_regs,
qproc->proxy_reg_count);
-
- qproc->proxy_unvoted = true;
-
- return IRQ_HANDLED;
-}
-
-static irqreturn_t q6v5_stop_ack_interrupt(int irq, void *dev)
-{
- struct q6v5 *qproc = dev;
-
- complete(&qproc->stop_done);
- return IRQ_HANDLED;
}
static int q6v5_init_mem(struct q6v5 *qproc, struct platform_device *pdev)
@@ -1154,30 +1075,6 @@ static int q6v5_init_reset(struct q6v5 *qproc)
return 0;
}
-static int q6v5_request_irq(struct q6v5 *qproc,
- struct platform_device *pdev,
- const char *name,
- irq_handler_t thread_fn)
-{
- int irq;
- int ret;
-
- irq = platform_get_irq_byname(pdev, name);
- if (irq < 0) {
- dev_err(&pdev->dev, "no %s IRQ defined\n", name);
- return irq;
- }
-
- ret = devm_request_threaded_irq(&pdev->dev, irq,
- NULL, thread_fn,
- IRQF_TRIGGER_RISING | IRQF_ONESHOT,
- "q6v5", qproc);
- if (ret)
- dev_err(&pdev->dev, "request %s IRQ failed\n", name);
-
- return ret ? : irq;
-}
-
static int q6v5_alloc_memory_region(struct q6v5 *qproc)
{
struct device_node *child;
@@ -1247,9 +1144,6 @@ static int q6v5_probe(struct platform_device *pdev)
qproc->rproc = rproc;
platform_set_drvdata(pdev, qproc);
- init_completion(&qproc->start_done);
- init_completion(&qproc->stop_done);
-
ret = q6v5_init_mem(qproc, pdev);
if (ret)
goto free_rproc;
@@ -1305,33 +1199,12 @@ static int q6v5_probe(struct platform_device *pdev)
qproc->version = desc->version;
qproc->has_alt_reset = desc->has_alt_reset;
qproc->need_mem_protection = desc->need_mem_protection;
- ret = q6v5_request_irq(qproc, pdev, "wdog", q6v5_wdog_interrupt);
- if (ret < 0)
- goto free_rproc;
- ret = q6v5_request_irq(qproc, pdev, "fatal", q6v5_fatal_interrupt);
- if (ret < 0)
- goto free_rproc;
-
- ret = q6v5_request_irq(qproc, pdev, "ready", q6v5_ready_interrupt);
- if (ret < 0)
- goto free_rproc;
-
- ret = q6v5_request_irq(qproc, pdev, "handover", q6v5_handover_interrupt);
- if (ret < 0)
- goto free_rproc;
- qproc->handover_irq = ret;
- disable_irq(qproc->handover_irq);
-
- ret = q6v5_request_irq(qproc, pdev, "stop-ack", q6v5_stop_ack_interrupt);
- if (ret < 0)
+ ret = qcom_q6v5_init(&qproc->q6v5, pdev, rproc, MPSS_CRASH_REASON_SMEM,
+ qcom_msa_handover);
+ if (ret)
goto free_rproc;
- qproc->state = qcom_smem_state_get(&pdev->dev, "stop", &qproc->stop_bit);
- if (IS_ERR(qproc->state)) {
- ret = PTR_ERR(qproc->state);
- goto free_rproc;
- }
qproc->mpss_perm = BIT(QCOM_SCM_VMID_HLOS);
qproc->mba_perm = BIT(QCOM_SCM_VMID_HLOS);
qcom_add_glink_subdev(rproc, &qproc->glink_subdev);
--
2.17.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCH 4/5] remoteproc: qcom: q6v5-pil: Use common q6v5 helpers
2018-05-23 5:20 ` [RFC PATCH 4/5] remoteproc: qcom: q6v5-pil: " Bjorn Andersson
@ 2018-06-01 6:42 ` Sricharan R
0 siblings, 0 replies; 19+ messages in thread
From: Sricharan R @ 2018-06-01 6:42 UTC (permalink / raw)
To: Bjorn Andersson, Ohad Ben-Cohen, Sibi Sankar, Rohit kumar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Hi Bjorn,
On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
> Migrate the MSS remoteproc driver to use the newly extracted helper
> functions.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> drivers/remoteproc/Kconfig | 4 +
> drivers/remoteproc/qcom_q6v5_pil.c | 157 +++--------------------------
> 2 files changed, 19 insertions(+), 142 deletions(-)
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index d51d155cf8bd..2316908e9788 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -116,6 +116,10 @@ config QCOM_Q6V5_PIL
> depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
> depends on QCOM_SYSMON || QCOM_SYSMON=n
> select MFD_SYSCON
> + select QCOM_Q6V5_COMMON
The below three are duplicate.
> + select QCOM_RPROC_COMMON
> + select QCOM_SCM
> + select QCOM_Q6V5_COMMON
> select QCOM_RPROC_COMMON
> select QCOM_SCM
<..>
> @@ -946,16 +934,10 @@ static int q6v5_stop(struct rproc *rproc)
>
> qproc->running = false;
>
> - qcom_smem_state_update_bits(qproc->state,
> - BIT(qproc->stop_bit), BIT(qproc->stop_bit));
> -
> - ret = wait_for_completion_timeout(&qproc->stop_done,
> - msecs_to_jiffies(5000));
> - if (ret == 0)
> + ret = qcom_q6v5_request_stop(&qproc->q6v5);
> + if (ret == -ETIMEDOUT)
> dev_err(qproc->dev, "timed out on wait\n");
>
> - qcom_smem_state_update_bits(qproc->state, BIT(qproc->stop_bit), 0);
> -
> q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_q6);
> q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem);
> q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc);
> @@ -976,9 +958,8 @@ static int q6v5_stop(struct rproc *rproc)
>
> q6v5_reset_assert(qproc);
>
> - disable_irq(qproc->handover_irq);
> -
> - if (!qproc->proxy_unvoted) {
> + ret = qcom_q6v5_unprepare(&qproc->q6v5);
> + if (ret) {
> q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
> qproc->proxy_clk_count);
> q6v5_regulator_disable(qproc, qproc->proxy_regs,
Nit, can qcom_msa_handover api be used instead here ?
Rest all looks good,
reviewed-by: Sricharan R <sricharan@codeaurora.org>
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
2018-05-23 5:20 [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Bjorn Andersson
` (3 preceding siblings ...)
2018-05-23 5:20 ` [RFC PATCH 4/5] remoteproc: qcom: q6v5-pil: " Bjorn Andersson
@ 2018-05-23 5:20 ` Bjorn Andersson
2018-05-23 6:05 ` Vinod
2018-06-01 13:33 ` [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Rohit Kumar
5 siblings, 1 reply; 19+ messages in thread
From: Bjorn Andersson @ 2018-05-23 5:20 UTC (permalink / raw)
To: Ohad Ben-Cohen, Bjorn Andersson, Sricharan R, Sibi Sankar, Rohit kumar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
From: Sricharan R <sricharan@codeaurora.org>
IPQ8074 has an integrated Hexagon dsp core q6v5 and a wireless lan
(Lithium) IP. An mdt type single image format is used for the
firmware. So the mdt_load function can be directly used to load
the firmware. Also add the relevant resets required for this core.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
[bjorn: Rewrote as a separate driver, intead of extending q6v5_pil.c]
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
Due to lack of hardware this is only compile tested. So I'm interested in both
feedback on the approach and testing of this.
drivers/remoteproc/Kconfig | 15 +-
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_q6v5_wcss.c | 580 ++++++++++++++++++++++++++++
3 files changed, 595 insertions(+), 1 deletion(-)
create mode 100644 drivers/remoteproc/qcom_q6v5_wcss.c
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 2316908e9788..4b55bfcfc8e1 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -119,12 +119,25 @@ config QCOM_Q6V5_PIL
select QCOM_Q6V5_COMMON
select QCOM_RPROC_COMMON
select QCOM_SCM
+ help
+ Say y here to support the Qualcomm Peripherial Image Loader for the
+ Hexagon V5 based remote processors.
+
+config QCOM_Q6V5_WCSS
+ tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader"
+ depends on OF && ARCH_QCOM
+ depends on QCOM_SMEM
+ depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n)
+ depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
+ depends on QCOM_SYSMON || QCOM_SYSMON=n
+ select MFD_SYSCON
+ select QCOM_MDT_LOADER
select QCOM_Q6V5_COMMON
select QCOM_RPROC_COMMON
select QCOM_SCM
help
Say y here to support the Qualcomm Peripherial Image Loader for the
- Hexagon V5 based remote processors.
+ Hexagon V5 based WCSS remote processors.
config QCOM_SYSMON
tristate "Qualcomm sysmon driver"
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 5dd0249cf76a..03332fa7e2ee 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_QCOM_ADSP_PIL) += qcom_adsp_pil.o
obj-$(CONFIG_QCOM_RPROC_COMMON) += qcom_common.o
obj-$(CONFIG_QCOM_Q6V5_COMMON) += qcom_q6v5.o
obj-$(CONFIG_QCOM_Q6V5_PIL) += qcom_q6v5_pil.o
+obj-$(CONFIG_QCOM_Q6V5_WCSS) += qcom_q6v5_wcss.o
obj-$(CONFIG_QCOM_SYSMON) += qcom_sysmon.o
obj-$(CONFIG_QCOM_WCNSS_PIL) += qcom_wcnss_pil.o
qcom_wcnss_pil-y += qcom_wcnss.o
diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
new file mode 100644
index 000000000000..f0b38eae52df
--- /dev/null
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -0,0 +1,580 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016-2018 Linaro Ltd.
+ * Copyright (C) 2014 Sony Mobile Communications AB
+ * Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.
+ */
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/soc/qcom/mdt_loader.h>
+#include "qcom_common.h"
+#include "qcom_q6v5.h"
+
+#define WCSS_CRASH_REASON 421
+
+/* QDSP6SS Register Offsets */
+#define QDSP6SS_RESET_REG 0x014
+#define QDSP6SS_GFMUX_CTL_REG 0x020
+#define QDSP6SS_PWR_CTL_REG 0x030
+#define QDSP6SS_MEM_PWR_CTL 0x0B0
+
+/* AXI Halt Register Offsets */
+#define AXI_HALTREQ_REG 0x0
+#define AXI_HALTACK_REG 0x4
+#define AXI_IDLE_REG 0x8
+
+#define HALT_ACK_TIMEOUT_MS 100
+
+/* QDSP6SS_RESET */
+#define Q6SS_STOP_CORE BIT(0)
+#define Q6SS_CORE_ARES BIT(1)
+#define Q6SS_BUS_ARES_ENABLE BIT(2)
+
+/* QDSP6SS_GFMUX_CTL */
+#define Q6SS_CLK_ENABLE BIT(1)
+
+/* QDSP6SS_PWR_CTL */
+#define Q6SS_L2DATA_STBY_N BIT(18)
+#define Q6SS_SLP_RET_N BIT(19)
+#define Q6SS_CLAMP_IO BIT(20)
+#define QDSS_BHS_ON BIT(21)
+
+/* QDSP6v56 parameters */
+#define QDSP6v56_LDO_BYP BIT(25)
+#define QDSP6v56_BHS_ON BIT(24)
+#define QDSP6v56_CLAMP_WL BIT(21)
+#define QDSP6v56_CLAMP_QMC_MEM BIT(22)
+#define HALT_CHECK_MAX_LOOPS 200
+#define QDSP6SS_XO_CBCR 0x0038
+
+/* QDSP6v5-WCSS config/status registers */
+#define TCSR_GLOBAL_CFG0 0x0
+#define TCSR_GLOBAL_CFG1 0x4
+#define SSCAON_CONFIG 0x8
+#define SSCAON_STATUS 0xc
+#define QDSP6SS_BHS_STATUS 0x78
+#define QDSP6SS_RST_EVB 0x10
+
+#define BHS_EN_REST_ACK BIT(0)
+#define SSCAON_ENABLE BIT(13)
+
+
+struct q6v5_wcss {
+ struct device *dev;
+
+ void __iomem *reg_base;
+ void __iomem *rmb_base;
+
+ struct regmap *halt_map;
+ u32 halt_q6;
+ u32 halt_wcss;
+ u32 halt_nc;
+
+ struct reset_control *wcss_aon_reset;
+ struct reset_control *wcss_reset;
+ struct reset_control *wcss_q6_reset;
+
+ struct qcom_q6v5 q6v5;
+
+ phys_addr_t mem_phys;
+ phys_addr_t mem_reloc;
+ void *mem_region;
+ size_t mem_size;
+};
+
+static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
+{
+ int ret;
+ u32 val;
+ int i;
+
+ /* Assert resets, stop core */
+ val = readl(wcss->reg_base + QDSP6SS_RESET_REG);
+ val |= Q6SS_CORE_ARES | Q6SS_BUS_ARES_ENABLE | Q6SS_STOP_CORE;
+ writel(val, wcss->reg_base + QDSP6SS_RESET_REG);
+
+ /* BHS require xo cbcr to be enabled */
+ val = readl(wcss->reg_base + QDSP6SS_XO_CBCR);
+ val |= 0x1;
+ writel(val, wcss->reg_base + QDSP6SS_XO_CBCR);
+
+ /* Read CLKOFF bit to go low indicating CLK is enabled */
+ ret = readl_poll_timeout(wcss->reg_base + QDSP6SS_XO_CBCR,
+ val, !(val & BIT(31)), 1,
+ HALT_CHECK_MAX_LOOPS);
+ if (ret) {
+ dev_err(wcss->dev,
+ "xo cbcr enabling timed out (rc:%d)\n", ret);
+ return ret;
+ }
+ /* Enable power block headswitch and wait for it to stabilize */
+ val = readl(wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+ val |= QDSP6v56_BHS_ON;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+ val |= readl(wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+ udelay(1);
+
+ /* Put LDO in bypass mode */
+ val |= QDSP6v56_LDO_BYP;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* Deassert QDSP6 compiler memory clamp */
+ val = readl(wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+ val &= ~QDSP6v56_CLAMP_QMC_MEM;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* Deassert memory peripheral sleep and L2 memory standby */
+ val |= Q6SS_L2DATA_STBY_N | Q6SS_SLP_RET_N;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* Turn on L1, L2, ETB and JU memories 1 at a time */
+ val = readl(wcss->reg_base + QDSP6SS_MEM_PWR_CTL);
+ for (i = 19; i >= 0; i--) {
+ val |= BIT(i);
+ writel(val, wcss->reg_base + QDSP6SS_MEM_PWR_CTL);
+ /*
+ * Read back value to ensure the write is done then
+ * wait for 1us for both memory peripheral and data
+ * array to turn on.
+ */
+ val |= readl(wcss->reg_base + QDSP6SS_MEM_PWR_CTL);
+ udelay(1);
+ }
+ /* Remove word line clamp */
+ val = readl(wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+ val &= ~QDSP6v56_CLAMP_WL;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* Remove IO clamp */
+ val &= ~Q6SS_CLAMP_IO;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* Bring core out of reset */
+ val = readl(wcss->reg_base + QDSP6SS_RESET_REG);
+ val &= ~Q6SS_CORE_ARES;
+ writel(val, wcss->reg_base + QDSP6SS_RESET_REG);
+
+ /* Turn on core clock */
+ val = readl(wcss->reg_base + QDSP6SS_GFMUX_CTL_REG);
+ val |= Q6SS_CLK_ENABLE;
+ writel(val, wcss->reg_base + QDSP6SS_GFMUX_CTL_REG);
+
+ /* Start core execution */
+ val = readl(wcss->reg_base + QDSP6SS_RESET_REG);
+ val &= ~Q6SS_STOP_CORE;
+ writel(val, wcss->reg_base + QDSP6SS_RESET_REG);
+
+ return 0;
+}
+
+static int q6v5_wcss_start(struct rproc *rproc)
+{
+ struct q6v5_wcss *wcss = rproc->priv;
+ int ret;
+
+ qcom_q6v5_prepare(&wcss->q6v5);
+
+ /* Release Q6 and WCSS reset */
+ ret = reset_control_deassert(wcss->wcss_reset);
+ if (ret)
+ dev_err(wcss->dev, "wcss_reset failed\n");
+
+ ret = reset_control_deassert(wcss->wcss_q6_reset);
+ if (ret)
+ dev_err(wcss->dev, "wcss_q6_reset failed\n");
+
+ /* Lithium configuration - clock gating and bus arbitration */
+ ret = regmap_update_bits(wcss->halt_map,
+ wcss->halt_nc + TCSR_GLOBAL_CFG0,
+ 0x1F, 0x14);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(wcss->halt_map,
+ wcss->halt_nc + TCSR_GLOBAL_CFG1,
+ 1, 0);
+ if (ret)
+ return ret;
+
+ /* Write bootaddr to EVB so that Q6WCSS will jump there after reset */
+ writel(rproc->bootaddr >> 4, wcss->reg_base + QDSP6SS_RST_EVB);
+
+ ret = q6v5_wcss_reset(wcss);
+ if (ret)
+ return ret;
+
+ ret = qcom_q6v5_wait_for_start(&wcss->q6v5, 5 * HZ);
+ if (ret == -ETIMEDOUT)
+ dev_err(wcss->dev, "start timed out\n");
+
+ return ret;
+}
+
+static void q6v5_wcss_halt_axi_port(struct q6v5_wcss *wcss,
+ struct regmap *halt_map,
+ u32 offset)
+{
+ unsigned long timeout;
+ unsigned int val;
+ int ret;
+
+ /* Check if we're already idle */
+ ret = regmap_read(halt_map, offset + AXI_IDLE_REG, &val);
+ if (!ret && val)
+ return;
+
+ /* Assert halt request */
+ regmap_write(halt_map, offset + AXI_HALTREQ_REG, 1);
+
+ /* Wait for halt */
+ timeout = jiffies + msecs_to_jiffies(HALT_ACK_TIMEOUT_MS);
+ for (;;) {
+ ret = regmap_read(halt_map, offset + AXI_HALTACK_REG, &val);
+ if (ret || val || time_after(jiffies, timeout))
+ break;
+
+ msleep(1);
+ }
+
+ ret = regmap_read(halt_map, offset + AXI_IDLE_REG, &val);
+ if (ret || !val)
+ dev_err(wcss->dev, "port failed halt\n");
+
+ /* Clear halt request (port will remain halted until reset) */
+ regmap_write(halt_map, offset + AXI_HALTREQ_REG, 0);
+}
+
+static int q6v5_wcss_powerdown(struct q6v5_wcss *wcss)
+{
+ int ret;
+ u32 val;
+
+ /* 1 - Assert WCSS/Q6 HALTREQ */
+ q6v5_wcss_halt_axi_port(wcss, wcss->halt_map, wcss->halt_wcss);
+
+ /* 2 - Enable WCSSAON_CONFIG */
+ val = readl(wcss->rmb_base + SSCAON_CONFIG);
+ val |= SSCAON_ENABLE;
+ writel(val, wcss->rmb_base + SSCAON_CONFIG);
+
+ /* 3 - Set SSCAON_CONFIG */
+ val |= BIT(15);
+ val &= ~BIT(16);
+ val &= ~BIT(17);
+ val &= ~BIT(18);
+ writel(val, wcss->rmb_base + SSCAON_CONFIG);
+
+ /* 4 - SSCAON_CONFIG 1 */
+ val |= BIT(1);
+ writel(val, wcss->rmb_base + SSCAON_CONFIG);
+
+ /* 5 - wait for SSCAON_STATUS */
+ ret = readl_poll_timeout(wcss->rmb_base + SSCAON_STATUS,
+ val, (val & 0xffff) == 0x400, 1000,
+ HALT_CHECK_MAX_LOOPS);
+ if (ret) {
+ dev_err(wcss->dev,
+ "can't get SSCAON_STATUS rc:%d)\n", ret);
+ }
+
+ /* 6 - De-assert WCSS_AON reset */
+ reset_control_assert(wcss->wcss_aon_reset);
+
+ /* 7 - Disable WCSSAON_CONFIG 13 */
+ val = readl(wcss->rmb_base + SSCAON_CONFIG);
+ val &= ~SSCAON_ENABLE;
+ writel(val, wcss->rmb_base + SSCAON_CONFIG);
+
+ /* 8 - De-assert WCSS/Q6 HALTREQ */
+ reset_control_assert(wcss->wcss_reset);
+
+ return ret;
+}
+
+static int q6v5_q6_powerdown(struct q6v5_wcss *wcss)
+{
+ int ret;
+ u32 val;
+ int i;
+
+ /* 1 - Halt Q6 bus interface */
+ q6v5_wcss_halt_axi_port(wcss, wcss->halt_map, wcss->halt_q6);
+
+ /* 2 - Disable Q6 Core clock */
+ val = readl(wcss->reg_base + QDSP6SS_GFMUX_CTL_REG);
+ val &= ~Q6SS_CLK_ENABLE;
+ writel(val, wcss->reg_base + QDSP6SS_GFMUX_CTL_REG);
+
+ /* 3 - Clamp I/O */
+ val = readl(wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+ val |= Q6SS_CLAMP_IO;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* 4 - Clamp WL */
+ val |= QDSS_BHS_ON;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* 5 - Clear Erase standby */
+ val &= ~Q6SS_L2DATA_STBY_N;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* 6 - Clear Sleep RTN */
+ val &= ~Q6SS_SLP_RET_N;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* 7 - turn off QDSP6 memory foot/head switch one bank at a time */
+ for (i = 0; i < 20; i++) {
+ val = readl(wcss->reg_base + QDSP6SS_MEM_PWR_CTL);
+ val &= ~BIT(i);
+ writel(val, wcss->reg_base + QDSP6SS_MEM_PWR_CTL);
+ mdelay(1);
+ }
+ /* 8 - Assert QMC memory RTN */
+ val = readl(wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+ val |= QDSP6v56_CLAMP_QMC_MEM;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+
+ /* 9 - Turn off BHS */
+ val &= ~QDSP6v56_BHS_ON;
+ writel(val, wcss->reg_base + QDSP6SS_PWR_CTL_REG);
+ udelay(1);
+ /* 10 - Wait till BHS Reset is done */
+ ret = readl_poll_timeout(wcss->reg_base + QDSP6SS_BHS_STATUS,
+ val, !(val & BHS_EN_REST_ACK), 1000,
+ HALT_CHECK_MAX_LOOPS);
+ if (ret)
+ dev_err(wcss->dev, "BHS_STATUS not OFF (rc:%d)\n", ret);
+
+ /* 11 - Assert WCSS reset */
+ reset_control_assert(wcss->wcss_reset);
+
+ /* 12 - Assert Q6 reset */
+ reset_control_assert(wcss->wcss_q6_reset);
+
+ return 0;
+}
+
+
+static int q6v5_wcss_stop(struct rproc *rproc)
+{
+ struct q6v5_wcss *wcss = rproc->priv;
+ int ret;
+
+ /* WCSS powerdown */
+ ret = qcom_q6v5_request_stop(&wcss->q6v5);
+ if (ret == -ETIMEDOUT) {
+ dev_err(wcss->dev, "timed out on wait\n");
+ return ret;
+ }
+
+ ret = q6v5_wcss_powerdown(wcss);
+ if (ret)
+ return ret;
+
+ /* Q6 Power down */
+ ret = q6v5_q6_powerdown(wcss);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static void *q6v5_wcss_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+ struct q6v5_wcss *wcss = rproc->priv;
+ int offset;
+
+ offset = da - wcss->mem_reloc;
+ if (offset < 0 || offset + len > wcss->mem_size)
+ return NULL;
+
+ return wcss->mem_region + offset;
+}
+
+static int q6v5_wcss_load(struct rproc *rproc, const struct firmware *fw)
+{
+ struct q6v5_wcss *wcss = rproc->priv;
+
+ return qcom_mdt_load_no_init(wcss->dev, fw, rproc->firmware,
+ 0, wcss->mem_region, wcss->mem_phys,
+ wcss->mem_size, &wcss->mem_reloc);
+}
+
+static const struct rproc_ops q6v5_wcss_ops = {
+ .start = q6v5_wcss_start,
+ .stop = q6v5_wcss_stop,
+ .da_to_va = q6v5_wcss_da_to_va,
+ .load = q6v5_wcss_load,
+ .get_boot_addr = rproc_elf_get_boot_addr,
+};
+
+static int q6v5_wcss_init_reset(struct q6v5_wcss *wcss)
+{
+ struct device *dev = wcss->dev;
+
+ wcss->wcss_aon_reset = devm_reset_control_get(dev, "wcss_aon_reset");
+ if (IS_ERR(wcss->wcss_aon_reset)) {
+ dev_err(wcss->dev, "unable to acquire wcss_aon_reset\n");
+ return PTR_ERR(wcss->wcss_aon_reset);
+ }
+
+ wcss->wcss_reset = devm_reset_control_get(dev, "wcss_reset");
+ if (IS_ERR(wcss->wcss_reset)) {
+ dev_err(wcss->dev, "unable to acquire wcss_reset\n");
+ return PTR_ERR(wcss->wcss_reset);
+ }
+
+ wcss->wcss_q6_reset = devm_reset_control_get(dev, "wcss_q6_reset");
+ if (IS_ERR(wcss->wcss_q6_reset)) {
+ dev_err(wcss->dev, "unable to acquire wcss_q6_reset\n");
+ return PTR_ERR(wcss->wcss_q6_reset);
+ }
+
+ return 0;
+}
+
+static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,
+ struct platform_device *pdev)
+{
+ struct of_phandle_args args;
+ struct resource *res;
+ int ret;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "qdsp6");
+ wcss->reg_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(wcss->reg_base))
+ return PTR_ERR(wcss->reg_base);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rmb");
+ wcss->rmb_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(wcss->rmb_base))
+ return PTR_ERR(wcss->rmb_base);
+
+ ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node,
+ "qcom,halt-regs", 3, 0, &args);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to parse qcom,halt-regs\n");
+ return -EINVAL;
+ }
+
+ wcss->halt_map = syscon_node_to_regmap(args.np);
+ of_node_put(args.np);
+ if (IS_ERR(wcss->halt_map))
+ return PTR_ERR(wcss->halt_map);
+
+ wcss->halt_q6 = args.args[0];
+ wcss->halt_wcss = args.args[1];
+ wcss->halt_nc = args.args[2];
+
+ return 0;
+}
+
+static int q6v5_alloc_memory_region(struct q6v5_wcss *wcss)
+{
+ struct reserved_mem *rmem = NULL;
+ struct device_node *node;
+ struct device *dev = wcss->dev;
+
+ node = of_parse_phandle(dev->of_node, "memory-region", 0);
+ if (node)
+ rmem = of_reserved_mem_lookup(node);
+ of_node_put(node);
+
+ if (!rmem) {
+ dev_err(dev, "unable to acquire memory-region\n");
+ return -EINVAL;
+ }
+
+ wcss->mem_phys = rmem->base;
+ wcss->mem_reloc = rmem->base;
+ wcss->mem_size = rmem->size;
+ wcss->mem_region = devm_ioremap_wc(dev, wcss->mem_phys, wcss->mem_size);
+ if (!wcss->mem_region) {
+ dev_err(dev, "unable to map memory region: %pa+%pa\n",
+ &rmem->base, &rmem->size);
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+static int q6v5_wcss_probe(struct platform_device *pdev)
+{
+ struct q6v5_wcss *wcss;
+ struct rproc *rproc;
+ int ret;
+
+ rproc = rproc_alloc(&pdev->dev, pdev->name, &q6v5_wcss_ops,
+ "IPQ8074/q6_fw.mdt", sizeof(*wcss));
+ if (!rproc) {
+ dev_err(&pdev->dev, "failed to allocate rproc\n");
+ return -ENOMEM;
+ }
+
+ wcss = rproc->priv;
+ wcss->dev = &pdev->dev;
+
+ ret = q6v5_wcss_init_mmio(wcss, pdev);
+ if (ret)
+ goto free_rproc;
+
+ ret = q6v5_alloc_memory_region(wcss);
+ if (ret)
+ goto free_rproc;
+
+ ret = q6v5_wcss_init_reset(wcss);
+ if (ret)
+ goto free_rproc;
+
+ ret = qcom_q6v5_init(&wcss->q6v5, pdev, rproc, WCSS_CRASH_REASON, NULL);
+ if (ret)
+ goto free_rproc;
+
+ ret = rproc_add(rproc);
+ if (ret)
+ goto free_rproc;
+
+ platform_set_drvdata(pdev, rproc);
+
+ return 0;
+
+free_rproc:
+ rproc_free(rproc);
+
+ return ret;
+}
+
+static int q6v5_wcss_remove(struct platform_device *pdev)
+{
+ struct rproc *rproc = platform_get_drvdata(pdev);
+
+ rproc_del(rproc);
+ rproc_free(rproc);
+
+ return 0;
+}
+
+static const struct of_device_id q6v5_wcss_of_match[] = {
+ { .compatible = "qcom,ipq8074-wcss-pil" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, q6v5_wcss_of_match);
+
+static struct platform_driver q6v5_wcss_driver = {
+ .probe = q6v5_wcss_probe,
+ .remove = q6v5_wcss_remove,
+ .driver = {
+ .name = "qcom-q6v5-wcss-pil",
+ .of_match_table = q6v5_wcss_of_match,
+ },
+};
+module_platform_driver(q6v5_wcss_driver);
+
+MODULE_DESCRIPTION("Hexagon WCSS Peripheral Image Loader");
+MODULE_LICENSE("GPL v2");
--
2.17.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
2018-05-23 5:20 ` [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver Bjorn Andersson
@ 2018-05-23 6:05 ` Vinod
2018-05-23 6:58 ` Bjorn Andersson
0 siblings, 1 reply; 19+ messages in thread
From: Vinod @ 2018-05-23 6:05 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Ohad Ben-Cohen, Sricharan R, Sibi Sankar, Rohit kumar,
Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
On 22-05-18, 22:20, Bjorn Andersson wrote:
> +static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
> +{
> + int ret;
> + u32 val;
> + int i;
> +
> + /* Assert resets, stop core */
> + val = readl(wcss->reg_base + QDSP6SS_RESET_REG);
> + val |= Q6SS_CORE_ARES | Q6SS_BUS_ARES_ENABLE | Q6SS_STOP_CORE;
> + writel(val, wcss->reg_base + QDSP6SS_RESET_REG);
> +
> + /* BHS require xo cbcr to be enabled */
> + val = readl(wcss->reg_base + QDSP6SS_XO_CBCR);
> + val |= 0x1;
> + writel(val, wcss->reg_base + QDSP6SS_XO_CBCR);
As commented on previous patch, it would help IMO to add a modify() wrapper
here which would perform read, modify and write.
Looking at the patch, few other comments would be applicable too, so would be
great if you/Sricharan can update this
--
~Vinod
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
2018-05-23 6:05 ` Vinod
@ 2018-05-23 6:58 ` Bjorn Andersson
2018-05-23 7:37 ` Vinod
0 siblings, 1 reply; 19+ messages in thread
From: Bjorn Andersson @ 2018-05-23 6:58 UTC (permalink / raw)
To: Vinod
Cc: Ohad Ben-Cohen, Sricharan R, Sibi Sankar, Rohit kumar,
Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
On Tue 22 May 23:05 PDT 2018, Vinod wrote:
> On 22-05-18, 22:20, Bjorn Andersson wrote:
>
> > +static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
> > +{
> > + int ret;
> > + u32 val;
> > + int i;
> > +
> > + /* Assert resets, stop core */
> > + val = readl(wcss->reg_base + QDSP6SS_RESET_REG);
> > + val |= Q6SS_CORE_ARES | Q6SS_BUS_ARES_ENABLE | Q6SS_STOP_CORE;
> > + writel(val, wcss->reg_base + QDSP6SS_RESET_REG);
> > +
> > + /* BHS require xo cbcr to be enabled */
> > + val = readl(wcss->reg_base + QDSP6SS_XO_CBCR);
> > + val |= 0x1;
> > + writel(val, wcss->reg_base + QDSP6SS_XO_CBCR);
>
> As commented on previous patch, it would help IMO to add a modify() wrapper
> here which would perform read, modify and write.
>
Iirc the code ended up like this because a lot of these operations ended
up being line wrapped and harder to read using some modify(reg, mask,
val) helper. That said, the function isn't very pretty in it's current
state either...
One of the parts of the RFC is that this sequence is a verbatim copy
from the qcom_q6v5_pil.c driver for 8996, so if we find this duplication
suitable I would prefer that we keep them the same.
The alternative to duplicating this function is as Sricharan proposed to
have the qcom_q6v5_pil.c be both a driver for both the single-stage
remoteproc and the two-stage (load boot loader, then modem firmware).
> Looking at the patch, few other comments would be applicable too, so would be
> great if you/Sricharan can update this
>
I agree, the primary purpose of this patch was rather to get feedback on
the structure of the drivers, I do expect this to take another round
through the editor to get some polishing touches. Sorry if this wasn't
clear from the description.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
2018-05-23 6:58 ` Bjorn Andersson
@ 2018-05-23 7:37 ` Vinod
2018-05-23 14:48 ` Sricharan R
0 siblings, 1 reply; 19+ messages in thread
From: Vinod @ 2018-05-23 7:37 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Ohad Ben-Cohen, Sricharan R, Sibi Sankar, Rohit kumar,
Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
On 22-05-18, 23:58, Bjorn Andersson wrote:
> On Tue 22 May 23:05 PDT 2018, Vinod wrote:
>
> > On 22-05-18, 22:20, Bjorn Andersson wrote:
> >
> > > +static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
> > > +{
> > > + int ret;
> > > + u32 val;
> > > + int i;
> > > +
> > > + /* Assert resets, stop core */
> > > + val = readl(wcss->reg_base + QDSP6SS_RESET_REG);
> > > + val |= Q6SS_CORE_ARES | Q6SS_BUS_ARES_ENABLE | Q6SS_STOP_CORE;
> > > + writel(val, wcss->reg_base + QDSP6SS_RESET_REG);
> > > +
> > > + /* BHS require xo cbcr to be enabled */
> > > + val = readl(wcss->reg_base + QDSP6SS_XO_CBCR);
> > > + val |= 0x1;
> > > + writel(val, wcss->reg_base + QDSP6SS_XO_CBCR);
> >
> > As commented on previous patch, it would help IMO to add a modify() wrapper
> > here which would perform read, modify and write.
> >
>
> Iirc the code ended up like this because a lot of these operations ended
> up being line wrapped and harder to read using some modify(reg, mask,
> val) helper. That said, the function isn't very pretty in it's current
> state either...
Agreed :) and i thought modify make help it make better
> One of the parts of the RFC is that this sequence is a verbatim copy
> from the qcom_q6v5_pil.c driver for 8996, so if we find this duplication
> suitable I would prefer that we keep them the same.
>
>
> The alternative to duplicating this function is as Sricharan proposed to
> have the qcom_q6v5_pil.c be both a driver for both the single-stage
> remoteproc and the two-stage (load boot loader, then modem firmware).
>
> > Looking at the patch, few other comments would be applicable too, so would be
> > great if you/Sricharan can update this
> >
>
> I agree, the primary purpose of this patch was rather to get feedback on
> the structure of the drivers, I do expect this to take another round
> through the editor to get some polishing touches. Sorry if this wasn't
> clear from the description.
Since Sricharan replied to comments, I though they would be fixed. Yeah this is
fine from RFC..
--
~Vinod
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
2018-05-23 7:37 ` Vinod
@ 2018-05-23 14:48 ` Sricharan R
2018-05-29 4:07 ` Bjorn Andersson
0 siblings, 1 reply; 19+ messages in thread
From: Sricharan R @ 2018-05-23 14:48 UTC (permalink / raw)
To: Vinod, Bjorn Andersson
Cc: Ohad Ben-Cohen, Sibi Sankar, Rohit kumar, Andy Gross,
linux-kernel, linux-remoteproc, linux-arm-msm
Hi Bjorn,
On 5/23/2018 1:07 PM, Vinod wrote:
> On 22-05-18, 23:58, Bjorn Andersson wrote:
>> On Tue 22 May 23:05 PDT 2018, Vinod wrote:
>>
>>> On 22-05-18, 22:20, Bjorn Andersson wrote:
>>>
>>>> +static int q6v5_wcss_reset(struct q6v5_wcss *wcss)
>>>> +{
>>>> + int ret;
>>>> + u32 val;
>>>> + int i;
>>>> +
>>>> + /* Assert resets, stop core */
>>>> + val = readl(wcss->reg_base + QDSP6SS_RESET_REG);
>>>> + val |= Q6SS_CORE_ARES | Q6SS_BUS_ARES_ENABLE | Q6SS_STOP_CORE;
>>>> + writel(val, wcss->reg_base + QDSP6SS_RESET_REG);
>>>> +
>>>> + /* BHS require xo cbcr to be enabled */
>>>> + val = readl(wcss->reg_base + QDSP6SS_XO_CBCR);
>>>> + val |= 0x1;
>>>> + writel(val, wcss->reg_base + QDSP6SS_XO_CBCR);
>>>
>>> As commented on previous patch, it would help IMO to add a modify() wrapper
>>> here which would perform read, modify and write.
>>>
>>
>> Iirc the code ended up like this because a lot of these operations ended
>> up being line wrapped and harder to read using some modify(reg, mask,
>> val) helper. That said, the function isn't very pretty in it's current
>> state either...
>
> Agreed :) and i thought modify make help it make better
>
>> One of the parts of the RFC is that this sequence is a verbatim copy
>> from the qcom_q6v5_pil.c driver for 8996, so if we find this duplication
>> suitable I would prefer that we keep them the same.
>>
>>
>> The alternative to duplicating this function is as Sricharan proposed to
>> have the qcom_q6v5_pil.c be both a driver for both the single-stage
>> remoteproc and the two-stage (load boot loader, then modem firmware).
>>
>>> Looking at the patch, few other comments would be applicable too, so would be
>>> great if you/Sricharan can update this
>>>
>>
>> I agree, the primary purpose of this patch was rather to get feedback on
>> the structure of the drivers, I do expect this to take another round
>> through the editor to get some polishing touches. Sorry if this wasn't
>> clear from the description.
>
> Since Sricharan replied to comments, I though they would be fixed. Yeah this is
> fine from RFC..
>
Thanks for this.
Tested this on ipq8074 and wcss rproc is up with this.
Tested-by: Sricharan R <sricharan@codeaurora.org>
So regarding the cleanup, as i see, this consolidates the code much better.
so about the point of avoiding duplication for soc specific functions like
qcv5_wcss_reset common between qcv5_wcss and qcv5_pil drivers as done in my
series, with a second thought that feels it might be difficult to maintain
in the longer run. Since the sequences are specific to each soc and for now
although some part of it is common, for a minor update in one soc, common
code needs to reworked every time and tested on all boards that share them.
So feels like having the duplication for hw init sequences is the cleaner way.
Apart from that for other comments on the q6v5 wcss driver, i can address them
on this final patch that you have posted and same can be included in your
next version. Please let me know how you want to go about it.
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
2018-05-23 14:48 ` Sricharan R
@ 2018-05-29 4:07 ` Bjorn Andersson
2018-05-29 8:32 ` Sricharan R
0 siblings, 1 reply; 19+ messages in thread
From: Bjorn Andersson @ 2018-05-29 4:07 UTC (permalink / raw)
To: Sricharan R
Cc: Vinod, Ohad Ben-Cohen, Sibi Sankar, Rohit kumar, Andy Gross,
linux-kernel, linux-remoteproc, linux-arm-msm
On Wed 23 May 07:48 PDT 2018, Sricharan R wrote:
> On 5/23/2018 1:07 PM, Vinod wrote:
> > On 22-05-18, 23:58, Bjorn Andersson wrote:
> >> On Tue 22 May 23:05 PDT 2018, Vinod wrote:
> >>> On 22-05-18, 22:20, Bjorn Andersson wrote:
[..]
> >>> Looking at the patch, few other comments would be applicable too, so would be
> >>> great if you/Sricharan can update this
> >>>
> >>
> >> I agree, the primary purpose of this patch was rather to get feedback on
> >> the structure of the drivers, I do expect this to take another round
> >> through the editor to get some polishing touches. Sorry if this wasn't
> >> clear from the description.
> >
> > Since Sricharan replied to comments, I though they would be fixed. Yeah this is
> > fine from RFC..
> >
>
> Thanks for this.
>
> Tested this on ipq8074 and wcss rproc is up with this.
>
> Tested-by: Sricharan R <sricharan@codeaurora.org>
>
Thanks Sricharan!
> So regarding the cleanup, as i see, this consolidates the code much better.
>
> so about the point of avoiding duplication for soc specific functions like
> qcv5_wcss_reset common between qcv5_wcss and qcv5_pil drivers as done in my
> series, with a second thought that feels it might be difficult to maintain
> in the longer run. Since the sequences are specific to each soc and for now
> although some part of it is common, for a minor update in one soc, common
> code needs to reworked every time and tested on all boards that share them.
>
> So feels like having the duplication for hw init sequences is the cleaner way.
>
Sounds good, then let's go with this approach!
> Apart from that for other comments on the q6v5 wcss driver, i can address them
> on this final patch that you have posted and same can be included in your
> next version. Please let me know how you want to go about it.
>
If you can help me review the first 4 patches and fix up and resend the
5th that would be greatly appreciated.
Regards,
Bjorn
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver
2018-05-29 4:07 ` Bjorn Andersson
@ 2018-05-29 8:32 ` Sricharan R
0 siblings, 0 replies; 19+ messages in thread
From: Sricharan R @ 2018-05-29 8:32 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Vinod, Ohad Ben-Cohen, Sibi Sankar, Rohit kumar, Andy Gross,
linux-kernel, linux-remoteproc, linux-arm-msm
Hi Bjorn,
On 5/29/2018 9:37 AM, Bjorn Andersson wrote:
> On Wed 23 May 07:48 PDT 2018, Sricharan R wrote:
>> On 5/23/2018 1:07 PM, Vinod wrote:
>>> On 22-05-18, 23:58, Bjorn Andersson wrote:
>>>> On Tue 22 May 23:05 PDT 2018, Vinod wrote:
>>>>> On 22-05-18, 22:20, Bjorn Andersson wrote:
> [..]
>>>>> Looking at the patch, few other comments would be applicable too, so would be
>>>>> great if you/Sricharan can update this
>>>>>
>>>>
>>>> I agree, the primary purpose of this patch was rather to get feedback on
>>>> the structure of the drivers, I do expect this to take another round
>>>> through the editor to get some polishing touches. Sorry if this wasn't
>>>> clear from the description.
>>>
>>> Since Sricharan replied to comments, I though they would be fixed. Yeah this is
>>> fine from RFC..
>>>
>>
>> Thanks for this.
>>
>> Tested this on ipq8074 and wcss rproc is up with this.
>>
>> Tested-by: Sricharan R <sricharan@codeaurora.org>
>>
>
> Thanks Sricharan!
>
>> So regarding the cleanup, as i see, this consolidates the code much better.
>>
>> so about the point of avoiding duplication for soc specific functions like
>> qcv5_wcss_reset common between qcv5_wcss and qcv5_pil drivers as done in my
>> series, with a second thought that feels it might be difficult to maintain
>> in the longer run. Since the sequences are specific to each soc and for now
>> although some part of it is common, for a minor update in one soc, common
>> code needs to reworked every time and tested on all boards that share them.
>>
>> So feels like having the duplication for hw init sequences is the cleaner way.
>>
>
> Sounds good, then let's go with this approach!
>
>> Apart from that for other comments on the q6v5 wcss driver, i can address them
>> on this final patch that you have posted and same can be included in your
>> next version. Please let me know how you want to go about it.
>>
>
> If you can help me review the first 4 patches and fix up and resend the
> 5th that would be greatly appreciated.
Sure, will do.
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Hexagon remoteproc spring cleaning
2018-05-23 5:20 [RFC PATCH 0/5] Hexagon remoteproc spring cleaning Bjorn Andersson
` (4 preceding siblings ...)
2018-05-23 5:20 ` [RFC PATCH 5/5] remoteproc: qcom: Introduce Hexagon V5 based WCSS driver Bjorn Andersson
@ 2018-06-01 13:33 ` Rohit Kumar
5 siblings, 0 replies; 19+ messages in thread
From: Rohit Kumar @ 2018-06-01 13:33 UTC (permalink / raw)
To: Bjorn Andersson, Ohad Ben-Cohen, Sricharan R, Sibi Sankar
Cc: Andy Gross, linux-kernel, linux-remoteproc, linux-arm-msm
Thanks Bjorn for the cleanup.
I validated PAS and Non- PAS ADSP PIL with the cleanup changes on SDM845.
Adding tag for ADSP PIL.
Reviewed-and-tested-by: Rohit kumar <rohitkr@codeaurora.org>
On 5/23/2018 10:50 AM, Bjorn Andersson wrote:
> With the introduction of support for the non-MSA Hexagon WCSS driver from
> Sricharan and the non-PAS ADSP driver from Rohit it makes sense to overhaul the
> structure of the Qualcomm "Q6V5 drivers".
>
> The first patch is from Sricharan's series and included here for completeness.
> The second patch introduces a set of helper functions, based on the current
> state of the qcom_q6v5_pil driver. The third and forth patch migrates the PAS
> and the MSA drivers over to using these helpers. Finally a (completely)
> reworked version of Sricharan's WCSS remoteproc driver is introduced.
>
>
> With this in place I suggest that we rename qcom_adsp_pil.c to qcom_q6v5_pas.c,
I am making changes for non-PAS ADSP PIL by creating new file.
In case you plan to rename qcom_adsp_pil.c to qcom_q6v5_pas, please
include that change with this.
Also, let me know the filename which we should keep it for non-PAS ADSP
PIL driver.
> qcom_q6v5_pil.c to qcom_q6v5_msa.c and depending on the details of the non-PAS
> ADSP we could potentially combine that into a qcom_q6v5_pil.c - or we carry
> them as separate files.
I tried comparing qcom_q6v5_pil.c with non-PAS ADSP pil driver. I don't
think that we can combine
them as there are good amount of code differences. However, we can take
a look into it once
I post non-PAS ADSP PIL.
>
> Looking at the remaining non-essential parts of these drivers we have
> memory-region handling and halt_axi handling. The prior is actively being
> worked on and the latter should (if no better abstraction is presented) be
> possible to just put in the new qcom_q6v5.c.
>
Thanks,
Rohit
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 19+ messages in thread