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@xxxxxxxxxx> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html